diff --git a/CLAUDE.md b/CLAUDE.md index ed482744a..4627c5602 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -67,10 +67,13 @@ Conventional Commits, enforced by commitlint (husky `commit-msg` hook) and consu semantic-release. Two repo-specific rules: - Commits of type `feat|fix|perf|refactor|style|revert` **must** use a scope of `bulma-ui`, `docs`, - `create-bestax`, `bestax-migrate`, or `bestax-mcp` — unscoped release types are rejected + `create-bestax`, `bestax-migrate`, or `bestax-mcp` — an unscoped commit of any of these + scope-gated types is rejected (`RELEASE_SCOPES` in `commitlint.config.js` is the source of truth). One exception worth knowing: commitlint's default ignores skip git's own `Revert "…"` messages, so the hook cannot - reject an unscoped revert in that form — keep reverts conventional and scoped by hand. + reject an unscoped revert in that form — keep reverts conventional and scoped by hand, and + know that a scoped revert releases nothing: ship a rollback as `fix()` (see + VERSIONING.md for why). - **Packages release independently, keyed off the scope**: `feat(bulma-ui)` bumps only `@allxsmith/bestax-bulma`; `fix(create-bestax)` bumps only `create-bestax`. The `releaseRules` in each package's `release.config.js` are the source of truth. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9e6867be8..93e153410 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -371,13 +371,17 @@ commitlint via the husky `commit-msg` hook ([`commitlint.config.js`](./commitlin - **Release types need a scope:** commits of type `feat`, `fix`, `perf`, `refactor`, `style` or `revert` **must** use a scope of `bulma-ui`, `docs`, `create-bestax`, `bestax-migrate` or `bestax-mcp` (repo-specific commitlint rule — the scope decides which package releases, - see [`VERSIONING.md`](./VERSIONING.md)). `revert` is in that list because - commit-analyzer ships `{ revert: true, release: 'patch' }`, so an unscoped revert would - match no package's negated-scope suppression and patch-release **all** of them. Note the - residual, which `commitlint.config.js` records: that rule fires on the parser's - `revertPattern` — git's own `Revert "…"` form — and commitlint's default `ignores` skip - those messages entirely, so scoping is a convention here rather than something the hook - can enforce. Keep reverts conventional and scoped. `RELEASE_TYPES` and `RELEASE_SCOPES` + see [`VERSIONING.md`](./VERSIONING.md)). One correction worth knowing about `revert`: + a scoped conventional `revert(): …` **releases nothing** — the only revert rule is + commit-analyzer's default `{ revert: true, release: 'patch' }`, keyed on the angular + `revertPattern`, which matches a `Revert "…"` **or** bare `revert: …` header followed by a + `This reverts commit ` body; a parenthesized `revert():` header matches + neither. The two detected forms differ in what fences them: an unscoped `revert: …` is + rejected by commitlint's scope rule before it can land, while git's own `Revert "…"` form + is skipped by commitlint's default `ignores` entirely — and its generated body **does** + trip the default patch rule for every package, making it the real hazard. So keep reverts + conventional and scoped, and ship an actual rollback release as a follow-up + `fix(): …`. `RELEASE_TYPES` and `RELEASE_SCOPES` in `commitlint.config.js` are the source of truth. - **Breaking changes** need a `BREAKING CHANGE:` footer in the body — a `!` after the type is **not** picked up by our release tooling. diff --git a/SECURITY.md b/SECURITY.md index 802fc8ad9..fc19f2cb0 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -50,9 +50,11 @@ Measures active in this repository and its release pipeline: `publishConfig.provenance` — having one would imply the flag was redundant, and dropping the flag is the quiet way to lose attestations entirely. The `prepack`/`prepublishOnly` guards refuse packers they recognise as not being - pnpm. They do not refuse a hand-run `pnpm publish` that omits the flag; the - `prepublishOnly` hook warns about it instead, and only outside CI, so that - inspecting a tarball with `pnpm pack` stays quiet. Why it works that way is in + pnpm. They cannot see CLI flags at all, so they do not refuse a hand-run + `pnpm publish` that omits `--provenance`; the `prepublishOnly` hook prints an + unconditional reminder on every non-CI hand publish — including one that + passes the flags correctly — and only outside CI, so that inspecting a + tarball with `pnpm pack` stays quiet. Why it works that way is in [`VERSIONING.md`](./VERSIONING.md#release-process). - **Licence text comes from the workspace root** — no package carries its own `LICENSE` file, and `pnpm publish` copies the root one into every tarball. diff --git a/VERSIONING.md b/VERSIONING.md index 47dd84732..5d7ff6de0 100644 --- a/VERSIONING.md +++ b/VERSIONING.md @@ -31,16 +31,22 @@ Notes: - **Breaking changes require a `BREAKING CHANGE:` footer** in the commit body. The angular commit-analyzer preset does **not** parse `feat(bulma-ui)!:` bang headers. -- Commits of a releasing type (`feat`, `fix`, `perf`, `refactor`, `style`, `revert`) **must** +- Commits of a scope-gated type (`feat`, `fix`, `perf`, `refactor`, `style`, `revert`) **must** carry a scope of `bulma-ui`, `docs`, `create-bestax`, `bestax-migrate`, or `bestax-mcp` — enforced by commitlint ([`commitlint.config.js`](./commitlint.config.js)) via the husky `commit-msg` hook. This is what guarantees the per-scope release rules can't be bypassed by an unscoped commit. -- `revert` is scope-gated too: commit-analyzer's default rules ship - `{ revert: true, release: 'patch' }`, so an unscoped revert would patch-release **every** - package. A scoped `revert(bulma-ui): …` patch-releases only its package. Caveat: commitlint's - default ignores skip git-revert-style `Revert "…"` messages entirely, so keep reverts in - conventional form. +- `revert` is scope-gated too, but a scoped revert **releases nothing** — plan rollbacks + accordingly. The only revert rule anywhere is commit-analyzer's default + `{ revert: true, release: 'patch' }`, keyed on the parser's revert _detection_, and the + angular `revertPattern` matches only a `Revert "…"`/`revert: …` header followed by a + `This reverts commit ` body — a scoped `revert(bulma-ui): …` header is invisible to + it, and no `releaseRules` entry here names the `revert` type. To actually publish a + rollback, follow the revert with `fix(): …` (or commit the rollback as a `fix` + directly). The hazard runs the other way for git's own `Revert "…"` form, which + commitlint's default ignore rules let through unexamined: with its generated body it trips the default + revert rule with **no scope to confine it** and would patch-release every package — so + keep reverts in conventional, scoped form, and don't expect them to publish on their own. - A commit scoped to `docs` never releases any package. ## Tags & Changelogs diff --git a/docs/docs/guides/getting-started/contributing.md b/docs/docs/guides/getting-started/contributing.md index 68e01d2e1..577c138d7 100644 --- a/docs/docs/guides/getting-started/contributing.md +++ b/docs/docs/guides/getting-started/contributing.md @@ -168,10 +168,14 @@ drive [semantic-release](https://semantic-release.gitbook.io/). Releasing types `perf`, `refactor`, `style`, `revert`) must carry one of the scopes in `RELEASE_SCOPES` (`bulma-ui`, `docs`, `create-bestax`, `bestax-migrate`, `bestax-mcp`); `docs`, `chore`, `ci`, `build`, and `test` don't publish — note `docs` is both a valid scope and a non-releasing -type, so `docs(bulma-ui):` and `docs:` alike publish nothing. `revert` is release-triggering because commit-analyzer -patch-releases reverts by default, so an unscoped one would bump every package — though -commitlint's default ignores skip git's own `Revert "…"` form, so scoping that one is a -convention the hook cannot enforce. Publishing uses npm **OIDC trusted publishing** with **provenance** +type, so `docs(bulma-ui):` and `docs:` alike publish nothing. A scoped `revert(scope):` +commit releases **nothing** — the only revert rule is commit-analyzer's default, and its +`revertPattern` matches a `Revert "…"` or bare `revert: …` header (never the parenthesized +`revert(scope):` form) followed by a `This reverts commit ` body; ship a rollback with a +follow-up `fix(scope):` commit. An unscoped `revert: …` is stopped by commitlint's scope rule, +but git's own `Revert "…"` form is the real hazard: commitlint's default ignore rules let it +through unexamined, and its generated body trips the default revert rule with no scope to +confine it, so it would patch-release every package. Publishing uses npm **OIDC trusted publishing** with **provenance** (no long-lived token). See [`CONTRIBUTING.md`](https://github.com/allxsmith/bestax/blob/main/CONTRIBUTING.md) for the full details. diff --git a/scripts/check-conformance.mjs b/scripts/check-conformance.mjs index 4fbb6a0ea..d68b95b0b 100644 --- a/scripts/check-conformance.mjs +++ b/scripts/check-conformance.mjs @@ -73,6 +73,7 @@ import { } from './lib/pack-time-protocols.mjs'; import { fenceMask, + fenceSpans, readRegions, sectionSpans, splitLines, @@ -906,6 +907,48 @@ const RELEASE_DOC_FACTS = [ * the file-wide search this exists to prevent; and a `---` or `:::` inside a * fenced example truncated it early, in the other direction. */ +// 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- +// space disagreement with fenceMask's CommonMark reading (#548 review). +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 + * what runs. + */ +function shellOperative(text) { + return text + .split('\n') + .map(l => l.replace(/"[^"]*"|'[^']*'/g, '""').replace(/#.*$/, '')) + .join('\n') + .replace(/\\\n\s*/g, ' '); +} + +/** + * 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). + */ +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 safeToRunBlock(src) { const { lines } = splitLines(src); const masked = fenceMask(lines); @@ -920,11 +963,7 @@ function safeToRunBlock(src) { 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() === ':::' || - /^\s{0,3}#{1,6}\s/.test(l) || - l.trim() === '---' - ) { + if (l.trim() === ':::' || MD_HEADING.test(l) || l.trim() === '---') { end = i; break; } @@ -948,18 +987,28 @@ function safeToRunBlock(src) { */ function dryRunRecipe(src) { const { lines } = splitLines(src); - const masked = fenceMask(lines); - const blocks = []; - let current = null; - for (let i = 0; i < lines.length; i++) { - if (masked[i]) (current ??= []).push(lines[i]); - else if (current) { - blocks.push(current.join('\n')); - current = null; - } - } - if (current) blocks.push(current.join('\n')); // unterminated fence at EOF - return blocks.find(b => b.includes('semantic-release --dry-run')) ?? null; + // 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. + 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 + ); } /** @@ -972,13 +1021,30 @@ function dryRunRecipe(src) { * `echo "bestax-mcp is released separately"` satisfies it just as well, which * is the same failure one step further out. */ -function recipeTargets(block) { - const commands = block - .split('\n') - .map(l => l.replace(/#.*$/, '')) - .join('\n'); - const loop = commands.match(/\bfor\s+\w+\s+in\s+([^;\n]+)/); - return loop ? loop[1] : commands; +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(' '); } /** @@ -999,50 +1065,49 @@ function recipeTargets(block) { function packagesBullet(src) { const { lines } = splitLines(src); const masked = fenceMask(lines); - // The anchor has to be the HEADING, not any prose that mentions trusted - // publishing. Matching body text meant deleting the heading left the - // section's own sentence anchoring the search, so the list still passed - // while the section it belongs to was gone. Requiring a heading also makes - // anchor..limit a real section rather than an arbitrary span. - const anchor = lines.findIndex( - (l, i) => - !masked[i] && /^\s{0,3}#{1,6}\s/.test(l) && /trusted[- ]publish/i.test(l) - ); - // No OIDC section means no trusted-publisher list, which is the violation. - // Falling back to line 0 restored the file-wide search this extractor exists - // to prevent: an unrelated "- Packages:" bullet elsewhere then satisfied the - // assertion even with the real guidance deleted. - if (anchor < 0) return ''; - // Bounded at the next heading, because scanning to EOF let a complete - // "- Packages:" list in some later, unrelated section stand in for an OIDC - // section that had lost its own — the same fail-open as the line-0 fallback, - // one step further down the file. - let limit = lines.length; - for (let i = anchor + 1; i < lines.length; i++) { - if (!masked[i] && /^\s{0,3}#{1,6}\s/.test(lines[i])) { - limit = i; - break; + 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 = []; + for (let i = 0; i < lines.length; i++) { + if (!(isHeading(i) && /trusted[- ]publish/i.test(lines[i]))) continue; + let limit = lines.length; + for (let j = i + 1; j < lines.length; j++) { + if (isHeading(j)) { + limit = j; + break; + } } - } - const start = lines.findIndex( - (l, i) => - i >= anchor && - i < limit && - !masked[i] && - l.trimStart().startsWith('- Packages:') - ); - if (start < 0) return ''; - // Bounded by `limit` as well: a heading immediately after the bullet, with no - // blank line between, otherwise swept the next section in — and a package - // named there could then cover an omission in this list. - let end = limit; - for (let i = start + 1; i < limit; i++) { - if (!lines[i].trim() || /^\s*[-*]\s/.test(lines[i])) { - end = i; - 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')); } - return lines.slice(start, end).join('\n'); + return bullets; } /** @@ -1072,7 +1137,8 @@ export function unreadableManifestViolations(unreadable) { return unreadable.map( dir => dir + - '/package.json could not be read or parsed, so the release-docs checks ' + + '/package.json could not be read or parsed, or is not a JSON object, ' + + 'so the release-docs checks ' + 'cannot tell whether ' + dir + ' needs to appear in the dry-run recipe and the trusted-publisher list. ' + @@ -1172,8 +1238,8 @@ 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 section = packagesBullet(docs.get(primary)); - if (!section) { + const sections = packagesBullet(docs.get(primary)); + if (!sections.length) { violations.push( primary + ' has no "- Packages:" line in the OIDC trusted publishing section. ' + @@ -1181,17 +1247,23 @@ export function releaseDocViolations(docs, packages) { 'npmjs.com; without it nobody can tell which ones do.' ); } else { - const named = packageTokens(section); - const missing = packages.filter(p => !named.has(p.name)); - if (missing.length) { - violations.push( - primary + - "'s trusted-publisher list omits " + - missing.map(p => p.name).join(', ') + - '. Every publishable package authenticates by OIDC, and a missing ' + - 'trusted publisher fails the publish after the release commit and ' + - 'tag are already pushed — which spends the version (#536).' - ); + // 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. + for (const section of sections) { + const named = packageTokens(section); + const missing = packages.filter(p => !named.has(p.name)); + if (missing.length) { + violations.push( + primary + + "'s trusted-publisher list omits " + + missing.map(p => p.name).join(', ') + + '. Every publishable package authenticates by OIDC, and a ' + + 'missing trusted publisher fails the publish after the release ' + + 'commit and tag are already pushed — which spends the version ' + + '(#536).' + ); + } } } } @@ -1227,6 +1299,16 @@ export async function publishablePackages(root = REPO) { unreadable.push(dir); continue; } + // JSON.parse succeeds on `null`, `42`, `"x"`, `[]` — shapes a truncated + // write really produces. `null` dereferenced below threw a TypeError past + // the runner's loop; an ARRAY passes typeof and instead vanished silently + // (no name, so it never joined the package list) — the same outcome by a + // quieter road. Both count as unreadable manifests, checked here in the + // open rather than by throwing into the catch above. + if (!pkg || typeof pkg !== 'object' || Array.isArray(pkg)) { + unreadable.push(dir); + continue; + } if (!pkg.private && pkg.name) packages.push({ dir, name: pkg.name }); } return { packages, unreadable }; diff --git a/scripts/lib/api-page.mjs b/scripts/lib/api-page.mjs index 43f8554aa..9813e1494 100644 --- a/scripts/lib/api-page.mjs +++ b/scripts/lib/api-page.mjs @@ -54,32 +54,48 @@ export function joinLines(lines, crlf) { */ export function fenceMask(lines) { const mask = new Array(lines.length).fill(false); + for (const { open, close } of fenceSpans(lines)) { + for (let i = open; i <= close; i++) mask[i] = true; + } + return mask; +} + +/** + * The fenced blocks of `lines`, as [{ open, close }] line-index pairs with + * both delimiters included (`close` is the last line for a fence left + * unterminated at EOF). The same CommonMark rules as `fenceMask` — in fact + * fenceMask is derived from this, so the two views cannot disagree. Consumers + * that need BLOCKS should use this rather than re-deriving structure from the + * boolean mask: in the mask, butted fences form one continuous run, and a + * content line shaped like a delimiter is indistinguishable from one. + */ +export function fenceSpans(lines) { + const spans = []; let fence = null; // { char, len } + let open = -1; for (let i = 0; i < lines.length; i++) { - const line = lines[i]; - const m = line.match(/^ {0,3}(`{3,}|~{3,})(.*)$/); + const m = lines[i].match(/^ {0,3}(`{3,}|~{3,})(.*)$/); if (!fence) { if (m) { // An opening ``` fence may not contain a backtick in its info string. const char = m[1][0]; if (!(char === '`' && m[2].includes('`'))) { fence = { char, len: m[1].length }; - mask[i] = true; + open = i; } } - } else { - mask[i] = true; - if ( - m && - m[1][0] === fence.char && - m[1].length >= fence.len && - !m[2].trim() - ) { - fence = null; - } + } else if ( + m && + m[1][0] === fence.char && + m[1].length >= fence.len && + !m[2].trim() + ) { + spans.push({ open, close: i }); + fence = null; } } - return mask; + if (fence) spans.push({ open, close: lines.length - 1 }); + return spans; } /** diff --git a/scripts/release-docs-sync.test.mjs b/scripts/release-docs-sync.test.mjs index 681ad5958..6d9072c74 100644 --- a/scripts/release-docs-sync.test.mjs +++ b/scripts/release-docs-sync.test.mjs @@ -15,12 +15,16 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; import { readFileSync } from 'node:fs'; +import { mkdtemp, mkdir, writeFile, rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; import { fileURLToPath } from 'node:url'; import { publishablePackages, releaseDocViolations, unreadableManifestViolations, + recipeTargets, } from './check-conformance.mjs'; const PACKAGES = [ @@ -456,3 +460,260 @@ test('an empty package list is refused, not silently satisfied', () => { assert.equal(v.length, 1); assert.match(v[0], /No publishable packages were found/); }); + +// --- 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. + const wrapped = + '```bash\nfor pkg in bulma-ui create-bestax \\\n' + + ' bestax-migrate bestax-mcp; do\n' + + ' ( cd "$pkg" && pnpm exec semantic-release --dry-run --no-ci )\ndone\n```'; + assert.deepEqual( + releaseDocViolations( + docs({ contributing: doc({ recipe: wrapped }) }), + PACKAGES + ), + [] + ); +}); + +test('an example fence butted above the recipe does not merge into it', () => { + // fenceMask marks delimiter lines true, so two butted fences formed one + // block — and an example loop naming all four dirs then masked a real + // omission in the recipe below it (fail-open), while a partial example + // produced false missing-package reds (false positive). + const butted = + '```bash\nfor pkg in bulma-ui create-bestax bestax-migrate bestax-mcp; do echo "$pkg"; done\n```\n' + + recipe(['bulma-ui']); // the REAL recipe omits three packages + const v = releaseDocViolations( + docs({ contributing: doc({ recipe: butted }) }), + PACKAGES + ); + assert.ok(v.length >= 1, 'the omission must not be masked by the example'); + assert.match(v.join(' '), /bestax-mcp/); +}); + +test('a fence butted under the Packages bullet does not extend it', () => { + // The continuation scan stopped at blank lines and bullets only, so a + // fenced example with no blank line above it was swept into the bullet + // text and its tokens counted as trusted publishers. + const swept = + publishers(PACKAGES.slice(0, 3).map(p => p.name)) + + '\n```bash\nnpm owner ls bestax-mcp\n```'; + const v = releaseDocViolations( + docs({ contributing: doc({ publishers: swept }) }), + PACKAGES + ); + assert.ok(v.length >= 1, 'the fenced mention must not stand in for the list'); + assert.match(v.join(' '), /bestax-mcp/); +}); + +test('an earlier trusted-publishing heading does not steal the anchor', () => { + // First-match anchoring meant a new earlier heading captured the search + // and produced a false "no Packages line" while the real section was fine. + const decoy = + '## Why trusted publishing?\n\nBecause tokens leak.\n\n' + doc(); + assert.deepEqual( + releaseDocViolations(docs({ contributing: decoy }), PACKAGES), + [] + ); +}); + +/** + * A throwaway workspace on disk: one directory per entry of + * `manifestsByDir`, each holding that literal package.json text, listed in + * pnpm-workspace.yaml in insertion order. The sibling suites' local fixture + * helper pattern (fixtureRepo, fixtureTree) — the manifest string is the + * only thing an unreadable-manifest case varies. + */ +async function fixtureWorkspace(manifestsByDir, fn) { + const root = await mkdtemp(join(tmpdir(), 'manifests-')); + try { + const dirs = Object.keys(manifestsByDir); + await writeFile( + join(root, 'pnpm-workspace.yaml'), + 'packages:\n' + dirs.map(d => ` - ${d}\n`).join('') + ); + for (const [dir, manifest] of Object.entries(manifestsByDir)) { + await mkdir(join(root, dir)); + await writeFile(join(root, dir, 'package.json'), manifest); + } + return await fn(root); + } finally { + await rm(root, { recursive: true, force: true }); + } +} + +test('a manifest that parses to null is unreadable, not a crash', async () => { + // JSON.parse succeeds on the literal `null` — a truncated-write artifact — + // and dereferencing it threw a TypeError past the runner's loop, aborting + // every remaining conformance check instead of reporting the one broken + // package through the violation written for exactly this case. + await fixtureWorkspace( + { good: '{"name": "good", "private": false}', broken: 'null' }, + async root => { + const { packages, unreadable } = await publishablePackages(root); + assert.deepEqual(packages, [{ dir: 'good', name: 'good' }]); + assert.deepEqual(unreadable, ['broken']); + } + ); +}); + +test('scalar manifests are unreadable too, not silently absent', async () => { + // The other parseable non-object shapes the shape-check comment names. + await fixtureWorkspace({ num: '42', str: '"x"' }, async root => { + const { packages, unreadable } = await publishablePackages(root); + assert.deepEqual(packages, []); + assert.deepEqual(unreadable, ['num', 'str']); + }); +}); + +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 + // quieter road. + await fixtureWorkspace({ arr: '[]' }, async root => { + const { packages, unreadable } = await publishablePackages(root); + assert.deepEqual(packages, []); + assert.deepEqual(unreadable, ['arr']); + }); +}); + +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. + 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```'; + const real = + '```bash\nfor pkg in bulma-ui; do\n pnpm exec semantic-release --dry-run --no-ci\ndone\n```'; + const v = releaseDocViolations( + docs({ contributing: doc({ recipe: example + '\n' + real }) }), + PACKAGES + ); + assert.ok(v.length >= 1, 'the real recipe must be the one checked'); + assert.match(v.join(' '), /bestax-mcp/); +}); + +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. + const block = [ + '~~~bash', + 'cat <= 1, 'the recipe must be found whole'); + assert.match(v.join(' '), /omits/); + assert.doesNotMatch(v.join(' '), /has no .* dry-run/); +}); + +test("a later aside's bullet cannot absorb a deleted trusted-publisher list", () => { + // The owning-section anchor was fail-open: deleting the real section's + // bullet let a later trusted-publishing-titled aside's stray bullet stand + // in. Every candidate section's bullet is validated now, so an incomplete + // stand-in still fails, and no-bullet-anywhere still fires. + const withAside = [ + '### npm authentication (OIDC trusted publishing)', + '', + 'The real section, its bullet deleted.', + '', + '### Why trusted publishing?', + '', + '- Packages: `bulma-ui`', + '', + ].join('\n'); + const v = releaseDocViolations( + docs({ contributing: doc({ publishers: withAside }) }), + PACKAGES + ); + assert.ok(v.length >= 1, 'the incomplete stand-in must not pass'); +});