fix: address fifth round of review feedback (symlink presence, rescaffold reconciliation, shared skills dir)

- _init_options.py: resolve_active_agent_for_registration() now treats a
  dangling init-options.json symlink as present (path.is_symlink() check
  alongside path.exists()), since Path.exists() follows symlinks and
  returns False for a broken one. Previously a broken symlink fell back
  to the legacy "no file" path and registered every detected agent
  instead of failing closed.
- presets/__init__.py (register_enabled_presets_for_agent): the
  integration use/switch rescaffold path now collects affected command
  names across all presets processed and runs
  _reconcile_composed_commands/_reconcile_skills once after the loop,
  matching install/remove. Previously rescaffolding wrote each preset's
  raw content directly with no follow-up reconciliation, so a
  project-level override (the highest-priority layer) could be clobbered
  by a lower-precedence preset after switching agents.
- presets/__init__.py (_unregister_skills): multiple integrations can
  share one physical skills directory (agy/codex/zed all resolve to
  .agents/skills). Provenance restoration now groups recorded agent
  entries by resolved directory and restores each physical directory
  exactly once, preferring the currently active agent's renderer when it
  owns that directory (otherwise any recorded owner, chosen
  deterministically). Previously each recorded agent key triggered its
  own restore pass against the same directory, with whichever agent was
  iterated last silently winning regardless of which agent was active.

Adds regression tests for each: a dangling init-options.json symlink
failing closed for both preset resolution and extension add; integration
use rescaffold preserving a project override over a lower-priority
preset; and a codex/agy shared-directory removal restoring the directory
exactly once in the active agent's format.

Targeted (tests/integrations/test_integration_subcommand.py,
tests/test_presets.py, tests/test_extensions.py,
tests/test_extension_skills.py,
tests/integrations/test_integration_opencode.py,
tests/integrations/test_integration_claude.py): 930 passed.
Full suite: 3930 passed, 109 skipped.
ruff check: clean on files touched by this change.

Refs #2948

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

src/specify_cli/_init_options.py

@@ -69,7 +69,12 @@ def resolve_active_agent_for_registration(
     - Returns the active agent key (a non-empty string) otherwise.
     """
     path = project_path / INIT_OPTIONS_FILE
-    if not path.exists():
+    # A dangling symlink's target doesn't exist, so Path.exists() (which
+    # follows symlinks) returns False even though the path itself is
+    # present as a broken/corrupted entry. Treat any symlink as "present"
+    # so a dangling one fails closed via the invalid-file branch below
+    # instead of being mistaken for "no file at all" (legacy fallback).
+    if not path.is_symlink() and not path.exists():
         return MISSING_INIT_OPTIONS_FILE
 
     active_agent = load_init_options(project_path).get("ai")

src/specify_cli/presets/__init__.py

@@ -752,6 +752,7 @@ class PresetManager:
             return
 
         resolver = PresetResolver(self.project_root)
+        affected_cmd_names: set = set()
         for pack_id, metadata in reversed(self.registry.list_by_priority()):
             pack_dir = self.presets_dir / pack_id
             manifest = resolver._get_manifest(pack_dir)
@@ -785,6 +786,10 @@ class PresetManager:
 
                 if updates:
                     self.registry.update(pack_id, updates)
+
+                for tmpl in manifest.templates:
+                    if tmpl.get("type") == "command":
+                        affected_cmd_names.add(tmpl["name"])
             except Exception as pack_err:
                 from .. import _print_cli_warning
 
@@ -797,6 +802,29 @@ class PresetManager:
                 )
                 continue
 
+        # _register_commands/_register_skills write each preset's own
+        # content directly and rely on reconciliation to resolve the final
+        # winner across the *entire* priority stack (including project
+        # overrides, which always outrank presets). install/remove already
+        # call this; without it here, rescaffolding on `integration use` /
+        # `switch` could leave the highest-precedence preset's raw content
+        # in place even when a project override or a higher-precedence
+        # preset should win (#2948).
+        if affected_cmd_names:
+            try:
+                self._reconcile_composed_commands(list(affected_cmd_names))
+                self._reconcile_skills(list(affected_cmd_names))
+            except Exception as exc:
+                import warnings
+
+                warnings.warn(
+                    f"Post-rescaffold reconciliation failed for '{agent_name}': "
+                    f"{exc}. Agent command files may be stale; re-run "
+                    f"'specify integration use {agent_name}' or reinstall "
+                    f"affected presets to refresh.",
+                    stacklevel=2,
+                )
+
     def _unregister_commands(self, registered_commands: Dict[str, List[str]]) -> None:
         """Remove previously registered command files from agent directories.
 
@@ -1619,13 +1647,43 @@ class PresetManager:
             return
 
         if isinstance(registered_skills, dict):
+            from .. import load_init_options
+
+            init_opts = load_init_options(self.project_root)
+            active_agent = init_opts.get("ai") if isinstance(init_opts, dict) else None
+            if not isinstance(active_agent, str) or not active_agent:
+                active_agent = None
+
+            # Multiple integration keys can share the same physical
+            # directory (e.g. agy/codex/zed all resolve to
+            # ``.agents/skills``). Restoring that directory once per
+            # recorded agent would have each pass's agent-specific
+            # rendering (frontmatter, post-processing) overwrite the
+            # previous one, with whichever agent is iterated *last* silently
+            # winning regardless of which agent is actually active. Group
+            # provenance by resolved directory so each physical directory is
+            # restored exactly once, using the active agent's renderer when
+            # it shares that directory (otherwise any recorded owner,
+            # chosen deterministically).
+            groups: Dict[Path, Dict[str, Any]] = {}
             for agent_name, skill_names in registered_skills.items():
                 if not skill_names:
                     continue
                 skills_dir = self._safe_skills_dir_for_agent(agent_name)
                 if skills_dir is None:
                     continue
-                self._unregister_skills_in_dir(skill_names, skills_dir, agent_name)
+                group = groups.setdefault(skills_dir, {"agents": [], "names": []})
+                group["agents"].append(agent_name)
+                for name in skill_names:
+                    if name not in group["names"]:
+                        group["names"].append(name)
+
+            for skills_dir, group in groups.items():
+                agents = group["agents"]
+                renderer_agent = (
+                    active_agent if active_agent in agents else sorted(agents)[0]
+                )
+                self._unregister_skills_in_dir(group["names"], skills_dir, renderer_agent)
             return
 
         # Legacy flat-list format: no record of which agent directory these

tests/integrations/test_integration_subcommand.py

@@ -1525,6 +1525,41 @@ class TestIntegrationInstall:
             "treated like a legacy project missing the file entirely (#2948)"
         )
 
