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 <noreply@paperclip.ing>
This commit is contained in:
parent
12a4f16257
commit
b5481a0417
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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({
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue