Fix: normalize Windows path spellings before plan-file containment check#228
Fix: normalize Windows path spellings before plan-file containment check#228houhou8 wants to merge 1 commit into
Conversation
On MSYS/MinGW/Cygwin one directory has two textual spellings, and the two sides of the plan-file containment check came from sources that disagree: git rev-parse --show-toplevel -> C:/Users/name/project CLAUDE_PROJECT_DIR (from Claude Code) -> C:\Users\name\project bash cd .. && pwd -> /c/Users/name/project setup-rlcr-loop.sh compares these as strings, so the check never matched and every Windows Git Bash user hit a hard failure at loop start: Error: Plan file must be within project directory Canonicalization did not help: MSYS realpath preserves whichever spelling it is handed rather than folding the two together. Add to_posix_path() in hooks/lib/project-root.sh, which folds drive-letter and backslash spellings to the POSIX form via cygpath, with a pure-bash fallback for Windows shells lacking it. Apply it to CLAUDE_PROJECT_DIR, to the git toplevel output, and to the resolved plan path, so every path crossing a comparison speaks one dialect. Any drive letter is supported and paths containing spaces are preserved. The helper is a no-op off Windows, guarded by a shell check, so a POSIX filename legitimately containing a backslash is not rewritten. Also gate the python3 fallback in canonicalize_path/canonicalize_path_prefix to non-Windows shells. A native Windows interpreter reads /c/x as drive-relative and answers C:\c\x -- a different directory -- so on Windows it silently produced a wrong path. MSYS always ships realpath, so nothing is lost. Add tests/test-path-normalization.sh covering both spellings of one directory, arbitrary drive letters, spaces, POSIX no-op behavior, and that a plan genuinely outside the project (including a prefix-sibling directory) is still rejected.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 15fa348aff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| # exports CLAUDE_PROJECT_DIR as `F:\Project\Public`, and git prints | ||
| # `F:/Project/Public`. Normalize before canonicalizing so the result is | ||
| # comparable with paths built from `pwd`. | ||
| root="$(to_posix_path "$root")" |
There was a problem hiding this comment.
Normalize remaining realpath comparisons too
On MSYS/MinGW this switches PROJECT_ROOT to /c/..., but the methodology-analysis read guard still compares raw realpath "$FILE_PATH" against raw realpath "$PROJECT_ROOT"/$PROJECT_ROOT in hooks/loop-read-validator.sh lines 91 and 154-158. When Claude supplies a drive-letter path such as C:/repo/src/x during methodology analysis, MSYS realpath preserves that spelling while PROJECT_ROOT is now /c/repo, so neither prefix check matches and project files are allowed to be read despite that phase's project-read block. Please run those remaining realpath outputs through the same normalization before comparing.
Useful? React with 👍 / 👎.
Problem
On MSYS/MinGW/Cygwin, the same directory can have multiple textual representations, and the two sides of the plan-file containment check may come from sources that use different forms:
setup-rlcr-loop.shcompares these paths as strings. When the project root and plan path use different representations, affected Windows Git Bash users hit a hard failure at loop start even though the plan file is inside the project:The existing canonicalization does not solve this mismatch because MSYS
realpathpreserves whichever path representation it is given rather than folding drive-letter and MSYS forms together.Fix
Add
to_posix_path()tohooks/lib/project-root.sh.The helper converts drive-letter and backslash paths to the MSYS/Cygwin POSIX form using
cygpath -u, with a pure-Bash fallback for Windows shells wherecygpathis unavailable.Apply it consistently to:
CLAUDE_PROJECT_DIRgit rev-parse --show-toplevelThis ensures that every path involved in the containment comparison uses the same representation. Arbitrary drive letters are supported, and paths containing spaces are preserved.
The helper is a no-op outside Windows environments, so a valid POSIX filename containing a backslash is not rewritten.
Also restrict the
python3fallback incanonicalize_path()andcanonicalize_path_prefix()to non-Windows shells. A native Windows Python interpreter can interpret an MSYS path such as/c/xas a different drive-relative path:This silently produces an incorrect path. MSYS environments provide
realpath, so disabling this fallback on Windows does not remove required functionality.Testing
Add
tests/test-path-normalization.shcovering:project-evilEnd-to-end verification confirms that the fixed branch starts the RLCR loop successfully without manually exporting
CLAUDE_PROJECT_DIR, while the currentmainbranch fails with the incorrect containment error.