Reconcile every historical agent on preset removal; validate child skill dirs

Fixes 3 findings from the Copilot review on HEAD 31c9b97 (#2948):

1. presets/__init__.py: remove()'s command reconciliation only recreated
   the surviving preset's content for the currently active agent, even
   though the removed preset's registered_commands could span multiple
   historical (now-inactive) agents recorded via prior rescaffolds. Now
   remove() captures every historical agent registered_commands actually
   targeted (before mutation) and passes it as extra_agents through
   _reconcile_composed_commands -> _register_for_non_skill_agents /
   _register_command_from_path -> registrar.register_commands_for_non_
   skill_agents, so the active-only restriction for install/use is
   preserved while post-removal reconciliation restores every touched
   directory.

2. presets/__init__.py: the analogous gap existed for skills. _unregister_
   skills() now returns {skills_dir: renderer_agent} for every directory it
   actually restored, and _reconcile_skills() accepts extra_skills_dirs to
   reconcile each of those directories (via a new apply_to_dir() helper),
   not only the currently active skills directory. _register_skills() gained
   optional target_dir/target_agent overrides (forcing
   create_missing_skills off for non-active directories) so a historical
   directory is only ever restored, never seeded with brand-new skills.

3. extensions/__init__.py: _extension_owned_skill_names() and both the
   fast and fallback paths of _unregister_extension_skills() validated only
   the parent skills_dir for symlink escape, then resolved
   skills_dir / skill_name and checked containment relative to that
   already-resolved parent. A per-skill child that is itself a symlink to
   a different, legitimate skill directory within the same (safe) root
   passed that containment check, so deleting/attributing through the
   symlink name could destroy or misattribute an unrelated skill reached
   only via the alias. All three call sites now run the shared
   _validate_safe_shared_directory() component-wise check against the full
   skills_dir / skill_name path (not just the parent) before any read or
   delete, rejecting a symlinked child outright rather than following it,
   even when its resolved target remains in-bounds.

Regression tests added (all confirmed red against pre-fix code, green
after):
- test_remove_reconciles_command_for_every_historical_agent
- test_remove_reconciles_skill_for_every_historical_agent
- test_extension_owned_skill_names_rejects_symlinked_child_skill_dir
- test_unregister_extension_skills_explicit_dir_rejects_symlinked_child
- test_unregister_extension_skills_fallback_rejects_symlinked_child

Tests: tests/test_presets.py (361), tests/test_extension_skills.py (69),
tests/test_extensions.py (338) all pass; tests/integrations (1768 passed,
1 skipped) pass; full suite 3902 passed / 74 skipped (90 pre-existing,
environment-only git-signing tests deselected — confirmed failing
identically on the pre-change baseline due to local 1Password SSH-agent
signing, unrelated to this change). ruff check clean on all changed files.

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

src/specify_cli/agents.py

@@ -11,7 +11,7 @@ import platform
 import re
 from copy import deepcopy
 from pathlib import Path
-from typing import Any, Dict, List, Optional
+from typing import Any, Dict, Iterable, List, Optional
 
 import yaml
 
@@ -1083,6 +1083,7 @@ class CommandRegistrar:
         link_outputs: bool = False,
         extension_id: Optional[str] = None,
         only_agent: Optional[str] = None,
+        extra_agents: Optional[Iterable[str]] = None,
     ) -> Dict[str, List[str]]:
         """Register commands for all non-skill agents in the project.
 
@@ -1102,14 +1103,25 @@ class CommandRegistrar:
             only_agent: If set, restrict registration to this single agent
                 (#2948). An agent name that matches no configured agent
                 (e.g. an empty string) yields no registrations at all.
+            extra_agents: Additional agent names to register for besides
+                ``only_agent``. Used by post-removal reconciliation to also
+                restore surviving content into historical agent directories
+                a just-removed preset actually wrote to, not only the
+                currently active agent (#2948). Ignored when ``only_agent``
+                is ``None`` (already unrestricted).
 
         Returns:
             Dictionary mapping agent names to list of registered commands
         """
         results = {}
         self._ensure_configs()
+        extra_agents_set = frozenset(extra_agents) if extra_agents else frozenset()
         for agent_name, agent_config in self.AGENT_CONFIGS.items():
-            if only_agent is not None and agent_name != only_agent:
+            if (
+                only_agent is not None
+                and agent_name != only_agent
+                and agent_name not in extra_agents_set
+            ):
                 continue
             if agent_config.get("extension") == "/SKILL.md":
                 continue

src/specify_cli/extensions/__init__.py

@@ -1290,10 +1290,19 @@ class ExtensionManager:
                 sn_path = Path(skill_name)
                 if sn_path.is_absolute() or len(sn_path.parts) != 1:
                     continue
+                skill_subdir = skills_dir / skill_name
+                # Validate every path component down to the skill's own
+                # subdirectory, not just the already-validated parent
+                # skills_dir: a per-skill child can itself be a symlink to
+                # another directory whose *resolved* target still lands
+                # inside this same (safe) skills root, which the previous
+                # resolve()+relative_to() containment check alone would
+                # not catch. Reject the symlink outright rather than
+                # following it, even when the target is otherwise
+                # in-bounds (#2948).
                 try:
-                    skill_subdir = (skills_dir / skill_name).resolve()
+                    _validate_safe_shared_directory(self.project_root, skill_subdir)
-                    skill_subdir.relative_to(skills_dir.resolve())  # raises if outside
+                except (ValueError, OSError):
-                except (OSError, ValueError):
                     continue
                 if not skill_subdir.is_dir():
                     continue
