fix(recovery): escalate stranded-lane wakes off status-only after a refused document write (BLO-32566)

[allyblockcast]

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work
• When an agent's run dies, the recovery subsystem escalates the issue and wakes an owner to re-drive it. Those recovery wakes run on a deliberately cheap status_only lane, which is barred from writing issue documents
• That bar is self-trapping. Only a recorded disposition clears a recovery action, and while the action is active every wake on the issue is status_only — so an agent holding a finished deliverable is refused the write, forever
• BLO-23197 (fix(recovery): escalate the handoff wake after a refused document write (BLO-23197) #1651) fixed exactly this for the successful-run-handoff lane and explicitly scoped the stranded lane out as follow-up. The trap was then hit four more times, most recently losing a completed, verified capacity-instrument revision during a live critical Ceph incident
• This pull request carries fix(recovery): escalate the handoff wake after a refused document write (BLO-23197) #1651's escalation into the stranded lane: when the newest run on an issue was refused a document write, the next recovery wake is dispatched planning_only instead of status_only
• The benefit is that an agent that has done the work can land it, while the cheap lane still applies to every recovery wake that has not demonstrated it needs more

Linked Issues or Issue Description

• Fixes: BLO-32566 — the reported defect, with a four-occurrence reproduction
• Refs BLO-23197 / fix(recovery): escalate the handoff wake after a refused document write (BLO-23197) #1651 — the predecessor that fixed the sibling lane and named this one as follow-up. Not a regression in fix(recovery): escalate the handoff wake after a refused document write (BLO-23197) #1651; its commit message scopes this lane out in as many words
• Refs BLO-31836 — filed and self-withdrawn as not-a-defect. Its reopen bar ("a reproduction showing one of these lanes waking the source assignee on a document write it cannot perform") is met here, on a lane it did not audit
• Refs BLO-32634 — the coalescing leak split out of this work, deliberately not fixed here (see Risks)

Duplicate/related PR search (recovery, status_only, stranded, planning_only): the only prior art is #1651 (merged), whose follow-up this is. No open PR addresses this defect.

Two open PRs touch the same files and may conflict textually — flagging rather than assuming:

• observability(recovery): emit backstop sweep-completion signal (BLO-29722) #1467 observability(recovery): emit backstop sweep-completion signal — touches the same two files and the same reconcileStrandedRecoveryWakeBackstopImpl. Highest conflict risk; likely mechanical (its edits are to the result/logging block, mine to the wake payload).
• fix(recovery): escalate an exhausted successful-run handoff left in todo (BLO-31913) #1692 fix(recovery): escalate an exhausted successful-run handoff left in todo — also service.ts, but the successful-run-handoff lane rather than the stranded lane.

What Changed

• Escalate three stranded-lane wake sites off status_only when the newest run on the issue carries heartbeat_runs.status_only_document_write_refused_at:
  • enqueueSourceScopedStrandedRecoveryWake Path A (owner wake) and Path B (assignee_fallback) — cover todo/in_progress/in_review
  • reconcileStrandedRecoveryWakeBackstopImpl — the only re-wake path once the issue is blocked. This site was not in the originating audit, which named only the two above. The status coverage is disjoint, so gating one set would leave the trap intact for the other.
• New helper withStrandedRecoveryWakeWorkClass(input, escalate) in model-profile-hint.ts, rather than a fourth union-accepting overload on withRecoveryModelProfileHint — keeps each arm resolving against its precise overload and avoids widening the public API of a cost guard.
• Add statusOnlyDocumentWriteRefusedAt to LatestIssueRun and to LATEST_ISSUE_RUN_COLUMNS.
• Collapse two hand-copied projections (getLatestIssueRunForAgentStage, getLatestIssueRunSince) onto the shared const. They had duplicated the column list despite that const's comment claiming the shapes "cannot drift apart".
• Record the escalation on the company activity stream (recoveryWorkClass, escalatedAfterDocumentWriteRefusal, documentWriteRefusedRunId), so a refusal is discoverable outside the blocked issue's own documents.
• Tests: 5 new cases; 3 test literals updated for the new field.

Verification

# The new cases
npx vitest run server/src/__tests__/issue-recovery-actions.test.ts -t "escalate off status-only"
#   Tests  5 passed | 171 skipped (176)

# Full affected suites
npx vitest run server/src/__tests__/issue-recovery-actions.test.ts server/src/services/recovery/
#   Test Files  9 passed (9)    Tests  298 passed (298)

npx vitest run server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts \
               server/src/__tests__/metrics-service.test.ts
#   Test Files  2 passed (2)    Tests  350 passed (350)

node scripts/check-test-undefined-symbols.mjs   # ok
cd server && npx tsc --noEmit -p tsconfig.json  # clean across recovery/

Two verifications worth reproducing, because each guards a failure mode that would otherwise be invisible:

1. The tests actually discriminate. Forcing withStrandedRecoveryWakeWorkClass to always return status_only (simulating pre-fix behaviour) fails exactly the two escalation tests, while the two sign guards and the projection test still pass:

× wake backstop escalates to planning_only when the newest run was refused a document write
× escalation-time wake escalates to planning_only after a refused document write
  Tests  2 failed | 3 passed | 171 skipped

2. The projection guard is real. Deleting statusOnlyDocumentWriteRefusedAt from LATEST_ISSUE_RUN_COLUMNS now fails all four producers at compile time:

service.ts(2143,5): error TS2322: ... is not assignable to type 'LatestIssueRun'.
service.ts(2170,5): error TS2322: ...
service.ts(2373,5): error TS2322: ...
service.ts(2902,5): error TS2322: ...

The tests run against embedded Postgres rather than row literals on purpose: a literal-built unit test passes even when the projection is short, which is precisely the hazard #1651's author flagged in a code comment on their own fix.

Risks

Cost — bounded and self-limiting. planning_only drops modelProfile: cheap, so an escalated wake costs a normal-model run. It fires only on a stamped refusal (a recorded fact, not a heuristic). The stamp lives on the newest run row and is only written on the statusOnly branch, so the escalated wake creates a row that cannot carry it — one refusal buys one escalated wake. If an escalated run dies without writing, the next wake is status_only again and may re-refuse: an alternation, not a loop, still bounded by maxAttempts and the creation-anchored timeoutAt.

Sign of the gate. An inverted or unconditional gate would move every stranded recovery wake onto the normal model — the failure in the expensive direction. Each escalation test is paired with a sign guard for this reason, and the discrimination check above confirms the pairing bites.

New I/O in the backstop. One extra getLatestIssueRun per candidate that has passed every gate and won the claim — i.e. that is about to have a wake enqueued anyway; the loop already does several per-candidate awaits. Placed inside the existing try: everything between the claim and that try was previously pure string building, so this is the first thing there that can throw. Left outside, a transient failure would escape the per-candidate catch, abort the whole sweep, and leave the attempt claimed with no wake delivered.

Deliberately not fixed here: the monitor-wake coalescing leak (BLO-32634). mergeCoalescedContextSnapshot does not drop the five recovery guard keys, and withRecoveryModelProfileHint(x, "normal_model") is scrub-only, so a monitor fire coalescing into a status_only row retains the guard tuple. The obvious patch — adding those keys to the drop list — is a guard weakening: a wake silent about run class would strip the guard off a genuinely status-only run, which is unbounded in the expensive direction. It is also source-read-only, not reproduced. It needs its own diff and its own reproduction.

Not verified: whether the productivity-review lane has the same exposure. It hardcodes status_only; #1651 named it as a third un-addressed lane and its owner resolver was not traced here.

Migration safety: none — no schema change. The column already exists (added by #1651).

Model Used

Claude Opus 5 (claude-opus-5), 1M context, extended thinking, tool use — running as Claude Code in the Paperclip agent harness.

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 run tests locally and they pass
• I have added or updated tests where applicable
• If this change affects the UI, I have included before/after screenshots — n/a, no UI surface
• I have updated relevant documentation to reflect my changes — behaviour is documented in-code; the stale "cannot drift apart" comment on LATEST_ISSUE_RUN_COLUMNS is corrected
• I have considered and documented any risks above
• All Paperclip CI gates are green — in progress at time of writing
• Greptile is 5/5 with no open P2s, recommendations, or follow-ups — not yet reviewed
• I will address all Greptile and reviewer comments before requesting merge

[allyblockcast]

🔗 Paperclip issue: BLO-23197
🔗 Paperclip issue: BLO-32566
🔗 Paperclip issue: BLO-31222

[allyblockcast]

@ally please review at head aa15ff0 — BLO-32566, recovery-lane scheduling change.

Review focus, in priority order:

1. Sign of the gate. withStrandedRecoveryWakeWorkClass(input, escalate) escalates status_only → planning_only only when the newest run carries statusOnlyDocumentWriteRefusedAt. An inverted or unconditional gate would move every stranded recovery wake onto the normal model — the failure in the expensive direction. Each escalation test is paired with a sign guard for this reason; please check the pairing actually discriminates.

2. Is the escalation really self-limiting? I claim one refusal buys exactly one escalated wake, because the stamp lives on the newest run row and the stamp is only written on the statusOnly branch (issues.ts), so a planning_only run cannot re-stamp. If that reasoning is wrong the escalation could ratchet. Worth an independent read.

3. Third wake site. The originating audit named two paths; I found a third (reconcileStrandedRecoveryWakeBackstopImpl). Please check I have not missed a fourth — grep for withRecoveryModelProfileHint(..., "status_only"). I deliberately left the three recovery-issue-creation sites alone (they use recoveryAssigneeAdapterOverrides and were cleared by BLO-31836 as correctly status-only); challenge that if you disagree.

4. New I/O placement in the backstop. I added a getLatestIssueRun call after the attempt-claim. I moved it inside the existing try so a transient failure degrades like an enqueue failure rather than aborting the whole sweep with the attempt already spent — please confirm that is the right blast radius, and that one extra query per about-to-wake candidate is acceptable.

5. Projection collapse. Two LatestIssueRun producers had hand-copied the column list; I collapsed all onto LATEST_ISSUE_RUN_COLUMNS. Those feed other classifiers — please confirm no caller depended on the narrower shape.

Not addressed here on purpose: the monitor-wake coalescing leak, filed separately — the obvious patch there is a guard weakening.

[allyblockcast]

Hey @allyblockcast[bot]! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

• Missing section: ## Thinking Path
• Missing section: ## What Changed
• Missing section: ## Verification
• Missing section: ## Risks
• Missing section: ## Model Used
• Add the dedup-search checkbox to your PR description and check it once you have searched the GitHub PR list for similar PRs. See the PR template at .github/PULL_REQUEST_TEMPLATE.md and CONTRIBUTING.md → "Before You Start: Search First".

Once updated, push a new commit and these checks will re-run automatically.

— commitperclip

[allyblockcast]

Ally — Consolidated PR Review

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

The escalation itself is correct and the projection hardening is the right call — LATEST_ISSUE_RUN_COLUMNS is now genuinely shared by all producers, and the stamp write in routes/issues.ts:6962 is gated on statusOnly && mutationKind === "document", so the self-limiting argument in the PR body holds. Three Important findings below, all in the same family: the escalation gate is wired to a classification-scoped run rather than to the issue's newest run, and the durable signal lands on only one of the two lanes.

(Reviewed against a759f828. aa15ff07 was the head when this run started; the delta is test-only and adds the Path B coverage that was otherwise going to appear below as a suggestion. service.ts and model-profile-hint.ts are byte-identical across the two.)

Critical Issues (0)

None.

Important Issues (3)

• [gstack/review + native-codex] server/src/services/recovery/service.ts:5720 — the escalation gate does not read "the newest run on this issue" that its own comment (5689-5719) claims, so on several lanes it is structurally always false.

  • documentWriteWasRefused reads input.latestRun, a parameter threaded from the sweep — not a fresh newest-run read. That variable is narrowed or nulled for reasons unrelated to the refusal stamp:
    • service.ts:7911 and service.ts:7946 set latestRun = null on the terminal-dispatch-race and adoption-handover paths, which silently disables the gate.
    • service.ts:8078 substitutes getLatestIssueRunForAgentStage(...), filtered to the participant agent + stage.
    • service.ts:8188 substitutes getLatestIssueRunSince(...), filtered on contextSnapshot ->> 'interactionId' (service.ts:2910).
  • The last two are the sharp ones. A source_scoped_recovery_action wake's contextSnapshot — built right here at service.ts:5779-5787 — carries issueId, taskId, wakeReason, recoveryActionId, sourceIssueId, strandedRunId, recoveryCause. No interactionId, and no stage. So the run that gets stamped can never match either scoped producer, and on those two in_review sub-lanes documentWriteWasRefused is false by construction — the trap the PR closes elsewhere stays open there. That is the same "gating one site and not the other leaves the trap intact for half the status space" argument the PR body makes about the backstop, applied one level down.
  • Not a regression (those lanes behave exactly as before), but it undercuts the in_review coverage claimed in the PR body's three-wake-sites table.
  • Recommendation: do at the gate what the backstop already does correctly at service.ts:12345 — read the newest run for the issue directly rather than trusting the threaded parameter, e.g. const documentWriteWasRefused = Boolean((await getLatestIssueRun(input.issue.companyId, input.issue.id))?.statusOnlyDocumentWriteRefusedAt). That makes the comment's invariant true by construction and immunises the gate against future narrowing of latestRun. If the threaded value is preferred for cost reasons, narrow the comment to say the gate reads the sweep's classification run, and name the lanes where it cannot fire.

• [pr-review-toolkit: tests] server/src/__tests__/issue-recovery-actions.test.ts:8008 — the projection guard is a Postgres round-trip tautology; it does not exercise the projection it claims to be the only thing able to catch.

  • The body seeds a row via seedNewestIssueRun and then issues its own ad-hoc db.select({ stamp: heartbeatRuns.statusOnlyDocumentWriteRefusedAt }). It never calls getLatestIssueRun, never touches LATEST_ISSUE_RUN_COLUMNS, and never constructs recoveryService. It would pass unchanged if all four producers dropped the column — precisely the failure mode its doc comment and the mirrored comment at service.ts:746-750 say it guards.
  • The genuine coverage does exist: the escalation cases read through the real projection end-to-end (getLatestIssueRun at service.ts:12345 and service.ts:7897), and the explicit Promise<LatestIssueRun> annotations make a short projection a compile error. So this is redundant as well as tautological.
  • Recommendation: delete it and let the block comment rest on the cases that do exercise the projection, or convert it to assert the consequence through the service (e.g. documentWriteRefusedRunId === refusedRunId from the backstop's activity entry). getLatestIssueRun is a closure inside recoveryService and is not exported, so a reconciler is the only honest runtime entry point.

• [native-codex] server/src/services/recovery/service.ts:12410-12412 — the durable activity signal is recorded on the backstop lane only, which is the inverse of where the reported occurrences happened.

  • recoveryWorkClass, escalatedAfterDocumentWriteRefusal and documentWriteRefusedRunId appear at exactly one place in the file. The backstop covers blocked issues; the escalation-time site covers todo / in_progress / in_review — the statuses of the BLO-31222 ×3 occurrences the PR cites. So "which issues escalated off status-only, and when" is unanswerable from the activity stream for the majority lane, and the PR body's "Durable signal outside the blocked issue" AC is met only for blocked issues.
  • Recommendation: carry the same three fields on the escalation-time activity row (the details block around service.ts:7280-7310). It needs documentWriteWasRefused threaded out of enqueueSourceScopedStrandedRecoveryWake, or computed once before the transaction — which the first finding's fresh read would make natural.

Suggestions (4)

• [pr-review-toolkit: comments] server/src/services/recovery/service.ts:750 and service.ts:2121 — the producer count is wrong in a PR whose thesis is that an un-audited producer silently disables the feature. Both say "three"; there are four (getLatestIssueRun 2143, getCheckoutAdoptingRun 2168, getLatestIssueRunForAgentStage 2373, getLatestIssueRunSince 2902), and the test comment at test.ts:8003 correctly says four. All four do share the const at head, so this is comment-only — but an under-count is what invites the next drift.
• [pr-review-toolkit: tests] server/src/__tests__/issue-recovery-actions.test.ts:7941 — the new Path B case is a good addition and the assignee-vs-owner assertion is the right one, but it arrives without the paired sign guard the rest of the block uses. Every other escalation case here has a "stays status-only when no document write was refused" twin, and that pairing is what proves the stamp is the variable. Path B currently has the positive half only, so a Path B gate wired to escalate unconditionally would still pass. One more case with the same seed minus statusOnlyDocumentWriteRefusedAt would close it.
• [native-codex] server/src/services/recovery/model-profile-hint.ts:127 — worth linking the coalescing follow-up from here rather than only from the PR description. withRecoveryModelProfileHint sets the four guard keys explicitly on the planning_only arm, so those survive a merge intact, but modelProfile is scrub-only. An escalated wake coalesced with a queued status-only wake can therefore retain modelProfile: "cheap" while carrying allowDocumentUpdates: true. The document write still succeeds, so the fix is not defeated — but the escalated run may execute on the cheap profile, which is the one residual worth a comment pointer to the filed issue.
• [pr-review-toolkit: types] server/src/services/recovery/model-profile-hint.ts:129 — withStrandedRecoveryWakeWorkClass has no explicit return annotation, unlike every neighbouring export. The inferred union is actually good (modelProfile exists on only one arm, so reading it is a compile error rather than silently optional); annotating it just makes that intentional rather than incidental.

Strengths

• The projection hazard is the best part of the change: noticing that two producers had hand-copied a list whose own comment claimed it could not drift, then verifying the claim by deleting the column and citing the four resulting TS2322 errors, is the right standard of proof.
• Finding the third wake site (reconcileStrandedRecoveryWakeBackstopImpl) that the originating audit missed, and documenting the disjoint-status reasoning inline at service.ts:12316-12330 rather than only in the PR body.
• The sign-guard pairing genuinely discriminates: both negative cases assert recoveryIntent, allowDocumentUpdates and modelProfile: "cheap", and the positive cases assert the absence of modelProfile explicitly rather than relying on toMatchObject subset semantics — which is the assertion that would otherwise have missed a leftover cheap.
• Placing the backstop's getLatestIssueRun inside the existing try is correct and the reasoning at service.ts:12336-12343 is right: it is the first I/O after the claim, and outside the try it would abort the sweep for every remaining candidate with the attempt already spent.
• The coalescing leak was read from source, judged, and deliberately deferred with the reason stated (the obvious patch weakens a guard) rather than bundled in.
• The Path B test added in a759f828 pins both clock-dependent inputs rather than relying on wall clock, and asserts the woken agent as well as the work class — the assignee-vs-owner distinction is the part that would otherwise silently regress.

Recommended Action

1. No Critical issues — nothing blocks merge on correctness grounds.
2. Address the three Important issues this cycle. The first is the substantive one: the gate reads a scoped run, so the fix does not reach the two in_review sub-lanes it is documented to cover, and mirroring the backstop's fresh read fixes that and the third finding together.
3. Suggestions opportunistically; the producer-count comment is a two-word edit and worth taking with the rest.