Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/engine/retry-fallback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,14 @@ export function withModelFallbackRetry(spawn: SpawnFn, fallback: string | undefi
return async (opts) => {
const first = await spawn(opts);
if (first.status === "failed" && first.retryable && fallback !== first.model && !signal?.aborted) {
return spawn({ ...opts, model: fallback });
const second = await spawn({ ...opts, model: fallback });
// #59: when the fallback also fails, compose the primary's failure into the surfaced error
// (same contract as the direct-foreground path in tools/subagent.ts) — the fallback's error
// alone masks why the primary failed.
if (second.status === "failed" && first.error && first.error !== second.error) {
second.error = `primary '${first.model}' failed: ${first.error}; fallback '${fallback}' failed: ${second.error ?? second.status}`;
}
return second;
}
return first;
};
Expand Down
3 changes: 3 additions & 0 deletions src/engine/spawnSubagent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,9 @@ async function finishRun(
type: "run:ended", runId, status, endedAt,
resultSummary: finalText.slice(0, 120), tokenTotal, costTotal, contextTokens,
resumedFrom: opts.resumeLink, forkedFrom: opts.forkLink,
// #59: journal the failure reason — the archived failing runs had run:ended with an empty
// resultSummary and no error field, making post-hoc diagnosis from the journal impossible.
error,
});
} catch { /* best-effort: journal is the index, not the product */ }
// SPEC-4: lifecycle phase children skip the per-run todo reconciliation — the lifecycle
Expand Down
2 changes: 2 additions & 0 deletions src/runtime/run-log.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ export interface RunEndedEvent {
costTotal?: number;
/** SPEC-6-1: latest context-token snapshot at run end. */
contextTokens?: number;
/** #59: the failure reason on failed runs (post-hoc diagnosability from the journal). */
error?: string;
}
export type RunLogEvent = RunMetaEvent | MessageEvent | ToolEvent | RunEndedEvent;

Expand Down
9 changes: 9 additions & 0 deletions src/tools/subagent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,15 @@ export function createSubagentTool(deps: SubagentToolDeps) {
});
retriedWithModel = fallback;
}
// #59: when the fallback retry ALSO fails, surface the PRIMARY's failure too — returning
// only the fallback's error masked why the primary (e.g. an explicit model string) failed
// at all, making provider diagnosis impossible from the controller's seat.
if (retriedWithModel && finalRes.status === "failed" && res.error && res.error !== finalRes.error) {
finalRes = {
...finalRes,
error: `primary '${res.model}' failed: ${res.error}; fallback '${retriedWithModel}' failed: ${finalRes.error ?? finalRes.status}`,
};
}
const isError = finalRes.status === "failed" || finalRes.status === "aborted";
return {
content: [{ type: "text" as const, text: isError ? (finalRes.error ?? finalRes.status) : finalRes.finalText }],
Expand Down
15 changes: 15 additions & 0 deletions test/retry-fallback.test.mts
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,18 @@ test("#39 wrapper: signal already aborted → NO retry (even on retryable failur
assert.equal(out.status, "failed", "first (failed) result returned");
assert.deepEqual(calls, ["(none)"], "no retry when signal aborted");
});

