test: lint hooks.json for cross-shell dispatch safety#83
Open
fdaviddpt wants to merge 8 commits into
Open
Conversation
added 4 commits
May 26, 2026 18:07
Guard rails for past hooks/hooks.json regressions and the Windows PowerShell ParserError reported in #82. - Structural: JSON shape, referenced scripts exist. - Static lint: ${CLAUDE_PLUGIN_ROOT} double-quoted (4d50166), no stderr redirects in commands (d18e02c), no bare ${VAR} outside `bash -c '...'` wrappers (#82). - Live parse: bash -n + pwsh dry-parse with stubbed env. GitHub-hosted runners (ubuntu/macos/windows) ship pwsh, so the PowerShell test fires across the existing matrix without workflow changes. Refs #82 Co-Authored-By: Max <noreply>
Python's list2cmdline on Windows mangles embedded double quotes when the command is passed via `-c "$cmd"`. Bash on Git Bash then saw a truncated command and returned exit 1 — false positive, not a real syntax error. Write each hook command to a temp .sh / .ps1 and dry-parse the file instead. Same coverage, no encoding hazards. Co-Authored-By: Max <noreply>
Temp-file approach failed on Windows runners — Git Bash mangles C:\\... style paths from pytest's tmp_path. Piping the command source via stdin removes the filename entirely: no path translation, no list2cmdline quoting, identical behaviour on every OS. Same change applied to the pwsh test path for symmetry. Co-Authored-By: Max <noreply>
`bash` on GitHub windows-latest resolves to C:\Windows\System32\bash.exe (the WSL launcher) which has no distro installed and returns exit 1 with a UTF-16 "Windows Subsystem for Linux has no installed distribution" message — not Git Bash. Bash parsing of hook commands is already covered on ubuntu and macos runners. Windows-side coverage is the pwsh test, which is what claude-code actually uses to dispatch hooks on Windows (#82). Co-Authored-By: Max <noreply>
added 4 commits
May 26, 2026 18:38
…values
Claude Code on Windows may expand the env variable before handing the
command string to pwsh. The raw template `${CLAUDE_PLUGIN_ROOT}` parses
fine, but after substitution an install path with apostrophes,
backticks, dollar signs, spaces, or non-ASCII chars can blow up
PowerShell's parser — possibly the mechanism behind #82.
Covers 10 install-path shapes likely to occur in the wild:
- plain
- spaces (`Program Files`, `Jane Doe`)
- non-ASCII usernames (Émilie, café, Łukasz)
- OneDrive-style apostrophe folders (Jane's OneDrive)
- PowerShell escape character (backtick)
- literal dollar sign
- mixed (Émilie's Files / Café (work) / …)
Skipped locally without pwsh; fires on all 3 GH-hosted runners.
Refs #82
Co-Authored-By: Max <noreply>
Python subprocess text mode on Windows defaults to cp1252, which can't encode chars like Ł — pwsh never even got the input. Set encoding=utf-8 explicitly so stdin/stdout/stderr survive the round trip. Side observation worth chasing: if Claude Code's own Windows dispatch uses default-codepage pipes, install paths with non-Latin1 chars (Łukasz, Việt, …) will break before pwsh sees the command — different failure shape from the ParserError in #82, but a near neighbour. Co-Authored-By: Max <noreply>
Best-effort coverage of install-path shapes that could break PowerShell
parsing or downstream dispatch:
- trailing space
- non-ASCII (Việt combining mark, 中文, مرحبا RTL, 🎉 surrogate pair)
- PS-hostile chars: backtick, $, [], (), {}, ;, #, @, '
- cmd-passthrough flavour: %var%, &
- slash mixing: all-backslash, mixed, 8.3 short name
- UNC paths (POSIX and Windows form)
- combined nightmare shapes
If any of these blow the pwsh parser on the windows runner, we will
have found a candidate cause for #82.
Refs #82
Co-Authored-By: Max <noreply>
Backslashes in CLAUDE_PLUGIN_ROOT values (UNC paths, all-backslash Windows paths) tripped re.sub's escape handling (\U, \s). Pass a lambda so the replacement string is taken verbatim. Co-Authored-By: Max <noreply>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
tests/test_hooks_json.pyto linthooks/hooks.jsonfor cross-shell dispatch safety. Guards against past regressions and the Windows PowerShell ParserError reported in #82.Coverage
scripts/exists on disk.${CLAUDE_PLUGIN_ROOT}must be double-quoted (catches a 4d50166-style regression — paths with spaces).2>/2>>redirects in commands (catches a d18e02c-style regression).${VAR}outsidebash -c '...'wrappers (PowerShell ParserError risk — SessionStart hook fails with ParserError on Windows (PowerShell dispatch) #82).bash -nandpwsh -Command [scriptblock]::Create(...)dry-parse with a stubbedCLAUDE_PLUGIN_ROOT. Skipped cleanly if the shell isn't on PATH.CI
GitHub-hosted runners (
ubuntu-latest,macos-latest,windows-latest) all shippwshpreinstalled, so the PowerShell test fires across the existing matrix without any workflow change.Out of scope
This PR does not fix #82 itself —
hooks.jsonis unchanged. A follow-up MR will wrap the commands so PowerShell sees an opaque body and forward to bash. With these tests in place, that fix can be validated automatically.Test plan
pytest tests/test_hooks_json.py— 6 pass locally (pwsh test skipped, no pwsh on dev box).