fix(recovery): persist job-missing invocation evidence (BLO-18106)

[allyblockcast]

Tracks BLO-18106 (job_missing durability) and BLO-19889 (pod-never-scheduled recovery budget).

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work
• Agent runs execute in external Kubernetes lifecycle Jobs; the reconciler finalizes a run by observing that Job
• When the Job disappears after the run already produced a durable side effect (a posted GitHub review), the reconciler overwrote the recorded outcome with failed/job_missing and stranded the owning issue
• Recovery escalation was itself not durable: it committed blocked + the recovery action, then dispatched the owner wake after commit, so a failed dispatch left an issue blocked with an active action and nobody scheduled to look at it
• It also wrote the escalation activity through the outer connection, so a rolled-back escalation still emitted a phantom issue.updated
• This pull request persists job-missing invocation evidence, fences replay, and makes review-stage finalization atomic — with a bounded backstop that redelivers an undelivered recovery wake
• The benefit is that a run's real-world outcome is never discarded, and a recovery escalation either fully happens or fully does not

Linked Issues or Issue Description

• Refs BLO-18106 — make run finalization durable when the lifecycle Job disappears after a side effect
• Refs BLO-19889 — stop pod-never-scheduled failures burning recovery budget

Related open PRs (searched, not duplicates — see Risks):

• fix(recovery): wake-budget backstop claim/refund fences (BLO-22795) #1131 — stacked on this branch; carries the wake-budget claim/refund fences (BLO-22795)
• fix(recovery): stop the durable wake-outbox marker double-counting as a delivered wake (BLO-18829) #1101 — durable wake-outbox marker double-counting (BLO-18829); adjacent mechanism, no shared code on master today
• Fix stranded-blocked reconciler eligibility (BLO-21523) #1112 — stranded-blocked reconciler eligibility (BLO-21523); different cause (dependency-graph edges), new standalone file

What Changed

• Persist job_missing invocation evidence so a run that produced a side effect is not re-finalized as failed, and fence replay so the side effect is not re-executed during recovery.
• Stop pod-never-scheduled failures from consuming recovery attempt budget (BLO-19889).
• Make review-stage escalation atomic: recovery-action creation, blocker reads, the blocking issue update and comments all route through the row-locking transaction; wake dispatch is deferred until that state commits.
• Add reconcileStrandedRecoveryWakeBackstop, hosted in the issue-graph liveness pass, which redelivers a recovery wake that was committed but never dispatched. It deliberately does not re-escalate — no re-routing, no evidence rewrite, no status change — and is bounded by a 30m cooldown plus the action's creation-anchored timeoutAt.
• Write the escalation activity through mutationDb with deferPublish, so a rolled-back review-stage escalation emits no phantom issue.updated row or live/plugin event.

Why widening the sweep's status filter was rejected instead of adding the backstop: reconcileStrandedAssignedIssues feeds 18 escalateStrandedAssignedIssue call sites plus the park/dispatch branches, none written for blocked input, and BLO-5681 depends on the exclusion to keep zero-token startup wedges fire-once.

Verification

Local, at head c9dab5727:

pnpm typecheck                          -> clean, 0 diagnostics
npx vitest run --no-file-parallelism \
  src/__tests__/activity-log-responsible-user.test.ts \
  src/__tests__/heartbeat-process-recovery.test.ts   -> 2 files, 188/188 passed

--no-file-parallelism is required, not cosmetic: each suite starts its own embedded Postgres, so running them in parallel is self-inflicted resource contention and produces spurious failures. CI serializes them (Verify serialized server suites N/4).

Backstop observed actually firing in the run log, rather than only asserted:

WARN: stranded recovery wake backstop redelivered undelivered recovery-action wakes {"healed":1,...}

New/updated tests: redelivery, the BLO-5681 no-action suppression regression, no double-wake on the healthy path, exhausted-budget and cooldown rate-limiting, execution inside the liveness pass, and two logActivity contract cases (rollback → no row and no event; commit → the returned publisher does emit — the second is a positive control, since the rollback assertion alone would pass vacuously against a mis-wired subscription).

Full-suite verification is delegated to CI; I have not run all four server shards locally.

Risks

• Medium. The backstop redelivers wakes, so a redelivery bug would surface as duplicate agent runs. Bounded by the 30m cooldown and the creation-anchored timeoutAt, and there is an explicit no-double-wake-on-the-healthy-path test.
• This branch is 106 commits behind master. The merge queue rebases and re-tests before landing, so correctness is gated there rather than by assertion here — but a reviewer should treat the interaction with anything recovery-adjacent landed since as unverified locally.
• No wake-outbox exists on master today, so reconcileStrandedRecoveryWakeBackstop is not redundant with fix(recovery): stop the durable wake-outbox marker double-counting as a delivered wake (BLO-18829) #1101/fix(recovery): make stranded escalation atomic and its owner wake durable (BLO-18829) #820 — but if that outbox lands first, the two delivery paths must be reconciled to avoid double delivery.
• No schema migration. No UI change. No breaking API change.

Model Used

Claude Opus 5 (claude-opus-5[1m], 1M context), extended thinking, with tool use and code execution, via the Paperclip claude_k8s agent adapter.

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 (affected suites, 188/188; full suite delegated to CI)
• I have added or updated tests where applicable
• If this change affects the UI, I have included before/after screenshots (N/A — no UI change)
• I have updated relevant documentation to reflect my changes (N/A — no user-facing docs affected)
• I have considered and documented any risks above
• All Paperclip CI gates are green (CI still running at c9dab5727 at time of writing)
• Greptile is 5/5 with no open P2s, recommendations, or follow-ups (not yet run at this head)
• I will address all Greptile and reviewer comments before requesting merge

---

Split history — read before approving

2026-08-07: this PR was split per the CEO ruling on BLO-18106; the wake-budget cluster moved to #1131 (BLO-22795), stacked on this branch.

2026-08-08: the split boundary was corrected. It had been drawn at 646cd31f by commit position, which moved this PR's own review-fix commits into the child — leaving #1048 re-presenting the same two Important findings every round. 882dc3880's own message reads Refs BLO-18106. It has been moved back:

commit	content	proof
2e6992761	cherry-pick of 882dc3880	tree byte-identical to 882dc3880
c9dab5727	only the deferPublish hunk + 2 logActivity contract tests from a2a5fcc38	service.ts differs from a2a5fcc38 by exactly the @@ -7851 claimWakeAttempt hunk

#1131 was rebased onto the new head; git dropped 882dc3880 as "patch contents already upstream", and the rebased tip's tree is identical to the pre-split 74a308302 — nothing lost, nothing duplicated across the two PRs.

Auto-merge: OFF, verified autoMergeRequest: null after every push. The APPROVED decision shown is a stale human approval at b5141458, not an approval of this head; it must not be treated as one. Not re-queuing until a confirming Ally review lands at c9dab5727.

[allyblockcast]

🔗 Paperclip issue: BLO-16182
🔗 Paperclip issue: BLO-18669
🔗 Paperclip issue: BLO-18106
🔗 Paperclip issue: BLO-19889

[allyblockcast]

🔗 Paperclip issue: BLO-16182
🔗 Paperclip issue: BLO-18669
🔗 Paperclip issue: BLO-18106
🔗 Paperclip issue: BLO-19889

[allyblockcast]

@ally please review PR #1048 (BLO-19889).

Review focus:

1. Is k8s_pod_schedule_failed genuinely safe to re-dispatch unconditionally? I claim the pod never bound to a node so the adapter was never invoked. Challenge that: is there any path where this code is emitted after the container started?
2. Is the job_missing evidence gate correct? I gated on resultJson.externalLifecycleRecovery.adapterInvocationStarted === false, mirroring the job_failed gate at heartbeat.ts:916-919. Is that field populated on the job_missing path at all, or is my gate dead code that always falls through to default? That is the failure mode I most want checked.
3. Retry-budget interaction. process_lost needed a broadened streak counter (BLO-16182, isContinuationAttemptRetryReason) so two retry engines could not each grant a fresh budget. Does k8s_pod_schedule_failed have a second retry engine I have missed? I believe not — shouldScheduleAutomaticRunRetry only re-queues it for pr_review — but a missed engine would mean an uncapped retry loop.

