From bbea1a5a7f3eea51fe2d0198d16a4441d40cf9f2 Mon Sep 17 00:00:00 2001 From: brady gaster Date: Mon, 17 Aug 2026 21:58:13 -0700 Subject: [PATCH 1/2] fix(workflows): quote squad-cast skill description so its frontmatter 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 --- test/gh-aw-quality.test.ts | 79 ++++++++++++++++++++++++++++++++++++-- workflows/squad.md | 2 +- 2 files changed, 76 insertions(+), 5 deletions(-) diff --git a/test/gh-aw-quality.test.ts b/test/gh-aw-quality.test.ts index 39a12361e..d6953f769 100644 --- a/test/gh-aw-quality.test.ts +++ b/test/gh-aw-quality.test.ts @@ -658,11 +658,60 @@ const SKILL_START_RE = /^##[ \t]+skill:[ \t]+`([a-z][a-z0-9_-]*)`[ \t]*$/gm; interface ExtractionResult { ambient: string; - skills: { name: string; bytes: number }[]; + skills: { name: string; bytes: number; body: string }[]; discardedBytes: number; markerBytes: number; } +/** + * Report why a skill's frontmatter would fail to parse as YAML, or null if it is fine. + * + * gh-aw's extractor (setup/js/extract_inline_skills.cjs) filters skill frontmatter + * LINE BY LINE and never parses it, so a malformed `description:` is written to + * .github/skills//SKILL.md verbatim and surfaces only when the agent tries to + * load that skill. `gh aw compile --strict` does not catch it either: at compile time + * a skill block is still ordinary markdown. This check closes that gap. + * + * Deliberately hand-rolled rather than delegating to a YAML library: this repo has no + * YAML parser in its dependency tree (frontmatter elsewhere is validated by actionlint, + * which only reads real workflow YAML), and a lint this narrow does not justify adding + * one. It targets the plain-scalar hazards that actually bite in a one-line description. + */ +function findFrontmatterScalarError(body: string): { key: string; value: string; reason: string } | null { + if (!body.startsWith('\n---\n') && !body.startsWith('---\n')) return null; + const opened = body.slice(body.indexOf('---\n') + 4); + const closeIdx = opened.indexOf('\n---'); + if (closeIdx === -1) return null; + + for (const line of opened.slice(0, closeIdx).split('\n')) { + const m = /^([A-Za-z_][A-Za-z0-9_-]*):[ \t]+(.*)$/.exec(line); + if (!m) continue; + const [, key, raw] = m; + const value = raw.trim(); + if (!value) continue; + + // A quoted scalar escapes every hazard below. + const quoted = + (value.startsWith('"') && value.endsWith('"') && value.length > 1) || + (value.startsWith("'") && value.endsWith("'") && value.length > 1); + if (quoted) continue; + + // Block scalars (`|`, `>`) carry their value on following lines. + if (/^[|>][-+0-9]*$/.test(value)) continue; + + if (/:\s/.test(value)) { + return { key, value, reason: 'unquoted value contains ": " and parses as a nested mapping' }; + } + if (/\s#/.test(value)) { + return { key, value, reason: 'unquoted value contains " #" and would be truncated as a comment' }; + } + if (/^[[{&*!%@`]/.test(value)) { + return { key, value, reason: `unquoted value starts with the YAML indicator "${value[0]}"` }; + } + } + return null; +} + /** Mirror of gh-aw's inline-skill extraction, including its discard behaviour. */ function extractInlineSkills(content: string): ExtractionResult { const starts = [...content.matchAll(SKILL_START_RE)]; @@ -678,7 +727,7 @@ function extractInlineSkills(content: string): ExtractionResult { let cursor = 0; let discardedBytes = 0; let markerBytes = 0; - const skills: { name: string; bytes: number }[] = []; + const skills: { name: string; bytes: number; body: string }[] = []; for (const start of starts) { const startIdx = start.index!; @@ -695,7 +744,11 @@ function extractInlineSkills(content: string): ExtractionResult { if (endMarker) { const bodyEnd = bodyStart + endMarker.index; - skills.push({ name: start[1], bytes: Buffer.byteLength(content.slice(bodyStart, bodyEnd), 'utf8') }); + skills.push({ + name: start[1], + bytes: Buffer.byteLength(content.slice(bodyStart, bodyEnd), 'utf8'), + body: content.slice(bodyStart, bodyEnd), + }); markerBytes += Buffer.byteLength(endMarker[0], 'utf8'); cursor = bodyEnd + endMarker[0].length; continue; @@ -704,7 +757,11 @@ function extractInlineSkills(content: string): ExtractionResult { // Implicit close: next H2 after the start marker, else EOF. const nextH2 = h2Positions.find(p => p > startIdx); const bodyEnd = nextH2 ?? content.length; - skills.push({ name: start[1], bytes: Buffer.byteLength(content.slice(bodyStart, bodyEnd), 'utf8') }); + skills.push({ + name: start[1], + bytes: Buffer.byteLength(content.slice(bodyStart, bodyEnd), 'utf8'), + body: content.slice(bodyStart, bodyEnd), + }); // Everything from here to the next start marker is dropped on the floor. const nextStart = starts.find(s => s.index! > startIdx)?.index ?? content.length; @@ -789,6 +846,20 @@ describe('gh-aw: inline skill extraction', () => { ).toBeLessThan(AMBIENT_BUDGET_BYTES); }); + it('gives every skill parseable YAML frontmatter', () => { + const offenders = result.skills + .map(s => ({ name: s.name, err: findFrontmatterScalarError(s.body) })) + .filter((s): s is { name: string; err: NonNullable> } => s.err !== null); + + expect( + offenders.map(o => `${o.name}: ${o.err.key} — ${o.err.reason}\n ${o.err.value}`).join('\n '), + 'Skill frontmatter must be valid YAML. Nothing upstream catches this: the extractor ' + + 'filters frontmatter line-by-line without parsing it, and `gh aw compile --strict` ' + + 'sees skill blocks as plain markdown. A broken value reaches the agent as a skill ' + + 'that cannot load. Quote the value.' + ).toBe(''); + }); + it('gives every dispatchable mode a skill to load', () => { const skillNames = new Set(result.skills.map(s => s.name)); const modes = extractModeTable(readText(SQUAD_WORKFLOW)); diff --git a/workflows/squad.md b/workflows/squad.md index b2c5453bd..7674516ac 100644 --- a/workflows/squad.md +++ b/workflows/squad.md @@ -295,7 +295,7 @@ gh pr list --state open --json number,url,headRefName --jq '[.[] | select(.headR ## skill: `squad-cast` --- -description: Cast a Squad team: analyze the repo, compose agents, assign character names, scaffold .squad/, open the Cast PR. +description: "Cast a Squad team: analyze the repo, compose agents, assign character names, scaffold .squad/, open the Cast PR." --- Analyze repo, compose team, assign character names from a fictional universe, generate `.squad/` scaffolding, open PR. From ae332ce904ffc5948908e3bd59f78e6c6ce48d18 Mon Sep 17 00:00:00 2001 From: brady gaster Date: Tue, 18 Aug 2026 08:54:22 -0700 Subject: [PATCH 2/2] fix(test): stop flagging valid YAML flow collections as frontmatter errors 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 --- test/gh-aw-quality.test.ts | 41 +++++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/test/gh-aw-quality.test.ts b/test/gh-aw-quality.test.ts index d6953f769..396a8f3a9 100644 --- a/test/gh-aw-quality.test.ts +++ b/test/gh-aw-quality.test.ts @@ -699,13 +699,26 @@ function findFrontmatterScalarError(body: string): { key: string; value: string; // Block scalars (`|`, `>`) carry their value on following lines. if (/^[|>][-+0-9]*$/.test(value)) continue; + // `[` and `{` open YAML flow collections, which are valid unquoted and parse + // fine -- `[a, b]` and `{a: b}` are not errors. Only an unterminated one is. + // Checked before the ": " rule below so a flow mapping is not mistaken for a + // nested mapping. + if (/^[[{]/.test(value)) { + const opens = (value.match(/[[{]/g) ?? []).length; + const closes = (value.match(/[\]}]/g) ?? []).length; + if (opens !== closes) { + return { key, value, reason: 'unquoted value opens a YAML flow collection that is never closed' }; + } + continue; + } + if (/:\s/.test(value)) { return { key, value, reason: 'unquoted value contains ": " and parses as a nested mapping' }; } if (/\s#/.test(value)) { return { key, value, reason: 'unquoted value contains " #" and would be truncated as a comment' }; } - if (/^[[{&*!%@`]/.test(value)) { + if (/^[&*!%@`]/.test(value)) { return { key, value, reason: `unquoted value starts with the YAML indicator "${value[0]}"` }; } } @@ -860,6 +873,32 @@ describe('gh-aw: inline skill extraction', () => { ).toBe(''); }); + it('flags real frontmatter scalar hazards without flagging valid YAML', () => { + const wrap = (line: string) => `\n---\n${line}\nname: x\n---\nbody`; + + // Valid unquoted plain scalars and flow collections must not be flagged. + for (const ok of [ + 'description: Cast a squad', + 'description: "Cast a squad: from a repo"', + "description: 'already quoted'", + 'allowed: [read, write]', + 'options: {mode: fast}', + 'description: |', + ]) { + expect(findFrontmatterScalarError(wrap(ok)), `false positive on: ${ok}`).toBeNull(); + } + + // Genuine hazards must still be caught. + for (const bad of [ + 'description: Cast a squad: from a repo', + 'description: Cast a squad #1', + 'description: &anchor', + 'allowed: [read, write', + ]) { + expect(findFrontmatterScalarError(wrap(bad)), `missed hazard in: ${bad}`).not.toBeNull(); + } + }); + it('gives every dispatchable mode a skill to load', () => { const skillNames = new Set(result.skills.map(s => s.name)); const modes = extractModeTable(readText(SQUAD_WORKFLOW));