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/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/scripts/powershell/create-new-feature.ps1 b/scripts/powershell/create-new-feature.ps1 index 9adae131d..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,12 +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 { - if ($_.Name -match '^(\d{3})-') { - $num = [int]$matches[1] - if ($num -gt $highest) { $highest = $num } + # Match sequential prefixes (>=3 digits), but skip timestamp dirs. + if ($_.Name -match '^(\d{3,})-' -and $_.Name -notmatch '^\d{8}-\d{6}-') { + [long]$num = 0 + if ([long]::TryParse($matches[1], [ref]$num) -and $num -gt $highest) { + $highest = $num + } } } } @@ -63,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) { @@ -71,10 +74,12 @@ 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})-') { - $num = [int]$matches[1] - if ($num -gt $highest) { $highest = $num } + # Extract sequential feature number (>=3 digits), skip timestamp branches. + if ($cleanBranch -match '^(\d{3,})-' -and $cleanBranch -notmatch '^\d{8}-\d{6}-') { + [long]$num = 0 + if ([long]::TryParse($matches[1], [ref]$num) -and $num -gt $highest) { + $highest = $num + } } } } @@ -290,4 +295,3 @@ if ($Json) { Write-Output "HAS_GIT: $hasGit" Write-Output "SPECIFY_FEATURE environment variable set to: $branchName" } - diff --git a/src/specify_cli/__init__.py b/src/specify_cli/__init__.py index d78609ad6..1f0eaf475 100644 --- a/src/specify_cli/__init__.py +++ b/src/specify_cli/__init__.py @@ -1490,12 +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 = { - "codex": ".agents/skills", # Codex agent layout override -} - # Default skills directory for agents not in AGENT_CONFIG DEFAULT_SKILLS_DIR = ".agents/skills" @@ -1528,13 +1522,9 @@ SKILL_DESCRIPTIONS = { 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: @@ -1648,10 +1638,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.replace('.', '-')}" # Create skill directory (additive — never removes existing content) skill_dir = skills_dir / skill_name @@ -1730,8 +1717,64 @@ 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 when equivalent hyphenated dir 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 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(): + # 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 + + return (migrated_count, removed_count) AGENT_SKILLS_MIGRATIONS = { @@ -2094,16 +2137,33 @@ 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) 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 +2348,7 @@ def init( 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..64617e843 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -10,6 +10,8 @@ from pathlib import Path from typing import Dict, List, Any import platform +import re +from copy import deepcopy import yaml @@ -211,24 +213,52 @@ class CommandRegistrar: 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) + for script_key in ("scripts", "agent_scripts"): scripts = frontmatter.get(script_key) if not isinstance(scripts, 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) + + # Only rewrite top-level style references so extension-local paths like + # ".specify/extensions//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/") + def render_markdown_command( self, frontmatter: dict, @@ -277,9 +307,25 @@ class CommandRegistrar: 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) @@ -308,8 +354,8 @@ class CommandRegistrar: 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 +370,8 @@ class CommandRegistrar: 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: @@ -346,7 +387,11 @@ class CommandRegistrar: 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" @@ -376,7 +421,8 @@ class CommandRegistrar: 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") + body = body.replace("{ARGS}", "$ARGUMENTS").replace("__AGENT__", agent_name) + return CommandRegistrar._rewrite_project_relative_paths(body) def _convert_argument_placeholder(self, content: str, from_placeholder: str, to_placeholder: str) -> str: """Convert argument placeholder format. @@ -400,8 +446,9 @@ class CommandRegistrar: 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..ed2d187ba 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -511,24 +511,32 @@ class ExtensionManager: 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) @@ -561,12 +569,17 @@ class ExtensionManager: return [] from . import load_init_options + from .agents import CommandRegistrar import yaml - opts = load_init_options(self.project_root) - selected_ai = opts.get("ai", "") - 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"] @@ -587,17 +600,12 @@ class ExtensionManager: 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 @@ -621,22 +629,11 @@ class ExtensionManager: 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}" @@ -738,11 +735,9 @@ class ExtensionManager: 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: @@ -1940,6 +1935,52 @@ class HookExecutor: 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. + + 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: Any) -> 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: Any) -> 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 +2224,27 @@ class HookExecutor: for hook in hooks: 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}") - lines.append(f"Command: `/{command}`") + lines.append(f"Command: `{display_invocation}`") if description: lines.append(f"Description: {description}") lines.append(f"\nPrompt: {prompt}") - lines.append(f"To execute: `/{command}`") + lines.append(f"To execute: `{display_invocation}`") else: lines.append(f"\n**Automatic Hook**: {extension}") - lines.append(f"Executing: `/{command}`") - lines.append(f"EXECUTE_COMMAND: {command}") + 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) @@ -2261,6 +2308,7 @@ class HookExecutor: """ 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 +2352,3 @@ class HookExecutor: hook["enabled"] = False self.save_project_config(config) - diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index 24d523aa8..a3f640628 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -556,24 +556,31 @@ class PresetManager: 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"): + if not isinstance(opts, dict): + opts = {} + agent = opts.get("ai") + if not isinstance(agent, str) or not agent: return None - agent = opts.get("ai") - if not agent: + 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) @@ -582,6 +589,76 @@ class PresetManager: return skills_dir + @staticmethod + 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."):] + + modern_skill_name = f"speckit-{raw_short_name.replace('.', '-')}" + 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 + + 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 + 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 + + 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 + + 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 restore_index + def _register_skills( self, manifest: "PresetManifest", @@ -629,9 +706,15 @@ class PresetManager: return [] from . import SKILL_DESCRIPTIONS, load_init_options + from .agents import CommandRegistrar - opts = load_init_options(self.project_root) - selected_ai = opts.get("ai", "") + 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 [] + registrar = CommandRegistrar() written: List[str] = [] @@ -643,62 +726,61 @@ class PresetManager: 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."):] - if selected_ai == "kimi": - skill_name = f"speckit.{short_name}" - else: - skill_name = f"speckit-{short_name}" + 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, legacy_skill_name = self._skill_names_for_command(cmd_name) + skill_title = self._skill_title_from_command(cmd_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(): + # 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).is_dir(): + target_skill_names.append(skill_name) + 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 # 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_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" + frontmatter = dict(frontmatter) + frontmatter["description"] = enhanced_desc + body = registrar.resolve_skill_placeholders( + selected_ai, frontmatter, body, self.project_root ) - skill_file = skill_subdir / "SKILL.md" - skill_file.write_text(skill_content, encoding="utf-8") - written.append(skill_name) + 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 {skill_title} Skill\n\n" + f"{body}\n" + ) + + 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 @@ -720,10 +802,17 @@ class PresetManager: 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) + 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() for skill_name in skill_names: # Derive command name from skill name (speckit-specify -> specify) @@ -735,7 +824,10 @@ class PresetManager: skill_subdir = skills_dir / skill_name skill_file = skill_subdir / "SKILL.md" - if not skill_file.exists(): + 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 @@ -746,19 +838,11 @@ class PresetManager: 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( @@ -776,16 +860,49 @@ class PresetManager: }, } 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") + continue + + 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) + 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 = self._skill_title_from_command(command_name) + + 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( @@ -915,17 +1032,26 @@ class PresetManager: 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_ai_skills.py b/tests/test_ai_skills.py index cf2b6b7b9..f0e220e26 100644 --- a/tests/test_ai_skills.py +++ b/tests/test_ai_skills.py @@ -24,8 +24,8 @@ import specify_cli from specify_cli import ( _get_skills_dir, + _migrate_legacy_kimi_dotted_skills, install_ai_skills, - AGENT_SKILLS_DIR_OVERRIDES, DEFAULT_SKILLS_DIR, SKILL_DESCRIPTIONS, AGENT_CONFIG, @@ -169,8 +169,8 @@ class TestGetSkillsDir: 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" @@ -203,12 +203,71 @@ class TestGetSkillsDir: # 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.""" + + 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_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("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" + + 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 ===== @@ -473,8 +532,7 @@ class TestInstallAiSkills: 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() @@ -773,6 +831,32 @@ class TestNewProjectCommandSkip: 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 @@ -1118,12 +1202,12 @@ class TestCliValidation: 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 +1221,7 @@ class TestCliValidation: ) 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_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: diff --git a/tests/test_extension_skills.py b/tests/test_extension_skills.py index b8d5202f5..47d40a3b9 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 @@ -195,6 +192,24 @@ class TestExtensionManagerGetSkillsDir: 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 ===== @@ -211,8 +226,8 @@ class TestExtensionSkillRegistration: # 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.""" @@ -222,13 +237,13 @@ class TestExtensionSkillRegistration: 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 @@ -244,7 +259,7 @@ class TestExtensionSkillRegistration: 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") @@ -252,7 +267,7 @@ class TestExtensionSkillRegistration: 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): @@ -281,7 +296,7 @@ class TestExtensionSkillRegistration: 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) @@ -296,9 +311,9 @@ class TestExtensionSkillRegistration: # 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.""" @@ -311,11 +326,11 @@ class TestExtensionSkillRegistration: 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") @@ -326,9 +341,80 @@ class TestExtensionSkillRegistration: ) 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_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.""" @@ -375,8 +461,8 @@ class TestExtensionSkillRegistration: ) 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 ===== @@ -393,16 +479,16 @@ class TestExtensionSkillUnregistration: ) # 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.""" @@ -433,8 +519,8 @@ class TestExtensionSkillUnregistration: ) # 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) @@ -457,6 +543,21 @@ class TestExtensionSkillUnregistration: 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 @@ -495,10 +596,10 @@ class TestExtensionSkillEdgeCases: 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 @@ -515,8 +616,8 @@ class TestExtensionSkillEdgeCases: ) 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.""" @@ -534,15 +635,15 @@ class TestExtensionSkillEdgeCases: ) # 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.""" @@ -591,7 +692,7 @@ class TestExtensionSkillEdgeCases: 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 @@ -607,7 +708,7 @@ class TestExtensionSkillEdgeCases: ) # 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" @@ -616,8 +717,8 @@ class TestExtensionSkillEdgeCases: # 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.""" @@ -628,7 +729,7 @@ class TestExtensionSkillEdgeCases: ) # 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) @@ -636,5 +737,5 @@ class TestExtensionSkillEdgeCases: # 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() diff --git a/tests/test_extensions.py b/tests/test_extensions.py index cd0f9ba44..92b9839ac 100644 --- a/tests/test_extensions.py +++ b/tests/test_extensions.py @@ -22,6 +22,7 @@ from specify_cli.extensions import ( ExtensionRegistry, ExtensionManager, CommandRegistrar, + HookExecutor, ExtensionCatalog, ExtensionError, ValidationError, @@ -759,6 +760,81 @@ $ARGUMENTS 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_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 + 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 @@ -875,11 +951,11 @@ $ARGUMENTS 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 +1020,7 @@ Agent __AGENT__ 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 +1070,12 @@ Agent __AGENT__ 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 +1132,7 @@ Then {AGENT_SCRIPT} 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() @@ -1065,6 +1141,62 @@ Then {AGENT_SCRIPT} 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 ): @@ -1121,7 +1253,7 @@ Then {AGENT_SCRIPT} 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 +3363,128 @@ class TestExtensionPriorityBackwardsCompatibility: 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 + + 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 + + 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_presets.py b/tests/test_presets.py index 95dca4122..1b2704c57 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -1942,10 +1942,10 @@ class TestInitOptions: 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 @@ -1995,6 +1995,26 @@ class TestPresetSkills: 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_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" @@ -2040,6 +2060,52 @@ class TestPresetSkills: 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") + 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") @@ -2054,6 +2120,304 @@ class TestPresetSkills: 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 + assert "# Speckit Fakeext Cmd Skill" in content + + 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 + 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.""" + 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") + 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", []) + + 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", []) + + def test_kimi_preset_skill_override_resolves_script_placeholders(self, project_dir, temp_dir): + """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") + (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" + "Review templates/checklist.md and memory/constitution.md\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 ".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 + + 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.""" diff --git a/tests/test_timestamp_branches.py b/tests/test_timestamp_branches.py index 0cf631e96..7e4f88ed0 100644 --- a/tests/test_timestamp_branches.py +++ b/tests/test_timestamp_branches.py @@ -14,6 +14,7 @@ import pytest 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" @@ -147,6 +148,24 @@ class TestSequentialBranch: 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}" + + 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 ───────────────────────────────────────────────