fix(fix-plan): preserve Windows backslashes in hook_integrity_check paths - #376
Merged
Merged
Conversation
…aths
shlex.split() in posix mode (the default) treats backslash as an escape
character, so any Windows path passed through resolve_script_operand lost
every backslash (C:\Users\me\hook.sh -> C:UsersMehook.sh), making
os.path.exists() fail and the script report a real, existing hook as
MISSING.
Use posix=False on win32, where shlex keeps backslashes literal; the
existing strip('"')/strip("'") calls already handle the quoting
difference that mode introduces.
Discovered as a pre-existing, unrelated CI-parity failure blocking an
unrelated PR's push -- test_installed_schema_is_audited was silently
broken by this bug already (its own fixture uses a real Windows tmp_path)
but nothing had isolated it to this function until now. Added 2 targeted
regression tests pinning the win32 branch explicitly via monkeypatch so
this reproduces deterministically on any CI platform, not just Windows.
daegunjhy
marked this pull request as ready for review
August 26, 2026 08:18
DrumRobot
added a commit
that referenced
this pull request
Aug 27, 2026
… gh pr merge (#381) gh pr checks right before merging only confirms CI freshness. It says nothing about whether the PR itself was already merged/closed in the gap between an AskUserQuestion approval and the actual merge call -- the user (or another process) can act on the same PR concurrently. Discovered this session: merged PR #376 in exactly this gap after the user had already merged it themselves; gh pr merge was rejected rather than double-merging, but the near-miss showed the existing rule only guards CI staleness, not merge-state staleness. Co-authored-by: DrumRobot <drumrobot43@gmail.com>
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
2 participants
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
shlex.split()in posix mode (the default) treats backslash as an escape character, so any Windows path passed throughresolve_script_operandlost every backslash (C:\Users\me\hook.sh->C:UsersMehook.sh), makingos.path.exists()fail and the script report a real, existing hook as MISSING.Fix: use
posix=Falseon win32, where shlex keeps backslashes literal; the existingstrip('"')/strip("'")calls already handle the quoting difference that mode introduces.Discovered as a pre-existing, unrelated CI-parity failure blocking a separate PR's push —
test_installed_schema_is_auditedwas silently broken by this bug already (its own fixture uses a real Windowstmp_path), but nothing had isolated it toresolve_script_operandspecifically until now.Test plan
test_resolve_preserves_windows_backslashes— pins the win32 branch explicitly viamonkeypatch.setattr(mod.sys, "platform", "win32"), deterministic on any CI platformtest_resolve_posix_path_unaffected_by_win32_branch— confirms ordinary POSIX-style paths and interpreter/flag skipping still resolve correctly underposix=False🤖 Generated with Claude Code