mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-09 14:38:59 +02:00
fix(sync): run gstack-brain-sync through bash, not cmd.exe, on Windows
The brain-sync stage failed on EVERY Windows run with "is not recognized as an internal or external command", so /sync-gbrain always reported ERR brain-sync among otherwise green stages. #1731 gave these spawns shell: NEEDS_SHELL_ON_WINDOWS. That is correct for the gbrain.cmd shim and does nothing here: shell:true routes through cmd.exe, which resolves .cmd/.bat via PATHEXT but has no concept of a shebang, so an extension-less bash script is rejected outright. A .cmd shim needs a shell; a shebang script needs an interpreter. The two cases look identical and are not. The failure was quiet rather than loud. artifacts_sync_mode defaults to pushing curated artifacts to git, so a Windows user's learnings piled up uncommitted in ~/.gstack indefinitely while the sync report showed one red line out of four. New bashScriptInvocation() resolves Git for Windows' bash explicitly and passes the script as argv[0]. It prefers Git bash over a bare `bash` on PATH because WindowsApps ships a bash.exe that is the WSL launcher, which would read C:\... as a Linux path; GSTACK_BASH overrides for unusual installs; forward slashes because bash treats backslashes as escapes; and it returns null when no bash exists so the stage says so plainly instead of surfacing an unactionable spawn error. The #1731 tripwire asserted the shape that does not work, so it now asserts the opposite (never a raw spawnSync(brainSyncPath, ...)) and six unit tests cover the resolver. Verified on Windows: the stage now reports "OK brain-sync curated artifacts pushed (4.2s)" and the artifacts repo committed + pushed on its own. Affected-test set unchanged at 14 pre-existing failures before and after, with 6 new passing tests.
This commit is contained in:
+26
-13
@@ -41,7 +41,7 @@ import { ensureSourceRegistered, sourcePageCount, parseSourcesList, cycleComplet
|
||||
import { detectAutopilot, decideSourceRemove, decideCodeSync } from "../lib/gbrain-guards";
|
||||
import { writeReceipt } from "../lib/egress-receipt";
|
||||
import { localEngineStatus, type LocalEngineStatus } from "../lib/gbrain-local-status";
|
||||
import { buildGbrainEnv, spawnGbrain, execGbrainJson, NEEDS_SHELL_ON_WINDOWS } from "../lib/gbrain-exec";
|
||||
import { buildGbrainEnv, spawnGbrain, execGbrainJson, NEEDS_SHELL_ON_WINDOWS, bashScriptInvocation } from "../lib/gbrain-exec";
|
||||
import { repoPolicyTier as sharedRepoPolicyTier } from "../lib/gbrain-repo-policy-client";
|
||||
import { checkOwnedStagingDir } from "../lib/staging-guard";
|
||||
|
||||
@@ -1245,18 +1245,31 @@ function runBrainSyncPush(args: CliArgs): StageResult {
|
||||
return { name: "brain-sync", ran: false, ok: true, duration_ms: 0, summary: "skipped (gstack-brain-sync not installed)" };
|
||||
}
|
||||
|
||||
// #1731: gstack-brain-sync is a bash shebang script; Windows can't spawn it
|
||||
// without a shell, which surfaced as "brain-sync exited undefined".
|
||||
spawnSync(brainSyncPath, ["--discover-new"], {
|
||||
stdio: args.quiet ? ["ignore", "ignore", "ignore"] : ["ignore", "inherit", "inherit"],
|
||||
timeout: 60 * 1000,
|
||||
shell: NEEDS_SHELL_ON_WINDOWS,
|
||||
});
|
||||
const result = spawnSync(brainSyncPath, ["--once"], {
|
||||
stdio: args.quiet ? ["ignore", "ignore", "ignore"] : ["ignore", "inherit", "inherit"],
|
||||
timeout: 60 * 1000,
|
||||
shell: NEEDS_SHELL_ON_WINDOWS,
|
||||
});
|
||||
// gstack-brain-sync is a bash shebang script, so it needs an INTERPRETER, not
|
||||
// a shell. #1731 gave it `shell: NEEDS_SHELL_ON_WINDOWS`, which is right for
|
||||
// the gbrain.cmd shim and useless here: cmd.exe resolves .cmd/.bat via PATHEXT
|
||||
// and rejects an extension-less shebang script outright ("is not recognized as
|
||||
// an internal or external command"), so this stage failed on EVERY Windows run
|
||||
// while looking like a single red line in an otherwise green report. See
|
||||
// bashScriptInvocation.
|
||||
const discover = bashScriptInvocation(brainSyncPath, ["--discover-new"]);
|
||||
const once = bashScriptInvocation(brainSyncPath, ["--once"]);
|
||||
if (!discover || !once) {
|
||||
return {
|
||||
name: "brain-sync",
|
||||
ran: false,
|
||||
ok: true,
|
||||
duration_ms: Date.now() - t0,
|
||||
summary: "skipped (no bash found; set GSTACK_BASH to your Git bash.exe)",
|
||||
};
|
||||
}
|
||||
|
||||
const stdio: "ignore"[] | ("ignore" | "inherit")[] = args.quiet
|
||||
? ["ignore", "ignore", "ignore"]
|
||||
: ["ignore", "inherit", "inherit"];
|
||||
|
||||
spawnSync(discover.cmd, discover.argv, { stdio, timeout: 60 * 1000, shell: discover.shell });
|
||||
const result = spawnSync(once.cmd, once.argv, { stdio, timeout: 60 * 1000, shell: once.shell });
|
||||
|
||||
return {
|
||||
name: "brain-sync",
|
||||
|
||||
@@ -136,6 +136,66 @@ export function buildGbrainEnv(opts: BuildGbrainEnvOptions = {}): NodeJS.Process
|
||||
*/
|
||||
export const NEEDS_SHELL_ON_WINDOWS = process.platform === "win32";
|
||||
|
||||
/** Where Git for Windows puts bash, most-specific first. */
|
||||
const WINDOWS_BASH_CANDIDATES = [
|
||||
"C:\\Program Files\\Git\\bin\\bash.exe",
|
||||
"C:\\Program Files\\Git\\usr\\bin\\bash.exe",
|
||||
"C:\\Program Files (x86)\\Git\\bin\\bash.exe",
|
||||
];
|
||||
|
||||
export interface ScriptInvocation {
|
||||
cmd: string;
|
||||
argv: string[];
|
||||
/** Always false: we resolve the interpreter ourselves rather than via cmd.exe. */
|
||||
shell: false;
|
||||
}
|
||||
|
||||
/**
|
||||
* How to invoke a **bash shebang script** (`gstack-brain-sync`) on this platform.
|
||||
*
|
||||
* POSIX execs it directly — the shebang does the work. Windows cannot, and
|
||||
* `shell: true` does NOT rescue it: that routes through cmd.exe, which resolves
|
||||
* `.cmd`/`.bat` via PATHEXT but has no concept of a shebang, so an
|
||||
* extension-less bash script comes back as *"is not recognized as an internal
|
||||
* or external command"*. This is why #1731's `shell: NEEDS_SHELL_ON_WINDOWS`
|
||||
* fix genuinely cured the `gbrain.cmd` shim while leaving the brain-sync stage
|
||||
* failing on **every** run on Windows. The two cases look identical and are not:
|
||||
* a `.cmd` shim needs a shell, a shebang script needs an interpreter.
|
||||
*
|
||||
* The consequence was quiet rather than loud. `artifacts_sync_mode` defaults to
|
||||
* pushing curated artifacts to git, so a Windows user's learnings accumulated in
|
||||
* `~/.gstack` and were never committed, while `/sync-gbrain` printed one red
|
||||
* line among four green ones.
|
||||
*
|
||||
* Git for Windows' bash is preferred over a bare `bash` on PATH because
|
||||
* WindowsApps ships a `bash.exe` that is the WSL launcher; if it wins PATH
|
||||
* order it interprets `C:\...` as a Linux path and the script never sees the
|
||||
* repo. `GSTACK_BASH` overrides everything for unusual installs.
|
||||
*
|
||||
* Returns `null` when no bash can be found, so the caller can say so plainly
|
||||
* instead of surfacing a spawn error nobody can act on.
|
||||
*/
|
||||
export function bashScriptInvocation(
|
||||
scriptPath: string,
|
||||
args: string[],
|
||||
opts: { platform?: string; exists?: (p: string) => boolean; env?: NodeJS.ProcessEnv } = {},
|
||||
): ScriptInvocation | null {
|
||||
const platform = opts.platform ?? process.platform;
|
||||
if (platform !== "win32") return { cmd: scriptPath, argv: args, shell: false };
|
||||
|
||||
const exists = opts.exists ?? existsSync;
|
||||
const env = opts.env ?? process.env;
|
||||
|
||||
const override = env.GSTACK_BASH?.trim();
|
||||
const candidates = [...(override ? [override] : []), ...WINDOWS_BASH_CANDIDATES];
|
||||
const bash = candidates.find((p) => exists(p));
|
||||
if (!bash) return null;
|
||||
|
||||
// Forward slashes: bash treats backslashes as escapes, so a Windows path
|
||||
// passed verbatim loses its separators.
|
||||
return { cmd: bash, argv: [scriptPath.replace(/\\/g, "/"), ...args], shell: false };
|
||||
}
|
||||
|
||||
export interface SpawnGbrainOptions {
|
||||
/** Timeout in milliseconds. Defaults to 30s. */
|
||||
timeout?: number;
|
||||
|
||||
@@ -2,6 +2,8 @@ import { describe, test, expect } from "bun:test";
|
||||
import * as fs from "fs";
|
||||
import * as path from "path";
|
||||
|
||||
import { bashScriptInvocation } from "../lib/gbrain-exec";
|
||||
|
||||
const ROOT = path.resolve(import.meta.dir, "..");
|
||||
const read = (rel: string) => fs.readFileSync(path.join(ROOT, rel), "utf-8");
|
||||
|
||||
@@ -34,12 +36,82 @@ describe("#1731 gbrain spawns carry the Windows shell flag", () => {
|
||||
});
|
||||
}
|
||||
|
||||
test("orchestrator brain-sync spawns carry the Windows shell flag", () => {
|
||||
// NOT the brain-sync script. `shell: true` is right for the gbrain.cmd shim
|
||||
// and wrong for a bash shebang script: cmd.exe resolves .cmd/.bat via PATHEXT
|
||||
// and has no concept of a shebang, so gstack-brain-sync came back as "is not
|
||||
// recognized as an internal or external command" on EVERY Windows run. It
|
||||
// needs an interpreter, not a shell — see bashScriptInvocation.
|
||||
test("orchestrator invokes brain-sync through bash, never a raw spawn", () => {
|
||||
const src = read("bin/gstack-gbrain-sync.ts");
|
||||
const brainSyncSpawns = src.match(/spawnSync\(brainSyncPath,/g)?.length ?? 0;
|
||||
expect(brainSyncSpawns).toBe(2);
|
||||
// Both spawnSync(brainSyncPath, ...) blocks must include the shell flag.
|
||||
const withShell = src.match(/spawnSync\(brainSyncPath,[\s\S]*?shell:\s*NEEDS_SHELL_ON_WINDOWS/g)?.length ?? 0;
|
||||
expect(withShell).toBe(2);
|
||||
expect(src).toMatch(/bashScriptInvocation\(brainSyncPath, \["--discover-new"\]\)/);
|
||||
expect(src).toMatch(/bashScriptInvocation\(brainSyncPath, \["--once"\]\)/);
|
||||
// The old shape must not come back: it fails silently-ish on Windows.
|
||||
expect(src).not.toMatch(/spawnSync\(brainSyncPath,/);
|
||||
expect(src).not.toMatch(/spawnSync\(brainSyncPath,[\s\S]*?shell:\s*NEEDS_SHELL_ON_WINDOWS/);
|
||||
});
|
||||
});
|
||||
|
||||
describe("bashScriptInvocation", () => {
|
||||
const WIN_BASH = "C:\\Program Files\\Git\\bin\\bash.exe";
|
||||
|
||||
test("POSIX execs the script directly, no interpreter needed", () => {
|
||||
const inv = bashScriptInvocation("/home/u/.claude/skills/gstack/bin/gstack-brain-sync", ["--once"], {
|
||||
platform: "linux",
|
||||
});
|
||||
expect(inv).toEqual({
|
||||
cmd: "/home/u/.claude/skills/gstack/bin/gstack-brain-sync",
|
||||
argv: ["--once"],
|
||||
shell: false,
|
||||
});
|
||||
});
|
||||
|
||||
test("Windows routes through Git bash with the script as argv[0]", () => {
|
||||
const inv = bashScriptInvocation("C:\\Users\\u\\.claude\\skills\\gstack\\bin\\gstack-brain-sync", ["--once"], {
|
||||
platform: "win32",
|
||||
exists: (p) => p === WIN_BASH,
|
||||
env: {},
|
||||
});
|
||||
expect(inv?.cmd).toBe(WIN_BASH);
|
||||
expect(inv?.argv[1]).toBe("--once");
|
||||
});
|
||||
|
||||
test("Windows forward-slashes the script path", () => {
|
||||
// bash treats backslashes as escapes, so a verbatim Windows path loses its
|
||||
// separators and the script is never found.
|
||||
const inv = bashScriptInvocation("C:\\Users\\u\\bin\\gstack-brain-sync", [], {
|
||||
platform: "win32",
|
||||
exists: (p) => p === WIN_BASH,
|
||||
env: {},
|
||||
});
|
||||
expect(inv?.argv[0]).toBe("C:/Users/u/bin/gstack-brain-sync");
|
||||
expect(inv?.argv[0]).not.toContain("\\");
|
||||
});
|
||||
|
||||
test("never asks for a shell — cmd.exe is what broke this", () => {
|
||||
const inv = bashScriptInvocation("C:\\x\\gstack-brain-sync", [], {
|
||||
platform: "win32",
|
||||
exists: (p) => p === WIN_BASH,
|
||||
env: {},
|
||||
});
|
||||
expect(inv?.shell).toBe(false);
|
||||
});
|
||||
|
||||
test("GSTACK_BASH overrides the search for unusual installs", () => {
|
||||
const custom = "D:\\tools\\git\\bin\\bash.exe";
|
||||
const inv = bashScriptInvocation("C:\\x\\gstack-brain-sync", [], {
|
||||
platform: "win32",
|
||||
exists: (p) => p === custom || p === WIN_BASH,
|
||||
env: { GSTACK_BASH: custom },
|
||||
});
|
||||
expect(inv?.cmd).toBe(custom);
|
||||
});
|
||||
|
||||
test("returns null when Windows has no bash, so the caller can say why", () => {
|
||||
const inv = bashScriptInvocation("C:\\x\\gstack-brain-sync", [], {
|
||||
platform: "win32",
|
||||
exists: () => false,
|
||||
env: {},
|
||||
});
|
||||
expect(inv).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user