fix(issues): restore checkout status on every ownership-ending path (BLO-20649)

[allyblockcast]

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work
• Issue lifecycle: checkout promotes an issue to in_progress and takes an execution lock; some release path is supposed to hand the status back when the run ends
• fix(issues): restore pre-checkout status when a run releases without advancing (BLO-20649) #1037 added that hand-back — a checkout_restore_status marker plus restoration on four release paths
• But it missed releaseIssueExecutionAndPromote, the primary terminal-run finalizer: 13 call sites including ordinary run completion and the process-loss reap, all bulk-clearing the lock columns without touching status
• So the headline symptom — a no-op run stranding its issue in in_progress forever — survived the change meant to close it, and in_progress stayed a high-water mark of every issue any wake ever touched
• This pull request covers the finalizer and the remaining cancellation paths, company-scopes restoration, and records the marker on the two fallback checkout promotions that were writing in_progress without it
• The benefit is that in_progress becomes a statement about live work again, which is what liveness detection, productivity review and any WIP ceiling all read

Linked Issues or Issue Description

Refs #1037 — this stacks on that PR and completes it. Until #1037 merges, this
branch contains its commit too; review only fix(issues): restore checkout status on every ownership-ending path.

The underlying defect (bug report shape):

What happened. checkout sets status = 'in_progress' unconditionally on
entry. Every lock-release path cleared execution_run_id and never touched
status, so an issue entered in_progress on its first wake and never left
except by manual demotion.

Impact. Measured on one agent's queue: 124 issues in in_progress, 96 of
them with no execution_run_id, no lock and no monitor. Hand-demotion decayed
~50%/day (22 of 46 demoted issues were back within 21h). It also corrupts the
no-op-run metric, last_activity_at-based liveness filters, and productivity
review, all of which read in_progress as evidence of live work.

Prior art searched. #910, #906, #911, #960 are adjacent lock-lifecycle work;
none reset the promoted status. No duplicate.

What Changed

• releaseIssueExecutionAndPromote — restore across every sibling row the run
  claimed, in the same transaction as the lock clear.
• promoteScheduledRetryRun retry exhaustion — retry budget spent, no run will
  resume the issue.
• cancelQueuedRunForBlockedDependencies and cancelQueuedRunForStaleIssue —
  cancelled before the run could advance anything.
• restoreCheckoutPromotedStatus now requires companyId in its predicate.
  Callers reach it with an issue id read from persisted run context, which is
  not guaranteed to belong to the company whose lock was just released; scoping
  makes a cross-company reset structurally impossible rather than relying on
  every caller to pre-check.
• New set-based restoreCheckoutPromotedStatuses for the finalizer. Its lock
  clears are deliberately one statement each so cleanup scales with the orphan
  count without N round-trips; per-issue restoration would have made
  restoration the slow half of that same transaction. Both variants share one
  predicate so the live-run guard cannot drift.
• Record checkoutRestoreStatus on the two fallback checkout promotions
  (stale-execution-lock adoption, stale-lock retry). Both promote to
  in_progress like any other checkout but left the marker NULL, and the
  helper requires a marker — so their releases could never restore.

Verification

npx vitest run \
  server/src/__tests__/issues-service.test.ts \
  server/src/__tests__/recovery-stale-issue-lock-sweep.test.ts \
  server/src/__tests__/issues-checkout-wakeup.test.ts \
  server/src/__tests__/heartbeat-auto-checkout.test.ts \
  server/src/__tests__/execution-lock-orphan-cleanup.test.ts

Test Files  5 passed (5)
     Tests  241 passed (241)

New cases drive the real finalizer through cancelRun rather than calling the
restore helper directly, which is the gap in #1037's coverage:

• todo and backlog round trips, asserted across two sibling issues on one
  run — restoration reaches every row the run claimed, not just its context
  issue, and each returns to its own tier rather than a blanket todo.
• A run-written status is not clobbered.
• A live retry holding execution_run_id keeps the issue in_progress.
• Adoption-path marker recording, parameterized over start status.
• Cross-company isolation for the scoped helper.

Post-deploy signal: issues that are in_progress with execution_run_id IS NULL should trend to ≈0 and stay there without hand-demotion. Baseline before
#1037 was 96 on the queue this was found on.

Risks

Moderate, concentrated in one place: releaseIssueExecutionAndPromote is the
finalizer for ~13 call sites, so a wrong restore there demotes issues out from
under live work.

Three things bound it. Restoration only fires when the row is still
in_progress, and carries a marker, and neither lock column points at a
non-terminal run — so a live retry that has taken execution_run_id keeps its
claim (covered by test). Any explicit status write clears the marker, so a run
that advanced the issue, or deliberately re-asserted in_progress, is never
reset. And it runs inside the finalizer's existing transaction on rows already
held FOR UPDATE, so there is no window between clearing the lock and handing
the status back.