@@ -1354,12 +1363,19 @@ class ExtensionManager:
                     sn_path = Path(skill_name)
                     if sn_path.is_absolute() or len(sn_path.parts) != 1:
                         continue
+                    skill_subdir = skills_candidate / skill_name
+                    # Validate every path component down to the skill's
+                    # own subdirectory, not just the already-validated
+                    # candidate parent: a per-skill child can itself be a
+                    # symlink to another directory whose resolved target
+                    # still lands inside this same candidate, which the
+                    # previous resolve()+relative_to() containment check
+                    # alone would not catch (#2948).
                     try:
-                        skill_subdir = (skills_candidate / skill_name).resolve()
+                        _validate_safe_shared_directory(
-                        skill_subdir.relative_to(
+                            self.project_root, skill_subdir
-                            skills_candidate.resolve()
+                        )
-                        )  # raises if outside
+                    except (ValueError, OSError):
-                    except (OSError, ValueError):
                         continue
                     if not skill_subdir.is_dir():
                         continue
@@ -1450,20 +1466,23 @@ class ExtensionManager:
                 _validate_safe_shared_directory(self.project_root, skills_candidate)
             except (ValueError, OSError):
                 continue
-            try:
-                resolved_candidate = skills_candidate.resolve()
-            except OSError:
-                continue
             for skill_name in skill_names:
                 if skill_name in owned:
                     continue
                 sn_path = Path(skill_name)
                 if sn_path.is_absolute() or len(sn_path.parts) != 1:
                     continue
+                skill_subdir = skills_candidate / skill_name
+                # Validate every path component down to the skill's own
+                # subdirectory, not just the already-validated candidate
+                # parent: a per-skill child can itself be a symlink to
+                # another directory whose resolved target still lands
+                # inside this same candidate, which the previous
+                # resolve()+relative_to() containment check alone would
+                # not catch (#2948).
                 try:
-                    skill_subdir = (skills_candidate / skill_name).resolve()
+                    _validate_safe_shared_directory(self.project_root, skill_subdir)
-                    skill_subdir.relative_to(resolved_candidate)  # raises if outside
+                except (ValueError, OSError):
-                except (OSError, ValueError):
                     continue
                 if not skill_subdir.is_dir():
                     continue

src/specify_cli/presets/__init__.py

@@ -16,7 +16,7 @@ import zipfile
 import shutil
 from dataclasses import dataclass
 from pathlib import Path
-from typing import TYPE_CHECKING, Optional, Dict, List, Any, Union
+from typing import TYPE_CHECKING, Optional, Dict, List, Any, Union, Set
 
 if TYPE_CHECKING:
     from ..agents import CommandRegistrar
@@ -910,7 +910,9 @@ class PresetManager:
         registrar = CommandRegistrar()
         registrar.unregister_commands(registered_commands, self.project_root)
 
-    def _reconcile_composed_commands(self, command_names: List[str]) -> None:
+    def _reconcile_composed_commands(
+        self, command_names: List[str], extra_agents: Optional[Set[str]] = None
+    ) -> None:
         """Re-resolve and re-register composed commands from the full stack.
 
         After install or remove, recompute the effective content for each
@@ -930,6 +932,13 @@ class PresetManager:
 
         Args:
             command_names: List of command names to reconcile
+            extra_agents: Additional agent names to also reconcile besides
+                the currently active one. Populated by ``remove()`` with the
+                historical agents a just-removed preset's
+                ``registered_commands`` actually targeted, so a surviving
+                lower-priority preset's content is restored there too — not
+                only for the currently active agent (#2948). Install/use
+                callers omit this, preserving pure active-only behavior.
         """
         if not command_names:
             return
@@ -996,7 +1005,7 @@ class PresetManager:
                                 if tmpl.get("name") == cmd_name and tmpl.get("type") == "command":
                                     self._register_for_non_skill_agents(
                                         registrar, [tmpl], manifest.id, pack_dir,
-                                        only_agent=only_agent,
+                                        only_agent=only_agent, extra_agents=extra_agents,
                                     )
                                     registered = True
                                     break
@@ -1024,7 +1033,7 @@ class PresetManager:
                                         matching_cmds, ext_id, ext_dir,
                                         self.project_root,
                                         context_note=f"\n<!-- Extension: {ext_id} -->\n<!-- Config: .specify/extensions/{ext_id}/ -->\n",
-                                        only_agent=only_agent,
+                                        only_agent=only_agent, extra_agents=extra_agents,
                                     )
                                     registered = True
                             except Exception:
@@ -1036,7 +1045,7 @@ class PresetManager:
                         self._register_command_from_path(
                             registrar, cmd_name, top_path,
                             source_id=source_id,
-                            only_agent=only_agent,
+                            only_agent=only_agent, extra_agents=extra_agents,
                         )
             else:
                 # Composed command — resolve from full stack
@@ -1073,7 +1082,11 @@ class PresetManager:
                             agent: cmd_names_to_unregister
                             for agent in registrar.AGENT_CONFIGS
                             if registrar.AGENT_CONFIGS[agent].get("extension") != "/SKILL.md"
-                            and (only_agent is None or agent == only_agent)
+                            and (
+                                only_agent is None
+                                or agent == only_agent
+                                or agent in (extra_agents or ())
+                            )
                         },
                         self.project_root,
                     )
@@ -1096,7 +1109,7 @@ class PresetManager:
                                 registrar,
                                 [{**tmpl, "file": f".composed/{cmd_name}.md"}],
                                 manifest.id, pack_dir,
