Skip to content

Commit e3bb151

Browse files
fix(extensions): resolve core-command dirs via _assets helpers (#3274)
`_load_core_command_names()` computed its candidate command dirs with bespoke `Path(__file__)` arithmetic. The #3014 move of this module from `specify_cli/extensions.py` to `specify_cli/extensions/__init__.py` pushed the file one directory deeper but left the `.parent` counts unchanged, so both candidates resolved to non-existent paths: wheel -> specify_cli/extensions/core_pack/commands (real: specify_cli/core_pack/commands) source -> src/templates/commands (real: repo-root templates/commands) Neither exists, so every call silently fell through to `_FALLBACK_CORE_COMMAND_NAMES`. Discovery is latent-dead: the fallback happens to equal the real stems today, but the shadowing guard (#1994) that depends on it now relies on someone hand-editing the fallback on every core-command add/remove (as already happened for `converge`, #3001). Delegate path resolution to the canonical `_locate_core_pack` / `_repo_root` resolvers in `_assets` — the same ones the presets and bundle loaders use. They are anchored to the package root, so discovery survives future module moves. Add regression tests that point the resolvers at a temp tree with *different* command names, proving discovery reads from disk rather than returning the fallback (they fail on the pre-fix code). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent d65f6bd commit e3bb151

2 files changed

Lines changed: 85 additions & 3 deletions

File tree

src/specify_cli/extensions/__init__.py

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
from packaging import version as pkg_version
2727
from packaging.specifiers import InvalidSpecifier, SpecifierSet
2828

29+
from .._assets import _locate_core_pack, _repo_root
2930
from .._init_options import is_ai_skills_enabled
3031
from .._invocation_style import is_dollar_skills_agent, is_slash_skills_agent
3132
from .._utils import dump_frontmatter, relative_extension_path_violation
@@ -62,14 +63,28 @@ def _load_core_command_names() -> frozenset[str]:
6263
Prefer the wheel-time ``core_pack`` bundle when present, and fall back to
6364
the source checkout when running from the repository. If neither is
6465
available, use the baked-in fallback set so validation still works.
66+
67+
Path resolution is delegated to the canonical ``_assets`` resolvers
68+
(``_locate_core_pack`` / ``_repo_root``) — the same ones the presets and
69+
bundle loaders use — rather than bespoke ``Path(__file__)`` arithmetic.
70+
Hand-counted ``.parent`` chains silently broke discovery once already: the
71+
#3014 move of this module from ``specify_cli/extensions.py`` to
72+
``specify_cli/extensions/__init__.py`` pushed the file one directory deeper
73+
without updating the counts, so both candidates resolved to non-existent
74+
paths and every call fell through to the fallback (#3274). The shared
75+
resolvers are anchored to the package root, so discovery survives future
76+
module moves.
6577
"""
78+
core_pack = _locate_core_pack()
6679
candidate_dirs = [
67-
Path(__file__).parent / "core_pack" / "commands",
68-
Path(__file__).resolve().parent.parent.parent / "templates" / "commands",
80+
# Wheel install: force-include maps templates/commands → core_pack/commands.
81+
core_pack / "commands" if core_pack is not None else None,
82+
# Source checkout / editable install: repo-root templates/commands.
83+
_repo_root() / "templates" / "commands",
6984
]
7085

7186
for commands_dir in candidate_dirs:
72-
if not commands_dir.is_dir():
87+
if commands_dir is None or not commands_dir.is_dir():
7388
continue
7489

7590
command_names = {

tests/test_extensions.py

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,73 @@ def test_core_command_names_match_bundled_templates(self):
227227

228228
assert CORE_COMMAND_NAMES == expected
229229

230+
def test_load_core_command_names_discovers_from_source_checkout(self, monkeypatch):
231+
"""Discovery must actually read the repo-root templates, not silently
232+
fall back (#3274).
233+
234+
The fallback set happens to equal the real command stems today, so an
235+
equality check against the live tree cannot tell a working loader apart
236+
from a dead one. Point ``_repo_root`` at a temp tree with *different*
237+
command names: the old off-by-one path math read nothing and returned
238+
the baked-in fallback; the fixed loader returns the temp stems.
239+
"""
240+
from specify_cli.extensions import (
241+
_load_core_command_names,
242+
_FALLBACK_CORE_COMMAND_NAMES,
243+
)
244+
import specify_cli.extensions as ext
245+
246+
with tempfile.TemporaryDirectory() as tmp:
247+
commands = Path(tmp) / "templates" / "commands"
248+
commands.mkdir(parents=True)
249+
(commands / "widget.md").write_text("# widget", encoding="utf-8")
250+
(commands / "gadget.md").write_text("# gadget", encoding="utf-8")
251+
(commands / "notacommand.txt").write_text("skip me", encoding="utf-8")
252+
253+
# No wheel bundle in this scenario; force the source-checkout path.
254+
monkeypatch.setattr(ext, "_locate_core_pack", lambda: None)
255+
monkeypatch.setattr(ext, "_repo_root", lambda: Path(tmp))
256+
257+
result = _load_core_command_names()
258+
259+
assert result == {"widget", "gadget"}
260+
assert result != _FALLBACK_CORE_COMMAND_NAMES
261+
262+
def test_load_core_command_names_prefers_wheel_core_pack(self, monkeypatch):
263+
"""When a wheel ``core_pack`` bundle exists, discovery reads
264+
``core_pack/commands`` (the force-include target) ahead of the source
265+
tree (#3274)."""
266+
from specify_cli.extensions import _load_core_command_names
267+
import specify_cli.extensions as ext
268+
269+
with tempfile.TemporaryDirectory() as tmp:
270+
core_pack = Path(tmp) / "core_pack"
271+
(core_pack / "commands").mkdir(parents=True)
272+
(core_pack / "commands" / "sprocket.md").write_text("# sprocket", encoding="utf-8")
273+
274+
monkeypatch.setattr(ext, "_locate_core_pack", lambda: core_pack)
275+
# Source fallback should be ignored while the bundle resolves.
276+
monkeypatch.setattr(ext, "_repo_root", lambda: Path(tmp) / "nonexistent")
277+
278+
result = _load_core_command_names()
279+
280+
assert result == {"sprocket"}
281+
282+
def test_load_core_command_names_falls_back_when_nothing_found(self, monkeypatch):
283+
"""With neither a bundle nor a source tree, discovery returns the
284+
baked-in fallback so validation still works (#3274)."""
285+
from specify_cli.extensions import (
286+
_load_core_command_names,
287+
_FALLBACK_CORE_COMMAND_NAMES,
288+
)
289+
import specify_cli.extensions as ext
290+
291+
with tempfile.TemporaryDirectory() as tmp:
292+
monkeypatch.setattr(ext, "_locate_core_pack", lambda: None)
293+
monkeypatch.setattr(ext, "_repo_root", lambda: Path(tmp) / "nonexistent")
294+
295+
assert _load_core_command_names() == _FALLBACK_CORE_COMMAND_NAMES
296+
230297
def test_missing_required_field(self, temp_dir):
231298
"""Test manifest missing required field."""
232299
import yaml

0 commit comments

Comments
 (0)