finish phase 1: correct the stale comments, and put the ndjson mapping under test

Path names: the serve's cwd is DATA_PATH/opencode_server, not opencode-sidecar. Two comments said
otherwise and would send the next reader to a directory that does not exist.

Version pin: the comment claimed "verified live against 1.17.9" as though that were a property of the
code. It is a property of whichever binary is installed, and this project already runs two — 1.17.9 here,
1.18.11 on the other machine. Says so now, and points at the test as the thing that actually enforces it.

Tests, the first on the OpenCode path. `runner.ts`'s NDJSON → ChatEvent mapping was described as pure and
untested; it was untested but not pure — it lived inside `handleLine` as a closure over `emit`, the
accumulated cost and a reported-session flag, so it could not be called without spawning a binary.

Extracted as `mapRunLine`, genuinely pure: line in, {sessionId, events, costDelta} out. The two concerns
that span lines stay with the caller, because they are not properties of a line — emitting the session id
exactly once, and accumulating cost across steps. Behaviour is unchanged.

11 tests over what the mapping forwards, what it drops and what it must not turn into NaN. The last one
matters: a missing `cost` on a step_finish would otherwise propagate NaN into the turn total.

Phase 1 is complete: dead code deleted (previous commit), comments corrected, tests added.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

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

src/servers/sidecar/opencode/index.ts

@@ -83,7 +83,7 @@ async function waitHealthy(baseUrl: string, timeoutMs: number): Promise<boolean>
   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

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();
+  });
+});

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<string, RunHandle>();
 
-// 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<string, unknown>) ?? {},
-        });
-        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<string, unknown>) ?? {},
+      });
+      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
+  }
+}