From 71143598bec2bc44e8696fad6a3e6c5b2ec9a961 Mon Sep 17 00:00:00 2001 From: toxicafunk Date: Wed, 8 Apr 2026 21:37:19 +0200 Subject: [PATCH] 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 --- src/specify_cli/agents.py | 8 +- .../integrations/forge/__init__.py | 56 ++++- tests/integrations/test_integration_forge.py | 226 +++++++++++++++++- 3 files changed, 282 insertions(+), 8 deletions(-) diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index 4b869283c..ec7af8876 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -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( diff --git a/src/specify_cli/integrations/forge/__init__.py b/src/specify_cli/integrations/forge/__init__.py index e3d534727..e1c4d9da6 100644 --- a/src/specify_cli/integrations/forge/__init__.py +++ b/src/specify_cli/integrations/forge/__init__.py @@ -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 diff --git a/tests/integrations/test_integration_forge.py b/tests/integrations/test_integration_forge.py index 10905723f..7affd0d16 100644 --- a/tests/integrations/test_integration_forge.py +++ b/tests/integrations/test_integration_forge.py @@ -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" + )