mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-09 22:48:57 +02:00
fix: detect bearer-token thin clients via host MCP registration (#2520)
The #2051 thin-client fix keys detection on the remote_mcp marker in ~/.gbrain/config.json — but that marker is only written by the OAuth path (gbrain init --mcp-only). Bearer-token installs (gbrain connect <url> --token, gbrain's own recommended default for local/personal use) never touch config.json, so they fell through to the local probe, failed against the dead-or-absent local engine, and landed on missing-config / broken-db / broken-config / engine-locked — silently suppressing brain blocks for a fully-working remote brain. New evidence source: hasRemoteOnlyGbrainMcp() reads ~/.claude.json MCP registrations (user scope AND project scope) with the same classification rules as gstack-gbrain-detect's tier-3 fallback. File-read only — no subprocess, no network (a classifier network probe is the #1964 pathology). Wired at two sites in freshClassify: - missing-config branch: a bearer thin client may never have run a local init; if the host's only gbrain registration is remote-HTTP, that registration IS the brain → thin-client. - post-probe-failure demotion: broken-db / broken-config / engine-locked reclassify to thin-client when the only gbrain registration is remote. A local-stdio sibling registration blocks the demotion (federation guard: a user running a local engine plus a remote team brain keeps precise local statuses). "timeout" is excluded — already usable, and may be a genuinely healthy slow local engine. 7 new unit tests in test/gbrain-local-status.test.ts: user-scope, project- scope, engine-locked/broken-db demotion, federation guard, no-registration discriminator, end-to-end --is-ok gate (35 pass total in the file). Root-cause analysis by @d-danielsun in #2520. Fixes #2520 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
co-authored by
Claude Fable 5
Claude Fable 5
parent
f9f3c9801a
commit
ce4a7bbb7e
+121
-28
@@ -24,12 +24,14 @@
|
||||
* Timeout → probe exceeded GSTACK_GBRAIN_PROBE_TIMEOUT_MS (default 15s) with no
|
||||
* recognized error — engine is likely healthy but slow (e.g. a cold
|
||||
* pooler connection, #1964). Consumers treat this as usable.
|
||||
* Thin-client → config carries gbrain's remote_mcp marker (#2051): NO local
|
||||
* engine by design; queries go to a remote-HTTP MCP brain. Usable
|
||||
* for brain-aware prose gates; sync stages that need a LOCAL engine
|
||||
* (code/memory/dream) skip. Remote reachability is verified at USE
|
||||
* time (gbrain calls degrade gracefully), never by a classifier
|
||||
* network probe — that's the #1964 pathology.
|
||||
* Thin-client → config carries gbrain's remote_mcp marker (#2051), OR the
|
||||
* agent host's MCP registration is remote-HTTP-only (#2520 — bearer
|
||||
* installs via `gbrain connect --token` never get the marker): NO
|
||||
* local engine by design; queries go to a remote-HTTP MCP brain.
|
||||
* Usable for brain-aware prose gates; sync stages that need a LOCAL
|
||||
* engine (code/memory/dream) skip. Remote reachability is verified
|
||||
* at USE time (gbrain calls degrade gracefully), never by a
|
||||
* classifier network probe — that's the #1964 pathology.
|
||||
* Ok → DB reachable, sources list returned valid JSON.
|
||||
*/
|
||||
|
||||
@@ -127,6 +129,76 @@ function gbrainConfigPath(env?: NodeJS.ProcessEnv): string {
|
||||
return join(gbrainHome, "config.json");
|
||||
}
|
||||
|
||||
/**
|
||||
* Bearer-token thin-client evidence (#2520). `gbrain connect <url> --token`
|
||||
* registers a remote-HTTP MCP server with the agent host but never writes
|
||||
* gbrain's remote_mcp marker into config.json — that marker is OAuth-only,
|
||||
* written by `gbrain init --mcp-only`. So the config-file marker check misses
|
||||
* bearer installs entirely: they fall through to the local probe, which fails
|
||||
* against the dead-or-absent local engine and lands on missing-config /
|
||||
* 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).
|
||||
* 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.
|
||||
*/
|
||||
export function hasRemoteOnlyGbrainMcp(env?: NodeJS.ProcessEnv): boolean {
|
||||
interface McpEntry {
|
||||
type?: string;
|
||||
transport?: string;
|
||||
command?: string;
|
||||
url?: string;
|
||||
}
|
||||
let cj: unknown;
|
||||
try {
|
||||
cj = JSON.parse(readFileSync(join(userHome(env), ".claude.json"), "utf-8"));
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
// Same classification rules as gstack-gbrain-detect's detectMcpMode tier 3,
|
||||
// including the #2051 name generalization (gbrain, gbrain-remote, gbrain_work).
|
||||
const classify = (entry: McpEntry): "remote" | "local" | null => {
|
||||
const mtype = entry.type || entry.transport || "";
|
||||
if (mtype === "url" || mtype === "http" || mtype === "sse") return "remote";
|
||||
if (mtype === "stdio") return "local";
|
||||
if (entry.url) return "remote";
|
||||
if (entry.command) return "local";
|
||||
return null;
|
||||
};
|
||||
let sawRemote = false;
|
||||
let sawLocal = false;
|
||||
const scan = (servers: unknown): void => {
|
||||
if (!servers || typeof servers !== "object") return;
|
||||
for (const [name, entry] of Object.entries(servers as Record<string, McpEntry>)) {
|
||||
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;
|
||||
}
|
||||
};
|
||||
const root = cj as {
|
||||
mcpServers?: unknown;
|
||||
projects?: Record<string, { mcpServers?: unknown }>;
|
||||
} | null;
|
||||
scan(root?.mcpServers);
|
||||
if (root?.projects && typeof root.projects === "object") {
|
||||
for (const proj of Object.values(root.projects)) {
|
||||
if (proj && typeof proj === "object") scan(proj.mcpServers);
|
||||
}
|
||||
}
|
||||
return sawRemote && !sawLocal;
|
||||
}
|
||||
|
||||
function configuredEngine(env?: NodeJS.ProcessEnv): "pglite" | "postgres" | null {
|
||||
try {
|
||||
const parsed = JSON.parse(readFileSync(gbrainConfigPath(env), "utf-8")) as { engine?: string };
|
||||
@@ -271,8 +343,12 @@ function freshClassify(env?: NodeJS.ProcessEnv): LocalEngineStatus {
|
||||
const gbrainBin = resolveGbrainBin(env);
|
||||
if (!gbrainBin) return "no-cli";
|
||||
|
||||
// 2. Config file present?
|
||||
if (!existsSync(gbrainConfigPath(env))) return "missing-config";
|
||||
// 2. Config file present? A bearer thin client (#2520) may never have run
|
||||
// a local init, so config.json can be absent while the remote-HTTP MCP
|
||||
// registration IS the user's brain.
|
||||
if (!existsSync(gbrainConfigPath(env))) {
|
||||
return hasRemoteOnlyGbrainMcp(env) ? "thin-client" : "missing-config";
|
||||
}
|
||||
|
||||
// 2.5 Thin client? gbrain's own marker (mirrors gbrain isThinClient():
|
||||
// truthy remote_mcp in config). A thin client has NO local engine — gbrain
|
||||
@@ -329,28 +405,45 @@ function freshClassify(env?: NodeJS.ProcessEnv): LocalEngineStatus {
|
||||
// couldn't read — gbrain's dispatch guard says e.g. "`gbrain sources` is
|
||||
// not routable ... (thin-client of <url>)"), then the more specific
|
||||
// DB-unreachable signal.
|
||||
if (/thin[- ]client/i.test(stderr)) return "thin-client";
|
||||
if (stderr.includes("Cannot connect to database")) return "broken-db";
|
||||
if (stderr.includes("config.json")) return "broken-config";
|
||||
const raw = ((): LocalEngineStatus => {
|
||||
if (/thin[- ]client/i.test(stderr)) return "thin-client";
|
||||
if (stderr.includes("Cannot connect to database")) return "broken-db";
|
||||
if (stderr.includes("config.json")) return "broken-config";
|
||||
|
||||
// PGLite is single-process. A long-lived `gbrain serve` can own the
|
||||
// embedded database, causing the CLI to finish with its own exit 124 and
|
||||
// "connect timed out" message. This is neither our watchdog timeout nor
|
||||
// evidence that the valid config is malformed (#2194).
|
||||
if (stderr.includes("connect timed out") || e.status === 124) {
|
||||
return configuredEngine(env) === "pglite" ? "engine-locked" : "broken-db";
|
||||
// PGLite is single-process. A long-lived `gbrain serve` can own the
|
||||
// embedded database, causing the CLI to finish with its own exit 124 and
|
||||
// "connect timed out" message. This is neither our watchdog timeout nor
|
||||
// evidence that the valid config is malformed (#2194).
|
||||
if (stderr.includes("connect timed out") || e.status === 124) {
|
||||
return configuredEngine(env) === "pglite" ? "engine-locked" : "broken-db";
|
||||
}
|
||||
|
||||
// Probe killed by the timeout with no recognized error: the engine is
|
||||
// most likely healthy but slow (cold pooler connections measured at
|
||||
// 6.9-10.7s in #1964). Don't tell the user their config is malformed.
|
||||
if (e.killed === true || e.signal === "SIGTERM" || e.code === "ETIMEDOUT") {
|
||||
return "timeout";
|
||||
}
|
||||
|
||||
// Defensive default per codex #8: unrecognized failures classify as
|
||||
// broken-config so the user sees the raw stderr surfaced upstream.
|
||||
return "broken-config";
|
||||
})();
|
||||
|
||||
// #2520 bearer-token fallback: the local probe failed, but the user's
|
||||
// only gbrain MCP registration is remote-HTTP — the dead-or-locked local
|
||||
// engine is not their brain (typical shape: a leftover local config plus
|
||||
// `gbrain connect --token`). Reclassify as thin-client so brain blocks
|
||||
// stay rendered and sync's local stages skip with the accurate "nothing
|
||||
// to do locally" message. "timeout" is deliberately excluded: it already
|
||||
// counts as usable and may be a genuinely healthy slow LOCAL engine.
|
||||
if (
|
||||
(raw === "broken-db" || raw === "broken-config" || raw === "engine-locked") &&
|
||||
hasRemoteOnlyGbrainMcp(env)
|
||||
) {
|
||||
return "thin-client";
|
||||
}
|
||||
|
||||
// Probe killed by the timeout with no recognized error: the engine is
|
||||
// most likely healthy but slow (cold pooler connections measured at
|
||||
// 6.9-10.7s in #1964). Don't tell the user their config is malformed.
|
||||
if (e.killed === true || e.signal === "SIGTERM" || e.code === "ETIMEDOUT") {
|
||||
return "timeout";
|
||||
}
|
||||
|
||||
// Defensive default per codex #8: unrecognized failures classify as
|
||||
// broken-config so the user sees the raw stderr surfaced upstream.
|
||||
return "broken-config";
|
||||
return raw;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -66,6 +66,8 @@ function makeEnv(opts: {
|
||||
withConfig?: boolean;
|
||||
/** #2051: config carries gbrain's remote_mcp thin-client marker. */
|
||||
thinClientConfig?: boolean;
|
||||
/** #2520: content for ~/.claude.json (host MCP registrations). */
|
||||
claudeJson?: object;
|
||||
}): FakeEnv {
|
||||
const tmp = mkdtempSync(join(tmpdir(), "gbrain-local-status-test-"));
|
||||
const bindir = join(tmp, "bin");
|
||||
@@ -99,6 +101,10 @@ function makeEnv(opts: {
|
||||
chmodSync(gbrainPath, 0o755);
|
||||
}
|
||||
|
||||
if (opts.claudeJson) {
|
||||
writeFileSync(join(home, ".claude.json"), JSON.stringify(opts.claudeJson));
|
||||
}
|
||||
|
||||
return {
|
||||
tmp,
|
||||
bindir,
|
||||
@@ -526,3 +532,125 @@ describe("lib/gbrain-local-status — thin-client (#2051)", () => {
|
||||
expect(r.status).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// #2520: bearer-token thin clients (`gbrain connect --token`) — no remote_mcp
|
||||
// marker in config.json; the evidence is the host's remote-HTTP MCP
|
||||
// registration in ~/.claude.json.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe("lib/gbrain-local-status — bearer-token thin-client (#2520)", () => {
|
||||
let env: FakeEnv | null = null;
|
||||
let restoreEnv: (() => void) | null = null;
|
||||
|
||||
afterEach(() => {
|
||||
if (restoreEnv) restoreEnv();
|
||||
if (env) env.cleanup();
|
||||
env = null;
|
||||
restoreEnv = null;
|
||||
});
|
||||
|
||||
const REMOTE_GBRAIN = {
|
||||
type: "http",
|
||||
url: "https://brain.example.com/mcp",
|
||||
headers: { Authorization: "Bearer test-token" },
|
||||
};
|
||||
const LOCAL_GBRAIN = { type: "stdio", command: "gbrain", args: ["serve"] };
|
||||
|
||||
it("returns 'thin-client' when config.json is absent but a remote-HTTP gbrain MCP is registered (user scope)", () => {
|
||||
env = makeEnv({
|
||||
withGbrain: true,
|
||||
gbrainBehavior: "ok",
|
||||
withConfig: false,
|
||||
claudeJson: { mcpServers: { gbrain: REMOTE_GBRAIN } },
|
||||
});
|
||||
restoreEnv = applyEnv(env);
|
||||
expect(localEngineStatus({ noCache: true })).toBe("thin-client");
|
||||
});
|
||||
|
||||
it("returns 'thin-client' when config.json is absent and the registration is PROJECT-scoped (#2499)", () => {
|
||||
env = makeEnv({
|
||||
withGbrain: true,
|
||||
gbrainBehavior: "ok",
|
||||
withConfig: false,
|
||||
claudeJson: {
|
||||
projects: { "/some/repo": { mcpServers: { "gbrain-remote": REMOTE_GBRAIN } } },
|
||||
},
|
||||
});
|
||||
restoreEnv = applyEnv(env);
|
||||
expect(localEngineStatus({ noCache: true })).toBe("thin-client");
|
||||
});
|
||||
|
||||
it("reclassifies a failed local probe (engine-locked) as 'thin-client' when the only gbrain MCP is remote", () => {
|
||||
// The reporter's exact shape: leftover local pglite config, dead/absent
|
||||
// local engine (probe exits 124 "connect timed out"), brain fully working
|
||||
// over remote-HTTP MCP with a bearer token.
|
||||
env = makeEnv({
|
||||
withGbrain: true,
|
||||
gbrainBehavior: "engine-locked",
|
||||
withConfig: true,
|
||||
claudeJson: { mcpServers: { gbrain: REMOTE_GBRAIN } },
|
||||
});
|
||||
restoreEnv = applyEnv(env);
|
||||
expect(localEngineStatus({ noCache: true })).toBe("thin-client");
|
||||
});
|
||||
|
||||
it("reclassifies broken-db as 'thin-client' when the only gbrain MCP is remote", () => {
|
||||
env = makeEnv({
|
||||
withGbrain: true,
|
||||
gbrainBehavior: "broken-db",
|
||||
withConfig: true,
|
||||
claudeJson: { mcpServers: { gbrain: REMOTE_GBRAIN } },
|
||||
});
|
||||
restoreEnv = applyEnv(env);
|
||||
expect(localEngineStatus({ noCache: true })).toBe("thin-client");
|
||||
});
|
||||
|
||||
it("preserves 'engine-locked' when a local-stdio gbrain MCP is ALSO registered (federation guard)", () => {
|
||||
// A local-stdio registration means the user runs a local engine —
|
||||
// local-engine statuses must keep their precise meaning, even if a
|
||||
// second (e.g. team) brain is registered remote-HTTP.
|
||||
env = makeEnv({
|
||||
withGbrain: true,
|
||||
gbrainBehavior: "engine-locked",
|
||||
withConfig: true,
|
||||
claudeJson: {
|
||||
mcpServers: { gbrain: LOCAL_GBRAIN, "gbrain-work": REMOTE_GBRAIN },
|
||||
},
|
||||
});
|
||||
restoreEnv = applyEnv(env);
|
||||
expect(localEngineStatus({ noCache: true })).toBe("engine-locked");
|
||||
});
|
||||
|
||||
it("still returns 'missing-config' when no gbrain MCP registration exists (discriminator)", () => {
|
||||
env = makeEnv({
|
||||
withGbrain: true,
|
||||
gbrainBehavior: "ok",
|
||||
withConfig: false,
|
||||
claudeJson: { mcpServers: { "other-server": { type: "http", url: "https://x.example/mcp" } } },
|
||||
});
|
||||
restoreEnv = applyEnv(env);
|
||||
expect(localEngineStatus({ noCache: true })).toBe("missing-config");
|
||||
});
|
||||
|
||||
it("--is-ok exits 0 on a bearer thin-client fixture (end-to-end gate)", () => {
|
||||
env = makeEnv({
|
||||
withGbrain: true,
|
||||
gbrainBehavior: "engine-locked",
|
||||
withConfig: true,
|
||||
claudeJson: { mcpServers: { gbrain: REMOTE_GBRAIN } },
|
||||
});
|
||||
const detectBin = join(import.meta.dir, "..", "bin", "gstack-gbrain-detect");
|
||||
const bunDir = dirname(process.execPath);
|
||||
const r = spawnSync(detectBin, ["--is-ok"], {
|
||||
encoding: "utf-8",
|
||||
env: {
|
||||
HOME: env.home,
|
||||
PATH: `${env.bindir}:${bunDir}:/usr/bin:/bin`,
|
||||
GSTACK_HOME: env.gstackHome,
|
||||
GSTACK_DETECT_NO_CACHE: "1",
|
||||
},
|
||||
});
|
||||
expect(r.status).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user