From ea3a5ea7d25f0842d00165c2590d3ac4ba89c595 Mon Sep 17 00:00:00 2001 From: Opaque <108089985+0xopaque@users.noreply.github.com> Date: Sat, 15 Aug 2026 14:21:34 +1000 Subject: [PATCH] fix(recovery): skip successful-run handoff for recovery-action-driven runs (#9010) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The recovery subsystem repairs issues stranded without a valid disposition: `decideSuccessfulRunHandoff` queues one corrective wake per successful-but-dispositionless run, and `source_scoped_recovery_action` wakes a recovery owner for stranded issues > - `decideSuccessfulRunHandoff` already refuses to treat corrective handoff runs, issue-monitor runs, and comment-driven wakes as handoff *sources* — but not runs woken by `source_scoped_recovery_action` > - Because the handoff idempotency key includes `sourceRunId`, every succeeding recovery run is a brand-new source: recovery run → handoff wake → corrective run → new recovery action → recovery run → … with `DEFAULT_MAX_SUCCESSFUL_RUN_HANDOFF_ATTEMPTS` never binding (it is per-source-run) and the source-scoped recovery action created with `maxAttempts: null` > - The cycle is unbounded, each leg is a ~15s no-op "succeeded" run, and the designed handoff-exhausted escalation (blocked + exhausted notice) never engages > - This PR adds recovery-action-driven runs to the existing skip list, so recovery runs own their own follow-up path and the stranded-issue escalation remains the exit when the disposition is still missing > - The benefit is that missing-disposition recovery converges (one handoff, then escalation) instead of ping-ponging wake volume unboundedly ## Linked Issues or Issue Description Refs #6523 — same wake-loop family (repeated `source_scoped_recovery_action` wakes); this PR fixes the variant where the loop partner is the successful-run handoff. **Observed behavior:** in a 16-agent deployment, one agent produced 223 runs in 2 hours, every run `succeeded` with ~15s duration, with `contextSnapshot.wakeReason` alternating exactly between `source_scoped_recovery_action` (109) and `finish_successful_run_handoff` (108). The source issue never reached the exhausted escalation. ## What Changed - `server/src/services/recovery/successful-run-handoff.ts`: new `isRecoveryActionDrivenRun` predicate (matches `contextSnapshot.wakeReason === "source_scoped_recovery_action"` or a present `contextSnapshot.recoveryActionId`), consulted in `decideSuccessfulRunHandoff` alongside the existing corrective-handoff / issue-monitor / comment-driven skip guards. - `server/src/services/recovery/successful-run-handoff.test.ts`: cases asserting recovery-driven runs are skipped via both markers. ## Verification - `pnpm -F @paperclipai/server exec vitest run src/services/recovery/successful-run-handoff.test.ts` → 17 passed (16 existing unchanged + 1 new). - Production validation (same logic deployed as a dist patch on 2026.626.0): the alternating recovery/handoff wake pattern stopped after restart; ordinary successful-run handoffs (first corrective wake per genuine source run) continue to queue. ## Risks Low-to-moderate, scoped to one decision function. The behavioral shift: a recovery-action run that succeeds without fixing the disposition no longer gets a corrective handoff wake — instead the stranded-issue detector escalates (blocked + recovery owner + exhausted notice), which per the existing `escalateStrandedAssignedIssue` code is the designed terminal path. Runs not woken by a recovery action are unaffected (covered by the existing 16 tests, all green). ## Model Used Claude Fable 5 (`claude-fable-5`), extended thinking, agentic tool use via Claude Code. Human-reviewed before submission. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --- .../recovery/successful-run-handoff.test.ts | 30 +++++++++++++++++++ .../recovery/successful-run-handoff.ts | 14 +++++++++ 2 files changed, 44 insertions(+) diff --git a/server/src/services/recovery/successful-run-handoff.test.ts b/server/src/services/recovery/successful-run-handoff.test.ts index 5aa042b041..552ec84c8e 100644 --- a/server/src/services/recovery/successful-run-handoff.test.ts +++ b/server/src/services/recovery/successful-run-handoff.test.ts @@ -207,6 +207,36 @@ describe("successful run handoff decision", () => { expect(instruction).not.toMatch(/[\u0000-\u0008\u000B-\u001F\u007F]/); }); + it("does not queue for a run woken by source_scoped_recovery_action", () => { + expect( + decide({ + run: { + ...run, + contextSnapshot: { + issueId: "issue-1", + wakeReason: "source_scoped_recovery_action", + }, + } as any, + }), + ).toEqual({ + kind: "skip", + reason: "recovery action run owns its own follow-up path", + }); + // the recoveryActionId marker alone is also enough (payloads carry it even + // when wakeReason is rewritten downstream) + expect( + decide({ + run: { + ...run, + contextSnapshot: { issueId: "issue-1", recoveryActionId: "recovery-action-1" }, + } as any, + }), + ).toEqual({ + kind: "skip", + reason: "recovery action run owns its own follow-up path", + }); + }); + it("does not queue when the issue already has a valid disposition", () => { expect(decide({ issue: { ...issue, status: "done" } as any })).toEqual({ kind: "skip", diff --git a/server/src/services/recovery/successful-run-handoff.ts b/server/src/services/recovery/successful-run-handoff.ts index e6a3533586..fa8bc04a1f 100644 --- a/server/src/services/recovery/successful-run-handoff.ts +++ b/server/src/services/recovery/successful-run-handoff.ts @@ -304,6 +304,19 @@ function isCorrectiveHandoffRun(run: HeartbeatRunRow) { readString(context.wakeReason) === FINISH_SUCCESSFUL_RUN_HANDOFF_REASON; } +// A run woken by source_scoped_recovery_action must not become the source of another +// successful-run handoff. The handoff idempotency key includes sourceRunId, so every +// succeeding recovery run mints a fresh handoff wake: recovery run → handoff wake → +// corrective run → new recovery action → recovery run → …, an unbounded ping-pong that +// never reaches the handoff-exhausted escalation. Recovery runs own their own follow-up +// path; if the disposition is still missing, the stranded-issue escalation (blocked + +// exhausted notice) is the designed exit, not another handoff. +function isRecoveryActionDrivenRun(run: HeartbeatRunRow) { + const context = readRecord(run.contextSnapshot); + return readString(context.wakeReason) === "source_scoped_recovery_action" || + readString(context.recoveryActionId) !== null; +} + function isIssueMonitorMaintenanceRun(run: HeartbeatRunRow) { const context = readRecord(run.contextSnapshot); const wakeReason = readString(context.wakeReason); @@ -423,6 +436,7 @@ export function decideSuccessfulRunHandoff(input: { if (run.status !== "succeeded") return { kind: "skip", reason: "source run did not succeed" }; if (isCorrectiveHandoffRun(run)) return { kind: "skip", reason: "source run is already a corrective handoff run" }; + if (isRecoveryActionDrivenRun(run)) return { kind: "skip", reason: "recovery action run owns its own follow-up path" }; if (isIssueMonitorMaintenanceRun(run)) return { kind: "skip", reason: "issue monitor run owns its own recovery path" }; if (isCommentDrivenWake(run)) return { kind: "skip", reason: "comment-driven wake already owns the next action" }; if (run.issueCommentStatus === "retry_queued" || run.issueCommentStatus === "retry_exhausted") {