diff --git a/src/servers/api/chat/opencode/server-manager.ts b/src/servers/api/chat/opencode/server-manager.ts index dd1377de..1190c545 100644 --- a/src/servers/api/chat/opencode/server-manager.ts +++ b/src/servers/api/chat/opencode/server-manager.ts @@ -1,7 +1,7 @@ import { getOpenCodeServerUrl } from './sidecar-server'; // The OpenCode server is owned by the officer-opencode sidecar, which starts `opencode serve` on a -// random port (cwd = DATA_PATH/opencode-sidecar) and reports it to the API (getOpenCodeServerUrl). +// random port (cwd = DATA_PATH/opencode_server) and reports it to the API (getOpenCodeServerUrl). // All OpenCode HTTP traffic routes to whatever port the sidecar last reported. // `isServerHealthy` lived here and had no callers — removed 2026-08-10 with the rest of the dead diff --git a/src/servers/sidecar/opencode/index.ts b/src/servers/sidecar/opencode/index.ts index dca3103d..a6aa4016 100644 --- a/src/servers/sidecar/opencode/index.ts +++ b/src/servers/sidecar/opencode/index.ts @@ -83,7 +83,7 @@ async function waitHealthy(baseUrl: string, timeoutMs: number): Promise return false; } -// ── Start the OpenCode server (cwd = DATA_PATH/opencode-sidecar) ── +// ── Start the OpenCode server (cwd = DATA_PATH/opencode_server) ── mkdirSync(SERVE_CWD, { recursive: true }); // An AGENTS.md used to be seeded here, telling the agent to read its working directory from the diff --git a/src/servers/sidecar/opencode/runner.test.ts b/src/servers/sidecar/opencode/runner.test.ts new file mode 100644 index 00000000..6627d229 --- /dev/null +++ b/src/servers/sidecar/opencode/runner.test.ts @@ -0,0 +1,111 @@ +import { describe, expect, it } from 'bun:test'; +import { mapRunLine } from './runner'; + +// The first tests on the OpenCode path, which had none. +// +// This covers the NDJSON → ChatEvent mapping from `opencode run --format json`, which is the piece most +// likely to break against a new release: the event shape is not a documented contract, and this project +// already runs two different opencode versions across two machines (1.17.9 here, 1.18.11 elsewhere). +// Before this, a shape change would have surfaced as a silently empty or malformed turn. +// +// The fixtures below are the shapes the live 1.17.9 binary emits. If one of these tests fails after an +// upgrade, re-read the real NDJSON from the installed binary before editing the expectation — the test +// failing is the feature. + +describe('mapRunLine — what it forwards', () => { + it('forwards a text part as final text, because run emits whole blocks not deltas', () => { + const out = mapRunLine(JSON.stringify({ type: 'text', sessionID: 'ses_1', part: { type: 'text', text: 'hello' } })); + + expect(out?.events).toEqual([{ type: 'text', text: 'hello' }]); + expect(out?.sessionId).toBe('ses_1'); + }); + + it('drops an empty text part rather than emitting a blank message', () => { + expect(mapRunLine(JSON.stringify({ type: 'text', part: { type: 'text', text: '' } }))?.events).toEqual([]); + }); + + it('splits one resolved tool part into start then result', () => { + // `run` hands over the tool already finished, unlike the SSE path which saw it transition. Both + // events are still emitted so the UI renders a call and its output rather than output alone. + const out = mapRunLine( + JSON.stringify({ + type: 'tool_use', + sessionID: 'ses_1', + part: { + type: 'tool', + tool: 'bash', + callID: 'call_1', + state: { status: 'completed', input: { cmd: 'ls' }, output: 'a\nb' }, + }, + }), + ); + + expect(out?.events).toEqual([ + { type: 'tool:start', toolCallId: 'call_1', toolName: 'bash', toolInput: { cmd: 'ls' } }, + { type: 'tool:result', toolCallId: 'call_1', output: 'a\nb', isError: false }, + ]); + }); + + it('reports a failed tool with its error as the output', () => { + const out = mapRunLine( + JSON.stringify({ + type: 'tool_use', + part: { + type: 'tool', + tool: 'bash', + callID: 'call_2', + state: { status: 'error', error: 'boom', output: 'ignored' }, + }, + }), + ); + + // The error replaces the output rather than sitting beside it: a failed call has nothing useful in + // `output`, and showing both would put a stale value under an error. + expect(out?.events[1]).toEqual({ type: 'tool:result', toolCallId: 'call_2', output: 'boom', isError: true }); + }); + + it('ignores a tool part with no callID, which cannot be correlated to a result', () => { + const out = mapRunLine(JSON.stringify({ type: 'tool_use', part: { type: 'tool', tool: 'bash' } })); + expect(out?.events).toEqual([]); + }); + + it('says nothing about step_start and other unknown types', () => { + expect(mapRunLine(JSON.stringify({ type: 'step_start', sessionID: 'ses_1' }))?.events).toEqual([]); + expect(mapRunLine(JSON.stringify({ type: 'something_new_in_1_19' }))?.events).toEqual([]); + }); + + it('returns null for a non-JSON line, because opencode interleaves plain logs with the stream', () => { + expect(mapRunLine('Shell cwd was reset to /somewhere')).toBeNull(); + expect(mapRunLine('')).toBeNull(); + }); +}); + +describe('mapRunLine — cost', () => { + it('reports per-step tokens and cost as a delta for the caller to accumulate', () => { + const out = mapRunLine( + JSON.stringify({ type: 'step_finish', part: { tokens: { input: 10, output: 4 }, cost: 0.002 } }), + ); + + expect(out?.costDelta).toEqual({ inputTokens: 10, outputTokens: 4, totalUSD: 0.002 }); + }); + + it('is zero for every line that is not a step_finish, so accumulation is unconditional', () => { + const text = mapRunLine(JSON.stringify({ type: 'text', part: { type: 'text', text: 'x' } })); + expect(text?.costDelta).toEqual({ inputTokens: 0, outputTokens: 0, totalUSD: 0 }); + }); + + it('treats missing tokens and a missing cost as zero rather than NaN', () => { + // A NaN here would propagate into the turn total and render as an empty or broken cost in the UI. + const out = mapRunLine(JSON.stringify({ type: 'step_finish', part: {} })); + expect(out?.costDelta).toEqual({ inputTokens: 0, outputTokens: 0, totalUSD: 0 }); + }); +}); + +describe('mapRunLine — session id', () => { + it('reports the session id from any line that carries one', () => { + // The caller emits it only the first time; this function has no memory, which is what makes it + // testable line by line. + expect(mapRunLine(JSON.stringify({ type: 'step_start', sessionID: 'ses_abc' }))?.sessionId).toBe('ses_abc'); + expect(mapRunLine(JSON.stringify({ type: 'text', part: { text: 'hi' } }))?.sessionId).toBeUndefined(); + }); +}); diff --git a/src/servers/sidecar/opencode/runner.ts b/src/servers/sidecar/opencode/runner.ts index efbbb474..7fefb315 100644 --- a/src/servers/sidecar/opencode/runner.ts +++ b/src/servers/sidecar/opencode/runner.ts @@ -35,7 +35,12 @@ type RunHandle = { proc: Subprocess; killedByUser: boolean }; // One turn per sessionKey; a new turn supersedes any stale process for that key. const running = new Map(); -// Shape of `opencode run --format json` events (verified live against 1.17.9). +// Shape of `opencode run --format json` events. +// +// Verified live against opencode 1.17.9 (this server) and reported working on 1.18.11 elsewhere. Nothing +// enforces either — the binary is whatever is installed on the machine, and two machines in this project +// already differ. `runner.test.ts` pins the mapping itself so a shape change fails a test rather than a +// turn; if it starts failing, re-read the NDJSON from the installed binary before editing the test. type RunPart = { type?: string; text?: string; @@ -158,60 +163,22 @@ export function runOpenCodeTurn(params: OpenCodeRunParams, config: RunnerConfig, })(); function handleLine(line: string): void { - let evt: RunEvent; - try { - evt = JSON.parse(line) as RunEvent; - } catch { - return; // non-JSON log line - } + const mapped = mapRunLine(line); + if (!mapped) return; // Report the OpenCode session id once, so the API can resume it (`--session`) next turn. - if (!reportedSession && evt.sessionID) { + if (!reportedSession && mapped.sessionId) { reportedSession = true; - emit({ type: 'opencode:session', sessionKey, sessionId: evt.sessionID }); + emit({ type: 'opencode:session', sessionKey, sessionId: mapped.sessionId }); } - switch (evt.type) { - case 'text': { - // `run` emits complete text parts (not token deltas) — forward each as final text. - const text = evt.part?.text; - if (typeof text === 'string' && text.length > 0) emitEvent({ type: 'text', text }); - return; - } - case 'tool_use': { - // In `run`, the tool part arrives already resolved (status + output). Emit start then result. - const part = evt.part; - if (!part || part.type !== 'tool' || !part.callID) return; - const st = part.state ?? {}; - emitEvent({ - type: 'tool:start', - toolCallId: part.callID, - toolName: part.tool ?? 'tool', - toolInput: (st.input as Record) ?? {}, - }); - const isError = st.status === 'error'; - emitEvent({ - type: 'tool:result', - toolCallId: part.callID, - output: String((isError ? st.error : st.output) ?? ''), - isError, - }); - return; - } - case 'step_finish': { - // Accumulate per-step tokens/cost into the turn's MessageCost. - const part = evt.part; - const t = part?.tokens; - cost = { - inputTokens: cost.inputTokens + (t?.input ?? 0), - outputTokens: cost.outputTokens + (t?.output ?? 0), - totalUSD: cost.totalUSD + (typeof part?.cost === 'number' ? part.cost : 0), - }; - return; - } - default: - return; // step_start etc. — nothing to forward - } + cost = { + inputTokens: cost.inputTokens + mapped.costDelta.inputTokens, + outputTokens: cost.outputTokens + mapped.costDelta.outputTokens, + totalUSD: cost.totalUSD + mapped.costDelta.totalUSD, + }; + + for (const event of mapped.events) emitEvent(event); } // ── Completion: process exit is the authoritative turn-done signal ── @@ -244,3 +211,81 @@ export function killOpenCodeTurn(sessionKey: string): void { } // proc.exited fires → finish({ type: 'stopped' }). } + +// ── The NDJSON → ChatEvent mapping, as a pure function ── +// +// Extracted from `handleLine` so it can be tested without spawning a binary. This is the piece most +// likely to break against a new OpenCode release — the event shape is not a stable contract and this +// project already runs two different versions across two machines — and it was the only untested part +// of the path. `runner.test.ts` pins it. +// +// Pure by construction: it takes a line and returns what should happen, holding no state. The caller +// owns the two stateful concerns, because they span lines rather than belonging to one: emitting the +// session id exactly once, and accumulating cost across steps. + +export type MappedRunLine = { + /** Present on any event that names a session; the caller emits it only the first time. */ + sessionId?: string; + /** Events to forward, in order. A tool part yields `tool:start` then `tool:result`. */ + events: ChatEvent[]; + /** Per-step tokens/cost to add to the turn total. Zeroes for every non-`step_finish` line. */ + costDelta: { inputTokens: number; outputTokens: number; totalUSD: number }; +}; + +const NO_COST = { inputTokens: 0, outputTokens: 0, totalUSD: 0 }; + +/** `null` for a line that is not JSON at all — `opencode` interleaves plain log lines with the stream. */ +export function mapRunLine(line: string): MappedRunLine | null { + let evt: RunEvent; + try { + evt = JSON.parse(line) as RunEvent; + } catch { + return null; // non-JSON log line + } + + const sessionId = evt.sessionID; + const events: ChatEvent[] = []; + + switch (evt.type) { + case 'text': { + // `run` emits complete text parts (not token deltas) — forward each as final text. + const text = evt.part?.text; + if (typeof text === 'string' && text.length > 0) events.push({ type: 'text', text }); + return { sessionId, events, costDelta: NO_COST }; + } + case 'tool_use': { + // In `run`, the tool part arrives already resolved (status + output). Emit start then result. + const part = evt.part; + if (!part || part.type !== 'tool' || !part.callID) return { sessionId, events, costDelta: NO_COST }; + const st = part.state ?? {}; + events.push({ + type: 'tool:start', + toolCallId: part.callID, + toolName: part.tool ?? 'tool', + toolInput: (st.input as Record) ?? {}, + }); + const isError = st.status === 'error'; + events.push({ + type: 'tool:result', + toolCallId: part.callID, + output: String((isError ? st.error : st.output) ?? ''), + isError, + }); + return { sessionId, events, costDelta: NO_COST }; + } + case 'step_finish': { + const t = evt.part?.tokens; + return { + sessionId, + events, + costDelta: { + inputTokens: t?.input ?? 0, + outputTokens: t?.output ?? 0, + totalUSD: typeof evt.part?.cost === 'number' ? evt.part.cost : 0, + }, + }; + } + default: + return { sessionId, events, costDelta: NO_COST }; // step_start etc. — nothing to forward + } +}