From 130219c0be4b019b6a9239e3721cb39e3b252740 Mon Sep 17 00:00:00 2001 From: sunghere Date: Fri, 12 Jun 2026 13:53:51 +0900 Subject: [PATCH] fix(recovery): exempt stranded escalation when assignee shows recent visible progress (#5213) 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 > - The recovery service watches `in_progress` agent-assigned issues every 30s and creates "Recover stalled issue …" child issues when execution looks stranded > - The `isRepeatedProductiveContinuationRecovery` branch escalates after just **two consecutive productive continuation runs** — fine for genuinely stuck agents that loop without doing anything, but a false positive for batch workflows that legitimately advance every heartbeat (e.g. multi-frame image generation that produces 1–2 frames + an attachment per heartbeat) > - In production this fired ~95 times for a 19-character batch run, burning a recovery owner heartbeat each time > - This pull request adds a "recent visible progress" exemption: if the assignee posted a comment or any attachment within the exemption window (default 30 min, env-tunable, 60s floor), skip the escalation and let the normal continuation-retry path enqueue the next wake > - The benefit is one platform tweak unblocks all current and future batch workflows without weakening the genuinely-stuck case — agents that go silent still escalate after the window elapses ## What Changed - `server/src/services/recovery/service.ts` - new `STRANDED_RECENT_PROGRESS_EXEMPTION_MS` constant (default 30 min, override via env, floored at 60s) - new `hasRecentVisibleProgress(companyId, issueId, assigneeAgentId, windowMs)` helper — single parallel query against `issue_comments` (filtered by `authorAgentId`) + `issue_attachments`, both using existing indexes - in `reconcileStrandedAssignedIssues`, the `isRepeatedProductiveContinuationRecovery` branch now consults the helper before escalating; on exemption it falls through to the existing continuation-retry enqueue path - new `recentProgressExempted` counter on the reconcile result, surfaced in the periodic recovery log via the existing `...reconciled` spread - `server/src/__tests__/heartbeat-process-recovery.test.ts` - new test: recent agent comment → no escalation, continuation re-queued, `recentProgressExempted: 1` - new test: stale (24h-old) agent comment → escalation still fires as before ## Verification - `pnpm typecheck` — green across the workspace - `pnpm exec vitest run server/src/__tests__/heartbeat-process-recovery.test.ts` — 39/39 pass (37 pre-existing + 2 new) - Smoke after deploy: confirm Image Spec multi-frame generation no longer creates `Recover stalled issue …` child issues per heartbeat ## Risks - **Behavioral shift, low blast radius.** A genuinely-stuck agent that posts cosmetic comments every <30 min would now escalate later instead of immediately. Mitigated by: - Window is configurable via `STRANDED_RECENT_PROGRESS_EXEMPTION_MS` - Other escalation paths are untouched (failed/cancelled/timed_out runs still escalate immediately, paused-tree handling unchanged, recovery-issue-on-recovery guard unchanged) - Periodic recovery log now reports `recentProgressExempted` so a runaway exemption is visible in operations - No DB migration required — both `issue_comments` and `issue_attachments` queries use existing indexes - Backward compatible: pre-existing test "blocks stranded in-progress work after a productive continuation retry was already used" still passes unchanged because no comment is seeded → no exemption → escalates ## Model Used - Claude Opus 4.7 (`claude-opus-4-7`), extended thinking, tool use enabled. Investigation, change, tests, and PR body all human-supervised through a Paperclip agent heartbeat. ## 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 run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] If this change affects the UI, I have included before/after screenshots — N/A, server-only - [ ] I have updated relevant documentation to reflect my changes — no docs touched the previous behavior; the env knob is self-documenting via the comment in `service.ts` - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge ## Cross-references and status (maintainer) Rebased onto current `master`. No duplicate PRs absorbed. Refs #6072 — related open report in the same `reconcileStrandedAssignedIssues` / `isRepeatedProductiveContinuationRecovery` family (stale productive-continuation evidence). This PR does not fix #6072, but the recent-visible-progress exemption added here shrinks the false-positive surface in that branch and the new `recentProgressExempted` counter gives operators visibility into the broader escalation path. Co-authored-by: sunghere Co-authored-by: Paperclip --- .../heartbeat-process-recovery.test.ts | 72 ++++++++++++++ server/src/services/recovery/service.ts | 96 ++++++++++++++++--- 2 files changed, 154 insertions(+), 14 deletions(-) diff --git a/server/src/__tests__/heartbeat-process-recovery.test.ts b/server/src/__tests__/heartbeat-process-recovery.test.ts index 3954ccb0..414aad1c 100644 --- a/server/src/__tests__/heartbeat-process-recovery.test.ts +++ b/server/src/__tests__/heartbeat-process-recovery.test.ts @@ -3418,6 +3418,78 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { expect(retryRun?.contextSnapshot as Record).not.toHaveProperty("modelProfile"); }); + it("exempts stranded-recovery escalation when assignee posted a recent comment (GGU-809)", async () => { + const { companyId, agentId, issueId, runId } = await seedStrandedIssueFixture({ + status: "in_progress", + runStatus: "succeeded", + retryReason: "issue_continuation_needed", + runSource: "issue.productive_terminal_continuation_recovery", + livenessState: "advanced", + }); + // Recent agent-authored comment should suppress the repeat-productive + // escalation and let the normal continuation-retry path proceed. + await db.insert(issueComments).values({ + companyId, + issueId, + authorAgentId: agentId, + body: "frame 02/08 generated, attaching shortly", + }); + const heartbeat = heartbeatService(db); + + const result = await heartbeat.reconcileStrandedAssignedIssues(); + expect(result.escalated).toBe(0); + expect(result.recentProgressExempted).toBe(1); + expect(result.continuationRequeued).toBe(1); + expect(result.issueIds).toEqual([issueId]); + + const issue = await db.select().from(issues).where(eq(issues.id, issueId)).then((rows) => rows[0] ?? null); + expect(issue?.status).toBe("in_progress"); + + const recoveryIssues = await db + .select() + .from(issues) + .where(and(eq(issues.companyId, companyId), eq(issues.originKind, "stranded_issue_recovery"))); + expect(recoveryIssues).toHaveLength(0); + + const runs = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.agentId, agentId)); + expect(runs).toHaveLength(2); + const retryRun = runs.find((row) => row.id !== runId); + expect(retryRun?.contextSnapshot as Record | undefined).toMatchObject({ + issueId, + retryReason: "issue_continuation_needed", + source: "issue.productive_terminal_continuation_recovery", + }); + }); + + it("still escalates stranded-recovery work when the recent comment is older than the exemption window (GGU-809)", async () => { + const { companyId, agentId, issueId } = await seedStrandedIssueFixture({ + status: "in_progress", + runStatus: "succeeded", + retryReason: "issue_continuation_needed", + runSource: "issue.productive_terminal_continuation_recovery", + livenessState: "advanced", + }); + // Comment older than the exemption window must NOT suppress escalation. + const stale = new Date(Date.now() - 24 * 60 * 60 * 1000); + await db.insert(issueComments).values({ + companyId, + issueId, + authorAgentId: agentId, + body: "old progress note", + createdAt: stale, + updatedAt: stale, + }); + const heartbeat = heartbeatService(db); + + const result = await heartbeat.reconcileStrandedAssignedIssues(); + expect(result.escalated).toBe(1); + expect(result.recentProgressExempted).toBe(0); + expect(result.continuationRequeued).toBe(0); + + const issue = await db.select().from(issues).where(eq(issues.id, issueId)).then((rows) => rows[0] ?? null); + expect(issue?.status).toBe("blocked"); + }); + it("does not reconcile user-assigned work through the agent stranded-work recovery path", async () => { const { issueId, runId } = await seedStrandedIssueFixture({ status: "todo", diff --git a/server/src/services/recovery/service.ts b/server/src/services/recovery/service.ts index 1a5fb8c5..5b93a7b8 100644 --- a/server/src/services/recovery/service.ts +++ b/server/src/services/recovery/service.ts @@ -16,6 +16,7 @@ import { heartbeatRunEvents, heartbeatRunWatchdogDecisions, heartbeatRuns, + issueAttachments, issueComments, issueApprovals, issueRecoveryActions, @@ -81,6 +82,18 @@ const SESSIONED_LOCAL_ADAPTERS = new Set([ "pi_local", ]); +// GGU-809: when a stranded `in_progress` issue would otherwise hit the +// `isRepeatedProductiveContinuationRecovery` escalation path, exempt the +// escalation if the assignee posted a comment or attachment within this window. +// Batch workflows (e.g. Image Spec multi-frame generation) make real progress +// every heartbeat and would otherwise trigger a recovery issue after just two +// productive heartbeats. Floor the override at 60s to keep the exemption from +// being effectively disabled by misconfiguration. +export const STRANDED_RECENT_PROGRESS_EXEMPTION_MS = Math.max( + 60_000, + Number(process.env.STRANDED_RECENT_PROGRESS_EXEMPTION_MS) || 30 * 60 * 1000, +); + type RecoveryWakeupOptions = { source?: "timer" | "assignment" | "on_demand" | "automation"; triggerDetail?: "manual" | "ping" | "callback" | "system"; @@ -584,6 +597,47 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup }) .then((rows) => Boolean(rows[0])); } + // GGU-809: visible-progress signal for stranded-recovery escalation guard. + // Returns true if the assignee posted a comment, OR any attachment was added + // to the issue, within `windowMs`. Used to suppress false-positive recovery + // issues for batch workflows that genuinely advance every heartbeat. + async function hasRecentVisibleProgress( + companyId: string, + issueId: string, + assigneeAgentId: string, + windowMs: number, + ) { + const since = new Date(Date.now() - windowMs); + const [comment, attachment] = await Promise.all([ + db + .select({ id: issueComments.id }) + .from(issueComments) + .where( + and( + eq(issueComments.companyId, companyId), + eq(issueComments.issueId, issueId), + eq(issueComments.authorAgentId, assigneeAgentId), + gt(issueComments.createdAt, since), + ), + ) + .limit(1) + .then((rows) => rows[0] ?? null), + db + .select({ id: issueAttachments.id }) + .from(issueAttachments) + .where( + and( + eq(issueAttachments.companyId, companyId), + eq(issueAttachments.issueId, issueId), + gt(issueAttachments.createdAt, since), + ), + ) + .limit(1) + .then((rows) => rows[0] ?? null), + ]); + return Boolean(comment || attachment); + } + async function enqueueStrandedIssueRecovery(input: { issueId: string; agentId: string; @@ -2475,6 +2529,7 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup }) orphanBlockersAssigned: 0, successfulRunHandoffEscalated: 0, escalated: 0, + recentProgressExempted: 0, skipped: 0, issueIds: [] as string[], }; @@ -2623,21 +2678,34 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup }) } if (isRepeatedProductiveContinuationRecovery(successfulRun)) { - const updated = await escalateStrandedAssignedIssue({ - issue, - previousStatus: "in_progress", - latestRun: successfulRun, - comment: - "Paperclip automatically retried continuation for this assigned `in_progress` issue and the retry " + - "made progress, but it still has no live execution path. Moving it to `blocked` so it is visible for intervention.", - }); - if (updated) { - result.escalated += 1; - result.issueIds.push(issue.id); - } else { - result.skipped += 1; + // GGU-809: skip escalation if the assignee has shown visible progress + // (comment or attachment) within the exemption window. Falling + // through here lets the normal continuation-retry path enqueue the + // next wake, which is the correct behaviour for batch workflows. + const exempted = await hasRecentVisibleProgress( + issue.companyId, + issue.id, + agentId, + STRANDED_RECENT_PROGRESS_EXEMPTION_MS, + ); + if (!exempted) { + const updated = await escalateStrandedAssignedIssue({ + issue, + previousStatus: "in_progress", + latestRun: successfulRun, + comment: + "Paperclip automatically retried continuation for this assigned `in_progress` issue and the retry " + + "made progress, but it still has no live execution path. Moving it to `blocked` so it is visible for intervention.", + }); + if (updated) { + result.escalated += 1; + result.issueIds.push(issue.id); + } else { + result.skipped += 1; + } + continue; } - continue; + result.recentProgressExempted += 1; } if (await isInvocationBudgetBlocked(issue, agentId)) {