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 <noreply@anthropic.com>
This commit is contained in:
parent
b5481a0417
commit
9869d82c2a
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue