Unify extension skill naming with hooks

Author: RbBtSn0w

src/specify_cli/extensions.py

@@ -560,12 +560,8 @@ def _register_extension_skills(
         if not skills_dir:
             return []
 
-        from . import load_init_options
         import yaml
 
-        opts = load_init_options(self.project_root)
-        selected_ai = opts.get("ai", "")
-
         written: List[str] = []
 
         for cmd_info in manifest.commands:
@@ -587,17 +583,12 @@ def _register_extension_skills(
             if not source_file.is_file():
                 continue
 
-            # Derive skill name from command name, matching the convention used by
-            # presets.py: strip the leading "speckit." prefix, then form:
-            #   Kimi  → "speckit.{short_name}"  (dot preserved for Kimi agent)
-            #   other → "speckit-{short_name}"  (hyphen separator)
+            # Derive skill name from command name using the same hyphenated
+            # convention as hook rendering and preset skill registration.
             short_name_raw = cmd_name
             if short_name_raw.startswith("speckit."):
                 short_name_raw = short_name_raw[len("speckit."):]
-            if selected_ai == "kimi":
-                skill_name = f"speckit.{short_name_raw}"
-            else:
-                skill_name = f"speckit-{short_name_raw}"
+            skill_name = f"speckit-{short_name_raw.replace('.', '-')}"
 
             # Check if skill already exists before creating the directory
             skill_subdir = skills_dir / skill_name

tests/test_extension_skills.py

@@ -208,8 +208,8 @@ def test_skills_created_when_ai_skills_active(self, skills_project, extension_di
 
         # Check that skill directories were created
         skill_dirs = sorted([d.name for d in skills_dir.iterdir() if d.is_dir()])
-        assert "speckit-test-ext.hello" in skill_dirs
-        assert "speckit-test-ext.world" in skill_dirs
+        assert "speckit-test-ext-hello" in skill_dirs
+        assert "speckit-test-ext-world" in skill_dirs
 
     def test_skill_md_content_correct(self, skills_project, extension_dir):
         """SKILL.md should have correct agentskills.io structure."""
@@ -219,13 +219,13 @@ def test_skill_md_content_correct(self, skills_project, extension_dir):
             extension_dir, "0.1.0", register_commands=False
         )
 
-        skill_file = skills_dir / "speckit-test-ext.hello" / "SKILL.md"
+        skill_file = skills_dir / "speckit-test-ext-hello" / "SKILL.md"
         assert skill_file.exists()
         content = skill_file.read_text()
 
         # Check structure
         assert content.startswith("---\n")
-        assert "name: speckit-test-ext.hello" in content
+        assert "name: speckit-test-ext-hello" in content
         assert "description:" in content
         assert "Test hello command" in content
         assert "source: extension:test-ext" in content
@@ -241,15 +241,15 @@ def test_skill_md_has_parseable_yaml(self, skills_project, extension_dir):
             extension_dir, "0.1.0", register_commands=False
         )
 
-        skill_file = skills_dir / "speckit-test-ext.hello" / "SKILL.md"
+        skill_file = skills_dir / "speckit-test-ext-hello" / "SKILL.md"
         content = skill_file.read_text()
 
         assert content.startswith("---\n")
         parts = content.split("---", 2)
         assert len(parts) >= 3
         parsed = yaml.safe_load(parts[1])
         assert isinstance(parsed, dict)
-        assert parsed["name"] == "speckit-test-ext.hello"
+        assert parsed["name"] == "speckit-test-ext-hello"
         assert "description" in parsed
 
     def test_no_skills_when_ai_skills_disabled(self, no_skills_project, extension_dir):
@@ -278,7 +278,7 @@ def test_existing_skill_not_overwritten(self, skills_project, extension_dir):
         project_dir, skills_dir = skills_project
 
         # Pre-create a custom skill
-        custom_dir = skills_dir / "speckit-test-ext.hello"
+        custom_dir = skills_dir / "speckit-test-ext-hello"
         custom_dir.mkdir(parents=True)
         custom_content = "# My Custom Hello Skill\nUser-modified content\n"
         (custom_dir / "SKILL.md").write_text(custom_content)
@@ -293,9 +293,9 @@ def test_existing_skill_not_overwritten(self, skills_project, extension_dir):
 
         # But the other skill should still be created
         metadata = manager.registry.get(manifest.id)
-        assert "speckit-test-ext.world" in metadata["registered_skills"]
+        assert "speckit-test-ext-world" in metadata["registered_skills"]
         # The pre-existing one should NOT be in registered_skills (it was skipped)
-        assert "speckit-test-ext.hello" not in metadata["registered_skills"]
+        assert "speckit-test-ext-hello" not in metadata["registered_skills"]
 
     def test_registered_skills_in_registry(self, skills_project, extension_dir):
         """Registry should contain registered_skills list."""
@@ -308,11 +308,11 @@ def test_registered_skills_in_registry(self, skills_project, extension_dir):
         metadata = manager.registry.get(manifest.id)
         assert "registered_skills" in metadata
         assert len(metadata["registered_skills"]) == 2
-        assert "speckit-test-ext.hello" in metadata["registered_skills"]
-        assert "speckit-test-ext.world" in metadata["registered_skills"]
+        assert "speckit-test-ext-hello" in metadata["registered_skills"]
+        assert "speckit-test-ext-world" in metadata["registered_skills"]
 
-    def test_kimi_uses_dot_notation(self, project_dir, temp_dir):
-        """Kimi agent should use dot notation for skill names."""
+    def test_kimi_uses_hyphenated_skill_names(self, project_dir, temp_dir):
+        """Kimi agent should use the same hyphenated skill names as hooks."""
         _create_init_options(project_dir, ai="kimi", ai_skills=True)
         _create_skills_dir(project_dir, ai="kimi")
         ext_dir = _create_extension_dir(temp_dir, ext_id="test-ext")
@@ -323,9 +323,8 @@ def test_kimi_uses_dot_notation(self, project_dir, temp_dir):
         )
 
         metadata = manager.registry.get(manifest.id)
