fix(recovery): treat routine duplicate-suppression as terminal-benign, not a strand (BLO-19954)

[allyblockcast]

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work
• Its recovery-action pipeline (server/src/services/recovery/service.ts) is what decides whether a stalled/cancelled agent run is a genuine failure needing a human/owner wake, versus benign noise
• Routine 4756349d ("Scan for open PR unreviewed HEADs") is concurrencyPolicy=always_enqueue under a single-owner dispatch lock, so every scheduled fire creates a routine-execution issue whose run is intentionally cancelled (routine_execution_duplicate_suppressed) unless it happens to be the lock owner — the cancellation message itself says "the owner run will continue the work"
• resolveStrandedRecoveryCause had no case for that error code, so it fell through to the generic stranded_assigned_issue cause, and escalateStrandedAssignedIssue flipped the issue to blocked, created a recovery action, and woke an owner (CTO, then CEO on repeat) — every single routine fire
• That converted a correctly-working routine into an hourly wake amplifier: 8+ CEO recovery wakes recorded against a routine whose actual work (PR review fan-out) was never broken
• This pull request short-circuits that classification: a duplicate-suppressed cancellation is treated as terminal-benign — the issue is cancelled directly, with no recovery action and no owner wake
• The benefit is the routine keeps firing and completing its real work without manufacturing a recovery escalation (and a wasted CTO/CEO context-switch) on every single non-owner tick

Linked Issues or Issue Description

