diff --git a/.github/workflows/skill-docs.yml b/.github/workflows/skill-docs.yml index 700a8222a..ba4c6d0ab 100644 --- a/.github/workflows/skill-docs.yml +++ b/.github/workflows/skill-docs.yml @@ -31,3 +31,5 @@ jobs: echo "Generated Factory SKILL.md files are stale. Run: bun run gen:skill-docs --host factory" exit 1 } + - name: Validate generated skill frontmatter + run: bun run skill:frontmatter diff --git a/package.json b/package.json index 5be9c32bc..1c7a42e79 100644 --- a/package.json +++ b/package.json @@ -31,6 +31,7 @@ "test:gemini": "EVALS=1 bun test test/gemini-e2e.test.ts", "test:gemini:all": "EVALS=1 EVALS_ALL=1 bun test test/gemini-e2e.test.ts", "skill:check": "bun run scripts/skill-check.ts", + "skill:frontmatter": "bun run scripts/skill-frontmatter-check.ts", "dev:skill": "bun run scripts/dev-skill.ts", "start": "bun run browse/src/server.ts", "eval:list": "bun run scripts/eval-list.ts", diff --git a/scripts/skill-check.ts b/scripts/skill-check.ts index 9182737ee..878869531 100644 --- a/scripts/skill-check.ts +++ b/scripts/skill-check.ts @@ -8,7 +8,7 @@ * - Freshness check (generated files match committed files) */ -import { validateSkill } from '../test/helpers/skill-parser'; +import { validateSkill, validateSkillFrontmatter } from '../test/helpers/skill-parser'; import { discoverTemplates, discoverSkillFiles } from './discover-skills'; import * as fs from 'fs'; import * as path from 'path'; @@ -35,6 +35,16 @@ let hasErrors = false; console.log(' Skills:'); for (const file of SKILL_FILES) { const fullPath = path.join(ROOT, file); + const frontmatterErrors = validateSkillFrontmatter(fullPath); + if (frontmatterErrors.length > 0) { + hasErrors = true; + console.log(` ❌ ${file.padEnd(30)} — invalid frontmatter`); + for (const err of frontmatterErrors) { + console.log(` line ${err.line}: ${err.message}`); + } + continue; + } + const result = validateSkill(fullPath); if (result.warnings.length > 0) { diff --git a/scripts/skill-frontmatter-check.ts b/scripts/skill-frontmatter-check.ts new file mode 100644 index 000000000..b82065cd5 --- /dev/null +++ b/scripts/skill-frontmatter-check.ts @@ -0,0 +1,54 @@ +#!/usr/bin/env bun +/** + * Validate YAML frontmatter for generated SKILL.md files. + * + * This is intentionally narrower than skill:check so CI can run it after + * regenerating only the host outputs relevant to a workflow. + */ + +import { discoverSkillFiles } from './discover-skills'; +import { validateSkillFrontmatter } from '../test/helpers/skill-parser'; +import * as fs from 'fs'; +import * as path from 'path'; + +const ROOT = path.resolve(import.meta.dir, '..'); + +function walkSkillFiles(dir: string, acc: string[]): void { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + if (entry.name === '.git' || entry.name === 'node_modules') continue; + const fullPath = path.join(dir, entry.name); + if (entry.isDirectory()) { + walkSkillFiles(fullPath, acc); + continue; + } + if (entry.name === 'SKILL.md') { + acc.push(path.relative(ROOT, fullPath)); + } + } +} + +const files = new Set(discoverSkillFiles(ROOT)); +for (const hostDir of ['.agents', '.factory']) { + const fullPath = path.join(ROOT, hostDir); + if (fs.existsSync(fullPath)) { + const hostFiles: string[] = []; + walkSkillFiles(fullPath, hostFiles); + for (const file of hostFiles) files.add(file); + } +} + +const errors: string[] = []; +for (const file of [...files].sort()) { + const fullPath = path.join(ROOT, file); + for (const error of validateSkillFrontmatter(fullPath)) { + errors.push(`${file}:${error.line}: ${error.message}`); + } +} + +if (errors.length > 0) { + console.error('Invalid SKILL.md frontmatter:'); + for (const error of errors) console.error(` ${error}`); + process.exit(1); +} + +console.log(`Validated frontmatter for ${files.size} SKILL.md files.`); diff --git a/test/helpers/skill-parser.ts b/test/helpers/skill-parser.ts index 0e3271ba1..d4152a6b6 100644 --- a/test/helpers/skill-parser.ts +++ b/test/helpers/skill-parser.ts @@ -34,6 +34,11 @@ export interface ValidationResult { warnings: string[]; } +export interface FrontmatterError { + line: number; + message: string; +} + /** * Extract all $B invocations from bash code blocks in a SKILL.md file. */ @@ -138,6 +143,50 @@ export function validateSkill(skillPath: string): ValidationResult { return result; } +/** + * Lightweight frontmatter validation for the YAML subset used by SKILL.md. + * + * This intentionally does not try to be a complete YAML parser. It catches the + * class of errors that make agents skip skills: missing delimiters and plain + * scalar values that contain ": " without being quoted or written as a block. + */ +export function validateSkillFrontmatter(skillPath: string): FrontmatterError[] { + const content = fs.readFileSync(skillPath, 'utf-8'); + const errors: FrontmatterError[] = []; + + if (!content.startsWith('---\n')) { + return [{ line: 1, message: 'missing opening frontmatter delimiter' }]; + } + + const fmEnd = content.indexOf('\n---', 4); + if (fmEnd === -1) { + return [{ line: 1, message: 'missing closing frontmatter delimiter' }]; + } + + const frontmatter = content.slice(4, fmEnd); + const lines = frontmatter.split('\n'); + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + const match = line.match(/^\s*[A-Za-z0-9_-]+:\s+(.+?)\s*$/); + if (!match) continue; + + const value = match[1].trim(); + const isQuoted = value.startsWith('"') || value.startsWith("'"); + const isBlockScalar = value === '|' || value === '>' || value.startsWith('|') || value.startsWith('>'); + const isFlowValue = value.startsWith('[') || value.startsWith('{'); + if (isQuoted || isBlockScalar || isFlowValue) continue; + + if (/:\s/.test(value)) { + errors.push({ + line: i + 2, + message: 'plain YAML scalar contains ": "; quote it or use a block scalar', + }); + } + } + + return errors; +} + /** * Extract all REMOTE_SLUG=$(...) assignment patterns from .md files in given subdirectories. * Returns a Map from filename → array of full assignment lines found. diff --git a/test/skill-validation.test.ts b/test/skill-validation.test.ts index df5cb7994..82b8385ce 100644 --- a/test/skill-validation.test.ts +++ b/test/skill-validation.test.ts @@ -1,5 +1,5 @@ import { describe, test, expect } from 'bun:test'; -import { validateSkill, extractRemoteSlugPatterns, extractWeightsFromTable } from './helpers/skill-parser'; +import { validateSkill, validateSkillFrontmatter, extractRemoteSlugPatterns, extractWeightsFromTable } from './helpers/skill-parser'; import { ALL_COMMANDS, COMMAND_DESCRIPTIONS, READ_COMMANDS, WRITE_COMMANDS, META_COMMANDS } from '../browse/src/commands'; import { SNAPSHOT_FLAGS } from '../browse/src/snapshot'; import * as fs from 'fs'; @@ -24,6 +24,30 @@ function readShipUnion(): string { } describe('SKILL.md command validation', () => { + test('all SKILL.md files have loadable YAML frontmatter', () => { + const skillFiles: string[] = []; + const walk = (dir: string) => { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + if (entry.name === 'node_modules' || entry.name === '.git') continue; + const fullPath = path.join(dir, entry.name); + if (entry.isDirectory()) { + walk(fullPath); + } else if (entry.name === 'SKILL.md') { + skillFiles.push(fullPath); + } + } + }; + walk(ROOT); + + const errors = skillFiles.flatMap((skillPath) => + validateSkillFrontmatter(skillPath).map((error) => + `${path.relative(ROOT, skillPath)}:${error.line}: ${error.message}` + ) + ); + + expect(errors).toEqual([]); + }); + test('all $B commands in SKILL.md are valid browse commands', () => { const result = validateSkill(path.join(ROOT, 'SKILL.md')); expect(result.invalid).toHaveLength(0);