Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 69 additions & 71 deletions src/specify_cli/presets/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -801,25 +801,6 @@ def check_compatibility(

return True

def _extension_installed_for_command(self, command_name: str) -> bool:
"""Whether *command_name* may be materialized in this project.

Extension command overrides follow ``speckit.<ext-id>.<cmd-name>``;
they must be skipped everywhere preset artifacts are written —
registration *and* reconciliation — when the extension isn't
installed, or reconciliation would materialize files that
registration refused to track. Core commands (single-dot names,
e.g. ``speckit.specify``) always pass.
"""
parts = command_name.split(".")
if len(parts) >= 3 and parts[0] == "speckit":
ext_id = parts[1]
if not (
self.project_root / ".specify" / "extensions" / ext_id
).is_dir():
return False
return True

def _register_commands(
self,
manifest: PresetManifest,
Expand Down Expand Up @@ -848,21 +829,20 @@ def _register_commands(
if not command_templates:
return {}

# Filter out extension command overrides if the extension isn't installed.
filtered = [
cmd
for cmd in command_templates
if self._extension_installed_for_command(cmd["name"])
]

if not filtered:
return {}

# A preset command template always ships its own body, so it is
# self-contained and scaffolds regardless of whether any similarly
# named extension is installed. Namespaced names (speckit.<ns>.<cmd>)
# are treated exactly like short names (speckit.<cmd>) — they are NOT
# filtered out just because ``.specify/extensions/<ns>/`` is absent.
# The only command that cannot be materialized is a composition
# (prepend/append/wrap) with no base layer to compose onto; that case
# is handled per-command below (warn + skip), not by dropping names up
# front.
# Handle composition strategies: resolve composed content for non-replace commands
resolver = PresetResolver(self.project_root)
composed_dir = None
commands_to_register = []
for cmd in filtered:
for cmd in command_templates:
strategy = cmd.get("strategy", "replace")
if strategy != "replace":
# Only pre-compose if this preset is the top composing layer.
Expand All @@ -885,13 +865,23 @@ def _register_commands(
"file": f".composed/{cmd['name']}.md",
})
else:
raise PresetValidationError(
f"Command '{cmd['name']}' uses '{strategy}' strategy "
f"but no base command layer exists to compose onto. "
f"Ensure a lower-priority preset, extension, or core "
f"command provides this command before using "
f"composition strategies."
# No base layer to compose onto (e.g. the command it
# would wrap comes from an extension that isn't
# installed). Warn and skip this single command rather
# than aborting the whole install — mirrors the
# "composed is None" branch in
# _reconcile_composed_commands so command-mode and
# reconciliation behave identically.
import warnings
warnings.warn(
f"Command '{cmd['name']}' uses '{strategy}' "
f"strategy but no base command layer exists to "
f"compose onto; skipping. Provide a lower-priority "
f"preset, extension, or core command for it before "
f"using composition strategies.",
stacklevel=2,
)
continue
Comment on lines +875 to +884

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — fixed in d5c2998.

_register_skills now applies the same skip as _register_commands: for a composition-strategy command (wrap/prepend/append) with no .composed file, it resolves the stack and skips when no base exists (resolve_content is None), so an uncomposable command is no longer materialized as a skill with a literal {CORE_TEMPLATE} (wrap) or as a bare preset fragment (prepend/append). The skip is silent there because _register_commands already warns for the same command in the same pass. Added a regression test (test_uncomposable_wrap_command_skips_skill_in_skills_mode) asserting no skill is written and no {CORE_TEMPLATE} leaks.

Posted on behalf of @mnriem by GitHub Copilot (model: Claude Opus 4.8, autonomous).

else:
# Not the top layer — register raw file; reconciliation
# will overwrite with the correct composed/winning content.
Expand Down Expand Up @@ -1659,21 +1649,13 @@ def _reconcile_composed_commands(
if not command_names:
return set()

# Never materialize extension-scoped commands whose extension isn't
# installed. Registration (_register_commands / _register_skills)
# already refuses them, so a reconciliation pass writing them would
# create files no registry entry tracks. Filtering here — the single
# chokepoint every install/remove/rescaffold reconciliation funnels
# through — keeps all callers consistent without each one re-applying
# the filter when seeding names from manifest templates.
command_names = [
name
for name in command_names
if self._extension_installed_for_command(name)
]
if not command_names:
return set()

# Every preset-owned command name flows through unchanged. Names are
# NOT filtered by the ``speckit.<ns>.<cmd>`` shape: a self-contained
# preset command scaffolds whether or not a like-named extension is
# installed (parity with _register_commands), and a name whose base
# layer has disappeared must still reach the loop below so its now
# uncomposable stale file gets unregistered. The loop already skips
# names that resolve to no layers at all (``if not layers: continue``).
try:
from ..agents import CommandRegistrar
except ImportError:
Expand Down Expand Up @@ -2114,14 +2096,11 @@ def _reconcile_skills(
if not command_names:
return set()

command_names = [
name
for name in command_names
if self._extension_installed_for_command(name)
]
if not command_names:
return set()

# Preset-owned command names are not filtered by the
# ``speckit.<ns>.<cmd>`` shape here either: a self-contained preset
# command renders its skill whether or not a like-named extension is
# installed. The per-name loop below skips anything that doesn't
# resolve to a managed skill directory.
resolver = PresetResolver(self.project_root)
active_skills_dir = self._get_skills_dir()

Expand Down Expand Up @@ -2651,21 +2630,17 @@ def _register_skills(
if not command_templates:
return {}

# Filter out extension command overrides if the extension isn't installed,
# matching the same logic used by _register_commands().
filtered = [
cmd
for cmd in command_templates
if self._extension_installed_for_command(cmd["name"])
]

if not filtered:
return {}

# Preset command templates are self-contained and render as skills
# regardless of whether a like-named extension is installed — the same
# rule _register_commands() uses. No ``speckit.<ns>.<cmd>`` name-shape
# filtering; the per-command loop below skips anything without a target
# skill directory.
skills_dir = target_dir if target_dir is not None else self._get_skills_dir()
if not skills_dir:
return {}

resolver = PresetResolver(self.project_root)

from .. import SKILL_DESCRIPTIONS, load_init_options
from ..agents import CommandRegistrar
from ..integrations import get_integration
Expand Down Expand Up @@ -2695,7 +2670,7 @@ def _register_skills(

written: List[str] = []

for cmd_tmpl in filtered:
for cmd_tmpl in command_templates:
cmd_name = cmd_tmpl["name"]
cmd_file_rel = cmd_tmpl["file"]
source_file = preset_dir / cmd_file_rel
Expand Down Expand Up @@ -2735,6 +2710,29 @@ def _register_skills(
content = source_file.read_text(encoding="utf-8")
frontmatter, body = registrar.parse_frontmatter(content)

# A composition-strategy command (wrap/prepend/append) needs a
# base layer to compose onto. When _register_commands produced no
# composed file for it and the stack still has no base
# (resolve_content is None) — e.g. the command it wraps comes from
# an extension that isn't installed — rendering the raw preset
# fragment as a skill would emit broken output: a literal
# {CORE_TEMPLATE} for wrap, or only the preset's own fragment for
# prepend/append. Skip it here too so command mode and skills mode
# agree (mirrors _register_commands, which skips the same command).
# _register_commands already warned for this command in the same
# pass, so the skip is silent here to avoid a duplicate warning.
effective_strategy = (
cmd_tmpl.get("strategy")
or frontmatter.get("strategy")
or "replace"
)
if (
effective_strategy != "replace"
and not composed_file.exists()
and resolver.resolve_content(cmd_name, "command") is None
):
continue

if frontmatter.get("strategy") == "wrap":
body, core_frontmatter = _substitute_core_template(body, cmd_name, self.project_root, registrar)
frontmatter = dict(frontmatter)
Expand Down
Loading