fix(agents): stop fallback after delivered-but-failed attempts (#114628)

A thrown channel-delivery attempt bypasses result classification in the
model-fallback loop, so the delivery guards never ran and the loop could
advance to a second candidate after the primary already delivered its
reply, producing a duplicate visible answer.

Wire the existing canFallbackAfterError backstop for channel-delivery
behaviors using the same live delivery evidence the result classifier
already consults, so the thrown-error exit suppresses fallback once a
reply was delivered, while attempts that delivered nothing still fall
back.

Closes #113788
This commit is contained in:
synth
2026-08-01 03:54:04 -04:00
committed by GitHub
parent 04f9a204fd
commit 8317e23c1c
2 changed files with 133 additions and 1 deletions

View File

@@ -22,6 +22,13 @@ type FallbackRunnerParams = {
attempt: number;
total: number;
}) => unknown;
canFallbackAfterError?: (params: {
provider: string;
model: string;
error: unknown;
attempt: number;
total: number;
}) => boolean | Promise<boolean>;
mergeExhaustedResult?: (params: {
latestResult: EmbeddedAgentRunResult;
preferredResult: EmbeddedAgentRunResult;
@@ -217,6 +224,116 @@ describe("runEmbeddedAgentEntry", () => {
expect(result.result.payloads).toEqual([{ text: "recovered" }]);
});
it("does not replay a thrown channel-delivery attempt that already delivered its reply (#113788)", async () => {
const failure = new Error("insufficient quota");
state.runWithModelFallback.mockImplementationOnce(async (params: FallbackRunnerParams) => {
// Mirror the fallback loop's thrown-error exit: the attempt error bypasses
// result classification, so the error-path backstop is the only guard that
// can stop the next candidate from replaying the delivered turn.
await expect(params.run(params.provider, params.model)).rejects.toBe(failure);
const allowed = await params.canFallbackAfterError?.({
provider: params.provider,
model: params.model,
error: failure,
attempt: 1,
total: 2,
});
expect(allowed).toBe(false);
throw failure;
});
const { runEmbeddedAgentEntry } = await import("./run-entry.js");
const runCandidate = vi.fn(async (_provider: string, _model: string) => {
throw failure;
});
await expect(
runEmbeddedAgentEntry({
selection: { cfg: {}, provider: "primary-provider", model: "primary-model" },
identity: { runId: "channel-throw", agentId: "main", sessionId: "session-1" },
harness: {
workspaceDir: "/tmp/workspace",
preparation: { kind: "direct" },
resolveRuntimeOverride: () => undefined,
},
behavior: {
kind: "channel-delivery",
readDeliveryEvidence: () => ({
hasDirectlySentBlockReply: true,
hasBlockReplyPipelineOutput: false,
}),
},
sessionOverride: { kind: "preserve" },
runCandidate,
}),
).rejects.toBe(failure);
expect(runCandidate).toHaveBeenCalledTimes(1);
});
it("still falls back when a thrown channel-delivery attempt delivered nothing", async () => {
const failure = new Error("insufficient quota");
state.runWithModelFallback.mockImplementationOnce(async (params: FallbackRunnerParams) => {
await expect(params.run(params.provider, params.model)).rejects.toBe(failure);
const allowed = await params.canFallbackAfterError?.({
provider: params.provider,
model: params.model,
error: failure,
attempt: 1,
total: 2,
});
expect(allowed).toBe(true);
const fallbackProvider = "fallback-provider";
const fallbackModel = "fallback-model";
const result = await params.run(fallbackProvider, fallbackModel, {
isFinalFallbackAttempt: true,
});
return {
outcome: "completed" as const,
result,
provider: fallbackProvider,
model: fallbackModel,
attempts: [
{
provider: params.provider,
model: params.model,
error: failure.message,
reason: "billing" as const,
},
],
};
});
const { runEmbeddedAgentEntry } = await import("./run-entry.js");
const runCandidate = vi.fn(async (provider: string, model: string) => {
if (provider === "primary-provider") {
throw failure;
}
return makeResult({ provider, model });
});
const result = await runEmbeddedAgentEntry({
selection: { cfg: {}, provider: "primary-provider", model: "primary-model" },
identity: { runId: "channel-throw-empty", agentId: "main", sessionId: "session-1" },
harness: {
workspaceDir: "/tmp/workspace",
preparation: { kind: "direct" },
resolveRuntimeOverride: () => undefined,
},
behavior: {
kind: "channel-delivery",
readDeliveryEvidence: () => ({
hasDirectlySentBlockReply: false,
hasBlockReplyPipelineOutput: false,
}),
},
sessionOverride: { kind: "preserve" },
runCandidate,
});
expect(runCandidate).toHaveBeenCalledTimes(2);
expect(result.outcome).toBe("completed");
expect(result.provider).toBe("fallback-provider");
});
it("retains non-visible follow-up results for terminal delivery", async () => {
state.runWithModelFallback.mockImplementationOnce(async (params: FallbackRunnerParams) => {
const result = await params.run(params.provider, params.model);

View File

@@ -195,6 +195,21 @@ export async function runEmbeddedAgentEntry<T extends EmbeddedAgentRunResult>(
let candidateIndex = 0;
const committedSideEffect =
params.behavior.kind === "command-rpc" ? params.behavior.hasCommittedSideEffect : undefined;
const readChannelDeliveryEvidence =
params.behavior.kind === "channel-delivery" ? params.behavior.readDeliveryEvidence : undefined;
// Thrown candidate errors skip result classification, so without an error-path
// backstop the loop advances to the next candidate even when the attempt already
// delivered its reply, producing a duplicate visible answer (#113788). Consult the
// same live delivery evidence the result classifier already uses so both exit
// paths suppress fallback after a delivered reply.
const canFallbackAfterError = committedSideEffect
? () => !committedSideEffect()
: readChannelDeliveryEvidence
? () => {
const evidence = readChannelDeliveryEvidence();
return !evidence.hasDirectlySentBlockReply && !evidence.hasBlockReplyPipelineOutput;
}
: undefined;
const fallbackResult = await runWithModelFallback<T>({
...params.selection,
...params.identity,
@@ -250,7 +265,7 @@ export async function runEmbeddedAgentEntry<T extends EmbeddedAgentRunResult>(
: effectiveClassification;
},
}),
...(committedSideEffect ? { canFallbackAfterError: () => !committedSideEffect() } : {}),
...(canFallbackAfterError ? { canFallbackAfterError } : {}),
...(params.behavior.kind === "maintenance"
? {}
: {