diff --git a/.claude/settings.json b/.claude/settings.json index 44376689b..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 -e \"import('node:fs').catch(()=>process.exit(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\"", + "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 -e \"import('node:fs').catch(()=>process.exit(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\"", + "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 000824e04..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 -e \"import('node:fs').catch(()=>process.exit(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\"", + "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 -e \"import('node:fs').catch(()=>process.exit(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\"", + "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 5c8616a5f..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 -e \"import('node:fs').catch(()=>process.exit(1))\" 2>/dev/null || 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 c51a954ac..7977aee2e 100644 --- a/.github/hooks/impeccable.json +++ b/.github/hooks/impeccable.json @@ -5,7 +5,7 @@ { "type": "command", "matcher": "edit|create|apply_patch", - "bash": "[ ! -f \"$(git rev-parse --show-toplevel)/.github/skills/impeccable/scripts/hook.mjs\" ] || ! node -e \"import('node:fs').catch(()=>process.exit(1))\" 2>/dev/null || 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 3cb2f24ed..581b7a739 100644 --- a/.grok/hooks/impeccable.json +++ b/.grok/hooks/impeccable.json @@ -6,7 +6,7 @@ "hooks": [ { "type": "command", - "command": "[ ! -f \".grok/skills/impeccable/scripts/hook.mjs\" ] || ! node -e \"import('node:fs').catch(()=>process.exit(1))\" 2>/dev/null || 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": "[ ! -f \".grok/skills/impeccable/scripts/hook.mjs\" ] || ! node -e \"import('node:fs').catch(()=>process.exit(1))\" 2>/dev/null || 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 501410231..01eef380c 100644 --- a/plugin/hooks/hooks.json +++ b/plugin/hooks/hooks.json @@ -6,7 +6,7 @@ "hooks": [ { "type": "command", - "command": "[ ! -f \"${CLAUDE_PLUGIN_ROOT}/skills/impeccable/scripts/hook.mjs\" ] || ! { node -e \"import('node:fs').catch(()=>process.exit(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\"", + "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": "[ ! -f \"${CLAUDE_PLUGIN_ROOT}/skills/impeccable/scripts/hook.mjs\" ] || ! { node -e \"import('node:fs').catch(()=>process.exit(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\"", + "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 52604e348..66dff719f 100644 --- a/scripts/lib/transformers/hooks.js +++ b/scripts/lib/transformers/hooks.js @@ -47,63 +47,48 @@ 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. // -// The runtime is guarded too (issue #410): a `node` on PATH too old for ESM, -// or no node at all, kills the hook script while it is still being parsed, -// before the script's own always-exit-0 contract can run. Nothing written in -// ESM can report that, the doctor and the sub-commands included, so the command -// string carries the probe, and no-ops at exit 0 when it fails. +// 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. // -// The probe imports `node:fs` rather than `fs` because that is what the hook -// closure actually imports, and the `node:` scheme needs 14.18: a bare `fs` -// probe passes on 12 and 13, which then die on the real import. `.catch` with an -// explicit exit is required, not decoration, because before Node 15 an unhandled -// rejection is only a warning and the process still exits 0, so a rejected probe -// would read as a pass. What the probe does NOT do is enforce the engines floor -// of 22. That is deliberate: it asks whether this runtime can load our code, not -// whether it is supported, so a 14.18-to-21 runtime that works today keeps -// working instead of being silently switched off. The notice names 22 because -// that is the version to install, and it is only ever shown to someone whose -// runtime already failed the probe. -// -// `notice` is the shell that reports the dead runtime to the user, and it is -// passed in rather than baked in because only some harnesses have a channel for -// it. Checked against each harness's hook reference, on the events we hook: -// -// Claude Code PostToolUse + Stop: `systemMessage` is a universal field shown -// to the user, parsed on exit 0. -> notice -// Codex PostToolUse + Stop: `systemMessage` is documented as text shown -// as a warning in the UI or event stream. -> notice -// Cursor preToolUse: output is permission-shaped, and its `user_message` -// is shown only when the action is DENIED. Warning would mean -// blocking the edit, which is worse than silence. -> probe only -// Grok Build PostToolUse + Stop are passive events: stdout is ignored -// outright, so a notice cannot reach anyone. -> probe only -// Copilot postToolUse: output contract not confirmed. Silence is the -// conservative read; do not guess a shape. -> probe only -// -// A harness with no channel still gets the probe, so an unsupported runtime stays -// as quiet there as it was before the probe existed. +// `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 -e "import('node:fs').catch(()=>process.exit(1))" 2>/dev/null || { ${notice}; exit 0; }; }` - : `! node -e "import('node:fs').catch(()=>process.exit(1))" 2>/dev/null`; + ? `! { ${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. The marker under -// ~/.impeccable holds it to one notice per machine rather than one per edit. -const NODE_NOTICE_TEXT = 'The impeccable design hook is not running: no Node 22 or newer on PATH. ' +// 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 is per machine, not per harness: a machine running both should be -// told once, not once each. +// 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}"}'; }`; diff --git a/tests/hook-build.test.mjs b/tests/hook-build.test.mjs index 3e8754f32..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,14 +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) plus the runtime probe - // (issue #410: exits 0 when node cannot load ESM). GitHub's portable - // `$(git rev-parse)` form is guarded too, so it lands in the same branch. + // 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/); } @@ -40,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(); @@ -166,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'));