From 0c19098754d615ed80a10c76992b88fa5df03493 Mon Sep 17 00:00:00 2001 From: Abdul Wahab Date: Sun, 26 Jul 2026 20:58:49 +0500 Subject: [PATCH] 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 {