mirror of
https://github.com/github/spec-kit.git
synced 2026-07-03 12:28:06 +08:00
fix(forge): use hyphen notation in frontmatter name field (#2075)
* fix(forge): use hyphen notation in frontmatter name field
- 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
* feat(forge): centralize name formatting to fix extension/preset command 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.
* refactor(forge): move name formatting logic to integration module
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
* fix(forge): make format_forge_command_name idempotent
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)
* test(forge): strengthen name field assertions and clarify comments
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
* docs(forge): clarify frontmatter name requirement and improve test isolation
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
* test(forge): use parse_frontmatter for precise YAML validation
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.
---------
Co-authored-by: ericnoam <eric.rodriguez@leovegas.com>
This commit is contained in:
@@ -420,7 +420,9 @@ class CommandRegistrar:
|
||||
frontmatter.pop(key, None)
|
||||
|
||||
if agent_config.get("inject_name") and not frontmatter.get("name"):
|
||||
frontmatter["name"] = cmd_name
|
||||
# Use custom name formatter if provided (e.g., Forge's hyphenated format)
|
||||
format_name = agent_config.get("format_name")
|
||||
frontmatter["name"] = format_name(cmd_name) if format_name else cmd_name
|
||||
|
||||
body = self._convert_argument_placeholder(
|
||||
body, "$ARGUMENTS", agent_config["args"]
|
||||
@@ -454,7 +456,9 @@ class CommandRegistrar:
|
||||
# For agents with inject_name, render with alias-specific frontmatter
|
||||
if agent_config.get("inject_name"):
|
||||
alias_frontmatter = deepcopy(frontmatter)
|
||||
alias_frontmatter["name"] = alias
|
||||
# Use custom name formatter if provided (e.g., Forge's hyphenated format)
|
||||
format_name = agent_config.get("format_name")
|
||||
alias_frontmatter["name"] = format_name(alias) if format_name else alias
|
||||
|
||||
if agent_config["extension"] == "/SKILL.md":
|
||||
alias_output = self.render_skill_command(
|
||||
|
||||
@@ -4,6 +4,7 @@ Forge has several unique behaviors compared to standard markdown agents:
|
||||
- Uses `{{parameters}}` instead of `$ARGUMENTS` for argument passing
|
||||
- Strips `handoffs` frontmatter key (Claude Code feature that causes Forge to hang)
|
||||
- Injects `name` field into frontmatter when missing
|
||||
- Uses a hyphenated frontmatter `name` value (e.g., `speckit-foo-bar`) for shell compatibility, especially with ZSH
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -15,6 +16,52 @@ from ..base import MarkdownIntegration
|
||||
from ..manifest import IntegrationManifest
|
||||
|
||||
|
||||
def format_forge_command_name(cmd_name: str) -> str:
|
||||
"""Convert command name to Forge-compatible hyphenated format.
|
||||
|
||||
Forge requires command names to use hyphens instead of dots for
|
||||
compatibility with ZSH and other shells. This function converts
|
||||
dot-notation command names to hyphenated format.
|
||||
|
||||
The function is idempotent: already-formatted names are returned unchanged.
|
||||
|
||||
Examples:
|
||||
>>> format_forge_command_name("plan")
|
||||
'speckit-plan'
|
||||
>>> format_forge_command_name("speckit.plan")
|
||||
'speckit-plan'
|
||||
>>> format_forge_command_name("speckit-plan")
|
||||
'speckit-plan'
|
||||
>>> format_forge_command_name("speckit.my-extension.example")
|
||||
'speckit-my-extension-example'
|
||||
>>> format_forge_command_name("speckit-my-extension-example")
|
||||
'speckit-my-extension-example'
|
||||
>>> format_forge_command_name("speckit.jira.sync-status")
|
||||
'speckit-jira-sync-status'
|
||||
|
||||
Args:
|
||||
cmd_name: Command name in dot notation (speckit.foo.bar),
|
||||
hyphenated format (speckit-foo-bar), or plain name (foo)
|
||||
|
||||
Returns:
|
||||
Hyphenated command name with 'speckit-' prefix
|
||||
"""
|
||||
# Already in hyphenated format - return as-is (idempotent)
|
||||
if cmd_name.startswith("speckit-"):
|
||||
return cmd_name
|
||||
|
||||
# Strip 'speckit.' prefix if present
|
||||
short_name = cmd_name
|
||||
if short_name.startswith("speckit."):
|
||||
short_name = short_name[len("speckit."):]
|
||||
|
||||
# Replace all dots with hyphens
|
||||
short_name = short_name.replace(".", "-")
|
||||
|
||||
# Return with 'speckit-' prefix
|
||||
return f"speckit-{short_name}"
|
||||
|
||||
|
||||
class ForgeIntegration(MarkdownIntegration):
|
||||
"""Integration for Forge (forgecode.dev).
|
||||
|
||||
@@ -39,6 +86,7 @@ class ForgeIntegration(MarkdownIntegration):
|
||||
"extension": ".md",
|
||||
"strip_frontmatter_keys": ["handoffs"],
|
||||
"inject_name": True,
|
||||
"format_name": format_forge_command_name, # Custom name formatter
|
||||
}
|
||||
context_file = "AGENTS.md"
|
||||
|
||||
@@ -106,7 +154,7 @@ class ForgeIntegration(MarkdownIntegration):
|
||||
"""Apply Forge-specific transformations to processed content.
|
||||
|
||||
1. Strip 'handoffs' frontmatter key (from Claude Code templates; incompatible with Forge)
|
||||
2. Inject 'name' field if missing
|
||||
2. Inject 'name' field if missing (using hyphenated format)
|
||||
"""
|
||||
# Parse frontmatter
|
||||
lines = content.split('\n')
|
||||
@@ -143,11 +191,11 @@ class ForgeIntegration(MarkdownIntegration):
|
||||
|
||||
filtered_frontmatter.append(line)
|
||||
|
||||
# 2. Inject 'name' field if missing
|
||||
# 2. Inject 'name' field if missing (using centralized formatter)
|
||||
has_name = any(line.strip().startswith('name:') for line in filtered_frontmatter)
|
||||
if not has_name:
|
||||
# Use the template name as the command name (e.g., "plan" -> "speckit.plan")
|
||||
cmd_name = f"speckit.{template_name}"
|
||||
# Use centralized formatter to ensure consistent hyphenated format
|
||||
cmd_name = format_forge_command_name(template_name)
|
||||
filtered_frontmatter.insert(0, f'name: {cmd_name}')
|
||||
|
||||
# Reconstruct content
|
||||
|
||||
@@ -2,6 +2,47 @@
|
||||
|
||||
from specify_cli.integrations import get_integration
|
||||
from specify_cli.integrations.manifest import IntegrationManifest
|
||||
from specify_cli.integrations.forge import format_forge_command_name
|
||||
|
||||
|
||||
class TestForgeCommandNameFormatter:
|
||||
"""Test the centralized Forge command name formatter."""
|
||||
|
||||
def test_simple_name_without_prefix(self):
|
||||
"""Test formatting a simple name without 'speckit.' prefix."""
|
||||
assert format_forge_command_name("plan") == "speckit-plan"
|
||||
assert format_forge_command_name("tasks") == "speckit-tasks"
|
||||
assert format_forge_command_name("specify") == "speckit-specify"
|
||||
|
||||
def test_name_with_speckit_prefix(self):
|
||||
"""Test formatting a name that already has 'speckit.' prefix."""
|
||||
assert format_forge_command_name("speckit.plan") == "speckit-plan"
|
||||
assert format_forge_command_name("speckit.tasks") == "speckit-tasks"
|
||||
|
||||
def test_extension_command_name(self):
|
||||
"""Test formatting extension command names with dots."""
|
||||
assert format_forge_command_name("speckit.my-extension.example") == "speckit-my-extension-example"
|
||||
assert format_forge_command_name("my-extension.example") == "speckit-my-extension-example"
|
||||
|
||||
def test_complex_nested_name(self):
|
||||
"""Test formatting deeply nested command names."""
|
||||
assert format_forge_command_name("speckit.jira.sync-status") == "speckit-jira-sync-status"
|
||||
assert format_forge_command_name("speckit.foo.bar.baz") == "speckit-foo-bar-baz"
|
||||
|
||||
def test_name_with_hyphens_preserved(self):
|
||||
"""Test that existing hyphens are preserved."""
|
||||
assert format_forge_command_name("my-extension") == "speckit-my-extension"
|
||||
assert format_forge_command_name("speckit.my-ext.test-cmd") == "speckit-my-ext-test-cmd"
|
||||
|
||||
def test_alias_formatting(self):
|
||||
"""Test formatting alias names."""
|
||||
assert format_forge_command_name("speckit.my-extension.example-short") == "speckit-my-extension-example-short"
|
||||
|
||||
def test_idempotent_already_hyphenated(self):
|
||||
"""Test that already-hyphenated names are returned unchanged (idempotent)."""
|
||||
assert format_forge_command_name("speckit-plan") == "speckit-plan"
|
||||
assert format_forge_command_name("speckit-my-extension-example") == "speckit-my-extension-example"
|
||||
assert format_forge_command_name("speckit-jira-sync-status") == "speckit-jira-sync-status"
|
||||
|
||||
|
||||
class TestForgeIntegration:
|
||||
@@ -123,19 +164,22 @@ class TestForgeIntegration:
|
||||
def test_forge_specific_transformations(self, tmp_path):
|
||||
"""Test Forge-specific processing: name injection and handoffs stripping."""
|
||||
from specify_cli.integrations.forge import ForgeIntegration
|
||||
from specify_cli.agents import CommandRegistrar
|
||||
forge = ForgeIntegration()
|
||||
m = IntegrationManifest("forge", tmp_path)
|
||||
forge.setup(tmp_path, m)
|
||||
commands_dir = tmp_path / ".forge" / "commands"
|
||||
|
||||
registrar = CommandRegistrar()
|
||||
for cmd_file in commands_dir.glob("speckit.*.md"):
|
||||
content = cmd_file.read_text(encoding="utf-8")
|
||||
frontmatter, _ = registrar.parse_frontmatter(content)
|
||||
|
||||
# Check that name field is injected in frontmatter
|
||||
assert "\nname: " in content, f"{cmd_file.name} missing injected 'name' field"
|
||||
assert "name" in frontmatter, f"{cmd_file.name} missing injected 'name' field in frontmatter"
|
||||
|
||||
# Check that handoffs frontmatter key is stripped
|
||||
assert "\nhandoffs:" not in content, f"{cmd_file.name} has unstripped 'handoffs' key"
|
||||
assert "handoffs" not in frontmatter, f"{cmd_file.name} has unstripped 'handoffs' key in frontmatter"
|
||||
|
||||
def test_uses_parameters_placeholder(self, tmp_path):
|
||||
"""Verify Forge replaces $ARGUMENTS with {{parameters}} in generated files."""
|
||||
@@ -168,3 +212,181 @@ class TestForgeIntegration:
|
||||
assert "{{parameters}}" in content, (
|
||||
"checklist should contain {{parameters}} in User Input section"
|
||||
)
|
||||
|
||||
def test_name_field_uses_hyphenated_format(self, tmp_path):
|
||||
"""Verify that injected name fields use hyphenated format (speckit-plan, not speckit.plan)."""
|
||||
from specify_cli.integrations.forge import ForgeIntegration
|
||||
from specify_cli.agents import CommandRegistrar
|
||||
forge = ForgeIntegration()
|
||||
m = IntegrationManifest("forge", tmp_path)
|
||||
forge.setup(tmp_path, m)
|
||||
commands_dir = tmp_path / ".forge" / "commands"
|
||||
|
||||
# Check that name fields use hyphenated format
|
||||
registrar = CommandRegistrar()
|
||||
for cmd_file in commands_dir.glob("speckit.*.md"):
|
||||
content = cmd_file.read_text(encoding="utf-8")
|
||||
# Extract the name field from frontmatter using the parser
|
||||
frontmatter, _ = registrar.parse_frontmatter(content)
|
||||
assert "name" in frontmatter, (
|
||||
f"{cmd_file.name} missing injected 'name' field in frontmatter"
|
||||
)
|
||||
name_value = frontmatter["name"]
|
||||
# Name should use hyphens, not dots
|
||||
assert "." not in name_value, (
|
||||
f"{cmd_file.name} has name field with dots: {name_value} "
|
||||
f"(should use hyphens for Forge/ZSH compatibility)"
|
||||
)
|
||||
assert name_value.startswith("speckit-"), (
|
||||
f"{cmd_file.name} name field should start with 'speckit-': {name_value}"
|
||||
)
|
||||
|
||||
|
||||
class TestForgeCommandRegistrar:
|
||||
"""Test CommandRegistrar's Forge-specific name formatting."""
|
||||
|
||||
def test_registrar_formats_extension_command_names_for_forge(self, tmp_path):
|
||||
"""Verify CommandRegistrar converts dot notation to hyphens for Forge."""
|
||||
from specify_cli.agents import CommandRegistrar
|
||||
|
||||
# Create a mock extension command file
|
||||
ext_dir = tmp_path / "extension"
|
||||
ext_dir.mkdir()
|
||||
cmd_dir = ext_dir / "commands"
|
||||
cmd_dir.mkdir()
|
||||
|
||||
# Create a test command with dot notation name
|
||||
cmd_file = cmd_dir / "example.md"
|
||||
cmd_file.write_text(
|
||||
"---\n"
|
||||
"description: Test extension command\n"
|
||||
"---\n\n"
|
||||
"Test content with $ARGUMENTS\n",
|
||||
encoding="utf-8"
|
||||
)
|
||||
|
||||
# Register with Forge
|
||||
registrar = CommandRegistrar()
|
||||
commands = [
|
||||
{
|
||||
"name": "speckit.my-extension.example",
|
||||
"file": "commands/example.md"
|
||||
}
|
||||
]
|
||||
|
||||
registered = registrar.register_commands(
|
||||
"forge",
|
||||
commands,
|
||||
"test-extension",
|
||||
ext_dir,
|
||||
tmp_path
|
||||
)
|
||||
|
||||
# Verify registration succeeded
|
||||
assert "speckit.my-extension.example" in registered
|
||||
|
||||
# Check the generated file has hyphenated name in frontmatter
|
||||
forge_cmd = tmp_path / ".forge" / "commands" / "speckit.my-extension.example.md"
|
||||
assert forge_cmd.exists()
|
||||
|
||||
content = forge_cmd.read_text(encoding="utf-8")
|
||||
# Parse frontmatter to validate name field precisely
|
||||
frontmatter, _ = registrar.parse_frontmatter(content)
|
||||
assert "name" in frontmatter, "name field should be injected in frontmatter"
|
||||
# Name field should use hyphens, not dots
|
||||
assert frontmatter["name"] == "speckit-my-extension-example"
|
||||
|
||||
def test_registrar_formats_alias_names_for_forge(self, tmp_path):
|
||||
"""Verify CommandRegistrar converts alias names to hyphens for Forge."""
|
||||
from specify_cli.agents import CommandRegistrar
|
||||
|
||||
# Create a mock extension command file
|
||||
ext_dir = tmp_path / "extension"
|
||||
ext_dir.mkdir()
|
||||
cmd_dir = ext_dir / "commands"
|
||||
cmd_dir.mkdir()
|
||||
|
||||
cmd_file = cmd_dir / "example.md"
|
||||
cmd_file.write_text(
|
||||
"---\n"
|
||||
"description: Test command with alias\n"
|
||||
"---\n\n"
|
||||
"Test content\n",
|
||||
encoding="utf-8"
|
||||
)
|
||||
|
||||
# Register with Forge including an alias
|
||||
registrar = CommandRegistrar()
|
||||
commands = [
|
||||
{
|
||||
"name": "speckit.my-extension.example",
|
||||
"file": "commands/example.md",
|
||||
"aliases": ["speckit.my-extension.ex"]
|
||||
}
|
||||
]
|
||||
|
||||
registrar.register_commands(
|
||||
"forge",
|
||||
commands,
|
||||
"test-extension",
|
||||
ext_dir,
|
||||
tmp_path
|
||||
)
|
||||
|
||||
# Check the alias file has hyphenated name in frontmatter
|
||||
alias_file = tmp_path / ".forge" / "commands" / "speckit.my-extension.ex.md"
|
||||
assert alias_file.exists()
|
||||
|
||||
content = alias_file.read_text(encoding="utf-8")
|
||||
# Parse frontmatter to validate alias name field precisely
|
||||
frontmatter, _ = registrar.parse_frontmatter(content)
|
||||
assert "name" in frontmatter, "name field should be injected in alias frontmatter"
|
||||
# Alias name field should also use hyphens
|
||||
assert frontmatter["name"] == "speckit-my-extension-ex"
|
||||
|
||||
def test_registrar_does_not_affect_other_agents(self, tmp_path):
|
||||
"""Verify format_name callback is Forge-specific and doesn't affect other agents."""
|
||||
from specify_cli.agents import CommandRegistrar
|
||||
|
||||
# Create a mock extension command file
|
||||
ext_dir = tmp_path / "extension"
|
||||
ext_dir.mkdir()
|
||||
cmd_dir = ext_dir / "commands"
|
||||
cmd_dir.mkdir()
|
||||
|
||||
cmd_file = cmd_dir / "example.md"
|
||||
cmd_file.write_text(
|
||||
"---\n"
|
||||
"description: Test command\n"
|
||||
"---\n\n"
|
||||
"Test content with $ARGUMENTS\n",
|
||||
encoding="utf-8"
|
||||
)
|
||||
|
||||
# Register with Windsurf (standard markdown agent without inject_name)
|
||||
registrar = CommandRegistrar()
|
||||
commands = [
|
||||
{
|
||||
"name": "speckit.my-extension.example",
|
||||
"file": "commands/example.md"
|
||||
}
|
||||
]
|
||||
|
||||
registrar.register_commands(
|
||||
"windsurf",
|
||||
commands,
|
||||
"test-extension",
|
||||
ext_dir,
|
||||
tmp_path
|
||||
)
|
||||
|
||||
# Windsurf uses standard markdown format without name injection.
|
||||
# The format_name callback should not be invoked for non-Forge agents.
|
||||
windsurf_cmd = tmp_path / ".windsurf" / "workflows" / "speckit.my-extension.example.md"
|
||||
assert windsurf_cmd.exists()
|
||||
|
||||
content = windsurf_cmd.read_text(encoding="utf-8")
|
||||
# Windsurf should NOT have a name field injected
|
||||
assert "name:" not in content, (
|
||||
"Windsurf should not inject name field - format_name callback should be Forge-only"
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user