From 77fbdea01eb7219ebde14e0f5cf6aa0a19f8ff9f 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 | 58 ++++++++++++++++++++++++++++---- 2 files changed, 102 insertions(+), 6 deletions(-) diff --git a/lib/gbrain-local-status.ts b/lib/gbrain-local-status.ts index f6332cf6b..873f6305c 100644 --- a/lib/gbrain-local-status.ts +++ b/lib/gbrain-local-status.ts @@ -213,6 +213,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: PROBE_TIMEOUT_MS, + 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. * @@ -253,6 +297,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 90744bb2c..6f4bf428a 100644 --- a/test/gbrain-local-status.test.ts +++ b/test/gbrain-local-status.test.ts @@ -53,9 +53,17 @@ 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" + | "thin-client-ok" + | "thin-client-unhealthy"; + function makeEnv(opts: { withGbrain?: boolean; - gbrainBehavior?: "ok" | "broken-db" | "broken-config" | "throws"; + gbrainBehavior?: GbrainBehavior; withConfig?: boolean; }): FakeEnv { const tmp = mkdtempSync(join(tmpdir(), "gbrain-local-status-test-")); @@ -95,9 +103,9 @@ function makeEnv(opts: { }; } -function makeFakeGbrainScript( - behavior: "ok" | "broken-db" | "broken-config" | "throws", -): string { +function makeFakeGbrainScript(behavior: GbrainBehavior): string { + 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' @@ -105,13 +113,27 @@ function makeFakeGbrainScript( ? '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":[]}' @@ -206,6 +228,30 @@ describe("lib/gbrain-local-status — five status cases", () => { restoreEnv = applyEnv(env); 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"); + }); }); describe("lib/gbrain-local-status — cache behavior", () => {