From 8317e23c1c2bef85989023ea0fcc59b1cfe061a3 Mon Sep 17 00:00:00 2001 From: synth Date: Sat, 1 Aug 2026 03:54:04 -0400 Subject: [PATCH] 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 --- .../embedded-agent-runner/run-entry.test.ts | 117 ++++++++++++++++++ src/agents/embedded-agent-runner/run-entry.ts | 17 ++- 2 files changed, 133 insertions(+), 1 deletion(-) diff --git a/src/agents/embedded-agent-runner/run-entry.test.ts b/src/agents/embedded-agent-runner/run-entry.test.ts index 2a55c7c8f88b..88f1e24f2810 100644 --- a/src/agents/embedded-agent-runner/run-entry.test.ts +++ b/src/agents/embedded-agent-runner/run-entry.test.ts @@ -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; 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); diff --git a/src/agents/embedded-agent-runner/run-entry.ts b/src/agents/embedded-agent-runner/run-entry.ts index 864cf354eb7c..7162648a75c2 100644 --- a/src/agents/embedded-agent-runner/run-entry.ts +++ b/src/agents/embedded-agent-runner/run-entry.ts @@ -195,6 +195,21 @@ export async function runEmbeddedAgentEntry( 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({ ...params.selection, ...params.identity, @@ -250,7 +265,7 @@ export async function runEmbeddedAgentEntry( : effectiveClassification; }, }), - ...(committedSideEffect ? { canFallbackAfterError: () => !committedSideEffect() } : {}), + ...(canFallbackAfterError ? { canFallbackAfterError } : {}), ...(params.behavior.kind === "maintenance" ? {} : {