mirror of
https://github.com/garrytan/gbrain.git
synced 2026-08-14 00:48:18 +00:00
fix(cli): stop thin-client jobs/config from fabricating a scratch PGLite (#2951)
`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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
3a5c4c194c
commit
f1031d5a0b
+31
@@ -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<string, string> = {
|
||||
'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 <id>` 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 {
|
||||
|
||||
+17
-1
@@ -132,9 +132,23 @@ function formatJobDetail(job: MinionJob): string {
|
||||
return lines.join('\n');
|
||||
}
|
||||
|
||||
export async function runJobs(engine: BrainEngine, args: string[]): Promise<void> {
|
||||
export async function runJobs(engineOrNull: BrainEngine | null, args: string[]): Promise<void> {
|
||||
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) {
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<string, string> = {',
|
||||
);
|
||||
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'\)/);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user