From edaf268d8c7e28efe650c511cea170dd575969f3 Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Thu, 27 Aug 2026 00:19:23 -0400 Subject: [PATCH 01/10] ci: scope the release-docs checks instead of parsing shell (#547) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #547 asked for a decision: generate the shared release-doc block into both copies between `bestax:generated` markers, or keep the check and record why the duplication persists. Declined the generator — it puts machine-owned regions in the repo's most-read contributor document, where every future hand-editor of raw CONTRIBUTING.md meets them. A broken marker pair fails loudly, so the objection is friction rather than safety, but that friction is paid by every editor forever and the drift class it removes is one enumeration per file. Both copies stay hand-written; the rule holds them together, and its header now says so rather than leaving the next reviewer to re-propose it. That decision comes with its price paid rather than deferred: the package-list half of the rule no longer parses shell. Reading loop word lists, command position and echo-vs-invoke bought one refinement over membership, and that refinement is where all four bugs the previous hardening round fixed lived — each a false red whose suggested remedy was to drop the file from RELEASE_DOC_FILES, i.e. switch the check off. - `recipeFences` replaces `dryRunRecipe` + `recipeTargets` + `invokesCommand`: every fence running the dry run is a recipe a contributor might copy, so every one of them is held to the full package list. Selecting a single "best" fence had been a bug in both directions. - `publisherSections` replaces `packagesBullet`. The `- Packages:` anchor stays — it is a one-line find and a stable marker — but the bullet is no longer sliced out of its section: three of that scan's four end-bounds had each been a bug. Fenced lines stay excluded, which is the one bound worth keeping. - `shellOperative` keeps only comment and quote stripping, which preserves the two cheapest exclusions without any shell grammar. - `safeToRunBlock` gets the residual `:::` fix #547 names: the terminator is run-length aware, so a nested `:::note` no longer truncates the block above most of the facts. Falls back to a width of 3 with no opener on the marker line, because the alternative reading is a fail-open. CONTRIBUTING.md and its docs mirror are unchanged. Trade recorded in the rule header: a package named anywhere in the recipe fence now counts, as does one named below the bullet in its own section. Both holes are bounded by a construct measured in lines, and on today's documents the names appear only in the loop word list and only in the bullet. Tests: the six cases that only exercised the deleted grammar are gone; six new ones cover the nesting fix, its fail-open counterpart, the every-fence rule, and the two new helpers directly. Also rebuilt the unterminated-fence case, which stripped a trailing fence doc() does not end with and so asserted the untouched fixture. --- scripts/check-conformance.mjs | 310 +++++++++++++++-------------- scripts/release-docs-sync.test.mjs | 243 ++++++++++++++-------- 2 files changed, 326 insertions(+), 227 deletions(-) diff --git a/scripts/check-conformance.mjs b/scripts/check-conformance.mjs index 25e66cea8..89d8819ca 100644 --- a/scripts/check-conformance.mjs +++ b/scripts/check-conformance.mjs @@ -1027,23 +1027,43 @@ async function checkStyleMappingSync() { // rather than restating them — the two staleness classes #536 found were both // "a list of packages that stopped being all of them". // -// Why this parses markdown at all, since the obvious simplification is to stop. -// Whole-file matching would have caught the failure that prompted #536: -// bestax-migrate and bestax-mcp appeared ZERO times in either guide, so a plain -// token-membership test over the whole file catches it, and four of the five -// facts below occur exactly once per file so they need no scoping either. The -// parsing buys one thing on top of that — a package that IS named somewhere in -// the file but missing from the recipe or the publisher list specifically — and -// that refinement is where every edge case lives: fence pairing, section -// anchors, continuation lines, sentinels. Measured, not assumed, and kept -// deliberately. If it ever needs paying for again, the cheap version is -// "every publishable package is named somewhere in each guide", and it is worth -// knowing that it would have been enough for the bug that started this. +// Why the duplication PERSISTS, which is the question this header used to leave +// open (#547). Deliberately NOT a byte diff: the two files legitimately differ, +// one wrapping the guidance in a blockquote and the other in a Docusaurus +// admonition, and the docs page links absolute GitHub URLs where the root file +// links relative paths. A diff would fail on all of that and teach people to +// ignore it. Generating the shared block into both files between +// `bestax:generated` markers — the #542 skill-roster shape — was considered for +// exactly this and DECLINED: it puts machine-owned regions in the repo's +// most-read contributor document, where every future hand-editor of raw +// CONTRIBUTING.md meets them. A broken marker pair fails loudly (readRegions +// throws), so the objection is friction rather than safety — but that friction +// is paid by every editor forever, and the drift class it removes is one +// enumeration per file. So both copies stay hand-written, and this rule holds +// them together. Please stop re-proposing the generator. // -// Deliberately NOT a byte diff. The two files legitimately differ: one uses a -// blockquote, the other a Docusaurus admonition, and the docs page links -// absolute GitHub URLs where the root file links relative paths. A diff would -// fail on all of that and teach people to ignore it. +// Why the assertions are SCOPED rather than file-wide. Whole-file matching +// would have caught the failure that prompted #536 — bestax-migrate and +// bestax-mcp appeared ZERO times in either guide — but it is vacuous for the +// facts (CONTRIBUTING.md links VERSIONING.md twice more for unrelated reasons) +// and nearly vacuous for the packages, since every publishable name is also +// spelled out in the commit-scope prose. Each assertion is therefore scoped to +// the construct that OWNS the list, and then reduced to whole-token membership +// inside it. +// +// What that scoping deliberately does NOT do, since the previous generation +// did: it does not parse shell. Reading loop word lists, command position and +// echo-vs-invoke bought one refinement — a package named inside the recipe +// fence but absent from the loop itself — and that refinement is where all four +// bugs #548 fixed lived, each one a false red whose suggested remedy was "drop +// this file from RELEASE_DOC_FILES", i.e. switch the check off. The trade taken +// in #547: a package named anywhere in the recipe fence now counts, as does one +// named anywhere below the `- Packages:` bullet in its own section. Both holes +// are bounded by a construct measured in lines, and on today's documents the +// names appear only in the loop word list and only in the bullet itself, so the +// two readings agree. Comment and quoted text are still stripped from the fence +// (shellOperative) and fenced lines are still dropped from the publisher list, +// which keeps the cheapest of those exclusions without any shell grammar. const RELEASE_DOC_FILES = [ 'CONTRIBUTING.md', 'docs/docs/guides/getting-started/contributing.md', @@ -1078,6 +1098,13 @@ const RELEASE_DOC_FACTS = [ * bugs: a `####` heading did not close the block, so it ran to EOF and restored * the file-wide search this exists to prevent; and a `---` or `:::` inside a * fenced example truncated it early, in the other direction. + * + * The `:::` terminator is nesting-aware for the same reason (#547). On the docs + * page the guidance IS the admonition, so its own closing `:::` is the end — + * but a nested `:::note` inside it closes first, and taking that as the end + * truncated the block above most of the facts. Docusaurus nests by widening the + * outer run, so a close only ends the block when it matches the run that opened + * it. */ // One definition of a markdown heading line for every release-docs // extractor: four hand-copies of this regex had already picked up a \s-vs- @@ -1086,39 +1113,33 @@ const MD_HEADING = /^ {0,3}#{1,6}\s/; /** * Comment- and quote-stripped shell text — the OPERATIVE part of a recipe. - * Quotes go because every fail-open in this family arrived through prose - * inside the block: an echoed banner naming the command, a quoted 'step - * done' ending a segment early. In these recipes the operative shell — loop - * word lists, the release invocation — is never quoted, so what survives is + * Quotes go because both fail-opens this family ever saw arrived through prose + * inside the fence: a `# …` line naming a package the loop skipped, and an + * echoed banner naming one. In these recipes the operative shell — the loop + * word list, the release invocation — is never quoted, so what survives is * what runs. + * + * This is all that is left of the shell reading. The command-position and + * loop-segmentation layers that used to sit here were deleted in #547; the + * header above records what that traded. */ function shellOperative(text) { return text .split('\n') .map(l => l.replace(/"[^"]*"|'[^']*'/g, '""').replace(/#.*$/, '')) - .join('\n') - .replace(/\\\n\s*/g, ' '); + .join('\n'); } /** - * True when the operative text INVOKES `cmd`: the mention sits in command - * position of some `;`/`|`/`&`/subshell segment, with shell keywords - * (do/then/else) peeled — so `do pnpm exec semantic-release` counts while - * `do echo semantic-release …` and a subshell echo do not, whatever the line - * starts with (#548 review: the first version only excluded echo at line - * start, so a one-line banner loop passed as the release loop). + * A Docusaurus admonition delimiter, or null. `:::tip Safe to run` OPENS, + * a bare `:::` CLOSES, and the run length matters — nesting works by widening + * the outer run (`::::tip` around `:::note`), exactly as CommonMark fences + * nest by widening the backtick run. */ -function invokesCommand(operative, cmd) { - return operative.split('\n').some(line => - line.split(/[;|&()]+/).some(seg => { - if (!seg.includes(cmd)) return false; - const first = seg - .trim() - .replace(/^(?:do|then|else)\s+/, '') - .split(/\s+/)[0]; - return first !== 'echo' && first !== 'printf'; - }) - ); +function admonitionDelim(line) { + const m = line.match(/^ {0,3}(:{3,})(.*)$/); + if (!m) return null; + return { len: m[1].length, open: Boolean(m[2].trim()) }; } function safeToRunBlock(src) { @@ -1131,11 +1152,32 @@ function safeToRunBlock(src) { (l, i) => !masked[i] && l.includes('Safe to run; never publishes') ); if (start < 0) return null; + // The run that opened the guidance, when the marker line IS the opener (the + // docs page). Only a close at least that wide ends the block; anything + // narrower is a nested admonition closing itself. + // + // Falling back to 3 rather than to Infinity, deliberately: Infinity reads + // better — "no opener, so no close can end this" — and is the fail-open + // direction, because the blockquote form would then run past a stray `:::` + // to the next heading and restore the file-wide search this scoping exists + // to prevent. 3 is what this line did before nesting was considered at all. + // The residual, recorded rather than chased: if the marker is moved OFF its + // `:::tip` line into the body, the opener's width is no longer visible here + // and a nested close truncates the block again. Finding the enclosing opener + // by scanning backwards would fix that shape, and is the kind of refinement + // the header above argues against buying. + const opened = admonitionDelim(lines[start]); + const outer = opened?.open ? opened.len : 3; let end = lines.length; for (let i = start + 1; i < lines.length; i++) { if (masked[i]) continue; // a terminator inside a fenced example is content const l = lines[i]; - if (l.trim() === ':::' || MD_HEADING.test(l) || l.trim() === '---') { + const delim = admonitionDelim(l); + if (delim && !delim.open && delim.len >= outer) { + end = i; + break; + } + if (MD_HEADING.test(l) || l.trim() === '---') { end = i; break; } @@ -1144,11 +1186,23 @@ function safeToRunBlock(src) { } /** - * The fenced block that runs the semantic-release dry run, or null. Located by - * content rather than by heading, so renaming the section does not silently - * switch this check off. + * Every fenced block that runs the semantic-release dry run, comment- and + * quote-stripped. Located by CONTENT rather than by heading, so renaming the + * section does not silently switch this check off. + * + * The fence is the scope, and that is the whole of the reading (#547): a fence + * is a recipe, so a prose mention of the command — "run `semantic-release + * --dry-run` to preview" in some other section — is excluded by construction + * rather than by inspecting command position. + * + * ALL matching fences are returned, not the best one. Picking a single fence + * was itself a bug twice over: first-match let an example fence above the + * recipe become "the recipe", and preferring the one that looked like it + * invoked let a stale second recipe hide behind a fresh one. Every fence that + * runs the dry run is a recipe a contributor might copy, so every one of them + * has to be complete. * - * Fences come from `fenceMask` rather than a /```[\s\S]*?```/g match, which + * Fences come from `fenceSpans` rather than a /```[\s\S]*?```/g match, which * pairs backtick runs in document order: one stray ``` in prose, or a * ````markdown wrapper around a nested fence, shifts every pair after it and the * real recipe stops being found. The remedy the violation then offers is "drop @@ -1157,7 +1211,7 @@ function safeToRunBlock(src) { * returned, for the same reason — dropping it produced that same false * violation. */ -function dryRunRecipe(src) { +export function recipeFences(src) { const { lines } = splitLines(src); // Real block spans from the fence state machine — never re-derived from // the boolean mask, where butted fences form one run and a content line @@ -1166,120 +1220,76 @@ function dryRunRecipe(src) { // ````markdown wrapper). An unterminated fence still runs to EOF and is // still returned: dropping it produced the switch-the-check-off violation // this docblock has always warned about. - const blocks = fenceSpans(lines).map(({ open, close }) => - lines.slice(open, close + 1).join('\n') - ); - // Selection prefers the block that INVOKES the dry run over the first one - // that merely mentions it: an example fence above the recipe, echoing the - // command it describes, otherwise becomes "the recipe" and the real one - // below is never checked. Containment stays as the fallback so a recipe - // driving the run through a variable keeps being found. - return ( - blocks.find(b => - invokesCommand(shellOperative(b), 'semantic-release --dry-run') - ) ?? - blocks.find(b => b.includes('semantic-release --dry-run')) ?? - null - ); + return fenceSpans(lines) + .map(({ open, close }) => + shellOperative(lines.slice(open, close + 1).join('\n')) + ) + .filter(b => b.includes('semantic-release --dry-run')); } /** - * The package names a dry-run recipe actually iterates over. + * The `- Packages:` list of every heading section whose TITLE claims trusted + * publishing — the section that owns the packages needing a publisher + * configured on npmjs.com. Each entry runs from the bullet to the end of its + * section, fenced lines dropped. * - * Two narrowings, each closing a way a package could look covered while the - * recipe skipped it. First comments are stripped, because a name in a `# …` - * line satisfied the assertion while being absent from the loop. Then, if the - * block drives a `for … in …` loop, ONLY that word list counts — otherwise an - * `echo "bestax-mcp is released separately"` satisfies it just as well, which - * is the same failure one step further out. - */ -export function recipeTargets(block) { - // Quote- and comment-stripped first (shellOperative): a quoted 'step done' - // used to end the release loop's segment above its invocation line, and a - // quoted banner naming the command classified its loop as the release - // loop. Backslash continuations are joined there too, so a wrapped - // word-list stays one list. - const commands = shellOperative(block); - // Segments run from each `for … in` header to the NEXT header (or end) — - // never to `done`: a missing or comment-hidden `done` dissolved every - // segment and the whole block became the word list, the exact vacuity this - // function exists to prevent (#548 review). - const headers = [...commands.matchAll(/\bfor\s+\w+\s+in\s+([^;\n]+)/g)]; - if (!headers.length) return commands; - const segments = headers.map((m, i) => ({ - list: m[1], - body: commands.slice(m.index, headers[i + 1]?.index ?? commands.length), - })); - // Only loops that INVOKE the release contribute their word lists, falling - // back to all loops when none does, so a recipe driving the run through a - // variable stays a presence check rather than a false red. - const release = segments.filter(s => - invokesCommand(s.body, 'semantic-release') - ); - return (release.length ? release : segments).map(s => s.list).join(' '); -} - -/** - * The `- Packages:` bullet from the OIDC section, with any wrapped - * continuation lines. + * What #547 changed: the bullet is still ANCHORED by its literal `- Packages:` + * prefix, which is a one-line find and a stable marker in the document, but it + * is no longer SLICED out of the section. The old end-bound scan stopped at the + * next bullet, blank line, fenced line or section end, and three of those four + * bounds had been a bug in their own right — a continuation line dropped at + * EOF, a butted heading sweeping the next section in, a butted fence doing the + * same. Running to the section end instead has no bounds to get wrong, and the + * section end is already computed. * - * A named extractor rather than an inline block, so the slicing can be driven - * from a test — its first version collapsed `findIndex`'s -1 sentinel with - * `Math.max(0, …)`, which dropped every continuation line whenever the bullet - * ran to the end of the file with no blank line after it. That is precisely the - * case the extractor was written to handle. + * The trade is that anything below the bullet in the same section now counts: + * on the committed page that is the Provider/Repository/Workflow bullets and + * one paragraph, none of which names a package. Fenced lines are the exception + * and stay excluded — `npm owner ls bestax-mcp` in an example must not stand in + * for the list, which is the one bound worth keeping. * - * Scoped to the OIDC section rather than the first match in the file, because - * file-wide is the vacuity already fixed once for RELEASE_DOC_FACTS: an earlier - * `- Packages:` bullet elsewhere would satisfy this while the real list went - * short. + * A candidate section with NO bullet contributes nothing rather than + * anchoring-and-failing, so a prose aside titled like this section does not + * produce a false red; but if no candidate owns a bullet, "the section lost its + * list" still fires from the caller. Every candidate that does own one is + * validated: the first-match anchor let an earlier aside steal the search + * (false red), and preferring the section that owned a bullet let a LATER + * aside absorb the check while the real list was deleted (fail-open — #548 + * review caught both generations). + * + * Sections run to the next heading at ANY level, so a `####` sub-heading closes + * one rather than letting it swallow the rest of the file. */ -function packagesBullet(src) { +export function publisherSections(src) { const { lines } = splitLines(src); const masked = fenceMask(lines); const isHeading = i => !masked[i] && MD_HEADING.test(lines[i]); - // ONE pass over the candidate headings, each judged on its own span. The - // first-match anchor let an earlier trusted-publishing-titled aside steal - // the search (false red); preferring the section that owns a bullet let a - // LATER aside's stray bullet absorb the check while the real section's - // list was deleted (fail-open — #548 review caught both generations). So: - // every candidate section that contains a `- Packages:` bullet contributes - // it, the caller validates each contribution, and no bullet anywhere among - // the candidates is the violation. A candidate with no bullet adds - // nothing rather than anchoring-and-failing, which keeps a prose aside - // titled like the section from producing a false red — but if NO candidate - // owns a bullet, "section exists but lost its list" still fires. - const bullets = []; + const sections = []; for (let i = 0; i < lines.length; i++) { if (!(isHeading(i) && /trusted[- ]publish/i.test(lines[i]))) continue; - let limit = lines.length; + let end = lines.length; for (let j = i + 1; j < lines.length; j++) { if (isHeading(j)) { - limit = j; - break; - } - } - let start = -1; - for (let j = i + 1; j < limit; j++) { - if (!masked[j] && lines[j].trimStart().startsWith('- Packages:')) { - start = j; - break; - } - } - if (start < 0) continue; - // Bounded by the next bullet, blank, masked line, or the section end: a - // fenced example or the next section butted under the list otherwise - // swept in, and a package named there covered an omission in the list. - let end = limit; - for (let j = start + 1; j < limit; j++) { - if (!lines[j].trim() || /^\s*[-*]\s/.test(lines[j]) || masked[j]) { end = j; break; } } - bullets.push(lines.slice(start, end).join('\n')); + const start = lines.findIndex( + (l, j) => + j > i && + j < end && + !masked[j] && + l.trimStart().startsWith('- Packages:') + ); + if (start < 0) continue; + sections.push( + lines + .slice(start, end) + .filter((_, j) => !masked[start + j]) + .join('\n') + ); } - return bullets; + return sections; } /** @@ -1374,16 +1384,20 @@ export function releaseDocViolations(docs, packages) { } // Derived, not restated: the recipe has to cover whatever releases today. - const recipe = dryRunRecipe(src); - if (recipe === null) { + const recipes = recipeFences(src); + if (!recipes.length) { violations.push( `${rel} has no fenced block running \`semantic-release --dry-run\`. ` + `That recipe is how a contributor previews a release without ` + `publishing — restore it, or drop ${rel} from RELEASE_DOC_FILES in ` + `scripts/check-conformance.mjs if the page no longer covers releases.` ); - } else { - const named = packageTokens(recipeTargets(recipe)); + } + // Every one of them, not the best-looking one: a contributor copies + // whichever fence they land on, so a second recipe that went short is a + // wrong instruction even with a complete one further up the page. + for (const recipe of recipes) { + const named = packageTokens(recipe); const missing = packages.filter(p => !named.has(p.dir)); if (missing.length) { violations.push( @@ -1410,7 +1424,7 @@ export function releaseDocViolations(docs, packages) { // other told a reader the file had no bullet when the file was never read. const [primary] = RELEASE_DOC_FILES; if (docs.has(primary)) { - const sections = packagesBullet(docs.get(primary)); + const sections = publisherSections(docs.get(primary)); if (!sections.length) { violations.push( primary + @@ -1419,9 +1433,9 @@ export function releaseDocViolations(docs, packages) { 'npmjs.com; without it nobody can tell which ones do.' ); } else { - // EVERY bullet under a trusted-publishing heading must be complete: a - // stale duplicate list must not hide behind a fresh one, whichever - // order they appear in. + // EVERY list under a trusted-publishing heading must be complete: a + // stale duplicate must not hide behind a fresh one, whichever order they + // appear in. for (const section of sections) { const named = packageTokens(section); const missing = packages.filter(p => !named.has(p.name)); diff --git a/scripts/release-docs-sync.test.mjs b/scripts/release-docs-sync.test.mjs index 6d9072c74..caa6495f0 100644 --- a/scripts/release-docs-sync.test.mjs +++ b/scripts/release-docs-sync.test.mjs @@ -24,7 +24,8 @@ import { publishablePackages, releaseDocViolations, unreadableManifestViolations, - recipeTargets, + recipeFences, + publisherSections, } from './check-conformance.mjs'; const PACKAGES = [ @@ -47,7 +48,7 @@ const recipe = (dirs = PACKAGES.map(p => p.dir)) => dirs.join(' ') + '; do\n ( cd "$pkg" && pnpm exec semantic-release --dry-run --no-ci )\ndone\n```'; -// The bullet AND the section heading that anchors it. packagesBullet locates +// The bullet AND the section heading that anchors it. publisherSections locates // the list by its OIDC section rather than by the first "- Packages:" in the // file, so a fixture without the anchor is not a valid stand-in for the real // document — and every fixture here was one until the anchor rule landed. @@ -245,9 +246,22 @@ test('a stray fence in prose does not hide the recipe', () => { test('an unterminated fence still counts as the recipe', () => { // Dropping it produced the same false violation as the stray-fence case. - const unterminated = doc().replace(/\n```\s*$/, ''); + // + // Asserted on the MIRROR, and built rather than edited. The previous version + // stripped a trailing ``` from doc(), which ends with the publishers bullet + // and so has none — it asserted the untouched fixture and would have stayed + // green through the regression it names. On the primary file it could not be + // asserted at all: a fence left open swallows the trusted-publisher bullet + // below it, which is a different (and correct) violation. + const unterminated = doc({ + publishers: '', + recipe: + '```bash\nfor pkg in ' + + PACKAGES.map(p => p.dir).join(' ') + + '; do\n ( cd "$pkg" && pnpm exec semantic-release --dry-run --no-ci )\ndone', + }); assert.deepEqual( - releaseDocViolations(docs({ contributing: unterminated }), PACKAGES), + releaseDocViolations(docs({ mirror: unterminated }), PACKAGES), [] ); }); @@ -464,8 +478,10 @@ test('an empty package list is refused, not silently satisfied', () => { // --- hardening round (#544 review) ------------------------------------------- test('a backslash-wrapped for-loop list is read in full', () => { - // recipeTargets stopped at the first newline, so names wrapped onto the - // continuation line were falsely reported missing from the recipe. + // The line-bounded read this covers is gone with the loop parsing (#547) — + // membership spans the whole fence, so a wrapped list cannot be cut in half. + // Kept because the false red it records is the one a future narrowing would + // most easily reintroduce. const wrapped = '```bash\nfor pkg in bulma-ui create-bestax \\\n' + ' bestax-migrate bestax-mcp; do\n' + @@ -570,21 +586,6 @@ test('scalar manifests are unreadable too, not silently absent', async () => { }); }); -test('a preliminary loop cannot cover for the release loop', () => { - // Union-of-all-loops was fail-open: an echo loop over every package let a - // release loop covering only one pass the presence check. Only loops whose - // body invokes the dry run contribute. - const twoLoops = - '```bash\nfor pkg in bulma-ui create-bestax bestax-migrate bestax-mcp; do echo "$pkg"; done\n' + - 'for pkg in bulma-ui; do\n ( cd "$pkg" && pnpm exec semantic-release --dry-run --no-ci )\ndone\n```'; - const v = releaseDocViolations( - docs({ contributing: doc({ recipe: twoLoops }) }), - PACKAGES - ); - assert.ok(v.length >= 1, 'the echo loop must not mask the omission'); - assert.match(v.join(' '), /bestax-mcp/); -}); - test('an array manifest is unreadable, not silently absent', async () => { // `[]` parses, passes typeof object, has no name — the package vanished // from the list with no violation, the same outcome as the null crash by a @@ -596,66 +597,8 @@ test('an array manifest is unreadable, not silently absent', async () => { }); }); -test('an echoed semantic-release mention is not an invoking loop', () => { - // A banner loop over every package must not stand in for (or dilute) the - // loop that actually runs the release. - const block = [ - 'for pkg in a b c d; do', - ' echo "will run semantic-release for $pkg"', - 'done', - 'for pkg in a b; do', - ' pnpm exec semantic-release --dry-run', - 'done', - ].join('\n'); - assert.equal(recipeTargets(block), 'a b'); -}); - // --- #548-review regressions -------------------------------------------------- -test('a one-line banner loop is not the release loop', () => { - // The line starts with `for`, not `echo`, so a line-anchored exclusion - // missed it; invocation is judged per `;`-segment with do/then/else peeled. - const block = [ - '```bash', - 'for pkg in bulma-ui create-bestax bestax-migrate bestax-mcp; do echo "next: semantic-release for $pkg"; done', - 'for pkg in bulma-ui; do', - ' pnpm exec semantic-release --dry-run --no-ci', - 'done', - '```', - ].join('\n'); - assert.equal(recipeTargets(block), 'bulma-ui'); -}); - -test('an unquoted subshell echo is still just a banner', () => { - const block = [ - 'for pkg in a b c; do (echo starting semantic-release run); done', - 'for pkg in a; do pnpm exec semantic-release --dry-run; done', - ].join('\n'); - assert.equal(recipeTargets(block), 'a'); -}); - -test("a quoted 'done' does not end the release loop's segment", () => { - const block = [ - 'for pkg in a b c d; do echo "$pkg"; done', - 'for pkg in a b; do', - ' echo "step done"', - ' pnpm exec semantic-release --dry-run', - 'done', - ].join('\n'); - assert.equal(recipeTargets(block), 'a b'); -}); - -test('a comment-hidden done confines the check to the loop, not the block', () => { - // With `done` swallowed by the comment strip, segmentation used to - // dissolve and the WHOLE block became the word list — an echo naming the - // other packages then satisfied the presence check. - const block = [ - 'for pkg in bulma-ui; do pnpm exec semantic-release --dry-run # done manually', - 'echo "later: create-bestax bestax-migrate bestax-mcp"', - ].join('\n'); - assert.equal(recipeTargets(block).trim(), 'bulma-ui'); -}); - test('an example fence spelling out the dry-run command does not become the recipe', () => { // Selection prefers the block that INVOKES the command; the example's // echoed (quoted) mention is stripped from its operative text. @@ -717,3 +660,145 @@ test("a later aside's bullet cannot absorb a deleted trusted-publisher list", () ); assert.ok(v.length >= 1, 'the incomplete stand-in must not pass'); }); + +// --- #547: scoped membership replaces the shell reading ----------------------- + +test('a nested admonition does not truncate the guidance block', () => { + // On the docs page the guidance IS an admonition, so its own closing ::: is + // the end of the block. A nested :::note closes first, and taking that as the + // end cut the block off above most of the facts — every one of them then read + // as deleted. Docusaurus nests by widening the OUTER run, so only a close at + // least as wide as the opener ends it. + const nested = [ + '::::tip Safe to run; never publishes', + '', + ':::note', + 'An aside. Its close is not the end of the guidance.', + ':::', + '', + ...FACTS, + '::::', + '', + recipe(), + publishers(), + ].join('\n'); + assert.deepEqual( + releaseDocViolations(docs({ contributing: nested }), PACKAGES), + [] + ); +}); + +test('the guidance block still ends at its own close', () => { + // The nesting fix must not switch the terminator off in the other direction: + // a fact that slipped BELOW the closing run is outside the block, and the + // file-wide search this scoping exists to prevent must not come back. + const escaped = [ + '::::tip Safe to run; never publishes', + ...FACTS.slice(0, -1), + '::::', + '', + FACTS.at(-1), + '', + recipe(), + publishers(), + ].join('\n'); + const v = releaseDocViolations(docs({ contributing: escaped }), PACKAGES); + assert.equal(v.length, 1, v.join('\n')); + assert.match(v[0], /"Safe to run" guidance no longer mentions/); +}); + +test('a second recipe fence must be complete too', () => { + // Picking one fence was a bug in both directions — first-match let an example + // stand in for the recipe, and preferring the one that looked like it invoked + // let a stale second recipe hide behind a fresh one. A contributor copies + // whichever fence they land on, so every fence that runs the dry run is held + // to the full list. + const two = recipe() + '\n\n' + recipe(['bulma-ui']); + const v = releaseDocViolations( + docs({ contributing: doc({ recipe: two }) }), + PACKAGES + ); + assert.equal(v.length, 1, v.join('\n')); + assert.match(v[0], /omits create-bestax, bestax-migrate, bestax-mcp/); +}); + +test('recipeFences reads fences only, and reads all of them', () => { + // The fence is the scope, which is the whole of the shell reading now: a + // prose mention of the command is excluded by construction rather than by + // inspecting command position, and a quoted banner is stripped before the + // fence is even considered a candidate. + const src = [ + 'Run `pnpm exec semantic-release --dry-run` first — prose, not a recipe.', + '', + recipe(['bulma-ui']), + '', + '```bash', + 'echo "pnpm exec semantic-release --dry-run" # a banner, not a run', + '```', + '', + recipe(['bestax-mcp']), + ].join('\n'); + const found = recipeFences(src); + assert.equal(found.length, 2, found.join('\n---\n')); + assert.match(found[0], /for pkg in bulma-ui;/); + assert.match(found[1], /for pkg in bestax-mcp;/); +}); + +test('publisherSections anchors on the bullet and drops fenced lines', () => { + // The three bounds this replaces were each a bug: a continuation line lost at + // EOF, a butted heading sweeping the next section in, a butted fence doing + // the same. Running to the section end has no bounds to get wrong — but + // fenced lines stay excluded, because an example command naming a package + // must not stand in for the list. + const src = [ + '## Why trusted publishing?', + '', + 'Prose naming `bestax-mcp` in passing. This aside owns no list.', + '', + '### npm authentication (OIDC trusted publishing)', + '', + 'An intro naming `create-bestax` above the bullet.', + '', + '- Packages: `@allxsmith/bestax-bulma`,', + ' `bestax-migrate`', + '- Provider: GitHub Actions', + '', + '```bash', + 'npm owner ls bestax-mcp', + '```', + ].join('\n'); + const [section, ...rest] = publisherSections(src); + assert.deepEqual(rest, [], 'a candidate with no bullet contributes nothing'); + assert.match(section, /bestax-migrate/, 'continuation lines survive'); + assert.match(section, /Provider/, 'the rest of the section counts'); + assert.doesNotMatch( + section, + /create-bestax/, + 'prose above the bullet is not part of the list' + ); + assert.doesNotMatch( + section, + /bestax-mcp/, + 'a fenced example must not stand in for the list' + ); +}); + +test('a stray ::: still closes a blockquote-form guidance block', () => { + // The nesting fix must not become a fail-open: with no opener on the marker + // line there is no width to match, and treating that as "nothing can close + // this" ran the block on to the next heading — the file-wide search this + // scoping exists to prevent, arrived at from a new direction. + const strayClose = [ + '> **Safe to run; never publishes:** everything above.', + ...FACTS.slice(0, -1).map(f => `> ${f}`), + ':::', + '', + FACTS.at(-1), + '', + recipe(), + publishers(), + ].join('\n'); + const v = releaseDocViolations(docs({ contributing: strayClose }), PACKAGES); + assert.equal(v.length, 1, v.join('\n')); + assert.match(v[0], /"Safe to run" guidance no longer mentions/); +}); From 82684bdd815bd1ae0b25f9d604da9eceb2f02ce8 Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Thu, 27 Aug 2026 00:27:57 -0400 Subject: [PATCH 02/10] ci: normalize recipe whitespace so a wrapped invocation is still a recipe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Raised in review against #547's deletion of the continuation join, and true of both sides of it: a fence whose command wraps before `--dry-run` — ( cd "$pkg" && pnpm exec semantic-release \ --dry-run --no-ci ) — was invisible to the recipe search, so the page read as having NO recipe at all. That is a false red whose stated remedy is to drop the file from RELEASE_DOC_FILES, i.e. the failure class this rule keeps being rewritten to avoid. Not a regression, which is worth recording because the obvious fix is the wrong one: the continuation join the rule already had left the space that sat BEFORE the backslash in place, so the joined text read `semantic-release --dry-run` with two spaces and the anchor missed it on main too. Restoring the join alone leaves the case broken. Collapsing horizontal runs afterwards is what closes it, and both replacements are now needed — each one deleted on its own fails the new test. The continuation regex also takes `[ \t]*` rather than `\s*`, so it joins one line rather than every blank line that happens to follow. --- scripts/check-conformance.mjs | 26 ++++++++++++++++++++++---- scripts/release-docs-sync.test.mjs | 28 ++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/scripts/check-conformance.mjs b/scripts/check-conformance.mjs index 89d8819ca..df81dea68 100644 --- a/scripts/check-conformance.mjs +++ b/scripts/check-conformance.mjs @@ -1119,15 +1119,33 @@ const MD_HEADING = /^ {0,3}#{1,6}\s/; * word list, the release invocation — is never quoted, so what survives is * what runs. * - * This is all that is left of the shell reading. The command-position and - * loop-segmentation layers that used to sit here were deleted in #547; the - * header above records what that traded. + * Whitespace is then normalised, which is NOT loop parsing and is load-bearing + * for the ANCHOR rather than for any word list. A wrapped invocation — + * `pnpm exec semantic-release \` breaking before `--dry-run` — splits the + * string recipeFences searches for, so the fence stops being a recipe at all + * and the page is reported as having none: a false red whose stated remedy is + * to drop the file from RELEASE_DOC_FILES, i.e. the exact failure this rule + * keeps being rewritten to avoid. + * + * It takes BOTH replacements, which is why the continuation join alone never + * fixed this and the case was broken before #547 as well (found in review). + * Joining leaves the space that sat BEFORE the backslash in place, so the + * result is `semantic-release --dry-run` with two spaces and the anchor still + * misses. Collapsing horizontal runs afterwards is what actually closes it. + * The continuation regex takes `[ \t]*` rather than `\s*` so it joins one + * line, not every blank line that happens to follow. + * + * Beyond that, this is all that is left of the shell reading. The + * command-position and loop-segmentation layers that used to sit here were + * deleted in #547; the header above records what that traded. */ function shellOperative(text) { return text .split('\n') .map(l => l.replace(/"[^"]*"|'[^']*'/g, '""').replace(/#.*$/, '')) - .join('\n'); + .join('\n') + .replace(/\\\n[ \t]*/g, ' ') + .replace(/[ \t]+/g, ' '); } /** diff --git a/scripts/release-docs-sync.test.mjs b/scripts/release-docs-sync.test.mjs index caa6495f0..ba73d509a 100644 --- a/scripts/release-docs-sync.test.mjs +++ b/scripts/release-docs-sync.test.mjs @@ -802,3 +802,31 @@ test('a stray ::: still closes a blockquote-form guidance block', () => { assert.equal(v.length, 1, v.join('\n')); assert.match(v[0], /"Safe to run" guidance no longer mentions/); }); + +test('a wrapped invocation is still found as a recipe', () => { + // A fence whose command wraps before `--dry-run` was invisible to the recipe + // search, so the page read as having NO recipe — a false red whose suggested + // remedy is to drop the file from RELEASE_DOC_FILES. + // + // Pre-existing, not a #547 regression: the continuation join this rule + // already had left the space that sat BEFORE the backslash in place, so the + // joined text read `semantic-release --dry-run` and the anchor missed it on + // main too. Raised in review against the deletion, and true of both sides of + // it; collapsing horizontal runs is what actually fixes it. + const wrapped = [ + '```bash', + 'for pkg in ' + PACKAGES.map(p => p.dir).join(' ') + '; do', + ' ( cd "$pkg" && pnpm exec semantic-release \\', + ' --dry-run --no-ci )', + 'done', + '```', + ].join('\n'); + assert.equal(recipeFences(wrapped).length, 1); + assert.deepEqual( + releaseDocViolations( + docs({ contributing: doc({ recipe: wrapped }) }), + PACKAGES + ), + [] + ); +}); From 6d4c1f12444ce997a257a8a8928564079b7f2b12 Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Thu, 27 Aug 2026 00:38:19 -0400 Subject: [PATCH 03/10] ci: judge every trusted-publisher list on its own, not merged MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The one regression #547's rewrite introduced, found in review. Slicing from the FIRST `- Packages:` anchor to the end of its section merged any later duplicate into the same token set, so a stale list passed on the strength of a complete one below it: - Packages: `@allxsmith/bestax-bulma`, `create-bestax` Superseded by: - Packages: `@allxsmith/bestax-bulma`, `create-bestax`, `bestax-migrate`, `bestax-mcp` Verified against origin/main, which reports the omission: its bounded-bullet scan stopped at the blank line, so the stale list was judged alone. The rewrite's caller still promised that every list is validated, and it no longer was. Each anchor is now bounded by the next one, which is the only bound this needs — continuation lines and the Provider/Repository/Workflow bullets sit before it, so they still count toward their own list. That is stronger than either previous generation: the old scan only ever validated the first bullet in a section, so a stale duplicate BELOW a good list went unchecked. Both directions now have a test. --- scripts/check-conformance.mjs | 72 ++++++++++++++++++------------ scripts/release-docs-sync.test.mjs | 45 +++++++++++++++++++ 2 files changed, 88 insertions(+), 29 deletions(-) diff --git a/scripts/check-conformance.mjs b/scripts/check-conformance.mjs index df81dea68..9a822caff 100644 --- a/scripts/check-conformance.mjs +++ b/scripts/check-conformance.mjs @@ -1246,25 +1246,31 @@ export function recipeFences(src) { } /** - * The `- Packages:` list of every heading section whose TITLE claims trusted + * Every `- Packages:` list in every heading section whose TITLE claims trusted * publishing — the section that owns the packages needing a publisher - * configured on npmjs.com. Each entry runs from the bullet to the end of its - * section, fenced lines dropped. + * configured on npmjs.com. Each entry runs from its bullet to the NEXT + * `- Packages:` bullet, or to the end of the section, fenced lines dropped. * * What #547 changed: the bullet is still ANCHORED by its literal `- Packages:` - * prefix, which is a one-line find and a stable marker in the document, but it - * is no longer SLICED out of the section. The old end-bound scan stopped at the - * next bullet, blank line, fenced line or section end, and three of those four - * bounds had been a bug in their own right — a continuation line dropped at - * EOF, a butted heading sweeping the next section in, a butted fence doing the - * same. Running to the section end instead has no bounds to get wrong, and the - * section end is already computed. + * prefix, which is a one-line find and a stable marker in the document, but the + * end-bound scan around it is gone. That scan stopped at the next bullet, blank + * line, fenced line or section end, and three of those four bounds had been a + * bug in their own right — a continuation line dropped at EOF, a butted heading + * sweeping the next section in, a butted fence doing the same. * - * The trade is that anything below the bullet in the same section now counts: - * on the committed page that is the Provider/Repository/Workflow bullets and - * one paragraph, none of which names a package. Fenced lines are the exception - * and stay excluded — `npm owner ls bestax-mcp` in an example must not stand in - * for the list, which is the one bound worth keeping. + * One bound survives, and it is not optional: the next anchor. Running to the + * section end merged a stale list with the complete one below it and the union + * passed — a fail-open, and the single regression this rewrite introduced, + * caught in review. Bounding each list by the next keeps every one of them + * judged on its own, which is stronger than either previous generation: the + * old scan only ever validated the FIRST bullet in a section, so a stale + * duplicate underneath a good list went unchecked. + * + * The trade is that anything else below a bullet counts toward it: on the + * committed page that is the Provider/Repository/Workflow bullets and one + * paragraph, none of which names a package. Fenced lines are the exception and + * stay excluded — `npm owner ls bestax-mcp` in an example must not stand in for + * the list. * * A candidate section with NO bullet contributes nothing rather than * anchoring-and-failing, so a prose aside titled like this section does not @@ -1292,20 +1298,28 @@ export function publisherSections(src) { break; } } - const start = lines.findIndex( - (l, j) => - j > i && - j < end && - !masked[j] && - l.trimStart().startsWith('- Packages:') - ); - if (start < 0) continue; - sections.push( - lines - .slice(start, end) - .filter((_, j) => !masked[start + j]) - .join('\n') - ); + // EVERY `- Packages:` anchor in the section, each bounded by the NEXT one. + // Running the first anchor to the section end merged a stale list with the + // complete one below it, and the union passed — a fail-open, and the one + // regression this rewrite introduced (found in review). Bounding by the + // next anchor is the only bound this needs: continuation lines and the + // Provider/Repository/Workflow bullets sit before it, so they still count, + // and each list is judged on its own. + const anchors = []; + for (let j = i + 1; j < end; j++) { + if (!masked[j] && lines[j].trimStart().startsWith('- Packages:')) { + anchors.push(j); + } + } + for (const [k, start] of anchors.entries()) { + const stop = anchors[k + 1] ?? end; + sections.push( + lines + .slice(start, stop) + .filter((_, j) => !masked[start + j]) + .join('\n') + ); + } } return sections; } diff --git a/scripts/release-docs-sync.test.mjs b/scripts/release-docs-sync.test.mjs index ba73d509a..df538a5d3 100644 --- a/scripts/release-docs-sync.test.mjs +++ b/scripts/release-docs-sync.test.mjs @@ -830,3 +830,48 @@ test('a wrapped invocation is still found as a recipe', () => { [] ); }); + +test('a duplicate list in one section cannot rescue a stale one', () => { + // Slicing from the FIRST anchor to the section end merged both lists into one + // token set, so a stale list passed on the strength of the complete one below + // it. Fail-open, and the one regression the #547 rewrite introduced — the + // bounded-bullet scan it replaced caught this. Each anchor is now bounded by + // the next, so every list is judged on its own. + const twoLists = [ + '### npm authentication (OIDC trusted publishing)', + '', + '- Packages: `@allxsmith/bestax-bulma`, `create-bestax`', + '', + 'Superseded by:', + '', + `- Packages: ${PACKAGES.map(p => `\`${p.name}\``).join(', ')}`, + ].join('\n'); + const v = releaseDocViolations( + docs({ contributing: doc({ publishers: twoLists }) }), + PACKAGES + ); + assert.equal(v.length, 1, v.join('\n')); + assert.match(v[0], /trusted-publisher list omits/); + assert.match(v[0], /bestax-migrate, bestax-mcp/); +}); + +test('a stale list BELOW a complete one is caught too', () => { + // The bounded scan on main only ever validated the first bullet in a section, + // so a stale duplicate underneath a good list went unchecked. Validating + // every anchor is strictly stronger than either previous generation. + const staleBelow = [ + '### npm authentication (OIDC trusted publishing)', + '', + `- Packages: ${PACKAGES.map(p => `\`${p.name}\``).join(', ')}`, + '', + 'An older copy nobody deleted:', + '', + '- Packages: `@allxsmith/bestax-bulma`, `create-bestax`', + ].join('\n'); + const v = releaseDocViolations( + docs({ contributing: doc({ publishers: staleBelow }) }), + PACKAGES + ); + assert.equal(v.length, 1, v.join('\n')); + assert.match(v[0], /bestax-migrate, bestax-mcp/); +}); From 95318297e0b2fec35fe01fd22063570ba67d6b05 Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Thu, 27 Aug 2026 00:43:32 -0400 Subject: [PATCH 04/10] ci: keep a fence info string out of the recipe's operative text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third review finding, two holes with one cause: the opening ``` delimiter sat inside the operative text in every generation of this check. Under membership that let an info string supply a package the loop omits — ```bash bestax-mcp above a loop over three packages read as naming all four. That one is a regression: the loop-word-list narrowing this rewrite deleted had excluded it, and origin/main reports the omission. It also let an info string carrying `semantic-release --dry-run` conjure a recipe out of a fence with no invocation at all, which was fail-open on main too. Sliced from open + 1 through `close`, not `close - 1`. For a fence left open at EOF, fenceSpans reports the last line of the file as `close` and that line is content — dropping it would trade this hole for the switch-the-check-off false red the docblock has always warned about. That bound turned out to be unguarded: the existing unterminated-fence case ends on a blank line, so slicing it off changed nothing and the test passed either way. Added one whose final line carries a wrapped `--debug`, so both directions of the slice now fail a test when moved. --- scripts/check-conformance.mjs | 10 ++++++- scripts/release-docs-sync.test.mjs | 46 ++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/scripts/check-conformance.mjs b/scripts/check-conformance.mjs index 9a822caff..960587959 100644 --- a/scripts/check-conformance.mjs +++ b/scripts/check-conformance.mjs @@ -1238,9 +1238,17 @@ export function recipeFences(src) { // ````markdown wrapper). An unterminated fence still runs to EOF and is // still returned: dropping it produced the switch-the-check-off violation // this docblock has always warned about. + // From open + 1: the OPENING delimiter is markup, not shell. Its info string + // was inside the operative text in every generation of this check, and under + // membership that let ```bash bestax-mcp count as naming bestax-mcp — the + // loop-word-list narrowing used to exclude it, so this one is a regression + // (found in review). It also let an info string carrying the anchor conjure + // a recipe out of a fence with no invocation at all, which was fail-open on + // main too. Sliced through `close`, not `close - 1`: for an unterminated + // fence `close` is the last line of the file and is real content. return fenceSpans(lines) .map(({ open, close }) => - shellOperative(lines.slice(open, close + 1).join('\n')) + shellOperative(lines.slice(open + 1, close + 1).join('\n')) ) .filter(b => b.includes('semantic-release --dry-run')); } diff --git a/scripts/release-docs-sync.test.mjs b/scripts/release-docs-sync.test.mjs index df538a5d3..b7d6a1cba 100644 --- a/scripts/release-docs-sync.test.mjs +++ b/scripts/release-docs-sync.test.mjs @@ -875,3 +875,49 @@ test('a stale list BELOW a complete one is caught too', () => { assert.equal(v.length, 1, v.join('\n')); assert.match(v[0], /bestax-migrate, bestax-mcp/); }); + +test('a fence info string is markup, not part of the recipe', () => { + // Two holes, one cause: the opening delimiter used to sit inside the + // operative text. Under membership its info string could supply a package + // the loop omits — a regression, since the loop-word-list narrowing excluded + // it — and an info string carrying the anchor could conjure a recipe from a + // fence with no invocation, which was fail-open on main too. + const infoStringNames = [ + '```bash bestax-mcp', + 'for pkg in bulma-ui create-bestax bestax-migrate; do', + ' ( cd "$pkg" && pnpm exec semantic-release --dry-run --no-ci )', + 'done', + '```', + ].join('\n'); + const v = releaseDocViolations( + docs({ contributing: doc({ recipe: infoStringNames }) }), + PACKAGES + ); + assert.equal(v.length, 1, v.join('\n')); + assert.match(v[0], /omits bestax-mcp/); + + // And an info string alone is not an invocation. + const infoStringOnly = [ + '```bash semantic-release --dry-run', + 'pnpm all', + '```', + ].join('\n'); + assert.equal(recipeFences(infoStringOnly).length, 0); +}); + +test('an unterminated fence keeps its final line, which is real content', () => { + // The companion to dropping the OPENING delimiter: the closing one may not be + // dropped with it. For a fence left open at EOF, fenceSpans reports the last + // line of the file as `close`, and that line is content. The existing + // unterminated-fence case could not see this — its final line is blank, so + // slicing it off changed nothing and the bound went unguarded. + const src = [ + '```bash', + 'pnpm exec semantic-release --dry-run --no-ci \\', + ' --debug # wraps onto the last line of the file', + ].join('\n'); + const [fence, ...rest] = recipeFences(src); + assert.ok(fence, 'an unterminated fence is still a recipe'); + assert.deepEqual(rest, []); + assert.match(fence, /--debug/, 'the final line is content, not a delimiter'); +}); From 89cc1461cb77b151380a51852c650afc2f44bd6a Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Thu, 27 Aug 2026 07:30:52 -0400 Subject: [PATCH 05/10] ci: ignore HTML-commented content in the release-doc extractors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit finding, and a real fail-open: a complete recipe fence or `- Packages:` list inside `` still satisfied its assertion. Since commented content renders as nothing, someone commenting a section out rather than deleting it removed the guidance a contributor actually sees while every gate stayed green. Pre-existing rather than a #547 regression — verified against origin/main, which counts commented content in all three extractors. The review named two sites; the third, safeToRunBlock, has the same defect and is fixed with them: a fully commented-out guidance block passed with zero violations, reading the facts out of text nobody can see. commentMask sits beside fenceMask and handles the interaction in both directions, which is where this is easy to get wrong: - a `` still satisfied its assertion. Someone + * commenting out a section rather than deleting it therefore removed the + * guidance a contributor actually sees while the check stayed green — the + * fail-open shape this rule exists to prevent, arrived at by editing rather + * than by drift. + * + * Fence interaction, in both directions. A `/g)) { + open = m[0] === '` renders as nothing, so counting it let the visible recipe be + // removed with the check still green (pre-#547, in every generation). + const { commented } = hiddenLines(lines); return fenceSpans(lines) + .filter(({ open }) => !commented[open]) .map(({ open, close }) => shellOperative(lines.slice(open + 1, close + 1).join('\n')) ) @@ -1257,7 +1313,8 @@ export function recipeFences(src) { * Every `- Packages:` list in every heading section whose TITLE claims trusted * publishing — the section that owns the packages needing a publisher * configured on npmjs.com. Each entry runs from its bullet to the NEXT - * `- Packages:` bullet, or to the end of the section, fenced lines dropped. + * `- Packages:` bullet, or to the end of the section, hidden lines dropped — + * fenced examples and HTML-commented content alike. * * What #547 changed: the bullet is still ANCHORED by its literal `- Packages:` * prefix, which is a one-line find and a stable marker in the document, but the @@ -1294,7 +1351,10 @@ export function recipeFences(src) { */ export function publisherSections(src) { const { lines } = splitLines(src); - const masked = fenceMask(lines); + const { fenced, commented } = hiddenLines(lines); + // Hidden either way: a commented-out heading does not anchor a section, a + // commented-out bullet is not a list, and neither counts toward one. + const masked = lines.map((_, i) => fenced[i] || commented[i]); const isHeading = i => !masked[i] && MD_HEADING.test(lines[i]); const sections = []; for (let i = 0; i < lines.length; i++) { diff --git a/scripts/release-docs-sync.test.mjs b/scripts/release-docs-sync.test.mjs index b7d6a1cba..8a52c8651 100644 --- a/scripts/release-docs-sync.test.mjs +++ b/scripts/release-docs-sync.test.mjs @@ -921,3 +921,80 @@ test('an unterminated fence keeps its final line, which is real content', () => assert.deepEqual(rest, []); assert.match(fence, /--debug/, 'the final line is content, not a delimiter'); }); + +// --- HTML comments are not rendered, so they are not evidence (CodeRabbit) ---- + +test('a commented-out recipe does not satisfy the recipe check', () => { + // Commenting a section out rather than deleting it removed what a contributor + // actually sees while every assertion stayed green. Pre-#547 — main counted + // commented content in all three extractors. + const commented = [''].join('\n'); + const v = releaseDocViolations( + docs({ contributing: doc({ recipe: commented }) }), + PACKAGES + ); + assert.equal(v.length, 1, v.join('\n')); + assert.match(v[0], /no fenced block running/); +}); + +test('a commented-out trusted-publisher list does not satisfy its check', () => { + const commented = [ + '### npm authentication (OIDC trusted publishing)', + '', + 'The list used to be here.', + '', + '', + ].join('\n'); + const v = releaseDocViolations( + docs({ contributing: doc({ publishers: commented }) }), + PACKAGES + ); + assert.equal(v.length, 1, v.join('\n')); + assert.match(v[0], /no "- Packages:" line/); +}); + +test('a commented-out guidance block does not satisfy the facts check', () => { + // The third site, which the review did not name but shares the defect: the + // facts were being read out of text nobody can see. + const commented = [ + '', + '', + recipe(), + publishers(), + ].join('\n'); + const v = releaseDocViolations(docs({ contributing: commented }), PACKAGES); + assert.equal(v.length, 1, v.join('\n')); + assert.match(v[0], /no "Safe to run; never publishes" guidance/); +}); + +test('an HTML comment inside a fence is literal text, not a comment', () => { + // Both directions of the fence/comment interaction. A `` opens and closes on its own line, so the next line is visible. + const withNote = ['', '', doc()].join('\n'); + assert.deepEqual( + releaseDocViolations(docs({ contributing: withNote }), PACKAGES), + [] + ); +}); From 77facb46ad297f953dd4db2b8a0c058ca4d33322 Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Thu, 27 Aug 2026 07:49:00 -0400 Subject: [PATCH 06/10] ci: recognize the alternate HTML comment close --!> in the comment mask MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeQL flagged the mask added in the previous commit (js/bad-tag-filter, high): it recognized only --> as a comment close. Browsers honor the parse-error form --!> per the HTML spec, so a comment closed that way stayed open in the mask and hid the entire VISIBLE document below it — reproduced: every assertion false-reds at once, five violations against a document whose rendered content is complete, and the violation text's own remedy is to drop the file from RELEASE_DOC_FILES. The repro sanitizer already breaks --!> for the same reason; this brings the mask in line with that precedent. Both directions tested: a --!>-closed aside above the docs is clean, and content sitting between a --!> close and a later comment still counts. Reverting the regex to -->-only fails the new test. --- scripts/check-conformance.mjs | 10 +++++++++- scripts/release-docs-sync.test.mjs | 27 +++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/scripts/check-conformance.mjs b/scripts/check-conformance.mjs index d501d076f..f0e99a146 100644 --- a/scripts/check-conformance.mjs +++ b/scripts/check-conformance.mjs @@ -1135,6 +1135,14 @@ const MD_HEADING = /^ {0,3}#{1,6}\s/; * with a comment opener reads as commented, which over-masks the visible half; * neither release doc contains an HTML comment at all today, so the precise * split would be machinery for a case that does not occur. + * + * Both close forms end a comment: `-->` and the parse-error form `--!>`, + * which browsers honor per the HTML spec. Recognizing only `-->` left a + * `--!>`-closed comment open in the mask, hiding the entire VISIBLE document + * below it — every assertion then false-reds at once, and the violation text's + * own remedy is to drop the file from RELEASE_DOC_FILES. CodeQL flags exactly + * this (js/bad-tag-filter), and the repro sanitizer already breaks `--!>` for + * the same reason. */ function commentMask(lines, fenced) { const mask = new Array(lines.length).fill(false); @@ -1145,7 +1153,7 @@ function commentMask(lines, fenced) { continue; } mask[i] = open || lines[i].includes('/g)) { + for (const m of lines[i].matchAll(/ ends a comment too', () => { + // Browsers honor `--!>` as a comment close per the HTML spec (a parse + // error, but effective), so everything after it is RENDERED. Recognizing + // only `-->` left the mask open, hiding the entire visible document — every + // assertion false-reds at once, and the violation's own remedy is to drop + // the file from RELEASE_DOC_FILES. CodeQL js/bad-tag-filter; the repro + // sanitizer breaks `--!>` for the same reason. + const alternateClose = [ + '', + '', + doc(), + ].join('\n'); + assert.deepEqual( + releaseDocViolations(docs({ contributing: alternateClose }), PACKAGES), + [] + ); + // And the hiding direction still holds: content only visible because of + // `--!>` is content — a recipe between `--!>` and `-->` counts. + const between = ['', doc(), ''].join( + '\n' + ); + assert.deepEqual( + releaseDocViolations(docs({ contributing: between }), PACKAGES), + [] + ); +}); From d65c8a3a38884660a136b9422d4a8ff7038d7799 Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Thu, 27 Aug 2026 08:40:50 -0400 Subject: [PATCH 07/10] ci: read the release docs as visible text, not through a boolean mask MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A /code-review max pass over this branch returned fifteen findings, nine of them verified failures of the comment mask added two commits ago. They share one root cause: a per-line boolean cannot represent "part of this line is rendered". Masking a whole line hid an anchor annotated with a trailing (false red on exactly the lines likeliest to carry such a note — the check's own anchors), while the guidance block, returned as raw lines, still counted a commented-out fact as evidence (the fail-open the mask was built to close, alive inside the construct it guards). docStructure replaces the mask: one interleaved pass over the source yielding per-line VISIBLE text (comment content excised), fence flags, and comment-aware fence spans. The extractors search, terminate, and return visible text; fenced lines keep their raw text inside the guidance block and contribute nothing anywhere else. What the single pass fixes beyond granularity: - Fences and comments gate each other in BOTH directions. Layering comment state over a comment-blind fenceMask let a commented-out ``` fragment open a phantom fence that swallowed the comment's own close, masking the document to EOF. Inside a fence comments cannot open; inside a comment fences cannot open. - and are complete empty comments (CommonMark 0.31 abrupt closes). Matching red-flagged every assertion in the file. - {/* … */} is the docs mirror's native comment syntax (the site is MDX-compiled), so HTML-only masking left the commented-out-content fail-open fully open on one of the two release docs. - A comment opener inside a code span — ` (browsers close there, remark-comment does not) stays modeled as a close, per CodeQL and the repro sanitizer's precedent. Both residuals need comment constructs no release doc contains. Nine regression tests added; each deliberate branch of docStructure and both reverted terminator behaviors fail at least one test when mutated. Three test comments describing machinery #547 deleted (invoke-preferring selection, packagesBullet, loop word lists) now describe the guards that remain. --- scripts/check-conformance.mjs | 322 ++++++++++++++++++----------- scripts/release-docs-sync.test.mjs | 172 ++++++++++++++- 2 files changed, 360 insertions(+), 134 deletions(-) diff --git a/scripts/check-conformance.mjs b/scripts/check-conformance.mjs index f0e99a146..38009217d 100644 --- a/scripts/check-conformance.mjs +++ b/scripts/check-conformance.mjs @@ -73,7 +73,6 @@ import { } from './lib/pack-time-protocols.mjs'; import { fenceMask, - fenceSpans, readRegions, sectionSpans, splitLines, @@ -1112,59 +1111,136 @@ const RELEASE_DOC_FACTS = [ const MD_HEADING = /^ {0,3}#{1,6}\s/; /** - * Boolean per line: true when the line is inside an HTML comment, i.e. NOT - * rendered. Every release-doc extractor consults this alongside fenceMask. + * The reader's view of a markdown source, in one interleaved pass: * - * The hole it closes, which predates #547 and lived in all three extractors - * (CodeRabbit named two of them): a complete recipe fence or `- Packages:` - * list commented out with `` still satisfied its assertion. Someone - * commenting out a section rather than deleting it therefore removed the - * guidance a contributor actually sees while the check stayed green — the - * fail-open shape this rule exists to prevent, arrived at by editing rather - * than by drift. + * fenced[i] — line i is part of a fenced code block (delimiters included) + * visible[i] — line i's RENDERED text: comment content excised, '' for + * fenced lines (their content is reachable via `lines` and the + * spans; it is example text, not prose) + * spans — the fenced blocks, as [{ open, close }] line-index pairs, + * delimiters included, an unterminated fence running to EOF * - * Fence interaction, in both directions. A `` (false red on the + * likeliest lines to receive such a note — the check's own anchors), while + * leaving a commented-out fact INSIDE the guidance block satisfying + * `includes()` (fail-open). Visible text gets both right for free: the + * annotation vanishes, the anchor stays; the commented fact vanishes, the + * assertion bites. * - * Both close forms end a comment: `-->` and the parse-error form `--!>`, - * which browsers honor per the HTML spec. Recognizing only `-->` left a - * `--!>`-closed comment open in the mask, hiding the entire VISIBLE document - * below it — every assertion then false-reds at once, and the violation text's - * own remedy is to drop the file from RELEASE_DOC_FILES. CodeQL flags exactly - * this (js/bad-tag-filter), and the repro sanitizer already breaks `--!>` for - * the same reason. + * The comment grammar, and why each production is here: + * + * `` and `` complete empty comments (CommonMark 0.31's abrupt + * closes). Matching `` red-flagged every assertion. + * `` / `--!>` the HTML comment; both close forms end it. `--!>` + * is the parse-error close browsers honor (CodeQL + * js/bad-tag-filter; the repro sanitizer breaks it + * for the same reason). + * `{/* … *\/}` the MDX comment (its close is written with a + * backslash here only because the raw sequence would + * end this docblock). The docs mirror is MDX-compiled + * — docusaurus.config.js leaves markdown.format as + * mdx — so MDX comments hide rendered content there. + * Without this production, the commented-out-content + * fail-open this file closes for HTML comments + * stayed fully open on one of the two release docs + * via its native comment syntax. In the + * GitHub-rendered file `{/*` is literal, but only if + * someone writes it in prose, which no release doc + * does. + * + * Inline code spans are protected: `` `` (browsers close there, remark-comment does + * not), so on the MDX mirror text between a `--!>` and a later `-->` counts + * as visible while the published page hides it — modeling per-file comment + * grammars buys that corner and nothing else. A code span AFTER a same-line + * comment close is scanned unstripped. Both need comment constructs no + * release doc contains. */ -function commentMask(lines, fenced) { - const mask = new Array(lines.length).fill(false); - let open = false; +const COMMENT_TOKEN = /||' || t === '') { + out += line.slice(from, m.index); + from = m.index + t.length; + } + // A stray close with nothing open is literal text. + } else if ( + comment === 'html' ? t === '-->' || t === '--!>' : t === '*/}' + ) { + comment = null; + from = m.index + t.length; + } } + if (!comment) out += line.slice(from); + visible[i] = out; } - return mask; -} - -/** Lines hidden from a reader: fenced-example content OR commented out. */ -function hiddenLines(lines) { - const fenced = fenceMask(lines); - const commented = commentMask(lines, fenced); - return { fenced, commented }; + if (fence) spans.push({ open: fence.open, close: lines.length - 1 }); + return { fenced, visible, spans }; } /** @@ -1191,6 +1267,16 @@ function hiddenLines(lines) { * The continuation regex takes `[ \t]*` rather than `\s*` so it joins one * line, not every blank line that happens to follow. * + * ORDER: comments per line, then the join, then quotes, then whitespace. + * Quote-stripping before the join was a fail-open the other way (#547 review, + * round 5): a double-quoted banner WRAPPED across a continuation has no quote + * pair on either physical line, so nothing stripped, and the join then + * manufactured the anchor inside pure echo text — a fence that runs nothing + * became "the recipe" and red-flagged every package. Joining first restores + * the pair, and the banner strips like any other quoted prose. Comments stay + * per-line and FIRST, so a `\` at the end of a commented line cannot join the + * next line into the comment. + * * Beyond that, this is all that is left of the shell reading. The * command-position and loop-segmentation layers that used to sit here were * deleted in #547; the header above records what that traded. @@ -1198,58 +1284,47 @@ function hiddenLines(lines) { function shellOperative(text) { return text .split('\n') - .map(l => l.replace(/"[^"]*"|'[^']*'/g, '""').replace(/#.*$/, '')) + .map(l => l.replace(/#.*$/, '')) .join('\n') .replace(/\\\n[ \t]*/g, ' ') + .replace(/"[^"\n]*"|'[^'\n]*'/g, '""') .replace(/[ \t]+/g, ' '); } -/** - * A Docusaurus admonition delimiter, or null. `:::tip Safe to run` OPENS, - * a bare `:::` CLOSES, and the run length matters — nesting works by widening - * the outer run (`::::tip` around `:::note`), exactly as CommonMark fences - * nest by widening the backtick run. - */ -function admonitionDelim(line) { - const m = line.match(/^ {0,3}(:{3,})(.*)$/); - if (!m) return null; - return { len: m[1].length, open: Boolean(m[2].trim()) }; -} - function safeToRunBlock(src) { const { lines } = splitLines(src); - const { fenced: masked, commented } = hiddenLines(lines); - // Hidden on the START too, not only on the terminator scan: a fenced example - // or a commented-out copy containing this marker would otherwise BE the - // block, and the real guidance could then be deleted with the check still - // green. - const start = lines.findIndex( - (l, i) => - !masked[i] && !commented[i] && l.includes('Safe to run; never publishes') + const { fenced, visible } = docStructure(lines); + // VISIBLE text throughout — the search, the terminators, and the returned + // block alike (#547 review, round 5). Searching raw lines let a fenced + // example or a commented-out copy of the marker BE the block; returning raw + // lines let a fact whose only occurrence was commented out still satisfy + // `includes()`, which is the commented-out-not-deleted fail-open this file + // closes elsewhere; and terminating on raw lines let a heading inside an + // HTML comment — a parked draft, invisible to every reader — end the block + // above most of the facts. Fenced lines are the one exception: their raw + // text stays in the returned block (an example inside the guidance is part + // of the guidance), and they never terminate it. + const text = i => (fenced[i] ? lines[i] : visible[i]); + const start = visible.findIndex(l => + l.includes('Safe to run; never publishes') ); if (start < 0) return null; // The run that opened the guidance, when the marker line IS the opener (the - // docs page). Only a close at least that wide ends the block; anything - // narrower is a nested admonition closing itself. - // - // Falling back to 3 rather than to Infinity, deliberately: Infinity reads - // better — "no opener, so no close can end this" — and is the fail-open - // direction, because the blockquote form would then run past a stray `:::` - // to the next heading and restore the file-wide search this scoping exists - // to prevent. 3 is what this line did before nesting was considered at all. - // The residual, recorded rather than chased: if the marker is moved OFF its - // `:::tip` line into the body, the opener's width is no longer visible here - // and a nested close truncates the block again. Finding the enclosing opener - // by scanning backwards would fix that shape, and is the kind of refinement - // the header above argues against buying. - const opened = admonitionDelim(lines[start]); - const outer = opened?.open ? opened.len : 3; + // docs page): only a bare close at least that wide ends the block; anything + // narrower is a nested admonition closing itself, since Docusaurus nests by + // widening the OUTER run. In the blockquote form there is no opener, and the + // terminator is exactly `:::` — a wider bare run is stray junk, not a close, + // which is what this scan did before nesting was considered (a `::::` line + // ending the blockquote block was an undocumented behavior flip the round-5 + // review caught). Closes match on trimmed text, so an indented `:::` still + // terminates, as it always had. + const outer = visible[start].match(/^ {0,3}(:{3,}).*\S/)?.[1].length ?? null; let end = lines.length; for (let i = start + 1; i < lines.length; i++) { - if (masked[i]) continue; // a terminator inside a fenced example is content - const l = lines[i]; - const delim = admonitionDelim(l); - if (delim && !delim.open && delim.len >= outer) { + if (fenced[i]) continue; // a terminator inside a fenced example is content + const l = visible[i]; + const close = l.trim().match(/^(:{3,})$/)?.[1].length ?? 0; + if (close && (outer === null ? close === 3 : close >= outer)) { end = i; break; } @@ -1258,7 +1333,10 @@ function safeToRunBlock(src) { break; } } - return lines.slice(start, end).join('\n'); + return lines + .slice(start, end) + .map((_, k) => text(start + k)) + .join('\n'); } /** @@ -1278,24 +1356,30 @@ function safeToRunBlock(src) { * runs the dry run is a recipe a contributor might copy, so every one of them * has to be complete. * - * Fences come from `fenceSpans` rather than a /```[\s\S]*?```/g match, which - * pairs backtick runs in document order: one stray ``` in prose, or a - * ````markdown wrapper around a nested fence, shifts every pair after it and the - * real recipe stops being found. The remedy the violation then offers is "drop - * this file from RELEASE_DOC_FILES", i.e. switch the check off, which is the - * worst way to be wrong. An UNTERMINATED fence runs to EOF and is still - * returned, for the same reason — dropping it produced that same false - * violation. + * Fences come from `docStructure`'s state machine rather than a + * /```[\s\S]*?```/g match, which pairs backtick runs in document order: one + * stray ``` in prose, or a ````markdown wrapper around a nested fence, shifts + * every pair after it and the real recipe stops being found. The remedy the + * violation then offers is "drop this file from RELEASE_DOC_FILES", i.e. + * switch the check off, which is the worst way to be wrong. An UNTERMINATED + * fence runs to EOF and is still returned, for the same reason — dropping it + * produced that same false violation. The spans are comment-aware by + * construction, so a complete recipe inside `` — which renders as + * nothing — is not a fence at all, and cannot cover for the visible recipe + * being removed (pre-#547, in every generation). + * + * What is knowingly NOT found (#547 review, round 5, declined): a recipe + * driving the invocation through a variable — `CMD="… --dry-run"; $CMD` — + * since quote-stripping deletes the anchor before the filter sees it. The + * previous generation's raw-containment fallback covered that, but the same + * fallback is what let a quoted echo banner count as a recipe, and membership + * alone cannot tell those two quoted anchors apart; telling them apart is + * command-position parsing, which is the layer #547 deleted. Both committed + * recipes invoke directly, and a variable-form rewrite that reds CI names its + * own remedy in the violation text. */ export function recipeFences(src) { const { lines } = splitLines(src); - // Real block spans from the fence state machine — never re-derived from - // the boolean mask, where butted fences form one run and a content line - // shaped like a delimiter split a real block mid-fence (#548 review; the - // seam heuristic this replaces misfired on ``` lines shown inside a ~~~ or - // ````markdown wrapper). An unterminated fence still runs to EOF and is - // still returned: dropping it produced the switch-the-check-off violation - // this docblock has always warned about. // From open + 1: the OPENING delimiter is markup, not shell. Its info string // was inside the operative text in every generation of this check, and under // membership that let ```bash bestax-mcp count as naming bestax-mcp — the @@ -1304,14 +1388,8 @@ export function recipeFences(src) { // a recipe out of a fence with no invocation at all, which was fail-open on // main too. Sliced through `close`, not `close - 1`: for an unterminated // fence `close` is the last line of the file and is real content. - // - // Commented-out fences are dropped entirely: a complete recipe inside - // `` renders as nothing, so counting it let the visible recipe be - // removed with the check still green (pre-#547, in every generation). - const { commented } = hiddenLines(lines); - return fenceSpans(lines) - .filter(({ open }) => !commented[open]) - .map(({ open, close }) => + return docStructure(lines) + .spans.map(({ open, close }) => shellOperative(lines.slice(open + 1, close + 1).join('\n')) ) .filter(b => b.includes('semantic-release --dry-run')); @@ -1321,8 +1399,8 @@ export function recipeFences(src) { * Every `- Packages:` list in every heading section whose TITLE claims trusted * publishing — the section that owns the packages needing a publisher * configured on npmjs.com. Each entry runs from its bullet to the NEXT - * `- Packages:` bullet, or to the end of the section, hidden lines dropped — - * fenced examples and HTML-commented content alike. + * `- Packages:` bullet, or to the end of the section, in VISIBLE text — + * fenced examples contribute nothing and commented content is excised. * * What #547 changed: the bullet is still ANCHORED by its literal `- Packages:` * prefix, which is a one-line find and a stable marker in the document, but the @@ -1359,14 +1437,17 @@ export function recipeFences(src) { */ export function publisherSections(src) { const { lines } = splitLines(src); - const { fenced, commented } = hiddenLines(lines); - // Hidden either way: a commented-out heading does not anchor a section, a - // commented-out bullet is not a list, and neither counts toward one. - const masked = lines.map((_, i) => fenced[i] || commented[i]); - const isHeading = i => !masked[i] && MD_HEADING.test(lines[i]); + const { visible } = docStructure(lines); + // VISIBLE text throughout (#547 review, round 5): a commented-out heading + // does not anchor a section, a commented-out bullet is not a list, and a + // fenced line's visible text is '' — while a bullet ANNOTATED with a + // trailing `` keeps its anchor, since only the comment + // is excised. Boolean masking got the last one wrong on exactly the lines + // likeliest to carry such a note: the check's own anchors. + const isHeading = i => MD_HEADING.test(visible[i]); const sections = []; for (let i = 0; i < lines.length; i++) { - if (!(isHeading(i) && /trusted[- ]publish/i.test(lines[i]))) continue; + if (!(isHeading(i) && /trusted[- ]publish/i.test(visible[i]))) continue; let end = lines.length; for (let j = i + 1; j < lines.length; j++) { if (isHeading(j)) { @@ -1383,18 +1464,13 @@ export function publisherSections(src) { // and each list is judged on its own. const anchors = []; for (let j = i + 1; j < end; j++) { - if (!masked[j] && lines[j].trimStart().startsWith('- Packages:')) { + if (visible[j].trimStart().startsWith('- Packages:')) { anchors.push(j); } } for (const [k, start] of anchors.entries()) { const stop = anchors[k + 1] ?? end; - sections.push( - lines - .slice(start, stop) - .filter((_, j) => !masked[start + j]) - .join('\n') - ); + sections.push(visible.slice(start, stop).join('\n')); } } return sections; diff --git a/scripts/release-docs-sync.test.mjs b/scripts/release-docs-sync.test.mjs index 54c05752f..f2acd5370 100644 --- a/scripts/release-docs-sync.test.mjs +++ b/scripts/release-docs-sync.test.mjs @@ -282,8 +282,9 @@ test('a package named only in a comment does not satisfy the recipe', () => { }); test('a package named only in an echo does not satisfy the recipe either', () => { - // Stripping comments closed one hole and left the one beside it: only the - // loop's own word list should count. + // Stripping comments closed one hole and left the one beside it. Since #547 + // the guard is quote-stripping (shellOperative): the banner's names live in + // a quoted string, and quoted text is not operative shell. const echoed = '```bash\n' + 'echo "bestax-migrate bestax-mcp are released separately"\n' + @@ -412,10 +413,11 @@ test('a heading butted against the bullet does not extend the list', () => { }); test('a missing OIDC section is not rescued by a bullet elsewhere', () => { - // packagesBullet fell back to line 0 when it found no "trusted publisher" - // anchor, which is the file-wide search it exists to prevent: an unrelated - // "- Packages:" bullet then satisfied the assertion with the real guidance - // deleted. + // An early extractor generation fell back to line 0 when it found no + // "trusted publisher" anchor, which is the file-wide search it exists to + // prevent: an unrelated "- Packages:" bullet then satisfied the assertion + // with the real guidance deleted. publisherSections contributes nothing + // without an anchoring heading, so the caller's no-section message fires. const noOidcSection = [ '- Packages: `@allxsmith/bestax-bulma`, `create-bestax`, `bestax-migrate` and `bestax-mcp`', '', @@ -600,8 +602,11 @@ test('an array manifest is unreadable, not silently absent', async () => { // --- #548-review regressions -------------------------------------------------- test('an example fence spelling out the dry-run command does not become the recipe', () => { - // Selection prefers the block that INVOKES the command; the example's - // echoed (quoted) mention is stripped from its operative text. + // The example's echoed mention is quoted, and quoted text is stripped from + // the operative text before the anchor filter — so the example is not a + // candidate at all and only the real recipe is held to the package list. + // (The invoke-preferring selection that used to make this call was deleted + // in #547; quote-stripping is the guard that remains.) const example = '```text\nfor pkg in bulma-ui create-bestax bestax-migrate bestax-mcp; do\n' + ' echo "pnpm exec semantic-release --dry-run --no-ci in $pkg"\ndone\n```'; @@ -616,9 +621,9 @@ test('an example fence spelling out the dry-run command does not become the reci }); test('fence-shaped content inside a ~~~ block does not split it', () => { - // Structure comes from fenceSpans, not from re-guessing seams in the mask: - // a ``` line shown INSIDE a ~~~ fence is content, and splitting there left - // the fragment loop-less. + // Structure comes from the fence state machine (docStructure), not from + // re-guessing seams in a boolean mask: a ``` line shown INSIDE a ~~~ fence + // is content, and splitting there left the fragment loop-less. const block = [ '~~~bash', 'cat < ends a comment too', () => { [] ); }); + +// --- round 5 (/code-review max): visible text replaces the boolean mask ------ + +test('a fact commented out inside the guidance block is caught', () => { + // The boolean mask closed the commented-out class for the block's START and + // left the block's CONTENT raw, so a fact whose only occurrence sat inside + // still satisfied includes() — the exact fail-open the mask was + // built to close, alive inside the construct it guards. The block is now + // visible text, so hidden facts are not evidence. + const hiddenFact = doc().replace( + '> see VERSIONING.md', + '' + ); + const v = releaseDocViolations(docs({ contributing: hiddenFact }), PACKAGES); + assert.equal(v.length, 1, v.join('\n')); + assert.match(v[0], /no longer mentions "VERSIONING\.md"/); +}); + +test('an abrupt empty comment is complete, not an opener', () => { + // and are complete empty comments (CommonMark 0.31 abrupt + // closes). Matching `` masked the whole + // visible document — every assertion red at once. + for (const empty of ['', '']) { + assert.deepEqual( + releaseDocViolations( + docs({ contributing: [empty, '', doc()].join('\n') }), + PACKAGES + ), + [], + empty + ); + } +}); + +test('a comment opener inside a code span is literal', () => { + // Prose documenting the repo's own marker syntax — ` between two fact lines — is + // invisible to every reader, but the terminator scan read it raw and ended + // the block above the facts below it: five false "no longer mentions" reds. + const parked = doc().replace( + '> see VERSIONING.md', + ['', '> see VERSIONING.md'].join('\n') + ); + assert.deepEqual( + releaseDocViolations(docs({ contributing: parked }), PACKAGES), + [] + ); +}); + +test('a fence delimiter inside a comment is comment text', () => { + // Layering comment state over a comment-blind fenceMask let a commented-out + // ``` fragment open a phantom fence that captured the comment's own close — + // the mask then stayed open to EOF and every assertion false-redded. One + // interleaved pass: inside a comment, fences cannot open. + const fragment = ['', '', doc()].join( + '\n' + ); + assert.deepEqual( + releaseDocViolations(docs({ contributing: fragment }), PACKAGES), + [] + ); +}); + +test('an inline comment annotating an anchor line does not hide it', () => { + // The likeliest lines to receive a trailing note are + // the check's own anchors, and boolean whole-line masking made exactly those + // anchors vanish. Excision removes the note and keeps the anchor. + const annotated = doc() + .replace( + '> **Safe to run; never publishes:** everything above.', + '> **Safe to run; never publishes:** everything above. ' + ) + .replace(/(- Packages: .*)$/m, '$1 '); + assert.deepEqual( + releaseDocViolations(docs({ contributing: annotated }), PACKAGES), + [] + ); +}); + +test('a wrapped quoted banner is not a recipe', () => { + // Quote-stripping ran per line BEFORE the continuation join, so a banner + // whose quoted text wrapped across a continuation had no pair on either + // physical line — nothing stripped — and the join then manufactured the + // anchor inside pure echo text: a fence that runs nothing red-flagged every + // package. Joining first restores the pair. + const banner = + '```bash\necho "preview with: pnpm exec semantic-release \\\n' + + ' --dry-run (no publish happens)"\n```'; + const both = doc().replace(recipe(), recipe() + '\n\n' + banner); + assert.deepEqual( + releaseDocViolations(docs({ contributing: both }), PACKAGES), + [] + ); +}); + +test('a stray :::: does not end a blockquote-form guidance block', () => { + // The width-aware close matched any bare run >= 3 in the blockquote form, + // where the terminator had always been exactly `:::` — a stray wider run + // ended the block early (false red). An INDENTED ::: still terminates, as + // it always had: closes match on trimmed text. + const strayWide = doc().replace( + '> see VERSIONING.md', + '::::\n> see VERSIONING.md' + ); + const v = releaseDocViolations(docs({ contributing: strayWide }), PACKAGES); + assert.deepEqual(v, [], v.join('\n')); + const indented = doc().replace( + '> see VERSIONING.md', + ' :::\n> see VERSIONING.md' + ); + const v2 = releaseDocViolations(docs({ contributing: indented }), PACKAGES); + assert.ok(v2.length >= 1, 'an indented ::: must still terminate the block'); +}); From 66ad7c3c3666511476fefb5477e0210327b668ef Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Thu, 27 Aug 2026 08:52:01 -0400 Subject: [PATCH 08/10] ci: strip shell comments quote-aware, blank multi-backtick code spans MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 6, two review findings against round 5's own fixes, both verified false reds before fixing. The comment strip moved ahead of the quote strip in round 5 (so a trailing backslash on a commented line cannot join code into the comment), and that order made a bare /#.*$/ take the hash inside `echo "Step #1"` as a comment opener — deleting the invocation after it and reporting the page as having no recipe. The strip is quote-aware now: the alternation consumes paired quoted strings intact, leaving them for the quote pass after the continuation join, and removes only a `#` that sits outside them. The residual is a hash inside a string that a continuation wraps, which needs all three at once and stays line-local by design. The code-span blanking knew only the single-tick form, and CommonMark pairs an N-backtick run with a closing run of exactly N — so the double-tick span in "Use ``` and a later `-->` counts * as visible while the published page hides it — modeling per-file comment * grammars buys that corner and nothing else. A code span AFTER a same-line - * comment close is scanned unstripped. Both need comment constructs no - * release doc contains. + * comment close is scanned unstripped. And the span protection is LINE-LOCAL + * even though CommonMark spans may wrap: a span whose first line carries the + * opener plus `||