fix(config): use as_posix() for tilde-expanded path_glob (#643 cluster E)#726
Merged
fix(config): use as_posix() for tilde-expanded path_glob (#643 cluster E)#726
Conversation
…r E) `NamespacePolicyRule._expand_and_validate_glob` stored the expanded path_glob via `str(Path(v).expanduser())`. On Windows that produces backslashes (e.g. `C:\Users\runneradmin\some\path\**`). The downstream consumer is `pathspec.GitIgnoreSpec` (`engine.py:_build_exclude_spec`) which interprets gitignore patterns: gitignore syntax is POSIX-only and treats `\` as an escape character, so a backslash-bearing pattern silently fails to match any file path on Windows. Net effect on Windows before this change: any namespace rule with a leading `~/` in its glob (a common config — `~/.claude/projects/**`, `~/Documents/notes/**`, etc.) silently never matched. Files that should have landed in a project-specific namespace fell through to `default_namespace`, with no warning. `Path(v).expanduser().as_posix()` keeps the tilde expansion (still absolute, still platform-correct expansion via `Path.home()`) but forces `/` separators in the stored pattern, matching gitignore semantics on every platform. Surfaced by `test_indexing_engine.py::TestNamespacePolicyRules:: test_home_tilde_in_path_glob_expanded` (Cluster E of the #643 sweep) — the test asserted `rule.path_glob.endswith("/some/path/**")` which already encoded the POSIX-glob invariant. macOS: 339/339 pass across `test_indexing_engine.py`, `test_config_overrides.py`, `test_validate_namespace.py` — no regressions. On POSIX `as_posix()` is a no-op for absolute paths. Co-Authored-By: Claude <[email protected]>
Windows CI on the previous commit (7974a26) surfaced a regression in `test_config_d_namespace_rules_dedup_after_home_expansion`: AssertionError: ['C:/Users/runneradmin/some/memtomem-test/**', 'C:\\Users\\runneradmin\\some\\memtomem-test\\**'] The validator normalized the `~/` form to forward slashes, but a config fragment that supplied an already-expanded absolute path went through unchanged with backslashes. Two fragments declaring the equivalent rule (one with `~/foo`, one with `C:\Users\me\foo`) then produced different stored strings, defeating the dedup hash that keys on `BaseSettings.model_dump`. Pre-#726 the test passed on Windows by accident: BOTH paths leaked backslashes (`str(Path("C:\\..."))` is identity), so the strings matched. The accident was masking the same gitignore-escape bug described in 7974a26 — neither rule would have actually matched any file on Windows, so dedup of two non-matching rules was moot. Now the validator handles both inputs symmetrically: - `~/...` → `Path(v).expanduser().as_posix()` - absolute → `Path(v).as_posix()` - relative → unchanged (gitignore patterns can legitimately use `\` as an escape character in non-path contexts; only normalize when the input is structurally a path) Test fix: `abs_form` is built via `as_posix()` so the equality check on the deduped rule's stored value works on Windows. POSIX semantics are unchanged. macOS: 339/339 still pass. Co-Authored-By: Claude <[email protected]>
Owner
Author
|
Heads-up: pushed a follow-up commit
The follow-up symmetrizes the normalization:
Pre-#726 the test passed on Windows by accident — both paths leaked backslashes and matched textually, while neither would have actually matched any file due to the gitignore-escape bug Test updated: |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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
Real production bug, surfaced by Cluster E of the #643 Windows sweep.
NamespacePolicyRule._expand_and_validate_glob(config.py:341-347) stored expanded path_globs viastr(Path(v).expanduser()). On Windows that yields backslashes (C:\Users\runneradmin\some\path\**). The downstream consumer ispathspec.GitIgnoreSpec(engine.py:_build_exclude_spec) which interprets gitignore patterns — gitignore syntax is POSIX-only and treats\as an escape character. A backslash-bearing pattern silently never matches any file on Windows.Pre-fix Windows behavior
Any namespace rule with a leading
~/in its glob — a common config shape (~/.claude/projects/**,~/Documents/notes/**, etc.) — silently failed to match. Files that should have landed in a project-specific namespace fell through todefault_namespace, with no warning.Fix
Path(v).expanduser().as_posix()instead ofstr(Path(v).expanduser()):Path.home())./separators in the stored pattern.Test plan
uv run pytest packages/memtomem/tests/test_indexing_engine.py packages/memtomem/tests/test_config_overrides.py packages/memtomem/tests/test_validate_namespace.py— 339/339 pass.ruff check+ruff format --checkclean.test_home_tilde_in_path_glob_expandedshould pass; tilde-prefixed namespace rules should now actually match.Why production fix (not test fix)
The failing test (
test_home_tilde_in_path_glob_expanded) assertedrule.path_glob.endswith("/some/path/**")— this already encoded the POSIX-glob invariant the rule downstream depends on. The test was right; production was lying about the invariant on Windows.Out of scope
Remaining: A (9 — dirty state, production change), H (2 — misc) = 11 fails after this lands. Cluster B already shipping via parallel work (
fix/720-source-filter-windowsmerged as88c3e81).🤖 Generated with Claude Code