mirror of
https://github.com/github/spec-kit.git
synced 2026-07-04 04:45:43 +08:00
fix(presets): preserve argument-hint in preset SKILL.md generation (#2978)
* fix(presets): preserve argument-hint in preset SKILL.md generation Preset-provided and extension-override commands that declare `argument-hint:` in their frontmatter had it dropped from the generated Claude SKILL.md, and it was re-dropped when a preset was removed and its overridden skill restored. This is the preset-side analog of the extension fix in #2903 / #2916. Factor the argument-hint carry-over into a shared CommandRegistrar.apply_argument_hint() helper and apply it at the four preset skill-generation sites (register, reconcile override-restore, and the core/extension unregister-restore paths). The extension path from The helper writes argument-hint into the frontmatter dict before serialization (so a folded multi-line description cannot be split into invalid YAML) and only for integrations that support it (those exposing inject_argument_hint -- currently Claude), leaving build_skill_frontmatter's shared shape unchanged for every other agent. Core templates carry no argument-hint, so the core-restore path is a no-op. No behavior change for non-Claude agents or the core path. Add regression tests covering a folding description (Claude) and the non-Claude gate (codex). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(presets): address review - guard skill_frontmatter type and tighten apply_argument_hint annotations Add a symmetric isinstance(skill_frontmatter, dict) guard so the helper stays a safe no-op if a caller passes a non-dict, and annotate the parameters as Dict[str, Any] with an optional integration to match real call-site usage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -356,6 +356,33 @@ class CommandRegistrar:
|
||||
}
|
||||
return skill_frontmatter
|
||||
|
||||
@staticmethod
|
||||
def apply_argument_hint(
|
||||
source_frontmatter: Dict[str, Any],
|
||||
skill_frontmatter: Dict[str, Any],
|
||||
integration: Optional[object] = None,
|
||||
) -> None:
|
||||
"""Carry a command's ``argument-hint`` into its generated skill frontmatter.
|
||||
|
||||
Copies ``argument-hint`` from the parsed source command frontmatter into
|
||||
*skill_frontmatter* (mutated in place) before serialization, so that a
|
||||
folded multi-line ``description`` cannot be split into invalid YAML. Only
|
||||
integrations that support the field — those exposing
|
||||
``inject_argument_hint`` (currently Claude) — receive the key, leaving
|
||||
:meth:`build_skill_frontmatter`'s shared shape unchanged for every other
|
||||
agent. Built-in templates carry no ``argument-hint``, so this is a no-op
|
||||
for the core path.
|
||||
"""
|
||||
if not isinstance(source_frontmatter, dict) or not isinstance(skill_frontmatter, dict):
|
||||
return
|
||||
argument_hint = source_frontmatter.get("argument-hint")
|
||||
if (
|
||||
argument_hint
|
||||
and integration is not None
|
||||
and hasattr(integration, "inject_argument_hint")
|
||||
):
|
||||
skill_frontmatter["argument-hint"] = str(argument_hint)
|
||||
|
||||
@staticmethod
|
||||
def resolve_skill_placeholders(
|
||||
agent_name: str, frontmatter: dict, body: str, project_root: Path
|
||||
|
||||
@@ -1061,20 +1061,10 @@ class ExtensionManager:
|
||||
)
|
||||
# Preserve the command's argument-hint in the generated skill,
|
||||
# mirroring the core template path (ClaudeIntegration.setup injects
|
||||
# it for built-in commands). The value is added to the frontmatter
|
||||
# dict before serialization — rather than via the string-based
|
||||
# inject_argument_hint helper — so that a folded multi-line
|
||||
# description cannot be split by the inserted line. Gated on the
|
||||
# integration exposing inject_argument_hint so only argument-hint
|
||||
# aware agents receive the key, leaving build_skill_frontmatter's
|
||||
# shared shape unchanged for every other agent.
|
||||
argument_hint = frontmatter.get("argument-hint")
|
||||
if (
|
||||
argument_hint
|
||||
and integration is not None
|
||||
and hasattr(integration, "inject_argument_hint")
|
||||
):
|
||||
frontmatter_data["argument-hint"] = str(argument_hint)
|
||||
# it for built-in commands). See CommandRegistrar.apply_argument_hint
|
||||
# for why the value is added to the dict before serialization rather
|
||||
# than via the string-based inject_argument_hint helper.
|
||||
registrar.apply_argument_hint(frontmatter, frontmatter_data, integration)
|
||||
frontmatter_text = dump_frontmatter(frontmatter_data)
|
||||
|
||||
# Derive a human-friendly title from the command name
|
||||
|
||||
@@ -1064,11 +1064,14 @@ class PresetManager:
|
||||
body = self._resolve_skill_command_refs(
|
||||
body, registrar, selected_ai
|
||||
)
|
||||
from ..integrations import get_integration
|
||||
integration = get_integration(selected_ai) if isinstance(selected_ai, str) else None
|
||||
fm_data = registrar.build_skill_frontmatter(
|
||||
selected_ai if isinstance(selected_ai, str) else "",
|
||||
skill_name, desc,
|
||||
f"override:{cmd_name}",
|
||||
)
|
||||
registrar.apply_argument_hint(fm, fm_data, integration)
|
||||
fm_text = dump_frontmatter(fm_data)
|
||||
skill_title = self._skill_title_from_command(cmd_name)
|
||||
skill_content = (
|
||||
@@ -1076,8 +1079,6 @@ class PresetManager:
|
||||
f"# Speckit {skill_title} Skill\n\n{body}\n"
|
||||
)
|
||||
# Apply integration post-processing (e.g. Claude flags)
|
||||
from ..integrations import get_integration
|
||||
integration = get_integration(selected_ai) if isinstance(selected_ai, str) else None
|
||||
if integration is not None and hasattr(integration, "post_process_skill_content"):
|
||||
skill_content = integration.post_process_skill_content(skill_content)
|
||||
skill_file.write_text(skill_content, encoding="utf-8")
|
||||
@@ -1346,6 +1347,7 @@ class PresetManager:
|
||||
enhanced_desc,
|
||||
f"preset:{manifest.id}",
|
||||
)
|
||||
registrar.apply_argument_hint(frontmatter, frontmatter_data, integration)
|
||||
frontmatter_text = dump_frontmatter(frontmatter_data)
|
||||
skill_content = (
|
||||
f"---\n"
|
||||
@@ -1442,6 +1444,7 @@ class PresetManager:
|
||||
enhanced_desc,
|
||||
f"templates/commands/{short_name}.md",
|
||||
)
|
||||
registrar.apply_argument_hint(frontmatter, frontmatter_data, integration)
|
||||
frontmatter_text = dump_frontmatter(frontmatter_data)
|
||||
skill_title = self._skill_title_from_command(short_name)
|
||||
skill_content = (
|
||||
@@ -1479,6 +1482,7 @@ class PresetManager:
|
||||
frontmatter.get("description", f"Extension command: {command_name}"),
|
||||
extension_restore["source"],
|
||||
)
|
||||
registrar.apply_argument_hint(frontmatter, frontmatter_data, integration)
|
||||
frontmatter_text = dump_frontmatter(frontmatter_data)
|
||||
skill_content = (
|
||||
f"---\n"
|
||||
|
||||
@@ -2997,6 +2997,84 @@ class TestPresetSkills:
|
||||
metadata = manager.registry.get("self-test")
|
||||
assert "speckit-specify" in metadata.get("registered_skills", [])
|
||||
|
||||
def _install_arg_hint_preset(self, project_dir, temp_dir, ai, skills_dir, description, arg_hint):
|
||||
"""Install a preset whose command declares argument-hint; return the SKILL.md path."""
|
||||
self._write_init_options(project_dir, ai=ai)
|
||||
self._create_skill(skills_dir, "speckit-hinttest-cmd")
|
||||
(project_dir / ".specify" / "extensions" / "hinttest").mkdir(parents=True, exist_ok=True)
|
||||
|
||||
preset_dir = temp_dir / f"hint-preset-{ai}"
|
||||
preset_dir.mkdir()
|
||||
(preset_dir / "commands").mkdir()
|
||||
(preset_dir / "commands" / "speckit.hinttest.cmd.md").write_text(
|
||||
"---\n"
|
||||
f'description: "{description}"\n'
|
||||
f'argument-hint: "{arg_hint}"\n'
|
||||
"---\n\n"
|
||||
"Preset command body.\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
manifest_data = {
|
||||
"schema_version": "1.0",
|
||||
"preset": {
|
||||
"id": f"hint-preset-{ai}",
|
||||
"name": "Hint Preset",
|
||||
"version": "1.0.0",
|
||||
"description": "Test",
|
||||
},
|
||||
"requires": {"speckit_version": ">=0.1.0"},
|
||||
"provides": {
|
||||
"templates": [
|
||||
{
|
||||
"type": "command",
|
||||
"name": "speckit.hinttest.cmd",
|
||||
"file": "commands/speckit.hinttest.cmd.md",
|
||||
}
|
||||
]
|
||||
},
|
||||
}
|
||||
with open(preset_dir / "preset.yml", "w") as f:
|
||||
yaml.dump(manifest_data, f)
|
||||
|
||||
manager = PresetManager(project_dir)
|
||||
manager.install_from_directory(preset_dir, "0.1.5")
|
||||
return skills_dir / "speckit-hinttest-cmd" / "SKILL.md"
|
||||
|
||||
def test_argument_hint_preserved_for_preset_command(self, project_dir, temp_dir):
|
||||
"""argument-hint from a preset command must survive into the SKILL.md.
|
||||
|
||||
Follow-up to #2903/#2916 for the preset skill generator. The
|
||||
description is long enough to fold across lines when serialized,
|
||||
guarding against an in-place string injection that would split the
|
||||
folded scalar into invalid YAML.
|
||||
"""
|
||||
long_description = (
|
||||
"Build and maintain a lean, static context/ knowledge folder so "
|
||||
"coding agents load only what is relevant and save tokens"
|
||||
)
|
||||
arg_hint = "<init | update | list | check> [area] [slug] [-- notes]"
|
||||
skills_dir = project_dir / ".claude" / "skills"
|
||||
|
||||
skill_file = self._install_arg_hint_preset(
|
||||
project_dir, temp_dir, "claude", skills_dir, long_description, arg_hint
|
||||
)
|
||||
assert skill_file.exists()
|
||||
parsed = yaml.safe_load(skill_file.read_text(encoding="utf-8").split("---", 2)[1])
|
||||
assert parsed["argument-hint"] == arg_hint
|
||||
assert parsed["description"] == long_description
|
||||
|
||||
def test_argument_hint_not_added_for_non_claude_preset_command(self, project_dir, temp_dir):
|
||||
"""Non-Claude skills agents must not receive argument-hint in preset skills."""
|
||||
arg_hint = "<init | update | list | check> [area]"
|
||||
skills_dir = project_dir / ".agents" / "skills"
|
||||
|
||||
skill_file = self._install_arg_hint_preset(
|
||||
project_dir, temp_dir, "codex", skills_dir, "Build context", arg_hint
|
||||
)
|
||||
assert skill_file.exists()
|
||||
parsed = yaml.safe_load(skill_file.read_text(encoding="utf-8").split("---", 2)[1])
|
||||
assert "argument-hint" not in parsed
|
||||
|
||||
def test_register_skills_resolves_command_refs(self, project_dir, temp_dir):
|
||||
"""Preset skill overrides must resolve __SPECKIT_COMMAND_*__ tokens (issue #2717).
|
||||
|
||||
|
||||
Reference in New Issue
Block a user