fix(recovery): leave a receipt when a routine execution is cancelled (BLO-27572)

[allyblockcast]

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work
• Scheduled routines dispatch execution issues; each window is supposed to leave a receipt on the routine's alert surface, so that silence on that surface always means failure rather than health
• BLO-21020 found the opposite: the agent-health routine went dark for ~17.5h and nothing said so, because a window that fails before user code runs cannot fire the runbook's own pre-flight heartbeat
• BLO-21395 added a scheduler-side receipt for that case, and BLO-24543 (fix(recovery): key scheduler failure heartbeat off receipt absence (BLO-24543) #1341) fixed its predicate to key off receipt absence rather than an activity proxy
• Both landed with exactly one call site — inside the stranded-issue recovery sweep — so retiring a window by cancelling its execution issue still produced silence, which the CEO had flagged on 2026-08-06 as "a second way to manufacture silence"
• This pull request extracts the heartbeat to a shared leaf module and fires it on the ordinary cancellation transition too, sharing the idempotency key so strand-then-cancel stays one row
• The benefit is that a routine window now leaves a receipt however it stops being live, closing the last of the three BLO-21395 pieces

Linked Issues or Issue Description

• Fixes: BLO-27572
• Refs BLO-21395 (parent), BLO-24543 / fix(recovery): key scheduler failure heartbeat off receipt absence (BLO-24543) #1341 (the receipt-absence predicate this builds on), BLO-21020 (originating incident)
• Related but distinct: fix(scheduler): stop dispatching runs against cancelled issues (BLO-23206) #1373 (stop dispatching runs against cancelled issues) touches the dispatch side of cancelled routine executions; it does not overlap with the receipt path changed here.

What Changed

• New server/src/services/recovery/routine-scheduler-heartbeat.ts — postRoutineSchedulerFailureHeartbeat moved out of the recovery service closure. It takes db, addComment and logger as injected dependencies and a disposition discriminated union (stranded | cancelled). Taking addComment as a port is what keeps it a leaf: the dependency direction is recovery/service.ts → issues.ts → recovery/origins.ts, so importing ../issues.js here would close a cycle.
• services/issues.ts — update now records a routine-execution → cancelled transition under the row lock and posts the receipt after the transaction commits, on db rather than the caller's tx. Mirrors the existing strand-time call site: the receipt targets a different issue, so an in-transaction failure would roll back a cancellation that is otherwise correct and already decided.
• services/issues.ts — new suppressRoutineSchedulerFailureHeartbeat option on update.
• services/recovery/service.ts — inline copy deleted; strand-time call site now uses the shared module. cancelDuplicateSuppressedRoutineExecutionIssue passes the suppression flag explicitly, because that window is not dark — another open execution issue owns the dispatch lock and is doing the work, so a receipt would be a false alarm.
• Both paths share buildSchedulerFailureHeartbeatKey({routineId, windowKey}), so strand-then-cancel collapses to one row via the existing onConflictDoNothing on comment idempotency keys.
• 4 new tests in issue-recovery-actions.test.ts.

Two small type fixes were needed and are called out in comments rather than left as mystery casts: isRoutineExecutionDuplicateSuppressedRun is a type predicate, so its negative branch narrows input.latestRun to null — the unnarrowed value is captured before the guard. And the deferred receipt is read through an accessor because control-flow analysis still has the variable narrowed to null at the read site.

Verification

npx vitest run server/src/__tests__/issue-recovery-actions.test.ts    # 87 passed (87)

The four new cases mirror the four acceptance criteria: cancel-with-no-receipt emits one; cancel-with-normal-receipt emits none; duplicate-suppressed cancel emits none; strand-then-cancel emits exactly one total. The three pre-existing BLO-21395 / BLO-24543 tests pass unchanged, which is the control proving the extraction did not alter strand-time behaviour.

Both negative-space claims were proven non-vacuous rather than assumed — two of the four assert an absence, which would pass against a no-op:

sabotage	test	result
hook disabled	...cancelled through the ordinary issue-update path	fails — expected [] to have a length of 1 but got +0
suppression flag flipped to false	...duplicate-suppressed routine execution is cancelled	fails — expected [ Array(1) ] to have a length of +0 but got 1

The second also confirms the duplicate-suppressed cancel genuinely flows through the new hook and would raise a false alarm unsuppressed — i.e. the flag is load-bearing, not decorative.

Wider runs:

• routines-service / routines-routes / routines-e2e / routine-run-telemetry / issue-comment-cancel-routes / issue-list-routine-executions-routes / issue-update-comment-wakeup-routes — 111 passed (111)
• 45 issue-* suites — 952/953. The one failure is issue-create-pr-review-duplicate-routes.test.ts dying on a Postgres 40P01 deadlock in the teardown truncate ... cascade against a concurrent test file; it passes 30/30 in isolation, is on the issue-create path, and is unrelated to this change.
• npx tsc --noEmit -p server/tsconfig.json — clean for all files touched here. Pre-existing errors remain in better-auth.ts, plugin-loader.ts, plugin-registry.ts, successful-run-handoff-state.ts, none of which this PR modifies.
• node scripts/check-test-undefined-symbols.mjs — exit 0.

Risks

Low-to-moderate, concentrated in one place: issueService's return { ... } became const service = { ... }; return service; so the factory can bind the addComment port. That is mechanically equivalent and type-checked, but it touches the shared entry point for every issue mutation, which is why the wider suites above were run rather than just the targeted file.

• New comments on routine alert surfaces. Any routine-execution cancellation whose window has no agent-health:<window>:* receipt now posts one row. Intended, and bounded to one row per (routine, window) by the shared idempotency key.
• Receipt visible before a window is unretriable. Deliberate and unchanged from the strand-time design: a "stranded, then recovered" pair is correct, and per the CEO's 2026-08-10 amendment the "never both" clause was withdrawn. Scheduler receipts do not count toward windowCoverage7d.
• Post-commit, non-atomic. If the process dies between commit and post, the cancellation stands with no receipt. Accepted for the same reason the strand-time path accepts it — the alternative rolls back correct decisions on a cross-issue write failure. The helper swallows its own errors.
• Suppression is caller-driven. A human who manually cancels a duplicate-suppressed execution issue (rather than letting recovery do it) would emit a receipt the lock owner makes unnecessary. Noted rather than papered over; a durable run-derived predicate would fix it and is not in this PR's scope.
• No migration, no schema change, no threshold retuned.

Model Used

• Claude Opus 4.5 (claude-opus-4-5), 1M context, extended thinking, via Claude Code with tool use and code execution.

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, server-side only
• I have updated relevant documentation to reflect my changes — rationale is carried in code comments next to each decision, per the convention in this subsystem
• I have considered and documented any risks above
• All Paperclip CI gates are green — pending first CI run
• Greptile is 5/5 with no open P2s, recommendations, or follow-ups — pending review
• I will address all Greptile and reviewer comments before requesting merge

[allyblockcast]

🔗 Paperclip issue: BLO-27572
🔗 Paperclip issue: BLO-24543
🔗 Paperclip issue: BLO-21020
🔗 Paperclip issue: BLO-21395

[allyblockcast]

🔗 Paperclip issue: BLO-27572
🔗 Paperclip issue: BLO-24543
🔗 Paperclip issue: BLO-21020
🔗 Paperclip issue: BLO-21395

[allyblockcast]

@ally please review at head 31ad236c6.

Review focus, in priority order:

1. Cycle safety of the extraction. services/recovery/routine-scheduler-heartbeat.ts is imported by both issues.ts and recovery/service.ts. It takes addComment as an injected port specifically to avoid import ... from "../issues.js", which would close issues -> recovery/routine-scheduler-heartbeat -> issues. Please confirm the module is genuinely a leaf and that the port type has not drifted from the real addComment signature (it is applied with an as cast at the bind site in issueService).

2. The issueService factory restructure. return { ... } became const service = { ... }; return service; plus a let addCommentPort! bound immediately after. This is the highest-blast-radius line in the diff — it is the shared entry point for every issue mutation. Is the definite-assignment assertion safe here? My reasoning: addCommentPort is read only from inside update, which cannot run until the factory has returned and the bind has executed.

3. Post-commit ordering. The cancel-time receipt fires after db.transaction(runUpdate) resolves, on db rather than the caller's dbOrTx, mirroring the strand-time call site. When update is handed an outer tx that later rolls back, we would have posted a receipt for a cancellation that did not happen. I judged that acceptable (idempotent key, receipt is a timestamped observation, and the same exposure already exists strand-side) but would like a second opinion on whether it should instead defer to the outermost caller.

4. Suppression completeness. suppressRoutineSchedulerFailureHeartbeat is caller-driven, set only by cancelDuplicateSuppressedRoutineExecutionIssue. A human manually cancelling a duplicate-suppressed execution issue would therefore emit a receipt the lock owner makes unnecessary. Is a durable run-derived predicate worth doing now, or correctly deferred?

Both negative-space tests were proven non-vacuous by deliberate sabotage runs (documented in the PR body) — please sanity-check that the two absence-asserting tests really would catch a regression, since those are the ones that fail silently.