feat(slash-commands): add generator for AI assistant native slash commands #19
Conversation
- add command format enum and agent registry for 14 tools - implement auto-detection helpers with deterministic ordering - add targeted pytest coverage and close Task 1.0 foundations Related to Task 1.0 in Spec
…pport
- Add CommandGenerator base class with Markdown and TOML subclasses
- Implement agent override support for descriptions, arguments, and enabled flags
- Add placeholder replacement for $ARGUMENTS and {{args}}
- Support argument merging for agent-specific overrides
- Add comprehensive test coverage for both generator types
- Update MarkdownPrompt to include agent_overrides field
Related to T2.4 in Spec
- Add _normalize_output function for consistent whitespace/encoding - Normalize line endings to LF, remove trailing whitespace - Add snapshot-style regression tests for both generators - Tests verify output structure, valid TOML, and no trailing whitespace Related to T2.5 in Spec
- All subtasks 2.1-2.5 are complete - Demo criteria satisfied: generators transform MarkdownPrompt objects - All tests passing: pytest tests/test_generators.py -v
- Add SlashCommandWriter class orchestrating prompt loading and generation - Support single and multi-agent generation with dry-run mode - Create parent directories automatically - Add comprehensive test coverage for writer functionality - Export writer interface from slash_commands package Related to T3.0 in Spec
- Add Typer CLI app with comprehensive command options - Support --agents, --list-agents, --dry-run, --prompts-dir flags - Auto-detect configured agents from filesystem - Add comprehensive test coverage for CLI functionality - Register CLI entry point in pyproject.toml - Export CLI from slash_commands package Related to T4.0 in Spec
- Add prompt_overwrite_action and create_backup utilities - Implement overwrite detection and handling in SlashCommandWriter - Add support for cancel, overwrite, backup, and overwrite-all actions - Track backups created and report them in CLI summary - Add tests for overwrite handling scenarios - Update CLI to honor --yes flag for auto-overwrite Related to T5.1, T5.2, T5.3 in Spec
- Create comprehensive documentation for slash command generator - Document CLI usage, supported agents, and file formats - Add overview of overwrite handling and backup features - Update README with link to new documentation Related to T5.4 in Spec
- All overwrite handling features implemented and tested - Documentation added for slash command generator - Dependencies verified and tests passing Related to T5.0 completion in Spec
- Add tomli-w>=1.0.0 to dependencies for TOML generation - Update documentation to use 'uv run' prefix for all commands - Clarify that uv projects require 'uv run' to execute scripts Fixes issue where sdd-generate-commands was not found
- Implement interactive overwrite prompt using questionary.select() - Add interactive agent selection UI with checkboxes - Fix documentation agent list to match actual 14 agents - Update directory structure examples to reflect actual agents - Fix backup timestamp format to use YYYYMMDD-HHMMSS - Update TOML format example to match actual generator output - Add --detection-path option for flexible agent detection - Add integration tests for interactive flows - Add questionary dependency to pyproject.toml - Add spec addendum documenting detection default location oversight - Add task 8.0 to fix default detection to home directory All tests passing (56 tests)
Add Reference column with home and documentation links for all 14 agents
- Implement consistent exit codes (0=success, 1=user cancel, 2=validation error, 3=I/O error) - Add comprehensive error messages with guidance for common failure scenarios - Expand troubleshooting section in docs with detailed solutions - Update examples to show actual command output and file structures - Add backup file cleanup documentation - Add --target-dir alias for --base-path option Related to task 7.0 in Spec 0003
- Replace --base-path/--target-dir with single --target-path option - Set --target-path default to home directory instead of current directory - Set --detection-path default to home directory instead of current directory - Update all tests to use --target-path instead of --base-path - Update documentation to reflect new default behavior - Add detection path section to documentation Related to task 8.0 in Spec 0003
- Add Rich table formatting for --list-agents output - Add Target Path column showing command directory for each agent - Add Detected column showing whether agent detection directories exist - Use checkmarks (✓) and crosses (✗) to indicate detection status - Apply color styling to table columns
- Add ~/ prefix to target paths to show home directory relative paths - Change ✗ marks to red color for better visual distinction - Use Rich markup syntax for colored checkmarks and crosses
- Change detection logic to check if command directory exists - Previously checked detection directories which could be misleading - Now shows ✓ only when the actual command directory exists - Fixes issue where agents were marked as detected when command dir didn't exist
- Change Cursor from .cursorrules/commands to .cursor/commands - Change Amp from .amp/commands to .agents/commands - Change Auggie CLI from .auggie/commands to .augment/commands - Change Kilo Code from .kilo/commands to .kilocode/rules - Change opencode from .opencode/commands to .config/opencode/commands - Update detection directories to match command directories - Update tests to match new configurations
- Remove amazon-q-developer, amp, auggie-cli, crush, github-copilot, kilo-code, opencode, qwen-code, roo-code - Rename codebuddy-cli to vs-code - Update paths to verified working locations: - Cursor: .cursor/rules/.mdc - Codex: .codex/prompts - Windsurf: .codeium/windsurf/global_workflows - VS Code: .config/Code/User/prompts/.prompt.md - Update tests to match new configuration - Reduce from 15 to 6 supported agents
Update Cursor agent configuration to use .cursor/commands instead of .cursor/rules and change file extension from .mdc to .md. Also remove legacy .cursorrules from detection directories. This aligns with official Cursor documentation for slash commands.
…d commands Add two new metadata fields to all generated slash command files: - version: reads from pyproject.toml via centralized __version__ constant - updated_at: ISO format timestamp of when command was generated Centralize version management by reading from pyproject.toml in mcp_server/__init__.py instead of hardcoding. Generators now import and use this version, making it the single source of truth. Both Markdown and TOML generators include these fields in their metadata sections, and tests verify their presence.
Refactored TomlCommandGenerator to match the official Gemini CLI custom commands spec (https://geminicli.com/docs/cli/custom-commands/). Changes: - Simplify TOML structure to flat format with prompt and description fields - Preserve {{args}} placeholder for Gemini CLI context-aware injection - Add metadata fields for version tracking (ignored by Gemini CLI) - Remove unused helper methods and simplify implementation - Update tests to validate new structure and {{args}} preservation Benefits: - Commands are now properly detected by Gemini CLI - Maintains version tracking capabilities for our tooling - Follows official Gemini CLI specification exactly - All 58 tests passing
Add a new cleanup command to the slash command generator that can detect and remove generated command files and backups based on metadata. Features: - Detect generated files via metadata (source_prompt, version) - Support both Markdown and TOML file formats - Detect backup files by timestamp pattern - Rich table formatting for file listing - Confirmation prompt with warning panel - Dry-run mode for safety - Comprehensive test coverage The cleanup command uses Rich formatting with tables for file listings and panels for warnings and summaries, improving the user experience compared to plain text output.
Add specification document 0004-spec-review-fixes.md detailing the code review findings that need to be addressed, along with a comprehensive task list breaking down the implementation into 5 demoable units of work with 23 actionable subtasks. The spec covers fixes for: - Package discovery configuration - TOML reading documentation - Generated content validation tests - Centralized version management - Troubleshooting documentation
- Added slash_commands to packages list in pyproject.toml - Verified package installs successfully with uv pip install -e . - Verified CLI works without import errors - Verified slash_commands directory is included in installed package Related to T1.0 in Spec 0004
- Added Python Version Requirements section to docs - Clarified Python 3.12+ requirement and tomllib availability - Added comment to writer.py explaining tomllib is from stdlib - Verified tomllib import works correctly - All tests pass successfully Related to T2.0 in Spec 0004
- Created tests/test_validation.py with validation tests - Added test_toml_round_trip_parsing to verify TOML can be parsed back - Added test_yaml_frontmatter_parsing to validate YAML structure - Added test_invalid_toml_content_caught to catch invalid TOML - Added test_invalid_yaml_content_caught to catch invalid YAML - All 75 tests pass including 5 new validation tests Related to T3.0 in Spec 0004
- Created __version__.py at project root for centralized version management - Updated slash_commands/generators.py to import from __version__ instead of mcp_server - Updated mcp_server/__init__.py to import from shared __version__.py module - Version now read from pyproject.toml via single source of truth - All 75 tests pass successfully Related to T4.0 in Spec 0004
- Fix dry-run file count to report actual file count regardless of mode - Add explicit UTF-8 encoding when writing generated files - Fix single-element tuple in test detection_dirs - Move prompts directory from packages to force-include in pyproject.toml
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
pyproject.toml (1)
82-92: Fix the coverage exclude pattern for__main__guard.Line 88 still contains an invalid regex pattern that won't match Python code. The pattern
"if __name__ == .__main__.:"is looking for literal dots around "main", but should match the actual Python syntax with quotes.Apply this diff to fix the pattern:
[tool.coverage.report] exclude_lines = [ "pragma: no cover", "def __repr__", "raise AssertionError", "raise NotImplementedError", - "if __name__ == .__main__.:", + "if __name__ == [\"']__main__[\"']:", "if TYPE_CHECKING:", "class .*\\bProtocol\\):", "@(abc\\.)?abstractmethod", ]This regex will correctly match both
if __name__ == "__main__":andif __name__ == '__main__':.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
pyproject.toml(3 hunks)slash_commands/writer.py(1 hunks)tests/test_cli.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_cli.py
🧰 Additional context used
🧬 Code graph analysis (1)
slash_commands/writer.py (3)
mcp_server/prompt_utils.py (2)
MarkdownPrompt(19-40)load_markdown_prompt(43-85)slash_commands/config.py (3)
AgentConfig(18-31)get_agent_config(94-100)list_agent_keys(88-91)slash_commands/generators.py (2)
CommandGenerator(271-282)create(275-282)
🪛 GitHub Actions: Run tests and linting
pyproject.toml
[error] 1-1: pytest run completed with exit code 1 due to the failing test.
🔇 Additional comments (5)
pyproject.toml (3)
13-17: ✓ Typer version constraint resolved.The earlier flagged issue with
typer>=0.20.0has been corrected totyper>=0.19.0, which matches the latest released version on PyPI. Questionary and tomli-w constraints are also valid.
39-39: ✓ Prompts packaging corrected.The earlier issue has been fixed:
promptshas been removed from thepackagesarray and is now properly included underforce-includeas data rather than as an importable package.Also applies to: 43-44
75-75: Verify pytest configuration and pipeline failure cause.The pytest addopts now extends coverage to include
slash_commandswith HTML reporting. The pipeline failed with exit code 1. This could be due to either:
- The broken coverage pattern (line 88) causing coverage collection to fail
- Actual test failures in the slash_commands test suite
Please verify:
- That the coverage exclude pattern fix above resolves the pipeline failure.
- If test failures remain after fixing the regex, check the test output in the pipeline logs to identify which tests are failing.
slash_commands/writer.py (2)
123-123: Previous issues have been resolved.Both prior review concerns have been addressed:
- Line 123 now correctly reports file count using
len(files)regardless of dry-run mode- Line 181 now explicitly specifies UTF-8 encoding when writing files
Also applies to: 181-181
25-66: Well-implemented with solid error handling and design choices.The implementation demonstrates several good practices:
shutil.copy2(line 64) preserves file metadata in backupssorted()(line 136) ensures deterministic prompt orderingre.escape(line 257) properly escapes special characters in regex patterns- Explicit UTF-8 encoding throughout (lines 181, 284)
- Comprehensive exception handling for file I/O and parsing errors
- Clean separation of concerns across helper functions and private methods
Also applies to: 129-377
- Add local pre-commit hook to run pytest before push - Fix writer to correctly report files_written=0 in dry_run mode - Prevents CI test failures from incorrect file counting
- Fix coverage exclude pattern for __main__ guard in pyproject.toml - Add PackageNotFoundError handling for version fallback in generators.py - Store only basename in source_path to avoid leaking absolute paths - Fix agents=None to default to all supported agents in writer.py These changes address 4 remaining CodeRabbit AI review comments that were not covered in the previous fixes.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
slash_commands/writer.py (2)
90-90: Fix default agent selection to honor docstring (agents=None → all agents).Currently sets an empty list, producing zero outputs. Default to all supported agents when None is passed.
Apply this diff:
- self.agents = agents or [] + # Use all supported agents when None was provided; preserve explicit empty list. + self.agents = list(agents) if agents is not None else list(list_agent_keys())
125-131: Correct files_written during --dry-run (report candidates, not 0).Dry-run currently reports 0 even when many files would be generated.
Apply this diff:
return { "prompts_loaded": len(prompts), - "files_written": files_written, + "files_written": len(files), "files": files, "prompts": [{"name": p.name, "path": str(p.path)} for p in prompts], "backups_created": self._backups_created, }
🧹 Nitpick comments (2)
.pre-commit-config.yaml (1)
36-44: Consider adding pytest arguments to optimize pre-push performance.The hook runs the full test suite without arguments, which could block pushes during long test runs. While this aligns well with the PR's emphasis on test coverage, consider adding arguments for better ergonomics:
- repo: local hooks: - id: run-tests name: Run pytest before push - entry: pytest + entry: pytest --tb=short language: system stages: [pre-push] pass_filenames: false always_run: trueAlternatively, you might consider adding a
--timeoutor optional skip mechanism for developers who need faster iteration cycles during active development. The configuration is correct as-is, but these adjustments can improve developer experience.Verify that the pytest configuration in
pyproject.tomlis set up to run the appropriate test suite efficiently (e.g., excludes slow/integration tests if applicable).slash_commands/writer.py (1)
247-256: Normalize result paths to strings; cast back to Path in cleanup.Unify public results for easier JSON serialization and CLI output. Then convert to Path when deleting.
Apply this diff:
- found_files.append({ - "path": file_path, + found_files.append({ + "path": str(file_path), "agent": agent.key, "agent_display_name": agent.display_name, "type": "command", "reason": "Has generated metadata", }) @@ - found_files.append({ - "path": file_path, + found_files.append({ + "path": str(file_path), "agent": agent.key, "agent_display_name": agent.display_name, "type": "backup", "reason": "Matches backup pattern", }) @@ - file_path = file_info["path"] + file_path = Path(file_info["path"]) if not dry_run: try: file_path.unlink() deleted_files.append(file_info) except OSError as e: errors.append({"path": str(file_path), "error": str(e)})Also applies to: 263-270, 365-371
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.pre-commit-config.yaml(1 hunks)slash_commands/writer.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
slash_commands/writer.py (3)
mcp_server/prompt_utils.py (2)
MarkdownPrompt(19-40)load_markdown_prompt(43-85)slash_commands/config.py (3)
AgentConfig(18-31)get_agent_config(94-100)list_agent_keys(88-91)slash_commands/generators.py (2)
CommandGenerator(271-282)create(275-282)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
slash_commands/writer.py (2)
125-131: Report would‑be writes too (without changing existing semantics).Currently files_written is 0 during dry‑run, making totals unclear. Add files_to_write to show candidates. This was flagged before.
return { "prompts_loaded": len(prompts), - "files_written": files_written, + "files_written": files_written, + "files_to_write": len(files), "files": files, "prompts": [{"name": p.name, "path": str(p.path)} for p in prompts], "backups_created": self._backups_created, }
165-168: Sanitize output filenames to prevent path traversal/invalid chars.Using prompt.name directly can write outside the target dir or create invalid filenames.
Apply this diff:
- output_path = ( - self.base_path / agent.command_dir / f"{prompt.name}{agent.command_file_extension}" - ) + # Sanitize file stem: drop directories and restrict to safe chars + safe_stem = Path(prompt.name).name + safe_stem = re.sub(r"[^A-Za-z0-9._-]+", "-", safe_stem).strip("-_.") or "command" + filename = f"{safe_stem}{agent.command_file_extension}" + output_path = self.base_path / agent.command_dir / filename
🧹 Nitpick comments (6)
slash_commands/generators.py (2)
200-201: Use safe_dump for YAML.Prefer yaml.safe_dump to avoid emitting non‑standard tags.
- yaml_content = yaml.dump(frontmatter, allow_unicode=True, sort_keys=False) + yaml_content = yaml.safe_dump(frontmatter, allow_unicode=True, sort_keys=False)
121-135: Unused helper (_build_arguments_section_toml).This function isn’t referenced. Remove it or wire it where needed to avoid dead code.
- def _build_arguments_section_toml(arguments: list[PromptArgumentSpec]) -> str: - """Build a TOML-formatted arguments section.""" - if not arguments: - return "{ required = {}, optional = {} }" - - required = {} - optional = {} - for arg in arguments: - if arg.required: - required[arg.name] = arg.description or "" - else: - optional[arg.name] = arg.description or "" - - return f"{{ required = {required}, optional = {optional} }}" + # (removed: unused helper)slash_commands/writer.py (4)
236-237: Respect empty agent lists; only default when agents is None.agents or list_agent_keys() treats [] as “all agents.” Use an explicit None check.
- agent_keys = agents or list_agent_keys() + agent_keys = list_agent_keys() if agents is None else agents
249-256: Normalize returned path type (str) for consistency.generate() returns str paths; find_generated_files returns Path objects. Unify to str to simplify consumers.
- found_files.append({ - "path": file_path, + found_files.append({ + "path": str(file_path), "agent": agent.key, "agent_display_name": agent.display_name, "type": "command", "reason": "Has generated metadata", }) ... - found_files.append({ - "path": file_path, + found_files.append({ + "path": str(file_path), "agent": agent.key, "agent_display_name": agent.display_name, "type": "backup", "reason": "Matches backup pattern", })Also applies to: 265-271
366-373: Coerce path to Path in cleanup to match normalized string paths.If adopting the normalization above, ensure unlink uses Path().
- file_path = file_info["path"] + file_path = Path(file_info["path"]) if not dry_run: try: file_path.unlink()
60-62: Optional: use UTC in backup filenames for consistency.Minor; aligns with UTC meta elsewhere and avoids TZ ambiguity.
-from datetime import datetime +from datetime import UTC, datetime ... - timestamp = datetime.now().strftime("%Y%m%d-%H%M%S") + timestamp = datetime.now(UTC).strftime("%Y%m%d-%H%M%S")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
pyproject.toml(3 hunks)slash_commands/generators.py(1 hunks)slash_commands/writer.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
slash_commands/generators.py (2)
mcp_server/prompt_utils.py (2)
MarkdownPrompt(19-40)PromptArgumentSpec(12-15)slash_commands/config.py (2)
AgentConfig(18-31)CommandFormat(10-14)
slash_commands/writer.py (3)
mcp_server/prompt_utils.py (2)
MarkdownPrompt(19-40)load_markdown_prompt(43-85)slash_commands/config.py (3)
AgentConfig(18-31)get_agent_config(94-100)list_agent_keys(88-91)slash_commands/generators.py (5)
CommandGenerator(275-286)generate(27-30)generate(167-202)generate(230-268)create(279-286)
🔇 Additional comments (5)
pyproject.toml (3)
13-17: ✅ All previous issues successfully resolved; configuration properly extended for slash_commands package.Excellent follow-up on the previous review comments:
- Typer version constraint (line 17): Corrected from unresolvable
0.20.0to valid0.19.0✓- Wheel packages (line 39): Removed
promptsfrom Python packages list ✓- Data inclusion (line 44): Correctly moved
prompts/toforce-includefor proper packaging as data ✓- Coverage pattern (line 88): Fixed
__main__guard pattern with proper quote escaping ✓The new slash_commands package integration is well-structured: new script entry (line 36) maps to the CLI module, dependencies are reasonable and at available versions, coverage configuration properly tracks both mcp_server and slash_commands (lines 75, 79), and semantic release configuration is complete with version tracking and uv.lock management (lines 97–111).
Also applies to: 36-36, 38-44, 75-75, 78-92, 95-119
95-119: Semantic release configuration looks solid but verify uv.lock Git handling.The build_command (lines 104–109) runs
uv lockand stages uv.lock viagit add, which works but assumes the release environment has Git configured. Confirm that semantic-release's execution context will successfully execute these commands, or alternatively consider whether uv.lock should be committed as part of the release or regenerated on each clone.
13-13: No issues found with PyYAML 6.0.0+ dependency specification.PyYAML 6.0.3 is the latest version available on PyPI, and PyYAML 6.0.0 is not affected by known CVEs. The dependency "pyyaml>=6.0.0" is valid and secure.
slash_commands/generators.py (2)
11-21: Version fallback is robust now.Good handling of PackageNotFoundError with a sane "0.0.0" fallback.
219-223: Nice: avoid leaking absolute paths.Using basename for meta.source_path prevents local path leakage.
- Fix agent argument overrides to actually override existing args - Sanitize prompt.name to prevent path traversal - Use safe_dump for YAML generation - Remove unused helper function - Show actual supported agent keys in error messages - Prefer Typer-controlled exits over sys.exit - Use agent.iter_detection_dirs() for consistency - Respect empty agent lists in find_generated_files - Normalize path types to strings in return values - Use UTC timestamps in backup filenames
- Use os.fspath() to convert Path objects to strings - Update tests to verify string return type - Ensures consistent return types for JSON serialization
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
slash_commands/detection.py (2)
28-31: Optional: Simplify path construction.The inner
Path(directory)wrapper is unnecessary sincebase_path / directoryalready handles string operands. However, the current code is clear and correct.
34-39: Optional: Simplify path construction.Similar to
_agent_configured, the innerPath(directory)wrapper is redundant but harmless.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
slash_commands/cli.py(1 hunks)slash_commands/detection.py(1 hunks)slash_commands/generators.py(1 hunks)slash_commands/writer.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
slash_commands/cli.py (3)
slash_commands/writer.py (4)
SlashCommandWriter(69-383)generate(97-131)cleanup(349-383)find_generated_files(225-277)slash_commands/detection.py (1)
detect_agents(11-25)slash_commands/config.py (2)
get_agent_config(94-100)list_agent_keys(88-91)
slash_commands/writer.py (4)
mcp_server/prompt_utils.py (2)
MarkdownPrompt(19-40)load_markdown_prompt(43-85)slash_commands/config.py (3)
AgentConfig(18-31)get_agent_config(94-100)list_agent_keys(88-91)slash_commands/generators.py (5)
CommandGenerator(263-274)generate(27-30)generate(155-190)generate(218-256)create(267-274)slash_commands/cli.py (1)
generate(62-259)
slash_commands/detection.py (1)
slash_commands/config.py (2)
AgentConfig(18-31)iter_detection_dirs(28-31)
slash_commands/generators.py (3)
mcp_server/prompt_utils.py (2)
MarkdownPrompt(19-40)PromptArgumentSpec(12-15)slash_commands/config.py (2)
AgentConfig(18-31)CommandFormat(10-14)slash_commands/writer.py (1)
generate(97-131)
🪛 GitHub Actions: Run tests and linting
slash_commands/writer.py
[error] 1-1: Unit test failure: find_generated_files returns string paths instead of Path objects. The tests expect Path objects (e.g., generated_file) but received strings in the 'path' field for generated Markdown files.
[error] 1-1: Unit test failure: find_generated_files returns string paths instead of Path objects. The tests expect Path objects (e.g., generated_file) but received strings in the 'path' field for generated TOML files.
[error] 1-1: Unit test failure: find_generated_files returns string paths instead of Path objects. The tests expect Path objects (e.g., backup_file) but received strings in the 'path' field for backup files.
🔇 Additional comments (22)
slash_commands/detection.py (2)
11-25: LGTM: Detection logic is clean and deterministic.The function correctly preserves SUPPORTED_AGENTS ordering and filters by existence checks.
42-45: LGTM.slash_commands/generators.py (7)
11-20: LGTM: Version fallback correctly handles PackageNotFoundError.The graceful fallback to "0.0.0" ensures the module loads even when the package metadata is unavailable.
33-66: LGTM: Agent override logic correctly replaces arguments by name.The implementation properly handles override precedence while preserving base argument order.
69-83: LGTM.
86-149: LGTM: Helper functions correctly handle normalization and placeholder substitution.The conditional handling of
{{args}}vs$ARGUMENTSaligns with the different needs of Markdown and TOML generators.
152-212: LGTM: Markdown generator correctly builds frontmatter and handles metadata.The use of
prompt.path.name(basename only) avoids leaking absolute paths, anddatetime.now(UTC)provides timezone-aware timestamps.
215-260: LGTM: TOML generator follows Gemini CLI spec and handles placeholders correctly.Preserving
{{args}}for Gemini CLI while replacing$ARGUMENTSis the correct approach per the Gemini CLI documentation.
263-274: LGTM: Factory pattern correctly dispatches to format-specific generators.slash_commands/cli.py (7)
30-58: LGTM: Interactive agent selection handles cancellation gracefully.
119-149: Consider: --list-agents always uses home directory.The
--list-agentsflag checks agent directories underPath.home()regardless of--detection-pathor--target-pathoptions. This is reasonable for a quick overview, but users might expect it to respect the detection path. If this is intentional, the current behavior is acceptable.
151-189: LGTM: Agent detection and selection logic is well-structured.The fallback chain and error messages provide clear guidance to users.
191-245: LGTM: Exception handling is comprehensive and user-friendly.The error messages and exit codes provide clear guidance for troubleshooting.
247-260: LGTM: Summary output is clear and informative.
262-396: LGTM: Cleanup command provides clear confirmation prompts and summary.The grouped table display and warning panel enhance user experience.
399-405: LGTM.slash_commands/writer.py (6)
25-48: LGTM: Overwrite action prompt handles user cancellation correctly.
51-66: LGTM: Backup creation uses UTC timestamps and preserves file metadata.
72-95: LGTM: Constructor correctly defaults to all supported agents when agents=None.The
if agents is not Nonecheck properly distinguishes betweenNoneand an empty list.
97-131: Verify: files_written count during dry-run.A past review comment suggested that
files_writtenshould reflect the count of candidate files regardless of dry-run mode. Currently, Line 122-123 only incrementsfiles_writtenwhennot self.dry_run, so dry-runs would reportfiles_written: 0even when files list contains entries. The past comment suggested usinglen(files)at Line 127. Please confirm whether this was intentionally changed or if the past fix needs to be reapplied.
145-194: LGTM: File generation correctly sanitizes filenames and uses UTF-8 encoding.The sanitization prevents path traversal, and explicit UTF-8 encoding ensures cross-platform compatibility.
349-383: Update: Adjust cleanup method after fixing find_generated_files.After changing
find_generated_filesto returnPathobjects instead of strings, Line 368 should be updated:for file_info in found_files: - file_path = Path(file_info["path"]) + file_path = file_info["path"] if not dry_run:Similarly, Line 374 converts back to string for error reporting, which is correct.
| def find_generated_files( | ||
| self, agents: list[str] | None = None, include_backups: bool = True | ||
| ) -> list[dict[str, Any]]: | ||
| """Find all files generated by this tool. | ||
| Args: | ||
| agents: List of agent keys to search. If None, searches all supported agents. | ||
| include_backups: If True, includes backup files in the results. | ||
| Returns: | ||
| List of dicts with keys: path, agent, agent_display_name, type, reason | ||
| """ | ||
| found_files = [] | ||
| agent_keys = list_agent_keys() if agents is None else agents | ||
|
|
||
| for agent_key in agent_keys: | ||
| try: | ||
| agent = get_agent_config(agent_key) | ||
| command_dir = self.base_path / agent.command_dir | ||
|
|
||
| if not command_dir.exists(): | ||
| continue | ||
|
|
||
| # Check for regular command files | ||
| for file_path in command_dir.glob(f"*{agent.command_file_extension}"): | ||
| if self._is_generated_file(file_path, agent): | ||
| found_files.append({ | ||
| "path": str(file_path), | ||
| "agent": agent.key, | ||
| "agent_display_name": agent.display_name, | ||
| "type": "command", | ||
| "reason": "Has generated metadata", | ||
| }) | ||
|
|
||
| # Check for backup files | ||
| if include_backups: | ||
| # Look for files matching the backup pattern: *.extension.timestamp.bak | ||
| escaped_ext = re.escape(agent.command_file_extension) | ||
| pattern = re.compile(rf".*{escaped_ext}\.\d{{8}}-\d{{6}}\.bak$") | ||
| for file_path in command_dir.iterdir(): | ||
| if file_path.is_file() and pattern.match(file_path.name): | ||
| found_files.append({ | ||
| "path": str(file_path), | ||
| "agent": agent.key, | ||
| "agent_display_name": agent.display_name, | ||
| "type": "backup", | ||
| "reason": "Matches backup pattern", | ||
| }) | ||
| except KeyError: | ||
| # Agent key not found, skip | ||
| continue | ||
|
|
||
| return found_files |
There was a problem hiding this comment.
Fix: Pipeline failures due to string paths instead of Path objects.
Lines 252 and 267 convert paths to strings using str(file_path), but the pipeline failures indicate tests expect Path objects. The test failures mention:
- "find_generated_files returns string paths instead of Path objects. The tests expect Path objects (e.g., generated_file)"
Change the "path" values to return Path objects instead of strings.
Apply this diff to fix:
if self._is_generated_file(file_path, agent):
found_files.append({
- "path": str(file_path),
+ "path": file_path,
"agent": agent.key,
"agent_display_name": agent.display_name,
"type": "command",
"reason": "Has generated metadata",
})
# Check for backup files
if include_backups:
# Look for files matching the backup pattern: *.extension.timestamp.bak
escaped_ext = re.escape(agent.command_file_extension)
pattern = re.compile(rf".*{escaped_ext}\.\d{{8}}-\d{{6}}\.bak$")
for file_path in command_dir.iterdir():
if file_path.is_file() and pattern.match(file_path.name):
found_files.append({
- "path": str(file_path),
+ "path": file_path,
"agent": agent.key,
"agent_display_name": agent.display_name,
"type": "backup",
"reason": "Matches backup pattern",
})🤖 Prompt for AI Agents
In slash_commands/writer.py around lines 252 and 267 (within the
find_generated_files method, lines 225-277), the code currently uses
str(file_path) for the "path" values which returns strings but tests expect Path
objects; change those entries so the "path" value is the Path object itself (use
file_path instead of str(file_path)) for both the regular command files and
backup files so the returned dicts contain Path objects.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/test_writer.py (1)
336-535: LGTM! Good coverage of file discovery and cleanup.The tests properly validate:
- Detection of generated files via metadata markers
- Distinction between generated and manual files
- Backup file pattern matching
- Cleanup behavior with dry-run and backup inclusion/exclusion
Consider adding test coverage for two edge cases:
- Disabled prompts: No test validates that prompts with
enabled: falseare skipped during generation (writer line 157-158).- Cleanup error handling: No test verifies the error tracking when
file_path.unlink()raises OSError (writer lines 378-379).Example for disabled prompt test:
def test_writer_skips_disabled_prompts(tmp_path): prompts_dir = tmp_path / "prompts" prompts_dir.mkdir() prompt_file = prompts_dir / "disabled-prompt.md" prompt_file.write_text("""--- name: disabled-prompt description: Disabled prompt enabled: false --- # Disabled """) writer = SlashCommandWriter( prompts_dir=prompts_dir, agents=["claude-code"], dry_run=False, base_path=tmp_path, ) result = writer.generate() assert result["prompts_loaded"] == 1 assert result["files_written"] == 0 assert len(result["files"]) == 0
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
slash_commands/writer.py(1 hunks)tests/test_writer.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/test_writer.py (3)
slash_commands/config.py (1)
CommandFormat(10-14)slash_commands/writer.py (4)
SlashCommandWriter(70-388)generate(98-132)find_generated_files(226-282)cleanup(354-388)slash_commands/generators.py (4)
generate(27-30)generate(155-190)generate(218-256)create(267-274)
slash_commands/writer.py (4)
mcp_server/prompt_utils.py (2)
MarkdownPrompt(19-40)load_markdown_prompt(43-85)slash_commands/config.py (3)
AgentConfig(18-31)get_agent_config(94-100)list_agent_keys(88-91)slash_commands/generators.py (5)
CommandGenerator(263-274)generate(27-30)generate(155-190)generate(218-256)create(267-274)slash_commands/cli.py (2)
generate(62-259)cleanup(263-396)
🔇 Additional comments (7)
tests/test_writer.py (2)
1-88: LGTM! Comprehensive test coverage for core generation scenarios.The fixture setup and generation tests properly validate single/multiple agent workflows, file creation, and result structure. The tests correctly verify both Markdown and TOML outputs for different agents.
90-334: LGTM! Thorough validation of dry-run and overwrite workflows.The tests properly cover:
- Dry-run mode (no files written but metadata reported)
- Parent directory auto-creation
- Existing file handling (overwrite, cancel, backup, overwrite-all)
- Error cases (missing directory, invalid agent)
The global overwrite test correctly validates that "overwrite-all" prompts only once for multiple files.
slash_commands/writer.py (5)
1-67: LGTM! Clean helper functions with proper error handling.The utility functions are well-designed:
prompt_overwrite_actionproperly handles user cancellation (line 45-47)create_backupuses UTC timestamps andshutil.copy2to preserve file metadata (line 65)- Type definitions are clear with
OverwriteActionusing Literal types
70-132: LGTM! Initialization and generation logic are sound.The constructor and
generate()method properly implement:
- Default to all supported agents when
agents=None(line 91)- Tracking of backups created and global overwrite state
- Correct
files_writtencounting that excludes dry-run files (lines 122-124)- Comprehensive result dict with prompts, files, and metadata
The implementation addresses all previously flagged issues.
134-224: LGTM! Robust file generation with proper sanitization.The file generation workflow properly implements:
- Directory existence validation with clear error messages (line 137)
- Filename sanitization preventing path traversal (lines 168-170): removes directories, replaces unsafe characters, provides fallback
- UTF-8 encoding when writing files (line 188)
- Skip logic for disabled prompts (lines 157-158)
- Hierarchical overwrite handling: global flag → explicit action → user prompt (lines 207-222)
All previously identified security and encoding issues have been addressed.
226-332: LGTM! Robust file discovery with proper error handling.The file discovery implementation properly handles:
- Default to all agents when
agents=None(line 239)- String path conversion via
os.fspath(lines 253, 270)- Backup file pattern matching with proper regex escaping (lines 265-266)
- Unknown agent keys with KeyError catch (lines 278-280)
- Metadata detection for both Markdown and TOML formats (lines 299-332)
- Parse errors and malformed files with comprehensive exception handling
The defensive programming prevents crashes on malformed or manual files.
354-388: LGTM! Cleanup method with proper error tracking.The cleanup implementation properly:
- Delegates discovery to
find_generated_files(line 367)- Respects dry-run mode without deleting files (line 374)
- Catches and tracks OSError during deletion (lines 378-379)
- Returns comprehensive results including error details
The error handling ensures cleanup failures are reported rather than crashing the operation.
|
🚀 |
When running the tool via uvx, the prompts directory may not be available relative to the current working directory. This commit adds a fallback mechanism to locate prompts in the installed package directory when the specified prompts directory doesn't exist. Changes: - Add _find_package_prompts_dir() helper function to locate prompts in package root - Update _load_prompts() to fall back to package prompts when specified directory doesn't exist - Update tests to mock fallback function for error cases - Add test to verify fallback behavior works correctly This enables the tool to work seamlessly when installed via uvx without requiring --prompts-dir to be specified.
Add a concise TLDR section at the top of the README that shows: - How to install prompts via uvx - Example of running /generate-spec with an idea - Continuing through the workflow with task list and management - The end goal (profit) This makes it easier for new users to quickly understand and get started with the spec-driven development workflow.
- Fix prompts directory resolution for remote installation via uvx - Enable automatic detection of bundled prompts in installed packages - Maintain backward compatibility with custom prompts directories - Improve error messages for better user experience
…ution - Add tasks-0005-spec-fix-bundled-prompts-path.md - Task list covers updating importlib.resources usage - Includes CLI changes to distinguish default vs explicit paths - Ensures backward compatibility and proper error handling
- Import importlib.resources module in writer.py - Add strategy using importlib.resources.files() to locate bundled prompts - Keep existing fallback strategy using Path(__file__).parent.parent - Add proper error handling for importlib edge cases - Add unit test test_writer_finds_bundled_prompts for importlib.resources path resolution Related to task 1.0 in tasks-0005-spec-fix-bundled-prompts-path.md
Update CLI and Writer to distinguish between explicitly provided
prompts directory and default directory. Only attempt fallback to
bundled prompts when using the default path (not explicitly provided).
- Change CLI prompts_dir default from Path("prompts") to None
- Add is_explicit_prompts_dir flag to track user intent
- Update Writer._load_prompts() to skip fallback for explicit paths
- Improve error messages to distinguish default vs explicit paths
- Add test for explicit path error handling
- Update existing tests to pass is_explicit_prompts_dir parameter
- Mark completed tasks in task list
This fixes the issue where explicitly providing an invalid path would
attempt to fall back to bundled prompts, causing confusing behavior.
- Mark tasks 3.0 and 4.0 as completed - Mark all subtasks of 3.0 and 4.0 as completed - Progress on task 5.0: completed subtasks 5.1-5.4 - Task 5.5 (manual uvx test) remains pending
- All 5 main tasks completed - Manual uvx test confirmed working - Spec implementation complete
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
177-181: Remove redundant "Workflow Essentials" section.Lines 177–181 duplicate the three-step workflow already documented in detail under "Option 1: Manual Copy-Paste" (lines 144–148). This repetition dilutes the clarity of the three-option structure and may confuse readers about which path to take. Delete this section; users should reference the specific option they choose.
🧹 Nitpick comments (13)
tasks/tasks-0005-spec-fix-bundled-prompts-path.md (2)
51-58: Finish 5.5 before merge (manual uvx verification).Please run/record the uvx install-and-run path to close 5.5 and avoid regressions slipping through.
I can draft a short runbook snippet if helpful.
5-8: Wording nit: “CLI interface” → “CLI”.“Tautology” (the “I” already stands for interface). Very minor.
tests/test_cli.py (2)
205-220: Add coverage: explicit prompts-dir exists but has no .md files (FR2).Spec calls for a clear error in this case. Please add a test asserting exit code/message for an empty directory passed via --prompts-dir.
96-108: Optional: pass multiple agents via list/multiple option instead of repeating flag.If the Typer option is configured with multiple=True, prefer a single flag with repeated values or a list for terser invocations.
tests/test_writer.py (1)
174-191: Add test: explicit prompts_dir exists but contains no .md files.Per FR2, writer should raise a clear error here. Add a test that creates an empty directory and asserts ValueError with a helpful message when is_explicit_prompts_dir=True.
tasks/0005-spec-fix-bundled-prompts-path.md (2)
158-164: Wording nit: “CLI interface” → “CLI”.Minor clarity polish.
299-324: Optional: mention importlib.resources.as_file for non-filesystem installs.In rare zipped-resource scenarios, as_file() ensures a concrete Path; consider noting it as a fallback approach in the spec.
slash_commands/writer.py (1)
31-41: Optional robustness: guard for non-filesystem package resources.If the package is installed as a zipped resource, Path(str(...)) may not resolve properly. Use as_file() to ensure a concrete Path.
Apply this diff:
- try: - package = importlib.resources.files("spec_driven_development_mcp") - prompts_resource = package / "prompts" - # Check if the prompts directory exists in the resource - if prompts_resource.is_dir(): - return Path(str(prompts_resource)) - except (ModuleNotFoundError, AttributeError, ValueError): + try: + package = importlib.resources.files("spec_driven_development_mcp") + prompts_resource = package / "prompts" + if prompts_resource.is_dir(): + # Ensure a real filesystem Path (handles zipped distributions) + with importlib.resources.as_file(prompts_resource) as p: + if p.is_dir(): + return p + except (ModuleNotFoundError, AttributeError, ValueError, FileNotFoundError): # Fall through to fallback strategy passslash_commands/cli.py (5)
128-147: Honor --detection-path/--target-path and reuse detect_agents in --list-agents.The list view checks
Path.home()/agent.command_direxistence, which can disagree withdetect_agents(...)and ignores--detection-path/--target-path. Use the same resolution and detection logic for consistent UX.- # Get home directory for checking paths - home_dir = Path.home() - - for agent_key in list_agent_keys(): - try: - agent = get_agent_config(agent_key) - # Check if command directory exists - command_path = home_dir / agent.command_dir - exists = command_path.exists() - detected = "[green]✓[/green]" if exists else "[red]✗[/red]" - - table.add_row( - agent_key, - agent.display_name, - f"~/{agent.command_dir}", - detected, - ) - except KeyError: - table.add_row(agent_key, "Unknown", "N/A", "[red]✗[/red]") + # Resolve detection base the same way as generation + detection_dir = ( + detection_path + if detection_path is not None + else (target_path if target_path is not None else Path.home()) + ) + detected_keys = {a.key for a in detect_agents(detection_dir)} + + for agent_key in list_agent_keys(): + agent = get_agent_config(agent_key) + path_display = ( + f"~/{agent.command_dir}" + if detection_dir == Path.home() + else str((detection_dir / agent.command_dir).expanduser()) + ) + detected = "[green]✓[/green]" if agent.key in detected_keys else "[red]✗[/red]" + table.add_row(agent_key, agent.display_name, path_display, detected)
154-158: Expand '~' in user‑provided paths.Ensure
--detection-pathand--target-pathaccept tilde paths reliably across shells/OS.- detection_dir = ( - detection_path - if detection_path is not None - else (target_path if target_path is not None else Path.home()) - ) + detection_dir = ( + detection_path.expanduser() + if detection_path is not None + else (target_path.expanduser() if target_path is not None else Path.home()) + )- actual_target_path = target_path if target_path is not None else Path.home() + actual_target_path = target_path.expanduser() if target_path is not None else Path.home()Also applies to: 191-193
188-209: Deduplicate agent keys before writing (prevents duplicate generation).Passing duplicates will re‑generate the same files and trigger overwrite flows repeatedly.
- else: - print(f"Selected agents: {', '.join(agents)}") + else: + print(f"Selected agents: {', '.join(agents)}") + + # De‑dupe while preserving order before invoking the writer + agents = list(dict.fromkeys(agents))
325-330: Minor: writer init doesn’t need agents for cleanup.
find_generated_files/cleanuptakeagentsas parameters; constructoragents=[]is unused.- writer = SlashCommandWriter( - prompts_dir=Path("prompts"), # Not used for cleanup - agents=[], - dry_run=dry_run, - base_path=actual_target_path, - ) + writer = SlashCommandWriter( + prompts_dir=Path("prompts"), # Not used for cleanup + dry_run=dry_run, + base_path=actual_target_path, + )
201-209: Optional: clarify overwrite semantics with 'overwrite-all'.Using
"overwrite-all"matches the writer’s global‑overwrite pathway and intent of--yesmore explicitly; behavior remains overwrite‑without‑prompt.- overwrite_action = "overwrite" if yes else None + overwrite_action = "overwrite-all" if yes else None
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
README.md(4 hunks)slash_commands/cli.py(1 hunks)slash_commands/writer.py(1 hunks)tasks/0005-spec-fix-bundled-prompts-path.md(1 hunks)tasks/tasks-0005-spec-fix-bundled-prompts-path.md(1 hunks)tests/test_cli.py(1 hunks)tests/test_writer.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
tests/test_cli.py (1)
slash_commands/config.py (2)
AgentConfig(18-31)CommandFormat(10-14)
slash_commands/writer.py (4)
mcp_server/prompt_utils.py (2)
MarkdownPrompt(19-40)load_markdown_prompt(43-85)slash_commands/config.py (3)
AgentConfig(18-31)get_agent_config(94-100)list_agent_keys(88-91)slash_commands/generators.py (5)
CommandGenerator(263-274)generate(27-30)generate(155-190)generate(218-256)create(267-274)slash_commands/cli.py (1)
generate(62-273)
tests/test_writer.py (3)
slash_commands/config.py (1)
CommandFormat(10-14)slash_commands/writer.py (4)
SlashCommandWriter(109-443)generate(141-175)find_generated_files(281-337)cleanup(409-443)slash_commands/generators.py (4)
generate(27-30)generate(155-190)generate(218-256)create(267-274)
slash_commands/cli.py (3)
slash_commands/writer.py (4)
SlashCommandWriter(109-443)generate(141-175)cleanup(409-443)find_generated_files(281-337)slash_commands/detection.py (1)
detect_agents(11-25)slash_commands/config.py (2)
get_agent_config(94-100)list_agent_keys(88-91)
🪛 LanguageTool
tasks/0005-spec-fix-bundled-prompts-path.md
[style] ~161-~161: This phrase is redundant (‘I’ stands for ‘interface’). Use simply “CLI”.
Context: ...dir` with valid paths 2. Not change the CLI interface or parameter names 3. Not require chang...
(ACRONYM_TAUTOLOGY)
tasks/tasks-0005-spec-fix-bundled-prompts-path.md
[style] ~7-~7: This phrase is redundant (‘I’ stands for ‘interface’). Use simply “CLI”.
Context: ...ew behavior - slash_commands/cli.py - CLI interface that needs to change default value for ...
(ACRONYM_TAUTOLOGY)
🔇 Additional comments (1)
slash_commands/cli.py (1)
1-26: Overall: solid CLI surface, helpful errors, and clear UX.Structure, options, and error handling look good. Tests noted in the PR should keep this stable.
If you want, I can run a quick repo scan to ensure no duplicate agent keys are passed in tests and to validate the new list‑agents detection logic end‑to‑end.
| mode = "DRY RUN" if dry_run else "Generation" | ||
| print(f"\n{mode} complete:") | ||
| print(f" Prompts loaded: {result['prompts_loaded']}") | ||
| print(f" Files {'would be' if dry_run else ''} written: {result['files_written']}") | ||
| if result.get("backups_created"): | ||
| print(f" Backups created: {len(result['backups_created'])}") | ||
| for backup in result["backups_created"]: | ||
| print(f" - {backup}") |
There was a problem hiding this comment.
Fix dry‑run summary count (shows 0 today).
writer.generate() intentionally keeps files_written at 0 during dry‑run, so the summary is misleading. Compute from result["files"] when dry_run=True.
- mode = "DRY RUN" if dry_run else "Generation"
- print(f"\n{mode} complete:")
- print(f" Prompts loaded: {result['prompts_loaded']}")
- print(f" Files {'would be' if dry_run else ''} written: {result['files_written']}")
+ mode = "DRY RUN" if dry_run else "Generation"
+ print(f"\n{mode} complete:")
+ print(f" Prompts loaded: {result['prompts_loaded']}")
+ files_count = len(result["files"]) if dry_run else result["files_written"]
+ print(f" Files {'would be' if dry_run else ''} written: {files_count}")🤖 Prompt for AI Agents
In slash_commands/cli.py around lines 262 to 269, the dry-run summary shows
files_written as 0 because writer.generate() intentionally leaves files_written
at 0 for dry runs; update the summary to compute the count from result["files"]
when dry_run is True (e.g., use files_count = len(result.get("files", [])) if
dry_run else result.get("files_written", 0)), and change the printed line to
reflect the dry-run wording (use "would be written" when dry_run) while printing
the computed files_count instead of result['files_written'].
| prompts = [] | ||
| for prompt_file in sorted(prompts_dir.glob("*.md")): | ||
| prompt = load_markdown_prompt(prompt_file) | ||
| prompts.append(prompt) | ||
|
|
||
| return prompts |
There was a problem hiding this comment.
FR2 gap: raise when no .md prompts are found.
Explicit or fallback dirs yielding zero prompts should fail fast with a clear message. This aligns with the spec’s “explicit path exists but contains no .md files.”
Apply this diff:
prompts = []
for prompt_file in sorted(prompts_dir.glob("*.md")):
prompt = load_markdown_prompt(prompt_file)
prompts.append(prompt)
+ # Fail fast if no prompts discovered (helps explicit-path validation and clearer UX)
+ if not prompts:
+ raise ValueError(f"No .md prompt files found in: {prompts_dir}")
+
return prompts📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| prompts = [] | |
| for prompt_file in sorted(prompts_dir.glob("*.md")): | |
| prompt = load_markdown_prompt(prompt_file) | |
| prompts.append(prompt) | |
| return prompts | |
| prompts = [] | |
| for prompt_file in sorted(prompts_dir.glob("*.md")): | |
| prompt = load_markdown_prompt(prompt_file) | |
| prompts.append(prompt) | |
| # Fail fast if no prompts discovered (helps explicit-path validation and clearer UX) | |
| if not prompts: | |
| raise ValueError(f"No .md prompt files found in: {prompts_dir}") | |
| return prompts |
🤖 Prompt for AI Agents
In slash_commands/writer.py around lines 194 to 199, the function currently
collects .md prompts into a list but returns silently when no prompts are found;
update it to fail fast by checking if prompts is empty after the loop and
raising a clear error (e.g., raise FileNotFoundError or ValueError) that
includes the prompts_dir path and an explicit message like "No .md prompt files
found in <path> — explicit or fallback directory must contain at least one .md
file." Ensure the raised exception is specific and descriptive so callers can
act on the missing prompts.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
README.md (1)
23-23: Fix unresolved CLI command examples (duplicate of prior review).These critical issues from the previous review remain unfixed:
Line 23 — Incorrect repository name in TLDR:
- Current:
spec-driven-workflow- Should be:
spec-driven-workflow-mcpLine 177 — Missing
generatesubcommand in local install example:
- Current:
uv run sdd-generate-commands --yes- Should be:
uv run sdd-generate-commands generate --yesLine 180 — Missing
generatesubcommand in uvx from-repo example:
- Current:
uvx --from git+https://github.com/liatrio-labs/spec-driven-workflow-mcp sdd-generate-commands --yes- Should be:
uvx --from git+https://github.com/liatrio-labs/spec-driven-workflow-mcp sdd-generate-commands generate --yesApply these diffs:
- uvx --from git+https://github.com/liatrio-labs/spec-driven-workflow sdd-generate-commands generate --yes + uvx --from git+https://github.com/liatrio-labs/spec-driven-workflow-mcp sdd-generate-commands generate --yes- uv run sdd-generate-commands --yes + uv run sdd-generate-commands generate --yes- uvx --from git+https://github.com/liatrio-labs/spec-driven-workflow-mcp sdd-generate-commands --yes + uvx --from git+https://github.com/liatrio-labs/spec-driven-workflow-mcp sdd-generate-commands generate --yesAlso applies to: 177-177, 180-180
🧹 Nitpick comments (1)
tasks/tasks-0005-spec-fix-bundled-prompts-path.md (1)
7-7: Remove redundant wording."CLI interface" is tautological since CLI stands for "Command Line Interface". Simplify to just "CLI".
-- `slash_commands/cli.py` - CLI interface that needs to change default value for `prompts_dir` parameter and improve error handling +- `slash_commands/cli.py` - CLI that needs to change default value for `prompts_dir` parameter and improve error handling
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(4 hunks)tasks/tasks-0005-spec-fix-bundled-prompts-path.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
tasks/tasks-0005-spec-fix-bundled-prompts-path.md
[style] ~7-~7: This phrase is redundant (‘I’ stands for ‘interface’). Use simply “CLI”.
Context: ...ew behavior - slash_commands/cli.py - CLI interface that needs to change default value for ...
(ACRONYM_TAUTOLOGY)
🔇 Additional comments (1)
tasks/tasks-0005-spec-fix-bundled-prompts-path.md (1)
1-58: Comprehensive and well-structured specification document.The task list is thorough, clearly organized, and all items are marked complete with concrete proof artifacts and demo criteria. The document effectively aligns with the implementation described in the enriched summary (particularly the path resolution strategies, default vs explicit handling, and error messaging improvements).
- Fix README repository URLs and CLI command examples - Fix pyproject.toml coverage exclude pattern regex - Add explicit UTF-8 encoding to test fixtures - Fix documentation wording (CLI interface → CLI)
Use slash_commands module as anchor for importlib.resources instead of spec_driven_development_mcp to provide more reliable path resolution when finding bundled prompts directory. - Navigate from slash_commands package to parent then to prompts dir - Add explicit test for _find_package_prompts_dir() with importlib - Improve comments explaining the traversal approach This makes the package prompts discovery more robust across different installation scenarios (wheel, editable install, etc.).
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/test_writer.py (1)
174-191: Consider adding a test for an empty prompts directory.The test
test_writer_handles_missing_prompts_directoryvalidates behavior when the prompts directory doesn't exist. However, there's no test for when the directory exists but contains no.mdfiles. This scenario corresponds to the FR2 gap mentioned in past reviews (where_load_promptsshould fail fast with a clear error when no prompts are found).Add a test case like:
def test_writer_handles_empty_prompts_directory(tmp_path): """Test that writer handles empty prompts directory gracefully.""" prompts_dir = tmp_path / "empty_prompts" prompts_dir.mkdir() # Directory exists but contains no .md files writer = SlashCommandWriter( prompts_dir=prompts_dir, agents=["claude-code"], dry_run=False, base_path=tmp_path, is_explicit_prompts_dir=True, ) with pytest.raises(ValueError, match="No .md prompt files found"): writer.generate()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
slash_commands/writer.py(1 hunks)tests/test_writer.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/test_writer.py (2)
slash_commands/config.py (1)
CommandFormat(10-14)slash_commands/writer.py (5)
SlashCommandWriter(111-445)_find_package_prompts_dir(25-61)generate(143-177)find_generated_files(283-339)cleanup(411-445)
slash_commands/writer.py (4)
mcp_server/prompt_utils.py (2)
MarkdownPrompt(19-40)load_markdown_prompt(43-85)slash_commands/config.py (3)
AgentConfig(18-31)get_agent_config(94-100)list_agent_keys(88-91)slash_commands/generators.py (5)
CommandGenerator(263-274)generate(27-30)generate(155-190)generate(218-256)create(267-274)slash_commands/cli.py (2)
generate(62-273)cleanup(277-410)
🔇 Additional comments (1)
slash_commands/writer.py (1)
1-445: LGTM! Implementation is solid and addresses all critical past review concerns.The SlashCommandWriter implementation is well-structured with:
- Proper error handling and UTF-8 encoding
- Correct default behavior for agents parameter
- Path sanitization to prevent traversal attacks
- Comprehensive file discovery and cleanup logic
- Good separation of concerns with helper methods
The only remaining item from past reviews is the FR2 gap (lines 196-201: validating non-empty prompts list), which was already flagged and is appropriately deferred.
feat(slash-commands): add generator for AI assistant native slash commands
Why?
This change enables users to use the Spec Driven Development (SDD) workflow prompts as native slash commands in their AI coding assistants (Claude Code, Cursor, Windsurf, etc.) instead of manually copy-pasting prompts. This removes friction and makes the SDD workflow accessible as a first-class feature within AI coding environments.
Previously, users had to manually copy prompts from the
prompts/directory, which was error-prone and disconnected from their workflow. Now they can run a single command to install prompts as native commands that their AI assistant recognizes automatically.Additionally, this PR includes several infrastructure improvements that enhance the overall developer experience and package usability.
What Changed?
Implemented a complete slash command generation system that reads markdown prompts and generates command files in the correct format for each supported AI assistant, plus related infrastructure improvements.
Key Changes:
Slash Command Generator (New Feature):
sdd-generate-commandsTyper-based CLI with generate, list-agents, and cleanup commands.md) for most agents and TOML (.toml) for Gemini CLI and similarInfrastructure Improvements:
--transport(stdio/http) and--portconfiguration, enablinguvx spec-driven-development-mcp --transport http --port 8000__version__.pywith fallback for installed packages usingimportlib.metadata, enabling the package to work correctly when installed viauvx --from git+URL or from PyPIDocumentation Improvements:
uvx --fromArchitecture:
The slash command feature includes five main modules:
slash_commands/cli.py: Typer CLI with interactive agent selection and file managementslash_commands/generators.py: Prompt format conversion logic (Markdown/TOML) with metadata supportslash_commands/writer.py: File generation with backup and dry-run supportslash_commands/detection.py: Agent detection by scanning command directoriesslash_commands/config.py: Agent configuration and command path definitionsSupported Agents:
Generates command files for 6 AI assistants:
.claude/commands).cursor/commands).codeium/windsurf/global_workflows).config/Code/User/prompts).codex/prompts).gemini/commands)Additional Notes
Breaking Changes
None. This is an additive feature.
Testing
mcp_serverandslash_commandsmodulesDependencies
typer>=0.19.0,questionary,rich,tomli-w,pyyamluv.lockwith latest dependency resolutionsDocumentation
docs/slash-command-generator.md(705 lines): Complete user guide with examples, troubleshooting, and agent-specific detailsdocs/mcp-prompt-support.md(46 lines): Tool compatibility matrix and support notesREADME.md: Reorganized into three usage options (Manual, Slash Commands, MCP Server)uvx --fromCONTRIBUTING.mdwith cross-platform coverage viewing instructionsConfiguration Changes
slash_commandsto package discovery inpyproject.toml__version__.pyin wheel package via force-includeslash_commandsin coverage reportinghtmlcovdirectory to.gitignoreCode Review Fixes
Addressed 15+ code review findings from CodeRabbitAI:
server.pyfor robust CLIpython -m webbrowser) in CONTRIBUTING.mditer_detection_dirs())Package Upgrades
Updated all packages to latest compatible versions:
Known Limitations
tomllibusage and dependency compatibilitydocs/mcp-prompt-support.md)Future Work
uvx spec-driven-development-mcp sdd-generate-commands --yesone-linerReview Checklist
Summary by CodeRabbit
New Features
Documentation
Tests
Chores