From ea780fed61ef246ee21a843639706c99dcc698cc Mon Sep 17 00:00:00 2001 From: ShahriarLak Date: Mon, 10 Aug 2026 21:16:04 +0100 Subject: [PATCH] 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. --- bin/gstack-gbrain-sync.ts | 39 ++++++++---- lib/gbrain-exec.ts | 60 ++++++++++++++++++ test/gbrain-spawn-windows-shell.test.ts | 84 +++++++++++++++++++++++-- 3 files changed, 164 insertions(+), 19 deletions(-) diff --git a/bin/gstack-gbrain-sync.ts b/bin/gstack-gbrain-sync.ts index 4cf6709df..cb4497d2d 100644 --- a/bin/gstack-gbrain-sync.ts +++ b/bin/gstack-gbrain-sync.ts @@ -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", diff --git a/lib/gbrain-exec.ts b/lib/gbrain-exec.ts index 0cb1ddc57..188d84b8f 100644 --- a/lib/gbrain-exec.ts +++ b/lib/gbrain-exec.ts @@ -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; diff --git a/test/gbrain-spawn-windows-shell.test.ts b/test/gbrain-spawn-windows-shell.test.ts index d968d2f68..eb319684e 100644 --- a/test/gbrain-spawn-windows-shell.test.ts +++ b/test/gbrain-spawn-windows-shell.test.ts @@ -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(); }); });