Fix upgrade-overwrites-copilot-skills: pass force=True to extension skill re-registration after upgrade

Apply the remediation from the bug assessment on issue #3849.

_register_extension_skills() had a skip guard that refused to overwrite
existing SKILL.md files (protecting user customizations). In the upgrade
path, setup() regenerates all core-template SKILL.md files first, then
calls register_enabled_extensions_for_agent(). The guard then sees those
freshly-written core files as 'existing' and skips every extension, leaving
only core template content on disk.

Fix: add force: bool = False to _register_extension_skills() and thread it
through register_enabled_extensions_for_agent() and
_register_extensions_for_agent(). In integration_upgrade(), pass force=True
so extension content layers on top of the just-regenerated core files.

The force flag is off-by-default so plain extension add still protects
user-modified skill files.

Refs #3849

Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: github-actions[bot]

src/specify_cli/extensions/__init__.py

@@ -1292,6 +1292,7 @@ def _register_extension_skills(
         manifest: ExtensionManifest,
         extension_dir: Path,
         link_outputs: bool = False,
+        force: bool = False,
     ) -> List[str]:
         """Generate SKILL.md files for extension commands as agent skills.
 
@@ -1305,6 +1306,11 @@ def _register_extension_skills(
             extension_dir: Installed extension directory.
             link_outputs: If True, create dev-mode symlinks for rendered
                 skill files when supported by the OS.
+            force: If True, overwrite existing SKILL.md files even when they
+                are not dev-mode symlinks.  Use in the upgrade path, where
+                ``setup()`` has just freshly regenerated core-template skill
+                files and the skip guard would otherwise prevent extension
+                content from being layered on top.
 
         Returns:
             List of skill names that were created (for registry storage).
@@ -1392,13 +1398,16 @@ def _replacement(match: re.Match[str]) -> str:
                 )
                 # Do not overwrite user-customized skills, but allow dev-mode
                 # symlinks that point back to this extension's generated cache
-                # to be refreshed on a subsequent dev install.
-                if not is_expected_dev_symlink:
+                # to be refreshed on a subsequent dev install.  In the upgrade
+                # path (force=True) the file was just written by setup(), so
+                # overwriting it with the composed extension content is correct.
+                if not is_expected_dev_symlink and not force:
                     continue
-            elif skill_dir_preexists:
+            elif skill_dir_preexists and not force:
                 # Never add files to a pre-existing user directory. Without a
                 # verifiable SKILL.md ownership marker, rollback/removal cannot
                 # distinguish our output from unrelated user artifacts.
+                # Skipped when force=True (upgrade path).
                 continue
 
             # Create skill directory; track whether we created it so we can clean
@@ -2637,7 +2646,7 @@ def unregister_agent_artifacts(
             if updates:
                 self.registry.update(ext_id, updates)
 
-    def register_enabled_extensions_for_agent(self, agent_name: str) -> None:
+    def register_enabled_extensions_for_agent(self, agent_name: str, *, force: bool = False) -> None:
         """Register installed, enabled extensions for ``agent_name``.
 
         Command-file registration is scoped to the explicit ``agent_name``
@@ -2755,7 +2764,7 @@ def register_enabled_extensions_for_agent(self, agent_name: str) -> None:
                 if agent_name == active_agent:
                     try:
                         registered_skills = self._register_extension_skills(
-                            manifest, ext_dir
+                            manifest, ext_dir, force=force
                         )
                     except Exception as skills_err:
                         # Skills are a companion artifact.  If command registration

src/specify_cli/integrations/_helpers.py

@@ -395,6 +395,7 @@ def _register_extensions_for_agent(
     agent_key: str,
     *,
     continuing: str,
+    force: bool = False,
 ) -> None:
     """Register all enabled extensions' commands/skills for ``agent_key``.
 
@@ -408,14 +409,19 @@ def _register_extensions_for_agent(
     before registering), so extension *skill* rendering — which is scoped to
     the active ``ai`` / ``ai_skills`` init-options — matches ``agent_key``.
 
+    When ``force=True``, existing skill files are overwritten even when they
+    are not dev-mode symlinks. Pass ``force=True`` in the upgrade path so that
+    extension content is layered on top of the core-template files that
+    ``setup()`` just regenerated (fixes the skip-guard bug for skills mode).
+
     Best-effort: never aborts the surrounding integration operation. Callers
     invoke it *after* the use/upgrade/switch transaction has committed so a
     failure here cannot trigger a rollback.
     """
     _best_effort_extension_op(
         project_root,
         agent_key,
-        lambda mgr, key: mgr.register_enabled_extensions_for_agent(key),
+        lambda mgr, key: mgr.register_enabled_extensions_for_agent(key, force=force),
         phase="register extension artifacts for",
         continuing=continuing,
     )

src/specify_cli/integrations/_migrate_commands.py

