From 0db59088ff527c6b9952a8fa6445328e26652808 Mon Sep 17 00:00:00 2001 From: Abdul Wahab Date: Sun, 26 Jul 2026 12:46:11 +0500 Subject: [PATCH 1/6] Stop the design hook erroring on a node too old for ESM The hook command invokes bare `node`. When that node predates ESM, `hook.mjs` dies while it is still being parsed, before the script's own always-exit-0 contract can run, so node exits 1 and the harness reports a hook error on every Stop and every edit. Probe the runtime in the command string before invoking the hook, and route the Claude plugin manifest through the guard that already covered the project-local manifests. On probe failure the command exits 0 and emits a one-time `systemMessage` naming the two fixes available to the user, since nothing written in ESM can report this condition. Fixes #410. Co-Authored-By: Claude Opus 5 --- .claude/settings.json | 4 ++-- .codex/hooks.json | 4 ++-- .cursor/hooks.json | 2 +- plugin/hooks/hooks.json | 4 ++-- scripts/lib/transformers/hooks.js | 26 +++++++++++++++++++++++--- 5 files changed, 30 insertions(+), 10 deletions(-) diff --git a/.claude/settings.json b/.claude/settings.json index 0391eb720..6500e609d 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 \"import('fs')\" 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 \"import('fs')\" 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..d7cd1b798 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 \"import('fs')\" 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 \"import('fs')\" 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..e4dd64f5e 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 \"import('fs')\" 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 \".cursor/skills/impeccable/scripts/hook-before-edit.mjs\"", "timeout": 5 } ] diff --git a/plugin/hooks/hooks.json b/plugin/hooks/hooks.json index 66fd18e7e..772012c02 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 \"import('fs')\" 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 \"import('fs')\" 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..72d39e006 100644 --- a/scripts/lib/transformers/hooks.js +++ b/scripts/lib/transformers/hooks.js @@ -51,7 +51,27 @@ const CLAUDE_PROJECT_HOOK = '${CLAUDE_PROJECT_DIR}/.claude/skills/impeccable/scr // 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 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 reports it instead: probe dynamic import, and when the probe fails +// hand the user the two things they can act on (install a supported node, or +// drop the hook) through the universal `systemMessage` field on exit 0. A +// marker under ~/.impeccable keeps it to one notice per machine rather than one +// per edit. 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 the message, which travels in a single-quoted shell string. +const NODE_NOTICE_TEXT = '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.'; +const NODE_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 guardedNode = (hookPath) => + `[ ! -f "${hookPath}" ] || ! { node -e "import('fs')" 2>/dev/null || { ${NODE_NOTICE}; exit 0; }; } ` + + `|| node "${hookPath}"`; 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 @@ -104,14 +124,14 @@ export function buildClaudePluginHooksManifest() { hooks: [ { type: 'command', - command: `node "${CLAUDE_PLUGIN_HOOK}"`, + command: guardedNode(CLAUDE_PLUGIN_HOOK), timeout: TIMEOUT_SECONDS, statusMessage: STATUS_MESSAGE, }, ], }, ], - Stop: [stopEntry(`node "${CLAUDE_PLUGIN_HOOK}"`)], + Stop: [stopEntry(guardedNode(CLAUDE_PLUGIN_HOOK))], }, }; } From 8397d532b9c6f7f9e8f2259e964dbbf89faf26ac Mon Sep 17 00:00:00 2001 From: Abdul Wahab Date: Sun, 26 Jul 2026 13:37:09 +0500 Subject: [PATCH 2/6] Keep the unsupported-node notice to the harness that can render it `systemMessage` on stdout is a Claude Code contract. The shared guard was emitting it for Codex and Cursor too, where what a harness does with stdout it did not ask for is unconfirmed, and a Cursor preToolUse hook printing an unexpected JSON object is the wrong thing to guess about. Pass the notice in per harness instead of baking it into the guard. Claude manifests opt in; Codex and Cursor take the runtime probe alone, so an unsupported runtime stays as quiet there as it was before the probe existed. Giving them their own shape later is one more argument at the call site. Co-Authored-By: Claude Opus 5 --- .codex/hooks.json | 4 +-- .cursor/hooks.json | 2 +- scripts/lib/transformers/hooks.js | 44 ++++++++++++++++++------------- 3 files changed, 29 insertions(+), 21 deletions(-) diff --git a/.codex/hooks.json b/.codex/hooks.json index d7cd1b798..b6784765e 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('fs')\" 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 \"import('fs')\" 2>/dev/null || 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('fs')\" 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 \"import('fs')\" 2>/dev/null || node \".codex/skills/impeccable/scripts/hook.mjs\"", "timeout": 30, "statusMessage": "Design deep pass" } diff --git a/.cursor/hooks.json b/.cursor/hooks.json index e4dd64f5e..c7dc57cea 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('fs')\" 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 \".cursor/skills/impeccable/scripts/hook-before-edit.mjs\"", + "command": "[ ! -f \".cursor/skills/impeccable/scripts/hook-before-edit.mjs\" ] || ! node -e \"import('fs')\" 2>/dev/null || node \".cursor/skills/impeccable/scripts/hook-before-edit.mjs\"", "timeout": 5 } ] diff --git a/scripts/lib/transformers/hooks.js b/scripts/lib/transformers/hooks.js index 72d39e006..e92ae29d8 100644 --- a/scripts/lib/transformers/hooks.js +++ b/scripts/lib/transformers/hooks.js @@ -56,22 +56,30 @@ const CLAUDE_PROJECT_HOOK = '${CLAUDE_PROJECT_DIR}/.claude/skills/impeccable/scr // 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 reports it instead: probe dynamic import, and when the probe fails -// hand the user the two things they can act on (install a supported node, or -// drop the hook) through the universal `systemMessage` field on exit 0. A -// marker under ~/.impeccable keeps it to one notice per machine rather than one -// per edit. 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 the message, which travels in a single-quoted shell string. -const NODE_NOTICE_TEXT = 'The impeccable design hook is not running: no Node 22 or newer on PATH. ' +// string carries the probe: dynamic import, and no-op at exit 0 when it fails. +// +// `notice` is the shell that reports the dead runtime to the user, and it is +// passed in rather than baked in because the wire format is per harness. Claude +// Code reads a `systemMessage` field off stdout on exit 0; what Codex and Cursor +// do with stdout they did not ask for is unconfirmed, so they take the probe +// alone and an unsupported runtime stays as quiet there as it was before the +// probe existed. Adding their shape later is one more argument at the call site. +const guardedNode = (hookPath, notice = '') => { + const probe = notice + ? `! { node -e "import('fs')" 2>/dev/null || { ${notice}; exit 0; }; }` + : `! node -e "import('fs')" 2>/dev/null`; + 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 CLAUDE_NODE_NOTICE_TEXT = '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.'; -const NODE_NOTICE = 'D="$HOME/.impeccable"; [ -f "$D/node-unsupported" ] || ' +const CLAUDE_NODE_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 guardedNode = (hookPath) => - `[ ! -f "${hookPath}" ] || ! { node -e "import('fs')" 2>/dev/null || { ${NODE_NOTICE}; exit 0; }; } ` - + `|| node "${hookPath}"`; + + `printf '%s' '{"systemMessage":"${CLAUDE_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 @@ -97,14 +105,14 @@ export function buildClaudeSettingsManifest() { hooks: [ { type: 'command', - command: guardedNode(CLAUDE_PROJECT_HOOK), + command: guardedNode(CLAUDE_PROJECT_HOOK, CLAUDE_NODE_NOTICE), timeout: TIMEOUT_SECONDS, statusMessage: STATUS_MESSAGE, }, ], }, ], - Stop: [stopEntry(guardedNode(CLAUDE_PROJECT_HOOK))], + Stop: [stopEntry(guardedNode(CLAUDE_PROJECT_HOOK, CLAUDE_NODE_NOTICE))], }, }; } @@ -124,14 +132,14 @@ export function buildClaudePluginHooksManifest() { hooks: [ { type: 'command', - command: guardedNode(CLAUDE_PLUGIN_HOOK), + command: guardedNode(CLAUDE_PLUGIN_HOOK, CLAUDE_NODE_NOTICE), timeout: TIMEOUT_SECONDS, statusMessage: STATUS_MESSAGE, }, ], }, ], - Stop: [stopEntry(guardedNode(CLAUDE_PLUGIN_HOOK))], + Stop: [stopEntry(guardedNode(CLAUDE_PLUGIN_HOOK, CLAUDE_NODE_NOTICE))], }, }; } From 0c19098754d615ed80a10c76992b88fa5df03493 Mon Sep 17 00:00:00 2001 From: Abdul Wahab Date: Sun, 26 Jul 2026 20:58:49 +0500 Subject: [PATCH 3/6] Guard the remaining harness manifests against a dead node runtime Bugbot caught the Codex plugin builder still invoking node directly, and the same reasoning covers GitHub Copilot and Grok Build: all three shipped the exact failure this branch exists to stop, and sat visibly inconsistent with their guarded siblings. Route them through guardedNode with no notice, matching Codex and Cursor. GitHub gains a second property from it: outside a git repository `$(git rev-parse --show-toplevel)` expands to nothing, so the old command handed node a path that could not exist and failed the turn. The file test now short-circuits that to exit 0. Every builder carries the probe; only the two Claude manifests carry the notice, which is the only harness whose response shape is confirmed. Co-Authored-By: Claude Opus 5 --- .github/hooks/impeccable.json | 2 +- .grok/hooks/impeccable.json | 4 ++-- scripts/lib/transformers/hooks.js | 20 +++++++++++--------- tests/hook-build.test.mjs | 5 +++-- 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/.github/hooks/impeccable.json b/.github/hooks/impeccable.json index 0e9378094..a909ad43b 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 \"import('fs')\" 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..48b80bf30 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 \"import('fs')\" 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 \"import('fs')\" 2>/dev/null || node \".grok/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 e92ae29d8..443ab2ef3 100644 --- a/scripts/lib/transformers/hooks.js +++ b/scripts/lib/transformers/hooks.js @@ -60,10 +60,12 @@ const CLAUDE_PROJECT_HOOK = '${CLAUDE_PROJECT_DIR}/.claude/skills/impeccable/scr // // `notice` is the shell that reports the dead runtime to the user, and it is // passed in rather than baked in because the wire format is per harness. Claude -// Code reads a `systemMessage` field off stdout on exit 0; what Codex and Cursor -// do with stdout they did not ask for is unconfirmed, so they take the probe -// alone and an unsupported runtime stays as quiet there as it was before the -// probe existed. Adding their shape later is one more argument at the call site. +// Code reads a `systemMessage` field off stdout on exit 0. Codex expects +// `hookSpecificOutput` and Cursor a permission-shaped payload, so handing either +// a Claude response gets it discarded or printed raw; until those shapes are +// confirmed every non-Claude harness takes the probe alone, and an unsupported +// runtime stays as quiet there as it was before the probe existed. Adding a +// shape later is one more argument at that harness's call site. const guardedNode = (hookPath, notice = '') => { const probe = notice ? `! { node -e "import('fs')" 2>/dev/null || { ${notice}; exit 0; }; }` @@ -156,14 +158,14 @@ export function buildCodexPluginHooksManifest() { hooks: [ { type: 'command', - command: `node "${CODEX_PLUGIN_HOOK}"`, + command: guardedNode(CODEX_PLUGIN_HOOK), timeout: TIMEOUT_SECONDS, statusMessage: STATUS_MESSAGE, }, ], }, ], - Stop: [stopEntry(`node "${CODEX_PLUGIN_HOOK}"`)], + Stop: [stopEntry(guardedNode(CODEX_PLUGIN_HOOK))], }, }; } @@ -226,7 +228,7 @@ export function buildGitHubHooksManifest() { { type: 'command', matcher: 'edit|create|apply_patch', - bash: `node "${GITHUB_PROJECT_HOOK}"`, + bash: guardedNode(GITHUB_PROJECT_HOOK), timeoutSec: TIMEOUT_SECONDS, }, ], @@ -248,14 +250,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..3e8754f32 100644 --- a/tests/hook-build.test.mjs +++ b/tests/hook-build.test.mjs @@ -28,8 +28,9 @@ function readJson(rel) { 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 + // (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. if (command.startsWith('[ ! -f "')) { assert.match(command, /\|\| node "/); } else { From 4f999ceff84d1d13d12a6778b3ab2330667019a3 Mon Sep 17 00:00:00 2001 From: Abdul Wahab Date: Sun, 26 Jul 2026 21:10:41 +0500 Subject: [PATCH 4/6] Give Codex the notice too; its hook reference documents systemMessage Commit 8397d532 took a reviewer's word that Codex expects hookSpecificOutput and dropped its notice on that basis. Codex documents `systemMessage` for PostToolUse and Stop as text shown as a warning in the UI or event stream, the same field Claude Code reads, so the notice belongs there and the earlier comment asserted something unverified. Checked the rest against their own references while here. Cursor's preToolUse output is permission-shaped and its user_message renders only when the action is DENIED, so warning would mean blocking the edit. Grok treats PostToolUse and Stop as passive events and ignores stdout outright. Copilot's contract is unconfirmed. Those three keep the probe alone, which is a verified limit now rather than an assumption. Co-Authored-By: Claude Opus 5 --- .codex/hooks.json | 4 +-- scripts/lib/transformers/hooks.js | 49 +++++++++++++++++++------------ 2 files changed, 33 insertions(+), 20 deletions(-) diff --git a/.codex/hooks.json b/.codex/hooks.json index b6784765e..d7cd1b798 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('fs')\" 2>/dev/null || node \".codex/skills/impeccable/scripts/hook.mjs\"", + "command": "[ ! -f \".codex/skills/impeccable/scripts/hook.mjs\" ] || ! { node -e \"import('fs')\" 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('fs')\" 2>/dev/null || node \".codex/skills/impeccable/scripts/hook.mjs\"", + "command": "[ ! -f \".codex/skills/impeccable/scripts/hook.mjs\" ] || ! { node -e \"import('fs')\" 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/scripts/lib/transformers/hooks.js b/scripts/lib/transformers/hooks.js index 443ab2ef3..f4745f125 100644 --- a/scripts/lib/transformers/hooks.js +++ b/scripts/lib/transformers/hooks.js @@ -59,13 +59,23 @@ const CLAUDE_PROJECT_HOOK = '${CLAUDE_PROJECT_DIR}/.claude/skills/impeccable/scr // string carries the probe: dynamic import, and no-op at exit 0 when it fails. // // `notice` is the shell that reports the dead runtime to the user, and it is -// passed in rather than baked in because the wire format is per harness. Claude -// Code reads a `systemMessage` field off stdout on exit 0. Codex expects -// `hookSpecificOutput` and Cursor a permission-shaped payload, so handing either -// a Claude response gets it discarded or printed raw; until those shapes are -// confirmed every non-Claude harness takes the probe alone, and an unsupported -// runtime stays as quiet there as it was before the probe existed. Adding a -// shape later is one more argument at that harness's call site. +// 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. const guardedNode = (hookPath, notice = '') => { const probe = notice ? `! { node -e "import('fs')" 2>/dev/null || { ${notice}; exit 0; }; }` @@ -77,11 +87,14 @@ const guardedNode = (hookPath, notice = '') => { // 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 CLAUDE_NODE_NOTICE_TEXT = 'The impeccable design hook is not running: no Node 22 or newer on PATH. ' +const NODE_NOTICE_TEXT = '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.'; -const CLAUDE_NODE_NOTICE = 'D="$HOME/.impeccable"; [ -f "$D/node-unsupported" ] || ' +// 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. +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":"${CLAUDE_NODE_NOTICE_TEXT}"}'; }`; + + `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 @@ -107,14 +120,14 @@ export function buildClaudeSettingsManifest() { hooks: [ { type: 'command', - command: guardedNode(CLAUDE_PROJECT_HOOK, CLAUDE_NODE_NOTICE), + command: guardedNode(CLAUDE_PROJECT_HOOK, SYSTEM_MESSAGE_NOTICE), timeout: TIMEOUT_SECONDS, statusMessage: STATUS_MESSAGE, }, ], }, ], - Stop: [stopEntry(guardedNode(CLAUDE_PROJECT_HOOK, CLAUDE_NODE_NOTICE))], + Stop: [stopEntry(guardedNode(CLAUDE_PROJECT_HOOK, SYSTEM_MESSAGE_NOTICE))], }, }; } @@ -134,14 +147,14 @@ export function buildClaudePluginHooksManifest() { hooks: [ { type: 'command', - command: guardedNode(CLAUDE_PLUGIN_HOOK, CLAUDE_NODE_NOTICE), + command: guardedNode(CLAUDE_PLUGIN_HOOK, SYSTEM_MESSAGE_NOTICE), timeout: TIMEOUT_SECONDS, statusMessage: STATUS_MESSAGE, }, ], }, ], - Stop: [stopEntry(guardedNode(CLAUDE_PLUGIN_HOOK, CLAUDE_NODE_NOTICE))], + Stop: [stopEntry(guardedNode(CLAUDE_PLUGIN_HOOK, SYSTEM_MESSAGE_NOTICE))], }, }; } @@ -158,14 +171,14 @@ export function buildCodexPluginHooksManifest() { hooks: [ { type: 'command', - command: guardedNode(CODEX_PLUGIN_HOOK), + command: guardedNode(CODEX_PLUGIN_HOOK, SYSTEM_MESSAGE_NOTICE), timeout: TIMEOUT_SECONDS, statusMessage: STATUS_MESSAGE, }, ], }, ], - Stop: [stopEntry(guardedNode(CODEX_PLUGIN_HOOK))], + Stop: [stopEntry(guardedNode(CODEX_PLUGIN_HOOK, SYSTEM_MESSAGE_NOTICE))], }, }; } @@ -183,14 +196,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))], }, }; } From 86cdf528c5c4cecaa15c6f822256bf4223114653 Mon Sep 17 00:00:00 2001 From: Abdul Wahab Date: Sun, 26 Jul 2026 21:20:26 +0500 Subject: [PATCH 5/6] Probe the import the hook actually uses, and fail closed on rejection Greptile flagged that the probe does not enforce the Node 22 engines floor. Two parts to that, and they land differently. The real defect is narrower and worse than stated: the hook closure imports `node:fs`, `node:os`, `node:path` and `node:url`, and the `node:` scheme needs 14.18, so a bare `import('fs')` probe passed on 12 and 13 and those runtimes then died on the real import, which is the banner this branch exists to remove. Probing `node:fs` closes that. The added `.catch(()=>process.exit(1))` is load bearing rather than tidiness: before Node 15 an unhandled rejection is only a warning and the process still exits 0, so a rejected probe would have read as a pass on exactly the versions in question. Not enforcing 22 is deliberate and stays. The probe asks whether this runtime can load our code, not whether it is a supported one, so a 14.18-to-21 runtime that works today keeps working rather than being silently switched off. The notice names 22 because that is the version worth installing, and it only ever reaches someone whose runtime already failed the probe, so no user is shown a threshold that contradicts what ran. Co-Authored-By: Claude Opus 5 --- .claude/settings.json | 4 ++-- .codex/hooks.json | 4 ++-- .cursor/hooks.json | 2 +- .github/hooks/impeccable.json | 2 +- .grok/hooks/impeccable.json | 4 ++-- plugin/hooks/hooks.json | 4 ++-- scripts/lib/transformers/hooks.js | 18 +++++++++++++++--- 7 files changed, 25 insertions(+), 13 deletions(-) diff --git a/.claude/settings.json b/.claude/settings.json index 6500e609d..44376689b 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('fs')\" 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 \"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\"", "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('fs')\" 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 \"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\"", "timeout": 30, "statusMessage": "Design deep pass" } diff --git a/.codex/hooks.json b/.codex/hooks.json index d7cd1b798..000824e04 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('fs')\" 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 \"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\"", "timeout": 5, "statusMessage": "Checking UI changes" } @@ -18,7 +18,7 @@ "hooks": [ { "type": "command", - "command": "[ ! -f \".codex/skills/impeccable/scripts/hook.mjs\" ] || ! { node -e \"import('fs')\" 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 \"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\"", "timeout": 30, "statusMessage": "Design deep pass" } diff --git a/.cursor/hooks.json b/.cursor/hooks.json index c7dc57cea..5c8616a5f 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('fs')\" 2>/dev/null || node \".cursor/skills/impeccable/scripts/hook-before-edit.mjs\"", + "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\"", "timeout": 5 } ] diff --git a/.github/hooks/impeccable.json b/.github/hooks/impeccable.json index a909ad43b..c51a954ac 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('fs')\" 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 \"import('node:fs').catch(()=>process.exit(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 48b80bf30..3cb2f24ed 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('fs')\" 2>/dev/null || node \".grok/skills/impeccable/scripts/hook.mjs\"", + "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\"", "timeout": 5, "statusMessage": "Checking UI changes" } @@ -18,7 +18,7 @@ "hooks": [ { "type": "command", - "command": "[ ! -f \".grok/skills/impeccable/scripts/hook.mjs\" ] || ! node -e \"import('fs')\" 2>/dev/null || node \".grok/skills/impeccable/scripts/hook.mjs\"", + "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\"", "timeout": 30, "statusMessage": "Design deep pass" } diff --git a/plugin/hooks/hooks.json b/plugin/hooks/hooks.json index 772012c02..501410231 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('fs')\" 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 \"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\"", "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('fs')\" 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 \"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\"", "timeout": 30, "statusMessage": "Design deep pass" } diff --git a/scripts/lib/transformers/hooks.js b/scripts/lib/transformers/hooks.js index f4745f125..52604e348 100644 --- a/scripts/lib/transformers/hooks.js +++ b/scripts/lib/transformers/hooks.js @@ -56,7 +56,19 @@ const CLAUDE_PROJECT_HOOK = '${CLAUDE_PROJECT_DIR}/.claude/skills/impeccable/scr // 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: dynamic import, and no-op at exit 0 when it fails. +// string carries the probe, and no-ops at exit 0 when it fails. +// +// 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 @@ -78,8 +90,8 @@ const CLAUDE_PROJECT_HOOK = '${CLAUDE_PROJECT_DIR}/.claude/skills/impeccable/scr // as quiet there as it was before the probe existed. const guardedNode = (hookPath, notice = '') => { const probe = notice - ? `! { node -e "import('fs')" 2>/dev/null || { ${notice}; exit 0; }; }` - : `! node -e "import('fs')" 2>/dev/null`; + ? `! { 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`; return `[ ! -f "${hookPath}" ] || ${probe} || node "${hookPath}"`; }; // The message says `on PATH` deliberately: the common cause is a hook shell whose From fd9076f4f02cb25dcec2ce0790375f9ebf994b8f Mon Sep 17 00:00:00 2001 From: Abdul Wahab Date: Sun, 26 Jul 2026 22:17:19 +0500 Subject: [PATCH 6/6] Enforce the engines floor in the probe instead of a capability check The probe asked whether node could load ESM, while the notice promised a Node 22 floor and package.json engines declares >=22.12.0. Reviewers kept flagging the gap, and they were right to: a 14.18-to-21 runtime passed the probe on the strength of one import while the hook and its detector bundle are only ever exercised on the engines floor, so "can load our code" was a weaker claim than the one being made for it. Check the floor directly: parseInt(process.versions.node) >= 22, in ES5-only syntax that parses on any node old enough to fail it. Probe and notice now derive from one NODE_MAJOR_FLOOR constant, so they cannot disagree, and the archaeology about node: scheme support and pre-15 unhandled-rejection semantics goes with the import it explained. Add the missing contract test: every generated hook command carries the probe, the notice appears exactly where a harness can render it (Claude and Codex, project and plugin), and the expected floor is read from package.json engines rather than repeated by hand. Verified against a fake pre-22 node, no node, and a real node: one notice then the marker holds it silent, exit 0 in every failure shape, and the hook's own exit code still passes through on a supported runtime. Co-Authored-By: Claude Fable 5 (via Cursor) Co-authored-by: Cursor --- .claude/settings.json | 4 +- .codex/hooks.json | 4 +- .cursor/hooks.json | 2 +- .github/hooks/impeccable.json | 2 +- .grok/hooks/impeccable.json | 4 +- plugin/hooks/hooks.json | 4 +- scripts/lib/transformers/hooks.js | 75 +++++++++++++------------------ tests/hook-build.test.mjs | 66 +++++++++++++++++++++++++-- 8 files changed, 103 insertions(+), 58 deletions(-) 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'));