diff --git a/README.md b/README.md index 3e4cb1b..4a744e4 100644 --- a/README.md +++ b/README.md @@ -234,10 +234,12 @@ Other launchers: `apex run opencode`, `apex run claude`, `apex run pi`, `apex ru Launchers pass extra arguments through unchanged (without a shell), can be overridden by explicit user arguments, and never run `init`. `apex run claude` sets the gateway URL, token, attribution flag and model aliases in the child process only, leaving your default Claude provider alone. -`apex run opencode` checks which OpenCode is installed: OpenCode 1 gets `--model`; OpenCode 2, -whose terminal interface has no `--model`, starts with `--standalone` and the model in -`OPENCODE_CONFIG_CONTENT`, so the private server sees the model and, for a config that references -the key variable, your key too. +`apex run opencode` asks the installed binary which interface it has (`--standalone` exists only on +OpenCode 2; its version number says nothing, because dev and beta builds of both generations report +`0.0.0-*`): OpenCode 1 gets `--model`; OpenCode 2, whose terminal interface has no `--model`, starts +with `--standalone` and the model in `OPENCODE_CONFIG_CONTENT`, so the private server sees the +model and, for a config that references the key variable, your key too. A binary that cannot be +asked launches with neither flag, on your own configuration, and says so. ## Safety and recovery diff --git a/src/assistants.js b/src/assistants.js index 526c848..7371f65 100644 --- a/src/assistants.js +++ b/src/assistants.js @@ -1,4 +1,5 @@ import { execFile } from 'node:child_process'; +import { promisify } from 'node:util'; import { access, readFile, stat } from 'node:fs/promises'; import { constants } from 'node:fs'; import { join, delimiter } from 'node:path'; @@ -97,17 +98,22 @@ export async function executable(command, env = process.env, platform = process. return null; } -// OpenCode 1 prints `1.18.34`, OpenCode 2 prints `opencode v2.0.29`, each on a line of its own, -// so a notice naming another version is not mistaken for it. Anything unreadable is treated as 1, -// the launch Apex CLI has always used. -export function openCodeMajor(binary) { - if (!binary) return Promise.resolve(1); - return new Promise(resolve => { - execFile(binary, ['--version'], { timeout: 10000 }, (error, stdout) => { - const match = !error && /^(?:opencode )?v?(\d+)\.\d+\.\d+\s*$/m.exec(stdout); - resolve(match ? Number(match[1]) : 1); - }); - }); +const execFileAsync = promisify(execFile); + +// Only OpenCode 2 has --standalone, so its --help tells the generations apart. The version can't: +// dev and beta builds of both report `0.0.0-*` or `local`. OpenCode 1 prints its help on stderr +// and OpenCode 2 on stdout, so both are checked. Resolves true (OpenCode 2), false (OpenCode 1) +// or null when the binary gave no usable answer; it never rejects. +export async function openCodeSupportsStandalone(binary) { + if (!binary) return null; + try { + const { stdout, stderr } = await execFileAsync(binary, ['--help'], { timeout: 5000 }); + return `${stdout}${stderr}`.includes('--standalone'); + } catch (error) { + // Covers nonzero exits, timeouts, spawn failures and synchronous throws (e.g. a .cmd on + // Windows). Output printed before a failure still counts; without the flag it proves nothing. + return `${error.stdout}${error.stderr}`.includes('--standalone') ? true : null; + } } export function locations(home = homedir(), env = process.env, platform = process.platform) { @@ -272,14 +278,26 @@ export function launchToken(env, provider) { return token; } -export function launchOptions(id, env = process.env, provider = defaultProvider(env), { openCode = 1 } = {}) { +// `openCodeStandalone` is the result of openCodeSupportsStandalone: true for OpenCode 2, false for +// OpenCode 1, null when unknown. Unknown adds no model flag, since a wrong one makes OpenCode exit. +export function launchOptions(id, env = process.env, provider = defaultProvider(env), { openCodeStandalone = false } = {}) { const { model } = provider; const token = launchToken(env, provider); const childEnv = { ...env }; switch (id) { case 'opencode': { const selected = `${provider.opencode}/${model}`; - if (openCode < 2) return { args: ['--model', selected], env: childEnv }; + if (openCodeStandalone === null) { + // Both generations start without a model flag, on the user's own configured model. + return { + args: [], env: childEnv, + note: '(OpenCode interface unknown: no model flag added)', + warning: 'Apex CLI: Could not ask which OpenCode interface is installed, so it launches' + + ' without a model flag. OpenCode 1 takes --model, OpenCode 2 takes --standalone;' + + ' opencode --help says which you have.', + }; + } + if (!openCodeStandalone) return { args: ['--model', selected], env: childEnv }; // OpenCode 2's interface has no --model, and its shared background service never sees this // environment. A private server (--standalone) does: it reads the model from inline config, // and the gateway key the config references. Its flags only follow a subcommand. @@ -308,7 +326,7 @@ export function launchOptions(id, env = process.env, provider = defaultProvider( // The placeholder token is never shown: only the argument list and the launch's note are. export function runExpansion(id, provider, installed = {}) { const { args, note } = launchOptions(id, { [provider.envVar]: 'placeholder' }, provider, installed); - return [`${id} ${args.join(' ')}`, note].filter(Boolean).join(' '); + return [[id, ...args].join(' '), note].filter(Boolean).join(' '); } // One line per key: the whole object on one line cannot be folded to a terminal, and a step that diff --git a/src/cli.js b/src/cli.js index 8143f37..84dfcc4 100644 --- a/src/cli.js +++ b/src/cli.js @@ -4,7 +4,7 @@ import { spawn } from 'node:child_process'; import { readFile, stat } from 'node:fs/promises'; import { basename } from 'node:path'; import { randomUUID } from 'node:crypto'; -import { IDS, MANUAL_IDS, NAMES, PROVIDERS, PROVIDER_IDS, RUNNABLE, defaultProvider, detect, executable, launchOptions, launchToken, manualSteps, openCodeMajor, planAssistant, runExpansion } from './assistants.js'; +import { IDS, MANUAL_IDS, NAMES, PROVIDERS, PROVIDER_IDS, RUNNABLE, defaultProvider, detect, executable, launchOptions, launchToken, manualSteps, openCodeSupportsStandalone, planAssistant, runExpansion } from './assistants.js'; import { removeChange, writeChange } from './config.js'; import { MODE_NOTE, describeFile, resolveMode, runWrites } from './plan.js'; import { appendEntry, batchIds, journalPath, markUndone, newEntry, planUndo, readJournal } from './journal.js'; @@ -379,8 +379,14 @@ async function commandInit(rest) { ui.dim('Apex CLI never reads, stores or logs the key itself.'), ]); const runnable = active.map(entry => entry.id).filter(id => RUNNABLE.includes(id)); - const installed = { openCode: await openCodeMajor(active.find(entry => entry.id === 'opencode')?.binary) }; - const rows = runnable.map(id => ({ command: `${apex} run ${id}`, expansion: runExpansion(id, provider, installed) })); + // OpenCode not on PATH can't be probed; show the bare command and let `apex run` report it. + const opencodeBinary = active.find(entry => entry.id === 'opencode')?.binary; + const probed = opencodeBinary ? await openCodeSupportsStandalone(opencodeBinary) : undefined; + const installed = { openCodeStandalone: probed }; + const rows = runnable.map(id => ({ + command: `${apex} run ${id}`, + expansion: id === 'opencode' && probed === undefined ? id : runExpansion(id, provider, installed), + })); const commandWidth = ui.columnWidth(rows.map(row => row.command)); ui.section(runnable.length ? `Use these commands to run ${provider.model} with your selected harnesses:` @@ -567,7 +573,10 @@ async function commandRun(rest) { throw new Error('Windows .cmd/.bat launchers are not executed through a shell. Use WSL or launch the configured assistant directly.'); } // OpenCode 1 and 2 take the model in different ways, so the launch depends on which is installed. - const options = launchOptions(id, process.env, provider, { openCode: await openCodeMajor(id === 'opencode' && binary) }); + // The warning goes to stderr because the child owns stdout. + const standalone = id === 'opencode' ? await openCodeSupportsStandalone(binary) : false; + const options = launchOptions(id, process.env, provider, { openCodeStandalone: standalone }); + if (options.warning) console.error(styleText('yellow', options.warning)); const args = passed[0] === '--' ? passed.slice(1) : passed; const child = spawn(binary, await launchArgs(options, args), { env: options.env, stdio: 'inherit', shell: false }); await new Promise((resolve, reject) => { diff --git a/test/apex.test.js b/test/apex.test.js index 39ff25d..794731b 100644 --- a/test/apex.test.js +++ b/test/apex.test.js @@ -4,7 +4,7 @@ import { chmod, mkdtemp, mkdir, writeFile, readFile, readdir, stat, lstat, symli import { tmpdir } from 'node:os'; import { dirname, join, resolve } from 'node:path'; import { spawnSync } from 'node:child_process'; -import { PROVIDERS, defaultProvider, detect, locations, planAssistant, launchOptions, manualSteps, runExpansion } from '../src/assistants.js'; +import { PROVIDERS, defaultProvider, detect, locations, planAssistant, launchOptions, manualSteps, openCodeSupportsStandalone, runExpansion } from '../src/assistants.js'; import { addToml, updateToml, editJson, parseJson, parseToml, readConfig, sha256, writeChange } from '../src/config.js'; import { diffValues, unifiedDiff } from '../src/diff.js'; import { isSecretKey, redactText } from '../src/secrets.js'; @@ -33,10 +33,12 @@ async function fixture(context) { // `npm test` and `npx` set this, and it decides how follow-up commands are spelled. delete env.CI; delete env.npm_command; - // A gateway key in the developer's shell would change the default gateway, and their own - // XDG_DATA_HOME would point OpenCode's key store at their real one. + // A gateway key in the developer's shell would change the default gateway, their own + // XDG_DATA_HOME would point OpenCode's key store at their real one, and their inline OpenCode + // config would reach the fake binaries. for (const provider of Object.values(PROVIDERS)) if (provider !== CALLSTACK) delete env[provider.envVar]; delete env.XDG_DATA_HOME; + delete env.OPENCODE_CONFIG_CONTENT; return { home, env, journal: join(home, '.apex-state', 'journal.json') }; } @@ -890,38 +892,84 @@ test('CLI run forwards argument boundaries, child environment and exit status', assert.equal(output.token, env.CALLSTACK_AUTH_TOKEN); }); +test('the OpenCode probe answers v2, v1, or nothing, and never rejects', async context => { + const { home } = await fixture(context); + const bin = join(home, 'bin'); + await mkdir(bin); + const script = stream => `#!${process.execPath} +process.${stream}.write('Usage: opencode [flags]\\n --standalone start a private server\\n'); +process.exit(0); +`; + await writeFile(join(bin, 'help-stdout'), script('stdout'), { mode: 0o755 }); + await writeFile(join(bin, 'help-stderr'), script('stderr'), { mode: 0o755 }); + // OpenCode 1's help has no --standalone. + await writeFile(join(bin, 'v1-help'), `#!${process.execPath} +process.stderr.write('Usage: opencode [flags]\\n --model model to use\\n'); +process.exit(0); +`, { mode: 0o755 }); + // Output from a failing --help still counts; a failure with no output is unknown. + await writeFile(join(bin, 'exit-3-flag'), `#!${process.execPath}\nprocess.stdout.write(' --standalone\\n');\nprocess.exit(3);\n`, { mode: 0o755 }); + await writeFile(join(bin, 'exit-1-silent'), `#!${process.execPath}\nprocess.exit(1);\n`, { mode: 0o755 }); + const paths = name => join(bin, name); + assert.equal(await openCodeSupportsStandalone(paths('help-stdout')), true, 'the flag on stdout'); + assert.equal(await openCodeSupportsStandalone(paths('help-stderr')), true, 'the flag on stderr, where OpenCode 1 writes'); + assert.equal(await openCodeSupportsStandalone(paths('v1-help')), false, 'a help page without the flag'); + assert.equal(await openCodeSupportsStandalone(paths('exit-3-flag')), true, 'a failing --help that printed the flag still answers v2'); + assert.equal(await openCodeSupportsStandalone(paths('exit-1-silent')), null, 'a failing --help that printed nothing is no answer'); + assert.equal(await openCodeSupportsStandalone(join(home, 'missing')), null, 'a binary that is not there'); + assert.equal(await openCodeSupportsStandalone(bin), null, 'a directory is not a binary'); + // execFile throws synchronously on some paths (a NUL byte here, a .cmd on Windows). + assert.equal(await openCodeSupportsStandalone('a\0b'), null, 'a path execFile throws synchronously on'); + assert.equal(await openCodeSupportsStandalone(''), null, 'no path, no answer'); + assert.equal(await openCodeSupportsStandalone(null), null, 'no binary, no answer'); +}); + test('CLI run picks the model the way the installed OpenCode accepts it', async context => { const { home, env } = await fixture(context); const bin = join(home, 'bin'); await mkdir(bin); + // The fake's --help output, and so the detected generation, is driven by environment variables. await writeFile(join(bin, 'opencode'), `#!${process.execPath} -if (process.argv[2] === '--version') { console.log(process.env.FAKE_OPENCODE_VERSION); process.exit(0); } +if (process.argv[2] === '--help') { + if (process.env.FAKE_OPENCODE_HELP_FAILS === '1') process.exit(1); + process.stdout.write(process.env.FAKE_OPENCODE_STANDALONE === '1' + ? ' --standalone start a private server\\n --print-logs write logs to a file\\n' + : ' --model model to use\\n --print-logs write logs to a file\\n'); + process.exit(0); +} console.log(JSON.stringify({ args: process.argv.slice(2), inline: process.env.OPENCODE_CONFIG_CONTENT })); `, { mode: 0o755 }); - const run = (version, extra = {}, passed = ['--x']) => { - const result = cli(['run', 'opencode', '--', ...passed], { ...env, PATH: bin, FAKE_OPENCODE_VERSION: version, ...extra }); + const V2 = { FAKE_OPENCODE_STANDALONE: '1' }; + const V1 = { FAKE_OPENCODE_STANDALONE: '0' }; + const run = (capability = {}, extra = {}, passed = ['--x']) => { + const result = cli(['run', 'opencode', '--', ...passed], { ...env, PATH: bin, ...capability, ...extra }); assert.equal(result.status, 0, result.stderr); return JSON.parse(result.stdout); }; - assert.deepEqual(run('1.18.34'), { args: ['--model', `callstack.ai/${MODEL}`, '--x'] }); - assert.deepEqual(run('1.18.34', {}, ['run', 'hi']).args, ['--model', `callstack.ai/${MODEL}`, 'run', 'hi']); + assert.deepEqual(run(V1), { args: ['--model', `callstack.ai/${MODEL}`, '--x'] }); + assert.deepEqual(run(V1, {}, ['run', 'hi']).args, ['--model', `callstack.ai/${MODEL}`, 'run', 'hi']); // OpenCode 2's TUI rejects --model; a private server reads the model from inline config. - const v2 = run('opencode v2.0.29', { OPENCODE_CONFIG_CONTENT: '{ // OpenCode reads JSONC here too\n"theme":"dark", }' }); + const v2 = run(V2, { OPENCODE_CONFIG_CONTENT: '{ // OpenCode reads JSONC here too\n"theme":"dark", }' }); assert.deepEqual(v2.args, ['--standalone', '--x']); - assert.deepEqual(run('2.0.29', {}, ['run', 'hi']).args, ['run', '--standalone', 'hi'], 'v2 flags follow the subcommand'); - assert.deepEqual(run('2.0.29', {}, [home]).args, ['--standalone', home], 'a directory starts the interface'); - assert.deepEqual(run('2.0.29', {}, ['mini']).args, ['mini', '--standalone']); - assert.deepEqual(run('Update available: 2.1.0\n1.18.34').args, ['--model', `callstack.ai/${MODEL}`, '--x'], - 'a notice naming another version is not the version'); - assert.deepEqual(run('2.0.29', {}, ['auth', 'list']).args, ['auth', 'list'], 'subcommands without --standalone pass through'); + assert.deepEqual(run(V2, {}, ['run', 'hi']).args, ['run', '--standalone', 'hi'], 'v2 flags follow the subcommand'); + assert.deepEqual(run(V2, {}, [home]).args, ['--standalone', home], 'a directory starts the interface'); + assert.deepEqual(run(V2, {}, ['mini']).args, ['mini', '--standalone']); + assert.deepEqual(run(V2, {}, ['auth', 'list']).args, ['auth', 'list'], 'subcommands without --standalone pass through'); assert.deepEqual(JSON.parse(v2.inline), { theme: 'dark', model: `callstack.ai/${MODEL}` }); - assert.deepEqual(run('unreadable').args, ['--model', `callstack.ai/${MODEL}`, '--x'], 'unknown versions keep the v1 launch'); - const broken = cli(['run', 'opencode'], { ...env, PATH: bin, FAKE_OPENCODE_VERSION: '2.0.0', OPENCODE_CONFIG_CONTENT: '{' }); + // An unknown generation gets neither flag, and a warning. + const unknown = run({ FAKE_OPENCODE_HELP_FAILS: '1' }); + assert.deepEqual(unknown.args, ['--x'], 'an unanswerable probe launches with neither --model nor --standalone'); + assert.equal(unknown.inline, undefined, 'and the inline config is left alone'); + assert.match(cli(['run', 'opencode', '--', '--x'], { ...env, PATH: bin, FAKE_OPENCODE_HELP_FAILS: '1' }).stderr, + /launches without a model flag/); + const broken = cli(['run', 'opencode'], { ...env, PATH: bin, ...V2, OPENCODE_CONFIG_CONTENT: '{' }); assert.equal(broken.status, 1); assert.match(broken.stderr, /Invalid JSON\/JSONC object in OPENCODE_CONFIG_CONTENT/); assert.equal(runExpansion('opencode', CALLSTACK), `opencode --model callstack.ai/${MODEL}`); - assert.equal(runExpansion('opencode', CALLSTACK, { openCode: 2 }), 'opencode --standalone (OPENCODE_CONFIG_CONTENT model set)'); + assert.equal(runExpansion('opencode', CALLSTACK, { openCodeStandalone: true }), 'opencode --standalone (OPENCODE_CONFIG_CONTENT model set)'); + assert.equal(runExpansion('opencode', CALLSTACK, { openCodeStandalone: null }), + 'opencode (OpenCode interface unknown: no model flag added)'); }); test('closing the pipe early exits quietly instead of crashing', async context => { @@ -1195,7 +1243,8 @@ test('the closing block shows what each run command expands to, and when undo is const applied = cli([...INIT, '--apply'], env); assert.equal(applied.status, 0, applied.stderr); assert.match(applied.stdout, at(/^Use these commands to run callstack\/Apex with your selected harnesses:$/)); - assert.match(applied.stdout, at(/^apex run opencode {2}opencode --model callstack\.ai\/callstack\/Apex$/)); + // The fixture has no opencode on PATH, so nothing is probed and the line is the bare command. + assert.match(applied.stdout, at(/^apex run opencode {2}opencode$/)); assert.match(applied.stdout, at(/^apex run codex +codex --profile callstack_ai$/)); assert.match(applied.stdout, at(/^apex run pi +pi --provider callstack --model callstack\/Apex --thinking medium$/)); assert.match(applied.stdout, at(/^\.\.\.or pick "callstack\/Apex" from the UI when setting up manually\.$/));