test("#59: both attempts fail → returned error includes the PRIMARY's failure (no masking)", async () => {
const { fn, calls } = fakeSpawn([
res({ retryable: true, model: "Ollama/glm", error: "rate limited" }),
res({ model: "openrouter/glm", error: "also rate limited" }),
]);
const wrapped = withModelFallbackRetry(fn, "openrouter/glm");
const out = await wrapped(baseOpts);
assert.equal(out.status, "failed");
assert.deepEqual(calls, ["(none)", "openrouter/glm"], "retried once on the fallback");
assert.ok(out.error!.includes("Ollama/glm"), `names the primary model: ${out.error}`);
assert.ok(out.error!.includes("rate limited"), "includes the primary's error text");
assert.ok(out.error!.includes("openrouter/glm"), "names the fallback model");
assert.ok(out.error!.includes("also rate limited"), "includes the fallback's error text");
});
24 changes: 23 additions & 1 deletion test/spawn-subagent-runlog.test.mts
Original file line number Diff line number Diff line change
Expand Up @@ -128,4 +128,26 @@ test("tool_error result is kept in full in the journal", async () => {
const toolEvent = log.replay(res.runId).find((e) => e.type === "tool") as any;
strictEqual(toolEvent.isError, true);
ok(toolEvent.result.length > 20, "error result not excerpted");
});
});
test("#59: run:ended carries the failure reason (error) on failed runs", async () => {
// The archived #59 failing runs had run:ended with empty resultSummary and NO error field —
// post-hoc diagnosis from the journal was impossible. The failure reason must be journaled.
const handlers: Array<(e: any) => void> = [];
const errChild: ChildSession = {
prompt: async () => { for (const h of handlers) h({ type: "message_end", message: { role: "assistant", stopReason: "error", content: [{ type: "text", text: "quota exhausted" }] } }); },
subscribe: (h: any) => { handlers.push(h); return () => {}; },
abort: async () => {}, dispose: () => {},
};
const factory: ChildSessionFactory = { create: async () => ({ session: errChild, model: "m" }) };
const log = new RunLog(logDir);
const h = harness(factory, log);
const res = await spawnSubagent({
agent: "g", task: "t", track: true, runLog: log,
registry: h.registry, todoSync: h.todoSync, runRegistry: h.runRegistry, lock: h.lock, backendRegistry: h.backendRegistry,
parentModel: PARENT, parentCwd: tmpDir,
});
strictEqual(res.status, "failed");
const ended = log.replay(res.runId).at(-1) as any;
strictEqual(ended.type, "run:ended");
ok(typeof ended.error === "string" && ended.error.includes("quota exhausted"), `run:ended.error present + meaningful: ${ended.error}`);
});
36 changes: 36 additions & 0 deletions test/subagent-tool.test.mts
Original file line number Diff line number Diff line change
Expand Up @@ -451,3 +451,39 @@ test("SPEC-6-5: same-cwd dispatch does NOT fire onNotify", async () => {
test("SPEC-6-5: subagentParams schema includes cwd", () => {
ok("cwd" in subagentParams.properties, "cwd field in the schema");
});

test("#59: when the fallback retry also fails, the surfaced error includes the PRIMARY's failure", async () => {
// Regression: the #39 retry returned only the FALLBACK's error — the primary's actual failure
// (e.g. why an explicit model string failed at all) was masked entirely (#59 dogfood finding:
// the surfaced error named 'openrouter/z-ai/glm-5.2' while the primary 'Ollama/glm-5.2:cloud'
// failure was invisible).
let createCalls = 0;
const makeErrorChild = (text: string) => {
const hs: Array<(e: any) => void> = [];
return {
prompt: async () => { for (const h of hs) h({ type: "message_end", message: { role: "assistant", stopReason: "error", content: [{ type: "text", text }] } }); },
subscribe: (h: any) => { hs.push(h); return () => {}; }, abort: async () => {}, dispose: () => {},
};
};
const factory: ChildSessionFactory = {
create: async () => {
createCalls += 1;
return createCalls === 1
? { session: makeErrorChild("primary rate limited"), model: "Ollama/glm-5.2:cloud" }
: { session: makeErrorChild("fallback rate limited too"), model: "openrouter/z-ai/glm-5.2" };
},
};
const deps = makeDeps();
const reg = new BackendRegistry();
reg.register({ id: "pi", factory, available: () => true, versionInfo: () => null, hookParity: PI_HOOK_PARITY });
deps.backendRegistry = reg;
const tool = createSubagentTool(deps);
const out = await tool.execute!("c", { agent: "g", task: "x", model: "Ollama/glm-5.2:cloud", modelFallback: "openrouter/z-ai/glm-5.2" } as any, new AbortController().signal, () => {}, {} as any);
strictEqual(createCalls, 2, "factory called twice (primary failed retryable → retried on fallback)");
strictEqual(out.isError, true, "both primary and fallback failed");
const text = (out.content as any)[0].text as string;
ok(text.includes("Ollama/glm-5.2:cloud"), `error names the PRIMARY model: ${text}`);
ok(text.includes("primary rate limited"), `error includes the PRIMARY's failure text: ${text}`);
ok(text.includes("openrouter/z-ai/glm-5.2"), `error names the FALLBACK model: ${text}`);
ok(text.includes("fallback rate limited too"), `error includes the FALLBACK's failure text: ${text}`);
});
Loading