From a1a80d56d1f88a4c2ffcbbb0877570a3eb8e7a3a Mon Sep 17 00:00:00 2001 From: ayobamiseun Date: Mon, 14 Sep 2026 14:04:13 +0100 Subject: [PATCH] fix(validator): strip every CommonMark fence form from linted prose (#437) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit stripFencedCodeBlocks matched fences with a single whole-document regex that only recognised a column-zero backtick fence closed by a run of exactly the same length. Three CommonMark-legal forms slipped through and their contents were linted as prose: tilde fences, fences indented one to three spaces, and closing fences longer than their opener. This is live on main today — spec-driven-development's list-nested examples use indented fences, so the linter has been reading their contents as skill text. Replace the regex with a line scanner that tracks the opener's marker and length, closes only on a same-marker run at least as long, treats an unterminated fence as running to end of file (so sections after it fail loud), and follows the CommonMark rule that a backtick fence's info string may not contain backticks. Stripped lines become empty lines so positions are preserved. Add unit coverage for each form plus CRLF input, and run scripts/lib/skill-lint-test.js in CI — it was the only test file the workflow did not execute. Revives the approach from #444 by @ujjwalks, which was approved and then closed after a rebase conflict. Closes #437 --- .github/workflows/test-plugin-install.yml | 3 + scripts/lib/skill-lint-test.js | 110 ++++++++++++++++++++++ scripts/lib/skill-lint.js | 43 ++++++++- 3 files changed, 155 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test-plugin-install.yml b/.github/workflows/test-plugin-install.yml index 1442ea3f..2d02e673 100644 --- a/.github/workflows/test-plugin-install.yml +++ b/.github/workflows/test-plugin-install.yml @@ -22,6 +22,9 @@ jobs: - name: Validate all skills run: node scripts/validate-skills.js + - name: Test skill lint rules + run: node --test scripts/lib/skill-lint-test.js + - name: Validate manifest versions run: node scripts/validate-versions.js diff --git a/scripts/lib/skill-lint-test.js b/scripts/lib/skill-lint-test.js index af898c0b..a9a1f253 100644 --- a/scripts/lib/skill-lint-test.js +++ b/scripts/lib/skill-lint-test.js @@ -139,3 +139,113 @@ test('reports a missing frontmatter block', () => { assert.equal(errors.length, 1); assert.match(errors[0], /Missing or malformed YAML frontmatter/); }); + +// ─── Fenced-block stripping (#437) ─────────────────────────────────────────── +// +// Observed through the required-section rule: a `## Overview` heading that +// lives inside a fenced block must NOT satisfy the check, so the presence of +// "Missing required section: ## Overview" proves the block was stripped, and +// its absence proves prose outside the block survived. + +const FENCE_KNOWN = new Set(['fenced']); + +/** A SKILL.md with every required section except Overview, which the caller supplies. */ +function skillWithOverview(overviewBlock) { + return [ + '---', + 'name: fenced', + 'description: Exercises fence parsing. Use when testing the linter.', + '---', + '', + overviewBlock, + '', + '## When to Use', + 'x', + '## Common Rationalizations', + 'x', + '## Red Flags', + 'x', + '## Verification', + 'x', + '', + ].join('\n'); +} + +const OVERVIEW = '## Overview\nx'; +const overviewMissing = ({ errors }) => errors.includes('Missing required section: ## Overview'); + +test('a real Overview heading satisfies the required-section check', () => { + const result = lintSkillContent('fenced', skillWithOverview(OVERVIEW), FENCE_KNOWN); + assert.deepEqual(result.errors, []); +}); + +for (const [form, block] of [ + ['a backtick fence', '```markdown\n## Overview\n```'], + ['an unlabeled fence', '```\n## Overview\n```'], + ['a tilde fence', '~~~markdown\n## Overview\n~~~'], + ['a fence indented one space', ' ```\n## Overview\n ```'], + ['a fence indented three spaces', ' ```\n## Overview\n ```'], + ['a fence with a longer closer', '```\n## Overview\n`````'], + ['a four-backtick fence', '````\n## Overview\n````'], +]) { + test(`a heading inside ${form} does not satisfy the check`, () => { + const result = lintSkillContent('fenced', skillWithOverview(block), FENCE_KNOWN); + assert.equal(overviewMissing(result), true, `heading inside ${form} leaked into prose`); + }); +} + +// The closer must be recognised, or every line after it is swallowed. +for (const [form, block] of [ + ['a same-length closer', '```\nexample\n```\n\n' + OVERVIEW], + ['a longer closer', '```\nexample\n`````\n\n' + OVERVIEW], + ['an indented closer', '```\nexample\n ```\n\n' + OVERVIEW], + ['a tilde fence closer', '~~~\nexample\n~~~\n\n' + OVERVIEW], + ['a closer with trailing spaces', '```\nexample\n``` \n\n' + OVERVIEW], +]) { + test(`prose after ${form} is still linted`, () => { + const result = lintSkillContent('fenced', skillWithOverview(block), FENCE_KNOWN); + assert.equal(overviewMissing(result), false, `prose after ${form} was swallowed`); + }); +} + +test('a shorter run of the same marker does not close a longer fence', () => { + const block = '````\n```\n## Overview\n```\n````'; + const result = lintSkillContent('fenced', skillWithOverview(block), FENCE_KNOWN); + assert.equal(overviewMissing(result), true); +}); + +test('a backtick run does not close a tilde fence, and vice versa', () => { + for (const block of ['~~~\n```\n## Overview\n~~~', '```\n~~~\n## Overview\n```']) { + const result = lintSkillContent('fenced', skillWithOverview(block), FENCE_KNOWN); + assert.equal(overviewMissing(result), true); + } +}); + +test('a fence indented four spaces is an indented code block, not a fence', () => { + // Four spaces makes the line indented code in CommonMark; the heading that + // follows is regular prose and must still satisfy the check. + const block = ' ```\n' + OVERVIEW; + const result = lintSkillContent('fenced', skillWithOverview(block), FENCE_KNOWN); + assert.equal(overviewMissing(result), false); +}); + +test('a backtick run followed by inline backticks is prose, not an opener', () => { + // CommonMark forbids backticks in the info string of a backtick fence, so + // this line is ordinary prose and the heading below it must still count. + const block = '```js``` is how you write inline code for a fence\n' + OVERVIEW; + const result = lintSkillContent('fenced', skillWithOverview(block), FENCE_KNOWN); + assert.equal(overviewMissing(result), false); +}); + +test('an unterminated fence swallows everything after it and fails loud', () => { + const block = '```\n' + OVERVIEW; + const result = lintSkillContent('fenced', skillWithOverview(block), FENCE_KNOWN); + assert.equal(overviewMissing(result), true); + assert.equal(result.errors.includes('Missing required section: ## Verification'), true); +}); + +test('CRLF line endings are handled', () => { + const content = skillWithOverview('```\n## Overview\n```').replace(/\n/g, '\r\n'); + const result = lintSkillContent('fenced', content, FENCE_KNOWN); + assert.equal(overviewMissing(result), true); +}); diff --git a/scripts/lib/skill-lint.js b/scripts/lib/skill-lint.js index d63a994e..b7a03e17 100644 --- a/scripts/lib/skill-lint.js +++ b/scripts/lib/skill-lint.js @@ -80,9 +80,50 @@ const SKILL_REF_PATTERNS = [ /** * Strip fenced code blocks from markdown content so that headings, references, * and trigger phrases inside examples or templates are not matched by lint rules. + * + * Scans line by line and follows the CommonMark fence rules rather than a single + * whole-document regex, which only recognised a column-zero backtick fence closed + * by a run of exactly the same length (#437). The forms that regex let through: + * + * - tilde fences (`~~~`) + * - fences indented by one to three spaces (common inside list items) + * - a closing fence longer than its opener + * + * A fence closes only on a run of the *same* marker at least as long as the + * opener; a shorter run, or the other marker, is content. An unterminated fence + * runs to end of file, so everything after it is treated as fenced and any + * required section below it is reported missing rather than silently accepted. + * + * Stripped lines are replaced with empty lines so line numbers are preserved + * for callers that report positions. */ function stripFencedCodeBlocks(content) { - return content.replace(/^(`{3,})[^\n]*\n[\s\S]*?^\1\s*$/gm, ''); + const out = []; + let open = null; // { marker: '`' | '~', length: number } while inside a fence + + for (const line of content.split(/\r?\n/)) { + if (open) { + const close = line.match(/^ {0,3}(`{3,}|~{3,})[ \t]*$/); + if (close && close[1][0] === open.marker && close[1].length >= open.length) { + open = null; + } + out.push(''); + continue; + } + + const start = line.match(/^ {0,3}(`{3,}|~{3,})(.*)$/); + // CommonMark: the info string of a backtick fence may not contain backticks, + // so a line like ````js``` is inline code` is prose, not an opener. + if (start && !(start[1][0] === '`' && start[2].includes('`'))) { + open = { marker: start[1][0], length: start[1].length }; + out.push(''); + continue; + } + + out.push(line); + } + + return out.join('\n'); } /**