From be812f37f836501c82fa200647bbd6ed124b790b Mon Sep 17 00:00:00 2001 From: jawwad-ali Date: Mon, 5 Oct 2026 21:35:39 +0500 Subject: [PATCH 1/2] fix(workflows): match overlay file extensions case-insensitively `ProjectOverlaySource.collect` matched `path.suffix` verbatim: if not path.is_file() or path.suffix not in (".yml", ".yaml"): continue so a hand-placed overlay named `.YML` or `.Yaml` was skipped and never applied, with nothing reported to say the file had been ignored. Overlay files are explicitly hand-authored (docs/reference/workflows.md documents the format and tells users to write them), so the casing is the author's choice. Reproduced on main -- three overlays in one directory, differing only in extension case: files on disk: ['Mixed.Yaml', 'UPPER.YML', 'lower.yml'] COLLECTED : ['lower'] SKIPPED : ['mixed', 'upper'] Every other YAML discovery path in the package already lowercases before matching: engine.py:941, _commands.py:1325, :1894, :2128. This brings the overlay loader in line with them. Rebased onto current main (files moved in the workflow/bundler restructure). Co-Authored-By: Claude Opus 5.5 (1M context) --- .../workflows/overlay/layer_sources.py | 7 ++- .../workflows/overlay/operations.py | 6 ++- .../workflows/overlay/test_command_list.py | 52 +++++++++++++++++++ tests/workflows/test_overlay_layer_sources.py | 44 ++++++++++++++++ 4 files changed, 107 insertions(+), 2 deletions(-) diff --git a/src/specify_cli/workflows/overlay/layer_sources.py b/src/specify_cli/workflows/overlay/layer_sources.py index a62cef9340..bbcfcd4ff2 100644 --- a/src/specify_cli/workflows/overlay/layer_sources.py +++ b/src/specify_cli/workflows/overlay/layer_sources.py @@ -147,7 +147,12 @@ def collect(self, workflow_id: str, *, include_disabled: bool = False) -> list[L workflow_overlay_dir, [f"Cannot enumerate overlays: {exc}"] ) from exc for path in entries: - if not path.is_file() or path.suffix not in (".yml", ".yaml"): + # Match the extension case-insensitively. A hand-placed overlay + # named ``.YML`` or ``.Yaml`` was skipped here and never + # applied, with nothing reported to say the file had been ignored. + # Every other YAML discovery path in the package already lowercases + # before matching (``WorkflowEngine``, ``workflow add`` / ``run``). + if not path.is_file() or path.suffix.lower() not in (".yml", ".yaml"): continue if path.is_symlink(): raise OverlayLoadError(path, ["Symlinked overlay files are not allowed"]) diff --git a/src/specify_cli/workflows/overlay/operations.py b/src/specify_cli/workflows/overlay/operations.py index 48b72b7f6d..7c28c93342 100644 --- a/src/specify_cli/workflows/overlay/operations.py +++ b/src/specify_cli/workflows/overlay/operations.py @@ -106,7 +106,11 @@ def _find_overlay_file(project_root: Path, workflow_id: str, overlay_id: str) -> return None matches: list[Path] = [] for path in entries: - if not path.is_file() or path.suffix not in (".yml", ".yaml"): + # Must match ``ProjectOverlaySource.collect`` exactly. With the loader + # lowercasing the suffix and this helper not, a ``.YML`` overlay was + # ACTIVE during resolution yet reported "not found" by overlay + # enable / disable / remove -- applied, but impossible to manage. + if not path.is_file() or path.suffix.lower() not in (".yml", ".yaml"): continue if path.is_symlink(): continue diff --git a/tests/specify_cli/workflows/overlay/test_command_list.py b/tests/specify_cli/workflows/overlay/test_command_list.py index dc1a43ece8..bb1106c6be 100644 --- a/tests/specify_cli/workflows/overlay/test_command_list.py +++ b/tests/specify_cli/workflows/overlay/test_command_list.py @@ -156,6 +156,58 @@ def test_duplicate_manifest_id_is_rejected(self, project_dir, monkeypatch): +class TestOverlayManagementMatchesUppercaseExtension: + """Overlay management finds the same files the resolver loads.""" + + @pytest.mark.parametrize("filename", ["lint.YML", "lint.Yaml"]) + def test_uppercase_extension_overlay_is_manageable( + self, project_dir, monkeypatch, filename + ): + """An overlay the resolver applies must be reachable by enable/disable/remove. + + `ProjectOverlaySource.collect` matches the suffix case-insensitively, so + a `lint.YML` overlay is ACTIVE during resolution. `_find_overlay_file` + matched it case-sensitively, so the same overlay was reported "not + found" by every management command — applied, but impossible to manage. + """ + monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir) + _write_workflow( + project_dir, + "wf", + { + "schema_version": "1.0", + "workflow": {"id": "wf", "name": "WF", "version": "1.0.0"}, + "steps": [{"id": "a", "type": "command", "command": "echo"}], + }, + ) + ov_dir = project_dir / ".specify" / "workflows" / "overlays" / "wf" + ov_dir.mkdir(parents=True, exist_ok=True) + overlay = ov_dir / filename + overlay.write_text( + yaml.safe_dump( + { + "id": "lint", + "extends": "wf", + "priority": 10, + "edits": [{"remove": "a"}], + } + ), + encoding="utf-8", + ) + + result = runner.invoke(app, ["workflow", "overlay", "disable", "wf", "lint"]) + assert result.exit_code == 0, result.output + assert yaml.safe_load(overlay.read_text(encoding="utf-8"))["enabled"] is False + + result = runner.invoke(app, ["workflow", "overlay", "enable", "wf", "lint"]) + assert result.exit_code == 0, result.output + assert yaml.safe_load(overlay.read_text(encoding="utf-8"))["enabled"] is True + + result = runner.invoke(app, ["workflow", "overlay", "remove", "wf", "lint"]) + assert result.exit_code == 0, result.output + assert not overlay.exists() + + class TestOverlayPathTraversal: """Overlay CLI must stay inside the overlay directory.""" diff --git a/tests/workflows/test_overlay_layer_sources.py b/tests/workflows/test_overlay_layer_sources.py index 172115df05..4907621e4e 100644 --- a/tests/workflows/test_overlay_layer_sources.py +++ b/tests/workflows/test_overlay_layer_sources.py @@ -85,6 +85,50 @@ def test_empty_document_still_reports_missing_fields( ) +class TestProjectOverlaySourceExtensionMatching: + """Overlay file extensions are matched case-insensitively.""" + + @pytest.mark.parametrize( + "filename", ["upper.YML", "mixed.Yaml", "shouty.YAML", "title.Yml"] + ) + def test_uppercase_extension_is_collected( + self, project_dir: Path, filename: str + ) -> None: + """A hand-placed `.YML` must not be silently ignored. + + `collect` matched `path.suffix` verbatim against `(".yml", ".yaml")`, + so an overlay whose extension differed only in case was skipped with + nothing reported — the author's overlay simply never applied. Every + other YAML discovery path in the package lowercases before matching. + """ + ov_dir = project_dir / ".specify" / "workflows" / "overlays" / "wf" + ov_dir.mkdir(parents=True, exist_ok=True) + (ov_dir / filename).write_text( + yaml.safe_dump( + { + "id": "lint", + "extends": "wf", + "priority": 10, + "edits": [{"remove": "a"}], + } + ), + encoding="utf-8", + ) + + layers = ProjectOverlaySource(project_dir).collect("wf") + + assert [layer.content.id for layer in layers] == ["lint"] + + def test_non_yaml_extensions_are_still_skipped(self, project_dir: Path) -> None: + """Broadening case must not broaden which extensions are accepted.""" + ov_dir = project_dir / ".specify" / "workflows" / "overlays" / "wf" + ov_dir.mkdir(parents=True, exist_ok=True) + for name in ("notes.txt", "backup.yml.bak", "README.md", "data.json"): + (ov_dir / name).write_text("id: lint\n", encoding="utf-8") + + assert ProjectOverlaySource(project_dir).collect("wf") == [] + + class TestProjectOverlaySourceFileReadErrors: """File-read errors must be wrapped in OverlayLoadError, not leaked as raw tracebacks.""" From 324dfef01ff66c076c319826c73f7cc88d5295ac Mon Sep 17 00:00:00 2001 From: jawwad-ali Date: Thu, 8 Oct 2026 00:24:47 +0500 Subject: [PATCH 2/2] test(workflows): cover each overlay command in its own mirrored suite The `_find_overlay_file` regression test drove `disable`, `enable` and `remove` from `test_command_list.py`, contrary to the mirrored test layout in design/cli.md ("Command-focused tests mirror the source command surface"). Split it into one focused test per command, in `test_command_disable.py`, `test_command_enable.py` and `test_command_remove.py`, each parametrized over `lint.YML` and `lint.Yaml`. `test_command_list.py` is back to its `main` content. Test-only change; all six new cases fail with the source reverted. Co-Authored-By: Claude Opus 5.5 --- .../workflows/overlay/test_command_disable.py | 45 ++++++++++++++++ .../workflows/overlay/test_command_enable.py | 48 ++++++++++++++++- .../workflows/overlay/test_command_list.py | 52 ------------------- .../workflows/overlay/test_command_remove.py | 45 ++++++++++++++++ 4 files changed, 137 insertions(+), 53 deletions(-) diff --git a/tests/specify_cli/workflows/overlay/test_command_disable.py b/tests/specify_cli/workflows/overlay/test_command_disable.py index 6ab9c72e07..84c0068845 100644 --- a/tests/specify_cli/workflows/overlay/test_command_disable.py +++ b/tests/specify_cli/workflows/overlay/test_command_disable.py @@ -4,6 +4,7 @@ from pathlib import Path +import pytest import yaml from typer.testing import CliRunner @@ -163,3 +164,47 @@ def test_enable_disable_with_mismatched_filename(self, project_dir, monkeypatch) ) ) assert data["enabled"] is True + + +class TestOverlayUppercaseExtension: + """``disable`` must find every overlay the resolver applies. + + ``ProjectOverlaySource.collect`` matches ``.yml``/``.yaml`` case-insensitively, + so a ``lint.YML`` overlay is ACTIVE during resolution. ``_find_overlay_file`` + matched the suffix case-sensitively, so ``disable`` reported that same overlay + "not found" -- applied, but impossible to switch off. + """ + + @pytest.mark.parametrize("filename", ["lint.YML", "lint.Yaml"]) + def test_disable_finds_uppercase_extension_overlay( + self, project_dir, monkeypatch, filename + ): + monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir) + _write_workflow( + project_dir, + "wf", + { + "schema_version": "1.0", + "workflow": {"id": "wf", "name": "WF", "version": "1.0.0"}, + "steps": [{"id": "a", "type": "command", "command": "echo"}], + }, + ) + ov_dir = project_dir / ".specify" / "workflows" / "overlays" / "wf" + ov_dir.mkdir(parents=True, exist_ok=True) + overlay = ov_dir / filename + overlay.write_text( + yaml.safe_dump( + { + "id": "lint", + "extends": "wf", + "priority": 10, + "edits": [{"remove": "a"}], + } + ), + encoding="utf-8", + ) + + result = runner.invoke(app, ["workflow", "overlay", "disable", "wf", "lint"]) + + assert result.exit_code == 0, result.output + assert yaml.safe_load(overlay.read_text(encoding="utf-8"))["enabled"] is False diff --git a/tests/specify_cli/workflows/overlay/test_command_enable.py b/tests/specify_cli/workflows/overlay/test_command_enable.py index e764b23de3..879512876c 100644 --- a/tests/specify_cli/workflows/overlay/test_command_enable.py +++ b/tests/specify_cli/workflows/overlay/test_command_enable.py @@ -2,7 +2,8 @@ from __future__ import annotations - +import pytest +import yaml from typer.testing import CliRunner from specify_cli import app @@ -30,3 +31,48 @@ def test_overlay_enable_rejects_traversal(self, project_dir, monkeypatch): result = runner.invoke(app, ["workflow", "overlay", "enable", "wf", "../other"]) assert result.exit_code != 0, result.output assert "invalid" in result.output.lower() or "traversal" in result.output.lower() + + +class TestOverlayUppercaseExtension: + """``enable`` must find every overlay the resolver applies. + + ``ProjectOverlaySource.collect`` matches ``.yml``/``.yaml`` case-insensitively, + so a ``lint.YML`` overlay is ACTIVE during resolution. ``_find_overlay_file`` + matched the suffix case-sensitively, so ``enable`` reported that same overlay + "not found" -- disabled, and impossible to switch back on. + """ + + @pytest.mark.parametrize("filename", ["lint.YML", "lint.Yaml"]) + def test_enable_finds_uppercase_extension_overlay( + self, project_dir, monkeypatch, filename + ): + monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir) + _write_workflow( + project_dir, + "wf", + { + "schema_version": "1.0", + "workflow": {"id": "wf", "name": "WF", "version": "1.0.0"}, + "steps": [{"id": "a", "type": "command", "command": "echo"}], + }, + ) + ov_dir = project_dir / ".specify" / "workflows" / "overlays" / "wf" + ov_dir.mkdir(parents=True, exist_ok=True) + overlay = ov_dir / filename + overlay.write_text( + yaml.safe_dump( + { + "id": "lint", + "extends": "wf", + "priority": 10, + "enabled": False, + "edits": [{"remove": "a"}], + } + ), + encoding="utf-8", + ) + + result = runner.invoke(app, ["workflow", "overlay", "enable", "wf", "lint"]) + + assert result.exit_code == 0, result.output + assert yaml.safe_load(overlay.read_text(encoding="utf-8"))["enabled"] is True diff --git a/tests/specify_cli/workflows/overlay/test_command_list.py b/tests/specify_cli/workflows/overlay/test_command_list.py index bb1106c6be..dc1a43ece8 100644 --- a/tests/specify_cli/workflows/overlay/test_command_list.py +++ b/tests/specify_cli/workflows/overlay/test_command_list.py @@ -156,58 +156,6 @@ def test_duplicate_manifest_id_is_rejected(self, project_dir, monkeypatch): -class TestOverlayManagementMatchesUppercaseExtension: - """Overlay management finds the same files the resolver loads.""" - - @pytest.mark.parametrize("filename", ["lint.YML", "lint.Yaml"]) - def test_uppercase_extension_overlay_is_manageable( - self, project_dir, monkeypatch, filename - ): - """An overlay the resolver applies must be reachable by enable/disable/remove. - - `ProjectOverlaySource.collect` matches the suffix case-insensitively, so - a `lint.YML` overlay is ACTIVE during resolution. `_find_overlay_file` - matched it case-sensitively, so the same overlay was reported "not - found" by every management command — applied, but impossible to manage. - """ - monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir) - _write_workflow( - project_dir, - "wf", - { - "schema_version": "1.0", - "workflow": {"id": "wf", "name": "WF", "version": "1.0.0"}, - "steps": [{"id": "a", "type": "command", "command": "echo"}], - }, - ) - ov_dir = project_dir / ".specify" / "workflows" / "overlays" / "wf" - ov_dir.mkdir(parents=True, exist_ok=True) - overlay = ov_dir / filename - overlay.write_text( - yaml.safe_dump( - { - "id": "lint", - "extends": "wf", - "priority": 10, - "edits": [{"remove": "a"}], - } - ), - encoding="utf-8", - ) - - result = runner.invoke(app, ["workflow", "overlay", "disable", "wf", "lint"]) - assert result.exit_code == 0, result.output - assert yaml.safe_load(overlay.read_text(encoding="utf-8"))["enabled"] is False - - result = runner.invoke(app, ["workflow", "overlay", "enable", "wf", "lint"]) - assert result.exit_code == 0, result.output - assert yaml.safe_load(overlay.read_text(encoding="utf-8"))["enabled"] is True - - result = runner.invoke(app, ["workflow", "overlay", "remove", "wf", "lint"]) - assert result.exit_code == 0, result.output - assert not overlay.exists() - - class TestOverlayPathTraversal: """Overlay CLI must stay inside the overlay directory.""" diff --git a/tests/specify_cli/workflows/overlay/test_command_remove.py b/tests/specify_cli/workflows/overlay/test_command_remove.py index 561f814479..dc6c4272ef 100644 --- a/tests/specify_cli/workflows/overlay/test_command_remove.py +++ b/tests/specify_cli/workflows/overlay/test_command_remove.py @@ -4,6 +4,7 @@ from pathlib import Path +import pytest import yaml from typer.testing import CliRunner @@ -170,3 +171,47 @@ def test_overlay_remove_rejects_symlink(self, project_dir, monkeypatch): assert result.exit_code != 0, result.output assert real_file.is_file() assert "symlink" in result.output.lower() or "Invalid" in result.output + + +class TestOverlayUppercaseExtension: + """``remove`` must find every overlay the resolver applies. + + ``ProjectOverlaySource.collect`` matches ``.yml``/``.yaml`` case-insensitively, + so a ``lint.YML`` overlay is ACTIVE during resolution. ``_find_overlay_file`` + matched the suffix case-sensitively, so ``remove`` reported that same overlay + "not found" -- applied, but impossible to uninstall. + """ + + @pytest.mark.parametrize("filename", ["lint.YML", "lint.Yaml"]) + def test_remove_finds_uppercase_extension_overlay( + self, project_dir, monkeypatch, filename + ): + monkeypatch.setattr("specify_cli._require_specify_project", lambda: project_dir) + _write_workflow( + project_dir, + "wf", + { + "schema_version": "1.0", + "workflow": {"id": "wf", "name": "WF", "version": "1.0.0"}, + "steps": [{"id": "a", "type": "command", "command": "echo"}], + }, + ) + ov_dir = project_dir / ".specify" / "workflows" / "overlays" / "wf" + ov_dir.mkdir(parents=True, exist_ok=True) + overlay = ov_dir / filename + overlay.write_text( + yaml.safe_dump( + { + "id": "lint", + "extends": "wf", + "priority": 10, + "edits": [{"remove": "a"}], + } + ), + encoding="utf-8", + ) + + result = runner.invoke(app, ["workflow", "overlay", "remove", "wf", "lint"]) + + assert result.exit_code == 0, result.output + assert not overlay.exists()