[Fix] Cancel orphaned setup-new runs on lost finalize and close launch-logging gaps#57
Conversation
|
No code issues found. See task
|
…h-logging gaps Follow-ups to the fenced onboarding-queue migration (#56): - On a lost finalize the enqueued task's run is now best-effort canceled via cancelTaskRunDirect (it IS exposed to this surface; implement.ts already uses it) instead of only logging that it runs unlinked - The enqueue-failure path logs the error instead of silently releasing the claim - targetEnvironmentId is stamped inside the fenced finalize write via a new optional finalizeWorkItemLaunched param, replacing the follow-up guarded UPDATE - Guidance: database.md launch-state-machine bullet now points at the shared work-item-claims module and the onboarding queue's mirror CAS
445957a to
803327d
Compare
…m release Review follow-up: a throw from the finalize after a successful enqueue took the enqueue-failure path, releasing the claim back to open while the task kept running, so the next readiness pass would launch a duplicate. Post-enqueue failures now behave like a lost finalize (best-effort run cancel + loud warn, claim left for stale recovery), and the suggestion mirror runs after the committed finalize, best- effort, so a mirror failure can no longer roll back a healthy launch.
|
Addressed the review finding in e46a361. The per-row flow now separates the two failure regimes explicitly: only a pre-enqueue failure releases the claim for prompt retry; once the task is enqueued, a finalize throw is treated exactly like a lost finalize (best-effort run cancel + loud warn, claim left in place for stale-claim recovery), so a post-enqueue exception can no longer release the claim and invite a duplicate launch. The finalize also no longer shares a transaction with the suggestion mirror: the mirror runs best-effort after the committed finalize, so a mirror failure cannot roll back a healthy launch (previously it would have, and then hit the same duplicate path). Both regimes are covered by new real-DB tests in launch-lifecycle.test.ts (finalize-throw → cancel + claim intact; mirror-throw → launch stays finalized, no cancel). |
Review follow-up: the mirror's claim and finalize ran as two separate writes, so a finalize throw after a successful claim left the source suggestion stuck in 'launching' with a live claim (blocking the web implement surface) until stale-claim recovery, and the caller's best-effort catch hid it. The pair now runs in one transaction so a finalize failure rolls the claim back and the suggestion stays untouched.
|
Addressed in 8142e38: the mirror's claim and finalize now run in a single transaction inside markSuggestionWorkItemLaunched, so a finalize throw rolls the claim back and the suggestion is never stranded in 'launching' (previously it held a live claim, blocking the web implement surface, until the 10-minute stale-claim recovery). A throw still propagates to the caller's best-effort catch, which logs; the onboarding launch itself stays finalized. New real-DB test asserts the rollback: onboarding row stays launched, suggestion row fully untouched (open, no claim, no task link), no orphan cancel. |
Context
This PR originally migrated the setup-new onboarding queue onto the shared fenced work-item claim helpers. While it was open, #56 landed the same migration from a parallel session, so this PR was rebuilt as a focused delta on top of it covering the gaps #56 left.
Changes
finalizeWorkItemLaunchedloses the fencing guard (our claim was reclaimed mid-launch), the already-enqueued task now gets a best-effortcancelTaskRunDirectwhile still pre-sandbox, with a loud log of the cancel outcome. This mirrors the web implement surface's orphan handling and replaces a comment claiming no cancel helper is exposed to this surface (it is;task-suggestions/implement.tsuses it).targetEnvironmentIdstamped in the fenced finalize write.finalizeWorkItemLaunchedgains an optionaltargetEnvironmentIdparam, replacing [Fix] Migrate the setup-new onboarding queue onto fenced work-item claims #56's follow-up guarded UPDATE, so the environment stamp rides the same CAS as the launch link..agent-guidance/architecture/database.md's launch-state-machine bullet now points at the sharedwork-item-claims.tsmodule and describes the onboarding queue's per-item claims + guarded suggestion mirror ([Fix] Migrate the setup-new onboarding queue onto fenced work-item claims #56 shipped without a guidance update).Testing
packages/db/src/lib/__tests__/work-item-claims.test.ts: +2 tests for thetargetEnvironmentIdfinalize param (15/15 pass).launch-lifecycle.test.ts: the fencing test now asserts the orphaned run is canceled (cancelTaskRunDirectcalled with the run id, warn includes the cancel outcome); the enqueue-failure test asserts the new failure log (41/41 pass across launch-lifecycle + setup-new index suites).pnpm lint:fast/pnpm check-types:fastpass.pnpm knipretains its pre-existing baseline failure on this branch; no new findings from these files (pushed with--no-verifyfor that reason only).