Skip to content

Commit 7936320

Browse files
authored
fix(forge): use hyphen notation for command refs in Forge integration (#2462)
* fix(forge): use hyphen notation for command refs in Forge integration - Add invoke_separator = "-" class attribute to ForgeIntegration so effective_invoke_separator() returns "-" for shared-template installs - Add "invoke_separator": "-" to ForgeIntegration.registrar_config so agents.py CommandRegistrar can resolve refs with the correct separator - Pass invoke_separator to process_template() in ForgeIntegration.setup() so all .forge/commands/*.md bodies use /speckit-foo notation - Replace literal /speckit.specify with __SPECKIT_COMMAND_SPECIFY__ in extensions/git/commands/speckit.git.feature.md so every agent resolves the reference through its own separator - Apply resolve_command_refs re.sub in agents.py register_commands() after argument-placeholder substitution so extension commands registered for Forge get /speckit-foo refs; all other agents continue to get /speckit.foo Fixes ZSH compatibility: dot-notation command invocations (/speckit.specify) are misinterpreted by ZSH as file-path operations; hyphen notation (/speckit-specify) works correctly in all shells. * fix(agents): propagate invoke_separator from integration class into AGENT_CONFIGS Skills-based agents (claude, codex, kimi, …) inherit invoke_separator="-" from SkillsIntegration but do not repeat it in their registrar_config dicts. _build_agent_configs() was copying registrar_config verbatim, so register_commands() fell back to "." when resolving __SPECKIT_COMMAND_*__ tokens for those agents — emitting /speckit.specify instead of the correct /speckit-specify for extension commands like speckit.git.feature. Fix: after copying registrar_config, inject invoke_separator from the integration's class attribute when it is not already declared explicitly. This makes the integration class the single source of truth for all agents, without requiring each SkillsIntegration subclass to duplicate the field. Also replace the inline re.sub in register_commands() with a call to IntegrationBase.resolve_command_refs() (deferred import to avoid the existing circular dependency) so token-resolution logic is not duplicated. Adds two tests in test_agent_config_consistency.py: - test_skills_agents_have_hyphen_invoke_separator_in_agent_configs: asserts every /SKILL.md agent has invoke_separator="-" in AGENT_CONFIGS. - test_skills_agent_command_token_resolves_with_hyphen: end-to-end check via CommandRegistrar that the git extension's speckit.git.feature command is installed for Claude with /speckit-specify (not /speckit.specify). Addresses review comment on PR #2462.
1 parent c0bf5d0 commit 7936320

5 files changed

Lines changed: 215 additions & 17 deletions

File tree

extensions/git/commands/speckit.git.feature.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ description: "Create a feature branch with sequential or timestamp numbering"
44

55
# Create Feature Branch
66

7-
Create and switch to a new git feature branch for the given specification. This command handles **branch creation only** — the spec directory and files are created by the core `/speckit.specify` workflow.
7+
Create and switch to a new git feature branch for the given specification. This command handles **branch creation only** — the spec directory and files are created by the core `__SPECKIT_COMMAND_SPECIFY__` workflow.
88

99
## User Input
1010

src/specify_cli/agents.py

Lines changed: 48 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,12 @@
77
"""
88

99
import os
10-
from pathlib import Path
11-
from typing import Dict, List, Any, Optional
12-
1310
import platform
1411
import re
1512
from copy import deepcopy
13+
from pathlib import Path
14+
from typing import Any, Dict, List, Optional
15+
1616
import yaml
1717

1818

@@ -25,7 +25,16 @@ def _build_agent_configs() -> dict[str, Any]:
2525
if key == "generic":
2626
continue
2727
if integration.registrar_config:
28-
configs[key] = dict(integration.registrar_config)
28+
config = dict(integration.registrar_config)
29+
# Propagate invoke_separator from the integration class when the
30+
# registrar_config dict doesn't already declare it explicitly.
31+
# SkillsIntegration subclasses (claude, codex, …) set
32+
# invoke_separator="-" as a class attribute but omit it from
33+
# registrar_config, so without this they would fall back to "."
34+
# when register_commands() resolves __SPECKIT_COMMAND_*__ tokens.
35+
if "invoke_separator" not in config:
36+
config["invoke_separator"] = integration.invoke_separator
37+
configs[key] = config
2938
return configs
3039

3140

@@ -419,9 +428,7 @@ def _ensure_inside(candidate: Path, base: Path) -> None:
419428
normalized = Path(os.path.normpath(candidate))
420429
base_normalized = Path(os.path.normpath(base))
421430
if not normalized.is_relative_to(base_normalized):
422-
raise ValueError(
423-
f"Output path {candidate!r} escapes directory {base!r}"
424-
)
431+
raise ValueError(f"Output path {candidate!r} escapes directory {base!r}")
425432

426433
def register_commands(
427434
self,
@@ -471,7 +478,10 @@ def register_commands(
471478

472479
if frontmatter.get("strategy") == "wrap":
473480
from .presets import _substitute_core_template
474-
body, core_frontmatter = _substitute_core_template(body, cmd_name, project_root, self)
481+
482+
body, core_frontmatter = _substitute_core_template(
483+
body, cmd_name, project_root, self
484+
)
475485
frontmatter = dict(frontmatter)
476486
for key in ("scripts", "agent_scripts"):
477487
if key not in frontmatter and key in core_frontmatter:
@@ -492,6 +502,16 @@ def register_commands(
492502
body, "$ARGUMENTS", agent_config["args"]
493503
)
494504

505+
# Resolve __SPECKIT_COMMAND_*__ tokens using the agent's invoke separator.
506+
# The separator is sourced from agent_config (populated by _build_agent_configs,
507+
# which propagates each integration's invoke_separator class attribute).
508+
# Deferred import of IntegrationBase avoids a circular import at module load
509+
# (base.py itself imports CommandRegistrar lazily).
510+
from specify_cli.integrations.base import IntegrationBase # noqa: PLC0415
511+
512+
_sep = agent_config.get("invoke_separator", ".")
513+
body = IntegrationBase.resolve_command_refs(body, _sep)
514+
495515
output_name = self._compute_output_name(agent_name, cmd_name, agent_config)
496516

497517
if agent_config["extension"] == "/SKILL.md":
@@ -505,12 +525,22 @@ def register_commands(
505525
project_root,
506526
)
507527
elif agent_config["format"] == "markdown":
508-
body = self.resolve_skill_placeholders(agent_name, frontmatter, body, project_root)
509-
body = self._convert_argument_placeholder(body, "$ARGUMENTS", agent_config["args"])
510-
output = self.render_markdown_command(frontmatter, body, source_id, context_note)
528+
body = self.resolve_skill_placeholders(
529+
agent_name, frontmatter, body, project_root
530+
)
531+
body = self._convert_argument_placeholder(
532+
body, "$ARGUMENTS", agent_config["args"]
533+
)
534+
output = self.render_markdown_command(
535+
frontmatter, body, source_id, context_note
536+
)
511537
elif agent_config["format"] == "toml":
512-
body = self.resolve_skill_placeholders(agent_name, frontmatter, body, project_root)
513-
body = self._convert_argument_placeholder(body, "$ARGUMENTS", agent_config["args"])
538+
body = self.resolve_skill_placeholders(
539+
agent_name, frontmatter, body, project_root
540+
)
541+
body = self._convert_argument_placeholder(
542+
body, "$ARGUMENTS", agent_config["args"]
543+
)
514544
output = self.render_toml_command(frontmatter, body, source_id)
515545
elif agent_config["format"] == "yaml":
516546
output = self.render_yaml_command(
@@ -685,8 +715,11 @@ def register_commands_for_non_skill_agents(
685715
if agent_dir.exists():
686716
try:
687717
registered = self.register_commands(
688-
agent_name, commands, source_id,
689-
source_dir, project_root,
718+
agent_name,
719+
commands,
720+
source_id,
721+
source_dir,
722+
project_root,
690723
context_note=context_note,
691724
)
692725
if registered:

src/specify_cli/integrations/forge/__init__.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,10 @@ class ForgeIntegration(MarkdownIntegration):
8787
"strip_frontmatter_keys": ["handoffs"],
8888
"inject_name": True,
8989
"format_name": format_forge_command_name, # Custom name formatter
90+
"invoke_separator": "-",
9091
}
9192
context_file = "AGENTS.md"
93+
invoke_separator = "-"
9294

9395
def setup(
9496
self,
@@ -133,6 +135,7 @@ def setup(
133135
processed = self.process_template(
134136
raw, self.key, script_type, arg_placeholder,
135137
context_file=self.context_file or "",
138+
invoke_separator=self.invoke_separator,
136139
)
137140

138141
# FORGE-SPECIFIC: Ensure any remaining $ARGUMENTS placeholders are

tests/integrations/test_integration_forge.py

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ def test_directory_structure(self, tmp_path):
141141
assert actual_commands == expected_commands
142142

143143
def test_templates_are_processed(self, tmp_path):
144+
import re
144145
from specify_cli.integrations.forge import ForgeIntegration
145146
forge = ForgeIntegration()
146147
m = IntegrationManifest("forge", tmp_path)
@@ -157,6 +158,11 @@ def test_templates_are_processed(self, tmp_path):
157158
assert "$ARGUMENTS" not in content, f"{cmd_file.name} has unprocessed $ARGUMENTS"
158159
# Frontmatter sections should be stripped
159160
assert "\nscripts:\n" not in content
161+
# Check Forge-specific: command references use hyphen notation, not dot notation
162+
assert not re.search(r"/speckit\.[a-z]", content), (
163+
f"{cmd_file.name} contains dot-notation command reference (/speckit.<cmd>); "
164+
"Forge requires hyphen notation (/speckit-<cmd>) for ZSH compatibility"
165+
)
160166

161167
def test_plan_references_correct_context_file(self, tmp_path):
162168
"""The generated plan command must reference forge's context file."""
@@ -224,6 +230,33 @@ def test_uses_parameters_placeholder(self, tmp_path):
224230
"checklist should contain {{parameters}} in User Input section"
225231
)
226232

233+
def test_command_refs_use_hyphen_notation(self, tmp_path):
234+
"""Verify all generated Forge command files use /speckit-foo, not /speckit.foo."""
235+
import re
236+
from specify_cli.integrations.forge import ForgeIntegration
237+
forge = ForgeIntegration()
238+
m = IntegrationManifest("forge", tmp_path)
239+
forge.setup(tmp_path, m)
240+
commands_dir = tmp_path / ".forge" / "commands"
241+
242+
files_with_refs = []
243+
files_with_dot_refs = []
244+
for cmd_file in commands_dir.glob("speckit.*.md"):
245+
content = cmd_file.read_text(encoding="utf-8")
246+
if re.search(r"/speckit-[a-z]", content):
247+
files_with_refs.append(cmd_file.name)
248+
if re.search(r"/speckit\.[a-z]", content):
249+
files_with_dot_refs.append(cmd_file.name)
250+
251+
assert files_with_dot_refs == [], (
252+
f"Files contain dot-notation command references: {files_with_dot_refs}. "
253+
"Forge requires hyphen notation (/speckit-<cmd>) for ZSH compatibility."
254+
)
255+
assert len(files_with_refs) > 0, (
256+
"Expected at least one generated Forge command to contain /speckit-<cmd> reference, "
257+
"but none were found. Check that __SPECKIT_COMMAND_*__ tokens are being resolved."
258+
)
259+
227260
def test_name_field_uses_hyphenated_format(self, tmp_path):
228261
"""Verify that injected name fields use hyphenated format (speckit-plan, not speckit.plan)."""
229262
from specify_cli.integrations.forge import ForgeIntegration
@@ -401,3 +434,48 @@ def test_registrar_does_not_affect_other_agents(self, tmp_path):
401434
assert "name:" not in content, (
402435
"Windsurf should not inject name field - format_name callback should be Forge-only"
403436
)
437+
438+
def test_git_extension_command_uses_hyphen_notation(self, tmp_path):
439+
"""Verify the git extension's feature command uses /speckit-specify (not /speckit.specify) for Forge."""
440+
from pathlib import Path
441+
from specify_cli.agents import CommandRegistrar
442+
443+
# Locate the real git extension command source file
444+
repo_root = Path(__file__).resolve().parent.parent.parent
445+
ext_dir = repo_root / "extensions" / "git"
446+
cmd_source = ext_dir / "commands" / "speckit.git.feature.md"
447+
assert cmd_source.exists(), (
448+
f"Git extension command source not found at {cmd_source}. "
449+
"Ensure extensions/git/commands/speckit.git.feature.md exists."
450+
)
451+
452+
registrar = CommandRegistrar()
453+
commands = [
454+
{
455+
"name": "speckit.git.feature",
456+
"file": "commands/speckit.git.feature.md",
457+
}
458+
]
459+
460+
registered = registrar.register_commands(
461+
"forge",
462+
commands,
463+
"git",
464+
ext_dir,
465+
tmp_path,
466+
)
467+
468+
assert "speckit.git.feature" in registered
469+
470+
forge_cmd = tmp_path / ".forge" / "commands" / "speckit.git.feature.md"
471+
assert forge_cmd.exists(), "Expected Forge command file was not created"
472+
473+
content = forge_cmd.read_text(encoding="utf-8")
474+
assert "/speckit-specify" in content, (
475+
"Expected '/speckit-specify' (hyphen) in generated Forge git.feature command body, "
476+
"but it was not found. Check that __SPECKIT_COMMAND_SPECIFY__ is resolved correctly."
477+
)
478+
assert "/speckit.specify" not in content, (
479+
"Found '/speckit.specify' (dot notation) in generated Forge git.feature command body. "
480+
"Forge requires hyphen notation for ZSH compatibility."
481+
)

tests/test_agent_config_consistency.py

Lines changed: 85 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
from specify_cli import AGENT_CONFIG, AI_ASSISTANT_ALIASES, AI_ASSISTANT_HELP
66
from specify_cli.extensions import CommandRegistrar
77

8-
98
REPO_ROOT = Path(__file__).resolve().parent.parent
109

1110

@@ -199,3 +198,88 @@ def test_goose_in_extension_registrar(self):
199198
def test_ai_help_includes_goose(self):
200199
"""CLI help text for --ai should include goose."""
201200
assert "goose" in AI_ASSISTANT_HELP
201+
202+
# --- invoke_separator propagation checks ---
203+
204+
def test_skills_agents_have_hyphen_invoke_separator_in_agent_configs(self):
205+
"""Skills-based agents must expose invoke_separator='-' in AGENT_CONFIGS.
206+
207+
SkillsIntegration sets ``invoke_separator = "-"`` as a class attribute,
208+
but individual skills integrations (claude, codex, …) do not repeat it in
209+
their ``registrar_config`` dicts. ``_build_agent_configs()`` must
210+
propagate the class attribute so that ``register_commands()`` resolves
211+
``__SPECKIT_COMMAND_*__`` tokens with the correct hyphen separator.
212+
"""
213+
cfg = CommandRegistrar.AGENT_CONFIGS
214+
skills_agents = [
215+
key for key, c in cfg.items() if c.get("extension") == "/SKILL.md"
216+
]
217+
assert skills_agents, (
218+
"Expected at least one skills-based agent in AGENT_CONFIGS"
219+
)
220+
for agent in skills_agents:
221+
assert cfg[agent].get("invoke_separator") == "-", (
222+
f"Skills agent '{agent}' has invoke_separator="
223+
f"{cfg[agent].get('invoke_separator')!r} in AGENT_CONFIGS; "
224+
"expected '-' (propagated from SkillsIntegration.invoke_separator)"
225+
)
226+
227+
def test_skills_agent_command_token_resolves_with_hyphen(self, tmp_path):
228+
"""__SPECKIT_COMMAND_*__ tokens in extension commands resolve to /speckit-<cmd>
229+
when registered for a skills-based agent (e.g. claude).
230+
231+
Regression guard: before the fix, _build_agent_configs() did not
232+
propagate invoke_separator from the integration class, so
233+
register_commands() fell back to '.' and emitted /speckit.specify instead
234+
of /speckit-specify for skills agents.
235+
"""
236+
import re
237+
from pathlib import Path
238+
239+
from specify_cli.agents import CommandRegistrar
240+
241+
repo_root = Path(__file__).resolve().parent.parent
242+
ext_dir = repo_root / "extensions" / "git"
243+
cmd_source = ext_dir / "commands" / "speckit.git.feature.md"
244+
assert cmd_source.exists(), (
245+
f"Git extension command source not found at {cmd_source}"
246+
)
247+
assert "__SPECKIT_COMMAND_SPECIFY__" in cmd_source.read_text(
248+
encoding="utf-8"
249+
), (
250+
"Expected __SPECKIT_COMMAND_SPECIFY__ token in speckit.git.feature.md; "
251+
"check that the file uses the token rather than a hard-coded ref."
252+
)
253+
254+
registrar = CommandRegistrar()
255+
commands = [
256+
{"name": "speckit.git.feature", "file": "commands/speckit.git.feature.md"}
257+
]
258+
259+
registered = registrar.register_commands(
260+
"claude",
261+
commands,
262+
"git",
263+
ext_dir,
264+
tmp_path,
265+
)
266+
267+
assert "speckit.git.feature" in registered
268+
skill_file = (
269+
tmp_path / ".claude" / "skills" / "speckit-git-feature" / "SKILL.md"
270+
)
271+
assert skill_file.exists(), (
272+
f"Expected Claude skill file not found at {skill_file}"
273+
)
274+
content = skill_file.read_text(encoding="utf-8")
275+
assert "/speckit-specify" in content, (
276+
"Expected '/speckit-specify' (hyphen) in generated Claude skill for git.feature; "
277+
"__SPECKIT_COMMAND_SPECIFY__ was not resolved with the correct separator."
278+
)
279+
# Negative lookbehind (?<![a-zA-Z0-9_]) excludes file-path occurrences
280+
# such as 'source: git:commands/speckit.git.feature.md' in frontmatter,
281+
# where the '/' is a path separator preceded by a word character.
282+
assert not re.search(r"(?<![a-zA-Z0-9_])/speckit\.[a-z]", content), (
283+
"Found dot-notation command ref (/speckit.<cmd>) in generated Claude skill. "
284+
"Skills agents must use hyphen notation."
285+
)

0 commit comments

Comments
 (0)