mirror of
https://github.com/max-sixty/worktrunk.git
synced 2026-09-14 20:00:38 +08:00
80e795b271
## Problem Codex resolves a hook `command` through the platform shell — `/bin/sh -lc` on Unix, `cmd.exe /C` on Windows ([`default_shell_command`](https://github.com/openai/codex/blob/main/codex-rs/hooks/src/engine/command_runner.rs)). All four Codex hooks in the plugin manifest lead with a bare `bash`, and under `cmd.exe` that resolves through the Windows PATH to `System32\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 a `Hook failed` banner (#4007). ## Solution Git for Windows stays a requirement; only the bare *name* `bash` is the problem, since `cmd.exe` resolves it through PATH. Each hook now also carries Codex's per-handler `commandWindows`, which **replaces** `command` on Windows (`command_windows.unwrap_or(command)` in `codex-rs/hooks/src/engine/discovery.rs`). It calls a new `cmd.exe` shim, `plugins/worktrunk/hooks/wt.cmd`, that locates `bash.exe` by path the way [`find_git_bash`](https://github.com/max-sixty/worktrunk/blob/2405b8b434cfa9d7a604405d3ace966ada60ff8a/src/shell_exec.rs#L481-L518) does in `src/shell_exec.rs` — derived from `git.exe`'s install directory, then the system-wide and per-user install defaults — and then runs the same `hooks/wt.sh` that Claude, Gemini, and Unix Codex already go through. Worktrunk's own binary resolution stays in that one script instead of being spelled a second time in cmd. Two details the shim inherits from the Rust resolver: `Git\bin\bash.exe` before `Git\usr\bin\bash.exe`, because the former is the wrapper that sets up the MSYS environment for a caller outside Git Bash (which is what puts `uname` within reach of `wt.sh`); and a PATH-scoped lookup, because an unscoped `where` searches the current directory first — for a hook that's the user's project, so a `git.exe` committed to a repo would otherwise choose the bash every event runs. The lookup is spelled `"%SystemRoot%\System32\where.exe" "$PATH:git.exe"` on both counts: the `$PATH:` prefix scopes what is searched, and the absolute path to `where.exe` closes the same surface one level up — `where` is `System32\where.exe` rather than a cmd built-in, so cmd resolves that bare name from the current directory too. `wt.sh` now clears `WT` before its branches. On Windows all of them can be skipped (neither `git-wt.exe` nor `wt` on PATH), and a hook is handed the caller's whole environment, so an inherited `WT` was what the final `command -v "$WT"` check accepted and ran. The Windows commands brace the plugin root as `${PLUGIN_ROOT}` because Codex substitutes only that form textually, before the shell runs; the unbraced `$PLUGIN_ROOT` the Unix commands use survives to `/bin/sh`, and `cmd.exe` would pass it through literally. 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. `.gitattributes` pins `plugins/worktrunk/hooks/*.cmd` to a CRLF checkout, since cmd.exe resolves a `goto` label by seeking through the file and can fail that search on an LF-only batch file. Two doc changes ride along. `plugins/worktrunk/CLAUDE.md` records why every hook carries `commandWindows`, the shim's resolution order and the two lookup-scoping decisions, and why `SessionEnd` keeps `timeout: 3` (Codex's ceiling for that event, so the longer Windows chain has no more budget to ask for). And because the CHANGELOG entry opens an `## Unreleased` section, `.claude/skills/release/SKILL.md` step 9 now says to rename that heading at release time rather than insert a new one above it — otherwise the release ships a stale `## Unreleased`. ## Testing Six tests, all new: - `test_codex_hooks_carry_windows_commands` (all platforms) — pins that every Codex command hook has a `commandWindows`, that it names neither `bash` nor bare `wt`, that it calls the shim, and that the two `PLUGIN_ROOT` spellings 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 real `commandWindows` the way Codex spawns it, reproducing both steps: the `${PLUGIN_ROOT}` substitution and `cmd.exe /C "<command>"` with the command line as a single quoted raw argument. It asserts `UserPromptSubmit` stores 🤖, `Stop` replaces it with 💬, `SessionEnd` clears it, and that a hook which cannot find worktrunk still exits 0. PATH is pinned to the shape a default Git for Windows install produces — `Git\cmd` and nothing else from the install — so the run covers cmd.exe's quote handling, the shim's search for bash, `wt.sh` running under the bash it picks, and the emoji surviving both hops. - `test_wt_sh_ignores_an_inherited_wt` (Windows leg) — points `WT` at a real worktrunk, spelled the way bash can run it, on a PATH where `wt.sh` finds none itself, and pins that the inherited value is not what runs. - `test_shim_ignores_a_git_in_the_current_directory` (Windows leg) — plants an unrunnable `git.exe` in the hook's current directory plus the `bash.exe` the shim would derive from it, puts a real Git and worktrunk on the pinned PATH, and asserts the shim prints a version line — which it could not do had it taken the decoy. - `test_shim_ignores_a_where_in_the_current_directory` (Windows leg) — the same question one level up: a `where.bat` in the hook's current directory names a decoy Git install whose `bin\bash.exe` exists, so an unscoped `where` would reach the derive branch and leave `BASH` set and unrunnable rather than falling through to a real install. A separate probe pins the premise — that the planted `where` really does shadow `System32\where.exe` — so the test cannot go green because the decoy was never consulted. - `test_shim_derives_bash_from_the_git_on_path` (Windows leg) — points `ProgramFiles` and `LOCALAPPDATA` at an empty directory, which leaves the derive branch as the only route to a bash. The two tests above go red only when the shim picks the *wrong* bash; this one goes red when the lookup finds nothing, so it is what observes that `"%SystemRoot%\System32\where.exe" "$PATH:git.exe"` survives the quoting `for /f` wraps it in. <details><summary>What this does not verify</summary> Nothing here drives a real Codex session on Windows, so the end-to-end claim — that Codex selects `commandWindows` and that the banner stops — rests on reading `codex-rs/hooks/src/engine/{discovery,command_runner}.rs` rather than on observation. What CI does exercise is the command string itself, executed the way that source says Codex executes it. The shim's two standard-install fallbacks (`%ProgramFiles%\Git`, `%LOCALAPPDATA%\Programs\Git`) are unexercised by CI. Every test that reaches a bash takes the derive branch above them, and the one test that touches the fallbacks empties them to force that branch rather than to exercise them; they share the whole tail with it. Two adjacent things are deliberately left alone, as separate concerns: the Gemini hooks at the repo-root `hooks/hooks.json` use the same bare `bash` (Gemini's Windows hook execution isn't established here), and Claude's `hooks/hooks.json` is unaffected because Claude Code runs hook commands through Git Bash on Windows. </details> --- Closes #4007 — automated triage --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com>
60 lines
2.8 KiB
Batchfile
60 lines
2.8 KiB
Batchfile
@echo off
|
|
rem Windows counterpart of the `bash "$PLUGIN_ROOT/hooks/wt.sh"` that the Unix
|
|
rem hook commands lead with: it finds Git Bash by path, then runs wt.sh through
|
|
rem it, so every integration resolves the worktrunk binary through that one
|
|
rem script.
|
|
rem
|
|
rem Codex runs hook commands through `cmd.exe /C` on Windows, where a bare
|
|
rem `bash` resolves through PATH to System32\bash.exe -- the WSL launcher, not
|
|
rem Git Bash -- and in a sandboxed session refuses to start at all (#4007). Only
|
|
rem the bare name is the problem, so this resolves bash.exe the way find_git_bash
|
|
rem does in src/shell_exec.rs and changes nothing else.
|
|
rem Usage: wt.cmd [args...]
|
|
rem
|
|
rem Every branch here is a bare `goto` or `call`: `if <cond> <cmd1> & <cmd2>`
|
|
rem runs cmd2 unconditionally, and `if <cond> <cmd1> || <cmd2>` tests the `if`
|
|
rem rather than cmd1, so neither connector can carry the control flow.
|
|
setlocal EnableExtensions
|
|
rem Clear the local: `setlocal` inherits the caller's environment, and Codex
|
|
rem hands each hook the session env snapshot, so an inherited BASH would name
|
|
rem what every event runs.
|
|
set "BASH="
|
|
|
|
rem git.exe installs at Git\cmd\git.exe or Git\bin\git.exe, and bash.exe at
|
|
rem Git\bin\bash.exe or Git\usr\bin\bash.exe. The lookup uses `where`'s `$var:`
|
|
rem prefix, which searches the directories named in that variable and nowhere
|
|
rem else: a bare `where git.exe` searches the current directory first, as cmd's
|
|
rem own bare-name lookup does, and a hook's current directory is the user's
|
|
rem project -- so a `git.exe` committed to a repo would otherwise choose the
|
|
rem bash every event runs. `where` itself is spelled absolutely for the same
|
|
rem reason: cmd resolves that name from the current directory too.
|
|
for /f "delims=" %%I in ('"%SystemRoot%\System32\where.exe" "$PATH:git.exe" 2^>nul') do if not defined BASH call :derive "%%~dpI"
|
|
|
|
rem A git.exe PATH doesn't name, or names through a shim outside its install:
|
|
rem the system-wide default, then the per-user one an install without admin
|
|
rem rights writes (#1259).
|
|
if not defined BASH call :accept "%ProgramFiles%\Git\bin\bash.exe"
|
|
if not defined BASH call :accept "%LOCALAPPDATA%\Programs\Git\bin\bash.exe"
|
|
|
|
if not defined BASH goto :missing
|
|
|
|
"%BASH%" "%~dp0wt.sh" %*
|
|
exit /b %ERRORLEVEL%
|
|
|
|
rem %1 is a Git install's cmd\ or bin\ directory, with the trailing separator
|
|
rem `%~dpI` leaves on. Git\bin\bash.exe first, as find_git_bash does: it is the
|
|
rem wrapper that sets up the MSYS environment for a caller outside Git Bash,
|
|
rem which is what puts `uname` and friends within reach of wt.sh.
|
|
:derive
|
|
call :accept "%~1..\bin\bash.exe"
|
|
if not defined BASH call :accept "%~1..\usr\bin\bash.exe"
|
|
goto :eof
|
|
|
|
:accept
|
|
if exist "%~1" set "BASH=%~f1"
|
|
goto :eof
|
|
|
|
:missing
|
|
echo worktrunk: Git for Windows is required but bash.exe was not found. Install from https://git-scm.com/download/win 1>&2
|
|
exit /b 1
|