This commit is contained in:
notandrewblejde 2026-09-13 12:04:16 +00:00 committed by GitHub
commit 59f4e6629f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 439 additions and 2 deletions

View File

@ -238,6 +238,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 }
@ -1287,6 +1289,7 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => {
kind?: string;
previousOwnerAgentId?: string | null;
returnOwnerAgentId?: string | null;
maxAttempts?: number | null;
}) {
const action = await waitForValue(async () =>
db
@ -1317,7 +1320,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,
@ -5721,6 +5724,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,
@ -5851,6 +5855,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,
@ -5859,6 +5864,129 @@ 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("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",
@ -6663,6 +6791,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",

View File

@ -175,6 +175,77 @@ export const STRANDED_RECENT_PROGRESS_EXEMPTION_MS = Math.max(
Number(process.env.STRANDED_RECENT_PROGRESS_EXEMPTION_MS) || 30 * 60 * 1000,
);
// 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,
);
// 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;
}
// 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";
@ -2522,7 +2593,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,
});
@ -4169,6 +4244,7 @@ export function recoveryService(
recentProgressExempted: 0,
operatorCancelExempted: 0,
onboardingFirstTaskExempted: 0,
armedMonitorExempted: 0,
skipped: 0,
issueIds: [] as string[],
};
@ -4243,6 +4319,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);
// A native chat can finish between the earlier settlement read and this
// fresh run read, before its response is materialized. Its trusted
@ -4977,6 +5059,34 @@ 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) {
const existingActiveMaxAttempts = resolveSuccessfulRunMissingStateMaxAttempts(
existingActive.maxAttempts,
);
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({
issue,
previousStatus: "in_progress",

View File

@ -0,0 +1,132 @@
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,
hasArmedMonitorWake,
parseSuccessfulRunMissingStateMaxAttempts,
resolveSuccessfulRunMissingStateMaxAttempts,
} from "./service.js";
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,
);
});
});
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,
);
});
});
// 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);
});
});