send images to opencode, which never needed the fork
B4 properly. The composer gate was the honest stopgap; this is the fix. opencode run takes attachments with --file, so images work on the subprocess path we already use — the parity doc had them down as phase 4, behind the serve migration, and they were not. The bug was one omission: handleOpenCodeChat`s msg type had no images field, so the browser sent them, the bubble rendered them, and they stopped at that signature. Nothing reported a loss anywhere. Attachments are paths, not inline data, so the sidecar spills each image to a temp file for the length of the turn and removes it in settle — the same place every other per-turn resource is released, so a killed or superseded turn cleans up too. The load-bearing detail is `--` before the prompt: --file is an array option, so without the separator the prompt is eaten as another filename and the turn dies with "File not found:" followed by the entire message. Confirmed against the binary, and pinned by a test that records argv from a stub. list-models now reports each model own capability instead of a hardcoded false — opencode publishes capabilities.input.image per model and nothing had ever read it. Defaults to false, so a model that does not declare it keeps the affordance hidden. Verified end to end: a red png sent over the chat socket to opencode/claude-sonnet-4-6 came back "Red". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -39,8 +39,12 @@ export function invalidateModelCache(): void {
|
|||||||
openCodeCache = null;
|
openCodeCache = null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
type OpenCodeModel = {
|
||||||
|
capabilities?: { input?: { image?: boolean }; reasoning?: boolean };
|
||||||
|
};
|
||||||
|
|
||||||
type ProvidersResponse = {
|
type ProvidersResponse = {
|
||||||
providers?: Array<{ id?: string; models?: Record<string, unknown> }>;
|
providers?: Array<{ id?: string; models?: Record<string, OpenCodeModel> }>;
|
||||||
};
|
};
|
||||||
|
|
||||||
// Enumerate OpenCode models from the fixed server's GET /config/providers (reliable — no subprocess).
|
// Enumerate OpenCode models from the fixed server's GET /config/providers (reliable — no subprocess).
|
||||||
@@ -58,19 +62,20 @@ async function listOpenCodeModels(): Promise<ModelInfo[]> {
|
|||||||
const models: ModelInfo[] = [];
|
const models: ModelInfo[] = [];
|
||||||
for (const provider of data.providers ?? []) {
|
for (const provider of data.providers ?? []) {
|
||||||
const providerId = provider.id ?? '';
|
const providerId = provider.id ?? '';
|
||||||
for (const modelId of Object.keys(provider.models ?? {})) {
|
for (const [modelId, model] of Object.entries(provider.models ?? {})) {
|
||||||
models.push({
|
models.push({
|
||||||
id: `${providerId}/${modelId}`,
|
id: `${providerId}/${modelId}`,
|
||||||
name: modelId,
|
name: modelId,
|
||||||
provider: providerId,
|
provider: providerId,
|
||||||
contextWindow: 200000,
|
contextWindow: 200000,
|
||||||
maxTokens: 8192,
|
maxTokens: 8192,
|
||||||
reasoning: false,
|
reasoning: model?.capabilities?.reasoning ?? false,
|
||||||
// False because nothing carries them: `handleOpenCodeChat`'s message type has no `images`
|
// Was hardcoded `false`, correctly, while nothing carried images — the composer gates on this
|
||||||
// field, so an attached image is rendered in the bubble, never sent, and silently dropped.
|
// flag, so advertising `true` offered a capability that did not exist. Images are now plumbed
|
||||||
// The composer gates on this flag, so advertising `true` offered a capability that did not
|
// through `OpenCodeRunParams` to `opencode run --file`, so the honest answer is the model's
|
||||||
// exist. Flip it back when images are plumbed through OpenCodeRunParams (parity doc, Phase 4).
|
// own: OpenCode publishes it per model and we had never read it. Defaults to false, so a model
|
||||||
images: false,
|
// that does not declare the capability keeps the affordance hidden rather than offering it.
|
||||||
|
images: model?.capabilities?.input?.image ?? false,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -396,6 +396,10 @@ async function handleOpenCodeChat(
|
|||||||
cwd?: string;
|
cwd?: string;
|
||||||
cwdRoot?: string;
|
cwdRoot?: string;
|
||||||
resumeSessionId?: 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,
|
effectivePrompt: string,
|
||||||
): Promise<void> {
|
): Promise<void> {
|
||||||
@@ -457,6 +461,7 @@ async function handleOpenCodeChat(
|
|||||||
cwd,
|
cwd,
|
||||||
model,
|
model,
|
||||||
resumeSessionId: msg.resumeSessionId,
|
resumeSessionId: msg.resumeSessionId,
|
||||||
|
images: msg.images,
|
||||||
onMessage,
|
onMessage,
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -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 { logger } from '@@/api/chat/logger';
|
||||||
import * as sidecar from '@@/sidecar-registry';
|
import * as sidecar from '@@/sidecar-registry';
|
||||||
import { getOpenCodeSession } from '@@/api/chat/opencode/state';
|
import { getOpenCodeSession } from '@@/api/chat/opencode/state';
|
||||||
@@ -20,6 +20,7 @@ type OpenCodeStreamingParams = {
|
|||||||
role?: string;
|
role?: string;
|
||||||
resumeSessionId?: string;
|
resumeSessionId?: string;
|
||||||
durable?: boolean;
|
durable?: boolean;
|
||||||
|
images?: PromptImage[];
|
||||||
// Finished turn messages, already committed by the sidecar; `seq` is the cursor id to deliver them under.
|
// Finished turn messages, already committed by the sidecar; `seq` is the cursor id to deliver them under.
|
||||||
onMessage: (msg: TurnMessage, seq?: number) => void;
|
onMessage: (msg: TurnMessage, seq?: number) => void;
|
||||||
};
|
};
|
||||||
@@ -59,6 +60,7 @@ export async function sendOpenCodeStreaming(params: OpenCodeStreamingParams): Pr
|
|||||||
model: params.model,
|
model: params.model,
|
||||||
resumeSessionId,
|
resumeSessionId,
|
||||||
durable: params.durable,
|
durable: params.durable,
|
||||||
|
images: params.images,
|
||||||
});
|
});
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
unsub();
|
unsub();
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
import { afterAll, describe, expect, it } from 'bun:test';
|
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 { tmpdir } from 'node:os';
|
||||||
import { join } from 'node:path';
|
import { join } from 'node:path';
|
||||||
import type { RunnerMessage } from './runner';
|
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
|
// 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
|
// 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
|
// could easily run different versions on its two machines — both are on 1.18.16 as of 2026-08-10, but
|
||||||
// — measured 2026-08-10; this file previously had them the wrong way round).
|
// only because they were upgraded together that day; before it they were 1.18.11 and 1.17.9, and this
|
||||||
// Before this, a shape change would have surfaced as a silently empty or malformed turn.
|
// 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
|
// The fixtures below were captured from a live binary. If one fails after an upgrade, re-read the real
|
||||||
// upgrade, re-read the real NDJSON from the installed binary before editing the expectation — the test
|
// NDJSON from the installed binary before editing the expectation — the test failing is the feature.
|
||||||
// failing is the feature.
|
|
||||||
|
|
||||||
describe('mapRunLine — what it forwards', () => {
|
describe('mapRunLine — what it forwards', () => {
|
||||||
it('forwards a text part as final text, because run emits whole blocks not deltas', () => {
|
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');
|
writeFileSync(FAILING_BIN, '#!/bin/sh\necho "boom" >&2\nexit 1\n');
|
||||||
chmodSync(FAILING_BIN, 0o755);
|
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 };
|
const CONFIG = { bin: STUB_BIN, fallbackCwd: stubDir };
|
||||||
|
|
||||||
afterAll(() => {
|
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);
|
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: <the user's entire message>` — 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 () => {
|
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.
|
// The guard must not overreach: an ordinary failure is still an error the user needs to see.
|
||||||
const sessionKey = 'sess-solo';
|
const sessionKey = 'sess-solo';
|
||||||
|
|||||||
@@ -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 { 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';
|
import type { OpenCodeRunParams } from '../protocol';
|
||||||
|
|
||||||
// Drives one chat turn by spawning `opencode run … --format json` and mapping its newline-delimited
|
// 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.cwd) args.push('--dir', params.cwd);
|
||||||
if (params.model) args.push('--model', params.model);
|
if (params.model) args.push('--model', params.model);
|
||||||
if (params.resumeSessionId) args.push('--session', params.resumeSessionId);
|
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: <your entire message>`. Verified against the binary.
|
||||||
|
if (imagePaths.length > 0) args.push('--');
|
||||||
args.push(params.prompt);
|
args.push(params.prompt);
|
||||||
|
|
||||||
const cwd = params.cwd && existsSync(params.cwd) ? params.cwd : config.fallbackCwd;
|
const cwd = params.cwd && existsSync(params.cwd) ? params.cwd : config.fallbackCwd;
|
||||||
@@ -138,6 +150,7 @@ export function runOpenCodeTurn(params: OpenCodeRunParams, config: RunnerConfig,
|
|||||||
clearTimeout(hardTimer);
|
clearTimeout(hardTimer);
|
||||||
if (inactivityTimer) clearTimeout(inactivityTimer);
|
if (inactivityTimer) clearTimeout(inactivityTimer);
|
||||||
if (running.get(sessionKey) === handle) running.delete(sessionKey);
|
if (running.get(sessionKey) === handle) running.delete(sessionKey);
|
||||||
|
cleanUpTurnImages(imagePaths);
|
||||||
if (event) emitEvent(event);
|
if (event) emitEvent(event);
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -268,6 +281,50 @@ export function listRunningOpenCodeTurns(): { sessionKey: string }[] {
|
|||||||
return Array.from(running.keys()).map((sessionKey) => ({ sessionKey }));
|
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<string, string> = {
|
||||||
|
'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.
|
* Kill every turn this process is running, because the process itself is going away.
|
||||||
*
|
*
|
||||||
|
|||||||
@@ -197,6 +197,14 @@ export type OpenCodeRunParams = {
|
|||||||
cwd?: string; // passed to `opencode run --dir` — hard-re-anchors tools to this directory
|
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
|
model?: string; // `providerID/modelID` (e.g. opencode/claude-haiku-4-5); passed to --model verbatim
|
||||||
resumeSessionId?: string; // OpenCode `ses_…` id to continue (`--session`)
|
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
|
durable?: boolean; // commit turn output to chat_session_events (default true) — see ClaudeSpawnStreamingParams
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user