diff --git a/src/fips_agents_cli/tools/patching.py b/src/fips_agents_cli/tools/patching.py index b4a9458..e4e6788 100644 --- a/src/fips_agents_cli/tools/patching.py +++ b/src/fips_agents_cli/tools/patching.py @@ -1,6 +1,7 @@ """Utilities for patching projects with template updates.""" import difflib +import fnmatch import json import shutil import tempfile @@ -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( diff --git a/tests/test_patch.py b/tests/test_patch.py index fbd736b..a66ce77 100644 --- a/tests/test_patch.py +++ b/tests/test_patch.py @@ -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 # ---------------------------------------------------------------------------