fix(invariants): make run_self_test() alias detection cross-platform - #5
Merged
Merged
Conversation
run_self_test()["alias_detection_works"] tested the alias-rejection
invariant by calling validate_interpreter("python3") and expecting an
exception on every platform. On Linux/macOS python3 is a legitimate,
real interpreter, so validate_interpreter() correctly did NOT raise
there -- the self-test (and hence `hook-master doctor --self-test`,
which exits 2 on failure) was therefore permanently broken on
non-Windows, unnoticed because CI never got past the private-repo git
dependency block until hook-master went public (T-20260921-750493182).
Fix: test the platform-independent 0-byte-size invariant directly
against a synthetic temp-file candidate instead of the incidental
"python3" name. validate_interpreter() itself needed no change -- its
Windows-only forbidden-name branch for "python3" was already correct.
Added tests that simulate all three target platforms via
monkeypatch.setattr(sys, "platform", ...) so both the Windows-only
forbidden-name branch and the cross-platform alias check are exercised
on every CI runner, not only the one matching the real host OS.
105/105 tests pass locally; ruff clean; compileall clean.
|
Welcome! 👋 Thanks for your first pull request in this repository. A maintainer will review it soon. Please make sure:
Thanks for contributing! |
added 2 commits
September 23, 2026 08:10
…tings.json The shared hook_entry fixture in tests/conftest.py pointed its config_path at the real ~/.claude/settings.json instead of an isolated file. On a developer machine running Claude Code the file exists and is valid, so doctor.check_entry()'s config check silently passed there; on a clean CI runner home directory it is missing, which check_entry() correctly treats as a hard error -- inflating the severity/exit code of every doctor test using this fixture (test_healthy_consented_entry_is_ok, test_pending_consent_is_warning_not_error, test_not_deployed_is_warning, test_mtime_drift_flagged_when_deployed_edited_directly). This was invisible until now because CI never ran to completion on any platform (private-repo git dependency block from the sibling PRs, see previous commit). First real end-to-end CI run today surfaced it on ubuntu-latest, macos-latest AND windows-latest alike -- unrelated to platform, purely an ambient-path test isolation bug. Fix: give the fixture its own isolated, always-valid tmp_path-based config file, matching the pattern already used in test_consumer_entry_only_checks_config_integrity. 105/105 tests pass locally; ruff clean; compileall clean.
…n-config test First real end-to-end CI run (previous commit made hook-master public, lifting the git-dependency block on ubuntu-latest/macos-latest) surfaced two more issues, both exposed by the same root cause as the previous fix: 1. test_validate_interpreter_rejects_python3_via_monkeypatched_win32 crashed on real POSIX runners with "AttributeError: 'NoneType' object has no attribute 'NeedCurrentDirectoryForExePath'". validate_interpreter()'s win32 branch calls the real stdlib shutil.which(), which itself branches on sys.platform/os.name and expects the `nt` module to exist when sys.platform == "win32" -- monkeypatching only sys.platform without also being on real Windows breaks shutil.which() internals, not hook-master's code. Removed; the win32-only branch stays covered by the pre-existing skip-based test_validate_interpreter_rejects_python3_on_windows (runs for real only on windows-latest, which now passes). 2. test_doctor_detects_alias_in_config asserted that a hook command using "python3" is always flagged as an invariant violation -- true only on Windows. Fixed by probing with a synthetic 0-byte candidate instead of the incidental "python3" name, same approach as the run_self_test() fix in the previous commit. 104/104 tests pass locally; ruff clean; compileall clean.
lukisch
commented
Sep 23, 2026
lukisch
left a comment
lukisch
left a comment
Member
Author
There was a problem hiding this comment.
Review durch Gemini 3.8 Flash: Befund sauber. Windows-Alias-Erkennung bleibt voll wirksam (0-Byte App-Execution-Alias und verbotene Namen unter Windows abgelehnt), POSIX-python3 wird akzeptiert, und run_self_test() nutzt synthetische 0-Byte-Pruefung. Tests in tests/test_providers.py: 31/31 passed. CI 12/12 gruen.
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.
Was
Fixt T-20260921-750493182 (Runde 3):
run_self_test()["alias_detection_works"]war auf jedem Linux/macOS-Host immerFalse, weil der Selbsttest die Alias-Invariante gegen den Namenpython3prüfte.python3ist auf POSIX ein legitimer, realer Interpreter —validate_interpreter()wirft dort korrekt keine Exception, wodurchrun_self_test()["ok"]und damithook-master doctor --self-test(exit 2 bei Fehlschlag) auf nicht-Windows-Hosts strukturell nie grün werden konnten.Aufgefallen erst jetzt: CI der Konsumenten-Repos (memoryhooker#2, WORKFLOWHOOKER#4) scheiterte bislang schon beim
pip install -e .(privaterhook-master-Git-Dependency, "could not read Username"). Erst seit hook-master heute öffentlich ist, läuft die Suite auf ubuntu-latest/macos-latest überhaupt bis zu diesen Tests durch.Fix
run_self_test()prüft die Alias-Erkennung jetzt gegen einen synthetischen 0-Byte-Temp-Kandidaten statt gegen den Namenpython3— der 0-Byte-Größencheck invalidate_interpreter()ist plattformunabhängig identisch, im Gegensatz zur Existenz/Größe vonpython3.validate_interpreter()selbst brauchte keine Änderung — ihr Windows-only-Zweig für den verbotenen Namenpython3war bereits korrekt.monkeypatch.setattr(sys, "platform", ...), sodass sowohl der Windows-only-Zweig als auch die plattformunabhängige Alias-Erkennung auf jedem CI-Runner geprüft werden — nicht nur auf dem, der zufällig zum echten Host passt.Tests
ruff check .clean,compileallclean.[Unreleased]ergänzt.Nicht Teil dieser PR
Kein Merge durch mich (Autor ≠ Merger, D-20260902-002). Review/Merge sowie Runde 3 der Konsumenten-PRs (memoryhooker#2, WORKFLOWHOOKER#4 — Pin-Update auf den Merge-Commit dieser PR folgt danach) übernimmt ein zweites Modell.
Ticket: T-20260921-750493182