+    def test_extension_add_dangling_init_options_symlink_fails_closed(self, tmp_path):
+        """A dangling init-options.json symlink must fail closed too, not be
+        treated the same as "no file at all".
+
+        ``Path.exists()`` follows symlinks and returns False for a broken
+        symlink whose target doesn't exist, so a naive presence check based
+        on ``Path.exists()`` alone mistakes a dangling symlink for "no file"
+        and falls back to registering every detected agent.
+        """
+        project = _init_project(tmp_path, "claude")
+
+        result = _run_in_project(project, [
+            "integration", "install", "codex",
+            "--script", "sh",
+        ])
+        assert result.exit_code == 0, result.output
+
+        init_options_path = project / ".specify" / "init-options.json"
+        init_options_path.unlink()
+        init_options_path.symlink_to(project / ".specify" / "does-not-exist.json")
+        assert not init_options_path.exists()  # sanity: dangling
+        assert init_options_path.is_symlink()
+
+        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 registered == {}, (
+            "a dangling init-options.json symlink must fail closed, not be "
+            "treated like a legacy project missing the file entirely (#2948)"
+        )
+
 
 # ── uninstall ────────────────────────────────────────────────────────
 

tests/test_presets.py

@@ -3113,6 +3113,26 @@ class TestResolveActiveAgentForRegistration:
 
         assert resolve_active_agent_for_registration(project_dir) is None
 
+    def test_dangling_symlink_fails_closed(self, project_dir):
+        """A dangling init-options.json symlink must fail closed, not fall
+        back to "no file" (#2948).
+
+        ``Path.exists()`` follows symlinks and returns False for a broken
+        symlink whose target is missing, so a naive presence check treats a
+        dangling symlink the same as "no file at all" and falls back to
+        legacy all-agent registration. The path is present (just broken),
+        so it must be treated as a corrupted file and fail closed instead.
+        """
+        from specify_cli._init_options import resolve_active_agent_for_registration
+
+        opts_file = project_dir / ".specify" / "init-options.json"
+        opts_file.parent.mkdir(parents=True, exist_ok=True)
+        opts_file.symlink_to(project_dir / ".specify" / "does-not-exist.json")
+
+        assert not opts_file.exists()  # sanity: this is what makes it dangling
+        assert opts_file.is_symlink()
+        assert resolve_active_agent_for_registration(project_dir) is None
+
 
 class TestPresetSkills:
     """Tests for preset skill registration and unregistration.
@@ -4152,6 +4172,53 @@ class TestPresetSkills:
             "but inactive non-skill agent (#2948)"
         )
 
+    def test_use_rescaffold_reconciles_project_override(self, project_dir, temp_dir):
+        """``integration use``/``switch`` rescaffolding must reconcile the
+        full priority stack, not just write each preset's own content.
+
+        Project overrides are the highest-priority layer, above every
+        preset. ``register_enabled_presets_for_agent`` (invoked by
+        ``integration use``/``switch``) calls ``_register_commands`` for
+        each enabled preset directly, the same as ``install_from_directory``
+        — but unlike install/remove, it never followed up with
+        ``_reconcile_composed_commands``. Before the fix, rescaffolding a
+        newly activated agent could leave the preset's raw content in
+        place instead of resolving the real winner (the project override)
+        from the full stack (#2948).
+        """
+        self._write_init_options(project_dir, ai="claude", ai_skills=True)
+
+        overrides_dir = project_dir / ".specify" / "templates" / "overrides"
+        overrides_dir.mkdir(parents=True, exist_ok=True)
+        (overrides_dir / "speckit.specify.md").write_text(
+            "---\ndescription: Override specify\n---\n\nOverride body\n",
+            encoding="utf-8",
+        )
+
+        gemini_dir = project_dir / ".gemini" / "commands"
+        gemini_dir.mkdir(parents=True)
+
+        preset_dir = self._create_command_preset(
+            temp_dir, "use-reconcile-preset", "speckit.specify",
+            "Preset specify", "Preset body",
+        )
+        manager = PresetManager(project_dir)
+        manager.install_from_directory(preset_dir, "0.1.5")
+
+        # Simulate `integration use gemini`: switch the active agent and
+        # rescaffold enabled presets for it, mirroring what the CLI does.
+        self._write_init_options(project_dir, ai="gemini", ai_skills=False)
+        manager.register_enabled_presets_for_agent("gemini")
+
+        cmd_file = gemini_dir / "speckit.specify.toml"
+        assert cmd_file.exists(), "sanity: gemini should get a command file at all"
+        content = cmd_file.read_text()
+        assert "Override body" in content, (
+            "the project override must still win after rescaffold "
+            "reconciliation, not the preset's raw content (#2948)"
+        )
+        assert "Preset body" not in content
+
     def test_copilot_skills_mode_skips_command_registration(self, project_dir, temp_dir):
         """``integration use copilot`` with skills mode enabled must only
         write the SKILL.md mirror, not also copilot's static command file.
@@ -4362,6 +4429,87 @@ class TestPresetSkills:
             "removing preset B must not disturb preset A's Claude override (#2948)"
         )
 
