From 6dfe9fd41342176e010f1b5ea732f4cf5241bda7 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Thu, 15 Oct 2026 13:28:26 -0500 Subject: [PATCH 01/10] refactor(integrations): own invocation rendering Move registered command naming, command-reference rendering, and hook invocation formatting behind resolved integration classes while preserving existing mode-specific behavior. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c44efad8-4dc0-4488-a27f-e2abace733ef --- design/integration.md | 13 ++ src/specify_cli/_invocation_style.py | 93 -------------- src/specify_cli/agents.py | 74 ++++++----- src/specify_cli/command_init.py | 13 +- src/specify_cli/extensions/__init__.py | 58 +++------ src/specify_cli/integration_runtime.py | 3 +- src/specify_cli/integrations/agy/__init__.py | 1 + .../integrations/alquimia/__init__.py | 1 + src/specify_cli/integrations/base.py | 119 +++++++++++++++--- .../integrations/claude/__init__.py | 1 + .../integrations/cline/__init__.py | 9 ++ .../integrations/codex/__init__.py | 2 + .../integrations/command_code/__init__.py | 2 + .../integrations/cursor_agent/__init__.py | 1 + .../integrations/forge/__init__.py | 9 ++ .../integrations/generic/__init__.py | 4 + .../integrations/hermes/__init__.py | 1 + .../integrations/junie/__init__.py | 9 ++ src/specify_cli/integrations/kimi/__init__.py | 20 +-- .../integrations/lingma/__init__.py | 1 + .../integrations/rovodev/__init__.py | 1 + src/specify_cli/integrations/vibe/__init__.py | 1 + .../integrations/zcode/__init__.py | 2 + src/specify_cli/presets/_manager_skills.py | 21 ++-- .../github/test_github_extension.py | 15 ++- tests/integrations/test_base.py | 25 ++-- .../integrations/test_integration_alquimia.py | 82 +++--------- .../test_integration_docker_agent.py | 20 +-- tests/integrations/test_integration_droid.py | 23 ++-- tests/integrations/test_integration_dsh.py | 4 +- tests/integrations/test_integration_zed.py | 8 +- 31 files changed, 312 insertions(+), 324 deletions(-) delete mode 100644 src/specify_cli/_invocation_style.py diff --git a/design/integration.md b/design/integration.md index 04509705d3..56d174f630 100644 --- a/design/integration.md +++ b/design/integration.md @@ -68,6 +68,19 @@ demands them. Keep mode selection, invocation spelling, and registration in sync. Agent-specific layouts and options belong in the integration code and the [supported-integrations reference](../docs/reference/integrations.md). +Integration classes are also authoritative for command naming and invocation +rendering. Shared init, registration, extension, preset, hook, and runtime +callers resolve the active integration and use its naming and mode hooks rather +than maintaining agent-name lists. `registrar_config["format_name"]` controls +registered command names when present; skills otherwise use the shared +`speckit-` convention. `invocation_prefix_for_mode()`, +`invoke_separator_for_mode()`, and `build_command_reference()` define displayed +and embedded command references. `build_hook_invocation()` preserves unrelated +custom hook commands unless an integration such as Cline, Forge, or Junie owns +their naming too. `build_command_invocation()` remains the runtime dispatch seam +for integrations such as Bob and Copilot whose executable behavior depends on +the installed layout. + Core templates that call scripts declare `sh`, `ps`, and `py` commands in `scripts:` frontmatter; template processing replaces `{SCRIPT}` with the selected variant. `py` is opt-in; non-interactive init defaults to `sh` on diff --git a/src/specify_cli/_invocation_style.py b/src/specify_cli/_invocation_style.py deleted file mode 100644 index 86926e23ba..0000000000 --- a/src/specify_cli/_invocation_style.py +++ /dev/null @@ -1,93 +0,0 @@ -"""Agent invocation-style constants and helpers. - -Agents that scaffold skills (``speckit-/SKILL.md``) use different -slash-command invocation formats depending on the agent. This module -centralises the mapping so that ``HookExecutor._render_hook_invocation`` -and ``specify init``'s next-steps output stay consistent. -""" - -from __future__ import annotations - -# Agents that render $speckit- (chat invocation) when in skills mode. -DOLLAR_SKILLS_AGENTS: frozenset[str] = frozenset({"codex", "zcode", "command-code"}) - -# Agents that always render /speckit-, regardless of ai_skills. -ALWAYS_SLASH_AGENTS: frozenset[str] = frozenset( - { - "devin", - "docker-agent", - "droid", - "dsh", - "grok", - "mcode", - "muse", - "qodercli", - "trae", - "zed", - } -) - -# Agents that render /speckit- only when ai_skills is enabled. -CONDITIONAL_SLASH_AGENTS: frozenset[str] = frozenset( - { - "agy", - "alquimia", - "bob", - "claude", - "copilot", - "cursor-agent", - "generic", - "hermes", - "lingma", - "rovodev", - "vibe", - } -) - -# Agents that render /skill: (skill-colon invocation) when in skills mode. -SKILL_COLON_AGENTS: frozenset[str] = frozenset({"kimi"}) - - -def is_dollar_skills_agent(selected_ai: str | None, ai_skills_enabled: bool) -> bool: - """Return ``True`` if *selected_ai* uses ``$speckit-`` invocations. - - Agents in `DOLLAR_SKILLS_AGENTS` (e.g. ``codex``, ``zcode``) render - ``$speckit-`` chat invocations when installed in skills mode. - """ - if not isinstance(selected_ai, str): - return False - return selected_ai in DOLLAR_SKILLS_AGENTS and ai_skills_enabled - - -def get_invocation_prefix(selected_ai: str | None, ai_skills_enabled: bool) -> str: - """Return the native invocation prefix for *selected_ai* in skills mode. - - Returns ``"$"`` for dollar-skills agents (Codex, ZCode), - ``"/skill:"`` for skill-colon agents (Kimi), and ``"/"`` for all others. - """ - if not isinstance(selected_ai, str): - return "/" - if selected_ai in DOLLAR_SKILLS_AGENTS and ai_skills_enabled: - return "$" - if selected_ai in SKILL_COLON_AGENTS and ai_skills_enabled: - return "/skill:" - return "/" - - -def is_slash_skills_agent(selected_ai: str | None, ai_skills_enabled: bool) -> bool: - """Return ``True`` if *selected_ai* uses ``/speckit-`` invocations. - - The decision is based on the agent sets defined in this module: - - * Agents in `ALWAYS_SLASH_AGENTS` always use slash invocations. - * Agents in `CONDITIONAL_SLASH_AGENTS` only use them when - *ai_skills_enabled* is ``True``. - * All other agents return ``False``. - """ - if selected_ai is None: - return False - if not isinstance(selected_ai, str): - return False - return selected_ai in ALWAYS_SLASH_AGENTS or ( - selected_ai in CONDITIONAL_SLASH_AGENTS and ai_skills_enabled - ) diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index 72966998a5..23173e3ec1 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -16,7 +16,6 @@ import yaml from ._init_options import is_ai_skills_enabled, load_init_options -from ._invocation_style import get_invocation_prefix from ._toml_string import escape_toml_basic as _escape_toml_basic from ._toml_string import has_illegal_toml_control as _has_illegal_toml_control from ._utils import relative_extension_path_violation @@ -582,18 +581,20 @@ def _compute_output_name( agent_name: str, cmd_name: str, agent_config: Dict[str, Any] ) -> str: """Compute the on-disk command or skill name for an agent.""" - if agent_config["extension"] != "/SKILL.md": - format_name = agent_config.get("format_name") - if format_name: - return format_name(cmd_name) - return cmd_name + from specify_cli.integrations import get_integration - short_name = cmd_name - if short_name.startswith("speckit."): - short_name = short_name[len("speckit.") :] - short_name = short_name.replace(".", "-") + integration = get_integration(agent_name) + if integration is not None: + return integration.format_registered_command_name( + cmd_name, agent_config + ) - return f"speckit-{short_name}" + format_name = agent_config.get("format_name") + if callable(format_name): + return str(format_name(cmd_name)) + if agent_config.get("extension") != "/SKILL.md": + return cmd_name + return f"speckit-{cmd_name.removeprefix('speckit.').replace('.', '-')}" @staticmethod def _ensure_inside(candidate: Path, base: Path) -> None: @@ -738,15 +739,17 @@ def register_commands( # ``.bob/commands``. _sep = agent_config.get("invoke_separator", ".") registrar_writes_skills = agent_config.get("extension") == "/SKILL.md" + _integration = None try: from specify_cli.integrations import get_integration # noqa: PLC0415 - _integ = get_integration(agent_name) - if _integ is not None: - _sep = _integ.invoke_separator_for_mode(registrar_writes_skills) + _integration = get_integration(agent_name) + if _integration is not None: + _sep = _integration.invoke_separator_for_mode( + registrar_writes_skills + ) except (ImportError, ValueError, KeyError): pass - _prefix = get_invocation_prefix(agent_name, registrar_writes_skills) for cmd_info in commands: cmd_name = cmd_info["name"] @@ -824,9 +827,15 @@ def register_commands( frontmatter.pop(key, None) if agent_config.get("inject_name") and not frontmatter.get("name"): - # Use custom name formatter if provided (e.g., Forge's hyphenated format) - format_name = agent_config.get("format_name") - frontmatter["name"] = format_name(cmd_name) if format_name else cmd_name + frontmatter["name"] = ( + _integration.format_registered_command_name( + cmd_name, agent_config + ) + if _integration is not None + else self._compute_output_name( + agent_name, cmd_name, agent_config + ) + ) if is_cline_ext: frontmatter = self._hyphenate_frontmatter_refs(frontmatter) @@ -845,11 +854,16 @@ def register_commands( # the project's persisted ai_skills state. Single-layout agents fall back # to the static AGENT_CONFIGS value unchanged (invoke_separator_for_mode # default). - # Deferred import of IntegrationBase avoids a circular import at module load - # (base.py itself imports CommandRegistrar lazily). - from specify_cli.integrations.base import IntegrationBase # noqa: PLC0415 + if _integration is not None: + body = _integration.resolve_command_refs_for_mode( + body, + skills_enabled=registrar_writes_skills, + ) + else: + # Deferred import avoids a circular import at module load. + from specify_cli.integrations.base import IntegrationBase # noqa: PLC0415 - body = IntegrationBase.resolve_command_refs(body, _sep, _prefix) + body = IntegrationBase.resolve_command_refs(body, _sep) output_name = self._compute_output_name(agent_name, cmd_name, agent_config) @@ -899,13 +913,7 @@ def register_commands( raise ValueError(f"Unsupported format: {agent_config['format']}") # -- Post-process for non-skills agents ----------------------- - _integration = None if agent_config["extension"] != "/SKILL.md": - from specify_cli.integrations import ( # noqa: PLC0415 - get_integration, - ) - - _integration = get_integration(agent_name) if _integration is not None: output = _integration.post_process_command_content(output) @@ -939,10 +947,14 @@ def register_commands( # For agents with inject_name, render with alias-specific frontmatter if agent_config.get("inject_name"): alias_frontmatter = deepcopy(frontmatter) - # Use custom name formatter if provided (e.g., Forge's hyphenated format) - format_name = agent_config.get("format_name") alias_frontmatter["name"] = ( - format_name(alias) if format_name else alias + _integration.format_registered_command_name( + alias, agent_config + ) + if _integration is not None + else self._compute_output_name( + agent_name, alias, agent_config + ) ) if agent_config["extension"] == "/SKILL.md": diff --git a/src/specify_cli/command_init.py b/src/specify_cli/command_init.py index ea21fb1a54..aa09ff9da0 100644 --- a/src/specify_cli/command_init.py +++ b/src/specify_cli/command_init.py @@ -1153,18 +1153,15 @@ def init( step_num += 1 usage_label = "skills" if native_skill_mode else "slash commands" - invoke_prefix = _invoke_prefix_for_integration( - resolved_integration, - resolved_integration.key, - integration_parsed_options or None, - project_path, - ) - invoke_separator = resolved_integration.effective_invoke_separator( + invocation_skills_mode = resolved_integration.is_skills_mode( integration_parsed_options or None, project_root=project_path ) def _display_cmd(name: str) -> str: - return f"{invoke_prefix}speckit{invoke_separator}{name}" + return resolved_integration.build_command_reference( + name, + skills_enabled=invocation_skills_mode, + ) steps_lines.append( f"{step_num}. Start using {usage_label} with your coding agent:" diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index d03cb85770..b651d99468 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -43,7 +43,6 @@ safe_extract_archive, ) 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 from ..catalogs import CatalogEntry as BaseCatalogEntry from ..catalogs import CatalogStackBase @@ -1614,7 +1613,6 @@ def _register_extension_skills( from .. import load_init_options from ..agents import CommandRegistrar from ..integrations import get_integration - from ..integrations.base import IntegrationBase from ..integrations._file_changes import changing_file, unlink, write_text written: List[str] = [] @@ -1634,19 +1632,13 @@ def _resolve_command_ref_tokens(body: str) -> str: def _replacement(match: re.Match[str]) -> str: command_name = "speckit." + match.group(1).lower().replace("_", ".") - if is_dollar_skills_agent(selected_ai, ai_skills_enabled): - return "$" + command_name.replace("speckit.", "speckit-").replace( - ".", "-" - ) - if is_slash_skills_agent(selected_ai, ai_skills_enabled): - return "/" + command_name.replace("speckit.", "speckit-").replace( - ".", "-" - ) if integration is not None: - return integration.build_command_invocation(command_name) - return IntegrationBase.resolve_command_refs( - match.group(0), agent_config.get("invoke_separator", ".") - ) + return integration.build_command_reference( + command_name, + skills_enabled=ai_skills_enabled, + ) + separator = agent_config.get("invoke_separator", ".") + return "/" + command_name.replace(".", separator) return re.sub( r"__SPECKIT_COMMAND_([A-Z][A-Z0-9_-]*)__", _replacement, body @@ -5672,35 +5664,15 @@ def _render_hook_invocation(self, command: Any) -> str: init_options = self._load_init_options() selected_ai = init_options.get("ai") ai_skills_enabled = is_ai_skills_enabled(init_options) - - dollar_skill_mode = is_dollar_skills_agent(selected_ai, ai_skills_enabled) - kimi_skill_mode = selected_ai == "kimi" - cline_mode = selected_ai == "cline" - forge_mode = selected_ai == "forge" - junie_mode = selected_ai == "junie" - - skill_name = self._skill_name_from_command(command_id) - if dollar_skill_mode and skill_name: - return f"${skill_name}" - if kimi_skill_mode and skill_name: - return f"/skill:{skill_name}" - if cline_mode: - from ..integrations.cline import format_cline_command_name - - return f"/{format_cline_command_name(command_id)}" - if forge_mode: - from ..integrations.forge import format_forge_command_name - - return f"/{format_forge_command_name(command_id)}" - if junie_mode: - from ..integrations.junie import format_junie_command_name - - return f"/{format_junie_command_name(command_id)}" - - use_slash = is_slash_skills_agent(selected_ai, ai_skills_enabled) - - if skill_name and use_slash: - return f"/{skill_name}" + if isinstance(selected_ai, str): + from ..integrations import get_integration + + integration = get_integration(selected_ai) + if integration is not None: + return integration.build_hook_invocation( + command_id, + skills_enabled=ai_skills_enabled, + ) return f"/{command_id}" diff --git a/src/specify_cli/integration_runtime.py b/src/specify_cli/integration_runtime.py index efcd8a9e63..5cd44d5e7f 100644 --- a/src/specify_cli/integration_runtime.py +++ b/src/specify_cli/integration_runtime.py @@ -5,7 +5,6 @@ from collections.abc import Callable from typing import Any -from ._invocation_style import get_invocation_prefix from .integration_state import integration_setting, integration_settings @@ -110,4 +109,4 @@ def invoke_prefix_for_integration( ) -> str: """Resolve the native invocation prefix for an integration's output mode.""" skills_mode = integration.is_skills_mode(parsed_options, project_root) - return get_invocation_prefix(key, skills_mode) + return integration.invocation_prefix_for_mode(skills_mode) diff --git a/src/specify_cli/integrations/agy/__init__.py b/src/specify_cli/integrations/agy/__init__.py index f9b10f43a2..9237873a73 100644 --- a/src/specify_cli/integrations/agy/__init__.py +++ b/src/specify_cli/integrations/agy/__init__.py @@ -42,6 +42,7 @@ class AgyIntegration(SkillsIntegration): """ key: ClassVar[str] = "agy" + skills_invocation_only = True config: ClassVar[dict[str, Any]] = { "name": "Antigravity", "folder": ".agents/", diff --git a/src/specify_cli/integrations/alquimia/__init__.py b/src/specify_cli/integrations/alquimia/__init__.py index 54ea47f486..631056009d 100644 --- a/src/specify_cli/integrations/alquimia/__init__.py +++ b/src/specify_cli/integrations/alquimia/__init__.py @@ -26,6 +26,7 @@ class AlquimiaAIIntegration(SkillsIntegration): """Integration for Alquimia AI skills.""" key = "alquimia" + skills_invocation_only = True config = { "name": "Alquimia AI", "folder": ".alquimia/", diff --git a/src/specify_cli/integrations/base.py b/src/specify_cli/integrations/base.py index b3dd0abc34..59f0b8ae2f 100644 --- a/src/specify_cli/integrations/base.py +++ b/src/specify_cli/integrations/base.py @@ -28,7 +28,6 @@ import yaml -from .._invocation_style import get_invocation_prefix, is_dollar_skills_agent from .._toml_string import escape_toml_basic as _escape_toml_basic from .._toml_string import has_illegal_toml_control as _has_illegal_toml_control from ..events import install_integration_events, remove_integration_events @@ -142,6 +141,12 @@ class IntegrationBase(ABC): invoke_separator: str = "." """Separator used in slash-command invocations (``"."`` → ``/speckit.plan``).""" + invocation_prefix: str = "/" + """Prefix used for rendered command references in the native UI.""" + + skills_invocation_only: bool = False + """Whether the skills invocation style applies only in skills mode.""" + dev_no_symlink: bool = False """Whether dev-mode registration should write files instead of symlinks.""" @@ -223,6 +228,83 @@ def invoke_separator_for_mode(self, skills_enabled: bool) -> str: cfg = self.registrar_config or {} return cfg.get("invoke_separator", self.invoke_separator) + def invocation_prefix_for_mode(self, skills_enabled: bool) -> str: + """Return the native command-reference prefix for the resolved mode.""" + if self.skills_invocation_only and not skills_enabled: + return "/" + return self.invocation_prefix + + def command_reference_separator_for_mode(self, skills_enabled: bool) -> str: + """Return the separator used by displayed and embedded references.""" + if self.skills_invocation_only and not skills_enabled: + return "." + return self.invoke_separator_for_mode(skills_enabled) + + def format_registered_command_name( + self, + command_name: str, + registrar_config: Mapping[str, Any] | None = None, + ) -> str: + """Return the on-disk command name used by command registration. + + Integrations may provide ``registrar_config["format_name"]`` for + command layouts with agent-specific naming. Skills use the shared + ``speckit-`` directory convention. + """ + config = registrar_config or self.registrar_config or {} + format_name = config.get("format_name") + if callable(format_name): + return str(format_name(command_name)) + if config.get("extension") != "/SKILL.md": + return command_name + + stem = command_name.removeprefix("speckit.") + return f"speckit-{stem.replace('.', '-')}" + + def build_command_reference( + self, + command_name: str, + args: str = "", + *, + skills_enabled: bool, + ) -> str: + """Render a command reference for guidance, templates, and hooks.""" + stem = command_name.removeprefix("speckit.") + separator = self.command_reference_separator_for_mode(skills_enabled) + prefix = self.invocation_prefix_for_mode(skills_enabled) + invocation = prefix + "speckit" + separator + stem.replace(".", separator) + return f"{invocation} {args}" if args else invocation + + def resolve_command_refs_for_mode( + self, + content: str, + *, + skills_enabled: bool, + ) -> str: + """Replace command-reference placeholders using this integration.""" + return re.sub( + r"__SPECKIT_COMMAND_([A-Z][A-Z0-9_-]*)__", + lambda match: self.build_command_reference( + "speckit." + match.group(1).lower().replace("_", "."), + skills_enabled=skills_enabled, + ), + content, + ) + + def build_hook_invocation( + self, + command_name: str, + *, + skills_enabled: bool, + ) -> str: + """Render a hook command without rewriting unrelated custom commands.""" + if not command_name.startswith("speckit."): + return f"/{command_name}" + return self.build_command_reference( + command_name, + skills_enabled=skills_enabled, + ) + def is_skills_mode( self, parsed_options: dict[str, Any] | None = None, @@ -989,10 +1071,21 @@ def process_template( content = CommandRegistrar.rewrite_project_relative_paths(content) - # 8. Replace __SPECKIT_COMMAND___ with invocation strings - invocation_prefix = get_invocation_prefix( - agent_name, invoke_separator == "-" - ) + # 8. Replace __SPECKIT_COMMAND___ with invocation strings. + # Integration classes own their native prefix; the separator remains + # an explicit rendering input because dual-layout setup paths resolve + # it from the selected mode before calling this shared transformer. + invocation_prefix = "/" + try: + from specify_cli.integrations import get_integration + + integration = get_integration(agent_name) + if integration is not None: + invocation_prefix = integration.invocation_prefix_for_mode( + invoke_separator == "-" + ) + except (ImportError, KeyError, ValueError): + pass content = IntegrationBase.resolve_command_refs( content, invoke_separator, invocation_prefix ) @@ -1746,15 +1839,11 @@ def skills_dest(self, project_root: Path) -> Path: def build_command_invocation(self, command_name: str, args: str = "") -> str: """Build the agent's native invocation for a hyphenated skill name.""" - stem = command_name - if stem.startswith("speckit."): - stem = stem[len("speckit."):] - - prefix = "$" if is_dollar_skills_agent(self.key, True) else "/" - invocation = prefix + "speckit-" + stem.replace(".", "-") - if args: - invocation = f"{invocation} {args}" - return invocation + return self.build_command_reference( + command_name, + args, + skills_enabled=True, + ) @staticmethod def _inject_hook_command_note( @@ -1811,7 +1900,7 @@ def post_process_skill_content(self, content: str) -> str: ``$speckit-git-commit``). Subclasses may override -- see ``ClaudeIntegration``. """ - invocation_prefix = get_invocation_prefix(self.key, True) + invocation_prefix = self.invocation_prefix_for_mode(True) return self._inject_hook_command_note(content, invocation_prefix) def setup( diff --git a/src/specify_cli/integrations/claude/__init__.py b/src/specify_cli/integrations/claude/__init__.py index 03d428bc51..8f918d75f6 100644 --- a/src/specify_cli/integrations/claude/__init__.py +++ b/src/specify_cli/integrations/claude/__init__.py @@ -41,6 +41,7 @@ class ClaudeIntegration(SkillsIntegration): """Integration for Claude Code skills.""" key = "claude" + skills_invocation_only = True config = { "name": "Claude Code", "folder": ".claude/", diff --git a/src/specify_cli/integrations/cline/__init__.py b/src/specify_cli/integrations/cline/__init__.py index db6961b5eb..9086e0723a 100644 --- a/src/specify_cli/integrations/cline/__init__.py +++ b/src/specify_cli/integrations/cline/__init__.py @@ -90,6 +90,15 @@ def build_command_invocation(self, command_name: str, args: str = "") -> str: invocation = f"{invocation} {args}" return invocation + def build_hook_invocation( + self, + command_name: str, + *, + skills_enabled: bool, + ) -> str: + """Cline formats every hook command through its registered name.""" + return self.build_command_invocation(command_name) + def process_template(self, *args, **kwargs): """Ensure shared templates render Cline command references with hyphens.""" kwargs.setdefault("invoke_separator", self.invoke_separator) diff --git a/src/specify_cli/integrations/codex/__init__.py b/src/specify_cli/integrations/codex/__init__.py index d88628b3ff..3f7ea7c2db 100644 --- a/src/specify_cli/integrations/codex/__init__.py +++ b/src/specify_cli/integrations/codex/__init__.py @@ -17,6 +17,8 @@ class CodexIntegration(SkillsIntegration): """Integration for OpenAI Codex CLI.""" key = "codex" + invocation_prefix = "$" + skills_invocation_only = True config = { "name": "Codex CLI", "folder": ".agents/", diff --git a/src/specify_cli/integrations/command_code/__init__.py b/src/specify_cli/integrations/command_code/__init__.py index 8eef9f5579..c86ddfb100 100644 --- a/src/specify_cli/integrations/command_code/__init__.py +++ b/src/specify_cli/integrations/command_code/__init__.py @@ -14,6 +14,8 @@ class CommandCodeIntegration(SkillsIntegration): """Integration for Command Code CLI.""" key = "command-code" + invocation_prefix = "$" + skills_invocation_only = True config = { "name": "Command Code", "folder": ".commandcode/", diff --git a/src/specify_cli/integrations/cursor_agent/__init__.py b/src/specify_cli/integrations/cursor_agent/__init__.py index 87b00236d8..ad93d40e76 100644 --- a/src/specify_cli/integrations/cursor_agent/__init__.py +++ b/src/specify_cli/integrations/cursor_agent/__init__.py @@ -21,6 +21,7 @@ class CursorAgentIntegration(SkillsIntegration): key = "cursor-agent" + skills_invocation_only = True config = { "name": "Cursor", "folder": ".cursor/", diff --git a/src/specify_cli/integrations/forge/__init__.py b/src/specify_cli/integrations/forge/__init__.py index 520e417523..69407ecdbf 100644 --- a/src/specify_cli/integrations/forge/__init__.py +++ b/src/specify_cli/integrations/forge/__init__.py @@ -133,6 +133,15 @@ def build_command_invocation(self, command_name: str, args: str = "") -> str: invocation = f"{invocation} {args}" return invocation + def build_hook_invocation( + self, + command_name: str, + *, + skills_enabled: bool, + ) -> str: + """Forge formats every hook command through its registered name.""" + return self.build_command_invocation(command_name) + def setup( self, project_root: Path, diff --git a/src/specify_cli/integrations/generic/__init__.py b/src/specify_cli/integrations/generic/__init__.py index 0dbe598ab7..dee3d4404d 100644 --- a/src/specify_cli/integrations/generic/__init__.py +++ b/src/specify_cli/integrations/generic/__init__.py @@ -92,6 +92,10 @@ def effective_invoke_separator( """ return "-" if self.is_skills_mode(parsed_options, project_root) else "." + def invoke_separator_for_mode(self, skills_enabled: bool) -> str: + """Match command references to the selected generic output layout.""" + return "-" if skills_enabled else "." + @classmethod def options(cls) -> list[IntegrationOption]: return [ diff --git a/src/specify_cli/integrations/hermes/__init__.py b/src/specify_cli/integrations/hermes/__init__.py index 5b3d1977cc..36160100c9 100644 --- a/src/specify_cli/integrations/hermes/__init__.py +++ b/src/specify_cli/integrations/hermes/__init__.py @@ -37,6 +37,7 @@ class HermesIntegration(SkillsIntegration): """ key = "hermes" + skills_invocation_only = True config = { "name": "Hermes Agent", "folder": ".hermes/", diff --git a/src/specify_cli/integrations/junie/__init__.py b/src/specify_cli/integrations/junie/__init__.py index 21ecac1f83..9037896ecb 100644 --- a/src/specify_cli/integrations/junie/__init__.py +++ b/src/specify_cli/integrations/junie/__init__.py @@ -85,6 +85,15 @@ def build_command_invocation(self, command_name: str, args: str = "") -> str: invocation = f"{invocation} {args}" return invocation + def build_hook_invocation( + self, + command_name: str, + *, + skills_enabled: bool, + ) -> str: + """Junie formats every hook command through its registered name.""" + return self.build_command_invocation(command_name) + def process_template(self, *args, **kwargs): """Ensure shared templates render Junie command references with hyphens.""" kwargs.setdefault("invoke_separator", self.invoke_separator) diff --git a/src/specify_cli/integrations/kimi/__init__.py b/src/specify_cli/integrations/kimi/__init__.py index 07142db854..876c21fe2c 100644 --- a/src/specify_cli/integrations/kimi/__init__.py +++ b/src/specify_cli/integrations/kimi/__init__.py @@ -23,6 +23,7 @@ class KimiIntegration(SkillsIntegration): """Integration for Kimi Code CLI (Moonshot AI).""" key = "kimi" + invocation_prefix = "/skill:" config = { "name": "Kimi Code", "folder": ".kimi-code/", @@ -38,25 +39,6 @@ class KimiIntegration(SkillsIntegration): } multi_install_safe = False - def build_command_invocation(self, command_name: str, args: str = "") -> str: - """Build Kimi's native skill invocation: ``/skill:speckit-``. - - Kimi Code CLI invokes installed skills with a ``/skill:`` - slash command (e.g. ``/skill:speckit-plan``), not the bare - ``/speckit-`` form produced by the generic skills base - class. Overriding here keeps ``dispatch_command()`` and workflow - command steps aligned with the ``/skill:`` guidance shown at init - time and in rendered hook invocations. - """ - stem = command_name - if stem.startswith("speckit."): - stem = stem[len("speckit.") :] - - invocation = "/skill:speckit-" + stem.replace(".", "-") - if args: - invocation = f"{invocation} {args}" - return invocation - def post_process_skill_content(self, content: str) -> str: """Ensure in-skill cross-command references use Kimi's `/skill:` syntax.""" return super().post_process_skill_content(content) diff --git a/src/specify_cli/integrations/lingma/__init__.py b/src/specify_cli/integrations/lingma/__init__.py index 0d2fa824aa..400dc36fc2 100644 --- a/src/specify_cli/integrations/lingma/__init__.py +++ b/src/specify_cli/integrations/lingma/__init__.py @@ -14,6 +14,7 @@ class LingmaIntegration(SkillsIntegration): """Integration for Lingma IDE.""" key = "lingma" + skills_invocation_only = True config = { "name": "Lingma", "folder": ".lingma/", diff --git a/src/specify_cli/integrations/rovodev/__init__.py b/src/specify_cli/integrations/rovodev/__init__.py index 198347be3d..5c45a43243 100644 --- a/src/specify_cli/integrations/rovodev/__init__.py +++ b/src/specify_cli/integrations/rovodev/__init__.py @@ -27,6 +27,7 @@ class RovodevIntegration(SkillsIntegration): """ key = "rovodev" + skills_invocation_only = True config = { "name": "RovoDev ACLI", "folder": ".rovodev/", diff --git a/src/specify_cli/integrations/vibe/__init__.py b/src/specify_cli/integrations/vibe/__init__.py index e0d0957d9a..85ea8f79de 100644 --- a/src/specify_cli/integrations/vibe/__init__.py +++ b/src/specify_cli/integrations/vibe/__init__.py @@ -32,6 +32,7 @@ class VibeIntegration(SkillsIntegration): """Integration for Mistral Vibe skills.""" key = "vibe" + skills_invocation_only = True config = { "name": "Mistral Vibe", "folder": ".vibe/", diff --git a/src/specify_cli/integrations/zcode/__init__.py b/src/specify_cli/integrations/zcode/__init__.py index 9d6802d436..0904444d02 100644 --- a/src/specify_cli/integrations/zcode/__init__.py +++ b/src/specify_cli/integrations/zcode/__init__.py @@ -19,6 +19,8 @@ class ZcodeIntegration(SkillsIntegration): """Integration for ZCode CLI (Z.AI).""" key = "zcode" + invocation_prefix = "$" + skills_invocation_only = True config = { "name": "ZCode", "folder": ".zcode/", diff --git a/src/specify_cli/presets/_manager_skills.py b/src/specify_cli/presets/_manager_skills.py index 5a4a4fb739..0a0a79acb6 100644 --- a/src/specify_cli/presets/_manager_skills.py +++ b/src/specify_cli/presets/_manager_skills.py @@ -11,7 +11,6 @@ from .._init_options import is_ai_skills_enabled -from .._invocation_style import get_invocation_prefix from .._utils import dump_frontmatter from ..integrations.base import IntegrationBase from ..integrations._registration import project_registration @@ -545,24 +544,30 @@ def _resolve_skill_command_refs( ``AGENT_CONFIGS`` value. """ separator = None - if project_root is not None and isinstance(selected_ai, str): + if isinstance(selected_ai, str): try: - from .. import load_init_options from ..integrations import get_integration integration = get_integration(selected_ai) if integration is not None: - separator = integration.invoke_separator_for_mode( - is_ai_skills_enabled(load_init_options(project_root)) + skills_enabled = True + if project_root is not None: + from .. import load_init_options + + skills_enabled = is_ai_skills_enabled( + load_init_options(project_root) + ) + return integration.resolve_command_refs_for_mode( + body, + skills_enabled=skills_enabled, ) - except Exception: + except (ImportError, KeyError, OSError, TypeError, ValueError): separator = None if separator is None: separator = registrar.AGENT_CONFIGS.get(selected_ai, {}).get( "invoke_separator", "." ) - prefix = get_invocation_prefix(selected_ai, separator == "-") - return IntegrationBase.resolve_command_refs(body, separator, prefix) + return IntegrationBase.resolve_command_refs(body, separator) def _build_extension_skill_restore_index(self) -> Dict[str, Dict[str, Any]]: """Index extension-backed skill restore data by skill directory name.""" diff --git a/tests/extensions/github/test_github_extension.py b/tests/extensions/github/test_github_extension.py index 718c1c8421..afc224e9ea 100644 --- a/tests/extensions/github/test_github_extension.py +++ b/tests/extensions/github/test_github_extension.py @@ -565,7 +565,7 @@ def test_readme_lists_every_dollar_skills_agent(self): Regression: Command Code was omitted, so its users were pointed at a syntax their agent does not use. """ - from specify_cli._invocation_style import DOLLAR_SKILLS_AGENTS + from specify_cli.integrations import INTEGRATION_REGISTRY readme = (EXT_DIR / "README.md").read_text(encoding="utf-8") note = next( @@ -577,10 +577,15 @@ def test_readme_lists_every_dollar_skills_agent(self): "zcode": "ZCode", "command-code": "Command Code", } - # If a new dollar-skills agent appears, this fails until it is named. - assert set(display_names) == set(DOLLAR_SKILLS_AGENTS), ( - "DOLLAR_SKILLS_AGENTS changed; update the README invocation note " - f"and this mapping: {sorted(DOLLAR_SKILLS_AGENTS)}" + dollar_skills_agents = { + key + for key, integration in INTEGRATION_REGISTRY.items() + if integration.invocation_prefix_for_mode(True) == "$" + } + # If a new dollar-skills integration appears, this fails until it is named. + assert set(display_names) == dollar_skills_agents, ( + "Dollar-skills integrations changed; update the README invocation note " + f"and this mapping: {sorted(dollar_skills_agents)}" ) for agent, display in display_names.items(): assert display in note, f"README omits {display} ({agent}): {note}" diff --git a/tests/integrations/test_base.py b/tests/integrations/test_base.py index 66faeb2b1b..51d43e6a29 100644 --- a/tests/integrations/test_base.py +++ b/tests/integrations/test_base.py @@ -270,7 +270,7 @@ def test_kimi_skill_post_processing_is_idempotent(self): """Kimi's post_process_skill_content must be idempotent. The hook-command note is injected with the /skill: prefix by the base - class (via get_invocation_prefix), so the idempotency check matches on + class (via the integration's prefix hook), so the idempotency check matches on re-runs without requiring the broad /speckit- -> /skill:speckit- body replacement to recognise a duplicate. """ @@ -289,14 +289,23 @@ class (via get_invocation_prefix), so the idempotency check matches on assert once.count("replace dots (`.`) with hyphens") == 1 assert "/skill:speckit-git-commit" in once - def test_get_invocation_prefix_skill_colon(self): - """get_invocation_prefix returns '/skill:' for Kimi in skills mode.""" - from specify_cli._invocation_style import get_invocation_prefix + @pytest.mark.parametrize( + ("integration_key", "skills_enabled", "expected"), + [ + ("kimi", True, "/skill:"), + ("kimi", False, "/skill:"), + ("codex", True, "$"), + ("codex", False, "/"), + ("claude", True, "/"), + ], + ) + def test_integration_owns_invocation_prefix( + self, integration_key, skills_enabled, expected + ): + from specify_cli.integrations import get_integration - assert get_invocation_prefix("kimi", True) == "/skill:" - assert get_invocation_prefix("kimi", False) == "/" - assert get_invocation_prefix("codex", True) == "$" - assert get_invocation_prefix("claude", True) == "/" + integration = get_integration(integration_key) + assert integration.invocation_prefix_for_mode(skills_enabled) == expected class TestResolveCommandRefs: """Tests for __SPECKIT_COMMAND___ placeholder resolution.""" diff --git a/tests/integrations/test_integration_alquimia.py b/tests/integrations/test_integration_alquimia.py index 9e25e14cc1..d17e22cb9e 100644 --- a/tests/integrations/test_integration_alquimia.py +++ b/tests/integrations/test_integration_alquimia.py @@ -37,31 +37,16 @@ def test_requires_cli_is_true(self): assert integration.config["requires_cli"] is True assert integration.multi_install_safe is True - def test_is_slash_skills_agent(self): - """Alquimia installs `.alquimia/skills/speckit-/SKILL.md`, so the - invocation helper must report the hyphenated form when skills are on. - - It was the only `SkillsIntegration` subclass absent from every set in - `_invocation_style`, so `is_slash_skills_agent` returned False and the - two callers that consult it — `HookExecutor._render_hook_invocation` - and `specify init`'s Next Steps panel — emitted the dotted - `/speckit.` form Alquimia never registers. - """ - from specify_cli._invocation_style import is_slash_skills_agent - - assert is_slash_skills_agent("alquimia", True) is True - # Conditional, not always -- matching the other *conditionally - # classified* skills-only agents. Being skills-only does not by itself - # imply always-slash: Droid, Grok, Trae and Zed are skills-only too and - # sit in `ALWAYS_SLASH_AGENTS`. - # - # This is NOT a claim that the dotted form is ever right for Alquimia: - # it never is. It records what the function returns. The `False` - # argument does not arise in practice because `ai_skills` is persisted - # straight from `is_skills_mode()`, which `SkillsIntegration` returns - # unconditionally -- see - # `test_ai_skills_is_always_persisted_for_alquimia`. - assert is_slash_skills_agent("alquimia", False) is False + def test_integration_owns_conditional_reference_style(self): + integration = get_integration("alquimia") + assert ( + integration.build_command_reference("plan", skills_enabled=True) + == "/speckit-plan" + ) + assert ( + integration.build_command_reference("plan", skills_enabled=False) + == "/speckit.plan" + ) def test_ai_skills_is_always_persisted_for_alquimia(self): """The conditional resolves to True for every real Alquimia project. @@ -82,39 +67,15 @@ def test_ai_skills_is_always_persisted_for_alquimia(self): assert integration.is_skills_mode() is True assert integration.is_skills_mode({}, project_root=None) is True - def test_classified_as_a_conditional_slash_agent(self): - """Alquimia is classified in `CONDITIONAL_SLASH_AGENTS`, not the always set. - - This asserts Alquimia's own classification only. Being a - `SkillsIntegration` does not by itself imply always-slash -- - `ALWAYS_SLASH_AGENTS` and `CONDITIONAL_SLASH_AGENTS` both contain - `SkillsIntegration` subclasses, and the split is a conventional grouping - rather than a rule the code enforces. For a skills-only integration the - two are behaviourally equivalent anyway, since `ai_skills` is always - recorded. - - Other integrations' classification is deliberately NOT asserted here: - that would make this suite fail whenever one of them is legitimately - reclassified, and any fixed peer list is liable to drift out of date. - """ - from specify_cli._invocation_style import ( - ALWAYS_SLASH_AGENTS, - CONDITIONAL_SLASH_AGENTS, - ) - - assert "alquimia" in CONDITIONAL_SLASH_AGENTS - assert "alquimia" not in ALWAYS_SLASH_AGENTS - def test_hook_invocation_renders_the_hyphenated_form( self, tmp_path, monkeypatch ): """The real hook renderer must emit `/speckit-git-commit`. - This drives `HookExecutor._render_hook_invocation` -- one of the two - production consumers of `is_slash_skills_agent` -- rather than - re-deciding the branch in the test. Its non-slash path ends at - `return f"/{command_id}"`, so before this change the renderer emitted - the dotted `/speckit.git.commit`, a command Alquimia never registers. + This drives `HookExecutor._render_hook_invocation` rather than + re-deciding the branch in the test. The renderer delegates to the + resolved integration, so it stays aligned with Alquimia's installed + skill layout. """ import specify_cli from specify_cli.extensions import HookExecutor @@ -130,18 +91,13 @@ def test_hook_invocation_renders_the_hyphenated_form( assert rendered == "/speckit-git-commit" assert "/speckit." not in rendered - def test_build_command_invocation_matches_the_invocation_helper(self): - """The integration's own renderer and the helper must agree. - - `SkillsIntegration.build_command_invocation` already returned - `/speckit-plan`; only the helper disagreed, which is why the two - outputs diverged for the same on-disk layout. - """ - from specify_cli._invocation_style import is_slash_skills_agent - + def test_build_command_invocation_matches_the_reference_renderer(self): integration = get_integration("alquimia") assert integration.build_command_invocation("plan") == "/speckit-plan" - assert is_slash_skills_agent("alquimia", True) is True + assert ( + integration.build_command_reference("plan", skills_enabled=True) + == "/speckit-plan" + ) def test_build_exec_args_uses_headless_prompt_flag(self): """Workflow dispatch relies on the inherited diff --git a/tests/integrations/test_integration_docker_agent.py b/tests/integrations/test_integration_docker_agent.py index 173f75efda..15a7e8a977 100644 --- a/tests/integrations/test_integration_docker_agent.py +++ b/tests/integrations/test_integration_docker_agent.py @@ -16,15 +16,17 @@ class TestDockerAgentIntegration(SkillsIntegrationTests): def test_multi_install_is_opt_in(self): assert DockerAgentIntegration().multi_install_safe is False - def test_is_slash_skills_agent(self): - """Docker Agent only installs `speckit-/SKILL.md` skills, so the - helper must agree with its own renderer with or without `ai_skills` - (always slash, like Zed and Muse Code, which share that layout).""" - from specify_cli._invocation_style import is_slash_skills_agent - - assert DockerAgentIntegration().build_command_invocation("plan") == "/speckit-plan" - assert is_slash_skills_agent("docker-agent", True) is True - assert is_slash_skills_agent("docker-agent", False) is True + def test_integration_owns_always_skill_reference_style(self): + integration = DockerAgentIntegration() + assert integration.build_command_invocation("plan") == "/speckit-plan" + assert ( + integration.build_command_reference("plan", skills_enabled=True) + == "/speckit-plan" + ) + assert ( + integration.build_command_reference("plan", skills_enabled=False) + == "/speckit-plan" + ) def test_hook_invocation_renders_the_hyphenated_form(self, tmp_path, monkeypatch): import specify_cli diff --git a/tests/integrations/test_integration_droid.py b/tests/integrations/test_integration_droid.py index 09848af930..82002bfb52 100644 --- a/tests/integrations/test_integration_droid.py +++ b/tests/integrations/test_integration_droid.py @@ -43,19 +43,16 @@ def test_multi_install_safe_is_true(self): i = get_integration(self.KEY) assert i.multi_install_safe is True - def test_is_slash_skills_agent(self): - """Droid is an always-skills agent whose commands install as - /speckit-, so is_slash_skills_agent must report True — otherwise - hook invocations and the init next-steps panel render the dotted - /speckit. form Droid never registers (mirrors grok/trae/zed/devin).""" - from specify_cli._invocation_style import is_slash_skills_agent - - # True in BOTH the enabled and disabled cases: Droid is *always* slash, - # not conditional. The disabled case is what distinguishes an - # ALWAYS_SLASH agent from a CONDITIONAL_SLASH one (which would be False - # when ai_skills is disabled). - assert is_slash_skills_agent("droid", True) is True - assert is_slash_skills_agent("droid", False) is True + def test_integration_owns_always_skill_reference_style(self): + integration = get_integration(self.KEY) + assert ( + integration.build_command_reference("plan", skills_enabled=True) + == "/speckit-plan" + ) + assert ( + integration.build_command_reference("plan", skills_enabled=False) + == "/speckit-plan" + ) def test_install_url_points_to_factory(self): i = get_integration(self.KEY) diff --git a/tests/integrations/test_integration_dsh.py b/tests/integrations/test_integration_dsh.py index 9b955f3386..8939cefe21 100644 --- a/tests/integrations/test_integration_dsh.py +++ b/tests/integrations/test_integration_dsh.py @@ -198,9 +198,7 @@ def test_dsh_root_does_not_overlap_other_safe_integrations(self): class TestDshHookInvocations: - """DSH is in ALWAYS_SLASH_AGENTS: hook messages and init output must - reference slash-invokable skills regardless of the persisted ai_skills - flag, because the DSH Web GUI invokes skills as ``/speckit-``.""" + """DSH owns a mode-independent slash-invocation style for its skills.""" def test_hooks_render_skill_invocation(self, tmp_path): from specify_cli.extensions import HookExecutor diff --git a/tests/integrations/test_integration_zed.py b/tests/integrations/test_integration_zed.py index 468d91daa6..d2fc8f975b 100644 --- a/tests/integrations/test_integration_zed.py +++ b/tests/integrations/test_integration_zed.py @@ -118,8 +118,8 @@ def test_init_persists_ai_skills_for_zed(self, tmp_path, monkeypatch): assert "EXECUTE_COMMAND_INVOCATION: /speckit-plan" in message -class TestSlashSkillsSets: - """Parameterized coverage for ALWAYS_SLASH_AGENTS / CONDITIONAL_SLASH_AGENTS.""" +class TestIntegrationOwnedInvocationStyles: + """Parameterized coverage for integration-owned hook invocation styles.""" @staticmethod def _render_invocation(project_path, ai: str, ai_skills: bool) -> str: @@ -138,7 +138,7 @@ def _render_invocation(project_path, ai: str, ai_skills: bool) -> str: @pytest.mark.parametrize( ("ai", "ai_skills", "expected"), [ - # ALWAYS_SLASH_AGENTS — unconditional on ai_skills + # Integrations whose skill spelling is mode-independent. ("devin", True, "/speckit-plan"), ("devin", False, "/speckit-plan"), ("grok", True, "/speckit-plan"), @@ -151,7 +151,7 @@ def _render_invocation(project_path, ai: str, ai_skills: bool) -> str: ("trae", False, "/speckit-plan"), ("zed", True, "/speckit-plan"), ("zed", False, "/speckit-plan"), - # CONDITIONAL_SLASH_AGENTS — only when ai_skills is enabled + # Integrations whose skill spelling applies only in skills mode. ("agy", True, "/speckit-plan"), ("agy", False, "/speckit.plan"), ("claude", True, "/speckit-plan"), From d0ff650c74e68402545d1328f36f1846f4da03cb Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Fri, 16 Oct 2026 16:12:00 -0500 Subject: [PATCH 02/10] fix(presets): preserve historical skill references Render restored SKILL.md references from the historical target registrar while retaining persisted mode for dual-layout integrations. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c44efad8-4dc0-4488-a27f-e2abace733ef --- src/specify_cli/presets/_manager_skills.py | 6 ++++- .../presets/test_manager_skills.py | 27 ++++++++++++------- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/specify_cli/presets/_manager_skills.py b/src/specify_cli/presets/_manager_skills.py index 0a0a79acb6..139dbcd25c 100644 --- a/src/specify_cli/presets/_manager_skills.py +++ b/src/specify_cli/presets/_manager_skills.py @@ -551,7 +551,11 @@ def _resolve_skill_command_refs( integration = get_integration(selected_ai) if integration is not None: skills_enabled = True - if project_root is not None: + agent_config = registrar.AGENT_CONFIGS.get(selected_ai, {}) + registrar_writes_skills = ( + agent_config.get("extension") == "/SKILL.md" + ) + if project_root is not None and not registrar_writes_skills: from .. import load_init_options skills_enabled = is_ai_skills_enabled( diff --git a/tests/specify_cli/presets/test_manager_skills.py b/tests/specify_cli/presets/test_manager_skills.py index 7ae833bfb9..a9e5367c9c 100644 --- a/tests/specify_cli/presets/test_manager_skills.py +++ b/tests/specify_cli/presets/test_manager_skills.py @@ -1484,28 +1484,24 @@ def test_remove_cleans_native_skill_missing_from_partial_skill_map( "when another agent makes registered_skills globally non-empty" ) - def test_skill_switch_then_remove_restores_every_skill_agent_dir( + def test_skill_switch_then_command_switch_remove_restores_skill_refs( self, project_dir, temp_dir ): - """Switching between two skill-mode agents before removing a preset - must restore both agents' directories, not just the currently - active one. + """Historical skill restores keep each target integration's references. ``registered_skills`` records exactly which agent directories the preset wrote to (``{agent_name: [skill_name, ...]}``); switching to codex and re-registering adds a "codex" entry alongside the - original "claude" entry, so removal restores both. Before the - provenance fix, ``_unregister_skills`` only restored the currently - active agent's skills directory; a preset used first under Claude - and later switched to Codex would have its Claude override left - behind permanently on removal (#2948). + original "claude" entry. A later switch to command-mode Qwen must not + make those historical SKILL.md restores use dotted command references. """ self._write_init_options(project_dir, ai="claude", ai_skills=True) core_cmds = project_dir / ".specify" / "templates" / "commands" core_cmds.mkdir(parents=True, exist_ok=True) (core_cmds / "specify.md").write_text( - "---\ndescription: Core specify command\n---\n\nCore specify body\n", + "---\ndescription: Core specify command\n---\n\n" + "Core specify body\nUse __SPECKIT_COMMAND_PLAN__ next.\n", encoding="utf-8", ) @@ -1551,8 +1547,17 @@ def test_skill_switch_then_remove_restores_every_skill_agent_dir( "preset actually wrote to (#2948)" ) + # Switch to a command-mode integration before removal. Historical + # skill restoration must use the target registrar's SKILL.md layout, + # not Qwen's active project-wide ai_skills=False state. + self._write_init_options(project_dir, ai="qwen", ai_skills=False) + assert manager.remove("multi-skill-agent-preset") is True + expected_references = { + "claude": "/speckit-plan", + "codex": "$speckit-plan", + } for skill_file, label in ((claude_skill, "claude"), (codex_skill, "codex")): assert skill_file.exists(), f"{label} skill file should still exist after removal" content = skill_file.read_text() @@ -1561,6 +1566,8 @@ def test_skill_switch_then_remove_restores_every_skill_agent_dir( "not orphaned permanently (#2948)" ) assert "Core specify body" in content + assert expected_references[label] in content + assert "/speckit.plan" not in content def test_infer_legacy_skill_provenance_does_not_falsely_attribute_command_mode_copilot( self, project_dir, temp_dir From 2c2952c99a71c3ab75354af9cab1d81e64015290 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Mon, 19 Oct 2026 07:46:24 -0500 Subject: [PATCH 03/10] fix(integrations): harden invocation adapter boundaries Validate public invocation adapter attributes, pin hook rendering to the executor project, and keep skill runtime dispatch aligned with installed hyphenated names. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c44efad8-4dc0-4488-a27f-e2abace733ef --- design/integration.md | 7 +- src/specify_cli/extensions/__init__.py | 14 +-- src/specify_cli/integrations/base.py | 9 +- src/specify_cli/integrations/installer.py | 4 + .../integrations/test_installed_adapters.py | 91 ++++++++++++++++++- 5 files changed, 110 insertions(+), 15 deletions(-) diff --git a/design/integration.md b/design/integration.md index 56d174f630..a7d9616c99 100644 --- a/design/integration.md +++ b/design/integration.md @@ -170,9 +170,10 @@ directory must match `folder/commands_subdir`. Output paths must be canonical, project-local, and outside `.git` and `.specify`. Registration extensions must be plain dotted filename suffixes (such as `.md`) or, for Markdown skills, `/SKILL.md`; path traversal is not allowed. -The public class attribute `invoke_separator` must be a non-empty string; -`dev_no_symlink` and `multi_install_safe` must be booleans, including when -registrar configuration supplies its own optional values. +The public class attributes `invoke_separator` and `invocation_prefix` must be +non-empty strings. `skills_invocation_only`, `dev_no_symlink`, and +`multi_install_safe` must be booleans, including when registrar configuration +supplies its own optional values. An explicit `registrar_config.invoke_separator` takes precedence over the class default. `dev_no_symlink` is enabled when either the class or registrar configuration enables it. Healthy registration and persisted recovery metadata diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index b651d99468..e519b9a16e 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -5666,13 +5666,15 @@ def _render_hook_invocation(self, command: Any) -> str: ai_skills_enabled = is_ai_skills_enabled(init_options) if isinstance(selected_ai, str): from ..integrations import get_integration + from ..integrations.installer import project_integrations - integration = get_integration(selected_ai) - if integration is not None: - return integration.build_hook_invocation( - command_id, - skills_enabled=ai_skills_enabled, - ) + with project_integrations(self.project_root): + integration = get_integration(selected_ai) + if integration is not None: + return integration.build_hook_invocation( + command_id, + skills_enabled=ai_skills_enabled, + ) return f"/{command_id}" diff --git a/src/specify_cli/integrations/base.py b/src/specify_cli/integrations/base.py index 59f0b8ae2f..33f0cd5e9a 100644 --- a/src/specify_cli/integrations/base.py +++ b/src/specify_cli/integrations/base.py @@ -1839,11 +1839,10 @@ def skills_dest(self, project_root: Path) -> Path: def build_command_invocation(self, command_name: str, args: str = "") -> str: """Build the agent's native invocation for a hyphenated skill name.""" - return self.build_command_reference( - command_name, - args, - skills_enabled=True, - ) + stem = command_name.removeprefix("speckit.") + prefix = self.invocation_prefix_for_mode(True) + invocation = prefix + "speckit-" + stem.replace(".", "-") + return f"{invocation} {args}" if args else invocation @staticmethod def _inject_hook_command_note( diff --git a/src/specify_cli/integrations/installer.py b/src/specify_cli/integrations/installer.py index b621a04bbf..f2d3829003 100644 --- a/src/specify_cli/integrations/installer.py +++ b/src/specify_cli/integrations/installer.py @@ -715,6 +715,10 @@ def _validate_implementation( raise IntegrationInstallError(f"Integration '{key}' multi_install_safe must be a boolean") if not isinstance(integration.invoke_separator, str) or not integration.invoke_separator: raise IntegrationInstallError(f"Integration '{key}' invoke_separator must be a non-empty string") + if not isinstance(integration.invocation_prefix, str) or not integration.invocation_prefix: + raise IntegrationInstallError(f"Integration '{key}' invocation_prefix must be a non-empty string") + if not isinstance(integration.skills_invocation_only, bool): + raise IntegrationInstallError(f"Integration '{key}' skills_invocation_only must be a boolean") if not isinstance(integration.dev_no_symlink, bool): raise IntegrationInstallError(f"Integration '{key}' dev_no_symlink must be a boolean") if integration.multi_install_safe: diff --git a/tests/specify_cli/integrations/test_installed_adapters.py b/tests/specify_cli/integrations/test_installed_adapters.py index 6f8fbadd92..e8020aa581 100644 --- a/tests/specify_cli/integrations/test_installed_adapters.py +++ b/tests/specify_cli/integrations/test_installed_adapters.py @@ -1128,7 +1128,11 @@ def fail_commit(*args, **kwargs): @pytest.mark.parametrize("field,value", [ ("invoke_separator", None), ("invoke_separator", ""), ("invoke_separator", 1), - ("invoke_separator", False), ("dev_no_symlink", "false"), + ("invoke_separator", False), ("invocation_prefix", None), + ("invocation_prefix", ""), ("invocation_prefix", 1), + ("invocation_prefix", False), ("skills_invocation_only", "false"), + ("skills_invocation_only", 1), ("skills_invocation_only", None), + ("dev_no_symlink", "false"), ("dev_no_symlink", 1), ("dev_no_symlink", None), ]) def test_round5_invalid_public_adapter_attributes_are_rejected(tmp_path, server, field, value): @@ -1161,6 +1165,33 @@ def test_round5_valid_public_attributes_propagate_to_rendering_and_recovery(tmp_ assert binding["files"] == record["files"] +def test_skill_dispatch_keeps_installed_name_with_reference_separator_override( + tmp_path, server +): + from specify_cli.integrations import get_integration + + publish( + server, + code=implementation(body=' invoke_separator = "_"\n'), + ) + project = catalog_project(tmp_path, server) + install(project) + load_installed_integrations(project) + integration = get_integration(KEY) + + assert integration is not None + assert ( + integration.build_command_reference( + "speckit.git.commit", skills_enabled=True + ) + == "/speckit_git_commit" + ) + assert ( + integration.build_command_invocation("speckit.git.commit") + == "/speckit-git-commit" + ) + + @pytest.mark.parametrize("damage", ["package", "files", "project"]) def test_round5_recovery_checks_local_package_identity(tmp_path, server, damage): from specify_cli.integrations.installer import _recovery_identity @@ -3555,6 +3586,64 @@ def snapshot_then_interleave(): assert not (first / ".other-agent").exists() +@pytest.mark.parametrize("scenario", ["fresh-process", "interleaved-project"]) +def test_hook_rendering_uses_executor_project_adapter( + tmp_path, server, scenario +): + from specify_cli.extensions import HookExecutor + + def package(marker, folder): + body = ''' def build_hook_invocation(self, command_name, *, skills_enabled): + from specify_cli.integrations import get_integration + assert get_integration(self.key) is self, "hook renderer escaped project scope" + from .helper import MARKER + return MARKER +''' + publish( + server, + code=implementation(folder=folder, body=body), + members={"helper.py": f"MARKER = {marker!r}\n".encode()}, + ) + + package("/first-project-hook", ".sample-agent") + first = catalog_project(tmp_path, server) + install(first) + (first / ".specify/init-options.json").write_text( + json.dumps({"ai": KEY, "ai_skills": True}), + encoding="utf-8", + ) + + if scenario == "fresh-process": + code = ( + "from pathlib import Path\n" + "from specify_cli.extensions import HookExecutor\n" + f"root = Path({str(first)!r})\n" + "print(HookExecutor(root)._render_hook_invocation('speckit.plan'))\n" + ) + process = subprocess.run( + [sys.executable, "-c", code], + cwd=first, + capture_output=True, + text=True, + encoding="utf-8", + check=False, + ) + assert process.returncode == 0, process.stderr + assert process.stdout.strip() == "/first-project-hook" + return + + package("/second-project-hook", ".other-agent") + second = catalog_project(tmp_path / "second", server) + install(second) + load_installed_integrations(second) + + assert ( + HookExecutor(first)._render_hook_invocation("speckit.plan") + == "/first-project-hook" + ) + assert not (first / ".other-agent").exists() + + @pytest.mark.parametrize("consumer", ["extension", "preset"]) def test_skill_directory_uses_pinned_project_configuration(tmp_path, server, monkeypatch, consumer): import specify_cli From dcf833d3fadb48e24f45a174bf761480332be009 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Mon, 19 Oct 2026 09:03:55 -0500 Subject: [PATCH 04/10] fix(integrations): pin cleanup and template modes Keep command removal scoped to the target project's adapter registry and pass resolved output mode independently from command-reference spelling. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c44efad8-4dc0-4488-a27f-e2abace733ef --- src/specify_cli/agents.py | 1 + src/specify_cli/integrations/base.py | 17 ++++- .../integrations/copilot/__init__.py | 1 + .../integrations/forge/__init__.py | 1 + .../integrations/generic/__init__.py | 2 + .../integrations/hermes/__init__.py | 1 + .../integrations/test_installed_adapters.py | 69 +++++++++++++++++++ 7 files changed, 89 insertions(+), 3 deletions(-) diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index 23173e3ec1..7e2f28da8e 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -1381,6 +1381,7 @@ def register_commands_for_non_skill_agents( continue return results + @project_registration def unregister_commands( self, registered_commands: Dict[str, List[str]], project_root: Path ) -> None: diff --git a/src/specify_cli/integrations/base.py b/src/specify_cli/integrations/base.py index 33f0cd5e9a..84834d436f 100644 --- a/src/specify_cli/integrations/base.py +++ b/src/specify_cli/integrations/base.py @@ -973,6 +973,7 @@ def process_template( arg_placeholder: str = "$ARGUMENTS", invoke_separator: str = ".", project_root: Path | None = None, + skills_enabled: bool | None = None, ) -> str: """Process a raw command template into agent-ready content. @@ -1073,16 +1074,22 @@ def process_template( # 8. Replace __SPECKIT_COMMAND___ with invocation strings. # Integration classes own their native prefix; the separator remains - # an explicit rendering input because dual-layout setup paths resolve - # it from the selected mode before calling this shared transformer. + # an explicit rendering input because adapters may use a non-standard + # spelling. Output mode is separate because it can affect the prefix + # without implying a particular separator. invocation_prefix = "/" try: from specify_cli.integrations import get_integration integration = get_integration(agent_name) if integration is not None: - invocation_prefix = integration.invocation_prefix_for_mode( + resolved_skills_enabled = ( invoke_separator == "-" + if skills_enabled is None + else skills_enabled + ) + invocation_prefix = integration.invocation_prefix_for_mode( + resolved_skills_enabled ) except (ImportError, KeyError, ValueError): pass @@ -1292,6 +1299,7 @@ def setup( processed = self.process_template( raw, self.key, script_type, arg_placeholder, project_root=project_root, + skills_enabled=False, ) dst_name = self.command_filename(src_file.stem) dst_file = self.write_file_and_record( @@ -1507,6 +1515,7 @@ def setup( processed = self.process_template( raw, self.key, script_type, arg_placeholder, project_root=project_root, + skills_enabled=False, ) _, body = self._split_frontmatter(processed) toml_content = self._render_toml(description, body) @@ -1747,6 +1756,7 @@ def setup( processed = self.process_template( raw, self.key, script_type, arg_placeholder, project_root=project_root, + skills_enabled=False, ) _, body = self._split_frontmatter(processed) yaml_content = self._render_yaml( @@ -1982,6 +1992,7 @@ def setup( raw, self.key, script_type, arg_placeholder, project_root=project_root, invoke_separator=self.invoke_separator, + skills_enabled=True, ) # Strip the processed frontmatter — we rebuild it for skills. # Preserve leading whitespace in the body to match release ZIP diff --git a/src/specify_cli/integrations/copilot/__init__.py b/src/specify_cli/integrations/copilot/__init__.py index c41f55e87a..1c27acf9d7 100644 --- a/src/specify_cli/integrations/copilot/__init__.py +++ b/src/specify_cli/integrations/copilot/__init__.py @@ -566,6 +566,7 @@ def _setup_commands( processed = self.process_template( raw, self.key, script_type, arg_placeholder, project_root=project_root, + skills_enabled=False, ) dst_name = self.command_filename(src_file.stem) dst_file = self.write_file_and_record( diff --git a/src/specify_cli/integrations/forge/__init__.py b/src/specify_cli/integrations/forge/__init__.py index 69407ecdbf..b0b4e047e9 100644 --- a/src/specify_cli/integrations/forge/__init__.py +++ b/src/specify_cli/integrations/forge/__init__.py @@ -186,6 +186,7 @@ def setup( raw, self.key, script_type, arg_placeholder, invoke_separator=self.invoke_separator, project_root=project_root, + skills_enabled=False, ) # FORGE-SPECIFIC: Ensure any remaining $ARGUMENTS placeholders are diff --git a/src/specify_cli/integrations/generic/__init__.py b/src/specify_cli/integrations/generic/__init__.py index dee3d4404d..bbc7d0fe65 100644 --- a/src/specify_cli/integrations/generic/__init__.py +++ b/src/specify_cli/integrations/generic/__init__.py @@ -192,6 +192,7 @@ def _build_skill_content( raw, self.key, script_type, "$ARGUMENTS", project_root=project_root, invoke_separator="-", + skills_enabled=True, ) if processed_body.startswith("---"): body_lines = processed_body.splitlines(keepends=True) @@ -283,6 +284,7 @@ def setup( processed = self.process_template( raw, self.key, script_type, arg_placeholder, project_root=project_root, + skills_enabled=False, ) dst_name = self.command_filename(src_file.stem) dst_file = self.write_file_and_record( diff --git a/src/specify_cli/integrations/hermes/__init__.py b/src/specify_cli/integrations/hermes/__init__.py index 36160100c9..4e5d30b10c 100644 --- a/src/specify_cli/integrations/hermes/__init__.py +++ b/src/specify_cli/integrations/hermes/__init__.py @@ -157,6 +157,7 @@ def setup( arg_placeholder, invoke_separator=self.invoke_separator, project_root=project_root, + skills_enabled=True, ) # Strip the processed frontmatter — we rebuild it for skills. # Scan for the closing ``---`` on its own line rather than diff --git a/tests/specify_cli/integrations/test_installed_adapters.py b/tests/specify_cli/integrations/test_installed_adapters.py index e8020aa581..0764dd8ac2 100644 --- a/tests/specify_cli/integrations/test_installed_adapters.py +++ b/tests/specify_cli/integrations/test_installed_adapters.py @@ -1192,6 +1192,31 @@ def test_skill_dispatch_keeps_installed_name_with_reference_separator_override( ) +def test_skill_templates_use_skills_prefix_with_reference_separator_override( + tmp_path, server +): + publish( + server, + code=implementation( + body=( + ' invoke_separator = "_"\n' + ' invocation_prefix = "$"\n' + " skills_invocation_only = True\n" + ) + ), + ) + project = catalog_project(tmp_path, server) + install(project) + + content = ( + project / ".sample-agent/skills/speckit-specify/SKILL.md" + ).read_text() + assert "$speckit_specify" in content + assert "$speckit_plan" in content + assert "/speckit_specify" not in content + assert "/speckit_plan" not in content + + @pytest.mark.parametrize("damage", ["package", "files", "project"]) def test_round5_recovery_checks_local_package_identity(tmp_path, server, damage): from specify_cli.integrations.installer import _recovery_identity @@ -3586,6 +3611,50 @@ def snapshot_then_interleave(): assert not (first / ".other-agent").exists() +def test_unregistration_keeps_requested_project_naming_hook( + tmp_path, server +): + def package(prefix, folder): + body = f''' def format_registered_command_name(self, command_name, registrar_config=None): + return {prefix!r} + command_name.removeprefix("speckit.") +''' + publish( + server, + code=implementation(flavor="markdown", folder=folder, body=body), + ) + + package("first-", ".sample-agent") + first = catalog_project(tmp_path, server) + install(first) + registrar = CommandRegistrar(first) + source = tmp_path / "source" + source.mkdir() + (source / "command.md").write_text( + "---\ndescription: Sample command\n---\nBody\n" + ) + registrar.register_commands( + KEY, + [{"name": "speckit.sample", "file": "command.md"}], + "sample", + source, + first, + ) + first_output = first / ".sample-agent/commands/first-sample.md" + unrelated_output = first / ".sample-agent/commands/second-sample.md" + unrelated_output.write_text("unrelated project output\n") + + package("second-", ".other-agent") + second = catalog_project(tmp_path / "second", server) + install(second) + load_installed_integrations(second) + + registrar.unregister_commands({KEY: ["speckit.sample"]}, first) + + assert not first_output.exists() + assert unrelated_output.read_text() == "unrelated project output\n" + assert not (first / ".other-agent").exists() + + @pytest.mark.parametrize("scenario", ["fresh-process", "interleaved-project"]) def test_hook_rendering_uses_executor_project_adapter( tmp_path, server, scenario From 2145e7386766498eae63b57f0890284c00540b0e Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Mon, 19 Oct 2026 09:38:03 -0500 Subject: [PATCH 05/10] fix(integrations): scope artifact naming lookups Pin integration-owned command naming to the target project during artifact resolution and extension update backup and rollback paths. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c44efad8-4dc0-4488-a27f-e2abace733ef --- src/specify_cli/artifacts/resolution.py | 12 +- .../extensions/_command_update_transaction.py | 19 +- .../integrations/test_installed_adapters.py | 213 ++++++++++++++++++ 3 files changed, 237 insertions(+), 7 deletions(-) diff --git a/src/specify_cli/artifacts/resolution.py b/src/specify_cli/artifacts/resolution.py index 43eea57f61..6d811a204d 100644 --- a/src/specify_cli/artifacts/resolution.py +++ b/src/specify_cli/artifacts/resolution.py @@ -202,7 +202,10 @@ def _materialized_command_source_path( except ImportError: return None - from ..integrations.installer import IntegrationInstallError + from ..integrations.installer import ( + IntegrationInstallError, + project_integrations, + ) try: registrar = CommandRegistrar(project_root) @@ -221,9 +224,10 @@ def _materialized_command_source_path( agent_config = registrar.AGENT_CONFIGS.get(agent_name) if agent_config is None: continue - output_name = registrar._compute_output_name( - agent_name, name, agent_config - ) + with project_integrations(project_root): + output_name = registrar._compute_output_name( + agent_name, name, agent_config + ) command_path = ( registrar._resolve_agent_dir(agent_name, agent_config, project_root) / f"{output_name}{agent_config['extension']}" diff --git a/src/specify_cli/extensions/_command_update_transaction.py b/src/specify_cli/extensions/_command_update_transaction.py index 585b3cc2bf..19eb193d6f 100644 --- a/src/specify_cli/extensions/_command_update_transaction.py +++ b/src/specify_cli/extensions/_command_update_transaction.py @@ -83,6 +83,17 @@ def run_update_command(extension: str | None) -> None: ) from exc hook_executor = HookExecutor(project_root) from ..agents import CommandRegistrar as _AgentReg # used in backup and rollback paths + from ..integrations.installer import project_integrations + + def project_output_name( + agent_name: str, + command_name: str, + agent_config: dict, + ) -> str: + with project_integrations(project_root): + return _AgentReg._compute_output_name( + agent_name, command_name, agent_config + ) # UNSET sentinel: backup not yet captured (exception before backup step) UNSET = object() @@ -288,7 +299,7 @@ def backup_extension_skills(skill_names, *, skills_dir=None): dirs_to_backup.append(legacy_dir) for cmd_name in cmd_names: - output_name = _AgentReg._compute_output_name( + output_name = project_output_name( agent_name, cmd_name, agent_config ) names_to_backup = [output_name] @@ -413,7 +424,7 @@ def backup_extension_skills(skill_names, *, skills_dir=None): ) in manager._command_registration_targets().items(): agent_config = registrar.AGENT_CONFIGS[agent_name] for command_name in new_command_names: - output_name = _AgentReg._compute_output_name( + output_name = project_output_name( agent_name, command_name, agent_config ) command_file = ( @@ -733,7 +744,9 @@ def backup_extension_skills(skill_names, *, skills_dir=None): ) for cmd_name in cmd_names: - output_name = _AgentReg._compute_output_name(agent_name, cmd_name, agent_config) + output_name = project_output_name( + agent_name, cmd_name, agent_config + ) cmd_file = commands_dir / f"{output_name}{agent_config['extension']}" # Delete if it exists and wasn't in our backup if cmd_file.exists() and str(cmd_file) not in backed_up_command_files: diff --git a/tests/specify_cli/integrations/test_installed_adapters.py b/tests/specify_cli/integrations/test_installed_adapters.py index 0764dd8ac2..e374a295d1 100644 --- a/tests/specify_cli/integrations/test_installed_adapters.py +++ b/tests/specify_cli/integrations/test_installed_adapters.py @@ -3655,6 +3655,219 @@ def package(prefix, folder): assert not (first / ".other-agent").exists() +def test_artifact_resolution_keeps_requested_project_naming_hook( + tmp_path, server, monkeypatch +): + from specify_cli.artifacts.resolution import _materialized_command_source_path + + def package(prefix, folder): + body = f''' def format_registered_command_name(self, command_name, registrar_config=None): + return {prefix!r} + command_name.removeprefix("speckit.") +''' + publish( + server, + code=implementation(flavor="markdown", folder=folder, body=body), + ) + + package("first-", ".sample-agent") + first = catalog_project(tmp_path, server) + install(first) + source = tmp_path / "source" + source.mkdir() + (source / "command.md").write_text( + "---\ndescription: Sample command\n---\nBody\n" + ) + CommandRegistrar(first).register_commands( + KEY, + [{"name": "speckit.sample", "file": "command.md"}], + "sample", + source, + first, + ) + + package("second-", ".other-agent") + second = catalog_project(tmp_path / "second", server) + install(second) + original_init = CommandRegistrar.__init__ + + def snapshot_first_then_interleave(registrar, *args, **kwargs): + original_init(registrar, *args, **kwargs) + load_installed_integrations(second) + + monkeypatch.setattr( + CommandRegistrar, "__init__", snapshot_first_then_interleave + ) + + assert _materialized_command_source_path( + first, + {"registered_commands": {KEY: ["speckit.sample"]}}, + "speckit.sample", + source="extension", + ) == ".sample-agent/commands/first-sample.md" + + +def test_failed_extension_update_restores_project_named_command( + tmp_path, server, monkeypatch +): + from specify_cli.extensions import ( + ExtensionCatalog, + ExtensionManager, + ExtensionRegistry, + HookExecutor, + ) + from specify_cli.extensions import _command_update_transaction + from specify_cli.extensions._command_update_discovery import UpdateCandidate + + def package(prefix, folder): + body = f''' def format_registered_command_name(self, command_name, registrar_config=None): + return {prefix!r} + command_name.removeprefix("speckit.") +''' + publish( + server, + code=implementation(flavor="markdown", folder=folder, body=body), + ) + + package("first-", ".sample-agent") + first = catalog_project(tmp_path, server) + install(first) + (first / ".specify/config.toml").write_text("ai = 'sample-agent'\n") + source = tmp_path / "source" + source.mkdir() + (source / "command.md").write_text( + "---\ndescription: Sample command\n---\nOriginal body\n" + ) + command_name = "speckit.sample" + CommandRegistrar(first).register_commands( + KEY, + [{"name": command_name, "file": "command.md"}], + "test-ext", + source, + first, + ) + command_file = first / ".sample-agent/commands/first-sample.md" + original_content = command_file.read_text() + + extension_dir = first / ".specify/extensions/test-ext" + extension_dir.mkdir(parents=True) + (extension_dir / "extension.yml").write_text( + yaml.safe_dump( + { + "schema_version": "1.0", + "extension": { + "id": "test-ext", + "name": "Test Ext", + "version": "1.0.0", + "description": "Project-scoped rollback test", + }, + "requires": {"speckit_version": ">=0"}, + "provides": { + "commands": [ + {"name": command_name, "file": "commands/run.md"} + ] + }, + } + ) + ) + registry_entry = { + "version": "1.0.0", + "enabled": True, + "registered_commands": {KEY: [command_name]}, + "registered_skills": [], + } + + archive = first / "update.zip" + with zipfile.ZipFile(archive, "w") as package_archive: + package_archive.writestr( + "extension.yml", + yaml.safe_dump( + { + "schema_version": "1.0", + "extension": { + "id": "test-ext", + "name": "Test Ext", + "version": "1.1.0", + "description": "Project-scoped rollback test", + }, + "requires": {"speckit_version": ">=0"}, + "provides": { + "commands": [ + { + "name": command_name, + "file": "commands/run.md", + } + ] + }, + } + ), + ) + package_archive.writestr( + "commands/run.md", + "---\ndescription: Updated command\n---\nUpdated body\n", + ) + + candidate = UpdateCandidate( + extension_id="test-ext", + name="Test Ext", + installed="1.0.0", + available="1.1.0", + download_url="https://example.com/test-ext.zip", + bundled_dir=None, + catalog_name=None, + ) + monkeypatch.setattr( + _command_update_transaction, + "discover_updates", + lambda manager, catalog, extension: ([candidate], [], True), + ) + monkeypatch.setattr("typer.confirm", lambda _: True) + monkeypatch.setattr( + ExtensionCatalog, + "download_extension", + lambda self, extension_id: archive, + ) + monkeypatch.setattr( + ExtensionRegistry, + "get", + lambda self, extension_id: registry_entry, + ) + + package("second-", ".other-agent") + second = catalog_project(tmp_path / "second", server) + install(second) + original_hook_init = HookExecutor.__init__ + + def interleave_after_registrar_snapshot(executor, project_root): + original_hook_init(executor, project_root) + load_installed_integrations(second) + + monkeypatch.setattr( + HookExecutor, "__init__", interleave_after_registrar_snapshot + ) + + def remove_old_extension(manager, extension_id, keep_config=False): + CommandRegistrar(first).unregister_commands( + {KEY: [command_name]}, first + ) + load_installed_integrations(second) + + monkeypatch.setattr(ExtensionManager, "remove", remove_old_extension) + monkeypatch.setattr( + ExtensionManager, + "install_from_zip", + lambda *args, **kwargs: (_ for _ in ()).throw( + RuntimeError("Install failed after command cleanup") + ), + ) + + result = run(first, ["extension", "update", "test-ext"]) + + assert result.exit_code == 1 + assert "Install failed after command cleanup" in result.output + assert "Rollback successful" in result.output + assert command_file.read_text() == original_content + assert not (first / ".other-agent").exists() + + @pytest.mark.parametrize("scenario", ["fresh-process", "interleaved-project"]) def test_hook_rendering_uses_executor_project_adapter( tmp_path, server, scenario From ecb79db0922a606a6fc156689ee6177e3d892b8c Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Mon, 19 Oct 2026 10:29:26 -0500 Subject: [PATCH 06/10] refactor(extensions): reuse command reference resolvers Delegate extension skill placeholders to integration-owned and base fallback resolvers so grammar and override behavior stay aligned. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c44efad8-4dc0-4488-a27f-e2abace733ef --- src/specify_cli/extensions/__init__.py | 22 +++---- tests/test_extensions.py | 79 ++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 13 deletions(-) diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index e519b9a16e..a6ac463c7a 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -1614,6 +1614,7 @@ def _register_extension_skills( from ..agents import CommandRegistrar from ..integrations import get_integration from ..integrations._file_changes import changing_file, unlink, write_text + from ..integrations.base import IntegrationBase written: List[str] = [] opts = load_init_options(self.project_root) @@ -1629,19 +1630,14 @@ def _register_extension_skills( def _resolve_command_ref_tokens(body: str) -> str: """Resolve explicit command-ref tokens with the active skill style.""" - - def _replacement(match: re.Match[str]) -> str: - command_name = "speckit." + match.group(1).lower().replace("_", ".") - if integration is not None: - return integration.build_command_reference( - command_name, - skills_enabled=ai_skills_enabled, - ) - separator = agent_config.get("invoke_separator", ".") - return "/" + command_name.replace(".", separator) - - return re.sub( - r"__SPECKIT_COMMAND_([A-Z][A-Z0-9_-]*)__", _replacement, body + if integration is not None: + return integration.resolve_command_refs_for_mode( + body, + skills_enabled=ai_skills_enabled, + ) + return IntegrationBase.resolve_command_refs( + body, + agent_config.get("invoke_separator", "."), ) for cmd_info in manifest.commands: diff --git a/tests/test_extensions.py b/tests/test_extensions.py index b512742389..d46a655135 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -3982,6 +3982,85 @@ def test_codex_skill_registration_uses_dollar_command_refs( assert "$speckit-plan" in content assert "/speckit-plan" not in content + def test_skill_registration_uses_integration_command_ref_resolver( + self, extension_dir, project_dir, monkeypatch + ): + from specify_cli.integrations import get_integration + + skills_dir = project_dir / ".agents" / "skills" + skills_dir.mkdir(parents=True) + command = extension_dir / "commands" / "hello.md" + command.write_text( + "---\ndescription: Test hello command\n---\n\n" + "Run __SPECKIT_COMMAND_PLAN__.", + encoding="utf-8", + ) + integration = get_integration("codex") + assert integration is not None + calls = [] + + def resolve(content, *, skills_enabled): + calls.append((content, skills_enabled)) + return content.replace( + "__SPECKIT_COMMAND_PLAN__", "custom-resolved-plan" + ) + + monkeypatch.setattr( + integration, "resolve_command_refs_for_mode", resolve + ) + + manifest = ExtensionManifest(extension_dir / "extension.yml") + CommandRegistrar().register_commands_for_agent( + "codex", manifest, extension_dir, project_dir + ) + + content = ( + skills_dir / "speckit-test-ext-hello" / "SKILL.md" + ).read_text(encoding="utf-8") + assert "custom-resolved-plan" in content + assert "__SPECKIT_COMMAND_PLAN__" not in content + assert calls and calls[0][1] is True + + def test_skill_registration_uses_base_command_ref_resolver_fallback( + self, extension_dir, project_dir, monkeypatch + ): + from specify_cli.integrations import base, get_integration + + skills_dir = project_dir / ".agents" / "skills" + skills_dir.mkdir(parents=True) + command = extension_dir / "commands" / "hello.md" + command.write_text( + "---\ndescription: Test hello command\n---\n\n" + "Run __SPECKIT_COMMAND_PLAN__.", + encoding="utf-8", + ) + original_get_integration = get_integration + monkeypatch.setattr( + "specify_cli.integrations.get_integration", + lambda key: None if key == "codex" else original_get_integration(key), + ) + calls = [] + + def resolve(content, separator=".", prefix="/"): + calls.append((content, separator, prefix)) + return content.replace( + "__SPECKIT_COMMAND_PLAN__", "fallback-resolved-plan" + ) + + monkeypatch.setattr(base.IntegrationBase, "resolve_command_refs", resolve) + + manifest = ExtensionManifest(extension_dir / "extension.yml") + CommandRegistrar().register_commands_for_agent( + "codex", manifest, extension_dir, project_dir + ) + + content = ( + skills_dir / "speckit-test-ext-hello" / "SKILL.md" + ).read_text(encoding="utf-8") + assert "fallback-resolved-plan" in content + assert "__SPECKIT_COMMAND_PLAN__" not in content + assert calls and calls[0][1:] == ("-", "/") + def test_codex_skill_registration_resolves_script_placeholders(self, project_dir, temp_dir): """Codex SKILL.md overrides should resolve script placeholders.""" import yaml From 40c34802dda8d3da0d78f432a1e530f46521dcc7 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:58:32 -0500 Subject: [PATCH 07/10] fix(integrations): honor core reference overrides Route core template placeholders through integration-owned resolver seams while preserving explicit separator behavior for the default renderer. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c44efad8-4dc0-4488-a27f-e2abace733ef --- src/specify_cli/integrations/base.py | 48 ++++++++++++----- tests/integrations/test_base.py | 79 ++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+), 13 deletions(-) diff --git a/src/specify_cli/integrations/base.py b/src/specify_cli/integrations/base.py index 84834d436f..b8d437c9b3 100644 --- a/src/specify_cli/integrations/base.py +++ b/src/specify_cli/integrations/base.py @@ -280,8 +280,19 @@ def resolve_command_refs_for_mode( content: str, *, skills_enabled: bool, + invoke_separator: str | None = None, ) -> str: """Replace command-reference placeholders using this integration.""" + if ( + invoke_separator is not None + and type(self).build_command_reference + is IntegrationBase.build_command_reference + ): + return IntegrationBase.resolve_command_refs( + content, + invoke_separator, + self.invocation_prefix_for_mode(skills_enabled), + ) return re.sub( r"__SPECKIT_COMMAND_([A-Z][A-Z0-9_-]*)__", lambda match: self.build_command_reference( @@ -1077,25 +1088,36 @@ def process_template( # an explicit rendering input because adapters may use a non-standard # spelling. Output mode is separate because it can affect the prefix # without implying a particular separator. - invocation_prefix = "/" try: from specify_cli.integrations import get_integration integration = get_integration(agent_name) - if integration is not None: - resolved_skills_enabled = ( - invoke_separator == "-" - if skills_enabled is None - else skills_enabled + except (ImportError, KeyError, ValueError): + integration = None + if integration is not None: + resolved_skills_enabled = ( + invoke_separator == "-" + if skills_enabled is None + else skills_enabled + ) + if ( + type(integration).resolve_command_refs_for_mode + is IntegrationBase.resolve_command_refs_for_mode + ): + content = integration.resolve_command_refs_for_mode( + content, + skills_enabled=resolved_skills_enabled, + invoke_separator=invoke_separator, ) - invocation_prefix = integration.invocation_prefix_for_mode( - resolved_skills_enabled + else: + content = integration.resolve_command_refs_for_mode( + content, + skills_enabled=resolved_skills_enabled, ) - except (ImportError, KeyError, ValueError): - pass - content = IntegrationBase.resolve_command_refs( - content, invoke_separator, invocation_prefix - ) + else: + content = IntegrationBase.resolve_command_refs( + content, invoke_separator + ) return content diff --git a/tests/integrations/test_base.py b/tests/integrations/test_base.py index 51d43e6a29..1e4c8b6a1c 100644 --- a/tests/integrations/test_base.py +++ b/tests/integrations/test_base.py @@ -392,6 +392,85 @@ def test_process_template_skills_separator(self): assert "/speckit-plan" in result assert "__SPECKIT_COMMAND_" not in result + def test_process_template_uses_integration_resolver_override( + self, monkeypatch + ): + class ResolverIntegration(StubIntegration): + def resolve_command_refs_for_mode( + self, content, *, skills_enabled + ): + assert skills_enabled is True + return content.replace( + "__SPECKIT_COMMAND_PLAN__", "/native-plan" + ) + + monkeypatch.setattr( + "specify_cli.integrations.get_integration", + lambda key: ResolverIntegration() if key == "resolver" else None, + ) + content = "---\ndescription: test\n---\nRun __SPECKIT_COMMAND_PLAN__ now." + + result = IntegrationBase.process_template( + content, + "resolver", + "sh", + invoke_separator="_", + skills_enabled=True, + ) + + assert "/native-plan" in result + assert "/speckit_plan" not in result + + def test_process_template_uses_command_reference_override( + self, monkeypatch + ): + class ReferenceIntegration(StubIntegration): + def build_command_reference( + self, command_name, args="", *, skills_enabled + ): + assert command_name == "speckit.plan" + assert skills_enabled is True + return "/native-plan" + + monkeypatch.setattr( + "specify_cli.integrations.get_integration", + lambda key: ReferenceIntegration() if key == "reference" else None, + ) + content = "---\ndescription: test\n---\nRun __SPECKIT_COMMAND_PLAN__ now." + + result = IntegrationBase.process_template( + content, + "reference", + "sh", + invoke_separator="_", + skills_enabled=True, + ) + + assert "/native-plan" in result + assert "/speckit_plan" not in result + + def test_process_template_keeps_explicit_separator_for_default_resolver( + self, monkeypatch + ): + integration = StubIntegration() + integration.invocation_prefix = "$" + monkeypatch.setattr( + "specify_cli.integrations.get_integration", + lambda key: integration if key == "stub" else None, + ) + content = "---\ndescription: test\n---\nRun __SPECKIT_COMMAND_PLAN__ now." + + result = IntegrationBase.process_template( + content, + "stub", + "sh", + invoke_separator="_", + skills_enabled=True, + ) + + assert "$speckit_plan" in result + assert "$speckit.plan" not in result + def test_unclosed_placeholder_unchanged(self): text = "Run __SPECKIT_COMMAND_PLAN to plan." assert IntegrationBase.resolve_command_refs(text, ".") == text From d54944e89a192df380d5f8330d2005ea6b5676b7 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Thu, 29 Oct 2026 16:18:48 -0500 Subject: [PATCH 08/10] fix(integrations): honor shared template reference overrides Pass the active integration and resolved output mode through shared template installation and refresh. Preserve static fallback rendering and shared script behavior, and cover lifecycle overrides and resolver failures. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- design/integration.md | 6 + src/specify_cli/__init__.py | 20 ++- src/specify_cli/command_init.py | 4 + src/specify_cli/integrations/_helpers.py | 4 + .../integrations/command_install.py | 4 + .../integrations/command_switch.py | 4 + .../integrations/command_upgrade.py | 8 + src/specify_cli/shared_infra.py | 37 ++++- tests/integrations/test_cli.py | 138 ++++++++++++++++++ 9 files changed, 218 insertions(+), 7 deletions(-) diff --git a/design/integration.md b/design/integration.md index a7d9616c99..a68bdff9a1 100644 --- a/design/integration.md +++ b/design/integration.md @@ -81,6 +81,12 @@ their naming too. `build_command_invocation()` remains the runtime dispatch seam for integrations such as Bob and Copilot whose executable behavior depends on the installed layout. +Shared page-template installation and refresh use the active integration's +`resolve_command_refs_for_mode()` with its resolved output mode, honoring both +resolver and `build_command_reference()` overrides. Without an active integration, +they retain the static separator/prefix fallback. Shared executable scripts keep +their existing separator/prefix rendering. + Core templates that call scripts declare `sh`, `ps`, and `py` commands in `scripts:` frontmatter; template processing replaces `{SCRIPT}` with the selected variant. `py` is opt-in; non-interactive init defaults to `sh` on diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 181b0feec1..98e723d16e 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -38,6 +38,7 @@ install_shared_infra as _install_shared_infra_impl, refresh_shared_templates as _refresh_shared_templates_impl, ) +from .integrations.base import IntegrationBase from ._console import ( BANNER as BANNER, @@ -117,6 +118,8 @@ def _refresh_shared_templates( invoke_separator: str, invoke_prefix: str = "/", force: bool = False, + integration: IntegrationBase | None = None, + skills_enabled: bool = False, ) -> None: """Refresh default-sensitive shared templates without touching scripts.""" _refresh_shared_templates_impl( @@ -128,6 +131,8 @@ def _refresh_shared_templates( invoke_separator=invoke_separator, invoke_prefix=invoke_prefix, force=force, + integration=integration, + skills_enabled=skills_enabled, ) @@ -140,6 +145,8 @@ def _install_shared_infra( invoke_prefix: str = "/", refresh_managed: bool = False, refresh_hint: str | None = None, + integration: IntegrationBase | None = None, + skills_enabled: bool = False, ) -> bool: """Install shared infrastructure files into *project_path*. @@ -148,9 +155,10 @@ def _install_shared_infra( installs PowerShell, and ``py`` installs Python plus the platform shell fallback. Tracks all installed files in ``speckit.manifest.json``. - Shared scripts and page templates are processed to resolve - ``__SPECKIT_COMMAND___`` placeholders using *invoke_separator* - (``"."`` for markdown agents, ``"-"`` for skills agents). + Page templates resolve ``__SPECKIT_COMMAND___`` placeholders through + *integration* with the resolved *skills_enabled* mode when supplied. + Shared scripts and the no-integration template fallback use *invoke_separator* + and *invoke_prefix* (``"."`` for markdown agents, ``"-"`` for skills agents). Overwrite policy: @@ -184,6 +192,8 @@ def _install_shared_infra( invoke_prefix=invoke_prefix, refresh_managed=refresh_managed, refresh_hint=refresh_hint, + integration=integration, + skills_enabled=skills_enabled, ) @@ -196,6 +206,8 @@ def _install_shared_infra_or_exit( invoke_prefix: str = "/", refresh_managed: bool = False, refresh_hint: str | None = None, + integration: IntegrationBase | None = None, + skills_enabled: bool = False, ) -> bool: try: return _install_shared_infra( @@ -207,6 +219,8 @@ def _install_shared_infra_or_exit( invoke_prefix=invoke_prefix, refresh_managed=refresh_managed, refresh_hint=refresh_hint, + integration=integration, + skills_enabled=skills_enabled, ) except (ValueError, OSError) as exc: console.print(f"[red]Error:[/red] Failed to install shared infrastructure: {exc}") diff --git a/src/specify_cli/command_init.py b/src/specify_cli/command_init.py index aa09ff9da0..c94ebc3e98 100644 --- a/src/specify_cli/command_init.py +++ b/src/specify_cli/command_init.py @@ -790,6 +790,10 @@ def init( selected_script, tracker=tracker, force=force, + integration=resolved_integration, + skills_enabled=resolved_integration.is_skills_mode( + integration_parsed_options, project_root=project_path + ), invoke_separator=resolved_integration.effective_invoke_separator( integration_parsed_options, project_root=project_path ), diff --git a/src/specify_cli/integrations/_helpers.py b/src/specify_cli/integrations/_helpers.py index 9bfec2e8c7..2bf170c63a 100644 --- a/src/specify_cli/integrations/_helpers.py +++ b/src/specify_cli/integrations/_helpers.py @@ -333,6 +333,10 @@ def _set_default_integration( _install_shared_infra( project_root, resolved_script, + integration=integration, + skills_enabled=integration.is_skills_mode( + parsed_options, project_root=project_root + ), invoke_separator=_invoke_separator_for_integration( integration, {"integration_settings": settings}, key, parsed_options, project_root=project_root, diff --git a/src/specify_cli/integrations/command_install.py b/src/specify_cli/integrations/command_install.py index c6b8e3e728..ab94194574 100644 --- a/src/specify_cli/integrations/command_install.py +++ b/src/specify_cli/integrations/command_install.py @@ -116,6 +116,10 @@ def integration_install( _install_shared_infra_or_exit( project_root, selected_script, + integration=infra_integration, + skills_enabled=infra_integration.is_skills_mode( + infra_parsed, project_root=project_root + ), invoke_separator=_invoke_separator_for_integration( infra_integration, current, infra_key, infra_parsed, project_root=project_root, diff --git a/src/specify_cli/integrations/command_switch.py b/src/specify_cli/integrations/command_switch.py index 24d0fd2c5a..6a30dc23ac 100644 --- a/src/specify_cli/integrations/command_switch.py +++ b/src/specify_cli/integrations/command_switch.py @@ -262,6 +262,10 @@ def integration_switch( selected_script, force=refresh_shared_infra, refresh_managed=True, + integration=target_integration, + skills_enabled=target_integration.is_skills_mode( + target_parsed_options, project_root=project_root + ), invoke_separator=_invoke_separator_for_integration( target_integration, current, target, target_parsed_options, project_root=project_root, diff --git a/src/specify_cli/integrations/command_upgrade.py b/src/specify_cli/integrations/command_upgrade.py index 004efa69af..904879c898 100644 --- a/src/specify_cli/integrations/command_upgrade.py +++ b/src/specify_cli/integrations/command_upgrade.py @@ -208,6 +208,10 @@ def integration_upgrade( project_root, selected_script, force=force, + integration=infra_integration, + skills_enabled=infra_integration.is_skills_mode( + infra_parsed, project_root=project_root + ), invoke_separator=_invoke_separator_for_integration( infra_integration, current, infra_key, infra_parsed, project_root=project_root, @@ -254,6 +258,10 @@ def integration_upgrade( _install_shared_infra( project_root, selected_script, + integration=integration, + skills_enabled=integration.is_skills_mode( + parsed_options, project_root=project_root + ), invoke_separator=_invoke_separator_for_integration( integration, {"integration_settings": settings}, key, parsed_options, project_root=project_root, diff --git a/src/specify_cli/shared_infra.py b/src/specify_cli/shared_infra.py index 134e329758..e7011c7326 100644 --- a/src/specify_cli/shared_infra.py +++ b/src/specify_cli/shared_infra.py @@ -405,6 +405,31 @@ def _resolve_dynamic_command_refs( ) +def _resolve_template_command_refs( + content: str, + integration: IntegrationBase | None, + skills_enabled: bool, + invoke_separator: str, + invoke_prefix: str, +) -> str: + if integration is None: + return IntegrationBase.resolve_command_refs( + content, invoke_separator, invoke_prefix + ) + if ( + type(integration).resolve_command_refs_for_mode + is IntegrationBase.resolve_command_refs_for_mode + ): + return integration.resolve_command_refs_for_mode( + content, + skills_enabled=skills_enabled, + invoke_separator=invoke_separator, + ) + return integration.resolve_command_refs_for_mode( + content, skills_enabled=skills_enabled + ) + + def refresh_shared_templates( project_path: Path, *, @@ -415,6 +440,8 @@ def refresh_shared_templates( invoke_separator: str, invoke_prefix: str = "/", force: bool = False, + integration: IntegrationBase | None = None, + skills_enabled: bool = False, ) -> None: """Refresh default-sensitive shared templates without touching scripts.""" templates_src = shared_templates_source(core_pack=core_pack, repo_root=repo_root) @@ -445,8 +472,8 @@ def refresh_shared_templates( continue content = src.read_text(encoding="utf-8") - content = IntegrationBase.resolve_command_refs( - content, invoke_separator, invoke_prefix + content = _resolve_template_command_refs( + content, integration, skills_enabled, invoke_separator, invoke_prefix ) planned_updates.append((dst, rel, content)) @@ -477,6 +504,8 @@ def install_shared_infra( invoke_prefix: str = "/", refresh_managed: bool = False, refresh_hint: str | None = None, + integration: IntegrationBase | None = None, + skills_enabled: bool = False, ) -> bool: """Install shared scripts and templates into *project_path*. @@ -682,8 +711,8 @@ def _ensure_or_bucket_dir(directory: Path) -> bool: continue content = src.read_text(encoding="utf-8") - content = IntegrationBase.resolve_command_refs( - content, invoke_separator, invoke_prefix + content = _resolve_template_command_refs( + content, integration, skills_enabled, invoke_separator, invoke_prefix ) planned_templates.append((dst, rel, content)) diff --git a/tests/integrations/test_cli.py b/tests/integrations/test_cli.py index ed6864253b..4afc17e591 100644 --- a/tests/integrations/test_cli.py +++ b/tests/integrations/test_cli.py @@ -1414,6 +1414,144 @@ def test_git_extension_commands_not_registered_by_default(self, tmp_path): class TestSharedInfraCommandRefs: """Verify _install_shared_infra resolves __SPECKIT_COMMAND_*__ in shared infra.""" + @pytest.mark.parametrize( + "operation", ["init", "install", "use", "switch", "upgrade", "upgrade-secondary"] + ) + @pytest.mark.parametrize("override", ["reference", "resolver"]) + def test_lifecycle_shared_templates_honor_integration_overrides( + self, tmp_path, monkeypatch, operation, override + ): + from typer.testing import CliRunner + from specify_cli import app + from specify_cli.integrations import get_integration + from specify_cli.integrations.base import IntegrationBase + + def reference(self, command_name, args="", *, skills_enabled): + mode = "skill" if skills_enabled else "command" + return f"/native-{self.key}-{mode}-{command_name.removeprefix('speckit.')}" + + def resolver(self, content, *, skills_enabled): + mode = "skill" if skills_enabled else "command" + return IntegrationBase.resolve_command_refs( + content, "-", f"/resolved-{self.key}-{mode}-" + ) + + for key in ("copilot", "claude"): + integration = get_integration(key) + method = ( + "build_command_reference" + if override == "reference" + else "resolve_command_refs_for_mode" + ) + monkeypatch.setattr( + type(integration), method, + reference if override == "reference" else resolver, + ) + + runner = CliRunner() + monkeypatch.chdir(tmp_path) + project = tmp_path / "project" + initial = "claude" if operation == "upgrade" else "copilot" + result = runner.invoke(app, [ + "init", str(project), "--integration", initial, "--script", "sh", + "--ignore-agent-tools", + ], catch_exceptions=False) + assert result.exit_code == 0, result.output + + if operation != "init": + monkeypatch.chdir(project) + if operation in ("use", "upgrade-secondary"): + result = runner.invoke( + app, ["integration", "install", "claude", "--force"], + catch_exceptions=False, + ) + assert result.exit_code == 0, result.output + (project / ".specify/templates/plan-template.md").unlink() + command = "upgrade" if operation == "upgrade-secondary" else operation + args = ["integration", command, "claude"] + if operation == "install": + args.append("--force") + result = runner.invoke( + app, args, catch_exceptions=False + ) + assert result.exit_code == 0, result.output + + active = ( + "copilot" if operation in ("init", "install", "upgrade-secondary") else "claude" + ) + prefix = "native" if override == "reference" else "resolved" + # The static resolver retains its conventional "speckit" stem. + stem = "" if override == "reference" else "speckit-" + content = (project / ".specify/templates/plan-template.md").read_text( + encoding="utf-8" + ) + assert f"/{prefix}-{active}-skill-{stem}plan" in content + assert "/speckit-plan" not in content + + @pytest.mark.parametrize("operation", ["install", "refresh"]) + def test_shared_template_resolver_failure_does_not_write_templates( + self, tmp_path, operation + ): + from specify_cli import _install_shared_infra, _refresh_shared_templates + from specify_cli.integrations.base import IntegrationBase + + class BrokenIntegration(IntegrationBase): + def resolve_command_refs_for_mode(self, content, *, skills_enabled): + raise ValueError("adapter resolver failed") + + kwargs = dict( + integration=BrokenIntegration(), skills_enabled=True, invoke_separator="-" + ) + with pytest.raises(ValueError, match="adapter resolver failed"): + if operation == "install": + _install_shared_infra(tmp_path, "sh", **kwargs) + else: + _refresh_shared_templates(tmp_path, **kwargs) + assert not list((tmp_path / ".specify/templates").glob("*.md")) + + @pytest.mark.parametrize("operation", ["install", "refresh"]) + @pytest.mark.parametrize("skills_enabled", [False, True]) + @pytest.mark.parametrize("override", ["reference", "resolver"]) + def test_shared_templates_honor_integration_overrides( + self, tmp_path, operation, skills_enabled, override + ): + from specify_cli import _install_shared_infra, _refresh_shared_templates + from specify_cli.integrations.base import IntegrationBase + + class ReferenceIntegration(IntegrationBase): + def build_command_reference(self, command_name, args="", *, skills_enabled): + mode = "skill" if skills_enabled else "command" + return f"/native-{mode}-{command_name.removeprefix('speckit.')}" + + class ResolverIntegration(IntegrationBase): + def resolve_command_refs_for_mode(self, content, *, skills_enabled): + mode = "skill" if skills_enabled else "command" + return content.replace( + "__SPECKIT_COMMAND_PLAN__", f"/resolved-{mode}-plan" + ) + + integration = ( + ReferenceIntegration() if override == "reference" else ResolverIntegration() + ) + kwargs = dict( + integration=integration, + skills_enabled=skills_enabled, + invoke_separator="-", + ) + if operation == "install": + _install_shared_infra(tmp_path, "sh", **kwargs) + else: + _refresh_shared_templates(tmp_path, **kwargs) + + content = (tmp_path / ".specify/templates/plan-template.md").read_text( + encoding="utf-8" + ) + prefix = "native" if override == "reference" else "resolved" + mode = "skill" if skills_enabled else "command" + assert f"/{prefix}-{mode}-plan" in content + assert "/speckit-plan" not in content + assert "__SPECKIT_COMMAND_PLAN__" not in content + @staticmethod def _combined_script_content(project, script_type): script_dir = "bash" if script_type == "sh" else "powershell" From cfa5aeb35b9b8ded4159273ce64abd576d4585e6 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Fri, 30 Oct 2026 08:26:10 -0500 Subject: [PATCH 09/10] test(integrations): read generated skills as UTF-8 Match the generated file encoding instead of using the Windows locale codec. Reproduced the reported cp1252 decode failure before the fix and verified the same regression passes afterward. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- tests/specify_cli/integrations/test_installed_adapters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/specify_cli/integrations/test_installed_adapters.py b/tests/specify_cli/integrations/test_installed_adapters.py index e374a295d1..b43edec0cb 100644 --- a/tests/specify_cli/integrations/test_installed_adapters.py +++ b/tests/specify_cli/integrations/test_installed_adapters.py @@ -1210,7 +1210,7 @@ def test_skill_templates_use_skills_prefix_with_reference_separator_override( content = ( project / ".sample-agent/skills/speckit-specify/SKILL.md" - ).read_text() + ).read_text(encoding="utf-8") assert "$speckit_specify" in content assert "$speckit_plan" in content assert "/speckit_specify" not in content From c78174335dcdb29d1aec649223e4d20f16834795 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Fri, 30 Oct 2026 11:21:19 -0500 Subject: [PATCH 10/10] fix(presets): propagate skill reference resolver failures Keep integration resolver execution outside the lookup fallback handler so resolver errors cannot install fallback slash references. Cover registration, core restoration, and extension restoration for all previously swallowed exception types, preserving existing skill bytes on failure and unavailable-integration fallback behavior. Assisted-by: GitHub Copilot (model: GPT-6.1 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- design/integration.md | 3 + src/specify_cli/presets/_manager_skills.py | 18 ++--- .../presets/test_manager_skills.py | 77 +++++++++++++++++++ 3 files changed, 89 insertions(+), 9 deletions(-) diff --git a/design/integration.md b/design/integration.md index d739e96f1e..dccdc822b2 100644 --- a/design/integration.md +++ b/design/integration.md @@ -82,6 +82,9 @@ their naming too. `build_command_invocation()` remains the runtime dispatch seam for integrations such as Bob and Copilot whose executable behavior depends on the installed layout. +Preset skill registration and restoration surface integration resolver failures +before writing the affected skill, rather than installing fallback references. + Shared page-template installation and refresh use the active integration's `resolve_command_refs_for_mode()` with its resolved output mode, honoring both resolver and `build_command_reference()` overrides. Without an active integration, diff --git a/src/specify_cli/presets/_manager_skills.py b/src/specify_cli/presets/_manager_skills.py index 139dbcd25c..2e7ca43b55 100644 --- a/src/specify_cli/presets/_manager_skills.py +++ b/src/specify_cli/presets/_manager_skills.py @@ -543,7 +543,7 @@ def _resolve_skill_command_refs( ``invoke_separator_for_mode`` rather than the single static ``AGENT_CONFIGS`` value. """ - separator = None + integration = None if isinstance(selected_ai, str): try: from ..integrations import get_integration @@ -561,16 +561,16 @@ def _resolve_skill_command_refs( skills_enabled = is_ai_skills_enabled( load_init_options(project_root) ) - return integration.resolve_command_refs_for_mode( - body, - skills_enabled=skills_enabled, - ) except (ImportError, KeyError, OSError, TypeError, ValueError): - separator = None - if separator is None: - separator = registrar.AGENT_CONFIGS.get(selected_ai, {}).get( - "invoke_separator", "." + integration = None + if integration is not None: + return integration.resolve_command_refs_for_mode( + body, + skills_enabled=skills_enabled, ) + separator = registrar.AGENT_CONFIGS.get(selected_ai, {}).get( + "invoke_separator", "." + ) return IntegrationBase.resolve_command_refs(body, separator) def _build_extension_skill_restore_index(self) -> Dict[str, Dict[str, Any]]: diff --git a/tests/specify_cli/presets/test_manager_skills.py b/tests/specify_cli/presets/test_manager_skills.py index a9e5367c9c..5edbe58afc 100644 --- a/tests/specify_cli/presets/test_manager_skills.py +++ b/tests/specify_cli/presets/test_manager_skills.py @@ -365,6 +365,83 @@ def test_restore_skill_preserves_dollar_command_refs(self, project_dir, temp_dir assert "$speckit-plan" in content assert "/speckit-plan" not in content + @pytest.mark.parametrize("operation", ["register", "restore-core", "restore-extension"]) + @pytest.mark.parametrize("error_type", [ValueError, TypeError, OSError, ImportError, KeyError]) + def test_skill_resolver_failure_preserves_existing_content( + self, project_dir, temp_dir, monkeypatch, operation, error_type + ): + from specify_cli.integrations import get_integration + from specify_cli.presets import PresetManifest + + self._write_init_options(project_dir, ai="zcode") + skills_dir = project_dir / ".zcode" / "skills" + skill_dir = self._create_skill(skills_dir, "speckit-specify") + skill_file = skill_dir / "SKILL.md" + original = skill_file.read_bytes() + preset_dir = self._create_command_preset( + temp_dir, "resolver-failure", "speckit.specify", "Override specify", + "Run `__SPECKIT_COMMAND_PLAN__`.\n", + ) + source_file = preset_dir / "commands" / "speckit.specify.md" + manager = PresetManager(project_dir) + if operation == "restore-core": + shutil.copy2( + source_file, + project_dir / ".specify" / "templates" / "commands" / "specify.md", + ) + elif operation == "restore-extension": + monkeypatch.setattr(manager, "_build_extension_skill_restore_index", lambda: { + "speckit-specify": { + "command_name": "speckit.specify", + "source_file": source_file, + "source": "extension:sample", + "extension_id": "sample", + "extension_dir": preset_dir, + } + }) + + failure = error_type("adapter resolver failed") + + def fail_resolution(body, *, skills_enabled): + raise failure + + monkeypatch.setattr( + get_integration("zcode"), "resolve_command_refs_for_mode", fail_resolution + ) + with pytest.raises(error_type) as raised: + if operation == "register": + manager._register_skills( + PresetManifest(preset_dir / "preset.yml"), preset_dir + ) + else: + manager._unregister_skills_in_dir( + ["speckit-specify"], skills_dir, "zcode" + ) + + assert raised.value is failure + assert skill_file.read_bytes() == original + + @pytest.mark.parametrize("lookup_error", [False, True]) + def test_skill_command_refs_keep_unavailable_integration_fallback( + self, project_dir, monkeypatch, lookup_error + ): + import specify_cli.integrations as integrations + + def unavailable(key): + if lookup_error: + raise ValueError("integration unavailable") + return None + + monkeypatch.setattr(integrations, "get_integration", unavailable) + manager = PresetManager(project_dir) + registrar = manager._command_registrar() + monkeypatch.setattr( + registrar, "AGENT_CONFIGS", {"unavailable": {"invoke_separator": "_"}} + ) + assert manager._resolve_skill_command_refs( + "Run __SPECKIT_COMMAND_PLAN__.", registrar, "unavailable", project_dir + ) == "Run /speckit_plan." + def test_reconcile_override_skill_resolves_command_refs(self, project_dir, temp_dir): """Reconcile's project-override restore must resolve command tokens (issue #2717).