-        # Kimi should use dots, not hyphens
-        assert "speckit.test-ext.hello" in metadata["registered_skills"]
-        assert "speckit.test-ext.world" in metadata["registered_skills"]
+        assert "speckit-test-ext-hello" in metadata["registered_skills"]
+        assert "speckit-test-ext-world" in metadata["registered_skills"]
 
     def test_missing_command_file_skipped(self, skills_project, temp_dir):
         """Commands with missing source files should be skipped gracefully."""
@@ -372,8 +371,8 @@ def test_missing_command_file_skipped(self, skills_project, temp_dir):
         )
 
         metadata = manager.registry.get(manifest.id)
-        assert "speckit-missing-cmd-ext.exists" in metadata["registered_skills"]
-        assert "speckit-missing-cmd-ext.ghost" not in metadata["registered_skills"]
+        assert "speckit-missing-cmd-ext-exists" in metadata["registered_skills"]
+        assert "speckit-missing-cmd-ext-ghost" not in metadata["registered_skills"]
 
 
 # ===== Extension Skill Unregistration Tests =====
@@ -390,16 +389,16 @@ def test_skills_removed_on_extension_remove(self, skills_project, extension_dir)
         )
 
         # Verify skills exist
-        assert (skills_dir / "speckit-test-ext.hello" / "SKILL.md").exists()
-        assert (skills_dir / "speckit-test-ext.world" / "SKILL.md").exists()
+        assert (skills_dir / "speckit-test-ext-hello" / "SKILL.md").exists()
+        assert (skills_dir / "speckit-test-ext-world" / "SKILL.md").exists()
 
         # Remove extension
         result = manager.remove(manifest.id, keep_config=False)
         assert result is True
 
         # Skills should be gone
-        assert not (skills_dir / "speckit-test-ext.hello").exists()
-        assert not (skills_dir / "speckit-test-ext.world").exists()
+        assert not (skills_dir / "speckit-test-ext-hello").exists()
+        assert not (skills_dir / "speckit-test-ext-world").exists()
 
     def test_other_skills_preserved_on_remove(self, skills_project, extension_dir):
         """Non-extension skills should not be affected by extension removal."""
@@ -430,8 +429,8 @@ def test_remove_handles_already_deleted_skills(self, skills_project, extension_d
         )
 
         # Manually delete skill dirs before calling remove
-        shutil.rmtree(skills_dir / "speckit-test-ext.hello")
-        shutil.rmtree(skills_dir / "speckit-test-ext.world")
+        shutil.rmtree(skills_dir / "speckit-test-ext-hello")
+        shutil.rmtree(skills_dir / "speckit-test-ext-world")
 
         # Should not raise
         result = manager.remove(manifest.id, keep_config=False)
@@ -492,10 +491,10 @@ def test_command_without_frontmatter(self, skills_project, temp_dir):
             ext_dir, "0.1.0", register_commands=False
         )
 
-        skill_file = skills_dir / "speckit-nofm-ext.plain" / "SKILL.md"
+        skill_file = skills_dir / "speckit-nofm-ext-plain" / "SKILL.md"
         assert skill_file.exists()
         content = skill_file.read_text()
