From 9d094b4911379fe3415de72d7a2e85e9f00f26ea Mon Sep 17 00:00:00 2001 From: Adam Boczek Date: Sun, 18 Oct 2026 11:51:06 +0200 Subject: [PATCH 01/10] Add .presetignore support for preset installation Share extension ignore loading and matching with presets while preserving extension behavior. Validate preset ignore files before the directory installer removes existing files, registry entries, or registered commands. Cover pattern semantics, encoding failures, ignored manifests and assets, archive reinstalls, and CLI updates. Document filtering rules and the existing remove-then-add update limitation. Validation: 1,520 tests pass in this checkout's virtual environment, with three failure-path warnings. Capture 25 failing preset cases before the implementation and verify that they pass afterward. Assisted-by: Codex (model: GPT-6, autonomous) --- docs/reference/presets.md | 43 +++++ src/specify_cli/_install_ignore.py | 70 ++++++++ src/specify_cli/extensions/__init__.py | 64 +------- src/specify_cli/presets/_manager.py | 6 +- .../test_preset_update_workflow.py | 8 +- tests/specify_cli/presets/test_manager.py | 151 ++++++++++++++++++ tests/test_extensions.py | 15 ++ 7 files changed, 293 insertions(+), 64 deletions(-) create mode 100644 src/specify_cli/_install_ignore.py diff --git a/docs/reference/presets.md b/docs/reference/presets.md index 8ca29547b7..0126513d2a 100644 --- a/docs/reference/presets.md +++ b/docs/reference/presets.md @@ -45,6 +45,49 @@ may omit the digest. > **Note:** All preset commands require a project already initialized with `specify init`. +### Exclude development files with `.presetignore` + +Place a UTF-8 `.presetignore` beside `preset.yml` in the preset source root to +filter files copied into `.specify/presets//`. For example: + +```gitignore +# Development-only directories +.git/ +.idea/ +tests/ +``` + +Filtering applies to local directory installs and extracted archives, including +reinstalls and replacements installed by `preset update`. Without +`.presetignore`, all source files are copied as before. **`.gitignore` is not +used for installation filtering**, and there are no implicit exclusions for +development files. + +Patterns follow the same Git-style matching and normalization as +`.extensionignore`: + +- Use one pattern per line; blank lines and `#` comments are ignored. +- `*` and `?` do not cross directory boundaries; `**` can match across directories. +- A trailing `/` matches directories only. A leading `/`, or a `/` within a + pattern other than its trailing slash, anchors the pattern to the source root. + Patterns without such a slash match names at any depth. +- A later `!` pattern re-includes a match. An excluded directory is not traversed, + so re-including a child requires re-including its parent directory too. +- Pattern whitespace is trimmed and backslashes are normalized to `/`, matching + the existing extension behavior. Use UTF-8 for non-ASCII names. + +Only the source root's `.presetignore` is read; nested ignore files do not add +rules. Files named `.presetignore` are themselves excluded, even if negated. +Manifest and declared asset files are not exempt from filtering: keep +`preset.yml` and any templates, commands, or scripts the preset needs included. +Excluding them can leave an incomplete or unusable installation. + +The source-directory installer reads and validates `.presetignore` before +removing an existing destination, including during a forced reinstall. Invalid +UTF-8 raises a preset validation error. The separate `preset update` command +retains its remove-then-add behavior described below; it does not preflight the +replacement ignore file before removal. + ## Update a Preset ```bash diff --git a/src/specify_cli/_install_ignore.py b/src/specify_cli/_install_ignore.py new file mode 100644 index 0000000000..d0e713b215 --- /dev/null +++ b/src/specify_cli/_install_ignore.py @@ -0,0 +1,70 @@ +"""Shared source-directory installation filtering for extensions and presets.""" + +from pathlib import Path +from typing import Callable, List, Optional, Set + +import pathspec + + +def load_install_ignore( + source_dir: Path, + ignore_filename: str, + validation_error: type[Exception], +) -> Optional[Callable[[str, List[str]], Set[str]]]: + """Load a UTF-8 ignore file as a copytree callback, or return None if absent. + + Match the extension installer's Git-style patterns, including whitespace + trimming and backslash normalization. Only the named root ignore file is + read; there are no default exclusions except the ignore filename itself. + Invalid encoding raises the caller's domain validation error before copying. + Excluded directories are pruned by copytree, so child negations cannot + restore files inside them unless the parent directory is also re-included. + """ + ignore_file = source_dir / ignore_filename + if not ignore_file.exists(): + return None + + # Explicit UTF-8 keeps non-ASCII patterns independent of the host locale. + try: + raw = ignore_file.read_text(encoding="utf-8") + except UnicodeDecodeError as e: + raise validation_error( + f"{ignore_filename} is not valid UTF-8: {ignore_file} " + f"({e.reason} at byte {e.start})" + ) + lines: List[str] = raw.splitlines() + + # Normalise backslashes in patterns so Windows-authored files work + normalised: List[str] = [] + for line in lines: + stripped = line.strip() + if stripped and not stripped.startswith("#"): + normalised.append(stripped.replace("\\", "/")) + else: + # Preserve blanks/comments so pathspec line numbers stay stable + normalised.append(line) + + # Always ignore the ignore file itself, even after a negation pattern. + normalised.append(ignore_filename) + + spec = pathspec.GitIgnoreSpec.from_lines(normalised) + + def _ignore(directory: str, entries: List[str]) -> Set[str]: + ignored: Set[str] = set() + rel_dir = Path(directory).relative_to(source_dir) + for entry in entries: + rel_path = str(rel_dir / entry) if str(rel_dir) != "." else entry + # Normalise to forward slashes for consistent matching + rel_path_fwd = rel_path.replace("\\", "/") + + entry_full = Path(directory) / entry + if entry_full.is_dir(): + # Append '/' so directory-only patterns (e.g. tests/) match + if spec.match_file(rel_path_fwd + "/"): + ignored.add(entry) + else: + if spec.match_file(rel_path_fwd): + ignored.add(entry) + return ignored + + return _ignore diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index d03cb85770..c22445ebe0 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -26,7 +26,6 @@ from pathlib import Path from typing import Any, BinaryIO, Callable, Dict, List, Optional, Set -import pathspec import yaml from packaging import version as pkg_version from packaging.specifiers import InvalidSpecifier, SpecifierSet @@ -43,6 +42,7 @@ safe_extract_archive, ) from .._init_options import is_ai_skills_enabled +from .._install_ignore import load_install_ignore from .._invocation_style import is_dollar_skills_agent, is_slash_skills_agent from .._utils import dump_frontmatter, relative_extension_path_violation, version_satisfies from ..catalogs import CatalogEntry as BaseCatalogEntry @@ -1275,67 +1275,7 @@ def _load_extensionignore( An ignore function compatible with shutil.copytree, or None if no .extensionignore file exists. """ - ignore_file = source_dir / ".extensionignore" - if not ignore_file.exists(): - return None - - # Pin UTF-8 explicitly: ``Path.read_text`` defaults to the system - # locale codec on Windows (cp1252 / gb2312 / cp932), which silently - # corrupts multibyte patterns when the file is shared across - # machines with different locales. The next line already - # normalises backslashes "so Windows-authored files work" — the - # codebase already expects Windows authors to write this file. - # - # A file that is not valid UTF-8 is a user-authoring mistake, so - # surface it as ``ValidationError`` with a pointer to the offending - # byte — the same pattern ``ExtensionManifest._load_yaml`` uses - # for ``extension.yml`` (see ``UnicodeDecodeError`` handler in - # this module). Without the wrap, the raw ``UnicodeDecodeError`` - # would abort installation with a Python traceback instead of a - # clear message naming the file. - try: - raw = ignore_file.read_text(encoding="utf-8") - except UnicodeDecodeError as e: - raise ValidationError( - f".extensionignore is not valid UTF-8: {ignore_file} " - f"({e.reason} at byte {e.start})" - ) - lines: List[str] = raw.splitlines() - - # Normalise backslashes in patterns so Windows-authored files work - normalised: List[str] = [] - for line in lines: - stripped = line.strip() - if stripped and not stripped.startswith("#"): - normalised.append(stripped.replace("\\", "/")) - else: - # Preserve blanks/comments so pathspec line numbers stay stable - normalised.append(line) - - # Always ignore the .extensionignore file itself - normalised.append(".extensionignore") - - spec = pathspec.GitIgnoreSpec.from_lines(normalised) - - def _ignore(directory: str, entries: List[str]) -> Set[str]: - ignored: Set[str] = set() - rel_dir = Path(directory).relative_to(source_dir) - for entry in entries: - rel_path = str(rel_dir / entry) if str(rel_dir) != "." else entry - # Normalise to forward slashes for consistent matching - rel_path_fwd = rel_path.replace("\\", "/") - - entry_full = Path(directory) / entry - if entry_full.is_dir(): - # Append '/' so directory-only patterns (e.g. tests/) match - if spec.match_file(rel_path_fwd + "/"): - ignored.add(entry) - else: - if spec.match_file(rel_path_fwd): - ignored.add(entry) - return ignored - - return _ignore + return load_install_ignore(source_dir, ".extensionignore", ValidationError) @project_registration def _get_skills_dir(self, *, create: bool = True) -> Optional[Path]: diff --git a/src/specify_cli/presets/_manager.py b/src/specify_cli/presets/_manager.py index 963bb5c640..cf6c61f434 100644 --- a/src/specify_cli/presets/_manager.py +++ b/src/specify_cli/presets/_manager.py @@ -12,6 +12,7 @@ from .._download_security import safe_extract_archive from .._init_options import is_ai_skills_enabled, resolve_active_agent_for_registration +from .._install_ignore import load_install_ignore from .._utils import version_satisfies from ..extensions import REINSTALL_COMMAND, normalize_priority from ..shared_infra import ( @@ -402,13 +403,16 @@ def install_from_directory( f"Preset '{manifest.id}' is already installed. " f"Use 'specify preset remove {manifest.id}' first." ) + # Validate before remove() changes registry/agent artifacts or deleting + # an unregistered destination left behind by an earlier installation. + ignore_fn = load_install_ignore(source_dir, ".presetignore", PresetValidationError) before_file_change(dest_dir, removal=True) try: if installed: self.remove(manifest.id) if dest_dir.exists(): shutil.rmtree(dest_dir) - shutil.copytree(source_dir, dest_dir) + shutil.copytree(source_dir, dest_dir, ignore=ignore_fn) finally: after_file_change(dest_dir) diff --git a/tests/integration/test_preset_update_workflow.py b/tests/integration/test_preset_update_workflow.py index a0d2e5b8a9..1049aff7a0 100644 --- a/tests/integration/test_preset_update_workflow.py +++ b/tests/integration/test_preset_update_workflow.py @@ -162,7 +162,8 @@ def test_preset_update_cli_contract(): assert "--dry-run" not in help_output -def test_preset_update_replaces_installed_preset(project: Path, tmp_path: Path): +@pytest.mark.parametrize("use_ignore", [False, True]) +def test_preset_update_replaces_installed_preset(project: Path, tmp_path: Path, use_ignore): """A successful update really swaps the installed preset on disk.""" runner = CliRunner() @@ -178,6 +179,9 @@ def test_preset_update_replaces_installed_preset(project: Path, tmp_path: Path): body="# Version Two Template\n", extra_file="only-in-v2.md", ) + (replacement / "dev-only.txt").write_text("development notes\n", encoding="utf-8") + if use_ignore: + (replacement / ".presetignore").write_text("dev-only.txt\n", encoding="utf-8") install = runner.invoke( app, ["preset", "add", "--dev", str(original), "--priority", "20"] @@ -201,6 +205,8 @@ def test_preset_update_replaces_installed_preset(project: Path, tmp_path: Path): template = installed_dir / "templates" / "spec-template.md" assert template.read_text(encoding="utf-8") == "# Version Two Template\n" assert (installed_dir / "only-in-v2.md").exists() + assert (installed_dir / "dev-only.txt").exists() is not use_ignore + assert not (installed_dir / ".presetignore").exists() assert not (installed_dir / "only-in-v1.md").exists(), ( "the previous preset's files must be removed, not merged with the " "replacement" diff --git a/tests/specify_cli/presets/test_manager.py b/tests/specify_cli/presets/test_manager.py index 91e0805593..1f0d9da07c 100644 --- a/tests/specify_cli/presets/test_manager.py +++ b/tests/specify_cli/presets/test_manager.py @@ -101,6 +101,157 @@ def unreadable(path, *args, **kwargs): ) +class TestPresetIgnore: + """Installation filtering follows the extension ignore contract.""" + + @staticmethod + def _write_files(root, paths): + for relative in paths: + path = root / relative + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(f"contents of {relative}\n", encoding="utf-8") + + @pytest.mark.parametrize("ignore_content", [None, "", "!.presetignore\n"]) + def test_no_default_exclusions(self, project_dir, pack_dir, ignore_content): + paths = [".git/config", ".idea/project.xml", "tests/test_example.py", "debug.log"] + self._write_files(pack_dir, paths) + (pack_dir / ".gitignore").write_text("*\n", encoding="utf-8") + (pack_dir / ".extensionignore").write_text("*\n", encoding="utf-8") + if ignore_content is not None: + (pack_dir / ".presetignore").write_text(ignore_content, encoding="utf-8") + + PresetManager(project_dir).install_from_directory(pack_dir, "0.1.5") + + dest = project_dir / ".specify/presets/test-pack" + for relative in [ + *paths, ".gitignore", ".extensionignore", "preset.yml", "templates/spec-template.md" + ]: + assert (dest / relative).read_bytes() == (pack_dir / relative).read_bytes() + assert not (dest / ".presetignore").exists() + + @pytest.mark.parametrize("root_ignore", [False, True]) + def test_nested_ignore_is_not_loaded(self, project_dir, pack_dir, root_ignore): + self._write_files(pack_dir, ["nested/keep.txt"]) + (pack_dir / "nested/.presetignore").write_bytes(b"*\n\xff\n") + if root_ignore: + (pack_dir / ".presetignore").write_text("", encoding="utf-8") + + PresetManager(project_dir).install_from_directory(pack_dir, "0.1.5") + + dest = project_dir / ".specify/presets/test-pack/nested" + assert (dest / "keep.txt").is_file() + assert (dest / ".presetignore").exists() is not root_ignore + + @pytest.mark.parametrize( + ("patterns", "excluded", "included"), + [ + (".git/\n.idea/\ntests/\n", [".git/config", ".idea/x", "tests/a.py"], ["README.md"]), + ("*.pyc\n", ["cache.pyc", "nested/cache.pyc"], ["nested/code.py"]), + ("# comment\n\n notes.txt \n\n", ["notes.txt"], ["README.md"]), + ("docs/internal/draft.md\n", ["docs/internal/draft.md"], ["docs/guide.md"]), + ("docs\\internal\\draft.md\n", ["docs/internal/draft.md"], ["docs/guide.md"]), + ("ドキュメント/\ncafé/\n", ["ドキュメント/private.md", "café/résumé.txt"], ["docs/guide.md"]), + ("docs/*.draft.md\n", ["docs/api.draft.md"], ["docs/sub/api.draft.md"]), + ("docs/**/*.draft.md\n", ["docs/api.draft.md", "docs/sub/api.draft.md"], ["docs/guide.md"]), + ("docs/*.md\n!docs/api.md\n", ["docs/guide.md"], ["docs/api.md"]), + ("/notes.txt\n", ["notes.txt"], ["nested/notes.txt"]), + ("docs/draft.md\n", ["docs/draft.md"], ["nested/docs/draft.md"]), + ("cache/\n", ["cache/data", "nested/cache/data"], ["other/cache"]), + ("file?.txt\n", ["file1.txt"], ["file12.txt"]), + ("../sibling/\n/etc/passwd\n", [], ["sibling/data", "passwd"]), + # copytree never visits an excluded parent; child negation cannot restore it. + ("docs/\n!docs/api.md\n", ["docs/api.md"], ["README.md"]), + ("docs/\n!docs/\n", [], ["docs/api.md"]), + ], + ) + def test_pattern_semantics(self, project_dir, pack_dir, patterns, excluded, included): + self._write_files(pack_dir, [*excluded, *included]) + (pack_dir / ".presetignore").write_text(patterns, encoding="utf-8") + + PresetManager(project_dir).install_from_directory(pack_dir, "0.1.5") + + dest = project_dir / ".specify/presets/test-pack" + for relative in excluded: + assert not (dest / relative).exists() + for relative in included: + assert (dest / relative).read_bytes() == (pack_dir / relative).read_bytes() + assert (dest / "preset.yml").is_file() + assert not (dest / ".presetignore").exists() + + @pytest.mark.parametrize("registered", [False, True]) + def test_invalid_utf8_preserves_existing_state( + self, project_dir, pack_dir, valid_pack_data, registered + ): + """Validate before deleting even an unregistered leftover destination.""" + valid_pack_data["provides"]["templates"].append({ + "type": "command", "name": "speckit.ignore.demo", "file": "commands/demo.md", + "description": "Ignore test command", + }) + (pack_dir / "preset.yml").write_text(yaml.safe_dump(valid_pack_data), encoding="utf-8") + self._write_files(pack_dir, ["commands/demo.md"]) + (project_dir / ".claude/skills").mkdir(parents=True) + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.5") + assert manager.registry.get("test-pack")["registered_commands"] + if not registered: + manager.registry.remove("test-pack") + + def snapshot(): + return { + str(path.relative_to(project_dir)): path.read_bytes() + for path in project_dir.rglob("*") if path.is_file() + } + + before = snapshot() + (pack_dir / ".presetignore").write_bytes(b"caf\xe9/\n") + with pytest.raises(PresetValidationError, match=r"\.presetignore is not valid UTF-8.*byte 3"): + manager.install_from_directory(pack_dir, "0.1.5", force=registered) + + assert snapshot() == before + assert manager.registry.is_installed("test-pack") is registered + + @pytest.mark.parametrize("route", ["directory", "zip", "tar"]) + def test_reinstall_filters_new_source(self, project_dir, pack_dir, temp_dir, route): + self._write_files(pack_dir, ["tests/dev.py", "keep.txt"]) + manager = PresetManager(project_dir) + manager.install_from_directory(pack_dir, "0.1.5") + dest = project_dir / ".specify/presets/test-pack" + assert (dest / "tests/dev.py").is_file() + (pack_dir / ".presetignore").write_text("tests/\n", encoding="utf-8") + (pack_dir / "keep.txt").write_text("updated\n", encoding="utf-8") + + if route == "directory": + manager.install_from_directory(pack_dir, "0.1.5", force=True) + elif route == "zip": + archive_path = temp_dir / "preset.zip" + with zipfile.ZipFile(archive_path, "w") as archive: + for path in pack_dir.rglob("*"): + if path.is_file(): + archive.write(path, Path("package") / path.relative_to(pack_dir)) + manager.install_from_zip(archive_path, "0.1.5", force=True) + else: + archive_path = temp_dir / "preset.tar.gz" + with tarfile.open(archive_path, "w:gz") as archive: + archive.add(pack_dir, arcname="package") + manager.install_from_archive(archive_path, "0.1.5", force=True) + + assert not (dest / "tests").exists() + assert not (dest / ".presetignore").exists() + assert (dest / "keep.txt").read_text(encoding="utf-8") == "updated\n" + assert manager.registry.is_installed("test-pack") + + @pytest.mark.parametrize("excluded", ["preset.yml", "templates/spec-template.md"]) + def test_declared_files_are_not_exempt(self, project_dir, pack_dir, excluded): + """Like extensions, filtering does not protect the manifest or declared assets.""" + (pack_dir / ".presetignore").write_text(excluded + "\n", encoding="utf-8") + manager = PresetManager(project_dir) + manifest = manager.install_from_directory(pack_dir, "0.1.5") + + assert manifest.id == "test-pack" + assert manager.registry.is_installed("test-pack") + assert not (project_dir / ".specify/presets/test-pack" / excluded).exists() + + class TestPresetManager: """Test PresetManager installation and removal.""" diff --git a/tests/test_extensions.py b/tests/test_extensions.py index b512742389..f2a75d08a4 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -7190,6 +7190,21 @@ def test_search_results_include_catalog_metadata(self, temp_dir): class TestExtensionIgnore: """Test .extensionignore support during extension installation.""" + @pytest.mark.parametrize("excluded", ["extension.yml", "commands/hello.md"]) + def test_declared_files_are_not_exempt(self, temp_dir, valid_manifest_data, excluded): + """Filtering does not implicitly protect manifests or declared command files.""" + ext_dir = self._make_extension( + temp_dir, valid_manifest_data, ignore_content=excluded + "\n" + ) + project = temp_dir / "project" + (project / ".specify").mkdir(parents=True) + manager = ExtensionManager(project) + + manifest = manager.install_from_directory(ext_dir, "0.1.0", register_commands=False) + + assert manager.registry.is_installed(manifest.id) + assert not (project / ".specify/extensions" / manifest.id / excluded).exists() + def _make_extension(self, temp_dir, valid_manifest_data, extra_files=None, ignore_content=None): """Helper to create an extension directory with optional extra files and .extensionignore.""" import yaml From ad2457156e81e22bfd954dc5092989c8e3ff41e4 Mon Sep 17 00:00:00 2001 From: Adam Boczek Date: Sun, 18 Oct 2026 11:58:26 +0200 Subject: [PATCH 02/10] Cover shared ignore contracts and validate preset scaffolding Move all 16 pattern cases into the mirrored test_install_ignore suite and run them for both extension and preset ignore files. Retain installer lifecycle coverage and test absent files, self-exclusion, domain validation errors, and read failures at the shared helper boundary. Validation: - 1,546 relevant tests pass with three existing failure-path warnings. - Full-suite collection increases from 12,748 to 12,774; all original pattern parameter sets remain represented. - Ruff 0.15.10 and git diff --check pass for the follow-up. - Initialize a temporary Codex skills project with a local preset using .presetignore and confirm development files are excluded while a template matched by .gitignore remains installed. - Execute the installed speckit-specify skill in the Codex session, resolve the preset template, and create a reading-list specification with its preset marker retained and all 16 quality-checklist criteria satisfied. - Run preset update with replacement ignore rules and verify the new retained file replaces the previously retained file. Manual workflow validation was performed by Codex on macOS with Bash project scripts; no human testing or cross-platform CI result is claimed. The full test suite and CI OS/Python matrix were not executed. Assisted-by: Codex (model: GPT-6, autonomous) --- tests/specify_cli/presets/test_manager.py | 36 ------- tests/specify_cli/test_install_ignore.py | 112 ++++++++++++++++++++++ 2 files changed, 112 insertions(+), 36 deletions(-) create mode 100644 tests/specify_cli/test_install_ignore.py diff --git a/tests/specify_cli/presets/test_manager.py b/tests/specify_cli/presets/test_manager.py index 1f0d9da07c..ff1738ab4c 100644 --- a/tests/specify_cli/presets/test_manager.py +++ b/tests/specify_cli/presets/test_manager.py @@ -142,42 +142,6 @@ def test_nested_ignore_is_not_loaded(self, project_dir, pack_dir, root_ignore): assert (dest / "keep.txt").is_file() assert (dest / ".presetignore").exists() is not root_ignore - @pytest.mark.parametrize( - ("patterns", "excluded", "included"), - [ - (".git/\n.idea/\ntests/\n", [".git/config", ".idea/x", "tests/a.py"], ["README.md"]), - ("*.pyc\n", ["cache.pyc", "nested/cache.pyc"], ["nested/code.py"]), - ("# comment\n\n notes.txt \n\n", ["notes.txt"], ["README.md"]), - ("docs/internal/draft.md\n", ["docs/internal/draft.md"], ["docs/guide.md"]), - ("docs\\internal\\draft.md\n", ["docs/internal/draft.md"], ["docs/guide.md"]), - ("ドキュメント/\ncafé/\n", ["ドキュメント/private.md", "café/résumé.txt"], ["docs/guide.md"]), - ("docs/*.draft.md\n", ["docs/api.draft.md"], ["docs/sub/api.draft.md"]), - ("docs/**/*.draft.md\n", ["docs/api.draft.md", "docs/sub/api.draft.md"], ["docs/guide.md"]), - ("docs/*.md\n!docs/api.md\n", ["docs/guide.md"], ["docs/api.md"]), - ("/notes.txt\n", ["notes.txt"], ["nested/notes.txt"]), - ("docs/draft.md\n", ["docs/draft.md"], ["nested/docs/draft.md"]), - ("cache/\n", ["cache/data", "nested/cache/data"], ["other/cache"]), - ("file?.txt\n", ["file1.txt"], ["file12.txt"]), - ("../sibling/\n/etc/passwd\n", [], ["sibling/data", "passwd"]), - # copytree never visits an excluded parent; child negation cannot restore it. - ("docs/\n!docs/api.md\n", ["docs/api.md"], ["README.md"]), - ("docs/\n!docs/\n", [], ["docs/api.md"]), - ], - ) - def test_pattern_semantics(self, project_dir, pack_dir, patterns, excluded, included): - self._write_files(pack_dir, [*excluded, *included]) - (pack_dir / ".presetignore").write_text(patterns, encoding="utf-8") - - PresetManager(project_dir).install_from_directory(pack_dir, "0.1.5") - - dest = project_dir / ".specify/presets/test-pack" - for relative in excluded: - assert not (dest / relative).exists() - for relative in included: - assert (dest / relative).read_bytes() == (pack_dir / relative).read_bytes() - assert (dest / "preset.yml").is_file() - assert not (dest / ".presetignore").exists() - @pytest.mark.parametrize("registered", [False, True]) def test_invalid_utf8_preserves_existing_state( self, project_dir, pack_dir, valid_pack_data, registered diff --git a/tests/specify_cli/test_install_ignore.py b/tests/specify_cli/test_install_ignore.py new file mode 100644 index 0000000000..6cc0052a6b --- /dev/null +++ b/tests/specify_cli/test_install_ignore.py @@ -0,0 +1,112 @@ +"""Contract tests for shared installation filtering in specify_cli._install_ignore.""" + +import shutil +from pathlib import Path + +import pytest + +from specify_cli._install_ignore import load_install_ignore +from specify_cli.extensions import ValidationError +from specify_cli.presets import PresetValidationError + + +@pytest.fixture(params=[ + pytest.param((".extensionignore", ValidationError), id="extension"), + pytest.param((".presetignore", PresetValidationError), id="preset"), +]) +def ignore_contract(request): + return request.param + + +@pytest.mark.parametrize( + ("patterns", "excluded", "included"), + [ + (".git/\n.idea/\ntests/\n", [".git/config", ".idea/x", "tests/a.py"], ["README.md"]), + ("*.pyc\n", ["cache.pyc", "nested/cache.pyc"], ["nested/code.py"]), + ("# comment\n\n notes.txt \n\n", ["notes.txt"], ["README.md"]), + ("docs/internal/draft.md\n", ["docs/internal/draft.md"], ["docs/guide.md"]), + ("docs\\internal\\draft.md\n", ["docs/internal/draft.md"], ["docs/guide.md"]), + ("ドキュメント/\ncafé/\n", ["ドキュメント/private.md", "café/résumé.txt"], ["docs/guide.md"]), + ("docs/*.draft.md\n", ["docs/api.draft.md"], ["docs/sub/api.draft.md"]), + ("docs/**/*.draft.md\n", ["docs/api.draft.md", "docs/sub/api.draft.md"], ["docs/guide.md"]), + ("docs/*.md\n!docs/api.md\n", ["docs/guide.md"], ["docs/api.md"]), + ("/notes.txt\n", ["notes.txt"], ["nested/notes.txt"]), + ("docs/draft.md\n", ["docs/draft.md"], ["nested/docs/draft.md"]), + ("cache/\n", ["cache/data", "nested/cache/data"], ["other/cache"]), + ("file?.txt\n", ["file1.txt"], ["file12.txt"]), + ("../sibling/\n/etc/passwd\n", [], ["sibling/data", "passwd"]), + # copytree never visits an excluded parent; child negation cannot restore it. + ("docs/\n!docs/api.md\n", ["docs/api.md"], ["README.md"]), + ("docs/\n!docs/\n", [], ["docs/api.md"]), + ], +) +def test_pattern_semantics(tmp_path, ignore_contract, patterns, excluded, included): + """Both callers use the same pattern contract with copytree traversal.""" + ignore_filename, validation_error = ignore_contract + source = tmp_path / "source" + source.mkdir() + for relative in [*excluded, *included, "keep.txt"]: + path = source / relative + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(f"contents of {relative}\n", encoding="utf-8") + (source / ignore_filename).write_text(patterns, encoding="utf-8") + + dest = tmp_path / "installed" + shutil.copytree(source, dest, ignore=load_install_ignore(source, ignore_filename, validation_error)) + + for relative in excluded: + assert not (dest / relative).exists() + for relative in [*included, "keep.txt"]: + assert (dest / relative).read_bytes() == (source / relative).read_bytes() + assert not (dest / ignore_filename).exists() + + +def test_absent_ignore_returns_no_filter(tmp_path, ignore_contract): + ignore_filename, validation_error = ignore_contract + (tmp_path / ".gitignore").write_text("*\n", encoding="utf-8") + + assert load_install_ignore(tmp_path, ignore_filename, validation_error) is None + + +@pytest.mark.parametrize("negate_self", [False, True]) +def test_ignore_file_is_always_excluded(tmp_path, ignore_contract, negate_self): + ignore_filename, validation_error = ignore_contract + source = tmp_path / "source" + source.mkdir() + (source / ignore_filename).write_text( + f"!{ignore_filename}\n" if negate_self else "", encoding="utf-8" + ) + (source / "keep.txt").write_text("keep\n", encoding="utf-8") + + dest = tmp_path / "installed" + shutil.copytree(source, dest, ignore=load_install_ignore(source, ignore_filename, validation_error)) + + assert sorted(path.name for path in dest.iterdir()) == ["keep.txt"] + + +def test_invalid_utf8_uses_callers_validation_error(tmp_path, ignore_contract): + ignore_filename, validation_error = ignore_contract + ignore_file = tmp_path / ignore_filename + ignore_file.write_bytes(b"caf\xe9/\n") + + with pytest.raises(validation_error) as caught: + load_install_ignore(tmp_path, ignore_filename, validation_error) + + assert f"{ignore_filename} is not valid UTF-8: {ignore_file}" in str(caught.value) + assert "byte 3" in str(caught.value) + + +def test_read_failure_is_propagated(tmp_path, ignore_contract, monkeypatch): + ignore_filename, validation_error = ignore_contract + ignore_file = tmp_path / ignore_filename + ignore_file.write_text("tests/\n", encoding="utf-8") + original_read = Path.read_text + + def fail_read(path, *args, **kwargs): + if path == ignore_file: + raise PermissionError("ignore file is unreadable") + return original_read(path, *args, **kwargs) + + monkeypatch.setattr(Path, "read_text", fail_read) + with pytest.raises(PermissionError, match="ignore file is unreadable"): + load_install_ignore(tmp_path, ignore_filename, validation_error) From a555146b729f8ed2f0197d78415337a0b9113635 Mon Sep 17 00:00:00 2001 From: Adam Boczek Date: Sun, 18 Oct 2026 16:11:58 +0200 Subject: [PATCH 03/10] Accept UTF-8 BOMs in installation ignore files Decode preset and extension ignore files with utf-8-sig so an optional BOM does not become part of the first pattern. Cover ASCII and non-ASCII first patterns with BOM and CRLF fixtures, and document BOM support. Validation: four new regression cases failed before the fix; the relevant preset, extension, and shared-ignore suites passed with 1550 tests and 3 warnings. Ruff and git diff --check passed. Assisted-by: Codex (model: GPT-6, autonomous) --- docs/reference/presets.md | 5 +++-- src/specify_cli/_install_ignore.py | 5 +++-- tests/specify_cli/test_install_ignore.py | 18 ++++++++++++++++++ 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/docs/reference/presets.md b/docs/reference/presets.md index 0126513d2a..c42e878c6a 100644 --- a/docs/reference/presets.md +++ b/docs/reference/presets.md @@ -47,8 +47,9 @@ may omit the digest. ### Exclude development files with `.presetignore` -Place a UTF-8 `.presetignore` beside `preset.yml` in the preset source root to -filter files copied into `.specify/presets//`. For example: +Place a UTF-8 `.presetignore` (with or without a byte-order mark) beside +`preset.yml` in the preset source root to filter files copied into +`.specify/presets//`. For example: ```gitignore # Development-only directories diff --git a/src/specify_cli/_install_ignore.py b/src/specify_cli/_install_ignore.py index d0e713b215..43c7ed7afa 100644 --- a/src/specify_cli/_install_ignore.py +++ b/src/specify_cli/_install_ignore.py @@ -24,9 +24,10 @@ def load_install_ignore( if not ignore_file.exists(): return None - # Explicit UTF-8 keeps non-ASCII patterns independent of the host locale. + # Accept UTF-8 with or without the BOM emitted by Windows PowerShell 5.1. + # Otherwise the BOM becomes part of the first pattern and prevents matching. try: - raw = ignore_file.read_text(encoding="utf-8") + raw = ignore_file.read_text(encoding="utf-8-sig") except UnicodeDecodeError as e: raise validation_error( f"{ignore_filename} is not valid UTF-8: {ignore_file} " diff --git a/tests/specify_cli/test_install_ignore.py b/tests/specify_cli/test_install_ignore.py index 6cc0052a6b..5ef4215e9c 100644 --- a/tests/specify_cli/test_install_ignore.py +++ b/tests/specify_cli/test_install_ignore.py @@ -68,6 +68,24 @@ def test_absent_ignore_returns_no_filter(tmp_path, ignore_contract): assert load_install_ignore(tmp_path, ignore_filename, validation_error) is None +@pytest.mark.parametrize("directory", ["tests", "café"]) +def test_utf8_bom_first_pattern(tmp_path, ignore_contract, directory): + """Accept the UTF-8 BOM and CRLF bytes emitted by Windows PowerShell 5.1.""" + ignore_filename, validation_error = ignore_contract + source = tmp_path / "source" + (source / directory).mkdir(parents=True) + (source / directory / "private.txt").write_text("development only", encoding="utf-8") + (source / "keep.txt").write_text("shipped", encoding="utf-8") + (source / ignore_filename).write_bytes(b"\xef\xbb\xbf" + f"{directory}/\r\n".encode("utf-8")) + + dest = tmp_path / "installed" + shutil.copytree(source, dest, ignore=load_install_ignore(source, ignore_filename, validation_error)) + + assert not (dest / directory).exists() + assert (dest / "keep.txt").read_text(encoding="utf-8") == "shipped" + assert not (dest / ignore_filename).exists() + + @pytest.mark.parametrize("negate_self", [False, True]) def test_ignore_file_is_always_excluded(tmp_path, ignore_contract, negate_self): ignore_filename, validation_error = ignore_contract From 1bbf6ae5265a80718be3d8023a1a2be3484210ed Mon Sep 17 00:00:00 2001 From: Adam Boczek Date: Wed, 21 Oct 2026 11:03:07 +0200 Subject: [PATCH 04/10] Report malformed installation ignore patterns as validation errors Translate pathspec parsing failures into the caller's domain validation error, retaining the ignore-file path and parser message. Add shared regression coverage for malformed preset and extension patterns without changing installation ordering. Validation: all four new regression cases failed before the fix. Relevant suites passed with 1554 tests and 3 warnings in this checkout's virtual environment. Ruff and git diff --check passed. Assisted-by: Codex (model: GPT-6, autonomous) --- src/specify_cli/_install_ignore.py | 10 ++++++++-- tests/specify_cli/test_install_ignore.py | 14 ++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/_install_ignore.py b/src/specify_cli/_install_ignore.py index 43c7ed7afa..188b0832b1 100644 --- a/src/specify_cli/_install_ignore.py +++ b/src/specify_cli/_install_ignore.py @@ -16,7 +16,8 @@ def load_install_ignore( Match the extension installer's Git-style patterns, including whitespace trimming and backslash normalization. Only the named root ignore file is read; there are no default exclusions except the ignore filename itself. - Invalid encoding raises the caller's domain validation error before copying. + Invalid encoding or patterns raise the caller's domain validation error + before copying. Excluded directories are pruned by copytree, so child negations cannot restore files inside them unless the parent directory is also re-included. """ @@ -48,7 +49,12 @@ def load_install_ignore( # Always ignore the ignore file itself, even after a negation pattern. normalised.append(ignore_filename) - spec = pathspec.GitIgnoreSpec.from_lines(normalised) + try: + spec = pathspec.GitIgnoreSpec.from_lines(normalised) + except ValueError as e: + raise validation_error( + f"Invalid patterns in {ignore_filename}: {ignore_file} ({e})" + ) from e def _ignore(directory: str, entries: List[str]) -> Set[str]: ignored: Set[str] = set() diff --git a/tests/specify_cli/test_install_ignore.py b/tests/specify_cli/test_install_ignore.py index 5ef4215e9c..769d5634f2 100644 --- a/tests/specify_cli/test_install_ignore.py +++ b/tests/specify_cli/test_install_ignore.py @@ -114,6 +114,20 @@ def test_invalid_utf8_uses_callers_validation_error(tmp_path, ignore_contract): assert "byte 3" in str(caught.value) +@pytest.mark.parametrize("patterns", ["!\n", "# comment\n\ntests/\n ! \n"]) +def test_malformed_pattern_uses_callers_validation_error(tmp_path, ignore_contract, patterns): + ignore_filename, validation_error = ignore_contract + ignore_file = tmp_path / ignore_filename + ignore_file.write_text(patterns, encoding="utf-8") + + with pytest.raises(validation_error) as caught: + load_install_ignore(tmp_path, ignore_filename, validation_error) + + assert f"Invalid patterns in {ignore_filename}: {ignore_file}" in str(caught.value) + assert isinstance(caught.value.__cause__, ValueError) + assert str(caught.value.__cause__) in str(caught.value) + + def test_read_failure_is_propagated(tmp_path, ignore_contract, monkeypatch): ignore_filename, validation_error = ignore_contract ignore_file = tmp_path / ignore_filename From fd38648cf6e9550ac94f69065f3480ac230546dd Mon Sep 17 00:00:00 2001 From: Adam Boczek Date: Wed, 21 Oct 2026 13:16:38 +0200 Subject: [PATCH 05/10] Preserve byte offsets in BOM-prefixed ignore file errors Decode as UTF-8 before removing one leading BOM so invalid-encoding errors report file-relative byte offsets. Cover BOM-prefixed and BOM-free invalid input for both presets and extensions. Validation: both new BOM-prefixed cases failed before the fix. Relevant suites passed with 1603 tests and 3 warnings. Ruff and git diff --check passed. Assisted-by: Codex (model: GPT-6, autonomous) --- src/specify_cli/_install_ignore.py | 4 ++-- tests/specify_cli/test_install_ignore.py | 13 ++++++++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/specify_cli/_install_ignore.py b/src/specify_cli/_install_ignore.py index 188b0832b1..a8e4e3312e 100644 --- a/src/specify_cli/_install_ignore.py +++ b/src/specify_cli/_install_ignore.py @@ -26,9 +26,9 @@ def load_install_ignore( return None # Accept UTF-8 with or without the BOM emitted by Windows PowerShell 5.1. - # Otherwise the BOM becomes part of the first pattern and prevents matching. + # Strip it after decoding so error offsets remain relative to the file. try: - raw = ignore_file.read_text(encoding="utf-8-sig") + raw = ignore_file.read_text(encoding="utf-8").removeprefix("\ufeff") except UnicodeDecodeError as e: raise validation_error( f"{ignore_filename} is not valid UTF-8: {ignore_file} " diff --git a/tests/specify_cli/test_install_ignore.py b/tests/specify_cli/test_install_ignore.py index 769d5634f2..bb2b1e3b71 100644 --- a/tests/specify_cli/test_install_ignore.py +++ b/tests/specify_cli/test_install_ignore.py @@ -102,16 +102,23 @@ def test_ignore_file_is_always_excluded(tmp_path, ignore_contract, negate_self): assert sorted(path.name for path in dest.iterdir()) == ["keep.txt"] -def test_invalid_utf8_uses_callers_validation_error(tmp_path, ignore_contract): +@pytest.mark.parametrize( + ("content", "offset"), + [ + pytest.param(b"caf\xe9/\n", 3, id="without-bom"), + pytest.param(b"\xef\xbb\xbfcaf\xe9/\n", 6, id="with-bom"), + ], +) +def test_invalid_utf8_uses_callers_validation_error(tmp_path, ignore_contract, content, offset): ignore_filename, validation_error = ignore_contract ignore_file = tmp_path / ignore_filename - ignore_file.write_bytes(b"caf\xe9/\n") + ignore_file.write_bytes(content) with pytest.raises(validation_error) as caught: load_install_ignore(tmp_path, ignore_filename, validation_error) assert f"{ignore_filename} is not valid UTF-8: {ignore_file}" in str(caught.value) - assert "byte 3" in str(caught.value) + assert f"at byte {offset})" in str(caught.value) @pytest.mark.parametrize("patterns", ["!\n", "# comment\n\ntests/\n ! \n"]) From 8d6d2accae1a5cffefc5163e7c108d819b0d0359 Mon Sep 17 00:00:00 2001 From: Adam Boczek Date: Wed, 21 Oct 2026 13:44:25 +0200 Subject: [PATCH 06/10] Clarify when nested preset ignore files are excluded Document that nested .presetignore files are excluded only when a root .presetignore exists and are otherwise copied unchanged. No code changes; git diff --check passed. Assisted-by: Codex (model: GPT-6, autonomous) --- docs/reference/presets.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/reference/presets.md b/docs/reference/presets.md index c42e878c6a..8dad7ae24c 100644 --- a/docs/reference/presets.md +++ b/docs/reference/presets.md @@ -78,7 +78,9 @@ Patterns follow the same Git-style matching and normalization as the existing extension behavior. Use UTF-8 for non-ASCII names. Only the source root's `.presetignore` is read; nested ignore files do not add -rules. Files named `.presetignore` are themselves excluded, even if negated. +rules. When a root `.presetignore` exists, files named `.presetignore` at any +depth are excluded, even if negated. Otherwise, nested `.presetignore` files +are copied unchanged. Manifest and declared asset files are not exempt from filtering: keep `preset.yml` and any templates, commands, or scripts the preset needs included. Excluding them can leave an incomplete or unusable installation. From d88d9c5656cb6da90bdec59ee79b8106577b5647 Mon Sep 17 00:00:00 2001 From: Adam Boczek Date: Wed, 21 Oct 2026 16:34:37 +0200 Subject: [PATCH 07/10] Report malformed ignore ranges as validation errors Catch regular expression errors from pathspec alongside ValueError, preserving the file-specific validation message. Add reversed-range regression coverage for preset and extension ignore files. Validation: both new reversed-range cases failed before the fix. Relevant suites passed with 1605 tests and 3 warnings. Ruff and git diff --check passed. Assisted-by: Codex (model: GPT-6, autonomous) --- src/specify_cli/_install_ignore.py | 3 ++- tests/specify_cli/test_install_ignore.py | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/specify_cli/_install_ignore.py b/src/specify_cli/_install_ignore.py index a8e4e3312e..0f535827a3 100644 --- a/src/specify_cli/_install_ignore.py +++ b/src/specify_cli/_install_ignore.py @@ -1,5 +1,6 @@ """Shared source-directory installation filtering for extensions and presets.""" +import re from pathlib import Path from typing import Callable, List, Optional, Set @@ -51,7 +52,7 @@ def load_install_ignore( try: spec = pathspec.GitIgnoreSpec.from_lines(normalised) - except ValueError as e: + except (ValueError, re.error) as e: raise validation_error( f"Invalid patterns in {ignore_filename}: {ignore_file} ({e})" ) from e diff --git a/tests/specify_cli/test_install_ignore.py b/tests/specify_cli/test_install_ignore.py index bb2b1e3b71..62065e2214 100644 --- a/tests/specify_cli/test_install_ignore.py +++ b/tests/specify_cli/test_install_ignore.py @@ -1,5 +1,6 @@ """Contract tests for shared installation filtering in specify_cli._install_ignore.""" +import re import shutil from pathlib import Path @@ -121,7 +122,7 @@ def test_invalid_utf8_uses_callers_validation_error(tmp_path, ignore_contract, c assert f"at byte {offset})" in str(caught.value) -@pytest.mark.parametrize("patterns", ["!\n", "# comment\n\ntests/\n ! \n"]) +@pytest.mark.parametrize("patterns", ["!\n", "# comment\n\ntests/\n ! \n", "[z-a].txt\n"]) def test_malformed_pattern_uses_callers_validation_error(tmp_path, ignore_contract, patterns): ignore_filename, validation_error = ignore_contract ignore_file = tmp_path / ignore_filename @@ -131,7 +132,7 @@ def test_malformed_pattern_uses_callers_validation_error(tmp_path, ignore_contra load_install_ignore(tmp_path, ignore_filename, validation_error) assert f"Invalid patterns in {ignore_filename}: {ignore_file}" in str(caught.value) - assert isinstance(caught.value.__cause__, ValueError) + assert isinstance(caught.value.__cause__, (ValueError, re.error)) assert str(caught.value.__cause__) in str(caught.value) From f93ec7127646deb7e5e25c9e5e598885b8a6bb92 Mon Sep 17 00:00:00 2001 From: Adam Boczek Date: Wed, 21 Oct 2026 17:22:29 +0200 Subject: [PATCH 08/10] Clarify preset ignore negation with a selective inclusion example Show how to retain only docs/api.md by excluding docs/*, and explain that re-including the parent directory can restore its other contents. Documentation only; git diff --check passed. Assisted-by: Codex (model: GPT-6, autonomous) --- docs/reference/presets.md | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/docs/reference/presets.md b/docs/reference/presets.md index 8dad7ae24c..c60a14e39e 100644 --- a/docs/reference/presets.md +++ b/docs/reference/presets.md @@ -73,10 +73,21 @@ Patterns follow the same Git-style matching and normalization as pattern other than its trailing slash, anchors the pattern to the source root. Patterns without such a slash match names at any depth. - A later `!` pattern re-includes a match. An excluded directory is not traversed, - so re-including a child requires re-including its parent directory too. + so a child cannot be re-included while its parent remains excluded. - Pattern whitespace is trimmed and backslashes are normalized to `/`, matching the existing extension behavior. Use UTF-8 for non-ASCII names. +To keep only `docs/api.md` within `docs/`, exclude the directory's contents +instead of the directory itself: + +```gitignore +docs/* +!docs/api.md +``` + +Using `docs/`, then `!docs/` and `!docs/api.md`, would re-include all files under +`docs/`, not just `api.md`, unless other patterns exclude them. + Only the source root's `.presetignore` is read; nested ignore files do not add rules. When a root `.presetignore` exists, files named `.presetignore` at any depth are excluded, even if negated. Otherwise, nested `.presetignore` files From bfb4a014d7c899cb32c27fa954c7e76fde96db27 Mon Sep 17 00:00:00 2001 From: Adam Boczek Date: Wed, 21 Oct 2026 18:09:35 +0200 Subject: [PATCH 09/10] Clarify preset ignore validation before removal Document malformed patterns alongside invalid UTF-8 and identify local directories and extracted archives as using pre-removal validation. Preserve the separate preset update exception. Documentation only; git diff --check passed. Assisted-by: Codex (model: GPT-6, autonomous) --- docs/reference/presets.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/reference/presets.md b/docs/reference/presets.md index c60a14e39e..9975578ce6 100644 --- a/docs/reference/presets.md +++ b/docs/reference/presets.md @@ -96,11 +96,12 @@ Manifest and declared asset files are not exempt from filtering: keep `preset.yml` and any templates, commands, or scripts the preset needs included. Excluding them can leave an incomplete or unusable installation. -The source-directory installer reads and validates `.presetignore` before -removing an existing destination, including during a forced reinstall. Invalid -UTF-8 raises a preset validation error. The separate `preset update` command -retains its remove-then-add behavior described below; it does not preflight the -replacement ignore file before removal. +For local directories and extracted archives, the source-directory installer +reads and validates `.presetignore` before removing an existing destination, +including during a forced reinstall. Invalid UTF-8 or malformed patterns raise +a preset validation error. The separate `preset update` command retains its +remove-then-add behavior described below; it does not preflight the replacement +ignore file before removal. ## Update a Preset From f368f80949980e29463eb2926089935faccda898 Mon Sep 17 00:00:00 2001 From: Adam Boczek Date: Wed, 21 Oct 2026 18:34:40 +0200 Subject: [PATCH 10/10] Identify malformed ignore rules by line and normalized pattern Diagnose individual source rules only after compilation fails, retaining the original exception as the cause. Add shared coverage for line numbering, normalization, the first malformed rule, and single compilation of valid files. Validation: eight malformed-input regression cases failed before the fix. Relevant suites passed with 1609 tests and 3 warnings. Ruff and git diff --check passed. Assisted-by: Codex (model: GPT-6, autonomous) --- src/specify_cli/_install_ignore.py | 11 +++++- tests/specify_cli/test_install_ignore.py | 46 ++++++++++++++++++++++-- 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/src/specify_cli/_install_ignore.py b/src/specify_cli/_install_ignore.py index 0f535827a3..68c3bcbcb7 100644 --- a/src/specify_cli/_install_ignore.py +++ b/src/specify_cli/_install_ignore.py @@ -53,8 +53,17 @@ def load_install_ignore( try: spec = pathspec.GitIgnoreSpec.from_lines(normalised) except (ValueError, re.error) as e: + # Locate the first malformed source rule only on failure. Valid files + # compile once; the appended self-exclusion is not a source line. + location = "" + for line_number, pattern in enumerate(normalised[:len(lines)], start=1): + try: + pathspec.GitIgnoreSpec.from_lines([pattern]) + except (ValueError, re.error): + location = f" at line {line_number}: {pattern!r}" + break raise validation_error( - f"Invalid patterns in {ignore_filename}: {ignore_file} ({e})" + f"Invalid patterns in {ignore_filename}: {ignore_file}{location} ({e})" ) from e def _ignore(directory: str, entries: List[str]) -> Set[str]: diff --git a/tests/specify_cli/test_install_ignore.py b/tests/specify_cli/test_install_ignore.py index 62065e2214..82a68b742e 100644 --- a/tests/specify_cli/test_install_ignore.py +++ b/tests/specify_cli/test_install_ignore.py @@ -4,6 +4,7 @@ import shutil from pathlib import Path +import pathspec import pytest from specify_cli._install_ignore import load_install_ignore @@ -122,20 +123,61 @@ def test_invalid_utf8_uses_callers_validation_error(tmp_path, ignore_contract, c assert f"at byte {offset})" in str(caught.value) -@pytest.mark.parametrize("patterns", ["!\n", "# comment\n\ntests/\n ! \n", "[z-a].txt\n"]) -def test_malformed_pattern_uses_callers_validation_error(tmp_path, ignore_contract, patterns): +@pytest.mark.parametrize( + ("patterns", "line_number", "normalized_pattern"), + [ + ("!\n", 1, "!"), + ("# comment\n\ntests/\n ! \n", 4, "!"), + ("[z-a].txt\n", 1, "[z-a].txt"), + ("# comment\n\ntests/\n docs\\[z-a].txt \n!\n", 4, "docs/[z-a].txt"), + ], +) +def test_malformed_pattern_uses_callers_validation_error( + tmp_path, ignore_contract, patterns, line_number, normalized_pattern, monkeypatch +): ignore_filename, validation_error = ignore_contract ignore_file = tmp_path / ignore_filename ignore_file.write_text(patterns, encoding="utf-8") + original_from_lines = pathspec.GitIgnoreSpec.from_lines + failures = [] + + def record_failures(lines): + try: + return original_from_lines(lines) + except (ValueError, re.error) as error: + failures.append(error) + raise + + monkeypatch.setattr(pathspec.GitIgnoreSpec, "from_lines", record_failures) with pytest.raises(validation_error) as caught: load_install_ignore(tmp_path, ignore_filename, validation_error) assert f"Invalid patterns in {ignore_filename}: {ignore_file}" in str(caught.value) + assert f"at line {line_number}: {normalized_pattern!r}" in str(caught.value) + assert caught.value.__cause__ is failures[0] assert isinstance(caught.value.__cause__, (ValueError, re.error)) assert str(caught.value.__cause__) in str(caught.value) +def test_valid_patterns_compile_once(tmp_path, ignore_contract, monkeypatch): + ignore_filename, validation_error = ignore_contract + (tmp_path / ignore_filename).write_text("# comment\n\ndocs/*\n!docs/api.md\n", encoding="utf-8") + original_from_lines = pathspec.GitIgnoreSpec.from_lines + calls = [] + + def record_compilation(lines): + calls.append(list(lines)) + return original_from_lines(calls[-1]) + + monkeypatch.setattr(pathspec.GitIgnoreSpec, "from_lines", record_compilation) + + ignore = load_install_ignore(tmp_path, ignore_filename, validation_error) + + assert len(calls) == 1 + assert ignore(str(tmp_path / "docs"), ["api.md", "draft.md"]) == {"draft.md"} + + def test_read_failure_is_propagated(tmp_path, ignore_contract, monkeypatch): ignore_filename, validation_error = ignore_contract ignore_file = tmp_path / ignore_filename