From 1a3f588c71799644a9de03f5ac1ae5b3a5cd442d Mon Sep 17 00:00:00 2001 From: Abdul Wahab <32850166+abdulwahabone@users.noreply.github.com> Date: Tue, 4 Aug 2026 02:55:01 +0500 Subject: [PATCH] Fix: skip POSIX hook guard on Windows installs (#452) (#453) * Fix: skip POSIX hook guard on Windows installs (#452) PowerShell rejects the `[ ! -f ... ] ||` guard at parse time, so Codex hooks generated by `npx impeccable install` on Windows never ran. Emit the direct `node "PATH"` invocation there; POSIX output is unchanged. AI-assisted (Cursor). Co-authored-by: Cursor * Keep the missing-file no-op in Windows-generated hook commands Greptile review on #453: project hook manifests are committable, so a bare `node "PATH"` written on Windows loses the silent no-op when a POSIX teammate without the skill consumes it. Replace the bare form with a shell-agnostic `node -e` existence guard that parses in PowerShell, cmd.exe, and sh, and forwards the hook's exit code. AI-assisted (Cursor). Co-authored-by: Cursor * Use Codex's commandWindows field for the Windows hook guard Per @PatrickSys on #452: Codex runs hooks through COMSPEC (cmd.exe /C), not PowerShell, and 0.146.0+ selects a commandWindows manifest field on Windows. Codex entries now always carry the POSIX guard in command plus an `if exist` cmd.exe guard in commandWindows (his Windows-tested form), so one .codex/hooks.json is correct on every OS regardless of where the install ran. Claude/Cursor keep the node -e wrapper on Windows installs since their manifests have no per-platform field. AI-assisted (Cursor). Co-authored-by: Cursor --------- Co-authored-by: Cursor --- cli/bin/commands/skills.mjs | 61 +++++++++++++++++++++++++++++-------- 1 file changed, 49 insertions(+), 12 deletions(-) diff --git a/cli/bin/commands/skills.mjs b/cli/bin/commands/skills.mjs index 85f5758f4..5366e6375 100644 --- a/cli/bin/commands/skills.mjs +++ b/cli/bin/commands/skills.mjs @@ -1340,7 +1340,41 @@ function hookScriptPathForProvider(skillRoot, provider) { // code when the file exists, so Claude's exit-2 blocking signal still reaches // the agent. POSIX-shell form, consistent with the project's other hook // commands (e.g. the GitHub manifest's `$(git rev-parse ...)`). -function guardHookCommand(quotedPath) { +// +// On Windows that guard is a hard failure, not a degraded one (issue #452). +// Codex runs hook commands through COMSPEC (`cmd.exe /C`), where `[` is not a +// command: the guard errors noisily and `||` then runs node even when the file +// is missing, trading the silent no-op for a MODULE_NOT_FOUND crash. Two +// remedies, by provider: +// +// * Codex manifests support a `commandWindows` sibling that Codex 0.146.0+ +// selects on Windows (`command_windows.unwrap_or(command)` in its hook +// discovery). rewriteHookCommandsForSkillRoot adds it with a cmd.exe +// `if exist` guard (form contributed and Windows-tested by @PatrickSys in +// issue #452; `exit /b` forwards node's errorlevel), so the same +// .codex/hooks.json is correct on every OS no matter where it was +// written, and `command` stays the plain POSIX guard. +// * Claude and Cursor manifests have no per-platform field, so a Windows +// install moves the existence check into node itself: a `node -e` wrapper +// that exits 0 when the target is missing and otherwise re-spawns node on +// it with inherited stdio, forwarding the hook's exit code. Cursor's +// hooks.json is committable and can be consumed on a teammate's POSIX +// machine, so the wrapper has to hold there too: it uses only characters +// that survive PowerShell, cmd.exe (issue #445: shims re-parse through +// `cmd /C`, which claims < > | & ^ % !), and sh double-quoting alike, +// with single quotes for the inner string literals. +const WIN32_HOOK_GUARD_SCRIPT = "const p=process.argv[1];const f=require('fs');if(f.existsSync(p)){const r=require('child_process').spawnSync(process.execPath,[p],{stdio:'inherit'});process.exit(r.status===null?1:r.status);}"; + +function windowsHookCommand(quotedPath) { + return `if exist ${quotedPath} (node ${quotedPath} & exit /b)`; +} + +function guardHookCommand(quotedPath, provider) { + // `.agents` (Codex) keeps the POSIX form unconditionally: its Windows + // consumers read the commandWindows sibling instead. + if (provider !== '.agents' && process.platform === 'win32') { + return `node -e "${WIN32_HOOK_GUARD_SCRIPT}" ${quotedPath}`; + } return `[ ! -f ${quotedPath} ] || node ${quotedPath}`; } @@ -1352,25 +1386,25 @@ function guardHookCommand(quotedPath) { // when a project hook points at a skill installed elsewhere (--scope=global). // * otherwise — keep the bundle's own ${CLAUDE_PROJECT_DIR}-relative path, // which correctly resolves for a project-scoped install. -// Either way the command is wrapped with the missing-file guard. +// Either way the command goes through guardHookCommand (POSIX shell guard, or +// the shell-agnostic node -e guard when installing on Windows), and Codex hook +// entries additionally get a `commandWindows` sibling for cmd.exe. function rewriteHookCommandsForSkillRoot(value, provider, { skillRoot, absolute }) { const hookScript = hookScriptPathForProvider(skillRoot, provider); // Providers we don't own a `node "PATH"` command hook for (.github, .grok) // carry their own portable command forms; leave them untouched. if (!hookScript) return value; + // Project-scope installs derive the provider's own project-relative path + // rather than trusting the bundle token, which for Codex points at + // `.codex/skills/...` while the CLI installs the skill at `.agents/skills/`. + const quotedPath = absolute + ? JSON.stringify(hookScript) + : JSON.stringify(hookScriptRelPathForProvider(provider)); + if (typeof value === 'string') { if (!valueHasImpeccableHookMarker(value)) return value; - let quotedPath; - if (absolute) { - quotedPath = JSON.stringify(hookScript); - } else { - // Project-scope install: derive the provider's own project-relative path - // rather than trusting the bundle token, which for Codex points at - // `.codex/skills/...` while the CLI installs the skill at `.agents/skills/`. - quotedPath = JSON.stringify(hookScriptRelPathForProvider(provider)); - } - return guardHookCommand(quotedPath); + return guardHookCommand(quotedPath, provider); } if (Array.isArray(value)) { return value.map(item => rewriteHookCommandsForSkillRoot(item, provider, { skillRoot, absolute })); @@ -1380,6 +1414,9 @@ function rewriteHookCommandsForSkillRoot(value, provider, { skillRoot, absolute for (const [key, child] of Object.entries(value)) { next[key] = rewriteHookCommandsForSkillRoot(child, provider, { skillRoot, absolute }); } + if (provider === '.agents' && typeof value.command === 'string' && valueHasImpeccableHookMarker(value.command)) { + next.commandWindows = windowsHookCommand(quotedPath); + } return next; } return value;