patch: Anchor never-patch matcher to full path (closes #47)#49
Merged
Conversation
`_should_never_patch` previously called `Path(file_str).match(pattern)`,
which matches from the right. So a bare entry like `"README.md"` in
NEVER_PATCH locked out every README.md in any subdirectory:
>>> Path("evals/README.md").match("README.md")
True
That meant `evals/README.md` (which the agent template ships as part
of the eval harness) and any other nested README was unreachable via
`patch` even though no category pattern named it directly. The bug
was found during e2e validation of #44.
Switch to `fnmatch.fnmatchcase`, which matches against the full path:
>>> import fnmatch
>>> fnmatch.fnmatchcase("evals/README.md", "README.md")
False
>>> fnmatch.fnmatchcase("README.md", "README.md")
True
Side effect: fnmatch's `*` matches across path separators, so patterns
like `src/tools/*.py` now also match `src/tools/sub/foo.py`. For
NEVER_PATCH that is a strictly-safer change — over-matching here
protects more user code, not less. All other patterns the CLI uses
today (`src/fipsagents/**`, `tests/**/*.py`, `.env*`, exact paths)
behave identically under both matchers; verified by 12 new
parametrized cases.
Assisted-by: Claude Code (Opus 4.7)
rdwj
force-pushed
the
fix/47-never-patch-matcher
branch
from
2a497a4 to
0532800
Compare
rdwj
added a commit
that referenced
this pull request
May 7, 2026
- Add v0.12.0 changelog entry (manifest loader, evals category, MCP claude category, never-patch matcher fix, pattern gap fills). - Update Patch Commands section: list .fips-template.yaml manifest support, add Gateway/UI category table, refresh per-type tables to match the actual category surface after #43, #46, #48, #49. - Expand the user-customized-files paragraph to cover the new AGENT_NEVER_PATCH entries and the gateway/UI never-patch list. Assisted-by: Claude Code (Opus 4.7)
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.
Found during e2e validation of #44 — a bare `"README.md"` entry in `NEVER_PATCH` locks out every README.md in any subdirectory because `Path.match` matches from the right:
```python
That makes `evals/README.md` (which the agent template ships as part of the eval harness) unreachable via `patch evals` even though it's listed in the eval category's patterns.
Fix
Switch `_should_never_patch` from `Path.match` to `fnmatch.fnmatchcase`, which matches against the full path:
```python
Side effect (intentional)
`fnmatch`'s `` matches across path separators, so `src/tools/.py` now also matches `src/tools/sub/foo.py`. For `NEVER_PATCH` this is strictly safer — over-matching here protects more user code, not less. All other patterns the CLI uses today (`src/fipsagents/`, `tests//.py`, `.env`, exact paths) behave identically under both matchers, verified by 12 new parametrized cases.
Independence
Branched off `main`. Does not depend on or conflict with the existing PR stack (#43 → #46 → #48). Can merge in any order.
Test plan