fix(process_host): forward-slash Windows hook paths for Git Bash (#773) - #827
Conversation
|
Warning Review limit reachedNext included review available in 4 minutes. View limit detailsLimit details: You’ve used all 2 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (9)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
pbean
changed the title
Claude Code runs shell-form hook commands under Git Bash on Windows, where an unquoted backslash is an escape: the registered relay C:\Users\me\.local\bin\bmad-loop.exe ran as C:Usersme.localbinbmad-loop.exe, no events were written, and every session stalled to session_timeout_min. WindowsProcessHost.shell_quote now normalizes separators to forward slashes before list2cmdline, so a path with spaces is still double-quoted. Every caller passes a path. Pre-#773 backslash registrations are still recognized as managed, so init and worktree provisioning replace them; the Codex trust diagnostic reads them as untrusted until re-init. Paths with spaces remain unsupported on the PowerShell fallback.
…nit's (#773) hooks.relay-stale compared Path objects. On Windows, Path equality and str(Path) both normalize separators, so a pre-#773 backslash registration, which Git Bash mangles and which stalls every in-place session, was never flagged, although init's merge_hooks (a raw-string comparison) rewrites it. relay_executable_text now returns the executable exactly as registered, and registered_relay_paths pairs each Path with that spelling. validate keeps its presence checks on the Path but compares the registered text with the one init would write now, so the warning means "re-running init changes something". It stays advisory (rc 0). The existing another-installation test now reads the expected spelling with relay_executable_text. It is unchanged on POSIX, and on Windows str(Path) would respell init's forward slashes.
88ea69d to
c681fc7
Compare
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Closes #773.
Summary
C:\Users\me\.local\bin\bmad-loop.exeran asC:Usersme.localbinbmad-loop.exe, no events were written, and every session stalled untilsession_timeout_min.WindowsProcessHost.shell_quotenow converts separators to forward slashes beforelist2cmdline, so a path with spaces is still double-quoted. Registrations written before [BUG] Native Windows: hook relay command is shell-mangled, so no session event ever fires #773 with backslashes are still recognized as managed, soinitand worktree provisioning replace them. The Codex trust diagnostic reads them as untrusted until the nextinit.validateflags a stale relay spelling.hooks.relay-stalecomparedPathobjects. On Windows,Pathequality andstr(Path)both normalize separators, so a pre-[BUG] Native Windows: hook relay command is shell-mangled, so no session event ever fires #773 backslash registration was never flagged, even thoughinit'smerge_hookscompares raw strings and does rewrite it. The newrelay_executable_textreturns the executable exactly as registered, andregistered_relay_pathspairs eachPathwith that text.validatestill checks presence with thePath, but compares the registered text against the textinitwould write now. So the warning means "re-runninginitwould change something". It is a warning only (rc 0), like the existing stale-installation warning.Notes for reviewers
test_validate_warns_on_backslash_windows_relay_registrationare win32-only and are skipped on Linux by design. No Windows host or psmux live gate was available while writing this. Please wait for the Windows CI leg to pass before merging.rcstays 0 for an advisory.test_validate_warns_when_registered_relay_uses_another_installationnow reads its expected value withrelay_executable_text. On POSIX nothing changes. On Windows,str(Path)would have convertedinit's forward slashes back to backslashes.test_validate_warns_when_relay_spelling_differs_but_path_is_equalwas ablation-checked: it fails if the check goes back to comparingPathobjects.Stack
Part of a stack; merge bottom-up. This is layer 2 of 8, on top of #826.
Stack order (bottom → top): #826 → #827 → #828 → #829 → #830 → #831 → #832 → #833.