-                                only_agent=only_agent,
+                                only_agent=only_agent, extra_agents=extra_agents,
                             )
                             registered = True
                             break
@@ -1118,7 +1131,7 @@ class PresetManager:
                     self._register_command_from_path(
                         registrar, cmd_name, composed_file,
                         source_id=source_id,
-                        only_agent=only_agent,
+                        only_agent=only_agent, extra_agents=extra_agents,
                     )
 
     def _register_command_from_path(
@@ -1128,6 +1141,7 @@ class PresetManager:
         cmd_path: Path,
         source_id: str = "reconciled",
         only_agent: Optional[str] = None,
+        extra_agents: Optional[Set[str]] = None,
     ) -> None:
         """Register a single command from a file path (non-preset source).
 
@@ -1140,6 +1154,8 @@ class PresetManager:
             cmd_path: Path to the command file
             source_id: Source attribution for rendered output
             only_agent: If set, restrict registration to this single agent (#2948).
+            extra_agents: Additional agent names to register for besides
+                ``only_agent`` (post-removal reconciliation only, #2948).
         """
         if not cmd_path.exists():
             return
@@ -1170,7 +1186,7 @@ class PresetManager:
                 pass  # best-effort alias loading
         self._register_for_non_skill_agents(
             registrar, [cmd_tmpl], source_id, cmd_path.parent,
-            only_agent=only_agent,
+            only_agent=only_agent, extra_agents=extra_agents,
         )
 
     def _register_for_non_skill_agents(
@@ -1180,6 +1196,7 @@ class PresetManager:
         source_id: str,
         source_dir: Path,
         only_agent: Optional[str] = None,
+        extra_agents: Optional[Set[str]] = None,
     ) -> None:
         """Register commands for non-skill agents during reconciliation.
 
@@ -1198,10 +1215,14 @@ class PresetManager:
             only_agent: If set, restrict registration to this single agent,
                 matching the active-only rule applied by ``_register_commands``
                 (#2948).
+            extra_agents: Additional agent names to register for besides
+                ``only_agent``. Used by post-removal reconciliation to also
+                restore surviving content into historical agent directories
+                a just-removed preset actually wrote to (#2948).
         """
         registrar.register_commands_for_non_skill_agents(
             commands, source_id, source_dir, self.project_root,
-            only_agent=only_agent,
+            only_agent=only_agent, extra_agents=extra_agents,
         )
 
     class _FilteredManifest:
@@ -1225,7 +1246,11 @@ class PresetManager:
                 if t.get("name") in self._cmd_names
             ]
 
-    def _reconcile_skills(self, command_names: List[str]) -> None:
+    def _reconcile_skills(
+        self,
+        command_names: List[str],
+        extra_skills_dirs: Optional[Dict[Path, Optional[str]]] = None,
+    ) -> None:
         """Re-register skills for commands whose winning layer changed.
 
         After a preset is removed, finds the next preset in the priority
@@ -1234,52 +1259,66 @@ class PresetManager:
 
         Args:
             command_names: List of command names to reconcile skills for
+            extra_skills_dirs: Additional ``{skills_dir: renderer_agent}``
+                pairs to reconcile besides the currently active skills
+                directory. Populated by ``remove()`` from the directories
+                ``_unregister_skills`` just restored to core/extension
+                content: a surviving lower-priority preset's override must
+                be re-applied to every one of those directories too, not
+                only the currently active agent's directory, or an
+                inactive integration is left with stale/missing content
+                even though the removed preset no longer wins there
+                either (#2948).
         """
         if not command_names:
             return
 
         resolver = PresetResolver(self.project_root)
-        skills_dir = self._get_skills_dir()
+        active_skills_dir = self._get_skills_dir()
+
+        from .. import load_init_options
+
+        init_opts = load_init_options(self.project_root)
+        active_ai = init_opts.get("ai") if isinstance(init_opts, dict) else None
+        if not isinstance(active_ai, str) or not active_ai:
+            active_ai = None
 
         # Cache registry once to avoid repeated filesystem reads
         presets_by_priority = list(self.registry.list_by_priority())
 
         # Group command names by winning preset to batch _register_skills calls
-        # while only registering skills for the specific commands being reconciled.
+        # while only registering skills for the specific commands being
+        # reconciled. This resolution (which preset/content wins) is
+        # directory-independent, so it's computed once and then applied to
+        # every affected directory below.
         preset_cmds: Dict[str, List[str]] = {}
         non_preset_skills: List[tuple] = []
+        managed_skill_names: set = set()
 
         for cmd_name in command_names:
             layers = resolver.collect_all_layers(cmd_name, "command")
             if not layers:
                 continue
 
