diff --git a/src/cli.ts b/src/cli.ts index 212aada0d..cc0d9d7f4 100755 --- a/src/cli.ts +++ b/src/cli.ts @@ -145,6 +145,36 @@ const CLI_ONLY_SELF_HELP = new Set([ 'migrate', 'retrieval-upgrade', ]); +/** + * Commands in CLI_ONLY_SELF_HELP whose handler honours `--help` as its first + * action, before reading the engine. Dispatching them here keeps `--help` + * answerable with no brain configured. + * + * Membership is behaviour, not taste: each entry is pinned by + * test/cli-help-without-brain.test.ts, which runs the CLI with an empty + * GBRAIN_HOME and requires exit 0 plus real help output. + */ +const SELF_HELP_WITHOUT_ENGINE: Record Promise<(engine: never, args: string[]) => unknown>> = { + models: async () => (await import('./commands/models.ts')).runModels as never, + watch: async () => (await import('./commands/watch.ts')).runWatch as never, + skillopt: async () => (await import('./commands/skillopt.ts')).runSkillOptCommand as never, + maintain: async () => (await import('./commands/maintain.ts')).runMaintain as never, + 'extract-conversation-facts': async () => + (await import('./commands/extract-conversation-facts.ts')).runExtractConversationFacts as never, +}; + +/** Returns true when the command's own help was printed. */ +async function printSelfHelpWithoutEngine(command: string, args: string[]): Promise { + const load = SELF_HELP_WITHOUT_ENGINE[command]; + if (!load) return false; + const run = await load(); + // The engine is never read on the help path; passing a placeholder keeps the + // handler signatures untouched. skillopt already declares `BrainEngine | null` + // for exactly this reason. + await run(null as never, args); + return true; +} + // v114 (#1941): alias -> operation lookup, kept separate from `cliOps` so // aliases don't double-list in printHelp's auto-generated section. Collisions // with a primary CLI name, a CLI_ONLY command, or another alias throw at module @@ -342,6 +372,12 @@ async function main() { printCliOnlyHelp(command); return; } + // Self-help members whose handler answers --help before it touches the + // engine. Without this they fall through to the normal dispatch, which + // connects first — so `gbrain models --help` on a machine with no brain + // exits 1 with "No brain configured", and the handler's own help block is + // unreachable. That is the state a reader is most likely to be in. + if (await printSelfHelpWithoutEngine(command, subArgs)) return; } // #2185: strict unknown-flag validation — pre-dispatch, pre-engine. A flag diff --git a/test/cli-help-without-brain.serial.test.ts b/test/cli-help-without-brain.serial.test.ts new file mode 100644 index 000000000..7c9611caf --- /dev/null +++ b/test/cli-help-without-brain.serial.test.ts @@ -0,0 +1,99 @@ +/** + * `--help` must be answerable with no brain configured. + * + * A reader runs `--help` most often right after install, before `gbrain init`. + * CLI_ONLY_SELF_HELP members skip the dispatcher's generic usage stub (that is + * the point — they print their own), but they were then reached through the + * normal dispatch, which connects the engine first. So on a machine with no + * brain they exited 1 with "No brain configured" and their own help block was + * unreachable code. + * + * The oracle here is behaviour, not a declaration: each command is actually + * run with an empty GBRAIN_HOME. + */ +import { describe, test, expect } from 'bun:test'; +import { mkdtempSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +const REPO = new URL('..', import.meta.url).pathname; + +/** Answer `--help` before touching the engine. */ +const HELP_WITHOUT_BRAIN = [ + 'models', + 'watch', + 'skillopt', + 'maintain', + 'extract-conversation-facts', +]; + +/** + * Self-help members that still need a brain for `--help`, because their handler + * has no `--help` branch to reach. Pinned rather than omitted so the list can + * only shrink deliberately: writing help for one of these, or dropping it from + * CLI_ONLY_SELF_HELP so the generic stub answers, fails this test until the + * entry moves. + */ +const STILL_NEEDS_A_BRAIN = [ + 'brainstorm', + 'config', + 'embed', + 'lsd', + 'migrate', + 'pages', + 'retrieval-upgrade', +]; + +async function runHelp(command: string): Promise<{ code: number; out: string }> { + const home = mkdtempSync(join(tmpdir(), 'gbrain-nobrain-')); + // An empty GBRAIN_HOME is not enough: loadConfig also honours + // GBRAIN_DATABASE_URL and DATABASE_URL (config.ts:550-551), so a developer + // or CI runner that exports either would let the CLI connect anyway — the + // positive assertions would pass on master and this guard would be inert. + const env: Record = { ...process.env, GBRAIN_HOME: home }; + delete env.GBRAIN_DATABASE_URL; + delete env.DATABASE_URL; + // --no-env-file: bun auto-loads .env from cwd, and GBRAIN_DATABASE_URL is + // honored unconditionally, so a developer's local .env would put back + // exactly what the deletes above removed. + const proc = Bun.spawn(['bun', '--no-env-file', 'run', 'src/cli.ts', command, '--help'], { + cwd: REPO, + env, + stdout: 'pipe', + stderr: 'pipe', + }); + const [stdout, stderr] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + ]); + const code = await proc.exited; + return { code, out: stdout + stderr }; +} + +describe('--help without a configured brain', () => { + for (const command of HELP_WITHOUT_BRAIN) { + test(`${command} --help answers`, async () => { + const { code, out } = await runHelp(command); + expect(code).toBe(0); + expect(out).not.toContain('No brain configured'); + // Real help, not a one-line stub or an empty exit. + expect(out.split('\n').filter(l => l.trim()).length).toBeGreaterThan(3); + expect(out.toLowerCase()).toContain(command.split('-')[0]); + }, 30_000); + } + + test('the known-unfixed set is exactly what it claims', async () => { + // Concurrently: seven sequential CLI spawns is most of this file's wall + // clock, and each one is independent (its own temp GBRAIN_HOME). + const results = await Promise.all( + STILL_NEEDS_A_BRAIN.map(async command => ({ command, ...(await runHelp(command)) })), + ); + const unexpectedlyWorking = results + .filter(r => !r.out.includes('No brain configured')) + .map(r => r.command); + // Not a wish that they stay broken — a tripwire. Fixing one is good and + // should move it to HELP_WITHOUT_BRAIN in the same change, so the coverage + // list never drifts away from reality in either direction. + expect(unexpectedlyWorking).toEqual([]); + }, 90_000); +});