fix: drop duplicate skill inventory and handle YAML block scalars in copilot executor (#578)

[Shayne Boyer (spboyer)]

Closes #578.

Problem

When waza run uses the copilot-sdk executor, the request to the model contained a duplicate <available_skills> block authored by waza itself, in addition to the skill catalog the Copilot SDK already advertises via SkillDirectories. Worse, waza's per-skill descriptions were parsed by a line-based scanner that read only the tokens after description: on the same line, so a skill authored with a YAML block-scalar description (>-, >, |, |-) — as waza new skill scaffolds by default — surfaced the literal indicator string (e.g. >-) as the description.

Fix

Two surgical changes in internal/execution/copilot.go:

1. Drop the synthetic <available_skills> inventory. buildSkillSystemMessage now emits only the <skill_context> block containing the target skill's full SKILL.md, and only when injectSkillBody is true. This preserves the primary-skill injection behavior while eliminating duplication with the SDK's own advertisement.
2. Rewrite parseSkillFrontmatter to use yaml.Unmarshal (matching how internal/skill parses frontmatter) so block-scalar name/description fields round-trip correctly. This keeps target-skill name matching robust for future callers even though the message body no longer serializes descriptions itself.

Tradeoff vs. removing only the synthetic block

Dropping the block alone would have solved the user-visible issue (no duplicate inventory, no corrupted descriptions in the prompt). We additionally hardened parseSkillFrontmatter because it's still on the call path for matching the target skill's name — so an author who writes name: >-\n my-skill still resolves correctly. The parser is now correct for any future consumer that reintroduces frontmatter-driven logic.

Non-Copilot executors (mock and command-line copilot-cli) are unaffected — the entire code path lives in CopilotEngine.

Regression coverage

internal/execution/skill_injection_test.go:

• All existing TestBuildSkillSystemMessage_* cases flipped to assert <available_skills> is absent.
• New TestBuildSkillSystemMessage_BlockScalarSkillIsMatchable — a skill authored with description: >- is still selectable as the target and its body still injects into <skill_context>.
• TestParseSkillFrontmatter extended with block-scalar cases: >-, >, |, |-, plus a block-scalar name: field.

Verification

go test -run 'TestBuildSkillSystemMessage|TestParseSkillFrontmatter|TestBuildInstructionSystemMessage' ./internal/execution/ -v   # all pass
go test ./internal/execution/...                                                                                                 # ok
go vet ./internal/execution/...                                                                                                  # clean
gofmt -l internal/execution/copilot.go internal/execution/skill_injection_test.go                                                # clean
golangci-lint run ./internal/execution/...                                                                                       # 0 issues

Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com

[Copilot]

Copilot review overview

🟢 Approval recommended

The changes are narrowly scoped to the Copilot SDK executor path and are backed by updated and newly added regression tests covering the reported failure modes.

Review tier: Lite
Findings: None

What changed in this PR

This pull request fixes an issue in the Copilot SDK executor where waza redundantly injected its own <available_skills> inventory (duplicating what the Copilot SDK already provides) and also mis-parsed YAML block-scalar frontmatter values in SKILL.md.

Changes:

• Removed waza-authored <available_skills> emission so only the target skill’s <skill_context> is injected (when enabled), eliminating duplicated/malformed skill listings.
• Reimplemented parseSkillFrontmatter using yaml.Unmarshal so YAML block scalars (>-, >, |, |-) are parsed correctly.
• Updated/expanded tests to assert the inventory is absent and to cover block-scalar parsing and target-skill matchability.

File	Description
internal/​execution/​copilot.go	Stops emitting a synthetic <available_skills> block and switches frontmatter parsing to yaml.Unmarshal to correctly handle YAML block scalars.
internal/​execution/​skill_injection_test.go	Updates assertions to ensure <available_skills> is absent and adds regression coverage for block-scalar frontmatter.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.