diff --git a/.claude/settings.json b/.claude/settings.json index 0391eb720..a69a72db6 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -7,7 +7,7 @@ "hooks": [ { "type": "command", - "command": "[ ! -f \"${CLAUDE_PROJECT_DIR}/.claude/skills/impeccable/scripts/hook.mjs\" ] || node \"${CLAUDE_PROJECT_DIR}/.claude/skills/impeccable/scripts/hook.mjs\"", + "command": "[ ! -f \"${CLAUDE_PROJECT_DIR}/.claude/skills/impeccable/scripts/hook.mjs\" ] || ! { node -e \"process.exit(parseInt(process.versions.node,10)>=22?0:1)\" 2>/dev/null || { D=\"$HOME/.impeccable\"; [ -f \"$D/node-unsupported\" ] || { mkdir -p \"$D\" 2>/dev/null && : > \"$D/node-unsupported\" 2>/dev/null && printf '%s' '{\"systemMessage\":\"The impeccable design hook is not running: no Node 22 or newer on PATH. Install one, or remove the impeccable hook from your harness settings.\"}'; }; exit 0; }; } || node \"${CLAUDE_PROJECT_DIR}/.claude/skills/impeccable/scripts/hook.mjs\"", "timeout": 5, "statusMessage": "Checking UI changes" } @@ -19,7 +19,7 @@ "hooks": [ { "type": "command", - "command": "[ ! -f \"${CLAUDE_PROJECT_DIR}/.claude/skills/impeccable/scripts/hook.mjs\" ] || node \"${CLAUDE_PROJECT_DIR}/.claude/skills/impeccable/scripts/hook.mjs\"", + "command": "[ ! -f \"${CLAUDE_PROJECT_DIR}/.claude/skills/impeccable/scripts/hook.mjs\" ] || ! { node -e \"process.exit(parseInt(process.versions.node,10)>=22?0:1)\" 2>/dev/null || { D=\"$HOME/.impeccable\"; [ -f \"$D/node-unsupported\" ] || { mkdir -p \"$D\" 2>/dev/null && : > \"$D/node-unsupported\" 2>/dev/null && printf '%s' '{\"systemMessage\":\"The impeccable design hook is not running: no Node 22 or newer on PATH. Install one, or remove the impeccable hook from your harness settings.\"}'; }; exit 0; }; } || node \"${CLAUDE_PROJECT_DIR}/.claude/skills/impeccable/scripts/hook.mjs\"", "timeout": 30, "statusMessage": "Design deep pass" } diff --git a/.codex/hooks.json b/.codex/hooks.json index dfcda438b..e610edc4b 100644 --- a/.codex/hooks.json +++ b/.codex/hooks.json @@ -6,7 +6,7 @@ "hooks": [ { "type": "command", - "command": "[ ! -f \".codex/skills/impeccable/scripts/hook.mjs\" ] || node \".codex/skills/impeccable/scripts/hook.mjs\"", + "command": "[ ! -f \".codex/skills/impeccable/scripts/hook.mjs\" ] || ! { node -e \"process.exit(parseInt(process.versions.node,10)>=22?0:1)\" 2>/dev/null || { D=\"$HOME/.impeccable\"; [ -f \"$D/node-unsupported\" ] || { mkdir -p \"$D\" 2>/dev/null && : > \"$D/node-unsupported\" 2>/dev/null && printf '%s' '{\"systemMessage\":\"The impeccable design hook is not running: no Node 22 or newer on PATH. Install one, or remove the impeccable hook from your harness settings.\"}'; }; exit 0; }; } || node \".codex/skills/impeccable/scripts/hook.mjs\"", "timeout": 5, "statusMessage": "Checking UI changes" } @@ -18,7 +18,7 @@ "hooks": [ { "type": "command", - "command": "[ ! -f \".codex/skills/impeccable/scripts/hook.mjs\" ] || node \".codex/skills/impeccable/scripts/hook.mjs\"", + "command": "[ ! -f \".codex/skills/impeccable/scripts/hook.mjs\" ] || ! { node -e \"process.exit(parseInt(process.versions.node,10)>=22?0:1)\" 2>/dev/null || { D=\"$HOME/.impeccable\"; [ -f \"$D/node-unsupported\" ] || { mkdir -p \"$D\" 2>/dev/null && : > \"$D/node-unsupported\" 2>/dev/null && printf '%s' '{\"systemMessage\":\"The impeccable design hook is not running: no Node 22 or newer on PATH. Install one, or remove the impeccable hook from your harness settings.\"}'; }; exit 0; }; } || node \".codex/skills/impeccable/scripts/hook.mjs\"", "timeout": 30, "statusMessage": "Design deep pass" } diff --git a/.cursor/hooks.json b/.cursor/hooks.json index 040483c56..826cf35e3 100644 --- a/.cursor/hooks.json +++ b/.cursor/hooks.json @@ -3,7 +3,7 @@ "hooks": { "preToolUse": [ { - "command": "[ ! -f \".cursor/skills/impeccable/scripts/hook-before-edit.mjs\" ] || node \".cursor/skills/impeccable/scripts/hook-before-edit.mjs\"", + "command": "[ ! -f \".cursor/skills/impeccable/scripts/hook-before-edit.mjs\" ] || ! node -e \"process.exit(parseInt(process.versions.node,10)>=22?0:1)\" 2>/dev/null || node \".cursor/skills/impeccable/scripts/hook-before-edit.mjs\"", "timeout": 5 } ] diff --git a/.github/hooks/impeccable.json b/.github/hooks/impeccable.json index 0e9378094..7977aee2e 100644 --- a/.github/hooks/impeccable.json +++ b/.github/hooks/impeccable.json @@ -5,7 +5,7 @@ { "type": "command", "matcher": "edit|create|apply_patch", - "bash": "node \"$(git rev-parse --show-toplevel)/.github/skills/impeccable/scripts/hook.mjs\"", + "bash": "[ ! -f \"$(git rev-parse --show-toplevel)/.github/skills/impeccable/scripts/hook.mjs\" ] || ! node -e \"process.exit(parseInt(process.versions.node,10)>=22?0:1)\" 2>/dev/null || node \"$(git rev-parse --show-toplevel)/.github/skills/impeccable/scripts/hook.mjs\"", "timeoutSec": 5 } ] diff --git a/.grok/hooks/impeccable.json b/.grok/hooks/impeccable.json index 7d007a6fa..581b7a739 100644 --- a/.grok/hooks/impeccable.json +++ b/.grok/hooks/impeccable.json @@ -6,7 +6,7 @@ "hooks": [ { "type": "command", - "command": "node \".grok/skills/impeccable/scripts/hook.mjs\"", + "command": "[ ! -f \".grok/skills/impeccable/scripts/hook.mjs\" ] || ! node -e \"process.exit(parseInt(process.versions.node,10)>=22?0:1)\" 2>/dev/null || node \".grok/skills/impeccable/scripts/hook.mjs\"", "timeout": 5, "statusMessage": "Checking UI changes" } @@ -18,7 +18,7 @@ "hooks": [ { "type": "command", - "command": "node \".grok/skills/impeccable/scripts/hook.mjs\"", + "command": "[ ! -f \".grok/skills/impeccable/scripts/hook.mjs\" ] || ! node -e \"process.exit(parseInt(process.versions.node,10)>=22?0:1)\" 2>/dev/null || node \".grok/skills/impeccable/scripts/hook.mjs\"", "timeout": 30, "statusMessage": "Design deep pass" } diff --git a/plugin/hooks/hooks.json b/plugin/hooks/hooks.json index 66fd18e7e..01eef380c 100644 --- a/plugin/hooks/hooks.json +++ b/plugin/hooks/hooks.json @@ -6,7 +6,7 @@ "hooks": [ { "type": "command", - "command": "node \"${CLAUDE_PLUGIN_ROOT}/skills/impeccable/scripts/hook.mjs\"", + "command": "[ ! -f \"${CLAUDE_PLUGIN_ROOT}/skills/impeccable/scripts/hook.mjs\" ] || ! { node -e \"process.exit(parseInt(process.versions.node,10)>=22?0:1)\" 2>/dev/null || { D=\"$HOME/.impeccable\"; [ -f \"$D/node-unsupported\" ] || { mkdir -p \"$D\" 2>/dev/null && : > \"$D/node-unsupported\" 2>/dev/null && printf '%s' '{\"systemMessage\":\"The impeccable design hook is not running: no Node 22 or newer on PATH. Install one, or remove the impeccable hook from your harness settings.\"}'; }; exit 0; }; } || node \"${CLAUDE_PLUGIN_ROOT}/skills/impeccable/scripts/hook.mjs\"", "timeout": 5, "statusMessage": "Checking UI changes" } @@ -18,7 +18,7 @@ "hooks": [ { "type": "command", - "command": "node \"${CLAUDE_PLUGIN_ROOT}/skills/impeccable/scripts/hook.mjs\"", + "command": "[ ! -f \"${CLAUDE_PLUGIN_ROOT}/skills/impeccable/scripts/hook.mjs\" ] || ! { node -e \"process.exit(parseInt(process.versions.node,10)>=22?0:1)\" 2>/dev/null || { D=\"$HOME/.impeccable\"; [ -f \"$D/node-unsupported\" ] || { mkdir -p \"$D\" 2>/dev/null && : > \"$D/node-unsupported\" 2>/dev/null && printf '%s' '{\"systemMessage\":\"The impeccable design hook is not running: no Node 22 or newer on PATH. Install one, or remove the impeccable hook from your harness settings.\"}'; }; exit 0; }; } || node \"${CLAUDE_PLUGIN_ROOT}/skills/impeccable/scripts/hook.mjs\"", "timeout": 30, "statusMessage": "Design deep pass" } diff --git a/scripts/lib/transformers/hooks.js b/scripts/lib/transformers/hooks.js index 95da1ed6c..66dff719f 100644 --- a/scripts/lib/transformers/hooks.js +++ b/scripts/lib/transformers/hooks.js @@ -47,11 +47,51 @@ function stopEntry(command) { }; } const CLAUDE_PROJECT_HOOK = '${CLAUDE_PROJECT_DIR}/.claude/skills/impeccable/scripts/hook.mjs'; +// The Node major the hook runtime requires, kept equal to the engines floor in +// package.json. The probe and the notice both derive from it so they cannot +// disagree about the supported version. +const NODE_MAJOR_FLOOR = 22; // A hook manifest can be copied into a user-level settings file (issue #399: // user-level hooks fire in every project, where a project-relative path may // not exist). Guard node invocations so a missing file exits 0 without // swallowing node's real exit code when the file is present. -const guardedNode = (hookPath) => `[ ! -f "${hookPath}" ] || node "${hookPath}"`; +// +// The runtime is guarded too (issue #410): a `node` on PATH too old for the +// hook's ESM syntax dies while hook.mjs is still being parsed, before the +// script's own always-exit-0 contract can run, so the harness reported a hook +// error on every edit and every Stop. Nothing written in ESM can report that +// condition, so the command string itself checks the version floor first, in +// ES5-only syntax that parses on any node old enough to fail it, and exits 0 +// when the runtime is unsupported or missing. +// +// `notice` reports the dead runtime to the user. It is passed per harness +// because only some have a channel for it, checked against each harness's own +// hook reference on the events we hook: +// Claude Code / Codex: `systemMessage` on stdout is shown to the user -> notice +// Cursor: preToolUse output is permission-shaped and its `user_message` +// renders only on DENY, so warning would block the edit -> probe only +// Grok Build: PostToolUse/Stop stdout is ignored outright -> probe only +// Copilot: output contract unconfirmed; do not guess a shape -> probe only +const NODE_PROBE = `node -e "process.exit(parseInt(process.versions.node,10)>=${NODE_MAJOR_FLOOR}?0:1)" 2>/dev/null`; +const guardedNode = (hookPath, notice = '') => { + const probe = notice + ? `! { ${NODE_PROBE} || { ${notice}; exit 0; }; }` + : `! ${NODE_PROBE}`; + return `[ ! -f "${hookPath}" ] || ${probe} || node "${hookPath}"`; +}; +// The message says `on PATH` deliberately: the common cause is a hook shell +// whose PATH misses the version manager, so a user already running Node 22 +// needs to know the hook's PATH is at issue and not their install. Apostrophes +// cannot appear in it, since it travels inside a single-quoted shell string. +const NODE_NOTICE_TEXT = `The impeccable design hook is not running: no Node ${NODE_MAJOR_FLOOR} or newer on PATH. ` + + 'Install one, or remove the impeccable hook from your harness settings.'; +// Claude Code and Codex both read `systemMessage`, so one payload serves both. +// The marker under ~/.impeccable holds it to one notice per machine (not per +// harness or per edit), and printf runs only after the marker write succeeds, +// so an unwritable HOME degrades to silence rather than a notice on every edit. +const SYSTEM_MESSAGE_NOTICE = 'D="$HOME/.impeccable"; [ -f "$D/node-unsupported" ] || ' + + '{ mkdir -p "$D" 2>/dev/null && : > "$D/node-unsupported" 2>/dev/null && ' + + `printf '%s' '{"systemMessage":"${NODE_NOTICE_TEXT}"}'; }`; const CLAUDE_PLUGIN_HOOK = '${CLAUDE_PLUGIN_ROOT}/skills/impeccable/scripts/hook.mjs'; const CODEX_PLUGIN_HOOK = '${PLUGIN_ROOT}/skills/impeccable/scripts/hook.mjs'; // Codex reads project hooks from `.codex/hooks.json`, but the skill payload the @@ -77,14 +117,14 @@ export function buildClaudeSettingsManifest() { hooks: [ { type: 'command', - command: guardedNode(CLAUDE_PROJECT_HOOK), + command: guardedNode(CLAUDE_PROJECT_HOOK, SYSTEM_MESSAGE_NOTICE), timeout: TIMEOUT_SECONDS, statusMessage: STATUS_MESSAGE, }, ], }, ], - Stop: [stopEntry(guardedNode(CLAUDE_PROJECT_HOOK))], + Stop: [stopEntry(guardedNode(CLAUDE_PROJECT_HOOK, SYSTEM_MESSAGE_NOTICE))], }, }; } @@ -104,14 +144,14 @@ export function buildClaudePluginHooksManifest() { hooks: [ { type: 'command', - command: `node "${CLAUDE_PLUGIN_HOOK}"`, + command: guardedNode(CLAUDE_PLUGIN_HOOK, SYSTEM_MESSAGE_NOTICE), timeout: TIMEOUT_SECONDS, statusMessage: STATUS_MESSAGE, }, ], }, ], - Stop: [stopEntry(`node "${CLAUDE_PLUGIN_HOOK}"`)], + Stop: [stopEntry(guardedNode(CLAUDE_PLUGIN_HOOK, SYSTEM_MESSAGE_NOTICE))], }, }; } @@ -128,14 +168,14 @@ export function buildCodexPluginHooksManifest() { hooks: [ { type: 'command', - command: `node "${CODEX_PLUGIN_HOOK}"`, + command: guardedNode(CODEX_PLUGIN_HOOK, SYSTEM_MESSAGE_NOTICE), timeout: TIMEOUT_SECONDS, statusMessage: STATUS_MESSAGE, }, ], }, ], - Stop: [stopEntry(`node "${CODEX_PLUGIN_HOOK}"`)], + Stop: [stopEntry(guardedNode(CODEX_PLUGIN_HOOK, SYSTEM_MESSAGE_NOTICE))], }, }; } @@ -153,14 +193,14 @@ export function buildCodexHooksManifest(skillDir = '.codex') { hooks: [ { type: 'command', - command: guardedNode(hookPath), + command: guardedNode(hookPath, SYSTEM_MESSAGE_NOTICE), timeout: TIMEOUT_SECONDS, statusMessage: STATUS_MESSAGE, }, ], }, ], - Stop: [stopEntry(guardedNode(hookPath))], + Stop: [stopEntry(guardedNode(hookPath, SYSTEM_MESSAGE_NOTICE))], }, }; } @@ -198,7 +238,7 @@ export function buildGitHubHooksManifest() { { type: 'command', matcher: 'edit|create|apply_patch', - bash: `node "${GITHUB_PROJECT_HOOK}"`, + bash: guardedNode(GITHUB_PROJECT_HOOK), timeoutSec: TIMEOUT_SECONDS, }, ], @@ -220,14 +260,14 @@ export function buildGrokHooksManifest() { hooks: [ { type: 'command', - command: `node "${GROK_PROJECT_HOOK}"`, + command: guardedNode(GROK_PROJECT_HOOK), timeout: TIMEOUT_SECONDS, statusMessage: STATUS_MESSAGE, }, ], }, ], - Stop: [stopEntry(`node "${GROK_PROJECT_HOOK}"`)], + Stop: [stopEntry(guardedNode(GROK_PROJECT_HOOK))], }, }; } diff --git a/tests/hook-build.test.mjs b/tests/hook-build.test.mjs index 937610775..11ca0d535 100644 --- a/tests/hook-build.test.mjs +++ b/tests/hook-build.test.mjs @@ -13,6 +13,7 @@ import { buildClaudeSettingsManifest, buildClaudePluginHooksManifest, buildCodexHooksManifest, + buildCodexPluginHooksManifest, buildCursorHooksManifest, buildGitHubHooksManifest, buildGrokHooksManifest, @@ -25,13 +26,25 @@ function readJson(rel) { return JSON.parse(fs.readFileSync(path.join(REPO_ROOT, rel), 'utf8')); } +// The runtime probe every hook command must carry (issue #410): a node below +// the engines floor exits the command at 0 instead of dying on ESM parse. The +// expected floor comes from package.json engines, so probe and contract cannot +// drift apart. +const ENGINES_NODE_MAJOR = parseInt( + JSON.parse(fs.readFileSync(path.join(REPO_ROOT, 'package.json'), 'utf8')).engines.node.replace(/[^\d.]/g, ''), + 10, +); +const NODE_PROBE = `process.exit(parseInt(process.versions.node,10)>=${ENGINES_NODE_MAJOR}?0:1)`; + function expectCommand(command, expectedPath) { assert.equal(typeof command, 'string'); // node-command providers carry the missing-file guard (issue #399: exits 0 - // when absent, preserves node's exit code when present); github/grok keep - // their own portable unguarded forms. + // when absent, preserves node's exit code when present) plus the runtime + // probe. GitHub's portable `$(git rev-parse)` form is guarded too, so it + // lands in the same branch. if (command.startsWith('[ ! -f "')) { assert.match(command, /\|\| node "/); + assert.ok(command.includes(NODE_PROBE), `missing runtime probe in ${command}`); } else { assert.match(command, /^node "|^bash -c|\$\(git rev-parse/); } @@ -39,6 +52,20 @@ function expectCommand(command, expectedPath) { assert.ok(!command.includes('hook-probe.mjs'), `probe hook still referenced in ${command}`); } +function manifestCommands(manifest) { + const commands = []; + const walk = (value) => { + if (Array.isArray(value)) { value.forEach(walk); return; } + if (value && typeof value === 'object') { + if (typeof value.command === 'string') commands.push(value.command); + if (typeof value.bash === 'string') commands.push(value.bash); + Object.values(value).forEach(walk); + } + }; + walk(manifest.hooks); + return commands; +} + describe('hook manifest builders', () => { it('builds Claude project settings for the real detector hook', () => { const manifest = buildClaudeSettingsManifest(); @@ -165,6 +192,40 @@ describe('hook manifest builders', () => { expectCommand(stop.command, '.grok/skills/impeccable/scripts/hook.mjs'); }); + it('probes the node runtime everywhere, and notices only where a channel exists', () => { + // Claude Code and Codex render a `systemMessage` from hook stdout, so their + // manifests carry the one-time unsupported-runtime notice. Cursor (output is + // permission-shaped; a message would block the edit), Grok (stdout ignored), + // and Copilot (contract unconfirmed) get the silent probe only. + const withNotice = [ + buildClaudeSettingsManifest(), + buildClaudePluginHooksManifest(), + buildCodexHooksManifest(), + buildCodexPluginHooksManifest(), + ]; + const probeOnly = [ + buildCursorHooksManifest(), + buildGitHubHooksManifest(), + buildGrokHooksManifest(), + ]; + for (const manifest of [...withNotice, ...probeOnly]) { + for (const command of manifestCommands(manifest)) { + assert.ok(command.includes(NODE_PROBE), `missing runtime probe in ${command}`); + } + } + for (const manifest of withNotice) { + for (const command of manifestCommands(manifest)) { + assert.ok(command.includes('systemMessage'), `missing notice in ${command}`); + assert.ok(command.includes('node-unsupported'), `missing once-only marker in ${command}`); + } + } + for (const manifest of probeOnly) { + for (const command of manifestCommands(manifest)) { + assert.ok(!command.includes('systemMessage'), `unexpected notice in ${command}`); + } + } + }); + it('routes supported hook builders and leaves other providers alone', () => { assert.ok(hooksJsonFor('claude')); assert.ok(hooksJsonFor('codex'));