From 61e8df677cbfaf54d6fb5bab5ded255dc6647556 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 1 Sep 2026 16:08:05 +0000 Subject: [PATCH] refactor(lib): shared isExecTimeout helper; export GbrainBinProbe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The killed/SIGTERM/ETIMEDOUT discrimination was hand-rolled at three sites (gbrain version probe, engine classifier, gitleaks probe) and free to drift; it now lives once in lib/gbrain-exec.ts. GbrainBinProbe is exported (it's the return type of exported probeGbrainBin) and the cache carries a rationale comment: caching a timeout for process lifetime is deliberate — the memo dedupes the ~3 probes of one short-lived preamble process. Co-Authored-By: Claude Fable 5 --- lib/gbrain-exec.ts | 13 +++++++++++++ lib/gbrain-local-status.ts | 14 +++++++------- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/lib/gbrain-exec.ts b/lib/gbrain-exec.ts index a7d32dea7..71d502880 100644 --- a/lib/gbrain-exec.ts +++ b/lib/gbrain-exec.ts @@ -149,6 +149,19 @@ export function buildGbrainEnv(opts: BuildGbrainEnvOptions = {}): NodeJS.Process */ export const NEEDS_SHELL_ON_WINDOWS = process.platform === "win32"; +/** + * Did an execFileSync/spawnSync failure come from the TIMEOUT budget (child + * killed) rather than the child itself failing? execFileSync kills the child + * when the budget runs out: `killed` with a SIGTERM on POSIX, ETIMEDOUT on + * runtimes that surface errno instead. Shared by the gbrain version probe, + * the engine classifier, and the gitleaks probe so the three sites can't + * drift on which shapes count as "slow, not broken". + */ +export function isExecTimeout(err: unknown): boolean { + const e = err as { killed?: boolean; signal?: string; code?: unknown }; + return e?.killed === true || e?.signal === "SIGTERM" || e?.code === "ETIMEDOUT"; +} + /** Where Git for Windows puts bash, most-specific first. */ const WINDOWS_BASH_CANDIDATES = [ "C:\\Program Files\\Git\\bin\\bash.exe", diff --git a/lib/gbrain-local-status.ts b/lib/gbrain-local-status.ts index 24b368c22..f75b167b3 100644 --- a/lib/gbrain-local-status.ts +++ b/lib/gbrain-local-status.ts @@ -48,7 +48,7 @@ import { import { atomicWriteSync } from "./fs-atomic"; import { homedir } from "os"; import { dirname, join } from "path"; -import { buildGbrainEnv, gbrainConfigDir, NEEDS_SHELL_ON_WINDOWS } from "./gbrain-exec"; +import { buildGbrainEnv, gbrainConfigDir, isExecTimeout, NEEDS_SHELL_ON_WINDOWS } from "./gbrain-exec"; export type LocalEngineStatus = | "ok" @@ -265,10 +265,13 @@ function hashPath(p: string): string { // 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 { +export interface GbrainBinProbe { bin: string | null; timedOut: boolean; } +// Caching a TIMEOUT for process lifetime is deliberate: the memo exists to +// dedupe the ~3 probes a single skill preamble fires, and preamble processes +// are short-lived — a retry next invocation gets a fresh probe anyway. const _gbrainBinCache = new Map(); // 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 @@ -299,10 +302,7 @@ export function probeGbrainBin(env?: NodeJS.ProcessEnv): GbrainBinProbe { // 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 }; + result = { bin: null, timedOut: isExecTimeout(err) }; } _gbrainBinCache.set(key, result); return result; @@ -507,7 +507,7 @@ function freshClassify(env?: NodeJS.ProcessEnv): LocalEngineStatus { // 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") { + if (isExecTimeout(e)) { return "timeout"; }