From e5be751e4f5f253f4fdd455d339633b6b5ae67cf Mon Sep 17 00:00:00 2001 From: joshavant <830519+joshavant@users.noreply.github.com> Date: Tue, 11 Aug 2026 04:44:31 -0500 Subject: [PATCH] fix(auto-reply): align CLI fallback delivery --- ...arness-source-delivery.integration.test.ts | 99 ++++++++++++++++--- .../reply/agent-runner-fallback-candidate.ts | 4 +- 2 files changed, 88 insertions(+), 15 deletions(-) diff --git a/src/agents/embedded-agent-runner/run.prepared-harness-source-delivery.integration.test.ts b/src/agents/embedded-agent-runner/run.prepared-harness-source-delivery.integration.test.ts index 93f14f2b5ae0..1a07eb44a40e 100644 --- a/src/agents/embedded-agent-runner/run.prepared-harness-source-delivery.integration.test.ts +++ b/src/agents/embedded-agent-runner/run.prepared-harness-source-delivery.integration.test.ts @@ -50,7 +50,7 @@ describe("prepared harness source delivery", () => { it.each([ { name: "delivers one streamed answer when preparation changes tool ownership to automatic", - failsCliPrimary: true, + candidatePath: "cli-failure-embedded" as const, preliminaryVisibleReplies: "message_tool" as const, preparedVisibleReplies: "automatic" as const, expectedTransitions: ["message_tool_only", "automatic"], @@ -61,7 +61,7 @@ describe("prepared harness source delivery", () => { }, { name: "suppresses live output when preparation changes automatic ownership to tool", - failsCliPrimary: false, + candidatePath: "embedded" as const, preliminaryVisibleReplies: "automatic" as const, preparedVisibleReplies: "message_tool" as const, expectedTransitions: ["message_tool_only"], @@ -72,7 +72,7 @@ describe("prepared harness source delivery", () => { }, { name: "lets implicit built-in automatic ownership yield to a prepared tool owner", - failsCliPrimary: false, + candidatePath: "embedded" as const, preliminaryVisibleReplies: undefined, preparedVisibleReplies: "message_tool" as const, expectedTransitions: ["message_tool_only"], @@ -83,15 +83,37 @@ describe("prepared harness source delivery", () => { }, { name: "keeps prepared tool ownership after a failed CLI primary", - failsCliPrimary: true, + candidatePath: "cli-failure-embedded" as const, preliminaryVisibleReplies: "automatic" as const, preparedVisibleReplies: "message_tool" as const, - expectedTransitions: ["message_tool_only", "message_tool_only"], + expectedTransitions: ["automatic", "message_tool_only"], expectedDeliveries: 0, expectedPartials: 0, expectedBlocks: 0, expectedFinals: 0, }, + { + name: "delivers a successful direct CLI reply with its session-stable ownership", + candidatePath: "cli" as const, + preliminaryVisibleReplies: undefined, + preparedVisibleReplies: "automatic" as const, + expectedTransitions: ["automatic"], + expectedDeliveries: 1, + expectedPartials: 0, + expectedBlocks: 0, + expectedFinals: 1, + }, + { + name: "delivers a successful API-to-CLI fallback with its session-stable ownership", + candidatePath: "embedded-failure-cli" as const, + preliminaryVisibleReplies: undefined, + preparedVisibleReplies: "automatic" as const, + expectedTransitions: ["automatic", "automatic"], + expectedDeliveries: 1, + expectedPartials: 0, + expectedBlocks: 0, + expectedFinals: 1, + }, ])("$name", async (testCase) => { await useProductionEmbeddedRunExecutionParamsForTest(); const { createBlockReplyDeliveryHandler } = await vi.importActual< @@ -146,6 +168,9 @@ describe("prepared harness source delivery", () => { await attemptParams.onBlockReply?.({ text: "Streaming progress" }); return makeAttemptResult({ assistantTexts: ["Short fallback final"] }); }); + if (testCase.candidatePath === "embedded-failure-cli") { + mockedRunEmbeddedAttempt.mockRejectedValueOnce(new Error("api primary failed")); + } useOpenAIPlatformAuthFixture(); let embeddedError: unknown; let embeddedParams: unknown; @@ -161,11 +186,35 @@ describe("prepared harness source delivery", () => { runnerState.isCliProviderMock.mockImplementation( (provider: unknown) => provider === "anthropic", ); - runnerState.runCliAgentMock.mockRejectedValueOnce(new Error("cli failed")); + if (testCase.candidatePath === "cli-failure-embedded") { + runnerState.runCliAgentMock.mockRejectedValueOnce(new Error("cli failed")); + } else { + runnerState.runCliAgentMock.mockResolvedValue({ + payloads: [{ text: "Short fallback final" }], + meta: {}, + }); + } runnerState.runWithModelFallbackMock.mockImplementationOnce( async (params: FallbackRunnerParams) => { - if (testCase.failsCliPrimary) { - await params.run("anthropic", "primary").catch(() => undefined); + if (testCase.candidatePath === "cli-failure-embedded") { + await params.run("anthropic", "cli-primary").catch(() => undefined); + } + if (testCase.candidatePath === "cli") { + return { + result: await params.run("anthropic", "cli-primary"), + provider: "anthropic", + model: "cli-primary", + attempts: [], + }; + } + if (testCase.candidatePath === "embedded-failure-cli") { + await params.run("custom", "api-primary").catch(() => undefined); + return { + result: await params.run("anthropic", "cli-fallback"), + provider: "anthropic", + model: "cli-fallback", + attempts: [], + }; } return { result: await params.run("custom", "plugin-fallback"), @@ -249,6 +298,15 @@ describe("prepared harness source delivery", () => { extraSystemPromptBySourceReplyDeliveryMode[ runtimeOpts.sourceReplyDeliveryMode ?? "automatic" ]; + const sessionStableDeliveryMode = + runtimeOpts.sessionPromptSourceReplyDeliveryMode ?? + runtimeOpts.sourceReplyDeliveryMode ?? + "automatic"; + followupRun.run.cliSessionBindingFacts = { + extraSystemPromptStatic: + extraSystemPromptBySourceReplyDeliveryMode[sessionStableDeliveryMode], + sourceReplyDeliveryMode: sessionStableDeliveryMode, + }; const sourceReplyDeliveryRuntime = createSourceReplyDeliveryRuntime({ origin: runtimeOpts.sourceReplyDeliveryModeOrigin ?? "stable_policy", initialMode: runtimeOpts.sourceReplyDeliveryMode ?? "automatic", @@ -311,11 +369,17 @@ describe("prepared harness source delivery", () => { }); await settleReplyDispatcher({ dispatcher }); - expect(mockedGlobalHookRunner.runBeforeModelResolve).toHaveBeenCalledWith( - { prompt: "hello" }, - expect.any(Object), - ); - expect(emittedStreamingCallbacks).toEqual(["partial", "block"]); + if (testCase.candidatePath === "cli") { + expect(mockedGlobalHookRunner.runBeforeModelResolve).not.toHaveBeenCalled(); + } else { + expect(mockedGlobalHookRunner.runBeforeModelResolve).toHaveBeenCalledWith( + { prompt: "hello" }, + expect.any(Object), + ); + } + const cliSucceeded = + testCase.candidatePath === "cli" || testCase.candidatePath === "embedded-failure-cli"; + expect(emittedStreamingCallbacks).toEqual(cliSucceeded ? [] : ["partial", "block"]); expect(onPartialReply).toHaveBeenCalledTimes(testCase.expectedPartials); expect(result.queuedFinal).toBe(testCase.expectedDeliveries === 1); expect(deliver).toHaveBeenCalledTimes(testCase.expectedDeliveries + testCase.expectedBlocks); @@ -342,7 +406,14 @@ describe("prepared harness source delivery", () => { }); expect(dispatcher.getFailedCounts()).toEqual({ tool: 0, block: 0, final: 0 }); expect(modeTransitions).toEqual(testCase.expectedTransitions); - if (testCase.preparedVisibleReplies === "automatic") { + if (cliSucceeded) { + const cliParams = runnerState.runCliAgentMock.mock.calls.at(-1)?.[0] as { + cliSessionBindingFacts?: { sourceReplyDeliveryMode?: string }; + sourceReplyDeliveryMode?: string; + }; + expect(cliParams.cliSessionBindingFacts?.sourceReplyDeliveryMode).toBe("automatic"); + expect(cliParams.sourceReplyDeliveryMode).toBe("automatic"); + } else if (testCase.preparedVisibleReplies === "automatic") { expect(modelVisiblePrompt).toContain("Current-session final text normally routes to source"); expect(modelVisiblePrompt).toContain( "Your replies are automatically sent to this conversation", diff --git a/src/auto-reply/reply/agent-runner-fallback-candidate.ts b/src/auto-reply/reply/agent-runner-fallback-candidate.ts index 61df2869d9a8..484f2d38f75f 100644 --- a/src/auto-reply/reply/agent-runner-fallback-candidate.ts +++ b/src/auto-reply/reply/agent-runner-fallback-candidate.ts @@ -194,9 +194,11 @@ export async function runAgentFallbackCandidates(params: AgentFallbackCycleParam ); const candidateRun = runtime.candidateRun; bindSourceReplyDeliveryRuntime(candidateRun, sourceReplyDeliveryRuntime); + // CLI prompts are fixed to their session binding, so dispatch must publish that + // same stable mode or a valid assistant reply can be silently suppressed. const candidateSourceReplyDeliveryMode = sourceReplyDeliveryModeOrigin === "runtime_default" && runtime.useCliExecution - ? "message_tool_only" + ? (candidateRun.cliSessionBindingFacts?.sourceReplyDeliveryMode ?? "automatic") : sourceReplyDeliveryRuntime.currentMode; const applySourceReplyDeliveryModeBeforeInvocation = sourceReplyDeliveryModeOrigin !== "runtime_default" || runtime.useCliExecution;