Fix unscoped extension-skill removal and legacy preset provenance on direct remove

- _unregister_extension_skills(): omitting skills_dir now always triggers
  the full multi-directory fallback scan instead of narrowing to the
  currently active agent's directory. Previously, remove() (the only
  caller that omits skills_dir) would resolve the active agent's dir and
  take the scoped fast path, orphaning a previously-active second agent's
  extension skill mirror during full removal.

- PresetManager.remove(): infer legacy flat-list registered_skills
  provenance (reusing _infer_legacy_skill_provenance from the prior
  rescaffold fix) before invoking _unregister_skills, so a direct
  `preset remove` with no intervening rescaffold/switch also restores
  every previously-active agent's directory instead of only the
  currently active one.

Added regression tests:
- test_remove_while_second_agent_still_in_skills_mode_cleans_up_first_agent_mirror
- test_remove_infers_legacy_flat_list_provenance_without_prior_rescaffold

Assisted-by: GitHub Copilot (model: Claude Sonnet 5, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

src/specify_cli/extensions/__init__.py

@@ -1241,32 +1241,32 @@ class ExtensionManager:
         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
+        When *skills_dir* is omitted, this is a genuinely unscoped removal
-        ``None`` (e.g. the user removed init-options.json or toggled
+        (e.g. full extension removal): ``registered_skills`` is a single
-        ai_skills after installation), we fall back to scanning all known
+        flat list covering mirrors created under *every* agent this
-        agent skills directories so that orphaned skill directories are
+        extension was ever activated under, not just the currently active
-        still cleaned up.  In that case each candidate directory is
+        one, so we always scan all known agent skills directories rather
-        verified against the SKILL.md ``metadata.source`` field before
+        than narrowing to whichever agent happens to be active right now.
-        removal to avoid accidentally deleting user-created skills with
+        Each candidate directory is verified against the SKILL.md
-        the same name.
+        ``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
+            skills_dir: Optional explicit skills directory to scope
-                of resolving via ``_get_skills_dir()``.  Useful when the
+                cleanup to. Useful when the caller needs to target a
-                caller needs to target a specific agent's skills directory
+                specific agent's skills directory regardless of the
-                regardless of the currently-active agent in init-options.
+                currently-active agent in init-options. When omitted,
+                every configured agent's skills directory is scanned
+                instead of resolving just the currently active one.
         """
         if not skill_names:
             return
 
         from ..shared_infra import _validate_safe_shared_directory
 
-        if skills_dir is None:
-            skills_dir = self._get_skills_dir()
-
         if skills_dir:
             # Reject the candidate directory itself (any path component,
             # including the final one) if it's a symlink escaping the

src/specify_cli/presets/__init__.py

@@ -2247,6 +2247,30 @@ class PresetManager:
         metadata = self.registry.get(pack_id)
         # Restore original skills when preset is removed
         registered_skills = metadata.get("registered_skills", []) if metadata else []
+        if isinstance(registered_skills, list) and registered_skills:
+            # Legacy flat-list registries predate per-agent provenance
+            # tracking. Migration to the per-agent dict form previously
+            # only happened during a rescaffold (register_enabled_presets_
+            # for_agent); if the *first* post-upgrade operation is instead
+            # `preset remove` (no intervening use/upgrade), the legacy
+            # branch of _unregister_skills restores only the currently
+            # active agent's directory, leaving this preset's overrides in
+            # every previously active agent's directory orphaned. Infer
+            # real per-agent ownership from the on-disk preset marker now,
+            # while pack_id is still known, and hand the resulting mapping
+            # through the same dict-based cleanup path already used for
+            # non-legacy registries (#2948).
+            from .. import load_init_options
+
+            init_opts = load_init_options(self.project_root)
+            fallback_agent = init_opts.get("ai") if isinstance(init_opts, dict) else None
+            if not isinstance(fallback_agent, str):
+                fallback_agent = ""
+            registered_skills = self._infer_legacy_skill_provenance(
+                [name for name in registered_skills if isinstance(name, str)],
+                pack_id,
+                fallback_agent=fallback_agent,
+            )
         registered_commands = metadata.get("registered_commands", {}) if metadata else {}
         pack_dir = self.presets_dir / pack_id
 

tests/test_extension_skills.py

@@ -1448,6 +1448,68 @@ class TestExtensionSkillRegistration:
             "prematurely dropped during the earlier toggle (#2948)"
         )
 
+    def test_remove_while_second_agent_still_in_skills_mode_cleans_up_first_agent_mirror(
+        self, project_dir, temp_dir
+    ):
+        """Full extension removal must clean up every previously-active
+        agent's mirror, not just the currently active one, even with no
+        intervening toggle.
+
+        Auggie is activated first (skills mode), writing a mirror.
+        Copilot is then activated (also skills mode, still active at
+        removal time) and writes its own mirror for the same names.
+        ``remove()`` calls ``_unregister_extension_skills(registered_skills,
+        extension_id)`` with no explicit ``skills_dir`` — genuinely
+        unscoped, "clean up everywhere this extension owns something".
+        Before this fix, omitting ``skills_dir`` caused the method to
+        resolve the *currently active* agent's directory via
+        ``_get_skills_dir()`` and take the scoped fast path instead of the
+        all-directory fallback scan, so only Copilot's mirror was removed
+        and Auggie's was silently left orphaned (#2948).
+        """
+        _create_init_options(project_dir, ai="auggie", ai_skills=True)
+        manager = ExtensionManager(project_dir)
+        manager.install_from_directory(
+            _create_extension_dir(temp_dir, ext_id="remove-multi-agent-ext"), "0.1.0",
+            register_commands=False,
+        )
+        manager.register_enabled_extensions_for_agent("auggie")
+
+        auggie_skills_dir = project_dir / ".augment" / "skills"
+        auggie_hello = auggie_skills_dir / "speckit-remove-multi-agent-ext-hello" / "SKILL.md"
+        auggie_world = auggie_skills_dir / "speckit-remove-multi-agent-ext-world" / "SKILL.md"
+        assert auggie_hello.exists() and auggie_world.exists(), (
+            "sanity: auggie's skills-mode activation should mirror both "
+            "extension commands as SKILL.md files"
+        )
+
+        _create_init_options(project_dir, ai="copilot", ai_skills=True)
+        manager.register_enabled_extensions_for_agent("copilot")
+
+        copilot_skills_dir = project_dir / ".github" / "skills"
+        copilot_hello = copilot_skills_dir / "speckit-remove-multi-agent-ext-hello" / "SKILL.md"
+        copilot_world = copilot_skills_dir / "speckit-remove-multi-agent-ext-world" / "SKILL.md"
+        assert copilot_hello.exists() and copilot_world.exists(), (
+            "sanity: copilot's skills-mode activation should also mirror "
+            "both extension commands"
+        )
+
+        # Remove the extension while copilot (the second agent) is still
+        # the active, skills-mode agent — no toggle, no intervening
+        # rescaffold for auggie.
+        assert manager.remove("remove-multi-agent-ext") is True
+
+        assert not copilot_hello.exists() and not copilot_world.exists(), (
+            "sanity: the currently active agent's mirrors must be removed"
+        )
+        assert not auggie_hello.exists() and not auggie_world.exists(), (
+            "removal must also clean up the first agent's (auggie) "
+            "mirrors even though it is no longer the active agent — "
+            "omitting skills_dir must trigger the all-directory fallback "
+            "scan, not silently narrow to the currently active agent's "
+            "directory (#2948)"
+        )
+
     def test_extension_owned_skill_names_rejects_symlinked_candidate_directory(
         self, project_dir, temp_dir
     ):

tests/test_presets.py

@@ -4907,6 +4907,83 @@ class TestPresetSkills:
             "side effect of probing for provenance (#2948)"
         )
 
+    def test_remove_infers_legacy_flat_list_provenance_without_prior_rescaffold(
+        self, project_dir, temp_dir
+    ):
+        """``preset remove`` on a legacy flat-list registry must restore
+        every previously active agent's directory, not just the currently
+        active one, even when it is the *very first* post-upgrade
+        operation (no intervening ``use``/``upgrade``/rescaffold).
+
+        Pre-#2948 registries recorded a flat ``registered_skills`` list
+        because presets were rendered for every detected skill-mode agent
+        at once, not just the active one. Migrating that legacy format to
+        the per-agent dict form previously only happened as a side effect
+        of ``register_enabled_presets_for_agent`` (i.e. a rescaffold or
+        ``integration use``/``switch``). If the user's first action after
+        upgrading is instead directly running ``preset remove``, the
+        legacy branch of ``_unregister_skills`` restored only the
+        currently active agent's directory (via ``_get_skills_dir()``),
+        permanently leaving this preset's override in every other,
+        previously active agent's directory (#2948).
+        """
+        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",
+            encoding="utf-8",
+        )
+
+        claude_skills_dir = project_dir / ".claude" / "skills"
+        self._create_skill(claude_skills_dir, "speckit-specify")
+        codex_skills_dir = project_dir / ".agents" / "skills"
+        self._create_skill(codex_skills_dir, "speckit-specify")
+
+        preset_dir = self._create_command_preset(
+            temp_dir, "remove-legacy-no-rescaffold-preset", "speckit.specify",
+            "Remove legacy no rescaffold test", "preset body",
+        )
+        manager = PresetManager(project_dir)
+        manager.install_from_directory(preset_dir, "0.1.5")
+
+        claude_skill = claude_skills_dir / "speckit-specify" / "SKILL.md"
+        assert "preset:remove-legacy-no-rescaffold-preset" in claude_skill.read_text(), (
+            "sanity: install should have written the override under "
+            "claude's skill directory"
+        )
+        # Simulate the pre-#2948 "register for every detected agent"
+        # install behaviour by also placing the marker under codex's
+        # directory directly (mirroring the old, non-active-only
+        # rendering that predates this PR).
+        codex_skill = codex_skills_dir / "speckit-specify" / "SKILL.md"
+        codex_skill.write_text(claude_skill.read_text(), encoding="utf-8")
+
+        # Simulate a pre-#2948 registry: a flat list with no per-agent
+        # provenance, even though both directories actually hold this
+        # preset's marker on disk.
+        manager.registry.update(
+            "remove-legacy-no-rescaffold-preset",
+            {"registered_skills": ["speckit-specify"]},
+        )
+
+        # No intervening use/upgrade/rescaffold: remove() is the very
+        # first operation run after the legacy registry was written.
+        assert manager.remove("remove-legacy-no-rescaffold-preset") is True
+
+        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()
+            assert "preset:remove-legacy-no-rescaffold-preset" not in content, (
+                f"{label}'s preset override must be restored on removal "
+                "even with no prior rescaffold to migrate the legacy "
+                "flat-list format first — remove() must infer real "
+                "per-agent ownership from on-disk provenance itself "
+                "(#2948)"
+            )
+            assert "Core specify body" in content
+
     def test_symlinked_skills_dir_rejected_on_removal(self, project_dir, temp_dir):
         """Removal must validate a recorded skill directory before touching it.