[codex] Fix Codex Windows hook fallback#1555
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes Codex hook installation on Windows by introducing Windows-native production hook wrappers (avoiding sh -c), switching Codex to those wrappers when a compatibility probe indicates the shell wrapper path is broken, and ensuring hook installs replace stale managed commands instead of accumulating duplicates.
Changes:
- Added
cmd.exe/where.exe-guarded production hook wrappers (silent + JSON/plain warning) and extended managed-hook detection to recognize them. - Updated Codex hook installation to choose Windows wrappers on Windows when the compatibility probe fails, and to replace stale managed hook commands rather than appending duplicates.
- Added/expanded tests around the new Windows wrappers, managed-hook detection, and Codex install migration behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| cmd/entire/cli/agent/hook_command.go | Adds Windows-native production hook wrappers + managed-hook detection updates + CMD escaping helper. |
| cmd/entire/cli/agent/hook_command_test.go | Adds unit tests for Windows wrappers and managed-hook detection. |
| cmd/entire/cli/agent/codex/hooks.go | Adds Windows wrapper compatibility probing and hook-command sync (replace vs append) for Codex installs. |
| cmd/entire/cli/agent/codex/hooks_test.go | Adds tests for Windows probe success/failure paths and migration behavior. |
3a6e411 to
7bea261
Compare
Entire-Checkpoint: 5ac1e71750f3
ea96703 to
a4d1ec9
Compare
Entire-Checkpoint: 36599a4bbda6
0d4399b to
db20ff4
Compare
Addresses the trail #684 review findings on the Windows hook fallback. - Restructure the cmd.exe wrappers from `where … || exit /b 0 & command` to `where … & if errorlevel 1 (…) else (command)`. The wrapped command now lives inside the else branch, so there is no unconditional trailing command and correctness no longer depends on `exit /b` aborting the whole `cmd /c` line from inside a parenthesized block. Present -> command runs and its exit code propagates; absent -> silent/warning branch, exit 0. - Add an execution-level test that runs each wrapper through cmd.exe and asserts present-runs / present-propagates-exit / absent-skips-exit-0 / JSON-valid behavior (skips off-Windows). - Lift the probe + OS gate + wrapper selection out of the codex package into the shared agent layer (UseWindowsProductionHooks, WrapProduction{Silent,JSONWarning}HookCommandForOS, SetWindowsHookProbeForTesting) so the other sh-based agents can adopt the Windows fallback without re-implementing it. codex/hooks.go is now a thin caller. Document why the probe is intentionally not memoized (re-probing is what drives self-correcting hook migration). - Stop caret-escaping `%` in escapeWindowsCMD: these are cmd /c command lines, not batch scripts, so `^%` is wrong and a lone `%` is already literal. Document the distinction and update the test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Soph
approved these changes
Jun 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://entire.io/gh/entireio/cli/trails/684
Summary
sh -cwhile preserving the missing-entirePATH guard.cmd.exewrappers when the existing shell wrapper probe fails..gitignoreconflict markers and delete strayt1.txt.Why
A Windows user had Codex hooks fail with exit code 1 when hooks were enabled and
entirewas installed, because the longsh -cproduction hook wrapper was not usable in that environment. The targeted fallback keeps the normal wrapper behavior for unaffected installs, but uses a Windows-compatible guarded wrapper when the probe detects the shell wrapper path is broken.Validation
mise run checkmise run lintgo test ./cmd/entire/cli/agent/... -count=1git diff --check origin/mainentire review --profile codex-temp --base origin/main --agent codexreturnedapprove - no actionable defects found.Note
Medium Risk
Changes how production hook commands are written and recognized on Windows; mis-detection could break Codex hooks or leave duplicate/stale entries, though scope is limited to Codex install paths and shared wrapper helpers with tests.
Overview
Fixes Codex production hook installs on Windows where long
sh -cwrappers fail in native shells, while keeping the existing Unix-style wrappers when a runtime probe showssh -cstill works.Adds
cmd.exe/where.exeproduction wrappers in the shared agent hook helpers (silent exit and JSON/plain missing-entirewarnings with CMD escaping), and extendsIsManagedHookCommandso uninstall/managed-hook detection recognizes those Windows wrapper forms.Codex
InstallHookspicks Windows vs shell wrappers on Windows production installs based on the probe, andsyncHookCommandreplaces stale Entire hook entries when the desired command string changes instead of leaving duplicates.Reviewed by Cursor Bugbot for commit c713d66. Configure here.