-            # Re-create the skill directory only if it was previously managed
+            skill_name, _ = self._skill_names_for_command(cmd_name)
-            # (i.e., listed in some preset's registered_skills). This avoids
+            # Track whether any preset previously registered this skill
-            # creating new skill dirs that _register_skills would normally skip.
+            # (i.e., it was actively managed), so a not-yet-existing skill
-            if skills_dir:
+            # dir can be re-created per affected directory below.
-                skill_name, _ = self._skill_names_for_command(cmd_name)
+            for _pid, meta in presets_by_priority:
-                skill_subdir = skills_dir / skill_name
+                if not isinstance(meta, dict):
-                if not skill_subdir.exists():
+                    continue
-                    # Check if any preset previously registered this skill
+                recorded = meta.get("registered_skills", [])
-                    was_managed = False
+                if isinstance(recorded, dict):
-                    for _pid, meta in presets_by_priority:
+                    in_any_agent = any(
-                        if not isinstance(meta, dict):
+                        skill_name in names
-                            continue
+                        for names in recorded.values()
-                        recorded = meta.get("registered_skills", [])
+                        if isinstance(names, list)
-                        if isinstance(recorded, dict):
+                    )
-                            in_any_agent = any(
+                else:
-                                skill_name in names
+                    in_any_agent = skill_name in recorded
-                                for names in recorded.values()
+                if in_any_agent:
-                                if isinstance(names, list)
+                    managed_skill_names.add(skill_name)
-                            )
+                    break
-                        else:
-                            in_any_agent = skill_name in recorded
-                        if in_any_agent:
-                            was_managed = True
-                            break
-                    if was_managed:
-                        skill_subdir.mkdir(parents=True, exist_ok=True)
 
             top_path = layers[0]["path"]
             # Find the preset that owns the winning layer
@@ -1293,39 +1332,44 @@ class PresetManager:
             if not found_preset:
                 # Winner is a non-preset source (core/extension/override).
                 # Track the winning layer path for skill restoration.
-                skill_name, _ = self._skill_names_for_command(cmd_name)
                 non_preset_skills.append((skill_name, cmd_name, layers[0]))
 
-        # Restore skills for commands whose winner is non-preset.
+        core_ext_skills = [s for s in non_preset_skills if s[2]["source"] != "project override"]
-        if non_preset_skills and skills_dir:
+        override_skills = [s for s in non_preset_skills if s[2]["source"] == "project override"]
-            # Separate override-backed skills from core/extension-backed ones.
-            # _unregister_skills can rmtree the skill dir, so overrides must
-            # be handled directly (create dir + write) without that call.
-            core_ext_skills = []
-            override_skills = []
-            for item in non_preset_skills:
-                if item[2]["source"] == "project override":
-                    override_skills.append(item)
-                else:
-                    core_ext_skills.append(item)
 
+        def apply_to_dir(
+            skills_dir: Path, dir_agent: Optional[str], *, is_active: bool
+        ) -> None:
+            # Re-create the skill directory only if it was previously
+            # managed (i.e., listed in some preset's registered_skills).
+            # This avoids creating new skill dirs that _register_skills
+            # would normally skip.
+            for skill_name in managed_skill_names:
+                skill_subdir = skills_dir / skill_name
+                if not skill_subdir.exists():
+                    skill_subdir.mkdir(parents=True, exist_ok=True)
+
+            # Restore skills for commands whose winner is non-preset.
+            # _unregister_skills_in_dir can rmtree the skill dir, so
+            # overrides must be handled directly (create dir + write)
+            # without that call.
             if core_ext_skills:
-                self._unregister_skills(
+                self._unregister_skills_in_dir(
-                    [s[0] for s in core_ext_skills], self.presets_dir
+                    [s[0] for s in core_ext_skills], skills_dir, dir_agent
                 )
 
             for skill_name, cmd_name, top_layer in override_skills:
                 skill_subdir = skills_dir / skill_name
                 # Same symlink guard as _register_skills's registration path
-                # (#2948): mkdir(exist_ok=True) alone would silently follow an
+                # (#2948): mkdir(exist_ok=True) alone would silently follow
-                # existing symlinked subdirectory before writing SKILL.md
+                # an existing symlinked subdirectory before writing SKILL.md
                 # through it.
                 if not self._validate_skill_subdir(skill_subdir, create=True):
                     continue
                 skill_file = skill_subdir / "SKILL.md"
                 try:
                     from ..agents import CommandRegistrar
-                    from .. import SKILL_DESCRIPTIONS, load_init_options
+                    from .. import SKILL_DESCRIPTIONS
                     registrar = CommandRegistrar()
                     content = top_layer["path"].read_text(encoding="utf-8")
                     fm, body = registrar.parse_frontmatter(content)
@@ -1336,9 +1380,8 @@ class PresetManager:
                         short_name.replace(".", "-"),
                         f"Command: {short_name}",
                     )
-                    init_opts = load_init_options(self.project_root)
+                    selected_ai = dir_agent if isinstance(dir_agent, str) else ""
-                    selected_ai = init_opts.get("ai") if isinstance(init_opts, dict) else ""
+                    if selected_ai:
-                    if isinstance(selected_ai, str):
                         body = registrar.resolve_skill_placeholders(
                             selected_ai, fm, body, self.project_root
                         )
@@ -1346,10 +1389,9 @@ class PresetManager:
                             body, registrar, selected_ai
                         )
                     from ..integrations import get_integration
-                    integration = get_integration(selected_ai) if isinstance(selected_ai, str) else None
+                    integration = get_integration(selected_ai) if selected_ai else None
                     fm_data = registrar.build_skill_frontmatter(
-                        selected_ai if isinstance(selected_ai, str) else "",
+                        selected_ai, skill_name, desc,
-                        skill_name, desc,
                         f"override:{cmd_name}",
                     )
                     registrar.apply_argument_hint(fm, fm_data, integration)
@@ -1366,21 +1408,38 @@ class PresetManager:
                 except Exception:
                     pass  # best-effort override skill restoration
 
