From 4057ddbf3d2028dd046631165029352ca419f25b Mon Sep 17 00:00:00 2001 From: Jayesh Betala Date: Tue, 2 Jun 2026 01:37:18 +0530 Subject: [PATCH] fix(gbrain): treat healthy thin-client as ok in local-status classifier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `freshClassify()` in lib/gbrain-local-status.ts probes the engine with `gbrain sources list --json`, a local-DB-only command. In thin-client mode (remote MCP, no local DB) gbrain refuses it with a "not routable" error that matches neither the broken-db nor broken-config stderr pattern, so the defensive default mislabels a perfectly healthy thin-client as `broken-config`. `gen-skill-docs --respect-detection` then suppresses every brain block, so brain-aware planning silently never engages on thin-client brains (advertised as supported since v1.52). Fix: on the "not routable" failure path only, confirm health via the mode-aware `gbrain doctor --json --fast` (already used elsewhere for detection) and return `ok` when doctor reports `mode: thin-client` with `ok`/`warnings` status. The cheap ~80ms sources-list probe still owns the healthy local-DB hot path — doctor is only consulted when the probe is refused. Parse failures or an unhealthy doctor status fall through to the existing defensive classification, so an unconfirmed brain is never upgraded. Tests: two cases in test/gbrain-local-status.test.ts — a healthy thin-client (not-routable probe + doctor ok) now classifies `ok`, and a not-routable brain whose doctor reports an unhealthy status stays `broken-config`. Fixes #1792 Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/gbrain-local-status.ts | 50 +++++++++++++++++++++++++++ test/gbrain-local-status.test.ts | 59 ++++++++++++++++++++++++++++---- 2 files changed, 103 insertions(+), 6 deletions(-) diff --git a/lib/gbrain-local-status.ts b/lib/gbrain-local-status.ts index 9e26d5ba4..bca4dad17 100644 --- a/lib/gbrain-local-status.ts +++ b/lib/gbrain-local-status.ts @@ -243,6 +243,50 @@ function writeCache(status: LocalEngineStatus, key: CacheEntry["key"]): void { } } +/** + * Confirm a healthy thin-client brain via `gbrain doctor --json --fast`. + * + * Thin-client mode (remote MCP, no local DB) intentionally refuses the + * `gbrain sources list` probe — `sources` is a local-DB-only command. The probe + * therefore throws with a "not routable" stderr that matches neither the + * broken-db nor broken-config pattern, so freshClassify would mislabel a + * perfectly healthy thin-client as broken-config and suppress every brain block + * (#1792). Doctor is the mode-aware signal: it reports `mode` + `status` for + * thin-clients. We only call it on the not-routable failure path, so the cheap + * ~80ms sources-list probe still owns the healthy local-DB hot path. + * + * Returns true only when doctor confirms thin-client mode AND a healthy status + * (`ok`/`warnings`). Any parse failure or unhealthy status falls through to the + * caller's defensive classification — we never upgrade an unconfirmed brain. + */ +function isHealthyThinClient(env?: NodeJS.ProcessEnv): boolean { + let parsed: Record | null = null; + try { + const out = execFileSync("gbrain", ["doctor", "--json", "--fast"], { + encoding: "utf-8", + timeout: probeTimeoutMs(env), + stdio: ["ignore", "pipe", "ignore"], + env: buildGbrainEnv({ baseEnv: env ?? process.env }), + shell: NEEDS_SHELL_ON_WINDOWS, // #1731: gbrain is a .cmd shim on Windows + }); + parsed = JSON.parse(out); + } catch (err) { + // doctor exits 1 whenever health_score < 100 (normal on warnings), but + // still prints the JSON to stdout. Recover it before giving up. See #1415. + try { + const stdout = (err as { stdout?: Buffer | string })?.stdout ?? ""; + const stdoutStr = typeof stdout === "string" ? stdout : stdout.toString("utf-8"); + if (stdoutStr) parsed = JSON.parse(stdoutStr); + } catch { + return false; + } + } + if (!parsed) return false; + const mode = parsed.mode; + const status = parsed.status; + return mode === "thin-client" && (status === "ok" || status === "warnings"); +} + /** * Probe via `gbrain sources list --json`. Classify the outcome. * @@ -287,6 +331,12 @@ function freshClassify(env?: NodeJS.ProcessEnv): LocalEngineStatus { // ENOENT can happen if gbrain disappeared between resolveGbrainBin and now. if (e.code === "ENOENT") return "no-cli"; + // Thin-client mode (remote MCP, no local DB) refuses `sources list` with a + // "not routable" error — `sources` is local-DB-only. This is NOT a broken + // brain: confirm health via the mode-aware doctor before classifying, so a + // healthy thin-client reports "ok" and keeps its brain blocks (#1792). + if (stderr.includes("not routable") && isHealthyThinClient(env)) return "ok"; + // Pattern match against gbrain's known error strings. Order matters: // "Cannot connect to database" is the more specific DB-unreachable signal. if (stderr.includes("Cannot connect to database")) return "broken-db"; diff --git a/test/gbrain-local-status.test.ts b/test/gbrain-local-status.test.ts index 703adfcad..398282f41 100644 --- a/test/gbrain-local-status.test.ts +++ b/test/gbrain-local-status.test.ts @@ -59,9 +59,18 @@ interface FakeEnv { * The classifier reads HOME via os.homedir() which reads process.env.HOME, so * we mutate process.env ambiently in each test (restored in afterEach). */ +type GbrainBehavior = + | "ok" + | "broken-db" + | "broken-config" + | "throws" + | "slow" + | "thin-client-ok" + | "thin-client-unhealthy"; + function makeEnv(opts: { withGbrain?: boolean; - gbrainBehavior?: "ok" | "broken-db" | "broken-config" | "throws" | "slow"; + gbrainBehavior?: GbrainBehavior; withConfig?: boolean; }): FakeEnv { const tmp = mkdtempSync(join(tmpdir(), "gbrain-local-status-test-")); @@ -101,9 +110,7 @@ function makeEnv(opts: { }; } -function makeFakeGbrainScript( - behavior: "ok" | "broken-db" | "broken-config" | "throws" | "slow", -): string { +function makeFakeGbrainScript(behavior: GbrainBehavior): string { // "slow": healthy engine on a cold pooler connection (#1964) — sleeps past // the (test-lowered) probe timeout, then would answer fine. if (behavior === "slow") { @@ -120,6 +127,8 @@ fi exit 0 `; } + const isThinClient = + behavior === "thin-client-ok" || behavior === "thin-client-unhealthy"; const stderrLine = behavior === "broken-db" ? 'echo "Cannot connect to database: . Fix: Check your connection URL in ~/.gbrain/config.json" >&2' @@ -127,13 +136,27 @@ exit 0 ? 'echo "Error: malformed config.json at ~/.gbrain/config.json" >&2' : behavior === "throws" ? 'echo "unexpected gbrain failure" >&2' - : ""; + : isThinClient + ? 'echo "\\`gbrain sources\\` is not routable. sources commands manage local DB + config rows. Per-subcommand thin-client routing lands in v0.31.x." >&2' + : ""; const exitCode = behavior === "ok" ? 0 : 1; + // Thin-client doctor: mode-aware health signal the classifier falls back to + // when `sources list` is refused. doctor exits 1 when health_score < 100 but + // still prints JSON to stdout (matches real gbrain + #1415 recovery path). + const doctorStatus = behavior === "thin-client-unhealthy" ? "error" : "ok"; + const doctorExit = behavior === "thin-client-unhealthy" ? "1" : "0"; + const doctorBlock = isThinClient + ? `if [ "$1 $2 $3" = "doctor --json --fast" ]; then + echo '{"mode":"thin-client","status":"${doctorStatus}"}' + exit ${doctorExit} +fi` + : ""; return `#!/bin/sh if [ "$1" = "--version" ]; then - echo "gbrain 0.33.1.0" + echo "gbrain 0.41.28.0" exit 0 fi +${doctorBlock} if [ "$1 $2" = "sources list" ]; then if [ ${exitCode} -eq 0 ]; then echo '{"sources":[]}' @@ -231,6 +254,30 @@ describe("lib/gbrain-local-status — status classification", () => { expect(localEngineStatus({ noCache: true })).toBe("ok"); }); + it("returns 'ok' for a healthy thin-client whose 'sources list' is not routable (#1792)", () => { + // Thin-client refuses the local-DB-only sources probe; doctor confirms + // mode=thin-client + status=ok. Must NOT be mislabeled broken-config. + env = makeEnv({ + withGbrain: true, + gbrainBehavior: "thin-client-ok", + withConfig: true, + }); + restoreEnv = applyEnv(env); + expect(localEngineStatus({ noCache: true })).toBe("ok"); + }); + + it("stays defensive (broken-config) when a not-routable brain is unhealthy per doctor (#1792)", () => { + // "not routable" alone must not upgrade an unconfirmed brain: doctor reports + // status=error, so we fall through to the defensive default. + env = makeEnv({ + withGbrain: true, + gbrainBehavior: "thin-client-unhealthy", + withConfig: true, + }); + restoreEnv = applyEnv(env); + expect(localEngineStatus({ noCache: true })).toBe("broken-config"); + }); + it("returns 'timeout' (not broken-config) when the probe exceeds the deadline (#1964)", () => { env = makeEnv({ withGbrain: true, gbrainBehavior: "slow", withConfig: true }); restoreEnv = applyEnv(env);