From c21249cbddb4daca92c8b903e51f6c2b4e178366 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Thu, 11 Jun 2026 20:29:19 -0700 Subject: [PATCH] fix(bins): cygpath-normalize SCRIPT_DIR for bun imports; surface learnings-log errors (#1950) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Under Windows git-bash, pwd yields a POSIX path (/c/Users/...) that Bun on Windows cannot resolve as an ES module specifier. gstack-learnings-log interpolates SCRIPT_DIR into a bun -e import, so every invocation died with "Cannot find module" — and 2>/dev/null swallowed the error, silently dropping every AI-logged learning for Windows users. - 3-line cygpath -m guard in gstack-learnings-log and gstack-question-log (which gains the same import shape in the next commit). Matches the duplicated IS_WINDOWS convention in setup; no shared shell lib exists. - learnings-log adopts question-log's set +e / TMPERR capture pattern wholesale: validation errors now print to stderr. The old `if [ $? -ne 0 ]` check was dead code under set -euo pipefail — the script exited at the failing assignment before reaching it. - New test/bin-windows-bun-import-paths.test.ts: static invariant (any bash bin interpolating $SCRIPT_DIR into a bun -e import must carry the guard) + behavioral end-to-end run invoked via `bash ` — added to the windows-free-tests workflow list so the conversion is proven on the only platform where the bug exists. Co-Authored-By: Claude Fable 5 --- .github/workflows/windows-free-tests.yml | 1 + bin/gstack-learnings-log | 22 +++- bin/gstack-question-log | 6 ++ test/bin-windows-bun-import-paths.test.ts | 119 ++++++++++++++++++++++ 4 files changed, 145 insertions(+), 3 deletions(-) create mode 100644 test/bin-windows-bun-import-paths.test.ts diff --git a/.github/workflows/windows-free-tests.yml b/.github/workflows/windows-free-tests.yml index 69e24c7e3..c05a7d002 100644 --- a/.github/workflows/windows-free-tests.yml +++ b/.github/workflows/windows-free-tests.yml @@ -114,6 +114,7 @@ jobs: browse/test/security.test.ts \ browse/test/server-sanitize-surrogates.test.ts \ test/setup-windows-fallback.test.ts \ + test/bin-windows-bun-import-paths.test.ts \ test/build-script-shell-compat.test.ts \ test/docs-config-keys.test.ts \ test/brain-sync-windows-paths.test.ts \ diff --git a/bin/gstack-learnings-log b/bin/gstack-learnings-log index ff544237d..8c946a2e4 100755 --- a/bin/gstack-learnings-log +++ b/bin/gstack-learnings-log @@ -7,13 +7,24 @@ # by gstack-learnings-search ("latest winner" per key+type). set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +# Windows git-bash (#1950): pwd yields a POSIX path (/c/Users/...), which Bun +# on Windows cannot resolve as an ES module specifier in the import below. +# cygpath -m converts to C:/Users/... which Bun accepts. +case "$(uname -s)" in + MINGW*|MSYS*|CYGWIN*) command -v cygpath >/dev/null 2>&1 && SCRIPT_DIR="$(cygpath -m "$SCRIPT_DIR")" ;; +esac eval "$("$SCRIPT_DIR/gstack-slug" 2>/dev/null)" GSTACK_HOME="${GSTACK_HOME:-$HOME/.gstack}" mkdir -p "$GSTACK_HOME/projects/$SLUG" INPUT="$1" -# Validate and sanitize input +# Validate and sanitize input. Errors surface (#1950): stderr is captured and +# printed on failure instead of swallowed — a silent exit 1 here cost Windows +# users every AI-logged learning. +TMPERR=$(mktemp) +trap 'rm -f "$TMPERR"' EXIT +set +e VALIDATED=$(printf '%s' "$INPUT" | bun -e " import { hasInjection } from '$SCRIPT_DIR/../lib/jsonl-store.ts'; const raw = await Bun.stdin.text(); @@ -63,9 +74,14 @@ if (!j.ts) j.ts = new Date().toISOString(); j.trusted = j.source === 'user-stated'; console.log(JSON.stringify(j)); -" 2>/dev/null) +" 2>"$TMPERR") +VALIDATE_RC=$? +set -e -if [ $? -ne 0 ] || [ -z "$VALIDATED" ]; then +if [ $VALIDATE_RC -ne 0 ] || [ -z "$VALIDATED" ]; then + if [ -s "$TMPERR" ]; then + cat "$TMPERR" >&2 + fi exit 1 fi diff --git a/bin/gstack-question-log b/bin/gstack-question-log index b8b266e8e..98ff20a1e 100755 --- a/bin/gstack-question-log +++ b/bin/gstack-question-log @@ -27,6 +27,12 @@ # Append-only JSONL. Dedup is at read time in gstack-question-sensitivity --read-log. set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +# Windows git-bash (#1950): pwd yields a POSIX path (/c/Users/...), which Bun +# on Windows cannot resolve as an ES module specifier in bun -e imports. +# cygpath -m converts to C:/Users/... which Bun accepts. +case "$(uname -s)" in + MINGW*|MSYS*|CYGWIN*) command -v cygpath >/dev/null 2>&1 && SCRIPT_DIR="$(cygpath -m "$SCRIPT_DIR")" ;; +esac eval "$("$SCRIPT_DIR/gstack-slug" 2>/dev/null)" # GSTACK_STATE_ROOT takes precedence over GSTACK_HOME (test isolation per D16). GSTACK_HOME="${GSTACK_STATE_ROOT:-${GSTACK_HOME:-$HOME/.gstack}}" diff --git a/test/bin-windows-bun-import-paths.test.ts b/test/bin-windows-bun-import-paths.test.ts new file mode 100644 index 000000000..d051f930e --- /dev/null +++ b/test/bin-windows-bun-import-paths.test.ts @@ -0,0 +1,119 @@ +/** + * #1950 — Windows git-bash POSIX paths break `bun -e` module imports. + * + * Under git-bash, `pwd` yields /c/Users/... which Bun on Windows cannot + * resolve as an ES module specifier. Any bash bin that interpolates + * $SCRIPT_DIR into a `bun -e` import must normalize it via `cygpath -m` + * first, or the bin exits 1 with "Cannot find module" — which, combined + * with stderr swallowing, silently dropped every AI-logged learning. + * + * Two layers: + * 1. Static invariant — every bash bin with a $SCRIPT_DIR bun-import + * interpolation carries the cygpath guard (catches future bins). + * 2. Behavioral — gstack-learnings-log, invoked the way Windows CI + * invokes bash bins (spawnSync("bash", [path])), writes a learning + * and surfaces validation errors on stderr instead of swallowing + * them. This file is in the windows-free-tests workflow list, so the + * cygpath conversion is proven on the only platform where #1950 + * exists. + */ + +import { describe, it, expect } from "bun:test"; +import { spawnSync } from "child_process"; +import { mkdtempSync, readdirSync, readFileSync, rmSync, statSync } from "fs"; +import { tmpdir } from "os"; +import { join } from "path"; + +const ROOT = join(import.meta.dir, ".."); +const BIN_DIR = join(ROOT, "bin"); + +const CYGPATH_GUARD = /cygpath/; +// A bun -e payload that imports through the interpolated $SCRIPT_DIR. +const BUN_IMPORT_INTERPOLATION = /bun -e "[^]*?from '\$SCRIPT_DIR\//; + +function bashBins(): string[] { + return readdirSync(BIN_DIR).filter((name) => { + const p = join(BIN_DIR, name); + if (!statSync(p).isFile()) return false; + const head = readFileSync(p, "utf-8").slice(0, 64); + return head.startsWith("#!") && head.includes("bash"); + }); +} + +describe("bin/ — Windows bun-import path guard (#1950)", () => { + it("every bash bin that interpolates $SCRIPT_DIR into a bun -e import has the cygpath guard", () => { + const offenders: string[] = []; + for (const name of bashBins()) { + const content = readFileSync(join(BIN_DIR, name), "utf-8"); + if (BUN_IMPORT_INTERPOLATION.test(content) && !CYGPATH_GUARD.test(content)) { + offenders.push(name); + } + } + expect( + offenders, + `bins interpolate $SCRIPT_DIR into a bun -e import without a cygpath guard ` + + `(breaks on Windows git-bash, #1950): ${offenders.join(", ")}`, + ).toEqual([]); + }); + + it("known-affected bins carry the guard explicitly", () => { + for (const name of ["gstack-learnings-log", "gstack-question-log"]) { + const content = readFileSync(join(BIN_DIR, name), "utf-8"); + expect(content).toContain("cygpath -m"); + } + }); +}); + +describe("gstack-learnings-log — behavioral (runs on Windows CI via git-bash)", () => { + function runViaBash(input: string, gstackHome: string) { + // spawnSync("bash", [path]) mirrors how git-bash users (and Windows CI) + // execute the bin — Windows CreateProcess cannot parse shebangs. + return spawnSync("bash", [join(BIN_DIR, "gstack-learnings-log"), input], { + encoding: "utf-8", + timeout: 20_000, + cwd: ROOT, + env: { ...process.env, GSTACK_HOME: gstackHome }, + }); + } + + it("writes a learning end-to-end (proves the bun import resolves on this platform)", () => { + const tmp = mkdtempSync(join(tmpdir(), "gstack-win-learn-")); + try { + const r = runViaBash( + JSON.stringify({ + skill: "test", + type: "operational", + key: "windows-path-check", + insight: "cygpath guard keeps the bun import resolvable", + confidence: 8, + source: "observed", + }), + tmp, + ); + expect(r.status).toBe(0); + const projects = readdirSync(join(tmp, "projects")); + expect(projects.length).toBeGreaterThan(0); + const written = readFileSync( + join(tmp, "projects", projects[0], "learnings.jsonl"), + "utf-8", + ); + expect(written).toContain("windows-path-check"); + } finally { + rmSync(tmp, { recursive: true, force: true }); + } + }); + + it("surfaces validation errors on stderr instead of swallowing them", () => { + const tmp = mkdtempSync(join(tmpdir(), "gstack-win-learn-")); + try { + const r = runViaBash( + JSON.stringify({ skill: "test", type: "not-a-type", key: "k", insight: "x", confidence: 5 }), + tmp, + ); + expect(r.status).not.toBe(0); + expect(r.stderr).toContain("invalid type"); + } finally { + rmSync(tmp, { recursive: true, force: true }); + } + }); +});