fix(plugin): give the Codex hooks a Windows command that avoids WSL bash - #4008
fix(plugin): give the Codex hooks a Windows command that avoids WSL bash#4008worktrunk-bot wants to merge 5 commits into
Conversation
Codex resolves a hook `command` through the platform shell — `/bin/sh -lc` on Unix, `cmd.exe /C` on Windows. Under cmd.exe the leading bare `bash` resolves through the Windows PATH to System32\bash.exe, the WSL launcher rather than Git Bash, which refuses to start in a sandboxed session (Bash/Service/CreateInstance/E_ACCESSDENIED). Every prompt, permission request, turn end, and session end then raised a "Hook failed" banner. Each hook now carries Codex's per-handler `commandWindows`, which replaces `command` on Windows. It calls a new cmd.exe shim, hooks/wt.cmd, that mirrors wt.sh's resolution — WORKTRUNK_BIN, else git-wt.exe, else a `wt.exe` PATH entry that isn't Windows Terminal's app alias — and tails with `|| exit /b 0` so a missing install stays best-effort, as `|| true` does on Unix. Closes #4007
worktrunk-bot
left a comment
There was a problem hiding this comment.
Verified the mechanism against the upstream sources the PR cites, since the whole change rests on them: HookHandlerConfig::Command in codex-rs/config/src/hook_config.rs carries #[serde(default, rename = "commandWindows", alias = "command_windows")], discovery.rs does command_windows.unwrap_or(command) under cfg!(windows), its substitution fold is command.replace(&format!("${{{key}}}"), value) — braced form only — and command_runner.rs spawns cmd.exe /C with raw_arg(format!(r#""{command_line}""#)). All four claims hold, and the enum has no deny_unknown_fields, so an older Codex ignores the new key rather than failing the manifest. test (windows) is green on this head, which is what exercises the cmd.exe quoting, the goto resolution, and the || exit /b 0 tail.
One finding, inline: the shim doesn't clear its own locals, so an inherited WT bypasses the PATH scan it exists to perform.
…ass the PATH scan setlocal scopes writes but inherits the caller's environment, and Codex hands each hook the session env snapshot, so a user's own WT arrives defined. The PATH scan then never runs: 'if not defined WT' is false on every iteration, 'if defined WT goto :run' fires, and the shim executes the inherited value — skipping the Windows Terminal rejection that block exists for. WT_SEEN had the mirror problem, picking the 'resolves to Windows Terminal' message when nothing was found.
worktrunk-bot
left a comment
There was a problem hiding this comment.
The inherited-WT finding from the last pass is addressed in c9a1721, and test_shim_ignores_an_inherited_wt is a real regression test for it — it fails on the pre-fix shim, and it stays honest about the runner's own PATH by pinning "the sentinel did not run" rather than "worktrunk ran".
Two things from this pass, both inline. The one worth a look before merge is the shim's binary resolution: where and cmd's bare-name lookup both reach the current directory, which is the repo the hook is running in — that is new surface relative to wt.sh, whose command -v is PATH-only. It is my own judgement that this wants a human rather than a repo rule; the deletion-surface hold in the review guidance doesn't cover it, since nothing here can delete anything. The other is the changelog entry.
Self-authored, so no approval either way. test (linux|macos|windows) and code-coverage were still in flight at this head; nothing had gone red.
`where git-wt.exe` searches the current directory before PATH, and cmd resolves the bare `git-wt.exe` that `:gitwt` stored the same way. A hook runs with the user's project as its current directory, so a `git-wt.exe` -- or a `wt.exe`, through the scan below it -- committed to a repo was what every prompt, permission request, stop, and session end executed. `wt.sh` has no such surface: `command -v` consults PATH only. Both lookups now use `where`'s `$PATH:` prefix, which searches only the directories that variable names, and `:run` executes the absolute path they return, so nothing re-resolves a bare name. `:gitwt` is gone with its bare-name store. `test_shim_ignores_a_binary_in_the_current_directory` plants decoys in the cwd and a real worktrunk on PATH; the decoys are not valid executables, so a version line pins that the shim resolved through PATH. Also folds in the review's changelog edit -- the entry cites its own PR and comes back under the 60-word ceiling -- and teaches the release skill to rename an existing `## Unreleased` heading rather than insert `## X.Y.Z` above it, which would ship the stale heading in the notes.
The new cwd test asserted the `--version` output carried CARGO_PKG_VERSION. CI's checkout embeds a bare commit hash instead (`wt ad1a14b`), so the assertion failed on the Windows leg even though the shim resolved correctly through PATH. Pin the `wt ` prefix, which is the part a decoy could never produce.
worktrunk-bot
left a comment
There was a problem hiding this comment.
test (windows) was red at fd89ee1b on this PR's own new test, and 9e7458d3 fixes it correctly — starts_with("wt ") is the pin test_mock_dispatch_ignores_degenerate_argv0 in tests/integration_tests/init.rs already uses for the same question. Recording why it failed, since the old assertion could never have passed here: version_str() in src/cli/mod.rs resolves option_env!("VERGEN_GIT_DESCRIBE") first and falls back to CARGO_PKG_VERSION only when the build script couldn't set it — the crates.io-package case, not CI.
That failed run is also the evidence the $PATH: prefix was still missing. The shim exited 0 and printed wt ad1a14b, so where "$PATH:git-wt.exe" resolved to the copy on the pinned PATH and not to the invalid git-wt.exe decoy in the cwd — had it taken the decoy, cmd could not have run it at all. The syntax no longer rests on the where documentation; it ran, and 4332 of 4333 tests passed on that leg with nothing else red.
One finding on this pass, inline: eight lines in the new test that restate a guarantee std::process::Command already gives.
Self-authored, so no approval either way. CI at 9e7458d3 is still in flight.
std::process::Command's environment map is case-insensitive on Windows (EnvKey compares with CompareStringOrdinal, bIgnoreCase = TRUE), so .env("PATH", ...) replaces an inherited `Path` rather than landing beside it. The vars_os scan restated that guarantee.
Problem
Codex resolves a hook
commandthrough the platform shell —/bin/sh -lcon Unix,cmd.exe /Con Windows (default_shell_command). All four Codex hooks in the plugin manifest lead with a barebash, and undercmd.exethat resolves through the Windows PATH toSystem32\bash.exe— the WSL launcher, not Git Bash. In a sandboxed session the launcher refuses to start (Access is denied. Error code: Bash/Service/CreateInstance/E_ACCESSDENIED), so every prompt, permission request, turn end, and session end raises aHook failedbanner (#4007).Solution
Each hook now also carries Codex's per-handler
commandWindows, which replacescommandon Windows (command_windows.unwrap_or(command)incodex-rs/hooks/src/engine/discovery.rs). It calls a newcmd.exeshim,plugins/worktrunk/hooks/wt.cmd, that mirrorswt.sh's resolution:WORKTRUNK_BIN, elsegit-wt.exe, else awt.exePATH entry that isn't Windows Terminal's app-execution alias — never barewt, which is Windows Terminal's name. The tail is|| exit /b 0, the cmd.exe spelling of the Unix|| true: a marker is decoration, and a nonzero exit is what raises the banner.The Windows commands brace the plugin root as
${PLUGIN_ROOT}because Codex substitutes only that form textually, before the shell runs; the unbraced$PLUGIN_ROOTthe Unix commands use survives to/bin/sh, andcmd.exewould pass it through literally..gitattributespins*.cmdto a CRLF checkout.Testing
Four tests, all new:
test_codex_hooks_carry_windows_commands(all platforms) — pins that every Codex command hook has acommandWindows, that it invokes neitherbashnor barewt, that it calls the shim, and that the twoPLUGIN_ROOTspellings stay on their respective sides. Written first: it failed on the manifest as shipped, with the exact hook command from the report.test_codex_windows_hook_commands_set_the_marker(Windows leg of CI) — runs the realcommandWindowsthe way Codex spawns it, reproducing both steps: the${PLUGIN_ROOT}substitution andcmd.exe /C "<command>"with the command line as a single quoted raw argument. It assertsUserPromptSubmitstores 🤖,Stopreplaces it with 💬,SessionEndclears it, and that a hook which cannot find worktrunk still exits 0. That covers cmd.exe's quote handling, the shim'sgoto-based resolution, and the emoji argument surviving cmd.exe intact.test_shim_ignores_an_inherited_wt(Windows leg) —setlocalinherits the caller's environment and Codex hands each hook the session env snapshot, so an inheritedWTwould short-circuit the PATH scan onto whatever it names. The test pointsWTat a sentinel script and pins that the sentinel never runs.test_shim_ignores_a_binary_in_the_current_directory(Windows leg) — a hook runs with the user's project as its current directory, and bothwhereand cmd's bare-name lookup reach there before PATH. The test plants unrunnablegit-wt.exeandwt.exedecoys in the cwd, puts a real worktrunk on the pinned PATH, and asserts the shim prints a version line — which it could not do had it taken a decoy.What this does not verify
Nothing here drives a real Codex session on Windows, so the end-to-end claim — that Codex selects
commandWindowsand that the banner stops — rests on readingcodex-rs/hooks/src/engine/{discovery,command_runner}.rsrather than on observation. What CI does exercise is the command string itself, executed the way that source says Codex executes it.The shim's Windows-Terminal-rejection branch is unexercised by CI: reaching it means a
WindowsApps\wt.exeon PATH, which the runner image does not provide and a decoy cannot stand in for, since the rejection matches the resolved path. Thegit-wt.exebranch is covered bytest_shim_ignores_a_binary_in_the_current_directory, and theWORKTRUNK_BINbranch by the marker test; both share the whole:runtail with the unexercised one.Two adjacent things are deliberately left alone, as separate concerns: the Gemini hooks at the repo-root
hooks/hooks.jsonuse the same barebash(Gemini's Windows hook execution isn't established here), and Claude'shooks/hooks.jsonis unaffected because Claude Code runs hook commands through Git Bash on Windows.Closes #4007 — automated triage