[Windows port] 2/8: Make the worktree safety gate cross-platform#20
Open
wmaciel wants to merge 3 commits into
Open
[Windows port] 2/8: Make the worktree safety gate cross-platform#20wmaciel wants to merge 3 commits into
wmaciel wants to merge 3 commits into
Conversation
The forbidden-base set holds only POSIX sensitive roots; the name now states the platform, pairing with the Windows-specific forbidden dirs added in the next commit. Pure rename, no behaviour change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The rm-rf safety gate hard-coded POSIX semantics and fail-closed on
Windows: it treated "absolute" as startswith("/") (rejecting real C:\
worktrees, so dream-cleanup could never remove them), split on "/" to
find ".." (missing backslash-delimited traversal), and only knew the
POSIX forbidden roots.
- Rule 2: os.path.isabs instead of startswith("/") (True for C:\ and
UNC shares, False for a bare leading slash on Windows).
- Rule 3: PurePath(p).parts to detect ".." with either separator.
- Rule 4: split into a filesystem-root check plus a normcase-based match
against the platform-appropriate forbidden set (static POSIX
_UNIX_FORBIDDEN_BASES, or env-derived Windows roots on nt).
Tests: guard the POSIX-literal cases with skipif(nt) (a leading slash is
not absolute on Windows) and add Windows-native cases (drive/UNC roots,
%SystemRoot%, backslash traversal, a real C:\ worktree, relative base).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Shadow artifacts (the top-level index and file discovery) are a
cross-platform, git-committed format that always uses forward slashes,
but on Windows the path operations returned OS-native "\", producing
"src\main.py" instead of "src/main.py".
- shadow-init.py _walk_files: emit Path.as_posix() instead of str().
- dream-reconcile.py rebuild_top_index: emit .as_posix() (adds a pathlib
import) so the recovered source path uses "/".
The sibling relpath in dream-reconcile (feeding only a startswith("_")
skip check, never written to disk) is left as-is. test_shadow_init's
test_walk_files_lists_all_non_excluded asserted os.path.join(...) and is
updated to the canonical "/".
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.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
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.
Part of #7. Closes #9. Phase 2 of 8 of the Windows port.
Why
Phase 1/1b pinned UTF-8 everywhere. This phase fixes the other Windows-specific class of bug: path separators. Two coherent halves, both "make
\-Windows behave like/-POSIX":_worktree_safety.py) fail-closes on Windows: it treated "absolute" as "starts with/", so a realC:\…worktree was rejected anddream-cleanupcould never remove it. That's the core of [Windows port] 2/8: Make the worktree safety gate cross-platform #9.\, where the shadow format requires/.Both keep POSIX accept/reject behaviour unchanged (only Windows behaviour changes), so Linux CI stays green.
What changed (3 commits)
refactor(dream): rename _FORBIDDEN_BASES to _UNIX_FORBIDDEN_BASES— pure rename (5 sites), no behaviour change. The constant only ever held POSIX roots; the name now says so, making room for the Windows equivalent.fix(dream): make _worktree_safety gate cross-platform— the gate's three portability rules:p.startswith("/")->os.path.isabs(p), soC:\…counts as absolute.".."->".." in PurePath(p).parts, which catches..under either separator without false-positives on names like..foo.normcase) compare, plus a filesystem-root guard. POSIX roots stay a static constant; Windows roots come from a function because they're environment-derived —_get_forbidden_bases()selects the platform-appropriate source.fix: record forward-slash paths in shadow index and file walk (Windows)—_walk_filesandrebuild_top_indexnow emit.as_posix(), so shadow paths are POSIX on every OS.Testing
Verified on Windows (Python 3.13):
test_worktree_safety.py— 54 passed / 26 skipped / 0 failed (skips are the POSIX-literal-path cases, platform-guarded — see note below)test_dream_reconcile.py— 187 passed / 0 failedtest_shadow_init.py— 241 passed (the one remaining failure is a pre-existing permission-model carve-out unrelated to this change, tracked for Phase 3)Note on the platform guards
On Windows Python 3.13+,
os.path.isabs("/tmp")isFalse(a leading slash is drive-relative, not absolute). The existing POSIX-literal-path tests therefore can't run on Windows and are guarded withskipif(os.name == "nt"); a newTestWindowsPathsclass covers theC:\…equivalents. Net gate behaviour on Linux is unchanged.One test assertion changed as a direct consequence of the
.as_posix()fix:test_walk_files_lists_all_non_excludedassertedos.path.join("sub", "b.txt")(which issub\b.txton Windows). Since the code now guarantees/on every OS, the assertion is pinned to the canonicalsub/b.txt.Co-authored by Copilot 🤖