From 51c2631981378c801cdad0e6d872d45480f283f0 Mon Sep 17 00:00:00 2001 From: jawwad-ali Date: Sun, 14 Jun 2026 16:09:42 +0500 Subject: [PATCH] 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 #2916 now uses the same helper. 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) --- src/specify_cli/agents.py | 27 ++++++++++++ src/specify_cli/extensions.py | 18 ++------ src/specify_cli/presets.py | 8 +++- tests/test_presets.py | 78 +++++++++++++++++++++++++++++++++++ 4 files changed, 115 insertions(+), 16 deletions(-) diff --git a/src/specify_cli/agents.py b/src/specify_cli/agents.py index 3c06418014..a5dda826ae 100644 --- a/src/specify_cli/agents.py +++ b/src/specify_cli/agents.py @@ -356,6 +356,33 @@ def build_skill_frontmatter( } return skill_frontmatter + @staticmethod + def apply_argument_hint( + source_frontmatter: dict, + skill_frontmatter: dict, + integration: object, + ) -> 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): + 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 diff --git a/src/specify_cli/extensions.py b/src/specify_cli/extensions.py index db53b7997f..6cea928274 100644 --- a/src/specify_cli/extensions.py +++ b/src/specify_cli/extensions.py @@ -1055,20 +1055,10 @@ def _register_extension_skills( ) # 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 = yaml.safe_dump(frontmatter_data, sort_keys=False).strip() # Derive a human-friendly title from the command name diff --git a/src/specify_cli/presets.py b/src/specify_cli/presets.py index a6b2ded49f..6111e7130e 100644 --- a/src/specify_cli/presets.py +++ b/src/specify_cli/presets.py @@ -1063,11 +1063,14 @@ def _reconcile_skills(self, command_names: List[str]) -> None: 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 = yaml.safe_dump(fm_data, sort_keys=False).strip() skill_title = self._skill_title_from_command(cmd_name) skill_content = ( @@ -1075,8 +1078,6 @@ def _reconcile_skills(self, command_names: List[str]) -> None: 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") @@ -1345,6 +1346,7 @@ def _register_skills( enhanced_desc, f"preset:{manifest.id}", ) + registrar.apply_argument_hint(frontmatter, frontmatter_data, integration) frontmatter_text = yaml.safe_dump(frontmatter_data, sort_keys=False).strip() skill_content = ( f"---\n" @@ -1441,6 +1443,7 @@ def _unregister_skills(self, skill_names: List[str], preset_dir: Path) -> None: enhanced_desc, f"templates/commands/{short_name}.md", ) + registrar.apply_argument_hint(frontmatter, frontmatter_data, integration) frontmatter_text = yaml.safe_dump(frontmatter_data, sort_keys=False).strip() skill_title = self._skill_title_from_command(short_name) skill_content = ( @@ -1478,6 +1481,7 @@ def _unregister_skills(self, skill_names: List[str], preset_dir: Path) -> None: frontmatter.get("description", f"Extension command: {command_name}"), extension_restore["source"], ) + registrar.apply_argument_hint(frontmatter, frontmatter_data, integration) frontmatter_text = yaml.safe_dump(frontmatter_data, sort_keys=False).strip() skill_content = ( f"---\n" diff --git a/tests/test_presets.py b/tests/test_presets.py index e32440145d..fd12210776 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -2997,6 +2997,84 @@ def test_skill_overridden_on_preset_install(self, project_dir, temp_dir): 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 = " [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 = " [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).