Skip to content

Commit d34f068

Browse files
author
iamaeroplane
committed
fix: address remaining Copilot review comments
1 parent 6483a0f commit d34f068

4 files changed

Lines changed: 118 additions & 3 deletions

File tree

src/specify_cli/agents.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -721,9 +721,14 @@ def register_commands(
721721
elif agent_config["format"] == "markdown":
722722
body = self.resolve_skill_placeholders(agent_name, frontmatter, body, project_root)
723723
body = self._convert_argument_placeholder(body, "$ARGUMENTS", agent_config["args"])
724-
behavior = frontmatter.get("behavior") if isinstance(frontmatter.get("behavior"), dict) else {}
725-
if agent_name == "copilot" and behavior:
726-
agents_overrides = frontmatter.get("agents") or {}
724+
behavior_raw = frontmatter.get("behavior")
725+
behavior = behavior_raw if isinstance(behavior_raw, dict) else {}
726+
agents_overrides = frontmatter.get("agents") or {}
727+
has_copilot_override = (
728+
isinstance(agents_overrides, dict)
729+
and isinstance(agents_overrides.get("copilot"), dict)
730+
)
731+
if agent_name == "copilot" and (isinstance(behavior_raw, dict) or has_copilot_override):
727732
extra_fields = translate_behavior(
728733
agent_name, behavior,
729734
agents_overrides if isinstance(agents_overrides, dict) else {}

src/specify_cli/integrations/base.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1486,6 +1486,14 @@ def _quote(v: str) -> str:
14861486
behavior_fm_lines += f"{bk}: {'true' if bv else 'false'}\n"
14871487
elif isinstance(bv, str):
14881488
behavior_fm_lines += f"{bk}: {_quote(bv)}\n"
1489+
elif isinstance(bv, (list, dict)):
1490+
dumped = yaml.safe_dump(
1491+
{bk: bv},
1492+
sort_keys=False,
1493+
allow_unicode=True,
1494+
default_flow_style=False,
1495+
).rstrip()
1496+
behavior_fm_lines += dumped + "\n"
14891497
else:
14901498
behavior_fm_lines += f"{bk}: {bv}\n"
14911499
except ImportError:

tests/test_agent_deployment.py

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,57 @@ def test_copilot_agents_override_survives(self, tmp_path):
279279
fm = yaml.safe_load(parts[1]) or {}
280280
assert fm.get("someCustomKey") == "someValue"
281281

282+
def test_copilot_agents_override_without_behavior_survives(self, tmp_path):
283+
"""Copilot agents override should apply even when behavior is absent."""
284+
root = self._setup_copilot_project(tmp_path)
285+
src = tmp_path / "ext" / "commands"
286+
src.mkdir(parents=True)
287+
(src / "custom2.md").write_text(dedent("""\
288+
---
289+
description: Custom command
290+
agents:
291+
copilot:
292+
someCustomKey: someValue
293+
---
294+
Do custom work
295+
"""))
296+
registrar = CommandRegistrar()
297+
registrar.register_commands(
298+
"copilot",
299+
[{"name": "speckit.test-ext.custom2", "file": "custom2.md"}],
300+
"test-ext", src, root,
301+
)
302+
agent_file = root / ".github" / "agents" / "speckit.test-ext.custom2.agent.md"
303+
content = agent_file.read_text()
304+
parts = content.split("---")
305+
fm = yaml.safe_load(parts[1]) or {}
306+
assert fm.get("someCustomKey") == "someValue"
307+
assert "agents" not in fm
308+
309+
def test_copilot_empty_behavior_is_stripped(self, tmp_path):
310+
"""Empty behavior mapping should still be stripped from Copilot output."""
311+
root = self._setup_copilot_project(tmp_path)
312+
src = tmp_path / "ext" / "commands"
313+
src.mkdir(parents=True)
314+
(src / "empty-behavior.md").write_text(dedent("""\
315+
---
316+
description: Empty behavior
317+
behavior: {}
318+
---
319+
Do work
320+
"""))
321+
registrar = CommandRegistrar()
322+
registrar.register_commands(
323+
"copilot",
324+
[{"name": "speckit.test-ext.empty-behavior", "file": "empty-behavior.md"}],
325+
"test-ext", src, root,
326+
)
327+
agent_file = root / ".github" / "agents" / "speckit.test-ext.empty-behavior.agent.md"
328+
content = agent_file.read_text()
329+
parts = content.split("---")
330+
fm = yaml.safe_load(parts[1]) or {}
331+
assert "behavior" not in fm
332+
282333
def test_copilot_non_dict_behavior_does_not_raise(self, tmp_path):
283334
"""A non-dict behavior value in Copilot execution:agent branch must not raise."""
284335
root = self._setup_copilot_project(tmp_path)

tests/test_pr2103_reviews.py

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -598,6 +598,57 @@ def test_empty_agents_override_does_not_crash(self, tmp):
598598

599599
assert len(created) > 0
600600

601+
def test_agents_override_non_scalar_yaml_remains_valid(self, tmp):
602+
"""Non-scalar override values must be emitted as valid YAML frontmatter."""
603+
from specify_cli.integrations import get_integration
604+
from specify_cli.integrations.manifest import IntegrationManifest
605+
606+
templates_dir = (
607+
Path(__file__).resolve().parent.parent / "templates" / "commands"
608+
)
609+
if not templates_dir.is_dir():
610+
pytest.skip("templates/commands not available")
611+
612+
src_template = next(iter(templates_dir.glob("*.md")), None)
613+
if src_template is None:
614+
pytest.skip("no template files found")
615+
616+
original_text = src_template.read_text()
617+
patched = (
618+
"---\n"
619+
"description: Test template\n"
620+
"agents:\n"
621+
" claude:\n"
622+
" handoffs:\n"
623+
" - label: Generate plan\n"
624+
" agent: speckit.plan\n"
625+
" send: true\n"
626+
"---\n"
627+
+ original_text.split("---", 2)[-1]
628+
if "---" in original_text
629+
else original_text
630+
)
631+
632+
patched_dir = tmp / "patched_templates4"
633+
patched_dir.mkdir()
634+
(patched_dir / src_template.name).write_text(patched)
635+
636+
integration = get_integration("claude")
637+
if integration is None:
638+
pytest.skip("claude integration not registered")
639+
640+
m = IntegrationManifest("claude", tmp)
641+
with patch.object(type(integration), "list_command_templates", return_value=list(patched_dir.glob("*.md"))):
642+
created = integration.setup(tmp, m)
643+
644+
skill_files = [f for f in created if f.name == "SKILL.md"]
645+
assert skill_files, "no SKILL.md files created"
646+
for sf in skill_files:
647+
parsed = _parse_frontmatter(sf.read_text())
648+
handoffs = parsed.get("handoffs")
649+
assert isinstance(handoffs, list), parsed
650+
assert handoffs[0].get("agent") == "speckit.plan", parsed
651+
601652

602653
# ---------------------------------------------------------------------------
603654
# Issue 4 — _behavior_overridable clobbers explicit source frontmatter

0 commit comments

Comments
 (0)