say a missing opencode binary out loud instead of hanging
Bun.spawn throws on a missing or non-executable binary rather than resolving to a failed process, and that throw escaped runOpenCodeTurn entirely — past the bookkeeping, out of the sidecar command handler, with no opencode:event ever emitted. The browser sat on a spinner nothing could end, because the code that ends turns had not been reached. A wrong OPENCODE_BIN is the ordinary way to get there, so the message names the path it tried: that is the difference between a fix and a debugging session. Also records that messageCount is not a gap. SessionList renders an OpenCode badge in place of the count for those rows, so the hardcoded 0 never reaches a screen, and computing a real one would cost an HTTP call per listed session — the session record has no count field — to populate something nothing shows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -102,6 +102,11 @@ passed that one model.
|
|||||||
Until the model question is answered, turns stay on `opencode run --dir`, which is verified working on
|
Until the model question is answered, turns stay on `opencode run --dir`, which is verified working on
|
||||||
1.18.16.
|
1.18.16.
|
||||||
|
|
||||||
|
**`messageCount` is a non-issue, not a gap.** `SessionList.tsx:197-203` renders an `OpenCode` badge in
|
||||||
|
place of the count for OpenCode rows, so the hardcoded `0` is never displayed. Computing a real count
|
||||||
|
would cost one HTTP call per listed session — the session record carries no count field — to populate
|
||||||
|
something nothing renders. Left alone deliberately.
|
||||||
|
|
||||||
**Images are done, and they never needed the fork** (bucket 1 lists them as "No — see B4", and Phase 4
|
**Images are done, and they never needed the fork** (bucket 1 lists them as "No — see B4", and Phase 4
|
||||||
put them behind the migration). `opencode run` takes attachments with `--file`, so the subprocess path
|
put them behind the migration). `opencode run` takes attachments with `--file`, so the subprocess path
|
||||||
carries them today: the sidecar spills each image to a temp file for the turn and removes it in
|
carries them today: the sidecar spills each image to a temp file for the turn and removes it in
|
||||||
|
|||||||
@@ -273,6 +273,28 @@ describe('runOpenCodeTurn — a second turn on a live session', () => {
|
|||||||
expect(argv.at(-1)).toBe('plain');
|
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 () => {
|
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';
|
||||||
|
|||||||
@@ -114,12 +114,30 @@ export function runOpenCodeTurn(params: OpenCodeRunParams, config: RunnerConfig,
|
|||||||
|
|
||||||
const cwd = params.cwd && existsSync(params.cwd) ? params.cwd : config.fallbackCwd;
|
const cwd = params.cwd && existsSync(params.cwd) ? params.cwd : config.fallbackCwd;
|
||||||
|
|
||||||
const proc = Bun.spawn([config.bin, ...args], {
|
// `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,
|
cwd,
|
||||||
stdin: 'ignore', // == /dev/null: `run` hangs waiting on stdin otherwise
|
stdin: 'ignore', // == /dev/null: `run` hangs waiting on stdin otherwise
|
||||||
stdout: 'pipe',
|
stdout: 'pipe',
|
||||||
stderr: '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
|
// `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.
|
// over `handle`, so the two cannot both be defined first. Nothing can call it in between.
|
||||||
|
|||||||
Reference in New Issue
Block a user