diff --git a/EOF b/EOF new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index d5f5aba2d5..c1b9f78595 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -2365,6 +2365,19 @@ def integration_switch( ) raise typer.Exit(1) + # Unregister extension commands for the old agent so they don't + # remain as orphans in the old agent's directory. + try: + from .extensions import ExtensionManager + + ext_mgr = ExtensionManager(project_root) + ext_mgr.unregister_agent_artifacts(installed_key) + except Exception as ext_err: + console.print( + f"[yellow]Warning:[/yellow] Could not clean up extension artifacts " + f"(commands, skills, registry entries) for '{installed_key}': {ext_err}" + ) + # Clear metadata so a failed Phase 2 doesn't leave stale references _remove_integration_json(project_root) opts = load_init_options(project_root) @@ -2404,6 +2417,19 @@ def integration_switch( _write_integration_json(project_root, target_integration.key) _update_init_options_for_integration(project_root, target_integration, script_type=selected_script) + # Re-register extension commands for the new agent so that + # previously-installed extensions are available in the new integration. + try: + from .extensions import ExtensionManager + + ext_mgr = ExtensionManager(project_root) + ext_mgr.register_enabled_extensions_for_agent(target) + except Exception as ext_err: + console.print( + f"[yellow]Warning:[/yellow] Could not register extension commands, skills, " + f"or related artifacts for '{target}': {ext_err}" + ) + except Exception as e: # Attempt rollback of any files written by setup try: diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index a419ebf1d2..9f4d64343c 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -962,29 +962,40 @@ def _register_extension_skills( return written - def _unregister_extension_skills(self, skill_names: List[str], extension_id: str) -> None: + def _unregister_extension_skills( + self, + skill_names: List[str], + extension_id: str, + skills_dir: Optional[Path] = None, + ) -> None: """Remove SKILL.md directories for extension skills. Called during extension removal to clean up skill files that were created by ``_register_extension_skills()``. - If ``_get_skills_dir()`` returns ``None`` (e.g. the user removed - init-options.json or toggled ai_skills after installation), we - fall back to scanning all known agent skills directories so that - orphaned skill directories are still cleaned up. In that case - each candidate directory is verified against the SKILL.md - ``metadata.source`` field before removal to avoid accidentally - deleting user-created skills with the same name. + If *skills_dir* is not provided and ``_get_skills_dir()`` returns + ``None`` (e.g. the user removed init-options.json or toggled + ai_skills after installation), we fall back to scanning all known + agent skills directories so that orphaned skill directories are + still cleaned up. In that case each candidate directory is + verified against the SKILL.md ``metadata.source`` field before + removal to avoid accidentally deleting user-created skills with + the same name. Args: skill_names: List of skill names to remove. extension_id: Extension ID used to verify ownership during fallback candidate scanning. + skills_dir: Optional explicit skills directory to use instead + of resolving via ``_get_skills_dir()``. Useful when the + caller needs to target a specific agent's skills directory + regardless of the currently-active agent in init-options. """ if not skill_names: return - skills_dir = self._get_skills_dir() + if skills_dir is None: + skills_dir = self._get_skills_dir() if skills_dir: # Fast path: we know the exact skills directory @@ -1332,6 +1343,156 @@ def remove(self, extension_id: str, keep_config: bool = False) -> bool: return True + @staticmethod + def _valid_name_list(value: Any) -> List[str]: + """Return string entries from a registry list, ignoring corrupt values.""" + if not isinstance(value, list): + return [] + return [item for item in value if isinstance(item, str)] + + def unregister_agent_artifacts(self, agent_name: str) -> None: + """Remove extension files registered for a specific agent. + + Extension command files are tracked per agent in ``registered_commands``. + Extension skills are scoped to the provided *agent_name*; they are removed + from that agent's skills directory (resolved via its integration config) + and the registry field is cleared. + + Skips cleanup when *agent_name* is not a supported agent to avoid + losing registry entries while leaving orphaned files on disk. + """ + if not agent_name: + return + + registrar = CommandRegistrar() + if agent_name not in registrar.AGENT_CONFIGS: + return + + # Resolve the skills directory for the specific agent so cleanup is + # agent-scoped and does not depend on the currently-active agent in + # init-options. Use the same helper that extension install uses. + from . import _get_skills_dir as resolve_skills_dir + + agent_skills_dir = resolve_skills_dir(self.project_root, agent_name) + + for ext_id, metadata in self.registry.list().items(): + updates: Dict[str, Any] = {} + + registered_commands = metadata.get("registered_commands", {}) + if isinstance(registered_commands, dict) and agent_name in registered_commands: + command_names = self._valid_name_list(registered_commands.get(agent_name)) + if command_names: + registrar.unregister_commands({agent_name: command_names}, self.project_root) + + new_registered = copy.deepcopy(registered_commands) + new_registered.pop(agent_name, None) + updates["registered_commands"] = new_registered + + registered_skills = self._valid_name_list(metadata.get("registered_skills", [])) + if registered_skills: + # Only pass the resolved skills_dir when it actually exists. + # Otherwise let _unregister_extension_skills fall back to + # scanning all known agent skills directories, which is useful + # for cleaning up stale entries created by earlier installs. + skills_dir = agent_skills_dir if agent_skills_dir.is_dir() else None + self._unregister_extension_skills( + registered_skills, ext_id, skills_dir=skills_dir + ) + + # Only reconcile registry state when cleanup was scoped to a + # specific existing directory. When skills_dir is None, + # _unregister_extension_skills falls back to scanning multiple + # candidate directories, so agent_skills_dir cannot be used to + # infer what was removed. When skills_dir is set, + # _unregister_extension_skills may intentionally skip deletion + # when ownership cannot be verified (e.g., corrupted/missing + # SKILL.md or mismatching metadata.source). Only drop registry + # entries for skill directories that were actually removed so + # future cleanup attempts can still find skipped ones. + if skills_dir is not None: + remaining_skills = [ + skill_name + for skill_name in registered_skills + if (skills_dir / skill_name).is_dir() + ] + if remaining_skills != registered_skills: + updates["registered_skills"] = remaining_skills + + if updates: + self.registry.update(ext_id, updates) + + def register_enabled_extensions_for_agent(self, agent_name: str) -> None: + """Register installed, enabled extensions for ``agent_name``. + + This is intended to be called after switching integrations. Command + registration is scoped to the explicit ``agent_name`` argument, but some + behavior still depends on the current init-options state (for example, + skills-mode handling uses the active ``ai`` / ``ai_skills`` settings). + + Callers should therefore pass the agent that has just been made active + in init-options; in normal use, ``agent_name`` is expected to match the + current ``ai`` value. This mirrors extension install behavior while + avoiding stale default-mode command directories when that active agent + is running in skills mode (notably Copilot ``--skills``). + """ + if not agent_name: + return + + from . import load_init_options + + registrar = CommandRegistrar() + agent_config = registrar.AGENT_CONFIGS.get(agent_name) + init_options = load_init_options(self.project_root) + if not isinstance(init_options, dict): + init_options = {} + + active_agent = init_options.get("ai") + skills_mode_active = ( + active_agent == agent_name + and bool(init_options.get("ai_skills")) + and bool(agent_config) + and agent_config.get("extension") != "/SKILL.md" + ) + + for ext_id, metadata in self.registry.list().items(): + if not metadata.get("enabled", True): + continue + + manifest = self.get_extension(ext_id) + if manifest is None: + continue + + ext_dir = self.extensions_dir / ext_id + updates: Dict[str, Any] = {} + + if agent_config and not skills_mode_active: + registered = registrar.register_commands_for_agent( + agent_name, manifest, ext_dir, self.project_root + ) + registered_commands = metadata.get("registered_commands", {}) + if not isinstance(registered_commands, dict): + registered_commands = {} + new_registered = copy.deepcopy(registered_commands) + if registered: + new_registered[agent_name] = registered + else: + # Registration returned empty list (e.g., corrupted + # manifest pointing at missing command files). Clear + # stale entry so later cleanup doesn't try to remove + # files that were never written. + new_registered.pop(agent_name, None) + if new_registered != registered_commands: + updates["registered_commands"] = new_registered + + registered_skills = self._register_extension_skills(manifest, ext_dir) + if registered_skills: + existing_skills = self._valid_name_list(metadata.get("registered_skills", [])) + merged_skills = list(dict.fromkeys(existing_skills + registered_skills)) + updates["registered_skills"] = merged_skills + + if updates: + self.registry.update(ext_id, updates) + def list_installed(self) -> List[Dict[str, Any]]: """List all installed extensions with metadata. diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index f5322bdf5e..3952557cf2 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -31,6 +31,16 @@ def _init_project(tmp_path, integration="copilot"): return project +def _run_in_project(project, args): + """Run a CLI command from inside a generated project.""" + old_cwd = os.getcwd() + try: + os.chdir(project) + return runner.invoke(app, args, catch_exceptions=False) + finally: + os.chdir(old_cwd) + + # ── list ───────────────────────────────────────────────────────────── @@ -334,6 +344,142 @@ def test_switch_between_integrations(self, tmp_path): data = json.loads((project / ".specify" / "integration.json").read_text(encoding="utf-8")) assert data["integration"] == "copilot" + def test_switch_migrates_extension_commands(self, tmp_path): + """Switching should migrate extension commands to the new agent directory.""" + project = _init_project(tmp_path, "kimi") + + # Install the bundled git extension + result = _run_in_project(project, ["extension", "add", "git"]) + assert result.exit_code == 0, f"extension add failed: {result.output}" + + # Verify git extension skills exist for kimi + kimi_git_feature = project / ".kimi" / "skills" / "speckit-git-feature" / "SKILL.md" + assert kimi_git_feature.exists(), "Git extension skill should exist for kimi" + + result = _run_in_project(project, [ + "integration", "switch", "opencode", + "--script", "sh", + ]) + assert result.exit_code == 0, result.output + + # Git extension commands should exist for opencode + opencode_git_feature = project / ".opencode" / "command" / "speckit.git.feature.md" + assert opencode_git_feature.exists(), "Git extension command should exist for opencode" + + # Old kimi extension skills should be removed + assert not kimi_git_feature.exists(), "Old kimi extension skill should be removed" + + # Extension registry should be updated + registry = json.loads( + (project / ".specify" / "extensions" / ".registry").read_text(encoding="utf-8") + ) + registered_commands = registry["extensions"]["git"]["registered_commands"] + assert "opencode" in registered_commands + assert "kimi" not in registered_commands + + # Switch to claude + result = _run_in_project(project, [ + "integration", "switch", "claude", + "--script", "sh", + ]) + assert result.exit_code == 0, result.output + + # Git extension skills should exist for claude + claude_git_feature = project / ".claude" / "skills" / "speckit-git-feature" / "SKILL.md" + assert claude_git_feature.exists(), "Git extension skill should exist for claude" + + # Old opencode extension commands should be removed + assert not opencode_git_feature.exists(), "Old opencode extension command should be removed" + + # Extension registry should be updated + registry = json.loads( + (project / ".specify" / "extensions" / ".registry").read_text(encoding="utf-8") + ) + registered_commands = registry["extensions"]["git"]["registered_commands"] + assert "claude" in registered_commands + assert "opencode" not in registered_commands + + def test_switch_migrates_copilot_skills_extension_commands(self, tmp_path): + """Copilot --skills should receive extension skills, not .agent.md files.""" + project = _init_project(tmp_path, "opencode") + + result = _run_in_project(project, ["extension", "add", "git"]) + assert result.exit_code == 0, f"extension add failed: {result.output}" + + result = _run_in_project(project, [ + "integration", "switch", "copilot", + "--script", "sh", + "--integration-options", "--skills", + ]) + assert result.exit_code == 0, result.output + + copilot_git_feature = project / ".github" / "skills" / "speckit-git-feature" / "SKILL.md" + copilot_agent_file = project / ".github" / "agents" / "speckit.git.feature.agent.md" + assert copilot_git_feature.exists(), "Git extension skill should exist for Copilot skills mode" + assert not copilot_agent_file.exists(), "Copilot skills mode should not create extension .agent.md files" + + # Verify Copilot-specific frontmatter: mode field should map from + # skill name (speckit-git-feature) back to dot notation (speckit.git-feature) + skill_content = copilot_git_feature.read_text(encoding="utf-8") + assert "mode: speckit.git-feature" in skill_content, ( + "Copilot skill frontmatter should contain mode mapped from skill name" + ) + + registry = json.loads( + (project / ".specify" / "extensions" / ".registry").read_text(encoding="utf-8") + ) + git_meta = registry["extensions"]["git"] + assert "speckit-git-feature" in git_meta["registered_skills"] + assert "copilot" not in git_meta["registered_commands"] + + result = _run_in_project(project, [ + "integration", "switch", "opencode", + "--script", "sh", + ]) + assert result.exit_code == 0, result.output + + opencode_git_feature = project / ".opencode" / "command" / "speckit.git.feature.md" + assert opencode_git_feature.exists(), "Git extension command should exist for opencode" + assert not copilot_git_feature.exists(), "Old Copilot extension skill should be removed" + + registry = json.loads( + (project / ".specify" / "extensions" / ".registry").read_text(encoding="utf-8") + ) + git_meta = registry["extensions"]["git"] + assert git_meta["registered_skills"] == [] + assert "opencode" in git_meta["registered_commands"] + assert "copilot" not in git_meta["registered_commands"] + + def test_switch_does_not_register_disabled_extensions(self, tmp_path): + """Disabled extensions should stay disabled and should not migrate commands.""" + project = _init_project(tmp_path, "opencode") + + result = _run_in_project(project, ["extension", "add", "git"]) + assert result.exit_code == 0, f"extension add failed: {result.output}" + result = _run_in_project(project, ["extension", "disable", "git"]) + assert result.exit_code == 0, result.output + + opencode_git_feature = project / ".opencode" / "command" / "speckit.git.feature.md" + assert opencode_git_feature.exists(), "Disabled extension command remains until integration switch" + + result = _run_in_project(project, [ + "integration", "switch", "claude", + "--script", "sh", + ]) + assert result.exit_code == 0, result.output + + claude_git_feature = project / ".claude" / "skills" / "speckit-git-feature" / "SKILL.md" + assert not claude_git_feature.exists(), "Disabled extension should not be registered for new agent" + assert not opencode_git_feature.exists(), "Old disabled extension command should be removed on switch" + + registry = json.loads( + (project / ".specify" / "extensions" / ".registry").read_text(encoding="utf-8") + ) + git_meta = registry["extensions"]["git"] + assert git_meta["enabled"] is False + assert "claude" not in git_meta["registered_commands"] + assert "opencode" not in git_meta["registered_commands"] + def test_switch_preserves_shared_infra(self, tmp_path): """Switching preserves shared scripts, templates, and memory.""" project = _init_project(tmp_path, "claude")