-        # Register skills only for the specific commands being reconciled,
+            # Register skills only for the specific commands being
-        # not all commands in each winning preset's manifest.
+            # reconciled, not all commands in each winning preset's
-        for pack_id, cmds in preset_cmds.items():
+            # manifest.
-            pack_dir = self.presets_dir / pack_id
+            for pack_id, cmds in preset_cmds.items():
-            manifest_path = pack_dir / "preset.yml"
+                pack_dir = self.presets_dir / pack_id
-            if not manifest_path.exists():
+                manifest_path = pack_dir / "preset.yml"
-                continue
+                if not manifest_path.exists():
-            try:
+                    continue
-                manifest = PresetManifest(manifest_path)
+                try:
-            except PresetValidationError:
+                    manifest = PresetManifest(manifest_path)
-                continue
+                except PresetValidationError:
-            # Filter manifest to only the commands being reconciled
+                    continue
-            cmds_set = set(cmds)
+                cmds_set = set(cmds)
-            filtered_manifest = self._FilteredManifest(manifest, cmds_set)
+                filtered_manifest = self._FilteredManifest(manifest, cmds_set)
-            self._register_skills(filtered_manifest, pack_dir)
+                if is_active:
+                    # Preserve exact prior behaviour for the currently
+                    # active directory (including the ability to create
+                    # brand-new skill subdirectories when ai_skills is on).
+                    self._register_skills(filtered_manifest, pack_dir)
+                else:
+                    self._register_skills(
+                        filtered_manifest, pack_dir,
+                        target_dir=skills_dir, target_agent=dir_agent or "",
+                    )
+
+        if active_skills_dir:
+            apply_to_dir(active_skills_dir, active_ai, is_active=True)
+
+        for extra_dir, extra_agent in (extra_skills_dirs or {}).items():
+            if extra_dir == active_skills_dir:
+                continue  # already reconciled above as the active directory
+            apply_to_dir(extra_dir, extra_agent, is_active=False)
 
     def _get_skills_dir(self) -> Optional[Path]:
         """Return the active skills directory for preset skill overrides.
@@ -1494,6 +1553,9 @@ class PresetManager:
         self,
         manifest: "PresetManifest",
         preset_dir: Path,
+        *,
+        target_dir: Optional[Path] = None,
+        target_agent: Optional[str] = None,
     ) -> Dict[str, List[str]]:
         """Generate SKILL.md files for preset command overrides.
 
@@ -1507,6 +1569,18 @@ class PresetManager:
         Args:
             manifest: Preset manifest.
             preset_dir: Installed preset directory.
+            target_dir: Explicit skills directory to render into, instead
+                of resolving the currently active one. Used by
+                ``_reconcile_skills`` to restore a surviving preset's
+                override into a historical (currently inactive) agent's
+                directory that removal of a higher-priority preset just
+                reverted (#2948).
+            target_agent: Explicit agent name to render for, paired with
+                ``target_dir``. When set, skills are only ever restored
+                into already-tracked directories/names — brand-new skill
+                subdirectories are never created for a non-active,
+                explicitly targeted directory (that creation path is only
+                meaningful for the currently active agent).
 
         Returns:
             ``{agent_name: [skill_name, ...]}`` for the single active
@@ -1535,7 +1609,7 @@ class PresetManager:
         if not filtered:
             return {}
 
-        skills_dir = self._get_skills_dir()
+        skills_dir = target_dir if target_dir is not None else self._get_skills_dir()
         if not skills_dir:
             return {}
 
@@ -1546,10 +1620,16 @@ class PresetManager:
         init_opts = load_init_options(self.project_root)
         if not isinstance(init_opts, dict):
             init_opts = {}
-        selected_ai = init_opts.get("ai")
+        selected_ai = target_agent if target_agent is not None else init_opts.get("ai")
         if not isinstance(selected_ai, str) or not selected_ai:
             return {}
-        ai_skills_enabled = is_ai_skills_enabled(init_opts)
+        # A target_dir/target_agent call reconciles an explicitly-known,
+        # already-tracked directory (see _reconcile_skills) rather than the
+        # currently active agent, so ai_skills_enabled must not be derived
+        # from the *current* project-wide toggle for that other agent — it
+        # only controls whether brand-new skill subdirectories may be
+        # created below, which is only meaningful for the active agent.
+        ai_skills_enabled = target_agent is None and is_ai_skills_enabled(init_opts)
         registrar = CommandRegistrar()
         integration = get_integration(selected_ai)
         agent_config = registrar.AGENT_CONFIGS.get(selected_ai, {})
@@ -1846,7 +1926,7 @@ class PresetManager:
         self,
         registered_skills: Union[Dict[str, List[str]], List[str]],
         preset_dir: Path,
-    ) -> None:
+    ) -> Dict[Path, Optional[str]]:
         """Restore original SKILL.md files after a preset is removed.
 
         For each skill that was overridden by the preset, attempts to
@@ -1867,9 +1947,17 @@ class PresetManager:
                 ``List[str]`` from a registry written before this
                 provenance tracking existed.
             preset_dir: The preset's installed directory (may already be deleted).
+
+        Returns:
+            ``{skills_dir: renderer_agent}`` for every directory actually
+            restored, so callers (e.g. ``remove()``) can hand these same
+            directories to :meth:`_reconcile_skills` — a surviving
+            lower-priority preset's override must be re-applied to every
+            directory this removal touched, not only the currently active
+            agent's directory (#2948).
         """
         if not registered_skills:
-            return
+            return {}
 
         if isinstance(registered_skills, dict):
             from .. import load_init_options
@@ -1909,25 +1997,32 @@ class PresetManager:
                     active_agent if active_agent in agents else sorted(agents)[0]
                 )
                 self._unregister_skills_in_dir(group["names"], skills_dir, renderer_agent)
