diff --git a/design/integration.md b/design/integration.md index 04509705d3..a7d9616c99 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 @@ -157,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/_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..7e2f28da8e 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": @@ -1369,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/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/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..a6ac463c7a 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,8 +1613,8 @@ 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 + from ..integrations.base import IntegrationBase written: List[str] = [] opts = load_init_options(self.project_root) @@ -1631,25 +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 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", ".") + if integration is not None: + return integration.resolve_command_refs_for_mode( + body, + skills_enabled=ai_skills_enabled, ) - - return re.sub( - r"__SPECKIT_COMMAND_([A-Z][A-Z0-9_-]*)__", _replacement, body + return IntegrationBase.resolve_command_refs( + body, + agent_config.get("invoke_separator", "."), ) for cmd_info in manifest.commands: @@ -5672,35 +5660,17 @@ 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) + if isinstance(selected_ai, str): + from ..integrations import get_integration + from ..integrations.installer import project_integrations - 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}" + 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/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/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..b8d437c9b3 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,94 @@ 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, + 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( + "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, @@ -891,6 +984,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. @@ -989,13 +1083,41 @@ 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 == "-" - ) - content = IntegrationBase.resolve_command_refs( - content, invoke_separator, invocation_prefix - ) + # 8. Replace __SPECKIT_COMMAND___ with invocation strings. + # Integration classes own their native prefix; the separator remains + # 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. + try: + from specify_cli.integrations import get_integration + + integration = get_integration(agent_name) + 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, + ) + else: + content = integration.resolve_command_refs_for_mode( + content, + skills_enabled=resolved_skills_enabled, + ) + else: + content = IntegrationBase.resolve_command_refs( + content, invoke_separator + ) return content @@ -1199,6 +1321,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( @@ -1414,6 +1537,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) @@ -1654,6 +1778,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( @@ -1746,15 +1871,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.""" - stem = command_name - if stem.startswith("speckit."): - stem = stem[len("speckit."):] - - prefix = "$" if is_dollar_skills_agent(self.key, True) else "/" + stem = command_name.removeprefix("speckit.") + prefix = self.invocation_prefix_for_mode(True) invocation = prefix + "speckit-" + stem.replace(".", "-") - if args: - invocation = f"{invocation} {args}" - return invocation + return f"{invocation} {args}" if args else invocation @staticmethod def _inject_hook_command_note( @@ -1811,7 +1931,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( @@ -1894,6 +2014,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/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/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/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..b0b4e047e9 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, @@ -177,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 0dbe598ab7..bbc7d0fe65 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 [ @@ -188,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) @@ -279,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 5b3d1977cc..4e5d30b10c 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/", @@ -156,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/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/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..139dbcd25c 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,34 @@ 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 + agent_config = registrar.AGENT_CONFIGS.get(selected_ai, {}) + registrar_writes_skills = ( + agent_config.get("extension") == "/SKILL.md" ) - except Exception: + if project_root is not None and not registrar_writes_skills: + 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 (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..1e4c8b6a1c 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.""" @@ -383,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 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"), diff --git a/tests/specify_cli/integrations/test_installed_adapters.py b/tests/specify_cli/integrations/test_installed_adapters.py index 6f8fbadd92..e374a295d1 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,58 @@ 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" + ) + + +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 @@ -3555,6 +3611,321 @@ 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() + + +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 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 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 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