mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-09 06:28:59 +02:00
fix(gbrain): a slow --version probe classifies as timeout, never no-cli (#2716)
resolveGbrainBin's bare catch collapsed 'gbrain missing' and 'gbrain present but the 2s --version budget expired' into the same null — freshClassify then said no-cli, which the --is-ok whitelist from #1964 does NOT forgive, so a bun-shim install on a loaded POSIX box silently lost every brain-aware block. The probe now returns a discriminated result (cached per-process, same lifetime the old null had) using the same killed/SIGTERM/ETIMEDOUT discrimination the sources-list probe below already uses; timeout routes to the forgiven 'timeout' status. GSTACK_GBRAIN_VERSION_PROBE_TIMEOUT_MS test override added (same precedent as the sources-probe override). Receipt: the slow-but-present sibling test fails on a v1.77.0.0 scratch worktree (classifies no-cli there). Fixes #2716 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
84c0eee9f2
commit
262a605794
+40
-11
@@ -259,31 +259,57 @@ function hashPath(p: string): string {
|
||||
* Memoized per-process keyed on PATH so detect's call and the classifier's
|
||||
* call share one fork-exec (~200ms saved per skill preamble).
|
||||
*/
|
||||
const _gbrainBinCache = new Map<string, string | null>();
|
||||
// #2716: the probe must tell "gbrain isn't installed" apart from "gbrain is
|
||||
// installed but the --version round trip blew the budget" (bun-shim installs
|
||||
// on a loaded POSIX box take >2s). Both used to collapse into `null` → the
|
||||
// classifier said `no-cli`, which the `--is-ok` whitelist does NOT forgive —
|
||||
// so a slow box silently lost every brain-aware block. The cache stores the
|
||||
// discriminated result (per-process, same lifetime the old null had).
|
||||
interface GbrainBinProbe {
|
||||
bin: string | null;
|
||||
timedOut: boolean;
|
||||
}
|
||||
const _gbrainBinCache = new Map<string, GbrainBinProbe>();
|
||||
// On Windows the shim is `gbrain.cmd` → `bun run cli.ts`; a cold spawn can
|
||||
// exceed 2s, and a false negative here poisons the 60s status cache with
|
||||
// "no-cli". Give the shim headroom; POSIX keeps the tight timeout.
|
||||
// `GSTACK_GBRAIN_VERSION_PROBE_TIMEOUT_MS` overrides for tests (same
|
||||
// precedent as GSTACK_GBRAIN_PROBE_TIMEOUT_MS on the sources probe).
|
||||
const VERSION_PROBE_TIMEOUT_MS = NEEDS_SHELL_ON_WINDOWS ? 10_000 : 2_000;
|
||||
export function resolveGbrainBin(env?: NodeJS.ProcessEnv): string | null {
|
||||
function versionProbeTimeoutMs(env?: NodeJS.ProcessEnv): number {
|
||||
const raw = (env ?? process.env).GSTACK_GBRAIN_VERSION_PROBE_TIMEOUT_MS;
|
||||
const n = raw ? Number(raw) : NaN;
|
||||
return Number.isFinite(n) && n > 0 ? n : VERSION_PROBE_TIMEOUT_MS;
|
||||
}
|
||||
export function probeGbrainBin(env?: NodeJS.ProcessEnv): GbrainBinProbe {
|
||||
const e = env ?? process.env;
|
||||
const key = e.PATH || "";
|
||||
if (_gbrainBinCache.has(key)) return _gbrainBinCache.get(key)!;
|
||||
let result: string | null = null;
|
||||
let result: GbrainBinProbe = { bin: null, timedOut: false };
|
||||
try {
|
||||
execFileSync("gbrain", ["--version"], {
|
||||
encoding: "utf-8",
|
||||
timeout: VERSION_PROBE_TIMEOUT_MS,
|
||||
timeout: versionProbeTimeoutMs(e),
|
||||
stdio: ["ignore", "ignore", "ignore"],
|
||||
env: e,
|
||||
shell: NEEDS_SHELL_ON_WINDOWS, // #1731: gbrain is a .cmd shim on Windows
|
||||
});
|
||||
result = "gbrain";
|
||||
} catch {
|
||||
result = null;
|
||||
result = { bin: "gbrain", timedOut: false };
|
||||
} catch (err) {
|
||||
// Same discrimination the `sources list` probe below already uses: a
|
||||
// killed/expired spawn is a TIMEOUT (binary present but slow), anything
|
||||
// else (ENOENT, non-zero exit) is genuinely no CLI.
|
||||
const ex = err as { killed?: boolean; signal?: string; code?: unknown };
|
||||
const timedOut =
|
||||
ex?.killed === true || ex?.signal === "SIGTERM" || ex?.code === "ETIMEDOUT";
|
||||
result = { bin: null, timedOut };
|
||||
}
|
||||
_gbrainBinCache.set(key, result);
|
||||
return result;
|
||||
}
|
||||
export function resolveGbrainBin(env?: NodeJS.ProcessEnv): string | null {
|
||||
return probeGbrainBin(env).bin;
|
||||
}
|
||||
|
||||
/** Memoized per-process. */
|
||||
const _gbrainVersionCache = new Map<string, string>();
|
||||
@@ -295,7 +321,7 @@ export function readGbrainVersion(env?: NodeJS.ProcessEnv): string {
|
||||
try {
|
||||
const out = execFileSync("gbrain", ["--version"], {
|
||||
encoding: "utf-8",
|
||||
timeout: VERSION_PROBE_TIMEOUT_MS,
|
||||
timeout: versionProbeTimeoutMs(e),
|
||||
stdio: ["ignore", "pipe", "ignore"],
|
||||
env: e,
|
||||
shell: NEEDS_SHELL_ON_WINDOWS, // #1731: gbrain is a .cmd shim on Windows
|
||||
@@ -385,9 +411,12 @@ function writeCache(status: LocalEngineStatus, key: CacheEntry["key"]): void {
|
||||
* error messages, classifier returns broken-config defensively (codex #8).
|
||||
*/
|
||||
function freshClassify(env?: NodeJS.ProcessEnv): LocalEngineStatus {
|
||||
// 1. CLI on PATH?
|
||||
const gbrainBin = resolveGbrainBin(env);
|
||||
if (!gbrainBin) return "no-cli";
|
||||
// 1. CLI on PATH? A probe that TIMED OUT means the binary exists but the
|
||||
// box is slow (#2716: bun-shim installs) — that's "timeout", which the
|
||||
// `--is-ok` whitelist forgives, never "no-cli", which it doesn't.
|
||||
const probe = probeGbrainBin(env);
|
||||
if (!probe.bin) return probe.timedOut ? "timeout" : "no-cli";
|
||||
const gbrainBin = probe.bin;
|
||||
|
||||
// 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
|
||||
|
||||
@@ -62,7 +62,7 @@ interface FakeEnv {
|
||||
*/
|
||||
function makeEnv(opts: {
|
||||
withGbrain?: boolean;
|
||||
gbrainBehavior?: "ok" | "broken-db" | "broken-config" | "engine-locked" | "engine-locked-v43" | "throws" | "slow" | "thin-refusal";
|
||||
gbrainBehavior?: "ok" | "broken-db" | "broken-config" | "engine-locked" | "engine-locked-v43" | "throws" | "slow" | "slow-version" | "thin-refusal";
|
||||
withConfig?: boolean;
|
||||
/** #2051: config carries gbrain's remote_mcp thin-client marker. */
|
||||
thinClientConfig?: boolean;
|
||||
@@ -116,8 +116,18 @@ function makeEnv(opts: {
|
||||
}
|
||||
|
||||
function makeFakeGbrainScript(
|
||||
behavior: "ok" | "broken-db" | "broken-config" | "engine-locked" | "engine-locked-v43" | "throws" | "slow" | "thin-refusal",
|
||||
behavior: "ok" | "broken-db" | "broken-config" | "engine-locked" | "engine-locked-v43" | "throws" | "slow" | "slow-version" | "thin-refusal",
|
||||
): string {
|
||||
// "slow-version": gbrain IS installed but even `--version` blows the
|
||||
// (test-lowered) budget — the #2716 bun-shim-on-a-loaded-POSIX-box shape.
|
||||
// Must classify as "timeout" (usable, --is-ok forgives), never "no-cli".
|
||||
if (behavior === "slow-version") {
|
||||
return `#!/bin/sh
|
||||
sleep 2
|
||||
echo "gbrain 0.43.0.0"
|
||||
exit 0
|
||||
`;
|
||||
}
|
||||
// "slow": healthy engine on a cold pooler connection (#1964) — sleeps past
|
||||
// the (test-lowered) probe timeout, then would answer fine.
|
||||
if (behavior === "slow") {
|
||||
@@ -222,6 +232,21 @@ describe("lib/gbrain-local-status — status classification", () => {
|
||||
expect(localEngineStatus({ noCache: true })).toBe("no-cli");
|
||||
});
|
||||
|
||||
// #2716: a present-but-slow gbrain (bun-shim install on a loaded POSIX box)
|
||||
// used to collapse into the same `null` as a missing binary — classified
|
||||
// "no-cli", which `--is-ok` does NOT forgive, so every brain-aware block
|
||||
// silently disappeared. Slow-but-present must classify "timeout" (forgiven).
|
||||
it("returns 'timeout' (not 'no-cli') when the --version probe blows its budget", () => {
|
||||
env = makeEnv({ withGbrain: true, gbrainBehavior: "slow-version", withConfig: true });
|
||||
restoreEnv = applyEnv(env);
|
||||
process.env.GSTACK_GBRAIN_VERSION_PROBE_TIMEOUT_MS = "300";
|
||||
try {
|
||||
expect(localEngineStatus({ noCache: true })).toBe("timeout");
|
||||
} finally {
|
||||
delete process.env.GSTACK_GBRAIN_VERSION_PROBE_TIMEOUT_MS;
|
||||
}
|
||||
});
|
||||
|
||||
it("returns 'missing-config' when CLI is present but ~/.gbrain/config.json absent", () => {
|
||||
env = makeEnv({ withGbrain: true, gbrainBehavior: "ok", withConfig: false });
|
||||
restoreEnv = applyEnv(env);
|
||||
|
||||
Reference in New Issue
Block a user