mirror of https://github.com/garrytan/gstack.git
fix(gbrain-sync): centralize gbrain spawn surface + seed DATABASE_URL
Cherry-picked from #1508 by jasshultz, restructured per codex review #4 and #7 to widen scope and centralize the spawn surface. The bug: gbrain auto-loads .env.local from cwd via dotenv. When /sync-gbrain runs inside a Next.js / Prisma / Rails project whose .env.local defines its own DATABASE_URL (pointing at the app's local DB), gbrain reads that value instead of its own ~/.gbrain/config.json — auth fails, code + memory stages crash. This commit: - Adds lib/gbrain-exec.ts: buildGbrainEnv, spawnGbrain, execGbrainJson, execGbrainText, spawnGbrainAsync (the last one for memory-ingest's streaming gbrain import call). buildGbrainEnv seeds DATABASE_URL from ${GBRAIN_HOME:-$HOME/.gbrain}/config.json, returns a fresh env object (never the caller's by identity — codex review #11), and honors the GSTACK_RESPECT_ENV_DATABASE_URL=1 escape hatch. - Routes every gbrain spawn in bin/gstack-gbrain-sync.ts and bin/gstack-memory-ingest.ts through the helpers. Both files now own zero direct spawnSync("gbrain"|spawn("gbrain"|execFileSync("gbrain" call sites. - Threads buildGbrainEnv into the spawnSync("bun", [memory-ingest], ...) grandchild in runMemoryIngest (codex review #7). Without this, the parent fix is half-baked — the bun child inherits a clean env but needs DATABASE_URL pre-seeded too. spawnGbrainAsync inside memory-ingest provides defense in depth for standalone invocations. - Adds GBRAIN_HOME support — aligns with detectEngineTier (already honors GBRAIN_HOME) so all gstack-side gbrain calls agree on which config file matters. Resolves baseEnv.HOME first, then homedir(), so test injection works without process-wide HOME mutation. - Adds test/build-gbrain-env.test.ts: 10 unit tests covering all five env-seeding branches (seed from config / override caller / GSTACK_RESPECT escape hatch / missing config / unparseable config / no database_url field / GBRAIN_HOME path / object-identity guard / unrelated-vars preservation / idempotent-when-matches). - Adds test/gbrain-exec-invariant.test.ts: static-source check that greps both bin/gstack-gbrain-sync.ts and bin/gstack-memory-ingest.ts for direct spawnSync("gbrain"|spawn("gbrain"|execFileSync("gbrain"| execSync(...gbrain matches and fails the build if any are found. Refactor-proof against future contributors adding a new gbrain spawn without env threading. The invariant is intentionally narrow — only the two files where the DATABASE_URL bug actually hurts users are guarded. Migrating the spawn sites in lib/gbrain-local-status.ts, lib/gstack-memory-helpers.ts, and bin/gstack-brain-context-load.ts is a follow-up. Co-Authored-By: Jason Shultz <jasshultz@gmail.com> Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Garry Tan
parent
f8e4291521
commit
0fb7fa6c1e
|
|
@ -39,6 +39,7 @@ import "../lib/conductor-env-shim";
|
|||
import { detectEngineTier, withErrorContext, canonicalizeRemote } from "../lib/gstack-memory-helpers";
|
||||
import { ensureSourceRegistered, sourcePageCount } from "../lib/gbrain-sources";
|
||||
import { localEngineStatus, type LocalEngineStatus } from "../lib/gbrain-local-status";
|
||||
import { buildGbrainEnv, spawnGbrain, execGbrainJson } from "../lib/gbrain-exec";
|
||||
|
||||
// ── Types ──────────────────────────────────────────────────────────────────
|
||||
|
||||
|
|
@ -262,11 +263,9 @@ export function _resetGbrainSupportsRenameCache(): void {
|
|||
function gbrainSupportsSourcesRename(env?: NodeJS.ProcessEnv): boolean {
|
||||
if (_gbrainSupportsRenameCache !== null) return _gbrainSupportsRenameCache;
|
||||
try {
|
||||
const r = spawnSync("gbrain", ["sources", "rename", "--help"], {
|
||||
encoding: "utf-8",
|
||||
const r = spawnGbrain(["sources", "rename", "--help"], {
|
||||
timeout: 5_000,
|
||||
stdio: ["ignore", "pipe", "pipe"],
|
||||
env: env || process.env,
|
||||
baseEnv: env,
|
||||
});
|
||||
const out = `${r.stdout || ""}\n${r.stderr || ""}`;
|
||||
// Match the exact argument shape: `rename <old> <new>` (with literal
|
||||
|
|
@ -290,20 +289,13 @@ function gbrainSupportsSourcesRename(env?: NodeJS.ProcessEnv): boolean {
|
|||
* helper can be exercised without a real gbrain CLI.
|
||||
*/
|
||||
export function sourceLocalPath(sourceId: string, env?: NodeJS.ProcessEnv): string | null {
|
||||
try {
|
||||
const r = spawnSync("gbrain", ["sources", "list", "--json"], {
|
||||
encoding: "utf-8",
|
||||
timeout: 30_000,
|
||||
stdio: ["ignore", "pipe", "pipe"],
|
||||
env: env || process.env,
|
||||
});
|
||||
if (r.status !== 0) return null;
|
||||
const list = JSON.parse(r.stdout || "[]") as Array<{ id: string; local_path?: string }>;
|
||||
const found = list.find((s) => s.id === sourceId);
|
||||
return found?.local_path ?? null;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
const list = execGbrainJson<Array<{ id: string; local_path?: string }>>(
|
||||
["sources", "list", "--json"],
|
||||
{ baseEnv: env },
|
||||
);
|
||||
if (!list) return null;
|
||||
const found = list.find((s) => s.id === sourceId);
|
||||
return found?.local_path ?? null;
|
||||
}
|
||||
|
||||
/** Result of `planHostnameFoldMigration` — informs `runCodeImport` of next steps. */
|
||||
|
|
@ -352,12 +344,7 @@ export function planHostnameFoldMigration(
|
|||
};
|
||||
}
|
||||
if (gbrainSupportsSourcesRename(env)) {
|
||||
const r = spawnSync("gbrain", ["sources", "rename", legacyPathHashId, newSourceId], {
|
||||
encoding: "utf-8",
|
||||
timeout: 30_000,
|
||||
stdio: ["ignore", "pipe", "pipe"],
|
||||
env: env || process.env,
|
||||
});
|
||||
const r = spawnGbrain(["sources", "rename", legacyPathHashId, newSourceId], { baseEnv: env });
|
||||
if (r.status === 0) {
|
||||
return { kind: "renamed", oldId: legacyPathHashId, newId: newSourceId };
|
||||
}
|
||||
|
|
@ -377,12 +364,8 @@ export function planHostnameFoldMigration(
|
|||
* flag-helper centralization in commit 4 (lib/gbrain-exec.ts) will resolve
|
||||
* the inconsistency across the codebase.
|
||||
*/
|
||||
export function removeOrphanedSource(oldId: string): boolean {
|
||||
const r = spawnSync("gbrain", ["sources", "remove", oldId, "--confirm-destructive"], {
|
||||
encoding: "utf-8",
|
||||
timeout: 30_000,
|
||||
stdio: ["ignore", "pipe", "pipe"],
|
||||
});
|
||||
export function removeOrphanedSource(oldId: string, env?: NodeJS.ProcessEnv): boolean {
|
||||
const r = spawnGbrain(["sources", "remove", oldId, "--confirm-destructive"], { baseEnv: env });
|
||||
return r.status === 0;
|
||||
}
|
||||
|
||||
|
|
@ -555,13 +538,16 @@ async function runCodeImport(args: CliArgs): Promise<StageResult> {
|
|||
// hits forever if we left the orphan in place. Remove the legacy id once
|
||||
// here so users don't accumulate orphans.
|
||||
// Failure is non-fatal — we still register the new id below.
|
||||
// gbrainEnv seeds DATABASE_URL from gbrain's config so this stage works
|
||||
// inside Next.js / Prisma / Rails projects with their own .env.local
|
||||
// (codex review #7 — bug fix is wider than #1508 as filed).
|
||||
const gbrainEnv = buildGbrainEnv({ announce: !args.quiet });
|
||||
const legacyId = deriveLegacyCodeSourceId(root);
|
||||
let legacyRemoved = false;
|
||||
if (legacyId !== sourceId) {
|
||||
const rm = spawnSync("gbrain", ["sources", "remove", legacyId, "--confirm-destructive"], {
|
||||
encoding: "utf-8",
|
||||
const rm = spawnGbrain(["sources", "remove", legacyId, "--confirm-destructive"], {
|
||||
timeout: 30_000,
|
||||
stdio: ["ignore", "pipe", "pipe"],
|
||||
baseEnv: gbrainEnv,
|
||||
});
|
||||
// Treat absent-source as success (clean state). gbrain emits "not found" on
|
||||
// missing id; treat any non-zero exit without "not found" as a soft fail.
|
||||
|
|
@ -575,7 +561,7 @@ async function runCodeImport(args: CliArgs): Promise<StageResult> {
|
|||
// pages); fall back to register-new → sync-OK → remove-old. Path-drift
|
||||
// (user moved the repo, etc.) skips migration with a warning.
|
||||
const pathOnlyHashLegacyId = derivePathOnlyHashLegacyId(root);
|
||||
const migration = planHostnameFoldMigration(root, sourceId, pathOnlyHashLegacyId);
|
||||
const migration = planHostnameFoldMigration(root, sourceId, pathOnlyHashLegacyId, gbrainEnv);
|
||||
if (migration.kind === "skipped-path-drift" && !args.quiet) {
|
||||
console.error(
|
||||
`[sync:code] hostname-fold migration skipped: legacy source ${migration.oldId} `
|
||||
|
|
@ -590,7 +576,7 @@ async function runCodeImport(args: CliArgs): Promise<StageResult> {
|
|||
// no synchronous duplicate here (per /codex review #12).
|
||||
let registered = false;
|
||||
try {
|
||||
const result = await ensureSourceRegistered(sourceId, root, { federated: true });
|
||||
const result = await ensureSourceRegistered(sourceId, root, { federated: true, env: gbrainEnv });
|
||||
registered = result.changed;
|
||||
} catch (err) {
|
||||
return {
|
||||
|
|
@ -608,9 +594,10 @@ async function runCodeImport(args: CliArgs): Promise<StageResult> {
|
|||
? ["reindex-code", "--source", sourceId, "--yes"]
|
||||
: ["sync", "--strategy", "code", "--source", sourceId];
|
||||
|
||||
const syncResult = spawnSync("gbrain", syncArgs, {
|
||||
const syncResult = spawnGbrain(syncArgs, {
|
||||
stdio: args.quiet ? ["ignore", "ignore", "ignore"] : ["ignore", "inherit", "inherit"],
|
||||
timeout: 35 * 60 * 1000,
|
||||
baseEnv: gbrainEnv,
|
||||
});
|
||||
|
||||
if (syncResult.status !== 0) {
|
||||
|
|
@ -633,13 +620,12 @@ async function runCodeImport(args: CliArgs): Promise<StageResult> {
|
|||
// the wrong/default source. Treat it as a stage failure (ok=false) so the
|
||||
// verdict block surfaces ERR and the user knows to retry rather than
|
||||
// trusting stale results.
|
||||
const attach = spawnSync("gbrain", ["sources", "attach", sourceId], {
|
||||
encoding: "utf-8",
|
||||
const attach = spawnGbrain(["sources", "attach", sourceId], {
|
||||
timeout: 10_000,
|
||||
cwd: root,
|
||||
stdio: ["ignore", "pipe", "pipe"],
|
||||
baseEnv: gbrainEnv,
|
||||
});
|
||||
const pageCount = sourcePageCount(sourceId);
|
||||
const pageCount = sourcePageCount(sourceId, gbrainEnv);
|
||||
|
||||
// Step 4: Deferred hostname-fold cleanup.
|
||||
// Only remove the pre-#1468 path-only-hash source NOW that the new source
|
||||
|
|
@ -649,7 +635,7 @@ async function runCodeImport(args: CliArgs): Promise<StageResult> {
|
|||
// safety: register → sync → verify → THEN delete.
|
||||
let hostnameLegacyRemoved = false;
|
||||
if (migration.kind === "pending-cleanup" && pageCount !== null && pageCount > 0) {
|
||||
hostnameLegacyRemoved = removeOrphanedSource(migration.oldId);
|
||||
hostnameLegacyRemoved = removeOrphanedSource(migration.oldId, gbrainEnv);
|
||||
if (hostnameLegacyRemoved && !args.quiet) {
|
||||
console.error(`[sync:code] hostname-fold migration: removed legacy ${migration.oldId} after new source sync verified (page_count=${pageCount})`);
|
||||
}
|
||||
|
|
@ -718,9 +704,14 @@ function runMemoryIngest(args: CliArgs): StageResult {
|
|||
else ingestArgs.push("--incremental");
|
||||
if (args.quiet) ingestArgs.push("--quiet");
|
||||
|
||||
// Thread the seeded env into the bun grandchild (codex review #7 — the
|
||||
// .env.local footgun affects gstack-memory-ingest.ts too, not just the
|
||||
// direct gbrain spawns in this file). The grandchild calls gbrain import
|
||||
// internally and must see the DATABASE_URL from gbrain's own config.
|
||||
const result = spawnSync("bun", ingestArgs, {
|
||||
encoding: "utf-8",
|
||||
timeout: 35 * 60 * 1000,
|
||||
env: buildGbrainEnv({ announce: false }),
|
||||
});
|
||||
|
||||
// D6: parse [memory-ingest] lines from the child's stderr. ERR-prefixed
|
||||
|
|
|
|||
|
|
@ -64,6 +64,7 @@ import {
|
|||
detectEngineTier,
|
||||
withErrorContext,
|
||||
} from "../lib/gstack-memory-helpers";
|
||||
import { execGbrainText, spawnGbrainAsync } from "../lib/gbrain-exec";
|
||||
|
||||
// ── Types ──────────────────────────────────────────────────────────────────
|
||||
|
||||
|
|
@ -813,11 +814,10 @@ function gbrainAvailable(): boolean {
|
|||
// `import <dir>` (batch markdown import via path-authoritative slugs).
|
||||
// If absent, we surface a single clean error here rather than failing
|
||||
// the whole stage with a confusing usage message from gbrain itself.
|
||||
const help = execFileSync("gbrain", ["--help"], {
|
||||
encoding: "utf-8",
|
||||
timeout: 5000,
|
||||
stdio: ["ignore", "pipe", "pipe"],
|
||||
});
|
||||
// `gbrain --help` probes only CLI availability, not DB connectivity, so
|
||||
// it doesn't strictly need DATABASE_URL. But routing through the helper
|
||||
// keeps the invariant test from chasing exceptions per call site.
|
||||
const help = execGbrainText(["--help"], { timeout: 5000 });
|
||||
_gbrainAvailability = /^\s+import\s/m.test(help);
|
||||
} catch {
|
||||
_gbrainAvailability = false;
|
||||
|
|
@ -1316,11 +1316,11 @@ function runGbrainImport(
|
|||
): Promise<{ status: number | null; stdout: string; stderr: string }> {
|
||||
installSignalForwarder();
|
||||
return new Promise((resolve) => {
|
||||
const child = spawn(
|
||||
"gbrain",
|
||||
["import", stagingDir, "--no-embed", "--json"],
|
||||
{ stdio: ["ignore", "pipe", "pipe"] },
|
||||
);
|
||||
// Seed DATABASE_URL from gbrain's own config so this stage works
|
||||
// inside Next.js / Prisma / Rails projects with their own
|
||||
// .env.local (codex review #7 — defense in depth on top of the
|
||||
// parent gstack-gbrain-sync seeding the bun grandchild's env).
|
||||
const child = spawnGbrainAsync(["import", stagingDir, "--no-embed", "--json"]);
|
||||
_activeImportChild = child;
|
||||
let stdout = "";
|
||||
let stderr = "";
|
||||
|
|
|
|||
|
|
@ -0,0 +1,174 @@
|
|||
/**
|
||||
* Centralized gbrain CLI invocation.
|
||||
*
|
||||
* Every `gbrain ...` spawn from `bin/gstack-gbrain-sync.ts` and
|
||||
* `bin/gstack-memory-ingest.ts` MUST go through `spawnGbrain` (or
|
||||
* `execGbrainJson`), and the invariant test
|
||||
* `test/gbrain-exec-invariant.test.ts` enforces this with a static-source
|
||||
* grep. The helper layer guarantees three properties:
|
||||
*
|
||||
* 1. **DATABASE_URL is seeded from gbrain's own config**, not from the
|
||||
* caller's `.env.local`. gbrain auto-loads `.env.local` via dotenv on
|
||||
* startup. When `/sync-gbrain` runs inside a Next.js / Prisma / Rails
|
||||
* project with its own `DATABASE_URL`, gbrain reads that one and not
|
||||
* its own `${GBRAIN_HOME:-$HOME/.gbrain}/config.json`. Auth fails;
|
||||
* code + memory stages crash; only brain-sync's git push survives.
|
||||
*
|
||||
* 2. **Bun-aware env passing.** Mutating `process.env.DATABASE_URL` does
|
||||
* NOT propagate to children of `child_process.spawnSync`/`spawn` in
|
||||
* Bun — the child gets the original startup env. So we cannot just
|
||||
* set process.env; we must thread an explicit `env:` dict to every
|
||||
* spawn. This is the central bug the helper exists to prevent
|
||||
* regressing on.
|
||||
*
|
||||
* 3. **`GBRAIN_HOME` honored consistently.** Other gstack helpers
|
||||
* (`detectEngineTier`) already honor `GBRAIN_HOME`. `buildGbrainEnv`
|
||||
* reads from `${GBRAIN_HOME:-$HOME/.gbrain}/config.json` so all
|
||||
* gstack-side gbrain calls agree on which config file matters.
|
||||
*
|
||||
* **Escape hatch:** `GSTACK_RESPECT_ENV_DATABASE_URL=1` returns the
|
||||
* caller's env unchanged. Use only when the brain intentionally lives in
|
||||
* the project's local DB (rare).
|
||||
*/
|
||||
|
||||
import { existsSync, readFileSync } from "fs";
|
||||
import { join } from "path";
|
||||
import { homedir } from "os";
|
||||
import { spawnSync, spawn, execFileSync, type SpawnSyncReturns, type ChildProcess, type SpawnOptions } from "child_process";
|
||||
|
||||
interface GbrainConfig {
|
||||
database_url?: string;
|
||||
}
|
||||
|
||||
export interface BuildGbrainEnvOptions {
|
||||
/**
|
||||
* Caller env to extend. Defaults to `process.env`. Tests inject a
|
||||
* synthetic env so the helper can be exercised without polluting the
|
||||
* real process env.
|
||||
*/
|
||||
baseEnv?: NodeJS.ProcessEnv;
|
||||
/**
|
||||
* When true, announce on stderr that we overrode the caller's
|
||||
* DATABASE_URL. Suppressed for the `--quiet` sync flow.
|
||||
*/
|
||||
announce?: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build an env dict with DATABASE_URL seeded from
|
||||
* `${GBRAIN_HOME:-$HOME/.gbrain}/config.json`. Returns the base env
|
||||
* unchanged when:
|
||||
* - `GSTACK_RESPECT_ENV_DATABASE_URL=1` (intentional opt-out),
|
||||
* - the config file is missing or unparseable,
|
||||
* - the config has no `database_url`,
|
||||
* - the caller already set DATABASE_URL to the same value.
|
||||
*
|
||||
* Always returns a fresh object — mutating the returned env never
|
||||
* affects the caller's env. Tests assert on effective values, not
|
||||
* object identity.
|
||||
*/
|
||||
export function buildGbrainEnv(opts: BuildGbrainEnvOptions = {}): NodeJS.ProcessEnv {
|
||||
const baseEnv = opts.baseEnv || process.env;
|
||||
const out: NodeJS.ProcessEnv = { ...baseEnv };
|
||||
if (baseEnv.GSTACK_RESPECT_ENV_DATABASE_URL === "1") return out;
|
||||
|
||||
const homeBase = baseEnv.HOME || homedir();
|
||||
const gbrainHome = baseEnv.GBRAIN_HOME || join(homeBase, ".gbrain");
|
||||
const configPath = join(gbrainHome, "config.json");
|
||||
if (!existsSync(configPath)) return out;
|
||||
|
||||
let cfg: GbrainConfig = {};
|
||||
try {
|
||||
cfg = JSON.parse(readFileSync(configPath, "utf-8")) as GbrainConfig;
|
||||
} catch {
|
||||
return out;
|
||||
}
|
||||
if (!cfg.database_url) return out;
|
||||
if (baseEnv.DATABASE_URL === cfg.database_url) return out;
|
||||
|
||||
const hadCaller = baseEnv.DATABASE_URL !== undefined;
|
||||
out.DATABASE_URL = cfg.database_url;
|
||||
if (opts.announce) {
|
||||
const note = hadCaller ? " (overrode value from caller env / .env.local)" : "";
|
||||
process.stderr.write(`[gbrain-exec] seeded DATABASE_URL from ${configPath}${note}\n`);
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
export interface SpawnGbrainOptions {
|
||||
/** Timeout in milliseconds. Defaults to 30s. */
|
||||
timeout?: number;
|
||||
/** Working directory for the child process. */
|
||||
cwd?: string;
|
||||
/** Stdio configuration. Defaults to capturing both stdout and stderr. */
|
||||
stdio?: "inherit" | "pipe" | "ignore" | Array<"inherit" | "pipe" | "ignore">;
|
||||
/**
|
||||
* Base env to extend before seeding DATABASE_URL. Defaults to
|
||||
* `process.env`. Tests inject a synthetic env so the spawn picks up a
|
||||
* gbrain shim on PATH and a fake `~/.gbrain/config.json`.
|
||||
*/
|
||||
baseEnv?: NodeJS.ProcessEnv;
|
||||
/** Whether to announce DATABASE_URL seeding on stderr. */
|
||||
announce?: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Spawn `gbrain <args>` with the seeded env. Returns the raw
|
||||
* `SpawnSyncReturns<string>` so callers can inspect `status`, `stdout`,
|
||||
* `stderr` exactly as they would with `spawnSync` directly.
|
||||
*/
|
||||
export function spawnGbrain(args: string[], opts: SpawnGbrainOptions = {}): SpawnSyncReturns<string> {
|
||||
return spawnSync("gbrain", args, {
|
||||
encoding: "utf-8",
|
||||
timeout: opts.timeout ?? 30_000,
|
||||
cwd: opts.cwd,
|
||||
stdio: opts.stdio || ["ignore", "pipe", "pipe"],
|
||||
env: buildGbrainEnv({ baseEnv: opts.baseEnv, announce: opts.announce }),
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Run `gbrain <args>` and parse stdout as JSON. Returns `null` on
|
||||
* non-zero exit, parse failure, or timeout. Useful for `gbrain sources
|
||||
* list --json` and similar.
|
||||
*/
|
||||
export function execGbrainJson<T = unknown>(args: string[], opts: SpawnGbrainOptions = {}): T | null {
|
||||
const r = spawnGbrain(args, opts);
|
||||
if (r.status !== 0) return null;
|
||||
try {
|
||||
return JSON.parse(r.stdout || "null") as T;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Async streaming variant for callers that need to attach stdout/stderr
|
||||
* listeners (e.g., `gbrain import` in `gstack-memory-ingest.ts`). Always
|
||||
* injects the seeded env. Returns the raw `ChildProcess` so the caller
|
||||
* can wire up its own promise around exit/timeout/signal handling.
|
||||
*/
|
||||
export function spawnGbrainAsync(
|
||||
args: string[],
|
||||
opts: { stdio?: SpawnOptions["stdio"]; cwd?: string; baseEnv?: NodeJS.ProcessEnv } = {},
|
||||
): ChildProcess {
|
||||
return spawn("gbrain", args, {
|
||||
stdio: opts.stdio || ["ignore", "pipe", "pipe"],
|
||||
cwd: opts.cwd,
|
||||
env: buildGbrainEnv({ baseEnv: opts.baseEnv, announce: false }),
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Run `gbrain <args>` via execFileSync. Throws on non-zero exit. Useful
|
||||
* for callers that want to surface gbrain's stderr as the error message.
|
||||
*/
|
||||
export function execGbrainText(args: string[], opts: SpawnGbrainOptions = {}): string {
|
||||
return execFileSync("gbrain", args, {
|
||||
encoding: "utf-8",
|
||||
timeout: opts.timeout ?? 30_000,
|
||||
cwd: opts.cwd,
|
||||
stdio: opts.stdio || ["ignore", "pipe", "pipe"],
|
||||
env: buildGbrainEnv({ baseEnv: opts.baseEnv, announce: opts.announce }),
|
||||
});
|
||||
}
|
||||
|
|
@ -0,0 +1,120 @@
|
|||
/**
|
||||
* Unit tests for `buildGbrainEnv` in lib/gbrain-exec.ts.
|
||||
*
|
||||
* The helper is the single source of truth for "what DATABASE_URL does
|
||||
* gbrain see when spawned from gstack." The bug it prevents: gbrain's
|
||||
* dotenv autoload pulls a host project's `.env.local` `DATABASE_URL`
|
||||
* instead of gbrain's own `~/.gbrain/config.json`. Every helper test
|
||||
* asserts on the **effective value** of the returned env, never object
|
||||
* identity — Codex review #11 flagged that returning the same mutable
|
||||
* object can leak later mutation.
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach } from "bun:test";
|
||||
import { mkdtempSync, writeFileSync, mkdirSync, rmSync } from "fs";
|
||||
import { tmpdir } from "os";
|
||||
import { join } from "path";
|
||||
|
||||
import { buildGbrainEnv } from "../lib/gbrain-exec";
|
||||
|
||||
describe("buildGbrainEnv", () => {
|
||||
let home: string;
|
||||
let gbrainHome: string;
|
||||
|
||||
beforeEach(() => {
|
||||
home = mkdtempSync(join(tmpdir(), "gstack-build-env-"));
|
||||
gbrainHome = join(home, ".gbrain");
|
||||
mkdirSync(gbrainHome, { recursive: true });
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
rmSync(home, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("seeds DATABASE_URL from ~/.gbrain/config.json when caller env has no DATABASE_URL", () => {
|
||||
writeFileSync(join(gbrainHome, "config.json"), JSON.stringify({ database_url: "postgresql://gbrain/db" }));
|
||||
const baseEnv = { HOME: home };
|
||||
const result = buildGbrainEnv({ baseEnv });
|
||||
expect(result.DATABASE_URL).toBe("postgresql://gbrain/db");
|
||||
});
|
||||
|
||||
it("overrides caller's DATABASE_URL when config differs", () => {
|
||||
writeFileSync(join(gbrainHome, "config.json"), JSON.stringify({ database_url: "postgresql://gbrain/db" }));
|
||||
const baseEnv = { HOME: home, DATABASE_URL: "postgresql://app-local/wrong" };
|
||||
const result = buildGbrainEnv({ baseEnv });
|
||||
expect(result.DATABASE_URL).toBe("postgresql://gbrain/db");
|
||||
});
|
||||
|
||||
it("leaves DATABASE_URL untouched when GSTACK_RESPECT_ENV_DATABASE_URL=1", () => {
|
||||
writeFileSync(join(gbrainHome, "config.json"), JSON.stringify({ database_url: "postgresql://gbrain/db" }));
|
||||
const baseEnv = {
|
||||
HOME: home,
|
||||
DATABASE_URL: "postgresql://intentional/app-db",
|
||||
GSTACK_RESPECT_ENV_DATABASE_URL: "1",
|
||||
};
|
||||
const result = buildGbrainEnv({ baseEnv });
|
||||
expect(result.DATABASE_URL).toBe("postgresql://intentional/app-db");
|
||||
});
|
||||
|
||||
it("returns caller env unchanged when config file is missing", () => {
|
||||
// No config.json written.
|
||||
const baseEnv = { HOME: home, DATABASE_URL: "postgresql://app/db" };
|
||||
const result = buildGbrainEnv({ baseEnv });
|
||||
expect(result.DATABASE_URL).toBe("postgresql://app/db");
|
||||
});
|
||||
|
||||
it("returns caller env unchanged when config file is unparseable", () => {
|
||||
writeFileSync(join(gbrainHome, "config.json"), "{not json");
|
||||
const baseEnv = { HOME: home, DATABASE_URL: "postgresql://app/db" };
|
||||
const result = buildGbrainEnv({ baseEnv });
|
||||
expect(result.DATABASE_URL).toBe("postgresql://app/db");
|
||||
});
|
||||
|
||||
it("returns caller env unchanged when config has no database_url field", () => {
|
||||
writeFileSync(join(gbrainHome, "config.json"), JSON.stringify({ engine: "pglite" }));
|
||||
const baseEnv = { HOME: home, DATABASE_URL: "postgresql://app/db" };
|
||||
const result = buildGbrainEnv({ baseEnv });
|
||||
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.
|
||||
const altGbrainHome = join(home, "alt-gbrain");
|
||||
mkdirSync(altGbrainHome, { recursive: true });
|
||||
writeFileSync(join(altGbrainHome, "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("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.
|
||||
writeFileSync(join(gbrainHome, "config.json"), JSON.stringify({ database_url: "postgresql://gbrain/db" }));
|
||||
const baseEnv: NodeJS.ProcessEnv = { HOME: home, FOO: "bar" };
|
||||
const result = buildGbrainEnv({ baseEnv });
|
||||
expect(result).not.toBe(baseEnv);
|
||||
// Mutating result must not affect baseEnv.
|
||||
result.FOO = "changed";
|
||||
expect(baseEnv.FOO).toBe("bar");
|
||||
});
|
||||
|
||||
it("preserves unrelated env vars from the base env", () => {
|
||||
writeFileSync(join(gbrainHome, "config.json"), JSON.stringify({ database_url: "postgresql://gbrain/db" }));
|
||||
const baseEnv = { HOME: home, PATH: "/usr/bin", FOO: "bar" };
|
||||
const result = buildGbrainEnv({ baseEnv });
|
||||
expect(result.PATH).toBe("/usr/bin");
|
||||
expect(result.FOO).toBe("bar");
|
||||
expect(result.HOME).toBe(home);
|
||||
});
|
||||
|
||||
it("does not modify DATABASE_URL when caller's value already matches config", () => {
|
||||
// Subtle: helper should be a no-op when caller already has the right value.
|
||||
// Lets us skip the stderr announce on idempotent re-invocation.
|
||||
writeFileSync(join(gbrainHome, "config.json"), JSON.stringify({ database_url: "postgresql://gbrain/db" }));
|
||||
const baseEnv = { HOME: home, DATABASE_URL: "postgresql://gbrain/db" };
|
||||
const result = buildGbrainEnv({ baseEnv });
|
||||
expect(result.DATABASE_URL).toBe("postgresql://gbrain/db");
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,80 @@
|
|||
/**
|
||||
* Static-source invariant: every gbrain CLI invocation in the hot-path
|
||||
* sync code MUST route through `lib/gbrain-exec.ts` (or accept env via
|
||||
* the existing `lib/gbrain-sources.ts` opts surface). A future contributor
|
||||
* who adds a `spawnSync("gbrain", ...)` call directly in
|
||||
* `bin/gstack-gbrain-sync.ts` or `bin/gstack-memory-ingest.ts` silently
|
||||
* regresses the DATABASE_URL fix from #1508 + codex review #7 — gbrain's
|
||||
* dotenv autoload pulls a host project's `.env.local` value instead of
|
||||
* gbrain's own config.
|
||||
*
|
||||
* This test reads each source file directly and asserts zero direct
|
||||
* `spawnSync("gbrain"`, `spawn("gbrain"`, `execFileSync("gbrain"`, or
|
||||
* `execSync(...gbrain` matches. Bun runs TS directly so there is no
|
||||
* compiled artifact to grep — the .ts source is the truth.
|
||||
*
|
||||
* The check is intentionally narrow: only the two files where the bug
|
||||
* actually hurts users are guarded. Other gbrain spawn sites
|
||||
* (`lib/gbrain-sources.ts`, `lib/gbrain-local-status.ts`,
|
||||
* `lib/gstack-memory-helpers.ts`, `bin/gstack-brain-context-load.ts`)
|
||||
* either already accept env from callers or run probes that don't need
|
||||
* DATABASE_URL. Expanding the invariant to those files is a follow-up.
|
||||
*/
|
||||
|
||||
import { describe, it, expect } from "bun:test";
|
||||
import { readFileSync } from "fs";
|
||||
import { join } from "path";
|
||||
|
||||
const ROOT = join(import.meta.dir, "..");
|
||||
|
||||
const GUARDED_FILES = [
|
||||
"bin/gstack-gbrain-sync.ts",
|
||||
"bin/gstack-memory-ingest.ts",
|
||||
];
|
||||
|
||||
// Patterns that would bypass lib/gbrain-exec.ts. Match the literal `"gbrain"`
|
||||
// as the first argument since these helpers are the failure mode.
|
||||
const BANNED_PATTERNS: Array<{ name: string; regex: RegExp }> = [
|
||||
{ name: 'spawnSync("gbrain", ...)', regex: /spawnSync\s*\(\s*["']gbrain["']/g },
|
||||
{ name: 'spawn("gbrain", ...)', regex: /\bspawn\s*\(\s*["']gbrain["']/g },
|
||||
{ name: 'execFileSync("gbrain", ...)', regex: /execFileSync\s*\(\s*["']gbrain["']/g },
|
||||
{ name: 'execSync("...gbrain...")', regex: /execSync\s*\(\s*["'`][^"'`]*\bgbrain\b/g },
|
||||
];
|
||||
|
||||
describe("gbrain-exec invariant", () => {
|
||||
for (const relpath of GUARDED_FILES) {
|
||||
it(`${relpath} routes every gbrain spawn through lib/gbrain-exec.ts`, () => {
|
||||
const source = readFileSync(join(ROOT, relpath), "utf-8");
|
||||
// Strip block comments and line comments before scanning — a
|
||||
// documentation reference like `// spawnSync("gbrain", ...)` in a
|
||||
// comment shouldn't trip the invariant. The strip is approximate
|
||||
// (sufficient for the patterns we care about); production code
|
||||
// should match cleanly.
|
||||
const stripped = source
|
||||
.replace(/\/\*[\s\S]*?\*\//g, "")
|
||||
.replace(/\/\/.*$/gm, "");
|
||||
|
||||
for (const { name, regex } of BANNED_PATTERNS) {
|
||||
const matches = stripped.match(regex) || [];
|
||||
if (matches.length > 0) {
|
||||
// Find the line numbers to make the failure actionable.
|
||||
const lines = stripped.split("\n");
|
||||
const hits: string[] = [];
|
||||
for (let i = 0; i < lines.length; i++) {
|
||||
if (new RegExp(regex.source).test(lines[i])) {
|
||||
hits.push(` ${relpath}:${i + 1}: ${lines[i].trim()}`);
|
||||
}
|
||||
}
|
||||
throw new Error(
|
||||
`Found ${matches.length} direct gbrain invocation(s) in ${relpath} matching \`${name}\`:\n${hits.join("\n")}\n\n`
|
||||
+ `Route every gbrain spawn through \`spawnGbrain\`/\`execGbrainJson\`/\`execGbrainText\` `
|
||||
+ `in lib/gbrain-exec.ts so DATABASE_URL is seeded from gbrain's config.`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Positive assertion: the file should import from lib/gbrain-exec.
|
||||
expect(source).toMatch(/from\s+["']\.\.\/lib\/gbrain-exec["']/);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
|
@ -425,7 +425,10 @@ describe("gstack-memory-ingest writer (gbrain v0.20+ batch `import` interface)",
|
|||
const source = readFileSync(SCRIPT, "utf-8");
|
||||
|
||||
expect(source).not.toContain('command -v gbrain');
|
||||
expect(source).toContain('execFileSync("gbrain", ["--help"]');
|
||||
// v1.40.0.0: probe routes through lib/gbrain-exec.ts's execGbrainText helper
|
||||
// (codex review #4 — centralized gbrain spawn surface). Pre-v1.40 the call
|
||||
// was a direct `execFileSync("gbrain", ["--help"], ...)` inline.
|
||||
expect(source).toContain('execGbrainText(["--help"]');
|
||||
});
|
||||
|
||||
it("invokes `gbrain import <dir> --no-embed --json` exactly once with hierarchical staging", () => {
|
||||
|
|
|
|||
Loading…
Reference in New Issue