Skip to content

fix(heartbeat): transfer issue lock ownership on process-loss/scheduled retries - #1

Open
ZataraMar wants to merge 1 commit into
masterfrom
fix/bbo-1547-process-lost-retry-lock-transfer
Open

ZataraMar wants to merge 1 commit into
masterfrom
fix/bbo-1547-process-lost-retry-lock-transfer

Conversation

@ZataraMar

Copy link
Copy Markdown
Owner

Summary

  • Fixes BBO-1547: process_lost_retry (and the equivalent scheduleBoundedRetryForRun codex-transient path) transferred an issue's checkoutRunId/executionRunId to the new retry run using a naive executionRunId = <dying run id> WHERE guard. If the lock had already drifted off that exact run id (BBO-1541: pointed at a third, already-terminal run), the UPDATE silently affected 0 rows — the retry run was created and woken, but the issue stayed pointed at a dead run forever, permanently rejecting every subsequent checkout/release/PATCH from the new run as an ownership conflict.
  • Both call sites now SELECT ... FOR UPDATE the issue row and decide from its live state instead of trusting the stale run snapshot: transfer authority to the new run whenever the current lock already belongs to the dying run, is unset, or points at a run that is itself terminal/missing (via TERMINAL_HEARTBEAT_RUN_STATUSES). Only refuse when a different, still-live run genuinely owns the issue.
  • Adds server/src/__tests__/heartbeat-process-loss-retry-lock-transfer.test.ts, reproducing the exact BBO-1541 divergence and asserting checkout/assertCheckoutOwner/release all succeed against the retry run afterward.

Test plan

  • pnpm exec vitest run server/src/__tests__/heartbeat-process-loss-retry-lock-transfer.test.ts — new regression test passes
  • pnpm exec vitest run across 7 related heartbeat lock/ownership/retry test files (heartbeat-lock-release-on-reassignment, heartbeat-retry-scheduling, heartbeat-process-recovery, heartbeat-start-lock, heartbeat-responsible-user-invariant, heartbeat-zombie-guard, heartbeat-stale-queue-invalidation) — 133/133 passing, no regressions
  • pnpm --filter @paperclipai/server typecheck — clean

Refs: BBO-1551, BBO-1547

…ed retries

process_lost_retry (and the equivalent scheduleBoundedRetryForRun codex-transient
path) transferred an issue's checkoutRunId/executionRunId to the new retry run
with a naive `executionRunId = <dying run id>` WHERE guard. If the issue's lock
had already drifted away from that exact run id by the time the transfer ran
(BBO-1541: the lock pointed at a third, older, already-terminal run — neither
the timed-out run nor the new retry run), the UPDATE silently affected 0 rows.
The retry run was created and woken, but the issue kept pointing at a dead run
forever, and every subsequent checkout/release/PATCH from the new run was
rejected as an ownership conflict with no automatic recovery (BBO-1547).

Both call sites now lock the issue row and decide from its live state instead
of trusting the stale run snapshot: transfer authority to the new run whenever
the current lock already belongs to the dying run, is unset, or points at a
run that is itself terminal/missing. Only refuse when a different, still-live
run genuinely owns the issue.

Adds a regression test reproducing the exact BBO-1541 divergence and asserting
checkout/assertCheckoutOwner/release all succeed against the retry run
afterward.
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