diff --git a/src/servers/api/chat/list-models.ts b/src/servers/api/chat/list-models.ts index 8d2b1a8b..1e545dcc 100644 --- a/src/servers/api/chat/list-models.ts +++ b/src/servers/api/chat/list-models.ts @@ -39,8 +39,12 @@ export function invalidateModelCache(): void { openCodeCache = null; } +type OpenCodeModel = { + capabilities?: { input?: { image?: boolean }; reasoning?: boolean }; +}; + type ProvidersResponse = { - providers?: Array<{ id?: string; models?: Record }>; + providers?: Array<{ id?: string; models?: Record }>; }; // Enumerate OpenCode models from the fixed server's GET /config/providers (reliable — no subprocess). @@ -58,19 +62,20 @@ async function listOpenCodeModels(): Promise { const models: ModelInfo[] = []; for (const provider of data.providers ?? []) { const providerId = provider.id ?? ''; - for (const modelId of Object.keys(provider.models ?? {})) { + for (const [modelId, model] of Object.entries(provider.models ?? {})) { models.push({ id: `${providerId}/${modelId}`, name: modelId, provider: providerId, contextWindow: 200000, maxTokens: 8192, - reasoning: false, - // False because nothing carries them: `handleOpenCodeChat`'s message type has no `images` - // field, so an attached image is rendered in the bubble, never sent, and silently dropped. - // The composer gates on this flag, so advertising `true` offered a capability that did not - // exist. Flip it back when images are plumbed through OpenCodeRunParams (parity doc, Phase 4). - images: false, + reasoning: model?.capabilities?.reasoning ?? false, + // Was hardcoded `false`, correctly, while nothing carried images — the composer gates on this + // flag, so advertising `true` offered a capability that did not exist. Images are now plumbed + // through `OpenCodeRunParams` to `opencode run --file`, so the honest answer is the model's + // own: OpenCode publishes it per model and we had never read it. Defaults to false, so a model + // that does not declare the capability keeps the affordance hidden rather than offering it. + images: model?.capabilities?.input?.image ?? false, }); } } diff --git a/src/servers/api/chat/websocket.ts b/src/servers/api/chat/websocket.ts index 32236343..7bac312d 100644 --- a/src/servers/api/chat/websocket.ts +++ b/src/servers/api/chat/websocket.ts @@ -396,6 +396,10 @@ async function handleOpenCodeChat( cwd?: string; cwdRoot?: string; resumeSessionId?: string; + // The whole of B4 lived in this omission. The browser sent images, the bubble rendered them, and + // they stopped at this signature — so they were never passed on and never reached the model, with + // nothing anywhere reporting a loss. + images?: PromptImage[]; }, effectivePrompt: string, ): Promise { @@ -457,6 +461,7 @@ async function handleOpenCodeChat( cwd, model, resumeSessionId: msg.resumeSessionId, + images: msg.images, onMessage, }); diff --git a/src/servers/channels/send-opencode.ts b/src/servers/channels/send-opencode.ts index 4be73c23..4ca28230 100644 --- a/src/servers/channels/send-opencode.ts +++ b/src/servers/channels/send-opencode.ts @@ -1,4 +1,4 @@ -import type { TurnMessage } from '@@/api/chat/types'; +import type { PromptImage, TurnMessage } from '@@/api/chat/types'; import { logger } from '@@/api/chat/logger'; import * as sidecar from '@@/sidecar-registry'; import { getOpenCodeSession } from '@@/api/chat/opencode/state'; @@ -20,6 +20,7 @@ type OpenCodeStreamingParams = { role?: string; resumeSessionId?: string; durable?: boolean; + images?: PromptImage[]; // Finished turn messages, already committed by the sidecar; `seq` is the cursor id to deliver them under. onMessage: (msg: TurnMessage, seq?: number) => void; }; @@ -59,6 +60,7 @@ export async function sendOpenCodeStreaming(params: OpenCodeStreamingParams): Pr model: params.model, resumeSessionId, durable: params.durable, + images: params.images, }); } catch (err) { unsub(); diff --git a/src/servers/sidecar/opencode/runner.test.ts b/src/servers/sidecar/opencode/runner.test.ts index 5932b238..6591db45 100644 --- a/src/servers/sidecar/opencode/runner.test.ts +++ b/src/servers/sidecar/opencode/runner.test.ts @@ -1,5 +1,5 @@ import { afterAll, describe, expect, it } from 'bun:test'; -import { mkdtempSync, writeFileSync, chmodSync, rmSync } from 'node:fs'; +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'; @@ -15,13 +15,13 @@ import { // // 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.18.11 on the Mac, 1.17.9 on alpha -// — measured 2026-08-10; this file previously had them the wrong way round). -// Before this, a shape change would have surfaced as a silently empty or malformed turn. +// 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 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. +// 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', () => { @@ -143,6 +143,12 @@ 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(() => { @@ -219,6 +225,54 @@ describe('runOpenCodeTurn — a second turn on a live session', () => { 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('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'; diff --git a/src/servers/sidecar/opencode/runner.ts b/src/servers/sidecar/opencode/runner.ts index 6f836bca..775f27c5 100644 --- a/src/servers/sidecar/opencode/runner.ts +++ b/src/servers/sidecar/opencode/runner.ts @@ -1,6 +1,8 @@ -import { existsSync } from 'node:fs'; +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 } from '../../api/chat/types'; +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 @@ -98,6 +100,16 @@ export function runOpenCodeTurn(params: OpenCodeRunParams, config: RunnerConfig, 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; @@ -138,6 +150,7 @@ export function runOpenCodeTurn(params: OpenCodeRunParams, config: RunnerConfig, clearTimeout(hardTimer); if (inactivityTimer) clearTimeout(inactivityTimer); if (running.get(sessionKey) === handle) running.delete(sessionKey); + cleanUpTurnImages(imagePaths); if (event) emitEvent(event); }; @@ -268,6 +281,50 @@ 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. * diff --git a/src/servers/sidecar/protocol.ts b/src/servers/sidecar/protocol.ts index dea40a5b..d6d55768 100644 --- a/src/servers/sidecar/protocol.ts +++ b/src/servers/sidecar/protocol.ts @@ -197,6 +197,14 @@ export type OpenCodeRunParams = { cwd?: string; // passed to `opencode run --dir` — hard-re-anchors tools to this directory model?: string; // `providerID/modelID` (e.g. opencode/claude-haiku-4-5); passed to --model verbatim resumeSessionId?: string; // OpenCode `ses_…` id to continue (`--session`) + /** + * Images for this turn, base64 as the browser sent them. + * + * `opencode run` takes attachments as PATHS (`-f`), not inline data, so the sidecar writes each one to + * a temp file and deletes it when the turn ends. They travel as data because that is what crosses the + * websocket from the browser, and because officer and the sidecar are not guaranteed to share a disk. + */ + images?: PromptImage[]; durable?: boolean; // commit turn output to chat_session_events (default true) — see ClaudeSpawnStreamingParams };