From cc1783c0e4a6fd3ab8c424dc9505b5b160493745 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sat, 1 Aug 2026 08:40:02 +0800 Subject: [PATCH] fix(cli): make --brain actually route to the named brain (#3576) Co-Authored-By: Garry Tan --- docs/architecture/KEY_FILES.md | 2 +- src/cli.ts | 61 ++++++++ src/core/cli-options.ts | 51 ++++++ test/brain-flag-routing.serial.test.ts | 200 ++++++++++++++++++++++++ test/cli-options.test.ts | 61 +++++++- test/doctor-orphan-ratio.test.ts | 2 +- test/e2e/orphan-reduction.test.ts | 2 +- test/extract-by-mention-resume.test.ts | 2 +- test/extract-by-mention.test.ts | 2 +- test/thin-client-upgrade-prompt.test.ts | 1 + 10 files changed, 374 insertions(+), 10 deletions(-) create mode 100644 test/brain-flag-routing.serial.test.ts diff --git a/docs/architecture/KEY_FILES.md b/docs/architecture/KEY_FILES.md index ef277c125..357f1cdde 100644 --- a/docs/architecture/KEY_FILES.md +++ b/docs/architecture/KEY_FILES.md @@ -305,7 +305,7 @@ per-release `**vX.Y.Z:**` narration — CI enforces this - `src/core/timeline-dedup-repair.ts` (#2038) — schema-drift self-heal for `idx_timeline_dedup`. The migration that widened the dedup index from `(page_id, date, summary)` to `(page_id, date, summary, source)` was renumbered during a master merge, so a brain that ran the old variant has its version counter stamped past the change while the index keeps the 3-column shape — and every `addTimelineEntry` batch then fails its 4-column `ON CONFLICT`, silently breaking timeline writes brain-wide. The version counter can't detect this, so the repair is keyed off the actual index SHAPE: `checkTimelineDedupIndex(engine)` returns `{tablePresent, indexPresent, columns, needsRepair}` (read-only; powers the `timeline_dedup_index` doctor check) and `repairTimelineDedupIndex(engine)` dedupes-then-rebuilds the index. `runMigrations` invokes the repair on every pass (including the no-pending early-return path); idempotent no-op when the index is already 4-column. `gbrain apply-migrations --force-schema` triggers it on demand. Pinned by `test/timeline-dedup-repair.test.ts`. - `src/core/progress.ts` — Shared bulk-action progress reporter. Writes to stderr. Modes: `auto` (TTY `\r`-rewriting; non-TTY plain lines), `human`, `json` (JSONL), `quiet`. Rate-gated by `minIntervalMs` and `minItems`. `startHeartbeat(reporter, note)` for single long queries. `child()` composes phase paths. Singleton SIGINT/SIGTERM coordinator emits `abort` events for every live phase. EPIPE defense on both sync throws and stream `'error'` events. Zero dependencies. `emitHumanLine` is prefix-aware — inside a `withSourcePrefix(id, ...)` scope from `src/core/console-prefix.ts` it prepends `[id] ` (and TTY-rewrite mode `\r\x1b[2K` carries the prefix inside the clear-to-EOL escape); `emitJson` is intentionally NOT prefixed so NDJSON consumers don't choke on a `[id] {...}` shape. - `src/core/console-prefix.ts` — `AsyncLocalStorage`-backed per-source line-prefix helper. Exports `withSourcePrefix(id, fn)` (runs `fn` with `id` as active prefix; nested wraps replace then restore), `getSourcePrefix()` (read-only accessor; test seam), and `slog(...)` / `serr(...)` (prefix-aware `console.log`/`console.error`). Embedded-newline-safe: a multi-line string under prefix `[foo]` emits `[foo] line1\n[foo] line2`. Outside a wrap, `slog`/`serr` fall through to bare `console.log`/`console.error` so single-source callers see identical output (back-compat invariant). Use `src.id` (slug-validated by `sources add`) NOT `src.name` (free-form) to defeat log-injection through newline/control-character names. Coverage: `src/commands/sync.ts` performSync + callees, `src/commands/embed.ts` runEmbedCore + helpers, `src/core/progress.ts` emitHumanLine. -- `src/core/cli-options.ts` — Global CLI flag parser. `parseGlobalFlags(argv)` returns `{cliOpts, rest}` with `--quiet` / `--progress-json` / `--progress-interval=` stripped. `getCliOptions()` / `setCliOptions()` expose a module-level singleton so commands reach resolved flags without parameter threading. `cliOptsToProgressOptions()` maps to reporter options. `childGlobalFlags()` returns the flag suffix to append to `execSync('gbrain ...')` calls in migration orchestrators. `OperationContext.cliOpts` extends shared-op dispatch for MCP callers. +- `src/core/cli-options.ts` — Global CLI flag parser. `parseGlobalFlags(argv)` returns `{cliOpts, rest}` with `--quiet` / `--progress-json` / `--progress-interval=` / `--brain ` stripped. `--brain` is the brain-axis (which database) selector: exact-match only (`--brain-*` per-command flags pass through), value validated against the mount-id regex at parse time, missing/malformed value THROWS — never a silent host fallback. `connectEngine` in `src/cli.ts` feeds it (plus the ambient `GBRAIN_BRAIN_ID` / `.gbrain-mount` / mount-path tiers) through `resolveBrainId` → `BrainRegistry.getBrain`, which throws `UnknownBrainError` for an unregistered id; mounts get no auto-migrations and keep the host-config AI gateway. `getCliOptions()` / `setCliOptions()` expose a module-level singleton so commands reach resolved flags without parameter threading. `cliOptsToProgressOptions()` maps to reporter options. `childGlobalFlags()` returns the flag suffix to append to `execSync('gbrain ...')` calls in migration orchestrators (propagates `--brain=` so children stay on the parent's brain). `OperationContext.cliOpts` extends shared-op dispatch for MCP callers. - `src/core/db-lock.ts` — generic `tryAcquireDbLock(engine, lockId, ttlMinutes)` over the `gbrain_cycle_locks` table. Parameterized lock id so scopes nest cleanly: `gbrain-cycle` for the broad cycle (held by `cycle.ts`) and `gbrain-sync` (`SYNC_LOCK_ID`) for `performSync`'s narrower writer window. UPSERT-with-TTL semantics survive PgBouncer transaction pooling (unlike session-scoped `pg_try_advisory_lock`); crashed holders auto-release once their TTL expires. It also does automatic same-host dead-pid takeover: when the upsert finds a held, NOT-TTL-expired lock whose holder is on this host and provably dead, it reclaims via a guarded `DELETE WHERE id=$1 AND holder_pid=$2` + one normal-upsert retry returning the standard handle (refresh/release intact). The liveness check is the exported `classifyHolderLiveness(pid, host, ageMs, opts?)` / `isHolderDeadLocally(...)` (injectable `process.kill` seam; `HOLDER_TAKEOVER_GRACE_MS = 60_000` PID-reuse guard; EPERM classified as `alive` so a live process you don't own is never stolen). TTL-expired locks stay the upsert's job; cross-host stays TTL-only. `runBreakLock` (`src/commands/sync.ts`) consumes the same predicate. Background reaper (#1972): `reapDeadHolderLocks(engine)` is the periodic sweep the contention path lacked — it deletes locks whose holder is `isHolderDeadLocally`, scoped to the `gbrain-sync:*` / `gbrain-cycle`/`gbrain-cycle:*` namespaces ONLY (election/supervisor/reindex locks keep TTL-only behavior, untouched), via `deleteLockRowExact(engine, id, pid, acquiredAt)` — a snapshot-matched delete (`date_trunc('milliseconds', acquired_at) = $3`, so the ms a JS Date keeps survives) that's TOCTOU-safe against a reused PID taking the lock between SELECT and DELETE. `cycle.ts` runs it at cycle start (before the sync phase); `gbrain doctor --fix` runs it for no-autopilot brains. `selectLockRows(engine, opts?)` + a shared row→`LockSnapshot` mapper are the single canonical reader now backing `inspectLock` + `listStaleLocks` + the reaper (was triplicated). `isLockHolderLive(snap, ttlMinutes)` (#2227) is the observability liveness predicate — freshness-keyed (`ttl_expired` plus the heartbeat steal-grace), never `process.kill`, so `gbrain jobs supervisor status` / `gbrain doctor` can report a live supervisor via its queue lock without a PID-reuse false-positive. Pinned by `test/db-lock-auto-takeover.test.ts` + `test/db-lock-reap.test.ts`. - `src/core/sync-concurrency.ts` — single source of truth for the parallel-sync policy. Exports `autoConcurrency(engine, fileCount, override?)` (PGLite always serial; explicit override clamped to >=1; auto path returns `DEFAULT_PARALLEL_WORKERS=4` when `fileCount > AUTO_CONCURRENCY_FILE_THRESHOLD=100`), `shouldRunParallel(workers, fileCount, explicit)` (explicit `--workers` bypasses the >50-file floor), and `parseWorkers(s)` (rejects `'0'`, `'-3'`, `'foo'`, `'1.5'`, trailing chars). Used by `performSync`, `performFullSync`, `runImport`, and the Minion `sync` handler so the sites can't drift. `DEFAULT_PARALLEL_SOURCES = 4` is a SEPARATE constant for the per-source fan-out under `gbrain sync --all` — kept distinct from `DEFAULT_PARALLEL_WORKERS` because total live Postgres connections per wave ≈ `DEFAULT_PARALLEL_SOURCES × DEFAULT_PARALLEL_WORKERS × 2 (per-file pool)` = 32 at both defaults (each per-file worker opens its own `PostgresEngine` with `poolSize = min(2, resolvePoolSize(2))`); `sync.ts` warns when `parallel × workers × 2 > 16`. `resolveWorkersWithClamp(engine, override, commandName, fileCount)` wraps `autoConcurrency` with a per-command stderr clamp warning on PGLite (per-(command, requested) dedup via module-scoped warned-once set with `_resetWorkersClampWarningsForTest()` seam) and is the canonical surface for every bulk-command `--workers N` flag (extract-conversation-facts, extract, edges-backfill, reindex-multimodal, reindex, reindex-code); embed.ts deliberately bypasses it and keeps `GBRAIN_EMBED_CONCURRENCY || 20`. `resolveMaxConnections()` (reads `GBRAIN_MAX_CONNECTIONS`, undefined when unset) + `clampWorkersForConnectionBudget(workers, perWorkerPool, maxConnections, parentPool)` back the opt-in single-sync connection-footprint clamp so a big sync stays under a low pooler cap (`parent_pool + workers×perWorkerPool ≤ budget`); `gbrain doctor`'s `pool_budget` check (`computePoolBudgetCheck` / `checkPoolBudget` in `src/commands/doctor.ts`) warns when the budget leaves no room for a worker, pointing at `GBRAIN_POOL_SIZE=2`. Pinned by `test/pglite-workers-clamp.test.ts`. - `src/core/worker-pool.ts` — Canonical sliding-pool + bounded-semaphore primitive (extracted from `src/commands/embed.ts` sliding-pool sites and `src/commands/eval-cross-modal.ts` `runWithLimit` semaphore). Two exports: `runSlidingPool({items, workers, onItem, signal?, onError?, failureLabel?, onProgress?})` + `runWithLimit({items, limit, fn, signal?})`. Atomicity invariant: `const idx = nextIdx++` is one synchronous JS statement (no `await` between read and write — guaranteed by the single-threaded event loop), documented in the module header AND enforced by `scripts/check-worker-pool-atomicity.sh` (wired into `bun run verify`), which rejects importing `worker_threads` in any consuming file and inserting `await` between the `nextIdx` read and write. `MUST_ABORT_ERROR_TAGS` set is seeded with `BUDGET_EXHAUSTED` from `src/core/budget/budget-tracker.ts`; tagged errors (matched via `err.tag === 'BUDGET_EXHAUSTED'` to avoid cross-module import) bypass `onError` and hard-abort the pool via `AbortController.abort()` to in-flight `onItem` — the budget cap is a structural ceiling under concurrency. `failures[]` shape is `{idx, label, error}` records (NOT full items; callers supply `failureLabel(item) => string`) for bounded memory under huge brains. Pinned by `test/worker-pool.test.ts` + `test/scripts/check-worker-pool-atomicity.test.ts`. Drives every `--workers N` bulk command. diff --git a/src/cli.ts b/src/cli.ts index a5dfd5e1e..35786d451 100755 --- a/src/cli.ts +++ b/src/cli.ts @@ -404,6 +404,18 @@ async function main() { if (op.localOnly) { refuseThinClient(command, cfgPre!.remote_mcp!.mcp_url); } + // A thin client has no local mounts — an explicit --brain cannot be + // honored and must not be silently dropped (same loud-beats-silent rule + // as applyThinClientSourceScope's --source refusal). Ambient tiers + // (GBRAIN_BRAIN_ID / .gbrain-mount) are ignored here, matching the + // source axis's ambient-with-nowhere-to-send behavior. + if (cliOpts.brain) { + console.error( + '--brain is not supported on a thin-client install: the remote server is a single brain. ' + + 'Remove the flag, or run from a machine with local mounts (gbrain mounts list).', + ); + process.exit(1); + } // #2098: the local path resolves --source / GBRAIN_SOURCE / .gbrain-source // inside makeContext (ctx.sourceId), which this route never reaches — so // scope must be mapped onto the op's source_id wire param before the call. @@ -1014,6 +1026,10 @@ export async function makeContext(engine: BrainEngine, params: Record { + const config = loadConfig(); + if (config) { + const { configureGateway } = await import('./core/ai/gateway.ts'); + configureGateway(buildGatewayConfig(config)); + } + const { loadRegistry } = await import('./core/brain-registry.ts'); + const handle = await loadRegistry().getBrain(brainId); + activeBrainId = brainId; + return handle.engine; +} + async function connectEngine(opts?: { probeOnly?: boolean }): Promise { + // Brain axis: resolve WHICH DATABASE this invocation targets before touching + // the host engine. --brain (global flag) / GBRAIN_BRAIN_ID / .gbrain-mount / + // mount-path-prefix resolve via the canonical 6-tier chain — the mirror of + // the source axis in makeContext. connectEngine is the single choke point + // every local CLI command routes through (shared ops, CLI-only commands, + // and the search-dashboard path), so routing lands here once. + const { resolveBrainId } = await import('./core/brain-resolver.ts'); + const brainId = resolveBrainId(getCliOptions().brain); + if (brainId !== 'host') return connectMountEngine(brainId); + const config = loadConfig(); if (!config) { console.error('No brain configured. Run: gbrain init'); diff --git a/src/core/cli-options.ts b/src/core/cli-options.ts index e005495ae..0660382c4 100644 --- a/src/core/cli-options.ts +++ b/src/core/cli-options.ts @@ -29,6 +29,15 @@ export interface CliOptions { * the reranker. Has no effect on other commands. */ explain: boolean; + /** + * `--brain ` — which BRAIN (database) this invocation targets: 'host' + * or a mount id from ~/.gbrain/mounts.json. Parsed here (stripped before + * per-command parsing, like --source) so it can never collide with + * per-op flag parsing. `null` = no explicit flag; connectEngine resolves + * the ambient tiers (GBRAIN_BRAIN_ID / .gbrain-mount / mount-path / 'host') + * via src/core/brain-resolver.ts. + */ + brain: string | null; } export const DEFAULT_CLI_OPTIONS: CliOptions = { @@ -37,8 +46,29 @@ export const DEFAULT_CLI_OPTIONS: CliOptions = { progressInterval: 1000, timeoutMs: null, explain: false, + brain: null, }; +/** + * Brain-id shape. Same regex as brain-registry's BRAIN_ID_RE (kept in sync; + * brain-resolver.ts follows the same convention). 'host' matches. Validated + * at parse time so an invalid id fails LOUDLY here — and so childGlobalFlags + * can safely splice the value into execSync('gbrain ...') command strings. + */ +const BRAIN_ID_RE = /^[a-z0-9](?:[a-z0-9-]{0,30}[a-z0-9])?$/; + +function parseBrainValue(val: string | undefined): string { + if (val === undefined || val.length === 0 || val.startsWith('-')) { + throw new Error('--brain requires a value (a mount id from `gbrain mounts list`, or "host").'); + } + if (!BRAIN_ID_RE.test(val)) { + throw new Error( + `Invalid --brain value "${val}". Must match [a-z0-9-]{1,32}, start+end alphanumeric.`, + ); + } + return val; +} + /** * Parse recognized global flags from the front / anywhere in argv and return * the resolved options plus the remaining argv (with global flags stripped). @@ -125,6 +155,20 @@ export function parseGlobalFlags(argv: string[]): { cliOpts: CliOptions; rest: s cliOpts.explain = true; continue; } + // --brain / --brain= — brain (database) axis. Exact-match only: + // `--brain-wide-max-cost-usd` (skillopt) and other `--brain-*` flags pass + // through to per-command parsers untouched. A missing or malformed value + // THROWS rather than falling through — a dropped --brain silently routes + // to the wrong database (the exact bug class this flag's wiring fixes). + if (a === '--brain') { + cliOpts.brain = parseBrainValue(argv[i + 1]); + i++; + continue; + } + if (a.startsWith('--brain=')) { + cliOpts.brain = parseBrainValue(a.slice('--brain='.length)); + continue; + } slots.push({ plain: a }); } @@ -256,6 +300,13 @@ export function childGlobalFlags(cliOpts?: CliOptions): string { if (opts.progressInterval !== DEFAULT_CLI_OPTIONS.progressInterval) { parts.push(`--progress-interval=${opts.progressInterval}`); } + // Brain routing must survive into child `gbrain ...` subprocesses: the env + // and dotfile tiers self-propagate (children inherit env + cwd), but an + // explicit --brain does not — without this, a parent routed to a mount + // spawns children that silently operate on the host brain. The value is + // BRAIN_ID_RE-validated at parse time, so splicing it into an exec string + // is safe. + if (opts.brain) parts.push(`--brain=${opts.brain}`); return parts.length > 0 ? ' ' + parts.join(' ') : ''; } diff --git a/test/brain-flag-routing.serial.test.ts b/test/brain-flag-routing.serial.test.ts new file mode 100644 index 000000000..d86b7652c --- /dev/null +++ b/test/brain-flag-routing.serial.test.ts @@ -0,0 +1,200 @@ +/** + * `--brain ` must actually route to the named mounted brain. + * + * The bug: docs/architecture/brains-and-sources.md promises + * `gbrain query "X" --brain media-team` runs against the team's DB, and + * src/core/brain-resolver.ts implements the full 6-tier chain — but nothing + * ever CALLED the resolver from the CLI dispatch path. `--brain media-team` + * was silently ignored (unknown flag) and the command ran against the HOST + * brain, returning confident wrong answers. Same silent-wrong-target class + * as #1712/#3524 on the source axis. + * + * These tests spawn the real CLI against a fake home with two distinct + * PGLite brains (host + one mount), each seeded with a uniquely-slugged + * page, and assert on WHICH brain's data comes back: + * - control: no flag → host page (default unchanged); + * - `--brain team-a` → the mount's page, not the host's; + * - `--brain nope` (unregistered) → hard error, NOT a silent host fallback; + * - `GBRAIN_BRAIN_ID=team-a` → the mount's page (env tier wired too). + * + * Pre-fix, the --brain/env spawns list the HOST page and the unknown-brain + * spawn exits 0 — all three fail behaviorally on an unfixed tree. + * + * Serial because it spawns subprocesses + writes tmpdirs. + */ +import { describe, test, expect, beforeAll, afterAll } from 'bun:test'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'fs'; +import { join } from 'path'; +import { tmpdir } from 'os'; +import { PGLiteEngine } from '../src/core/pglite-engine.ts'; + +const REPO = new URL('..', import.meta.url).pathname.replace(/\/$/, ''); + +let home: string; +let mountsPath: string; + +async function seedBrain(databasePath: string, slug: string): Promise { + const engine = new PGLiteEngine(); + await engine.connect({ engine: 'pglite', database_path: databasePath }); + await engine.initSchema(); + await engine.putPage(slug, { + type: 'note', + title: slug, + compiled_truth: `content of ${slug}`, + frontmatter: {}, + }); + await engine.disconnect(); +} + +function cliEnv(extra: Record = {}): Record { + return { + ...process.env as Record, + HOME: home, + GBRAIN_HOME: home, + GBRAIN_MOUNTS_PATH: mountsPath, + GBRAIN_SKIP_STARTUP_HOOKS: '1', + // Neutralize ambient routing signals from the invoking shell/CI. + GBRAIN_BRAIN_ID: '', + GBRAIN_SOURCE: '', + GBRAIN_DATABASE_URL: '', + DATABASE_URL: '', + ...extra, + }; +} + +async function runCli( + args: string[], + env: Record, + timeoutMs = 90_000, +): Promise<{ exitCode: number; stdout: string; stderr: string }> { + const proc = Bun.spawn(['bun', 'run', `${REPO}/src/cli.ts`, ...args], { + cwd: REPO, + env, + stdout: 'pipe', + stderr: 'pipe', + }); + const killer = setTimeout(() => { + try { proc.kill('SIGKILL'); } catch { /* already dead */ } + }, timeoutMs); + try { + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + proc.exited, + ]); + return { exitCode, stdout, stderr }; + } finally { + clearTimeout(killer); + } +} + +beforeAll(async () => { + home = mkdtempSync(join(tmpdir(), 'gbrain-brain-flag-')); + mkdirSync(join(home, '.gbrain'), { recursive: true }); + mkdirSync(join(home, 'team-a-clone'), { recursive: true }); + + const hostDb = join(home, '.gbrain', 'brain.pglite'); + const teamDb = join(home, 'team-a.pglite'); + + writeFileSync( + join(home, '.gbrain', 'config.json'), + JSON.stringify({ engine: 'pglite', database_path: hostDb, embedding_dimensions: 1536 }) + '\n', + ); + mountsPath = join(home, '.gbrain', 'mounts.json'); + writeFileSync( + mountsPath, + JSON.stringify({ + version: 1, + mounts: [ + { + id: 'team-a', + path: join(home, 'team-a-clone'), + engine: 'pglite', + database_path: teamDb, + enabled: true, + }, + ], + }) + '\n', + ); + + // Two brains, two distinct pages. WHICH slug comes back tells us WHICH + // database the CLI actually queried. + await seedBrain(hostDb, 'host-page'); + await seedBrain(teamDb, 'team-page'); +}, 240_000); + +afterAll(() => { + try { rmSync(home, { recursive: true, force: true }); } catch { /* best effort */ } +}); + +describe('--brain routes the CLI to the named mounted brain', () => { + test('control: no brain signal → host brain (default unchanged)', async () => { + const r = await runCli(['list'], cliEnv()); + expect(r.exitCode).toBe(0); + expect(r.stdout).toContain('host-page'); + expect(r.stdout).not.toContain('team-page'); + }, 120_000); + + test('--brain team-a → the mount database, not host', async () => { + const r = await runCli(['list', '--brain', 'team-a'], cliEnv()); + expect(r.exitCode).toBe(0); + expect(r.stdout).toContain('team-page'); + expect(r.stdout).not.toContain('host-page'); + }, 120_000); + + test('--brain hard-errors — never a silent host fallback', async () => { + const r = await runCli(['list', '--brain', 'nope'], cliEnv()); + expect(r.exitCode).not.toBe(0); + expect(r.stdout + r.stderr).toMatch(/Unknown brain/i); + // The silent-wrong-results bug: pre-fix this listed the host's pages. + expect(r.stdout).not.toContain('host-page'); + }, 120_000); + + test('GBRAIN_BRAIN_ID=team-a env tier is wired through the same seam', async () => { + const r = await runCli(['list'], cliEnv({ GBRAIN_BRAIN_ID: 'team-a' })); + expect(r.exitCode).toBe(0); + expect(r.stdout).toContain('team-page'); + expect(r.stdout).not.toContain('host-page'); + }, 120_000); +}); + +// ── Trust boundary: brain selection is NEVER caller-controlled ──────────── +// +// Brain routing happens at engine-connect time in the local CLI process +// (trusted, remote === false). An untrusted caller over MCP must have no way +// to name a brain: no op declares a brain param, and neither context builder +// reads one from params. Fail-closed pins for the new surface. + +describe('untrusted callers cannot cross brains', () => { + test('no operation exposes a brain/brain_id param an MCP caller could set', async () => { + const { operations } = await import('../src/core/operations.ts'); + for (const op of operations) { + expect(`${op.name}:${'brain' in op.params}`).toBe(`${op.name}:false`); + expect(`${op.name}:${'brain_id' in op.params}`).toBe(`${op.name}:false`); + } + }); + + test('makeContext ignores caller-supplied params.brain (stays on the connected engine)', async () => { + const { makeContext } = await import('../src/cli.ts'); + const stub = { + kind: 'pglite', + executeRaw: async () => [], + getConfig: async () => null, + } as any; + const ctx = await makeContext(stub, { brain: 'team-a', brain_id: 'team-a' }); + expect(ctx.engine).toBe(stub); + // Local process default is the host brain; params must not move it. + expect(ctx.brainId ?? 'host').toBe('host'); + }); + + test('remote dispatch context never derives a brain from params (fail-closed)', async () => { + const { buildOperationContext } = await import('../src/mcp/dispatch.ts'); + const stub = { kind: 'pglite' } as any; + const ctx = buildOperationContext(stub, { brain: 'team-a', brain_id: 'team-a' }, { + remote: true, + sourceId: 'default', + }); + expect(ctx.engine).toBe(stub); + expect(ctx.brainId).toBeUndefined(); + }); +}); diff --git a/test/cli-options.test.ts b/test/cli-options.test.ts index ab01c9851..29cdc6bcc 100644 --- a/test/cli-options.test.ts +++ b/test/cli-options.test.ts @@ -65,7 +65,7 @@ describe('parseGlobalFlags', () => { test('all global flags combined', () => { const r = parseGlobalFlags(['--quiet', '--progress-json', '--progress-interval=250', 'sync']); - expect(r.cliOpts).toEqual({ quiet: true, progressJson: true, progressInterval: 250, timeoutMs: null, explain: false }); + expect(r.cliOpts).toEqual({ quiet: true, progressJson: true, progressInterval: 250, timeoutMs: null, explain: false, brain: null }); expect(r.rest).toEqual(['sync']); }); @@ -96,7 +96,7 @@ describe('getCliOptions / setCliOptions singleton', () => { test('setCliOptions applies + getCliOptions returns a copy', () => { _resetCliOptionsForTest(); - setCliOptions({ quiet: false, progressJson: true, progressInterval: 250, timeoutMs: null, explain: false }); + setCliOptions({ quiet: false, progressJson: true, progressInterval: 250, timeoutMs: null, explain: false, brain: null }); expect(getCliOptions().progressJson).toBe(true); expect(getCliOptions().progressInterval).toBe(250); }); @@ -156,12 +156,12 @@ describe('CLI integration: progress streams to the right channel', () => { describe('cliOptsToProgressOptions', () => { test('--quiet → quiet mode', () => { - const opts = cliOptsToProgressOptions({ quiet: true, progressJson: false, progressInterval: 1000, timeoutMs: null, explain: false }); + const opts = cliOptsToProgressOptions({ quiet: true, progressJson: false, progressInterval: 1000, timeoutMs: null, explain: false, brain: null }); expect(opts.mode).toBe('quiet'); }); test('--progress-json → json mode with interval', () => { - const opts = cliOptsToProgressOptions({ quiet: false, progressJson: true, progressInterval: 500, timeoutMs: null, explain: false }); + const opts = cliOptsToProgressOptions({ quiet: false, progressJson: true, progressInterval: 500, timeoutMs: null, explain: false, brain: null }); expect(opts.mode).toBe('json'); expect(opts.minIntervalMs).toBe(500); }); @@ -173,7 +173,7 @@ describe('cliOptsToProgressOptions', () => { }); test('quiet takes priority over progressJson', () => { - const opts = cliOptsToProgressOptions({ quiet: true, progressJson: true, progressInterval: 1000, timeoutMs: null, explain: false }); + const opts = cliOptsToProgressOptions({ quiet: true, progressJson: true, progressInterval: 1000, timeoutMs: null, explain: false, brain: null }); expect(opts.mode).toBe('quiet'); }); }); @@ -224,3 +224,54 @@ describe('--timeout flag', () => { expect(r.cliOpts.timeoutMs).toBe(null); }); }); + +describe('--brain flag (brain axis routing)', () => { + test('--brain space form: parsed + stripped from rest', () => { + const r = parseGlobalFlags(['query', 'X', '--brain', 'media-team']); + expect(r.cliOpts.brain).toBe('media-team'); + expect(r.rest).toEqual(['query', 'X']); + }); + + test('--brain= equals form: parsed + stripped from rest', () => { + const r = parseGlobalFlags(['--brain=media-team', 'query', 'X']); + expect(r.cliOpts.brain).toBe('media-team'); + expect(r.rest).toEqual(['query', 'X']); + }); + + test('--brain host is a valid explicit value', () => { + const r = parseGlobalFlags(['stats', '--brain', 'host']); + expect(r.cliOpts.brain).toBe('host'); + }); + + test('missing value throws (loud, never a silent host fallback)', () => { + expect(() => parseGlobalFlags(['query', 'X', '--brain'])).toThrow(/--brain requires a value/); + expect(() => parseGlobalFlags(['--brain=', 'query'])).toThrow(/--brain requires a value/); + // A following flag is not a value. + expect(() => parseGlobalFlags(['--brain', '--quiet'])).toThrow(/--brain requires a value/); + }); + + test('malformed id throws (validated at parse time)', () => { + expect(() => parseGlobalFlags(['--brain', 'Bad_Id!'])).toThrow(/Invalid --brain value/); + expect(() => parseGlobalFlags(['--brain=$(rm -rf /)'])).toThrow(/Invalid --brain value/); + }); + + test('--brain-* per-command flags pass through untouched (skillopt collision guard)', () => { + const r = parseGlobalFlags(['skillopt', '--brain-wide-max-cost-usd', '5']); + expect(r.cliOpts.brain).toBe(null); + expect(r.rest).toEqual(['skillopt', '--brain-wide-max-cost-usd', '5']); + }); + + test('default brain is null (ambient resolution applies)', () => { + const r = parseGlobalFlags(['query', 'X']); + expect(r.cliOpts.brain).toBe(null); + }); +}); + +describe('childGlobalFlags propagates --brain', () => { + test('explicit brain rides into child gbrain subprocess commands', async () => { + const { childGlobalFlags } = await import('../src/core/cli-options.ts'); + expect(childGlobalFlags({ ...DEFAULT_CLI_OPTIONS, brain: 'media-team' })) + .toContain('--brain=media-team'); + expect(childGlobalFlags({ ...DEFAULT_CLI_OPTIONS })).not.toContain('--brain'); + }); +}); diff --git a/test/doctor-orphan-ratio.test.ts b/test/doctor-orphan-ratio.test.ts index 29d293b1e..cd0089fbd 100644 --- a/test/doctor-orphan-ratio.test.ts +++ b/test/doctor-orphan-ratio.test.ts @@ -41,7 +41,7 @@ beforeAll(async () => { engine = new PGLiteEngine(); await engine.connect({}); await engine.initSchema(); - setCliOptions({ quiet: true, progressJson: false, progressInterval: 1000, explain: false, timeoutMs: null }); + setCliOptions({ quiet: true, progressJson: false, progressInterval: 1000, explain: false, timeoutMs: null, brain: null }); }, 60_000); afterAll(async () => { diff --git a/test/e2e/orphan-reduction.test.ts b/test/e2e/orphan-reduction.test.ts index 422aa38be..96778717e 100644 --- a/test/e2e/orphan-reduction.test.ts +++ b/test/e2e/orphan-reduction.test.ts @@ -47,7 +47,7 @@ beforeAll(async () => { engine = new PGLiteEngine(); await engine.connect({}); await engine.initSchema(); - setCliOptions({ quiet: true, progressJson: false, progressInterval: 1000, explain: false, timeoutMs: null }); + setCliOptions({ quiet: true, progressJson: false, progressInterval: 1000, explain: false, timeoutMs: null, brain: null }); }, 60_000); afterAll(async () => { diff --git a/test/extract-by-mention-resume.test.ts b/test/extract-by-mention-resume.test.ts index f66b97732..cdbf9cc66 100644 --- a/test/extract-by-mention-resume.test.ts +++ b/test/extract-by-mention-resume.test.ts @@ -46,7 +46,7 @@ beforeAll(async () => { engine = new PGLiteEngine(); await engine.connect({}); await engine.initSchema(); - setCliOptions({ quiet: true, progressJson: false, progressInterval: 1000, explain: false, timeoutMs: null }); + setCliOptions({ quiet: true, progressJson: false, progressInterval: 1000, explain: false, timeoutMs: null, brain: null }); }, 60_000); afterAll(async () => { diff --git a/test/extract-by-mention.test.ts b/test/extract-by-mention.test.ts index 8205f2cb1..788a9b1ad 100644 --- a/test/extract-by-mention.test.ts +++ b/test/extract-by-mention.test.ts @@ -73,7 +73,7 @@ beforeAll(async () => { await engine.initSchema(); // Default CLI options (quiet enough that the progress reporter doesn't // pollute the capture buffer beyond what the assertions need). - setCliOptions({ quiet: false, progressJson: false, progressInterval: 1000, explain: false, timeoutMs: null }); + setCliOptions({ quiet: false, progressJson: false, progressInterval: 1000, explain: false, timeoutMs: null, brain: null }); }, 60_000); afterAll(async () => { diff --git a/test/thin-client-upgrade-prompt.test.ts b/test/thin-client-upgrade-prompt.test.ts index 2aed5561f..779801615 100644 --- a/test/thin-client-upgrade-prompt.test.ts +++ b/test/thin-client-upgrade-prompt.test.ts @@ -39,6 +39,7 @@ const DEFAULT_CLI_OPTS: CliOptions = { progressInterval: 1000, timeoutMs: null, explain: false, + brain: null, }; let tmpHome: string;