mirror of https://github.com/garrytan/gstack.git
fix(gbrain): engine detection survives gbrain ≥0.25 schema + non-zero doctor exit
freshDetectEngineTier() in lib/gstack-memory-helpers.ts returned engine:
"unknown" for every Supabase user on gbrain ≥0.25. Two stacking bugs:
1. execSync("gbrain doctor --json --fast 2>/dev/null") threw on non-zero
exit. gbrain doctor exits 1 whenever health_score < 100, which is
essentially every fresh install due to resolver_health warnings. The
JSON output never reached the parser.
2. gbrain ≥0.25 shipped schema_version:2 doctor output that dropped the
top-level 'engine' field entirely.
Result: every /sync-gbrain on Supabase logged 'engine=unknown' and skipped
all sync stages silently.
Fix:
- Replace execSync with execFileSync (no shell, no bash-specific 2>/dev/null
redirect; portable to Windows).
- Recover stdout from the thrown error object so non-zero exits still parse.
- Fall back to reading gbrain's config.json (respecting GBRAIN_HOME env var,
defaulting to ~/.gbrain/config.json) when doctor output doesn't surface
an engine field.
- Add logGbrainError() helper that appends one-line JSONL to
~/.gstack/.gbrain-errors.jsonl on parse failure, so future regressions
leave a forensic trail.
The "supabase" tier here means "remote postgres" in practice — gbrain
config uses engine:"postgres" for both real Supabase and any other
remote postgres (e.g. local-postgres-for-testing). Downstream sync code
treats them identically, so the label compression is intentional and
documented inline.
Regression test: existing detectEngineTier suite now isolates HOME +
GBRAIN_HOME + PATH to temp dirs (closes a flake source where the prior
tests would read whatever was on the reviewer's machine). New test
forces gbrain off PATH, writes a synthetic config.json with
engine:"postgres", asserts detectEngineTier() returns
engine:"supabase".
Fixes #1415. Patch shape contributed by Shiv @shivasymbl (tested on
gstack v1.31.0.0 + gbrain v0.31.3 + Supabase).
This commit is contained in:
parent
f94aef7303
commit
9ae98e9d4d
|
|
@ -244,21 +244,82 @@ export function detectEngineTier(): EngineDetect {
|
|||
return fresh;
|
||||
}
|
||||
|
||||
// Returns gbrain's config.json path, honoring GBRAIN_HOME env var with a
|
||||
// fallback to ~/.gbrain. gbrain >=0.25 dropped the top-level `engine` field
|
||||
// from doctor output, so this file is the only reliable source for engine
|
||||
// detection on that version. See #1415.
|
||||
function gbrainConfigPath(): string {
|
||||
const root = process.env.GBRAIN_HOME || join(homedir(), ".gbrain");
|
||||
return join(root, "config.json");
|
||||
}
|
||||
|
||||
// Best-effort JSONL append to ~/.gstack/.gbrain-errors.jsonl. Never throws.
|
||||
function logGbrainError(kind: string, detail: string): void {
|
||||
try {
|
||||
const path = errorLogPath();
|
||||
mkdirSync(dirname(path), { recursive: true });
|
||||
appendFileSync(
|
||||
path,
|
||||
JSON.stringify({ ts: new Date().toISOString(), kind, detail: detail.slice(0, 500) }) + "\n",
|
||||
"utf-8"
|
||||
);
|
||||
} catch { /* logging is best-effort */ }
|
||||
}
|
||||
|
||||
function freshDetectEngineTier(): EngineDetect {
|
||||
const now = Date.now();
|
||||
let parsed: Record<string, unknown> | null = null;
|
||||
|
||||
// execFileSync (not execSync) avoids shell redirection — portable to
|
||||
// environments where `2>/dev/null` is bash-specific. The stdio array
|
||||
// suppresses stderr without invoking a shell.
|
||||
try {
|
||||
const out = execSync("gbrain doctor --json --fast 2>/dev/null", { encoding: "utf-8", timeout: 5000 });
|
||||
const parsed = JSON.parse(out);
|
||||
const engine: EngineTier = parsed?.engine === "supabase" ? "supabase" : parsed?.engine === "pglite" ? "pglite" : "unknown";
|
||||
return {
|
||||
engine,
|
||||
supabase_url: parsed?.supabase_url || undefined,
|
||||
detected_at: now,
|
||||
schema_version: 1,
|
||||
};
|
||||
} catch {
|
||||
return { engine: "unknown", detected_at: now, schema_version: 1 };
|
||||
const out = execFileSync("gbrain", ["doctor", "--json", "--fast"], {
|
||||
encoding: "utf-8",
|
||||
timeout: 5000,
|
||||
stdio: ["ignore", "pipe", "ignore"],
|
||||
});
|
||||
parsed = JSON.parse(out);
|
||||
} catch (err: unknown) {
|
||||
// execFileSync throws on non-zero exit; stdout is still on the error
|
||||
// object. gbrain doctor exits 1 whenever health_score < 100, which is
|
||||
// essentially always on fresh installs (resolver_health warnings are
|
||||
// normal). Recover stdout and re-parse. 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 (parseErr) {
|
||||
logGbrainError("doctor_parse_failure", String(parseErr));
|
||||
}
|
||||
}
|
||||
|
||||
let engine: EngineTier =
|
||||
parsed?.engine === "supabase" ? "supabase" :
|
||||
parsed?.engine === "pglite" ? "pglite" : "unknown";
|
||||
|
||||
// gbrain >=0.25 ships schema_version:2 doctor output which dropped the
|
||||
// top-level `engine` field. Fall back to gbrain's config.json (respects
|
||||
// GBRAIN_HOME). "supabase" here means "remote postgres" — gbrain config
|
||||
// uses engine:"postgres" for real Supabase AND any other remote postgres
|
||||
// (e.g. local-postgres-for-testing). Downstream sync code treats them the
|
||||
// same, so the label compression is intentional.
|
||||
if (engine === "unknown") {
|
||||
try {
|
||||
const cfg = JSON.parse(readFileSync(gbrainConfigPath(), "utf-8"));
|
||||
if (cfg?.engine === "pglite") engine = "pglite";
|
||||
else if (cfg?.engine === "postgres" || cfg?.database_url) engine = "supabase";
|
||||
} catch (cfgErr) {
|
||||
logGbrainError("config_read_failure", String(cfgErr));
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
engine,
|
||||
supabase_url: parsed?.supabase_url as string | undefined,
|
||||
detected_at: now,
|
||||
schema_version: 1,
|
||||
};
|
||||
}
|
||||
|
||||
// ── Public: parseSkillManifest ────────────────────────────────────────────
|
||||
|
|
|
|||
|
|
@ -272,17 +272,36 @@ describe("withErrorContext", () => {
|
|||
|
||||
describe("detectEngineTier", () => {
|
||||
let savedHome: string | undefined;
|
||||
let savedGbrainHome: string | undefined;
|
||||
let savedRealHome: string | undefined;
|
||||
let savedPath: string | undefined;
|
||||
let testHome: string;
|
||||
let testGbrainHome: string;
|
||||
|
||||
beforeEach(() => {
|
||||
savedHome = process.env.GSTACK_HOME;
|
||||
savedGbrainHome = process.env.GBRAIN_HOME;
|
||||
savedRealHome = process.env.HOME;
|
||||
savedPath = process.env.PATH;
|
||||
testHome = mkdtempSync(join(tmpdir(), "gstack-test-engine-"));
|
||||
testGbrainHome = mkdtempSync(join(tmpdir(), "gstack-test-gbrain-"));
|
||||
process.env.GSTACK_HOME = testHome;
|
||||
process.env.GBRAIN_HOME = testGbrainHome;
|
||||
// Isolate HOME too — even though gbrainConfigPath() prefers GBRAIN_HOME
|
||||
// when set, defense-in-depth against future code reading ~/.gbrain
|
||||
// directly. See #1415 codex review finding #6.
|
||||
process.env.HOME = testHome;
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
if (savedHome === undefined) delete process.env.GSTACK_HOME;
|
||||
else process.env.GSTACK_HOME = savedHome;
|
||||
if (savedGbrainHome === undefined) delete process.env.GBRAIN_HOME;
|
||||
else process.env.GBRAIN_HOME = savedGbrainHome;
|
||||
if (savedRealHome === undefined) delete process.env.HOME;
|
||||
else process.env.HOME = savedRealHome;
|
||||
if (savedPath === undefined) delete process.env.PATH;
|
||||
else process.env.PATH = savedPath;
|
||||
});
|
||||
|
||||
it("returns a valid EngineDetect shape (engine, detected_at, schema_version)", () => {
|
||||
|
|
@ -307,4 +326,19 @@ describe("detectEngineTier", () => {
|
|||
const second = detectEngineTier();
|
||||
expect(second.detected_at).toBe(first.detected_at);
|
||||
});
|
||||
|
||||
it("falls back to GBRAIN_HOME/config.json when gbrain doctor omits engine (schema_version:2 case)", () => {
|
||||
// Regression test for #1415: gbrain >=0.25 doctor output dropped the
|
||||
// top-level `engine` field. The detect path must fall back to config.json.
|
||||
// We force the doctor call to fail (PATH stripped of gbrain) and write a
|
||||
// synthetic config to GBRAIN_HOME so the fallback path is deterministic.
|
||||
process.env.PATH = "/nonexistent-no-gbrain-here";
|
||||
writeFileSync(
|
||||
join(testGbrainHome, "config.json"),
|
||||
JSON.stringify({ engine: "postgres", database_url: "postgresql://test/example" }),
|
||||
"utf-8"
|
||||
);
|
||||
const result = detectEngineTier();
|
||||
expect(result.engine).toBe("supabase");
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue