From a70de9f3322d092630dd4747452bffe17d227a06 Mon Sep 17 00:00:00 2001 From: CTO Date: Wed, 12 Aug 2026 00:31:49 +0000 Subject: [PATCH 1/6] SPC-21314: cap reconcileStrandedAssignedIssues re-escalation on same-cause missing-disposition recovery Rebuilt on current master (prior stacked PR notandrewblejde/paperclip#12 was 663 commits behind and conflicting). Decoupled from the SPC-21224 general-cap so this fix stands alone and can merge to master directly. - New env-overridable SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS (default 3). - ensureSourceScopedStrandedRecoveryAction records that cap as maxAttempts on new missing_disposition actions (was maxAttempts: null / unbounded). - reconcileStrandedAssignedIssues short-circuits the exhausted-handoff escalate path with a logger.warn when a same-cause active recovery action has already hit the cap, so no further source_scoped_recovery_action wake is enqueued. Fixes the SPC-21292 flap (attemptCount hit 30 in 17min, ~1 wake/min). Unblocks SPC-30744 (MDE run storm root cause). Tests: new "caps re-escalation once the same-cause missing-disposition recovery action hits the attempt cap" + maxAttempts assertions on the two existing missing_disposition escalation tests. Co-Authored-By: Paperclip --- .../heartbeat-process-recovery.test.ts | 68 ++++++++++++++++++- server/src/services/recovery/service.ts | 46 ++++++++++++- 2 files changed, 112 insertions(+), 2 deletions(-) diff --git a/server/src/__tests__/heartbeat-process-recovery.test.ts b/server/src/__tests__/heartbeat-process-recovery.test.ts index f77f756285..d9bfd42b29 100644 --- a/server/src/__tests__/heartbeat-process-recovery.test.ts +++ b/server/src/__tests__/heartbeat-process-recovery.test.ts @@ -230,6 +230,8 @@ import { UNMANAGED_BACKGROUND_TASK_LIVENESS_REASON, UNMANAGED_BACKGROUND_TASK_STOP_REASON, } from "@paperclipai/adapter-utils/server-utils"; +import { SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS } from "../services/recovery/service.ts"; + const externalTestDatabaseUrl = process.env.PAPERCLIP_TEST_DATABASE_URL?.trim(); const embeddedPostgresSupport = externalTestDatabaseUrl ? { supported: true } @@ -1279,6 +1281,7 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { kind?: string; previousOwnerAgentId?: string | null; returnOwnerAgentId?: string | null; + maxAttempts?: number | null; }) { const action = await waitForValue(async () => db @@ -1309,7 +1312,7 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { returnOwnerAgentId: input.returnOwnerAgentId ?? input.agentId, cause: input.cause ?? "stranded_assigned_issue", attemptCount: 1, - maxAttempts: null, + maxAttempts: input.maxAttempts ?? null, }); expect(action.evidence).toMatchObject({ sourceIssueId: input.issueId, @@ -5637,6 +5640,7 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { retryReason: null, cause: SUCCESSFUL_RUN_MISSING_STATE_REASON, kind: "missing_disposition", + maxAttempts: SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS, }); expect(recoveryAction.evidence).toMatchObject({ sourceRunId, @@ -5767,6 +5771,7 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { retryReason: null, cause: SUCCESSFUL_RUN_MISSING_STATE_REASON, kind: "missing_disposition", + maxAttempts: SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS, }); expect(recoveryAction.evidence).toMatchObject({ sourceRunId, @@ -5775,6 +5780,67 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { }); }); + it("caps re-escalation once the same-cause missing-disposition recovery action hits the attempt cap (SPC-21314)", async () => { + const { companyId, agentId, runId, issueId } = await seedStrandedIssueFixture({ + status: "in_progress", + runStatus: "succeeded", + livenessState: "advanced", + }); + const sourceRunId = randomUUID(); + await db + .update(heartbeatRuns) + .set({ + contextSnapshot: { + issueId, + taskId: issueId, + wakeReason: "finish_successful_run_handoff", + sourceRunId, + resumeFromRunId: sourceRunId, + handoffRequired: true, + handoffReason: "successful_run_missing_state", + missingDisposition: "clear_next_step", + handoffAttempt: 1, + maxHandoffAttempts: 1, + }, + }) + .where(eq(heartbeatRuns.id, runId)); + const heartbeat = heartbeatService(db); + + // First reconcile escalates once and opens the missing-disposition action. + const firstResult = await heartbeat.reconcileStrandedAssignedIssues(); + expect(firstResult.successfulRunHandoffEscalated).toBe(1); + const action = await expectSourceScopedStrandedRecoveryAction({ + companyId, + agentId, + issueId, + runId, + previousStatus: "in_progress", + retryReason: null, + cause: SUCCESSFUL_RUN_MISSING_STATE_REASON, + kind: "missing_disposition", + maxAttempts: SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS, + }); + + // Simulate the flap: the action has re-escalated up to its cap and the owner + // has PATCHed the issue back to in_progress without recording a disposition. + await db + .update(issueRecoveryActions) + .set({ attemptCount: SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS }) + .where(eq(issueRecoveryActions.id, action.id)); + await db.update(issues).set({ status: "in_progress" }).where(eq(issues.id, issueId)); + + // Second reconcile must NOT re-escalate — the same-cause cap short-circuits. + const secondResult = await heartbeat.reconcileStrandedAssignedIssues(); + expect(secondResult.successfulRunHandoffEscalated).toBe(0); + + const after = await db + .select() + .from(issueRecoveryActions) + .where(eq(issueRecoveryActions.id, action.id)) + .then((rows) => rows[0] ?? null); + expect(after?.attemptCount).toBe(SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS); + }); + it("converts a continuation parked for review into a dependency wait on its open sub-tasks", async () => { const { companyId, agentId, issueId } = await seedStrandedIssueFixture({ status: "in_progress", diff --git a/server/src/services/recovery/service.ts b/server/src/services/recovery/service.ts index 0361621ec1..563c5f85fb 100644 --- a/server/src/services/recovery/service.ts +++ b/server/src/services/recovery/service.ts @@ -173,6 +173,19 @@ export const STRANDED_RECENT_PROGRESS_EXEMPTION_MS = Math.max( Number(process.env.STRANDED_RECENT_PROGRESS_EXEMPTION_MS) || 30 * 60 * 1000, ); +// SPC-21314: cap re-escalation for the `successful_run_missing_state` recovery +// cause. The flap on SPC-21292 (2026-07-11) burned ~1 wake/min for 17+ minutes +// when an owner PATCHed `in_progress` on every recovery wake without recording a +// valid disposition, and `reconcileStrandedAssignedIssues` re-escalated on every +// tick because the exhausted-handoff path never consulted the existing active +// recovery action's attemptCount (it was left unbounded, maxAttempts=null). Cap +// re-escalation at 3 attempts so the reconciler surfaces the loop for +// intervention instead of enqueuing another wake. +export const SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS = Math.max( + 1, + Number(process.env.SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS) || 3, +); + type RecoveryWakeupOptions = { source?: "timer" | "assignment" | "on_demand" | "automation"; triggerDetail?: "manual" | "ping" | "callback" | "system"; @@ -2518,7 +2531,11 @@ export function recoveryService( monitorPolicy: isProviderQuotaWait ? { type: "wait_recovery", retryAgentId: routing.returnOwnerAgentId } : null, - maxAttempts: null, + // SPC-21314: carry the missing-disposition attempt cap on the row itself so + // the reconciler gate (and any future consumer) can bound re-escalation. + maxAttempts: recoveryCause === SUCCESSFUL_RUN_MISSING_STATE_REASON + ? SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS + : null, lastAttemptAt: now, }); @@ -4949,6 +4966,33 @@ export function recoveryService( continue; } + // SPC-21314: same-cause re-escalation cap. When a prior + // `successful_run_missing_state` recovery action is still active and has + // already hit its attempt cap, stop re-escalating. Without this gate the + // reconciler re-escalates every tick (owner PATCHes `in_progress` on each + // recovery wake without recording a valid disposition), producing an + // unbounded ~1 wake/min flap (observed attemptCount=30 in 17min on + // SPC-21292). The exhausted action stays as first-class evidence for + // board/human intervention. + const existingActive = await recoveryActionsSvc.getActiveForIssue(issue.companyId, issue.id); + if ( + existingActive && + existingActive.cause === SUCCESSFUL_RUN_MISSING_STATE_REASON && + existingActive.attemptCount >= SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS + ) { + logger.warn( + { + issueId: issue.id, + actionId: existingActive.id, + attemptCount: existingActive.attemptCount, + maxAttempts: existingActive.maxAttempts ?? SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS, + }, + "recovery.stranded.repeated_missing_disposition — skipping re-escalation", + ); + result.skipped += 1; + continue; + } + const updated = await escalateStrandedAssignedIssue({ issue, previousStatus: "in_progress", From 0cca08af80eb90638d36fc145054ba0bdf2e244d Mon Sep 17 00:00:00 2001 From: Claude Code Date: Thu, 13 Aug 2026 04:22:18 +0000 Subject: [PATCH 2/6] fix(recovery): persist only integer missing-state retry caps Reject non-integer env values (decimals, Infinity, scientific notation) and values outside the PostgreSQL integer column range so issue_recovery_actions.max_attempts never receives an unpersistable number. Co-Authored-By: Paperclip --- server/src/services/recovery/service.ts | 40 ++++++++++---- .../successful-run-missing-state-cap.test.ts | 53 +++++++++++++++++++ 2 files changed, 82 insertions(+), 11 deletions(-) create mode 100644 server/src/services/recovery/successful-run-missing-state-cap.test.ts diff --git a/server/src/services/recovery/service.ts b/server/src/services/recovery/service.ts index 563c5f85fb..23ea37ac8c 100644 --- a/server/src/services/recovery/service.ts +++ b/server/src/services/recovery/service.ts @@ -173,17 +173,35 @@ export const STRANDED_RECENT_PROGRESS_EXEMPTION_MS = Math.max( Number(process.env.STRANDED_RECENT_PROGRESS_EXEMPTION_MS) || 30 * 60 * 1000, ); -// SPC-21314: cap re-escalation for the `successful_run_missing_state` recovery -// cause. The flap on SPC-21292 (2026-07-11) burned ~1 wake/min for 17+ minutes -// when an owner PATCHed `in_progress` on every recovery wake without recording a -// valid disposition, and `reconcileStrandedAssignedIssues` re-escalated on every -// tick because the exhausted-handoff path never consulted the existing active -// recovery action's attemptCount (it was left unbounded, maxAttempts=null). Cap -// re-escalation at 3 attempts so the reconciler surfaces the loop for -// intervention instead of enqueuing another wake. -export const SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS = Math.max( - 1, - Number(process.env.SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS) || 3, +// Default + hard ceiling for the `successful_run_missing_state` re-escalation +// cap. The ceiling is PostgreSQL `integer` (int32) so a parsed env value can +// never be written into `issue_recovery_actions.max_attempts` as Infinity, +// a decimal, or a number outside the column range. +export const SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS_DEFAULT = 3; +export const SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS_CEILING = 2_147_483_647; + +// Accept only a finite integer in [1, int32]. Reject decimals ("3.5"), +// Infinity, scientific notation, and out-of-range values. `Number("3.5")` +// is finite and `Math.max(1, 3.5)` would persist 3.5 into an integer column. +export function parseSuccessfulRunMissingStateMaxAttempts( + raw: string | undefined, + fallback = SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS_DEFAULT, +): number { + const trimmed = raw?.trim(); + if (!trimmed || !/^[+-]?\d+$/.test(trimmed)) return fallback; + const parsed = Number.parseInt(trimmed, 10); + if (!Number.isSafeInteger(parsed) || parsed < 1 || parsed > SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS_CEILING) { + return fallback; + } + return parsed; +} + +// Cap re-escalation for the `successful_run_missing_state` recovery cause. +// Without a bound, `reconcileStrandedAssignedIssues` re-escalates every tick +// when an owner PATCHes `in_progress` on every recovery wake without recording +// a valid disposition (observed ~1 wake/min for 17+ minutes, attemptCount=30). +export const SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS = parseSuccessfulRunMissingStateMaxAttempts( + process.env.SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS, ); type RecoveryWakeupOptions = { diff --git a/server/src/services/recovery/successful-run-missing-state-cap.test.ts b/server/src/services/recovery/successful-run-missing-state-cap.test.ts new file mode 100644 index 0000000000..0757ae1f50 --- /dev/null +++ b/server/src/services/recovery/successful-run-missing-state-cap.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, it } from "vitest"; +import { + SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS_CEILING, + SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS_DEFAULT, + parseSuccessfulRunMissingStateMaxAttempts, +} from "./service.ts"; + +describe("parseSuccessfulRunMissingStateMaxAttempts", () => { + it("returns the default for missing, empty, or non-integer values", () => { + expect(parseSuccessfulRunMissingStateMaxAttempts(undefined)).toBe( + SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS_DEFAULT, + ); + expect(parseSuccessfulRunMissingStateMaxAttempts("")).toBe( + SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS_DEFAULT, + ); + expect(parseSuccessfulRunMissingStateMaxAttempts(" ")).toBe( + SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS_DEFAULT, + ); + expect(parseSuccessfulRunMissingStateMaxAttempts("3.5")).toBe( + SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS_DEFAULT, + ); + expect(parseSuccessfulRunMissingStateMaxAttempts("Infinity")).toBe( + SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS_DEFAULT, + ); + expect(parseSuccessfulRunMissingStateMaxAttempts("1e3")).toBe( + SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS_DEFAULT, + ); + expect(parseSuccessfulRunMissingStateMaxAttempts("abc")).toBe( + SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS_DEFAULT, + ); + }); + + it("rejects zero, negatives, and values above the int32 ceiling", () => { + expect(parseSuccessfulRunMissingStateMaxAttempts("0")).toBe( + SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS_DEFAULT, + ); + expect(parseSuccessfulRunMissingStateMaxAttempts("-1")).toBe( + SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS_DEFAULT, + ); + expect( + parseSuccessfulRunMissingStateMaxAttempts(String(SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS_CEILING + 1)), + ).toBe(SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS_DEFAULT); + }); + + it("accepts finite integers in the persistable range", () => { + expect(parseSuccessfulRunMissingStateMaxAttempts("1")).toBe(1); + expect(parseSuccessfulRunMissingStateMaxAttempts("3")).toBe(3); + expect(parseSuccessfulRunMissingStateMaxAttempts(" 12 ")).toBe(12); + expect(parseSuccessfulRunMissingStateMaxAttempts(String(SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS_CEILING))).toBe( + SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS_CEILING, + ); + }); +}); From 12a4f162570b98428bdffe2ad3cb8c424c535846 Mon Sep 17 00:00:00 2001 From: Claude Code Date: Thu, 13 Aug 2026 06:18:03 +0000 Subject: [PATCH 3/6] fix(recovery): import missing-state cap tests with .js path tsc compiles this unit file and rejects a .ts import unless allowImportingTsExtensions is on. Match sibling recovery tests so Typecheck + Build can pass. SPC-21314 Co-Authored-By: Paperclip --- .../services/recovery/successful-run-missing-state-cap.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/services/recovery/successful-run-missing-state-cap.test.ts b/server/src/services/recovery/successful-run-missing-state-cap.test.ts index 0757ae1f50..ef7fd97334 100644 --- a/server/src/services/recovery/successful-run-missing-state-cap.test.ts +++ b/server/src/services/recovery/successful-run-missing-state-cap.test.ts @@ -3,7 +3,7 @@ import { SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS_CEILING, SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS_DEFAULT, parseSuccessfulRunMissingStateMaxAttempts, -} from "./service.ts"; +} from "./service.js"; describe("parseSuccessfulRunMissingStateMaxAttempts", () => { it("returns the default for missing, empty, or non-integer values", () => { From b5481a0417f9768f8a3a8c79cc12538840250441 Mon Sep 17 00:00:00 2001 From: Claude Code Date: Thu, 13 Aug 2026 08:33:46 +0000 Subject: [PATCH 4/6] fix(recovery): honor persisted missing-state retry cap Compare the stranded-issue re-escalation gate against the recovery action's stored maxAttempts. A later env change must not reopen or stop an in-flight missing-disposition action. Co-Authored-By: Paperclip --- .../heartbeat-process-recovery.test.ts | 62 +++++++++++++++++++ server/src/services/recovery/service.ts | 47 +++++++++----- .../successful-run-missing-state-cap.test.ts | 23 +++++++ 3 files changed, 117 insertions(+), 15 deletions(-) diff --git a/server/src/__tests__/heartbeat-process-recovery.test.ts b/server/src/__tests__/heartbeat-process-recovery.test.ts index d9bfd42b29..4f1048318f 100644 --- a/server/src/__tests__/heartbeat-process-recovery.test.ts +++ b/server/src/__tests__/heartbeat-process-recovery.test.ts @@ -5841,6 +5841,68 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { expect(after?.attemptCount).toBe(SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS); }); + it("honors a persisted missing-disposition cap that differs from the process default (SPC-21314)", async () => { + const { companyId, agentId, runId, issueId } = await seedStrandedIssueFixture({ + status: "in_progress", + runStatus: "succeeded", + livenessState: "advanced", + }); + const sourceRunId = randomUUID(); + await db + .update(heartbeatRuns) + .set({ + contextSnapshot: { + issueId, + taskId: issueId, + wakeReason: "finish_successful_run_handoff", + sourceRunId, + resumeFromRunId: sourceRunId, + handoffRequired: true, + handoffReason: "successful_run_missing_state", + missingDisposition: "clear_next_step", + handoffAttempt: 1, + maxHandoffAttempts: 1, + }, + }) + .where(eq(heartbeatRuns.id, runId)); + const heartbeat = heartbeatService(db); + + const firstResult = await heartbeat.reconcileStrandedAssignedIssues(); + expect(firstResult.successfulRunHandoffEscalated).toBe(1); + const action = await expectSourceScopedStrandedRecoveryAction({ + companyId, + agentId, + issueId, + runId, + previousStatus: "in_progress", + retryReason: null, + cause: SUCCESSFUL_RUN_MISSING_STATE_REASON, + kind: "missing_disposition", + maxAttempts: SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS, + }); + + // Simulate a pre-restart action that recorded a lower cap than the current + // process env. The gate must honor the persisted value, not the new default. + const persistedCap = 1; + expect(persistedCap).toBeLessThan(SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS); + await db + .update(issueRecoveryActions) + .set({ attemptCount: persistedCap, maxAttempts: persistedCap }) + .where(eq(issueRecoveryActions.id, action.id)); + await db.update(issues).set({ status: "in_progress" }).where(eq(issues.id, issueId)); + + const secondResult = await heartbeat.reconcileStrandedAssignedIssues(); + expect(secondResult.successfulRunHandoffEscalated).toBe(0); + + const after = await db + .select() + .from(issueRecoveryActions) + .where(eq(issueRecoveryActions.id, action.id)) + .then((rows) => rows[0] ?? null); + expect(after?.attemptCount).toBe(persistedCap); + expect(after?.maxAttempts).toBe(persistedCap); + }); + it("converts a continuation parked for review into a dependency wait on its open sub-tasks", async () => { const { companyId, agentId, issueId } = await seedStrandedIssueFixture({ status: "in_progress", diff --git a/server/src/services/recovery/service.ts b/server/src/services/recovery/service.ts index 23ea37ac8c..961d17e110 100644 --- a/server/src/services/recovery/service.ts +++ b/server/src/services/recovery/service.ts @@ -204,6 +204,22 @@ export const SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS = parseSuccessfulRunMissi process.env.SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS, ); +// Honor the cap persisted on the recovery-action row. A later env change must +// not re-open or prematurely stop an in-flight missing-disposition action. +export function resolveSuccessfulRunMissingStateMaxAttempts( + persistedMaxAttempts: number | null | undefined, +): number { + if ( + typeof persistedMaxAttempts === "number" && + Number.isSafeInteger(persistedMaxAttempts) && + persistedMaxAttempts >= 1 && + persistedMaxAttempts <= SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS_CEILING + ) { + return persistedMaxAttempts; + } + return SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS; +} + type RecoveryWakeupOptions = { source?: "timer" | "assignment" | "on_demand" | "automation"; triggerDetail?: "manual" | "ping" | "callback" | "system"; @@ -4993,22 +5009,23 @@ export function recoveryService( // SPC-21292). The exhausted action stays as first-class evidence for // board/human intervention. const existingActive = await recoveryActionsSvc.getActiveForIssue(issue.companyId, issue.id); - if ( - existingActive && - existingActive.cause === SUCCESSFUL_RUN_MISSING_STATE_REASON && - existingActive.attemptCount >= SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS - ) { - logger.warn( - { - issueId: issue.id, - actionId: existingActive.id, - attemptCount: existingActive.attemptCount, - maxAttempts: existingActive.maxAttempts ?? SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS, - }, - "recovery.stranded.repeated_missing_disposition — skipping re-escalation", + if (existingActive && existingActive.cause === SUCCESSFUL_RUN_MISSING_STATE_REASON) { + const existingActiveMaxAttempts = resolveSuccessfulRunMissingStateMaxAttempts( + existingActive.maxAttempts, ); - result.skipped += 1; - continue; + if (existingActive.attemptCount >= existingActiveMaxAttempts) { + logger.warn( + { + issueId: issue.id, + actionId: existingActive.id, + attemptCount: existingActive.attemptCount, + maxAttempts: existingActiveMaxAttempts, + }, + "recovery.stranded.repeated_missing_disposition — skipping re-escalation", + ); + result.skipped += 1; + continue; + } } const updated = await escalateStrandedAssignedIssue({ diff --git a/server/src/services/recovery/successful-run-missing-state-cap.test.ts b/server/src/services/recovery/successful-run-missing-state-cap.test.ts index ef7fd97334..197d1dbbce 100644 --- a/server/src/services/recovery/successful-run-missing-state-cap.test.ts +++ b/server/src/services/recovery/successful-run-missing-state-cap.test.ts @@ -1,8 +1,10 @@ import { describe, expect, it } from "vitest"; import { + SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS, SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS_CEILING, SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS_DEFAULT, parseSuccessfulRunMissingStateMaxAttempts, + resolveSuccessfulRunMissingStateMaxAttempts, } from "./service.js"; describe("parseSuccessfulRunMissingStateMaxAttempts", () => { @@ -51,3 +53,24 @@ describe("parseSuccessfulRunMissingStateMaxAttempts", () => { ); }); }); + +describe("resolveSuccessfulRunMissingStateMaxAttempts", () => { + it("uses the persisted integer cap when it is in the persistable range", () => { + expect(resolveSuccessfulRunMissingStateMaxAttempts(1)).toBe(1); + expect(resolveSuccessfulRunMissingStateMaxAttempts(5)).toBe(5); + expect(resolveSuccessfulRunMissingStateMaxAttempts(SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS_CEILING)).toBe( + SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS_CEILING, + ); + }); + + it("falls back to the process cap for null, missing, or unpersistable values", () => { + expect(resolveSuccessfulRunMissingStateMaxAttempts(null)).toBe(SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS); + expect(resolveSuccessfulRunMissingStateMaxAttempts(undefined)).toBe(SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS); + expect(resolveSuccessfulRunMissingStateMaxAttempts(0)).toBe(SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS); + expect(resolveSuccessfulRunMissingStateMaxAttempts(-1)).toBe(SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS); + expect(resolveSuccessfulRunMissingStateMaxAttempts(3.5)).toBe(SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS); + expect(resolveSuccessfulRunMissingStateMaxAttempts(Number.POSITIVE_INFINITY)).toBe( + SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS, + ); + }); +}); From 9869d82c2ab6d83ad2e986a730fb689268bd7a88 Mon Sep 17 00:00:00 2001 From: Claude Code Date: Sat, 12 Sep 2026 12:15:16 +0000 Subject: [PATCH 5/6] fix(recovery): skip stranded-issue escalation while a monitor wake is armed SPC-21314 named a third design deficiency that PR #11242 did not yet cover: reconcileStrandedAssignedIssues had no signal to skip issues with a durable, future monitor wake before escalating/continuation-waking them. SPC-37112 (2026-09-09) reproduced this exactly: an in_progress issue with executionPolicy.monitor.nextCheckAt armed 9 days out was rewoken via issue_continuation_needed 10+ times in ~20 minutes before the assignee worked around it by force-setting status to blocked (itself a known-bad move, since it silently clears the monitor). Add hasArmedMonitorWake(issue, now) and call it first in the per-issue reconciler loop, before any escalation branch runs, whenever the issue is in_progress/in_review with monitorNextCheckAt set in the future. The existing hasPersistedDurableWaitPath/hasPendingProviderQuotaRecoveryMonitor checks only fire deep in the loop and only cover a `succeeded` latest run or a provider-quota-tagged monitor respectively, so they miss cases like SPC-37112 where the run is cancelled/failed and the monitor is a general one. Covered by 5 new unit tests for hasArmedMonitorWake and one new integration test against reconcileStrandedAssignedIssues asserting no escalation and no issue_continuation_needed wake when a future monitor is armed. Co-Authored-By: Claude Sonnet 5 --- .../heartbeat-process-recovery.test.ts | 67 +++++++++++++++++++ server/src/services/recovery/service.ts | 31 +++++++++ .../successful-run-missing-state-cap.test.ts | 56 ++++++++++++++++ 3 files changed, 154 insertions(+) diff --git a/server/src/__tests__/heartbeat-process-recovery.test.ts b/server/src/__tests__/heartbeat-process-recovery.test.ts index 4f1048318f..c7ad11a150 100644 --- a/server/src/__tests__/heartbeat-process-recovery.test.ts +++ b/server/src/__tests__/heartbeat-process-recovery.test.ts @@ -6707,6 +6707,73 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { }); }); + // SPC-21314 deficiency #3 / SPC-37112 / SPC-39089: without this gate the + // exact same fixture (paused, non-invokable source owner) escalates every + // reconciler tick via the branch exercised above — even when the issue has + // a legitimate monitor wake armed days out. That flap burned an in_progress + // issue with a 9-day-out monitor with 10+ issue_continuation_needed wakes + // in ~20 minutes on SPC-37112. + it("does not escalate a stranded-looking issue with a monitor wake armed far in the future", async () => { + const { companyId, agentId, issueId } = await seedStrandedIssueFixture({ + status: "in_progress", + runStatus: "cancelled", + retryReason: "issue_continuation_needed", + runErrorCode: "issue_continuation_waiting_on_review", + // The guard compares against wall-clock `new Date()`, not the fixture's + // fixed 2026-03-19 fixture timestamps, so this must be in the real + // future regardless of when the suite runs. + monitorNextCheckAt: new Date("2099-03-19T00:00:00.000Z"), + }); + await db + .update(agents) + .set({ status: "paused" }) + .where(eq(agents.id, agentId)); + + const result = await heartbeatService(db).reconcileStrandedAssignedIssues(); + expect(result.escalated).toBe(0); + expect(result.armedMonitorExempted).toBe(1); + expect(result.issueIds).toEqual([]); + + const sourceIssue = await db + .select() + .from(issues) + .where(eq(issues.id, issueId)) + .then((rows) => rows[0] ?? null); + expect(sourceIssue).toMatchObject({ + status: "in_progress", + assigneeAgentId: agentId, + }); + + const actions = await db + .select() + .from(issueRecoveryActions) + .where( + and( + eq(issueRecoveryActions.companyId, companyId), + eq(issueRecoveryActions.sourceIssueId, issueId), + ), + ); + expect(actions).toHaveLength(0); + + const wakeups = await db + .select() + .from(agentWakeupRequests) + .where( + and( + eq(agentWakeupRequests.companyId, companyId), + eq(agentWakeupRequests.agentId, agentId), + ), + ); + expect( + wakeups.some( + (wake) => + wake.reason === "issue_continuation_needed" || + (wake.payload as { retryReason?: string } | null)?.retryReason === + "issue_continuation_needed", + ), + ).toBe(false); + }); + it("keeps a legacy agent-owned recovery action readable without scheduling another takeover wake", async () => { const { companyId, agentId, issueId } = await seedStrandedIssueFixture({ status: "in_progress", diff --git a/server/src/services/recovery/service.ts b/server/src/services/recovery/service.ts index 961d17e110..61f266ad0c 100644 --- a/server/src/services/recovery/service.ts +++ b/server/src/services/recovery/service.ts @@ -220,6 +220,30 @@ export function resolveSuccessfulRunMissingStateMaxAttempts( return SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS; } +// SPC-21314 deficiency #3 / SPC-37112 / SPC-39089: an issue with an armed, +// unexpired `executionState.monitor` (persisted denormalized as +// `issue.monitorNextCheckAt`) owns its own wake cadence via +// `tickDueIssueMonitors`. Treating it as "stranded" in +// `reconcileStrandedAssignedIssues` duplicates that cadence and can +// re-escalate on every reconciler tick: SPC-37112 saw an in_progress issue +// with a monitor armed 9 days out rewoken via `issue_continuation_needed` +// 10+ times in ~20 minutes (~1-2 min cadence) before the assignee worked +// around it by force-setting status to `blocked` — itself a known-bad move +// because it silently clears the monitor. `monitorNextCheckAt` is only +// non-null while the monitor is "scheduled" (armed); it is nulled out on +// trigger/clear/exhaustion, so a future value here is a reliable proxy for +// "not cleared, not expired." +export function hasArmedMonitorWake( + issue: { status: string; monitorNextCheckAt: Date | null }, + now: Date, +): boolean { + return ( + (issue.status === "in_progress" || issue.status === "in_review") && + !!issue.monitorNextCheckAt && + issue.monitorNextCheckAt.getTime() > now.getTime() + ); +} + type RecoveryWakeupOptions = { source?: "timer" | "assignment" | "on_demand" | "automation"; triggerDetail?: "manual" | "ping" | "callback" | "system"; @@ -4216,6 +4240,7 @@ export function recoveryService( recentProgressExempted: 0, operatorCancelExempted: 0, onboardingFirstTaskExempted: 0, + armedMonitorExempted: 0, skipped: 0, issueIds: [] as string[], }; @@ -4278,6 +4303,12 @@ export function recoveryService( continue; } + if (hasArmedMonitorWake(issue, new Date())) { + result.armedMonitorExempted += 1; + result.skipped += 1; + continue; + } + let latestRun = await getLatestIssueRun(issue.companyId, issue.id); const agent = await getAgent(agentId); diff --git a/server/src/services/recovery/successful-run-missing-state-cap.test.ts b/server/src/services/recovery/successful-run-missing-state-cap.test.ts index 197d1dbbce..2496e2e7e6 100644 --- a/server/src/services/recovery/successful-run-missing-state-cap.test.ts +++ b/server/src/services/recovery/successful-run-missing-state-cap.test.ts @@ -3,6 +3,7 @@ import { SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS, SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS_CEILING, SUCCESSFUL_RUN_MISSING_STATE_MAX_ATTEMPTS_DEFAULT, + hasArmedMonitorWake, parseSuccessfulRunMissingStateMaxAttempts, resolveSuccessfulRunMissingStateMaxAttempts, } from "./service.js"; @@ -74,3 +75,58 @@ describe("resolveSuccessfulRunMissingStateMaxAttempts", () => { ); }); }); + +// SPC-21314 deficiency #3 / SPC-37112 / SPC-39089: reconcileStrandedAssignedIssues +// must not treat an issue as stranded while it has a legitimately armed, +// unexpired monitor wake — that cadence is owned by tickDueIssueMonitors, not +// the stranded-issue reconciler. +describe("hasArmedMonitorWake", () => { + const now = new Date("2026-09-09T00:00:00.000Z"); + const nineDaysOut = new Date("2026-09-18T00:00:00.000Z"); + const oneMinuteAgo = new Date("2026-09-08T23:59:00.000Z"); + + it("is true for an in_progress issue with a future monitor wake", () => { + expect( + hasArmedMonitorWake( + { status: "in_progress", monitorNextCheckAt: nineDaysOut }, + now, + ), + ).toBe(true); + }); + + it("is true for an in_review issue with a future monitor wake", () => { + expect( + hasArmedMonitorWake( + { status: "in_review", monitorNextCheckAt: nineDaysOut }, + now, + ), + ).toBe(true); + }); + + it("is false once the monitor wake is in the past (due, not armed)", () => { + expect( + hasArmedMonitorWake( + { status: "in_progress", monitorNextCheckAt: oneMinuteAgo }, + now, + ), + ).toBe(false); + }); + + it("is false when there is no scheduled monitor", () => { + expect( + hasArmedMonitorWake({ status: "in_progress", monitorNextCheckAt: null }, now), + ).toBe(false); + }); + + it("is false for statuses the monitor cannot be scheduled on", () => { + expect( + hasArmedMonitorWake({ status: "todo", monitorNextCheckAt: nineDaysOut }, now), + ).toBe(false); + expect( + hasArmedMonitorWake({ status: "blocked", monitorNextCheckAt: nineDaysOut }, now), + ).toBe(false); + expect( + hasArmedMonitorWake({ status: "done", monitorNextCheckAt: nineDaysOut }, now), + ).toBe(false); + }); +});