From efe6010fbaa8c142bf2f9795e86c11fbf347bdc0 Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Wed, 25 Mar 2026 17:01:39 +0800 Subject: [PATCH 01/23] fix: unify hyphenated skills and migrate legacy kimi dotted dirs --- .../scripts/create-release-packages.ps1 | 5 +- .../scripts/create-release-packages.sh | 5 +- src/specify_cli/__init__.py | 69 ++++++++++-- src/specify_cli/agents.py | 3 +- src/specify_cli/extensions.py | 59 +++++++++- src/specify_cli/presets.py | 11 +- tests/test_ai_skills.py | 47 ++++++-- tests/test_extensions.py | 103 ++++++++++++++++-- tests/test_presets.py | 47 ++++++++ 9 files changed, 304 insertions(+), 45 deletions(-) diff --git a/.github/workflows/scripts/create-release-packages.ps1 b/.github/workflows/scripts/create-release-packages.ps1 index 8f3cfec36..5bd600e5f 100644 --- a/.github/workflows/scripts/create-release-packages.ps1 +++ b/.github/workflows/scripts/create-release-packages.ps1 @@ -202,8 +202,7 @@ agent: $basename } # Create skills in \\SKILL.md format. -# Most agents use hyphenated names (e.g. speckit-plan); Kimi is the -# current dotted-name exception (e.g. speckit.plan). +# Skills use hyphenated names (e.g. speckit-plan). # # Technical debt note: # Keep SKILL.md frontmatter aligned with `install_ai_skills()` and extension @@ -463,7 +462,7 @@ function Build-Variant { 'kimi' { $skillsDir = Join-Path $baseDir ".kimi/skills" New-Item -ItemType Directory -Force -Path $skillsDir | Out-Null - New-Skills -SkillsDir $skillsDir -ScriptVariant $Script -AgentName 'kimi' -Separator '.' + New-Skills -SkillsDir $skillsDir -ScriptVariant $Script -AgentName 'kimi' } 'trae' { $rulesDir = Join-Path $baseDir ".trae/rules" diff --git a/.github/workflows/scripts/create-release-packages.sh b/.github/workflows/scripts/create-release-packages.sh index d07e4a2df..a83494c3a 100755 --- a/.github/workflows/scripts/create-release-packages.sh +++ b/.github/workflows/scripts/create-release-packages.sh @@ -140,8 +140,7 @@ EOF } # Create skills in //SKILL.md format. -# Most agents use hyphenated names (e.g. speckit-plan); Kimi is the -# current dotted-name exception (e.g. speckit.plan). +# Skills use hyphenated names (e.g. speckit-plan). # # Technical debt note: # Keep SKILL.md frontmatter aligned with `install_ai_skills()` and extension @@ -321,7 +320,7 @@ build_variant() { generate_commands vibe md "\$ARGUMENTS" "$base_dir/.vibe/prompts" "$script" ;; kimi) mkdir -p "$base_dir/.kimi/skills" - create_skills "$base_dir/.kimi/skills" "$script" "kimi" "." ;; + create_skills "$base_dir/.kimi/skills" "$script" "kimi" ;; trae) mkdir -p "$base_dir/.trae/rules" generate_commands trae md "\$ARGUMENTS" "$base_dir/.trae/rules" "$script" ;; diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index d78609ad6..f5fe79d23 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1492,9 +1492,7 @@ def load_init_options(project_path: Path) -> dict[str, Any]: # Agent-specific skill directory overrides for agents whose skills directory # doesn't follow the standard /skills/ pattern -AGENT_SKILLS_DIR_OVERRIDES = { - "codex": ".agents/skills", # Codex agent layout override -} +AGENT_SKILLS_DIR_OVERRIDES = {} # Default skills directory for agents not in AGENT_CONFIG DEFAULT_SKILLS_DIR = ".agents/skills" @@ -1648,10 +1646,7 @@ def install_ai_skills( command_name = command_name[len("speckit."):] if command_name.endswith(".agent"): command_name = command_name[:-len(".agent")] - if selected_ai == "kimi": - skill_name = f"speckit.{command_name}" - else: - skill_name = f"speckit-{command_name}" + skill_name = f"speckit-{command_name}" # Create skill directory (additive — never removes existing content) skill_dir = skills_dir / skill_name @@ -1730,8 +1725,48 @@ def _has_bundled_skills(project_path: Path, selected_ai: str) -> bool: if not skills_dir.is_dir(): return False - pattern = "speckit.*/SKILL.md" if selected_ai == "kimi" else "speckit-*/SKILL.md" - return any(skills_dir.glob(pattern)) + return any(skills_dir.glob("speckit-*/SKILL.md")) + + +def _migrate_legacy_kimi_dotted_skills(skills_dir: Path) -> tuple[int, int]: + """Migrate legacy Kimi dotted skill dirs (speckit.xxx) to hyphenated format. + + Temporary migration helper: + - Intended removal window: after 2026-06-25. + - Purpose: one-time cleanup for projects initialized before Kimi moved to + hyphenated skills (speckit-xxx). + + Returns: + Tuple[migrated_count, removed_count] + - migrated_count: old dotted dir renamed to hyphenated dir + - removed_count: old dotted dir deleted because hyphenated dir already existed + """ + if not skills_dir.is_dir(): + return (0, 0) + + migrated_count = 0 + removed_count = 0 + + for legacy_dir in sorted(skills_dir.glob("speckit.*")): + if not legacy_dir.is_dir(): + continue + if not (legacy_dir / "SKILL.md").exists(): + continue + + suffix = legacy_dir.name[len("speckit."):] + if not suffix: + continue + + target_dir = skills_dir / f"speckit-{suffix.replace('.', '-')}" + + if target_dir.exists(): + shutil.rmtree(legacy_dir) + removed_count += 1 + else: + shutil.move(str(legacy_dir), str(target_dir)) + migrated_count += 1 + + return (migrated_count, removed_count) AGENT_SKILLS_MIGRATIONS = { @@ -2097,13 +2132,23 @@ def init( if ai_skills: if selected_ai in NATIVE_SKILLS_AGENTS: skills_dir = _get_skills_dir(project_path, selected_ai) + migrated_legacy_kimi_skills = 0 + removed_legacy_kimi_skills = 0 + if selected_ai == "kimi": + migrated_legacy_kimi_skills, removed_legacy_kimi_skills = _migrate_legacy_kimi_dotted_skills(skills_dir) bundled_found = _has_bundled_skills(project_path, selected_ai) if bundled_found: + detail = f"bundled skills → {skills_dir.relative_to(project_path)}" + if migrated_legacy_kimi_skills or removed_legacy_kimi_skills: + detail += ( + f" (migrated {migrated_legacy_kimi_skills}, " + f"removed {removed_legacy_kimi_skills} legacy Kimi dotted skills)" + ) if tracker: tracker.start("ai-skills") - tracker.complete("ai-skills", f"bundled skills → {skills_dir.relative_to(project_path)}") + tracker.complete("ai-skills", detail) else: - console.print(f"[green]✓[/green] Using bundled agent skills in {skills_dir.relative_to(project_path)}/") + console.print(f"[green]✓[/green] Using {detail}") else: # Compatibility fallback: convert command templates to skills # when an older template archive does not include native skills. @@ -2288,7 +2333,7 @@ def _display_cmd(name: str) -> str: if codex_skill_mode: return f"$speckit-{name}" if kimi_skill_mode: - return f"/skill:speckit.{name}" + return f"/skill:speckit-{name}" return f"/speckit.{name}" steps_lines.append(f"{step_num}. Start using {usage_label} with your AI agent:") diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index 7fe531606..743f8193d 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -400,8 +400,9 @@ def _compute_output_name(agent_name: str, cmd_name: str, agent_config: Dict[str, short_name = cmd_name if short_name.startswith("speckit."): short_name = short_name[len("speckit."):] + short_name = short_name.replace(".", "-") - return f"speckit.{short_name}" if agent_name == "kimi" else f"speckit-{short_name}" + return f"speckit-{short_name}" def register_commands( self, diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index d71480ac4..3f97c9955 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -1931,6 +1931,8 @@ def has_value(self, key_path: str) -> bool: class HookExecutor: """Manages extension hook execution.""" + INIT_OPTIONS_FILE = ".specify/init-options.json" + def __init__(self, project_root: Path): """Initialize hook executor. @@ -1941,6 +1943,49 @@ def __init__(self, project_root: Path): self.extensions_dir = project_root / ".specify" / "extensions" self.config_file = project_root / ".specify" / "extensions.yml" + def _load_init_options(self) -> Dict[str, Any]: + """Load persisted init options used to determine invocation style.""" + options_file = self.project_root / self.INIT_OPTIONS_FILE + if not options_file.exists(): + return {} + try: + payload = json.loads(options_file.read_text(encoding="utf-8")) + return payload if isinstance(payload, dict) else {} + except (json.JSONDecodeError, OSError, UnicodeError): + return {} + + @staticmethod + def _skill_name_from_command(command: str) -> str: + """Map a command id like speckit.plan to speckit-plan skill name.""" + if not isinstance(command, str): + return "" + command_id = command.strip() + if not command_id.startswith("speckit."): + return "" + return f"speckit-{command_id[len('speckit.'):].replace('.', '-')}" + + def _render_hook_invocation(self, command: str) -> str: + """Render an agent-specific invocation string for a hook command.""" + if not isinstance(command, str): + return "" + + command_id = command.strip() + if not command_id: + return "" + + init_options = self._load_init_options() + selected_ai = init_options.get("ai") + codex_skill_mode = selected_ai == "codex" and bool(init_options.get("ai_skills")) + kimi_skill_mode = selected_ai == "kimi" + + skill_name = self._skill_name_from_command(command_id) + if codex_skill_mode and skill_name: + return f"${skill_name}" + if kimi_skill_mode and skill_name: + return f"/skill:{skill_name}" + + return f"/{command_id}" + def get_project_config(self) -> Dict[str, Any]: """Load project-level extension configuration. @@ -2183,21 +2228,27 @@ def format_hook_message( for hook in hooks: extension = hook.get("extension") command = hook.get("command") + invocation = self._render_hook_invocation(command) optional = hook.get("optional", True) prompt = hook.get("prompt", "") description = hook.get("description", "") if optional: lines.append(f"\n**Optional Hook**: {extension}") - lines.append(f"Command: `/{command}`") + if invocation: + lines.append(f"Command: `{invocation}`") if description: lines.append(f"Description: {description}") lines.append(f"\nPrompt: {prompt}") - lines.append(f"To execute: `/{command}`") + if invocation: + lines.append(f"To execute: `{invocation}`") else: lines.append(f"\n**Automatic Hook**: {extension}") - lines.append(f"Executing: `/{command}`") + if invocation: + lines.append(f"Executing: `{invocation}`") lines.append(f"EXECUTE_COMMAND: {command}") + if invocation: + lines.append(f"EXECUTE_COMMAND_INVOCATION: {invocation}") return "\n".join(lines) @@ -2261,6 +2312,7 @@ def execute_hook(self, hook: Dict[str, Any]) -> Dict[str, Any]: """ return { "command": hook.get("command"), + "invocation": self._render_hook_invocation(hook.get("command")), "extension": hook.get("extension"), "optional": hook.get("optional", True), "description": hook.get("description", ""), @@ -2304,4 +2356,3 @@ def disable_hooks(self, extension_id: str): hook["enabled"] = False self.save_project_config(config) - diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 24d523aa8..a79a84e91 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -628,10 +628,7 @@ def _register_skills( if not skills_dir: return [] - from . import SKILL_DESCRIPTIONS, load_init_options - - opts = load_init_options(self.project_root) - selected_ai = opts.get("ai", "") + from . import SKILL_DESCRIPTIONS written: List[str] = [] @@ -646,10 +643,8 @@ def _register_skills( short_name = cmd_name if short_name.startswith("speckit."): short_name = short_name[len("speckit."):] - if selected_ai == "kimi": - skill_name = f"speckit.{short_name}" - else: - skill_name = f"speckit-{short_name}" + short_name = short_name.replace(".", "-") + skill_name = f"speckit-{short_name}" # Only overwrite if the skill already exists (i.e. --ai-skills was used) skill_subdir = skills_dir / skill_name diff --git a/tests/test_ai_skills.py b/tests/test_ai_skills.py index cf2b6b7b9..1300fac9e 100644 --- a/tests/test_ai_skills.py +++ b/tests/test_ai_skills.py @@ -24,6 +24,7 @@ from specify_cli import ( _get_skills_dir, + _migrate_legacy_kimi_dotted_skills, install_ai_skills, AGENT_SKILLS_DIR_OVERRIDES, DEFAULT_SKILLS_DIR, @@ -169,8 +170,8 @@ def test_copilot_skills_dir(self, project_dir): result = _get_skills_dir(project_dir, "copilot") assert result == project_dir / ".github" / "skills" - def test_codex_uses_override(self, project_dir): - """Codex should use the AGENT_SKILLS_DIR_OVERRIDES value.""" + def test_codex_skills_dir_from_agent_config(self, project_dir): + """Codex should resolve skills directory from AGENT_CONFIG folder.""" result = _get_skills_dir(project_dir, "codex") assert result == project_dir / ".agents" / "skills" @@ -211,6 +212,39 @@ def test_override_takes_precedence_over_config(self, project_dir): assert result == expected +class TestKimiLegacySkillMigration: + """Test temporary migration from Kimi dotted skill names to hyphenated names.""" + + def test_migrates_legacy_dotted_skill_directory(self, project_dir): + skills_dir = project_dir / ".kimi" / "skills" + legacy_dir = skills_dir / "speckit.plan" + legacy_dir.mkdir(parents=True) + (legacy_dir / "SKILL.md").write_text("legacy") + + migrated, removed = _migrate_legacy_kimi_dotted_skills(skills_dir) + + assert migrated == 1 + assert removed == 0 + assert not legacy_dir.exists() + assert (skills_dir / "speckit-plan" / "SKILL.md").exists() + + def test_removes_legacy_dir_when_hyphenated_target_exists(self, project_dir): + skills_dir = project_dir / ".kimi" / "skills" + legacy_dir = skills_dir / "speckit.plan" + legacy_dir.mkdir(parents=True) + (legacy_dir / "SKILL.md").write_text("legacy") + target_dir = skills_dir / "speckit-plan" + target_dir.mkdir(parents=True) + (target_dir / "SKILL.md").write_text("new") + + migrated, removed = _migrate_legacy_kimi_dotted_skills(skills_dir) + + assert migrated == 0 + assert removed == 1 + assert not legacy_dir.exists() + assert (target_dir / "SKILL.md").read_text() == "new" + + # ===== install_ai_skills Tests ===== class TestInstallAiSkills: @@ -473,8 +507,7 @@ def test_skills_install_for_all_agents(self, temp_dir, agent_key): skills_dir = _get_skills_dir(proj, agent_key) assert skills_dir.exists() skill_dirs = [d.name for d in skills_dir.iterdir() if d.is_dir()] - # Kimi uses dotted skill names; other agents use hyphen-separated names. - expected_skill_name = "speckit.specify" if agent_key == "kimi" else "speckit-specify" + expected_skill_name = "speckit-specify" assert expected_skill_name in skill_dirs assert (skills_dir / expected_skill_name / "SKILL.md").exists() @@ -1118,12 +1151,12 @@ def _fake_download(*args, **kwargs): assert "Optional skills that you can use for your specs" in result.output def test_kimi_next_steps_show_skill_invocation(self, monkeypatch): - """Kimi next-steps guidance should display /skill:speckit.* usage.""" + """Kimi next-steps guidance should display /skill:speckit-* usage.""" from typer.testing import CliRunner def _fake_download(*args, **kwargs): project_path = Path(args[0]) - skill_dir = project_path / ".kimi" / "skills" / "speckit.specify" + skill_dir = project_path / ".kimi" / "skills" / "speckit-specify" skill_dir.mkdir(parents=True, exist_ok=True) (skill_dir / "SKILL.md").write_text("---\ndescription: Test skill\n---\n\nBody.\n") @@ -1137,7 +1170,7 @@ def _fake_download(*args, **kwargs): ) assert result.exit_code == 0 - assert "/skill:speckit.constitution" in result.output + assert "/skill:speckit-constitution" in result.output assert "/speckit.constitution" not in result.output assert "Optional skills that you can use for your specs" in result.output diff --git a/tests/test_extensions.py b/tests/test_extensions.py index cd0f9ba44..2a297b284 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -22,6 +22,7 @@ ExtensionRegistry, ExtensionManager, CommandRegistrar, + HookExecutor, ExtensionCatalog, ExtensionError, ValidationError, @@ -875,11 +876,11 @@ def test_codex_skill_registration_writes_skill_frontmatter(self, extension_dir, registrar = CommandRegistrar() registrar.register_commands_for_agent("codex", manifest, extension_dir, project_dir) - skill_file = skills_dir / "speckit-test.hello" / "SKILL.md" + skill_file = skills_dir / "speckit-test-hello" / "SKILL.md" assert skill_file.exists() content = skill_file.read_text() - assert "name: speckit-test.hello" in content + assert "name: speckit-test-hello" in content assert "description: Test hello command" in content assert "compatibility:" in content assert "metadata:" in content @@ -944,7 +945,7 @@ def test_codex_skill_registration_resolves_script_placeholders(self, project_dir registrar = CommandRegistrar() registrar.register_commands_for_agent("codex", manifest, ext_dir, project_dir) - skill_file = skills_dir / "speckit-test.plan" / "SKILL.md" + skill_file = skills_dir / "speckit-test-plan" / "SKILL.md" assert skill_file.exists() content = skill_file.read_text() @@ -994,12 +995,12 @@ def test_codex_skill_alias_frontmatter_matches_alias_name(self, project_dir, tem registrar = CommandRegistrar() registrar.register_commands_for_agent("codex", manifest, ext_dir, project_dir) - primary = skills_dir / "speckit-alias.cmd" / "SKILL.md" + primary = skills_dir / "speckit-alias-cmd" / "SKILL.md" alias = skills_dir / "speckit-shortcut" / "SKILL.md" assert primary.exists() assert alias.exists() - assert "name: speckit-alias.cmd" in primary.read_text() + assert "name: speckit-alias-cmd" in primary.read_text() assert "name: speckit-shortcut" in alias.read_text() def test_codex_skill_registration_uses_fallback_script_variant_without_init_options( @@ -1056,7 +1057,7 @@ def test_codex_skill_registration_uses_fallback_script_variant_without_init_opti registrar = CommandRegistrar() registrar.register_commands_for_agent("codex", manifest, ext_dir, project_dir) - skill_file = skills_dir / "speckit-fallback.plan" / "SKILL.md" + skill_file = skills_dir / "speckit-fallback-plan" / "SKILL.md" assert skill_file.exists() content = skill_file.read_text() @@ -1121,7 +1122,7 @@ def test_codex_skill_registration_fallback_prefers_powershell_on_windows( registrar = CommandRegistrar() registrar.register_commands_for_agent("codex", manifest, ext_dir, project_dir) - skill_file = skills_dir / "speckit-windows.plan" / "SKILL.md" + skill_file = skills_dir / "speckit-windows-plan" / "SKILL.md" assert skill_file.exists() content = skill_file.read_text() @@ -3231,3 +3232,91 @@ def test_mixed_legacy_and_new_extensions_ordering(self, temp_dir): assert result[0][0] == "ext-with-priority" assert result[1][0] == "legacy-ext" assert result[2][0] == "ext-low-priority" + + +class TestHookInvocationRendering: + """Test hook invocation formatting for different agent modes.""" + + def test_kimi_hooks_render_skill_invocation(self, project_dir): + """Kimi projects should render /skill:speckit-* invocations.""" + init_options = project_dir / ".specify" / "init-options.json" + init_options.parent.mkdir(parents=True, exist_ok=True) + init_options.write_text(json.dumps({"ai": "kimi", "ai_skills": False})) + + hook_executor = HookExecutor(project_dir) + message = hook_executor.format_hook_message( + "before_plan", + [ + { + "extension": "test-ext", + "command": "speckit.plan", + "optional": False, + } + ], + ) + + assert "Executing: `/skill:speckit-plan`" in message + assert "EXECUTE_COMMAND: speckit.plan" in message + assert "EXECUTE_COMMAND_INVOCATION: /skill:speckit-plan" in message + + def test_codex_hooks_render_dollar_skill_invocation(self, project_dir): + """Codex projects with --ai-skills should render $speckit-* invocations.""" + init_options = project_dir / ".specify" / "init-options.json" + init_options.parent.mkdir(parents=True, exist_ok=True) + init_options.write_text(json.dumps({"ai": "codex", "ai_skills": True})) + + hook_executor = HookExecutor(project_dir) + execution = hook_executor.execute_hook( + { + "extension": "test-ext", + "command": "speckit.tasks", + "optional": False, + } + ) + + assert execution["command"] == "speckit.tasks" + assert execution["invocation"] == "$speckit-tasks" + + def test_non_skill_command_keeps_slash_invocation(self, project_dir): + """Custom hook commands should keep slash invocation style.""" + init_options = project_dir / ".specify" / "init-options.json" + init_options.parent.mkdir(parents=True, exist_ok=True) + init_options.write_text(json.dumps({"ai": "kimi", "ai_skills": False})) + + hook_executor = HookExecutor(project_dir) + message = hook_executor.format_hook_message( + "before_tasks", + [ + { + "extension": "test-ext", + "command": "pre_tasks_test", + "optional": False, + } + ], + ) + + assert "Executing: `/pre_tasks_test`" in message + assert "EXECUTE_COMMAND: pre_tasks_test" in message + assert "EXECUTE_COMMAND_INVOCATION: /pre_tasks_test" in message + + def test_extension_command_uses_hyphenated_skill_invocation(self, project_dir): + """Multi-segment extension command ids should map to hyphenated skills.""" + init_options = project_dir / ".specify" / "init-options.json" + init_options.parent.mkdir(parents=True, exist_ok=True) + init_options.write_text(json.dumps({"ai": "kimi", "ai_skills": False})) + + hook_executor = HookExecutor(project_dir) + message = hook_executor.format_hook_message( + "after_tasks", + [ + { + "extension": "test-ext", + "command": "speckit.test.hello", + "optional": False, + } + ], + ) + + assert "Executing: `/skill:speckit-test-hello`" in message + assert "EXECUTE_COMMAND: speckit.test.hello" in message + assert "EXECUTE_COMMAND_INVOCATION: /skill:speckit-test-hello" in message diff --git a/tests/test_presets.py b/tests/test_presets.py index 95dca4122..77e6540df 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -2054,6 +2054,53 @@ def test_no_skills_registered_when_no_skill_dir_exists(self, project_dir, temp_d metadata = manager.registry.get("self-test") assert metadata.get("registered_skills", []) == [] + def test_extension_skill_override_matches_hyphenated_multisegment_name(self, project_dir, temp_dir): + """Preset overrides for speckit.. should target speckit-- skills.""" + self._write_init_options(project_dir, ai="codex") + skills_dir = project_dir / ".agents" / "skills" + self._create_skill(skills_dir, "speckit-fakeext-cmd", body="untouched") + (project_dir / ".specify" / "extensions" / "fakeext").mkdir(parents=True, exist_ok=True) + + preset_dir = temp_dir / "ext-skill-override" + preset_dir.mkdir() + (preset_dir / "commands").mkdir() + (preset_dir / "commands" / "speckit.fakeext.cmd.md").write_text( + "---\ndescription: Override fakeext cmd\n---\n\npreset:ext-skill-override\n" + ) + manifest_data = { + "schema_version": "1.0", + "preset": { + "id": "ext-skill-override", + "name": "Ext Skill Override", + "version": "1.0.0", + "description": "Test", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "templates": [ + { + "type": "command", + "name": "speckit.fakeext.cmd", + "file": "commands/speckit.fakeext.cmd.md", + } + ] + }, + } + with open(preset_dir / "preset.yml", "w") as f: + yaml.dump(manifest_data, f) + + manager = PresetManager(project_dir) + manager.install_from_directory(preset_dir, "0.1.5") + + skill_file = skills_dir / "speckit-fakeext-cmd" / "SKILL.md" + assert skill_file.exists() + content = skill_file.read_text() + assert "preset:ext-skill-override" in content + assert "name: speckit-fakeext-cmd" in content + + metadata = manager.registry.get("ext-skill-override") + assert "speckit-fakeext-cmd" in metadata.get("registered_skills", []) + class TestPresetSetPriority: """Test preset set-priority CLI command.""" From 17cea06357de45035f9aec5092c19de585062607 Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Wed, 25 Mar 2026 21:50:52 +0800 Subject: [PATCH 02/23] fix: preserve legacy kimi dotted preset skill overrides --- src/specify_cli/presets.py | 63 +++++++++++++++++++++----------------- tests/test_presets.py | 21 +++++++++++++ 2 files changed, 56 insertions(+), 28 deletions(-) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index a79a84e91..008bba92a 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -640,15 +640,21 @@ def _register_skills( continue # Derive the short command name (e.g. "specify" from "speckit.specify") - short_name = cmd_name - if short_name.startswith("speckit."): - short_name = short_name[len("speckit."):] - short_name = short_name.replace(".", "-") + raw_short_name = cmd_name + if raw_short_name.startswith("speckit."): + raw_short_name = raw_short_name[len("speckit."):] + short_name = raw_short_name.replace(".", "-") skill_name = f"speckit-{short_name}" - - # Only overwrite if the skill already exists (i.e. --ai-skills was used) - skill_subdir = skills_dir / skill_name - if not skill_subdir.exists(): + legacy_skill_name = f"speckit.{raw_short_name}" + + # Only overwrite existing skills (i.e. --ai-skills was used). + # If both modern and legacy directories exist, update both. + target_skill_names: List[str] = [] + if (skills_dir / skill_name).exists(): + target_skill_names.append(skill_name) + if legacy_skill_name != skill_name and (skills_dir / legacy_skill_name).exists(): + target_skill_names.append(legacy_skill_name) + if not target_skill_names: continue # Parse the command file @@ -673,27 +679,28 @@ def _register_skills( original_desc or f"Spec-kit workflow command: {short_name}", ) - frontmatter_data = { - "name": skill_name, - "description": enhanced_desc, - "compatibility": "Requires spec-kit project structure with .specify/ directory", - "metadata": { - "author": "github-spec-kit", - "source": f"preset:{manifest.id}", - }, - } - frontmatter_text = yaml.safe_dump(frontmatter_data, sort_keys=False).strip() - skill_content = ( - f"---\n" - f"{frontmatter_text}\n" - f"---\n\n" - f"# Speckit {short_name.title()} Skill\n\n" - f"{body}\n" - ) + for target_skill_name in target_skill_names: + frontmatter_data = { + "name": target_skill_name, + "description": enhanced_desc, + "compatibility": "Requires spec-kit project structure with .specify/ directory", + "metadata": { + "author": "github-spec-kit", + "source": f"preset:{manifest.id}", + }, + } + frontmatter_text = yaml.safe_dump(frontmatter_data, sort_keys=False).strip() + skill_content = ( + f"---\n" + f"{frontmatter_text}\n" + f"---\n\n" + f"# Speckit {short_name.title()} Skill\n\n" + f"{body}\n" + ) - skill_file = skill_subdir / "SKILL.md" - skill_file.write_text(skill_content, encoding="utf-8") - written.append(skill_name) + skill_file = skills_dir / target_skill_name / "SKILL.md" + skill_file.write_text(skill_content, encoding="utf-8") + written.append(target_skill_name) return written diff --git a/tests/test_presets.py b/tests/test_presets.py index 77e6540df..99575daf1 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -2101,6 +2101,27 @@ def test_extension_skill_override_matches_hyphenated_multisegment_name(self, pro metadata = manager.registry.get("ext-skill-override") assert "speckit-fakeext-cmd" in metadata.get("registered_skills", []) + def test_kimi_legacy_dotted_skill_override_still_applies(self, project_dir, temp_dir): + """Preset overrides should still target legacy dotted Kimi skill directories.""" + self._write_init_options(project_dir, ai="kimi") + skills_dir = project_dir / ".kimi" / "skills" + self._create_skill(skills_dir, "speckit.specify", body="untouched") + + (project_dir / ".kimi" / "commands").mkdir(parents=True, exist_ok=True) + + manager = PresetManager(project_dir) + self_test_dir = Path(__file__).parent.parent / "presets" / "self-test" + manager.install_from_directory(self_test_dir, "0.1.5") + + skill_file = skills_dir / "speckit.specify" / "SKILL.md" + assert skill_file.exists() + content = skill_file.read_text() + assert "preset:self-test" in content + assert "name: speckit.specify" in content + + metadata = manager.registry.get("self-test") + assert "speckit.specify" in metadata.get("registered_skills", []) + class TestPresetSetPriority: """Test preset set-priority CLI command.""" From 3c8c42dccc9d1254831f1532bc3c66434802fdca Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Wed, 25 Mar 2026 21:52:58 +0800 Subject: [PATCH 03/23] fix: migrate kimi legacy dotted skills without ai-skills flag --- src/specify_cli/__init__.py | 21 ++++++++++++++++----- tests/test_ai_skills.py | 26 ++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index f5fe79d23..3cd3e89e0 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -2129,13 +2129,24 @@ def init( ensure_constitution_from_template(project_path, tracker=tracker) + # Determine skills directory and migrate any legacy Kimi dotted skills. + migrated_legacy_kimi_skills = 0 + removed_legacy_kimi_skills = 0 + skills_dir: Optional[Path] = None + if selected_ai in NATIVE_SKILLS_AGENTS: + skills_dir = _get_skills_dir(project_path, selected_ai) + if selected_ai == "kimi" and skills_dir.is_dir(): + ( + migrated_legacy_kimi_skills, + removed_legacy_kimi_skills, + ) = _migrate_legacy_kimi_dotted_skills(skills_dir) + if ai_skills: if selected_ai in NATIVE_SKILLS_AGENTS: - skills_dir = _get_skills_dir(project_path, selected_ai) - migrated_legacy_kimi_skills = 0 - removed_legacy_kimi_skills = 0 - if selected_ai == "kimi": - migrated_legacy_kimi_skills, removed_legacy_kimi_skills = _migrate_legacy_kimi_dotted_skills(skills_dir) + if skills_dir is None: + raise RuntimeError( + f"Could not resolve skills directory for agent: {selected_ai}" + ) bundled_found = _has_bundled_skills(project_path, selected_ai) if bundled_found: detail = f"bundled skills → {skills_dir.relative_to(project_path)}" diff --git a/tests/test_ai_skills.py b/tests/test_ai_skills.py index 1300fac9e..7f3e18f87 100644 --- a/tests/test_ai_skills.py +++ b/tests/test_ai_skills.py @@ -806,6 +806,32 @@ def fake_download(project_path, *args, **kwargs): mock_skills.assert_called_once() assert mock_skills.call_args.kwargs.get("overwrite_existing") is True + def test_kimi_legacy_migration_runs_without_ai_skills_flag(self, tmp_path): + """Kimi init should migrate dotted legacy skills even when --ai-skills is not set.""" + from typer.testing import CliRunner + + runner = CliRunner() + target = tmp_path / "kimi-legacy-no-ai-skills" + + def fake_download(project_path, *args, **kwargs): + legacy_dir = project_path / ".kimi" / "skills" / "speckit.plan" + legacy_dir.mkdir(parents=True, exist_ok=True) + (legacy_dir / "SKILL.md").write_text("---\nname: speckit.plan\n---\n\nlegacy\n") + + with patch("specify_cli.download_and_extract_template", side_effect=fake_download), \ + patch("specify_cli.ensure_executable_scripts"), \ + patch("specify_cli.ensure_constitution_from_template"), \ + patch("specify_cli.is_git_repo", return_value=False), \ + patch("specify_cli.shutil.which", return_value="/usr/bin/kimi"): + result = runner.invoke( + app, + ["init", str(target), "--ai", "kimi", "--script", "sh", "--no-git"], + ) + + assert result.exit_code == 0 + assert not (target / ".kimi" / "skills" / "speckit.plan").exists() + assert (target / ".kimi" / "skills" / "speckit-plan" / "SKILL.md").exists() + def test_codex_ai_skills_here_mode_preserves_existing_codex_dir(self, tmp_path, monkeypatch): """Codex --here skills init should not delete a pre-existing .codex directory.""" from typer.testing import CliRunner From 316a221b8b926e16bc66b063fe056ed18f340665 Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Wed, 25 Mar 2026 22:09:23 +0800 Subject: [PATCH 04/23] fix: harden kimi migration and cache hook init options --- src/specify_cli/__init__.py | 35 +++++++++++++++++++---------------- src/specify_cli/extensions.py | 21 ++++++++++++--------- tests/test_ai_skills.py | 30 +++++++++++++++++++----------- tests/test_extensions.py | 15 +++++++++++++++ 4 files changed, 65 insertions(+), 36 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 3cd3e89e0..bb0bcf3dc 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1490,10 +1490,6 @@ def load_init_options(project_path: Path) -> dict[str, Any]: return {} -# Agent-specific skill directory overrides for agents whose skills directory -# doesn't follow the standard /skills/ pattern -AGENT_SKILLS_DIR_OVERRIDES = {} - # Default skills directory for agents not in AGENT_CONFIG DEFAULT_SKILLS_DIR = ".agents/skills" @@ -1526,13 +1522,9 @@ def load_init_options(project_path: Path) -> dict[str, Any]: def _get_skills_dir(project_path: Path, selected_ai: str) -> Path: """Resolve the agent-specific skills directory for the given AI assistant. - Uses ``AGENT_SKILLS_DIR_OVERRIDES`` first, then falls back to - ``AGENT_CONFIG[agent]["folder"] + "skills"``, and finally to - ``DEFAULT_SKILLS_DIR``. + Uses ``AGENT_CONFIG[agent]["folder"] + "skills"`` and falls back to + ``DEFAULT_SKILLS_DIR`` for unknown agents. """ - if selected_ai in AGENT_SKILLS_DIR_OVERRIDES: - return project_path / AGENT_SKILLS_DIR_OVERRIDES[selected_ai] - agent_config = AGENT_CONFIG.get(selected_ai, {}) agent_folder = agent_config.get("folder", "") if agent_folder: @@ -1646,7 +1638,7 @@ def install_ai_skills( command_name = command_name[len("speckit."):] if command_name.endswith(".agent"): command_name = command_name[:-len(".agent")] - skill_name = f"speckit-{command_name}" + skill_name = f"speckit-{command_name.replace('.', '-')}" # Create skill directory (additive — never removes existing content) skill_dir = skills_dir / skill_name @@ -1739,7 +1731,7 @@ def _migrate_legacy_kimi_dotted_skills(skills_dir: Path) -> tuple[int, int]: Returns: Tuple[migrated_count, removed_count] - migrated_count: old dotted dir renamed to hyphenated dir - - removed_count: old dotted dir deleted because hyphenated dir already existed + - removed_count: old dotted dir deleted when equivalent hyphenated dir existed """ if not skills_dir.is_dir(): return (0, 0) @@ -1759,12 +1751,23 @@ def _migrate_legacy_kimi_dotted_skills(skills_dir: Path) -> tuple[int, int]: target_dir = skills_dir / f"speckit-{suffix.replace('.', '-')}" - if target_dir.exists(): - shutil.rmtree(legacy_dir) - removed_count += 1 - else: + if not target_dir.exists(): shutil.move(str(legacy_dir), str(target_dir)) migrated_count += 1 + continue + + # If the new target already exists, avoid destructive cleanup unless + # both SKILL.md files are byte-identical. + target_skill = target_dir / "SKILL.md" + legacy_skill = legacy_dir / "SKILL.md" + if target_skill.is_file(): + try: + if target_skill.read_bytes() == legacy_skill.read_bytes(): + shutil.rmtree(legacy_dir) + removed_count += 1 + except OSError: + # Best-effort migration: preserve legacy dir on read failures. + pass return (migrated_count, removed_count) diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index 3f97c9955..f57b79ca8 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -1942,17 +1942,20 @@ def __init__(self, project_root: Path): self.project_root = project_root self.extensions_dir = project_root / ".specify" / "extensions" self.config_file = project_root / ".specify" / "extensions.yml" + self._init_options_cache: Optional[Dict[str, Any]] = None def _load_init_options(self) -> Dict[str, Any]: - """Load persisted init options used to determine invocation style.""" - options_file = self.project_root / self.INIT_OPTIONS_FILE - if not options_file.exists(): - return {} - try: - payload = json.loads(options_file.read_text(encoding="utf-8")) - return payload if isinstance(payload, dict) else {} - except (json.JSONDecodeError, OSError, UnicodeError): - return {} + """Load persisted init options used to determine invocation style. + + Uses the shared helper from specify_cli and caches values per executor + instance to avoid repeated filesystem reads during hook rendering. + """ + if self._init_options_cache is None: + from . import load_init_options + + payload = load_init_options(self.project_root) + self._init_options_cache = payload if isinstance(payload, dict) else {} + return self._init_options_cache @staticmethod def _skill_name_from_command(command: str) -> str: diff --git a/tests/test_ai_skills.py b/tests/test_ai_skills.py index 7f3e18f87..64cca9551 100644 --- a/tests/test_ai_skills.py +++ b/tests/test_ai_skills.py @@ -26,7 +26,6 @@ _get_skills_dir, _migrate_legacy_kimi_dotted_skills, install_ai_skills, - AGENT_SKILLS_DIR_OVERRIDES, DEFAULT_SKILLS_DIR, SKILL_DESCRIPTIONS, AGENT_CONFIG, @@ -204,14 +203,6 @@ def test_all_configured_agents_resolve(self, project_dir): # Should always end with "skills" assert result.name == "skills" - def test_override_takes_precedence_over_config(self, project_dir): - """AGENT_SKILLS_DIR_OVERRIDES should take precedence over AGENT_CONFIG.""" - for agent_key in AGENT_SKILLS_DIR_OVERRIDES: - result = _get_skills_dir(project_dir, agent_key) - expected = project_dir / AGENT_SKILLS_DIR_OVERRIDES[agent_key] - assert result == expected - - class TestKimiLegacySkillMigration: """Test temporary migration from Kimi dotted skill names to hyphenated names.""" @@ -228,20 +219,37 @@ def test_migrates_legacy_dotted_skill_directory(self, project_dir): assert not legacy_dir.exists() assert (skills_dir / "speckit-plan" / "SKILL.md").exists() - def test_removes_legacy_dir_when_hyphenated_target_exists(self, project_dir): + def test_removes_legacy_dir_when_hyphenated_target_exists_with_same_content(self, project_dir): skills_dir = project_dir / ".kimi" / "skills" legacy_dir = skills_dir / "speckit.plan" legacy_dir.mkdir(parents=True) (legacy_dir / "SKILL.md").write_text("legacy") target_dir = skills_dir / "speckit-plan" target_dir.mkdir(parents=True) - (target_dir / "SKILL.md").write_text("new") + (target_dir / "SKILL.md").write_text("legacy") migrated, removed = _migrate_legacy_kimi_dotted_skills(skills_dir) assert migrated == 0 assert removed == 1 assert not legacy_dir.exists() + assert (target_dir / "SKILL.md").read_text() == "legacy" + + def test_keeps_legacy_dir_when_hyphenated_target_differs(self, project_dir): + skills_dir = project_dir / ".kimi" / "skills" + legacy_dir = skills_dir / "speckit.plan" + legacy_dir.mkdir(parents=True) + (legacy_dir / "SKILL.md").write_text("legacy") + target_dir = skills_dir / "speckit-plan" + target_dir.mkdir(parents=True) + (target_dir / "SKILL.md").write_text("new") + + migrated, removed = _migrate_legacy_kimi_dotted_skills(skills_dir) + + assert migrated == 0 + assert removed == 0 + assert legacy_dir.exists() + assert (legacy_dir / "SKILL.md").read_text() == "legacy" assert (target_dir / "SKILL.md").read_text() == "new" diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 2a297b284..90859ae49 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -3320,3 +3320,18 @@ def test_extension_command_uses_hyphenated_skill_invocation(self, project_dir): assert "Executing: `/skill:speckit-test-hello`" in message assert "EXECUTE_COMMAND: speckit.test.hello" in message assert "EXECUTE_COMMAND_INVOCATION: /skill:speckit-test-hello" in message + + def test_hook_executor_caches_init_options_lookup(self, project_dir, monkeypatch): + """Init options should be loaded once per executor instance.""" + calls = {"count": 0} + + def fake_load_init_options(_project_root): + calls["count"] += 1 + return {"ai": "kimi", "ai_skills": False} + + monkeypatch.setattr("specify_cli.load_init_options", fake_load_init_options) + + hook_executor = HookExecutor(project_dir) + assert hook_executor._render_hook_invocation("speckit.plan") == "/skill:speckit-plan" + assert hook_executor._render_hook_invocation("speckit.tasks") == "/skill:speckit-tasks" + assert calls["count"] == 1 From f858a0d7831400ff2ea02d59b490e374a58ceac0 Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Wed, 25 Mar 2026 22:25:19 +0800 Subject: [PATCH 05/23] fix: apply kimi preset skill overrides without ai-skills flag --- src/specify_cli/extensions.py | 2 -- src/specify_cli/presets.py | 15 ++++++++++----- tests/test_presets.py | 21 +++++++++++++++++++++ 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index f57b79ca8..1ce6a3247 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -1931,8 +1931,6 @@ def has_value(self, key_path: str) -> bool: class HookExecutor: """Manages extension hook execution.""" - INIT_OPTIONS_FILE = ".specify/init-options.json" - def __init__(self, project_root: Path): """Initialize hook executor. diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 008bba92a..d6641686a 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -556,26 +556,31 @@ def _unregister_commands(self, registered_commands: Dict[str, List[str]]) -> Non registrar.unregister_commands(registered_commands, self.project_root) def _get_skills_dir(self) -> Optional[Path]: - """Return the skills directory if ``--ai-skills`` was used during init. + """Return the active skills directory for preset skill overrides. Reads ``.specify/init-options.json`` to determine whether skills are enabled and which agent was selected, then delegates to the module-level ``_get_skills_dir()`` helper for the concrete path. + Kimi is treated as a native-skills agent: if ``ai == "kimi"`` and + ``.kimi/skills`` exists, presets should still propagate command + overrides to skills even when ``ai_skills`` is false. + Returns: The skills directory ``Path``, or ``None`` if skills were not - enabled or the init-options file is missing. + enabled and no native-skills fallback applies. """ from . import load_init_options, _get_skills_dir opts = load_init_options(self.project_root) - if not opts.get("ai_skills"): - return None - agent = opts.get("ai") if not agent: return None + ai_skills_enabled = bool(opts.get("ai_skills")) + if not ai_skills_enabled and agent != "kimi": + return None + skills_dir = _get_skills_dir(self.project_root, agent) if not skills_dir.is_dir(): return None diff --git a/tests/test_presets.py b/tests/test_presets.py index 99575daf1..3c853eb8e 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -2122,6 +2122,27 @@ def test_kimi_legacy_dotted_skill_override_still_applies(self, project_dir, temp metadata = manager.registry.get("self-test") assert "speckit.specify" in metadata.get("registered_skills", []) + def test_kimi_skill_updated_even_when_ai_skills_disabled(self, project_dir, temp_dir): + """Kimi presets should still propagate command overrides to existing skills.""" + self._write_init_options(project_dir, ai="kimi", ai_skills=False) + skills_dir = project_dir / ".kimi" / "skills" + self._create_skill(skills_dir, "speckit-specify", body="untouched") + + (project_dir / ".kimi" / "commands").mkdir(parents=True, exist_ok=True) + + manager = PresetManager(project_dir) + self_test_dir = Path(__file__).parent.parent / "presets" / "self-test" + manager.install_from_directory(self_test_dir, "0.1.5") + + skill_file = skills_dir / "speckit-specify" / "SKILL.md" + assert skill_file.exists() + content = skill_file.read_text() + assert "preset:self-test" in content + assert "name: speckit-specify" in content + + metadata = manager.registry.get("self-test") + assert "speckit-specify" in metadata.get("registered_skills", []) + class TestPresetSetPriority: """Test preset set-priority CLI command.""" From 83323e3feccda7d20d7e5edb855bed58c8c179ed Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Wed, 25 Mar 2026 22:35:15 +0800 Subject: [PATCH 06/23] fix: keep sequential branch numbering beyond 999 --- scripts/bash/create-new-feature.sh | 12 ++++++------ tests/test_timestamp_branches.py | 12 ++++++++++++ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/scripts/bash/create-new-feature.sh b/scripts/bash/create-new-feature.sh index 579d34752..a393edd32 100644 --- a/scripts/bash/create-new-feature.sh +++ b/scripts/bash/create-new-feature.sh @@ -89,9 +89,9 @@ get_highest_from_specs() { for dir in "$specs_dir"/*; do [ -d "$dir" ] || continue dirname=$(basename "$dir") - # Only match sequential prefixes (###-*), skip timestamp dirs - if echo "$dirname" | grep -q '^[0-9]\{3\}-'; then - number=$(echo "$dirname" | grep -o '^[0-9]\{3\}') + # Match sequential prefixes (>=3 digits), but skip timestamp dirs. + if echo "$dirname" | grep -Eq '^[0-9]{3,}-' && ! echo "$dirname" | grep -Eq '^[0-9]{8}-[0-9]{6}-'; then + number=$(echo "$dirname" | grep -Eo '^[0-9]+') number=$((10#$number)) if [ "$number" -gt "$highest" ]; then highest=$number @@ -115,9 +115,9 @@ get_highest_from_branches() { # Clean branch name: remove leading markers and remote prefixes clean_branch=$(echo "$branch" | sed 's/^[* ]*//; s|^remotes/[^/]*/||') - # Extract feature number if branch matches pattern ###-* - if echo "$clean_branch" | grep -q '^[0-9]\{3\}-'; then - number=$(echo "$clean_branch" | grep -o '^[0-9]\{3\}' || echo "0") + # Extract sequential feature number (>=3 digits), skip timestamp branches. + if echo "$clean_branch" | grep -Eq '^[0-9]{3,}-' && ! echo "$clean_branch" | grep -Eq '^[0-9]{8}-[0-9]{6}-'; then + number=$(echo "$clean_branch" | grep -Eo '^[0-9]+' || echo "0") number=$((10#$number)) if [ "$number" -gt "$highest" ]; then highest=$number diff --git a/tests/test_timestamp_branches.py b/tests/test_timestamp_branches.py index 0cf631e96..c6f881035 100644 --- a/tests/test_timestamp_branches.py +++ b/tests/test_timestamp_branches.py @@ -147,6 +147,18 @@ def test_sequential_ignores_timestamp_dirs(self, git_repo: Path): branch = line.split(":", 1)[1].strip() assert branch == "003-next-feat", f"expected 003-next-feat, got: {branch}" + def test_sequential_supports_four_digit_prefixes(self, git_repo: Path): + """Sequential numbering should continue past 999 without truncation.""" + (git_repo / "specs" / "999-last-3digit").mkdir(parents=True) + (git_repo / "specs" / "1000-first-4digit").mkdir(parents=True) + result = run_script(git_repo, "--short-name", "next-feat", "Next feature") + assert result.returncode == 0, result.stderr + branch = None + for line in result.stdout.splitlines(): + if line.startswith("BRANCH_NAME:"): + branch = line.split(":", 1)[1].strip() + assert branch == "1001-next-feat", f"expected 1001-next-feat, got: {branch}" + # ── check_feature_branch Tests ─────────────────────────────────────────────── From 65e922a2b1a63ed0eaaee9a4d3d86f4dcda9e319 Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Wed, 25 Mar 2026 22:39:53 +0800 Subject: [PATCH 07/23] test: align kimi scaffold skill path with hyphen naming --- tests/test_core_pack_scaffold.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_core_pack_scaffold.py b/tests/test_core_pack_scaffold.py index 92848bb16..92b747a29 100644 --- a/tests/test_core_pack_scaffold.py +++ b/tests/test_core_pack_scaffold.py @@ -142,7 +142,7 @@ def _expected_cmd_dir(project_path: Path, agent: str) -> Path: # Agents whose commands are laid out as //SKILL.md. # Maps agent -> separator used in skill directory names. -_SKILL_AGENTS: dict[str, str] = {"codex": "-", "kimi": "."} +_SKILL_AGENTS: dict[str, str] = {"codex": "-", "kimi": "-"} def _expected_ext(agent: str) -> str: From c161c61b095a45c246e96407d63d991ceb9018ac Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Wed, 25 Mar 2026 22:43:26 +0800 Subject: [PATCH 08/23] chore: align hook typing and preset skill comment --- src/specify_cli/extensions.py | 4 ++-- src/specify_cli/presets.py | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index 1ce6a3247..dd48b8ec5 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -1956,7 +1956,7 @@ def _load_init_options(self) -> Dict[str, Any]: return self._init_options_cache @staticmethod - def _skill_name_from_command(command: str) -> str: + def _skill_name_from_command(command: Any) -> str: """Map a command id like speckit.plan to speckit-plan skill name.""" if not isinstance(command, str): return "" @@ -1965,7 +1965,7 @@ def _skill_name_from_command(command: str) -> str: return "" return f"speckit-{command_id[len('speckit.'):].replace('.', '-')}" - def _render_hook_invocation(self, command: str) -> str: + def _render_hook_invocation(self, command: Any) -> str: """Render an agent-specific invocation string for a hook command.""" if not isinstance(command, str): return "" diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index d6641686a..3b0f1f180 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -652,7 +652,8 @@ def _register_skills( skill_name = f"speckit-{short_name}" legacy_skill_name = f"speckit.{raw_short_name}" - # Only overwrite existing skills (i.e. --ai-skills was used). + # Only overwrite skills that already exist under skills_dir, + # including Kimi native skills when ai_skills is false. # If both modern and legacy directories exist, update both. target_skill_names: List[str] = [] if (skills_dir / skill_name).exists(): From 4892a21ce9727d01caed58eb3cd82e27c3142509 Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Thu, 26 Mar 2026 08:06:43 +0800 Subject: [PATCH 09/23] fix: restore AGENT_SKILLS_DIR_OVERRIDES compatibility export --- src/specify_cli/__init__.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index bb0bcf3dc..3fa2f5d86 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1493,6 +1493,11 @@ def load_init_options(project_path: Path) -> dict[str, Any]: # Default skills directory for agents not in AGENT_CONFIG DEFAULT_SKILLS_DIR = ".agents/skills" +# Backward-compatible export: kept for downstream tests/tools that still +# import this symbol, even though skills directory resolution now relies on +# AGENT_CONFIG folder + "skills" convention. +AGENT_SKILLS_DIR_OVERRIDES: dict[str, str] = {} + # Agents whose downloaded template already contains skills in the final layout. # # Technical debt note: From aca322643d8e72685c1fa5874e47b22dc57d0cd3 Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Thu, 26 Mar 2026 08:48:58 +0800 Subject: [PATCH 10/23] refactor: remove AGENT_SKILLS_DIR_OVERRIDES and update callers --- src/specify_cli/__init__.py | 5 ----- src/specify_cli/extensions.py | 4 +--- tests/test_extension_skills.py | 15 ++++++--------- 3 files changed, 7 insertions(+), 17 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index 3fa2f5d86..bb0bcf3dc 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1493,11 +1493,6 @@ def load_init_options(project_path: Path) -> dict[str, Any]: # Default skills directory for agents not in AGENT_CONFIG DEFAULT_SKILLS_DIR = ".agents/skills" -# Backward-compatible export: kept for downstream tests/tools that still -# import this symbol, even though skills directory resolution now relies on -# AGENT_CONFIG folder + "skills" convention. -AGENT_SKILLS_DIR_OVERRIDES: dict[str, str] = {} - # Agents whose downloaded template already contains skills in the final layout. # # Technical debt note: diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index dd48b8ec5..b54655beb 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -738,11 +738,9 @@ def _unregister_extension_skills(self, skill_names: List[str], extension_id: str shutil.rmtree(skill_subdir) else: # Fallback: scan all possible agent skills directories - from . import AGENT_CONFIG, AGENT_SKILLS_DIR_OVERRIDES, DEFAULT_SKILLS_DIR + from . import AGENT_CONFIG, DEFAULT_SKILLS_DIR candidate_dirs: set[Path] = set() - for override_path in AGENT_SKILLS_DIR_OVERRIDES.values(): - candidate_dirs.add(self.project_root / override_path) for cfg in AGENT_CONFIG.values(): folder = cfg.get("folder", "") if folder: diff --git a/tests/test_extension_skills.py b/tests/test_extension_skills.py index b8d5202f5..f8b89bcb5 100644 --- a/tests/test_extension_skills.py +++ b/tests/test_extension_skills.py @@ -41,17 +41,14 @@ def _create_init_options(project_root: Path, ai: str = "claude", ai_skills: bool def _create_skills_dir(project_root: Path, ai: str = "claude") -> Path: """Create and return the expected skills directory for the given agent.""" # Match the logic in _get_skills_dir() from specify_cli - from specify_cli import AGENT_CONFIG, AGENT_SKILLS_DIR_OVERRIDES, DEFAULT_SKILLS_DIR + from specify_cli import AGENT_CONFIG, DEFAULT_SKILLS_DIR - if ai in AGENT_SKILLS_DIR_OVERRIDES: - skills_dir = project_root / AGENT_SKILLS_DIR_OVERRIDES[ai] + agent_config = AGENT_CONFIG.get(ai, {}) + agent_folder = agent_config.get("folder", "") + if agent_folder: + skills_dir = project_root / agent_folder.rstrip("/") / "skills" else: - agent_config = AGENT_CONFIG.get(ai, {}) - agent_folder = agent_config.get("folder", "") - if agent_folder: - skills_dir = project_root / agent_folder.rstrip("/") / "skills" - else: - skills_dir = project_root / DEFAULT_SKILLS_DIR + skills_dir = project_root / DEFAULT_SKILLS_DIR skills_dir.mkdir(parents=True, exist_ok=True) return skills_dir From 1a8374bdd68f8120eb592f6d1f9ac4891dbea01b Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Thu, 26 Mar 2026 08:59:41 +0800 Subject: [PATCH 11/23] fix(ps1): support sequential branch numbers above 999 --- scripts/powershell/create-new-feature.ps1 | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/powershell/create-new-feature.ps1 b/scripts/powershell/create-new-feature.ps1 index 9adae131d..e48bd0769 100644 --- a/scripts/powershell/create-new-feature.ps1 +++ b/scripts/powershell/create-new-feature.ps1 @@ -51,7 +51,8 @@ function Get-HighestNumberFromSpecs { $highest = 0 if (Test-Path $SpecsDir) { Get-ChildItem -Path $SpecsDir -Directory | ForEach-Object { - if ($_.Name -match '^(\d{3})-') { + # Match sequential prefixes (>=3 digits), but skip timestamp dirs. + if ($_.Name -match '^(\d{3,})-' -and $_.Name -notmatch '^\d{8}-\d{6}-') { $num = [int]$matches[1] if ($num -gt $highest) { $highest = $num } } @@ -71,8 +72,8 @@ function Get-HighestNumberFromBranches { # Clean branch name: remove leading markers and remote prefixes $cleanBranch = $branch.Trim() -replace '^\*?\s+', '' -replace '^remotes/[^/]+/', '' - # Extract feature number if branch matches pattern ###-* - if ($cleanBranch -match '^(\d{3})-') { + # Extract sequential feature number (>=3 digits), skip timestamp branches. + if ($cleanBranch -match '^(\d{3,})-' -and $cleanBranch -notmatch '^\d{8}-\d{6}-') { $num = [int]$matches[1] if ($num -gt $highest) { $highest = $num } } @@ -290,4 +291,3 @@ if ($Json) { Write-Output "HAS_GIT: $hasGit" Write-Output "SPECIFY_FEATURE environment variable set to: $branchName" } - From 9609fc3b63938092742b3c5a143a67ecc1dfb6a0 Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Thu, 26 Mar 2026 09:08:18 +0800 Subject: [PATCH 12/23] fix: resolve preset skill placeholders for skills agents --- src/specify_cli/agents.py | 15 ++++------- src/specify_cli/presets.py | 28 ++++++++++---------- tests/test_presets.py | 53 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 70 insertions(+), 26 deletions(-) diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index 743f8193d..590ff3961 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -308,8 +308,8 @@ def render_skill_command( if not isinstance(frontmatter, dict): frontmatter = {} - if agent_name == "codex": - body = self._resolve_codex_skill_placeholders(frontmatter, body, project_root) + if agent_name in {"codex", "kimi"}: + body = self.resolve_skill_placeholders(agent_name, frontmatter, body, project_root) description = frontmatter.get("description", f"Spec-kit workflow command: {skill_name}") skill_frontmatter = { @@ -324,13 +324,8 @@ def render_skill_command( return self.render_frontmatter(skill_frontmatter) + "\n" + body @staticmethod - def _resolve_codex_skill_placeholders(frontmatter: dict, body: str, project_root: Path) -> str: - """Resolve script placeholders for Codex skill overrides. - - This intentionally scopes the fix to Codex, which is the newly - migrated runtime path in this PR. Existing Kimi behavior is left - unchanged for now. - """ + def resolve_skill_placeholders(agent_name: str, frontmatter: dict, body: str, project_root: Path) -> str: + """Resolve script placeholders for skills-backed agents.""" try: from . import load_init_options except ImportError: @@ -376,7 +371,7 @@ def _resolve_codex_skill_placeholders(frontmatter: dict, body: str, project_root agent_script_command = agent_script_command.replace("{ARGS}", "$ARGUMENTS") body = body.replace("{AGENT_SCRIPT}", agent_script_command) - return body.replace("{ARGS}", "$ARGUMENTS").replace("__AGENT__", "codex") + return body.replace("{ARGS}", "$ARGUMENTS").replace("__AGENT__", agent_name) def _convert_argument_placeholder(self, content: str, from_placeholder: str, to_placeholder: str) -> str: """Convert argument placeholder format. diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 3b0f1f180..8319ecdc0 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -633,7 +633,14 @@ def _register_skills( if not skills_dir: return [] - from . import SKILL_DESCRIPTIONS + from . import SKILL_DESCRIPTIONS, load_init_options + from .agents import CommandRegistrar + + init_opts = load_init_options(self.project_root) + selected_ai = init_opts.get("ai") + if not isinstance(selected_ai, str): + return [] + registrar = CommandRegistrar() written: List[str] = [] @@ -665,25 +672,18 @@ def _register_skills( # Parse the command file content = source_file.read_text(encoding="utf-8") - if content.startswith("---"): - parts = content.split("---", 2) - if len(parts) >= 3: - frontmatter = yaml.safe_load(parts[1]) - if not isinstance(frontmatter, dict): - frontmatter = {} - body = parts[2].strip() - else: - frontmatter = {} - body = content - else: - frontmatter = {} - body = content + frontmatter, body = registrar.parse_frontmatter(content) original_desc = frontmatter.get("description", "") enhanced_desc = SKILL_DESCRIPTIONS.get( short_name, original_desc or f"Spec-kit workflow command: {short_name}", ) + frontmatter = dict(frontmatter) + frontmatter["description"] = enhanced_desc + body = registrar.resolve_skill_placeholders( + selected_ai, frontmatter, body, self.project_root + ) for target_skill_name in target_skill_names: frontmatter_data = { diff --git a/tests/test_presets.py b/tests/test_presets.py index 3c853eb8e..9623eebea 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -1942,10 +1942,10 @@ def test_load_returns_empty_on_invalid_json(self, project_dir): class TestPresetSkills: """Tests for preset skill registration and unregistration.""" - def _write_init_options(self, project_dir, ai="claude", ai_skills=True): + def _write_init_options(self, project_dir, ai="claude", ai_skills=True, script="sh"): from specify_cli import save_init_options - save_init_options(project_dir, {"ai": ai, "ai_skills": ai_skills}) + save_init_options(project_dir, {"ai": ai, "ai_skills": ai_skills, "script": script}) def _create_skill(self, skills_dir, skill_name, body="original body"): skill_dir = skills_dir / skill_name @@ -2143,6 +2143,55 @@ def test_kimi_skill_updated_even_when_ai_skills_disabled(self, project_dir, temp metadata = manager.registry.get("self-test") assert "speckit-specify" in metadata.get("registered_skills", []) + def test_kimi_preset_skill_override_resolves_script_placeholders(self, project_dir, temp_dir): + """Kimi preset skill overrides should resolve {SCRIPT} and __AGENT__ placeholders.""" + self._write_init_options(project_dir, ai="kimi", ai_skills=False, script="sh") + skills_dir = project_dir / ".kimi" / "skills" + self._create_skill(skills_dir, "speckit-specify", body="untouched") + (project_dir / ".kimi" / "commands").mkdir(parents=True, exist_ok=True) + + preset_dir = temp_dir / "kimi-placeholder-override" + preset_dir.mkdir() + (preset_dir / "commands").mkdir() + (preset_dir / "commands" / "speckit.specify.md").write_text( + "---\n" + "description: Kimi placeholder override\n" + "scripts:\n" + " sh: scripts/bash/create-new-feature.sh --json \"{ARGS}\"\n" + "---\n\n" + "Execute `{SCRIPT}` for __AGENT__\n" + ) + manifest_data = { + "schema_version": "1.0", + "preset": { + "id": "kimi-placeholder-override", + "name": "Kimi Placeholder Override", + "version": "1.0.0", + "description": "Test", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "templates": [ + { + "type": "command", + "name": "speckit.specify", + "file": "commands/speckit.specify.md", + } + ] + }, + } + with open(preset_dir / "preset.yml", "w") as f: + yaml.dump(manifest_data, f) + + manager = PresetManager(project_dir) + manager.install_from_directory(preset_dir, "0.1.5") + + content = (skills_dir / "speckit-specify" / "SKILL.md").read_text() + assert "{SCRIPT}" not in content + assert "__AGENT__" not in content + assert "scripts/bash/create-new-feature.sh --json \"$ARGUMENTS\"" in content + assert "for kimi" in content + class TestPresetSetPriority: """Test preset set-priority CLI command.""" From e4e550355feea162f32894cb717414c328d9114b Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Thu, 26 Mar 2026 09:19:12 +0800 Subject: [PATCH 13/23] Fix legacy kimi migration safety and preset skill dir checks --- src/specify_cli/__init__.py | 9 +++++++-- src/specify_cli/presets.py | 4 ++-- tests/test_ai_skills.py | 17 +++++++++++++++++ tests/test_presets.py | 17 +++++++++++++++++ 4 files changed, 43 insertions(+), 4 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index bb0bcf3dc..ad7300a38 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1763,8 +1763,13 @@ def _migrate_legacy_kimi_dotted_skills(skills_dir: Path) -> tuple[int, int]: if target_skill.is_file(): try: if target_skill.read_bytes() == legacy_skill.read_bytes(): - shutil.rmtree(legacy_dir) - removed_count += 1 + # Preserve legacy directory when it contains extra user files. + has_extra_entries = any( + child.name != "SKILL.md" for child in legacy_dir.iterdir() + ) + if not has_extra_entries: + shutil.rmtree(legacy_dir) + removed_count += 1 except OSError: # Best-effort migration: preserve legacy dir on read failures. pass diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 8319ecdc0..b10a64a3c 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -663,9 +663,9 @@ def _register_skills( # including Kimi native skills when ai_skills is false. # If both modern and legacy directories exist, update both. target_skill_names: List[str] = [] - if (skills_dir / skill_name).exists(): + if (skills_dir / skill_name).is_dir(): target_skill_names.append(skill_name) - if legacy_skill_name != skill_name and (skills_dir / legacy_skill_name).exists(): + if legacy_skill_name != skill_name and (skills_dir / legacy_skill_name).is_dir(): target_skill_names.append(legacy_skill_name) if not target_skill_names: continue diff --git a/tests/test_ai_skills.py b/tests/test_ai_skills.py index 64cca9551..f0e220e26 100644 --- a/tests/test_ai_skills.py +++ b/tests/test_ai_skills.py @@ -252,6 +252,23 @@ def test_keeps_legacy_dir_when_hyphenated_target_differs(self, project_dir): assert (legacy_dir / "SKILL.md").read_text() == "legacy" assert (target_dir / "SKILL.md").read_text() == "new" + def test_keeps_legacy_dir_when_matching_target_but_extra_files_exist(self, project_dir): + skills_dir = project_dir / ".kimi" / "skills" + legacy_dir = skills_dir / "speckit.plan" + legacy_dir.mkdir(parents=True) + (legacy_dir / "SKILL.md").write_text("legacy") + (legacy_dir / "notes.txt").write_text("custom") + target_dir = skills_dir / "speckit-plan" + target_dir.mkdir(parents=True) + (target_dir / "SKILL.md").write_text("legacy") + + migrated, removed = _migrate_legacy_kimi_dotted_skills(skills_dir) + + assert migrated == 0 + assert removed == 0 + assert legacy_dir.exists() + assert (legacy_dir / "notes.txt").read_text() == "custom" + # ===== install_ai_skills Tests ===== diff --git a/tests/test_presets.py b/tests/test_presets.py index 9623eebea..6636e3ff1 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -2040,6 +2040,23 @@ def test_skill_restored_on_preset_remove(self, project_dir, temp_dir): assert "preset:self-test" not in content, "Preset content should be gone" assert "templates/commands/specify.md" in content, "Should reference core template" + def test_skill_not_overridden_when_skill_path_is_file(self, project_dir): + """Preset install should skip non-directory skill targets.""" + self._write_init_options(project_dir, ai="claude") + skills_dir = project_dir / ".claude" / "skills" + skills_dir.mkdir(parents=True, exist_ok=True) + (skills_dir / "speckit-specify").write_text("not-a-directory") + + (project_dir / ".claude" / "commands").mkdir(parents=True, exist_ok=True) + + manager = PresetManager(project_dir) + SELF_TEST_DIR = Path(__file__).parent.parent / "presets" / "self-test" + manager.install_from_directory(SELF_TEST_DIR, "0.1.5") + + assert (skills_dir / "speckit-specify").is_file() + metadata = manager.registry.get("self-test") + assert "speckit-specify" not in metadata.get("registered_skills", []) + def test_no_skills_registered_when_no_skill_dir_exists(self, project_dir, temp_dir): """Skills should not be created when no existing skill dir is found.""" self._write_init_options(project_dir, ai="claude") From dd0de3f4f47e19a751fbb34dd3cb3517c4c6f074 Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Thu, 26 Mar 2026 12:21:10 +0800 Subject: [PATCH 14/23] Harden TOML rendering and consolidate preset skill restore parsing --- src/specify_cli/agents.py | 25 ++++++++++++++++++--- src/specify_cli/presets.py | 24 +++++++++----------- tests/test_extensions.py | 45 ++++++++++++++++++++++++++++++++++++++ tests/test_presets.py | 29 ++++++++++++++++++++++++ 4 files changed, 106 insertions(+), 17 deletions(-) diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index 590ff3961..ac37d5197 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -10,6 +10,7 @@ from typing import Dict, List, Any import platform +from copy import deepcopy import yaml @@ -219,6 +220,8 @@ def _adjust_script_paths(self, frontmatter: dict) -> dict: Returns: Modified frontmatter with adjusted paths """ + frontmatter = deepcopy(frontmatter) + for script_key in ("scripts", "agent_scripts"): scripts = frontmatter.get(script_key) if not isinstance(scripts, dict): @@ -277,9 +280,25 @@ def render_toml_command( toml_lines.append(f"# Source: {source_id}") toml_lines.append("") - toml_lines.append('prompt = """') - toml_lines.append(body) - toml_lines.append('"""') + # Keep TOML output valid even when body contains triple-quote delimiters. + # Prefer multiline forms, then fall back to escaped basic string. + if '"""' not in body: + toml_lines.append('prompt = """') + toml_lines.append(body) + toml_lines.append('"""') + elif "'''" not in body: + toml_lines.append("prompt = '''") + toml_lines.append(body) + toml_lines.append("'''") + else: + escaped_body = ( + body.replace("\\", "\\\\") + .replace('"', '\\"') + .replace("\n", "\\n") + .replace("\r", "\\r") + .replace("\t", "\\t") + ) + toml_lines.append(f'prompt = "{escaped_body}"') return "\n".join(toml_lines) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index b10a64a3c..a1e383215 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -728,10 +728,14 @@ def _unregister_skills(self, skill_names: List[str], preset_dir: Path) -> None: if not skills_dir: return - from . import SKILL_DESCRIPTIONS + from . import SKILL_DESCRIPTIONS, load_init_options + from .agents import CommandRegistrar # Locate core command templates from the project's installed templates core_templates_dir = self.project_root / ".specify" / "templates" / "commands" + init_opts = load_init_options(self.project_root) + selected_ai = init_opts.get("ai") + registrar = CommandRegistrar() for skill_name in skill_names: # Derive command name from skill name (speckit-specify -> specify) @@ -754,19 +758,11 @@ def _unregister_skills(self, skill_names: List[str], preset_dir: Path) -> None: if core_file: # Restore from core template content = core_file.read_text(encoding="utf-8") - if content.startswith("---"): - parts = content.split("---", 2) - if len(parts) >= 3: - frontmatter = yaml.safe_load(parts[1]) - if not isinstance(frontmatter, dict): - frontmatter = {} - body = parts[2].strip() - else: - frontmatter = {} - body = content - else: - frontmatter = {} - body = content + frontmatter, body = registrar.parse_frontmatter(content) + if isinstance(selected_ai, str): + body = registrar.resolve_skill_placeholders( + selected_ai, frontmatter, body, self.project_root + ) original_desc = frontmatter.get("description", "") enhanced_desc = SKILL_DESCRIPTIONS.get( diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 90859ae49..f8183417a 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -760,6 +760,51 @@ def test_render_frontmatter_unicode(self): assert "Prüfe Konformität" in output assert "\\u" not in output + def test_adjust_script_paths_does_not_mutate_input(self): + """Path adjustments should not mutate caller-owned frontmatter dicts.""" + from specify_cli.agents import CommandRegistrar as AgentCommandRegistrar + registrar = AgentCommandRegistrar() + original = { + "scripts": { + "sh": "../../scripts/bash/setup-plan.sh {ARGS}", + "ps": "../../scripts/powershell/setup-plan.ps1 {ARGS}", + } + } + before = json.loads(json.dumps(original)) + + adjusted = registrar._adjust_script_paths(original) + + assert original == before + assert adjusted["scripts"]["sh"] == ".specify/scripts/bash/setup-plan.sh {ARGS}" + assert adjusted["scripts"]["ps"] == ".specify/scripts/powershell/setup-plan.ps1 {ARGS}" + + def test_render_toml_command_handles_embedded_triple_double_quotes(self): + """TOML renderer should stay valid when body includes triple double-quotes.""" + from specify_cli.agents import CommandRegistrar as AgentCommandRegistrar + registrar = AgentCommandRegistrar() + output = registrar.render_toml_command( + {"description": "x"}, + 'line1\n"""danger"""\nline2', + "extension:test-ext", + ) + + assert "prompt = '''" in output + assert '"""danger"""' in output + + def test_render_toml_command_escapes_when_both_triple_quote_styles_exist(self): + """If body has both triple quote styles, fall back to escaped basic string.""" + from specify_cli.agents import CommandRegistrar as AgentCommandRegistrar + registrar = AgentCommandRegistrar() + output = registrar.render_toml_command( + {"description": "x"}, + 'a """ b\nc \'\'\' d', + "extension:test-ext", + ) + + assert 'prompt = "' in output + assert "\\n" in output + assert "\\\"\\\"\\\"" in output + def test_register_commands_for_claude(self, extension_dir, project_dir): """Test registering commands for Claude agent.""" # Create .claude directory diff --git a/tests/test_presets.py b/tests/test_presets.py index 6636e3ff1..1088312fb 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -2040,6 +2040,35 @@ def test_skill_restored_on_preset_remove(self, project_dir, temp_dir): assert "preset:self-test" not in content, "Preset content should be gone" assert "templates/commands/specify.md" in content, "Should reference core template" + def test_skill_restored_on_remove_resolves_script_placeholders(self, project_dir): + """Core restore should resolve {SCRIPT}/{ARGS} placeholders like other skill paths.""" + self._write_init_options(project_dir, ai="claude", ai_skills=True, script="sh") + skills_dir = project_dir / ".claude" / "skills" + self._create_skill(skills_dir, "speckit-specify", body="old") + (project_dir / ".claude" / "commands").mkdir(parents=True, exist_ok=True) + + core_cmds = project_dir / ".specify" / "templates" / "commands" + core_cmds.mkdir(parents=True, exist_ok=True) + (core_cmds / "specify.md").write_text( + "---\n" + "description: Core specify command\n" + "scripts:\n" + " sh: .specify/scripts/bash/create-new-feature.sh --json \"{ARGS}\"\n" + "---\n\n" + "Run:\n" + "{SCRIPT}\n" + ) + + manager = PresetManager(project_dir) + SELF_TEST_DIR = Path(__file__).parent.parent / "presets" / "self-test" + manager.install_from_directory(SELF_TEST_DIR, "0.1.5") + manager.remove("self-test") + + content = (skills_dir / "speckit-specify" / "SKILL.md").read_text() + assert "{SCRIPT}" not in content + assert "{ARGS}" not in content + assert ".specify/scripts/bash/create-new-feature.sh --json \"$ARGUMENTS\"" in content + def test_skill_not_overridden_when_skill_path_is_file(self, project_dir): """Preset install should skip non-directory skill targets.""" self._write_init_options(project_dir, ai="claude") From 70c521d4404bd403ab1ede74ac0a4e00a0d6c356 Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Thu, 26 Mar 2026 12:58:21 +0800 Subject: [PATCH 15/23] Fix PowerShell overflow and hook message fallback for empty invocations --- scripts/powershell/create-new-feature.ps1 | 18 +++++++++++------- src/specify_cli/extensions.py | 18 +++++++++--------- tests/test_extensions.py | 22 ++++++++++++++++++++++ tests/test_timestamp_branches.py | 7 +++++++ 4 files changed, 49 insertions(+), 16 deletions(-) diff --git a/scripts/powershell/create-new-feature.ps1 b/scripts/powershell/create-new-feature.ps1 index e48bd0769..b1ca0ac82 100644 --- a/scripts/powershell/create-new-feature.ps1 +++ b/scripts/powershell/create-new-feature.ps1 @@ -5,7 +5,7 @@ param( [switch]$Json, [string]$ShortName, [Parameter()] - [int]$Number = 0, + [long]$Number = 0, [switch]$Timestamp, [switch]$Help, [Parameter(Position = 0, ValueFromRemainingArguments = $true)] @@ -48,13 +48,15 @@ if ([string]::IsNullOrWhiteSpace($featureDesc)) { function Get-HighestNumberFromSpecs { param([string]$SpecsDir) - $highest = 0 + [long]$highest = 0 if (Test-Path $SpecsDir) { Get-ChildItem -Path $SpecsDir -Directory | ForEach-Object { # Match sequential prefixes (>=3 digits), but skip timestamp dirs. if ($_.Name -match '^(\d{3,})-' -and $_.Name -notmatch '^\d{8}-\d{6}-') { - $num = [int]$matches[1] - if ($num -gt $highest) { $highest = $num } + [long]$num = 0 + if ([long]::TryParse($matches[1], [ref]$num) -and $num -gt $highest) { + $highest = $num + } } } } @@ -64,7 +66,7 @@ function Get-HighestNumberFromSpecs { function Get-HighestNumberFromBranches { param() - $highest = 0 + [long]$highest = 0 try { $branches = git branch -a 2>$null if ($LASTEXITCODE -eq 0) { @@ -74,8 +76,10 @@ function Get-HighestNumberFromBranches { # Extract sequential feature number (>=3 digits), skip timestamp branches. if ($cleanBranch -match '^(\d{3,})-' -and $cleanBranch -notmatch '^\d{8}-\d{6}-') { - $num = [int]$matches[1] - if ($num -gt $highest) { $highest = $num } + [long]$num = 0 + if ([long]::TryParse($matches[1], [ref]$num) -and $num -gt $highest) { + $highest = $num + } } } } diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index b54655beb..6abfb5e6a 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -2228,26 +2228,26 @@ def format_hook_message( extension = hook.get("extension") command = hook.get("command") invocation = self._render_hook_invocation(command) + command_text = command if isinstance(command, str) and command.strip() else "" + display_invocation = invocation or ( + f"/{command_text}" if command_text != "" else "/" + ) optional = hook.get("optional", True) prompt = hook.get("prompt", "") description = hook.get("description", "") if optional: lines.append(f"\n**Optional Hook**: {extension}") - if invocation: - lines.append(f"Command: `{invocation}`") + lines.append(f"Command: `{display_invocation}`") if description: lines.append(f"Description: {description}") lines.append(f"\nPrompt: {prompt}") - if invocation: - lines.append(f"To execute: `{invocation}`") + lines.append(f"To execute: `{display_invocation}`") else: lines.append(f"\n**Automatic Hook**: {extension}") - if invocation: - lines.append(f"Executing: `{invocation}`") - lines.append(f"EXECUTE_COMMAND: {command}") - if invocation: - lines.append(f"EXECUTE_COMMAND_INVOCATION: {invocation}") + lines.append(f"Executing: `{display_invocation}`") + lines.append(f"EXECUTE_COMMAND: {command_text}") + lines.append(f"EXECUTE_COMMAND_INVOCATION: {display_invocation}") return "\n".join(lines) diff --git a/tests/test_extensions.py b/tests/test_extensions.py index f8183417a..91130fcdb 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -3380,3 +3380,25 @@ def fake_load_init_options(_project_root): assert hook_executor._render_hook_invocation("speckit.plan") == "/skill:speckit-plan" assert hook_executor._render_hook_invocation("speckit.tasks") == "/skill:speckit-tasks" assert calls["count"] == 1 + + def test_hook_message_falls_back_when_invocation_is_empty(self, project_dir): + """Hook messages should still render actionable command placeholders.""" + init_options = project_dir / ".specify" / "init-options.json" + init_options.parent.mkdir(parents=True, exist_ok=True) + init_options.write_text(json.dumps({"ai": "kimi", "ai_skills": False})) + + hook_executor = HookExecutor(project_dir) + message = hook_executor.format_hook_message( + "after_tasks", + [ + { + "extension": "test-ext", + "command": None, + "optional": False, + } + ], + ) + + assert "Executing: `/`" in message + assert "EXECUTE_COMMAND: " in message + assert "EXECUTE_COMMAND_INVOCATION: /" in message diff --git a/tests/test_timestamp_branches.py b/tests/test_timestamp_branches.py index c6f881035..7e4f88ed0 100644 --- a/tests/test_timestamp_branches.py +++ b/tests/test_timestamp_branches.py @@ -14,6 +14,7 @@ PROJECT_ROOT = Path(__file__).resolve().parent.parent CREATE_FEATURE = PROJECT_ROOT / "scripts" / "bash" / "create-new-feature.sh" +CREATE_FEATURE_PS = PROJECT_ROOT / "scripts" / "powershell" / "create-new-feature.ps1" COMMON_SH = PROJECT_ROOT / "scripts" / "bash" / "common.sh" @@ -159,6 +160,12 @@ def test_sequential_supports_four_digit_prefixes(self, git_repo: Path): branch = line.split(":", 1)[1].strip() assert branch == "1001-next-feat", f"expected 1001-next-feat, got: {branch}" + def test_powershell_scanner_uses_long_tryparse_for_large_prefixes(self): + """PowerShell scanner should parse large prefixes without [int] casts.""" + content = CREATE_FEATURE_PS.read_text(encoding="utf-8") + assert "[long]::TryParse($matches[1], [ref]$num)" in content + assert "$num = [int]$matches[1]" not in content + # ── check_feature_branch Tests ─────────────────────────────────────────────── From 691b4dd2ee08b25736040048f5b2e68b3c0aeda6 Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Thu, 26 Mar 2026 18:37:04 +0800 Subject: [PATCH 16/23] Restore preset skills from extensions --- src/specify_cli/agents.py | 27 ++++++++-- src/specify_cli/presets.py | 104 +++++++++++++++++++++++++++++++++++-- tests/test_presets.py | 89 ++++++++++++++++++++++++++++++- 3 files changed, 211 insertions(+), 9 deletions(-) diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index ac37d5197..bcf9fcecf 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -10,6 +10,7 @@ from typing import Dict, List, Any import platform +import re from copy import deepcopy import yaml @@ -228,10 +229,29 @@ def _adjust_script_paths(self, frontmatter: dict) -> dict: continue for key, script_path in scripts.items(): - if isinstance(script_path, str) and script_path.startswith("../../scripts/"): - scripts[key] = f".specify/scripts/{script_path[14:]}" + if isinstance(script_path, str): + scripts[key] = self._rewrite_project_relative_paths(script_path) return frontmatter + @staticmethod + def _rewrite_project_relative_paths(text: str) -> str: + """Rewrite repo-relative paths to their generated project locations.""" + if not isinstance(text, str) or not text: + return text + + for old, new in ( + ("../../memory/", ".specify/memory/"), + ("../../scripts/", ".specify/scripts/"), + ("../../templates/", ".specify/templates/"), + ): + text = text.replace(old, new) + + text = re.sub(r"(? str: """Convert argument placeholder format. diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index a1e383215..8c478916c 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -587,6 +587,68 @@ def _get_skills_dir(self) -> Optional[Path]: return skills_dir + @staticmethod + def _skill_names_for_command(cmd_name: str) -> List[str]: + """Return the modern and legacy skill directory names for a command.""" + raw_short_name = cmd_name + if raw_short_name.startswith("speckit."): + raw_short_name = raw_short_name[len("speckit."):] + + skill_names = [f"speckit-{raw_short_name.replace('.', '-')}"] + legacy_skill_name = f"speckit.{raw_short_name}" + if legacy_skill_name not in skill_names: + skill_names.append(legacy_skill_name) + + return skill_names + + def _find_extension_skill_restore(self, skill_name: str) -> Optional[Dict[str, Any]]: + """Find the highest-priority installed extension template for a skill.""" + from .extensions import ExtensionManifest, ValidationError + + resolver = PresetResolver(self.project_root) + extensions_dir = self.project_root / ".specify" / "extensions" + + for _priority, ext_id, _metadata in resolver._get_all_extensions_by_priority(): + ext_dir = extensions_dir / ext_id + manifest_path = ext_dir / "extension.yml" + if not manifest_path.is_file(): + continue + + try: + manifest = ExtensionManifest(manifest_path) + except ValidationError: + continue + + ext_root = ext_dir.resolve() + for cmd_info in manifest.commands: + cmd_name = cmd_info.get("name") + cmd_file_rel = cmd_info.get("file") + if not isinstance(cmd_name, str) or not isinstance(cmd_file_rel, str): + continue + if skill_name not in self._skill_names_for_command(cmd_name): + continue + + cmd_path = Path(cmd_file_rel) + if cmd_path.is_absolute(): + continue + + try: + source_file = (ext_root / cmd_path).resolve() + source_file.relative_to(ext_root) + except (OSError, ValueError): + continue + + if not source_file.is_file(): + continue + + return { + "command_name": cmd_name, + "source_file": source_file, + "source": f"extension:{manifest.id}", + } + + return None + def _register_skills( self, manifest: "PresetManifest", @@ -656,8 +718,7 @@ def _register_skills( if raw_short_name.startswith("speckit."): raw_short_name = raw_short_name[len("speckit."):] short_name = raw_short_name.replace(".", "-") - skill_name = f"speckit-{short_name}" - legacy_skill_name = f"speckit.{raw_short_name}" + skill_name, legacy_skill_name = self._skill_names_for_command(cmd_name) # Only overwrite skills that already exist under skills_dir, # including Kimi native skills when ai_skills is false. @@ -747,7 +808,7 @@ def _unregister_skills(self, skill_names: List[str], preset_dir: Path) -> None: skill_subdir = skills_dir / skill_name skill_file = skill_subdir / "SKILL.md" - if not skill_file.exists(): + if not skill_subdir.is_dir(): continue # Try to find the core command template @@ -788,8 +849,43 @@ def _unregister_skills(self, skill_names: List[str], preset_dir: Path) -> None: f"{body}\n" ) skill_file.write_text(skill_content, encoding="utf-8") + continue + + extension_restore = self._find_extension_skill_restore(skill_name) + if extension_restore: + content = extension_restore["source_file"].read_text(encoding="utf-8") + frontmatter, body = registrar.parse_frontmatter(content) + if isinstance(selected_ai, str): + body = registrar.resolve_skill_placeholders( + selected_ai, frontmatter, body, self.project_root + ) + + command_name = extension_restore["command_name"] + title_name = command_name + if title_name.startswith("speckit."): + title_name = title_name[len("speckit."):] + title_name = title_name.replace(".", " ").replace("-", " ").title() + + frontmatter_data = { + "name": skill_name, + "description": frontmatter.get("description", f"Extension command: {command_name}"), + "compatibility": "Requires spec-kit project structure with .specify/ directory", + "metadata": { + "author": "github-spec-kit", + "source": extension_restore["source"], + }, + } + frontmatter_text = yaml.safe_dump(frontmatter_data, sort_keys=False).strip() + skill_content = ( + f"---\n" + f"{frontmatter_text}\n" + f"---\n\n" + f"# {title_name} Skill\n\n" + f"{body}\n" + ) + skill_file.write_text(skill_content, encoding="utf-8") else: - # No core template — remove the skill entirely + # No core or extension template — remove the skill entirely shutil.rmtree(skill_subdir) def install_from_directory( diff --git a/tests/test_presets.py b/tests/test_presets.py index 1088312fb..9ed35cf68 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -2147,6 +2147,88 @@ def test_extension_skill_override_matches_hyphenated_multisegment_name(self, pro metadata = manager.registry.get("ext-skill-override") assert "speckit-fakeext-cmd" in metadata.get("registered_skills", []) + def test_extension_skill_restored_on_preset_remove(self, project_dir, temp_dir): + """Preset removal should restore an extension-backed skill instead of deleting it.""" + self._write_init_options(project_dir, ai="codex") + skills_dir = project_dir / ".agents" / "skills" + self._create_skill(skills_dir, "speckit-fakeext-cmd", body="original extension skill") + + extension_dir = project_dir / ".specify" / "extensions" / "fakeext" + (extension_dir / "commands").mkdir(parents=True, exist_ok=True) + (extension_dir / "commands" / "cmd.md").write_text( + "---\n" + "description: Extension fakeext cmd\n" + "scripts:\n" + " sh: ../../scripts/bash/setup-plan.sh --json \"{ARGS}\"\n" + "---\n\n" + "extension:fakeext\n" + "Run {SCRIPT}\n" + ) + extension_manifest = { + "schema_version": "1.0", + "extension": { + "id": "fakeext", + "name": "Fake Extension", + "version": "1.0.0", + "description": "Test", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "commands": [ + { + "name": "speckit.fakeext.cmd", + "file": "commands/cmd.md", + "description": "Fake extension command", + } + ] + }, + } + with open(extension_dir / "extension.yml", "w") as f: + yaml.dump(extension_manifest, f) + + preset_dir = temp_dir / "ext-skill-restore" + preset_dir.mkdir() + (preset_dir / "commands").mkdir() + (preset_dir / "commands" / "speckit.fakeext.cmd.md").write_text( + "---\ndescription: Override fakeext cmd\n---\n\npreset:ext-skill-restore\n" + ) + preset_manifest = { + "schema_version": "1.0", + "preset": { + "id": "ext-skill-restore", + "name": "Ext Skill Restore", + "version": "1.0.0", + "description": "Test", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "templates": [ + { + "type": "command", + "name": "speckit.fakeext.cmd", + "file": "commands/speckit.fakeext.cmd.md", + } + ] + }, + } + with open(preset_dir / "preset.yml", "w") as f: + yaml.dump(preset_manifest, f) + + manager = PresetManager(project_dir) + manager.install_from_directory(preset_dir, "0.1.5") + + skill_file = skills_dir / "speckit-fakeext-cmd" / "SKILL.md" + assert "preset:ext-skill-restore" in skill_file.read_text() + + manager.remove("ext-skill-restore") + + assert skill_file.exists() + content = skill_file.read_text() + assert "preset:ext-skill-restore" not in content + assert "source: extension:fakeext" in content + assert "extension:fakeext" in content + assert '.specify/scripts/bash/setup-plan.sh --json "$ARGUMENTS"' in content + def test_kimi_legacy_dotted_skill_override_still_applies(self, project_dir, temp_dir): """Preset overrides should still target legacy dotted Kimi skill directories.""" self._write_init_options(project_dir, ai="kimi") @@ -2190,7 +2272,7 @@ def test_kimi_skill_updated_even_when_ai_skills_disabled(self, project_dir, temp assert "speckit-specify" in metadata.get("registered_skills", []) def test_kimi_preset_skill_override_resolves_script_placeholders(self, project_dir, temp_dir): - """Kimi preset skill overrides should resolve {SCRIPT} and __AGENT__ placeholders.""" + """Kimi preset skill overrides should resolve placeholders and rewrite project paths.""" self._write_init_options(project_dir, ai="kimi", ai_skills=False, script="sh") skills_dir = project_dir / ".kimi" / "skills" self._create_skill(skills_dir, "speckit-specify", body="untouched") @@ -2206,6 +2288,7 @@ def test_kimi_preset_skill_override_resolves_script_placeholders(self, project_d " sh: scripts/bash/create-new-feature.sh --json \"{ARGS}\"\n" "---\n\n" "Execute `{SCRIPT}` for __AGENT__\n" + "Review templates/checklist.md and memory/constitution.md\n" ) manifest_data = { "schema_version": "1.0", @@ -2235,7 +2318,9 @@ def test_kimi_preset_skill_override_resolves_script_placeholders(self, project_d content = (skills_dir / "speckit-specify" / "SKILL.md").read_text() assert "{SCRIPT}" not in content assert "__AGENT__" not in content - assert "scripts/bash/create-new-feature.sh --json \"$ARGUMENTS\"" in content + assert ".specify/scripts/bash/create-new-feature.sh --json \"$ARGUMENTS\"" in content + assert ".specify/templates/checklist.md" in content + assert ".specify/memory/constitution.md" in content assert "for kimi" in content From 3a638632306b98b128fbc6a867073dfdc31671d3 Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Thu, 26 Mar 2026 18:52:52 +0800 Subject: [PATCH 17/23] Refine preset skill restore helpers --- src/specify_cli/agents.py | 9 +++++++-- src/specify_cli/presets.py | 27 ++++++++++++++------------- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index bcf9fcecf..7910e6667 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -213,13 +213,18 @@ def render_frontmatter(fm: dict) -> str: return f"---\n{yaml_str}---\n" def _adjust_script_paths(self, frontmatter: dict) -> dict: - """Adjust script paths from extension-relative to repo-relative. + """Normalize script paths in frontmatter to generated project locations. + + Rewrites known repo-relative and top-level script paths under the + `scripts` and `agent_scripts` keys (for example `../../scripts/`, + `../../templates/`, `../../memory/`, `scripts/`, `templates/`, and + `memory/`) to the `.specify/...` paths used in generated projects. Args: frontmatter: Frontmatter dictionary Returns: - Modified frontmatter with adjusted paths + Modified frontmatter with normalized project paths """ frontmatter = deepcopy(frontmatter) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 8c478916c..59b9fc357 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -588,25 +588,23 @@ def _get_skills_dir(self) -> Optional[Path]: return skills_dir @staticmethod - def _skill_names_for_command(cmd_name: str) -> List[str]: + def _skill_names_for_command(cmd_name: str) -> tuple[str, str]: """Return the modern and legacy skill directory names for a command.""" raw_short_name = cmd_name if raw_short_name.startswith("speckit."): raw_short_name = raw_short_name[len("speckit."):] - skill_names = [f"speckit-{raw_short_name.replace('.', '-')}"] + modern_skill_name = f"speckit-{raw_short_name.replace('.', '-')}" legacy_skill_name = f"speckit.{raw_short_name}" - if legacy_skill_name not in skill_names: - skill_names.append(legacy_skill_name) + return modern_skill_name, legacy_skill_name - return skill_names - - def _find_extension_skill_restore(self, skill_name: str) -> Optional[Dict[str, Any]]: - """Find the highest-priority installed extension template for a skill.""" + def _build_extension_skill_restore_index(self) -> Dict[str, Dict[str, Any]]: + """Index extension-backed skill restore data by skill directory name.""" from .extensions import ExtensionManifest, ValidationError resolver = PresetResolver(self.project_root) extensions_dir = self.project_root / ".specify" / "extensions" + restore_index: Dict[str, Dict[str, Any]] = {} for _priority, ext_id, _metadata in resolver._get_all_extensions_by_priority(): ext_dir = extensions_dir / ext_id @@ -625,8 +623,6 @@ def _find_extension_skill_restore(self, skill_name: str) -> Optional[Dict[str, A cmd_file_rel = cmd_info.get("file") if not isinstance(cmd_name, str) or not isinstance(cmd_file_rel, str): continue - if skill_name not in self._skill_names_for_command(cmd_name): - continue cmd_path = Path(cmd_file_rel) if cmd_path.is_absolute(): @@ -641,13 +637,17 @@ def _find_extension_skill_restore(self, skill_name: str) -> Optional[Dict[str, A if not source_file.is_file(): continue - return { + restore_info = { "command_name": cmd_name, "source_file": source_file, "source": f"extension:{manifest.id}", } + modern_skill_name, legacy_skill_name = self._skill_names_for_command(cmd_name) + restore_index.setdefault(modern_skill_name, restore_info) + if legacy_skill_name != modern_skill_name: + restore_index.setdefault(legacy_skill_name, restore_info) - return None + return restore_index def _register_skills( self, @@ -797,6 +797,7 @@ def _unregister_skills(self, skill_names: List[str], preset_dir: Path) -> None: init_opts = load_init_options(self.project_root) selected_ai = init_opts.get("ai") registrar = CommandRegistrar() + extension_restore_index = self._build_extension_skill_restore_index() for skill_name in skill_names: # Derive command name from skill name (speckit-specify -> specify) @@ -851,7 +852,7 @@ def _unregister_skills(self, skill_names: List[str], preset_dir: Path) -> None: skill_file.write_text(skill_content, encoding="utf-8") continue - extension_restore = self._find_extension_skill_restore(skill_name) + extension_restore = extension_restore_index.get(skill_name) if extension_restore: content = extension_restore["source_file"].read_text(encoding="utf-8") frontmatter, body = registrar.parse_frontmatter(content) From ac580bd35d61facd01553275ea088417becc896f Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Thu, 26 Mar 2026 19:45:22 +0800 Subject: [PATCH 18/23] Harden skill path and preset checks --- src/specify_cli/__init__.py | 4 ---- src/specify_cli/agents.py | 8 +++++--- src/specify_cli/presets.py | 2 +- tests/test_extensions.py | 30 ++++++++++++++++++++++++++++++ tests/test_presets.py | 10 ++++++++++ 5 files changed, 46 insertions(+), 8 deletions(-) diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index ad7300a38..1f0eaf475 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -2151,10 +2151,6 @@ def init( if ai_skills: if selected_ai in NATIVE_SKILLS_AGENTS: - if skills_dir is None: - raise RuntimeError( - f"Could not resolve skills directory for agent: {selected_ai}" - ) bundled_found = _has_bundled_skills(project_path, selected_ai) if bundled_found: detail = f"bundled skills → {skills_dir.relative_to(project_path)}" diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index 7910e6667..b3d3f3fc2 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -251,9 +251,11 @@ def _rewrite_project_relative_paths(text: str) -> str: ): text = text.replace(old, new) - text = re.sub(r"(?/scripts/..." remain intact. + text = re.sub(r'(^|[\s`"\'(])(?:\.?/)?memory/', r"\1.specify/memory/", text) + text = re.sub(r'(^|[\s`"\'(])(?:\.?/)?scripts/', r"\1.specify/scripts/", text) + text = re.sub(r'(^|[\s`"\'(])(?:\.?/)?templates/', r"\1.specify/templates/", text) return text.replace(".specify/.specify/", ".specify/").replace(".specify.specify/", ".specify/") diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 59b9fc357..795375a81 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -574,7 +574,7 @@ def _get_skills_dir(self) -> Optional[Path]: opts = load_init_options(self.project_root) agent = opts.get("ai") - if not agent: + if not isinstance(agent, str) or not agent: return None ai_skills_enabled = bool(opts.get("ai_skills")) diff --git a/tests/test_extensions.py b/tests/test_extensions.py index 91130fcdb..d960d6805 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -778,6 +778,36 @@ def test_adjust_script_paths_does_not_mutate_input(self): assert adjusted["scripts"]["sh"] == ".specify/scripts/bash/setup-plan.sh {ARGS}" assert adjusted["scripts"]["ps"] == ".specify/scripts/powershell/setup-plan.ps1 {ARGS}" + def test_adjust_script_paths_preserves_extension_local_paths(self): + """Extension-local script paths should not be rewritten into .specify/.specify.""" + from specify_cli.agents import CommandRegistrar as AgentCommandRegistrar + registrar = AgentCommandRegistrar() + original = { + "scripts": { + "sh": ".specify/extensions/test-ext/scripts/setup.sh {ARGS}", + "ps": "scripts/powershell/setup-plan.ps1 {ARGS}", + } + } + + adjusted = registrar._adjust_script_paths(original) + + assert adjusted["scripts"]["sh"] == ".specify/extensions/test-ext/scripts/setup.sh {ARGS}" + assert adjusted["scripts"]["ps"] == ".specify/scripts/powershell/setup-plan.ps1 {ARGS}" + + def test_rewrite_project_relative_paths_preserves_extension_local_body_paths(self): + """Body rewrites should preserve extension-local assets while fixing top-level refs.""" + from specify_cli.agents import CommandRegistrar as AgentCommandRegistrar + + body = ( + "Read `.specify/extensions/test-ext/templates/spec.md`\n" + "Run scripts/bash/setup-plan.sh\n" + ) + + rewritten = AgentCommandRegistrar._rewrite_project_relative_paths(body) + + assert ".specify/extensions/test-ext/templates/spec.md" in rewritten + assert ".specify/scripts/bash/setup-plan.sh" in rewritten + def test_render_toml_command_handles_embedded_triple_double_quotes(self): """TOML renderer should stay valid when body includes triple double-quotes.""" from specify_cli.agents import CommandRegistrar as AgentCommandRegistrar diff --git a/tests/test_presets.py b/tests/test_presets.py index 9ed35cf68..1b1685b70 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -1995,6 +1995,16 @@ def test_skill_not_updated_when_ai_skills_disabled(self, project_dir, temp_dir): content = skill_file.read_text() assert "untouched" in content, "Skill should not be modified when ai_skills=False" + def test_get_skills_dir_returns_none_for_non_string_ai(self, project_dir): + """Corrupted init-options ai values should not crash preset skill resolution.""" + init_options = project_dir / ".specify" / "init-options.json" + init_options.parent.mkdir(parents=True, exist_ok=True) + init_options.write_text('{"ai":["codex"],"ai_skills":true,"script":"sh"}') + + manager = PresetManager(project_dir) + + assert manager._get_skills_dir() is None + def test_skill_not_updated_without_init_options(self, project_dir, temp_dir): """When no init-options.json exists, preset install should not touch skills.""" skills_dir = project_dir / ".claude" / "skills" From 734efdf41d82306c8a282a27d6d94011c00e21a4 Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Thu, 26 Mar 2026 19:54:28 +0800 Subject: [PATCH 19/23] Guard non-dict init options --- src/specify_cli/agents.py | 6 +++- src/specify_cli/presets.py | 6 ++++ tests/test_extensions.py | 56 ++++++++++++++++++++++++++++++++++++++ tests/test_presets.py | 27 ++++++++++++++++++ 4 files changed, 94 insertions(+), 1 deletion(-) diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index b3d3f3fc2..64617e843 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -387,7 +387,11 @@ def resolve_skill_placeholders(agent_name: str, frontmatter: dict, body: str, pr if not isinstance(agent_scripts, dict): agent_scripts = {} - script_variant = load_init_options(project_root).get("script") + init_opts = load_init_options(project_root) + if not isinstance(init_opts, dict): + init_opts = {} + + script_variant = init_opts.get("script") if script_variant not in {"sh", "ps"}: fallback_order = [] default_variant = "ps" if platform.system().lower().startswith("win") else "sh" diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 795375a81..a9948f97b 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -573,6 +573,8 @@ def _get_skills_dir(self) -> Optional[Path]: from . import load_init_options, _get_skills_dir opts = load_init_options(self.project_root) + if not isinstance(opts, dict): + opts = {} agent = opts.get("ai") if not isinstance(agent, str) or not agent: return None @@ -699,6 +701,8 @@ def _register_skills( from .agents import CommandRegistrar init_opts = load_init_options(self.project_root) + if not isinstance(init_opts, dict): + init_opts = {} selected_ai = init_opts.get("ai") if not isinstance(selected_ai, str): return [] @@ -795,6 +799,8 @@ def _unregister_skills(self, skill_names: List[str], preset_dir: Path) -> None: # Locate core command templates from the project's installed templates core_templates_dir = self.project_root / ".specify" / "templates" / "commands" init_opts = load_init_options(self.project_root) + if not isinstance(init_opts, dict): + init_opts = {} selected_ai = init_opts.get("ai") registrar = CommandRegistrar() extension_restore_index = self._build_extension_skill_restore_index() diff --git a/tests/test_extensions.py b/tests/test_extensions.py index d960d6805..92b9839ac 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -1141,6 +1141,62 @@ def test_codex_skill_registration_uses_fallback_script_variant_without_init_opti assert '.specify/scripts/bash/setup-plan.sh --json "$ARGUMENTS"' in content assert ".specify/scripts/bash/update-agent-context.sh codex" in content + def test_codex_skill_registration_handles_non_dict_init_options( + self, project_dir, temp_dir + ): + """Non-dict init-options payloads should not crash skill placeholder resolution.""" + import yaml + + ext_dir = temp_dir / "ext-script-list-init" + ext_dir.mkdir() + (ext_dir / "commands").mkdir() + + manifest_data = { + "schema_version": "1.0", + "extension": { + "id": "ext-script-list-init", + "name": "List init options", + "version": "1.0.0", + "description": "Test", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "commands": [ + { + "name": "speckit.list.plan", + "file": "commands/plan.md", + } + ] + }, + } + with open(ext_dir / "extension.yml", "w") as f: + yaml.dump(manifest_data, f) + + (ext_dir / "commands" / "plan.md").write_text( + """--- +description: "List init scripted command" +scripts: + sh: ../../scripts/bash/setup-plan.sh --json "{ARGS}" +--- + +Run {SCRIPT} +""" + ) + + init_options = project_dir / ".specify" / "init-options.json" + init_options.parent.mkdir(parents=True, exist_ok=True) + init_options.write_text("[]") + + skills_dir = project_dir / ".agents" / "skills" + skills_dir.mkdir(parents=True) + + manifest = ExtensionManifest(ext_dir / "extension.yml") + registrar = CommandRegistrar() + registrar.register_commands_for_agent("codex", manifest, ext_dir, project_dir) + + content = (skills_dir / "speckit-list-plan" / "SKILL.md").read_text() + assert '.specify/scripts/bash/setup-plan.sh --json "$ARGUMENTS"' in content + def test_codex_skill_registration_fallback_prefers_powershell_on_windows( self, project_dir, temp_dir, monkeypatch ): diff --git a/tests/test_presets.py b/tests/test_presets.py index 1b1685b70..03bc5289d 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -2005,6 +2005,16 @@ def test_get_skills_dir_returns_none_for_non_string_ai(self, project_dir): assert manager._get_skills_dir() is None + def test_get_skills_dir_returns_none_for_non_dict_init_options(self, project_dir): + """Corrupted non-dict init-options payloads should fail closed.""" + init_options = project_dir / ".specify" / "init-options.json" + init_options.parent.mkdir(parents=True, exist_ok=True) + init_options.write_text("[]") + + manager = PresetManager(project_dir) + + assert manager._get_skills_dir() is None + def test_skill_not_updated_without_init_options(self, project_dir, temp_dir): """When no init-options.json exists, preset install should not touch skills.""" skills_dir = project_dir / ".claude" / "skills" @@ -2333,6 +2343,23 @@ def test_kimi_preset_skill_override_resolves_script_placeholders(self, project_d assert ".specify/memory/constitution.md" in content assert "for kimi" in content + def test_preset_skill_registration_handles_non_dict_init_options(self, project_dir, temp_dir): + """Non-dict init-options payloads should not crash preset install/remove flows.""" + init_options = project_dir / ".specify" / "init-options.json" + init_options.parent.mkdir(parents=True, exist_ok=True) + init_options.write_text("[]") + + skills_dir = project_dir / ".claude" / "skills" + self._create_skill(skills_dir, "speckit-specify", body="untouched") + (project_dir / ".claude" / "commands").mkdir(parents=True, exist_ok=True) + + manager = PresetManager(project_dir) + self_test_dir = Path(__file__).parent.parent / "presets" / "self-test" + manager.install_from_directory(self_test_dir, "0.1.5") + + content = (skills_dir / "speckit-specify" / "SKILL.md").read_text() + assert "untouched" in content + class TestPresetSetPriority: """Test preset set-priority CLI command.""" From 4aeaf5bf1b3b2b87e1af98e4f5634d4b970d5919 Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Thu, 26 Mar 2026 20:12:29 +0800 Subject: [PATCH 20/23] Avoid deleting unmanaged preset skill dirs --- src/specify_cli/presets.py | 22 +++++++++++---- tests/test_presets.py | 56 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 5 deletions(-) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index a9948f97b..5d9dabf2a 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -817,6 +817,9 @@ def _unregister_skills(self, skill_names: List[str], preset_dir: Path) -> None: skill_file = skill_subdir / "SKILL.md" if not skill_subdir.is_dir(): continue + if not skill_file.is_file(): + # Only manage directories that contain the expected skill entrypoint. + continue # Try to find the core command template core_file = core_templates_dir / f"{short_name}.md" if core_templates_dir.exists() else None @@ -1022,17 +1025,26 @@ def remove(self, pack_id: str) -> bool: if not self.registry.is_installed(pack_id): return False - # Unregister commands from AI agents metadata = self.registry.get(pack_id) - registered_commands = metadata.get("registered_commands", {}) if metadata else {} - if registered_commands: - self._unregister_commands(registered_commands) - # Restore original skills when preset is removed registered_skills = metadata.get("registered_skills", []) if metadata else [] + registered_commands = metadata.get("registered_commands", {}) if metadata else {} pack_dir = self.presets_dir / pack_id if registered_skills: self._unregister_skills(registered_skills, pack_dir) + try: + from . import NATIVE_SKILLS_AGENTS + except ImportError: + NATIVE_SKILLS_AGENTS = set() + registered_commands = { + agent_name: cmd_names + for agent_name, cmd_names in registered_commands.items() + if agent_name not in NATIVE_SKILLS_AGENTS + } + + # Unregister non-skill command files from AI agents. + if registered_commands: + self._unregister_commands(registered_commands) if pack_dir.exists(): shutil.rmtree(pack_dir) diff --git a/tests/test_presets.py b/tests/test_presets.py index 03bc5289d..3b57d2450 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -2249,6 +2249,62 @@ def test_extension_skill_restored_on_preset_remove(self, project_dir, temp_dir): assert "extension:fakeext" in content assert '.specify/scripts/bash/setup-plan.sh --json "$ARGUMENTS"' in content + def test_preset_remove_skips_skill_dir_without_skill_file(self, project_dir, temp_dir): + """Preset removal should not delete arbitrary directories missing SKILL.md.""" + self._write_init_options(project_dir, ai="codex") + skills_dir = project_dir / ".agents" / "skills" + stray_skill_dir = skills_dir / "speckit-fakeext-cmd" + stray_skill_dir.mkdir(parents=True, exist_ok=True) + note_file = stray_skill_dir / "notes.txt" + note_file.write_text("user content", encoding="utf-8") + + preset_dir = temp_dir / "ext-skill-missing-file" + preset_dir.mkdir() + (preset_dir / "commands").mkdir() + (preset_dir / "commands" / "speckit.fakeext.cmd.md").write_text( + "---\ndescription: Override fakeext cmd\n---\n\npreset:ext-skill-missing-file\n" + ) + preset_manifest = { + "schema_version": "1.0", + "preset": { + "id": "ext-skill-missing-file", + "name": "Ext Skill Missing File", + "version": "1.0.0", + "description": "Test", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "templates": [ + { + "type": "command", + "name": "speckit.fakeext.cmd", + "file": "commands/speckit.fakeext.cmd.md", + } + ] + }, + } + with open(preset_dir / "preset.yml", "w") as f: + yaml.dump(preset_manifest, f) + + manager = PresetManager(project_dir) + installed_preset_dir = manager.presets_dir / "ext-skill-missing-file" + shutil.copytree(preset_dir, installed_preset_dir) + manager.registry.add( + "ext-skill-missing-file", + { + "version": "1.0.0", + "source": str(preset_dir), + "provides_templates": ["speckit.fakeext.cmd"], + "registered_skills": ["speckit-fakeext-cmd"], + "priority": 10, + }, + ) + + manager.remove("ext-skill-missing-file") + + assert stray_skill_dir.is_dir() + assert note_file.read_text(encoding="utf-8") == "user content" + def test_kimi_legacy_dotted_skill_override_still_applies(self, project_dir, temp_dir): """Preset overrides should still target legacy dotted Kimi skill directories.""" self._write_init_options(project_dir, ai="kimi") From bd7f6a14e70f7f69949407f0039b6d95068728b0 Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Thu, 26 Mar 2026 20:27:08 +0800 Subject: [PATCH 21/23] Unify extension skill naming with hooks --- src/specify_cli/extensions.py | 15 ++----- tests/test_extension_skills.py | 77 +++++++++++++++++----------------- 2 files changed, 41 insertions(+), 51 deletions(-) diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index 6abfb5e6a..39abaebef 100644 --- a/src/specify_cli/extensions.py +++ b/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 diff --git a/tests/test_extension_skills.py b/tests/test_extension_skills.py index f8b89bcb5..610edf82a 100644 --- a/tests/test_extension_skills.py +++ b/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,7 +241,7 @@ 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") @@ -249,7 +249,7 @@ def test_skill_md_has_parseable_yaml(self, skills_project, extension_dir): 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,7 +624,7 @@ 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) @@ -633,5 +632,5 @@ def test_remove_cleans_up_when_ai_skills_toggled(self, skills_project, extension # 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() From 0d0382f7bee6f846260110ae5559c3486659a564 Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Thu, 26 Mar 2026 20:41:48 +0800 Subject: [PATCH 22/23] Harden extension native skill registration --- src/specify_cli/extensions.py | 48 ++++++++------- tests/test_extension_skills.py | 105 +++++++++++++++++++++++++++++++++ 2 files changed, 132 insertions(+), 21 deletions(-) diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index 39abaebef..ed2d187ba 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -511,24 +511,32 @@ def _ignore(directory: str, entries: List[str]) -> Set[str]: return _ignore def _get_skills_dir(self) -> Optional[Path]: - """Return the skills directory if ``--ai-skills`` was used during init. + """Return the active skills directory for extension skill registration. Reads ``.specify/init-options.json`` to determine whether skills are enabled and which agent was selected, then delegates to the module-level ``_get_skills_dir()`` helper for the concrete path. + Kimi is treated as a native-skills agent: if ``ai == "kimi"`` and + ``.kimi/skills`` exists, extension installs should still propagate + command skills even when ``ai_skills`` is false. + Returns: The skills directory ``Path``, or ``None`` if skills were not - enabled or the init-options file is missing. + enabled and no native-skills fallback applies. """ from . import load_init_options, _get_skills_dir as resolve_skills_dir opts = load_init_options(self.project_root) - if not opts.get("ai_skills"): - return None + if not isinstance(opts, dict): + opts = {} agent = opts.get("ai") - if not agent: + if not isinstance(agent, str) or not agent: + return None + + ai_skills_enabled = bool(opts.get("ai_skills")) + if not ai_skills_enabled and agent != "kimi": return None skills_dir = resolve_skills_dir(self.project_root, agent) @@ -560,9 +568,18 @@ def _register_extension_skills( if not skills_dir: return [] + from . import load_init_options + from .agents import CommandRegistrar import yaml written: List[str] = [] + opts = load_init_options(self.project_root) + if not isinstance(opts, dict): + opts = {} + selected_ai = opts.get("ai") + if not isinstance(selected_ai, str) or not selected_ai: + return [] + registrar = CommandRegistrar() for cmd_info in manifest.commands: cmd_name = cmd_info["name"] @@ -612,22 +629,11 @@ def _register_extension_skills( except OSError: pass # best-effort cleanup continue - if content.startswith("---"): - parts = content.split("---", 2) - if len(parts) >= 3: - try: - frontmatter = yaml.safe_load(parts[1]) - except yaml.YAMLError: - frontmatter = {} - if not isinstance(frontmatter, dict): - frontmatter = {} - body = parts[2].strip() - else: - frontmatter = {} - body = content - else: - frontmatter = {} - body = content + frontmatter, body = registrar.parse_frontmatter(content) + frontmatter = registrar._adjust_script_paths(frontmatter) + body = registrar.resolve_skill_placeholders( + selected_ai, frontmatter, body, self.project_root + ) original_desc = frontmatter.get("description", "") description = original_desc or f"Extension command: {cmd_name}" diff --git a/tests/test_extension_skills.py b/tests/test_extension_skills.py index 610edf82a..47d40a3b9 100644 --- a/tests/test_extension_skills.py +++ b/tests/test_extension_skills.py @@ -192,6 +192,24 @@ def test_returns_none_when_skills_dir_missing(self, project_dir): result = manager._get_skills_dir() assert result is None + def test_returns_kimi_skills_dir_when_ai_skills_disabled(self, project_dir): + """Kimi should still use its native skills dir when ai_skills is false.""" + _create_init_options(project_dir, ai="kimi", ai_skills=False) + skills_dir = _create_skills_dir(project_dir, ai="kimi") + manager = ExtensionManager(project_dir) + result = manager._get_skills_dir() + assert result == skills_dir + + def test_returns_none_for_non_dict_init_options(self, project_dir): + """Corrupted-but-parseable init-options should not crash skill-dir lookup.""" + opts_file = project_dir / ".specify" / "init-options.json" + opts_file.parent.mkdir(parents=True, exist_ok=True) + opts_file.write_text("[]") + _create_skills_dir(project_dir, ai="claude") + manager = ExtensionManager(project_dir) + result = manager._get_skills_dir() + assert result is None + # ===== Extension Skill Registration Tests ===== @@ -326,6 +344,78 @@ def test_kimi_uses_hyphenated_skill_names(self, project_dir, temp_dir): assert "speckit-test-ext-hello" in metadata["registered_skills"] assert "speckit-test-ext-world" in metadata["registered_skills"] + def test_kimi_creates_skills_when_ai_skills_disabled(self, project_dir, temp_dir): + """Kimi should still auto-register extension skills in native-skills mode.""" + _create_init_options(project_dir, ai="kimi", ai_skills=False) + skills_dir = _create_skills_dir(project_dir, ai="kimi") + ext_dir = _create_extension_dir(temp_dir, ext_id="test-ext") + + manager = ExtensionManager(project_dir) + manifest = manager.install_from_directory( + ext_dir, "0.1.0", register_commands=False + ) + + metadata = manager.registry.get(manifest.id) + assert "speckit-test-ext-hello" in metadata["registered_skills"] + assert "speckit-test-ext-world" in metadata["registered_skills"] + assert (skills_dir / "speckit-test-ext-hello" / "SKILL.md").exists() + + def test_skill_registration_resolves_script_placeholders(self, project_dir, temp_dir): + """Auto-registered extension skills should resolve script placeholders.""" + _create_init_options(project_dir, ai="claude", ai_skills=True) + skills_dir = _create_skills_dir(project_dir, ai="claude") + + ext_dir = temp_dir / "scripted-ext" + ext_dir.mkdir() + manifest_data = { + "schema_version": "1.0", + "extension": { + "id": "scripted-ext", + "name": "Scripted Extension", + "version": "1.0.0", + "description": "Test", + }, + "requires": {"speckit_version": ">=0.1.0"}, + "provides": { + "commands": [ + { + "name": "speckit.scripted-ext.plan", + "file": "commands/plan.md", + "description": "Scripted plan command", + } + ] + }, + } + with open(ext_dir / "extension.yml", "w") as f: + yaml.dump(manifest_data, f) + + (ext_dir / "commands").mkdir() + (ext_dir / "commands" / "plan.md").write_text( + "---\n" + "description: Scripted plan command\n" + "scripts:\n" + " sh: ../../scripts/bash/setup-plan.sh --json \"{ARGS}\"\n" + "agent_scripts:\n" + " sh: ../../scripts/bash/update-agent-context.sh __AGENT__\n" + "---\n\n" + "Run {SCRIPT}\n" + "Then {AGENT_SCRIPT}\n" + "Review templates/checklist.md and memory/constitution.md for __AGENT__.\n" + ) + + manager = ExtensionManager(project_dir) + manager.install_from_directory(ext_dir, "0.1.0", register_commands=False) + + content = (skills_dir / "speckit-scripted-ext-plan" / "SKILL.md").read_text() + assert "{SCRIPT}" not in content + assert "{AGENT_SCRIPT}" not in content + assert "{ARGS}" not in content + assert "__AGENT__" not in content + assert '.specify/scripts/bash/setup-plan.sh --json "$ARGUMENTS"' in content + assert ".specify/scripts/bash/update-agent-context.sh claude" in content + assert ".specify/templates/checklist.md" in content + assert ".specify/memory/constitution.md" in content + def test_missing_command_file_skipped(self, skills_project, temp_dir): """Commands with missing source files should be skipped gracefully.""" project_dir, skills_dir = skills_project @@ -453,6 +543,21 @@ def test_remove_no_skills_when_not_active(self, no_skills_project, extension_dir class TestExtensionSkillEdgeCases: """Test edge cases in extension skill registration.""" + def test_install_with_non_dict_init_options_does_not_crash(self, project_dir, extension_dir): + """Corrupted init-options payloads should disable skill registration, not crash install.""" + opts_file = project_dir / ".specify" / "init-options.json" + opts_file.parent.mkdir(parents=True, exist_ok=True) + opts_file.write_text("[]") + _create_skills_dir(project_dir, ai="claude") + + manager = ExtensionManager(project_dir) + manifest = manager.install_from_directory( + extension_dir, "0.1.0", register_commands=False + ) + + metadata = manager.registry.get(manifest.id) + assert metadata["registered_skills"] == [] + def test_command_without_frontmatter(self, skills_project, temp_dir): """Commands without YAML frontmatter should still produce valid skills.""" project_dir, skills_dir = skills_project From 0dbbda1e2e0ef7320838d82e2b763b10f12ffdf3 Mon Sep 17 00:00:00 2001 From: rbbtsn0w Date: Thu, 26 Mar 2026 20:44:25 +0800 Subject: [PATCH 23/23] Normalize preset skill titles --- src/specify_cli/presets.py | 19 +++++++++++++------ tests/test_presets.py | 2 ++ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 5d9dabf2a..a3f640628 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -600,6 +600,14 @@ def _skill_names_for_command(cmd_name: str) -> tuple[str, str]: legacy_skill_name = f"speckit.{raw_short_name}" return modern_skill_name, legacy_skill_name + @staticmethod + def _skill_title_from_command(cmd_name: str) -> str: + """Return a human-friendly title for a skill command name.""" + title_name = cmd_name + if title_name.startswith("speckit."): + title_name = title_name[len("speckit."):] + return title_name.replace(".", " ").replace("-", " ").title() + def _build_extension_skill_restore_index(self) -> Dict[str, Dict[str, Any]]: """Index extension-backed skill restore data by skill directory name.""" from .extensions import ExtensionManifest, ValidationError @@ -723,6 +731,7 @@ def _register_skills( raw_short_name = raw_short_name[len("speckit."):] short_name = raw_short_name.replace(".", "-") skill_name, legacy_skill_name = self._skill_names_for_command(cmd_name) + skill_title = self._skill_title_from_command(cmd_name) # Only overwrite skills that already exist under skills_dir, # including Kimi native skills when ai_skills is false. @@ -765,7 +774,7 @@ def _register_skills( f"---\n" f"{frontmatter_text}\n" f"---\n\n" - f"# Speckit {short_name.title()} Skill\n\n" + f"# Speckit {skill_title} Skill\n\n" f"{body}\n" ) @@ -851,11 +860,12 @@ def _unregister_skills(self, skill_names: List[str], preset_dir: Path) -> None: }, } frontmatter_text = yaml.safe_dump(frontmatter_data, sort_keys=False).strip() + skill_title = self._skill_title_from_command(short_name) skill_content = ( f"---\n" f"{frontmatter_text}\n" f"---\n\n" - f"# Speckit {short_name.title()} Skill\n\n" + f"# Speckit {skill_title} Skill\n\n" f"{body}\n" ) skill_file.write_text(skill_content, encoding="utf-8") @@ -871,10 +881,7 @@ def _unregister_skills(self, skill_names: List[str], preset_dir: Path) -> None: ) command_name = extension_restore["command_name"] - title_name = command_name - if title_name.startswith("speckit."): - title_name = title_name[len("speckit."):] - title_name = title_name.replace(".", " ").replace("-", " ").title() + title_name = self._skill_title_from_command(command_name) frontmatter_data = { "name": skill_name, diff --git a/tests/test_presets.py b/tests/test_presets.py index 3b57d2450..1b2704c57 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -2163,6 +2163,7 @@ def test_extension_skill_override_matches_hyphenated_multisegment_name(self, pro content = skill_file.read_text() assert "preset:ext-skill-override" in content assert "name: speckit-fakeext-cmd" in content + assert "# Speckit Fakeext Cmd Skill" in content metadata = manager.registry.get("ext-skill-override") assert "speckit-fakeext-cmd" in metadata.get("registered_skills", []) @@ -2248,6 +2249,7 @@ def test_extension_skill_restored_on_preset_remove(self, project_dir, temp_dir): assert "source: extension:fakeext" in content assert "extension:fakeext" in content assert '.specify/scripts/bash/setup-plan.sh --json "$ARGUMENTS"' in content + assert "# Fakeext Cmd Skill" in content def test_preset_remove_skips_skill_dir_without_skill_file(self, project_dir, temp_dir): """Preset removal should not delete arbitrary directories missing SKILL.md."""