From 3562aac520bb590cda93f5bd4c139c07f1220d74 Mon Sep 17 00:00:00 2001 From: dhruv-15-03 Date: Tue, 30 Jun 2026 23:17:33 +0530 Subject: [PATCH] fix(extensions): restore core-command discovery broken by module move `_load_core_command_names()` resolved its command dirs with bespoke Path(__file__) math that was correct only while the code lived at src/specify_cli/extensions.py. The #3014 refactor moved it into the specify_cli/extensions/ package one directory deeper without updating the parent counts, so both candidate dirs pointed at non-existent paths and discovery always fell through to _FALLBACK_CORE_COMMAND_NAMES. Delegate path resolution to the canonical _assets resolvers (_locate_core_pack / _repo_root), mirroring presets/__init__.py. They are anchored to the package root, so discovery is immune to future module moves. Add regression tests that pin live discovery across the wheel/source/fallback branches; they fail on the pre-fix code. Fixes #3274 Assisted-by: GitHub Copilot (model: Claude Opus 4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/extensions/__init__.py | 19 ++++- tests/test_extensions.py | 102 +++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 4 deletions(-) diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index f219c7e8d8..7e6d9f768a 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -26,6 +26,7 @@ from packaging import version as pkg_version from packaging.specifiers import InvalidSpecifier, SpecifierSet +from .._assets import _locate_core_pack, _repo_root from .._init_options import is_ai_skills_enabled from .._invocation_style import is_dollar_skills_agent, is_slash_skills_agent from .._utils import dump_frontmatter, relative_extension_path_violation, version_satisfies @@ -62,11 +63,21 @@ def _load_core_command_names() -> frozenset[str]: Prefer the wheel-time ``core_pack`` bundle when present, and fall back to the source checkout when running from the repository. If neither is available, use the baked-in fallback set so validation still works. + + Path resolution is delegated to the canonical ``_assets`` helpers + (``_locate_core_pack`` / ``_repo_root``), which are anchored to the package + root rather than this module's location. The previous bespoke + ``Path(__file__)`` math was correct only while this code lived at + ``specify_cli/extensions.py``; once it moved into the + ``specify_cli/extensions/`` package the relative parents fell one level + short, so discovery always missed the real command dirs and silently fell + through to ``_FALLBACK_CORE_COMMAND_NAMES`` below. """ - candidate_dirs = [ - Path(__file__).parent / "core_pack" / "commands", - Path(__file__).resolve().parent.parent.parent / "templates" / "commands", - ] + candidate_dirs: list[Path] = [] + core_pack = _locate_core_pack() + if core_pack is not None: + candidate_dirs.append(core_pack / "commands") + candidate_dirs.append(_repo_root() / "templates" / "commands") for commands_dir in candidate_dirs: if not commands_dir.is_dir(): diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 4a07a21053..3c83552d41 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -205,6 +205,108 @@ def test_boolean_returns_default(self): assert normalize_priority(True, default=5) == 5 +# ===== Core Command Discovery Tests ===== + + +class TestCoreCommandDiscovery: + """Regression tests for dynamic core-command discovery. + + ``_load_core_command_names()`` must read the bundled command templates via + the canonical ``_assets`` resolvers. A prior off-by-one in this module's + bespoke ``Path(__file__)`` math (introduced when the code moved from + ``specify_cli/extensions.py`` into the ``specify_cli/extensions/`` package) + made discovery always miss the real dirs and silently return the hardcoded + ``_FALLBACK_CORE_COMMAND_NAMES``. These tests pin the live-discovery + behaviour so the regression cannot return unnoticed. + """ + + def test_discovery_returns_real_bundled_set_not_fallback(self, monkeypatch): + """Discovery must read the real bundled templates, not echo the fallback. + + This is the core regression guard. ``_FALLBACK_CORE_COMMAND_NAMES`` + currently equals the real command set, so a totally dead discovery path + looks healthy from the outside. We temporarily swap the fallback for a + sentinel: live discovery still returns the real bundled names, whereas + the pre-fix off-by-one (which always fell through to the fallback) would + return the sentinel and fail this assertion. + """ + import specify_cli.extensions as ext + + commands_dir = Path(__file__).resolve().parent.parent / "templates" / "commands" + bundled = frozenset( + command_file.stem + for command_file in commands_dir.iterdir() + if command_file.is_file() and command_file.suffix == ".md" + ) + sentinel = frozenset({"__sentinel_fallback_must_not_be_used__"}) + monkeypatch.setattr(ext, "_FALLBACK_CORE_COMMAND_NAMES", sentinel) + + result = ext._load_core_command_names() + + assert result == bundled + assert result != sentinel + + def test_discovers_commands_from_source_tree(self, tmp_path, monkeypatch): + """Source checkout: read ``/templates/commands`` via ``_repo_root``.""" + import specify_cli.extensions as ext + + commands_dir = tmp_path / "templates" / "commands" + commands_dir.mkdir(parents=True) + for stem in ("alpha", "beta", "gamma"): + (commands_dir / f"{stem}.md").write_text("---\n---\n", encoding="utf-8") + # Non-markdown files and subdirectories must be ignored. + (commands_dir / "README.txt").write_text("ignore me", encoding="utf-8") + (commands_dir / "nested").mkdir() + + monkeypatch.setattr(ext, "_locate_core_pack", lambda: None) + monkeypatch.setattr(ext, "_repo_root", lambda: tmp_path) + + # A set distinct from _FALLBACK_CORE_COMMAND_NAMES proves discovery is + # live rather than masked by the fallback (which equals the real set). + assert ext._load_core_command_names() == frozenset({"alpha", "beta", "gamma"}) + + def test_prefers_wheel_core_pack(self, tmp_path, monkeypatch): + """Wheel install: read ``/commands`` when ``_locate_core_pack`` resolves.""" + import specify_cli.extensions as ext + + core_pack = tmp_path / "core_pack" + commands_dir = core_pack / "commands" + commands_dir.mkdir(parents=True) + (commands_dir / "wheelonly.md").write_text("---\n---\n", encoding="utf-8") + + monkeypatch.setattr(ext, "_locate_core_pack", lambda: core_pack) + # Repo-root path intentionally absent so only the core_pack branch can match. + monkeypatch.setattr(ext, "_repo_root", lambda: tmp_path / "missing") + + assert ext._load_core_command_names() == frozenset({"wheelonly"}) + + def test_falls_back_when_no_command_dir_exists(self, tmp_path, monkeypatch): + """With neither dir present, the baked-in fallback set is returned.""" + import specify_cli.extensions as ext + + monkeypatch.setattr(ext, "_locate_core_pack", lambda: None) + monkeypatch.setattr(ext, "_repo_root", lambda: tmp_path / "missing") + + assert ext._load_core_command_names() == ext._FALLBACK_CORE_COMMAND_NAMES + + def test_fallback_matches_bundled_commands(self): + """The safety-net fallback must stay in lockstep with the bundled templates. + + Discovery now auto-syncs, but the hardcoded fallback remains the last + resort and should not rot if a core command is added or removed. + """ + import specify_cli.extensions as ext + + commands_dir = Path(__file__).resolve().parent.parent / "templates" / "commands" + bundled = frozenset( + command_file.stem + for command_file in commands_dir.iterdir() + if command_file.is_file() and command_file.suffix == ".md" + ) + + assert ext._FALLBACK_CORE_COMMAND_NAMES == bundled + + # ===== ExtensionManifest Tests ===== class TestExtensionManifest: