* Enforce documentation updates via lint:docs + PR templates (#3213) New scripts/lint-docs-required.cjs + Docs Required CI workflow fail any PR whose changeset fragment is typed Added / Changed / Deprecated / Removed without modifying at least one file under docs/. Mirrors scripts/changeset/lint.cjs: pure evaluateLint({ changedFiles, fragments, labels }) returning { ok, reason, triggering } over a frozen LINT_REASON enum; CLI wrapper reads the PR diff and parses each touched changeset fragment via the existing parseFragment helper. Escape hatches: - no-docs PR label (global) - per-fragment <!-- docs-exempt: <reason> --> marker, all triggering fragments must carry it for the PR to pass Fixed and Security fragments do not trigger the lint — bug fixes restore documented behavior, they do not introduce new behavior to document. PR templates (enhancement.md, feature.md) gain a Documentation checklist section pointing at the which-doc-to-update matrix. CONTRIBUTING.md adds a Documentation Updates section codifying that matrix, the English-canonical language policy for docs/ and the root README, and the two opt-out routes. Closes #3213 * Address Codex review: fail-closed on malformed fragments and strip docs-exempt marker from rendered release notes (#3213) Two P2 issues caught by `codex review --base main`: 1) Malformed fragments could silently bypass docs enforcement. parseFragment would return ok:false on a triggering Added fragment with bad frontmatter and readFragmentsFromDisk dropped it, so evaluateLint saw no triggering fragments and passed. The changeset-required lint only checks fragment _presence_ not _validity_, so the assumed fallback did not catch it. Fix: readFragmentsFromDisk now returns { fragments, malformed }; evaluateLint accepts a malformed param and emits a new FAIL_MALFORMED_FRAGMENT verdict that outranks every OK path (including the no-docs label) — a parse failure must be fixed before docs lint can decide anything else. 2) The per-fragment <!-- docs-exempt: reason --> marker lived in the fragment body, so the existing changelog (serializeChangelog) and GitHub release-notes (formatBullet) serializers published it verbatim. Worse, both serializers append `(#NNNN)` to the body's last line — with the marker as the trailing line, the PR suffix attached to the hidden comment instead of the visible bullet. Fix: parseFragment now extracts the marker into a typed `docsExempt` field and strips it from `body`, so all downstream renderers produce clean output without remembering to strip. The regex is anchored to its own line (^...$ with m flag) so inline mentions of the marker syntax in documentation (e.g. inside backticks) cannot accidentally exempt a fragment. Bounded character class [^\n>] keeps the regex linear-time. Test additions: - tests/lint-docs-required.test.cjs: FAIL_MALFORMED_FRAGMENT coverage, end-to-end "Added fragment with bad pr → malformed → fail-closed" regression test, updated readFragmentsFromDisk return-shape assertions, isExemptFragment now checks the typed docsExempt field rather than body content. - tests/changeset-parse.test.cjs: extractDocsExempt extraction cases (with/ without reason, case-insensitive, EMPTY_BODY when body is only a marker), inline-mention false-positive guard, real-marker-wins-when-also-inline test. - tests/changeset-new.test.cjs: fragment shape now includes docsExempt: null. CONTRIBUTING.md updated to clarify the "on its own line" requirement and the parse-time stripping behavior. The bootstrap fragment cleaned up so its body no longer contains a literal marker example that would have triggered the false-positive case. Full suite: 9696/9696 pass. * CRLF-safe docs-exempt marker stripping (Codex review pass 2, #3213) Second `codex review --commit` pass caught a CRLF regression in the docs-exempt extraction added in the previous commit. Repro: a Windows-authored fragment ---\r\ntype: Added\r\npr: 1\r\n---\r\nFeature.\r\n\r\n<!-- docs-exempt: x -->\r\n would parse to body `Feature.\r\n\r\n\r` because: - The previous trailing-newline slice trimmed only `\n`, leaving `\r`. - DOCS_EXEMPT_RE was anchored with `$` only — in multiline mode `$` matches before `\n` but does not consume `\r`, so the marker line's trailing `\r` was left behind after replace. - The cleanup regex stripped trailing `\n` but not `\r`. Net effect: serializeChangelog emitted - Feature.\r \r \r (#1) — the `(#1)` PR suffix landed on a blank line instead of attached to the visible bullet. Same bug surfaces in github-release-notes formatBullet. Fix: - DOCS_EXEMPT_RE: add `\r?` before `$` so the regex consumes the CR of a CRLF terminator. Switch reason character class from `[^\n>]` to `[^\r\n>]` so CRLF-authored reasons don't carry a trailing `\r`. - extractDocsExempt cleanup: `[ \t\r]+$/gm` strips trailing `\r` on each line; `(?:\r?\n){3,}` collapses CRLF triple-blank-lines; `[\r\n]+$` strips every trailing line terminator (LF or CR). - parseFragment trailing-newline slice: CRLF-aware — strips `\r\n` (2 chars) before falling through to single `\n`. Tests: two CRLF regression cases in tests/changeset-parse.test.cjs — Codex's exact repro (end-to-end through serializeChangelog) plus the no-marker CRLF passthrough case. Full suite: 9698/9698 pass. * CRLF regression test asserts on parseChangelog IR not rendered text (Codex review pass 3, #3213) Third `codex review` pass caught that the CRLF regression test added in the previous commit asserted on serializeChangelog's rendered Markdown via `out.split('\n')` + `assert.match`. That violates CONTRIBUTING.md's "Prohibited: Raw Text Matching on Test Outputs" rule and the documented serializer contract in `serialize.cjs`: > tests assert via round-trip (parse(serialize(ir))) > rather than by inspecting serialized text Replace the regex check with the established `parseChangelog(out)` round-trip and assert on the structured `{ body: 'Feature.', pr: 1 }` bullet. This is also a stronger regression check than the substring match: Codex's own probe in the review session confirmed the pre-fix buggy body shape (`Feature.\r\n\r\n\r`) breaks parseChangelog's bullet regex entirely (returns `bullets: []`), so the round-trip catches the exact failure mode end-to-end. Full suite: 9698/9698 pass. * Address CodeRabbit findings: anchor link + require non-empty docs-exempt reason (#3213) CodeRabbit's review on the PR caught two actionable issues, both quick wins. Anchor link in PR templates pointed to a heading that does not exist. The CONTRIBUTING.md heading "Documentation Updates — Update the Relevant Docs" contains an em-dash, which GitHub strips entirely when generating anchor slugs (it does NOT collapse to a hyphen). The actual anchor is #documentation-updates-update-the-relevant-docs (single hyphen between every word), not #documentation-updates--update-the-relevant-docs (double hyphen where the em-dash was). Both feature.md and enhancement.md fixed. The docs-exempt marker matched a bare `<!-- docs-exempt -->` with no reason, which defeats the entire purpose of the escape hatch — the marker exists to leave an audit trail explaining WHY a PR is exempt. Without a reason it is a silent bypass. Fix: DOCS_EXEMPT_RE now requires both the colon AND a non-whitespace first reason character. Bare `<!-- docs-exempt -->`, empty `<!-- docs-exempt: -->`, and whitespace-only `<!-- docs-exempt: -->` are all rejected as if the marker were not present (`docsExempt: null`). The lint then falls through to its normal docs-required / no-docs-label checks. `isExemptFragment` in the lint module tightened too — defense-in-depth: even if a caller constructs a fragment with `docsExempt: ''` directly, it does not count as exempt. The predicate now requires `typeof === 'string'` and non-empty after trim. Tests: - changeset-parse.test.cjs: three new explicit-rejection cases (bare marker, empty reason, whitespace-only reason). Existing DOCS_EXEMPT_RE shape test extended with negative assertions for the same three forms. - lint-docs-required.test.cjs: prior "empty reason still exempt" test inverted — empty/whitespace docsExempt now produces FAIL_DOCS_MISSING. isExemptFragment helper test extended with the same negative cases. - CONTRIBUTING.md: clarified that the reason is required and non-empty. Skipped CodeRabbit's third finding ("use `npm run lint:docs` in CI workflow instead of `node scripts/lint-docs-required.cjs`") — the existing changeset-required.yml uses the direct-node form for the equivalent changeset lint, so the new docs-required.yml is convention-consistent. Switching one without the other would create drift, and switching both is out of scope for #3213. Bootstrap fragment continues to extract cleanly under the stricter regex (verified — `docsExempt` field still contains the full bootstrap reason). Full suite: 9701/9701 pass.
195 lines
9.2 KiB
JavaScript
195 lines
9.2 KiB
JavaScript
'use strict';
|
|
process.env.GSD_TEST_MODE = '1';
|
|
|
|
const { test, describe } = require('node:test');
|
|
const assert = require('node:assert/strict');
|
|
const path = require('node:path');
|
|
|
|
const { parseFragment, extractDocsExempt, FRAGMENT_ERROR, DOCS_EXEMPT_RE } = require(path.join(__dirname, '..', 'scripts', 'changeset', 'parse.cjs'));
|
|
|
|
describe('changeset parse: fragment file → typed record (#2975)', () => {
|
|
test('returns { ok: true, fragment } for a well-formed fragment', () => {
|
|
const src = '---\ntype: Fixed\npr: 2975\n---\nfix the thing.\n';
|
|
const result = parseFragment(src);
|
|
assert.equal(result.ok, true);
|
|
assert.deepEqual(result.fragment, {
|
|
type: 'Fixed',
|
|
pr: 2975,
|
|
body: 'fix the thing.',
|
|
docsExempt: null,
|
|
});
|
|
});
|
|
|
|
test('preserves verbatim body content (e.g. code blocks) — does not trim significant whitespace', () => {
|
|
const src = '---\ntype: Fixed\npr: 1\n---\n```js\nlet x = 1;\n```\n';
|
|
const r = parseFragment(src);
|
|
assert.equal(r.ok, true);
|
|
assert.equal(r.fragment.body, '```js\nlet x = 1;\n```');
|
|
});
|
|
|
|
test('exposes a frozen FRAGMENT_ERROR enum with the documented codes', () => {
|
|
assert.deepEqual(
|
|
Object.keys(FRAGMENT_ERROR).sort(),
|
|
['EMPTY_BODY', 'INVALID_PR', 'INVALID_TYPE', 'MISSING_FRONTMATTER', 'MISSING_PR', 'MISSING_TYPE'],
|
|
);
|
|
});
|
|
|
|
for (const [label, src, expectedReason] of [
|
|
['fails MISSING_FRONTMATTER when no frontmatter block present',
|
|
'just a body, no frontmatter\n', 'MISSING_FRONTMATTER'],
|
|
['fails MISSING_TYPE when frontmatter omits type:',
|
|
'---\npr: 2975\n---\nfix.\n', 'MISSING_TYPE'],
|
|
['fails INVALID_TYPE for a type not in the Keep-a-Changelog set',
|
|
'---\ntype: Refactored\npr: 2975\n---\nfix.\n', 'INVALID_TYPE'],
|
|
['fails MISSING_PR when frontmatter omits pr:',
|
|
'---\ntype: Fixed\n---\nfix.\n', 'MISSING_PR'],
|
|
['fails INVALID_PR when pr: is not a positive integer',
|
|
'---\ntype: Fixed\npr: 0\n---\nfix.\n', 'INVALID_PR'],
|
|
['fails EMPTY_BODY when the body is whitespace-only',
|
|
'---\ntype: Fixed\npr: 2975\n---\n \n', 'EMPTY_BODY'],
|
|
]) {
|
|
test(label, () => {
|
|
const r = parseFragment(src);
|
|
assert.equal(r.ok, false);
|
|
assert.equal(r.reason, FRAGMENT_ERROR[expectedReason]);
|
|
});
|
|
}
|
|
});
|
|
|
|
describe('changeset parse: docs-exempt extraction (#3213)', () => {
|
|
test('extractDocsExempt returns { docsExempt: null, body } when no marker present', () => {
|
|
const out = extractDocsExempt('plain body text');
|
|
assert.deepEqual(out, { docsExempt: null, body: 'plain body text' });
|
|
});
|
|
|
|
test('extractDocsExempt captures the reason and strips the marker from body', () => {
|
|
const out = extractDocsExempt('feature note.\n\n<!-- docs-exempt: internal-only -->');
|
|
assert.equal(out.docsExempt, 'internal-only');
|
|
assert.doesNotMatch(out.body, /docs-exempt/);
|
|
assert.match(out.body, /feature note\./);
|
|
});
|
|
|
|
test('extractDocsExempt REJECTS bare marker without colon — reason is required (CodeRabbit finding)', () => {
|
|
// A bare `<!-- docs-exempt -->` provides no audit trail; intentionally
|
|
// not extracted so the lint requires either docs/ updates or a marker
|
|
// with a real reason.
|
|
const out = extractDocsExempt('body\n<!-- docs-exempt -->');
|
|
assert.equal(out.docsExempt, null);
|
|
assert.match(out.body, /docs-exempt/); // unchanged — bare marker stays in body
|
|
});
|
|
|
|
test('extractDocsExempt REJECTS marker with empty reason (<!-- docs-exempt: -->)', () => {
|
|
const out = extractDocsExempt('body\n<!-- docs-exempt: -->');
|
|
assert.equal(out.docsExempt, null);
|
|
});
|
|
|
|
test('extractDocsExempt REJECTS marker with whitespace-only reason', () => {
|
|
const out = extractDocsExempt('body\n<!-- docs-exempt: -->');
|
|
assert.equal(out.docsExempt, null);
|
|
});
|
|
|
|
test('extractDocsExempt is case-insensitive on the marker token', () => {
|
|
const out = extractDocsExempt('body\n<!-- DOCS-EXEMPT: shouty reason -->');
|
|
assert.equal(out.docsExempt, 'shouty reason');
|
|
});
|
|
|
|
test('parseFragment surfaces docsExempt on the fragment record', () => {
|
|
const src = '---\ntype: Added\npr: 3213\n---\nbootstrap.\n\n<!-- docs-exempt: bootstrap -->\n';
|
|
const r = parseFragment(src);
|
|
assert.equal(r.ok, true);
|
|
assert.equal(r.fragment.docsExempt, 'bootstrap');
|
|
// Marker must not appear in the rendered body. CHANGELOG and GitHub
|
|
// release-notes serializers append `(#NNNN)` to the body's last line;
|
|
// a trailing comment line would attach the suffix to the wrong content.
|
|
assert.doesNotMatch(r.fragment.body, /docs-exempt/);
|
|
assert.match(r.fragment.body, /bootstrap\./);
|
|
});
|
|
|
|
test('parseFragment fails EMPTY_BODY when the body is only a docs-exempt marker', () => {
|
|
const src = '---\ntype: Added\npr: 1\n---\n<!-- docs-exempt: nothing else -->\n';
|
|
const r = parseFragment(src);
|
|
assert.equal(r.ok, false);
|
|
assert.equal(r.reason, FRAGMENT_ERROR.EMPTY_BODY);
|
|
});
|
|
|
|
test('DOCS_EXEMPT_RE is exposed and matches the documented shape (colon + non-empty reason required)', () => {
|
|
assert.ok(DOCS_EXEMPT_RE instanceof RegExp);
|
|
assert.match('<!-- docs-exempt: x -->', DOCS_EXEMPT_RE);
|
|
assert.match('<!-- docs-exempt: bootstrap reason with spaces -->', DOCS_EXEMPT_RE);
|
|
assert.doesNotMatch('docs-exempt: not in a comment', DOCS_EXEMPT_RE);
|
|
assert.doesNotMatch('<!-- docs-exempt -->', DOCS_EXEMPT_RE); // no colon
|
|
assert.doesNotMatch('<!-- docs-exempt: -->', DOCS_EXEMPT_RE); // empty reason
|
|
assert.doesNotMatch('<!-- docs-exempt: -->', DOCS_EXEMPT_RE); // whitespace-only reason
|
|
});
|
|
|
|
test('inline mention inside backticks does NOT count as a marker (false-positive guard)', () => {
|
|
// Fragment body documents the marker syntax inline as part of release notes.
|
|
// Without the line-anchor, the regex would mis-identify this as an actual
|
|
// exemption and strip release-note content.
|
|
const src =
|
|
'---\ntype: Added\npr: 3213\n---\n' +
|
|
'New escape hatch: `<!-- docs-exempt: <reason> -->` on its own line at the end of a fragment body exempts that fragment from docs lint.\n';
|
|
const r = parseFragment(src);
|
|
assert.equal(r.ok, true);
|
|
assert.equal(r.fragment.docsExempt, null);
|
|
// The literal syntax example must remain in the rendered body — it is
|
|
// legitimate release-note content explaining the new feature.
|
|
assert.match(r.fragment.body, /docs-exempt/);
|
|
});
|
|
|
|
test('CRLF-authored fragments: marker is stripped cleanly without residual \\r (Codex finding)', () => {
|
|
// Codex's exact repro from the second review pass:
|
|
// Feature.\r\n\r\n<!-- docs-exempt: x -->\r\n
|
|
// Before the fix this parsed to body `Feature.\r\n\r\n\r`, which made
|
|
// serializeChangelog emit `- Feature.\r\n\r\n\r (#1)` — the PR suffix
|
|
// landed on a blank line instead of attached to the visible bullet.
|
|
const src = '---\r\ntype: Added\r\npr: 1\r\n---\r\nFeature.\r\n\r\n<!-- docs-exempt: x -->\r\n';
|
|
const r = parseFragment(src);
|
|
assert.equal(r.ok, true);
|
|
assert.equal(r.fragment.docsExempt, 'x');
|
|
assert.doesNotMatch(r.fragment.body, /[\r]/); // no residual CR characters
|
|
assert.doesNotMatch(r.fragment.body, /docs-exempt/);
|
|
// End-to-end: round-trip through serialize → parse to assert on the
|
|
// structured changelog IR, not rendered text (CONTRIBUTING.md:
|
|
// "Prohibited: Raw Text Matching on Test Outputs"). The buggy pre-fix
|
|
// body shape (`Feature.\r\n\r\n\r`) breaks `parseChangelog`'s bullet
|
|
// regex — it returns an empty `bullets: []` — so this round-trip is
|
|
// a stronger regression check than a substring match.
|
|
const { serializeChangelog, parseChangelog } = require(path.join(__dirname, '..', 'scripts', 'changeset', 'serialize.cjs'));
|
|
const out = serializeChangelog({
|
|
releaseHeader: { version: '1.0.0', date: '2026-01-01' },
|
|
sections: [{ type: 'Added', bullets: [{ pr: r.fragment.pr, body: r.fragment.body }] }],
|
|
priorChangelog: null,
|
|
});
|
|
const parsed = parseChangelog(out);
|
|
assert.equal(parsed.releases.length, 1);
|
|
assert.deepEqual(parsed.releases[0].sections, [
|
|
{ type: 'Added', bullets: [{ body: 'Feature.', pr: 1 }] },
|
|
]);
|
|
});
|
|
|
|
test('CRLF-authored fragment without marker: no stripping needed, body unchanged in semantics', () => {
|
|
const src = '---\r\ntype: Fixed\r\npr: 5\r\n---\r\nbug fix.\r\n';
|
|
const r = parseFragment(src);
|
|
assert.equal(r.ok, true);
|
|
assert.equal(r.fragment.docsExempt, null);
|
|
assert.match(r.fragment.body, /bug fix\./);
|
|
});
|
|
|
|
test('marker on its own line trailing a fragment body still wins (real-marker positive case)', () => {
|
|
const src =
|
|
'---\ntype: Added\npr: 3213\n---\n' +
|
|
'New escape hatch: `<!-- docs-exempt: <reason> -->` documents the syntax.\n' +
|
|
'\n' +
|
|
'<!-- docs-exempt: bootstrap of the lint itself -->\n';
|
|
const r = parseFragment(src);
|
|
assert.equal(r.ok, true);
|
|
assert.equal(r.fragment.docsExempt, 'bootstrap of the lint itself');
|
|
// The trailing real-marker line is stripped — the "bootstrap" reason
|
|
// should not appear anywhere in the rendered body.
|
|
assert.doesNotMatch(r.fragment.body, /bootstrap of the lint itself/);
|
|
// … but the inline syntax example is preserved.
|
|
assert.match(r.fragment.body, /docs-exempt: <reason>/);
|
|
});
|
|
});
|