@@ -870,6 +870,7 @@ def integration_upgrade(
         _register_extensions_for_agent(
             project_root,
             key,
+            force=True,
             continuing="The integration was upgraded, but installed extensions may need re-registration.",
         )
         _register_presets_for_agent(

tests/test_extension_skills.py

@@ -2943,6 +2943,137 @@ def fail_recovered_claude_registration(self, agent_name, *args, **kwargs):
         assert "speckit-early-ext-world" in metadata["registered_skills"]
 
 
+
+# ===== Regression test: upgrade-overwrites-copilot-skills (#3849) =====
+
+class TestRegisterExtensionSkillsForceFlag:
+    """Regression tests for the ``force`` flag on ``_register_extension_skills``.
+
+    Issue #3849: ``integration upgrade --force`` called ``setup()`` which
+    regenerated all core-template SKILL.md files, then called
+    ``register_enabled_extensions_for_agent()``.  The skip-guard in
+    ``_register_extension_skills`` treated the freshly-written core files as
+    existing user content and skipped every extension skill, leaving only core
+    template content on disk.
+
+    The fix introduces ``force=True`` in the upgrade path so the guard does not
+    fire for core-template files that setup() just wrote.
+    """
+
+    def test_force_false_skips_existing_skill(self, project_dir, temp_dir):
+        """Without force=True the skip guard must still protect existing files."""
+        _create_init_options(project_dir, ai="claude", ai_skills=True)
+        skills_dir = _create_skills_dir(project_dir, ai="claude")
+        ext_dir = _create_extension_dir(temp_dir)
+
+        # Manually pre-create a SKILL.md as if setup() had already written it
+        skill_subdir = skills_dir / "speckit-test-ext-hello"
+        skill_subdir.mkdir(parents=True, exist_ok=True)
+        skill_file = skill_subdir / "SKILL.md"
+        skill_file.write_text("core-template content only", encoding="utf-8")
+
+        manager = ExtensionManager(project_dir)
+        manifest = ExtensionManifest(ext_dir / "extension.yml")
+
+        # Default (force=False): existing file must not be overwritten
+        written = manager._register_extension_skills(manifest, ext_dir, force=False)
+        assert "speckit-test-ext-hello" not in written
+        assert skill_file.read_text(encoding="utf-8") == "core-template content only"
+
+    def test_force_true_overwrites_existing_skill(self, project_dir, temp_dir):
+        """With force=True the function must overwrite the existing SKILL.md.
+
+        This is the core regression test for #3849: calling
+        ``_register_extension_skills(force=True)`` after ``setup()`` has
+        written a fresh core-template SKILL.md must replace it with the
+        composed extension content.
+        """
+        _create_init_options(project_dir, ai="claude", ai_skills=True)
+        skills_dir = _create_skills_dir(project_dir, ai="claude")
+        ext_dir = _create_extension_dir(temp_dir)
+
+        # Simulate what setup() writes: a bare core-template SKILL.md
+        skill_subdir = skills_dir / "speckit-test-ext-hello"
+        skill_subdir.mkdir(parents=True, exist_ok=True)
+        skill_file = skill_subdir / "SKILL.md"
+        skill_file.write_text("core-template content only", encoding="utf-8")
+
+        manager = ExtensionManager(project_dir)
+        manifest = ExtensionManifest(ext_dir / "extension.yml")
+
+        # Upgrade path (force=True): extension content should replace the core file
+        written = manager._register_extension_skills(manifest, ext_dir, force=True)
+        assert "speckit-test-ext-hello" in written, (
+            "force=True should overwrite the core-template file and return the skill name"
+        )
+        content = skill_file.read_text(encoding="utf-8")
+        assert "Run this to say hello." in content, (
+            "Extension command body must appear in the overwritten SKILL.md"
+        )
+        assert "core-template content only" not in content, (
+            "Core-template placeholder must have been replaced by extension content"
+        )
+
+    def test_register_enabled_extensions_for_agent_force_flag_threads_through(
+        self, project_dir, temp_dir
+    ):
+        """force=True on register_enabled_extensions_for_agent must reach _register_extension_skills.
+
+        End-to-end check: after an upgrade writes a fresh core-template SKILL.md,
+        ``register_enabled_extensions_for_agent(force=True)`` must produce a
+        SKILL.md that contains the extension content.
+        """
+        _create_init_options(project_dir, ai="claude", ai_skills=True)
+        skills_dir = _create_skills_dir(project_dir, ai="claude")
+        ext_dir = _create_extension_dir(temp_dir)
+
+        manager = ExtensionManager(project_dir)
+        # Install extension so it is in the registry
+        ext_manifest = manager.install_from_directory(
+            ext_dir, "0.1.0", register_commands=False
+        )
+
+        # Simulate a freshly-regenerated core-template SKILL.md (as setup() would write)
+        skill_file = skills_dir / "speckit-test-ext-hello" / "SKILL.md"
+        skill_file.write_text("core-template content only", encoding="utf-8")
+
+        # Re-register with force=True (upgrade path)
+        manager.register_enabled_extensions_for_agent("claude", force=True)
+
+        content = skill_file.read_text(encoding="utf-8")
+        assert "Run this to say hello." in content, (
+            "After register_enabled_extensions_for_agent(force=True), the SKILL.md "
+            "must contain the extension body, not just the core-template stub."
+        )
+
+    def test_force_true_with_preexisting_dir_but_no_skill_file(
+        self, project_dir, temp_dir
+    ):
+        """force=True must write into a pre-existing directory with no SKILL.md.
+
+        The second skip guard (``elif skill_dir_preexists``) should also be
+        bypassed by force=True so an upgrade can create a missing SKILL.md
+        even when the skill sub-directory already exists.
+        """
+        _create_init_options(project_dir, ai="claude", ai_skills=True)
+        skills_dir = _create_skills_dir(project_dir, ai="claude")
+        ext_dir = _create_extension_dir(temp_dir)
+
+        # Create the skill directory without the SKILL.md file
+        skill_subdir = skills_dir / "speckit-test-ext-hello"
+        skill_subdir.mkdir(parents=True, exist_ok=True)
+        skill_file = skill_subdir / "SKILL.md"
+        assert not skill_file.exists()
+
+        manager = ExtensionManager(project_dir)
+        manifest = ExtensionManifest(ext_dir / "extension.yml")
+
+        written = manager._register_extension_skills(manifest, ext_dir, force=True)
+        assert "speckit-test-ext-hello" in written
+        assert skill_file.exists()
+        assert "Run this to say hello." in skill_file.read_text(encoding="utf-8")
+
+
 # ===== Extension Skill Unregistration Tests =====
 
 class TestExtensionSkillUnregistration: