From d8a207fdd722b73b8d596500bdef0afd19ab32ca Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 17 Aug 2026 10:43:54 -0700 Subject: [PATCH] fix(gbrain-status): MCP scoping is per-project, and project-local beats user scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit hasRemoteOnlyGbrainMcp scanned EVERY project's mcpServers in ~/.claude.json, so one project's remote gbrain registration reclassified broken local engines as thin-client machine-wide. It now reads user scope plus only the cwd's nearest-ancestor project key. The precedence itself was verified empirically and hermetically (fake HOME + CLAUDE_CONFIG_DIR fixtures, claude 2.1.233): with both scopes defining gbrain, 'claude mcp get gbrain' reports Scope: Local config — PROJECT-LOCAL WINS. Both in-repo consumers assumed the opposite; brain-cache's endpoint resolution flips to nearest-ancestor-project-first, and the stale user-first pin in brain-cache-roundtrip now pins the verified precedence. (The user-first jq in the brain-sync preamble resolver gets the same swap in the template block.) Co-Authored-By: Claude Fable 5 --- bin/gstack-brain-cache | 53 +++++++------ lib/gbrain-local-status.ts | 75 +++++++++++++----- test/brain-cache-roundtrip.test.ts | 13 +++- test/brain-cache-spec.test.ts | 61 ++++++++++++++- test/gbrain-local-status.test.ts | 118 ++++++++++++++++++++++++++++- 5 files changed, 272 insertions(+), 48 deletions(-) diff --git a/bin/gstack-brain-cache b/bin/gstack-brain-cache index abf45a013..5ced064c0 100755 --- a/bin/gstack-brain-cache +++ b/bin/gstack-brain-cache @@ -127,15 +127,19 @@ function sha8(input: string): string { * stable identity hash. Used to detect when the user switches brains * (different endpoint → different cache). * - * Reads BOTH registration scopes in ~/.claude.json (#2499): user scope - * (.mcpServers.gbrain) first, then project scope + * Reads BOTH registration scopes in ~/.claude.json (#2499): project scope * (.projects["/abs/path"].mcpServers.gbrain — what `claude mcp add` - * WITHOUT --scope user writes), preferring the nearest ancestor of cwd - * (longest matching project key) so nested repos resolve to their own - * brain. Before the project-scope read, two different project-scoped - * brains both hashed to 'local', so switching between them never - * invalidated the cache — the exact scenario this function exists to - * catch. + * WITHOUT --scope user writes) first, preferring the nearest ancestor of + * cwd (longest matching project key) so nested repos resolve to their own + * brain, then user scope (.mcpServers.gbrain) as the fallback. That order + * is Claude Code's own name-conflict precedence (local beats user) — + * verified empirically against claude 2.1.233 with a hermetic fake $HOME: + * `claude mcp get gbrain` reports "Scope: Local config" and the + * project-local URL when both scopes define the name — so the hash tracks + * the endpoint the project actually talks to. Before the project-scope + * read, two different project-scoped brains both hashed to 'local', so + * switching between them never invalidated the cache — the exact scenario + * this function exists to catch. * * Params exist for tests; production callers use the defaults. */ @@ -163,9 +167,11 @@ interface McpEntryish { } /** - * User-scope gbrain entry, else the nearest-ancestor project-scope entry - * for cwd (#2499). Path-boundary-aware: /a/repo never matches /a/repo2. - * Both separators are accepted so Windows project keys resolve. + * Nearest-ancestor project-scope gbrain entry for cwd, else the user-scope + * entry (#2499). Project-local first — Claude Code's own precedence for a + * same-name conflict (see detectEndpointHash's docstring for the empirical + * evidence). Path-boundary-aware: /a/repo never matches /a/repo2. Both + * separators are accepted so Windows project keys resolve. */ function resolveGbrainMcpEntry( cfg: unknown, @@ -175,20 +181,21 @@ function resolveGbrainMcpEntry( mcpServers?: Record; projects?: Record }>; } | null; - if (root?.mcpServers?.gbrain) return root.mcpServers.gbrain; const projects = root?.projects; - if (!projects || typeof projects !== 'object') return undefined; - let best: { key: string; entry: McpEntryish } | undefined; - for (const [key, val] of Object.entries(projects)) { - if (!val || typeof val !== 'object') continue; - const entry = val.mcpServers?.gbrain; - if (!entry || typeof entry !== 'object') continue; - const isAncestor = - cwd === key || cwd.startsWith(`${key}/`) || cwd.startsWith(`${key}\\`); - if (!isAncestor) continue; - if (!best || key.length > best.key.length) best = { key, entry }; + if (projects && typeof projects === 'object') { + let best: { key: string; entry: McpEntryish } | undefined; + for (const [key, val] of Object.entries(projects)) { + if (!val || typeof val !== 'object') continue; + const entry = val.mcpServers?.gbrain; + if (!entry || typeof entry !== 'object') continue; + const isAncestor = + cwd === key || cwd.startsWith(`${key}/`) || cwd.startsWith(`${key}\\`); + if (!isAncestor) continue; + if (!best || key.length > best.key.length) best = { key, entry }; + } + if (best) return best.entry; } - return best?.entry; + return root?.mcpServers?.gbrain; } // ────────────────────────────────────────────────────────────────────────── diff --git a/lib/gbrain-local-status.ts b/lib/gbrain-local-status.ts index ec2b115e1..5398c71a8 100644 --- a/lib/gbrain-local-status.ts +++ b/lib/gbrain-local-status.ts @@ -142,16 +142,33 @@ function gbrainConfigPath(env?: NodeJS.ProcessEnv): string { * broken-db / broken-config / engine-locked, silently suppressing brain * blocks for a fully-working remote brain. * - * Evidence read: ~/.claude.json MCP registrations — user scope AND project - * scope (project-scoped registrations are otherwise invisible, #2499). + * Evidence read: ~/.claude.json MCP registrations — user scope plus the + * cwd's NEAREST-ANCESTOR project scope only (#2499 made project scope + * visible; the per-project scoping fixes the machine-wide bleed where ONE + * project's remote registration reclassified broken local engines as + * thin-client for EVERY cwd). Ancestor matching mirrors the + * GBRAIN_MCP_ENTRY_JQ resolution in + * scripts/resolvers/preamble/generate-brain-sync-block.ts: cwd == key or + * cwd startswith key + separator, longest matching key that actually + * carries a gbrain entry wins (a nested project WITHOUT gbrain doesn't + * shadow its parent's registration). + * + * Same-name conflicts resolve project-local over user scope — Claude + * Code's own precedence, verified empirically against claude 2.1.233 with + * a hermetic fake $HOME: `claude mcp get gbrain` reports "Scope: Local + * config" and the project-local URL when both scopes define the name. + * * File-read only: no subprocess, no network (a classifier network probe is - * the #1964 pathology). Returns true only when a gbrain registration is - * remote-HTTP AND no gbrain registration is local-stdio — a local-stdio - * entry means the user runs a local engine (possibly alongside a remote one, - * e.g. federation), and local-engine statuses like engine-locked must keep - * their precise meaning there. + * the #1964 pathology). Returns true only when a visible gbrain + * registration is remote-HTTP AND no visible gbrain registration is + * local-stdio — a local-stdio entry means the user runs a local engine + * (possibly alongside a remote one, e.g. federation), and local-engine + * statuses like engine-locked must keep their precise meaning there. */ -export function hasRemoteOnlyGbrainMcp(env?: NodeJS.ProcessEnv): boolean { +export function hasRemoteOnlyGbrainMcp( + env?: NodeJS.ProcessEnv, + cwd: string = process.cwd(), +): boolean { interface McpEntry { type?: string; transport?: string; @@ -174,31 +191,53 @@ export function hasRemoteOnlyGbrainMcp(env?: NodeJS.ProcessEnv): boolean { if (entry.command) return "local"; return null; }; - let sawRemote = false; - let sawLocal = false; - const scan = (servers: unknown): void => { - if (!servers || typeof servers !== "object") return; + /** Extract the gbrain-relevant entries from an mcpServers object. */ + const gbrainEntries = (servers: unknown): Record => { + const out: Record = {}; + if (!servers || typeof servers !== "object") return out; for (const [name, entry] of Object.entries(servers as Record)) { if (!entry || typeof entry !== "object") continue; const isGbrainName = /^gbrain([-_][\w-]*)?$/.test(name); const cmdMentionsGbrain = typeof entry.command === "string" && /\bgbrain\b/.test(entry.command); if (!isGbrainName && !cmdMentionsGbrain) continue; - const c = classify(entry); - if (c === "remote") sawRemote = true; - if (c === "local") sawLocal = true; + out[name] = entry; } + return out; }; const root = cj as { mcpServers?: unknown; projects?: Record; } | null; - scan(root?.mcpServers); + const userGbrain = gbrainEntries(root?.mcpServers); + // Nearest-ancestor project entry for cwd that carries a gbrain server. + // Path-boundary-aware (/a/repo never matches /a/repo2); both separators + // accepted so Windows project keys resolve. + let projectGbrain: Record = {}; if (root?.projects && typeof root.projects === "object") { - for (const proj of Object.values(root.projects)) { - if (proj && typeof proj === "object") scan(proj.mcpServers); + let bestKey: string | null = null; + for (const [key, proj] of Object.entries(root.projects)) { + if (!proj || typeof proj !== "object") continue; + const entries = gbrainEntries((proj as { mcpServers?: unknown }).mcpServers); + if (Object.keys(entries).length === 0) continue; + const isAncestor = + cwd === key || cwd.startsWith(`${key}/`) || cwd.startsWith(`${key}\\`); + if (!isAncestor) continue; + if (bestKey === null || key.length > bestKey.length) { + bestKey = key; + projectGbrain = entries; + } } } + // Effective view for this cwd: project-local shadows user scope per name. + const effective: Record = { ...userGbrain, ...projectGbrain }; + let sawRemote = false; + let sawLocal = false; + for (const entry of Object.values(effective)) { + const c = classify(entry); + if (c === "remote") sawRemote = true; + if (c === "local") sawLocal = true; + } return sawRemote && !sawLocal; } diff --git a/test/brain-cache-roundtrip.test.ts b/test/brain-cache-roundtrip.test.ts index 0466bd287..b27121237 100644 --- a/test/brain-cache-roundtrip.test.ts +++ b/test/brain-cache-roundtrip.test.ts @@ -166,7 +166,12 @@ describe('brain-cache endpoint detection', () => { expect(outer).not.toBe('local'); }); - test('detectEndpointHash still prefers user scope over project scope (#2499)', async () => { + test('detectEndpointHash prefers project-local scope over user scope (#2392 wave)', async () => { + // Empirically verified against claude 2.1.233 with hermetic fixtures: + // `claude mcp get gbrain` reports "Scope: Local config" when both scopes + // define the server — project-local WINS. The old pin here encoded the + // opposite (user-first) assumption, which mis-hashed endpoints whenever + // the two scopes disagreed. const mod = await importCache(); const cj = join(TMP_HOME, 'claude.json'); writeFileSync(cj, JSON.stringify({ @@ -175,14 +180,14 @@ describe('brain-cache endpoint detection', () => { '/w/repo': { mcpServers: { gbrain: { url: 'https://proj.example/mcp' } } }, }, })); - const userScoped = mod.detectEndpointHash(cj, '/w/repo'); - // Same file minus the user-scope entry → different hash proves user scope won. + const conflictHash = mod.detectEndpointHash(cj, '/w/repo'); + // Same file minus the USER entry → identical hash proves project scope won. writeFileSync(cj, JSON.stringify({ projects: { '/w/repo': { mcpServers: { gbrain: { url: 'https://proj.example/mcp' } } }, }, })); - expect(mod.detectEndpointHash(cj, '/w/repo')).not.toBe(userScoped); + expect(mod.detectEndpointHash(cj, '/w/repo')).toBe(conflictHash); }); }); diff --git a/test/brain-cache-spec.test.ts b/test/brain-cache-spec.test.ts index 21a012f1c..05fb1fbc4 100644 --- a/test/brain-cache-spec.test.ts +++ b/test/brain-cache-spec.test.ts @@ -11,7 +11,10 @@ * Gate-tier, free, pure import + assertion. Runs in <100ms. */ -import { describe, test, expect } from 'bun:test'; +import { describe, test, expect, afterAll } from 'bun:test'; +import { mkdtempSync, writeFileSync, rmSync } from 'fs'; +import { join } from 'path'; +import { tmpdir } from 'os'; import { BRAIN_CACHE_ENTITIES, SKILL_DIGEST_SUBSETS, @@ -167,3 +170,59 @@ describe('brain-cache-spec internal consistency', () => { expect(getPreflightSkills().sort()).toEqual(expected.sort()); }); }); + +describe('brain-cache MCP scope precedence (C15 pin)', () => { + // Claude Code resolves a same-name MCP conflict in favor of the + // PROJECT-LOCAL entry (.projects[cwd].mcpServers) over the user-scope + // entry (.mcpServers). Verified empirically against claude 2.1.233 with a + // hermetic fake $HOME: `claude mcp get gbrain` reported "Scope: Local + // config" and the project-local URL when both scopes defined gbrain. + // detectEndpointHash must hash the endpoint the project actually talks + // to, or a brain switch would never invalidate the cache. + const TMP = mkdtempSync(join(tmpdir(), 'brain-cache-precedence-')); + afterAll(() => rmSync(TMP, { recursive: true, force: true })); + + const cache = () => import('../bin/gstack-brain-cache'); + const writeFixture = (name: string, cfg: object): string => { + const p = join(TMP, name); + writeFileSync(p, JSON.stringify(cfg)); + return p; + }; + const USER_URL = { type: 'http', url: 'https://user.example/mcp' }; + const PROJ_URL = { type: 'http', url: 'https://proj.example/mcp' }; + + test('project-local gbrain entry beats user scope for a cwd inside the project', async () => { + const mod = await cache(); + const conflict = writeFixture('claude-conflict.json', { + mcpServers: { gbrain: USER_URL }, + projects: { '/w/repo': { mcpServers: { gbrain: PROJ_URL } } }, + }); + const conflictHash = mod.detectEndpointHash(conflict, '/w/repo/src'); + // Same hash as the project entry alone → the project-local entry won. + const projOnly = writeFixture('claude-proj-only.json', { + projects: { '/w/repo': { mcpServers: { gbrain: PROJ_URL } } }, + }); + expect(conflictHash).toBe(mod.detectEndpointHash(projOnly, '/w/repo/src')); + // And NOT the user entry's hash. + const userOnly = writeFixture('claude-user-only.json', { + mcpServers: { gbrain: USER_URL }, + }); + expect(conflictHash).not.toBe(mod.detectEndpointHash(userOnly, '/w/repo/src')); + }); + + test('user scope still resolves when the cwd has no project-local entry', async () => { + const mod = await cache(); + const cj = writeFixture('claude-user-fallback.json', { + mcpServers: { gbrain: USER_URL }, + projects: { '/other/repo': { mcpServers: { gbrain: PROJ_URL } } }, + }); + const hash = mod.detectEndpointHash(cj, '/w/unrelated'); + expect(hash).toHaveLength(8); + // Matches the user-only hash — the OTHER project's entry is invisible + // outside its own tree. + const userOnly = writeFixture('claude-user-only-2.json', { + mcpServers: { gbrain: USER_URL }, + }); + expect(hash).toBe(mod.detectEndpointHash(userOnly, '/w/unrelated')); + }); +}); diff --git a/test/gbrain-local-status.test.ts b/test/gbrain-local-status.test.ts index 299f32f44..85bebd2a0 100644 --- a/test/gbrain-local-status.test.ts +++ b/test/gbrain-local-status.test.ts @@ -594,13 +594,16 @@ describe("lib/gbrain-local-status — bearer-token thin-client (#2520)", () => { expect(localEngineStatus({ noCache: true })).toBe("thin-client"); }); - it("returns 'thin-client' when config.json is absent and the registration is PROJECT-scoped (#2499)", () => { + it("returns 'thin-client' when config.json is absent and the registration is PROJECT-scoped for THIS cwd (#2499)", () => { + // The project key must be the running process's cwd (or an ancestor): + // per-project scoping (C15) means only registrations visible to this + // cwd count. env = makeEnv({ withGbrain: true, gbrainBehavior: "ok", withConfig: false, claudeJson: { - projects: { "/some/repo": { mcpServers: { "gbrain-remote": REMOTE_GBRAIN } } }, + projects: { [process.cwd()]: { mcpServers: { "gbrain-remote": REMOTE_GBRAIN } } }, }, }); restoreEnv = applyEnv(env); @@ -659,6 +662,117 @@ describe("lib/gbrain-local-status — bearer-token thin-client (#2520)", () => { expect(localEngineStatus({ noCache: true })).toBe("missing-config"); }); + // ── C15: project scan is scoped to the cwd's nearest-ancestor project ── + // Before the fix, hasRemoteOnlyGbrainMcp scanned EVERY project's + // mcpServers, so one project's remote registration reclassified broken + // local engines as thin-client machine-wide. + + it("C15: an OTHER project's remote entry no longer flips thin-client for this cwd (no config)", () => { + env = makeEnv({ + withGbrain: true, + gbrainBehavior: "ok", + withConfig: false, + claudeJson: { + projects: { "/some/other/repo": { mcpServers: { gbrain: REMOTE_GBRAIN } } }, + }, + }); + restoreEnv = applyEnv(env); + expect(localEngineStatus({ noCache: true })).toBe("missing-config"); + }); + + it("C15: an OTHER project's remote entry no longer reclassifies a broken local engine", () => { + env = makeEnv({ + withGbrain: true, + gbrainBehavior: "engine-locked", + withConfig: true, + claudeJson: { + projects: { "/some/other/repo": { mcpServers: { gbrain: REMOTE_GBRAIN } } }, + }, + }); + restoreEnv = applyEnv(env); + expect(localEngineStatus({ noCache: true })).toBe("engine-locked"); + }); + + it("C15: path boundary — a sibling-prefix project key is NOT this cwd's project", () => { + // /path/to/repo2 must never match a scan from /path/to/repo (and vice + // versa) — same boundary rule as the jq resolver and brain-cache. + env = makeEnv({ + withGbrain: true, + gbrainBehavior: "ok", + withConfig: false, + claudeJson: { + projects: { [`${process.cwd()}-sibling`]: { mcpServers: { gbrain: REMOTE_GBRAIN } } }, + }, + }); + restoreEnv = applyEnv(env); + expect(localEngineStatus({ noCache: true })).toBe("missing-config"); + }); + + it("C15: an ANCESTOR project key of this cwd still counts (nearest-ancestor matching)", () => { + env = makeEnv({ + withGbrain: true, + gbrainBehavior: "ok", + withConfig: false, + claudeJson: { + projects: { [dirname(process.cwd())]: { mcpServers: { gbrain: REMOTE_GBRAIN } } }, + }, + }); + restoreEnv = applyEnv(env); + expect(localEngineStatus({ noCache: true })).toBe("thin-client"); + }); + + it("C15: a nearer project WITHOUT gbrain does not shadow an ancestor's registration (jq parity)", () => { + env = makeEnv({ + withGbrain: true, + gbrainBehavior: "ok", + withConfig: false, + claudeJson: { + projects: { + [dirname(process.cwd())]: { mcpServers: { gbrain: REMOTE_GBRAIN } }, + [process.cwd()]: { mcpServers: { "other-server": { type: "http", url: "https://x.example/mcp" } } }, + }, + }, + }); + restoreEnv = applyEnv(env); + expect(localEngineStatus({ noCache: true })).toBe("thin-client"); + }); + + // ── C15: adopted precedence — project-local beats user scope per name ── + // Claude Code's own conflict resolution, verified empirically against + // claude 2.1.233 with a hermetic fake $HOME (`claude mcp get gbrain` + // reports "Scope: Local config" when both scopes define the name). + + it("C15 precedence: THIS project's remote gbrain shadows a user-scope local-stdio gbrain → thin-client", () => { + // Union semantics would see the user-scope stdio entry and keep + // engine-locked; the adopted precedence says this project's queries go + // remote, so thin-client is the truthful classification here. + env = makeEnv({ + withGbrain: true, + gbrainBehavior: "engine-locked", + withConfig: true, + claudeJson: { + mcpServers: { gbrain: LOCAL_GBRAIN }, + projects: { [process.cwd()]: { mcpServers: { gbrain: REMOTE_GBRAIN } } }, + }, + }); + restoreEnv = applyEnv(env); + expect(localEngineStatus({ noCache: true })).toBe("thin-client"); + }); + + it("C15 precedence: THIS project's local-stdio gbrain shadows a user-scope remote gbrain → local statuses keep their meaning", () => { + env = makeEnv({ + withGbrain: true, + gbrainBehavior: "engine-locked", + withConfig: true, + claudeJson: { + mcpServers: { gbrain: REMOTE_GBRAIN }, + projects: { [process.cwd()]: { mcpServers: { gbrain: LOCAL_GBRAIN } } }, + }, + }); + restoreEnv = applyEnv(env); + expect(localEngineStatus({ noCache: true })).toBe("engine-locked"); + }); + it("--is-ok exits 0 on a bearer thin-client fixture (end-to-end gate)", () => { env = makeEnv({ withGbrain: true,