fix: clean up extension skills across all agents on remove (#2948)

Addresses Copilot review feedback: remove() called
_unregister_extension_skills() without a directory, so its fast
path removed skill files only from the resolved active-agent
directory whenever one resolved, and never scanned the
directories of other agents whose skills were rendered via the
new multi-agent registration paths added earlier in this PR —
leaving those orphaned on the filesystem after removal.

_unregister_extension_skills() now always scans every known
agent skills directory (previously this broader scan only ran
as a fallback when no active-agent directory could be resolved
at all), in addition to any explicitly-passed or resolved
directory. Each candidate directory's matching skill
subdirectories are still verified against the SKILL.md
metadata.source field before removal, so this doesn't touch
the existing safety guarantees or the registered_skills schema
(intentionally kept as a flat, deduped list rather than a
per-agent dict, to avoid a breaking registry-schema change for
this fix).

Tests:
- Added test_remove_cleans_up_skills_for_non_active_agent,
  covering removal after skills were rendered for both an
  active and a non-active agent.
- pytest tests/ -k extension: 620/620 pass (619 previous + 1 new).
- git diff --check: clean.

Note: a related Copilot suggestion to also have plain
'integration install' immediately register extension
commands/skills for the newly-installed agent was evaluated but
not applied here, since it directly conflicts with the
maintainer-requested, explicitly-tested deferred-registration
design from #2886 (see
tests/integrations/test_integration_subcommand.py::TestIntegrationInstall::test_install_defers_extension_commands_until_use).
Flagging this for maintainer input rather than silently
reversing that prior decision.

Author: Nimraakram12

src/specify_cli/extensions/__init__.py

@@ -1299,19 +1299,19 @@ def _unregister_extension_skills(
         Called during extension removal to clean up skill files that
         were created by ``_register_extension_skills()``.
 
-        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.
+        Skills may have been rendered for more than one agent (#2948),
+        so this always scans every known agent skills directory (plus
+        *skills_dir* or the resolved active-agent directory, if any)
+        rather than only a single resolved directory, to avoid orphaning
+        skills registered for a non-active agent. 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.
+                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
@@ -1365,8 +1365,13 @@ def _unregister_extension_skills(
                 except (OSError, UnicodeDecodeError, Exception):
                     continue
                 shutil.rmtree(skill_subdir)
-        else:
-            # Fallback: scan all possible agent skills directories
+        # Additionally always scan every other known agent skills
+        # directory: skills may have been rendered for more than one
+        # agent (#2948), so limiting cleanup to only the directory above
+        # would orphan skills registered for a non-active agent.
+        # Directories already handled above are silently skipped since
+        # their matching skill subdirectories no longer exist.
+        if True:
             from .. import AGENT_CONFIG, DEFAULT_SKILLS_DIR
 
             candidate_dirs: set[Path] = set()

tests/test_extension_skills.py

@@ -2034,3 +2034,42 @@ def test_extension_add_renders_skills_for_all_installed_skills_mode_agents(
         assert (
             copilot_skills_dir / "speckit-test-ext-hello" / "SKILL.md"
         ).exists()
+
+    def test_remove_cleans_up_skills_for_non_active_agent(
+        self, project_dir, extension_dir
+    ):
+        """remove() must clean up skills rendered for a non-active agent too (#2948)."""
+        _create_init_options(project_dir, ai="claude", ai_skills=True)
+        claude_skills_dir = _create_skills_dir(project_dir, ai="claude")
+        _create_integration_json(
+            project_dir,
+            default_agent="claude",
+            installed=["claude", "copilot"],
+            skills_by_agent={"copilot": True},
+        )
+        copilot_skills_dir = _create_skills_dir(project_dir, ai="copilot")
+
+        manager = ExtensionManager(project_dir)
+        manifest = manager.install_from_directory(
+            extension_dir, "0.1.0", register_commands=False
+        )
+
+        # Precondition: both agents got the skill rendered.
+        assert (
+            claude_skills_dir / "speckit-test-ext-hello" / "SKILL.md"
+        ).exists()
+        assert (
+            copilot_skills_dir / "speckit-test-ext-hello" / "SKILL.md"
+        ).exists()
+
+        result = manager.remove(manifest.id, keep_config=False)
+        assert result is True
+
+        # Removal must clean up both agents' skill directories, not just
+        # the active one.
+        assert not (
+            claude_skills_dir / "speckit-test-ext-hello"
+        ).exists()
+        assert not (
+            copilot_skills_dir / "speckit-test-ext-hello"
+        ).exists()