Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions src/fips_agents_cli/tools/patching.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""Utilities for patching projects with template updates."""

import difflib
import fnmatch
import json
import shutil
import tempfile
Expand Down Expand Up @@ -555,12 +556,23 @@ def _files_identical(file1: Path, file2: Path) -> bool:


def _should_never_patch(file_path: Path, never_patch: list[str]) -> bool:
"""Check if a file should never be patched, given the rule list."""
"""Return True when ``file_path`` matches any never-patch pattern.

Patterns are matched against the full relative path with
:func:`fnmatch.fnmatchcase`. This anchors the comparison to the
whole path rather than the right-hand side of it, so a bare entry
like ``"README.md"`` matches only the project-root README and not
nested READMEs like ``evals/README.md``. See issue #47.

Note that fnmatch's ``*`` is greedy and matches across path
separators — for our never-patch use case that loosens existing
patterns slightly (e.g. ``src/tools/*.py`` now also matches
``src/tools/sub/foo.py``). This is safer for never-patch since
the goal is to keep the user's code off-limits; over-matching
here protects more, not less.
"""
file_str = str(file_path)
for pattern in never_patch:
if Path(file_str).match(pattern):
return True
return False
return any(fnmatch.fnmatchcase(file_str, pattern) for pattern in never_patch)


def _show_diff_and_ask(
Expand Down
89 changes: 89 additions & 0 deletions tests/test_patch.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,95 @@ def test_user_directory_is_never_patched(self, expected):
assert expected in AGENT_NEVER_PATCH


class TestShouldNeverPatch:
"""Issue #47: bare patterns like "README.md" used to match nested
files (evals/README.md, docs/foo/README.md) because Path.match
matches from the right. The matcher now anchors to the full path
via fnmatch.fnmatchcase.
"""

@pytest.mark.parametrize(
"rel_path,expected",
[
# The bug from #47 — bare README.md must NOT lock out nested ones
("evals/README.md", False),
("docs/sub/README.md", False),
# The project-root README.md must still be protected
("README.md", True),
# Negative: a non-README file is unaffected
("evals/discovery.py", False),
],
)
def test_bare_filename_matches_only_root(self, rel_path, expected):
# Minimal never_patch list — just the bare filename
assert patching._should_never_patch(Path(rel_path), ["README.md"]) is expected

@pytest.mark.parametrize(
"rel_path,expected",
[
("agent.yaml", True),
("not-agent.yaml", False),
("chart/values.yaml", True),
("chart/templates/values.yaml", False),
("src/agent.py", True),
("src/agents.py", False),
],
)
def test_exact_path_patterns(self, rel_path, expected):
patterns = ["agent.yaml", "chart/values.yaml", "src/agent.py"]
assert patching._should_never_patch(Path(rel_path), patterns) is expected

@pytest.mark.parametrize(
"rel_path,expected",
[
("src/fipsagents/foo.py", True),
("src/fipsagents/sub/foo.py", True),
("src/fipsagents/sub/deeper/foo.py", True),
("src/other/foo.py", False),
],
)
def test_recursive_glob(self, rel_path, expected):
assert patching._should_never_patch(Path(rel_path), ["src/fipsagents/**"]) is expected

@pytest.mark.parametrize(
"rel_path,expected",
[
# tests/**/*.py requires at least one nested directory
("tests/foo/bar.py", True),
("tests/foo/bar/baz.py", True),
("tests/foo.py", False), # No nested dir
("other/foo.py", False),
],
)
def test_tests_recursive_pattern(self, rel_path, expected):
assert patching._should_never_patch(Path(rel_path), ["tests/**/*.py"]) is expected

@pytest.mark.parametrize(
"rel_path,expected",
[
(".env", True),
(".env.local", True),
(".env.production", True),
("env", False),
("not-.env", False),
],
)
def test_dotenv_glob(self, rel_path, expected):
assert patching._should_never_patch(Path(rel_path), [".env*"]) is expected

def test_empty_pattern_list_never_matches(self):
assert patching._should_never_patch(Path("anything.py"), []) is False

def test_real_never_patch_lists_protect_root_readme_only(self):
# Regression: the actual constants must not lock out evals/README.md
# (which the agent template ships as part of the eval harness).
assert patching._should_never_patch(Path("README.md"), AGENT_NEVER_PATCH) is True
assert patching._should_never_patch(Path("evals/README.md"), AGENT_NEVER_PATCH) is False
# Same expectation for the MCP list
assert patching._should_never_patch(Path("README.md"), MCP_NEVER_PATCH) is True
assert patching._should_never_patch(Path("docs/README.md"), MCP_NEVER_PATCH) is False


# ---------------------------------------------------------------------------
# Unit tests — find_fips_project_root walks up to .template-info
# ---------------------------------------------------------------------------
Expand Down
Loading