-        assert "name: speckit-nofm-ext.plain" in content
+        assert "name: speckit-nofm-ext-plain" in content
         # Fallback description when no frontmatter description
         assert "Extension command: speckit.nofm-ext.plain" in content
         assert "Body without frontmatter." in content
@@ -512,8 +511,8 @@ def test_gemini_agent_skills(self, project_dir, temp_dir):
         )
 
         skills_dir = project_dir / ".gemini" / "skills"
-        assert (skills_dir / "speckit-test-ext.hello" / "SKILL.md").exists()
-        assert (skills_dir / "speckit-test-ext.world" / "SKILL.md").exists()
+        assert (skills_dir / "speckit-test-ext-hello" / "SKILL.md").exists()
+        assert (skills_dir / "speckit-test-ext-world" / "SKILL.md").exists()
 
     def test_multiple_extensions_independent_skills(self, skills_project, temp_dir):
         """Installing and removing different extensions should be independent."""
@@ -531,15 +530,15 @@ def test_multiple_extensions_independent_skills(self, skills_project, temp_dir):
         )
 
         # Both should have skills
-        assert (skills_dir / "speckit-ext-a.hello" / "SKILL.md").exists()
-        assert (skills_dir / "speckit-ext-b.hello" / "SKILL.md").exists()
+        assert (skills_dir / "speckit-ext-a-hello" / "SKILL.md").exists()
+        assert (skills_dir / "speckit-ext-b-hello" / "SKILL.md").exists()
 
         # Remove ext-a
         manager.remove("ext-a", keep_config=False)
 
         # ext-a skills gone, ext-b skills preserved
-        assert not (skills_dir / "speckit-ext-a.hello").exists()
-        assert (skills_dir / "speckit-ext-b.hello" / "SKILL.md").exists()
+        assert not (skills_dir / "speckit-ext-a-hello").exists()
+        assert (skills_dir / "speckit-ext-b-hello" / "SKILL.md").exists()
 
     def test_malformed_frontmatter_handled(self, skills_project, temp_dir):
         """Commands with invalid YAML frontmatter should still produce valid skills."""
@@ -588,7 +587,7 @@ def test_malformed_frontmatter_handled(self, skills_project, temp_dir):
             ext_dir, "0.1.0", register_commands=False
         )
 
-        skill_file = skills_dir / "speckit-badfm-ext.broken" / "SKILL.md"
+        skill_file = skills_dir / "speckit-badfm-ext-broken" / "SKILL.md"
         assert skill_file.exists()
         content = skill_file.read_text()
         # Fallback description since frontmatter was invalid
@@ -604,7 +603,7 @@ def test_remove_cleans_up_when_init_options_deleted(self, skills_project, extens
         )
 
         # Verify skills exist
-        assert (skills_dir / "speckit-test-ext.hello" / "SKILL.md").exists()
+        assert (skills_dir / "speckit-test-ext-hello" / "SKILL.md").exists()
 
         # Delete init-options.json to simulate user change
         init_opts = project_dir / ".specify" / "init-options.json"
@@ -613,8 +612,8 @@ def test_remove_cleans_up_when_init_options_deleted(self, skills_project, extens
         # Remove should still clean up via fallback scan
         result = manager.remove(manifest.id, keep_config=False)
         assert result is True
-        assert not (skills_dir / "speckit-test-ext.hello").exists()
-        assert not (skills_dir / "speckit-test-ext.world").exists()
+        assert not (skills_dir / "speckit-test-ext-hello").exists()
+        assert not (skills_dir / "speckit-test-ext-world").exists()
 
     def test_remove_cleans_up_when_ai_skills_toggled(self, skills_project, extension_dir):
         """Skills should be cleaned up even if ai_skills is toggled to false after install."""
@@ -625,13 +624,13 @@ def test_remove_cleans_up_when_ai_skills_toggled(self, skills_project, extension
         )
 
         # Verify skills exist
-        assert (skills_dir / "speckit-test-ext.hello" / "SKILL.md").exists()
+        assert (skills_dir / "speckit-test-ext-hello" / "SKILL.md").exists()
 
         # Toggle ai_skills to false
         _create_init_options(project_dir, ai="claude", ai_skills=False)
 
         # Remove should still clean up via fallback scan
         result = manager.remove(manifest.id, keep_config=False)
         assert result is True
-        assert not (skills_dir / "speckit-test-ext.hello").exists()
-        assert not (skills_dir / "speckit-test-ext.world").exists()
+        assert not (skills_dir / "speckit-test-ext-hello").exists()
+        assert not (skills_dir / "speckit-test-ext-world").exists()