Skip to content

refactor(api): use zero agent id instead of compose id for run creation#6239

Merged
lancy merged 2 commits intomainfrom
feat/issue-6231-switch-createzerorun-to-zeroagentid
Mar 24, 2026
Merged

refactor(api): use zero agent id instead of compose id for run creation#6239
lancy merged 2 commits intomainfrom
feat/issue-6231-switch-createzerorun-to-zeroagentid

Conversation

@lancy
Copy link
Copy Markdown
Contributor

@lancy lancy commented Mar 23, 2026

Summary

  • Change ZeroRunParams to accept zeroAgentId instead of composeId, with createZeroRun() resolving composeId internally via new resolveZeroAgent() helper
  • Update all 8 callers (email trigger/reply, slack DM/mention, telegram DM/mention, github issues, schedule, web API, slack interactive) to pass zeroAgentId
  • Remove buildAgentIdentityPrompt() in favor of direct formatAgentIdentityPrompt() call with data from resolveZeroAgent()
  • Update createTestCompose() to auto-create zeroAgents row and createTestZeroAgent() to handle conflicts

Test plan

  • All 4326 tests pass (424 test files)
  • TypeScript type check passes
  • Lint passes (0 warnings, 0 errors)
  • Format check passes
  • Knip passes (no unused exports)
  • Verified composeId only appears in resolveZeroAgent() for startRun() passthrough
  • Verified buildAgentIdentityPrompt is fully removed from zero directory

Closes #6231

🤖 Generated with Claude Code

Change ZeroRunParams to accept zeroAgentId instead of composeId.
createZeroRun() now resolves composeId internally via resolveZeroAgent()
for the downstream startRun() call. All 8 callers updated to pass
zeroAgentId. buildAgentIdentityPrompt() removed in favor of direct
formatAgentIdentityPrompt() call with data from resolveZeroAgent().

Closes #6231

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 23, 2026

Codecov Report

❌ Patch coverage is 87.87879% with 4 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...o/apps/web/src/lib/email/handlers/inbound-reply.ts 50.00% 2 Missing ⚠️
turbo/apps/web/app/api/zero/runs/route.ts 75.00% 1 Missing ⚠️
...bo/apps/web/src/lib/github/handlers/issue-event.ts 66.66% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@lancy
Copy link
Copy Markdown
Contributor Author

lancy commented Mar 23, 2026

Code Review: PR #6239 (Round 1)

Summary

This PR refactors createZeroRun() to accept zeroAgentId instead of composeId, updating all 8 trigger paths (web, schedule, slack, telegram, email, github). A new resolveZeroAgent() function resolves the composeId internally from zeroAgentId. Tests and helpers are properly updated.

Key Findings

Critical Issues (P0)

None.

High Priority (P1)

  1. Non-null assertions on zeroAgentId (4 locations) — The getWorkspaceAgent() function returns zeroAgentId: string | null, but callers use the ! operator to assert non-null. If a zero_agents row doesn't exist for a compose, null will be passed as a string at runtime.

    • slack-org/handlers/direct-message.ts:180agent.zeroAgentId!
    • slack-org/handlers/mention.ts:178agent.zeroAgentId!
    • telegram/handlers/direct-message.ts:163defaultAgent.zeroAgentId!
    • telegram/handlers/mention.ts:150defaultAgent.zeroAgentId!
  2. Empty string fallbacks as silent failure (2 locations) — When no agent is found, "" is silently passed as zeroAgentId, causing resolveZeroAgent("") to return all-null fields. This masks configuration errors instead of failing fast.

    • app/api/zero/runs/route.ts:78agent?.id ?? ""
    • app/api/zero/slack/interactive/route.ts:562agentInfo?.zeroAgentId ?? ""

Testing Review

Coverage

  • Test file zero-run-service.test.ts properly updated to use zeroAgentId instead of composeId
  • Test helpers (createTestCompose, createTestZeroAgent, getTestZeroAgentId) correctly ensure zero_agents rows exist
  • All tests use real database — no new mocks introduced

Convention Compliance

  • No any types, no lint suppressions, no dynamic imports, no hardcoded URLs
  • No unnecessary try/catch blocks (existing ones do meaningful error categorization)
  • Test helpers use onConflictDoNothing/onConflictDoUpdate for idempotency

Testing Verdict: Adequate

Verdict: Changes Requested

Fixing P1 issues (non-null assertions and empty string fallbacks) and will re-review.


Round 1 of automated review-fix loop

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 23, 2026

⚡ Lighthouse — Web

Category Score
🟠 Performance 67
🟢 Accessibility 96
🟠 Best Practices 79
🟢 SEO 92

Tested URL: https://pr-6239-www.vm6.ai/ · Full report

- Make getWorkspaceAgent return undefined when zero agent is missing
- Add fail-fast error responses in runs route and slack interactive route
- Update seedTestCompose to create zero_agents row for test consistency

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@lancy
Copy link
Copy Markdown
Contributor Author

lancy commented Mar 23, 2026

Code Review: PR #6239 (Round 2) — LGTM 🎉

All P0 and P1 issues have been resolved.

Summary

This PR refactors createZeroRun() to accept zeroAgentId instead of composeId, consistently updating all 8 trigger paths (web, schedule, slack, telegram, email, github). The fix commit addressed all high-priority findings:

  • 4 non-null assertions replacedgetWorkspaceAgent() in both slack and telegram now returns undefined when zero agent is missing, eliminating the need for ! assertions. The return type was narrowed from zeroAgentId: string | null to zeroAgentId: string.
  • 2 empty string fallbacks replacedruns/route.ts now returns a proper 404 response, and slack/interactive/route.ts shows an error card when the agent is not found.
  • Test infrastructure updatedseedTestCompose now creates zero_agents rows for consistency.

Remaining P2 Items (Deferrable)

  • resolveZeroAgent() returns defaults instead of throwing on not-found (low risk since callers now validate)
  • Duplicated getWorkspaceAgent() implementations in slack/telegram shared modules
  • Inline composeId-to-zeroAgentId JOIN queries in runs/route.ts and inbound-reply.ts

Verdict: LGTM ✅

No critical or high-priority issues remaining. This PR is ready for merge.


Completed after 2 round(s) of automated review-fix loop

@github-actions
Copy link
Copy Markdown
Contributor

⚡ Lighthouse — App

Category Score
🔴 Performance 44
🟢 Accessibility 96
🟠 Best Practices 79

Tested URL: https://pr-6239-app.vm6.ai/ · Full report

@lancy
Copy link
Copy Markdown
Contributor Author

lancy commented Mar 24, 2026

Acceptance Report: #6231 — refactor(api): switch createZeroRun from composeId to zeroAgentId

Epic: #6226 — Migrate Email Integration from Infra Layer to Zero App Layer
PR: #6239
Reviewer: Tech Lead (via /epic-review)


1. Requirements (Definition of Done)

# Requirement Status Notes
1 ZeroRunParams accepts zeroAgentId instead of composeId ✅ PASS interface ZeroRunParams { zeroAgentId: string; ... } — no composeId in interface
2 createZeroRun() resolves composeId internally via resolveZeroAgent() ✅ PASS resolveZeroAgent() does leftJoin zeroAgents → agentComposes, passes composeId to startRun()
3 Agent identity prompt built from zeroAgentId directly (no agentComposes indirection) ✅ PASS Uses formatAgentIdentityPrompt(agent) with data from zeroAgents table directly. buildAgentIdentityPrompt() deleted.
4 All 6 callers updated to pass zeroAgentId ✅ PASS All 7 call sites (6 + /api/zero/runs) verified: email-trigger, email-reply, slack, telegram, github, schedule, web API
5 All existing tests pass ✅ PASS CI: test-web (1-4), test-app, test-cli, test-other — all pass
6 Lint, type-check, format pass ✅ PASS CI: lint-eslint ✅, lint-types ✅, lint-format ✅
7 Knip passes (no unused exports) ✅ PASS CI: lint-knip ✅

2. Code Review

Dimension Status Findings
Correctness resolveZeroAgent() uses leftJoin (forward-compatible for when agentComposes is removed). All callers properly resolve zeroAgentId before calling createZeroRun(). Edge cases handled (null agent → error responses).
Project Standards No any types. No eslint-disable. No defensive try-catch. No over-engineering.
Consistency All integration handlers (email, slack, telegram, github, schedule) follow the same resolution pattern. Test helpers updated consistently with onConflictDoNothing/onConflictDoUpdate.
Security Parameterized queries throughout. No secret leakage. Auth checks unchanged.
Performance resolveZeroAgent() adds one indexed query (on zeroAgents.id PK + leftJoin on indexed orgId, name). Some callers add a lookup query, but these are per-request, not N+1.

3. Architectural Alignment

Check Status Notes
Source of truth correct createZeroRun() now takes zeroAgentId as the primary identifier. composeId is resolved internally as a passthrough for startRun().
No residual old patterns buildAgentIdentityPrompt(composeId) fully deleted. ZeroRunParams.composeId replaced with zeroAgentId.
Schema matches Epic design No schema changes in this sub-issue (as designed — schema migration is a separate sub-issue).
All call sites migrated 7 callers verified: all pass zeroAgentId. grep createZeroRun shows zero instances of composeId being passed.

4. Independence

Check Status
Compiles standalone ✅ — deploy-web passes
No forward dependencies ✅ — no references to tables/columns from other sub-issues
Migration applies cleanly ✅ — no DB migration in this PR
Tests pass in isolation ✅ — all test shards pass

5. Issues Found

Must Fix (blocking acceptance)

None.

Should Fix (non-blocking, but recommended)

None.

Nits (optional)

  1. Slack/Telegram RunAgentParams.composeId retainedcomposeId is still in the interface and destructured, but only used for error logging context. This is acceptable for now since these handlers still operate in a composeId-aware world. Can be cleaned up in the callback route migration sub-issue.

Verdict

ACCEPTED

Clean, well-executed implementation. All 7 createZeroRun() callers migrated to zeroAgentId. The resolveZeroAgent() helper is forward-compatible with eventual agentComposes removal. buildAgentIdentityPrompt() properly deleted. Test helpers updated to ensure zeroAgents rows exist. All CI checks pass (lint, types, format, knip, tests, deploy).

@lancy lancy added this pull request to the merge queue Mar 24, 2026
Merged via the queue into main with commit 51a1e64 Mar 24, 2026
64 checks passed
@github-actions github-actions Bot mentioned this pull request Mar 24, 2026
@github-actions github-actions Bot deleted the feat/issue-6231-switch-createzerorun-to-zeroagentid branch March 24, 2026 03:49
@github-actions github-actions Bot mentioned this pull request Mar 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(api): switch createZeroRun from composeId to zeroAgentId

1 participant