Repository navigation
fix(workflows): quote squad-cast skill description so its frontmatter parses - #1742
Merged
Merged
Conversation
… parses The `squad-cast` inline skill carried an unquoted description containing ": ", which YAML reads as a nested mapping inside a compact mapping. The other 20 skill blocks parse cleanly; this one does not. Nothing upstream catches it: * `gh aw compile --strict` succeeds -- at compile time a `## skill:` block is still ordinary markdown, not YAML. * gh-aw's extractor (setup/js/extract_inline_skills.cjs, pinned v0.86.2) filters skill frontmatter LINE BY LINE and never parses it, so the bad value is written to .github/skills/squad-cast/SKILL.md verbatim, with no warning on any layer. * The extraction suite modelled byte conservation but not frontmatter validity, so it stayed green. Verified by running the real extractor over the assembled prompt and checking all 21 emitted frontmatter blocks: squad-cast is the only failure. Every other SKILL.md in this repo whose description contains a colon quotes the value, so this is a deviation from existing convention rather than a style choice. Add a class-level assertion so any skill frontmatter that would fail to parse is caught here instead of at agent load time. Hand-rolled rather than pulling in a YAML parser: this repo has no YAML library in its dependency tree (frontmatter elsewhere is checked by actionlint, which only reads real workflow YAML), and a lint this narrow does not justify adding one. Confirmed the assertion fails against the pre-fix file and passes after. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 090e9874-9835-489a-b38d-05a450b3fc53
Contributor
🛫 PR Readiness Check
PR Scope: 🔧 Infrastructure
|
| Status | Check | Details |
|---|---|---|
| ❌ | Single commit | 2 commits — consider squashing before review |
| ✅ | Not in draft | Ready for review |
| ✅ | Branch up to date | Up to date with dev |
| ❌ | Copilot review | No Copilot review yet — it may still be processing |
| ✅ | Changeset present | No source files changed — changeset not required |
| ✅ | Scope clean | No .squad/ or docs/proposals/ files |
| ✅ | No merge conflicts | No merge conflicts |
| ✅ | Copilot threads resolved | 0 active Copilot thread(s) resolved (1 outdated skipped) |
| ✅ | CI passing | All checks passing |
Files Changed (2 files, +115 −5)
| File | +/− |
|---|---|
test/gh-aw-quality.test.ts |
+114 −4 |
workflows/squad.md |
+1 −1 |
Total: +115 −5
This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.
Contributor
🟡 Impact Analysis — PR #1742Risk tier: 🟡 MEDIUM 📊 Summary
🎯 Risk Factors
📦 Modules Affectedroot (1 file)
tests (1 file)
This report is generated automatically for every PR. See #733 for details. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes an invalid YAML frontmatter value in the squad-cast inline skill (caused by an unquoted : in the description) and adds a test assertion to prevent similar frontmatter hazards from silently shipping through the gh-aw inline-skill extraction pipeline.
Changes:
- Quote the
squad-castskilldescriptioninworkflows/squad.mdso the frontmatter is valid YAML. - Extend the inline-skill extraction test harness to retain each skill’s body text, enabling deeper validation.
- Add a new test that checks skill frontmatter for plain-scalar hazards (e.g.,
:and#) that can break YAML parsing/meaning.
Show a summary per file
| File | Description |
|---|---|
| workflows/squad.md | Quotes the squad-cast description frontmatter value to avoid YAML parsing ambiguity/breakage. |
| test/gh-aw-quality.test.ts | Adds a frontmatter hazard check and plumbs extracted skill bodies into the test harness. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
…rrors
findFrontmatterScalarError() treated a leading `[` or `{` as a hazard, but
those open YAML flow collections that are valid and parseable unquoted --
`allowed: [read, write]` and `options: {mode: fast}` are not errors. The
latter was doubly wrong: it tripped the ": " nested-mapping rule first.
Flow collections are now checked ahead of the ": " rule and flagged only when
left unterminated, which genuinely does not parse. Added a regression test
covering both directions: valid scalars and flow collections stay clean, while
": ", " #", a leading anchor, and an unclosed flow sequence are still caught.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 090e9874-9835-489a-b38d-05a450b3fc53
This was referenced Aug 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #1739.
The defect
workflows/squad.md— thesquad-castinline skill's frontmatter is not valid YAML:The unquoted
:parses as a nested mapping inside a compact mapping. It is 1 of 21 skill blocks; the other 20 are fine.Why every existing gate missed it
I traced each layer rather than assuming:
gh aw compile --strict## skill:block is still ordinary markdown, not YAMLextract_inline_skills.cjsfilters frontmatter line by line and never parses ittest/gh-aw-quality.test.tsSo the malformed value is written to
.github/skills/squad-cast/SKILL.mdverbatim, with no signal anywhere in the chain. Verified by running gh-aw's actual extractor (pinned SHA6aab9e5, v0.86.2) over the assembled prompt and checking all 21 emitted frontmatter blocks —squad-castis the only one that fails.Severity, stated honestly
I proved the YAML is invalid. I did not prove Copilot's skill loader rejects it — that runtime isn't reachable from here. So this is "defect with unknown blast radius," not "cast is definitely down."
What raises the stakes:
castis the onboarding entry point, and #1739 also added "If the parsed mode's skill cannot be loaded, report the failure in plain language and stop. Never improvise a mode playbook from memory." — a load failure is a hard stop by design, with no fallback.This is a deviation, not a style choice
Every other
SKILL.mdin the repo whosedescriptioncontains a colon quotes the value —ci-validation-gates,git-workflow,coordinator-source-of-truth.squad-castis the lone exception.Changes
1. Quote the description (
workflows/squad.md) — one line.2. Class-level assertion (
test/gh-aw-quality.test.ts) —gives every skill parseable YAML frontmatter, covering the plain-scalar hazards that bite in a one-line description: unquoted:, unquoted#(silently truncates as a comment), and leading YAML indicators. Quoted values and block scalars pass through.Hand-rolled rather than importing a YAML parser: this repo has no YAML library in its dependency tree (frontmatter elsewhere is validated by actionlint, which only reads real workflow YAML). Adding a dependency for a lint this narrow isn't worth it, and relying on a hoisted transitive
yamlwould break mysteriously the day it stops being hoisted.Validation
devbyte-for-byte (git diffempty), ran the new assertion → failed, naming the skill and reason:test/gh-aw-quality.test.ts: 80 passed, 13 skipped (was 79/13 — the +1 is this assertion). No regressions.eslint test/gh-aw-quality.test.tsclean.packages/*/src/changes, so no changeset required.Deliberately not included
While investigating I hit a false positive worth recording: my first extractor run reported "no frontmatter" for all 21 skills, because this worktree materializes
squad.mdas CRLF and the extractor guards oncontent.startsWith("---\n").That is local only —
git ls-files --eolconfirmsi/lf w/crlf, so the index is LF and CI sees LF..gitattributespins LF for*.yml,*.sh, andDockerfile*but not*.md.Pinning
*.md text eol=lfwould be reasonable hygiene, but it renormalizes 61 files (53 CRLF + 8 mixed in the index) and would bury a one-line fix under line-ending churn. Left for a separate PR.