Skip to content

Commit 21fb1bb

Browse files
jawwad-aliclaude
andauthored
fix(bundler): resolve built-in step types when checking bundle component references (#3885)
* fix(bundler): resolve built-in step types when checking bundle references `_resolved_locally` gives three of the four component kinds a "is it bundled with Spec Kit?" check before the installed-in-project one: presets -> _locate_bundled_preset or PresetManager.get_pack extensions -> _locate_bundled_extension or ExtensionManager...is_installed workflows -> _locate_bundled_workflow or WorkflowRegistry.is_installed steps -> StepRegistry.is_installed <-- no bundled check `StepRegistry` tracks *community* step types installed under `.specify/workflows/steps/`. Spec Kit ships 11 step types as built-ins registered in `STEP_REGISTRY`, so every one of them looked unresolved: steps/shell -> False steps/gate -> False steps/command -> False steps/if -> False A bundle declaring a dependency on any built-in step type was therefore reported as an unresolved reference — an error online, a warning offline. There is no `_locate_bundled_step` to mirror, because step types are not an on-disk asset directory; `STEP_REGISTRY` is the equivalent check, and is what `specify workflow step info` reports as "built-in". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(bundler): check an immutable built-in step set, not the mutable registry Review catch: `STEP_REGISTRY` is not limited to bundled steps. `load_custom_steps` adds project-installed ids to that process-global mapping and never removes them, so in a long-lived process a community step loaded while working on project A would be accepted as "bundled" when validating a bundle for project B — before B's own StepRegistry is consulted. Snapshot the shipped ids into `BUILTIN_STEP_TYPES` immediately after `_register_builtin_steps()`, before `load_custom_steps` is even defined, and check that frozenset instead. Verified: with the check on STEP_REGISTRY the new cross-project test fails (a leaked community id resolves as bundled); with BUILTIN_STEP_TYPES it passes. 1 failed, 5 passed -> 6 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent bf88c9f commit 21fb1bb

3 files changed

Lines changed: 92 additions & 0 deletions

File tree

src/specify_cli/bundler/services/references.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,21 @@ def _resolved_locally(root: Path, component: ComponentRef) -> bool:
4040
return True
4141
return WorkflowRegistry(root).is_installed(component.id)
4242
if kind == "steps":
43+
from ...workflows import BUILTIN_STEP_TYPES
4344
from ...workflows.catalog import StepRegistry
4445

46+
# Step types ship with Spec Kit as built-ins (shell, gate, if, ...)
47+
# rather than as an on-disk asset directory, so there is no
48+
# ``_locate_bundled_step`` to mirror the three lookups above.
49+
# ``BUILTIN_STEP_TYPES`` is the bundled-with-Spec-Kit check for this
50+
# kind. Deliberately NOT ``STEP_REGISTRY``: ``load_custom_steps``
51+
# adds project-installed ids to that process-global mapping and
52+
# never removes them, so in a long-lived process a community step
53+
# loaded for one project would be accepted as "bundled" when
54+
# validating another. Without any bundled check at all, every
55+
# built-in step type looked unresolved.
56+
if component.id in BUILTIN_STEP_TYPES:
57+
return True
4558
return StepRegistry(root).is_installed(component.id)
4659
except Exception: # noqa: BLE001 - resolution is best-effort
4760
return False

src/specify_cli/workflows/__init__.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,14 @@ def _register_builtin_steps() -> None:
7171

7272
_register_builtin_steps()
7373

74+
# The step types Spec Kit ships, snapshotted before any community step can be
75+
# loaded. ``load_custom_steps`` adds project-installed ids to the process-global
76+
# ``STEP_REGISTRY`` and never removes them, so ``STEP_REGISTRY`` cannot answer
77+
# "is this bundled with Spec Kit?" in a long-lived process: a step loaded for one
78+
# project would look built-in for the next. Callers that need the immutable set
79+
# (e.g. the bundler's reference checker) must use this instead.
80+
BUILTIN_STEP_TYPES: frozenset[str] = frozenset(STEP_REGISTRY)
81+
7482

7583
def load_custom_steps(project_root: Path) -> list[str]:
7684
"""Load community-installed custom step types into STEP_REGISTRY.

tests/unit/test_bundler_references.py

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,77 @@ def test_bundled_extension_resolves(tmp_path: Path):
2424
assert warnings == []
2525

2626

27+
def test_builtin_step_type_resolves(tmp_path: Path):
28+
"""A built-in step type must resolve, like a bundled extension.
29+
30+
Spec Kit ships 11 step types as built-ins registered in ``STEP_REGISTRY``
31+
rather than as on-disk asset directories, so there is no
32+
``_locate_bundled_step``. The ``steps`` branch of ``_resolved_locally`` only
33+
asked ``StepRegistry(root).is_installed()``, which tracks *community* step
34+
types installed under ``.specify/workflows/steps/`` — so every built-in step
35+
type was reported as an unresolved reference.
36+
"""
37+
from specify_cli.workflows import BUILTIN_STEP_TYPES
38+
39+
root = make_project(tmp_path)
40+
warnings: list[str] = []
41+
check = make_reference_checker(root, allow_network=True, warnings=warnings)
42+
43+
for step_id in ("shell", "gate", "command", "if"):
44+
assert step_id in BUILTIN_STEP_TYPES, step_id
45+
assert check(_ref("steps", step_id)) is None, step_id
46+
assert warnings == []
47+
48+
49+
def test_community_step_is_not_treated_as_bundled(tmp_path: Path):
50+
"""A community step loaded for one project must not resolve for another.
51+
52+
`load_custom_steps` adds project-installed ids to the process-global
53+
`STEP_REGISTRY` and never removes them, so checking `STEP_REGISTRY` here
54+
would accept project A's community step as "bundled" while validating
55+
project B. `BUILTIN_STEP_TYPES` is snapshotted before any custom step can
56+
load, which is why the check uses it instead.
57+
"""
58+
from specify_cli.workflows import (
59+
BUILTIN_STEP_TYPES,
60+
STEP_REGISTRY,
61+
_register_step,
62+
)
63+
from specify_cli.workflows.base import StepBase, StepResult, StepStatus
64+
65+
class _CommunityStep(StepBase):
66+
type_key = "community-only-step"
67+
68+
def execute(self, config, context): # pragma: no cover - never run
69+
return StepResult(status=StepStatus.COMPLETED)
70+
71+
# Simulate project A having loaded a community step into the global registry.
72+
_register_step(_CommunityStep())
73+
try:
74+
assert "community-only-step" in STEP_REGISTRY
75+
assert "community-only-step" not in BUILTIN_STEP_TYPES
76+
77+
# Project B does not have it installed, so it must NOT resolve locally.
78+
root = make_project(tmp_path)
79+
warnings: list[str] = []
80+
check = make_reference_checker(root, allow_network=True, warnings=warnings)
81+
problem = check(_ref("steps", "community-only-step"))
82+
assert problem is not None, "leaked community step resolved as bundled"
83+
assert "community-only-step" in problem
84+
finally:
85+
STEP_REGISTRY.pop("community-only-step", None)
86+
87+
88+
def test_unknown_step_type_still_errors_online(tmp_path: Path):
89+
"""The guard must not make every step id resolve."""
90+
root = make_project(tmp_path)
91+
warnings: list[str] = []
92+
check = make_reference_checker(root, allow_network=True, warnings=warnings)
93+
problem = check(_ref("steps", "no-such-step-type"))
94+
assert problem is not None
95+
assert "no-such-step-type" in problem
96+
97+
2798
def test_unknown_reference_errors_online(tmp_path: Path):
2899
root = make_project(tmp_path)
29100
warnings: list[str] = []

0 commit comments

Comments
 (0)