Skip to content

Commit 6483a0f

Browse files
author
iamaeroplane
committed
fix: address remaining PR2103 review threads
1 parent 23c9452 commit 6483a0f

3 files changed

Lines changed: 151 additions & 5 deletions

File tree

src/specify_cli/extensions.py

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,11 @@
2222
import pathspec
2323

2424
import yaml
25-
from specify_cli.behavior import get_deployment_type as _get_deployment_type
25+
from specify_cli.behavior import (
26+
get_deployment_type as _get_deployment_type,
27+
strip_behavior_keys,
28+
translate_behavior,
29+
)
2630
from packaging import version as pkg_version
2731
from packaging.specifiers import SpecifierSet, InvalidSpecifier
2832

@@ -930,6 +934,19 @@ def _register_extension_skills(
930934

931935
skill_subdir.mkdir(parents=True, exist_ok=True)
932936
frontmatter, body = registrar.parse_frontmatter(content)
937+
938+
for key in ("agents",):
939+
if key in cmd_info and key not in frontmatter:
940+
frontmatter[key] = cmd_info[key]
941+
if "behavior" in cmd_info:
942+
manifest_behavior = cmd_info["behavior"]
943+
if isinstance(manifest_behavior, dict):
944+
source_behavior = frontmatter.get("behavior")
945+
if isinstance(source_behavior, dict):
946+
frontmatter["behavior"] = {**manifest_behavior, **source_behavior}
947+
else:
948+
frontmatter["behavior"] = manifest_behavior
949+
933950
frontmatter = registrar._adjust_script_paths(frontmatter)
934951
body = registrar.rewrite_extension_paths(body, manifest.id, extension_dir)
935952
body = registrar.resolve_skill_placeholders(
@@ -939,12 +956,27 @@ def _register_extension_skills(
939956
original_desc = frontmatter.get("description", "")
940957
description = original_desc or f"Extension command: {cmd_name}"
941958

959+
clean_frontmatter = strip_behavior_keys(frontmatter)
942960
frontmatter_data = registrar.build_skill_frontmatter(
943961
selected_ai,
944962
skill_name,
945963
description,
946964
f"extension:{manifest.id}",
965+
source_frontmatter=clean_frontmatter,
947966
)
967+
968+
behavior = frontmatter.get("behavior") if isinstance(frontmatter.get("behavior"), dict) else {}
969+
agents_overrides = frontmatter.get("agents") or {}
970+
behavior_fields = translate_behavior(
971+
selected_ai,
972+
behavior,
973+
agents_overrides if isinstance(agents_overrides, dict) else {},
974+
)
975+
overridable_defaults = {"disable-model-invocation", "user-invocable"}
976+
for key, value in behavior_fields.items():
977+
if key not in frontmatter_data or key in overridable_defaults:
978+
frontmatter_data[key] = value
979+
948980
frontmatter_text = yaml.safe_dump(frontmatter_data, sort_keys=False).strip()
949981

950982
# Derive a human-friendly title from the command name

src/specify_cli/integrations/base.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1465,14 +1465,20 @@ def _quote(v: str) -> str:
14651465
# to produce `disable-model-invocation: false` in the skill.
14661466
# Fields are emitted here so downstream post-processors (e.g.
14671467
# ClaudeIntegration.setup) see them already set and skip injection.
1468-
behavior = frontmatter.get("behavior") or {}
1468+
behavior = frontmatter.get("behavior")
1469+
behavior_dict = behavior if isinstance(behavior, dict) else {}
1470+
agents_overrides = frontmatter.get("agents") or {}
1471+
has_agent_override = (
1472+
isinstance(agents_overrides, dict)
1473+
and isinstance(agents_overrides.get(self.key), dict)
1474+
)
14691475
behavior_fm_lines = ""
1470-
if isinstance(behavior, dict) and behavior:
1476+
if behavior_dict or has_agent_override:
14711477
try:
14721478
from specify_cli.behavior import translate_behavior
1473-
agents_overrides = frontmatter.get("agents") or {}
1479+
14741480
behavior_fields = translate_behavior(
1475-
self.key, behavior,
1481+
self.key, behavior_dict,
14761482
agents_overrides if isinstance(agents_overrides, dict) else {}
14771483
)
14781484
for bk, bv in behavior_fields.items():

tests/test_pr2103_reviews.py

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,71 @@ def test_no_behavior_no_manifest_behavior_writes_skill(self, tmp):
338338
assert skill_file.exists()
339339

340340

341+
class TestExtensionSkillFrontmatterPropagation:
342+
"""Issue 6: extension skill rendering must preserve merged source frontmatter."""
343+
344+
@pytest.fixture
345+
def tmp(self):
346+
d = tempfile.mkdtemp()
347+
yield Path(d)
348+
shutil.rmtree(d)
349+
350+
def test_manifest_behavior_is_applied_to_skill_frontmatter(self, tmp):
351+
"""Manifest-level behavior must affect generated SKILL.md frontmatter."""
352+
ext_dir = _make_ext_dir(tmp, "ext3", [{
353+
"name": "speckit.ext3.cmd",
354+
"file": "commands/cmd.md",
355+
"behavior": {"invocation": "automatic"},
356+
}])
357+
(ext_dir / "commands" / "cmd.md").write_text(
358+
"---\n"
359+
"description: Test\n"
360+
"---\n\n"
361+
"Body"
362+
)
363+
364+
proj = _make_project(tmp, ai="claude", ai_skills=True)
365+
(proj / ".claude" / "skills").mkdir(parents=True)
366+
367+
from specify_cli.extensions import ExtensionManifest, ExtensionManager
368+
369+
manifest = ExtensionManifest(ext_dir / "extension.yml")
370+
mgr = ExtensionManager(proj)
371+
created = mgr._register_extension_skills(manifest, ext_dir)
372+
assert created == ["speckit-ext3-cmd"]
373+
374+
skill_file = proj / ".claude" / "skills" / "speckit-ext3-cmd" / "SKILL.md"
375+
parsed = _parse_frontmatter(skill_file.read_text())
376+
assert parsed.get("disable-model-invocation") is False, parsed
377+
378+
def test_source_frontmatter_passthrough_is_preserved(self, tmp):
379+
"""Passthrough keys like model must be preserved in generated SKILL.md."""
380+
ext_dir = _make_ext_dir(tmp, "ext4", [{
381+
"name": "speckit.ext4.cmd",
382+
"file": "commands/cmd.md",
383+
}])
384+
(ext_dir / "commands" / "cmd.md").write_text(
385+
"---\n"
386+
"description: Test\n"
387+
"model: claude-opus-4-6\n"
388+
"---\n\n"
389+
"Body"
390+
)
391+
392+
proj = _make_project(tmp, ai="claude", ai_skills=True)
393+
(proj / ".claude" / "skills").mkdir(parents=True)
394+
395+
from specify_cli.extensions import ExtensionManifest, ExtensionManager
396+
397+
manifest = ExtensionManifest(ext_dir / "extension.yml")
398+
mgr = ExtensionManager(proj)
399+
mgr._register_extension_skills(manifest, ext_dir)
400+
401+
skill_file = proj / ".claude" / "skills" / "speckit-ext4-cmd" / "SKILL.md"
402+
parsed = _parse_frontmatter(skill_file.read_text())
403+
assert parsed.get("model") == "claude-opus-4-6", parsed
404+
405+
341406
# ---------------------------------------------------------------------------
342407
# Issue 3 — SkillsIntegration.setup() ignores agents: override
343408
# ---------------------------------------------------------------------------
@@ -406,6 +471,49 @@ def test_agents_override_applied_in_skills_integration(self, tmp):
406471
f"agents: override not applied; got frontmatter: {parsed}"
407472
)
408473

474+
def test_agents_override_without_behavior_is_applied(self, tmp):
475+
"""agents-only template frontmatter must still apply for the target agent."""
476+
from specify_cli.integrations import get_integration
477+
from specify_cli.integrations.manifest import IntegrationManifest
478+
479+
templates_dir = (
480+
Path(__file__).resolve().parent.parent / "templates" / "commands"
481+
)
482+
if not templates_dir.is_dir():
483+
pytest.skip("templates/commands not available")
484+
485+
src_template = next(iter(templates_dir.glob("*.md")), None)
486+
if src_template is None:
487+
pytest.skip("no template files found")
488+
489+
original = src_template.read_text()
490+
patched = (
491+
"---\n"
492+
"description: Test patched template\n"
493+
"agents:\n"
494+
" codex:\n"
495+
" effort: max\n"
496+
"---\n" + original.split("---", 2)[-1] if "---" in original else original
497+
)
498+
499+
patched_dir = tmp / "patched_templates_agents_only"
500+
patched_dir.mkdir()
501+
(patched_dir / src_template.name).write_text(patched)
502+
503+
integration = get_integration("codex")
504+
if integration is None:
505+
pytest.skip("codex integration not registered")
506+
507+
m = IntegrationManifest("codex", tmp)
508+
with patch.object(type(integration), "list_command_templates", return_value=list(patched_dir.glob("*.md"))):
509+
created = integration.setup(tmp, m)
510+
511+
skill_files = [f for f in created if f.name == "SKILL.md"]
512+
assert skill_files, "no SKILL.md files created"
513+
for sf in skill_files:
514+
parsed = _parse_frontmatter(sf.read_text())
515+
assert parsed.get("effort") == "max", parsed
516+
409517
def test_agents_override_for_other_agent_not_applied(self, tmp):
410518
"""agents: override for a different agent must not bleed into codex output."""
411519
from specify_cli.integrations import get_integration

0 commit comments

Comments
 (0)