mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-09 22:48:57 +02:00
fix(gbrain): classify probe timeout as its own status; sync proceeds instead of skipping (#1964)
The 5s engine probe misclassified healthy-but-slow engines (cold Supabase pooler connections measured at 6.9-10.7s) as broken-config, so /sync-gbrain silently skipped code+memory and told the user their config was malformed. - New "timeout" status: probe killed at the deadline with no recognized stderr pattern. Default deadline is now 15s, overridable via GSTACK_GBRAIN_PROBE_TIMEOUT_MS (tests set 300ms against a fake that sleeps 2s). - Sync stages PROCEED on timeout with a stderr warning naming the env knob; a genuinely-dead engine surfaces its real error at the first operation instead of a false config diagnosis. - Consistency everywhere "ok" gated behavior: gstack-gbrain-detect --is-ok exits 0 on timeout, and gen-skill-docs' detection gate accepts it, so a slow engine no longer silently suppresses brain-aware features. - Status cache: key now includes the effective probe timeout (raising it invalidates a cached timeout) and GBRAIN_HOME; config detection honors GBRAIN_HOME so relocated-home users stop being misclassified as missing-config. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
8e4662e930
commit
da06ef1504
@@ -6,15 +6,17 @@
|
||||
* on PATH that emits canned exit codes + stderr matching the patterns the
|
||||
* classifier looks for.
|
||||
*
|
||||
* Five status cases:
|
||||
* Six status cases:
|
||||
* 1. no-cli — gbrain absent from PATH
|
||||
* 2. missing-config — gbrain present, ~/.gbrain/config.json absent
|
||||
* 2. missing-config — gbrain present, config.json absent (honors GBRAIN_HOME)
|
||||
* 3. broken-config — gbrain present, config exists, stderr contains "config.json"
|
||||
* 4. broken-db — gbrain present, config exists, stderr contains "Cannot connect to database"
|
||||
* 5. ok — gbrain present, config exists, sources list returns valid JSON
|
||||
* 5. timeout — probe exceeds GSTACK_GBRAIN_PROBE_TIMEOUT_MS with no recognized error (#1964)
|
||||
* 6. ok — gbrain present, config exists, sources list returns valid JSON
|
||||
*
|
||||
* Plus cache behavior: hit, TTL expiry, invariant invalidation (HOME change),
|
||||
* --no-cache bypass.
|
||||
* Plus cache behavior: hit, TTL expiry, invariant invalidation (HOME change,
|
||||
* probe-timeout change), --no-cache bypass. Timeout tests keep runtime sane by
|
||||
* setting GSTACK_GBRAIN_PROBE_TIMEOUT_MS=300 against a fake gbrain that sleeps 2s.
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach } from "bun:test";
|
||||
@@ -31,10 +33,14 @@ import {
|
||||
import { tmpdir } from "os";
|
||||
import { join } from "path";
|
||||
|
||||
import { spawnSync } from "child_process";
|
||||
|
||||
import {
|
||||
localEngineStatus,
|
||||
cacheFilePath,
|
||||
probeTimeoutMs,
|
||||
CACHE_TTL_MS,
|
||||
DEFAULT_PROBE_TIMEOUT_MS,
|
||||
type LocalEngineStatus,
|
||||
} from "../lib/gbrain-local-status";
|
||||
|
||||
@@ -55,7 +61,7 @@ interface FakeEnv {
|
||||
*/
|
||||
function makeEnv(opts: {
|
||||
withGbrain?: boolean;
|
||||
gbrainBehavior?: "ok" | "broken-db" | "broken-config" | "throws";
|
||||
gbrainBehavior?: "ok" | "broken-db" | "broken-config" | "throws" | "slow";
|
||||
withConfig?: boolean;
|
||||
}): FakeEnv {
|
||||
const tmp = mkdtempSync(join(tmpdir(), "gbrain-local-status-test-"));
|
||||
@@ -96,8 +102,24 @@ function makeEnv(opts: {
|
||||
}
|
||||
|
||||
function makeFakeGbrainScript(
|
||||
behavior: "ok" | "broken-db" | "broken-config" | "throws",
|
||||
behavior: "ok" | "broken-db" | "broken-config" | "throws" | "slow",
|
||||
): string {
|
||||
// "slow": healthy engine on a cold pooler connection (#1964) — sleeps past
|
||||
// the (test-lowered) probe timeout, then would answer fine.
|
||||
if (behavior === "slow") {
|
||||
return `#!/bin/sh
|
||||
if [ "$1" = "--version" ]; then
|
||||
echo "gbrain 0.33.1.0"
|
||||
exit 0
|
||||
fi
|
||||
if [ "$1 $2" = "sources list" ]; then
|
||||
sleep 2
|
||||
echo '{"sources":[]}'
|
||||
exit 0
|
||||
fi
|
||||
exit 0
|
||||
`;
|
||||
}
|
||||
const stderrLine =
|
||||
behavior === "broken-db"
|
||||
? 'echo "Cannot connect to database: . Fix: Check your connection URL in ~/.gbrain/config.json" >&2'
|
||||
@@ -136,17 +158,19 @@ function applyEnv(env: FakeEnv): () => void {
|
||||
HOME: process.env.HOME,
|
||||
PATH: process.env.PATH,
|
||||
GSTACK_HOME: process.env.GSTACK_HOME,
|
||||
GBRAIN_HOME: process.env.GBRAIN_HOME,
|
||||
GSTACK_GBRAIN_PROBE_TIMEOUT_MS: process.env.GSTACK_GBRAIN_PROBE_TIMEOUT_MS,
|
||||
};
|
||||
process.env.HOME = env.home;
|
||||
process.env.PATH = `${env.bindir}:/usr/bin:/bin`;
|
||||
process.env.GSTACK_HOME = env.gstackHome;
|
||||
delete process.env.GBRAIN_HOME;
|
||||
delete process.env.GSTACK_GBRAIN_PROBE_TIMEOUT_MS;
|
||||
return () => {
|
||||
if (prev.HOME === undefined) delete process.env.HOME;
|
||||
else process.env.HOME = prev.HOME;
|
||||
if (prev.PATH === undefined) delete process.env.PATH;
|
||||
else process.env.PATH = prev.PATH;
|
||||
if (prev.GSTACK_HOME === undefined) delete process.env.GSTACK_HOME;
|
||||
else process.env.GSTACK_HOME = prev.GSTACK_HOME;
|
||||
for (const [k, v] of Object.entries(prev)) {
|
||||
if (v === undefined) delete process.env[k];
|
||||
else process.env[k] = v;
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
@@ -206,6 +230,73 @@ describe("lib/gbrain-local-status — five status cases", () => {
|
||||
restoreEnv = applyEnv(env);
|
||||
expect(localEngineStatus({ noCache: true })).toBe("ok");
|
||||
});
|
||||
|
||||
it("returns 'timeout' (not broken-config) when the probe exceeds the deadline (#1964)", () => {
|
||||
env = makeEnv({ withGbrain: true, gbrainBehavior: "slow", withConfig: true });
|
||||
restoreEnv = applyEnv(env);
|
||||
process.env.GSTACK_GBRAIN_PROBE_TIMEOUT_MS = "300";
|
||||
expect(localEngineStatus({ noCache: true })).toBe("timeout");
|
||||
});
|
||||
|
||||
it("honors GBRAIN_HOME for config detection (codex D11)", () => {
|
||||
// Config lives ONLY at the alternate GBRAIN_HOME; ~/.gbrain has none.
|
||||
env = makeEnv({ withGbrain: true, gbrainBehavior: "ok", withConfig: false });
|
||||
restoreEnv = applyEnv(env);
|
||||
const altHome = join(env.tmp, "alt-gbrain");
|
||||
mkdirSync(altHome, { recursive: true });
|
||||
writeFileSync(
|
||||
join(altHome, "config.json"),
|
||||
JSON.stringify({ engine: "pglite", database_url: "pglite:///fake" }),
|
||||
);
|
||||
// Without GBRAIN_HOME: misclassified as missing-config.
|
||||
expect(localEngineStatus({ noCache: true })).toBe("missing-config");
|
||||
// With GBRAIN_HOME: the relocated config is found.
|
||||
process.env.GBRAIN_HOME = altHome;
|
||||
expect(localEngineStatus({ noCache: true })).toBe("ok");
|
||||
});
|
||||
});
|
||||
|
||||
describe("gstack-gbrain-detect --is-ok — timeout is usable (eng review D1)", () => {
|
||||
it("exits 0 when the engine probe times out (slow-but-healthy must not suppress brain features)", () => {
|
||||
const env = makeEnv({ withGbrain: true, gbrainBehavior: "slow", withConfig: true });
|
||||
try {
|
||||
const detect = join(import.meta.dir, "..", "bin", "gstack-gbrain-detect");
|
||||
const r = spawnSync(process.execPath, [detect, "--is-ok"], {
|
||||
encoding: "utf-8",
|
||||
timeout: 20_000,
|
||||
env: {
|
||||
...process.env,
|
||||
HOME: env.home,
|
||||
GSTACK_HOME: env.gstackHome,
|
||||
PATH: `${env.bindir}:/usr/bin:/bin`,
|
||||
GSTACK_GBRAIN_PROBE_TIMEOUT_MS: "300",
|
||||
GSTACK_DETECT_NO_CACHE: "1",
|
||||
GBRAIN_HOME: "",
|
||||
},
|
||||
});
|
||||
expect(r.status).toBe(0);
|
||||
} finally {
|
||||
env.cleanup();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("probeTimeoutMs — env override parsing", () => {
|
||||
it("defaults to 15s when unset", () => {
|
||||
expect(probeTimeoutMs({})).toBe(DEFAULT_PROBE_TIMEOUT_MS);
|
||||
expect(DEFAULT_PROBE_TIMEOUT_MS).toBe(15_000);
|
||||
});
|
||||
|
||||
it("parses a numeric override", () => {
|
||||
expect(probeTimeoutMs({ GSTACK_GBRAIN_PROBE_TIMEOUT_MS: "300" })).toBe(300);
|
||||
});
|
||||
|
||||
it("falls back to the default on non-numeric, empty, and non-positive values", () => {
|
||||
expect(probeTimeoutMs({ GSTACK_GBRAIN_PROBE_TIMEOUT_MS: "fast" })).toBe(DEFAULT_PROBE_TIMEOUT_MS);
|
||||
expect(probeTimeoutMs({ GSTACK_GBRAIN_PROBE_TIMEOUT_MS: "" })).toBe(DEFAULT_PROBE_TIMEOUT_MS);
|
||||
expect(probeTimeoutMs({ GSTACK_GBRAIN_PROBE_TIMEOUT_MS: "0" })).toBe(DEFAULT_PROBE_TIMEOUT_MS);
|
||||
expect(probeTimeoutMs({ GSTACK_GBRAIN_PROBE_TIMEOUT_MS: "-5" })).toBe(DEFAULT_PROBE_TIMEOUT_MS);
|
||||
});
|
||||
});
|
||||
|
||||
describe("lib/gbrain-local-status — cache behavior", () => {
|
||||
@@ -275,6 +366,31 @@ describe("lib/gbrain-local-status — cache behavior", () => {
|
||||
expect(localEngineStatus({ noCache: false })).toBe("broken-db");
|
||||
});
|
||||
|
||||
it("caches a 'timeout' result (sync probes 3x/run — uncached would cost 3 deadlines)", () => {
|
||||
env = makeEnv({ withGbrain: true, gbrainBehavior: "slow", withConfig: true });
|
||||
restoreEnv = applyEnv(env);
|
||||
process.env.GSTACK_GBRAIN_PROBE_TIMEOUT_MS = "300";
|
||||
expect(localEngineStatus({ noCache: false })).toBe("timeout");
|
||||
|
||||
// Swap the fake to a fast-ok binary; the cached timeout should still win
|
||||
// within TTL (same key — proving the result was cached, not re-probed).
|
||||
writeFileSync(join(env.bindir, "gbrain"), makeFakeGbrainScript("ok"));
|
||||
chmodSync(join(env.bindir, "gbrain"), 0o755);
|
||||
expect(localEngineStatus({ noCache: false })).toBe("timeout");
|
||||
});
|
||||
|
||||
it("invalidates a cached 'timeout' when GSTACK_GBRAIN_PROBE_TIMEOUT_MS changes (key invariant, codex D13)", () => {
|
||||
env = makeEnv({ withGbrain: true, gbrainBehavior: "slow", withConfig: true });
|
||||
restoreEnv = applyEnv(env);
|
||||
process.env.GSTACK_GBRAIN_PROBE_TIMEOUT_MS = "300";
|
||||
expect(localEngineStatus({ noCache: false })).toBe("timeout");
|
||||
|
||||
// User raises the timeout past the fake's 2s sleep: cache key changes,
|
||||
// re-probe succeeds.
|
||||
process.env.GSTACK_GBRAIN_PROBE_TIMEOUT_MS = "5000";
|
||||
expect(localEngineStatus({ noCache: false })).toBe("ok");
|
||||
});
|
||||
|
||||
it("invalidates cache when HOME changes (key invariant)", () => {
|
||||
env = makeEnv({ withGbrain: true, gbrainBehavior: "ok", withConfig: true });
|
||||
restoreEnv = applyEnv(env);
|
||||
|
||||
@@ -42,7 +42,7 @@ interface FakeEnv {
|
||||
*/
|
||||
function makeEnv(opts: {
|
||||
withGbrain: boolean;
|
||||
gbrainBehavior?: "ok" | "broken-db" | "broken-config";
|
||||
gbrainBehavior?: "ok" | "broken-db" | "broken-config" | "slow";
|
||||
withConfig: boolean;
|
||||
}): FakeEnv {
|
||||
const tmp = mkdtempSync(join(tmpdir(), "gbrain-sync-skip-"));
|
||||
@@ -65,19 +65,26 @@ function makeEnv(opts: {
|
||||
|
||||
if (opts.withGbrain) {
|
||||
const behavior = opts.gbrainBehavior || "ok";
|
||||
const stderrLine =
|
||||
behavior === "broken-db"
|
||||
? 'echo "Cannot connect to database: . Fix: Check your connection URL in ~/.gbrain/config.json" >&2'
|
||||
: behavior === "broken-config"
|
||||
? 'echo "Error: malformed config.json" >&2'
|
||||
: "";
|
||||
const exitCode = behavior === "ok" ? 0 : 1;
|
||||
// "slow": healthy engine, cold pooler connection (#1964) — sleeps past the
|
||||
// (test-lowered) probe timeout on `sources list`, then answers fine.
|
||||
const sourcesBlock =
|
||||
behavior === "slow"
|
||||
? ` sleep 2
|
||||
echo '{"sources":[]}'
|
||||
exit 0`
|
||||
: behavior === "ok"
|
||||
? ` echo '{"sources":[]}'
|
||||
exit 0`
|
||||
: ` ${
|
||||
behavior === "broken-db"
|
||||
? 'echo "Cannot connect to database: . Fix: Check your connection URL in ~/.gbrain/config.json" >&2'
|
||||
: 'echo "Error: malformed config.json" >&2'
|
||||
}
|
||||
exit 1`;
|
||||
const fake = `#!/bin/sh
|
||||
if [ "$1" = "--version" ]; then echo "gbrain 0.33.1.0"; exit 0; fi
|
||||
if [ "$1 $2" = "sources list" ]; then
|
||||
if [ ${exitCode} -eq 0 ]; then echo '{"sources":[]}'; exit 0; fi
|
||||
${stderrLine}
|
||||
exit ${exitCode}
|
||||
${sourcesBlock}
|
||||
fi
|
||||
if [ "$1" = "--help" ]; then echo " import"; exit 0; fi
|
||||
exit 0
|
||||
@@ -95,7 +102,11 @@ exit 0
|
||||
};
|
||||
}
|
||||
|
||||
function runOrchestrator(env: FakeEnv, args: string[]): { stdout: string; stderr: string; exitCode: number } {
|
||||
function runOrchestrator(
|
||||
env: FakeEnv,
|
||||
args: string[],
|
||||
extraEnv: Record<string, string> = {},
|
||||
): { stdout: string; stderr: string; exitCode: number } {
|
||||
// Initialize a git repo in the sandbox so repoRoot() finds it (otherwise
|
||||
// code stage skips with "not in git repo" before our check ever fires).
|
||||
spawnSync("git", ["init", "-q", env.home], { encoding: "utf-8" });
|
||||
@@ -113,6 +124,7 @@ function runOrchestrator(env: FakeEnv, args: string[]): { stdout: string; stderr
|
||||
HOME: env.home,
|
||||
GSTACK_HOME: env.gstackHome,
|
||||
PATH: `${env.bindir}:/usr/bin:/bin`,
|
||||
...extraEnv,
|
||||
},
|
||||
});
|
||||
return {
|
||||
@@ -123,6 +135,23 @@ function runOrchestrator(env: FakeEnv, args: string[]): { stdout: string; stderr
|
||||
}
|
||||
|
||||
describe("gstack-gbrain-sync — split-engine SKIP (plan D12)", () => {
|
||||
it("PROCEEDS (with warning) when the engine probe times out — slow is not broken (#1964)", () => {
|
||||
const env = makeEnv({ withGbrain: true, gbrainBehavior: "slow", withConfig: true });
|
||||
try {
|
||||
const r = runOrchestrator(env, ["--code-only"], {
|
||||
GSTACK_GBRAIN_PROBE_TIMEOUT_MS: "300",
|
||||
});
|
||||
const out = r.stdout + r.stderr;
|
||||
// The stage must NOT be skipped with the local-engine reason...
|
||||
expect(out).not.toContain("local engine timeout");
|
||||
expect(out).not.toContain("config.json is malformed");
|
||||
// ...and the proceed-with-warning line must name the env knob.
|
||||
expect(out).toContain("GSTACK_GBRAIN_PROBE_TIMEOUT_MS");
|
||||
} finally {
|
||||
env.cleanup();
|
||||
}
|
||||
}, 30_000); // proceeding runs the real code-import path against the slow fake (~11s)
|
||||
|
||||
it("SKIPs code stage when local engine is broken-db; brain-sync still attempted", () => {
|
||||
const env = makeEnv({ withGbrain: true, gbrainBehavior: "broken-db", withConfig: true });
|
||||
try {
|
||||
|
||||
Reference in New Issue
Block a user