* fix(#3329): reconcile stale managed .sh hook commands on install/update applySettingsJsonHooks registers the four .sh managed hooks only-if-absent, so entries registered before the #580/#3393 shellHookOmitsBashRunner fix kept their bash-runner-prefixed commands forever — /gsd-update re-invokes the installer but never re-derived existing entries. Add reconcileManagedShellHookCommands (wired into applySettingsJsonHooks): on win32+claude it rewrites existing managed .sh entries to the command this install would generate today, scoped to exact managed basenames so user-authored hooks are untouched, and inert wherever the bash runner is still the correct shape. Also bumps the allow-test-rule ceiling 301→302: PR #3455 added tests/milestone-lock.test.cjs (the 302nd marked file) without the ratchet bump, leaving lint-tests red on next. * chore(#3329): add changeset fragment * chore(#3329): backfill changeset pr number 3460 --------- Co-authored-by: sim <sim@local>
This commit is contained in:
5
.changeset/quick-rams-greet.md
Normal file
5
.changeset/quick-rams-greet.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 3460
|
||||
---
|
||||
Windows/Claude Code: /gsd-update and re-running the installer now migrate stale `.sh` managed hook commands (gsd-validate-commit, gsd-graphify-update, gsd-session-state, gsd-phase-boundary) in settings.json/settings.local.json to the current bash-runner-omission format — removing the redundant nested bash that the pre-#580/#3393 shape spawns on every hook fire. Custom user hooks are never touched.
|
||||
@@ -1106,6 +1106,15 @@ function rewriteLegacyManagedNodeHookCommands(settings, absoluteRunner, opts) {
|
||||
return hooksSurface.rewriteLegacyManagedNodeHookCommands(settings, absoluteRunner, opts);
|
||||
}
|
||||
|
||||
// #3329: rewrite already-registered managed `.sh` hook commands to the shape
|
||||
// the current installer would generate (the #580/#3393 bash-runner-omission
|
||||
// migration the register-only-if-absent path never applied to existing
|
||||
// entries). Invoked inside applySettingsJsonHooks in the compiled surface
|
||||
// module; re-bound here for tests/consumers, mirroring the #2979 rewriter.
|
||||
function reconcileManagedShellHookCommands(settings, expected, opts) {
|
||||
return hooksSurface.reconcileManagedShellHookCommands(settings, expected, opts);
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the GSD-managed Codex SessionStart hook block for config.toml.
|
||||
*
|
||||
@@ -13784,6 +13793,7 @@ module.exports = {
|
||||
referencesHook,
|
||||
applySettingsJsonHooks,
|
||||
rewriteLegacyManagedNodeHookCommands,
|
||||
reconcileManagedShellHookCommands,
|
||||
buildCodexHookBlock,
|
||||
rewriteLegacyCodexHookBlock,
|
||||
buildCodexHookWindowsShimIR,
|
||||
|
||||
@@ -568,6 +568,95 @@ function rewriteLegacyManagedNodeHookCommands(settings: Settings, absoluteRunner
|
||||
return changed;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Shared: reconcileManagedShellHookCommands (#3329)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Rewrite already-registered managed `.sh` hook `command` strings to the shape
|
||||
* the current installer would generate (#3329).
|
||||
*
|
||||
* applySettingsJsonHooks registers the four `.sh` managed hooks only-if-absent,
|
||||
* so an entry registered by an older installer keeps its old command forever —
|
||||
* `/gsd-update` (which re-invokes the installer) never re-derived it. On
|
||||
* Claude/win32 that left the pre-#580/#3393 bash-runner-prefixed commands
|
||||
* (`bash "<script>.sh"`, `"<git>/bash.exe" "<script>.sh"`) in place, spawning a
|
||||
* nested bash on every hook fire. The #2979 rewriter above cannot help: it is
|
||||
* Node-only by design (its basename gate contains only `.js` filenames).
|
||||
*
|
||||
* Scoping / safety:
|
||||
* - Inert unless `shellHookOmitsBashRunner({ platform, runtime, isShellHook:
|
||||
* true })` — the exact combination whose correct command shape changed. Where
|
||||
* the bash runner is still correct (non-Windows, non-claude), nothing is
|
||||
* rewritten, so the reconcile cannot churn unrelated installs.
|
||||
* - Only entries whose parsed script token's basename exactly equals one of the
|
||||
* expected managed `.sh` filenames are touched. A user hook would have to
|
||||
* live at a path ending in exactly `gsd-session-state.sh` etc. — i.e. the
|
||||
* GSD-installed file — to match. Extra-token commands (env prefixes, extra
|
||||
* args) and args-form launcher entries (#976) never match the strict
|
||||
* `[runner ]<script>` two-token shape and are left alone.
|
||||
* - A null/empty expected command disables rewriting for that hook (a
|
||||
* bash-runner-unavailable install must not have its entry nulled).
|
||||
*
|
||||
* @param settings settings.json-shaped object; mutated in place
|
||||
* @param expected map of managed `.sh` filename → the command this install
|
||||
* would register today (from buildHookCommand / buildLocalShellHookCommand)
|
||||
* @param opts platform/runtime override (default: current process)
|
||||
* @returns true when any command was rewritten
|
||||
*/
|
||||
function reconcileManagedShellHookCommands(
|
||||
settings: Settings,
|
||||
expected: Record<string, string | null | undefined>,
|
||||
opts?: RewriteOpts
|
||||
): boolean {
|
||||
if (!settings || !settings.hooks || !expected) return false;
|
||||
if (!opts) opts = {};
|
||||
const platform = opts.platform || process.platform;
|
||||
const runtime = opts.runtime || 'generic';
|
||||
if (!shellHookOmitsBashRunner({ platform, runtime, isShellHook: true })) return false;
|
||||
|
||||
const expectedByBasename = new Map<string, string>();
|
||||
for (const [hookFile, command] of Object.entries(expected)) {
|
||||
if (typeof command === 'string' && command.length > 0) {
|
||||
expectedByBasename.set(hookFile, command);
|
||||
}
|
||||
}
|
||||
if (expectedByBasename.size === 0) return false;
|
||||
|
||||
let changed = false;
|
||||
for (const entries of Object.values(settings.hooks)) {
|
||||
if (!Array.isArray(entries)) continue;
|
||||
for (const entry of entries) {
|
||||
if (!entry || !Array.isArray(entry.hooks)) continue;
|
||||
for (const h of entry.hooks) {
|
||||
if (!h || typeof h.command !== 'string') continue;
|
||||
if (Array.isArray(h.args) && h.args.length > 0) continue;
|
||||
let trimmed = h.command.trim();
|
||||
const hadPowerShellCallOperator = platform === 'win32' && /^&\s+/.test(trimmed);
|
||||
if (hadPowerShellCallOperator) {
|
||||
trimmed = trimmed.replace(/^&\s+/, '').trim();
|
||||
}
|
||||
// Strict `[runner ]<script>` shape: an optional single runner token
|
||||
// (bare, 'single-quoted', or "double-quoted") followed by the script
|
||||
// token. Anything else (env prefixes, extra flags, pipelines) does not
|
||||
// match and is left untouched.
|
||||
const m = trimmed.match(/^(?:(?:"([^"]+)"|'([^']+)'|(\S+))\s+)?(?:"([^"]+)"|'([^']+)'|(\S+))\s*$/);
|
||||
if (!m) continue;
|
||||
const scriptToken = m[4] || m[5] || m[6] || '';
|
||||
if (!scriptToken) continue;
|
||||
const basename = shellCmdProjection.posixNormalize(scriptToken).split('/').pop() || '';
|
||||
const expectedCommand = expectedByBasename.get(basename);
|
||||
if (!expectedCommand) continue;
|
||||
if (h.command === expectedCommand) continue;
|
||||
|
||||
h.command = expectedCommand;
|
||||
changed = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
return changed;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Codex TOML hook block builder
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -2096,6 +2185,23 @@ function applySettingsJsonHooks(settings: any, opts: ApplySettingsJsonHooksOpts)
|
||||
console.warn(` ${yellow}⚠${reset} Skipped phase boundary hook — Bash executable path unavailable (#3393)`);
|
||||
}
|
||||
|
||||
// #3329: the four `.sh` sites above register only-if-absent, so an entry
|
||||
// registered by an older installer keeps its old command forever —
|
||||
// /gsd-update (which re-invokes the installer) never re-derived it. On
|
||||
// Claude/win32 that left the pre-#580/#3393 bash-runner-prefixed commands
|
||||
// in settings.json indefinitely. Reconcile existing managed `.sh` entries
|
||||
// to the command this install would generate today. Inert wherever the
|
||||
// bash runner is still the correct shape; scoped to exact managed
|
||||
// basenames so user-authored hooks are never touched.
|
||||
if (reconcileManagedShellHookCommands(settings as Settings, {
|
||||
'gsd-validate-commit.sh': validateCommitCommand,
|
||||
'gsd-graphify-update.sh': graphifyUpdateCommand,
|
||||
'gsd-session-state.sh': sessionStateCommand,
|
||||
'gsd-phase-boundary.sh': phaseBoundaryCommand,
|
||||
}, { platform: hookOpts.platform, runtime })) {
|
||||
console.log(` ${green}✓${reset} Reconciled managed .sh hook commands to current format (#3329)`);
|
||||
}
|
||||
|
||||
// ── Extended hook events: SubagentStop / Stop / PreCompact / SubagentStart
|
||||
// (#788 + #770 + #2092) ────────────────────────────────────────────────
|
||||
// Claude Code (since #770) and Qwen Code (since #788) both support the
|
||||
@@ -2559,6 +2665,7 @@ export = {
|
||||
applySettingsJsonHooks,
|
||||
referencesHook,
|
||||
rewriteLegacyManagedNodeHookCommands,
|
||||
reconcileManagedShellHookCommands,
|
||||
normalizeNodePath,
|
||||
resolveNodeRunner,
|
||||
resolveBashRunner,
|
||||
|
||||
@@ -1426,3 +1426,193 @@ describe('#3333 regression: copyWithPathReplacement tolerates a source file vani
|
||||
'vanish.md destination must not exist — the vanished source must be skipped, not partially written');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── #3329 — /gsd-update never migrates stale .sh hook commands ───────────────
|
||||
//
|
||||
// /gsd-update re-invokes the installer (workflows/update.md), but
|
||||
// applySettingsJsonHooks registers the four `.sh` managed hooks only-if-absent:
|
||||
// an already-registered entry keeps whatever command shape an older installer
|
||||
// emitted. On Windows+Claude that left the pre-#580/#3393 bash-runner-prefixed
|
||||
// commands (`bash "<script>.sh"` / `"<git>/bash.exe" "<script>.sh"`) in place
|
||||
// forever — each hook fire spawns a nested bash grandchild. The fix adds
|
||||
// reconcileManagedShellHookCommands, wired into applySettingsJsonHooks, which
|
||||
// rewrites existing managed `.sh` entries to the command this install would
|
||||
// generate today — gated on shellHookOmitsBashRunner so it never fires where
|
||||
// the bash runner is correct, and scoped to exact managed basenames so
|
||||
// user-authored hooks are untouched.
|
||||
|
||||
describe('#3329 regression: stale managed .sh hook commands are reconciled on install/update', () => {
|
||||
const { reconcileManagedShellHookCommands } = installExports || {};
|
||||
|
||||
const GLOBAL_EXPECTED = {
|
||||
'gsd-validate-commit.sh': '"C:/Users/u/.claude/hooks/gsd-validate-commit.sh"',
|
||||
'gsd-graphify-update.sh': '"C:/Users/u/.claude/hooks/gsd-graphify-update.sh"',
|
||||
'gsd-session-state.sh': '"C:/Users/u/.claude/hooks/gsd-session-state.sh"',
|
||||
'gsd-phase-boundary.sh': '"C:/Users/u/.claude/hooks/gsd-phase-boundary.sh"',
|
||||
};
|
||||
|
||||
const LOCAL_EXPECTED = {
|
||||
'gsd-validate-commit.sh': '"$CLAUDE_PROJECT_DIR"/.claude/hooks/gsd-validate-commit.sh',
|
||||
'gsd-graphify-update.sh': '"$CLAUDE_PROJECT_DIR"/.claude/hooks/gsd-graphify-update.sh',
|
||||
'gsd-session-state.sh': '"$CLAUDE_PROJECT_DIR"/.claude/hooks/gsd-session-state.sh',
|
||||
'gsd-phase-boundary.sh': '"$CLAUDE_PROJECT_DIR"/.claude/hooks/gsd-phase-boundary.sh',
|
||||
};
|
||||
|
||||
function settingsWith(entriesByEvent) {
|
||||
const hooks = {};
|
||||
for (const [event, commands] of Object.entries(entriesByEvent)) {
|
||||
hooks[event] = commands.map(command => ({
|
||||
hooks: [{ type: 'command', command }],
|
||||
}));
|
||||
}
|
||||
return { hooks };
|
||||
}
|
||||
|
||||
function allCommands(settings) {
|
||||
const out = [];
|
||||
for (const entries of Object.values(settings.hooks)) {
|
||||
for (const entry of entries) {
|
||||
for (const h of entry.hooks || []) out.push(h.command);
|
||||
}
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
test('reconcileManagedShellHookCommands is exported from bin/install.js', () => {
|
||||
assert.strictEqual(typeof reconcileManagedShellHookCommands, 'function',
|
||||
'reconcileManagedShellHookCommands must be exported from bin/install.js (#3329)');
|
||||
});
|
||||
|
||||
test('win32+claude: bare `bash`-prefixed global entries are rewritten to the bare script path', () => {
|
||||
// Exact stale shapes reported in #3329 (global install, ~/.claude/settings.json).
|
||||
const settings = settingsWith({
|
||||
SessionStart: ['bash "C:/Users/u/.claude/hooks/gsd-session-state.sh"'],
|
||||
PreToolUse: ['bash "C:/Users/u/.claude/hooks/gsd-validate-commit.sh"'],
|
||||
PostToolUse: ['"C:/Program Files/Git/bin/bash.exe" "C:/Users/u/.claude/hooks/gsd-graphify-update.sh"'],
|
||||
});
|
||||
|
||||
const changed = reconcileManagedShellHookCommands(settings, GLOBAL_EXPECTED, {
|
||||
platform: 'win32',
|
||||
runtime: 'claude',
|
||||
});
|
||||
|
||||
assert.strictEqual(changed, true, 'stale entries must be reported as changed');
|
||||
assert.deepStrictEqual(allCommands(settings), [
|
||||
'"C:/Users/u/.claude/hooks/gsd-session-state.sh"',
|
||||
'"C:/Users/u/.claude/hooks/gsd-validate-commit.sh"',
|
||||
'"C:/Users/u/.claude/hooks/gsd-graphify-update.sh"',
|
||||
], 'all three stale shapes must be rewritten to the shellHookOmitsBashRunner form');
|
||||
});
|
||||
|
||||
test('win32+claude: local anchored-prefix entries are rewritten (buildLocalShellHookCommand shape)', () => {
|
||||
const settings = settingsWith({
|
||||
PreToolUse: ['bash "$CLAUDE_PROJECT_DIR"/.claude/hooks/gsd-validate-commit.sh'],
|
||||
PostToolUse: ['"C:/Program Files/Git/bin/bash.exe" "$CLAUDE_PROJECT_DIR"/.claude/hooks/gsd-phase-boundary.sh'],
|
||||
});
|
||||
|
||||
const changed = reconcileManagedShellHookCommands(settings, LOCAL_EXPECTED, {
|
||||
platform: 'win32',
|
||||
runtime: 'claude',
|
||||
});
|
||||
|
||||
assert.strictEqual(changed, true);
|
||||
assert.deepStrictEqual(allCommands(settings), [
|
||||
'"$CLAUDE_PROJECT_DIR"/.claude/hooks/gsd-validate-commit.sh',
|
||||
'"$CLAUDE_PROJECT_DIR"/.claude/hooks/gsd-phase-boundary.sh',
|
||||
], 'local-install .sh entries must reconcile to the anchored bare script path');
|
||||
});
|
||||
|
||||
test('idempotent: already-current entries are left untouched and report no change', () => {
|
||||
const settings = settingsWith({
|
||||
SessionStart: ['"C:/Users/u/.claude/hooks/gsd-session-state.sh"'],
|
||||
});
|
||||
|
||||
const changed = reconcileManagedShellHookCommands(settings, GLOBAL_EXPECTED, {
|
||||
platform: 'win32',
|
||||
runtime: 'claude',
|
||||
});
|
||||
|
||||
assert.strictEqual(changed, false, 'no-op when nothing is stale');
|
||||
assert.strictEqual(
|
||||
settings.hooks.SessionStart[0].hooks[0].command,
|
||||
'"C:/Users/u/.claude/hooks/gsd-session-state.sh"',
|
||||
);
|
||||
});
|
||||
|
||||
test('over-fire guard: non-Windows platforms are untouched even when commands are stale', () => {
|
||||
const original = 'bash "/home/u/.claude/hooks/gsd-session-state.sh"';
|
||||
const settings = settingsWith({ SessionStart: [original] });
|
||||
|
||||
const changed = reconcileManagedShellHookCommands(settings, {
|
||||
'gsd-session-state.sh': 'bash "/home/u/.claude/hooks/gsd-session-state.sh"',
|
||||
}, { platform: 'linux', runtime: 'claude' });
|
||||
|
||||
assert.strictEqual(changed, false, 'linux must not reconcile (bash runner is correct there)');
|
||||
assert.strictEqual(settings.hooks.SessionStart[0].hooks[0].command, original);
|
||||
});
|
||||
|
||||
test('over-fire guard: win32 non-claude runtimes are untouched', () => {
|
||||
const original = 'bash "C:/Users/u/.claude/hooks/gsd-session-state.sh"';
|
||||
const settings = settingsWith({ SessionStart: [original] });
|
||||
|
||||
const changed = reconcileManagedShellHookCommands(settings, GLOBAL_EXPECTED, {
|
||||
platform: 'win32',
|
||||
runtime: 'qwen',
|
||||
});
|
||||
|
||||
assert.strictEqual(changed, false, 'win32+qwen keeps the bash runner by design');
|
||||
assert.strictEqual(settings.hooks.SessionStart[0].hooks[0].command, original);
|
||||
});
|
||||
|
||||
test('scope guard: non-managed and user-authored hooks are never rewritten', () => {
|
||||
const entries = [
|
||||
'bash "/home/u/hooks/my-own-hook.sh"', // user hook, foreign basename
|
||||
'node "C:/Users/u/.claude/hooks/gsd-check-update.js"', // .js hook — owned by the #2979 rewriter
|
||||
'FOO=1 bash "C:/Users/u/.claude/hooks/gsd-session-state.sh"', // 3-token wrapper, not the managed shape
|
||||
'bash "C:/Users/u/.claude/hooks/gsd-session-state.sh" --flag', // extra args after the script
|
||||
];
|
||||
const settings = settingsWith({ SessionStart: entries.slice() });
|
||||
|
||||
const changed = reconcileManagedShellHookCommands(settings, GLOBAL_EXPECTED, {
|
||||
platform: 'win32',
|
||||
runtime: 'claude',
|
||||
});
|
||||
|
||||
assert.strictEqual(changed, false, 'no managed 2-token entry present — nothing to rewrite');
|
||||
assert.deepStrictEqual(allCommands(settings), entries, 'every entry must be byte-identical');
|
||||
});
|
||||
|
||||
test('scope guard: args-form launcher entries are skipped (#976 parity)', () => {
|
||||
const settings = {
|
||||
hooks: {
|
||||
SessionStart: [{
|
||||
hooks: [{
|
||||
type: 'command',
|
||||
command: '/usr/local/bin/node-launcher',
|
||||
args: ['C:/Users/u/.claude/hooks/gsd-session-state.sh'],
|
||||
}],
|
||||
}],
|
||||
},
|
||||
};
|
||||
|
||||
const changed = reconcileManagedShellHookCommands(settings, GLOBAL_EXPECTED, {
|
||||
platform: 'win32',
|
||||
runtime: 'claude',
|
||||
});
|
||||
|
||||
assert.strictEqual(changed, false, 'args-form entries are intentional wrappers');
|
||||
assert.strictEqual(settings.hooks.SessionStart[0].hooks[0].command, '/usr/local/bin/node-launcher');
|
||||
});
|
||||
|
||||
test('null expected commands are never written (bash-runner-unavailable installs stay intact)', () => {
|
||||
const original = 'bash "C:/Users/u/.claude/hooks/gsd-session-state.sh"';
|
||||
const settings = settingsWith({ SessionStart: [original] });
|
||||
|
||||
const changed = reconcileManagedShellHookCommands(settings, {
|
||||
'gsd-session-state.sh': null,
|
||||
}, { platform: 'win32', runtime: 'claude' });
|
||||
|
||||
assert.strictEqual(changed, false, 'a null expected command must disable rewriting for that hook');
|
||||
assert.strictEqual(settings.hooks.SessionStart[0].hooks[0].command, original);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user