mirror of
https://github.com/github/spec-kit.git
synced 2026-08-15 14:02:04 +02:00
[bug-fix] Fix preset-wrap-drops-argument-hint: inherit argument-hint from core template (#3996)
* Fix preset-wrap-drops-argument-hint: inherit argument-hint from core Apply the remediation from the bug assessment on issue #3991. Extend the inheritance allowlist in _register_skills and _compose_layers to include 'argument-hint', so wrap-strategy presets that omit this key will inherit it from the core template rather than silently dropping it and risking its value being leaked into description. Refs #3991 Assisted-by: GitHub Copilot (model: claude-sonnet-4.6, autonomous) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test(presets): guard wrap argument-hint inheritance for unmapped command The existing regression test for #3991 wraps `speckit.specify`, whose stem is in Claude's ARGUMENT_HINTS map. The string-injection fallback in post_process_skill_content re-adds argument-hint even when wrap composition drops it, so that test passes with or without the inheritance fix and does not actually guard the regression. Add a parallel test that wraps an extension-like command (`speckit.myfeature`) absent from ARGUMENT_HINTS, so the wrap-composition inheritance is the only path that can carry argument-hint into the SKILL.md. This test fails without the fix and passes with it. Refs #3991 Assisted-by: GitHub Copilot (model: claude-opus-4.8, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 970babe2-48cd-4c41-adae-0282d879a9ce --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Manfred Riem <15701806+mnriem@users.noreply.github.com> Copilot-Session: 970babe2-48cd-4c41-adae-0282d879a9ce
This commit is contained in:
co-authored by
Copilot App
github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Manfred Riem
parent
d751231468
commit
1a60d1b6b8
@@ -2759,7 +2759,7 @@ class PresetManager:
|
||||
if frontmatter.get("strategy") == "wrap":
|
||||
body, core_frontmatter = _substitute_core_template(body, cmd_name, self.project_root, registrar)
|
||||
frontmatter = dict(frontmatter)
|
||||
for key in ("scripts", "agent_scripts"):
|
||||
for key in ("scripts", "agent_scripts", "argument-hint"):
|
||||
if key not in frontmatter and key in core_frontmatter:
|
||||
frontmatter[key] = core_frontmatter[key]
|
||||
|
||||
@@ -5814,7 +5814,7 @@ class PresetResolver:
|
||||
# Inherit scripts/agent_scripts from base frontmatter if missing
|
||||
if base_frontmatter_text and base_frontmatter_text != top_frontmatter_text:
|
||||
base_fm = _parse_fm_yaml(base_frontmatter_text)
|
||||
for key in ("scripts", "agent_scripts"):
|
||||
for key in ("scripts", "agent_scripts", "argument-hint"):
|
||||
if key not in top_fm and key in base_fm:
|
||||
top_fm[key] = base_fm[key]
|
||||
|
||||
|
||||
@@ -4727,6 +4727,163 @@ class TestPresetSkills:
|
||||
parsed = yaml.safe_load(skill_file.read_text(encoding="utf-8").split("---", 2)[1])
|
||||
assert "argument-hint" not in parsed
|
||||
|
||||
def test_wrap_preset_inherits_argument_hint_from_core(self, project_dir, temp_dir):
|
||||
"""A wrap-strategy preset that omits argument-hint must inherit it from the core template.
|
||||
|
||||
Regression for issue #3991: the wrap-composition path in _register_skills
|
||||
previously inherited only scripts/agent_scripts from core_frontmatter,
|
||||
silently discarding argument-hint and leaking its value into description.
|
||||
"""
|
||||
core_arg_hint = "Describe the feature you want to specify"
|
||||
preset_description = "Wrapped speckit.specify — extra project context added"
|
||||
self._write_init_options(project_dir, ai="claude")
|
||||
skills_dir = project_dir / ".claude" / "skills"
|
||||
self._create_skill(skills_dir, "speckit-specify")
|
||||
|
||||
# Place a core template that declares argument-hint
|
||||
core_cmds = project_dir / ".specify" / "templates" / "commands"
|
||||
core_cmds.mkdir(parents=True, exist_ok=True)
|
||||
(core_cmds / "specify.md").write_text(
|
||||
"---\n"
|
||||
"description: Core specify description.\n"
|
||||
f'argument-hint: "{core_arg_hint}"\n'
|
||||
"---\n\n"
|
||||
"Core specify body.\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
# Wrap preset: only declares description (no argument-hint)
|
||||
preset_dir = temp_dir / "wrap-hint-preset"
|
||||
preset_dir.mkdir()
|
||||
(preset_dir / "commands").mkdir()
|
||||
(preset_dir / "commands" / "speckit.specify.md").write_text(
|
||||
"---\n"
|
||||
f'description: "{preset_description}"\n'
|
||||
"strategy: wrap\n"
|
||||
"---\n\n"
|
||||
"{CORE_TEMPLATE}\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
manifest_data = {
|
||||
"schema_version": "1.0",
|
||||
"preset": {
|
||||
"id": "wrap-hint-preset",
|
||||
"name": "Wrap Hint Preset",
|
||||
"version": "1.0.0",
|
||||
"description": "Test wrap hint inheritance",
|
||||
},
|
||||
"requires": {"speckit_version": ">=0.1.0"},
|
||||
"provides": {
|
||||
"templates": [
|
||||
{
|
||||
"type": "command",
|
||||
"name": "speckit.specify",
|
||||
"file": "commands/speckit.specify.md",
|
||||
"strategy": "wrap",
|
||||
}
|
||||
]
|
||||
},
|
||||
}
|
||||
import yaml as _yaml
|
||||
with open(preset_dir / "preset.yml", "w") as f:
|
||||
_yaml.dump(manifest_data, f)
|
||||
|
||||
manager = PresetManager(project_dir)
|
||||
manager.install_from_directory(preset_dir, "1.0.0")
|
||||
|
||||
skill_file = skills_dir / "speckit-specify" / "SKILL.md"
|
||||
assert skill_file.exists()
|
||||
parsed = yaml.safe_load(skill_file.read_text(encoding="utf-8").split("---", 2)[1])
|
||||
# argument-hint must be inherited from core, not dropped
|
||||
assert parsed.get("argument-hint") == core_arg_hint, (
|
||||
f"argument-hint was not inherited from core; parsed={parsed}"
|
||||
)
|
||||
# description must be exactly the preset's declared value, not concatenated
|
||||
assert parsed["description"] == preset_description, (
|
||||
f"description was corrupted; parsed={parsed}"
|
||||
)
|
||||
|
||||
def test_wrap_preset_inherits_argument_hint_for_unmapped_command(self, project_dir, temp_dir):
|
||||
"""Wrap inheritance must carry argument-hint for a command NOT in ARGUMENT_HINTS.
|
||||
|
||||
Regression guard for issue #3991. The companion test above wraps
|
||||
``speckit.specify``, whose stem is in Claude's ``ARGUMENT_HINTS`` map, so
|
||||
the string-injection fallback in ``post_process_skill_content`` re-adds
|
||||
``argument-hint`` even when wrap composition drops it — masking the bug.
|
||||
This test wraps an extension-like command (``speckit.myfeature``) that is
|
||||
absent from that map, so the *only* thing that can carry the hint into the
|
||||
SKILL.md is the wrap-composition inheritance fix itself. Without the fix
|
||||
the key is dropped and this test fails.
|
||||
"""
|
||||
core_arg_hint = "Custom hint that lives only on the core template"
|
||||
preset_description = "Wrapped speckit.myfeature — extra project context added"
|
||||
self._write_init_options(project_dir, ai="claude")
|
||||
skills_dir = project_dir / ".claude" / "skills"
|
||||
self._create_skill(skills_dir, "speckit-myfeature")
|
||||
|
||||
# Place a core template (extension-like command) that declares argument-hint
|
||||
core_cmds = project_dir / ".specify" / "templates" / "commands"
|
||||
core_cmds.mkdir(parents=True, exist_ok=True)
|
||||
(core_cmds / "myfeature.md").write_text(
|
||||
"---\n"
|
||||
"description: Core myfeature description.\n"
|
||||
f'argument-hint: "{core_arg_hint}"\n'
|
||||
"---\n\n"
|
||||
"Core myfeature body.\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
# Wrap preset: only declares description (no argument-hint)
|
||||
preset_dir = temp_dir / "wrap-hint-preset-unmapped"
|
||||
preset_dir.mkdir()
|
||||
(preset_dir / "commands").mkdir()
|
||||
(preset_dir / "commands" / "speckit.myfeature.md").write_text(
|
||||
"---\n"
|
||||
f'description: "{preset_description}"\n'
|
||||
"strategy: wrap\n"
|
||||
"---\n\n"
|
||||
"{CORE_TEMPLATE}\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
manifest_data = {
|
||||
"schema_version": "1.0",
|
||||
"preset": {
|
||||
"id": "wrap-hint-preset-unmapped",
|
||||
"name": "Wrap Hint Preset Unmapped",
|
||||
"version": "1.0.0",
|
||||
"description": "Test wrap hint inheritance for an unmapped command",
|
||||
},
|
||||
"requires": {"speckit_version": ">=0.1.0"},
|
||||
"provides": {
|
||||
"templates": [
|
||||
{
|
||||
"type": "command",
|
||||
"name": "speckit.myfeature",
|
||||
"file": "commands/speckit.myfeature.md",
|
||||
"strategy": "wrap",
|
||||
}
|
||||
]
|
||||
},
|
||||
}
|
||||
import yaml as _yaml
|
||||
with open(preset_dir / "preset.yml", "w") as f:
|
||||
_yaml.dump(manifest_data, f)
|
||||
|
||||
manager = PresetManager(project_dir)
|
||||
manager.install_from_directory(preset_dir, "1.0.0")
|
||||
|
||||
skill_file = skills_dir / "speckit-myfeature" / "SKILL.md"
|
||||
assert skill_file.exists()
|
||||
parsed = yaml.safe_load(skill_file.read_text(encoding="utf-8").split("---", 2)[1])
|
||||
# argument-hint must be inherited from core, not dropped
|
||||
assert parsed.get("argument-hint") == core_arg_hint, (
|
||||
f"argument-hint was not inherited from core; parsed={parsed}"
|
||||
)
|
||||
# description must be exactly the preset's declared value, not concatenated
|
||||
assert parsed["description"] == preset_description, (
|
||||
f"description was corrupted; parsed={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