fix: address PR review round 4

- Reorder kimi migration: run super().setup() first so hyphenated
  targets exist, then migrate dotted dirs (prevents user content loss)
- Move _strip_ansi() to shared tests/conftest.py, import from there
  in test_extensions.py, test_presets.py, test_ai_skills.py
- Remove now-unused re imports from all three test files

Author: mnriem

src/specify_cli/integrations/kimi/__init__.py

@@ -63,14 +63,19 @@ def setup(
     ) -> list[Path]:
         """Install skills with optional legacy dotted-name migration."""
         parsed_options = parsed_options or {}
+
+        # Run base setup first so hyphenated targets (speckit-*) exist,
+        # then migrate/clean legacy dotted dirs without risking user content loss.
+        created = super().setup(
+            project_root, manifest, parsed_options=parsed_options, **opts
+        )
+
         if parsed_options.get("migrate_legacy", False):
             skills_dir = self.skills_dest(project_root)
             if skills_dir.is_dir():
                 _migrate_legacy_kimi_dotted_skills(skills_dir)
 
-        return super().setup(
-            project_root, manifest, parsed_options=parsed_options, **opts
-        )
+        return created
 
 
 def _migrate_legacy_kimi_dotted_skills(skills_dir: Path) -> tuple[int, int]:

tests/conftest.py

@@ -0,0 +1,10 @@
+"""Shared test helpers for the Spec Kit test suite."""
+
+import re
+
+_ANSI_ESCAPE_RE = re.compile(r"\x1b\[[0-?]*[ -/]*[@-~]")
+
+
+def strip_ansi(text: str) -> str:
+    """Remove ANSI escape codes from Rich-formatted CLI output."""
+    return _ANSI_ESCAPE_RE.sub("", text)

tests/test_ai_skills.py

@@ -10,7 +10,6 @@
 - CLI validation: --ai-skills requires --ai
 """
 
-import re
 import zipfile
 import pytest
 import tempfile
@@ -21,6 +20,7 @@
 from unittest.mock import patch
 
 import specify_cli
+from tests.conftest import strip_ansi
 
 from specify_cli import (
     _get_skills_dir,
@@ -923,7 +923,7 @@ def test_ai_skills_flag_appears_in_help(self):
         runner = CliRunner()
         result = runner.invoke(app, ["init", "--help"])
 
-        plain = re.sub(r'\x1b\[[0-9;]*m', '', result.output)
+        plain = strip_ansi(result.output)
         assert "--ai-skills" in plain
         assert "agent skills" in plain.lower()
 

tests/test_extensions.py

@@ -9,14 +9,14 @@
 - Catalog stack (multi-catalog support)
 """
 
-import re
 import pytest
 import json
 import tempfile
 import shutil
 from pathlib import Path
 from datetime import datetime, timezone
 
+from tests.conftest import strip_ansi
 from specify_cli.extensions import (
     CatalogEntry,
     CORE_COMMAND_NAMES,
@@ -34,11 +34,6 @@
 )
 
 
-def _strip_ansi(text: str) -> str:
-    """Remove ANSI escape codes from Rich-formatted CLI output."""
-    return re.sub(r'\x1b\[[0-9;]*m', '', text)
-
-
 # ===== Fixtures =====
 
 @pytest.fixture
@@ -3132,7 +3127,7 @@ def test_list_shows_extension_id(self, extension_dir, project_dir):
             result = runner.invoke(app, ["extension", "list"])
 
         assert result.exit_code == 0, result.output
-        plain = _strip_ansi(result.output)
+        plain = strip_ansi(result.output)
         # Verify the extension ID is shown in the output
         assert "test-ext" in plain
         # Verify name and version are also shown
@@ -3367,7 +3362,7 @@ def test_list_shows_priority(self, extension_dir, project_dir):
             result = runner.invoke(app, ["extension", "list"])
 
         assert result.exit_code == 0, result.output
-        plain = _strip_ansi(result.output)
+        plain = strip_ansi(result.output)
         assert "Priority: 7" in plain
 
     def test_set_priority_changes_priority(self, extension_dir, project_dir):
@@ -3389,7 +3384,7 @@ def test_set_priority_changes_priority(self, extension_dir, project_dir):
             result = runner.invoke(app, ["extension", "set-priority", "test-ext", "5"])
 
         assert result.exit_code == 0, result.output
-        plain = _strip_ansi(result.output)
+        plain = strip_ansi(result.output)
         assert "priority changed: 10 → 5" in plain
 
         # Reload registry to see updated value
@@ -3412,7 +3407,7 @@ def test_set_priority_same_value_no_change(self, extension_dir, project_dir):
             result = runner.invoke(app, ["extension", "set-priority", "test-ext", "5"])
 
         assert result.exit_code == 0, result.output
-        plain = _strip_ansi(result.output)
+        plain = strip_ansi(result.output)
         assert "already has priority 5" in plain
 
     def test_set_priority_invalid_value(self, extension_dir, project_dir):

tests/test_presets.py

@@ -11,7 +11,6 @@
 """
 
 import pytest
-import re
 import json
 import tempfile
 import shutil
@@ -21,6 +20,7 @@
 
 import yaml
 
+from tests.conftest import strip_ansi
 from specify_cli.presets import (
     PresetManifest,
     PresetRegistry,
@@ -36,11 +36,6 @@
 from specify_cli.extensions import ExtensionRegistry
 
 
-def _strip_ansi(text: str) -> str:
-    """Remove ANSI escape codes from Rich-formatted CLI output."""
-    return re.sub(r'\x1b\[[0-9;]*m', '', text)
-
-
 # ===== Fixtures =====
 
 
@@ -2447,7 +2442,7 @@ def test_set_priority_changes_priority(self, project_dir, pack_dir):
             result = runner.invoke(app, ["preset", "set-priority", "test-pack", "5"])
 
         assert result.exit_code == 0, result.output
-        plain = _strip_ansi(result.output)
+        plain = strip_ansi(result.output)
         assert "priority changed: 10 → 5" in plain
 
         # Reload registry to see updated value
@@ -2470,7 +2465,7 @@ def test_set_priority_same_value_no_change(self, project_dir, pack_dir):
             result = runner.invoke(app, ["preset", "set-priority", "test-pack", "5"])
 
         assert result.exit_code == 0, result.output
-        plain = _strip_ansi(result.output)
+        plain = strip_ansi(result.output)
         assert "already has priority 5" in plain
 
     def test_set_priority_invalid_value(self, project_dir, pack_dir):