From 98cc1a84c0d6206cd5eb03a0db4c3409dcf1666d Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 12 May 2026 17:32:50 -0400 Subject: [PATCH] fix: scope Windows hook syntax to Gemini runtime (#3438) * test: cover Windows hook shell drift for Claude (#3413) * fix: scope Windows hook syntax to Gemini runtime (#3413) * docs: add changeset for #3413 * docs: set changeset pr for #3413 * fix: route Windows hook formatting through runtime-aware projection seam * test: cover runtime projection edge cases for Windows hooks * fix(docs): add shell-command-projection to inventory parity * docs: align CLI module shipped count after rebase --- .changeset/zesty-ravens-rest.md | 5 ++ bin/install.js | 42 +++++----- docs/INVENTORY-MANIFEST.json | 1 + docs/INVENTORY.md | 3 +- .../bin/lib/shell-command-projection.cjs | 52 +++++++++++++ tests/bug-2979-hook-absolute-node.test.cjs | 77 +++++++++++++++---- ...bug-3413-shell-command-projection.test.cjs | 73 ++++++++++++++++++ 7 files changed, 214 insertions(+), 39 deletions(-) create mode 100644 .changeset/zesty-ravens-rest.md create mode 100644 get-shit-done/bin/lib/shell-command-projection.cjs create mode 100644 tests/bug-3413-shell-command-projection.test.cjs diff --git a/.changeset/zesty-ravens-rest.md b/.changeset/zesty-ravens-rest.md new file mode 100644 index 000000000..c2c73a251 --- /dev/null +++ b/.changeset/zesty-ravens-rest.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3438 +--- +**Windows Claude hook commands no longer use PowerShell-only `&` syntax** — managed hooks keep Gemini's Windows PowerShell wrapper without breaking Claude Code's bash-executed hook path. diff --git a/bin/install.js b/bin/install.js index af390795c..2f0142411 100755 --- a/bin/install.js +++ b/bin/install.js @@ -5,6 +5,10 @@ const path = require('path'); const os = require('os'); const readline = require('readline'); const crypto = require('crypto'); +const { + formatHookCommandForRuntime: formatHookCommandForShell, + formatManagedHookScriptToken, +} = require('../get-shit-done/bin/lib/shell-command-projection.cjs'); // Colors const cyan = '\x1b[36m'; @@ -602,17 +606,6 @@ function resolveNodeRunner() { * * Returns true if any entry was rewritten. */ -function formatHookCommandForShell(command, opts) { - const platform = (opts && opts.platform) || process.platform; - return platform === 'win32' ? `& ${command}` : command; -} - -function formatManagedHookScriptToken(scriptPath, opts) { - const platform = (opts && opts.platform) || process.platform; - if (platform !== 'win32') return null; - return JSON.stringify(scriptPath.replace(/\\/g, '/')); -} - function resolveBashRunner(opts) { const platform = (opts && opts.platform) || process.platform; if (platform !== 'win32') return 'bash'; @@ -691,8 +684,9 @@ function rewriteLegacyManagedNodeHookCommands(settings, absoluteRunner, opts) { const runnerPath = (m[2] || m[3] || m[4] || '').replace(/\\/g, '/'); const stableRunner = normalizeNodePath(runnerPath); // Process Cellar paths so they normalize to a stable symlink. On - // Windows, also process already-absolute runners so PowerShell gets - // the call operator needed to invoke a quoted executable path (#3362). + // Windows, already-absolute runners still flow through the projection + // seam because some runtimes need additional wrapper policy while + // others must stay shell-neutral (#3362, #3413). if (stableRunner === runnerPath && platform !== 'win32') continue; scriptToken = m[5]; scriptPath = m[6] || m[7] || m[8] || ''; @@ -704,13 +698,16 @@ function rewriteLegacyManagedNodeHookCommands(settings, absoluteRunner, opts) { const scriptBase = scriptPath.split(/[\\/]/).pop() || ''; if (!MANAGED_HOOK_FILES.has(scriptBase)) continue; - // Skip if already using the desired stable runner. - if (runnerToken !== 'node' && runnerToken === absoluteRunner) { - if (platform !== 'win32' || hadPowerShellCallOperator) continue; - } - const safeScriptToken = formatManagedHookScriptToken(scriptPath, opts) || scriptToken; - h.command = formatHookCommandForShell(`${absoluteRunner} ${safeScriptToken}`, opts); + const projectedCommand = formatHookCommandForShell(`${absoluteRunner} ${safeScriptToken}`, opts); + + // Skip only when the existing managed command already matches the + // desired runtime-aware projected shape. This preserves Gemini's + // required PowerShell prefix while still letting Claude strip stale + // prefixes on reinstall (#3413). + if (h.command === projectedCommand) continue; + + h.command = projectedCommand; changed = true; } } @@ -818,11 +815,12 @@ function rewriteLegacyCodexHookBlock(content, absoluteRunner) { * * @param {string} configDir - Resolved absolute config directory path * @param {string} hookName - Hook filename (e.g. 'gsd-statusline.js') - * @param {{ portableHooks?: boolean, platform?: NodeJS.Platform }} [opts] - Options + * @param {{ portableHooks?: boolean, platform?: NodeJS.Platform, runtime?: string }} [opts] - Options * portableHooks: when true, emit $HOME-relative paths instead of absolute paths. * Safe for Linux/macOS global installs and WSL/Docker bind-mount scenarios. * Not suitable for pure Windows (cmd.exe/PowerShell do not expand $HOME). * platform: test injection for shell command formatting. Defaults to process.platform. + * runtime: target runtime name for shell projection policy. */ function buildHookCommand(configDir, hookName, opts) { if (!opts) opts = {}; @@ -8833,7 +8831,7 @@ function install(isGlobal, runtime = 'claude', options = {}) { // existing managed hook entries stay bare-`node`-prefixed across reinstalls // and remain broken under GUI/minimal-PATH runtimes. const settingsRunner = resolveNodeRunner(); - if (settingsRunner && rewriteLegacyManagedNodeHookCommands(settings, settingsRunner)) { + if (settingsRunner && rewriteLegacyManagedNodeHookCommands(settings, settingsRunner, { platform: process.platform, runtime })) { console.log(` ${green}✓${reset} Rewrote legacy bare-node managed-hook commands to absolute path (#2979)`); } // Local installs anchor hook paths so they resolve regardless of cwd (#1906). @@ -8843,7 +8841,7 @@ function install(isGlobal, runtime = 'claude', options = {}) { const localPrefix = (runtime === 'gemini' || runtime === 'antigravity') ? dirName : '"$CLAUDE_PROJECT_DIR"/' + dirName; - const hookOpts = { portableHooks: hasPortableHooks }; + const hookOpts = { portableHooks: hasPortableHooks, runtime }; // #2979: local-install hook commands also use the absolute node path so // GUI/minimal-PATH runtimes can resolve them. Bare `node` fails when the // host launches the runtime with a stripped PATH (Finder/Antigravity/etc). diff --git a/docs/INVENTORY-MANIFEST.json b/docs/INVENTORY-MANIFEST.json index 9cf7de990..e23998782 100644 --- a/docs/INVENTORY-MANIFEST.json +++ b/docs/INVENTORY-MANIFEST.json @@ -299,6 +299,7 @@ "schema-detect.cjs", "secrets.cjs", "security.cjs", + "shell-command-projection.cjs", "state-command-router.cjs", "state-document.cjs", "state.cjs", diff --git a/docs/INVENTORY.md b/docs/INVENTORY.md index 572a2f1cd..bd2e8d453 100644 --- a/docs/INVENTORY.md +++ b/docs/INVENTORY.md @@ -359,7 +359,7 @@ The `gsd-planner` agent is decomposed into a core agent plus reference modules t --- -## CLI Modules (54 shipped) +## CLI Modules (55 shipped) Full listing: `get-shit-done/bin/lib/*.cjs`. @@ -407,6 +407,7 @@ Full listing: `get-shit-done/bin/lib/*.cjs`. | `schema-detect.cjs` | Schema-drift detection for ORM patterns (Prisma, Drizzle, etc.) | | `secrets.cjs` | Secret-config masking convention (`****`) for integration keys managed by `/gsd-config --integrations` — keeps plaintext out of `config-set` output | | `security.cjs` | Path traversal prevention, prompt injection detection, safe JSON/shell helpers | +| `shell-command-projection.cjs` | Runtime-aware shell command projection for managed hook serialization: decides PowerShell call-operator usage by runtime/platform and normalizes Windows script path tokens | | `state-command-router.cjs` | Thin CJS subcommand router adapter for `gsd-tools state` | | `state.cjs` | STATE.md parsing, updating, progression, metrics | | `state-document.cjs` | Pure STATE.md field extraction, replacement, status normalization, and progress calculation transforms | diff --git a/get-shit-done/bin/lib/shell-command-projection.cjs b/get-shit-done/bin/lib/shell-command-projection.cjs new file mode 100644 index 000000000..0a01d2251 --- /dev/null +++ b/get-shit-done/bin/lib/shell-command-projection.cjs @@ -0,0 +1,52 @@ +'use strict'; + +/** + * Shell Command Projection Module + * + * Tracer-bullet seam for runtime-aware projection of serialized command text + * that GSD writes into runtime config or prints for copy/paste. This module + * does NOT execute commands; it only renders command text for external shells + * and runtimes. + */ + +/** + * Return true when a managed hook command must be prefixed with PowerShell's + * call operator so a quoted executable token is invokable by the target + * runtime/shell combination. + * + * Current evidence-backed policy: + * - Gemini on Windows requires `& ` for quoted node/bash runners. + * - Claude Code on Windows does NOT: its hook commands execute under bash/Git + * Bash and `& ` breaks there (#3413). + * + * Keep the policy conservative until another runtime has a verified need. + */ +function hookCommandNeedsPowerShellCallOperator(opts = {}) { + const platform = opts.platform || process.platform; + const runtime = opts.runtime || 'generic'; + return platform === 'win32' && runtime === 'gemini'; +} + +/** + * Project a fully-assembled hook command string for the target runtime. + */ +function formatHookCommandForRuntime(command, opts = {}) { + return hookCommandNeedsPowerShellCallOperator(opts) ? `& ${command}` : command; +} + +/** + * Project a managed hook script path token for serialized shell commands. + * Windows managed hook commands normalize to forward slashes so the same path + * survives JSON/TOML/config surfaces consistently. + */ +function formatManagedHookScriptToken(scriptPath, opts = {}) { + const platform = opts.platform || process.platform; + if (platform !== 'win32') return null; + return JSON.stringify(scriptPath.replace(/\\/g, '/')); +} + +module.exports = { + hookCommandNeedsPowerShellCallOperator, + formatHookCommandForRuntime, + formatManagedHookScriptToken, +}; diff --git a/tests/bug-2979-hook-absolute-node.test.cjs b/tests/bug-2979-hook-absolute-node.test.cjs index c260ae35c..eebb6ad45 100644 --- a/tests/bug-2979-hook-absolute-node.test.cjs +++ b/tests/bug-2979-hook-absolute-node.test.cjs @@ -103,24 +103,52 @@ describe('Bug #2979: buildHookCommand for .js hooks emits absolute node runner', }); }); -describe('Bug #3362: Windows PowerShell hook commands use the call operator', () => { - test('global install: .js hook command starts with & so quoted runners execute in PowerShell', () => { +describe('Bug #3362 / #3413: Windows hook commands are runtime-aware', () => { + test('Gemini global install: .js hook command starts with & so quoted runners execute in PowerShell', () => { const cmd = buildHookCommand('C:/Program Files/Gemini/.gemini', 'gsd-check-update.js', { platform: 'win32', + runtime: 'gemini', }); - assert.ok(cmd.startsWith('& '), `PowerShell commands need call operator, got: ${cmd}`); + assert.ok(cmd.startsWith('& '), `Gemini PowerShell commands need call operator, got: ${cmd}`); assert.ok(cmd.includes('"C:/Program Files/Gemini/.gemini/hooks/gsd-check-update.js"')); }); - test('portable install: .js hook command also uses & on Windows PowerShell', () => { + test('Gemini portable install: .js hook command also uses & on Windows PowerShell', () => { const home = require('node:os').homedir().replace(/\\/g, '/'); const cmd = buildHookCommand(`${home}/.gemini`, 'gsd-check-update.js', { portableHooks: true, platform: 'win32', + runtime: 'gemini', }); - assert.ok(cmd.startsWith('& '), `PowerShell commands need call operator, got: ${cmd}`); + assert.ok(cmd.startsWith('& '), `Gemini PowerShell commands need call operator, got: ${cmd}`); assert.equal(parseHookCommand(cmd.slice(2)).hookPath, '$HOME/.gemini/hooks/gsd-check-update.js'); }); + + test('Claude global install: .js hook command stays shell-neutral on Windows Git Bash', () => { + const cmd = buildHookCommand('C:/Users/me/.claude', 'gsd-check-update.js', { + platform: 'win32', + runtime: 'claude', + }); + assert.ok(!cmd.startsWith('& '), `Claude hook command must not use PowerShell call operator: ${cmd}`); + assert.equal(parseHookCommand(cmd).hookPath, 'C:/Users/me/.claude/hooks/gsd-check-update.js'); + }); + + test('Windows .js hook with no runtime stays shell-neutral', () => { + const cmd = buildHookCommand('C:/Users/me/.claude', 'gsd-check-update.js', { + platform: 'win32', + }); + assert.ok(!cmd.startsWith('& '), `Missing runtime must not imply PowerShell syntax: ${cmd}`); + assert.equal(parseHookCommand(cmd).hookPath, 'C:/Users/me/.claude/hooks/gsd-check-update.js'); + }); + + test('Gemini runtime on non-Windows platform does not get PowerShell syntax', () => { + const cmd = buildHookCommand('/home/me/.claude', 'gsd-check-update.js', { + platform: 'linux', + runtime: 'gemini', + }); + assert.ok(!cmd.startsWith('& '), `Non-Windows Gemini hook must stay shell-neutral: ${cmd}`); + assert.equal(parseHookCommand(cmd).hookPath, '/home/me/.claude/hooks/gsd-check-update.js'); + }); }); describe('Bug #2979: buildHookCommand for .sh hooks still uses bare "bash" (POSIX std PATH always has /bin)', () => { @@ -138,7 +166,7 @@ describe('Bug #2979: buildHookCommand for .sh hooks still uses bare "bash" (POSI }); assert.equal( cmd, - '& "C:/Program Files/Git/bin/bash.exe" "C:/Users/me/.codex/hooks/gsd-validate-commit.sh"', + '"C:/Program Files/Git/bin/bash.exe" "C:/Users/me/.codex/hooks/gsd-validate-commit.sh"', ); }); @@ -195,7 +223,7 @@ describe('Bug #2979 (#3002 CR): rewriteLegacyManagedNodeHookCommands rewrites ba assert.equal(settings.hooks.SessionStart[0].hooks[0].command, before); }); - test('adds PowerShell call operator to existing quoted managed hooks on Windows', () => { + test('Gemini on Windows adds PowerShell call operator to existing quoted managed hooks', () => { const settings = { hooks: { SessionStart: [{ @@ -204,7 +232,7 @@ describe('Bug #2979 (#3002 CR): rewriteLegacyManagedNodeHookCommands rewrites ba }, }; const runner = '"/usr/local/bin/node"'; - const changed = rewriteLegacyManagedNodeHookCommands(settings, runner, { platform: 'win32' }); + const changed = rewriteLegacyManagedNodeHookCommands(settings, runner, { platform: 'win32', runtime: 'gemini' }); assert.equal(changed, true); assert.equal( settings.hooks.SessionStart[0].hooks[0].command, @@ -212,7 +240,7 @@ describe('Bug #2979 (#3002 CR): rewriteLegacyManagedNodeHookCommands rewrites ba ); }); - test('does NOT double-prefix managed hooks that already use the PowerShell call operator', () => { + test('Gemini on Windows does NOT double-prefix managed hooks that already use the PowerShell call operator', () => { const settings = { hooks: { SessionStart: [{ @@ -222,12 +250,12 @@ describe('Bug #2979 (#3002 CR): rewriteLegacyManagedNodeHookCommands rewrites ba }; const runner = '"/usr/local/bin/node"'; const before = settings.hooks.SessionStart[0].hooks[0].command; - const changed = rewriteLegacyManagedNodeHookCommands(settings, runner, { platform: 'win32' }); + const changed = rewriteLegacyManagedNodeHookCommands(settings, runner, { platform: 'win32', runtime: 'gemini' }); assert.equal(changed, false); assert.equal(settings.hooks.SessionStart[0].hooks[0].command, before); }); - test('rewrites PowerShell bare-node managed hooks to absolute runner without dropping &', () => { + test('Gemini on Windows rewrites PowerShell bare-node managed hooks to absolute runner without dropping &', () => { const settings = { hooks: { SessionStart: [{ @@ -236,7 +264,7 @@ describe('Bug #2979 (#3002 CR): rewriteLegacyManagedNodeHookCommands rewrites ba }, }; const runner = '"/usr/local/bin/node"'; - const changed = rewriteLegacyManagedNodeHookCommands(settings, runner, { platform: 'win32' }); + const changed = rewriteLegacyManagedNodeHookCommands(settings, runner, { platform: 'win32', runtime: 'gemini' }); assert.equal(changed, true); assert.equal( settings.hooks.SessionStart[0].hooks[0].command, @@ -244,6 +272,23 @@ describe('Bug #2979 (#3002 CR): rewriteLegacyManagedNodeHookCommands rewrites ba ); }); + test('Claude on Windows strips stale PowerShell prefix from managed hooks on reinstall (#3413)', () => { + const settings = { + hooks: { + SessionStart: [{ + hooks: [{ type: 'command', command: '& "/usr/local/bin/node" "C:/Users/me/.claude/hooks/gsd-check-update.js"' }], + }], + }, + }; + const runner = '"/usr/local/bin/node"'; + const changed = rewriteLegacyManagedNodeHookCommands(settings, runner, { platform: 'win32', runtime: 'claude' }); + assert.equal(changed, true); + assert.equal( + settings.hooks.SessionStart[0].hooks[0].command, + '"/usr/local/bin/node" "C:/Users/me/.claude/hooks/gsd-check-update.js"', + ); + }); + test('does NOT touch user-authored bare-node hooks (filename not in managed allowlist)', () => { const settings = { hooks: { @@ -357,23 +402,23 @@ describe('Bug #2979 (#3002 CR): rewriteLegacyManagedNodeHookCommands rewrites ba assert.equal(changed, true); }); - test('normalizes single-quoted Windows managed hook paths to double-quoted forward-slash paths (#3392)', () => { + test('Gemini on Windows normalizes single-quoted managed hook paths to double-quoted forward-slash paths (#3392)', () => { const settings = { hooks: { PreToolUse: [{ hooks: [{ type: 'command', - command: "node 'C:\\Users\\me\\.codex\\hooks\\gsd-prompt-guard.js'", + command: "node 'C:\\Users\\me\\.gemini\\hooks\\gsd-prompt-guard.js'", }], }], }, }; const runner = '"C:/nvm4w/nodejs/node.exe"'; - const changed = rewriteLegacyManagedNodeHookCommands(settings, runner, { platform: 'win32' }); + const changed = rewriteLegacyManagedNodeHookCommands(settings, runner, { platform: 'win32', runtime: 'gemini' }); assert.equal(changed, true); assert.equal( settings.hooks.PreToolUse[0].hooks[0].command, - '& "C:/nvm4w/nodejs/node.exe" "C:/Users/me/.codex/hooks/gsd-prompt-guard.js"', + '& "C:/nvm4w/nodejs/node.exe" "C:/Users/me/.gemini/hooks/gsd-prompt-guard.js"', ); }); }); diff --git a/tests/bug-3413-shell-command-projection.test.cjs b/tests/bug-3413-shell-command-projection.test.cjs new file mode 100644 index 000000000..ed2e68c36 --- /dev/null +++ b/tests/bug-3413-shell-command-projection.test.cjs @@ -0,0 +1,73 @@ +'use strict'; + +process.env.GSD_TEST_MODE = '1'; + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); +const path = require('node:path'); + +const projection = require(path.join(__dirname, '..', 'get-shit-done', 'bin', 'lib', 'shell-command-projection.cjs')); +const install = require(path.join(__dirname, '..', 'bin', 'install.js')); + +const { hookCommandNeedsPowerShellCallOperator, formatHookCommandForRuntime } = projection; +const { buildHookCommand, rewriteLegacyManagedNodeHookCommands } = install; + +describe('bug #3413: Shell Command Projection Module uses runtime-aware hook policy', () => { + test('Gemini on Windows requires PowerShell call operator', () => { + assert.equal( + hookCommandNeedsPowerShellCallOperator({ platform: 'win32', runtime: 'gemini' }), + true, + ); + assert.equal( + formatHookCommandForRuntime('"C:/node.exe" "C:/hook.js"', { platform: 'win32', runtime: 'gemini' }), + '& "C:/node.exe" "C:/hook.js"', + ); + }); + + test('Claude on Windows stays shell-neutral', () => { + assert.equal( + hookCommandNeedsPowerShellCallOperator({ platform: 'win32', runtime: 'claude' }), + false, + ); + assert.equal( + formatHookCommandForRuntime('"C:/node.exe" "C:/hook.js"', { platform: 'win32', runtime: 'claude' }), + '"C:/node.exe" "C:/hook.js"', + ); + }); + + test('runtime omitted stays conservative (no PowerShell prefix)', () => { + assert.equal( + formatHookCommandForRuntime('"C:/node.exe" "C:/hook.js"', { platform: 'win32' }), + '"C:/node.exe" "C:/hook.js"', + ); + }); +}); + +describe('bug #3413: installer hook surfaces consume runtime-aware projection', () => { + test('buildHookCommand emits shell-neutral Claude hook command on Windows', () => { + const cmd = buildHookCommand('C:/Users/me/.claude', 'gsd-check-update.js', { + platform: 'win32', + runtime: 'claude', + }); + assert.equal(cmd.startsWith('& '), false, `Claude hook command must not use PowerShell prefix: ${cmd}`); + }); + + test('rewriteLegacyManagedNodeHookCommands removes stale PowerShell prefix for Claude on Windows', () => { + const settings = { + hooks: { + SessionStart: [{ + hooks: [{ type: 'command', command: '& "/usr/local/bin/node" "C:/Users/me/.claude/hooks/gsd-check-update.js"' }], + }], + }, + }; + const changed = rewriteLegacyManagedNodeHookCommands(settings, '"/usr/local/bin/node"', { + platform: 'win32', + runtime: 'claude', + }); + assert.equal(changed, true); + assert.equal( + settings.hooks.SessionStart[0].hooks[0].command, + '"/usr/local/bin/node" "C:/Users/me/.claude/hooks/gsd-check-update.js"', + ); + }); +});