fix(hooks): a driveless rooted path is not cwd-relative on Windows (#2795) - #2796
fix(hooks): a driveless rooted path is not cwd-relative on Windows (#2795)#2796abhay-codes07 wants to merge 1 commit into
Conversation
…raphify-Labs#1840) The read guard's out-of-project check short-circuited on `not Path(v).is_absolute()` with the comment "relative -> anchored at cwd == in project". That premise does not hold on Windows for a rooted path carrying no drive, which is the form POSIX-shaped hosts, WSL and Git Bash send: Path("/somewhere/else/x.py").is_absolute() -> False Path("/somewhere/else/x.py").resolve() -> C:\somewhere\else\x.py Windows anchors it at the current DRIVE root, not at cwd, so it lands outside the project unless the project sits at the drive root. Reading it as cwd-relative made the guard declare it in-project and emit the read nudge -- and in strict mode the once-per-session deny -- for files the graph never indexed. `C:x.py` is the mirror case: drive-relative, anchored at that drive's current directory rather than ours. The question the guard actually asks is "is this resolved against cwd?", whose answer is "no root and no drive", not "not absolute". These remain the host's own rules -- the path is about to be resolved against this filesystem, so paths.is_absolute_any_platform, which is for stored portable paths, is deliberately not used. On POSIX `root` is set exactly when a path is absolute and `drive` is always empty, so behaviour there is unchanged, and a test pins that equivalence. Fixes tests/test_hook_strict.py::test_out_of_project_read_silenced, which has been failing on Windows.
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Formal verification. 1 change(s) tested, no difference found (not proven).
Graphify review — findings
This PR changes how the read hook's out-of-project guard decides whether a file path is anchored at the current working directory. It introduces a new _is_cwd_relative helper that classifies a path based on whether it lacks both a root and a drive (using platform-specific PureWindowsPath/PurePosixPath), replacing the previous Path(v).is_absolute() check in _run_hook_guard. The stated intent is to correctly handle rooted-but-driveless paths (e.g. /tmp/x.py) and drive-relative paths on Windows. It also adds a new test file (tests/test_hook_out_of_project_paths.py) covering the classification helper across forced os.name values, POSIX-vs-Windows rules, and end-to-end guard behavior (with some Windows-only cases gated via skipif). Surface area is limited to graphify/cli.py and the new test module.
No blocking issues surfaced.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 237 functions depend on the 66 functions this change touches.
Health — this change adds coupling hotspots:
- new:
dispatch_command()— 2 callers, 117 callees - new:
_stale_graph_sources()— 7 callers, 6 callees - new:
_run_hook_guard()— 4 callers, 7 callees - new:
test_poisoned_manifest_is_healed()— 0 callers, 6 callees
Verification — 237 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 194 function(s) in the blast radius were not formally verified this run
Formal verification
No difference found (not proven): No behavior difference found in \_run\_hook\_guard (not a proof).
The verifier ran both versions of \_run\_hook\_guard on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
· 4 more finding(s) on lines outside this diff (see the check run).
Fixes #2795.
The bug
_run_hook_guard's out-of-project check short-circuits onnot Path(v).is_absolute():The comment is the bug. On Windows a rooted path carrying no drive is not absolute, but it is not cwd-anchored either — Windows anchors it at the current drive root:
So the guard declares it in-project and never reaches the
relative_to(root)check two lines down./-rooted paths are exactly what POSIX-shaped hosts, WSL and Git Bash send, so on those setups theMANDATORYnudge fires for files the graph never indexed — and in strict mode the session's single deny can be spent on one.C:x.pyis the mirror case: drive-relative, anchored at that drive's current directory rather than ours.Change
The question the guard asks is "is this resolved against cwd?", whose answer is no root and no drive, not "not absolute". That is now a named helper,
_is_cwd_relative.I deliberately did not reach for
paths.is_absolute_any_platformhere. Its docstring already rules that out — "Code resolving a path against the real local filesystem (cli,detect,hooks) must keep usingPath.is_absolute()" — and that guidance is right: this path is about to be resolved against this filesystem, so the host's rules are the correct ones. The defect was that "absolute" is the wrong local predicate for "cwd-anchored", not that the path should be judged by both platforms' rules. Flagging it since the two look similar at a glance.POSIX is untouched
On POSIX
rootis set exactly when a path is absolute, anddriveis always empty, so_is_cwd_relativereduces tonot is_absolute(). That is not just asserted in prose —test_matches_is_absolute_on_every_posix_inputpins the equivalence directly, so the fix cannot start meaning something new on Linux later.Tests
tests/test_hook_out_of_project_paths.py(30 tests, 4 of them Windows-gated).The classification tests drive
_is_cwd_relativewithos.nameforced, so Windows semantics are exercised on Linux CI too —PureWindowsPathworks on any host. That matters here: this bug survived because the test job runsubuntu-latestonly, and on POSIX/somewhere/else/x.pyis genuinely absolute, so the guard was never wrong about it.Reverting
cli.pyand keeping the tests:test_hook_strict.py::test_out_of_project_read_silenced;C:x.py,C:/proj/a.py,C:\proj\a.py,\somewhere\else\x.py,\\server\share\a.py).So the suite keeps teeth on your CI, but I want to be straight that the two headline cases —
/somewhere/else/x.pyand/tmp/scratch.py— can only fail on a Windows host, since on POSIX the old code got them right by accident of flavour. The end-to-end tests through_run_hook_guardneed a real WindowsPathflavour and are gated withskipif; the in-project and out-of-project controls beside them run everywhere.Validation
Windows 11, Python 3.12, branched off
4fca621(0.9.44).20 failed, 4469 passed→20 failed, 4499 passed.test_out_of_project_read_silencedmoves from fail to pass; no other failure changes state. The remaining 20 are the pre-existing Windows failures (symlink privileges, FIFO/socket fixtures, and similar) and are unrelated.test_incremental_mtime_collision.py::test_same_size_rewrite_in_one_tick_is_requeued, flaked once under load in the post-change run and passes in isolation. It is timing-sensitive by design (same-tick mtime collision) and untouched by this change — calling it out rather than quietly folding it into the count.Note on #2317
That PR rewrites the same hunk (
.resolve()→_resolve_path()in this exact loop) but leaves theis_absolute()line as context, so the bug survives it. Expect a small textual conflict and no semantic one. Happy to rebase on top of it if it lands first — no preference on ordering.