-            return
+            return {
+                skills_dir: (
+                    active_agent if active_agent in group["agents"] else sorted(group["agents"])[0]
+                )
+                for skills_dir, group in groups.items()
+            }
 
         # Legacy flat-list format: no record of which agent directory these
         # names were written under, so best-effort restore is limited to the
         # currently active agent's directory (the pre-provenance behaviour).
         skills_dir = self._get_skills_dir()
         if not skills_dir:
-            return
+            return {}
         from .. import load_init_options
 
         init_opts = load_init_options(self.project_root)
         if not isinstance(init_opts, dict):
             init_opts = {}
         selected_ai = init_opts.get("ai")
+        selected_ai = selected_ai if isinstance(selected_ai, str) else None
         self._unregister_skills_in_dir(
             registered_skills,
             skills_dir,
-            selected_ai if isinstance(selected_ai, str) else None,
+            selected_ai,
         )
+        return {skills_dir: selected_ai}
 
     def _unregister_skills_in_dir(
         self, skill_names: List[str], skills_dir: Path, selected_ai: Optional[str]
@@ -2274,6 +2369,27 @@ class PresetManager:
         registered_commands = metadata.get("registered_commands", {}) if metadata else {}
         pack_dir = self.presets_dir / pack_id
 
+        # Record which historical agents this preset's registered_commands
+        # actually targeted, *before* any filtering below, so post-removal
+        # reconciliation can restore a surviving lower-priority preset's
+        # override into every one of those directories too — not only the
+        # currently active agent's. Without this, removing a preset that
+        # was rendered under a previously-active (now inactive) agent
+        # deletes that agent's command file via _unregister_commands below,
+        # but active-only reconciliation would only recreate the surviving
+        # winner for the current agent, leaving the inactive integration
+        # with a missing/stale file (#2948).
+        try:
+            from ..agents import CommandRegistrar as _CommandRegistrarForScope
+        except ImportError:
+            _CommandRegistrarForScope = None
+        affected_command_agents = {
+            agent_name
+            for agent_name in registered_commands
+            if _CommandRegistrarForScope is None
+            or _CommandRegistrarForScope.AGENT_CONFIGS.get(agent_name, {}).get("extension") != "/SKILL.md"
+        }
+
         # Collect ALL command names before filtering for reconciliation,
         # so commands registered only for skill-based agents are also
         # reconciled. Every command-type template's primary name is added
@@ -2304,8 +2420,9 @@ class PresetManager:
                 # names from registered_commands are still unregistered.
                 pass
 
+        affected_skill_dirs: Dict[Path, Optional[str]] = {}
         if registered_skills:
-            self._unregister_skills(registered_skills, pack_dir)
+            affected_skill_dirs = self._unregister_skills(registered_skills, pack_dir)
             try:
                 from ..agents import CommandRegistrar
             except ImportError:
@@ -2330,8 +2447,12 @@ class PresetManager:
         # re-resolve from the remaining stack so the next layer takes effect.
         if removed_cmd_names:
             try:
-                self._reconcile_composed_commands(list(removed_cmd_names))
+                self._reconcile_composed_commands(
-                self._reconcile_skills(list(removed_cmd_names))
+                    list(removed_cmd_names), extra_agents=affected_command_agents
+                )
+                self._reconcile_skills(
+                    list(removed_cmd_names), extra_skills_dirs=affected_skill_dirs
+                )
             except Exception as exc:
                 import warnings
                 warnings.warn(

tests/test_extension_skills.py

@@ -1824,6 +1824,149 @@ class TestExtensionSkillRegistration:
             "removed via an explicit symlinked directory argument"
         )
 
+    def test_extension_owned_skill_names_rejects_symlinked_child_skill_dir(
+        self, project_dir, temp_dir
+    ):
+        """Provenance probing must reject a per-skill child directory that
+        is itself a symlink, even when its resolved target stays inside
+        the (real, non-symlinked) skills root.
+
+        Both ``_extension_owned_skill_names`` and
+        ``_unregister_extension_skills`` previously only validated the
+        *parent* ``skills_dir`` for symlink escape, then resolved
+        ``skills_dir / skill_name`` and checked containment relative to
+        the already-resolved parent. A child symlink whose target
+        resolves inside that same root passes that containment check, so
+        a corrupted or attacker-controlled registry entry naming a
+        symlink alias could cause a legitimate, unrelated skill directory
+        to be falsely attributed as extension-owned via the alias.
+        """
+        skills_dir = project_dir / ".claude" / "skills"
+        skills_dir.mkdir(parents=True)
+
+        # A real, legitimately marker-matching skill directory under its
+        # own name — this represents genuine extension-owned content.
+        real_skill_dir = skills_dir / "speckit-child-sym-real"
+        real_skill_dir.mkdir()
+        (real_skill_dir / "SKILL.md").write_text(
+            "---\n"
+            "name: speckit-child-sym-real\n"
+            "description: real skill\n"
+            "metadata:\n"
+            "  source: extension:child-sym-ext\n"
+            "---\n\n"
+            "real body\n",
+            encoding="utf-8",
+        )
+
+        if not _can_create_symlink(temp_dir):
+            pytest.skip("Current platform/user cannot create symlinks")
+
+        # A *different* registered name that is merely a symlink alias
+        # pointing at the real skill directory above — both still live
+        # inside the same, non-symlinked skills root.
+        alias_name = "speckit-child-sym-alias"
+        os.symlink(str(real_skill_dir), str(skills_dir / alias_name))
+
+        manager = ExtensionManager(project_dir)
+        owned = manager._extension_owned_skill_names(
+            [alias_name], "child-sym-ext"
+        )
+
+        assert owned == [], (
+            "a per-skill child directory that is itself a symlink must "
+            "never be followed for provenance attribution, even when its "
+            "resolved target remains inside the skills root"
+        )
+
+    def test_unregister_extension_skills_explicit_dir_rejects_symlinked_child(
+        self, project_dir, temp_dir
+    ):
+        """Fast (explicit ``skills_dir``) removal path must refuse to
+        delete through a per-skill child directory that is itself a
+        symlink, even when the resolved target stays inside the skills
+        root — deleting the resolved target would destroy a legitimate,
+        differently-named skill directory via the alias.
+        """
+        skills_dir = project_dir / ".claude" / "skills"
+        skills_dir.mkdir(parents=True)
+
+        precious_skill_dir = skills_dir / "speckit-child-sym-precious"
+        precious_skill_dir.mkdir()
+        precious_skill_md = precious_skill_dir / "SKILL.md"
+        precious_skill_md.write_text(
+            "---\n"
+            "name: speckit-child-sym-precious\n"
+            "description: precious skill\n"
+            "metadata:\n"
+            "  source: extension:child-sym-ext2\n"
+            "---\n\n"
+            "precious body\n",
+            encoding="utf-8",
+        )
+
+        if not _can_create_symlink(temp_dir):
+            pytest.skip("Current platform/user cannot create symlinks")
+
+        alias_name = "speckit-child-sym-alias2"
+        os.symlink(str(precious_skill_dir), str(skills_dir / alias_name))
+
+        manager = ExtensionManager(project_dir)
+        manager._unregister_extension_skills(
+            [alias_name], "child-sym-ext2", skills_dir=skills_dir,
+        )
+
+        assert precious_skill_dir.exists(), (
+            "the real skill directory reached only through a symlink "
+            "alias must survive removal of the alias name (#2948)"
+        )
+        assert precious_skill_md.exists(), (
+            "the real skill directory's SKILL.md must not be deleted via "
+            "a differently-named symlink alias"
+        )
+
+    def test_unregister_extension_skills_fallback_rejects_symlinked_child(
+        self, project_dir, temp_dir
+    ):
+        """Fallback (unscoped, ``skills_dir=None``) removal scan must also
+        refuse to delete through a per-skill child symlink, mirroring the
+        explicit-dir fast path.
+        """
+        skills_dir = project_dir / ".claude" / "skills"
+        skills_dir.mkdir(parents=True)
+
+        precious_skill_dir = skills_dir / "speckit-child-sym-precious3"
+        precious_skill_dir.mkdir()
+        precious_skill_md = precious_skill_dir / "SKILL.md"
+        precious_skill_md.write_text(
+            "---\n"
+            "name: speckit-child-sym-precious3\n"
+            "description: precious skill\n"
+            "metadata:\n"
+            "  source: extension:child-sym-ext3\n"
+            "---\n\n"
+            "precious body\n",
+            encoding="utf-8",
+        )
+
+        if not _can_create_symlink(temp_dir):
+            pytest.skip("Current platform/user cannot create symlinks")
+
+        alias_name = "speckit-child-sym-alias3"
+        os.symlink(str(precious_skill_dir), str(skills_dir / alias_name))
+
+        manager = ExtensionManager(project_dir)
+        manager._unregister_extension_skills([alias_name], "child-sym-ext3")
+
+        assert precious_skill_dir.exists(), (
+            "the real skill directory reached only through a symlink "
+            "alias must survive the unscoped fallback removal scan (#2948)"
+        )
+        assert precious_skill_md.exists(), (
+            "the real skill directory's SKILL.md must not be deleted via "
+            "a differently-named symlink alias during fallback removal"
+        )
+
     def test_existing_agent_command_path_file_is_not_detected(
         self, project_dir, temp_dir
     ):

tests/test_presets.py

@@ -5295,6 +5295,157 @@ class TestPresetSkills:
             "inactive agent's directory (#2948)"
         )
 
