Skip to content

fix: windows contributor verification#332

Open
syahmiharith wants to merge 1 commit intoJuliusBrussee:mainfrom
syahmiharith:codex/windows-contributor-verification
Open

fix: windows contributor verification#332
syahmiharith wants to merge 1 commit intoJuliusBrussee:mainfrom
syahmiharith:codex/windows-contributor-verification

Conversation

@syahmiharith
Copy link
Copy Markdown

Summary

  • Pin shell scripts to LF line endings so Windows checkouts with core.autocrlf=true do not break Bash syntax checks.
  • Document Windows contributor checkout expectations, Git Bash preference, and CAVEMAN_BASH override.
  • Make compress module output UTF-8-safe when imported directly, and update hook verification tests to prefer Git Bash on Windows.

Why

Windows contributors can currently hit CRLF shell scripts, WSL bash.exe path/tool mismatches, and CP932 Unicode output failures while running local verification. These issues obscure otherwise valid changes and make the starter contributor path harder than necessary.

Validation

  • python tests/verify_repo.py
  • python -m unittest tests.test_compress_safety tests.test_validate_inline tests.test_hooks
  • node tests/test_caveman_init.js
  • node tests/test_mcp_shrink.js
  • node tests/test_symlink_flag.js
  • node tests/test_caveman_stats.js

@syahmiharith syahmiharith changed the title [codex] fix windows contributor verification fix: windows contributor verification May 4, 2026
@syahmiharith syahmiharith marked this pull request as ready for review May 4, 2026 18:48
@Qodo-Free-For-OSS
Copy link
Copy Markdown

Hi, tests/test_hooks.py now prefers Git Bash but still sets HOME/USERPROFILE to a Windows path with backslashes, which Bash treats as escape characters; hook scripts will compute CLAUDE_DIR/HOOKS_DIR from a mangled HOME and write files to the wrong location, failing the tests on Windows.

Severity: action required | Category: correctness

How to fix: Normalize HOME for Git Bash

Agent prompt to fix - you can give this to your LLM of choice:

Issue description

tests/test_hooks.py runs hook scripts using Git Bash on Windows, but it passes HOME as a Windows backslash path. Bash treats backslashes as escapes, so the scripts compute the wrong ~/.claude path and tests fail.

Issue Context

tests/verify_repo.py already has shell_path() that converts \ to / on Windows for bash compatibility.

Fix Focus Areas

  • tests/test_hooks.py[35-49]
    • When os.name == "nt", set HOME and USERPROFILE to forward-slash form (e.g. str(home).replace('\\','/')) or reuse a small helper like shell_path().
    • Optionally also set CLAUDE_CONFIG_DIR to the same normalized path to avoid ambiguity.

We noticed a couple of other issues in this PR as well - happy to share if helpful.


Found by Qodo code review. FYI, Qodo is free for open-source.

@Qodo-Free-For-OSS
Copy link
Copy Markdown

Hi, resolve_bash() intends to avoid WSL’s System32 bash on Windows, but still adds shutil.which("bash") as a candidate; on machines with WSL installed and Git Bash absent, verification will select WSL bash and still hit the Windows-path incompatibilities this PR is trying to avoid.

Severity: action required | Category: reliability

How to fix: Exclude System32/WSL bash

Agent prompt to fix - you can give this to your LLM of choice:

Issue description

resolve_bash() is meant to prefer Git Bash on Windows, but it can still select WSL’s C:\Windows\System32\bash.exe via shutil.which('bash'), defeating the stated intent.

Issue Context

Hook verification uses Windows paths normalized for Git Bash; WSL bash commonly fails with these.

Fix Focus Areas

  • tests/verify_repo.py[19-37]
    • When os.name == 'nt', ignore shutil.which('bash') results that point to System32\\bash.exe (or generally to the Windows inbox/WSL shim).
    • Alternatively, positively detect Git for Windows bash (e.g., path containing \\Git\\bin\\bash.exe or \\Git\\usr\\bin\\bash.exe) before accepting.

We noticed a couple of other issues in this PR as well - happy to share if helpful.


Found by Qodo code review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants