fix: migrate legacy flat-list registered_skills on rescaffold even when unchanged

Fix a valid finding from GitHub Copilot's review of HEAD b9d9053 (#2948):
register_enabled_presets_for_agent() normalizes a legacy flat-list
registered_skills value (predating per-agent provenance) to the
{agent_name: [...]} dict shape in memory via _normalize_registered_skills,
but the persistence check only compared the two *normalized* forms. When
the freshly rescaffolded skill names are identical to what the legacy
list already held — the common case, since nothing about the preset or
skill actually changed — that comparison is a no-op and registry.update()
is skipped, leaving the *raw* on-disk value as the un-migrated flat list.

A later switch to a different skill-mode agent and removal then follows
_unregister_skills's legacy best-effort path (restore only the currently
active agent's directory) instead of the per-agent provenance path,
permanently orphaning the first agent's override.

Fix: track the raw (pre-normalization) existing value and force
persistence whenever it's a non-empty list, independent of whether the
normalized content changed. Traced registered_commands for the same
class of bug: its registry value has always been Dict[str, List[str]]
(no legacy flat-list format ever existed for it — the existing
`if not isinstance(existing_commands, dict): existing_commands = {}`
guard is not a lossy migration path), so this fix stays scoped to
registered_skills only.

Added red-first regression test
test_rescaffold_migrates_legacy_flat_list_registered_skills: installs a
preset, overwrites its registry entry with a raw legacy flat list,
rescaffolds the *same* active agent with unchanged skill names, and
asserts the raw registry is migrated to per-agent dict form. Extends the
scenario with a switch to a second skill-mode agent and preset removal
to prove both agents' directories restore cleanly instead of orphaning
the first. Failed against the prior code (raw value stayed a list) and
passes after the fix.

Targeted (tests/test_presets.py, tests/test_extensions.py: 692 passed)
and full (3938 passed, 109 skipped) suites and ruff check on changed
files pass clean.

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

src/specify_cli/presets/__init__.py

@@ -805,8 +805,9 @@ class PresetManager:
                     self.registry.update(pack_id, {"registered_commands": merged_commands})
 
                 registered_skills = self._register_skills(manifest, pack_dir)
+                raw_existing_skills = metadata.get("registered_skills")
                 existing_skills = self._normalize_registered_skills(
-                    metadata.get("registered_skills"), fallback_agent=agent_name
+                    raw_existing_skills, fallback_agent=agent_name
                 )
                 merged_skills = copy.deepcopy(existing_skills)
                 if registered_skills.get(agent_name):
@@ -821,7 +822,19 @@ class PresetManager:
                     self._unregister_skills(
                         {agent_name: merged_skills.pop(agent_name)}, pack_dir
                     )
-                if merged_skills != existing_skills:
+                # A legacy flat-list registered_skills value (predating
+                # per-agent provenance) must migrate to the dict format on
+                # disk even when the rescaffolded names are unchanged from
+                # what the list already held — comparing only the
+                # *normalized* forms would otherwise treat that as a no-op
+                # and leave the raw un-migrated list in the registry, which
+                # later removal/switch handling treats as legacy
+                # best-effort (restoring only the currently active agent's
+                # directory) instead of per-agent provenance (#2948).
+                needs_migration = (
+                    isinstance(raw_existing_skills, list) and raw_existing_skills
+                )
+                if merged_skills != existing_skills or needs_migration:
                     self.registry.update(pack_id, {"registered_skills": merged_skills})
 
                 for tmpl in manifest.templates:

tests/test_presets.py

@@ -4512,6 +4512,104 @@ class TestPresetSkills:
             )
             assert "Core specify body" in content
 
+    def test_rescaffold_migrates_legacy_flat_list_registered_skills(
+        self, project_dir, temp_dir
+    ):
+        """Rescaffolding a preset with a legacy flat-list ``registered_skills``
+        entry must persist the migrated per-agent dict even when the
+        rescaffolded skill names are unchanged from before.
+
+        ``_normalize_registered_skills`` converts a legacy flat ``List[str]``
+        (predating per-agent provenance) into ``{agent_name: [...]}`` in
+        memory, but the persistence check compared only the *normalized*
+        ``merged_skills`` against the *normalized* ``existing_skills`` —
+        both derived from the same raw legacy list. When the freshly
+        registered names are identical to what the legacy list already
+        held (the common case: nothing about the preset or skill actually
+        changed), that comparison is a no-op and ``registry.update()`` is
+        skipped, leaving the *raw* on-disk value as the un-migrated flat
+        list. A later switch to a different skill-mode agent and removal
+        then follows the legacy best-effort restore path (only the
+        currently active agent's directory) instead of the per-agent
+        provenance path, orphaning the first agent's override (#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, "legacy-skills-preset", "speckit.specify",
+            "Legacy skills test", "preset body",
+        )
+        manager = PresetManager(project_dir)
+        manager.install_from_directory(preset_dir, "0.1.5")
+
+        # Simulate a registry entry written by a pre-#2948 spec-kit version:
+        # registered_skills stored as a flat list with no per-agent
+        # provenance, rather than the dict shape install_from_directory
+        # writes today.
+        manager.registry.update(
+            "legacy-skills-preset", {"registered_skills": ["speckit-specify"]},
+        )
+        metadata = manager.registry.get("legacy-skills-preset")
+        assert isinstance(metadata["registered_skills"], list), (
+            "sanity: the injected legacy format is a flat list"
+        )
+
+        # Rescaffold for the *same* active agent (claude) with no actual
+        # change to the registered skill names, mirroring `integration
+        # upgrade claude` re-running registration for the active
+        # integration.
+        manager.register_enabled_presets_for_agent("claude")
+
+        metadata = manager.registry.get("legacy-skills-preset")
+        registered_skills = metadata.get("registered_skills")
+        assert isinstance(registered_skills, dict), (
+            "rescaffold must migrate a legacy flat-list registered_skills "
+            "entry to the per-agent dict format even when the "
+            "rescaffolded names are unchanged, or the raw registry stays "
+            "un-migrated and later removal loses per-agent provenance "
+            "(#2948)"
+        )
+        assert registered_skills.get("claude") == ["speckit-specify"]
+
+        # Switch to a different skill-mode agent and rescaffold again —
+        # with the dict format now in place, both directories should be
+        # tracked and therefore restorable on removal.
+        self._write_init_options(project_dir, ai="codex", ai_skills=True)
+        manager.register_enabled_presets_for_agent("codex")
+
+        metadata = manager.registry.get("legacy-skills-preset")
+        assert set(metadata.get("registered_skills", {})) == {"claude", "codex"}, (
+            "the migrated dict must keep recording every agent directory "
+            "the preset actually wrote to, exactly like a preset that was "
+            "always in dict format (#2948)"
+        )
+
+        assert manager.remove("legacy-skills-preset") is True
+
+        claude_skill = claude_skills_dir / "speckit-specify" / "SKILL.md"
+        codex_skill = codex_skills_dir / "speckit-specify" / "SKILL.md"
+        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:legacy-skills-preset" not in content, (
+                f"{label}'s preset override must be restored on removal, "
+                "not orphaned because the registry stayed in legacy "
+                "flat-list format (#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.