From 6fa76fa090baab4f3c877c84cb00dc923336c1e7 Mon Sep 17 00:00:00 2001 From: Abhishek Sharma Date: Mon, 21 Sep 2026 11:17:42 -0700 Subject: [PATCH 1/2] fix(validator): reject frontmatter that is not valid YAML MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `parseFrontmatter` splits each line on its first colon. That is enough to read `name` and `description`, and it is forgiving by design — but it means a SKILL.md whose frontmatter is not valid YAML passes every check in the linter. The hosts that consume these skills do not share that forgiveness. Per Cursor staff in #494, attaching a skill to a message parses its frontmatter as YAML, and a parse failure fails the request with a generic server error and takes the rest of the chat's context with it. So the repo could ship frontmatter that breaks a host while CI stayed green. Three shapes pass the current linter and are rejected by a strict parser (confirmed against ruby's psych, each isolated by mutating only the frontmatter of a real SKILL.md that otherwise lints clean): description: Use this: when X colon-space: YAML reads a nested mapping \tkey: value tab indentation is invalid YAML whitespace description: "unterminated an unclosed quote All three now fail with the line number and the fix. Deliberate boundaries: a colon with no trailing space is untouched, so `https://example.com/a:b` still passes, and a duplicate key is NOT reported because `safe_load` accepts one — the rule tracks the parser, not taste. This was verified once by hand against all 25 skills in the #494 thread. This makes it a check instead of a memory. Narrow on purpose: the repo has no package.json and therefore no YAML parser to depend on, so this is a small set of rules aimed at the classes actually observed to break a host, not a YAML implementation. No current frontmatter uses block scalars or multi-line values and the check says so. All 25 skills still pass with 0 errors, every validator exits 0, and the six script suites are green (35 pass in skill-lint-test). Mutation-tested 4/4: removing the call, widening the colon rule to any colon, and dropping either the tab or the unterminated-quote rule each fail the new tests. Refs #494 --- scripts/lib/skill-lint-test.js | 88 ++++++++++++++++++++++++++++++++++ scripts/lib/skill-lint.js | 84 ++++++++++++++++++++++++++++++++ 2 files changed, 172 insertions(+) diff --git a/scripts/lib/skill-lint-test.js b/scripts/lib/skill-lint-test.js index a9a1f253..d2ebf936 100644 --- a/scripts/lib/skill-lint-test.js +++ b/scripts/lib/skill-lint-test.js @@ -249,3 +249,91 @@ test('CRLF line endings are handled', () => { const result = lintSkillContent('fenced', content, FENCE_KNOWN); assert.equal(overviewMissing(result), true); }); + +// ── Frontmatter must be valid YAML, not merely splittable ──────────────────── +// `parseFrontmatter` splits each line on its first colon, which is forgiving by +// design. The hosts that read these skills are not: Cursor parses the +// frontmatter as YAML when a skill is attached to a message, and a parse +// failure fails the whole request and takes the chat's context with it (#494). +// Each shape below was confirmed rejected by a strict parser (ruby psych) while +// passing every other check in this linter. + +/** Frontmatter that is otherwise complete, so only YAML validity varies. */ +function fmLines(...lines) { + return withAllSections(['---', 'name: alpha', ...lines, '---'].join('\n')); +} + +const yamlErrors = result => result.errors.filter(e => e.startsWith('Frontmatter line ')); + +test('a valid frontmatter reports no YAML error', () => { + const result = lintSkillContent('alpha', fmLines('description: Use when you need alpha'), KNOWN); + assert.deepEqual(yamlErrors(result), []); +}); + +test('an unquoted value containing a colon is rejected', () => { + // YAML reads `Use when: X` as a nested mapping and errors; the split-on-first + // -colon parser reads it as a plain string and never notices. + const result = lintSkillContent( + 'alpha', + fmLines('description: Use when you need alpha: auth, secrets and review'), + KNOWN, + ); + assert.equal(yamlErrors(result).length, 1); + assert.match(yamlErrors(result)[0], /unquoted value containing/); +}); + +test('quoting the same value makes it valid again', () => { + const result = lintSkillContent( + 'alpha', + fmLines('description: "Use when you need alpha: auth, secrets and review"'), + KNOWN, + ); + assert.deepEqual(yamlErrors(result), []); +}); + +test('a colon with no trailing space is left alone', () => { + // `https://example.com` is a perfectly good YAML scalar. The rule keys on + // colon-space, not on colons, so ordinary URLs do not trip it. + const result = lintSkillContent( + 'alpha', + fmLines('description: Use when you need alpha', 'docs: https://example.com/a:b'), + KNOWN, + ); + assert.deepEqual(yamlErrors(result), []); +}); + +test('a tab used for indentation is rejected', () => { + const result = lintSkillContent( + 'alpha', + fmLines('description: Use when you need alpha', 'meta:', '\tlevel: core'), + KNOWN, + ); + assert.equal(yamlErrors(result).length, 1); + assert.match(yamlErrors(result)[0], /indents with a tab/); +}); + +test('an unterminated quote is rejected', () => { + const result = lintSkillContent('alpha', fmLines('description: "Use when you need alpha'), KNOWN); + assert.equal(yamlErrors(result).length, 1); + assert.match(yamlErrors(result)[0], /never closes/); +}); + +test('a duplicate key is not reported, because YAML accepts it', () => { + // Deliberate boundary: `safe_load` accepts duplicate keys, so flagging them + // here would fail files no host rejects. The rule tracks the parser, not taste. + const result = lintSkillContent( + 'alpha', + fmLines('description: Use when you need alpha', 'description: Use when you need alpha'), + KNOWN, + ); + assert.deepEqual(yamlErrors(result), []); +}); + +test('the error names the line so the fix is obvious', () => { + const result = lintSkillContent( + 'alpha', + fmLines('description: Use when you need alpha', 'owner: team: platform'), + KNOWN, + ); + assert.match(yamlErrors(result)[0], /^Frontmatter line 4 /); +}); diff --git a/scripts/lib/skill-lint.js b/scripts/lib/skill-lint.js index fa2aad58..be4a455d 100644 --- a/scripts/lib/skill-lint.js +++ b/scripts/lib/skill-lint.js @@ -146,6 +146,85 @@ function parseFrontmatter(content) { return result; } +/** + * Report frontmatter that this file's own parser accepts but a real YAML + * parser rejects. + * + * `parseFrontmatter` above splits each line on its first colon. That is enough + * to read `name` and `description`, and it is deliberately forgiving — but it + * means a SKILL.md whose frontmatter is not valid YAML passes every check here. + * The hosts that consume these skills do not share that forgiveness: Cursor + * parses the frontmatter as YAML when a skill is attached to a message, and a + * parse failure there fails the whole request with a generic server error and + * takes the rest of the chat's context with it (#494). + * + * So the repo could ship frontmatter that breaks a host while CI stayed green. + * That was verified once by hand, against all 25 skills, in the #494 thread; + * this makes it a check instead of a memory. + * + * Scope is deliberately narrow — the three shapes a strict parser rejects and + * the split-on-first-colon parser does not, confirmed against ruby's psych: + * + * description: Use this: when X unquoted value with a colon-space: YAML + * reads a nested mapping and errors + * \tkey: value tab indentation: invalid YAML whitespace + * description: "unterminated an unclosed quote + * + * It is not a YAML implementation. This repo has no package.json and therefore + * no parser to depend on, so the check stays a small set of rules aimed at the + * classes actually observed to break a host. No current frontmatter uses block + * scalars or multi-line values, and the check assumes that stays true; add a + * rule here rather than loosening one if it changes. + */ +function frontmatterYamlErrors(content) { + const match = content.match(/^---[ \t]*\r?\n([\s\S]*?)\r?\n---[ \t]*\r?\n/); + if (!match) return []; + + const errors = []; + const lines = match[1].split(/\r?\n/); + lines.forEach((line, i) => { + const lineNo = i + 2; // the opening `---` is line 1 + if (!line.trim()) return; + + if (/^[ ]*\t/.test(line)) { + errors.push( + `Frontmatter line ${lineNo} indents with a tab — YAML forbids tabs in indentation, ` + + `so a host that parses this frontmatter rejects the whole file` + ); + return; + } + + const colonIdx = line.indexOf(':'); + if (colonIdx === -1) return; + const value = line.slice(colonIdx + 1).trim(); + if (!value) return; + + const quote = value[0] === '"' || value[0] === "'" ? value[0] : null; + if (quote) { + // An opening quote must close. `endsWith` is enough here because a + // trailing comment after a quoted scalar does not appear in this repo. + if (value.length < 2 || !value.endsWith(quote)) { + errors.push( + `Frontmatter line ${lineNo} opens a ${quote === '"' ? 'double' : 'single'} quote that never closes — ` + + `a host parsing this frontmatter as YAML rejects the whole file` + ); + } + return; + } + + // Unquoted scalar. A colon followed by a space (or ending the line) makes + // YAML read a nested mapping where a plain string was meant. + if (/:(\s|$)/.test(value)) { + errors.push( + `Frontmatter line ${lineNo} has an unquoted value containing ': ' — YAML reads that as a ` + + `nested mapping and rejects the file; wrap the value in quotes` + ); + } + }); + + return errors; +} + /** * Collect all explicit skill cross-references from content. * Only matches against the SKILL_REF_PATTERNS list to avoid @@ -183,6 +262,10 @@ function lintSkillContent(dirName, content, knownSkills) { return { errors, warnings, exempt }; } + // The parser above is forgiving by design; the hosts that read this + // frontmatter are not (#494). + errors.push(...frontmatterYamlErrors(content)); + if (!fm.name) { errors.push("Frontmatter missing required field: 'name'"); } else if (fm.name !== dirName) { @@ -311,6 +394,7 @@ function lintSkill(dirName, skillsDir, knownSkills) { module.exports = { stripFencedCodeBlocks, parseFrontmatter, + frontmatterYamlErrors, extractSkillReferences, lintSkillContent, lintSkill, From 4af909b7c5a0b7104a8fe29f35fe5e255ad6478d Mon Sep 17 00:00:00 2001 From: Abhishek Sharma Date: Mon, 21 Sep 2026 12:19:04 -0700 Subject: [PATCH 2/2] feat(validator): hold commands to the same frontmatter rules as skills MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `descriptionFromMd` in validate-commands.js splits each frontmatter line on its first colon — the same forgiving read the skill linter used before the strict rules landed. So a command whose frontmatter is not valid YAML passes every check here, and Claude Code parses that frontmatter when the command is loaded. Same class of gap as the SKILL.md one, on the other set of files: the #494 thread verified all 25 SKILL.md files *and* the command files by hand. This makes the second half a check too, sharing the rules rather than restating them. All nine current commands already pass — confirmed independently against ruby's psych before writing this — so it is a guard, not a repair. That is the same shape the skills half had, and the same reason: the manual verification was real, it just could not stay true on its own. Scoped to `.claude/commands/*.md`. The `.gemini/` and `commands/` directories are TOML and already go through a real TOML parser, so a malformed one surfaces as a missing description in the existing check. A test pins that the YAML rules are NOT applied to them, since a TOML description legitimately carries a colon and running these rules over it would fail nearly every command. The test sandbox copies the validator into a temp tree, so it now copies scripts/lib/skill-lint.js beside it. Without that every existing test fails with a module-not-found, which is how I found it. Five new tests. Mutation-tested 2/2: skipping the check, and reporting its errors without counting them, each fail two. Every validator exits 0 and all six script suites pass. Refs #494 --- scripts/validate-commands-test.js | 66 +++++++++++++++++++++++++++++++ scripts/validate-commands.js | 35 ++++++++++++++++ 2 files changed, 101 insertions(+) diff --git a/scripts/validate-commands-test.js b/scripts/validate-commands-test.js index 1bcf77c4..8c65c867 100644 --- a/scripts/validate-commands-test.js +++ b/scripts/validate-commands-test.js @@ -10,6 +10,9 @@ const { spawnSync } = require('node:child_process'); const { afterEach, test } = require('node:test'); const VALIDATOR = path.join(__dirname, 'validate-commands.js'); +// The validator shares the frontmatter-validity rules with validate-skills, so +// the sandbox needs the lib alongside it, not just the script. +const SKILL_LINT = path.join(__dirname, 'lib', 'skill-lint.js'); const sandboxes = []; function makeSandbox() { @@ -17,6 +20,8 @@ function makeSandbox() { const scriptsDir = path.join(root, 'scripts'); fs.mkdirSync(scriptsDir, { recursive: true }); fs.copyFileSync(VALIDATOR, path.join(scriptsDir, 'validate-commands.js')); + fs.mkdirSync(path.join(scriptsDir, 'lib'), { recursive: true }); + fs.copyFileSync(SKILL_LINT, path.join(scriptsDir, 'lib', 'skill-lint.js')); sandboxes.push(root); return root; } @@ -148,3 +153,64 @@ test('parses escaped quotes in double-quoted TOML descriptions', () => { assert.equal(result.status, 0, result.stdout + result.stderr); assert.match(result.stdout, /1 commands checked — 0 error\(s\) — PASSED/); }); + +// Claude parses a command's frontmatter as YAML when the command is loaded, and +// `descriptionFromMd` splits each line on its first colon — so a command whose +// frontmatter is not valid YAML passed every check here. Same class as the +// SKILL.md gap, on the other set of files the #494 thread checked by hand. + +test('a command whose frontmatter is valid YAML passes', () => { + const root = makeSandbox(); + writeMatchingCommands(root, 'build', 'Build the thing'); + const result = run(root); + assert.equal(result.status, 0, result.stdout); + assert.match(result.stdout, /Checking Claude command frontmatter/); +}); + +test('an unquoted colon in a command description is rejected', () => { + const root = makeSandbox(); + // Valid to the splitter, rejected by a real YAML parser: it reads a nested + // mapping. The TOML siblings keep the same text so only the .md is at fault. + writeClaudeCommand(root, 'build', 'description: Build the thing: quickly'); + writeTomlCommand(root, path.join('.gemini', 'commands'), 'build', 'description = "Build the thing: quickly"'); + writeTomlCommand(root, 'commands', 'build', 'description = "Build the thing: quickly"'); + + const result = run(root); + assert.equal(result.status, 1, result.stdout); + assert.match(result.stdout, /unquoted value containing/); +}); + +test('quoting the same description makes it pass', () => { + const root = makeSandbox(); + writeClaudeCommand(root, 'build', 'description: "Build the thing: quickly"'); + writeTomlCommand(root, path.join('.gemini', 'commands'), 'build', 'description = "Build the thing: quickly"'); + writeTomlCommand(root, 'commands', 'build', 'description = "Build the thing: quickly"'); + + const result = run(root); + assert.equal(result.status, 0, result.stdout); +}); + +test('a tab-indented command frontmatter is rejected', () => { + const root = makeSandbox(); + writeFile( + root, + path.join('.claude', 'commands', 'build.md'), + '---\ndescription: Build the thing\nmeta:\n\tlevel: core\n---\n\n# Command\n', + ); + writeTomlCommand(root, path.join('.gemini', 'commands'), 'build', 'description = "Build the thing"'); + writeTomlCommand(root, 'commands', 'build', 'description = "Build the thing"'); + + const result = run(root); + assert.equal(result.status, 1, result.stdout); + assert.match(result.stdout, /indents with a tab/); +}); + +test('the TOML directories are not put through the YAML rules', () => { + const root = makeSandbox(); + // A TOML description legitimately carries a colon. Running the YAML rules over + // these files would fail every command that has one. + writeMatchingCommands(root, 'build', 'Build the thing quickly'); + const result = run(root); + assert.equal(result.status, 0, result.stdout); + assert.doesNotMatch(result.stdout, /\.gemini.*unquoted value/); +}); diff --git a/scripts/validate-commands.js b/scripts/validate-commands.js index a9ff4399..f26736bb 100644 --- a/scripts/validate-commands.js +++ b/scripts/validate-commands.js @@ -10,6 +10,7 @@ * Checks (errors block CI): * - Every command present in one directory exists in all three * - The 'description' field is identical across all three equivalents + * - Claude command frontmatter is valid YAML, not merely splittable * * What this does NOT check: * Prompt body differences are intentional — each tool has its own @@ -23,6 +24,14 @@ const fs = require('fs'); const path = require('path'); +// The same frontmatter-validity rules validate-skills applies to SKILL.md. +// `descriptionFromMd` below splits each line on its first colon, exactly as the +// skill reader used to, so a command whose frontmatter is not valid YAML passes +// every check here — and Claude Code parses that frontmatter when the command is +// loaded. The #494 thread verified all 25 SKILL.md files AND the command files by +// hand; this makes the second half a check too. +const { frontmatterYamlErrors } = require(path.join(__dirname, 'lib', 'skill-lint.js')); + // ─── Config ─────────────────────────────────────────────────────────────────── const ROOT = path.resolve(__dirname, '..'); @@ -136,6 +145,32 @@ function main() { } } + // ── Claude frontmatter validity ───────────────────────────────────────────── + // Only the .md directory: the TOML dirs are parsed by a real TOML parser + // already, so a malformed one surfaces as a missing description above. + console.log('\nChecking Claude command frontmatter...'); + + for (const stem of claudeStems) { + const full = path.join(DIRS.claude.dir, `${stem}${DIRS.claude.ext}`); + let yamlErrors = []; + try { + yamlErrors = frontmatterYamlErrors(fs.readFileSync(full, 'utf8')); + } catch (e) { + console.log(` ✗ ${stem} — cannot read file: ${e.message}`); + errors++; + continue; + } + if (yamlErrors.length === 0) { + console.log(` ✓ ${stem}`); + continue; + } + console.log(` ✗ ${stem}`); + for (const message of yamlErrors) { + console.log(` ${message}`); + errors++; + } + } + // ── Description sync check ────────────────────────────────────────────────── console.log('\nChecking description sync...');