+    def test_remove_reconciles_command_for_every_historical_agent(
+        self, project_dir, temp_dir
+    ):
+        """Removing a preset must reconcile every historical agent its
+        ``registered_commands`` actually targeted, not only the currently
+        active one.
+
+        Preset B (lower precedence, survives) is installed while gemini is
+        active, then preset A (higher precedence) overrides the same
+        command while gemini is still active. Switching the active
+        integration to opencode and rescaffolding re-registers both
+        presets under opencode too, so preset A's ``registered_commands``
+        now spans two agents: gemini (now inactive) and opencode (active).
+        Removing A deletes its command file from *both* directories via
+        ``_unregister_commands``, but active-only reconciliation used to
+        recreate the surviving preset B's content only for the active
+        agent (opencode), leaving gemini's directory with a stale/missing
+        file (#2948).
+        """
+        self._write_init_options(project_dir, ai="gemini", ai_skills=False)
+        gemini_dir = project_dir / ".gemini" / "commands"
+        gemini_dir.mkdir(parents=True)
+
+        preset_b_dir = self._create_command_preset(
+            temp_dir, "hist-preset-b", "speckit.specify",
+            "Preset B", "preset B body",
+        )
+        preset_a_dir = self._create_command_preset(
+            temp_dir, "hist-preset-a", "speckit.specify",
+            "Preset A", "preset A body",
+        )
+
+        manager = PresetManager(project_dir)
+        manager.install_from_directory(preset_b_dir, "0.1.5", priority=10)
+        manager.install_from_directory(preset_a_dir, "0.1.5", priority=1)
+
+        gemini_cmd_files = list(gemini_dir.glob("*specify*"))
+        assert gemini_cmd_files, "sanity: gemini should have the command file"
+        assert "preset A body" in gemini_cmd_files[0].read_text(), (
+            "sanity: preset A (higher precedence) should win initially"
+        )
+
+        # Switch the active integration to opencode and rescaffold, mirroring
+        # `integration use opencode`. This merges opencode into both
+        # presets' registered_commands alongside the pre-existing gemini
+        # entry recorded while gemini was active.
+        self._write_init_options(project_dir, ai="opencode", ai_skills=False)
+        opencode_dir = project_dir / ".opencode" / "commands"
+        opencode_dir.mkdir(parents=True, exist_ok=True)
+        manager.register_enabled_presets_for_agent("opencode")
+
+        metadata_a = manager.registry.get("hist-preset-a")
+        assert set(metadata_a.get("registered_commands", {})) == {"gemini", "opencode"}, (
+            "sanity: preset A's registered_commands must span both the "
+            "historical (gemini) and currently active (opencode) agents"
+        )
+
+        assert manager.remove("hist-preset-a") is True
+
+        gemini_cmd_files = list(gemini_dir.glob("*specify*"))
+        opencode_cmd_files = list(opencode_dir.glob("*specify*"))
+        assert gemini_cmd_files, "gemini's command file must still exist after removal"
+        assert opencode_cmd_files, "opencode's command file must still exist after removal"
+        assert "preset B body" in gemini_cmd_files[0].read_text(), (
+            "removing the higher-precedence preset must restore the "
+            "surviving preset's content in the historical (inactive) "
+            "agent's directory too, not only the active agent's (#2948)"
+        )
+        assert "preset B body" in opencode_cmd_files[0].read_text(), (
+            "the surviving preset's content must also be restored for the "
+            "currently active agent"
+        )
+
+    def test_remove_reconciles_skill_for_every_historical_agent(
+        self, project_dir, temp_dir
+    ):
+        """Removing a preset must reconcile every historical skills
+        directory its ``registered_skills`` actually targeted, not only
+        the currently active one.
+
+        Preset B (survives) is installed while claude is active, then
+        preset A (higher precedence) overrides the same command while
+        claude is still active. Switching to codex and rescaffolding
+        records codex too, so preset A's ``registered_skills`` spans both
+        claude (now inactive) and codex (active) directories. Removing A
+        restores both directories to core/extension via
+        ``_unregister_skills``, but ``_reconcile_skills`` used to only
+        resolve/apply the surviving winner for the currently active
+        skills directory, leaving claude's directory reverted to
+        core/extension content instead of preset B's override (#2948).
+        """
+        self._write_init_options(project_dir, ai="claude", ai_skills=True)
+        claude_skills_dir = project_dir / ".claude" / "skills"
+
+        # A core template fallback is required so unregistering the
+        # top-priority preset's SKILL.md restores core content rather than
+        # deleting the skill directory outright when no preset remains to
+        # apply on top of it (mirrors the pre-existing skills-reconciliation
+        # fixtures elsewhere in this file).
+        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",
+        )
+
+        preset_b_dir = self._create_command_preset(
+            temp_dir, "hist-skill-preset-b", "speckit.specify",
+            "Preset B", "preset B body",
+        )
+        preset_a_dir = self._create_command_preset(
+            temp_dir, "hist-skill-preset-a", "speckit.specify",
+            "Preset A", "preset A body",
+        )
+
+        manager = PresetManager(project_dir)
+        manager.install_from_directory(preset_b_dir, "0.1.5", priority=10)
+        manager.install_from_directory(preset_a_dir, "0.1.5", priority=1)
+
+        claude_skill_file = claude_skills_dir / "speckit-specify" / "SKILL.md"
+        assert "preset:hist-skill-preset-a" in claude_skill_file.read_text(), (
+            "sanity: preset A (higher precedence) should win initially"
+        )
+
+        # Switch the active integration to codex (a distinct skills
+        # directory) and rescaffold, mirroring `integration use codex`.
+        self._write_init_options(project_dir, ai="codex", ai_skills=True)
+        codex_skills_dir = project_dir / ".agents" / "skills"
+        manager.register_enabled_presets_for_agent("codex")
+
+        metadata_a = manager.registry.get("hist-skill-preset-a")
+        assert set(metadata_a.get("registered_skills", {})) == {"claude", "codex"}, (
+            "sanity: preset A's registered_skills must span both the "
+            "historical (claude) and currently active (codex) agents"
+        )
+
+        assert manager.remove("hist-skill-preset-a") is True
+
+        codex_skill_file = codex_skills_dir / "speckit-specify" / "SKILL.md"
+        assert claude_skill_file.exists(), "claude's skill file must still exist after removal"
+        assert codex_skill_file.exists(), "codex's skill file must still exist after removal"
+        assert "preset:hist-skill-preset-b" in claude_skill_file.read_text(), (
+            "removing the higher-precedence preset must restore the "
+            "surviving preset's override in the historical (inactive) "
+            "agent's directory too, not only the active agent's (#2948)"
+        )
+        assert "preset:hist-skill-preset-b" in codex_skill_file.read_text(), (
+            "the surviving preset's override must also be restored for "
+            "the currently active agent"
+        )
+
     def test_symlinked_skill_subdir_rejected_on_restore(self, project_dir, temp_dir):
         """Restore must validate each per-skill subdirectory, not just its parent.