fix(runs): replay worker retries only for the writing lease; fence retention handoffs
A Run worker can now retry an event or artifact it already wrote and get the stored row back, even after its lease expired, was replaced, or was cleared by its own terminal event. A stale lease holder can no longer get a success reply for someone else's work or overwrite the replacement worker's state.
What changed:
-
run_eventsandrun_artifactsrecord which lease wrote each row: its generation and the SHA-256 of its token. The new migration is20261019120000_run_writer_lease. It is additive with nullable columns, so it stands alone. -
ingestRunEventandrecordRunArtifactapply the retry rule below before they check the lease. A request with a new id still needs the live lease. -
Worker responses drop the writer columns.
-
scheduleAttemptRetentionneeds the worker's lease token and generation and a terminal attempt. A stale worker'sfinallyblock therefore can't overwrite the replacement's VM, workspace and terminal handles or its retention deadline. A terminal event clears only the lease expiry and keeps the token until this handoff consumes it. -
Cancel during VM create no longer leaks the VM. A cancel that lands while
createVmis in flight bumps the lease generation before the worker has stored the VM id. The worker'sattempt.startedingest and its fencedscheduleAttemptRetentionhandoff were then both rejected, so no row pointed at the VM and the reaper never destroyed it. Now, when the fenced handoff is rejected,scheduleAttemptRetentionstill records the provider VM id for immediate reaping (terminalRetainUntil = now). It does this only when all of these hold:- the attempt is terminal and its lease generation is newer than the caller's;
- the attempt has no VM id, retention deadline or cleanup lease;
- no event for the attempt ever carried a
providerVmId.
Nothing else can own that VM, because create is idempotent per attempt. A VM the reaper already destroyed and cleared is never handed back to it. No other field of the fenced worker is written.
Carved from #21305 (commits bdbf27bd05, 4d9248ff18, f8b5cc631d, 585deed335). The durable Run schema and repository landed on feat-cmux-next in #21337, and web/services/runs hasn't changed there since. This PR is the lease-fencing delta on top of it, now rebased onto current origin/feat-cmux-next. main's separate Run implementation (#21444 and its follow-ups) has no worker leases, so nothing there supersedes this.
Decision
Rule: a request that reuses a stored event id or artifact name gets the stored row back (200, nothing written) only when the caller's lease token hash and generation match the lease that wrote that row and the request is identical:
- for events: kind, attempt sequence and payload, compared by jsonb equality;
- for artifacts: digest, size and URI.
Any other reuse is a 409:
run_event_conflictorrun_artifact_conflictwhen the caller is the writer or holds the live lease;- otherwise
run_lease_conflict.
A stale caller gets the same reply for a reused id as for a new one, so it learns nothing about stored rows. Rows with no recorded writer match nobody.
Cases, each a DB behavior test in web/tests/run-repository-db-behavior.test.ts:
| Case | Result |
|---|---|
| Same holder retries an identical terminal event after its expiry was cleared, and again after the retention handoff cleared the token | Stored row, attempt stays succeeded, no new journal rows |
| Stale holder sends a different event (new id) after a replacement took the lease | run_lease_conflict |
| Stale holder reuses an id the replacement wrote | run_lease_conflict |
| The writer retries an identical non-terminal event after being replaced | Stored row, no new journal rows |
| The writer retries the same id with different content | run_event_conflict |
Replacement holder sends an identical copy of the old lease's attempt-started | run_event_conflict; attempt stays leased |
| Row with no recorded writer | Nobody gets the stored row |
| Artifacts | Same rule |
Counterargument: I asked a review agent to argue against the first draft. These changes came out of it:
- Dropped "the live holder may replay any identical row": that clause let a replacement worker's identical
attempt-startedreturn the old lease's row. The attempt then stayedleasedwhile the new worker believed it had started. - Payload compared in SQL: jsonb equality, so key order and number formatting don't break a retry.
- Writer columns on every lease-created row, including the
attempt.artifactjournal row. - Identity is token hash plus generation: all workers share the worker token, and generations are guessable.
Open points it raised, not changed here:
- Fixed per-attempt ids can block a replacement worker. A replacement worker that reuses ids such as
attempt-failedwith low attempt sequences collides with the old lease's rows and with per-attempt sequence uniqueness, so it can't record a terminal event for that attempt. This was already true before this PR. Fixing it needs ids and sequences scoped by generation, and that's a separate change. - Replies don't say the lease is gone. A stale writer whose retry succeeds isn't told its lease is gone; it finds out on its next renew. That's harmless to data.
Validation
- Locally, on huge-blue with Postgres 16 in podman, migrated with
bun scripts/db-migrate.mjs:CMUX_DB_TEST=1 bun test tests/run-retention-db-behavior.test.ts:- Before the fix (
e966878b7a7, test commit only),schedules cleanup for a VM whose create returned after cancellation fenced the workerfails withstale or expired attempt lease. - After the fix (
d7fe72e9565), all 5 pass.
- Before the fix (
- The guard case,
does not hand a stale worker's VM back to retention after the reaper destroyed it, passes both before and after. - Full
bun run test:db:behavioron head: exit 0, no failures. tsgo --noEmit: no errors inweb/servicesorweb/tests/run-*. The only errors are fromworkers/sources, which that partial copy didn't have. CIweb-typecheckis green.
- Commits put tests before fixes:
1c0ece6d38d(tests), thena8498dd0335(fix);99c25ae938e(tests), then2ced057fdac(fix);e966878b7a7(tests), thend7fe72e9565(fix).
Proof
- Not UI: web Run repository and DB behavior tests only (
web/services/runs,web/tests, one additive migration). No app, webview or Swift code. - Runtime test:
run-retention-db-behavior.test.ts(schedules cleanup for a VM whose create returned after cancellation fenced the worker,does not hand a stale worker's VM back to retention after the reaper destroyed it, plus the existing retention cases) andrun-repository-db-behavior.test.ts(the lease-fencing retry cases), inweb / web-db-migrationsond7fe72e9565: https://github.com/manaflow-ai/cmux/actions/runs/39344503727/job/118374018262
Changelog
Fixed: Cloud Run workers can safely retry their own events and artifacts, a replaced worker can no longer overwrite the new worker's state, and cancelling a Run while its VM is being created no longer leaks the VM.
š¤ Generated with Claude Code