From f1be90b656e98a95e095b982ade6b01d9bcdd016 Mon Sep 17 00:00:00 2001 From: Drew Stone Date: Sun, 18 Oct 2026 17:41:14 -0600 Subject: [PATCH] fix(opencode): carry the provider's HTTP status on a failed turn, so a 429 pauses in agent-runtime instead of retrying --- src/backends/opencode.ts | 42 ++++++++++---- tests/opencode-error-message.test.ts | 86 ---------------------------- 2 files changed, 31 insertions(+), 97 deletions(-) delete mode 100644 tests/opencode-error-message.test.ts diff --git a/src/backends/opencode.ts b/src/backends/opencode.ts index bb33d9a..2f6958a 100644 --- a/src/backends/opencode.ts +++ b/src/backends/opencode.ts @@ -17,7 +17,7 @@ * right headers so Moonshot's backend accepts the call. */ -import type { Backend, ChatDelta, ChatRequest, BackendHealth } from './types.js' +import type { Backend, BackendFailureReason, ChatDelta, ChatRequest, BackendHealth } from './types.js' import { versionHealth } from './health.js' import { BackendError, terminalOutcome } from './types.js' import { assertModeSupported } from '../modes.js' @@ -241,7 +241,7 @@ export class OpencodeBackend implements Backend { if (!stdinResult.ok) { throw new BackendError(`opencode stdin write failed: ${stdinResult.error}`, 'upstream') } - let sawError: string | null = null + let sawError: BackendFailureReason | null = null let emittedContent = false let emittedToolCall = false let stepUsage: CollectedUsage | undefined @@ -299,14 +299,23 @@ export class OpencodeBackend implements Backend { // fallback below fired on all of them — 16 of them in this host's bridge logs, each // one a readable provider message rendered as an unreadable JSON dump truncated at // 400 chars, which is where `data` was being cut off. - const errorObject = ev.error as { message?: unknown; name?: unknown; data?: { message?: unknown } } | undefined - sawError = String( - ev.message ?? - errorObject?.data?.message ?? - errorObject?.message ?? - errorObject?.name ?? - `opencode error event without a message: ${JSON.stringify(ev).slice(0, 400)}`, - ) + const errorObject = ev.error as { message?: unknown; name?: unknown; data?: { message?: unknown; statusCode?: unknown } } | undefined + // opencode's APIError states the provider's HTTP status as `data.statusCode`. Without it a + // provider's 429 ("Rate limit reached for requests", "Usage limit reached") reached + // agent-runtime as bare prose, was classed a transient fault, and was retried every few + // seconds instead of paused: magnetar-rust-20261017c ended 636 driver attempts that way. + const status = providerStatus(errorObject?.data?.statusCode) + sawError = { + message: String( + ev.message ?? + errorObject?.data?.message ?? + errorObject?.message ?? + errorObject?.name ?? + `opencode error event without a message: ${JSON.stringify(ev).slice(0, 400)}`, + ), + type: 'upstream', + ...(status === undefined ? {} : { status }), + } continue } @@ -350,7 +359,11 @@ export class OpencodeBackend implements Backend { yield { finish_reason: 'error', internal_session_id: internalSessionId } return } - if (sawError) throw new BackendError(`opencode: ${sawError}`, 'upstream') + // Ended through the delta envelope, not a throw, so the provider's status reaches the caller. + if (sawError) { + yield { ...terminalOutcome('opencode', sawError, emittedToolCall), internal_session_id: internalSessionId } + return + } if (exit.code !== 0) { throw new BackendError(await describeCliExit(spawned, 'opencode', exit.code, stderr.render(), exit.signal), 'upstream') } @@ -574,3 +587,10 @@ function extractText(ev: Record): string | null { } return null } + +/** A provider status stated as a field; the bridge never infers one from prose. */ +function providerStatus(value: unknown): number | undefined { + return typeof value === 'number' && Number.isInteger(value) && value >= 100 && value <= 599 + ? value + : undefined +} diff --git a/tests/opencode-error-message.test.ts b/tests/opencode-error-message.test.ts deleted file mode 100644 index 1846698..0000000 --- a/tests/opencode-error-message.test.ts +++ /dev/null @@ -1,86 +0,0 @@ -import { EventEmitter } from 'node:events' -import { PassThrough } from 'node:stream' -import { describe, expect, it } from 'vitest' - -import { OpencodeBackend } from '../src/backends/opencode.js' -import type { ChatRequest } from '../src/backends/types.js' -import type { SpawnResult, Spawner } from '../src/executors/types.js' - -class FakeChild extends EventEmitter { - stdout = new PassThrough() - stderr = new PassThrough() - stdin = new PassThrough() - exitCode: number | null = null -} - -function streamingSpawner(lines: Array>): Spawner { - return async (): Promise => { - const child = new FakeChild() - queueMicrotask(() => { - for (const line of lines) child.stdout.write(`${JSON.stringify(line)}\n`) - child.stdout.end() - child.stderr.end() - setTimeout(() => { - child.exitCode = 0 - child.emit('close', 0) - }, 10) - }) - return { child: child as never, release() {}, spawnError: () => null } - } -} - -/** - * Every opencode error is a NamedError: `{ name, data: { message, ... } }`. There is no - * top-level `error.message` on any of them, and opencode's own reader takes `data.message` - * first. Reading only `error.message` therefore missed every error and reported each one as - * a raw JSON dump truncated at 400 chars — which is exactly where `data` got cut off. - */ -describe('opencode error event message extraction', () => { - const errorFrom = async (event: Record): Promise => { - const backend = new OpencodeBackend({ - bin: 'opencode', - timeoutMs: 0, - spawner: streamingSpawner([event]), - } as unknown as ConstructorParameters[0]) - const request = { - model: 'opencode/zai-coding-plan/glm-5.2', - messages: [{ role: 'user', content: 'hi' }], - } as unknown as ChatRequest - try { - for await (const _ of backend.chat(request, null, new AbortController().signal)) { /* drain */ } - } catch (error) { - return error instanceof Error ? error.message : String(error) - } - return '' - } - - it('reads the provider message out of error.data.message', async () => { - const message = await errorFrom({ - type: 'error', - timestamp: 1789101762046, - sessionID: 'ses_test', - error: { - name: 'APIError', - data: { message: 'Rate limit reached for requests', statusCode: 429, isRetryable: true }, - }, - }) - expect(message).toContain('Rate limit reached for requests') - expect(message).not.toContain('without a message') - }) - - it('still reads a top-level error.message when one is present', async () => { - const message = await errorFrom({ type: 'error', error: { message: 'plain top-level message' } }) - expect(message).toContain('plain top-level message') - }) - - it('falls back to the error name rather than dumping the raw event', async () => { - const message = await errorFrom({ type: 'error', error: { name: 'ProviderAuthError' } }) - expect(message).toContain('ProviderAuthError') - expect(message).not.toContain('without a message') - }) - - it('keeps the raw-event fallback when the event carries no readable error at all', async () => { - const message = await errorFrom({ type: 'error', error: {} }) - expect(message).toContain('without a message') - }) -})