diff --git a/src/servers/sidecar/opencode/index.ts b/src/servers/sidecar/opencode/index.ts index b5e6c283..aac33861 100644 --- a/src/servers/sidecar/opencode/index.ts +++ b/src/servers/sidecar/opencode/index.ts @@ -7,17 +7,16 @@ import { sweepRecordedServe } from './serve-sweep'; import { connectProviderCredential } from './connect-credential'; import { createSessionLogStore } from '../claude/session-log'; import type { SidecarCommand, SidecarEvent } from '../protocol'; -import type { RunnerMessage } from './runner'; -import { runOpenCodeTurn, killOpenCodeTurn, listRunningOpenCodeTurns, stopAllOpenCodeTurns } from './runner'; +import type { RunnerMessage } from './serve-runner'; import { runOpenCodeTurnOnServe, killServeTurn, listRunningServeTurns, stopAllServeTurns } from './serve-runner'; // The OpenCode sidecar (officer-opencode). Same philosophy as officer-claude: a singleton process that // OWNS its runtime — here, an `opencode serve` — registers with the API server, and answers commands. It // listens on a random port, reported to the API on connect so it can route there. // -// The serve's working directory is DATA_PATH/opencode_server, and that is now ALL it is: turns do not go -// through the serve, they are `opencode run --dir ` subprocesses (runner.ts). The serve is used for -// session CRUD and model enumeration only. +// The serve runs EVERYTHING: turns (serve-runner.ts), session CRUD and model enumeration. It used to be +// CRUD only, with turns spawned as `opencode run --dir ` subprocesses — that path was deleted on +// 2026-08-10 once the serve had streaming, mid-turn injection and interrupt working end to end. const API_URL = process.env.API_URL ?? `ws://127.0.0.1:${process.env.PORT ?? '5000'}`; const OPENCODE_BIN = process.env.OPENCODE_BIN || join(homedir(), '.opencode', 'bin', 'opencode'); @@ -144,19 +143,8 @@ void connectProviderCredential(baseUrl); type ReplyFn = (msg: SidecarEvent) => void; -const RUNNER_CONFIG = { bin: OPENCODE_BIN, fallbackCwd: SERVE_CWD }; const SERVE_RUNNER_CONFIG = { baseUrl, fallbackCwd: SERVE_CWD }; -// Which engine runs a turn. `subprocess` (the default) spawns `opencode run`; `serve` drives the -// serve's /api/session surface, which is the only way to get streaming, steer, queue and a stop that -// leaves the session alive. -// -// A switch rather than a replacement, and defaulted to the old path on purpose: the subprocess has -// worked all day and the serve path has not been lived with yet. A bad evening should cost one restart -// with OPENCODE_TURNS unset, not a revert. -const USE_SERVE_TURNS = (process.env.OPENCODE_TURNS ?? 'subprocess').toLowerCase() === 'serve'; -console.log(`[opencode] turn engine: ${USE_SERVE_TURNS ? 'serve (/api/session)' : 'subprocess (opencode run)'}`); - function handleCommand(cmd: SidecarCommand, reply: ReplyFn) { switch (cmd.type) { case 'ping': @@ -178,24 +166,18 @@ function handleCommand(cmd: SidecarCommand, reply: ReplyFn) { connection.send(msg); }; - if (USE_SERVE_TURNS) void runOpenCodeTurnOnServe(cmd.params, SERVE_RUNNER_CONFIG, onMessage); - else runOpenCodeTurn(cmd.params, RUNNER_CONFIG, onMessage); + void runOpenCodeTurnOnServe(cmd.params, SERVE_RUNNER_CONFIG, onMessage); reply({ type: 'opencode:spawned', id: cmd.id, sessionKey }); break; } case 'opencode:list': - reply({ - type: 'opencode:sessions', - id: cmd.id, - sessions: USE_SERVE_TURNS ? listRunningServeTurns() : listRunningOpenCodeTurns(), - }); + reply({ type: 'opencode:sessions', id: cmd.id, sessions: listRunningServeTurns() }); break; case 'opencode:kill': - // On the serve this is an INTERRUPT: the turn stops and the session survives, so the conversation - // can be continued rather than only re-opened. - if (USE_SERVE_TURNS) void killServeTurn(cmd.sessionKey, SERVE_RUNNER_CONFIG); - else killOpenCodeTurn(cmd.sessionKey); + // An INTERRUPT, not a kill: the turn stops and the session survives, so the conversation can be + // continued rather than only re-opened. + void killServeTurn(cmd.sessionKey, SERVE_RUNNER_CONFIG); sessionLog.drop(cmd.sessionKey); break; default: @@ -247,7 +229,7 @@ async function shutdown(signal: string) { // over this socket. Tearing it down first would stop every turn silently — the exact outcome this is // here to prevent. const message = `The OpenCode sidecar restarted (${signal}), so this turn stopped.`; - const stopped = USE_SERVE_TURNS ? stopAllServeTurns(message) : stopAllOpenCodeTurns(message); + const stopped = stopAllServeTurns(message); if (stopped > 0) { console.log(`[opencode] ${signal} received, stopping ${stopped} in-flight turn(s)...`); await Promise.race([sessionLog.flush(), Bun.sleep(SHUTDOWN_FLUSH_MS)]); diff --git a/src/servers/sidecar/opencode/runner.test.ts b/src/servers/sidecar/opencode/runner.test.ts deleted file mode 100644 index b5b438ce..00000000 --- a/src/servers/sidecar/opencode/runner.test.ts +++ /dev/null @@ -1,311 +0,0 @@ -import { afterAll, describe, expect, it } from 'bun:test'; -import { chmodSync, existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; -import { tmpdir } from 'node:os'; -import { join } from 'node:path'; -import type { RunnerMessage } from './runner'; -import { - killOpenCodeTurn, - listRunningOpenCodeTurns, - mapRunLine, - runOpenCodeTurn, - stopAllOpenCodeTurns, -} 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 -// could easily run different versions on its two machines — both are on 1.18.16 as of 2026-08-10, but -// only because they were upgraded together that day; before it they were 1.18.11 and 1.17.9, and this -// file recorded which was which backwards. Without these tests a shape change surfaces as a silently -// empty or malformed turn. -// -// The fixtures below were captured from a live binary. If one 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(); - }); -}); - -// ── The turn lifecycle, which `mapRunLine`'s extraction deliberately left with the caller ── -// -// Emitting the session id once and accumulating cost were kept out of the pure mapper because they span -// lines. That was right, and it left them as the only untested logic on this path — which is exactly -// where the supersede defect lived: a replaced turn's `proc.exited` fired late and ran `finish()` against -// the turn that had replaced it. -// -// No real `opencode` needed. `RunnerConfig.bin` is the only injection point, so a shell script that -// outlives the test stands in for a turn that is still generating. - -const stubDir = mkdtempSync(join(tmpdir(), 'oc-runner-test-')); - -/** Stands in for a turn that is still generating. */ -const STUB_BIN = join(stubDir, 'fake-opencode'); -writeFileSync(STUB_BIN, '#!/bin/sh\nsleep 30\n'); -chmodSync(STUB_BIN, 0o755); - -/** Stands in for a turn that fails on its own. A script, not `/bin/false` — that is `/usr/bin/false` on macOS. */ -const FAILING_BIN = join(stubDir, 'failing-opencode'); -writeFileSync(FAILING_BIN, '#!/bin/sh\necho "boom" >&2\nexit 1\n'); -chmodSync(FAILING_BIN, 0o755); - -/** Records the argv it was invoked with, so the command line itself can be asserted. */ -const ARGS_FILE = join(stubDir, 'args.txt'); -const RECORDING_BIN = join(stubDir, 'recording-opencode'); -writeFileSync(RECORDING_BIN, `#!/bin/sh\nprintf '%s\\n' "$@" > ${ARGS_FILE}\nexit 0\n`); -chmodSync(RECORDING_BIN, 0o755); - -const CONFIG = { bin: STUB_BIN, fallbackCwd: stubDir }; - -afterAll(() => { - rmSync(stubDir, { recursive: true, force: true }); -}); - -/** - * Wait for something to BECOME true, rather than sleeping a guessed interval and hoping. - * - * These tests spawn real processes, so every "has it happened yet" is at the mercy of machine load — - * and a fixed `sleep(750)` duly failed once on a box that was busy running opencode probes. Polling - * makes a slow machine slow instead of red. Absence assertions still need a fixed wait, since there is - * no event to wait for; those are marked where they appear. - */ -async function waitFor(what: () => boolean, timeoutMs = 8000): Promise { - const deadline = Date.now() + timeoutMs; - while (Date.now() < deadline) { - if (what()) return; - await Bun.sleep(25); - } -} - -describe('runOpenCodeTurn — a second turn on a live session', () => { - it('lets the replacement keep the session: no error, still listed, still killable', async () => { - const sessionKey = 'sess-supersede'; - const messages: RunnerMessage[] = []; - const emit = (msg: RunnerMessage) => messages.push(msg); - - runOpenCodeTurn({ sessionKey, prompt: 'first', cwd: stubDir }, CONFIG, emit); - runOpenCodeTurn({ sessionKey, prompt: 'second', cwd: stubDir }, CONFIG, emit); - - // Wait for the superseded child to actually die — its exit handler is what used to reach across. - // Absence assertion, so there is no event to wait for: give it a generous fixed window instead, and - // wait on something observable (the kill landing) rather than purely on the clock. - await waitFor(() => listRunningOpenCodeTurns().length === 1); - await Bun.sleep(1500); - - // 1. Nothing is emitted for a turn the system replaced on purpose. This one mattered most: the emit - // is committed to chat_session_events by the sidecar, so a false "OpenCode exited" became history. - expect(messages.filter((m) => m.type === 'opencode:event' && m.event.type === 'error')).toEqual([]); - - // 2. The replacement is still enumerable — the Live panel's whole purpose. - expect(listRunningOpenCodeTurns()).toContainEqual({ sessionKey }); - - // 3. And still reachable by the stop button, rather than orphaned with no handle. - killOpenCodeTurn(sessionKey); - await waitFor(() => messages.some((m) => m.type === 'opencode:event' && m.event.type === 'stopped')); - expect(listRunningOpenCodeTurns()).not.toContainEqual({ sessionKey }); - expect(messages.some((m) => m.type === 'opencode:event' && m.event.type === 'stopped')).toBe(true); - }); - - it('stops every in-flight turn on shutdown, and says why in the transcript', async () => { - // `opencode run` is spawned, not supervised, so a sidecar restart used to leave these alive: - // reparented, still spending tokens and still writing files, with nothing reading their output. - const messages: RunnerMessage[] = []; - const emit = (m: RunnerMessage) => messages.push(m); - - runOpenCodeTurn({ sessionKey: 'sess-x', prompt: 'a', cwd: stubDir }, CONFIG, emit); - runOpenCodeTurn({ sessionKey: 'sess-y', prompt: 'b', cwd: stubDir }, CONFIG, emit); - expect(listRunningOpenCodeTurns()).toHaveLength(2); - - const stopped = stopAllOpenCodeTurns('sidecar restarted'); - - // Synchronous on purpose: the caller is about to call process.exit, so nothing that waits for - // `proc.exited` would ever run. A turn killed that way just trails off mid-tool-call. - expect(stopped).toBe(2); - expect(listRunningOpenCodeTurns()).toEqual([]); - const errors = messages.filter((m) => m.type === 'opencode:event' && m.event.type === 'error'); - expect(errors).toHaveLength(2); - - // And the late exits must not add a second, worse ending on top of the one just written. - await Bun.sleep(500); - expect(messages.filter((m) => m.type === 'opencode:event' && m.event.type === 'error')).toHaveLength(2); - expect(messages.some((m) => m.type === 'opencode:event' && m.event.type === 'stopped')).toBe(false); - }); - - it('puts `--` between the attachments and the prompt, or the prompt is eaten as a filename', async () => { - // `--file` is an ARRAY option in opencode's parser, so it keeps consuming positionals. Without the - // separator the turn dies with `File not found: ` — confirmed against the - // real binary before this was written. Nothing else in the arg list can catch that, so it is pinned - // here rather than left to the next person to rediscover. - const messages: RunnerMessage[] = []; - runOpenCodeTurn( - { - sessionKey: 'sess-images', - prompt: 'describe this', - cwd: stubDir, - images: [{ mediaType: 'image/png', data: Buffer.from('not-really-a-png').toString('base64') }], - }, - { ...CONFIG, bin: RECORDING_BIN }, - (m) => messages.push(m), - ); - - await waitFor(() => existsSync(ARGS_FILE)); - const argv = readFileSync(ARGS_FILE, 'utf8').trim().split('\n'); - - const fileFlag = argv.indexOf('--file'); - const separator = argv.indexOf('--'); - expect(fileFlag).toBeGreaterThan(-1); - expect(separator).toBeGreaterThan(fileFlag); - expect(argv.at(-1)).toBe('describe this'); - // The attachment is a path on disk, not inline data — that is the whole reason for the temp file. - expect(argv[fileFlag + 1]).toMatch(/officer-oc-.*\.png$/); - }); - - it('leaves no `--` and no temp files behind when the turn carries no images', async () => { - const messages: RunnerMessage[] = []; - rmSync(ARGS_FILE, { force: true }); - runOpenCodeTurn( - { sessionKey: 'sess-noimg', prompt: 'plain', cwd: stubDir }, - { ...CONFIG, bin: RECORDING_BIN }, - (m) => messages.push(m), - ); - - await waitFor(() => existsSync(ARGS_FILE)); - const argv = readFileSync(ARGS_FILE, 'utf8').trim().split('\n'); - - // A bare `--` would be harmless here, but its absence is what proves the separator is tied to the - // attachments rather than added unconditionally. - expect(argv).not.toContain('--'); - expect(argv).not.toContain('--file'); - expect(argv.at(-1)).toBe('plain'); - }); - - it('reports a missing binary instead of throwing out of the handler', async () => { - // `Bun.spawn` throws on ENOENT rather than returning a failed process, and that throw used to escape - // `runOpenCodeTurn` before any event was emitted — so the browser kept a spinner nothing could end. - // A wrong OPENCODE_BIN is the ordinary way to get here. - const messages: RunnerMessage[] = []; - expect(() => - runOpenCodeTurn( - { sessionKey: 'sess-nobin', prompt: 'hi', cwd: stubDir }, - { ...CONFIG, bin: join(stubDir, 'nope') }, - (m) => messages.push(m), - ), - ).not.toThrow(); - - const errors = messages.filter((m) => m.type === 'opencode:event' && m.event.type === 'error'); - expect(errors).toHaveLength(1); - // The path is in the message: this is nearly always a misconfiguration, and naming the binary it - // tried is the difference between a fix and a debugging session. - expect(JSON.stringify(errors[0])).toContain('nope'); - // And it must not leave a phantom entry behind for the Live panel or the stop button. - expect(listRunningOpenCodeTurns()).not.toContainEqual({ sessionKey: 'sess-nobin' }); - }); - - it('still reports a turn that dies on its own, rather than swallowing every exit', async () => { - // The guard must not overreach: an ordinary failure is still an error the user needs to see. - const sessionKey = 'sess-solo'; - const messages: RunnerMessage[] = []; - - runOpenCodeTurn({ sessionKey, prompt: 'only', cwd: stubDir }, { ...CONFIG, bin: FAILING_BIN }, (m) => - messages.push(m), - ); - await waitFor(() => messages.some((m) => m.type === 'opencode:event' && m.event.type === 'error')); - - expect(messages.some((m) => m.type === 'opencode:event' && m.event.type === 'error')).toBe(true); - expect(listRunningOpenCodeTurns()).not.toContainEqual({ sessionKey }); - }); -}); diff --git a/src/servers/sidecar/opencode/runner.ts b/src/servers/sidecar/opencode/runner.ts deleted file mode 100644 index 9ee005f0..00000000 --- a/src/servers/sidecar/opencode/runner.ts +++ /dev/null @@ -1,463 +0,0 @@ -import { existsSync, unlinkSync, writeFileSync } from 'node:fs'; -import { tmpdir } from 'node:os'; -import { join } from 'node:path'; -import type { Subprocess } from 'bun'; -import type { ChatEvent, MessageCost, PromptImage } from '../../api/chat/types'; -import type { OpenCodeRunParams } from '../protocol'; - -// Drives one chat turn by spawning `opencode run … --format json` and mapping its newline-delimited -// JSON events to the shared ChatEvent contract. This is the reliable path: `--dir ` hard-anchors -// every tool to the chat's working directory, and `run` (unlike serve + POST /message) reports tool -// completion faithfully and exits when the turn is done — no wedged "running" tools. `--dangerously- -// skip-permissions` auto-approves so tools never block on an approval we can't answer over this channel. -// -// A watchdog guards against a genuinely hung child (an interactive prompt, a `sleep`, a network stall): -// an inactivity timer (reset on every stdout chunk) and an absolute per-turn ceiling both kill the -// process and emit a clean `error`, so the UI never sits at "Working…" forever. - -const INACTIVITY_MS = 120_000; // no stdout for this long → assume wedged, kill -const HARD_CAP_MS = 10 * 60_000; // absolute per-turn ceiling - -export type RunnerConfig = { - bin: string; // absolute path to the opencode binary - fallbackCwd: string; // used when params.cwd is missing/nonexistent -}; - -// What a turn reports to the sidecar it runs in. `opencode:event` is deliberately not a wire event any -// more: the sidecar translates each one into a TurnMessage and commits it before officer sees anything, -// so the durable record does not depend on officer being up (see index.ts). -export type RunnerMessage = - | { type: 'opencode:event'; sessionKey: string; event: ChatEvent } - | { type: 'opencode:session'; sessionKey: string; sessionId: string }; - -type Emit = (msg: RunnerMessage) => void; - -type RunHandle = { - proc: Subprocess; - killedByUser: boolean; - /** - * Set when a newer turn has taken this sessionKey over. - * - * A killed process dies asynchronously, so a replaced turn's `proc.exited` fires LONG after its - * replacement is already running and registered under the same key. Without this flag that late - * handler ran the full completion path against the wrong turn: it emitted `OpenCode exited with code - * 143` — which the sidecar commits to `chat_session_events`, so a false failure became permanent - * history — and then deleted its replacement from `running`, which blinded the Live panel, made the - * stop button a no-op, and orphaned a process nothing could reach. - */ - superseded: boolean; - /** - * End this turn from outside the closure that owns it, with a reason. - * - * `killOpenCodeTurn` can kill a process and let `proc.exited` do the rest, because it has time. - * Shutdown does not: the sidecar is about to call `process.exit`, so nothing asynchronous will ever - * run again and a turn killed that way would simply stop mid-sentence, leaving a transcript that - * trails off. Settling synchronously is what puts the explanation in the log before we go. - */ - finish: (event: ChatEvent) => void; -}; - -// 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 opencode 1.18.11 (this Mac) and 1.17.9 (alpha) — MEASURED on 2026-08-10, having -// previously been recorded the other way round here: the "this server" in the original note meant alpha, -// and the comment was copied to a machine where it was false. Nothing enforces a version anyway; the -// binary is whatever is installed, and the two machines in this project already differ. -// -// `runner.test.ts` pins the mapping 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; - tool?: string; - callID?: string; - state?: { status?: string; input?: unknown; output?: unknown; error?: unknown }; - tokens?: { input?: number; output?: number }; - cost?: number; -}; -type RunEvent = { type?: string; sessionID?: string; part?: RunPart }; - -export function runOpenCodeTurn(params: OpenCodeRunParams, config: RunnerConfig, emit: Emit): void { - const { sessionKey } = params; - - // Supersede any lingering turn for this session. Mark it BEFORE killing: the flag is what tells its - // own exit handler that this death was intentional and belongs to nobody. - const stale = running.get(sessionKey); - if (stale) { - stale.superseded = true; - try { - stale.proc.kill(); - } catch { - /* already gone */ - } - running.delete(sessionKey); - } - - const args = ['run', '--format', 'json', '--dangerously-skip-permissions']; - if (params.cwd) args.push('--dir', params.cwd); - if (params.model) args.push('--model', params.model); - if (params.resumeSessionId) args.push('--session', params.resumeSessionId); - - // Attachments are paths, not inline data, so the images are spilled to temp files for the length of - // the turn and removed in `settle`. - const imagePaths = writeTurnImages(params.images); - for (const path of imagePaths) args.push('--file', path); - - // `--` BEFORE the prompt, and it is load-bearing: `--file` is an ARRAY option, so without the - // separator the prompt is swallowed as another filename and the turn dies with - // `File not found: `. Verified against the binary. - if (imagePaths.length > 0) args.push('--'); - args.push(params.prompt); - - const cwd = params.cwd && existsSync(params.cwd) ? params.cwd : config.fallbackCwd; - - // `Bun.spawn` THROWS on a missing or non-executable binary rather than resolving to a failed process, - // and that throw used to escape `runOpenCodeTurn` entirely: past the session bookkeeping below, out of - // the sidecar's command handler, with no `opencode:event` ever emitted. The browser sat on a spinner - // that nothing would ever end, because the code that ends turns had not been reached yet. - // - // A wrong `OPENCODE_BIN` is the ordinary cause, and it deserves to say so on screen instead of hanging. - let proc: Subprocess; - try { - proc = Bun.spawn([config.bin, ...args], { - cwd, - stdin: 'ignore', // == /dev/null: `run` hangs waiting on stdin otherwise - stdout: 'pipe', - stderr: 'pipe', - }); - } catch (err) { - cleanUpTurnImages(imagePaths); - const reason = err instanceof Error ? err.message : String(err); - emit({ - type: 'opencode:event', - sessionKey, - event: { type: 'error', message: `Could not start OpenCode (${config.bin}): ${reason}` }, - }); - return; - } - - // `finish` is a placeholder for the few synchronous lines until the real one below exists — it closes - // over `handle`, so the two cannot both be defined first. Nothing can call it in between. - const handle: RunHandle = { proc, killedByUser: false, superseded: false, finish: () => {} }; - running.set(sessionKey, handle); - - let done = false; - let reportedSession = false; - let cost: MessageCost = { inputTokens: 0, outputTokens: 0, totalUSD: 0 }; - let stderrTail = ''; - - const emitEvent = (event: ChatEvent) => emit({ type: 'opencode:event', sessionKey, event }); - - let inactivityTimer: ReturnType | undefined; - - /** - * Retire this turn: stop its watchdogs, release its slot, and optionally say why it ended. - * - * The delete is identity-checked because `sessionKey` is not this turn's to own once it has been - * superseded — the map may already hold a live replacement under that key, and deleting by name alone - * removed it. `null` retires silently, which is what a superseded turn needs: it must still clear its - * timers (an armed 10-minute `hardTimer` would otherwise fire an error at whichever turn holds the key - * by then, reproducing the same cross-talk on a delay) while emitting nothing at all. - */ - const settle = (event: ChatEvent | null) => { - if (done) return; - done = true; - clearTimeout(hardTimer); - if (inactivityTimer) clearTimeout(inactivityTimer); - if (running.get(sessionKey) === handle) running.delete(sessionKey); - cleanUpTurnImages(imagePaths); - if (event) emitEvent(event); - }; - - const finish = (event: ChatEvent) => settle(event); - handle.finish = finish; - - // ── Watchdogs ── - const hardTimer = setTimeout(() => { - try { - proc.kill(); - } catch { - /* already gone */ - } - finish({ type: 'error', message: `OpenCode turn exceeded ${HARD_CAP_MS / 1000}s and was stopped` }); - }, HARD_CAP_MS); - - const bumpInactivity = () => { - if (done) return; - if (inactivityTimer) clearTimeout(inactivityTimer); - inactivityTimer = setTimeout(() => { - try { - proc.kill(); - } catch { - /* already gone */ - } - finish({ - type: 'error', - message: `OpenCode turn stalled (no output for ${INACTIVITY_MS / 1000}s) and was stopped`, - }); - }, INACTIVITY_MS); - }; - bumpInactivity(); - - // ── Capture a tail of stderr for error reporting ── - void (async () => { - const dec = new TextDecoder(); - try { - for await (const chunk of proc.stderr as unknown as AsyncIterable) { - stderrTail = (stderrTail + dec.decode(chunk, { stream: true })).slice(-2000); - } - } catch { - /* stream closed */ - } - })(); - - // ── Parse stdout: newline-delimited JSON events ── - void (async () => { - const dec = new TextDecoder(); - let buf = ''; - try { - for await (const chunk of proc.stdout as unknown as AsyncIterable) { - bumpInactivity(); - buf += dec.decode(chunk, { stream: true }); - let nl: number; - while ((nl = buf.indexOf('\n')) >= 0) { - const line = buf.slice(0, nl).trim(); - buf = buf.slice(nl + 1); - if (line) handleLine(line); - } - } - const last = buf.trim(); - if (last) handleLine(last); - } catch { - /* stream closed / process killed */ - } - })(); - - function handleLine(line: string): void { - // A retired turn says nothing more. Stdout is drained asynchronously, so a killed process can still - // have buffered lines in flight — and for a superseded turn those would be emitted under a - // sessionKey that now belongs to its replacement, interleaving one turn's output into another's. - if (done) return; - - const mapped = mapRunLine(line); - if (!mapped) return; - - // Report the OpenCode session id once, so the API can resume it (`--session`) next turn. - if (!reportedSession && mapped.sessionId) { - reportedSession = true; - emit({ type: 'opencode:session', sessionKey, sessionId: mapped.sessionId }); - } - - 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 ── - void proc.exited.then((code) => { - if (done) return; - // Replaced on purpose: not a result, not an error, and not this turn's session any more. - if (handle.superseded) { - settle(null); - return; - } - if (handle.killedByUser) { - finish({ type: 'stopped' }); - return; - } - if (code === 0) { - finish({ type: 'result', cost }); - return; - } - const tail = stderrTail.trim(); - finish({ - type: 'error', - message: tail ? `OpenCode exited (${code}): ${tail.slice(-500)}` : `OpenCode exited with code ${code}`, - }); - }); -} - -/** - * The turns this process is running right now. - * - * The OpenCode analog of `claude-manager.listSessions`, and deliberately thinner. Claude holds a warm - * session that outlives a turn, so it can report one that is merely open; OpenCode spawns a subprocess - * per turn and has nothing between them. So a session appears here only while it is generating — which - * is exactly the state the Live panel exists to show, and the state that was invisible for OpenCode. - * - * No `pendingTasks`: `opencode run` has no background-task concept, so reporting 0 would suggest a - * capability that does not exist rather than an empty one. - */ -export function listRunningOpenCodeTurns(): { sessionKey: string }[] { - return Array.from(running.keys()).map((sessionKey) => ({ sessionKey })); -} - -// ── Image attachments ── -// -// `opencode run` takes files by PATH (`--file`), while the browser sends base64 over the socket, so the -// two are bridged by a temp file per image that lives exactly as long as the turn. -// -// Failing to write one is deliberately not fatal: an image that cannot be spilled costs the model that -// image, and sending the text anyway is better than failing a turn the user has already waited for. -// This is the same call the composer's own gate makes — degrade, do not remove the feature. - -/** Extension by media type, so the model sees a file it can identify. Unknown types keep `.bin`. */ -const IMAGE_EXTENSIONS: Record = { - 'image/png': 'png', - 'image/jpeg': 'jpg', - 'image/jpg': 'jpg', - 'image/gif': 'gif', - 'image/webp': 'webp', -}; - -function writeTurnImages(images: PromptImage[] | undefined): string[] { - if (!images?.length) return []; - const written: string[] = []; - for (const [index, image] of images.entries()) { - try { - const ext = IMAGE_EXTENSIONS[image.mediaType?.toLowerCase() ?? ''] ?? 'bin'; - const path = join(tmpdir(), `officer-oc-${process.pid}-${Date.now()}-${index}.${ext}`); - writeFileSync(path, Buffer.from(image.data, 'base64')); - written.push(path); - } catch (err) { - console.error('[opencode] could not write an attachment; continuing without it:', err); - } - } - return written; -} - -function cleanUpTurnImages(paths: string[]): void { - for (const path of paths) { - try { - unlinkSync(path); - } catch { - /* already gone */ - } - } -} - -/** - * Kill every turn this process is running, because the process itself is going away. - * - * A turn is a child of this sidecar only in the bookkeeping sense: `opencode run` is spawned, not - * supervised, so `pm2 restart officer-opencode` used to leave every in-flight turn ALIVE — reparented, - * still spending tokens, and still writing files as the agent, while the only reader of its stdout had - * exited. The turn's output went nowhere and the transcript simply stopped mid-tool-call, which is - * indistinguishable from the agent hanging. - * - * Both halves matter. Killing the children stops the invisible work; settling them synchronously writes - * a reason into the transcript, so a reload after a restart explains itself instead of trailing off. - * Returns how many were stopped, so the caller can skip the flush wait when there were none. - */ -export function stopAllOpenCodeTurns(message: string): number { - const handles = [...running.values()]; - for (const handle of handles) { - // Suppress the exit handler's own error: this death is accounted for, and `finish` below is the - // account. Without it a late `proc.exited` would be a second, less accurate ending. - handle.killedByUser = true; - try { - handle.proc.kill(); - } catch { - /* already gone */ - } - handle.finish({ type: 'error', message }); - } - return handles.length; -} - -export function killOpenCodeTurn(sessionKey: string): void { - const handle = running.get(sessionKey); - if (!handle) return; - handle.killedByUser = true; - try { - handle.proc.kill(); - } catch { - /* already gone */ - } - // 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 - } -} diff --git a/src/servers/sidecar/opencode/serve-runner.ts b/src/servers/sidecar/opencode/serve-runner.ts index a077b372..3812ff79 100644 --- a/src/servers/sidecar/opencode/serve-runner.ts +++ b/src/servers/sidecar/opencode/serve-runner.ts @@ -1,19 +1,17 @@ import type { ChatEvent, MessageCost } from '../../api/chat/types'; import type { OpenCodeRunParams } from '../protocol'; -import type { RunnerMessage } from './runner'; import { mapServeEvent } from './serve-events'; -// Phase B: drive a turn through the serve instead of spawning `opencode run`. +// How an OpenCode turn runs. The only way, since 2026-08-10. // -// OFF BY DEFAULT. `index.ts` picks between this and `runner.ts` on `OPENCODE_TURNS`, and the subprocess -// stays the default until this has run for a while — a bad day should be one restart from the path that -// has worked all along, not a rollback. +// It used to be `opencode run --format json`, a subprocess per turn with `stdin: 'ignore'`. Everything +// that path could not do followed from that one closed pipe: no token streaming, no mid-turn injection, +// no queue, and a stop that could only kill the session rather than interrupt it. The serve offers all +// four as primitives, each verified end to end before the subprocess was deleted +// (docs/opencode-fork-decision.md, docs/opencode-serve-migration-plan.md). // -// ── Why bother, given the subprocess works ── -// -// Everything the subprocess cannot do is a consequence of `stdin: 'ignore'`: no token streaming, no -// mid-turn injection, no queue, no interrupt that leaves the session alive. The serve offers all four as -// primitives, verified against 1.18.16 (docs/opencode-fork-decision.md). +// There is no fallback engine any more. If this path breaks, the recovery is git, not a config flag — +// a deliberate choice made while nothing depended on OpenCode. // // ── One global stream, demultiplexed ── // @@ -39,6 +37,18 @@ type ServeConfig = { fallbackCwd: string; }; +/** + * What a turn reports to the sidecar it runs in. + * + * Lived in `runner.ts` until the subprocess path was deleted (Phase D). `opencode:event` is not a wire + * event: the sidecar translates each one into a TurnMessage and commits it to `chat_session_events` + * before officer sees anything, so the durable record does not depend on officer being up. + * `opencode:session` is the routing fact — which `ses_…` to resume — and goes over the wire live. + */ +export type RunnerMessage = + | { type: 'opencode:event'; sessionKey: string; event: ChatEvent } + | { type: 'opencode:session'; sessionKey: string; sessionId: string }; + type Emit = (msg: RunnerMessage) => void; type ServeTurn = { @@ -301,7 +311,7 @@ function promptFiles(images: OpenCodeRunParams['images']): { uri: string; name: })); } -/** The serve analog of `listRunningOpenCodeTurns`. Same shape, so the Live panel needs no changes. */ +/** The turns running right now, for `opencode:list` and the Live panel. */ export function listRunningServeTurns(): { sessionKey: string }[] { return [...bySessionKey.keys()].map((sessionKey) => ({ sessionKey })); } @@ -326,7 +336,14 @@ export async function killServeTurn(sessionKey: string, config: ServeConfig): Pr retire(turn, { type: 'stopped' }); } -/** Retire every live turn, for shutdown. Mirrors `stopAllOpenCodeTurns` on the subprocess path. */ +/** + * Retire every live turn, for shutdown. + * + * Note what this does NOT do, and why it is right: the turns keep running inside the serve, which is a + * separate process and survives us. Officer stops routing them and says so in the transcript. When the + * subprocess ran turns, shutdown had to kill children or they were orphaned; here the work is somebody + * else's and killing it would be the wrong call. + */ export function stopAllServeTurns(message: string): number { const turns = [...bySessionKey.values()]; for (const turn of turns) retire(turn, { type: 'error', message }); diff --git a/src/servers/sidecar/protocol.ts b/src/servers/sidecar/protocol.ts index d6d55768..95bd97c7 100644 --- a/src/servers/sidecar/protocol.ts +++ b/src/servers/sidecar/protocol.ts @@ -26,8 +26,8 @@ export type LiveClaudeSession = { }; /** - * An OpenCode turn in flight. Only ever the generating ones — see `listRunningOpenCodeTurns` for why - * this carries neither `isGenerating` (it is always true) nor `pendingTasks` (no such concept). + * An OpenCode turn in flight. Only ever the generating ones, which is why it carries neither + * `isGenerating` (always true) nor `pendingTasks` (no such concept on this harness). * * It deliberately carries no id for OpenCode's own `ses_…` session, and does not need to: the sidecar * only ever knows its own `sessionKey`, while the `ses_…` is reported separately over `opencode:session` @@ -63,12 +63,13 @@ export type SidecarCommand = // records are in memory and die with `pm2 restart officer`, while the agent keeps running. Without it // a live session is invisible until a browser happens to reconnect to it by id. | { type: 'claude:list'; id: string } - // OpenCode — drive a turn via `opencode run … --format json` (tools re-anchored to cwd via --dir) + // OpenCode — drive a turn through the serve (POST /api/session/{id}/prompt), anchored to the chat cwd + // by a per-request location header. Was an `opencode run` subprocess until 2026-08-10. | { type: 'opencode:run-streaming'; id: string; params: OpenCodeRunParams } | { type: 'opencode:kill'; id: string; sessionKey: string } // Which OpenCode turns are running right now. The counterpart of `claude:list`, and thinner for a // reason: OpenCode has no warm session between turns, so there is nothing to report but the running - // ones. See `listRunningOpenCodeTurns`. + // ones — see `listRunningServeTurns`. | { type: 'opencode:list'; id: string } // VNC | { type: 'vnc:start'; id: string; params: VncStartParams }