diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index d37fb4557..6eaacae4d 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -9675,6 +9675,7 @@ export class AgentSession { if (isContextOverflow(message, contextWindow)) return false; if (this.#isClassifierRefusal(message)) return true; + if (this.#isProviderErrorFinishReasonBeforeToolUse(message)) return true; if (this.#streamInterruptedAfterObservableOutput(message)) return false; if (this.#isStaleOpenAIResponsesReplayError(message)) return true; @@ -9719,6 +9720,12 @@ export class AgentSession { return stopType === "refusal" || stopType === "sensitive"; } + #isProviderErrorFinishReasonBeforeToolUse(message: AssistantMessage): boolean { + if (!message.errorMessage) return false; + if (message.content.some(block => block.type === "toolCall")) return false; + return /\bProvider (?:returned error finish_reason|finish_reason:\s*error)\b/i.test(message.errorMessage); + } + #isTransientErrorMessage(errorMessage: string): boolean { return ( this.#isTransientEnvelopeErrorMessage(errorMessage) || this.#isTransientTransportErrorMessage(errorMessage) diff --git a/packages/coding-agent/src/task/executor.ts b/packages/coding-agent/src/task/executor.ts index 5610a7108..551518467 100644 --- a/packages/coding-agent/src/task/executor.ts +++ b/packages/coding-agent/src/task/executor.ts @@ -516,6 +516,7 @@ export function finalizeSubprocessOutput(args: FinalizeSubprocessOutputArgs): Fi const { yieldItems, reportFindings, doneAborted, signalAborted, outputSchema } = args; let abortedViaYield = false; const hasYield = Array.isArray(yieldItems) && yieldItems.length > 0; + const hadFailureBeforeYield = exitCode !== 0 && stderr.trim().length > 0; if (hasYield) { const lastYield = yieldItems[yieldItems.length - 1]; @@ -553,12 +554,16 @@ export function finalizeSubprocessOutput(args: FinalizeSubprocessOutputArgs): Fi const errorMessage = err instanceof Error ? err.message : String(err); rawOutput = `{"error":"Failed to serialize yield data: ${errorMessage}"}`; } - exitCode = 0; - stderr = overridden - ? SUBAGENT_WARNING_SCHEMA_OVERRIDDEN - : schemaError - ? `invalid output schema: ${schemaError}` - : ""; + if (!hadFailureBeforeYield) { + exitCode = 0; + stderr = overridden + ? SUBAGENT_WARNING_SCHEMA_OVERRIDDEN + : schemaError + ? `invalid output schema: ${schemaError}` + : ""; + } else if (!stderr) { + stderr = "Subagent failed after yielding a result."; + } } } } diff --git a/packages/coding-agent/test/agent-session-retry-fallback.test.ts b/packages/coding-agent/test/agent-session-retry-fallback.test.ts index 96687c7d4..fa2d600f3 100644 --- a/packages/coding-agent/test/agent-session-retry-fallback.test.ts +++ b/packages/coding-agent/test/agent-session-retry-fallback.test.ts @@ -1513,4 +1513,63 @@ describe("AgentSession retry fallback", () => { } expect(contentBlock.text).toBe("Recovered after Gemini malformed function call"); }); + + it("auto-retries provider finish_reason errors after partial text", async () => { + const model = getBundledModel("openai", "gpt-4o-mini"); + if (!model) { + throw new Error("Expected bundled OpenAI test model to exist"); + } + + const errorMessage = "Provider returned error finish_reason"; + const mock = createMockModel({ + responses: [ + { content: ["partial output before gateway error"], stopReason: "error", errorMessage }, + { content: ["Recovered after provider finish_reason error"] }, + ], + }); + const agent = new Agent({ + getApiKey: model => `${model.provider}-test-key`, + initialState: { + model, + systemPrompt: ["Test"], + tools: [], + messages: [], + }, + streamFn: (requestedModel, context, options) => mock.stream(requestedModel, context, options), + }); + + const settings = Settings.isolated({ + "compaction.enabled": false, + "retry.baseDelayMs": 5, + "retry.maxRetries": 1, + "retry.modelFallback": false, + }); + settings.setModelRole("default", `${model.provider}/${model.id}`); + + session = new AgentSession({ + agent, + sessionManager: SessionManager.inMemory(), + settings, + modelRegistry, + }); + const { retryStartEvents, retryEndEvents } = trackRetryEvents(session); + + await session.prompt("recover from provider finish_reason error"); + await session.waitForIdle(); + + expect(mock.calls).toHaveLength(2); + expect(retryStartEvents).toHaveLength(1); + expect(retryStartEvents[0].errorMessage).toBe(errorMessage); + expect(retryEndEvents).toHaveLength(1); + expect(session.agent.state.messages).toHaveLength(2); + const assistantMsg = session.agent.state.messages[1]; + if (assistantMsg.role !== "assistant") { + throw new Error(`Expected assistant message, got ${assistantMsg.role}`); + } + const contentBlock = assistantMsg.content[0]; + if (contentBlock.type !== "text") { + throw new Error(`Expected text content block, got ${contentBlock.type}`); + } + expect(contentBlock.text).toBe("Recovered after provider finish_reason error"); + }); }); diff --git a/packages/coding-agent/test/task/executor-subagent-reminders.test.ts b/packages/coding-agent/test/task/executor-subagent-reminders.test.ts index 7f69af243..d3023329f 100644 --- a/packages/coding-agent/test/task/executor-subagent-reminders.test.ts +++ b/packages/coding-agent/test/task/executor-subagent-reminders.test.ts @@ -7,7 +7,11 @@ import type { CreateAgentSessionResult } from "@oh-my-pi/pi-coding-agent/sdk"; import * as sdkModule from "@oh-my-pi/pi-coding-agent/sdk"; import type { AgentSession, AgentSessionEvent, PromptOptions } from "@oh-my-pi/pi-coding-agent/session/agent-session"; import type { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage"; -import { runSubprocess, SUBAGENT_WARNING_MISSING_YIELD } from "@oh-my-pi/pi-coding-agent/task/executor"; +import { + finalizeSubprocessOutput, + runSubprocess, + SUBAGENT_WARNING_MISSING_YIELD, +} from "@oh-my-pi/pi-coding-agent/task/executor"; import type { AgentDefinition } from "@oh-my-pi/pi-coding-agent/task/types"; import { EventBus } from "@oh-my-pi/pi-coding-agent/utils/event-bus"; import { logger } from "@oh-my-pi/pi-utils"; @@ -336,6 +340,38 @@ describe("runSubprocess yield reminders", () => { expect(result.exitCode).toBe(0); expect(result.output).toContain('"ok": true'); }); + + it("keeps a real run failure from being masked by a successful yield", () => { + const result = finalizeSubprocessOutput({ + rawOutput: "partial output", + exitCode: 1, + stderr: "Provider returned error finish_reason", + doneAborted: false, + signalAborted: false, + yieldItems: [{ status: "success", data: { ok: true } }], + outputSchema: undefined, + }); + + expect(result.exitCode).toBe(1); + expect(result.stderr).toBe("Provider returned error finish_reason"); + expect(result.rawOutput).toContain('"ok": true'); + }); + + it("lets a valid yield clear internal termination without stderr", () => { + const result = finalizeSubprocessOutput({ + rawOutput: "", + exitCode: 1, + stderr: "", + doneAborted: true, + signalAborted: false, + yieldItems: [{ status: "success", data: { ok: true } }], + outputSchema: undefined, + }); + + expect(result.exitCode).toBe(0); + expect(result.stderr).toBe(""); + expect(result.rawOutput).toContain('"ok": true'); + }); it("uses provided thinking level when model override has no explicit suffix", async () => { vi.clearAllMocks(); const session = createMockSession(({ emit }) => {