feat: implement ai-skills command line switch#1600
feat: implement ai-skills command line switch#1600dhilipkumars wants to merge 2 commits intogithub:mainfrom
Conversation
|
@mnriem i re-worked on this a bit, i spent a little bit of time on this and realized each coding agent uses its own directory to store project specific skills, so sort of re-wrote it. PTAL. eg:
|
There was a problem hiding this comment.
Pull request overview
This PR implements a new --ai-skills command-line flag for the specify init command that installs Prompt.MD templates from templates/commands/ as agent skills following the agentskills.io specification. The feature creates SKILL.md files in agent-specific skills directories (e.g., .claude/skills/, .gemini/skills/) and optionally cleans up duplicate command files to prevent conflicts.
Changes:
- Added
install_ai_skills()function to convert 9 command templates into agent skills with enhanced descriptions - Added
_get_skills_dir()helper to resolve agent-specific skills directories with override support - Added validation requiring
--aiflag when using--ai-skills - Version bumped from 0.1.0 to 0.1.1
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| src/specify_cli/init.py | Core implementation: added yaml import, AGENT_SKILLS_DIR_OVERRIDES/SKILL_DESCRIPTIONS constants, install_ai_skills() and _get_skills_dir() functions, --ai-skills CLI option, validation logic, and tracker integration |
| pyproject.toml | Version bump from 0.1.0 to 0.1.1 |
| README.md | Added documentation for --ai-skills flag in options table and usage examples |
| CHANGELOG.md | Added 0.1.1 release notes documenting the new agent skills installation feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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``. | ||
| """ | ||
| 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: | ||
| return project_path / agent_folder.rstrip("/") / "skills" | ||
|
|
||
| return project_path / DEFAULT_SKILLS_DIR | ||
|
|
||
|
|
||
| def install_ai_skills(project_path: Path, selected_ai: str, tracker: StepTracker | None = None) -> bool: | ||
| """Install Prompt.MD files from templates/commands/ as agent skills. | ||
|
|
||
| Skills are written to the agent-specific skills directory following the | ||
| `agentskills.io <https://agentskills.io/specification>`_ specification. | ||
| Installation is additive — existing files are never removed and prompt | ||
| command files in the agent's commands directory are left untouched. | ||
|
|
||
| Args: | ||
| project_path: Target project directory. | ||
| selected_ai: AI assistant key from ``AGENT_CONFIG``. | ||
| tracker: Optional progress tracker. | ||
|
|
||
| Returns: | ||
| ``True`` if at least one skill was installed, ``False`` otherwise. | ||
| """ | ||
| # Locate the bundled templates directory | ||
| script_dir = Path(__file__).parent.parent.parent # up from src/specify_cli/ | ||
| templates_dir = script_dir / "templates" / "commands" | ||
|
|
||
| if not templates_dir.exists(): | ||
| if tracker: | ||
| tracker.error("ai-skills", "templates/commands not found") | ||
| else: | ||
| console.print("[yellow]Warning: templates/commands directory not found, skipping skills installation[/yellow]") | ||
| return False | ||
|
|
||
| command_files = sorted(templates_dir.glob("*.md")) | ||
| if not command_files: | ||
| if tracker: | ||
| tracker.skip("ai-skills", "no command templates found") | ||
| else: | ||
| console.print("[yellow]No command templates found to install[/yellow]") | ||
| return False | ||
|
|
||
| # Resolve the correct skills directory for this agent | ||
| skills_dir = _get_skills_dir(project_path, selected_ai) | ||
| skills_dir.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| if tracker: | ||
| tracker.start("ai-skills") | ||
|
|
||
| installed_count = 0 | ||
| for command_file in command_files: | ||
| try: | ||
| content = command_file.read_text(encoding="utf-8") | ||
|
|
||
| # Parse YAML frontmatter | ||
| if content.startswith("---"): | ||
| parts = content.split("---", 2) | ||
| if len(parts) >= 3: | ||
| frontmatter = yaml.safe_load(parts[1]) | ||
| body = parts[2].strip() | ||
| else: | ||
| frontmatter = {} | ||
| body = content | ||
| else: | ||
| frontmatter = {} | ||
| body = content | ||
|
|
||
| command_name = command_file.stem | ||
| skill_name = f"speckit-{command_name}" | ||
|
|
||
| # Create skill directory (additive — never removes existing content) | ||
| skill_dir = skills_dir / skill_name | ||
| skill_dir.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| # Select the best description available | ||
| original_desc = frontmatter.get("description", "") if frontmatter else "" | ||
| enhanced_desc = SKILL_DESCRIPTIONS.get(command_name, original_desc or f"Spec-kit workflow command: {command_name}") | ||
|
|
||
| # Build SKILL.md following agentskills.io spec | ||
| skill_content = f"""--- | ||
| name: {skill_name} | ||
| description: {enhanced_desc} | ||
| compatibility: Requires spec-kit project structure with .specify/ directory | ||
| metadata: | ||
| author: github-spec-kit | ||
| source: templates/commands/{command_file.name} | ||
| --- | ||
|
|
||
| # Speckit {command_name.title()} Skill | ||
|
|
||
| {body} | ||
| """ | ||
|
|
||
| skill_file = skill_dir / "SKILL.md" | ||
| skill_file.write_text(skill_content, encoding="utf-8") | ||
| installed_count += 1 | ||
|
|
||
| except Exception as e: | ||
| console.print(f"[yellow]Warning: Failed to install skill {command_file.stem}: {e}[/yellow]") | ||
| continue | ||
|
|
||
| if tracker: | ||
| if installed_count > 0: | ||
| tracker.complete("ai-skills", f"{installed_count} skills → {skills_dir.relative_to(project_path)}") | ||
| else: | ||
| tracker.error("ai-skills", "no skills installed") | ||
| else: | ||
| if installed_count > 0: | ||
| console.print(f"[green]✓[/green] Installed {installed_count} agent skills to {skills_dir.relative_to(project_path)}/") | ||
| else: | ||
| console.print("[yellow]No skills were installed[/yellow]") | ||
|
|
||
| # When skills are installed, remove the duplicate command files that were | ||
| # extracted from the template archive. This prevents the agent from | ||
| # seeing both /commands and /skills for the same functionality. | ||
| if installed_count > 0: | ||
| agent_config = AGENT_CONFIG.get(selected_ai, {}) | ||
| agent_folder = agent_config.get("folder", "") | ||
| if agent_folder: | ||
| commands_dir = project_path / agent_folder.rstrip("/") / "commands" | ||
| if commands_dir.exists(): | ||
| removed = 0 | ||
| for cmd_file in list(commands_dir.glob("speckit.*")): | ||
| try: | ||
| cmd_file.unlink() | ||
| removed += 1 | ||
| except OSError: | ||
| pass | ||
| # Remove the commands directory if it is now empty | ||
| try: | ||
| if commands_dir.exists() and not any(commands_dir.iterdir()): | ||
| commands_dir.rmdir() | ||
| except OSError: | ||
| pass | ||
|
|
||
| return installed_count > 0 |
There was a problem hiding this comment.
The new install_ai_skills function and _get_skills_dir helper function lack test coverage. Given that the repository has comprehensive test coverage for the extensions module (tests/test_extensions.py with 40+ test cases), the same standard should apply to this new functionality. Consider adding unit tests that verify: 1) correct skills directory resolution for different agents, 2) proper YAML frontmatter parsing and skill file generation, 3) cleanup of duplicate command files, 4) handling of missing templates directory, 5) error handling for malformed templates, and 6) validation of the --ai-skills flag requiring --ai.
| # Parse YAML frontmatter | ||
| if content.startswith("---"): | ||
| parts = content.split("---", 2) | ||
| if len(parts) >= 3: | ||
| frontmatter = yaml.safe_load(parts[1]) | ||
| body = parts[2].strip() | ||
| else: | ||
| frontmatter = {} | ||
| body = content | ||
| else: | ||
| frontmatter = {} | ||
| body = content |
There was a problem hiding this comment.
The frontmatter parsing logic using split("---", 2) is less robust than the approach used in the extensions module. The extensions.py module at lines 683-708 uses content.find("---", 3) to locate the second delimiter, which is more precise. While split with maxsplit=2 will work in most cases, using find() to locate the exact position of the second delimiter is a more reliable approach. Consider refactoring to use the parse_frontmatter method from the CommandRegistrar class or implementing a similar approach.
| Installation is additive — existing files are never removed and prompt | ||
| command files in the agent's commands directory are left untouched. |
There was a problem hiding this comment.
The docstring claims that "prompt command files in the agent's commands directory are left untouched" (line 1034), but this is incorrect. The implementation at lines 1133-1154 explicitly removes command files matching the pattern "speckit.*" from the commands directory and may even delete the commands directory if it becomes empty. This is a direct contradiction. The docstring should be updated to accurately reflect that the function removes duplicate command files from the commands directory after installing skills.
| Installation is additive — existing files are never removed and prompt | |
| command files in the agent's commands directory are left untouched. | |
| Installation is additive with respect to skills — existing skills are not | |
| removed. After installing skills, duplicate prompt command files generated | |
| by this tool (matching ``speckit.*``) may be removed from the agent's | |
| commands directory, and the commands directory itself may be deleted if it | |
| becomes empty. User-created non-``speckit.*`` command files are left | |
| untouched. |
| skill_content = f"""--- | ||
| name: {skill_name} | ||
| description: {enhanced_desc} | ||
| compatibility: Requires spec-kit project structure with .specify/ directory | ||
| metadata: | ||
| author: github-spec-kit | ||
| source: templates/commands/{command_file.name} | ||
| --- | ||
|
|
||
| # Speckit {command_name.title()} Skill | ||
|
|
||
| {body} | ||
| """ | ||
|
|
There was a problem hiding this comment.
The YAML frontmatter content in the skill file is being constructed using string formatting (f-string), which can produce invalid YAML if the description or other fields contain special YAML characters like colons, quotes, or newlines. For example, if enhanced_desc contains a colon (which it does in some cases like "Structured clarification workflow for underspecified requirements. Use before planning..."), the YAML could be malformed. Consider using yaml.dump() to properly serialize the frontmatter data structure instead of string formatting, or at minimum escape/quote values appropriately.
| skill_content = f"""--- | |
| name: {skill_name} | |
| description: {enhanced_desc} | |
| compatibility: Requires spec-kit project structure with .specify/ directory | |
| metadata: | |
| author: github-spec-kit | |
| source: templates/commands/{command_file.name} | |
| --- | |
| # Speckit {command_name.title()} Skill | |
| {body} | |
| """ | |
| frontmatter_data = { | |
| "name": skill_name, | |
| "description": enhanced_desc, | |
| "compatibility": "Requires spec-kit project structure with .specify/ directory", | |
| "metadata": { | |
| "author": "github-spec-kit", | |
| "source": f"templates/commands/{command_file.name}", | |
| }, | |
| } | |
| frontmatter_yaml = yaml.safe_dump(frontmatter_data, sort_keys=False).rstrip() | |
| skill_content = ( | |
| f"---\n" | |
| f"{frontmatter_yaml}\n" | |
| f"---\n\n" | |
| f"# Speckit {command_name.title()} Skill\n\n" | |
| f"{body}\n" | |
| ) |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 19 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if content.startswith("---"): | ||
| parts = content.split("---", 2) | ||
| if len(parts) >= 3: | ||
| frontmatter = yaml.safe_load(parts[1]) |
There was a problem hiding this comment.
The yaml.safe_load() call could return None if the frontmatter section is empty (e.g., "---\n---"). This would cause issues on line 1096 where frontmatter.get() is called. Consider adding a check to handle None values: frontmatter = yaml.safe_load(parts[1]) or {}
| frontmatter = yaml.safe_load(parts[1]) | |
| frontmatter = yaml.safe_load(parts[1]) or {} |
| # Agent-specific skill directory overrides for agents whose skills directory | ||
| # doesn't follow the standard <agent_folder>/skills/ pattern | ||
| AGENT_SKILLS_DIR_OVERRIDES = { | ||
| "codex": ".agents/skills", # per https://developers.openai.com/codex/skills |
There was a problem hiding this comment.
The URL reference 'https://developers.openai.com/codex/skills' in the comment may not be accurate. OpenAI Codex has been deprecated, and this URL pattern doesn't match the current OpenAI documentation structure. Consider verifying this URL or removing the reference if it cannot be confirmed.
| "codex": ".agents/skills", # per https://developers.openai.com/codex/skills | |
| "codex": ".agents/skills", # legacy Codex agent layout override |
|
|
||
| - **Agent Skills Installation**: New `--ai-skills` CLI option to install Prompt.MD templates as agent skills following [agentskills.io specification](https://agentskills.io/specification) | ||
| - Skills are installed to agent-specific directories (e.g., `.claude/skills/`, `.gemini/skills/`, `.github/skills/`) | ||
| - Codex uses `.agents/skills/` per [OpenAI Codex docs](https://developers.openai.com/codex/skills) |
There was a problem hiding this comment.
The URL reference 'https://developers.openai.com/codex/skills' may not be accurate. OpenAI Codex has been deprecated, and this URL pattern doesn't match the current OpenAI documentation structure. Consider verifying this URL or removing the reference if it cannot be confirmed.
| - Codex uses `.agents/skills/` per [OpenAI Codex docs](https://developers.openai.com/codex/skills) | |
| - Codex uses `.agents/skills/` following the legacy OpenAI Codex directory convention |
| def test_skills_installed_with_correct_structure(self, project_dir, templates_dir): | ||
| """Verify SKILL.md files have correct agentskills.io structure.""" | ||
| # Directly call install_ai_skills with a patched templates dir path | ||
| import specify_cli | ||
|
|
||
| orig_file = specify_cli.__file__ | ||
| # We need to make Path(__file__).parent.parent.parent resolve to temp root | ||
| fake_init = templates_dir.parent.parent / "src" / "specify_cli" / "__init__.py" | ||
| fake_init.parent.mkdir(parents=True, exist_ok=True) | ||
| fake_init.touch() | ||
|
|
||
| with patch.object(specify_cli, "__file__", str(fake_init)): | ||
| result = install_ai_skills(project_dir, "claude") | ||
|
|
||
| assert result is True | ||
|
|
||
| skills_dir = project_dir / ".claude" / "skills" | ||
| assert skills_dir.exists() | ||
|
|
||
| # Check that skill directories were created | ||
| skill_dirs = sorted([d.name for d in skills_dir.iterdir() if d.is_dir()]) | ||
| assert skill_dirs == ["speckit-plan", "speckit-specify", "speckit-tasks"] | ||
|
|
||
| # Verify SKILL.md content for speckit-specify | ||
| skill_file = skills_dir / "speckit-specify" / "SKILL.md" | ||
| assert skill_file.exists() | ||
| content = skill_file.read_text() | ||
|
|
||
| # Check agentskills.io frontmatter | ||
| assert content.startswith("---\n") | ||
| assert "name: speckit-specify" in content | ||
| assert "description:" in content | ||
| assert "compatibility:" in content | ||
| assert "metadata:" in content | ||
| assert "author: github-spec-kit" in content | ||
| assert "source: templates/commands/specify.md" in content | ||
|
|
||
| # Check body content is included | ||
| assert "# Speckit Specify Skill" in content | ||
| assert "Run this to create a spec." in content | ||
|
|
There was a problem hiding this comment.
The tests don't verify that generated SKILL.md files contain valid, parseable YAML frontmatter. Consider adding a test that reads back the generated SKILL.md and parses the YAML to ensure it's valid. This would catch issues with special characters in descriptions that could break YAML parsing.
| "Body without frontmatter.\n", | ||
| encoding="utf-8", | ||
| ) | ||
|
|
There was a problem hiding this comment.
Consider adding a test case for templates with empty YAML frontmatter (e.g., "---\n---\n# Content"). This would test the edge case where yaml.safe_load() returns None, which could cause issues in the current implementation.
| # Template with empty YAML frontmatter (safe_load() returns None) | |
| (tpl_root / "empty_frontmatter.md").write_text( | |
| "---\n" | |
| "---\n" | |
| "\n" | |
| "# Empty Frontmatter Command\n" | |
| "\n" | |
| "Body with empty frontmatter.\n", | |
| encoding="utf-8", | |
| ) |
|
|
||
| def test_existing_commands_preserved_gemini(self, project_dir, templates_dir, commands_dir_gemini): | ||
| """install_ai_skills must NOT remove pre-existing .gemini/commands files.""" | ||
| import specify_cli |
There was a problem hiding this comment.
Module 'specify_cli' is imported with both 'import' and 'import from'.
|
|
||
| def test_commands_dir_not_removed(self, project_dir, templates_dir, commands_dir_claude): | ||
| """install_ai_skills must not remove the commands directory.""" | ||
| import specify_cli |
There was a problem hiding this comment.
Module 'specify_cli' is imported with both 'import' and 'import from'.
|
|
||
| def test_no_commands_dir_no_error(self, project_dir, templates_dir): | ||
| """No error when agent has no commands directory at all.""" | ||
| import specify_cli |
There was a problem hiding this comment.
Module 'specify_cli' is imported with both 'import' and 'import from'.
| # Directly call install_ai_skills with a patched templates dir path | ||
| import specify_cli | ||
|
|
||
| orig_file = specify_cli.__file__ |
There was a problem hiding this comment.
Variable orig_file is not used.
| orig_file = specify_cli.__file__ |
|
|
||
| # Replicate the init() logic | ||
| if ai_skills and not here: | ||
| import shutil as _shutil |
There was a problem hiding this comment.
This statement is unreachable.
Implements agent skills with a command line switch
--ai-skills, needs to be used in combination with--ai.Additional help text when you
specify init --helpSample run
will result in
ai coding agent: anti-gravity