diff --git a/.changeset/reviewer-provider-error-escalation.md b/.changeset/reviewer-provider-error-escalation.md
new file mode 100644
index 0000000000..9d42486857
--- /dev/null
+++ b/.changeset/reviewer-provider-error-escalation.md
@@ -0,0 +1,7 @@
+---
+"@runfusion/fusion": patch
+---
+
+summary: Stop reviewer rate limits and network blips from looping and spamming the task log.
+category: fix
+dev: The reviewer was the only AI lane that never classified provider errors, so a 429 became an `UNAVAILABLE` verdict. With no validator fallback configured the fallback ladder re-ran the SAME model instantly, and `fn_review_step` told the agent "code review remains blocking; retry once" — bounding the loop with prompt text rather than code. The tool's catch-all also swallowed the error into tool output, so `withRateLimitRetry`, `UsageLimitPauser`, and `RetryStormError` never fired. Reviewer provider failures now throw `ReviewerProviderError` (usage-limit → global pause; transient → bounded recovery), transient blips retry in-lane with jittered backoff via `withRetry`, code review gets a real `MAX_CODE_REVIEW_UNAVAILABLE_RETRIES` counter, and the fatal escapes `agentWork` via `throwDeferredReviewerFatal` (pi-agent-core converts tool throws into `tool_error` results, so a tool cannot throw out of `session.prompt()`). Separately, `AgentLogType` gains `status` for complete engine messages: `text` means "streamed delta" and is re-glued with `join("")`, which is why N standalone markers rendered as one run-on string.
\ No newline at end of file
diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts
index 87fa7187c3..b8838bd537 100644
--- a/packages/core/src/types.ts
+++ b/packages/core/src/types.ts
@@ -1275,8 +1275,15 @@ export interface ActivityLogEntry {
/** The set of agent roles that produce log entries. */
export type AgentRole = "triage" | "executor" | "reviewer" | "merger";
-/** The discriminator for agent log entry types. */
-export type AgentLogType = "text" | "tool" | "thinking" | "tool_result" | "tool_error";
+/*
+FNXC:AgentLog-EntryTypes 2026-07-15-11:20:
+`text` means a STREAMED DELTA FRAGMENT: renderers re-glue consecutive `text` rows with `join("")` and no separator, because that is the only way to reconstitute a streamed message (the FN-5787/5789/5803 streamed-spacing lineage). `AgentLogger` is the only producer of true deltas.
+
+`status` means a COMPLETE, SELF-CONTAINED engine message (e.g. "Reviewer using model: x/y", "Deterministic merge verification passed") written directly by an engine lane rather than streamed from a model. It exists because engine lanes previously wrote these as `text`, so N consecutive standalone messages were glued edge-to-edge into one run-on string under an accurate-but-misleading "N entries" header.
+
+Never emit `status` for model-streamed output, and never emit `text` for a whole standalone message. Renderers must render each `status` row as its own block and must never `join("")` them. Rows written before this type existed persist as `text`, so read paths that resolve engine markers out of the log must accept BOTH types (see dashboard effective-model-resolution.ts).
+*/
+export type AgentLogType = "text" | "status" | "tool" | "thinking" | "tool_result" | "tool_error";
/** A single chunk of agent output persisted to disk (JSONL in agent.log). */
export interface AgentLogEntry {
@@ -1284,9 +1291,9 @@ export interface AgentLogEntry {
timestamp: string;
/** The task this log entry belongs to. */
taskId: string;
- /** The text content (delta for "text"/"thinking", tool name for "tool"/"tool_result"/"tool_error"). */
+ /** The text content (delta for "text"/"thinking", complete message for "status", tool name for "tool"/"tool_result"/"tool_error"). */
text: string;
- /** The kind of entry — text delta, tool invocation marker, thinking block, tool result, or tool error. */
+ /** The kind of entry — streamed text delta, standalone engine status message, tool invocation marker, thinking block, tool result, or tool error. */
type: AgentLogType;
/** For tool entries: human-readable summary of tool args (e.g. file path, command).
* For tool_result/tool_error: summary of the result or error message. */
diff --git a/packages/dashboard/app/components/TaskChatTab.tsx b/packages/dashboard/app/components/TaskChatTab.tsx
index dbb931e640..a9c8d6ccbf 100644
--- a/packages/dashboard/app/components/TaskChatTab.tsx
+++ b/packages/dashboard/app/components/TaskChatTab.tsx
@@ -367,9 +367,26 @@ function segmentGroupEntries(entries: AgentLogEntry[]): TaskChatSegment[] {
continue;
}
+ /*
+ FNXC:TaskChat-StatusEntries 2026-07-15-11:20:
+ A `status` row is a COMPLETE engine message, so it gets its own segment and is never merged with a neighbour. Merging is only correct for `text`, whose rows are streamed delta fragments that `TaskChatText` re-glues with `join("")`.
+
+ This is why a provider outage rendered as one run-on string: engine markers were written as `text`, so N standalone messages ("Reviewer using model: x/y" ×14) were glued edge-to-edge under a "14 entries" header. Fixing it with a separator in `TaskChatText` would corrupt legitimate streamed text (the FN-5787/5789/5803 regression lineage) — the split has to happen here, on the type.
+ */
+ if (entry.type === "status") {
+ segments.push({ kind: "text", entries: [entry], startIndex: index });
+ index += 1;
+ continue;
+ }
+
const startIndex = index;
const textEntries: AgentLogEntry[] = [];
- while (index < entries.length && !isToolLikeEntry(entries[index]) && entries[index].type !== "thinking") {
+ while (
+ index < entries.length
+ && !isToolLikeEntry(entries[index])
+ && entries[index].type !== "thinking"
+ && entries[index].type !== "status"
+ ) {
textEntries.push(entries[index]);
index += 1;
}
diff --git a/packages/dashboard/app/components/__tests__/TaskChatTab.test.tsx b/packages/dashboard/app/components/__tests__/TaskChatTab.test.tsx
index ce41fec122..7d7a984280 100644
--- a/packages/dashboard/app/components/__tests__/TaskChatTab.test.tsx
+++ b/packages/dashboard/app/components/__tests__/TaskChatTab.test.tsx
@@ -2889,3 +2889,71 @@ describe("TaskChatTab", () => {
expect(css).toContain(".task-chat-entry--user");
});
});
+
+/*
+FNXC:TaskChat-StatusEntries 2026-07-15-11:20:
+Regression coverage for standalone engine messages rendering as one run-on string.
+
+## Symptom Verification
+Original symptom: a Reviewer card headed "14 entries" rendered as
+"Reviewer using model: umans/umans-kimi-k2.7Reviewer using model: umans/umans-kimi-k2.7..." —
+14 complete messages glued edge-to-edge with no separator.
+Exact reproduction: consecutive same-role standalone engine rows in one group.
+Assertion it is gone: each `status` row renders as its own block, and no rendered block contains
+two concatenated messages.
+
+## Surface Enumeration
+- `status` rows must never be glued to each other (the reported case).
+- `status` rows must never be glued to adjacent `text` rows either.
+- `text` rows MUST still be glued with no separator — they are streamed deltas, and inserting a
+ separator here is the FN-5787/5789/5803 streamed-spacing regression this must not reintroduce.
+- Model resolution reads markers out of the log and must accept `status` AND legacy `text` rows
+ (covered in effective-model-resolution.test.ts).
+*/
+describe("TaskChatTab — standalone status entries are not glued like streamed deltas", () => {
+ const marker = "Reviewer using model: umans/umans-kimi-k2.7";
+
+ it("renders repeated standalone status messages as separate blocks", () => {
+ mockLogs([
+ makeEntry({ agent: "reviewer", type: "status", text: marker }),
+ makeEntry({ agent: "reviewer", type: "status", text: marker }),
+ makeEntry({ agent: "reviewer", type: "status", text: marker }),
+ ]);
+
+ render();
+
+ const blocks = Array.from(document.querySelectorAll(".task-chat-markdown"));
+ const rendered = blocks.map((el) => el.textContent ?? "");
+ // The bug: one block containing the message repeated with no separator.
+ expect(rendered.some((text) => text.includes(`${marker}${marker}`))).toBe(false);
+ expect(rendered.filter((text) => text.trim() === marker)).toHaveLength(3);
+ });
+
+ it("does not glue a status message onto an adjacent streamed text block", () => {
+ mockLogs([
+ makeEntry({ agent: "reviewer", type: "status", text: marker }),
+ makeEntry({ agent: "reviewer", type: "text", text: "Verdict: " }),
+ makeEntry({ agent: "reviewer", type: "text", text: "APPROVE" }),
+ ]);
+
+ render();
+
+ const rendered = Array.from(document.querySelectorAll(".task-chat-markdown")).map((el) => el.textContent ?? "");
+ expect(rendered.some((text) => text.includes(`${marker}Verdict`))).toBe(false);
+ // Streamed deltas around it still re-glue with NO separator.
+ expect(rendered.some((text) => text.includes("Verdict: APPROVE"))).toBe(true);
+ });
+
+ it("still joins consecutive streamed text deltas with no separator", () => {
+ mockLogs([
+ makeEntry({ agent: "reviewer", type: "text", text: "review" }),
+ makeEntry({ agent: "reviewer", type: "text", text: "ing the" }),
+ makeEntry({ agent: "reviewer", type: "text", text: " diff" }),
+ ]);
+
+ render();
+
+ const rendered = Array.from(document.querySelectorAll(".task-chat-markdown")).map((el) => el.textContent ?? "");
+ expect(rendered.some((text) => text.includes("reviewing the diff"))).toBe(true);
+ });
+});
diff --git a/packages/dashboard/app/components/__tests__/effective-model-resolution.test.ts b/packages/dashboard/app/components/__tests__/effective-model-resolution.test.ts
index 4cad069adf..161b317064 100644
--- a/packages/dashboard/app/components/__tests__/effective-model-resolution.test.ts
+++ b/packages/dashboard/app/components/__tests__/effective-model-resolution.test.ts
@@ -77,6 +77,32 @@ describe("effective model resolution", () => {
expect(extractPlanningModelFromLog(entries)).toEqual({ provider: "triage-provider", modelId: "triage-model" });
});
+ /*
+ FNXC:TaskLogModelThinking 2026-07-15-11:20:
+ Engine lanes now write the "using model" markers as standalone `status` rows so they are not
+ glued together like streamed deltas. Resolution must read BOTH types: `status` for new rows,
+ `text` for the markers already persisted in every existing task's log. Accepting only one type
+ silently blanks the provider icons / effective-model headers on one side of that cutover.
+ */
+ it("resolves model markers from both new status rows and legacy text rows", () => {
+ const statusEntries = [
+ { ...log("executor", "Executor using model: status-provider/status-model"), type: "status" as const },
+ { ...log("reviewer", "Reviewer using model: status-reviewer/status-reviewer-model"), type: "status" as const },
+ { ...log("triage", "Triage using model: status-triage/status-triage-model"), type: "status" as const },
+ ];
+ expect(extractExecutorModelFromLog(statusEntries)).toEqual({ provider: "status-provider", modelId: "status-model" });
+ expect(extractReviewerModelFromLog(statusEntries)).toEqual({ provider: "status-reviewer", modelId: "status-reviewer-model" });
+ expect(extractPlanningModelFromLog(statusEntries)).toEqual({ provider: "status-triage", modelId: "status-triage-model" });
+
+ // Legacy rows written before the `status` type existed still resolve.
+ const legacyEntries = [log("executor", "Executor using model: legacy-provider/legacy-model")];
+ expect(extractExecutorModelFromLog(legacyEntries)).toEqual({ provider: "legacy-provider", modelId: "legacy-model" });
+
+ // A tool row is still never a model marker, whatever its text says.
+ const toolEntries = [{ ...log("executor", "Executor using model: tool-provider/tool-model"), type: "tool" as const }];
+ expect(extractExecutorModelFromLog(toolEntries)).toBeNull();
+ });
+
it("parses runtime model markers for all roles while ignoring parenthesized diagnostics", () => {
expect(parseRuntimeModelMarker("Triage using model: google/gemini-pro", "Triage")).toEqual({ provider: "google", modelId: "gemini-pro" });
expect(parseRuntimeModelMarker("Executor using model: openai/gpt-4o (thinking effort: high)", "Executor")).toEqual({ provider: "openai", modelId: "gpt-4o" });
diff --git a/packages/dashboard/app/components/effective-model-resolution.ts b/packages/dashboard/app/components/effective-model-resolution.ts
index f6dc7ffb0d..6e9fbd6540 100644
--- a/packages/dashboard/app/components/effective-model-resolution.ts
+++ b/packages/dashboard/app/components/effective-model-resolution.ts
@@ -34,6 +34,14 @@ Runtime "using model" markers may append parenthesized diagnostics such as think
*/
const MODEL_MARKER_PATTERN = /^(Triage|Executor|Reviewer) using model: ([^/\s]+)\/(.+?)(?:\s+\([^)]*\))*$/;
+/*
+FNXC:TaskLogModelThinking 2026-07-15-11:20:
+Engine lanes now write standalone messages (including the "using model" markers) as `status` rather than `text`, so complete messages are never glued together like streamed deltas. Model resolution must accept BOTH: `status` for markers written after that change, `text` for the rows already persisted in every existing task's log. Dropping `text` here would silently blank the provider icons and effective-model headers on historical tasks.
+*/
+function isEngineMarkerEntryType(type: AgentLogEntry["type"]): boolean {
+ return type === "status" || type === "text";
+}
+
export function parseRuntimeModelMarker(text: string, role: "Triage" | "Executor" | "Reviewer"): { provider: string; modelId: string } | null {
const match = text.match(MODEL_MARKER_PATTERN);
if (!match || match[1] !== role) return null;
@@ -43,7 +51,7 @@ export function parseRuntimeModelMarker(text: string, role: "Triage" | "Executor
export function extractExecutorModelFromLog(entries: AgentLogEntry[]): { provider: string; modelId: string } | null {
let result: { provider: string; modelId: string } | null = null;
entries.forEach((entry) => {
- if (entry.agent !== "executor" || entry.type !== "text") return;
+ if (entry.agent !== "executor" || !isEngineMarkerEntryType(entry.type)) return;
const match = parseRuntimeModelMarker(entry.text, "Executor");
if (match) {
result = match;
@@ -55,7 +63,7 @@ export function extractExecutorModelFromLog(entries: AgentLogEntry[]): { provide
export function extractReviewerModelFromLog(entries: AgentLogEntry[]): { provider: string; modelId: string } | null {
let result: { provider: string; modelId: string } | null = null;
entries.forEach((entry) => {
- if (entry.agent !== "reviewer" || entry.type !== "text") return;
+ if (entry.agent !== "reviewer" || !isEngineMarkerEntryType(entry.type)) return;
const match = parseRuntimeModelMarker(entry.text, "Reviewer");
if (match) {
result = match;
@@ -143,7 +151,7 @@ export function resolveEffectiveValidator(
export function extractPlanningModelFromLog(entries: AgentLogEntry[]): { provider: string; modelId: string } | null {
let result: { provider: string; modelId: string } | null = null;
entries.forEach((entry) => {
- if (entry.agent !== "triage" || entry.type !== "text") return;
+ if (entry.agent !== "triage" || !isEngineMarkerEntryType(entry.type)) return;
const match = parseRuntimeModelMarker(entry.text, "Triage");
if (match) {
result = match;
diff --git a/packages/engine/src/__tests__/executor-browser-verification.test.ts b/packages/engine/src/__tests__/executor-browser-verification.test.ts
index 4470312dc7..df4f38887a 100644
--- a/packages/engine/src/__tests__/executor-browser-verification.test.ts
+++ b/packages/engine/src/__tests__/executor-browser-verification.test.ts
@@ -230,21 +230,21 @@ describe("browser-verification workflow-step browser capability", () => {
expect(store.appendAgentLog).toHaveBeenCalledWith(
"FN-7130",
expect.stringContaining("[browser-verification] starting browser verification"),
- "text",
+ "status",
undefined,
"reviewer",
);
expect(store.appendAgentLog).toHaveBeenCalledWith(
"FN-7130",
"[browser-verification] agent-browser available — version agent-browser 9.9.9",
- "text",
+ "status",
undefined,
"reviewer",
);
expect(store.appendAgentLog).toHaveBeenCalledWith(
"FN-7130",
"[browser-verification] finished browser verification for task FN-7130: verdict APPROVE",
- "text",
+ "status",
undefined,
"reviewer",
);
@@ -276,7 +276,7 @@ describe("browser-verification workflow-step browser capability", () => {
expect(result.success).toBe(true);
expect(formatAgentBrowserAvailabilityLog({ available: false, reason: "not installed" })).toBe(warning);
expect(store.logEntry).toHaveBeenCalledWith("FN-7130", warning);
- expect(store.appendAgentLog).toHaveBeenCalledWith("FN-7130", warning, "text", undefined, "reviewer");
+ expect(store.appendAgentLog).toHaveBeenCalledWith("FN-7130", warning, "status", undefined, "reviewer");
});
it("keeps flag-absent prompt steps byte-inert for browser logging and skills", async () => {
diff --git a/packages/engine/src/__tests__/executor-model-marker.test.ts b/packages/engine/src/__tests__/executor-model-marker.test.ts
index 1f9e63b72f..38c28c291b 100644
--- a/packages/engine/src/__tests__/executor-model-marker.test.ts
+++ b/packages/engine/src/__tests__/executor-model-marker.test.ts
@@ -51,10 +51,13 @@ describe("TaskExecutor model marker logging", () => {
undefined,
expect.any(Object),
);
+ // FNXC:AgentLog-EntryTypes 2026-07-15-11:20: the marker is a complete standalone message,
+ // so it is a `status` row — `text` means "streamed delta fragment" and gets glued to its
+ // neighbours with no separator.
expect(store.appendAgentLog).toHaveBeenCalledWith(
"FN-7370",
"Executor using model: mock-provider/mock-model (thinking effort: high)",
- "text",
+ "status",
undefined,
"executor",
);
diff --git a/packages/engine/src/__tests__/fallback-model-observer.test.ts b/packages/engine/src/__tests__/fallback-model-observer.test.ts
index dc54aa5dd1..699ba05ba0 100644
--- a/packages/engine/src/__tests__/fallback-model-observer.test.ts
+++ b/packages/engine/src/__tests__/fallback-model-observer.test.ts
@@ -40,7 +40,7 @@ describe("createFallbackModelObserver", () => {
expect(store.appendAgentLog).toHaveBeenCalledWith(
"FN-123",
expectedMessage,
- "text",
+ "status",
undefined,
"Executor Agent",
);
diff --git a/packages/engine/src/__tests__/merger-file-scope-invariant.test.ts b/packages/engine/src/__tests__/merger-file-scope-invariant.test.ts
index 6075fa6f82..7946ab2945 100644
--- a/packages/engine/src/__tests__/merger-file-scope-invariant.test.ts
+++ b/packages/engine/src/__tests__/merger-file-scope-invariant.test.ts
@@ -203,7 +203,7 @@ describe("assertSquashOverlapsFileScope", () => {
expect(store.appendAgentLog).toHaveBeenCalledWith(
"FN-4073",
"file-scope invariant bypassed via scopeOverride",
- "text",
+ "status",
undefined,
"merger",
);
@@ -227,7 +227,7 @@ describe("assertSquashOverlapsFileScope", () => {
expect(store.appendAgentLog).toHaveBeenCalledWith(
"FN-4073",
"file-scope invariant bypassed via scopeOverride — reason: hotfix",
- "text",
+ "status",
undefined,
"merger",
);
@@ -262,7 +262,7 @@ describe("enforceSquashFileScopeInvariant audit emission", () => {
expect(store.appendAgentLog).toHaveBeenCalledWith(
"FN-4073",
expect.stringContaining("Warning only — continuing merge."),
- "text",
+ "status",
expect.stringContaining("declaredScope:"),
"merger",
);
@@ -321,7 +321,7 @@ describe("enforceSquashFileScopeInvariant audit emission", () => {
expect(store.appendAgentLog).toHaveBeenCalledWith(
"FN-4073",
expect.stringContaining("File-scope invariant violation"),
- "text",
+ "status",
expect.stringContaining("declaredScope:"),
"merger",
);
@@ -380,7 +380,7 @@ describe("file-scope invariant wiring", () => {
expect(store.appendAgentLog).toHaveBeenCalledWith(
"FN-4073",
expect.stringContaining("Warning only — continuing merge."),
- "text",
+ "status",
expect.stringContaining("declaredScope:"),
"merger",
);
@@ -460,7 +460,7 @@ describe("file-scope invariant wiring", () => {
expect(store.appendAgentLog).toHaveBeenCalledWith(
"FN-4073",
"file-scope invariant bypassed via scopeOverride — reason: hotfix",
- "text",
+ "status",
undefined,
"merger",
);
@@ -500,7 +500,7 @@ describe("file-scope invariant wiring", () => {
expect(store.appendAgentLog).toHaveBeenCalledWith(
"FN-4073",
expect.stringContaining("Warning only — continuing merge."),
- "text",
+ "status",
expect.stringContaining("stagedFiles:"),
"merger",
);
diff --git a/packages/engine/src/__tests__/merger-merge-lifecycle.test.ts b/packages/engine/src/__tests__/merger-merge-lifecycle.test.ts
index cf63605c72..961b8ef546 100644
--- a/packages/engine/src/__tests__/merger-merge-lifecycle.test.ts
+++ b/packages/engine/src/__tests__/merger-merge-lifecycle.test.ts
@@ -2231,7 +2231,7 @@ describe("aiMergeTask — retry logic with escalating strategies", () => {
expect(store.appendAgentLog).toHaveBeenCalledWith(
"FN-050",
expect.stringContaining("Overlap guard detected 1 recent-main overlap file(s) for smart-prefer-main (warn-only)"),
- "text",
+ "status",
undefined,
"merger",
);
@@ -3125,7 +3125,7 @@ describe("aiMergeTask post-squash audit gate", () => {
strategy: "squash",
squashSha: "mergedcommit123",
});
- expect(store.appendAgentLog).toHaveBeenCalledWith("FN-050", "post-squash audit clean", "text", undefined, "merger");
+ expect(store.appendAgentLog).toHaveBeenCalledWith("FN-050", "post-squash audit clean", "status", undefined, "merger");
expect(store.moveTask).toHaveBeenCalledWith("FN-050", "done");
});
@@ -3157,7 +3157,7 @@ describe("aiMergeTask post-squash audit gate", () => {
rangeBaseSha: "basehead123",
rangeHeadSha: "landedcommit002",
});
- expect(store.appendAgentLog).toHaveBeenCalledWith("FN-050", "post-rebase range audit clean", "text", undefined, "merger");
+ expect(store.appendAgentLog).toHaveBeenCalledWith("FN-050", "post-rebase range audit clean", "status", undefined, "merger");
});
it("degrades to squash fallback when no usable base can be resolved on the rebase route", async () => {
@@ -3200,7 +3200,7 @@ describe("aiMergeTask post-squash audit gate", () => {
expect(store.appendAgentLog).toHaveBeenCalledWith(
"FN-050",
expect.stringContaining("post-merge audit degraded to single-commit squash fallback"),
- "text",
+ "status",
undefined,
"merger",
);
@@ -3246,7 +3246,7 @@ describe("aiMergeTask post-squash audit gate", () => {
expect(store.appendAgentLog).toHaveBeenCalledWith(
"FN-050",
expect.stringContaining("post-merge audit degraded to single-commit squash fallback"),
- "text",
+ "status",
undefined,
"merger",
);
@@ -3336,7 +3336,7 @@ describe("aiMergeTask post-squash audit gate", () => {
expect(store.appendAgentLog).toHaveBeenCalledWith(
"FN-050",
expect.stringContaining("post-rebase audit overlap cleared by deterministic verification"),
- "text",
+ "status",
expect.any(String),
"merger",
);
diff --git a/packages/engine/src/__tests__/merger-post-merge-audit-rangebase.test.ts b/packages/engine/src/__tests__/merger-post-merge-audit-rangebase.test.ts
index 018da71f03..36660e00bf 100644
--- a/packages/engine/src/__tests__/merger-post-merge-audit-rangebase.test.ts
+++ b/packages/engine/src/__tests__/merger-post-merge-audit-rangebase.test.ts
@@ -85,7 +85,7 @@ describe("resolvePostMergeAuditInvocation", { timeout: 30_000 }, () => {
expect(appendAgentLog).toHaveBeenCalledWith(
"FN-4961",
expect.stringContaining("from diffBaseRef"),
- "text",
+ "status",
undefined,
"merger",
);
@@ -113,7 +113,7 @@ describe("resolvePostMergeAuditInvocation", { timeout: 30_000 }, () => {
expect(appendAgentLog).toHaveBeenCalledWith(
"FN-4961",
expect.stringContaining("from baseCommitSha"),
- "text",
+ "status",
undefined,
"merger",
);
@@ -141,7 +141,7 @@ describe("resolvePostMergeAuditInvocation", { timeout: 30_000 }, () => {
expect(appendAgentLog).toHaveBeenCalledWith(
"FN-4961",
expect.stringContaining("from merge-base"),
- "text",
+ "status",
undefined,
"merger",
);
@@ -169,7 +169,7 @@ describe("resolvePostMergeAuditInvocation", { timeout: 30_000 }, () => {
expect(appendAgentLog).toHaveBeenCalledWith(
"FN-4961",
expect.stringContaining("post-merge audit degraded to single-commit squash fallback"),
- "text",
+ "status",
undefined,
"merger",
);
diff --git a/packages/engine/src/__tests__/merger-verification.test.ts b/packages/engine/src/__tests__/merger-verification.test.ts
index 16a6684f31..f35f86b297 100644
--- a/packages/engine/src/__tests__/merger-verification.test.ts
+++ b/packages/engine/src/__tests__/merger-verification.test.ts
@@ -895,7 +895,7 @@ describe("aiMergeTask — deterministic merge verification", () => {
expect(store.appendAgentLog).toHaveBeenCalledWith(
"FN-050",
"Running deterministic merge verification (test: vitest run)",
- "text",
+ "status",
undefined,
"merger",
);
@@ -920,7 +920,7 @@ describe("aiMergeTask — deterministic merge verification", () => {
expect(store.appendAgentLog).toHaveBeenCalledWith(
"FN-050",
"Deterministic merge verification passed",
- "text",
+ "status",
undefined,
"merger",
);
diff --git a/packages/engine/src/__tests__/reviewer.test.ts b/packages/engine/src/__tests__/reviewer.test.ts
index 094a8ba3f6..98cc4b0206 100644
--- a/packages/engine/src/__tests__/reviewer.test.ts
+++ b/packages/engine/src/__tests__/reviewer.test.ts
@@ -25,7 +25,7 @@ vi.mock("../pi.js", () => ({
}));
import { resolveAgentPrompt } from "@fusion/core";
-import { reviewStep } from "../reviewer.js";
+import { reviewStep, ReviewerProviderError } from "../reviewer.js";
import { createFnAgent, promptWithFallback } from "../pi.js";
const DEFAULT_REVIEWER_PROMPT = resolveAgentPrompt("reviewer");
@@ -237,10 +237,13 @@ describe("reviewStep — model settings threading", () => {
"FN-100",
"Reviewer using model: mock-provider/mock-model (thinking effort: high)",
);
+ // FNXC:AgentLog-EntryTypes 2026-07-15-11:20: the marker is a complete standalone message,
+ // so it is a `status` row — `text` means "streamed delta fragment" and gets glued to its
+ // neighbours with no separator.
expect(store.appendAgentLog).toHaveBeenCalledWith(
"FN-100",
"Reviewer using model: mock-provider/mock-model (thinking effort: high)",
- "text",
+ "status",
undefined,
"reviewer",
);
@@ -1590,3 +1593,259 @@ describe("reviewStep — subagent lifecycle hooks", () => {
expect(onSessionEnded).toHaveBeenCalledTimes(2);
});
});
+
+/*
+FNXC:ReviewerProviderErrors 2026-07-15-11:20:
+Regression coverage for the reviewer provider-error loop.
+
+## Symptom Verification
+Original symptom: a task's Chat tab filled with 14 identical "Reviewer using model: umans/umans-kimi-k2.7" rows and no review text, while the engine kept re-hitting an already-rate-limited provider.
+Exact reproduction: the reviewer's prompt rejects with a rate-limit error (the provider condition behind the report).
+Assertion it is gone: the rate limit escalates as a typed `ReviewerProviderError` instead of becoming an `UNAVAILABLE` verdict, and exactly ONE session is created — no same-model re-hit, and therefore no repeated marker.
+
+## Surface Enumeration
+Provider-error classes: usage-limit, transient (recovered + exhausted), permanent (must keep the existing fallback ladder).
+Fallback configurations: configured fallback model AND no configured fallback (the reported case — the "fallback" degrades to a same-model strict-prompt rerun, so a rate limit was re-hit instantly).
+Review types: code (blocking) and plan/spec (advisory) share `reviewStep`, so both are asserted.
+Budget: `reviewerFallbackRetryCount` must not be burned by an outage.
+Marker emission: deduped per model, but a real model change must still emit.
+*/
+describe("reviewStep — provider errors are not review verdicts", () => {
+ const RATE_LIMIT_ERROR = "429 rate_limit_error: too many requests";
+
+ beforeEach(() => {
+ vi.clearAllMocks();
+ vi.useRealTimers();
+ mockedPromptWithFallback.mockImplementation(async (session: any, prompt: any, options: any) => {
+ if (options == null) await session.prompt(prompt);
+ else await session.prompt(prompt, options);
+ });
+ });
+
+ it("escalates a rate limit as ReviewerProviderError instead of an UNAVAILABLE verdict", async () => {
+ mockedCreateFnAgent.mockResolvedValue(createMockSession("unused"));
+ mockedPromptWithFallback.mockRejectedValue(new Error(RATE_LIMIT_ERROR));
+
+ const error = await reviewStep(
+ "/tmp/worktree", "FN-RL", 2, "Rate limited", "code", "# prompt", "abc123", {},
+ ).then(() => null, (err: unknown) => err);
+
+ expect(error).toBeInstanceOf(ReviewerProviderError);
+ expect((error as ReviewerProviderError).classification).toBe("usage-limit");
+ // The reported bug: with no configured fallback the ladder re-ran the SAME model
+ // immediately, so a 429 spawned a second session (and a second identical marker).
+ expect(mockedCreateFnAgent).toHaveBeenCalledTimes(1);
+ });
+
+ it("does not spend the configured fallback model on a rate limit", async () => {
+ mockedCreateFnAgent.mockResolvedValue(createMockSession("unused"));
+ mockedPromptWithFallback.mockRejectedValue(new Error(RATE_LIMIT_ERROR));
+
+ await expect(
+ reviewStep("/tmp/worktree", "FN-RL2", 2, "Rate limited", "code", "# prompt", "abc123", {
+ projectValidatorFallbackProvider: "openai",
+ projectValidatorFallbackModelId: "gpt-5-mini",
+ }),
+ ).rejects.toBeInstanceOf(ReviewerProviderError);
+
+ expect(mockedCreateFnAgent).toHaveBeenCalledTimes(1);
+ });
+
+ it("escalates a rate limit for advisory plan reviews too, not just blocking code reviews", async () => {
+ mockedCreateFnAgent.mockResolvedValue(createMockSession("unused"));
+ mockedPromptWithFallback.mockRejectedValue(new Error(RATE_LIMIT_ERROR));
+
+ await expect(
+ reviewStep("/tmp/worktree", "FN-RL3", 1, "Plan", "plan", "# prompt", undefined, {}),
+ ).rejects.toBeInstanceOf(ReviewerProviderError);
+
+ expect(mockedCreateFnAgent).toHaveBeenCalledTimes(1);
+ });
+
+ it("does not burn the reviewer fallback retry budget on a provider outage", async () => {
+ mockedCreateFnAgent.mockResolvedValue(createMockSession("unused"));
+ mockedPromptWithFallback.mockRejectedValue(new Error(RATE_LIMIT_ERROR));
+
+ const store = {
+ getSettings: vi.fn().mockResolvedValue({}),
+ getTask: vi.fn().mockResolvedValue({ id: "FN-RL4", reviewerFallbackRetryCount: 0, steps: [] }),
+ updateTask: vi.fn().mockResolvedValue(undefined),
+ logEntry: vi.fn().mockResolvedValue(undefined),
+ appendAgentLog: vi.fn().mockResolvedValue(undefined),
+ };
+
+ await expect(
+ reviewStep("/tmp/worktree", "FN-RL4", 2, "Rate limited", "code", "# prompt", "abc123", {
+ store: store as any,
+ taskId: "FN-RL4",
+ settings: {} as any,
+ }),
+ ).rejects.toBeInstanceOf(ReviewerProviderError);
+
+ // The budget bounds BAD REVIEWS. Spending it on an outage would fail healthy tasks.
+ expect(store.updateTask).not.toHaveBeenCalledWith(
+ "FN-RL4",
+ expect.objectContaining({ reviewerFallbackRetryCount: expect.anything() }),
+ );
+ });
+
+ it("keeps the fallback ladder for a genuine (permanent) reviewer error", async () => {
+ mockedCreateFnAgent
+ .mockResolvedValueOnce(createMockSession("unused"))
+ .mockResolvedValueOnce(createMockSession("### Verdict: REVISE\n### Summary\nRecovered."));
+ mockedPromptWithFallback
+ .mockRejectedValueOnce(new Error("reviewer produced malformed output"))
+ .mockImplementation(async (session: any, prompt: any, options: any) => {
+ if (options == null) await session.prompt(prompt);
+ else await session.prompt(prompt, options);
+ });
+
+ const result = await reviewStep(
+ "/tmp/worktree", "FN-PERM", 2, "Retry", "code", "# prompt", "abc123",
+ { projectValidatorFallbackProvider: "openai", projectValidatorFallbackModelId: "gpt-5-mini" },
+ );
+
+ // A permanent error is a REVIEW problem — the ladder still applies.
+ expect(result.verdict).toBe("REVISE");
+ expect(mockedCreateFnAgent).toHaveBeenCalledTimes(2);
+ });
+
+ it("absorbs a flaky network blip by retrying the attempt with backoff", async () => {
+ vi.useFakeTimers();
+ mockedCreateFnAgent
+ .mockResolvedValueOnce(createMockSession("unused"))
+ .mockResolvedValueOnce(createMockSession("### Verdict: APPROVE\n### Summary\nRecovered."));
+ mockedPromptWithFallback
+ .mockRejectedValueOnce(new Error("socket hang up"))
+ .mockImplementation(async (session: any, prompt: any, options: any) => {
+ if (options == null) await session.prompt(prompt);
+ else await session.prompt(prompt, options);
+ });
+
+ const pending = reviewStep(
+ "/tmp/worktree", "FN-NET", 2, "Flaky", "code", "# prompt", "abc123", {},
+ );
+ await vi.advanceTimersByTimeAsync(60_000);
+ const result = await pending;
+
+ // A network blip must not surface as a failed review or a rate-limit escalation.
+ expect(result.verdict).toBe("APPROVE");
+ vi.useRealTimers();
+ });
+
+ it("escalates as transient once the network retry budget is exhausted", async () => {
+ vi.useFakeTimers();
+ mockedCreateFnAgent.mockResolvedValue(createMockSession("unused"));
+ mockedPromptWithFallback.mockRejectedValue(new Error("ECONNREFUSED connection refused"));
+
+ const pending = reviewStep(
+ "/tmp/worktree", "FN-NET2", 2, "Down", "code", "# prompt", "abc123", {},
+ ).then(() => null, (err: unknown) => err);
+ await vi.advanceTimersByTimeAsync(300_000);
+ const error = await pending;
+
+ expect(error).toBeInstanceOf(ReviewerProviderError);
+ expect((error as ReviewerProviderError).classification).toBe("transient");
+ vi.useRealTimers();
+ });
+});
+
+/*
+FNXC:ReviewerModelMarker 2026-07-15-11:20:
+The marker only carries information when the model CHANGES — the dashboard resolves the effective
+model from the latest matching row. Re-emitting it per retry is what produced the run-on
+"14 entries" card, so dedupe on marker text while keeping a real model switch visible.
+*/
+describe("reviewStep — model marker emission", () => {
+ beforeEach(() => {
+ vi.clearAllMocks();
+ vi.useRealTimers();
+ mockedPromptWithFallback.mockImplementation(async (session: any, prompt: any, options: any) => {
+ if (options == null) await session.prompt(prompt);
+ else await session.prompt(prompt, options);
+ });
+ });
+
+ function markerStore() {
+ return {
+ getSettings: vi.fn().mockResolvedValue({}),
+ getTask: vi.fn().mockResolvedValue({ id: "FN-MARK", reviewerFallbackRetryCount: 0, steps: [] }),
+ updateTask: vi.fn().mockResolvedValue(undefined),
+ logEntry: vi.fn().mockResolvedValue(undefined),
+ appendAgentLog: vi.fn().mockResolvedValue(undefined),
+ };
+ }
+
+ const markerRows = (store: ReturnType) =>
+ store.appendAgentLog.mock.calls.filter((call) => String(call[1]).startsWith("Reviewer using model:"));
+
+ it("emits the model marker once when the same model is retried", async () => {
+ // Two same-model sessions (unparseable verdict -> same-model strict-prompt rerun).
+ mockedCreateFnAgent
+ .mockResolvedValueOnce(createMockSession("no parseable verdict #1"))
+ .mockResolvedValueOnce(createMockSession("no parseable verdict #2"));
+ const store = markerStore();
+
+ await reviewStep("/tmp/worktree", "FN-MARK", 2, "Marker", "spec", "# prompt", undefined, {
+ store: store as any,
+ taskId: "FN-MARK",
+ settings: {} as any,
+ });
+
+ // Two sessions, one marker — the reported symptom was one marker PER session.
+ expect(mockedCreateFnAgent).toHaveBeenCalledTimes(2);
+ expect(markerRows(store)).toHaveLength(1);
+ });
+
+ it("still emits a marker when the reviewer actually switches models", async () => {
+ /*
+ Tag each session with the model it represents and resolve `describeModel` from the session
+ itself. A `mockReturnValueOnce` chain is NOT safe here: `describeModel` is also called by
+ agent-session-helpers (its "built-in fallback model" warning), which silently consumes a
+ queued value and makes this test assert the wrong thing.
+ */
+ const taggedSession = (reviewText: string, model: string) => {
+ const mock = createMockSession(reviewText);
+ mock.session.__testModel = model;
+ return mock;
+ };
+ mockedCreateFnAgent
+ .mockResolvedValueOnce(taggedSession("no parseable verdict", "primary-provider/primary-model"))
+ .mockResolvedValueOnce(taggedSession("### Verdict: APPROVE\n### Summary\nok", "fallback-provider/fallback-model"));
+ const { describeModel } = await import("../pi.js");
+ vi.mocked(describeModel).mockImplementation(
+ (session: any) => session?.__testModel ?? "mock-provider/mock-model",
+ );
+ const store = markerStore();
+
+ await reviewStep("/tmp/worktree", "FN-MARK2", 2, "Marker", "spec", "# prompt", undefined, {
+ store: store as any,
+ taskId: "FN-MARK2",
+ settings: {} as any,
+ projectValidatorFallbackProvider: "fallback-provider",
+ projectValidatorFallbackModelId: "fallback-model",
+ });
+
+ // Dedupe is on marker TEXT, so a genuine model switch is never hidden. Assert the actual
+ // rows, not just the count — a count-only check would pass even if both rows named the
+ // same model, which is exactly the bug this guards.
+ expect(markerRows(store).map((call) => call[1])).toEqual([
+ "Reviewer using model: primary-provider/primary-model",
+ "Reviewer using model: fallback-provider/fallback-model",
+ ]);
+ });
+
+ it("writes the marker as a standalone status row, never as a streamed text delta", async () => {
+ mockedCreateFnAgent.mockResolvedValue(createMockSession("### Verdict: APPROVE\n### Summary\nok"));
+ const store = markerStore();
+
+ await reviewStep("/tmp/worktree", "FN-MARK3", 2, "Marker", "code", "# prompt", "abc123", {
+ store: store as any,
+ taskId: "FN-MARK3",
+ settings: {} as any,
+ });
+
+ // `text` rows are re-glued with join("") by the renderers; a whole message must be `status`.
+ expect(markerRows(store).every((call) => call[2] === "status")).toBe(true);
+ });
+});
diff --git a/packages/engine/src/__tests__/triage.test.ts b/packages/engine/src/__tests__/triage.test.ts
index 21cc939b1a..599486b7b0 100644
--- a/packages/engine/src/__tests__/triage.test.ts
+++ b/packages/engine/src/__tests__/triage.test.ts
@@ -4359,7 +4359,7 @@ describe("taskCreate tool model inheritance", () => {
expect(store.appendAgentLog).toHaveBeenCalledWith(
"FN-7437",
"Triage using model: mock-model (thinking effort: low)",
- "text",
+ "status",
undefined,
"triage",
);
@@ -5137,7 +5137,7 @@ describe("taskCreate tool model inheritance", () => {
expect(store.appendAgentLog).toHaveBeenCalledWith(
"FN-300",
"Triage using model: mock-model (thinking effort: high)",
- "text",
+ "status",
undefined,
"triage",
);
diff --git a/packages/engine/src/agent-tools.ts b/packages/engine/src/agent-tools.ts
index b4eba237c5..ca298a509d 100644
--- a/packages/engine/src/agent-tools.ts
+++ b/packages/engine/src/agent-tools.ts
@@ -1466,7 +1466,7 @@ export function createTaskFileScopeAddTool(store: TaskStore, taskId: string, run
.appendAgentLog(
taskId,
`Added to File Scope: ${toAdd.join(", ")}${params.reason ? ` — ${params.reason}` : ""}`,
- "text",
+ "status",
)
.catch(() => {});
diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts
index 72435b3d4a..54ff4d9173 100644
--- a/packages/engine/src/executor.ts
+++ b/packages/engine/src/executor.ts
@@ -96,7 +96,7 @@ import {
import { buildSessionSkillContext } from "./session-skill-context.js";
import type { SkillSelectionContext } from "./skill-resolver.js";
import { assertMcpResolutionSucceeded, resolveMcpServersForStore } from "./mcp-resolution.js";
-import { reviewStep, proseSignalsClearApproval, extractJsonObjectCandidates, type ReviewVerdict, type ReviewResult } from "./reviewer.js";
+import { reviewStep, proseSignalsClearApproval, extractJsonObjectCandidates, ReviewerProviderError, type ReviewVerdict, type ReviewResult } from "./reviewer.js";
import { buildUserCommentsPromptSection, selectUserCommentsForAgentContext } from "./agent-user-comments.js";
import { resolveSandboxBackend } from "./sandbox/index.js";
import type { SandboxBackend } from "./sandbox/types.js";
@@ -514,6 +514,31 @@ export const EXECUTE_REQUEUE_LOOP_VISIBLE_THRESHOLD = 3;
*/
const MAX_TRANSIENT_GRAPH_RESUME_RETRIES = 2;
const TRANSIENT_GRAPH_RESUME_RETRY_BACKOFF_MS = process.env.VITEST || process.env.NODE_ENV === "test" ? 0 : 1_000;
+/**
+ * FNXC:ReviewerUnavailableBudget 2026-07-15-11:20:
+ * Hard cap on how many times one step's CODE review may come back UNAVAILABLE before the tool
+ * stops inviting the model to retry. Mirrors the `planSpecUnavailableCounts` limiter's posture
+ * (and `STEP_REVIEW_UNAVAILABLE_RETRY_CAP` on the graph path) so both review paths bound repeats
+ * in code rather than in prompt text. See `createReviewStepTool`.
+ */
+const MAX_CODE_REVIEW_UNAVAILABLE_RETRIES = 3;
+
+/**
+ * FNXC:ReviewerProviderErrors 2026-07-15-11:20:
+ * Re-raise a provider failure that a `fn_review_step` tool handler recorded but could not throw
+ * (pi-agent-core turns tool throws into `tool_error` results the model just reads and retries).
+ * Called immediately after each `promptWithFallback`, alongside `checkSessionError`, so the error
+ * escapes `agentWork` and reaches `withRateLimitRetry` + the outer usage-limit/transient handlers.
+ * Clears the ref so a later successful prompt in the same run cannot re-throw a stale error.
+ */
+function throwDeferredReviewerFatal(ref: { current: Error | null }): void {
+ const deferred = ref.current;
+ if (deferred) {
+ ref.current = null;
+ throw deferred;
+ }
+}
+
/** How long to wait before recovering a completed task still stuck in in-progress. */
const COMPLETED_TASK_WATCHDOG_MS = 60_000;
/** How long to wait before retrying a workflow rerun handoff that never reached in-progress. */
@@ -10988,6 +11013,15 @@ export class TaskExecutor {
let wasPaused = false;
// Mutable ref — populated after createFnAgent, tools access lazily via closure
const sessionRef: { current: AgentSession | null } = { current: null };
+ /*
+ FNXC:ReviewerProviderErrors 2026-07-15-11:20:
+ Deferred re-raise channel for provider failures that surface INSIDE `fn_review_step`.
+
+ Why a ref instead of just throwing: pi-agent-core's `executePreparedToolCall` catches every throw from a tool handler and converts it into a `tool_error` result fed back to the model — a tool can NOT propagate an error out of `session.prompt()`. So throwing a rate-limit error from the review tool would just become more text for the model to read and retry against, which is exactly the loop this fixes (14 identical reviewer model markers hammering an already-limited provider).
+
+ Instead the tool records the fatal error here and returns a stop instruction; `agentWork` re-raises it right after `promptWithFallback` returns (next to `checkSessionError`, which exists for the same reason on the session's own errors). Once thrown from `agentWork` it reaches the machinery that already handles it: `withRateLimitRetry` backoff, then the outer catch's `UsageLimitPauser` global pause (usage-limit) or bounded `computeRecoveryDecision` requeue (transient).
+ */
+ const reviewerFatalRef: { current: Error | null } = { current: null };
// Keyed by 0-indexed step (stepIndex) to match fn_review_step.
const stepCheckpoints = new Map();
@@ -11047,7 +11081,7 @@ export class TaskExecutor {
Workflow-graph execution owns plan/code/browser review gates as nodes. Do not expose legacy in-session `fn_review_step` during graph-owned execute seams; otherwise default coding can duplicate Plan Review inside implementation steps after the workflow-level Plan Review has already passed.
*/
...(executionMode !== "fast" && !this.graphCompletionInterceptors.has(task.id) ? [
- this.createReviewStepTool(task.id, worktreePath, detail.prompt, codeReviewVerdicts, sessionRef, stepCheckpoints, detail, stuckDetector),
+ this.createReviewStepTool(task.id, worktreePath, detail.prompt, codeReviewVerdicts, sessionRef, stepCheckpoints, detail, reviewerFatalRef, stuckDetector),
] : []),
this.createSpawnAgentTool(task.id, worktreePath, settings, taskEnv),
this.createTaskDocumentWriteTool(task.id),
@@ -11344,7 +11378,7 @@ export class TaskExecutor {
await this.store.updateTask(task.id, { sessionFile });
}
}
- await this.store.appendAgentLog(task.id, executorModelMarker, "text", undefined, "executor");
+ await this.store.appendAgentLog(task.id, executorModelMarker, "status", undefined, "executor");
// Make session available to custom tools (fn_task_update checkpoint capture, fn_review_step rewind)
sessionRef.current = session;
@@ -11421,6 +11455,9 @@ export class TaskExecutor {
// session.prompt() resolves normally even when retries are exhausted —
// the error is stored on session.state.error instead of being thrown.
checkSessionError(session);
+ // FNXC:ReviewerProviderErrors 2026-07-15-11:20: same re-raise contract as
+ // checkSessionError above, for provider failures a tool handler could not throw.
+ throwDeferredReviewerFatal(reviewerFatalRef);
await accumulateSessionTokenUsage(this.store, task.id, session, {
agentId: task.assignedAgentId ?? undefined,
role: "executor",
@@ -11482,6 +11519,9 @@ export class TaskExecutor {
await promptWithFallback(session, resumePrompt);
checkSessionError(session);
+ // FNXC:ReviewerProviderErrors 2026-07-15-11:20: a resumed session runs the same
+ // review tool, so it needs the same deferred provider-error re-raise.
+ throwDeferredReviewerFatal(reviewerFatalRef);
await accumulateSessionTokenUsage(this.store, task.id, session, {
agentId: task.assignedAgentId ?? undefined,
role: "executor",
@@ -14288,11 +14328,21 @@ export class TaskExecutor {
sessionRef: { current: AgentSession | null },
stepCheckpoints: Map,
detail: TaskDetail,
+ reviewerFatalRef: { current: Error | null },
stuckDetector?: StuckTaskDetector,
): ToolDefinition {
const store = this.store;
const options = this.options;
const planSpecUnavailableCounts = new Map();
+ /*
+ FNXC:ReviewerUnavailableBudget 2026-07-15-11:20:
+ Code review needs the same hard UNAVAILABLE bound plan/spec reviews already have.
+
+ Plan/spec reviews cap repeats via `planSpecUnavailableCounts` and tell the model "Do NOT re-call". Code review had NO counter and instead told the model "retry once or escalate" — but "retry once" is a SUGGESTION TO AN LLM, not a bound. A model that ignores it re-calls the tool indefinitely, and each call spawns reviewer sessions against a provider that is already failing. Prompt text is not a control-flow mechanism; this counter is.
+
+ Code review stays BLOCKING on exhaustion (unlike advisory plan/spec, which proceed) — an unreviewed code change must not pass as approved. The step simply stops being retried and the operator is pointed at the dashboard.
+ */
+ const codeUnavailableCounts = new Map();
return {
name: "fn_review_step",
@@ -14526,10 +14576,15 @@ export class TaskExecutor {
}
text = `UNAVAILABLE (advisory) — reviewer could not produce a verdict after fallback retry. ${advisoryType === "plan" ? "Plan" : "Spec"} reviews are advisory; proceed with implementation. Do NOT re-call fn_review_step for the ${advisoryType} of Step ${step}.`;
} else {
- const blockingMessage = `code review Step ${step}: UNAVAILABLE — blocking until reviewer returns a usable verdict`;
+ const key = `code:${step}`;
+ const count = (codeUnavailableCounts.get(key) ?? 0) + 1;
+ codeUnavailableCounts.set(key, count);
+ const blockingMessage = `code review Step ${step}: UNAVAILABLE (${count}/${MAX_CODE_REVIEW_UNAVAILABLE_RETRIES}) — blocking until reviewer returns a usable verdict`;
await store.logEntry(taskId, blockingMessage);
reviewerLog.warn(`${taskId}: ${blockingMessage}`);
- text = "UNAVAILABLE — reviewer did not produce a usable verdict. Code review remains blocking; retry once or escalate via dashboard.";
+ text = count >= MAX_CODE_REVIEW_UNAVAILABLE_RETRIES
+ ? `UNAVAILABLE — the reviewer failed to produce a usable verdict ${count} times for Step ${step}. Do NOT re-call fn_review_step for the code review of Step ${step}; retrying is not working. Code review is blocking, so this step cannot be marked done — stop working on it and report that the code review is stuck so an operator can inspect the reviewer logs in the dashboard.`
+ : "UNAVAILABLE — reviewer did not produce a usable verdict. Code review remains blocking; retry once or escalate via dashboard.";
}
break;
}
@@ -14540,6 +14595,28 @@ export class TaskExecutor {
const errorMessage = err instanceof Error ? err.message : String(err);
reviewerLog.error(`${taskId}: review failed: ${errorMessage}`);
await store.logEntry(taskId, `${reviewType} review failed: ${errorMessage}`);
+
+ /*
+ FNXC:ReviewerProviderErrors 2026-07-15-11:20:
+ This catch used to swallow EVERY reviewer failure into an in-band "UNAVAILABLE" string handed back to the model. That is what neutralized the engine's existing protections: because the error became tool OUTPUT rather than a thrown exception, `withRateLimitRetry` never backed off, `UsageLimitPauser` never paused, and `RetryStormError` never reached its terminalizer — while the model, told code review was still blocking, simply called the tool again against a failing provider.
+
+ Two classes must escape instead of becoming model-visible text:
+ - `ReviewerProviderError` — a rate limit or a network outage that survived the reviewer's own bounded backoff. Not a verdict; the engine must back off or pause, not re-ask.
+ - `RetryStormError` — the deliberate "budget exhausted, stop" signal from `recordRetry`. Answering it with a retry instruction is precisely backwards.
+
+ They cannot be re-thrown here (pi-agent-core converts tool throws into `tool_error` results — see `reviewerFatalRef`), so record them for `agentWork` to re-raise after the prompt and return a STOP instruction so the model stops calling in the meantime. Everything else keeps the existing in-band UNAVAILABLE behavior: a genuinely unparseable review is a review problem, and the model can legitimately act on it.
+ */
+ if (err instanceof ReviewerProviderError || err instanceof RetryStormError) {
+ reviewerFatalRef.current ??= err;
+ return {
+ content: [{
+ type: "text" as const,
+ text: `UNAVAILABLE — the review could not run and this task is being handed back to the engine to retry: ${errorMessage}. Do NOT call fn_review_step again. Stop working and end your turn now.`,
+ }],
+ details: {},
+ };
+ }
+
return {
content: [{ type: "text" as const, text: `UNAVAILABLE — reviewer error: ${errorMessage}` }],
details: {},
@@ -14835,7 +14912,7 @@ Do not refactor, rename broadly, or make opportunistic improvements.
await this.store.appendAgentLog(
task.id,
`Fix agent started (model: ${describeModel(session)}, attempt ${retryNumber}/${maxRetries})`,
- "text",
+ "status",
undefined,
"executor",
);
@@ -14883,7 +14960,7 @@ ${failureContext.output.slice(0, VERIFICATION_LOG_MAX_CHARS)}
await this.store.appendAgentLog(
task.id,
`Re-running verification (attempt ${retryNumber}/${maxRetries})`,
- "text",
+ "status",
undefined,
"executor",
);
@@ -15843,7 +15920,7 @@ You have access to the file system to review changes.${inlineFixBlock}${verdictB
const additionalSkillPaths = mergeAdditionalSkillPaths(skillContext.additionalSkillPaths, ceSkillsDir ? [ceSkillsDir] : undefined);
const logBrowserVerificationActivity = async (message: string) => {
await this.store.logEntry(task.id, message);
- await this.store.appendAgentLog(task.id, message, "text", undefined, "reviewer");
+ await this.store.appendAgentLog(task.id, message, "status", undefined, "reviewer");
};
if (workflowStep.requiresBrowser === true) {
effectiveSkillSelection = augmentSessionSkillsForBrowserStep(effectiveSkillSelection, this.rootDir);
@@ -16253,7 +16330,7 @@ You have access to the file system to review changes.${inlineFixBlock}${verdictB
}
const message = `[recovery] reclaimed existing worktree for ${task.id} at ${livePath} (${count} commits preserved, tip ${tipSha.slice(0, 12)})`;
await this.store.logEntry(task.id, message, undefined, this.getRunContextFor(task.id));
- await this.store.appendAgentLog(task.id, "Branch conflict auto-recovery", "text", message, "executor");
+ await this.store.appendAgentLog(task.id, "Branch conflict auto-recovery", "status", message, "executor");
}
private async handleBranchConflict(task: Task, error: BranchConflictError): Promise<"retry" | "reclaimed" | "sticky"> {
@@ -16284,7 +16361,7 @@ You have access to the file system to review changes.${inlineFixBlock}${verdictB
await this.store.updateTask(task.id, { worktree: null, branch: null, baseCommitSha: null });
const message = `[recovery] ${task.id} stage-A: pruned stale admin entry for ${error.branchName}`;
await this.store.logEntry(task.id, message, undefined, this.getRunContextFor(task.id));
- await this.store.appendAgentLog(task.id, "Branch conflict auto-recovery", "text", message, "executor");
+ await this.store.appendAgentLog(task.id, "Branch conflict auto-recovery", "status", message, "executor");
return "retry";
}
@@ -16313,7 +16390,7 @@ You have access to the file system to review changes.${inlineFixBlock}${verdictB
await this.store.updateTask(task.id, { worktree: null, branch: null, baseCommitSha: null });
const message = `[recovery] ${task.id} stage-A: tip-already-merged cleanup for ${error.branchName} (${inspection.tipSha.slice(0, 12)} on ${inspection.integrationRef})`;
await this.store.logEntry(task.id, message, undefined, this.getRunContextFor(task.id));
- await this.store.appendAgentLog(task.id, "Branch conflict auto-recovery", "text", message, "executor");
+ await this.store.appendAgentLog(task.id, "Branch conflict auto-recovery", "status", message, "executor");
return "retry";
}
diff --git a/packages/engine/src/fallback-model-observer.ts b/packages/engine/src/fallback-model-observer.ts
index 37d176a452..1992661ee8 100644
--- a/packages/engine/src/fallback-model-observer.ts
+++ b/packages/engine/src/fallback-model-observer.ts
@@ -1,3 +1,4 @@
+import type { AgentLogType } from "@fusion/core";
import { notifyFallbackUsed } from "./notifier.js";
import type { FallbackModelUsedPayload } from "./pi.js";
@@ -6,7 +7,9 @@ type FallbackLogStore = {
appendAgentLog?(
taskId: string,
text: string,
- type: "text" | "thinking" | "tool" | "tool_result" | "tool_error",
+ // FNXC:AgentLog-EntryTypes 2026-07-15-11:20: reference the canonical AgentLogType rather than
+ // re-listing the members — the hand-copied union silently drifted when `status` was added.
+ type: AgentLogType,
detail?: string,
agent?: string,
): Promise;
@@ -50,7 +53,7 @@ export function createFallbackModelObserver(options: FallbackModelObserverOption
await options.store.logEntry(taskId, message).catch(() => undefined);
}
if (taskId && options.store?.appendAgentLog) {
- await options.store.appendAgentLog(taskId, message, "text", undefined, options.agent).catch(() => undefined);
+ await options.store.appendAgentLog(taskId, message, "status", undefined, options.agent).catch(() => undefined);
}
await notifyFallbackUsed({
diff --git a/packages/engine/src/merger-ai.ts b/packages/engine/src/merger-ai.ts
index a035f0e5ed..239b170e59 100644
--- a/packages/engine/src/merger-ai.ts
+++ b/packages/engine/src/merger-ai.ts
@@ -1015,7 +1015,7 @@ export async function runAiMerge(
// Surface progress on the task detail (status pill) + the task log stream.
const log = async (message: string): Promise => {
await store.logEntry(taskId, message, "AiMerge").catch(() => undefined);
- await store.appendAgentLog(taskId, message, "text", undefined, "merger").catch(() => undefined);
+ await store.appendAgentLog(taskId, message, "status", undefined, "merger").catch(() => undefined);
};
const setStatus = (status: string | null): Promise =>
store.updateTask(taskId, { status }).catch(() => undefined);
@@ -1418,7 +1418,7 @@ export async function landWorkspaceTask(
});
const log = async (message: string): Promise => {
await store.logEntry(taskId, message, "AiMerge").catch(() => undefined);
- await store.appendAgentLog(taskId, message, "text", undefined, "merger").catch(() => undefined);
+ await store.appendAgentLog(taskId, message, "status", undefined, "merger").catch(() => undefined);
};
const setStatus = (status: string | null): Promise =>
store.updateTask(taskId, { status }).catch(() => undefined);
diff --git a/packages/engine/src/merger.ts b/packages/engine/src/merger.ts
index 290cf0655c..e2455bedaf 100644
--- a/packages/engine/src/merger.ts
+++ b/packages/engine/src/merger.ts
@@ -1608,7 +1608,7 @@ async function runDeterministicVerification(
const msg = `Skipping deterministic verification — cached pass for tree ${sha7} (recorded at ${cacheHit.recordedAt}, by ${cacheHit.taskId ?? "unknown"})`;
mergerLog.log(`${taskId}: ${msg}`);
await store.logEntry(taskId, msg);
- await store.appendAgentLog(taskId, msg, "text", undefined, "merger");
+ await store.appendAgentLog(taskId, msg, "status", undefined, "merger");
const syntheticResult: VerificationCommandResult = {
command: "",
exitCode: 0,
@@ -1639,7 +1639,7 @@ async function runDeterministicVerification(
(hasTestCommand ? ` (test${testSourceDisplayLabel}: ${normalizedTestCommand})` : "") +
(hasBuildCommand ? ` (build${buildSource === "inferred" ? " [inferred]" : ""}: ${normalizedBuildCommand})` : "");
await store.logEntry(taskId, deterministicVerificationMessage);
- await store.appendAgentLog(taskId, deterministicVerificationMessage, "text", undefined, "merger");
+ await store.appendAgentLog(taskId, deterministicVerificationMessage, "status", undefined, "merger");
const bootstrapScriptPath = join(rootDir, "scripts/ensure-test-artifacts.mjs");
if (hasTestCommand || hasBuildCommand) {
@@ -1647,7 +1647,7 @@ async function runDeterministicVerification(
const bootstrapMissingMessage = `${taskId}: [verification:bootstrap] script missing at scripts/ensure-test-artifacts.mjs — skipping preamble`;
mergerLog.warn(bootstrapMissingMessage);
await store.logEntry(taskId, bootstrapMissingMessage);
- await store.appendAgentLog(taskId, bootstrapMissingMessage, "text", undefined, "merger");
+ await store.appendAgentLog(taskId, bootstrapMissingMessage, "status", undefined, "merger");
} else {
const bootstrapCommand = "node scripts/ensure-test-artifacts.mjs";
await store.logEntry(taskId, `[verification:bootstrap] running: ${bootstrapCommand}`);
@@ -1827,7 +1827,7 @@ async function runDeterministicVerification(
mergerLog.log(`${taskId}: deterministic verification passed`);
await store.logEntry(taskId, "Deterministic merge verification passed");
- await store.appendAgentLog(taskId, "Deterministic merge verification passed", "text", undefined, "merger");
+ await store.appendAgentLog(taskId, "Deterministic merge verification passed", "status", undefined, "merger");
// ── Record cache pass ──────────────────────────────────────────────────
if (treeSha) {
@@ -2013,7 +2013,7 @@ Do not refactor, rename broadly, or make opportunistic improvements.
await store.appendAgentLog(
taskId,
`Fix agent started (model: ${describeModel(session)})`,
- "text",
+ "status",
undefined,
"merger",
);
@@ -2080,7 +2080,7 @@ ${failureContext.output.slice(0, VERIFICATION_LOG_MAX_CHARS)}
await store.appendAgentLog(
taskId,
`Fix agent made no changes — skipping verification re-run`,
- "text",
+ "status",
undefined,
"merger",
);
@@ -2114,7 +2114,7 @@ ${failureContext.output.slice(0, VERIFICATION_LOG_MAX_CHARS)}
`Failing files: [${failingFiles.join(", ")}]. Branch diff files: [${branchFiles.slice(0, 10).join(", ")}${branchFiles.length > 10 ? ", ..." : ""}].`;
mergerLog.warn(`${taskId}: ${msg}`);
await store.logEntry(taskId, msg);
- await store.appendAgentLog(taskId, "Out-of-scope verification failure detected — not retrying", "text", undefined, "merger");
+ await store.appendAgentLog(taskId, "Out-of-scope verification failure detected — not retrying", "status", undefined, "merger");
throw new OutOfScopeVerificationError(msg, failingFiles, branchFiles);
}
}
@@ -2132,7 +2132,7 @@ ${failureContext.output.slice(0, VERIFICATION_LOG_MAX_CHARS)}
await store.appendAgentLog(
taskId,
`Re-running verification (attempt ${fixAttemptNumber ?? "unknown"})`,
- "text",
+ "status",
undefined,
"merger",
);
@@ -3201,7 +3201,7 @@ ${fileList}
await store.appendAgentLog(
taskId,
`Autostash conflict agent started (model: ${describeModel(session)}, files: ${conflictedFiles.length})`,
- "text",
+ "status",
undefined,
"merger",
);
@@ -3621,7 +3621,7 @@ ${fileList}
await store.appendAgentLog(
taskId,
`Autostash hard-fail recovery agent started (model: ${describeModel(session)}, files: ${stashFiles.length})`,
- "text",
+ "status",
undefined,
"merger",
);
@@ -4949,7 +4949,7 @@ export async function applyLayer3ConflictScopePartition(params: {
const declaredScope = await store.parseFileScopeFromPrompt(taskId);
if (task.scopeOverride === true) {
const reasonSuffix = task.scopeOverrideReason?.trim() ? ` — reason: ${task.scopeOverrideReason.trim()}` : "";
- await store.appendAgentLog(taskId, `Layer 3 arbiter scope partition bypassed via scopeOverride${reasonSuffix}`, "text", undefined, "merger");
+ await store.appendAgentLog(taskId, `Layer 3 arbiter scope partition bypassed via scopeOverride${reasonSuffix}`, "status", undefined, "merger");
if (auditor) {
await auditor.git({
type: "merge:layer3:scope-override-bypass",
@@ -5014,7 +5014,7 @@ export async function applyLayer3ConflictScopePartition(params: {
await store.appendAgentLog(
taskId,
`Layer 2.5 auto-widened File Scope: ${widenedFiles.join(", ")}`,
- "text",
+ "status",
undefined,
"merger",
);
@@ -5046,7 +5046,7 @@ export async function applyLayer3ConflictScopePartition(params: {
if (outOfScope.length > 0) {
const summary = `Layer 3 arbiter: skipped ${outOfScope.length} foreign file(s) — took main's version for: ${outOfScope.join(", ")}`;
- await store.appendAgentLog(taskId, summary, "text", undefined, "merger");
+ await store.appendAgentLog(taskId, summary, "status", undefined, "merger");
await store.logEntry(taskId, summary, "Layer3AIArbiterScopeSkip");
if (auditor) {
await auditor.git({
@@ -5128,7 +5128,7 @@ export async function assertSquashOverlapsFileScope(params: {
await store.appendAgentLog(
taskId,
`file-scope invariant bypassed via scopeOverride${reasonSuffix}`,
- "text",
+ "status",
undefined,
"merger",
);
@@ -5244,7 +5244,7 @@ export async function enforceSquashFileScopeInvariant(params: {
await params.store.appendAgentLog(
params.taskId,
warningMessage,
- "text",
+ "status",
formatFileScopeViolationAgentLog(error),
"merger",
);
@@ -6578,7 +6578,7 @@ export async function resolvePostMergeAuditInvocation(
const infoMessage = `${opts.taskId}: post-merge audit using rebase range base from ${candidate.source} (${resolved.slice(0, 8)}..${opts.auditSha.slice(0, 8)})`;
opts.mergerLog.log(infoMessage);
- await opts.store.appendAgentLog(opts.taskId, infoMessage, "text", undefined, "merger");
+ await opts.store.appendAgentLog(opts.taskId, infoMessage, "status", undefined, "merger");
return {
rootDir: opts.rootDir,
strategy: "rebase",
@@ -6608,7 +6608,7 @@ export async function resolvePostMergeAuditInvocation(
if (mergeBaseSha) {
const infoMessage = `${opts.taskId}: post-merge audit using rebase range base from merge-base (${mergeBaseSha.slice(0, 8)}..${opts.auditSha.slice(0, 8)})`;
opts.mergerLog.log(infoMessage);
- await opts.store.appendAgentLog(opts.taskId, infoMessage, "text", undefined, "merger");
+ await opts.store.appendAgentLog(opts.taskId, infoMessage, "status", undefined, "merger");
return {
rootDir: opts.rootDir,
strategy: "rebase",
@@ -6619,7 +6619,7 @@ export async function resolvePostMergeAuditInvocation(
const degradedMessage = `${opts.taskId}: post-merge audit degraded to single-commit squash fallback (multi-commit branch, no usable rangeBase)`;
opts.mergerLog.warn(degradedMessage);
- await opts.store.appendAgentLog(opts.taskId, degradedMessage, "text", undefined, "merger");
+ await opts.store.appendAgentLog(opts.taskId, degradedMessage, "status", undefined, "merger");
return {
rootDir: opts.rootDir,
strategy: "squash",
@@ -6736,7 +6736,7 @@ export async function handleDirtyPostMergeAuditOutcome(opts: {
await opts.store.appendAgentLog(
opts.taskId,
passLabel,
- "text",
+ "status",
formatSquashAuditAgentLog(opts.findings),
"merger",
);
@@ -9127,7 +9127,7 @@ export async function aiMergeTask(
await store.appendAgentLog(
taskId,
`Pre-merge auto-prerebase: ${branch} → local HEAD ${mainHead.slice(0, 8)} (${prerebaseDecision.reason})`,
- "text",
+ "status",
undefined,
"merger",
);
@@ -9222,7 +9222,7 @@ export async function aiMergeTask(
await store.appendAgentLog(
taskId,
`Pre-merge rebase: ${branch} → local HEAD ${localHead.slice(0, 8)}${label ? ` (${label})` : ""}`,
- "text",
+ "status",
undefined,
"merger",
);
@@ -9262,7 +9262,7 @@ export async function aiMergeTask(
await store.appendAgentLog(
taskId,
`Pre-merge rebase: ${branch} → ${remoteRef}`,
- "text",
+ "status",
undefined,
"merger",
);
@@ -9595,7 +9595,7 @@ export async function aiMergeTask(
`Overlap guard detected ${overlap.overlappingFiles.length} recent-main overlap file(s) ` +
`for smart-prefer-main (${mergeStrategyOverlapBehavior}): ${overlapSummary}`;
mergerLog.warn(`${taskId}: ${overlapMessage}`);
- await store.appendAgentLog(taskId, overlapMessage, "text", undefined, "merger");
+ await store.appendAgentLog(taskId, overlapMessage, "status", undefined, "merger");
await store.logEntry(taskId, overlapMessage);
if (mergeStrategyOverlapBehavior === "flip-to-prefer-branch") {
@@ -9665,7 +9665,7 @@ export async function aiMergeTask(
`Direct merge commit routing: ${selectedPostMergeAuditStrategy} ` +
`(setting ${configuredRoute.strategy} from ${configuredRoute.source})${classificationSummary}`;
mergerLog.log(`${taskId}: ${routeMessage}`);
- await store.appendAgentLog(taskId, routeMessage, "text", undefined, "merger");
+ await store.appendAgentLog(taskId, routeMessage, "status", undefined, "merger");
}
const [aiMergeSummary, aiMergeBody, aiMergeSubject] = settings.useAiMergeCommitSummary
@@ -9746,7 +9746,7 @@ export async function aiMergeTask(
await store.appendAgentLog(
taskId,
`Starting merge ${attemptLabel}`,
- "text",
+ "status",
undefined,
"merger",
);
@@ -9885,7 +9885,7 @@ export async function aiMergeTask(
await store.appendAgentLog(
taskId,
`Verification failed — attempting in-merge fix (up to ${maxFixRetries} attempts)`,
- "text",
+ "status",
undefined,
"merger",
);
@@ -9910,7 +9910,7 @@ export async function aiMergeTask(
await store.appendAgentLog(
taskId,
`In-merge verification fix attempt ${fixAttempt}/${maxFixRetries}`,
- "text",
+ "status",
undefined,
"merger",
);
@@ -10045,7 +10045,7 @@ export async function aiMergeTask(
await store.appendAgentLog(
taskId,
"Build verification failed — attempting in-merge fix",
- "text",
+ "status",
undefined,
"merger",
);
@@ -10064,7 +10064,7 @@ export async function aiMergeTask(
await store.appendAgentLog(
taskId,
`In-merge verification fix attempt ${fixAttempt}/${maxFixRetries}`,
- "text",
+ "status",
undefined,
"merger",
);
@@ -10419,13 +10419,13 @@ export async function aiMergeTask(
await store.appendAgentLog(
taskId,
selectedPostMergeAuditStrategy === "rebase" ? "post-rebase range audit clean" : "post-squash audit clean",
- "text",
+ "status",
undefined,
"merger",
);
}
} else if (auditSha && postMergeAuditMode === "off") {
- await store.appendAgentLog(taskId, "post-merge audit skipped (mode=off)", "text", undefined, "merger");
+ await store.appendAgentLog(taskId, "post-merge audit skipped (mode=off)", "status", undefined, "merger");
mergerLog.log(`${taskId}: post-merge audit skipped (mode=off)`);
}
if (isEmptyCommit) {
@@ -10460,7 +10460,7 @@ export async function aiMergeTask(
await store.appendAgentLog(
taskId,
`merger: landed-files attribution failed, falling back to full-range capture (${message})`,
- "text",
+ "status",
undefined,
"merger",
);
@@ -10605,7 +10605,7 @@ export async function aiMergeTask(
await store.appendAgentLog(
taskId,
summaryParts.join(" · "),
- "text",
+ "status",
undefined,
"merger",
);
diff --git a/packages/engine/src/reviewer.ts b/packages/engine/src/reviewer.ts
index 866087c779..0bb59a9c37 100644
--- a/packages/engine/src/reviewer.ts
+++ b/packages/engine/src/reviewer.ts
@@ -21,6 +21,8 @@ import { recordRetry } from "./retry-burned-logger.js";
import { mergeEffectiveSettings } from "./effective-settings.js";
import { describeModel, formatModelMarkerDetails, promptWithFallback } from "./pi.js";
import { isContextLimitError } from "./context-limit-detector.js";
+import { classifyError } from "./transient-error-detector.js";
+import { withRetry } from "./retry-with-backoff.js";
import { createResolvedAgentSession, extractRuntimeHint, resolveValidatorSessionModel } from "./agent-session-helpers.js";
import { buildSessionSkillContext } from "./session-skill-context.js";
import { AgentLogger } from "./agent-logger.js";
@@ -40,6 +42,29 @@ import { resolveMcpServersForStore } from "./mcp-resolution.js";
export type ReviewType = "plan" | "code" | "spec";
export type ReviewVerdict = "APPROVE" | "REVISE" | "RETHINK" | "UNAVAILABLE";
+/*
+FNXC:ReviewerProviderErrors 2026-07-15-11:20:
+A reviewer provider failure (rate limit / flaky network) is NOT a review verdict, and must never be laundered into `UNAVAILABLE`.
+
+Root cause this type exists to fix: the reviewer was the only AI lane that never classified provider errors. A 429 became `UNAVAILABLE`, which drove the fallback ladder to re-hit the SAME rate-limited model instantly (when no validator fallback is configured the "fallback" is a same-model strict-prompt rerun), and `fn_review_step` then told the model "code review remains blocking; retry once", so the executor's agent re-called the tool indefinitely. Observed symptom: 14 identical "Reviewer using model: umans/umans-kimi-k2.7" markers with no review text, one per spawned session, hammering an already-limited provider.
+
+`UNAVAILABLE` is reserved for its real meaning: the reviewer RAN and could not produce a parseable verdict. Provider failures throw this instead so they reach the machinery that already exists to handle them — `withRateLimitRetry` backoff, `UsageLimitPauser` global pause, and the executor's bounded transient recovery. See `getDeferredReviewerFatal` in executor.ts for why the escape needs a deferred re-raise.
+*/
+/** Bounded local retry budget for transient network blips inside one review attempt. */
+const REVIEWER_TRANSIENT_MAX_RETRIES = 3;
+
+export class ReviewerProviderError extends Error {
+ constructor(
+ message: string,
+ /** `usage-limit` → global pause; `transient` → bounded recovery retry. Never `permanent`. */
+ public readonly classification: "usage-limit" | "transient",
+ options?: { cause?: unknown },
+ ) {
+ super(message, options);
+ this.name = "ReviewerProviderError";
+ }
+}
+
export interface ReviewResult {
verdict: ReviewVerdict;
review: string;
@@ -374,6 +399,14 @@ export async function reviewStep(
};
};
+ /*
+ FNXC:ReviewerModelMarker 2026-07-15-11:20:
+ The "Reviewer using model:" marker is emitted per SESSION CONSTRUCTION, and the dashboard resolves the reviewer's effective model by taking the LATEST matching marker (effective-model-resolution.ts). So the marker only carries information when the model CHANGES; re-emitting an identical marker for every retry of the same model tells operators nothing and is what turned a provider outage into 14 glued repetitions in the Chat tab.
+
+ Dedupe on marker text, not on attempt count: a real fallback to a DIFFERENT model still emits (the model changed, so the dashboard must see it), while same-model retries stay silent. Scoped per reviewStep call so each review still records the model it actually ran on.
+ */
+ let lastEmittedModelMarker: string | undefined;
+
const createReviewerSession = async (
overrides?: { forceProvider?: string; forceModelId?: string },
): Promise => {
@@ -457,9 +490,10 @@ export async function reviewStep(
const reviewerModelDetails = formatModelMarkerDetails(reviewerModelDesc, options.defaultThinkingLevel);
const reviewerModelMarker = `Reviewer using model: ${reviewerModelDetails}`;
reviewerLog.log(`${taskId}: reviewer using model ${reviewerModelDetails}`);
- if (options.store && options.taskId) {
+ if (options.store && options.taskId && reviewerModelMarker !== lastEmittedModelMarker) {
+ lastEmittedModelMarker = reviewerModelMarker;
await options.store.logEntry(options.taskId, reviewerModelMarker);
- await options.store.appendAgentLog(options.taskId, reviewerModelMarker, "text", undefined, "reviewer").catch(() => undefined);
+ await options.store.appendAgentLog(options.taskId, reviewerModelMarker, "status", undefined, "reviewer").catch(() => undefined);
}
activeSessions.add(session);
@@ -485,7 +519,7 @@ export async function reviewStep(
checkSessionError(session);
};
- const runAttempt = async (
+ const runAttemptOnce = async (
attemptRequest: string,
sessionOptions?: { forceProvider?: string; forceModelId?: string },
): Promise<{ verdict: ReviewVerdict; summary: string; review: string }> => {
@@ -567,6 +601,33 @@ export async function reviewStep(
return { verdict, review: reviewText, summary };
};
+ /*
+ FNXC:ReviewerTransientRetry 2026-07-15-11:20:
+ A flaky network must degrade gracefully, not bounce the task. A dropped socket / gateway blip during a review is a temporary infrastructure condition, so absorb it HERE with exponential backoff + jitter rather than surfacing it as a failed review: a whole-attempt retry gets a clean session and a clean `reviewText` buffer (a half-streamed response would otherwise poison verdict extraction).
+
+ Deliberately NOT a retry-storm: `withRetry` re-throws usage-limit errors immediately without sleeping (rate limits need the global pause, not a local retry that re-hits the limited provider), and re-throws permanent errors immediately (a genuinely broken review must reach the fallback ladder, not spin). Only `classifyError() === "transient"` retries, capped and with jitter so concurrent reviewers do not thunder.
+
+ Backoff is short (2s base) relative to the executor's rate-limit curve (30s base) because a network blip resolves in seconds while a rate limit needs a real cooldown.
+ */
+ const runAttempt = async (
+ attemptRequest: string,
+ sessionOptions?: { forceProvider?: string; forceModelId?: string },
+ ): Promise<{ verdict: ReviewVerdict; summary: string; review: string }> =>
+ withRetry(() => runAttemptOnce(attemptRequest, sessionOptions), {
+ maxRetries: REVIEWER_TRANSIENT_MAX_RETRIES,
+ baseDelayMs: 2_000,
+ maxDelayMs: 30_000,
+ jitter: "full",
+ isRetryable: (err) => classifyError(err instanceof Error ? err.message : String(err)) === "transient",
+ onRetry: (attempt, delayMs, error) => {
+ const message = `${reviewType} review hit a transient network error — retry ${attempt}/${REVIEWER_TRANSIENT_MAX_RETRIES} in ${Math.round(delayMs / 1000)}s: ${error.message}`;
+ reviewerLog.warn(`${taskId}: ${message}`);
+ if (options.store && options.taskId) {
+ void options.store.logEntry(options.taskId, message).catch(() => undefined);
+ }
+ },
+ });
+
const fallbackReviewRequest = `${request}\n\nIMPORTANT: Respond with exactly one of: APPROVE | REVISE | RETHINK on a line starting with "Verdict:".`;
const logFallbackRetry = async (reason: string, mode: string): Promise => {
@@ -605,6 +666,24 @@ export async function reviewStep(
try {
firstAttempt = await runAttempt(request);
} catch (err) {
+ /*
+ FNXC:ReviewerProviderErrors 2026-07-15-11:20:
+ Classify BEFORE the fallback ladder. The ladder's premise is "this model produced a bad review, try another prompt/model" — a premise that is false for provider failures and actively harmful for them:
+ - usage-limit: the ladder's same-model strict-prompt rerun (taken whenever no validator fallback is configured) re-hits the exact model that just rate-limited us, with no delay. Escalate instead so `withRateLimitRetry` backs off and `UsageLimitPauser` pauses every lane.
+ - transient: `runAttempt` already spent its bounded backoff budget above, so the network is genuinely down. Escalate to the executor's bounded recovery (requeue with delay) rather than burning the reviewer fallback budget on a dead link.
+ Neither burns `reviewerFallbackRetryCount` — that budget exists to bound BAD REVIEWS, and spending it on an outage would fail tasks that have nothing wrong with them.
+ */
+ const providerErrorMessage = err instanceof Error ? err.message : String(err);
+ const classification = classifyError(providerErrorMessage);
+ if (classification === "usage-limit" || classification === "transient") {
+ const escalationMessage = `${reviewType} review could not reach the model (${classification}) — escalating to the engine retry path: ${providerErrorMessage}`;
+ reviewerLog.warn(`${taskId}: ${escalationMessage}`);
+ if (options.store && options.taskId) {
+ await options.store.logEntry(options.taskId, escalationMessage).catch(() => undefined);
+ }
+ throw new ReviewerProviderError(providerErrorMessage, classification, { cause: err });
+ }
+
if (hasConfiguredFallback) {
await logFallbackRetry("reviewer error", `${validatorFallbackProvider}/${validatorFallbackModelId}`);
if (options.store && options.taskId && retrySettings && typeof options.store.getTask === "function") {
diff --git a/packages/engine/src/step-session-executor.ts b/packages/engine/src/step-session-executor.ts
index 29a2e8e744..ec1ab678db 100644
--- a/packages/engine/src/step-session-executor.ts
+++ b/packages/engine/src/step-session-executor.ts
@@ -1499,7 +1499,7 @@ Follow instructions precisely and avoid unrelated changes.`,
await this.store.appendAgentLog(
taskDetail.id,
`[step-exec] Reduced-prompt recovery succeeded for step ${stepIndex}`,
- "text",
+ "status",
);
const result: StepResult = {
stepIndex,
diff --git a/packages/engine/src/triage.ts b/packages/engine/src/triage.ts
index 4ba874cbe5..d07f64a572 100644
--- a/packages/engine/src/triage.ts
+++ b/packages/engine/src/triage.ts
@@ -1270,7 +1270,7 @@ export class TriageProcessor {
await this.store.appendAgentLog(
task.id,
`Triage using model: ${modelDesc}`,
- "text",
+ "status",
undefined,
"triage",
);