From f1031d5a0b15b25640c66b8f1f898c598131f33d Mon Sep 17 00:00:00 2001 From: Sailesh Sivakumar <32437884+ss251@users.noreply.github.com> Date: Mon, 20 Jul 2026 23:55:46 +0530 Subject: [PATCH] fix(cli): stop thin-client jobs/config from fabricating a scratch PGLite (#2951) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `jobs list|get` have had remote MCP routing since v0.32, but the CLI shell still ran connectEngine() before dispatch — on a thin-client install that fabricates an empty scratch PGLite in the thin-client GBRAIN_HOME and replays the entire migration chain on every invocation, before the remote call even runs. Host-only jobs subcommands (work, supervisor, submit, ...) and `config` did the same instead of refusing. - cli.ts: dispatch thin-client `jobs list|get` engine-free (runJobs(null, ...)); refuse the other jobs subcommands with a pinpoint hint; add `config` to THIN_CLIENT_REFUSED_COMMANDS with a hint (it reads/writes the host brain's config plane). - jobs.ts: widen runJobs to accept a null engine, guarded so null can only reach the MCP-routed list/get branches. - tests: behavioral (no scratch store created, no migration replay, refusals carry hints) + source-audit pins in the existing idioms. Co-authored-by: Claude Fable 5 --- src/cli.ts | 31 +++++++++++++++++ src/commands/jobs.ts | 18 +++++++++- test/cli-dispatch-thin-client.test.ts | 44 ++++++++++++++++++++++++ test/thin-client-routing-audit.test.ts | 46 ++++++++++++++++++++++++++ 4 files changed, 138 insertions(+), 1 deletion(-) diff --git a/src/cli.ts b/src/cli.ts index 53622bb05..154f449b1 100755 --- a/src/cli.ts +++ b/src/cli.ts @@ -998,6 +998,13 @@ const THIN_CLIENT_REFUSED_COMMANDS = new Set([ // - `code-def`/`code-refs`/`code-callers`/`code-callees` have NO MCP ops // in operations.ts:2630-2671; cannot be "fixed by routing" yet 'pages', 'files', 'eval', 'code-def', 'code-refs', 'code-callers', 'code-callees', + // scratch-DB audit: `config` get/set operate on the host brain's config + // plane (DB rows / host file-plane). On a thin client they fabricated an + // ephemeral local PGLite (full migration replay per call) and read/wrote + // config nobody would ever see. NOTE: `jobs` is deliberately NOT here — + // it gets a partial dispatch (list/get route over MCP engine-free, the + // rest refuse) in the main dispatch before connectEngine(). + 'config', ]); /** @@ -1035,6 +1042,9 @@ const THIN_CLIENT_REFUSE_HINTS: Record = { 'code-refs': '`code-refs` has no MCP op yet. Run on the host.', 'code-callers': '`code-callers` has no MCP op yet. Run on the host.', 'code-callees': '`code-callees` has no MCP op yet. Run on the host.', + // scratch-DB audit additions + config: "config reads/writes the host brain's config plane. Edit the host's .gbrain/config.json (file-plane keys) or run on the host with GBRAIN_HOME set.", + jobs: '`jobs list` and `jobs get ` are thin-client routable; this subcommand runs against the host queue. Use the submit_job / list_jobs / get_job MCP tools from your agent, or run on the host with GBRAIN_HOME set.', }; /** @@ -1593,6 +1603,27 @@ async function handleCliOnly(command: string, args: string[]) { } } + // Thin-client `jobs` dispatch: `list` and `get` route over MCP (v0.32 + // routing branches in commands/jobs.ts) and never touch a local engine — + // but falling through to connectEngine() below fabricates an empty + // scratch PGLite in the thin-client GBRAIN_HOME and replays the entire + // migration chain on every invocation before the remote call even runs. + // Dispatch them engine-free here; every other jobs subcommand is + // host-queue-bound, so refuse with a pinpoint hint instead of building + // the scratch store. + if (command === 'jobs') { + const cfgJobs = loadConfig(); + if (isThinClient(cfgJobs)) { + const jobsSub = args[0]; + if (jobsSub === 'list' || jobsSub === 'get') { + const { runJobs } = await import('./commands/jobs.ts'); + await runJobs(null, args); + return; + } + refuseThinClient('jobs', cfgJobs!.remote_mcp!.mcp_url); + } + } + // All remaining CLI-only commands need a DB connection const engine = await connectEngine(); try { diff --git a/src/commands/jobs.ts b/src/commands/jobs.ts index 32902b55a..52ba691ce 100644 --- a/src/commands/jobs.ts +++ b/src/commands/jobs.ts @@ -132,9 +132,23 @@ function formatJobDetail(job: MinionJob): string { return lines.join('\n'); } -export async function runJobs(engine: BrainEngine, args: string[]): Promise { +export async function runJobs(engineOrNull: BrainEngine | null, args: string[]): Promise { const sub = args[0]; + // Thin-client dispatch (cli.ts) passes engine=null for the subcommands + // with remote MCP routing (`list`, `get`) so no scratch local engine is + // ever built. Any other subcommand arriving with a null engine is a + // routing bug upstream of this function — refuse instead of crashing + // inside MinionQueue. + if (!engineOrNull && sub !== 'list' && sub !== 'get') { + console.error(`\`gbrain jobs ${sub ?? ''}\` needs a local engine and cannot run on a thin client.`); + process.exit(1); + } + // Null only ever reaches the MCP-routed `list`/`get` branches, which + // never touch the engine — narrowed once here so the host-only cases + // below typecheck unchanged. + const engine = engineOrNull as BrainEngine; + if (!sub || sub === '--help' || sub === '-h') { console.log(`gbrain jobs — Minions job queue @@ -217,6 +231,8 @@ HANDLER TYPES (built in) return; } + // The constructor just stores the reference; on the null (thin-client + // list/get) paths no queue method is ever reached. const queue = new MinionQueue(engine); switch (sub) { diff --git a/test/cli-dispatch-thin-client.test.ts b/test/cli-dispatch-thin-client.test.ts index 08807c36d..568e1b2db 100644 --- a/test/cli-dispatch-thin-client.test.ts +++ b/test/cli-dispatch-thin-client.test.ts @@ -174,3 +174,47 @@ describe('regression — local config still passes through normally', () => { expect(r.stdout).not.toContain('"mode":"thin-client"'); }); }); + +describe('thin-client scratch-DB guard — jobs partial dispatch + config refusal', () => { + test('`gbrain config set x y` is refused with pinpoint hint', async () => { + seedThinClientConfig(); + const r = await run(['config', 'set', 'search.reranker.enabled', 'false']); + expect(r.exitCode).toBe(1); + expect(r.stderr).toContain('gbrain config'); + expect(r.stderr).toContain('not routable'); + expect(r.stderr).toContain('thin-client of https://brain-host.example/mcp'); + }); + + test('`gbrain jobs work` is refused with pinpoint hint (host-queue-bound)', async () => { + seedThinClientConfig(); + const r = await run(['jobs', 'work']); + expect(r.exitCode).toBe(1); + expect(r.stderr).toContain('gbrain jobs'); + expect(r.stderr).toContain('not routable'); + expect(r.stderr).toContain('thin-client of https://brain-host.example/mcp'); + }); + + test('`gbrain jobs get` never fabricates a scratch local engine', async () => { + // The regression this pins: on a thin-client install with a PGLite + // engine key, `jobs get` connected a LOCAL engine before its remote + // routing branch ran — creating an empty scratch PGLite store in the + // thin-client GBRAIN_HOME and replaying the entire migration chain + // ("Schema version 1 → N") on every invocation. The remote call to + // brain-host.example will fail (unreachable) — irrelevant here. What + // matters: no local store is created and no migration replay runs. + seedThinClientConfig({ engine: 'pglite' }); + const r = await run(['jobs', 'get', '999']); + const { existsSync } = await import('fs'); + expect(existsSync(join(tmp, '.gbrain', 'brain.pglite'))).toBe(false); + expect(r.stdout + r.stderr).not.toContain('Schema version'); + expect(r.stdout + r.stderr).not.toContain('migration(s) pending'); + }); + + test('`gbrain jobs list` never fabricates a scratch local engine', async () => { + seedThinClientConfig({ engine: 'pglite' }); + const r = await run(['jobs', 'list']); + const { existsSync } = await import('fs'); + expect(existsSync(join(tmp, '.gbrain', 'brain.pglite'))).toBe(false); + expect(r.stdout + r.stderr).not.toContain('Schema version'); + }); +}); diff --git a/test/thin-client-routing-audit.test.ts b/test/thin-client-routing-audit.test.ts index b9a4e45db..0c2bcba10 100644 --- a/test/thin-client-routing-audit.test.ts +++ b/test/thin-client-routing-audit.test.ts @@ -127,3 +127,49 @@ describe('thin-client routing audit — v0.32 ROUTE additions wire callRemoteToo expect(src).toContain(`callRemoteTool(cfg!, 'get_job'`); }); }); + +describe('thin-client routing audit — scratch-DB additions (jobs partial dispatch + config refusal)', () => { + // `jobs list|get` route over MCP but the CLI shell still connected a + // local engine first, fabricating an empty scratch PGLite in the + // thin-client GBRAIN_HOME and replaying the full migration chain on + // every invocation. `config` did the same with no remote path at all. + + test('cli.ts dispatches thin-client jobs list/get engine-free (runJobs(null, ...))', () => { + expect(CLI_SOURCE).toMatch(/command === 'jobs'/); + expect(CLI_SOURCE).toMatch(/runJobs\(null, args\)/); + }); + + test('cli.ts refuses non-routable jobs subcommands on thin clients via refuseThinClient', () => { + const dispatchStart = CLI_SOURCE.indexOf("if (command === 'jobs') {"); + expect(dispatchStart).toBeGreaterThan(-1); + const dispatchBlock = CLI_SOURCE.slice(dispatchStart, dispatchStart + 900); + expect(dispatchBlock).toContain('isThinClient'); + expect(dispatchBlock).toContain("refuseThinClient('jobs'"); + }); + + test("'config' is in THIN_CLIENT_REFUSED_COMMANDS with a hint", () => { + const setStart = CLI_SOURCE.indexOf('const THIN_CLIENT_REFUSED_COMMANDS = new Set(['); + const setEnd = CLI_SOURCE.indexOf(']);', setStart); + expect(CLI_SOURCE.slice(setStart, setEnd)).toContain("'config'"); + const hintsStart = CLI_SOURCE.indexOf( + 'const THIN_CLIENT_REFUSE_HINTS: Record = {', + ); + const hintsEnd = CLI_SOURCE.indexOf('};', hintsStart); + expect(/\bconfig\s*:/.test(CLI_SOURCE.slice(hintsStart, hintsEnd))).toBe(true); + }); + + test("'jobs' is NOT in THIN_CLIENT_REFUSED_COMMANDS (partial dispatch owns it)", () => { + const setStart = CLI_SOURCE.indexOf('const THIN_CLIENT_REFUSED_COMMANDS = new Set(['); + const setEnd = CLI_SOURCE.indexOf(']);', setStart); + expect(CLI_SOURCE.slice(setStart, setEnd)).not.toContain("'jobs'"); + }); + + test('jobs.ts guards the null-engine path to list/get only', () => { + const src = readFileSync( + join(import.meta.dir, '..', 'src', 'commands', 'jobs.ts'), + 'utf8', + ); + expect(src).toContain('engineOrNull: BrainEngine | null'); + expect(src).toMatch(/if \(!engineOrNull && sub !== 'list' && sub !== 'get'\)/); + }); +});