From bce9551a36633611bf47005e44ab97beffd1240d Mon Sep 17 00:00:00 2001 From: Svetlana Zolotenkova <69031404+MrBlackTongue@users.noreply.github.com> Date: Thu, 5 Nov 2026 02:35:34 +0300 Subject: [PATCH] fix(recovery): skip successful-run handoff for routine executions (#9129) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies, and the issue board is the surface those agents work from > - Recovery is the subsystem that notices when an agent run finished but left no next step, and hands the issue back so the work is not silently dropped > - `decideSuccessfulRunHandoff` implements that check: after a successful run it looks for a valid disposition on the issue and enqueues a corrective handoff wake when it finds none > - Routine execution issues (`originKind = "routine_execution"`) deliberately stay `in_progress` between cron fires — one open issue is coalesced across dispatches through the `issues_open_routine_execution_uq` unique index > - Recovery read that intended steady state as a *missing* disposition, so every successful routine run enqueued a handoff wake, exhausted its single allowed attempt, and left the issue `blocked` with a `source_scoped_recovery_action` that re-woke the assigned agent > - This pull request exempts routine executions from the successful-run handoff, mirroring the existing issue-monitor maintenance skip (`isIssueMonitorMaintenanceRun`) > - The benefit is that routines stop generating self-inflicted recovery churn while genuine stranded issues keep being caught ## Issue _No public issue exists for this yet; describing it inline per CONTRIBUTING.md ("Link Issues or Describe Them In-PR"), following the bug report template._ **What happened?** After every successful run of a routine execution issue, recovery enqueued a corrective handoff wake because the issue had no terminal disposition. The single allowed handoff attempt was then exhausted and the issue landed in `blocked` with an active `source_scoped_recovery_action`, which re-woke the assigned agent. Observed in production as roughly 27 runs per hour on a single orphan-check routine, which additionally tripped a false productivity review on the assigned agent. **Expected behavior** A successful routine run that leaves the routine execution issue `in_progress` is the intended waiting state. Recovery should skip the successful-run handoff for `routine_execution` issues and let the routine lifecycle own their disposition. **Steps to reproduce** 1. Create a routine that dispatches an execution issue on a short cron interval. 2. Let one dispatched run complete successfully without writing a terminal status on the routine execution issue. 3. Observe the corrective handoff wake, the exhausted attempt, and the issue moving to `blocked` with an active source-scoped recovery action. 4. Repeat on the next cron fire — the cycle restarts and the run rate climbs. **Paperclip version or commit** `master` at the time this branch was cut; the guard is absent on current `master`. **Deployment mode / adapters / database** Adapter-independent — the path is server-side in `server/src/services/recovery/successful-run-handoff.ts`. Covered in tests against embedded Postgres. ## What Changed - `decideSuccessfulRunHandoff` returns `{ kind: "skip", reason: "routine execution owns its own disposition" }` when `issue.originKind === "routine_execution"`, alongside the existing issue-monitor maintenance skip. - `originKind` was added to the handoff `IssueRow` projection and to the issue select in `handleSuccessfulRunHandoff` in the heartbeat service, so the guard has the field it needs. - Added a regression test in `successful-run-handoff.test.ts` asserting that a `routine_execution` issue is skipped rather than enqueued. **Related PRs** (dedup search): [#8622](https://github.com/paperclipai/paperclip/pull/8622) touches the same routine-execution uniqueness index from the migration side; [#6527](https://github.com/paperclipai/paperclip/pull/6527) auto-resolves stale source recovery actions, which is the symptom this PR stops producing. No PR was found that implements this guard. ## Verification - `cd server && pnpm exec vitest run src/services/recovery --config vitest.config.ts` — recovery suites pass. - `cd server && pnpm exec vitest run src/__tests__/heartbeat-process-recovery.test.ts --config vitest.config.ts` — passes; 59 tests across both suites. - Regression check: with the `routine_execution` guard removed, the new test falls through to `enqueue` and fails; with the guard it passes. - `pnpm --filter @paperclipai/server typecheck` — clean. ## Risks - Low risk, behavioural only. No schema change and no migration in this change. - The guard is narrow: it keys on `originKind === "routine_execution"`, so ordinary assigned issues keep their existing successful-run handoff behaviour unchanged. - If a routine execution issue ever did need corrective handoff, it would now be skipped. That trade-off is deliberate — the routine lifecycle already owns these issues, and the coalescing unique index guarantees at most one open issue per routine. ## Model Used - Claude (Anthropic), Claude Opus 4.6 via the Claude Code agent harness, extended thinking enabled, with tool use (file edits, shell, ripgrep) against a local checkout. ## 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) - [ ] 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 - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --- **Review request (2026-08-19).** Rechecked against `master` a5f3ac6b4: `decideSuccessfulRunHandoff` skips on `hasActiveRoutineContinuation` (another live run on the same issue) but never on `originKind === "routine_execution"`, so a routine issue coalesced across fires still draws a missing-disposition handoff. Green and merges cleanly. We cannot use the reviewer field as outside contributors, so flagging the recent maintainers of the touched files instead: @cryppadotta, @devinfoley. Happy to close if this is not wanted — no need to reply otherwise. Co-authored-by: Claude Opus 5.5 --- .../services/recovery/successful-run-handoff.test.ts | 12 ++++++++++++ .../src/services/recovery/successful-run-handoff.ts | 9 +++++++++ 2 files changed, 21 insertions(+) diff --git a/server/src/services/recovery/successful-run-handoff.test.ts b/server/src/services/recovery/successful-run-handoff.test.ts index 67baafc1290..24cb4b8f701 100644 --- a/server/src/services/recovery/successful-run-handoff.test.ts +++ b/server/src/services/recovery/successful-run-handoff.test.ts @@ -420,6 +420,18 @@ describe("successful run handoff decision", () => { }); }); + it("does not queue for routine execution issues left in_progress between fires", () => { + // A successful orphan-check routine run left its coalesced routine_execution + // issue in `in_progress`, and the handoff wrongly treated that steady state + // as a missing disposition -> blocked + recovery loop. + expect(decide({ + issue: { ...issue, originKind: "routine_execution" } as any, + })).toEqual({ + kind: "skip", + reason: "routine execution owns its own disposition", + }); + }); + it("does not queue for successful comment-driven wakes", () => { expect(decide({ run: { diff --git a/server/src/services/recovery/successful-run-handoff.ts b/server/src/services/recovery/successful-run-handoff.ts index cf33b61cf51..5c199f73cf4 100644 --- a/server/src/services/recovery/successful-run-handoff.ts +++ b/server/src/services/recovery/successful-run-handoff.ts @@ -35,6 +35,12 @@ export const SUCCESSFUL_RUN_HANDOFF_OPTIONS = [ "delegate_or_continue_from_checkpoint", ] as const; +// Routine execution issues stay `in_progress` between cron fires by design — one +// open issue is coalesced across dispatches (see issues_open_routine_execution_uq). +// Their disposition is owned by the routine lifecycle, not by the missing-disposition +// handoff, so a successful routine run must not be treated as a missing next step. +const ROUTINE_EXECUTION_ORIGIN_KIND = "routine_execution"; + const PRODUCTIVE_SUCCESS_LIVENESS_STATES = new Set([ "advanced", "completed", @@ -493,6 +499,9 @@ export function decideSuccessfulRunHandoff(input: { return { kind: "skip", reason: "missing issue comment retry owns the next action" }; } if (!issue) return { kind: "skip", reason: "issue not found" }; + if (issue.originKind === ROUTINE_EXECUTION_ORIGIN_KIND) { + return { kind: "skip", reason: "routine execution owns its own disposition" }; + } if (!agent) return { kind: "skip", reason: "agent not found" }; if (issue.companyId !== run.companyId || agent.companyId !== run.companyId) { return { kind: "skip", reason: "company scope mismatch" };