From a8cf205745beb48a8533b3899424024b50b53e52 Mon Sep 17 00:00:00 2001 From: DrumRobot Date: Tue, 25 Aug 2026 14:55:21 +0900 Subject: [PATCH] fix(fix-plan): preserve Windows backslashes in hook_integrity_check paths 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. --- .../fix-plan/scripts/hook_integrity_check.py | 7 ++++++- tests/test_hook_integrity_check.py | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/skills/fix-plan/scripts/hook_integrity_check.py b/skills/fix-plan/scripts/hook_integrity_check.py index 5d73777..cf556e4 100644 --- a/skills/fix-plan/scripts/hook_integrity_check.py +++ b/skills/fix-plan/scripts/hook_integrity_check.py @@ -64,7 +64,12 @@ def resolve_script_operand(command): resolves to `/path/hook.sh`, not to the interpreter. """ try: - tokens = shlex.split(command) + # posix=True (the default) treats backslash as an escape character, + # so a Windows path like C:\Users\... loses every backslash + # (\U -> U, \A -> A, ...) and the resolved path silently stops + # existing. posix=False keeps backslashes literal; the manual + # strip('"')/strip("'") calls below still handle quoting. + tokens = shlex.split(command, posix=(sys.platform != "win32")) except ValueError: tokens = command.split() for tok in tokens: diff --git a/tests/test_hook_integrity_check.py b/tests/test_hook_integrity_check.py index 39eb241..c821ad6 100644 --- a/tests/test_hook_integrity_check.py +++ b/tests/test_hook_integrity_check.py @@ -88,6 +88,24 @@ def test_resolve_plain_path_unchanged(): assert mod.resolve_script_operand('"/p/with space/hook.sh"') == "/p/with space/hook.sh" +def test_resolve_preserves_windows_backslashes(monkeypatch): + # posix=True shlex.split treats backslash as an escape character, so a + # Windows path silently loses every backslash (\U -> U, \A -> A, ...) + # and the resolved path stops existing. Guards the fix for that. + # Force the win32 branch explicitly so this test is deterministic + # regardless of the platform actually running it (CI runs on Linux). + monkeypatch.setattr(mod.sys, "platform", "win32") + assert mod.resolve_script_operand(r"python3 C:\Users\me\hook.sh") == r"C:\Users\me\hook.sh" + + +def test_resolve_posix_path_unaffected_by_win32_branch(monkeypatch): + # On win32, posix=False is used -- confirm ordinary POSIX paths and + # interpreter/flag skipping still resolve correctly under that mode. + monkeypatch.setattr(mod.sys, "platform", "win32") + assert mod.resolve_script_operand("python3 /p/hook.sh") == "/p/hook.sh" + assert mod.resolve_script_operand('"/p/with space/hook.sh"') == "/p/with space/hook.sh" + + # --- check_hook_integrity: end-to-end on the installed schema --- def test_installed_schema_is_audited(tmp_path, monkeypatch):