fix(forge): use hyphen notation in frontmatter name field#2075
fix(forge): use hyphen notation in frontmatter name field#2075toxicafunk wants to merge 7 commits intogithub:mainfrom
Conversation
- Changed injected name field from 'speckit.{command}' to 'speckit-{command}'
- Keeps standard filename format 'speckit.{command}.md'
- Aligns with Forge's command naming convention requirements
- All tests pass
There was a problem hiding this comment.
Pull request overview
Updates the Forge integration’s frontmatter generation so Forge command name: values use hyphen notation (e.g., speckit-plan) instead of dot notation (e.g., speckit.plan), aligning with Forge’s command naming expectations and enabling additional invocation modes (like ZSH support).
Changes:
- Adjusts Forge frontmatter
name:injection to producespeckit-{command}. - Updates the inline comment/example to reflect the hyphenated naming format.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mnriem
left a comment
There was a problem hiding this comment.
Please address Copilot feedback
…nd names Address PR feedback by centralizing Forge command name formatting to ensure consistent hyphenated names across both core template setup and extension/preset command registration. Changes: - Add format_forge_command_name() utility function in forge integration - Update ForgeIntegration._apply_forge_transformations() to use centralized formatter - Add _format_name_for_agent() helper in CommandRegistrar to apply agent-specific formatting - Update CommandRegistrar.register_commands() to format names for Forge (both primary commands and aliases) - Add comprehensive test coverage for the formatter and registrar behavior Impact: - Extension commands installed for Forge now use 'name: speckit-my-extension-example' instead of 'name: speckit.my-extension.example' - Fixes ZSH/shell compatibility issues with dot notation in command names - Maintains backward compatibility for all other agents (they continue using dot notation) - Eliminates duplication between integration setup and registrar paths Example transformation: Before: name: speckit.jira.sync-status (breaks in ZSH/Forge) After: name: speckit-jira-sync-status (works everywhere) Fixes inconsistency where core templates used hyphens but extension/preset commands preserved dots, breaking Forge's naming requirements.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Move _format_name_for_agent function logic into Forge integration's registrar_config as a 'format_name' callback, improving separation of concerns and keeping Forge-specific logic within its integration module. Changes: - Remove _format_name_for_agent() from agents.py (shared module) - Add 'format_name' callback to Forge's registrar_config pointing to format_forge_command_name - Update CommandRegistrar to use format_name callback when available - Maintains same behavior: Forge commands use hyphenated names, others use dot notation Benefits: - Better encapsulation: Forge-specific logic lives in forge integration - More extensible: Other integrations can provide custom formatters via registrar_config - Cleaner separation: agents.py doesn't need to know about specific agent requirements
Handle already-hyphenated names (speckit-foo) to prevent double-prefixing
(speckit-speckit-foo). The function now returns already-formatted names
unchanged, making it safe to call multiple times.
Changes:
- Add early return for names starting with 'speckit-'
- Update docstring to clarify accepted input formats
- Add examples showing idempotent behavior
- Add test coverage for idempotent behavior
Examples:
format_forge_command_name('speckit-plan') -> 'speckit-plan' (unchanged)
format_forge_command_name('speckit.plan') -> 'speckit-plan' (converted)
format_forge_command_name('plan') -> 'speckit-plan' (prefixed)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mnriem
left a comment
There was a problem hiding this comment.
Please address Copilot feedback. If not applicable, please explain why
Improve test_name_field_uses_hyphenated_format to fail loudly when the name field is missing instead of silently passing. Changes: - Add explicit assertion that name_match is not None before validating value - Ensures test fails if regex doesn't match (e.g., frontmatter rendering changes) - Clarify Claude comment: it doesn't use inject_name path but SKILL.md frontmatter still includes hyphenated name via build_skill_frontmatter() Before: Test would silently pass if 'name:' field was missing from frontmatter After: Test explicitly asserts field presence before validating format
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mnriem
left a comment
There was a problem hiding this comment.
Please address Copilot feedback. If not applicable, please explain why
…olation Fix misleading docstring and improve test to properly validate that the format_name callback is Forge-specific. Changes to src/specify_cli/integrations/forge/__init__.py: - Reword module docstring to clarify the requirement is specifically for the frontmatter 'name' field value, not command files or invocation - Before: 'Requires hyphenated command names ... instead of dot notation' (implied dot notation unsupported overall) - After: 'Uses a hyphenated frontmatter name value ... for shell compatibility' (clarifies it's the frontmatter field, and Forge still supports dot filenames) Changes to tests/integrations/test_integration_forge.py: - Replace Claude with Windsurf in test_registrar_does_not_affect_other_agents - Claude uses build_skill_frontmatter() which always includes hyphenated names, so testing it didn't validate that format_name callback is Forge-only - Windsurf is a standard markdown agent without inject_name - Now asserts NO 'name:' field is present, proving format_name isn't invoked - This properly validates the callback mechanism is isolated to Forge
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Please address Copilot feedback |
it seemed like such a small change :-) |
Replace regex and string searches with CommandRegistrar.parse_frontmatter() to validate only YAML frontmatter, not entire file content. Prevents false positives if command body contains 'name:' lines. Changes: - test_forge_specific_transformations: Parse frontmatter dict instead of string search - test_name_field_uses_hyphenated_format: Replace regex with frontmatter parsing - test_registrar_formats_extension_command_names_for_forge: Use dict validation - test_registrar_formats_alias_names_for_forge: Use dict validation Benefits: More precise, robust against body content, better error messages, consistent with existing codebase utilities.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
Fixed Forge integration to use hyphen notation (
speckit-analyze) instead of dot notation (speckit.analyze) in the frontmattername:field. This aligns with Forge's command naming convention requirements.Context
Current Forge integration works for the following modes:
Mode 1: Direct Execution from Shell
forge cmd execute speckit.constitution "derive a constitution for this repo"Mode 2: Interactive Session
This PR allows a 4th ZSH mode.
Changes:
_apply_forge_transformations()to injectname: speckit-{command}(hyphen notation)speckit.{command}.md(dot notation remains for filenames)Example output:
Testing
uv run specify --helpuv sync && uv run pytestTest results:
AI Disclosure
This PR was created with assistance from Forge AI coding agent. The agent helped identify the issue, implement the fix, write tests, and ensure the solution aligned with the project's architecture and documentation standards.