From 9d094b4911379fe3415de72d7a2e85e9f00f26ea Mon Sep 17 00:00:00 2001 From: Adam Boczek Date: Sun, 18 Oct 2026 11:51:06 +0200 Subject: [PATCH 1/3] 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 2/3] 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 3/3] 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