Skip to content

fix(forge): use hyphen notation in frontmatter name field#2075

Open
toxicafunk wants to merge 7 commits intogithub:mainfrom
toxicafunk:feature/fix-forge-zsh-mode
Open

fix(forge): use hyphen notation in frontmatter name field#2075
toxicafunk wants to merge 7 commits intogithub:mainfrom
toxicafunk:feature/fix-forge-zsh-mode

Conversation

@toxicafunk
Copy link
Copy Markdown

@toxicafunk toxicafunk commented Apr 3, 2026

Description

Fixed Forge integration to use hyphen notation (speckit-analyze) instead of dot notation (speckit.analyze) in the frontmatter name: 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
      forge cmd execute speckit.constitution "derive a constitution for this repo"
  • Mode 2: Interactive Session

forge
# Then inside the session:
/speckit.constitution derive a constitution for this repo
  • Mode 3: Single Command with forge CLI
forge "/speckit.constitution derive a constitution for this repo"

This PR allows a 4th ZSH mode.

: speckit-constitution "derive a constitution for this repo"

Changes:

  • Updated _apply_forge_transformations() to inject name: speckit-{command} (hyphen notation)
  • Keeps standard filename format speckit.{command}.md (dot notation remains for filenames)
  • Updated comment to reflect the correct example

Example output:

---
name: speckit-analyze
description: Perform a non-destructive cross-artifact consistency...
---

Testing

  • Tested locally with uv run specify --help
  • Ran existing tests with uv sync && uv run pytest
  • All 11 Forge integration tests pass
  • Verified generated frontmatter uses hyphen notation
  • Confirmed filenames remain in standard dot notation format

Test results:

tests/integrations/test_integration_forge.py::TestForgeIntegration
  ✓ test_forge_key_and_config PASSED
  ✓ test_command_filename_md PASSED
  ✓ test_setup_creates_md_files PASSED
  ✓ test_setup_installs_update_scripts PASSED
  ✓ test_all_created_files_tracked_in_manifest PASSED
  ✓ test_install_uninstall_roundtrip PASSED
  ✓ test_modified_file_survives_uninstall PASSED
  ✓ test_directory_structure PASSED
  ✓ test_templates_are_processed PASSED
  ✓ test_forge_specific_transformations PASSED
  ✓ test_uses_parameters_placeholder PASSED

11 passed in 0.18s

AI Disclosure

  • I did use AI assistance (describe below)

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.

- 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
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 produce speckit-{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.

Copy link
Copy Markdown
Collaborator

@mnriem mnriem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
@mnriem mnriem requested a review from Copilot April 3, 2026 13:21
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

ericnoam added 2 commits April 3, 2026 15:25
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)
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 mnriem self-requested a review April 3, 2026 13:38
Copy link
Copy Markdown
Collaborator

@mnriem mnriem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator

@mnriem mnriem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@mnriem
Copy link
Copy Markdown
Collaborator

mnriem commented Apr 3, 2026

Please address Copilot feedback

@toxicafunk
Copy link
Copy Markdown
Author

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.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants