mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-09 22:48:57 +02:00
fix: resolve GBRAIN_HOME with gbrain's parent-dir semantics (#2521)
gstack treated GBRAIN_HOME as the config directory; gbrain's configDir() treats it as the PARENT and always appends `.gbrain` itself (the contract is explicit in gbrain's source: GBRAIN_HOME=/tmp/x → /tmp/x/.gbrain/ config.json). With GBRAIN_HOME set, gstack classified engine status from a file gbrain never reads — the probe's two halves (file checks vs the spawned `gbrain sources list`) looked at DIFFERENT installs, so any resulting status was arbitrary: missing-config/broken-config against healthy installs, or a thin-client marker gstack saw that gbrain itself reported as "No brain configured". New shared resolver `gbrainConfigDir()` in lib/gbrain-exec.ts is the single source of truth. All seven gstack sites route through the contract: - lib/gbrain-local-status.ts gbrainConfigPath (the classifier's file half) - bin/gstack-gbrain-detect GBRAIN_CONFIG + readRemoteMcpUrl - lib/gbrain-exec.ts buildGbrainEnv (the probe's DATABASE_URL seed — fixing only the classifier would have left the split-brain in the spawn half, flagged by the reporter) - lib/gbrain-guards.ts gbrainHome (clones-dir + autopilot-lock paths) - lib/gstack-memory-helpers.ts gbrainConfigPath (engine-tier fallback) - bin/gstack-gbrain-install pre-doctor config check (shell) Unit tests cover GBRAIN_HOME set (config found at $GBRAIN_HOME/.gbrain), the old flat layout explicitly NOT read (both classifier and buildGbrainEnv), and unset (~/.gbrain unchanged). Existing fixtures that encoded the deviant flat layout are updated to gbrain's contract. Root-cause analysis by @d-danielsun in #2521. Deviation from the 3-site plan spec: the same deviant resolution existed in four more sites (buildGbrainEnv, gbrain-guards, memory-helpers, gbrain-install); fixing only three would have left gstack disagreeing with itself as well as with gbrain, so the whole class moved to the shared resolver in one change. Fixes #2521 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
ce4a7bbb7e
commit
5854d122d3
@@ -77,17 +77,34 @@ describe("buildGbrainEnv", () => {
|
||||
expect(result.DATABASE_URL).toBe("postgresql://app/db");
|
||||
});
|
||||
|
||||
it("honors GBRAIN_HOME when set (config aligned with detectEngineTier)", () => {
|
||||
// Move the config to an alternate dir; set GBRAIN_HOME to point at it.
|
||||
it("honors GBRAIN_HOME when set, with gbrain's parent-dir semantics (#2521)", () => {
|
||||
// Move the config to an alternate dir; set GBRAIN_HOME to point at its
|
||||
// PARENT — gbrain's configDir() appends `.gbrain` itself, so
|
||||
// GBRAIN_HOME=/x reads /x/.gbrain/config.json.
|
||||
const altGbrainHome = join(home, "alt-gbrain");
|
||||
mkdirSync(altGbrainHome, { recursive: true });
|
||||
writeFileSync(join(altGbrainHome, "config.json"), JSON.stringify({ database_url: "postgresql://alt/db" }));
|
||||
mkdirSync(join(altGbrainHome, ".gbrain"), { recursive: true });
|
||||
writeFileSync(
|
||||
join(altGbrainHome, ".gbrain", "config.json"),
|
||||
JSON.stringify({ database_url: "postgresql://alt/db" }),
|
||||
);
|
||||
// No file at the default ~/.gbrain location.
|
||||
const baseEnv = { HOME: home, GBRAIN_HOME: altGbrainHome };
|
||||
const result = buildGbrainEnv({ baseEnv });
|
||||
expect(result.DATABASE_URL).toBe("postgresql://alt/db");
|
||||
});
|
||||
|
||||
it("ignores a config at $GBRAIN_HOME/config.json — gbrain never reads that file (#2521)", () => {
|
||||
const altGbrainHome = join(home, "alt-gbrain-flat");
|
||||
mkdirSync(altGbrainHome, { recursive: true });
|
||||
writeFileSync(
|
||||
join(altGbrainHome, "config.json"),
|
||||
JSON.stringify({ database_url: "postgresql://alt/db" }),
|
||||
);
|
||||
const baseEnv = { HOME: home, GBRAIN_HOME: altGbrainHome };
|
||||
const result = buildGbrainEnv({ baseEnv });
|
||||
expect(result.DATABASE_URL).toBeUndefined();
|
||||
});
|
||||
|
||||
it("returns a fresh env object — never the caller's env by identity", () => {
|
||||
// Codex review #11: object-identity equality lets later mutation of the
|
||||
// returned env leak back into the caller's view. The helper MUST clone.
|
||||
|
||||
@@ -953,7 +953,9 @@ exit 1
|
||||
...process.env,
|
||||
GSTACK_HOME: home,
|
||||
HOME: home,
|
||||
GBRAIN_HOME: gbrainHome,
|
||||
// #2521: GBRAIN_HOME is the PARENT of .gbrain per gbrain's configDir()
|
||||
// contract — pointing it at `home` resolves to home/.gbrain/config.json.
|
||||
GBRAIN_HOME: home,
|
||||
PATH: `${shimDir}:${process.env.PATH}`,
|
||||
// Dead loopback port → the Sourcebot probe fails fast + deterministically
|
||||
// (connection refused) instead of poking whatever operator dev server
|
||||
|
||||
@@ -269,20 +269,46 @@ describe("lib/gbrain-local-status — status classification", () => {
|
||||
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.
|
||||
it("honors GBRAIN_HOME for config detection (codex D11) with gbrain's parent-dir semantics (#2521)", () => {
|
||||
// Config lives ONLY under the alternate GBRAIN_HOME; ~/.gbrain has none.
|
||||
// gbrain's configDir() treats GBRAIN_HOME as a PARENT dir and appends
|
||||
// `.gbrain` itself: GBRAIN_HOME=/x → /x/.gbrain/config.json.
|
||||
env = makeEnv({ withGbrain: true, gbrainBehavior: "ok", withConfig: false });
|
||||
restoreEnv = applyEnv(env);
|
||||
const altHome = join(env.tmp, "alt-gbrain");
|
||||
mkdirSync(join(altHome, ".gbrain"), { recursive: true });
|
||||
writeFileSync(
|
||||
join(altHome, ".gbrain", "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 at $GBRAIN_HOME/.gbrain.
|
||||
process.env.GBRAIN_HOME = altHome;
|
||||
expect(localEngineStatus({ noCache: true })).toBe("ok");
|
||||
});
|
||||
|
||||
it("does NOT read $GBRAIN_HOME/config.json directly — gbrain never reads that file (#2521)", () => {
|
||||
// A config placed at gstack's OLD (wrong) resolution must be invisible:
|
||||
// gbrain itself would report "No brain configured" for this layout, so
|
||||
// gstack classifying "ok" from it is the #2521 split-brain.
|
||||
env = makeEnv({ withGbrain: true, gbrainBehavior: "ok", withConfig: false });
|
||||
restoreEnv = applyEnv(env);
|
||||
const altHome = join(env.tmp, "alt-gbrain-flat");
|
||||
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("missing-config");
|
||||
});
|
||||
|
||||
it("with GBRAIN_HOME unset, config resolution stays at ~/.gbrain (#2521 unset half)", () => {
|
||||
env = makeEnv({ withGbrain: true, gbrainBehavior: "ok", withConfig: true });
|
||||
restoreEnv = applyEnv(env);
|
||||
// applyEnv deletes GBRAIN_HOME; config was written at $HOME/.gbrain.
|
||||
expect(process.env.GBRAIN_HOME).toBeUndefined();
|
||||
expect(localEngineStatus({ noCache: true })).toBe("ok");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -462,10 +462,13 @@ describe("detectEngineTier", () => {
|
||||
// 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.
|
||||
// synthetic config under GBRAIN_HOME so the fallback path is
|
||||
// deterministic. Per gbrain's configDir() contract (#2521), GBRAIN_HOME
|
||||
// is a parent dir — the config lives at $GBRAIN_HOME/.gbrain/config.json.
|
||||
process.env.PATH = "/nonexistent-no-gbrain-here";
|
||||
mkdirSync(join(testGbrainHome, ".gbrain"), { recursive: true });
|
||||
writeFileSync(
|
||||
join(testGbrainHome, "config.json"),
|
||||
join(testGbrainHome, ".gbrain", "config.json"),
|
||||
JSON.stringify({ engine: "postgres", database_url: "postgresql://test/example" }),
|
||||
"utf-8"
|
||||
);
|
||||
@@ -500,8 +503,9 @@ exit 0
|
||||
{ mode: 0o755 }
|
||||
);
|
||||
process.env.PATH = `${binDir}:${process.env.PATH || ""}`;
|
||||
mkdirSync(join(testGbrainHome, ".gbrain"), { recursive: true });
|
||||
writeFileSync(
|
||||
join(testGbrainHome, "config.json"),
|
||||
join(testGbrainHome, ".gbrain", "config.json"),
|
||||
JSON.stringify({ engine: "pglite" }),
|
||||
"utf-8"
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user