fix(recovery): skip successful-run handoff for recovery-action-driven runs (#9010)
## 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
This commit is contained in:
parent
57edb26db4
commit
ea3a5ea7d2
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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") {
|
||||
|
|
|
|||
Loading…
Reference in New Issue