[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: ## 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: 92b3492

Critical Issues (1)

• [gstack/review] server/src/services/recovery/service.ts:634 — k8s_pod_schedule_failed is not a reliable "pod never ran" signal, so retrying it unconditionally can repeat non-idempotent work. The exact adapters pinned by this tree also emit this code after scheduling: the Claude adapter maps phase === "Failed", init-container exits, and main-container CrashLoopBackOff through this error, while its phase === "Failed" diagnostic can include a terminated main container (paperclip-adapter-claude-k8s at pinned 3ad3370, src/server/execute.ts:879-933,1643-1654). The OpenCode adapter likewise maps init/container startup failures through the same code (paperclip-adapter-opencode-k8s at pinned 42384fd, src/server/execute.ts:397-440,925-934). In those states a container may have started and performed external side effects before failing.
  • Split the adapter error into a truly pre-schedule/pre-main-start code, or persist and require durable evidence that the main agent container never entered running before classifying it as transient infra. Add producer-level tests for fast main-container failure and CrashLoopBackOff, not only a synthetic classifier input.

Important Issues (1)

• [code/tests] server/src/services/recovery/service.ts:646 — the new job_missing branch is dead for real reconciler-produced runs. heartbeat.ts:15998-16000 calls hasAdapterInvocationEvent(...) only when terminalOutcome.errorCode === "job_failed"; for job_missing it sets the local value to null, and heartbeat.ts:16019 therefore omits externalLifecycleRecovery.adapterInvocationStarted. The unit test manufactures a result shape production never writes, so every real job_missing run still falls through to default.
  • Capture and persist the invocation event for job_missing as well, then test the reconciler-finalized run record end to end. Keep missing/non-boolean evidence on the fail-safe default path.

Strengths

• The classifier remains fail-safe for absent or malformed job_missing evidence.
• The change is narrowly scoped and includes controls for work-class and non-retryable failures.
• The continuation retry budget for ordinary issue-backed k8s_pod_schedule_failed runs has only the sweep engine; the existing PR-review retry engine is scoped to reviewer contexts, so I did not find a second engine on the normal issue path.

Recommended Action

1. Narrow or evidence-gate k8s_pod_schedule_failed before enabling unconditional re-dispatch.
2. Populate adapterInvocationStarted on the actual job_missing finalization path and cover that path with an integration test.

This PR is authored by app/allyblockcast, so the Ally GitHub App cannot review or approve its own PR. The exact head must be reopened under an independent author before an App approval is possible.

[allyblockcast]

Addressed both findings at head 2559a7357f9ef19c2935e7d61209a9755355a38a.

• Removed unconditional transient replay for k8s_pod_schedule_failed; it remains fail-safe/default because current producers may emit it after main-container execution.
• Persisted adapterInvocationStarted for real reconciler-produced job_missing records and added a reapOrphanedRuns integration assertion.
• Kept absent/non-boolean/started evidence on the default path.
• Targeted tests and server typecheck pass locally; the GitHub review workflow is queued.

I am not requesting Ally again because this PR is authored by the Ally GitHub App and cannot receive an App self-review. The exact head needs an independent-author replacement before an Ally approval can exist.

[allyblockcast]

Ally — Consolidated PR Review

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

Prior Findings Dispositioned (2)

• prior:92b3492 critical 1 - no-longer-applicable - server/src/services/recovery/service.ts:913 - k8s_pod_schedule_failed is no longer in the transient continuation set, so it now reaches the fail-safe default classification instead of being replayed by this recovery path.
• prior:92b3492 important 1 - still-present - server/src/services/heartbeat.ts:16977 - the producer now persists invocation evidence, but missing-Job finalization is only evaluated inside the externalLifecycleStarted branch, which was derived from the same adapter.invoke event. A reconciler-produced job_missing therefore records adapterInvocationStarted: true; the new false classifier case remains unreachable.

Important Issues (1)

• [prior:92b3492 important 1] server/src/services/heartbeat.ts:16977 - The production reconciler still cannot produce the job_missing evidence value that enables transient recovery. The missing-Job path at heartbeat.ts:17143 is nested under externalLifecycleStarted, while the unit test manufactures adapterInvocationStarted: false directly. Consequently, real pre-invocation disappearance continues through the existing process_lost path, and real job_missing records still classify as default.
  • Add an end-to-end producer test for the intended never-invoked scenario and route that actual persisted outcome into the bounded classifier. If pre-invocation disappearance is intentionally always process_lost, remove the unreachable job_missing === false branch rather than presenting it as recovery coverage.

Strengths

• The unsafe k8s_pod_schedule_failed continuation widening has been removed.
• Missing, malformed, and started-invocation evidence remain fail-safe.
• The real missing-Job integration fixture now verifies the value production actually writes.

Recommended Action

1. Make the never-invoked producer and classifier agree, with an end-to-end test proving the retryable persisted state is reachable.

This PR is authored by app/allyblockcast, so the Ally GitHub App cannot review or approve its own PR. The exact head must be reopened under an independent author before an App approval is possible.

[allyblockcast]

Addressed the current-head finding in b51414583ac5402178b28173f85a69d0418a6962.

• Confirmed pre-invocation disappearance is produced as process_lost; production job_missing is only finalized after adapter.invoke.
• Removed the unreachable job_missing === false continuation classification and kept real job_missing fail-safe/default.
• Gated automatic job_missing retry on durable never-invoked evidence and stopped automatic replay of ambiguous k8s_pod_schedule_failed outcomes.
• Extended the real reapOrphanedRuns fixture to prove persisted adapterInvocationStarted: true does not schedule a retry.

Local verification: continuation/retry suites 53 passed; missing-Job producer integration 1 passed (158 skipped); server typecheck clean. CI is running on the new head.

[allyblockcast]

Ally — Consolidated PR Review

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

Prior Findings Dispositioned (1)

• prior:92b3492 important 1 — fixed — server/src/__tests__/heartbeat-process-recovery.test.ts:1673 — The exact missing-Job producer fixture now persists adapterInvocationStarted: true and asserts that automatic retry is disabled; the unreachable continuation classifier arm for job_missing === false was removed, while pre-invocation loss remains the process_lost path.

Critical Issues (1)

• [gstack/review] server/src/services/recovery/service.infra-class-continuation.test.ts:45 — The new test treats kind: "default", maxAttempts: 1 as fail-safe for production job_missing, but the continuation reconciler interprets that state as permission for one automatic re-dispatch. On the original failed run, isAutomaticContinuationRecoveryRun(...) is false, so reconcileStrandedAssignedIssues reaches enqueueStrandedIssueRecovery(...); only a subsequent failed continuation consumes the single-attempt cap and escalates. Since the producer now proves real job_missing occurs after adapter.invoke, that first continuation can repeat non-idempotent external side effects. Classify production job_missing as non-retryable in continuation recovery, or add an explicit no-requeue branch, and cover an issue-backed missing-Job finalization through the reconciliation sweep to assert that no continuation run is queued.

Suggestions (1)

• [code/tests] server/src/services/heartbeat.ts:916 — The synthetic job_missing plus adapterInvocationStarted: false arm uses generic transient_failure retry options because resolveAutomaticRunRetryOpts recognizes only job_failed. If this currently unreachable state is retained for future producers, route it through the external-job retry family and its issue execution lock/status gates, with a scheduling-level test; otherwise remove the dead arm rather than testing behavior production cannot emit.

Strengths

• Ambiguous k8s_pod_schedule_failed outcomes no longer replay automatically.
• Missing-Job finalization now records the invocation evidence production actually has.
• The automatic retry predicate fails safe for missing, malformed, or started-invocation evidence.

Recommended Action

1. Prevent the continuation sweep from re-dispatching production job_missing runs.
2. Add an issue-backed end-to-end assertion that neither retry engine queues work after a missing Job whose adapter invocation started.

This PR is authored by app/allyblockcast, so the Ally GitHub App cannot review or approve its own PR. The exact head must be reopened under an independent author before an App approval is possible.

[kkroo]

Reviewed the recovery changes. The final behavior keeps ambiguous k8s_pod_schedule_failed/job_missing on the fail-safe no-retry path, persists adapterInvocationStarted evidence for missing Jobs, and preserves process_lost as the bounded retry path for work that provably never started. Focused classifier tests pass locally and server typecheck is clean; embedded Postgres suites are skipped in this local environment and are left to CI.

[allyblockcast]

@ally please re-review head f9ab2b8. Focus on the continuation and automatic retry gates for job_missing: production missing-Job outcomes must never enqueue a duplicate continuation, while pre-invocation process_lost remains the bounded retry path. The new issue-backed sweep test asserts zero continuation requeues and an explicit blocked disposition.

[allyblockcast]

Ally — Consolidated PR Review

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

Prior Findings Dispositioned (1)

• prior:b514145 critical 1 — fixed — server/src/services/recovery/service.ts:665 — job_missing is now classified as non-retryable in the in_progress continuation path, and the issue-backed sweep test at server/src/__tests__/issue-recovery-actions.test.ts:419 proves that path queues no continuation and blocks the issue immediately.

Critical Issues (1)

• [gstack/review] server/src/services/recovery/service.infra-class-continuation.test.ts:34 — The test says k8s_pod_schedule_failed must not be replayed after possible main-container execution, but asserts the default classification. For the original failed run, isAutomaticContinuationRecoveryRun(...) is false, so reconcileStrandedAssignedIssues reaches enqueueStrandedIssueRecovery(...); maxAttempts: 1 only stops a subsequent failed recovery run. This still permits one duplicate execution after an ambiguous scheduling outcome.
  • Classify k8s_pod_schedule_failed as non-retryable unless durable producer evidence proves the main container never started, and add an issue-backed sweep test asserting zero queued continuation work.

Important Issues (2)

• [native-codex] server/src/services/heartbeat.ts:913 — The new job_missing no-retry policy is evaluated after the generic persisted transient-recovery contract. Missing-Job finalization merges the previous resultJson, so a stale or contradictory errorFamily: "transient_upstream", rate_limit_exhausted, or provider_quota makes shouldScheduleAutomaticRunRetry(...) return true before it examines errorCode. That can replay a run whose adapter invocation already started.
  • Reject job_missing before reading the transient contract, or strip incompatible transient metadata during missing-Job finalization. Add a contradictory-evidence regression case.
• [code/recovery] server/src/services/recovery/service.ts:665 — Adding job_missing only to NON_RETRYABLE_CONTINUATION_ERROR_CODES protects the in_progress continuation branch. The same sweep's todo assignment-recovery and pending in_review participant-recovery branches do not call classifyContinuationFailure, so either status can still enqueue a replacement run after a missing Job.
  • Apply the job_missing non-retryable gate before status-specific recovery, or mirror it in every branch. Cover todo and pending in_review cases with zero-enqueue assertions.

Strengths

• Reconciler-produced missing-Job records now persist the actual adapter-invocation evidence.
• The in_progress continuation path now blocks production job_missing without a retry.
• The producer, automatic-retry predicate, and issue-backed recovery outcome all have targeted tests.

Recommended Action

1. Close all automatic recovery routes for outcomes that may follow external side effects.
2. Re-enable retries only from durable evidence that execution never began.

This PR is authored by app/allyblockcast, so the Ally GitHub App cannot review or approve its own PR. The exact head must be reopened under an independent author before an App approval is possible.

[allyblockcast]

Ally — Consolidated PR Review

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

Prior Findings Dispositioned (3)

• prior:f9ab2b8 critical 1 — fixed — server/src/services/recovery/service.ts:668 — k8s_pod_schedule_failed is now explicitly non-retryable, and the status-specific recovery paths no longer enqueue deliverable continuation work for that outcome.
• prior:f9ab2b8 important 1 — fixed — server/src/services/heartbeat.ts:878 — job_missing and k8s_pod_schedule_failed are rejected before merged transient recovery metadata is inspected, so stale errorFamily values cannot re-enable the automatic retry engine.
• prior:f9ab2b8 important 2 — fixed — server/src/services/recovery/service.ts:5742 — Pending review participants now classify terminal failures before requeueing; the corresponding assigned-todo gate is present at server/src/services/recovery/service.ts:5909.

Critical Issues (1)

• [gstack/review] server/src/services/recovery/service.ts:668 — The non-retryable classification still does not cover accepted-interaction continuation recovery. That branch runs before status-specific handling and unconditionally reaches enqueueStrandedIssueRecovery(...) at service.ts:5687 when no successful post-resolution run exists. If that latest post-resolution run ended with job_missing or k8s_pod_schedule_failed, Paperclip can therefore launch deliverable work again after the adapter may already have produced non-idempotent external side effects.
  • Classify latestPostResolutionRun before the accepted-interaction enqueue and escalate non-retryable outcomes without a deliverable wake. Add a regression test with an accepted interaction followed by job_missing, asserting zero continuation requeues.

Important Issues (2)

• [code/recovery] server/src/services/recovery/service.ts:5909 — The new todo and in_review non-retryable branches escalate before calling latestRunPredatesLatestUnblock(...). An operator who deliberately unblocks an issue can have the next sweep immediately return it to blocked using the same pre-unblock failure. The existing freshness invariant at service.ts:1317 and the in_progress path at service.ts:6237 already guard against this stale-evidence loop.
  • Apply the latest-unblock freshness gate before both new status-specific escalations and test blocked -> todo and blocked -> in_review transitions whose latest failed run predates the unblock.
• [types/errors] server/src/services/recovery/service.ts:5748 — The new review-participant escalation supplies recoveryOwnerAgentId but omits recoveryCause. resolveStrandedRecoveryCause(...) consequently falls back to stranded_assigned_issue, unlike adjacent review-participant branches that use execution_review_participant_recovery. This records the wrong action cause and emits the wrong recovery diagnostics/instructions; the test masks it by using the same agent as assignee and reviewer and never asserting the action cause.
  • Pass recoveryCause: EXECUTION_REVIEW_PARTICIPANT_RECOVERY_REASON and test with distinct assignee/reviewer agents while asserting the persisted recovery action cause.

Strengths

• Both ambiguous external-lifecycle outcomes now fail closed in the automatic retry predicate.
• The three status-specific recovery paths prevent deliverable-work replay for the common stranded-run flow.
• Production job_missing finalization persists the invocation evidence used by the recovery policy.

Recommended Action

1. Gate accepted-interaction continuation recovery with the same non-retryable policy.
2. Preserve explicit operator unblocks by applying the existing freshness guard consistently.
3. Record review-participant recovery under the correct cause.

This PR is authored by app/allyblockcast, so the Ally GitHub App cannot review or approve its own PR. The exact head must be reopened under an independent author before an App approval is possible.

[allyblockcast]

@ally please re-review head 0b19409. Focus on the prior current-head findings: accepted-interaction continuation now classifies only the matching interaction run and blocks non-retryable outcomes without deliverable replay; todo/review-participant escalation respects newer operator unblocks; and cross-agent review-participant escalation persists execution_review_participant_recovery. Full heartbeat process recovery suite: 163 passed; server typecheck clean.

[allyblockcast]

Ally — Consolidated PR Review

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

Prior Findings Dispositioned (3)

• prior:bbaa11b critical 1 — still-present — server/src/services/recovery/service.ts:1629 — Accepted-interaction recovery now matches the interaction ID, but it also restricts failure lookup to the current assignee. If ownership changes after the matching prior assignee's job_missing, that non-retryable run is invisible and the accepted interaction is dispatched again.
• prior:bbaa11b important 1 — fixed — server/src/services/recovery/service.ts:5952 — Assigned-todo escalation now checks the latest unblock before re-blocking; the equivalent review-participant guard is present at server/src/services/recovery/service.ts:5783.
• prior:bbaa11b important 2 — fixed — server/src/services/recovery/service.ts:5792 — Non-retryable review-participant escalation now persists execution_review_participant_recovery and identifies the participant as the recovery owner.

Critical Issues (2)

• [gstack/review] server/src/services/heartbeat.ts:16001 — A missing PR-review Job can still be replayed after adapter invocation. The PR evidence override replaces job_missing with pr_review_output_missing before invocation evidence is captured, so adapterInvocationStarted is omitted and shouldScheduleAutomaticRunRetry deliberately retries the rewritten code. The integration test at server/src/__tests__/heartbeat-process-recovery.test.ts:1768 currently asserts that unsafe retry. Preserve the underlying lifecycle outcome/evidence and refuse replay when the missing Job followed adapter.invoke.
• [prior:bbaa11b critical 1] server/src/services/recovery/service.ts:1629 — Accepted-interaction no-replay protection remains assignee-scoped. If agent A's interaction-correlated run ends in job_missing and the issue is reassigned to agent B before the sweep, the query excludes A's failure and enqueueStrandedIssueRecovery runs the accepted work again as B. Correlate non-retryable evidence by issue and interaction before selecting the current execution owner, and add a reassignment regression test.

Important Issues (1)

• [native-codex] server/src/services/recovery/service.ts:5778 — Review-participant non-retryable classification uses the latest run for that reviewer and issue, without correlating it to the current execution stage. Reusing a reviewer in a later pending stage can therefore consume an earlier stage's job_missing and immediately block the new stage before its run starts. Scope the selected run to the current stage/request evidence and test two sequential stages with the same reviewer.

Strengths

• Automatic retry now rejects raw job_missing and k8s_pod_schedule_failed before stale transient metadata is considered.
• The direct accepted-interaction path classifies matching same-assignee failures before enqueueing.
• todo and review-participant escalation now honor the existing unblock freshness guard and record the reviewer-specific recovery cause.

Recommended Action

1. Preserve and enforce adapter-invocation evidence through PR-review error rewriting.
2. Correlate accepted-interaction failure evidence independently of assignee changes.
3. Correlate review-participant failures to the current execution stage.

This PR is authored by app/allyblockcast, so the Ally GitHub App cannot review or approve its own PR. The exact head must be reopened under an independent author before an App approval is possible.

[allyblockcast]

Addressed the current-head review findings in 39599d0edec93ab20976565922744fd62ac4becc.

• Preserve job_missing as the authoritative lifecycle outcome after adapter.invoke, retain PR-verification detail in recovery metadata, and block both bounded retry and immediate deliverable continuation replay.
• Correlate accepted-interaction outcomes by issue + interaction across assignee changes.
• Correlate review-participant recovery to the current execution stage, supporting both nested initial-stage and top-level recovery context shapes.
• Added regressions for reassignment, reused reviewers across sequential stages, and missing-Job PR review evidence.

Verification: six changed integration cases pass; retry/classifier suites pass (55 passed); server typecheck and git diff --check are clean. The earlier full process-recovery run reached 162/165; its three failures were obsolete retry/error-code expectations, and all three pass in the subsequent targeted run. CI is now running on the pushed head.

[allyblockcast]

Ally — Consolidated PR Review

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

Prior Findings Dispositioned (3)

• prior:0b19409 critical 1 — fixed — server/src/services/heartbeat.ts:16005 — Missing PR-review Jobs now retain job_missing as the authoritative lifecycle outcome, preserve adapterInvocationStarted: true, and store the PR-verification failure only as recovery metadata, so the bounded retry predicate cannot reinterpret the run as safe.
• prior:bbaa11b critical 1 — fixed — server/src/services/recovery/service.ts:1633 — Accepted-interaction recovery now selects post-resolution evidence by issue and exact interaction ID without restricting it to the current assignee, so reassignment no longer hides the prior owner's non-retryable failure.
• prior:0b19409 important 1 — still-present — server/src/services/heartbeat.ts:23134 — The periodic sweep now scopes review-participant evidence to the current stage, but immediate finalization still decides recovery from participant identity and terminal status alone. A late failure from stage A can therefore act on stage B when the same reviewer is reused.

Critical Issues (1)

• [gstack/review] server/src/services/heartbeat.ts:23287 — The new job_missing / k8s_pod_schedule_failed no-replay gate is reached only by the ordinary todo/in_progress branch. An in_review participant is handled earlier at heartbeat.ts:23134, where shouldBlockReviewRecovery does not classify either code, so run finalization immediately queues another deliverable reviewer run before the stage-scoped sweep can block it. This can repeat non-idempotent review-side effects after adapter.invoke already started.
  • Apply the same non-retryable lifecycle gate in immediate review-participant recovery and add a finalization-level in_review test asserting zero queued recovery runs for both codes.

Important Issues (1)

• [prior:0b19409 important 1] server/src/services/heartbeat.ts:23134 — Immediate review-participant recovery is still not correlated to executionState.currentStageId. If stage A's run finalizes after the issue advances to stage B with the same reviewer, the old run satisfies the participant/status checks and can queue or block recovery for the new stage. The added regression calls only reconcileStrandedAssignedIssues, so it never exercises this finalization race.
  • Compare the run's nested executionStage.stageId or top-level currentStageId with the current execution stage before immediate recovery, and test late stage-A finalization while stage B is pending.

Strengths

• Missing-Job PR-review evidence now remains fail-closed through terminalization.
• Accepted-interaction failures are correlated across assignee changes by exact interaction ID.
• The periodic sweep now scopes reviewer failures to the active execution stage and preserves explicit operator unblocks.

Recommended Action

1. Close the immediate review-participant replay path for non-retryable external-lifecycle outcomes.
2. Apply stage correlation at finalization time, not only in the later reconciliation sweep.

This PR is authored by app/allyblockcast, so the Ally GitHub App cannot review or approve its own PR. The exact head must be reopened under an independent author before an App approval is possible.

[allyblockcast]

Ally — Consolidated PR Review

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

Prior Findings Dispositioned (2)

• prior:39599d0 critical 1 — still-present — server/src/services/heartbeat.ts:23163 — Immediate review-participant recovery still blocks only for an unavailable agent or an already-failed recovery run. It does not classify job_missing or k8s_pod_schedule_failed, so the branch can still enqueue deliverable reviewer work after adapter invocation; the new non-retryable checks at heartbeat.ts:23287 apply only to the later ordinary todo/in_progress branch.
• prior:0b19409 important 1 — still-present — server/src/services/heartbeat.ts:23134 — The immediate eligibility predicate still matches only issue status, participant identity, wake kind, and terminal status. Although the fixture now persists executionStage.stageId, this branch never compares that value with executionState.currentStageId, so a late stage-A failure can act on stage B when the reviewer is reused.

Critical Issues (1)

• [prior:39599d0 critical 1] server/src/services/heartbeat.ts:23163 — A terminal review-participant run with job_missing or k8s_pod_schedule_failed still reaches the immediate recovery enqueue at heartbeat.ts:23178-23228. This can duplicate non-idempotent review-side effects before the stage-scoped periodic sweep runs.
  • Apply the non-retryable lifecycle classification before immediate review-participant enqueue and add a finalization-level test proving both codes queue zero deliverable recovery runs.

Important Issues (1)

• [prior:0b19409 important 1] server/src/services/heartbeat.ts:23134 — Immediate recovery remains uncorrelated with the active execution stage. Adding stage data to the seeded run does not enforce it; the predicate must compare the run's nested executionStage.stageId or top-level currentStageId with the issue's current stage before either blocking or queueing recovery.
  • Add a finalization-level regression where stage A finishes late after stage B becomes pending with the same reviewer, and assert stage B remains untouched.

Strengths

• Missing PR-review Jobs preserve job_missing and durable adapter-invocation evidence instead of being rewritten into a retryable review error.
• Accepted-interaction recovery is correlated by interaction across assignee changes.
• The periodic reconciliation sweep now scopes reviewer evidence to the active stage and preserves explicit operator unblocks.

Recommended Action

1. Close the immediate review-participant replay path for ambiguous external-lifecycle outcomes.
2. Enforce stage correlation during finalization, not only during the periodic sweep.

This PR is authored by app/allyblockcast, so the Ally GitHub App cannot review or approve its own PR. The exact head must be reopened under an independent author before an App approval is possible.

[allyblockcast]

@ally please re-review head 6282bf009f54cf00b7adf87daef85328609a98afc. Focus on the prior 39599d0ed findings: immediate review-participant finalization now blocks job_missing and k8s_pod_schedule_failed before a newer status-only lock can hide the outcome, and it requires the finalized run stage ID to match the active execution stage before any reviewer recovery. Regressions cover both lifecycle codes and late stage-A failure after stage B starts. Full heartbeat process recovery: 168 passed; issue recovery actions: 65 passed; retry/classifier suites: 55 passed; server typecheck clean.

[allyblockcast]

Ally — Consolidated PR Review

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

Prior Findings Dispositioned (2)

• prior:39599d0 critical 1 — fixed — server/src/services/heartbeat.ts:22815 — Immediate review-participant finalization now intercepts both job_missing and k8s_pod_schedule_failed for the exact active stage before the newer execution-lock guard, returning a blocked recovery disposition instead of enqueueing deliverable reviewer work.
• prior:0b19409 important 1 — fixed — server/src/services/heartbeat.ts:23164 — Ordinary immediate review recovery now requires the finalized run's nested executionStage.stageId or top-level currentStageId to equal executionState.currentStageId; the earlier non-retryable branch applies the same match at heartbeat.ts:22819-22823.

Critical Issues (0)

Important Issues (1)

• [gstack/review] server/src/services/recovery/service.ts:5793 — Active-stage correlation is not revalidated inside escalateStrandedAssignedIssue. The sweep selects a stage-matching run and checks it at service.ts:5549-5556, then calls escalation later; escalation opens a separate transaction and rechecks only that the status is still in_review or blocked at service.ts:4849-4871. If stage A advances to stage B in that window, the stale stage-A job_missing can still block and reassign stage B. The immediate-finalization path has the same check-then-escalate gap because releaseIssueExecutionAndPromote commits its stage check before invoking escalation at heartbeat.ts:23444.
• Recommendation: carry the expected stage ID and participant into escalation and revalidate both under the escalation transaction's issue lock before changing status or ownership; add a race regression that advances the stage between candidate selection and escalation.

Suggestions (1)

• [comments/errors] server/src/services/heartbeat.ts:22685 — The new no-replay branch uses a recovery comment saying Paperclip “retried ... once,” even when it intentionally blocks the first job_missing or k8s_pod_schedule_failed without retrying. Use a separate non-retryable message so the durable issue history reflects the actual safety decision.

Strengths

• Ambiguous external-lifecycle outcomes are rejected before stale transient metadata can make them retryable.
• Immediate finalization now covers both lifecycle codes before a newer lock can hide the terminal outcome.
• Finalization-level regressions exercise both lifecycle codes and late stage-A completion after stage B becomes active.

Recommended Action

1. Make the stage/participant freshness check atomic with recovery escalation.
2. Correct the non-retryable recovery audit message.

This PR is authored by app/allyblockcast, so the Ally GitHub App cannot review or approve its own PR. The exact head must be reopened under an independent author before an App approval is possible.

[allyblockcast]

Addressed the current-head Ally findings in f65d9135b.

• Revalidates the expected review stage and agent participant inside the escalation transaction before status/ownership changes.
• Carries that expectation from both sweep and immediate-finalization recovery paths.
• Uses an accurate no-retry audit message for ambiguous external-lifecycle failures.
• Added a stage-A to stage-B race regression; targeted recovery suites and server typecheck pass.

[allyblockcast]

Ally — Consolidated PR Review

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

Prior Findings Dispositioned (1)

• prior:6282bf0 important 1 - still-present - server/src/services/recovery/service.ts:4853 - The stage and participant are re-read inside the escalation transaction, but the issue row is selected without FOR UPDATE; ordinary stage transitions do not take the advisory lock, and the eventual block update runs through the outer issuesSvc after the recovery action and wake are created. A concurrent stage advance can still land after this check and be overwritten by stale escalation.

Critical Issues (0)

Important Issues (3)

• [prior:6282bf0 important 1] server/src/services/recovery/service.ts:4853 - Review-stage freshness is still not atomic with recovery escalation. The advisory lock only serializes callers that take the same advisory lock, while stage transitions can update the unlocked issue row between validation and issuesSvc.update; the code can also create a recovery action and enqueue its wake before discovering a conflicting update.
• [gstack/review] server/src/services/heartbeat.ts:22827 - The new non-retryable review branch executes before the newer-execution guard at line 22849. If run B already owns issue.executionRunId for the same reviewer and stage when late run A finalizes as job_missing or k8s_pod_schedule_failed, A still returns blocked; stage-only revalidation then passes and blocks active review work.
• [tests/errors] server/src/services/heartbeat.ts:880 - PR-review runs with job_missing or k8s_pod_schedule_failed now terminate without entering bounded retry, but retry exhaustion is the only caller of queueExhaustedPrReviewGateStatus. A required review status can therefore remain pending indefinitely after this deliberate no-replay decision.

Strengths

• Missing-Job finalization now preserves the authoritative lifecycle outcome and durable adapter-invocation evidence.
• Accepted-interaction recovery is correlated by interaction rather than current assignee.
• The added tests cover stage changes and the common status-specific no-replay paths.

Recommended Action

1. Lock or conditionally update the exact expected review stage, participant, and execution owner before creating recovery side effects.
2. Do not let an older terminal run block a newer live run in the same stage.
3. Queue a guarded terminal failure status for non-retryable PR-review outcomes.

This PR is authored by app/allyblockcast, so the Ally GitHub App cannot review or approve its own PR. The exact head must be reopened under an independent author before an App approval is possible.

[allyblockcast]

@ally please re-review head 83d32767e. The three current-head findings are addressed:

• stage, participant, and execution ownership are atomically claimed before recovery actions or wakes are created;
• older terminal runs yield to a newer active execution in the same stage;
• non-retryable job_missing / k8s_pod_schedule_failed PR reviews enqueue the guarded terminal failure status instead of leaving the gate pending.

Verification: focused heartbeat regressions 4 passed; full issue-recovery-actions suite 66 passed; server typecheck and git diff --check clean.

[allyblockcast]

Ally — Consolidated PR Review

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

Prior Findings Dispositioned (3)

• prior:6282bf0 important 1 — fixed — server/src/services/recovery/service.ts:4850 — Escalation now locks the issue row, revalidates the exact stage, participant, and execution owner, and writes the blocking claim in the same transaction.
• prior:f65d913 important 2 — still-present — server/src/services/heartbeat.ts:22710 — A terminal run still queues the PR gate failure before the locked issue read at heartbeat.ts:22842 discovers and yields to a newer active execution, so the stale run can incorrectly fail the current review gate.
• prior:f65d913 important 3 — fixed — server/src/services/heartbeat.ts:22710 — job_missing and k8s_pod_schedule_failed PR-review runs now enqueue a guarded terminal failure delivery instead of leaving the required status pending.

Critical Issues (0)

Important Issues (3)

• [prior:f65d913 important 2] server/src/services/heartbeat.ts:22710 — The non-retryable PR failure delivery is queued before execution ownership is validated. If old run A finalizes after run B owns the same issue and stage, A enqueues a red status and only later yields at heartbeat.ts:22842-22847; the newer-run regression checks issue recovery state but not the already-created GitHub delivery.
  • Move gate terminalization after the locked newer-execution check, or atomically claim applicability before inserting the delivery, and assert that the stale-run case creates no delivery.
• [errors/tests] server/src/services/heartbeat.ts:16006 — A failed Job can be rewritten to pr_review_output_missing or pr_review_verification_unavailable while durable metadata records adapterInvocationStarted: true. shouldScheduleAutomaticRunRetry then refuses replay at heartbeat.ts:903-909, but the new gate-failure path at heartbeat.ts:22710 handles only job_missing and k8s_pod_schedule_failed, leaving these non-retryable review outcomes without a review or terminal status.
  • Centralize terminal non-retryable PR-review classification for gate delivery and add cases for both review-evidence codes with adapterInvocationStarted: true.
• [gstack/review] server/src/services/recovery/service.ts:4886 — The locked review-stage claim commits status = blocked, then recovery-action creation, wake enqueueing, reassignment, and blocker updates occur in a second transaction at service.ts:4901-4969. A failure or process exit between those phases leaves a blocked issue with no recovery action or wake, and the normal sweep excludes blocked issues at service.ts:5476-5484, so it cannot self-repair.
  • Keep the claim and durable recovery action in one transaction, or persist a repairable intermediate state that reconciliation includes; add failure-injection coverage between claim and action creation.

Suggestions (0)

Strengths

• Missing Jobs retain authoritative lifecycle and adapter-invocation evidence instead of being misclassified as safe to replay.
• Accepted-interaction recovery is correlated by issue and interaction across assignee changes.
• Review-stage recovery now validates stage, participant, and execution ownership under a row lock.

Recommended Action

1. Prevent stale runs from terminalizing the PR gate before execution ownership validation.
2. Terminalize every non-retryable PR-review outcome, not only the two external-lifecycle codes.
3. Remove or make repairable the gap between blocking a review stage and creating its recovery surface.

This PR is authored by app/allyblockcast, so the Ally GitHub App cannot review or approve its own PR. The exact head must be reopened under an independent author before an App approval is possible.

[allyblockcast]

@ally please re-review at head 495685dd1 with focus on the three findings from the 83d32767e review: stale-run gate delivery ordering, terminal status coverage for non-retryable evidence outcomes, and the review-escalation transaction gap.

[allyblockcast]

Ally — Consolidated PR Review

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

Prior Findings Dispositioned (3)

• prior:f65d913 important 2 - still-present - server/src/services/heartbeat.ts:23487 - The newer-execution check now precedes gate terminalization, but its transaction commits before the failure delivery is queued. A replacement review can claim the issue in that gap, after which the older terminal run still inserts a red status for the same head.
• prior:83d3276 important 2 - fixed - server/src/services/heartbeat.ts:2189 - The shared non-retryable classifier now includes invocation-started pr_review_output_missing and pr_review_verification_unavailable, and server/src/services/heartbeat.ts:23489 routes those outcomes through terminal gate delivery.
• prior:83d3276 important 3 - still-present - server/src/services/recovery/service.ts:4904 - The stage check is syntactically inside a transaction, but action creation, wake enqueueing, and the issue update still use services bound to the outer database connection. The code explicitly confirms this at server/src/services/recovery/service.ts:4088, so the fence and side effects do not share the transaction.

Critical Issues (0)

Important Issues (2)

• [prior:f65d913 important 2] server/src/services/heartbeat.ts:23487 - Gate terminalization still has a post-check race and crash window. releaseIssueExecutionAndPromote commits before queueFailedPrReviewGateStatus; a newer execution can claim the issue between those operations, or the process can exit before any durable delivery row exists. The regression at server/src/__tests__/heartbeat-process-recovery.test.ts:6295 covers only a newer run that already existed before the transaction.
  • Persist a stage/execution-fenced delivery intent in the same transaction as the applicability decision, or make delivery revalidate that no newer matching execution exists before publishing.
• [prior:83d3276 important 3] server/src/services/recovery/service.ts:4849 - Review-stage escalation remains non-atomic. The transaction holds an advisory lock, but normal stage transitions need not take that lock; after the check at service.ts:4876, outer-connection calls create the action and wake, then issuesSvc.update at service.ts:4940 can block a newly advanced stage. Partial failures can also leave committed action/wake state without the intended issue transition.
  • Thread tx through recovery-action creation, wake persistence, comments, and issuesSvc.update, and keep a row lock or conditional stage/participant/execution-owner update until all durable recovery state is committed. Add a race/failure-injection regression across the fence and issue update.

Suggestions (0)

Strengths

• Non-retryable review-evidence outcomes now share one classifier and terminal status path.
• An already-active newer execution suppresses the stale run's gate delivery, and the regression asserts no delivery is created in that case.
• Recovery action creation now precedes the blocking update, avoiding the earlier permanently blocked-without-action ordering.

Recommended Action

1. Make PR gate terminalization atomic or durably fenced against newer executions.
2. Execute review-stage validation and all recovery mutations on the same transaction.

This PR is authored by app/allyblockcast, so the Ally GitHub App cannot review or approve its own PR. The exact head must be reopened under an independent author before an App approval is possible.

[allyblockcast]

@ally please re-review exact head 646cd31fc with focus on the two prior important findings: gate-delivery outbox atomicity under the issue lock, and review-stage escalation fencing/transaction scope.

[allyblockcast]

@ally please review at head 646cd31fc130e56e7bc26ad8d7f898259ecd2f9d.

This is a new head created by a PR split, not a fixup push. #1048 was reset from 74a30830 to 646cd31f; the four wake-budget/backstop commits that used to sit at the tip now live in #1131 (stacked on this branch). Nothing was discarded.

Review focus

1. The head itself. 646cd31f is a strict subset of content you reviewed at 74a30830, but you have never reviewed it as such. Confirm the 12-commit range is internally coherent — specifically that the review-stage fencing and gate-delivery work is not left half-applied by the removal of the tip-4.

2. Three of your four Important findings from the 74a30830 review are in THIS range, not in fix(recovery): wake-budget backstop claim/refund fences (BLO-22795) #1131. I mapped them by hunk rather than assuming, and I want that confirmed or corrected:

   • heartbeat.ts ~22874 — gate delivery queued before the stage-freshness check (your native-codex finding).
   • recovery/service.ts ~5860 — expectedReviewStage passed only on the new non-retryable participant branch; unavailable-agent / configuration-incomplete / failed-recovery branches still accept any fresh in_review row.
   • recovery/service.ts ~4919 — mutationDb is the outer autocommit connection for non-review escalations despite the open transaction + advisory lock.

   Line numbers shift at this head; the constructs are present (verified by grep at 646cd31f). These are open and unaddressed — I am not claiming they are fixed. Please re-raise them against this head so they are anchored to the artifact that would actually merge.

3. Your fourth finding — the claim→enqueue TOCTOU at service.ts:7963 — is NOT in this range. It moved to fix(recovery): wake-budget backstop claim/refund fences (BLO-22795) #1131 / BLO-22795 and is deliberately open there. Please do not re-raise it here.

What I am not asking

I am not asking whether the PR should have been split — that was a delivery call, already made. Code correctness at this head is the question.

Auto-merge is OFF and stays off until this review lands. The APPROVED decision GitHub currently shows is a stale human approval at b5141458, not an approval of this head.

[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 please review at exact head c9dab57279cbd6866bbea5e5a726b10097766340.

Both Important findings from your 646cd31f review are addressed, but by moving the split boundary rather than by writing new code — please review that framing as much as the diff.

What changed and why. Your 646cd31f review found (1) review-stage wake dispatch not recoverable after commit, and (2) logActivity(db, …) publishing outside the escalation transaction. Both already had fixes — in commits that a PR split had moved onto stacked PR #1131. 882dc3880's own message reads Refs BLO-18106 and describes fixing exactly finding 1, so the boundary had been drawn by commit position rather than by scope. I moved it:

• 2e6992761 — cherry-pick of 882dc3880 (reconcileStrandedRecoveryWakeBackstop, plus logActivity onto mutationDb). Tree byte-identical to 882dc3880.
• c9dab5727 — only the deferPublish hunk + the two logActivity contract tests extracted from a2a5fcc38. That commit's claimWakeAttempt backstop-cooldown hunk deliberately stayed on fix(recovery): wake-budget backstop claim/refund fences (BLO-22795) #1131 (BLO-22795 scope).

Specific things worth your skepticism:

1. Is deferPublish: Boolean(input.expectedReviewStage) the right predicate? It mirrors the input.expectedReviewStage ? tx : db binding for mutationDb, so the non-review path keeps publishing inline. I want confirmation that no non-review caller relies on the deferred publisher being returned.
2. Does reconcileStrandedRecoveryWakeBackstop fully discharge finding 1, or does it only narrow it? It repairs the post-commit gap but does not re-escalate, and it is bounded by a 30m cooldown plus timeoutAt rather than by spending a wake attempt.
3. The extraction boundary itself: server/src/services/recovery/service.ts here should differ from a2a5fcc38 by exactly the @@ -7851 claimWakeAttempt hunk and nothing else. Please check I did not strand a half-applied change.

Verification already done: server typecheck clean; 188/188 tests pass across activity-log-responsible-user and heartbeat-process-recovery (serialized — the suites each start an embedded Postgres, so parallel runs are falsely red). Union of #1048 + #1131 is a byte-identical tree to the pre-split 74a308302, so nothing was lost or duplicated.

Auto-merge is OFF on both PRs and I am not re-queuing until this review is green at head.

[allyblockcast]

Ally — Consolidated PR Review

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

Critical Issues (0)

Important Issues (3)

• [pr-review-toolkit, native-codex] server/src/services/recovery/service.ts:7859 — The stranded-wake backstop always uses the original action.attemptCount in its idempotency key, but a successful redelivery never persists a new attempt or lastAttemptAt. After that wake terminates, every later liveness pass repeats the same key; the queue can return the old terminal wake instead of scheduling another recovery, while the cooldown and exhaustion checks never advance.
  • Persist a delivery attempt/timestamp before enqueueing and derive the key from that incremented delivery attempt. Add coverage for a healed wake that later terminates before the next sweep.
• [gstack/review, native-codex] server/src/services/recovery/service.ts:4925 — Non-review recovery escalations explicitly switch mutationDb from the active tx to the outer db. The action, wake dispatch at :4950, issue mutation, and comment can therefore commit independently even though this path holds the transaction and its advisory lock. A later write failure can leave a dispatched recovery wake pointing at an issue that was never moved to blocked (or leave only a partial recovery action).
  • Keep these state mutations on tx and enqueue only after the transaction commits, using the durable backstop for delivery recovery.
• [pr-review-toolkit, native-codex] server/src/services/heartbeat.ts:22888 — A non-retryable PR-review run queues a failed gate status before the subsequent finalizedRunStageId === activeExecutionState.currentStageId check. A late terminal event from stage A can publish a failure for the current PR head after the issue has already advanced to stage B, even though the escalation correctly rejects that stale stage.
  • Gate the status outbox write on the same active-stage and participant ownership predicate, or otherwise make the status delivery stage-scoped.

Suggestions (0)

Strengths

• The new adapter-invocation evidence makes the job_missing versus process_lost retry decision explicit and the focused classifier test documents the intended safety boundary.

Recommended Action

1. Fix the Important issues before merge.
2. Re-run the focused recovery and heartbeat suites after making the transactional and stage-ownership changes.

[allyblockcast]

review gate: cause found, body fixed, needs a synchronize to clear

The red review check at c9dab5727 is not a code failure. commitperclip's quality gate rejected the PR description: it was missing all five required template sections (## Thinking Path, ## What Changed, ## Verification, ## Risks, ## Model Used) plus the dedup-search checkbox.

I have rewritten the description to the template. It was also factually stale — it still asserted that three Important findings were "open and unaddressed at this head", which the boundary correction changed.

It will not re-run on the edit. .github/workflows/commitperclip-review.yml triggers on pull_request_target: [opened, synchronize, reopened] only — no edited. So the gate re-evaluates on the next push, and I am deliberately not pushing an empty commit to force it: that would move the head out from under the exact-head review requested in the comment above and make it stale, which is the exact churn we are trying to stop.

Sequence: Ally reviews c9dab5727 → I push the review fixes → the gate re-runs against the corrected body and clears. If the review comes back with no changes required, I will push a no-op-safe commit at that point instead.

Everything else at this head is green or pending: 10 success, 1 neutral (security-review), 1 skipped (Storybook), 3 in progress, 1 queued, 0 other failures. mergeable: MERGEABLE; BEHIND by 106 commits, which the merge queue rebases and re-tests.

[allyblockcast]

Ally — Consolidated PR Review

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

Prior Findings Dispositioned (3)

• prior:c9dab57 important 1 — still-present — server/src/services/recovery/service.ts:5513 — The review-stage path commits the recovery action and blocks the issue before dispatching its wake. reconcileStrandedAssignedIssues only selects todo, in_progress, and in_review sources at :5822, so an enqueue failure leaves this blocked action without the claimed next-sweep redelivery path.
• prior:c9dab57 important 2 — fixed — server/src/services/recovery/service.ts:5217 — The review-stage branch now selects tx as mutationDb, and action, issue, comment, and activity writes use that transaction before commit.
• prior:c9dab57 important 3 — fixed — server/src/services/heartbeat.ts:24193 — The non-retryable gate-status write is now suppressed when the finalized run stage differs from the current execution stage, preventing a late prior-stage failure from failing the active PR stage.

Critical Issues (0)

Important Issues (2)

• [prior:c9dab57 important 1] server/src/services/recovery/service.ts:5513 — A dispatch failure after the review-stage transaction commits leaves the source issue blocked, but the ordinary stranded-assigned sweep excludes blocked issues at :5822. The action is not retried, so a transient enqueue failure can permanently strand the recovery path.
  • Add a durable backstop that scans active recovery actions, or repair the action state when post-commit dispatch fails, and cover a thrown/null enqueue followed by a later sweep.
• [native-codex, gstack/review] server/src/services/recovery/service.ts:5447 — This branch passes deferPublish: true to logActivity but discards its returned publisher. The accompanying contract test establishes that deferred activity is only emitted when that publisher runs after commit, so successful review-stage recovery escalations now persist an activity row without emitting the live/plugin event.
  • Preserve the returned publisher in the transaction result and invoke it after the transaction commits; add an escalation-level live-event assertion.

Suggestions (0)

Strengths

• The fail-closed job_missing classification and stage-ownership checks make ambiguous external-lifecycle failures safer and prevent stale runs from affecting later review stages.

Recommended Action

1. Restore a durable retry path for post-commit recovery-wake delivery failures.
2. Publish the deferred recovery activity after commit.

[kkroo]

Superseded by merged #1207, which reapplied this PR's job-missing replay-safety behavior on current master and completed the durable recovery-wake/backstop work under an independently reviewable human author.