From 4d31cd4013a102c53a6e51d837657f8787021e0d Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Mon, 4 May 2026 12:48:29 +0700 Subject: [PATCH] fix(opencode): forward provider retries instead of swallowing them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OpenCode subproviders (e.g. opencode/kimi-k2.6 on OpenCode Zen) emit session.status:retry events with messages like "Internal server error" when the upstream provider returns 5xx. opencode itself retries indefinitely with backoff and never emits a terminal event for these. Previously the adapter only surfaced retry messages on a small allowlist of "fatal" tokens (insufficient balance, invalid api key, etc.) and silently dropped everything else. The agent appeared hung from the user's perspective — no message, no spinner update, nothing — until the upstream eventually recovered or the user manually interrupted. Forward every session.status:retry as a non-terminal timeline error item so the user can see what opencode is doing, mirroring opencode's own TUI. Drop the fatal-token allowlist: classifying which retries are "really" terminal is opencode's job, and synthesizing turn_failed for ones we guess at is misleading anyway because opencode keeps spending upstream while we tell the user the agent is done. Also a small design pass on the timeline error rendering: - drop the redundant "Agent error" prefix (the message is descriptive) - drop the colored box background (visual weight too high for a retry) - align the icon vertically to the first text line (height+center) - make the message text selectable so users can copy errors --- packages/app/src/components/message.tsx | 10 +-- packages/app/src/types/stream.ts | 6 +- ...code-agent.error-handling.real.e2e.test.ts | 33 ++-------- .../server/agent/providers/opencode-agent.ts | 39 +++++------- .../opencode/event-translator.test.ts | 63 +++++++++++-------- 5 files changed, 65 insertions(+), 86 deletions(-) diff --git a/packages/app/src/components/message.tsx b/packages/app/src/components/message.tsx index f2f2f6ea0..5267eff2d 100644 --- a/packages/app/src/components/message.tsx +++ b/packages/app/src/components/message.tsx @@ -1810,9 +1810,7 @@ const activityLogStylesheet = StyleSheet.create((theme) => ({ successBg: { backgroundColor: "rgba(20, 83, 45, 0.3)", }, - errorBg: { - backgroundColor: "rgba(127, 29, 29, 0.3)", - }, + errorBg: {}, artifactBg: { backgroundColor: "rgba(30, 58, 138, 0.4)", }, @@ -1827,6 +1825,8 @@ const activityLogStylesheet = StyleSheet.create((theme) => ({ }, iconContainer: { flexShrink: 0, + height: 20, + justifyContent: "center", }, textContainer: { flex: 1, @@ -1936,7 +1936,9 @@ export const ActivityLog = memo(function ActivityLog({ - {displayMessage} + + {displayMessage} + {metadata && ( Details diff --git a/packages/app/src/types/stream.ts b/packages/app/src/types/stream.ts index 9f8570731..b2b7e69c8 100644 --- a/packages/app/src/types/stream.ts +++ b/packages/app/src/types/stream.ts @@ -543,10 +543,6 @@ function appendTodoList( return [...state, entry]; } -function formatErrorMessage(message: string): string { - return `Agent error\n${message}`; -} - function reduceTimelineToolCall( state: StreamItem[], event: Extract, @@ -672,7 +668,7 @@ function reduceTimelineEvent( id: createTimelineId("error", item.message ?? "", timestamp), timestamp, activityType: "error", - message: formatErrorMessage(item.message ?? "Unknown error"), + message: item.message ?? "Unknown error", }; return finalizeActiveThoughts(appendActivityLog(state, activity)); } diff --git a/packages/server/src/server/agent/providers/opencode-agent.error-handling.real.e2e.test.ts b/packages/server/src/server/agent/providers/opencode-agent.error-handling.real.e2e.test.ts index 356219afa..bf852d30c 100644 --- a/packages/server/src/server/agent/providers/opencode-agent.error-handling.real.e2e.test.ts +++ b/packages/server/src/server/agent/providers/opencode-agent.error-handling.real.e2e.test.ts @@ -148,31 +148,10 @@ describe("opencode agent error handling (real)", () => { } }, 45_000); - test("surfaces fatal retry status from zai/glm-5.1 instead of hanging forever", async () => { - const client = new OpenCodeAgentClient(pino({ level: "silent" })); - const session = await client.createSession({ - provider: "opencode", - cwd: process.cwd(), - modeId: "build", - }); - - try { - await session.setModel("zai/glm-5.1"); - - const events: AgentStreamEvent[] = []; - for await (const event of streamSession(session, "Say hello")) { - events.push(event); - if (isTerminalEvent(event)) break; - } - - const terminal = events.find(isTerminalEvent); - expect(terminal).toBeDefined(); - expect(terminal!.type).toBe("turn_failed"); - expect((terminal!.type === "turn_failed" ? terminal.error : "").toLowerCase()).toMatch( - /insufficient balance|resource package|recharge/, - ); - } finally { - await session.close().catch(() => undefined); - } - }, 45_000); + // Note: there used to be a real-API test here pinned to zai/glm-5.1's + // "insufficient balance" retry. It's been removed because retry behavior is + // entirely upstream-dependent — opencode itself decides when to retry, and + // OpenCode Zen's quota/availability changes over time. The translation logic + // (session.status:retry → timeline error item) is covered by unit tests in + // opencode/event-translator.test.ts. }); diff --git a/packages/server/src/server/agent/providers/opencode-agent.ts b/packages/server/src/server/agent/providers/opencode-agent.ts index 9b02eb5f2..9b44eb7fe 100644 --- a/packages/server/src/server/agent/providers/opencode-agent.ts +++ b/packages/server/src/server/agent/providers/opencode-agent.ts @@ -117,18 +117,6 @@ const OPENCODE_HANDLED_BUILTIN_SLASH_COMMANDS: AgentSlashCommand[] = [ { name: "compact", description: "Compact the current session", argumentHint: "" }, { name: "summarize", description: "Compact the current session", argumentHint: "" }, ]; -const OPENCODE_FATAL_RETRY_MESSAGE_TOKENS = [ - "insufficient balance", - "no resource package", - "please recharge", - "invalid api key", - "unauthorized", - "authentication", - "model not found", - "unknown model", - "does not exist", - "unsupported model", -] as const; const OPENCODE_HEADERS_TIMEOUT_TOKENS = [ "headers timeout", "headers timeout error", @@ -329,14 +317,6 @@ async function reconcileOpenCodeSessionClose(params: { } } -function isFatalOpenCodeRetryMessage(message: string | null | undefined): boolean { - const normalized = typeof message === "string" ? message.trim().toLowerCase() : ""; - if (!normalized) { - return false; - } - return OPENCODE_FATAL_RETRY_MESSAGE_TOKENS.some((token) => normalized.includes(token)); -} - function isOpenCodeHeadersTimeoutFailure(error: unknown): boolean { const diagnostics = new Set(); const queue: unknown[] = [error]; @@ -2161,15 +2141,24 @@ function appendOpenCodeSessionStatus( events.push({ type: "turn_completed", provider: "opencode", usage: undefined }); return; } - if (status.type === "retry" && isFatalOpenCodeRetryMessage(status.message)) { - resetOpenCodeTurnTrackingState(state); + if (status.type === "retry") { + // Mirror what opencode's TUI shows: retry attempts are visible activity, not + // terminal. opencode itself never gives up — it backs off and tries again + // forever. If we silently swallow these the user sees a spinner with no + // explanation. Forwarding as a timeline error item is a no-op for old + // clients (the schema already supports it). + const message = typeof status.message === "string" ? status.message.trim() : ""; + const text = message + ? `Provider retry (attempt ${status.attempt}): ${message}` + : `Provider retry (attempt ${status.attempt})`; events.push({ - type: "turn_failed", + type: "timeline", provider: "opencode", - error: toDiagnosticErrorMessage(status.message), + item: { type: "error", message: text }, }); + return; } - // "retry" and "busy" are transient — no terminal event. + // "busy" is transient — no terminal event, no surfaced activity. } interface Deferred { diff --git a/packages/server/src/server/agent/providers/opencode/event-translator.test.ts b/packages/server/src/server/agent/providers/opencode/event-translator.test.ts index 7e5522306..daae7c51a 100644 --- a/packages/server/src/server/agent/providers/opencode/event-translator.test.ts +++ b/packages/server/src/server/agent/providers/opencode/event-translator.test.ts @@ -735,7 +735,7 @@ describe("translateOpenCodeEvent", () => { expect(state.partTypes.size).toBe(0); }); - it("emits turn_failed from fatal session.status retry", () => { + it("forwards session.status retry as a non-terminal timeline error item", () => { const state = createState(); state.streamedPartKeys.add("text:part-1"); state.partTypes.set("part-1", "text"); @@ -747,8 +747,8 @@ describe("translateOpenCodeEvent", () => { sessionID: "session-1", status: { type: "retry", - attempt: 2, - message: "Invalid API key", + attempt: 3, + message: "Internal server error", next: Date.now() + 1000, }, }, @@ -758,16 +758,46 @@ describe("translateOpenCodeEvent", () => { expect(result).toEqual([ { - type: "turn_failed", + type: "timeline", provider: "opencode", - error: "Invalid API key", + item: { type: "error", message: "Provider retry (attempt 3): Internal server error" }, }, ]); - expect(state.streamedPartKeys.size).toBe(0); - expect(state.partTypes.size).toBe(0); + // Streaming state must NOT be reset — the turn is still alive, opencode + // will eventually either succeed or emit session.idle / session.error. + expect(state.streamedPartKeys.size).toBe(1); + expect(state.partTypes.size).toBe(1); }); - it("ignores transient session.status updates", () => { + it("forwards retry without a message using just the attempt number", () => { + const state = createState(); + + const result = translateOpenCodeEvent( + { + type: "session.status", + properties: { + sessionID: "session-1", + status: { + type: "retry", + attempt: 1, + message: "", + next: Date.now() + 1000, + }, + }, + }, + state, + ); + + expect(result).toEqual([ + { + type: "timeline", + provider: "opencode", + item: { type: "error", message: "Provider retry (attempt 1)" }, + }, + ]); + }); + + it("ignores transient session.status busy updates", () => { const state = createState(); const busy = translateOpenCodeEvent( @@ -781,24 +811,7 @@ describe("translateOpenCodeEvent", () => { state, ); - const retry = translateOpenCodeEvent( - { - type: "session.status", - properties: { - sessionID: "session-1", - status: { - type: "retry", - attempt: 1, - message: "rate limited", - next: Date.now() + 1000, - }, - }, - }, - state, - ); - expect(busy).toEqual([]); - expect(retry).toEqual([]); }); it("emits structured assistant output when schema mode completes without text parts", () => {