From 7f3119a29f0633c71a10bef4b48d768210041e98 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Thu, 27 Aug 2026 12:15:21 -0400 Subject: [PATCH] fix(#3729): hoist the .planning and CI gates above the graphify hook's node parse (#3935) * test(#3729): non-GSD repo and CI must bail before the graphify hook node parse * fix(#3729): hoist the .planning and CI gates above the graphify hook's node parse * chore(#3729): changeset fragment (pr number backfilled after PR creation) * chore(#3729): backfill changeset PR number (3935) --------- Co-authored-by: sim --- .changeset/daring-tigers-squeak.md | 5 ++ hooks/gsd-graphify-update.sh | 40 +++++++------- tests/graphify-auto-update.slow.test.cjs | 67 ++++++++++++++++++++++++ 3 files changed, 94 insertions(+), 18 deletions(-) create mode 100644 .changeset/daring-tigers-squeak.md diff --git a/.changeset/daring-tigers-squeak.md b/.changeset/daring-tigers-squeak.md new file mode 100644 index 000000000..6fcbdb48e --- /dev/null +++ b/.changeset/daring-tigers-squeak.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3935 +--- +**No more console-window flash on Windows in non-GSD repositories** — the graphify auto-update hook ran a hidden `node` process to parse its payload before checking whether the project uses GSD at all; the cheap `.planning/config.json` and `CI` checks now run first, so non-GSD projects and CI pay for zero child processes per Bash tool call. (#3729) diff --git a/hooks/gsd-graphify-update.sh b/hooks/gsd-graphify-update.sh index 295c73273..b82cc3d70 100755 --- a/hooks/gsd-graphify-update.sh +++ b/hooks/gsd-graphify-update.sh @@ -9,17 +9,20 @@ # graphify.auto_update defaults to false so existing users see no behavior change. # # Gates (in fast-fail order — each shaves work off the common non-dispatch path): +# 0. .planning/config.json exists AND $CI unset/empty (#3729 — both are +# parse-free shell tests; running them before the Gate 1 node spawn keeps +# non-GSD repositories and CI off the spawn path entirely, which on +# Windows otherwise flashes a console window per Bash call) # 1. Stdin payload present and tool_name == "Bash" # 2. tool_input.command matches a HEAD-advancing git op (shell-direct or # the exact `gsd-tools query commit` command shape; the SDK command invokes # git internally, so the literal "git commit" substring never appears — # see #3653) -# 3. $CI is unset/empty -# 4. Inside a git repo -# 5. Current branch == default branch (git.base_branch override, else main/master/trunk) -# 6. .planning/config.json sets graphify.enabled=true AND graphify.auto_update=true -# 7. graphify binary on PATH -# 8. No rebuild already in flight (PID lock — kill -0 check, stale-tolerant) +# 3. Inside a git repo +# 4. Current branch == default branch (git.base_branch override, else main/master/trunk) +# 5. .planning/config.json sets graphify.enabled=true AND graphify.auto_update=true +# 6. graphify binary on PATH +# 7. No rebuild already in flight (PID lock — kill -0 check, stale-tolerant) # # When all gates pass: # - Writes .planning/graphs/.last-build-status.json with status="running" @@ -30,6 +33,13 @@ set -uo pipefail +# Gate 0 — GSD project at all, and not CI (#3729). Both are pure shell tests; +# they must precede the Gate 1 node spawn so the common non-GSD/CI path pays +# for zero child processes. Hoisting is behavior-preserving: old Gates 3 and 6 +# made the same decisions, just later. +[ -f .planning/config.json ] || exit 0 +[ -z "${CI:-}" ] || exit 0 + # Gate 1 — tool_name == Bash; extract command INPUT=$(cat 2>/dev/null || true) [ -n "$INPUT" ] || exit 0 @@ -71,22 +81,17 @@ case "$COMMAND" in *) exit 0 ;; esac -# Gate 3 — not CI -[ -z "${CI:-}" ] || exit 0 - -# Gate 4 — inside git repo +# Gate 3 — inside git repo git rev-parse --git-dir >/dev/null 2>&1 || exit 0 -# Gate 5 — current branch == default branch +# Gate 4 — current branch == default branch (config guaranteed by Gate 0) DEFAULT_BRANCH="" -if [ -f .planning/config.json ]; then - DEFAULT_BRANCH=$(node -e ' +DEFAULT_BRANCH=$(node -e ' try { const c = require("./.planning/config.json"); process.stdout.write(c.git?.base_branch || ""); } catch { process.stdout.write(""); } ' 2>/dev/null || echo "") -fi if [ -z "$DEFAULT_BRANCH" ]; then for cand in main master trunk; do if git rev-parse --verify "$cand" >/dev/null 2>&1; then @@ -100,8 +105,7 @@ fi CURRENT_BRANCH=$(git rev-parse --abbrev-ref HEAD 2>/dev/null || echo "") [ "$CURRENT_BRANCH" = "$DEFAULT_BRANCH" ] || exit 0 -# Gate 6 — both graphify gates true in config -[ -f .planning/config.json ] || exit 0 +# Gate 5 — both graphify gates true in config (file existence checked at Gate 0) GATES=$(node -e ' try { const c = require("./.planning/config.json"); @@ -111,11 +115,11 @@ try { ' 2>/dev/null || echo "0") [ "$GATES" = "1" ] || exit 0 -# Gate 7 — graphify on PATH +# Gate 6 — graphify on PATH GRAPHIFY_BIN=$(command -v graphify 2>/dev/null || true) [ -n "$GRAPHIFY_BIN" ] || exit 0 -# Gate 8 — no live rebuild in flight +# Gate 7 — no live rebuild in flight mkdir -p .planning/graphs LOCK_FILE=".planning/graphs/.rebuild.lock" if [ -f "$LOCK_FILE" ]; then diff --git a/tests/graphify-auto-update.slow.test.cjs b/tests/graphify-auto-update.slow.test.cjs index 75b369be7..39a7ce10f 100644 --- a/tests/graphify-auto-update.slow.test.cjs +++ b/tests/graphify-auto-update.slow.test.cjs @@ -428,6 +428,73 @@ describe('auto-update', () => { ); assert.strictEqual(r.status, 0, 'must not break commits when graphify missing'); }); + + // #3729 — a sentinel `node` shim records every spawn. In a non-GSD repo the + // hook must bail at Gate 0 ([ -f .planning/config.json ]) without spawning + // node at all: pre-fix, the Gate 1 payload parse spawned node first, which + // on Windows allocates a console window per Bash tool call. + function makeSentinelNodeBin(markerPath) { + const binDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-3729-node-')); + const shim = path.join(binDir, 'node'); + fs.writeFileSync( + shim, + [ + '#!/usr/bin/env bash', + `printf 'x' >> ${JSON.stringify(markerPath)}`, + 'exit 1', + ].join('\n') + '\n', + { mode: 0o755 }, + ); + return { binDir, markerPath }; + } + + test('non-GSD repo bails at Gate 0 with zero node spawns (#3729)', (t) => { + const tmpDir = createTempGitRepo({ config: undefined }); + const sentinel = makeSentinelNodeBin(path.join(tmpDir, '.node-spawned')); + t.after(() => { + cleanupHookRepo(tmpDir); + cleanup(sentinel.binDir); + }); + const r = runHook( + tmpDir, + { tool_name: 'Bash', tool_input: { command: 'git commit -m x' } }, + { pathPrepend: sentinel.binDir }, + ); + assert.strictEqual(r.status, 0, 'hook must exit 0 in a non-GSD repo'); + assert.ok( + !fs.existsSync(path.join(tmpDir, '.planning/graphs/.last-build-status.json')), + 'no status file should be created in a non-GSD repo', + ); + assert.ok( + !fs.existsSync(sentinel.markerPath), + 'hook must not spawn node before the Gate 0 .planning/config.json bail (#3729)', + ); + }); + + test('CI set bails before the node parse in a GSD project (#3729)', (t) => { + const tmpDir = createTempGitRepo({ + config: { graphify: { enabled: true, auto_update: true } }, + }); + const sentinel = makeSentinelNodeBin(path.join(tmpDir, '.node-spawned')); + t.after(() => { + cleanupHookRepo(tmpDir); + cleanup(sentinel.binDir); + }); + const r = runHook( + tmpDir, + { tool_name: 'Bash', tool_input: { command: 'git commit -m x' } }, + { env: { CI: 'true' }, pathPrepend: sentinel.binDir }, + ); + assert.strictEqual(r.status, 0); + assert.ok( + !fs.existsSync(path.join(tmpDir, '.planning/graphs/.last-build-status.json')), + 'CI must suppress dispatch', + ); + assert.ok( + !fs.existsSync(sentinel.markerPath), + 'CI gate must run before the Gate 1 node parse (#3729)', + ); + }); }); describe('hook — dispatch path (all gates pass)',