From d3feac15ad67a7a752937a33e0d3d37502ee35ed Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sat, 30 May 2026 10:38:01 -0700 Subject: [PATCH] fix(gen-skill-docs): quote frontmatter descriptions with interior colons (#1778) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generated SKILL.md frontmatter emitted the catalog-trimmed description: as a plain YAML scalar. A description with an interior ": " (e.g. "Ship workflow: detect...") parses as a nested mapping under strict YAML loaders, so Codex/OpenAI skill loading rejected those skills. applyCatalogTrim now routes the value through toYamlInlineScalar, which quotes (via JSON.stringify) only when a plain scalar would be invalid — interior ": ", inline " #", leading indicator char, or surrounding whitespace. Strings that are already valid plain scalars pass through unchanged to keep regen diffs small. The frontmatter test now parses every generated block (Claude + Codex hosts) with Bun.YAML.parse instead of string-checking that name:/description: substrings exist, so the regression can't reappear. Runs under `bun test` (already in CI). Co-Authored-By: Claude Opus 4.8 (1M context) --- scripts/gen-skill-docs.ts | 28 +++++++++++++++++++++++++++- test/gen-skill-docs.test.ts | 35 +++++++++++++++++++++++++++++++---- 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/scripts/gen-skill-docs.ts b/scripts/gen-skill-docs.ts index d030e79ad..fb72e4bd9 100644 --- a/scripts/gen-skill-docs.ts +++ b/scripts/gen-skill-docs.ts @@ -356,6 +356,28 @@ export function buildWhenToInvokeSection(parts: CatalogParts): string { return lines.join('\n'); } +/** + * Render a string as a YAML inline scalar value (the text after `key: `), + * quoting only when a plain scalar would be invalid or ambiguous. + * + * The bug this guards (#1778): a description like "Ship workflow: detect..." + * emitted as a plain scalar has an interior ": " that a strict YAML parser + * (Codex/OpenAI skill loading) reads as a nested mapping and rejects with + * "mapping values are not allowed in this context". When quoting is needed we + * fall back to JSON.stringify, which produces a double-quoted scalar that YAML + * accepts verbatim (YAML is a superset of JSON for flow scalars). Strings that + * are already valid plain scalars pass through unchanged to keep regen diffs small. + */ +export function toYamlInlineScalar(s: string): string { + const needsQuote = + s.length === 0 || + s !== s.trim() || // leading/trailing whitespace + /:(\s|$)/.test(s) || // "foo: bar" / trailing colon → mapping ambiguity + /\s#/.test(s) || // " #" → inline comment + /^[\s>|&*!%@`"'#,\[\]{}?-]/.test(s); // leading YAML indicator char + return needsQuote ? JSON.stringify(s) : s; +} + /** * Apply catalog trim to a SKILL.md body: * - shorten frontmatter `description:` to lead + (gstack) @@ -397,8 +419,12 @@ export function applyCatalogTrim(content: string, skillName: string): { content: // Replace description in frontmatter — keep trailing newline so the next // YAML field doesn't collide on the same line as the description value. + // Quote the value when it would be an invalid YAML plain scalar (the common + // case: an interior ": " like "Ship workflow: detect..." which a strict YAML + // parser reads as a nested mapping and rejects — #1778). toYamlInlineScalar + // only quotes when needed, so descriptions without special chars stay plain. const newDesc = buildTrimmedDescription(parts); - const newFrontmatter = frontmatter.replace(descMatch[0], `description: ${newDesc}\n`); + const newFrontmatter = frontmatter.replace(descMatch[0], `description: ${toYamlInlineScalar(newDesc)}\n`); let newContent = '---\n' + newFrontmatter + content.slice(fmEnd); // Insert body section after frontmatter (after the closing ---\n and any diff --git a/test/gen-skill-docs.test.ts b/test/gen-skill-docs.test.ts index a405c2da9..49b5dadac 100644 --- a/test/gen-skill-docs.test.ts +++ b/test/gen-skill-docs.test.ts @@ -155,12 +155,39 @@ describe('gen-skill-docs', () => { } }); - test('every generated SKILL.md has valid YAML frontmatter', () => { + // #1778: strict YAML parsers (Codex/OpenAI skill loading) reject frontmatter + // whose plain `description:` scalar contains an interior ": " (read as a nested + // mapping). Parse EVERY generated frontmatter block with a strict YAML parser, + // not just string-check that name:/description: exist. + function frontmatterBlock(content: string): string { + expect(content.startsWith('---\n')).toBe(true); + const end = content.indexOf('\n---', 4); + expect(end).toBeGreaterThan(0); + return content.slice(4, end); + } + + test('every generated SKILL.md frontmatter parses as strict YAML', () => { for (const skill of CLAUDE_GENERATED_SKILLS) { const content = fs.readFileSync(path.join(ROOT, skill.dir, 'SKILL.md'), 'utf-8'); - expect(content.startsWith('---\n')).toBe(true); - expect(content).toContain('name:'); - expect(content).toContain('description:'); + const fm = frontmatterBlock(content); + let parsed: any; + expect(() => { parsed = Bun.YAML.parse(fm); }, + `frontmatter for ${skill.dir} must be valid YAML`).not.toThrow(); + expect(typeof parsed?.name).toBe('string'); + expect(typeof parsed?.description).toBe('string'); + } + }); + + test('every generated Codex (.agents/skills) frontmatter parses as strict YAML', () => { + const agentsDir = path.join(ROOT, '.agents', 'skills'); + if (!fs.existsSync(agentsDir)) return; // skip if external hosts not generated + for (const entry of fs.readdirSync(agentsDir, { withFileTypes: true })) { + if (!entry.isDirectory()) continue; + const mdPath = path.join(agentsDir, entry.name, 'SKILL.md'); + if (!fs.existsSync(mdPath)) continue; + const fm = frontmatterBlock(fs.readFileSync(mdPath, 'utf-8')); + expect(() => Bun.YAML.parse(fm), + `Codex frontmatter for ${entry.name} must be valid YAML`).not.toThrow(); } });