fix(engine): bound planning retries and cap the planning turn
Three changes to the triage planning path.
1. Unclassified planning failures are bounded. specifyTask's catch-all branch —
the one reached by every error the classifiers above do not recognize — restored
the card's claimable status and wrote nothing else: no counter, no
nextRecoveryAt, no park. Triage rediscovery re-admitted the card on the very next
poll, and replaceActiveTaskWorkflowContinuation replaced the terminal work item
with a fresh one carrying no attempt count, so nothing recorded that the task had
already failed N times. It now consumes the same recoveryRetryCount/nextRecoveryAt
budget the transient branch uses (MAX_RECOVERY_RETRIES = 3, 60s/120s/300s jittered
backoff) and parks status:"failed" with a PLANNING_FAILED_EXHAUSTED: error once
spent — status:"failed" is what suppresses rediscovery. Classifying one error
string fixes one symptom; this budget is what makes the NEXT unrecognized error
fail safely instead of looping for a day.
2. The planning turn has a ceiling. Fusion set no timeout on it at all:
workflowStepTimeoutMs covers pre-merge workflow steps only, and the provider SDK's
300s APIConnectionTimeoutError caps time-to-first-byte and is cleared once headers
arrive, after which the stream is uncapped. configureHttpDispatcher, which would
install undici idle timeouts, is only called from pi's CLI entrypoints and never
in the in-process engine. Observed consequence: single attempts ran to 126 minutes,
with failed-attempt durations spread smoothly from 1 to 126 min and no clustering —
the signature of nothing enforcing a bound. New workflow-native planningTimeoutMs
(default 90 min) aborts the session; the failure consumes one bounded attempt.
The default is deliberately generous rather than tight. Successful planning work
items measured over 7 days ran p50 12.7 / p90 39.5 / p99 105.7 minutes, so a
tighter bound would abort legitimate plans and pay for the restart — the churn
this work exists to remove. It bounds hung turns, not slow ones.
3. [event:task:moved] executor tracing dropped from log to debug. It fires on
every dispatch, rebound, requeue, archive and self-healing move across every task,
which made it the loudest line in engine output and buried operator-actionable
events. No test pins the level; the information remains at debug.
Also fixes a test break shipped in 963dba6f80: the review blocking-severity
settings landed inside BUILTIN_REVIEW_REVISION_SETTINGS, whose contents
builtin-workflow-settings-triage.test.ts asserts exactly.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
gsxdsm
7
.changeset/planning-retry-budget-and-timeout.md
Normal file
7
.changeset/planning-retry-budget-and-timeout.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": minor
|
||||
---
|
||||
|
||||
summary: Planning failures now retry with backoff and park after 3 attempts instead of looping forever.
|
||||
category: fix
|
||||
dev: Two bounds on the triage planning path plus a log-level fix. (1) The unclassified-failure branch in `specifyTask` restored the card's claimable status and wrote no counter, no `nextRecoveryAt` and no park, so triage rediscovery re-admitted it every poll indefinitely; it now consumes the shared `recoveryRetryCount`/`nextRecoveryAt` budget (`MAX_RECOVERY_RETRIES` = 3, 60s/120s/300s backoff) and parks `status: "failed"` with a `PLANNING_FAILED_EXHAUSTED:` error once spent. (2) New workflow-native setting `planningTimeoutMs` (default `DEFAULT_PLANNING_TIMEOUT_MS` = 5_400_000, declared in `BUILTIN_TRIAGE_POLICY_SETTINGS`) caps a planning turn — previously nothing did, since `workflowStepTimeoutMs` covers pre-merge steps only and the provider SDK's 300s cap is time-to-first-byte and is cleared once headers arrive; a timeout aborts the session and consumes one bounded attempt. Default is generous by design (successful plans measured p99 ≈ 106 min) — it bounds hung turns, not slow ones. (3) `[event:task:moved]` executor tracing dropped from `log` to `debug`; it fired on every move and was the loudest line in engine output.
|
||||
@@ -5,6 +5,7 @@ import {
|
||||
BUILTIN_REVIEW_REVISION_SETTINGS,
|
||||
BUILTIN_TRIAGE_POLICY_SETTINGS,
|
||||
BUILTIN_WORKFLOW_SETTINGS,
|
||||
DEFAULT_PLANNING_TIMEOUT_MS,
|
||||
renderTriagePolicyPlaceholders,
|
||||
} from "../workflows/builtin-workflow-settings.js";
|
||||
import { MOVED_SETTINGS_KEYS } from "../config/moved-settings.js";
|
||||
@@ -32,6 +33,12 @@ const expectedDefaults: Record<string, { type: string; default: unknown }> = {
|
||||
triageDefaultWorkflowId: { type: "string", default: "" },
|
||||
leanPlanning: { type: "boolean", default: false },
|
||||
autoApproveSpec: { type: "boolean", default: false },
|
||||
/*
|
||||
FNXC:TriagePlanningTimeout 2026-08-10-18:32:
|
||||
Driven off the exported constant, not a literal — same anti-drift rule the maxPostReviewFixes
|
||||
parity anchor documents. The planning turn previously had no Fusion-side ceiling at all.
|
||||
*/
|
||||
planningTimeoutMs: { type: "number", default: DEFAULT_PLANNING_TIMEOUT_MS },
|
||||
};
|
||||
|
||||
describe("workflow-native built-in workflow settings", () => {
|
||||
@@ -59,12 +66,34 @@ describe("workflow-native built-in workflow settings", () => {
|
||||
const movedIds = new Set(BUILTIN_MOVED_WORKFLOW_SETTINGS.map((setting) => setting.id));
|
||||
const movedKeyIds = new Set(MOVED_SETTINGS_KEYS);
|
||||
|
||||
/*
|
||||
FNXC:ReviewSeverityGate 2026-08-10-18:32:
|
||||
The blocking-severity pair is review-loop policy and belongs in this catalog alongside the
|
||||
revision caps: the caps bound how many times a REVISE may cycle, the thresholds decide whether a
|
||||
REVISE blocks at all. They are enum-typed with defaults, so the number-typed assertions below
|
||||
deliberately continue to cover only the three cap settings.
|
||||
*/
|
||||
expect(BUILTIN_REVIEW_REVISION_SETTINGS.map((setting) => setting.id)).toEqual([
|
||||
"reviewerInlineFixes",
|
||||
"planReviewMaxRevisions",
|
||||
"codeReviewMaxRevisions",
|
||||
"planReviewBlockingSeverity",
|
||||
"codeReviewBlockingSeverity",
|
||||
"planReviewReplanCap",
|
||||
]);
|
||||
for (const id of ["planReviewBlockingSeverity", "codeReviewBlockingSeverity"]) {
|
||||
const setting = revisionById.get(id);
|
||||
expect(setting, `${id} should be declared`).toBeDefined();
|
||||
expect(setting?.type).toBe("enum");
|
||||
// Defaulted (unlike the caps): the gate is always active, with "any" restoring pre-gate blocking.
|
||||
expect(setting).toHaveProperty("default");
|
||||
expect(setting?.options?.map((option) => option.value)).toContain("any");
|
||||
expect(fullIds.has(id), `${id} should be in the full built-in catalog`).toBe(true);
|
||||
expect(movedIds.has(id), `${id} should not be in the moved-key catalog`).toBe(false);
|
||||
expect(movedKeyIds.has(id), `${id} should not be in MOVED_SETTINGS_KEYS`).toBe(false);
|
||||
}
|
||||
expect(revisionById.get("planReviewBlockingSeverity")?.default).toBe("high");
|
||||
expect(revisionById.get("codeReviewBlockingSeverity")?.default).toBe("critical");
|
||||
const inlineFixes = revisionById.get("reviewerInlineFixes");
|
||||
expect(inlineFixes).toMatchObject({
|
||||
type: "boolean",
|
||||
|
||||
@@ -293,6 +293,7 @@ export {
|
||||
BUILTIN_TRIAGE_POLICY_SETTINGS,
|
||||
BUILTIN_OVERSIGHT_SETTINGS,
|
||||
DEFAULT_PLANNER_OVERSEER_EXECUTOR_STUCK_AFTER_MS,
|
||||
DEFAULT_PLANNING_TIMEOUT_MS,
|
||||
PLANNER_HEARTBEAT_PATROL_ENABLED_SETTING_ID,
|
||||
renderTriagePolicyPlaceholders,
|
||||
} from "./workflows/builtin-workflow-settings.js";
|
||||
|
||||
@@ -338,6 +338,7 @@ export {
|
||||
BUILTIN_TRIAGE_POLICY_SETTINGS,
|
||||
BUILTIN_OVERSIGHT_SETTINGS,
|
||||
DEFAULT_MAX_POST_REVIEW_FIXES,
|
||||
DEFAULT_PLANNING_TIMEOUT_MS,
|
||||
DEFAULT_PLANNER_OVERSEER_EXECUTOR_STUCK_AFTER_MS,
|
||||
PLANNER_HEARTBEAT_PATROL_ENABLED_SETTING_ID,
|
||||
renderTriagePolicyPlaceholders,
|
||||
|
||||
@@ -2434,6 +2434,18 @@ export interface Settings extends GlobalSettings, ProjectSettings {
|
||||
leanPlanning?: boolean;
|
||||
/** Auto-approve generated specs and skip the independent spec reviewer. */
|
||||
autoApproveSpec?: boolean;
|
||||
/** Wall-clock timeout (ms) for a single triage planning/specification AI turn.
|
||||
*
|
||||
* FNXC:TriagePlanningTimeout 2026-08-10-18:32:
|
||||
* Workflow-native (like `leanPlanning`/`autoApproveSpec` above) — it never lived in project or
|
||||
* global settings, so it is declared HERE rather than on `ProjectSettings`, and must never be
|
||||
* added to `MOVED_SETTINGS_KEYS`. `workflowStepTimeoutMs` covers pre-merge workflow STEPS only
|
||||
* and never applied to the planning session, which had no Fusion-side ceiling at all: the
|
||||
* provider SDK's 300s `APIConnectionTimeoutError` caps time-to-first-byte and is cleared once
|
||||
* headers arrive, so a hung stream ran unbounded (observed: 126 minutes). On timeout the session
|
||||
* is aborted and the failure consumes one attempt of the bounded planning retry budget.
|
||||
* Default: {@link DEFAULT_PLANNING_TIMEOUT_MS}. */
|
||||
planningTimeoutMs?: number;
|
||||
/** Index signature for dynamic settings access */
|
||||
[key: string]: unknown;
|
||||
}
|
||||
|
||||
@@ -1,4 +1,14 @@
|
||||
import { THINKING_LEVELS, type Settings } from "../types.js";
|
||||
|
||||
/*
|
||||
FNXC:TriagePlanningTimeout 2026-08-10-18:32:
|
||||
Single source for the planning-turn ceiling, mirroring DEFAULT_MAX_POST_REVIEW_FIXES: the declaration
|
||||
default and the engine's `settings.planningTimeoutMs ?? N` read site must not drift into two separate
|
||||
literals. 90 minutes is deliberately generous — successful planning work items measured over 7 days
|
||||
ran p50 12.7 / p90 39.5 / p99 105.7 minutes, so a tighter bound would abort legitimate plans and pay
|
||||
for the restart, which is the churn this bound exists to remove.
|
||||
*/
|
||||
export const DEFAULT_PLANNING_TIMEOUT_MS = 5_400_000;
|
||||
import {
|
||||
DEFAULT_CODE_REVIEW_BLOCKING_SEVERITY,
|
||||
DEFAULT_PLAN_REVIEW_BLOCKING_SEVERITY,
|
||||
@@ -509,6 +519,25 @@ export const BUILTIN_TRIAGE_POLICY_SETTINGS: WorkflowSettingDefinition[] = [
|
||||
default: false,
|
||||
description: "Auto-approve the generated PROMPT.md and skip the independent spec reviewer.",
|
||||
},
|
||||
{
|
||||
id: "planningTimeoutMs",
|
||||
name: "Planning timeout (ms)",
|
||||
type: "number",
|
||||
minimum: 60_000,
|
||||
integer: true,
|
||||
/*
|
||||
* FNXC:TriagePlanningTimeout 2026-08-10-18:32:
|
||||
* Workflow-native triage policy — it never lived in project/global settings, so it belongs here
|
||||
* and must never be added to MOVED_SETTINGS_KEYS. The planning turn previously had NO
|
||||
* Fusion-side ceiling: `workflowStepTimeoutMs` covers pre-merge workflow STEPS only, and the
|
||||
* provider SDK's 300s cap is time-to-first-byte (cleared once headers arrive), so a hung stream
|
||||
* ran unbounded — observed at 126 minutes, with failed-attempt durations showing a smooth
|
||||
* 1-126 min spread and no clustering, the signature of nothing enforcing a bound.
|
||||
*/
|
||||
default: DEFAULT_PLANNING_TIMEOUT_MS,
|
||||
description:
|
||||
"Maximum time a single planning/specification turn may run before the session is aborted. A timeout consumes one attempt of the bounded planning retry budget. Generous by design — it bounds hung turns, not slow ones.",
|
||||
},
|
||||
];
|
||||
|
||||
export const BUILTIN_REVIEW_REVISION_SETTINGS: WorkflowSettingDefinition[] = [
|
||||
|
||||
@@ -5781,6 +5781,83 @@ describe("taskCreate tool model inheritance", () => {
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:TriagePlanningRetry 2026-08-10-18:32:
|
||||
An UNCLASSIFIED planning failure used to restore the card's claimable status and write nothing
|
||||
else — no counter, no nextRecoveryAt, no park — so triage rediscovery re-admitted it every poll
|
||||
forever. Measured: 48 unrecognized "Request timed out." failures across 10 tasks in 30 hours with
|
||||
0-minute gaps; FN-8950 alone burned 8 attempts over ~8 hours without ever reaching implementation.
|
||||
These pin the bound: retry with backoff while budget remains, then park `failed` for a human.
|
||||
*/
|
||||
it("schedules a bounded backoff retry for an unclassified planning failure", async () => {
|
||||
const task = {
|
||||
id: "FN-GENERIC-RETRY",
|
||||
title: "Generic planning failure",
|
||||
description: "Unclassified planning failure should retry with backoff",
|
||||
column: "triage",
|
||||
status: "planning",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
} as unknown as Task;
|
||||
const store = createMockStore({
|
||||
getTask: vi.fn().mockResolvedValue({ ...task, attachments: [] }),
|
||||
});
|
||||
// Deliberately a string no classifier recognizes.
|
||||
mockCreateFnAgent.mockRejectedValue(new Error("kaboom: something nobody classified"));
|
||||
|
||||
const processor = new TriageProcessor(store, "/test/root", { pollIntervalMs: 100_000 });
|
||||
await processor.specifyTask(task);
|
||||
|
||||
const retryWrite = store.updateTask.mock.calls.find(
|
||||
([id, patch]) => id === "FN-GENERIC-RETRY" && patch?.recoveryRetryCount === 1,
|
||||
);
|
||||
expect(retryWrite, "an unclassified failure must consume a bounded retry").toBeDefined();
|
||||
expect(typeof retryWrite?.[1]?.nextRecoveryAt).toBe("string");
|
||||
// Must NOT park while budget remains.
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith(
|
||||
"FN-GENERIC-RETRY",
|
||||
expect.objectContaining({ status: "failed" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("parks an unclassified planning failure once the retry budget is exhausted", async () => {
|
||||
const task = {
|
||||
id: "FN-GENERIC-EXHAUSTED",
|
||||
title: "Generic planning failure",
|
||||
description: "Unclassified planning failure should park after the budget is spent",
|
||||
column: "triage",
|
||||
status: "planning",
|
||||
recoveryRetryCount: 3,
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
} as unknown as Task;
|
||||
const store = createMockStore({
|
||||
getTask: vi.fn().mockResolvedValue({ ...task, attachments: [] }),
|
||||
});
|
||||
mockCreateFnAgent.mockRejectedValue(new Error("kaboom: something nobody classified"));
|
||||
|
||||
const processor = new TriageProcessor(store, "/test/root", { pollIntervalMs: 100_000 });
|
||||
await processor.specifyTask(task);
|
||||
|
||||
const parkWrite = store.updateTask.mock.calls.find(
|
||||
([id, patch]) => id === "FN-GENERIC-EXHAUSTED" && patch?.status === "failed",
|
||||
);
|
||||
// `status: "failed"` is what suppresses triage rediscovery — without it the card is re-picked.
|
||||
expect(parkWrite, "an exhausted budget must park the card for a human").toBeDefined();
|
||||
expect(parkWrite?.[1]?.error).toContain("PLANNING_FAILED_EXHAUSTED:");
|
||||
expect(parkWrite?.[1]?.error).toContain("kaboom: something nobody classified");
|
||||
expect(parkWrite?.[1]?.recoveryRetryCount).toBeNull();
|
||||
expect(parkWrite?.[1]?.nextRecoveryAt).toBeNull();
|
||||
});
|
||||
|
||||
it("does not overwrite an existing title during terminal fallback exhaustion", async () => {
|
||||
const task = {
|
||||
id: "FN-7961-EXISTING",
|
||||
|
||||
@@ -270,7 +270,15 @@ export function wireExecutorLifecycle(deps: WireExecutorLifecycleDeps): WireExec
|
||||
as a live inertness path rather than defensive dead code.
|
||||
*/
|
||||
deps.store.on("task:moved", ({ task, from, to, source, lanes }) => {
|
||||
executorLog.log(`[event:task:moved] ${task.id}: ${from} → ${to}`);
|
||||
/*
|
||||
FNXC:Diagnostics 2026-08-10-18:32:
|
||||
Per-move tracing is DEBUG. This listener fires on every task:moved event — every dispatch,
|
||||
rebound, requeue, archive and self-healing move across every task — so at `log` level it was the
|
||||
single loudest line in engine output and buried the events an operator actually needs to see.
|
||||
The information is still available at debug level; nothing here is an operator-actionable signal
|
||||
on its own.
|
||||
*/
|
||||
executorLog.debug(`[event:task:moved] ${task.id}: ${from} → ${to}`);
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-31-21:30 (fleet):
|
||||
Lanes come from the EMITTER (see `moves.ts`), not from a resolver called here.
|
||||
@@ -294,7 +302,7 @@ export function wireExecutorLifecycle(deps: WireExecutorLifecycleDeps): WireExec
|
||||
return;
|
||||
}
|
||||
deps.clearWorkflowRerunWatchdog(task.id);
|
||||
executorLog.log(`[event:task:moved] Initiating execute() for ${task.id}`);
|
||||
executorLog.debug(`[event:task:moved] Initiating execute() for ${task.id}`);
|
||||
void (async () => {
|
||||
// FN-5256: if the prior session is still being torn down (because the
|
||||
// task was just moved away from in-progress), wait for the worktree-
|
||||
@@ -303,7 +311,7 @@ export function wireExecutorLifecycle(deps: WireExecutorLifecycleDeps): WireExec
|
||||
// executor's own conflict cleanup against a still-live shell.
|
||||
const pending = deps.pendingTaskDisposals.get(task.id);
|
||||
if (pending) {
|
||||
executorLog.log(`[event:task:moved] Awaiting pending disposal for ${task.id} before dispatch`);
|
||||
executorLog.debug(`[event:task:moved] Awaiting pending disposal for ${task.id} before dispatch`);
|
||||
await pending;
|
||||
}
|
||||
const taskForExecution = await deps.resetMergeStateIfNeeded(task, from);
|
||||
@@ -364,7 +372,7 @@ export function wireExecutorLifecycle(deps: WireExecutorLifecycleDeps): WireExec
|
||||
);
|
||||
} else if (from === wipLane) {
|
||||
if (deps.workflowLifecycleMovesInFlight.has(task.id) && deps.graphRouting.has(task.id)) {
|
||||
executorLog.log(
|
||||
executorLog.debug(
|
||||
`[event:task:moved] Preserving graph run for ${task.id} across its own ${from} → ${to} boundary`,
|
||||
);
|
||||
return;
|
||||
|
||||
@@ -252,7 +252,7 @@ import {
|
||||
import { createRunAuditor, generateSyntheticRunId } from "./util/run-audit.js";
|
||||
import { resolveAndEmitGoalContext } from "./goals/goal-injection-diagnostics.js";
|
||||
import { accumulateSessionTokenUsage } from "./execution/session-token-usage.js";
|
||||
import { finalizePlanningSegment, startPlanningSegment } from "@fusion/core";
|
||||
import { DEFAULT_PLANNING_TIMEOUT_MS, finalizePlanningSegment, startPlanningSegment } from "@fusion/core";
|
||||
import { collectPlanReviewFeedbackHistory, isPlanReviewRevisionLog } from "./plan-review-feedback-history.js";
|
||||
import type { AgentActionGateContext } from "./agents/agent-action-gate.js";
|
||||
import { buildAgentGatedActionSummary } from "./agents/permanent-agent-gating.js";
|
||||
@@ -3291,11 +3291,55 @@ export class TriageProcessor {
|
||||
planReviewFeedbackHistory,
|
||||
},
|
||||
);
|
||||
await promptWithFallback(
|
||||
session,
|
||||
agentPrompt,
|
||||
imageContents.length > 0 ? { images: imageContents } : undefined,
|
||||
);
|
||||
/*
|
||||
FNXC:TriagePlanningTimeout 2026-08-10-18:32:
|
||||
Hard ceiling on the planning turn. Fusion previously set NO timeout here, and the only
|
||||
inherited one is the provider SDK's `APIConnectionTimeoutError` (300s) which caps
|
||||
TIME-TO-FIRST-BYTE only — it is cleared as soon as response headers arrive, after which the
|
||||
stream is uncapped. `configureHttpDispatcher` (which would install undici body/headers idle
|
||||
timeouts) is only called from pi's own CLI entrypoints, never in the in-process engine, so
|
||||
there was no idle timeout either. Measured consequence: single planning attempts ran to 126
|
||||
minutes, and failed-attempt durations showed a smooth 1-126 min spread with no clustering —
|
||||
the signature of nothing enforcing a bound.
|
||||
|
||||
The stuck detector does not cover this: `recordActivity` fires on every streamed token, so a
|
||||
session that emits anything between provider stalls never trips its inactivity threshold.
|
||||
|
||||
Default is deliberately GENEROUS (90 min) rather than tight. Successful planning work items
|
||||
measured over 7 days: p50 12.7 min, p90 39.5 min, p99 105.7 min. A tight ceiling would abort
|
||||
legitimate long plans and pay for the restart, which is the churn this work is removing. The
|
||||
ceiling exists to make a HUNG turn terminate at all, not to discipline slow ones. A timeout
|
||||
here is recoverable, not fatal: it surfaces as a transient error and consumes one attempt of
|
||||
the bounded planning budget below.
|
||||
*/
|
||||
const planningTimeoutMs = Math.max(60_000, settings.planningTimeoutMs ?? DEFAULT_PLANNING_TIMEOUT_MS);
|
||||
let planningTimeoutHandle: ReturnType<typeof setTimeout> | undefined;
|
||||
const planningTimeoutPromise = new Promise<"timeout">((resolveTimeout) => {
|
||||
planningTimeoutHandle = setTimeout(() => resolveTimeout("timeout"), planningTimeoutMs);
|
||||
});
|
||||
try {
|
||||
const planningOutcome = await Promise.race([
|
||||
promptWithFallback(
|
||||
session,
|
||||
agentPrompt,
|
||||
imageContents.length > 0 ? { images: imageContents } : undefined,
|
||||
).then(() => "completed" as const),
|
||||
planningTimeoutPromise,
|
||||
]);
|
||||
if (planningOutcome === "timeout") {
|
||||
planLog.warn(`${task.id}: planning turn exceeded ${planningTimeoutMs}ms — disposing session`);
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
`Planning turn timed out after ${Math.round(planningTimeoutMs / 60_000)} min — aborting session`,
|
||||
).catch(() => undefined);
|
||||
try { session.dispose(); } catch { /* best-effort */ }
|
||||
// Phrased to match the provider-timeout transient pattern so this routes into the
|
||||
// bounded planning retry budget instead of the unclassified-failure park.
|
||||
throw new Error(`Planning request timed out after ${planningTimeoutMs}ms`);
|
||||
}
|
||||
} finally {
|
||||
if (planningTimeoutHandle) clearTimeout(planningTimeoutHandle);
|
||||
}
|
||||
/*
|
||||
FNXC:TriagePlanningRetry 2026-08-03-01:01:
|
||||
Plan Review needs a finite runtime-owned admission-close signal, not a global async-hooks
|
||||
@@ -3807,12 +3851,31 @@ export class TriageProcessor {
|
||||
this.options.onSpecifyError?.(task, err instanceof Error ? err : new Error(errorMessage));
|
||||
return;
|
||||
}
|
||||
// For interrupted recovery states, restore the original triage-held status;
|
||||
// otherwise clear to null so the next poll can re-pick ordinary tasks up.
|
||||
const restoreStatus = this.restoreStatusAfterInterruptedTriageWork(task);
|
||||
await this.updatePlanningStateIfStillCurrent(task, { status: restoreStatus }).catch((restoreErr: unknown) => {
|
||||
const msg = restoreErr instanceof Error ? restoreErr.message : String(restoreErr);
|
||||
planLog.warn(`${task.id}: failed to restore status to '${restoreStatus}' after planning error: ${msg}`);
|
||||
/*
|
||||
FNXC:TriagePlanningRetry 2026-08-10-18:32:
|
||||
UNCLASSIFIED planning failures are bounded. This branch is the catch-all for every error the
|
||||
classifiers above did not recognize, and it used to restore the card's claimable status and
|
||||
write NOTHING else — no counter, no `nextRecoveryAt`, no park. Triage rediscovery therefore
|
||||
re-admitted the card on the very next poll, forever, and `replaceActiveTaskWorkflowContinuation`
|
||||
replaced the terminal work item with a fresh one carrying no attempt count, so nothing anywhere
|
||||
recorded that the task had already failed N times.
|
||||
|
||||
Measured cost of that hole: `"Request timed out."` (unrecognized until the companion fix to
|
||||
`transient-error-patterns.ts`) produced 48 failures across 10 tasks in 30 hours with zero
|
||||
backoff — FN-8950 burned 8 consecutive attempts over ~8 hours and never reached implementation.
|
||||
Classifying that ONE string fixes that ONE symptom; this budget is what makes the NEXT
|
||||
unrecognized error string fail safely instead of looping for a day.
|
||||
|
||||
Deliberately reuses the `recoveryRetryCount`/`nextRecoveryAt` pair (and its 60s/120s/300s
|
||||
jittered backoff) that the transient branch above already uses, rather than adding a parallel
|
||||
counter: the budget answers "this task keeps failing to plan", which is true regardless of
|
||||
which classifier recognized the error, and sharing it avoids a schema migration for a counter
|
||||
that means the same thing. On exhaustion the card is parked `failed` for a human — unlike a
|
||||
transient exhaustion, an unrecognized error has no evidence it is retryable at all.
|
||||
*/
|
||||
const genericDecision = computeRecoveryDecision({
|
||||
recoveryRetryCount: task.recoveryRetryCount,
|
||||
nextRecoveryAt: task.nextRecoveryAt,
|
||||
});
|
||||
planLog.error(`✗ ${task.id} planning failed:`, errorDetail);
|
||||
if (errorStack) {
|
||||
@@ -3821,6 +3884,54 @@ export class TriageProcessor {
|
||||
planLog.warn(`${task.id}: failed to persist specification-failure stack trace: ${msg}`);
|
||||
});
|
||||
}
|
||||
|
||||
if (genericDecision.shouldRetry) {
|
||||
const attempt = genericDecision.nextState.recoveryRetryCount;
|
||||
const delay = formatDelay(genericDecision.delayMs);
|
||||
planLog.warn(`⚡ ${task.id} planning failed — retry ${attempt}/${MAX_RECOVERY_RETRIES} in ${delay}: ${errorMessage}`);
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
`Specification failed (retry ${attempt}/${MAX_RECOVERY_RETRIES} in ${delay}): ${errorMessage}`,
|
||||
).catch((logErr: unknown) => {
|
||||
const msg = logErr instanceof Error ? logErr.message : String(logErr);
|
||||
planLog.warn(`${task.id}: failed to log planning-failure retry entry: ${msg}`);
|
||||
});
|
||||
// For interrupted recovery states, restore the original triage-held status;
|
||||
// otherwise clear to null so the next poll can re-pick ordinary tasks up.
|
||||
const restoreStatus = this.restoreStatusAfterInterruptedTriageWork(task);
|
||||
await this.updatePlanningStateIfStillCurrent(task, {
|
||||
status: restoreStatus,
|
||||
recoveryRetryCount: genericDecision.nextState.recoveryRetryCount,
|
||||
nextRecoveryAt: genericDecision.nextState.nextRecoveryAt,
|
||||
}).catch((restoreErr: unknown) => {
|
||||
const msg = restoreErr instanceof Error ? restoreErr.message : String(restoreErr);
|
||||
planLog.warn(`${task.id}: failed to restore status to '${restoreStatus}' after planning error: ${msg}`);
|
||||
});
|
||||
this.options.onSpecifyError?.(task, err instanceof Error ? err : new Error(errorMessage));
|
||||
return;
|
||||
}
|
||||
|
||||
/*
|
||||
Budget exhausted — park for a human. Mirrors the in-file `maxStuckKills` park
|
||||
(status `failed` + a prefixed error a human can grep) so the card stops being re-picked:
|
||||
`status: "failed"` is what suppresses triage rediscovery.
|
||||
*/
|
||||
const exhaustedMessage = `PLANNING_FAILED_EXHAUSTED: specification failed ${MAX_RECOVERY_RETRIES} times — last error: ${errorMessage}`;
|
||||
planLog.error(`✗ ${task.id} planning retries exhausted (${MAX_RECOVERY_RETRIES} attempts) — parking failed: ${errorMessage}`);
|
||||
await this.store.logEntry(task.id, exhaustedMessage).catch((logErr: unknown) => {
|
||||
const msg = logErr instanceof Error ? logErr.message : String(logErr);
|
||||
planLog.warn(`${task.id}: failed to log planning-retries-exhausted entry: ${msg}`);
|
||||
});
|
||||
await this.updatePlanningStateIfStillCurrent(task, {
|
||||
status: "failed",
|
||||
error: exhaustedMessage,
|
||||
recoveryRetryCount: null,
|
||||
nextRecoveryAt: null,
|
||||
}).catch((restoreErr: unknown) => {
|
||||
const msg = restoreErr instanceof Error ? restoreErr.message : String(restoreErr);
|
||||
planLog.warn(`${task.id}: failed to park task after planning retries exhausted: ${msg}`);
|
||||
});
|
||||
await this.backfillBlankTitleAfterTerminalTriageFailure(task);
|
||||
this.options.onSpecifyError?.(task, err instanceof Error ? err : new Error(errorMessage));
|
||||
}
|
||||
} finally {
|
||||
|
||||
Reference in New Issue
Block a user