Skip to content

Stop deleting live folded-run rows on post-deploy grants/clone failure - #476

Merged
TheGreatAxios merged 3 commits into
mainfrom
cl-7213-rollback-data-loss
Aug 30, 2026
Merged

TheGreatAxios merged 3 commits into
mainfrom
cl-7213-rollback-data-loss

Conversation

@TheGreatAxios

Copy link
Copy Markdown
Contributor

Summary

deployAtHead in packages/folded-runs/src/launch.ts writes markRunDeployClone and sends the run's grants frame after deployAdoptedWorkflowFromSource resolves — meaning a live child already exists on the sidecar by that point. A plain Error thrown out of either step used to be indistinguishable from "the deploy never happened," so launchFoldedRun's failure-path rollback deleted the run's workflow_run and folded_run rows, leaving a live, unauthorized, and now completely untracked agent running.

Both post-deploy failure sites now throw SessionLaunchError(phase, cause, leakedAgent: true) — the same signal deployAdoptedWorkflowFromSource itself already uses elsewhere in the vendored session service — so launchFoldedRun's existing leaked branch marks the run failed but leaves it routable instead of deleting its rows.

Verification

  • Grepped every caller of launchFoldedRun (packages/folded-run-one-shot, packages/chat/src/platform-adapter.ts, apps/hub/src/routine-launcher.ts, scripts/db-setup.ts, others): none do type-based inspection on the thrown error beyond a generic instanceof Error for message extraction, which SessionLaunchError still satisfies. No caller behavior changes.
  • bun run typecheck — clean
  • bun run lint — 0 errors (pre-existing unrelated warnings only)
  • packages/folded-runs test suite — 63 pass, 0 fail, including the two new leak-rollback regression tests

Scope

Limited to the rollback path in packages/folded-runs/src/launch.ts. Does not touch the credentialCipher wiring or the crypto cache work living in the same package under other tickets.

Linear: CL-7213

deployAdoptedWorkflowFromSource resolving means a live child now exists
on the sidecar; a plain Error out of markRunDeployClone or
sendRunGrants afterward currently reads as "nothing was deployed" and
deletes the run's rows, orphaning a live, unauthorized agent with no
DB trace (CL-7213).
deployAdoptedWorkflowFromSource resolving means a live child now exists
on the sidecar. A plain Error out of markRunDeployClone or sendRunGrants
afterward used to read as "nothing was deployed," so launchFoldedRun's
failure-path rollback deleted the run's rows, leaving a live agent
running, unauthorized, and untracked (CL-7213). Both failure sites now
throw SessionLaunchError(..., leakedAgent: true), matching the existing
signal the rollback already branches on.
… phases

Both post-deploy failure sites threw SessionLaunchError with the same
"grants" phase; SessionLaunchError.phase is surfaced in upstream cleanup
diagnostics, so a markRunDeployClone failure was misreported as a
grants failure. Each step now throws with its own phase label.
@TheGreatAxios

Copy link
Copy Markdown
Contributor Author

Read the diff directly, traced the acceptance criteria against it, and ran the code-review skill against this branch (note: a parallel review lane was saturating the shared subagent pool during part of this pass, so some of the deeper read was done directly rather than via a sub-agent — recorded here for the record).

Verified the try/catch boundary in deployAtHead starts exactly where deployAdoptedWorkflowFromSource has resolved (a live child now exists) and stops exactly at the function's return — nothing after the deploy call escapes it. deployAdoptedWorkflowFromSource itself already classifies its own internal failures via the same SessionLaunchError.leakedAgent mechanism, so the pre-deploy region is correctly left alone. Checked the other caller of the shared deployAtHead step (wakeFoldedRun, packages/folded-runs/src/wake.ts): it never deletes rows on failure, so it was never exposed to this bug and needed no change — confirms the fix's stated scope. All three CL-7213 acceptance criteria are genuinely met, including the new sendRunGrants-failure regression test.

One nit found and fixed: both post-deploy failure sites (markRunDeployClone and sendRunGrants) threw SessionLaunchError with the same "grants" phase label. SessionLaunchError.phase is surfaced in upstream cleanup diagnostics (session-service.ts's deployAgentUndeploy failure logging), so a markRunDeployClone failure would have been misreported as a grants failure. Split into two per-step throws with distinct phase labels ("clone" / "grants").

Verified: bunx prettier --check, WORKBENCH_CHECK_CONCURRENCY=2 WORKBENCH_CHECK_SINCE=origin/main bun run typecheck, eslint on the touched files, and packages/folded-runs's full test suite (63/63, run twice under a concurrency cap given the host's current load) all clean.

@TheGreatAxios
TheGreatAxios merged commit 3498e94 into main Aug 30, 2026
5 checks passed
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.

1 participant