+    def test_shared_skills_dir_restored_once_using_active_agent(
+        self, project_dir, temp_dir
+    ):
+        """Removal must restore a physical skills directory shared by
+        multiple agents exactly once, using the active agent's renderer.
+
+        Codex and Antigravity (agy) both resolve their skills directory to
+        ``.agents/skills``. Registering a preset under codex, switching to
+        agy, then switching back to codex records provenance for *both*
+        agent keys even though they share one physical directory. Before
+        the fix, ``_unregister_skills`` restored once per recorded agent
+        key rather than once per unique directory, so the directory was
+        written twice on removal with whichever agent was iterated *last*
+        silently winning — regardless of which agent is actually active
+        (#2948).
+        """
+        self._write_init_options(project_dir, ai="codex", ai_skills=True)
+        shared_skills_dir = project_dir / ".agents" / "skills"
+        self._create_skill(shared_skills_dir, "speckit-specify")
+
+        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_dir = self._create_command_preset(
+            temp_dir, "shared-dir-preset", "speckit.specify",
+            "Shared dir test", "preset body",
+        )
+        manager = PresetManager(project_dir)
+        manager.install_from_directory(preset_dir, "0.1.5")
+
+        # Switch to agy (shares .agents/skills with codex) and back to
+        # codex, mirroring `integration use agy` then `integration use
+        # codex`. Both agent keys end up recorded in registered_skills even
+        # though they refer to the same physical directory.
+        self._write_init_options(project_dir, ai="agy", ai_skills=True)
+        manager.register_enabled_presets_for_agent("agy")
+        self._write_init_options(project_dir, ai="codex", ai_skills=True)
+        manager.register_enabled_presets_for_agent("codex")
+
+        metadata = manager.registry.get("shared-dir-preset")
+        registered_skills = metadata.get("registered_skills", {})
+        assert set(registered_skills) == {"codex", "agy"}, (
+            "both agent keys must be recorded even though they share one "
+            "physical directory (#2948)"
+        )
+
+        from unittest.mock import patch
+
+        # Exercise `_unregister_skills` directly (the method this fix
+        # changed) rather than the full `remove()` flow, which separately
+        # triggers post-removal reconciliation that may also touch the
+        # active agent's directory — an unrelated call this test isn't
+        # targeting.
+        with patch.object(
+            manager,
+            "_unregister_skills_in_dir",
+            wraps=manager._unregister_skills_in_dir,
+        ) as spy:
+            manager._unregister_skills(registered_skills, preset_dir)
+
+        assert spy.call_count == 1, (
+            "a physical directory shared by multiple recorded agents must "
+            "be restored exactly once, not once per agent key (#2948)"
+        )
+        (_names, called_dir, called_agent), _kwargs = spy.call_args
+        assert called_dir == shared_skills_dir
+        assert called_agent == "codex", (
+            "the currently active agent must be used as the renderer when "
+            "it shares the restored directory, not whichever agent was "
+            "recorded last (#2948)"
+        )
+
+        skill_file = shared_skills_dir / "speckit-specify" / "SKILL.md"
+        content = skill_file.read_text()
+        assert "preset:shared-dir-preset" not in content
+        assert "Core specify body" in content
+
     def test_copilot_skills_registration_restored_after_process_restart(
         self, project_dir, temp_dir
     ):