Fixes: Paperclip issue BLO-19954 — "Routine duplicate-suppression is misclassified as a strand, manufacturing hourly CEO recovery wakes" (filed by the CEO; tracked in Paperclip, not GitHub Issues, since this repo's issue tracking for this org lives in Paperclip).

What Changed

• Added isRoutineExecutionDuplicateSuppressedRun and cancelDuplicateSuppressedRoutineExecutionIssue in server/src/services/recovery/service.ts.
• escalateStrandedAssignedIssue now short-circuits at the top: when the latest run is cancelled with errorCode: routine_execution_duplicate_suppressed, it cancels the routine-execution issue directly (advisory-lock transaction, fresh re-read, explanatory system comment) instead of escalating to blocked with a recovery action + wake_owner.
• All other causes (adapter_failed, process_lost, workspace faults, provider quota, etc.) are untouched — they don't carry this error code, so genuine strands still escalate exactly as before.
• Added a test in server/src/__tests__/issue-recovery-actions.test.ts asserting this cause yields zero recovery actions + status: cancelled + no wake, placed directly next to (and paired with) the existing adapter_failed test that still asserts one recovery action + blocked.

Verification

• npx vitest run server/src/__tests__/issue-recovery-actions.test.ts — 60/60 passed (includes the new paired test).
• npx vitest run server/src/__tests__/heartbeat-process-recovery.test.ts -t "cancels a queued stale routine duplicate" — unaffected, still passes (this exercises the run-cancellation gate itself, upstream of this fix).
• tsc --noEmit (server package) — clean.
• Live data check via paperclipListIssues(originId=4756349d-b78d-4e66-9006-68826a30f885): all 61 routine-execution issues for this routine are currently terminal (31 done, 30 cancelled, zero blocked/todo/in_progress) — confirms the backlog named in the filing (BLO-18745, BLO-18750) was already cleared and this PR is the prospective code fix.
• Post-deploy: re-check the same query 24h out — done executions should keep accruing at the routine's fire rate with zero new blocked issues carrying an active stranded_assigned_issue recovery action for this routine.

Risks

• Low risk, narrowly scoped: the new code path only activates for the single, specific error code routine_execution_duplicate_suppressed, which is set in exactly one place (cancelClaimedRunForRoutineExecutionDuplicate in heartbeat.ts) and only for routine_execution-origin issues under a dispatch-lock conflict.
• Behavioral shift: a routine-execution issue that previously would have sat blocked (visible on an owner's board, requiring manual dismissal) now auto-cancels with an explanatory comment instead. This is the intended fix — the blocked state was never actionable (there is nothing to repair; the lock owner already has the work) — but it does mean these issues disappear from active queues faster than before.
• No schema/migration changes. No changes to the run-cancellation gate itself (heartbeat.ts), only to how the recovery sweep classifies its aftermath.

Model Used

Claude Sonnet 5 (claude-sonnet-5[1m], 1M context), running as the Paperclip PlatformSREEngineer agent via Claude Code / the Claude Agent SDK. No extended-thinking mode; standard tool use (Read/Edit/Bash/vitest/tsc) to investigate, implement, and verify.

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 (none found)
• 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 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 (pending CI run on this push)
• Greptile is 5/5 with no open P2s, recommendations, or follow-ups (pending)
• I will address all Greptile and reviewer comments before requesting merge

🤖 Generated with Claude Code

[allyblockcast]

🔗 Paperclip issue: BLO-19954
🔗 Paperclip issue: BLO-18745
🔗 Paperclip issue: BLO-18750

[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
• No linked issue or inline issue description found — either tag an existing issue with Fixes #NNN / Closes #NNN / Refs #NNN, or describe the underlying issue inline in the PR body following one of our issue templates (https://github.com/paperclipai/paperclip/tree/master/.github/ISSUE_TEMPLATE). See CONTRIBUTING.md → "Link Issues or Describe Them In-PR".
• 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]

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
• No linked issue or inline issue description found — either tag an existing issue with Fixes #NNN / Closes #NNN / Refs #NNN, or describe the underlying issue inline in the PR body following one of our issue templates (https://github.com/paperclipai/paperclip/tree/master/.github/ISSUE_TEMPLATE). See CONTRIBUTING.md → "Link Issues or Describe Them In-PR".
• 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: 711c69c

Important Issues (2)

• [gstack/review + native-codex] server/src/services/recovery/service.ts:4783 — The terminal cancellation can overwrite a newly recovered issue using stale run evidence. The duplicate-suppressed classification happens before the advisory lock; after acquiring it, this path only checks that the issue is nonterminal. Unlike the existing escalation path's expected-status guard, it does not require fresh.status === input.previousStatus, re-confirm that latestRun.id is still the latest run, or reject a newly established execution path. A concurrent dispatch or manual transition to todo, in_review, or blocked can therefore occur after the sweep's initial read and then be replaced with cancelled, while issuesSvc.update also clears the live execution lock.
• Recommendation: Revalidate the expected status and current run/execution evidence under the lock before cancelling. Add a race test where the issue status or latest run changes between candidate selection and the locked re-read and assert that cancellation becomes a no-op.
• [pr-review-toolkit + gstack/review + native-codex] server/src/services/recovery/service.ts:4788 — The activity row and live/plugin event are written through the outer db while the issue update and comment use tx. logActivity explicitly documents that transaction callers must pass the transaction with deferPublish; here consumers can observe issue.updated before the cancellation commits, and a later comment or commit failure can roll back the issue while leaving a committed phantom cancellation activity.
• Recommendation: Call logActivity(tx, ..., { deferPublish: true }), return the publisher from the transaction, and invoke it only after commit. Add a rollback test proving neither the activity row nor event survives when the transactional comment/update fails.

Strengths

• The dedicated error-code predicate keeps the intended benign path narrow, and the paired test verifies the basic no-recovery-action/no-wake behavior.
• The transaction and per-issue advisory lock provide the right serialization point; the missing steps are revalidating the decision inputs and keeping activity publication atomic with the state change.

Recommended Action

1. Address both Important issues before merge.
2. Re-run the focused recovery tests after adding concurrency and rollback coverage.

Because this PR is authored by app/allyblockcast, the Ally GitHub App cannot review its own PR. The exact head must be reopened under an independent author before an App approval satisfying review/ally-complete is possible.

[kkroo]

Approved: duplicate-suppressed routine runs are explicitly terminal-benign, with regression coverage for no recovery action or owner wake.