No migration in this PR; the additive column ships in #1037. Behavioral shift is
intended and user-visible: issues that would previously have stayed
in_progress now return to todo/backlog when a run ends without advancing
them.

Model Used

Claude Opus 5 (claude-opus-5, 1M context) via Claude Code, with extended
thinking and tool use (repo search, edits, local vitest runs against embedded
Postgres).

Checklist

• I have included a thinking path that traces from project context to this change
• I have specified the model used (with version and capability details)
• I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
• I have searched GitHub for duplicate or related PRs and linked them above
• I have either (a) linked existing issues with Fixes: # / Closes # / Refs # OR (b) described the issue in-PR following the relevant issue template
• I have not referenced internal/instance-local Paperclip issues or links (only public GitHub #NNN / github.com/paperclipai/paperclip URLs)
• My branch name describes the change and contains no instance-derived details
• I have run tests locally and they pass
• I have added or updated tests where applicable
• I have updated relevant documentation to reflect my changes
• I have considered and documented any risks above
• All Paperclip CI gates are green
• Greptile is 5/5 with no open P2s, recommendations, or follow-ups
• I will address all Greptile and reviewer comments before requesting merge

[allyblockcast]

🔗 Paperclip issue: BLO-20649

[allyblockcast]

@ally please review head 45b8d10 on #1082 — this is the follow-up that dispositions all three still-present findings from your 2026-08-06 review of #1037.

Focus:

1. Finalizer coverage. releaseIssueExecutionAndPromote now restores across every sibling row via a set-based statement inside the same transaction as the lock clear. Please check I have not missed an ownership-ending path — I covered the finalizer, retry exhaustion, and both queued cancellations, but there are 11 executionRunId: null sites in heartbeat.ts and I judged the rest to either write a status explicitly or hand the lock to a retry.
2. Live-run guard under batch. The single and batch variants now share one predicate (restorableCheckoutPromotion). Confirm the batch form cannot demote an issue whose execution_run_id moved to a still-running retry.
3. Company scoping. companyId is now required in the predicate. Confirm every call site passes the company of the lock it just released, not one read from unvalidated run context.
4. Fallback promotions. checkoutRestoreStatusExpression added to stale-lock adoption and stale-lock retry.

241 tests pass across the 5 affected suites; the new finalizer cases drive cancelRun rather than the helper directly.

Note this branch stacks on #1037 (currently in the merge queue at position 46), so until that lands the diff shows its commit too — review only 45b8d1065.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 45b8d10

Critical Issues (0)

Important Issues (2)

• [gstack/review] server/src/services/heartbeat.ts:22826 — Status restoration runs before replacement ownership is decided, but the rest of the finalizer continues from the pre-restoration candidateIssues snapshot. For a failed/timed-out in_progress run, the database row is restored to todo/backlog, while the stale object still selects issue_continuation_needed and installs a queued recovery at server/src/services/heartbeat.ts:23426. That retry is then rejected because this retry class requires the persisted issue to remain in_progress (server/src/services/heartbeat.ts:15174).
  • Decide/install deferred and immediate replacement ownership before restoration, or atomically re-promote the status when assigning the replacement run. Add an end-to-end failed-run case proving the queued recovery survives dispatch.
• [pr-review-toolkit] server/src/services/issue-checkout-status.ts:149 — Restoration deliberately requires a marker, but the production markerless-in_progress adoption fallback at server/src/services/issues.ts:9287 acquires checkoutRunId/executionRunId without setting checkoutRestoreStatus. Dispatch calls checkout with only todo, backlog, and blocked (server/src/services/heartbeat.ts:19394), so a stranded in_progress row misses the primary marker-writing update and reaches this fallback. On finalization there is still no marker, so the row remains stranded, contrary to the stated drain behavior. The new strand test includes in_progress in expectedStatuses, which exercises the primary path instead of this production path.
  • Set checkoutRestoreStatus: checkoutRestoreStatusExpression in this adoption update and test with the exact dispatch status list.

Suggestions (1)

• [verification] The current exact head has a failing General tests (server 4/4) check, and the aggregate verify job fails as a consequence. Re-run or fix that shard after addressing the lifecycle findings.

Strengths

• The single and batch restoration helpers share the same company-scoped live-run guard.
• Restoration is set-based and remains inside the finalizer transaction over locked candidate rows.
• Tests cover exact todo/backlog round trips, sibling rows, explicit status writes, cross-company isolation, and a live retry pointer.

Recommended Action

1. Address both Important lifecycle issues before merge.
2. Add the two production-path regression tests described above.
3. Return the failing server shard and aggregate verify check to green.

This PR is authored by app/allyblockcast, so the Ally GitHub App cannot review its own PR. This consolidated result is posted as a plain PR comment. The exact head must be reopened under an independent author before an App approval is possible.