From af9205fc2cd75acad0f0d148e54fb118d2416fff Mon Sep 17 00:00:00 2001 From: marcelsafin <179933638+marcelsafin@users.noreply.github.com> Date: Fri, 10 Jul 2026 20:23:09 +0200 Subject: [PATCH] fix: register extensions for the active integration only extension add registered commands for every detected agent, and integration upgrade back-filled enabled extensions for non-active integrations. Maintainer direction on #2948: treat the project as single-active. Only the active integration gets extension artifacts; use/switch rescaffold the target when the user selects it. - extension add now routes through the all-agents pass restricted to the active integration (only_agent), keeping detection and missing-skills-dir recovery safeguards. Projects without recorded init-options fall back to detection-based registration. - integration upgrade re-registers extensions only when upgrading the active integration, reversing the #2886 back-fill for non-active targets at maintainer request. Fixes #2948 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/specify_cli/agents.py | 5 + src/specify_cli/extensions/__init__.py | 91 +++++++++++---- src/specify_cli/integrations/_helpers.py | 19 ++-- .../integrations/_migrate_commands.py | 24 ++-- .../test_integration_subcommand.py | 105 ++++++++++++++---- tests/test_extensions.py | 7 +- 6 files changed, 185 insertions(+), 66 deletions(-) diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index c53586912..26add257f 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -932,6 +932,7 @@ class CommandRegistrar: link_outputs: bool = False, create_missing_active_skills_dir: bool = False, extension_id: Optional[str] = None, + only_agent: Optional[str] = None, ) -> Dict[str, List[str]]: """Register commands for all detected agents in the project. @@ -949,6 +950,8 @@ class CommandRegistrar: skills directory) and is skipped when safe resolution or creation fails. extension_id: Extension id when rendering extension-owned commands. + only_agent: If set, restrict registration to this single agent + while keeping all detection and recovery safeguards (#2948). Returns: Dictionary mapping agent names to list of registered commands @@ -972,6 +975,8 @@ class CommandRegistrar: ) active_created_skills_dir: Optional[Path] = None for agent_name, agent_config in self.AGENT_CONFIGS.items(): + if only_agent is not None and agent_name != only_agent: + continue active_skills_output = ( agent_name == active_skills_agent and agent_config.get("extension") == "/SKILL.md" diff --git a/src/specify_cli/extensions/__init__.py b/src/specify_cli/extensions/__init__.py index b84b83654..ea1ce4364 100644 --- a/src/specify_cli/extensions/__init__.py +++ b/src/specify_cli/extensions/__init__.py @@ -975,6 +975,64 @@ class ExtensionManager: return _ensure_usable(agent_skills_dir) return _ensure_usable(skills_dir) + def _register_commands_for_active_agent( + self, + manifest: ExtensionManifest, + extension_dir: Path, + link_outputs: bool = False, + ) -> Dict[str, List[str]]: + """Register extension commands for the active integration only. + + Maintainer-requested behavior for #2948: ``extension add`` treats the + project as single-active — only the integration recorded in + init-options gets command files. Non-active integrations receive them + when selected via ``integration use`` / ``switch`` (rescaffold). + + Projects without a recorded active integration (pre-init-options + layouts or direct library use) fall back to detection-based + registration for all agents. + + Returns: + Mapping of agent name to registered command names, matching the + ``registered_commands`` registry shape. + """ + from .. import load_init_options + + registrar = CommandRegistrar() + init_options = load_init_options(self.project_root) + if not isinstance(init_options, dict): + init_options = {} + active_agent = init_options.get("ai") + + if not active_agent or active_agent not in registrar.AGENT_CONFIGS: + return registrar.register_commands_for_all_agents( + manifest, + extension_dir, + self.project_root, + link_outputs=link_outputs, + create_missing_active_skills_dir=True, + ) + + agent_config = registrar.AGENT_CONFIGS[active_agent] + if ( + is_ai_skills_enabled(init_options) + and agent_config.get("extension") != "/SKILL.md" + ): + # Active agent runs skills mode: extension artifacts render as + # skills via _register_extension_skills, not as command files. + return {} + + # Route through the all-agents pass restricted to the active agent so + # detection and missing-skills-dir recovery safeguards still apply. + return registrar.register_commands_for_all_agents( + manifest, + extension_dir, + self.project_root, + link_outputs=link_outputs, + create_missing_active_skills_dir=True, + only_agent=active_agent, + ) + def _register_extension_skills( self, manifest: ExtensionManifest, @@ -1404,17 +1462,11 @@ class ExtensionManager: ignore_fn = self._load_extensionignore(source_dir) shutil.copytree(source_dir, dest_dir, ignore=ignore_fn) - # Register commands with AI agents + # Register commands with AI agents (active integration only, #2948) registered_commands = {} if register_commands: - registrar = CommandRegistrar() - # Register for all detected agents - registered_commands = registrar.register_commands_for_all_agents( - manifest, - dest_dir, - self.project_root, - link_outputs=link_commands, - create_missing_active_skills_dir=True, + registered_commands = self._register_commands_for_active_agent( + manifest, dest_dir, link_outputs=link_commands ) # Auto-register extension commands as agent skills when skills mode @@ -1701,11 +1753,10 @@ class ExtensionManager: """Register installed, enabled extensions for ``agent_name``. Command-file registration is scoped to the explicit ``agent_name`` - argument, so this method can be used after install, upgrade, or switch. - Extension skill rendering is still scoped to the active ``ai`` / - ``ai_skills`` settings in init-options, so non-active skills-mode - targets receive command files here. Per-agent skills parity is tracked - separately in #2948. + argument. Since #2948, callers pass the active agent only (``use`` / + ``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``. """ if not agent_name: return @@ -1765,13 +1816,11 @@ class ExtensionManager: # Extension *skills* are only ever rendered for the active agent: # `_register_extension_skills` resolves the skills dir and # frontmatter from init-options["ai"], ignoring ``agent_name``. - # When this method runs for a non-active agent — as install/upgrade - # now do for a secondary integration (#2886) — the skills pass would - # re-render the *active* agent's extension skills as a side effect, + # Running the skills pass for a non-active agent would re-render + # the *active* agent's extension skills as a side effect, # resurrecting skill files the user deliberately deleted. Skip it - # unless the target is the active agent; `switch` is unaffected - # because it activates the target before registering. (Rendering - # skills for a non-active target is tracked separately in #2948.) + # unless the target is the active agent (defense in depth: since + # #2948 callers only pass the active agent anyway). if agent_name == active_agent: try: registered_skills = self._register_extension_skills( @@ -1970,6 +2019,7 @@ class CommandRegistrar: project_root: Path, link_outputs: bool = False, create_missing_active_skills_dir: bool = False, + only_agent: Optional[str] = None, ) -> Dict[str, List[str]]: """Register extension commands for all detected agents.""" context_note = f"\n\n\n" @@ -1981,6 +2031,7 @@ class CommandRegistrar: context_note=context_note, link_outputs=link_outputs, create_missing_active_skills_dir=create_missing_active_skills_dir, + only_agent=only_agent, extension_id=manifest.id, ) diff --git a/src/specify_cli/integrations/_helpers.py b/src/specify_cli/integrations/_helpers.py index d1bf051f7..1ba228723 100644 --- a/src/specify_cli/integrations/_helpers.py +++ b/src/specify_cli/integrations/_helpers.py @@ -376,19 +376,14 @@ def _register_extensions_for_agent( """Register all enabled extensions' commands/skills for ``agent_key``. ``use`` / ``switch`` re-register enabled extensions for the agent they - activate; ``upgrade`` backfills them for the refreshed agent. Plain - ``install`` deliberately does not call this helper so adding a secondary - integration has no extension side effects until it is selected or upgraded. - See issue #2886. + activate (rescaffold); ``upgrade`` does so only for the *active* + integration. Plain ``install`` and upgrade of a non-active integration + deliberately skip this helper so a secondary integration has no extension + side effects until it is selected. See issues #2886 and #2948. - Known limitation: extension *skill* rendering is scoped to the active - agent (init-options track a single ``ai`` / ``ai_skills`` pair). A - skills-mode agent registered while it is *not* the active agent (e.g. - Copilot ``--skills`` registered while non-active) therefore - receives command files rather than skills here — matching ``extension - add``'s multi-agent behavior. ``use`` / ``switch`` avoid this because they - make the target the active agent first. Per-agent skills parity is tracked in - #2948. + Callers always pass the active agent (use/switch activate the target + before registering), so extension *skill* rendering — which is scoped to + the active ``ai`` / ``ai_skills`` init-options — matches ``agent_key``. Best-effort: never aborts the surrounding integration operation. Callers invoke it *after* the use/upgrade/switch transaction has committed so a diff --git a/src/specify_cli/integrations/_migrate_commands.py b/src/specify_cli/integrations/_migrate_commands.py index 6568d1af1..f8f5e16c5 100644 --- a/src/specify_cli/integrations/_migrate_commands.py +++ b/src/specify_cli/integrations/_migrate_commands.py @@ -491,17 +491,19 @@ def integration_upgrade( if stale_removed: console.print(f" Removed {len(stale_removed)} stale file(s) from previous install") - # Re-register enabled extensions for the upgraded agent so its extension - # commands are (re)created — including agents installed before this - # back-fill existed. Mirrors switch for command registration; see #2886. - # Done after the upgrade has fully settled (Phase 2 included) and outside - # the try/except above so this best-effort step cannot affect upgrade - # success. - _register_extensions_for_agent( - project_root, - key, - continuing="The integration was upgraded, but installed extensions may need re-registration.", - ) + # Re-register enabled extensions only when upgrading the *active* + # integration, so its extension commands are (re)created after the + # upgrade settled (Phase 2 included). Done outside the try/except above + # so this best-effort step cannot affect upgrade success. Non-active + # integrations are rescaffolded by `use` / `switch` instead — the #2886 + # back-fill for non-active agents was removed at maintainer request + # (#2948). + if key == installed_key: + _register_extensions_for_agent( + project_root, + key, + continuing="The integration was upgraded, but installed extensions may need re-registration.", + ) name = (integration.config or {}).get("name", key) console.print(f"\n[green]✓[/green] Integration '{name}' upgraded successfully") diff --git a/tests/integrations/test_integration_subcommand.py b/tests/integrations/test_integration_subcommand.py index 56f338cc8..d4b9085e4 100644 --- a/tests/integrations/test_integration_subcommand.py +++ b/tests/integrations/test_integration_subcommand.py @@ -1371,6 +1371,53 @@ class TestIntegrationInstall: project / ".github" / "skills" / "speckit-git-feature" / "SKILL.md" ).exists() + def test_extension_add_registers_active_integration_only(self, tmp_path): + """``extension add`` registers commands for the active integration only. + + Maintainer-requested behavior for #2948: with multiple integrations + installed, ``extension add`` must treat the project as single-active — + only the current integration gets the new extension's commands. + Non-active integrations receive them when selected via + ``integration use`` / ``switch`` (rescaffold). + """ + project = _init_project(tmp_path, "claude") + + result = _run_in_project(project, [ + "integration", "install", "codex", + "--script", "sh", + ]) + assert result.exit_code == 0, result.output + + result = _run_in_project(project, ["extension", "add", "git"]) + assert result.exit_code == 0, f"extension add failed: {result.output}" + + registry_path = project / ".specify" / "extensions" / ".registry" + registered = json.loads(registry_path.read_text(encoding="utf-8"))[ + "extensions" + ]["git"]["registered_commands"] + assert "claude" in registered, "active integration gets the extension" + assert "codex" not in registered, ( + "non-active integration must not be registered on add (#2948)" + ) + assert ( + project / ".claude" / "skills" / "speckit-git-feature" / "SKILL.md" + ).exists() + assert not ( + project / ".agents" / "skills" / "speckit-git-feature" / "SKILL.md" + ).exists() + + # Selecting the other integration rescaffolds it with the extension. + result = _run_in_project(project, ["integration", "use", "codex"]) + assert result.exit_code == 0, result.output + + registered = json.loads(registry_path.read_text(encoding="utf-8"))[ + "extensions" + ]["git"]["registered_commands"] + assert "codex" in registered, "use registers extensions for the new active agent" + assert ( + project / ".agents" / "skills" / "speckit-git-feature" / "SKILL.md" + ).exists() + # ── uninstall ──────────────────────────────────────────────────────── @@ -2492,13 +2539,13 @@ class TestIntegrationUpgrade: "shared .sh scripts must be executable after upgrade" ) - def test_upgrade_backfills_extension_commands_for_agent(self, tmp_path): - """Upgrade re-registers enabled extensions for the upgraded agent. + def test_upgrade_does_not_backfill_non_active_integration(self, tmp_path): + """Upgrading a non-active integration must not register extensions for it. - Regression for #2886: agents installed before extension back-fill - existed (or whose extension artifacts went missing) should regain the - enabled extensions' commands on ``upgrade``, reaching parity with - ``switch``. + Maintainer-requested behavior for #2948 (reverses the #2886 upgrade + back-fill): non-active integrations only receive extension artifacts + when selected via ``integration use`` / ``switch``. Upgrade of a + non-active integration refreshes its own files and nothing else. """ project = _init_project(tmp_path, "claude") @@ -2511,21 +2558,10 @@ class TestIntegrationUpgrade: ]) assert result.exit_code == 0, result.output - # Simulate a project created before the install/upgrade back-fill: drop - # codex's extension registration and its rendered artifacts. registry_path = project / ".specify" / "extensions" / ".registry" - registry = json.loads(registry_path.read_text(encoding="utf-8")) - registry["extensions"]["git"]["registered_commands"].pop("codex", None) - registry_path.write_text(json.dumps(registry), encoding="utf-8") - agents_skills = project / ".agents" / "skills" - for skill_dir in agents_skills.glob("speckit-git-*"): - shutil.rmtree(skill_dir) - - # Precondition: codex is now missing the git extension. assert "codex" not in json.loads(registry_path.read_text(encoding="utf-8"))[ "extensions" ]["git"]["registered_commands"] - assert not (agents_skills / "speckit-git-feature" / "SKILL.md").exists() result = _run_in_project(project, [ "integration", "upgrade", "codex", @@ -2533,12 +2569,41 @@ class TestIntegrationUpgrade: ]) assert result.exit_code == 0, result.output - # Upgrade back-filled the git extension for codex. registered = json.loads(registry_path.read_text(encoding="utf-8"))[ "extensions" ]["git"]["registered_commands"] - assert "codex" in registered, "upgrade should re-register extension commands (#2886)" - assert (agents_skills / "speckit-git-feature" / "SKILL.md").exists() + assert "codex" not in registered, ( + "upgrade must not back-fill non-active integrations (#2948)" + ) + assert not ( + project / ".agents" / "skills" / "speckit-git-feature" / "SKILL.md" + ).exists() + + def test_upgrade_active_integration_reregisters_extensions(self, tmp_path): + """Upgrading the active integration restores its extension commands. + + The active integration keeps the re-registration pass on upgrade so + missing or stale extension command files are recreated (#2948 scopes + the pass to the active integration; #2886 introduced it). + """ + project = _init_project(tmp_path, "claude") + + result = _run_in_project(project, ["extension", "add", "git"]) + assert result.exit_code == 0, f"extension add failed: {result.output}" + + cmd_file = project / ".claude" / "skills" / "speckit-git-feature" / "SKILL.md" + assert cmd_file.exists(), "precondition: extension command registered" + cmd_file.unlink() + + result = _run_in_project(project, [ + "integration", "upgrade", "claude", + "--script", "sh", + ]) + assert result.exit_code == 0, result.output + + assert cmd_file.exists(), ( + "upgrade of the active integration re-registers extension commands" + ) def test_upgrade_non_active_agent_preserves_active_agent_skills(self, tmp_path): """Upgrading a non-active agent must not touch the active agent's skills. diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 288518036..98be75dd1 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -1148,6 +1148,7 @@ class TestExtensionManager: link_outputs=False, create_missing_active_skills_dir=False, extension_id=None, + only_agent=None, ): captured["create_missing_active_skills_dir"] = ( create_missing_active_skills_dir @@ -7448,7 +7449,7 @@ $ARGUMENTS from specify_cli import app from specify_cli.agents import CommandRegistrar - project_dir, ext_dir, claude_commands_dir = self._setup_mock_extension(tmp_path, "claude") + project_dir, ext_dir, agents_commands_dir = self._setup_mock_extension(tmp_path, "amp") # 3. Run specify extension add runner = CliRunner() @@ -7465,8 +7466,8 @@ $ARGUMENTS assert "speckit-mock-ext-hello" not in result.output # Verify on-disk command names are dotted - hello_file = claude_commands_dir / "speckit.mock-ext.hello.md" - greet_file = claude_commands_dir / "speckit.mock-ext.greet.md" + hello_file = agents_commands_dir / "speckit.mock-ext.hello.md" + greet_file = agents_commands_dir / "speckit.mock-ext.greet.md" assert hello_file.exists() assert greet_file.exists()