fix: address PR #2324 review feedback

- Reset _skills_mode at start of setup() to prevent singleton state leak
- Tighten skills auto-detection to require speckit-*/SKILL.md (not any
  non-empty .github/skills/ directory)
- Add copilot_skill_mode to init next-steps so skills mode renders
  /speckit-plan instead of /speckit.plan
- Fix docstring quoting to match actual unquoted output
- Add 4 tests covering singleton reset, auto-detection false positive,
  speckit layout detection, and next-steps skill syntax
- Fix skipped test_invalid_metadata_error_returns_unknown by simulating
  InvalidMetadataError on Python versions that lack it

Author: mnriem

src/specify_cli/__init__.py

@@ -1514,15 +1514,16 @@ def init(
     # Determine skill display mode for the next-steps panel.
     # Skills integrations (codex, kimi, agy, trae, cursor-agent) should show skill invocation syntax.
     from .integrations.base import SkillsIntegration as _SkillsInt
-    _is_skills_integration = isinstance(resolved_integration, _SkillsInt)
+    _is_skills_integration = isinstance(resolved_integration, _SkillsInt) or getattr(resolved_integration, "_skills_mode", False)
 
     codex_skill_mode = selected_ai == "codex" and (ai_skills or _is_skills_integration)
     claude_skill_mode = selected_ai == "claude" and (ai_skills or _is_skills_integration)
     kimi_skill_mode = selected_ai == "kimi"
     agy_skill_mode = selected_ai == "agy" and _is_skills_integration
     trae_skill_mode = selected_ai == "trae"
     cursor_agent_skill_mode = selected_ai == "cursor-agent" and (ai_skills or _is_skills_integration)
-    native_skill_mode = codex_skill_mode or claude_skill_mode or kimi_skill_mode or agy_skill_mode or trae_skill_mode or cursor_agent_skill_mode
+    copilot_skill_mode = selected_ai == "copilot" and _is_skills_integration
+    native_skill_mode = codex_skill_mode or claude_skill_mode or kimi_skill_mode or agy_skill_mode or trae_skill_mode or cursor_agent_skill_mode or copilot_skill_mode
 
     if codex_skill_mode and not ai_skills:
         # Integration path installed skills; show the helpful notice
@@ -1543,7 +1544,7 @@ def _display_cmd(name: str) -> str:
             return f"/speckit-{name}"
         if kimi_skill_mode:
             return f"/skill:speckit-{name}"
-        if cursor_agent_skill_mode:
+        if cursor_agent_skill_mode or copilot_skill_mode:
             return f"/speckit-{name}"
         return f"/speckit.{name}"
 

src/specify_cli/integrations/copilot/__init__.py

@@ -182,8 +182,11 @@ def dispatch_command(
         skills_mode = self._skills_mode
         if not skills_mode and project_root:
             skills_dir = project_root / ".github" / "skills"
-            if skills_dir.is_dir() and any(skills_dir.iterdir()):
-                skills_mode = True
+            if skills_dir.is_dir():
+                skills_mode = any(
+                    d.is_dir() and (d / "SKILL.md").is_file()
+                    for d in skills_dir.glob("speckit-*")
+                )
 
         if skills_mode:
             prompt = self.build_command_invocation(command_name, args)
@@ -242,7 +245,7 @@ def command_filename(self, template_name: str) -> str:
     def post_process_skill_content(self, content: str) -> str:
         """Inject Copilot-specific ``mode:`` field into SKILL.md frontmatter.
 
-        Inserts ``mode: "speckit.<stem>"`` before the closing ``---`` so
+        Inserts ``mode: speckit.<stem>`` before the closing ``---`` so
         Copilot can associate the skill with its agent mode.
         """
         lines = content.splitlines(keepends=True)
@@ -306,8 +309,8 @@ def setup(
         Otherwise uses the default ``.agent.md`` + ``.prompt.md`` layout.
         """
         parsed_options = parsed_options or {}
-        if parsed_options.get("skills"):
-            self._skills_mode = True
+        self._skills_mode = bool(parsed_options.get("skills"))
+        if self._skills_mode:
             return self._setup_skills(project_root, manifest, parsed_options, **opts)
         return self._setup_default(project_root, manifest, parsed_options, **opts)
 

tests/integrations/test_integration_copilot.py

@@ -600,4 +600,90 @@ def test_complete_file_inventory_skills_sh(self, tmp_path):
         assert actual == expected, (
             f"Missing: {sorted(set(expected) - set(actual))}\n"
             f"Extra: {sorted(set(actual) - set(expected))}"
+        )
+
+    # -- Singleton leak: _skills_mode must reset --------------------------
+
+    def test_skills_mode_resets_on_default_setup(self, tmp_path):
+        """setup() with skills=True then without must reset _skills_mode."""
+        copilot = self._make_copilot()
+
+        # First call: skills mode
+        (tmp_path / "proj1").mkdir()
+        m1 = IntegrationManifest("copilot", tmp_path / "proj1")
+        copilot.setup(tmp_path / "proj1", m1, parsed_options={"skills": True})
+        assert copilot._skills_mode is True
+
+        # Second call: default mode (no skills option)
+        (tmp_path / "proj2").mkdir()
+        m2 = IntegrationManifest("copilot", tmp_path / "proj2")
+        copilot.setup(tmp_path / "proj2", m2)
+        assert copilot._skills_mode is False
+
+        # build_command_invocation must use default (dotted) mode
+        assert copilot.build_command_invocation("plan", "args") == "args"
+
+    # -- Auto-detection must ignore unrelated .github/skills/ -------------
+
+    def test_dispatch_ignores_unrelated_skills_directory(self, tmp_path):
+        """dispatch_command() must not treat unrelated .github/skills/ as skills mode."""
+        copilot = self._make_copilot()
+        # Create a .github/skills/ with non-speckit content (e.g. GitHub Skills training)
+        unrelated = tmp_path / ".github" / "skills" / "introduction-to-github"
+        unrelated.mkdir(parents=True)
+        (unrelated / "README.md").write_text("# GitHub Skills training\n")
+
+        # Should NOT detect skills mode — cli_args should contain --agent
+        import unittest.mock as mock
+        with mock.patch("subprocess.run") as mock_run:
+            mock_run.return_value = mock.Mock(returncode=0, stdout="", stderr="")
+            copilot.dispatch_command("plan", "my args", project_root=tmp_path, stream=False)
+            call_args = mock_run.call_args[0][0]
+            assert "--agent" in call_args, (
+                f"Expected --agent in cli_args but got: {call_args}"
+            )
+            assert "speckit.plan" in call_args
+
+    def test_dispatch_detects_speckit_skills_layout(self, tmp_path):
+        """dispatch_command() detects speckit-*/SKILL.md as skills mode."""
+        copilot = self._make_copilot()
+        skill_dir = tmp_path / ".github" / "skills" / "speckit-plan"
+        skill_dir.mkdir(parents=True)
+        (skill_dir / "SKILL.md").write_text("---\nname: speckit-plan\n---\n")
+
+        import unittest.mock as mock
+        with mock.patch("subprocess.run") as mock_run:
+            mock_run.return_value = mock.Mock(returncode=0, stdout="", stderr="")
+            copilot.dispatch_command("plan", "my args", project_root=tmp_path, stream=False)
+            call_args = mock_run.call_args[0][0]
+            assert "--agent" not in call_args, (
+                f"Skills mode should not use --agent, got: {call_args}"
+            )
+
+    # -- Next-steps display for Copilot skills mode -----------------------
+
+    def test_init_skills_next_steps_show_skill_syntax(self, tmp_path):
+        """specify init --integration copilot --integration-options='--skills' shows /speckit-plan not /speckit.plan."""
+        from typer.testing import CliRunner
+        from specify_cli import app
+        project = tmp_path / "copilot-nextsteps"
+        project.mkdir()
+        old_cwd = os.getcwd()
+        try:
+            os.chdir(project)
+            result = CliRunner().invoke(app, [
+                "init", "--here", "--integration", "copilot",
+                "--integration-options", "--skills",
+                "--script", "sh", "--no-git",
+            ], catch_exceptions=False)
+        finally:
+            os.chdir(old_cwd)
+        assert result.exit_code == 0, f"init failed: {result.output}"
+        # Skills mode should show /speckit-plan (hyphenated)
+        assert "/speckit-plan" in result.output, (
+            f"Expected /speckit-plan in next steps but got:\n{result.output}"
+        )
+        # Must NOT show the dotted /speckit.plan form
+        assert "/speckit.plan" not in result.output, (
+            f"Should not show /speckit.plan in skills mode:\n{result.output}"
         )

tests/test_upgrade.py

@@ -100,12 +100,25 @@ class TestInstalledVersion:
     def test_invalid_metadata_error_returns_unknown(self):
         invalid_metadata_error = getattr(importlib.metadata, "InvalidMetadataError", None)
         if invalid_metadata_error is None:
-            pytest.skip("InvalidMetadataError is not available on this Python version")
-        with patch(
-            "importlib.metadata.version",
-            side_effect=invalid_metadata_error("bad metadata"),
-        ):
-            assert _get_installed_version() == "unknown"
+            # Python versions without InvalidMetadataError: simulate with a
+            # custom exception to verify the guarded except path works.
+            class _FakeInvalidMetadataError(Exception):
+                pass
+            invalid_metadata_error = _FakeInvalidMetadataError
+            # Patch the attribute onto importlib.metadata so the production
+            # getattr() finds it during this test.
+            with patch.object(importlib.metadata, "InvalidMetadataError", invalid_metadata_error, create=True):
+                with patch(
+                    "importlib.metadata.version",
+                    side_effect=invalid_metadata_error("bad metadata"),
+                ):
+                    assert _get_installed_version() == "unknown"
+        else:
+            with patch(
+                "importlib.metadata.version",
+                side_effect=invalid_metadata_error("bad metadata"),
+            ):
+                assert _get_installed_version() == "unknown"
 
 
 class TestNormalizeTag: