diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index d03cb85770..e5db7854a1 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -3924,7 +3924,82 @@ def safe_project_dir(relative: str) -> Optional[Path]: return removed - def register_enabled_extensions_for_agent(self, agent_name: str, *, force: bool = False) -> None: + def invocation_outputs( + self, extension_id: str, agent_name: str + ) -> List[tuple[str, Path]]: + """Return ``(name, path)`` for each invocation output of ``agent_name``. + + Checks the layout the agent currently uses on disk rather than + ``registered_commands``, which some layouts leave empty: skill + directories in skills mode, otherwise every file command registration + writes (primary output, aliases, and Copilot's companion + ``.prompt.md``), under the directory registration itself resolves + (home-relative and legacy directories included). Commands whose + source file is absent are skipped, as registration skips them too. + + Raises: + ExtensionError: If the manifest is unreadable or the skills + directory cannot be resolved safely. + """ + import warnings + + from .. import load_init_options + from ..agents import CommandRegistrar + + manifest = self.get_extension(extension_id) + if manifest is None: + raise ExtensionError(f"Cannot read manifest for '{extension_id}'") + registrar = CommandRegistrar(self.project_root) + agent_config = registrar.AGENT_CONFIGS.get(agent_name) + if not manifest.commands or not agent_config: + return [] + opts = load_init_options(self.project_root) + if not isinstance(opts, dict): + opts = {} + skills_mode = ( + opts.get("ai") == agent_name + and is_ai_skills_enabled(opts) + and agent_config.get("extension") != "/SKILL.md" + ) + if skills_mode: + root = self._get_skills_dir(create=False) + if root is None: + raise ExtensionError( + f"Cannot resolve a usable skills directory for '{agent_name}'" + ) + else: + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + root = registrar._resolve_agent_dir( + agent_name, agent_config, self.project_root + ) + + ext_dir = self.extensions_dir / extension_id + outputs: List[tuple[str, Path]] = [] + for cmd in manifest.commands: + if not (ext_dir / cmd["file"]).is_file(): + continue + if skills_mode: + name = self._skill_name_for_command(cmd["name"]) + outputs.append((name, root / name / "SKILL.md")) + continue + aliases = cmd.get("aliases") or [] + for invoked in [cmd["name"], *[a for a in aliases if isinstance(a, str)]]: + name = registrar._compute_output_name(agent_name, invoked, agent_config) + outputs.append((name, root / f"{name}{agent_config['extension']}")) + if agent_name == "copilot": + prompt = self.project_root / ".github" / "prompts" / f"{invoked}.prompt.md" + outputs.append((f"{invoked}.prompt.md", prompt)) + + return outputs + + def register_enabled_extensions_for_agent( + self, + agent_name: str, + *, + force: bool = False, + extension_ids: Optional[Set[str]] = None, + ) -> None: """Register installed, enabled extensions for ``agent_name``. Command-file registration is scoped to the explicit ``agent_name`` @@ -3932,6 +4007,8 @@ def register_enabled_extensions_for_agent(self, agent_name: str, *, force: bool ``switch`` activate the target first; ``upgrade`` calls it only for the active integration), so extension skill rendering — scoped to the active ``ai`` / ``ai_skills`` init-options — matches ``agent_name``. + + ``extension_ids``, when given, limits registration to those extensions. """ if not agent_name: return @@ -3970,6 +4047,8 @@ def register_enabled_extensions_for_agent(self, agent_name: str, *, force: bool agent_skills_dir = _resolve_agent_skills_dir(self.project_root, agent_name) for ext_id, metadata in self.registry.list().items(): + if extension_ids is not None and ext_id not in extension_ids: + continue if not metadata.get("enabled", True): continue diff --git a/src/specify_cli/extensions/command_enable.py b/src/specify_cli/extensions/command_enable.py index 34bab2f279..f305747e39 100644 --- a/src/specify_cli/extensions/command_enable.py +++ b/src/specify_cli/extensions/command_enable.py @@ -12,6 +12,42 @@ from . import _commands +def _present(path) -> bool: + return path.exists() or path.is_symlink() + + +def _register_if_absent(manager, extension_id: str, agent: str, metadata: dict) -> None: + """Register ``extension_id`` for ``agent`` when none of its outputs exist. + + Surviving outputs (customized files, dev symlinks) are never rewritten. + Every output was absent before registration, so if it leaves any missing + or raises, the outputs it did write and its registry tracking are undone, + and a retry starts from the same clean state. + """ + from . import ExtensionError + + outputs = manager.invocation_outputs(extension_id, agent) + if not outputs or any(_present(path) for _, path in outputs): + return + try: + manager.register_enabled_extensions_for_agent(agent, extension_ids={extension_id}) + missing = sorted(name for name, path in outputs if not _present(path)) + if missing: + raise ExtensionError("Missing invocation artifacts: " + ", ".join(missing)) + except Exception: + for _, path in outputs: + if path.is_symlink() or path.is_file(): + path.unlink() + if path.name == "SKILL.md" and path.parent.is_dir() and not any(path.parent.iterdir()): + path.parent.rmdir() + manager.registry.update(extension_id, { + key: metadata[key] + for key in ("registered_commands", "registered_skills") + if key in metadata + }) + raise + + @_commands.extension_app.command("enable") def extension_enable( extension: str = typer.Argument(help="Extension ID or name to enable"), @@ -81,6 +117,25 @@ def extension_enable( f"for '{_escape_markup(str(extension_id))}': {_escape_markup(str(exc))}" ) raise typer.Exit(1) from exc + else: + # `integration use`/`switch` register only enabled extensions for the + # agent they activate, and `switch` removes a disabled extension's + # commands from the old one, so an extension disabled at that point has + # no commands for the now-active agent. Register them only when none + # of the extension's outputs exist for the active layout, so surviving + # files, edits and dev symlinks are never rewritten, and roll back if + # registration leaves any output missing. + active_ai = init_options.get("ai") + if isinstance(active_ai, str) and active_ai: + try: + _register_if_absent(manager, extension_id, active_ai, metadata) + except Exception as exc: + manager.registry.update(extension_id, {"enabled": False}) + console.print( + f"[red]Error:[/red] Could not register invocations " + f"for '{_escape_markup(str(extension_id))}': {_escape_markup(str(exc))}" + ) + raise typer.Exit(1) from exc # Enable hooks in extensions.yml config = hook_executor.get_project_config() diff --git a/tests/specify_cli/extensions/test_command_enable.py b/tests/specify_cli/extensions/test_command_enable.py index 3543917af0..7b1ed37d78 100644 --- a/tests/specify_cli/extensions/test_command_enable.py +++ b/tests/specify_cli/extensions/test_command_enable.py @@ -5,9 +5,13 @@ from __future__ import annotations +import json +import shutil from pathlib import Path from unittest.mock import patch +import pytest +import yaml from typer.testing import CliRunner from specify_cli import app @@ -15,6 +19,19 @@ ExtensionManager, ExtensionRegistry, ) +from tests.specify_cli.integrations._helpers import _init_project, _run_in_project + + +def _registration(project: Path, extension_id: str) -> dict: + """Return the artifacts the registry tracks for *extension_id*.""" + registry = json.loads( + (project / ".specify" / "extensions" / ".registry").read_text(encoding="utf-8") + ) + entry = registry["extensions"][extension_id] + return { + "registered_commands": entry["registered_commands"], + "registered_skills": entry["registered_skills"], + } class TestExtensionEnableCLI: @@ -50,3 +67,288 @@ def test_enable_registry_error_escapes_extension_id_markup(self, tmp_path): assert result.exit_code == 1, result.output assert "Extension '[red]bad[/red]' not found in registry" in result.output + + +class TestExtensionEnableAfterIntegrationChange: + """``enable`` must leave the extension usable by the active integration.""" + + # (integration, --integration-options, generated file, registry entry). + # Claude renders extension commands as skills itself; Copilot in skills + # mode gets them through the extension skill registration instead. + LAYOUTS = [ + pytest.param( + "claude", + None, + ".claude/skills/speckit-test-ext-hello/SKILL.md", + {"registered_commands": {"claude": ["speckit.test-ext.hello"]}, + "registered_skills": []}, + id="claude", + ), + pytest.param( + "copilot", + "--skills", + ".github/skills/speckit-test-ext-hello/SKILL.md", + {"registered_commands": {}, + "registered_skills": ["speckit-test-ext-hello"]}, + id="copilot-skills", + ), + ] + + @pytest.mark.parametrize(("integration", "options", "artifact", "expected"), LAYOUTS) + def test_enable_registers_commands_for_integration_switched_while_disabled( + self, tmp_path, extension_dir, integration, options, artifact, expected + ): + """``integration switch`` registers only enabled extensions for the new + agent and removes a disabled one's commands from the old agent, so the + re-enabled extension must be registered for the now-active agent.""" + project = _init_project(tmp_path, "opencode") + assert _run_in_project( + project, ["extension", "add", str(extension_dir), "--dev"] + ).exit_code == 0 + assert _run_in_project(project, ["extension", "disable", "test-ext"]).exit_code == 0 + switch = ["integration", "switch", integration, "--script", "sh"] + if options: + switch += ["--integration-options", options] + switched = _run_in_project(project, switch) + assert switched.exit_code == 0, switched.output + generated = project / artifact + assert not generated.exists(), "precondition: switch skips disabled extensions" + + result = _run_in_project(project, ["extension", "enable", "test-ext"]) + + assert result.exit_code == 0, result.output + assert generated.is_file() + assert _registration(project, "test-ext") == expected + + @pytest.mark.parametrize(("integration", "options", "artifact", "expected"), LAYOUTS) + def test_enable_leaves_commands_still_registered_for_active_integration( + self, tmp_path, extension_dir, integration, options, artifact, expected + ): + """A plain disable/enable cycle does not rewrite the agent's files.""" + project = _init_project(tmp_path, integration, options) + assert _run_in_project( + project, ["extension", "add", str(extension_dir), "--dev"] + ).exit_code == 0 + generated = project / artifact + generated.unlink() + generated.write_text("customized by the user\n", encoding="utf-8") + assert _run_in_project(project, ["extension", "disable", "test-ext"]).exit_code == 0 + + result = _run_in_project(project, ["extension", "enable", "test-ext"]) + + assert result.exit_code == 0, result.output + assert generated.read_text(encoding="utf-8") == "customized by the user\n" + assert _registration(project, "test-ext") == expected + + @pytest.mark.parametrize(("integration", "options", "artifact", "expected"), LAYOUTS) + def test_enable_keeps_dev_symlinked_outputs( + self, tmp_path, extension_dir, integration, options, artifact, expected + ): + """A disable/enable cycle keeps a ``--dev`` install's symlinked output.""" + project = _init_project(tmp_path, integration, options) + assert _run_in_project( + project, ["extension", "add", str(extension_dir), "--dev"] + ).exit_code == 0 + generated = project / artifact + if not generated.is_symlink(): + pytest.skip("dev-mode symlinks are not available here") + assert _run_in_project(project, ["extension", "disable", "test-ext"]).exit_code == 0 + + result = _run_in_project(project, ["extension", "enable", "test-ext"]) + + assert result.exit_code == 0, result.output + assert generated.is_symlink() + + +def _enabled(project: Path, extension_id: str) -> bool: + registry = json.loads( + (project / ".specify" / "extensions" / ".registry").read_text(encoding="utf-8") + ) + return registry["extensions"][extension_id]["enabled"] + + +class TestExtensionEnableRegistrationFailure: + """``enable`` must not report success when registration leaves files missing.""" + + LAYOUTS = [ + pytest.param("claude", None, ".claude/skills/speckit-test-ext-hello/SKILL.md", id="claude"), + pytest.param("copilot", "--skills", ".github/skills/speckit-test-ext-hello/SKILL.md", id="copilot-skills"), + pytest.param("gemini", None, ".gemini/commands/speckit.test-ext.hello.toml", id="gemini-commands"), + ] + + def _disabled_after_switch(self, tmp_path, extension_dir, integration, options): + project = _init_project(tmp_path, "opencode") + assert _run_in_project( + project, ["extension", "add", str(extension_dir), "--dev"] + ).exit_code == 0 + assert _run_in_project(project, ["extension", "disable", "test-ext"]).exit_code == 0 + switch = ["integration", "switch", integration, "--script", "sh"] + if options: + switch += ["--integration-options", options] + assert _run_in_project(project, switch).exit_code == 0 + return project + + @pytest.mark.parametrize( + "failure", + [ + pytest.param(lambda *a, **k: None, id="writes-nothing"), + pytest.param( + lambda *a, **k: (_ for _ in ()).throw(RuntimeError("adapter setup failed")), + id="raises", + ), + ], + ) + @pytest.mark.parametrize(("integration", "options", "artifact"), LAYOUTS) + def test_failed_registration_keeps_extension_disabled_and_retry_works( + self, tmp_path, extension_dir, integration, options, artifact, failure + ): + project = self._disabled_after_switch(tmp_path, extension_dir, integration, options) + generated = project / artifact + + with patch.object( + ExtensionManager, "register_enabled_extensions_for_agent", failure + ): + failed = _run_in_project(project, ["extension", "enable", "test-ext"]) + + assert failed.exit_code == 1, failed.output + assert "Could not register invocations" in failed.output + assert _enabled(project, "test-ext") is False + assert not generated.exists() + + retried = _run_in_project(project, ["extension", "enable", "test-ext"]) + + assert retried.exit_code == 0, retried.output + assert _enabled(project, "test-ext") is True + assert generated.exists() + + +class TestExtensionEnableOutputLayouts: + """``enable`` checks the outputs registration actually writes.""" + + def test_unusable_skills_dir_fails_enable_and_retry_works_after_repair( + self, tmp_path, extension_dir + ): + project = _init_project(tmp_path, "opencode") + assert _run_in_project(project, ["extension", "add", str(extension_dir), "--dev"]).exit_code == 0 + assert _run_in_project(project, ["extension", "disable", "test-ext"]).exit_code == 0 + assert _run_in_project( + project, + ["integration", "switch", "copilot", "--script", "sh", + "--integration-options", "--skills"], + ).exit_code == 0 + skills_dir = project / ".github" / "skills" + shutil.rmtree(skills_dir) + skills_dir.write_text("not a directory\n", encoding="utf-8") + + failed = _run_in_project(project, ["extension", "enable", "test-ext"]) + + assert failed.exit_code == 1, failed.output + assert _enabled(project, "test-ext") is False + + skills_dir.unlink() + retried = _run_in_project(project, ["extension", "enable", "test-ext"]) + + assert retried.exit_code == 0, retried.output + assert (skills_dir / "speckit-test-ext-hello" / "SKILL.md").is_file() + + def test_home_relative_outputs_survive_disable_enable( + self, tmp_path, extension_dir, monkeypatch + ): + home = tmp_path / "home" + home.mkdir() + monkeypatch.setattr(Path, "home", lambda: home) + project = _init_project(tmp_path, "hermes") + assert _run_in_project(project, ["extension", "add", str(extension_dir), "--dev"]).exit_code == 0 + skill = home / ".hermes" / "skills" / "speckit-test-ext-hello" / "SKILL.md" + assert skill.exists() + skill.unlink() + skill.write_text("customized by the user\n", encoding="utf-8") + assert _run_in_project(project, ["extension", "disable", "test-ext"]).exit_code == 0 + + result = _run_in_project(project, ["extension", "enable", "test-ext"]) + + assert result.exit_code == 0, result.output + assert _enabled(project, "test-ext") is True + assert skill.read_text(encoding="utf-8") == "customized by the user\n" + + def test_legacy_command_dir_outputs_survive_disable_enable( + self, tmp_path, extension_dir + ): + project = _init_project(tmp_path, "opencode") + assert _run_in_project(project, ["extension", "add", str(extension_dir), "--dev"]).exit_code == 0 + (project / ".opencode" / "commands").rename(project / ".opencode" / "command") + legacy = project / ".opencode" / "command" / "speckit.test-ext.hello.md" + assert legacy.exists() + legacy.unlink() + legacy.write_text("customized by the user\n", encoding="utf-8") + assert _run_in_project(project, ["extension", "disable", "test-ext"]).exit_code == 0 + + result = _run_in_project(project, ["extension", "enable", "test-ext"]) + + assert result.exit_code == 0, result.output + assert _enabled(project, "test-ext") is True + assert legacy.read_text(encoding="utf-8") == "customized by the user\n" + + def test_failure_after_primary_write_rolls_back_and_retry_completes( + self, tmp_path, extension_dir + ): + """Copilot command mode writes an agent file and a companion prompt; a + prompt failure must not leave a half-registered, enabled extension.""" + from specify_cli.agents import CommandRegistrar + + project = _init_project(tmp_path, "opencode") + assert _run_in_project(project, ["extension", "add", str(extension_dir), "--dev"]).exit_code == 0 + assert _run_in_project(project, ["extension", "disable", "test-ext"]).exit_code == 0 + assert _run_in_project( + project, + ["integration", "switch", "copilot", "--script", "sh", + "--integration-options", "--commands"], + ).exit_code == 0 + agent_file = project / ".github" / "agents" / "speckit.test-ext.hello.agent.md" + prompt_file = project / ".github" / "prompts" / "speckit.test-ext.hello.prompt.md" + + def fail_prompt(project_root, cmd_name): + raise OSError("disk full") + + with patch.object(CommandRegistrar, "write_copilot_prompt", staticmethod(fail_prompt)): + failed = _run_in_project(project, ["extension", "enable", "test-ext"]) + + assert failed.exit_code == 1, failed.output + assert _enabled(project, "test-ext") is False + assert not agent_file.exists() + + retried = _run_in_project(project, ["extension", "enable", "test-ext"]) + + assert retried.exit_code == 0, retried.output + assert agent_file.is_file() + assert prompt_file.is_file() + + def test_partially_present_outputs_are_not_rewritten( + self, tmp_path, extension_dir + ): + """With one of two commands' outputs gone, enable must not re-register + the extension and overwrite the other, customized output.""" + manifest_path = extension_dir / "extension.yml" + manifest = yaml.safe_load(manifest_path.read_text(encoding="utf-8")) + manifest["provides"]["commands"].append( + {"name": "speckit.test-ext.bye", "file": "commands/bye.md", + "description": "Bye"} + ) + manifest_path.write_text(yaml.dump(manifest), encoding="utf-8") + (extension_dir / "commands" / "bye.md").write_text( + "---\ndescription: \"Bye\"\n---\n\n# Bye\n\n$ARGUMENTS\n", encoding="utf-8" + ) + project = _init_project(tmp_path, "claude") + assert _run_in_project(project, ["extension", "add", str(extension_dir), "--dev"]).exit_code == 0 + skills = project / ".claude" / "skills" + hello = skills / "speckit-test-ext-hello" / "SKILL.md" + bye = skills / "speckit-test-ext-bye" / "SKILL.md" + hello.unlink() + hello.write_text("customized by the user\n", encoding="utf-8") + shutil.rmtree(bye.parent) + assert _run_in_project(project, ["extension", "disable", "test-ext"]).exit_code == 0 + + result = _run_in_project(project, ["extension", "enable", "test-ext"]) + + assert result.exit_code == 0, result.output + assert hello.read_text(encoding="utf-8") == "customized by the user\n"