diff --git a/.changeset/steady-zebras-click.md b/.changeset/steady-zebras-click.md new file mode 100644 index 000000000..881950e46 --- /dev/null +++ b/.changeset/steady-zebras-click.md @@ -0,0 +1,8 @@ +--- +type: Added +pr: 3213 +--- +**Added: `lint:docs` enforcement.** 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/`. Escape hatches: the `no-docs` PR label, or a per-fragment HTML-comment marker on its own line at the end of the fragment body (extracted at the `parseFragment` seam so it never bleeds into CHANGELOG.md or GitHub release-notes output). `Fixed` and `Security` fragments are not gated. Malformed fragments now fail closed via the new `FAIL_MALFORMED_FRAGMENT` verdict — a triggering fragment with bad frontmatter cannot silently bypass docs enforcement. PR templates (`enhancement.md`, `feature.md`) gain a Documentation checklist; `CONTRIBUTING.md` adds a `Documentation Updates` section codifying the which-doc-to-update matrix and English-canonical language policy. + + + diff --git a/.github/PULL_REQUEST_TEMPLATE/enhancement.md b/.github/PULL_REQUEST_TEMPLATE/enhancement.md index d79e11205..4c952ffe6 100644 --- a/.github/PULL_REQUEST_TEMPLATE/enhancement.md +++ b/.github/PULL_REQUEST_TEMPLATE/enhancement.md @@ -66,6 +66,22 @@ Closes # --- +## Documentation + +> CI enforces this — `lint:docs` fails any PR with an `Added` / `Changed` / `Deprecated` / `Removed` +> changeset fragment that does not also touch at least one file under `docs/`. +> See [CONTRIBUTING.md → Documentation Updates](../../CONTRIBUTING.md#documentation-updates-update-the-relevant-docs). + +- [ ] Updated the relevant file(s) under `docs/` to reflect this change + - Behavior or output change → `docs/USER-GUIDE.md` and/or `docs/COMMANDS.md` + - Configuration / schema change → `docs/CONFIGURATION.md` + - Architectural change → `docs/ARCHITECTURE.md` and/or `docs/adr/` + - Agent or skill change → `docs/AGENTS.md` +- [ ] All `docs/` content added in this PR is written in English +- [ ] If genuinely no user-facing docs impact (infrastructure / internal refactor / test-only), + apply the `no-docs` label **or** add `` inside each + triggering changeset fragment and leave a comment explaining why. + ## Checklist - [ ] Issue linked above with `Closes #NNN` — **PR will be auto-closed if missing** @@ -74,7 +90,6 @@ Closes # - [ ] All existing tests pass (`npm test`) - [ ] New or updated tests cover the enhanced behavior - [ ] `.changeset/` fragment added (`npm run changeset -- --type Changed --pr --body "..."`) — or `no-changelog` label applied if not user-facing -- [ ] Documentation updated if behavior or output changed - [ ] No unnecessary dependencies added ## Breaking changes diff --git a/.github/PULL_REQUEST_TEMPLATE/feature.md b/.github/PULL_REQUEST_TEMPLATE/feature.md index 47d232a60..798d0e717 100644 --- a/.github/PULL_REQUEST_TEMPLATE/feature.md +++ b/.github/PULL_REQUEST_TEMPLATE/feature.md @@ -86,6 +86,26 @@ Closes # --- +## Documentation + +> CI enforces this — `lint:docs` fails any PR with an `Added` / `Changed` / `Deprecated` / `Removed` +> changeset fragment that does not also touch at least one file under `docs/`. Features almost +> always trigger `Added`. See +> [CONTRIBUTING.md → Documentation Updates](../../CONTRIBUTING.md#documentation-updates-update-the-relevant-docs). + +- [ ] Updated the relevant file(s) under `docs/` to reflect this feature + - New command or flag → `docs/COMMANDS.md` and `docs/FEATURES.md` + - New workflow or behavior → `docs/USER-GUIDE.md` + - Configuration / schema change → `docs/CONFIGURATION.md` + - Architectural change → `docs/ARCHITECTURE.md` and/or `docs/adr/` + - Agent or skill change → `docs/AGENTS.md` +- [ ] All `docs/` content added in this PR is written in English + (translated READMEs `README.pt-BR.md` / `README.zh-CN.md` / `README.ja-JP.md` / `README.ko-KR.md` + are community-maintained and do not need to be updated in this PR) +- [ ] If genuinely no user-facing docs impact (rare for features — explain in PR), apply the + `no-docs` label **or** add `` inside each triggering + changeset fragment. + ## Checklist - [ ] Issue linked above with `Closes #NNN` — **PR will be auto-closed if missing** @@ -95,7 +115,6 @@ Closes # - [ ] All existing tests pass (`npm test`) - [ ] New tests cover the happy path, error cases, and edge cases - [ ] `.changeset/` fragment added with a user-facing description of the feature (`npm run changeset -- --type Added --pr --body "..."`) -- [ ] Documentation updated — commands, workflows, references, README if applicable - [ ] No unnecessary external dependencies added - [ ] Works on Windows (backslash paths handled) diff --git a/.github/workflows/docs-required.yml b/.github/workflows/docs-required.yml new file mode 100644 index 000000000..05e88bf3f --- /dev/null +++ b/.github/workflows/docs-required.yml @@ -0,0 +1,24 @@ +name: Docs Required + +on: + pull_request: + types: [opened, synchronize, reopened, labeled, unlabeled] + +permissions: + contents: read + pull-requests: read + +jobs: + docs-lint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - uses: actions/setup-node@v4 + with: + node-version: '24' + - name: Run docs-required lint + env: + GITHUB_BASE_REF: ${{ github.base_ref }} + run: node scripts/lint-docs-required.cjs diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2e091a769..b1f8e60d8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -150,6 +150,40 @@ Fragments are consolidated into `CHANGELOG.md` at release time by the release wo **Opt-out:** PRs with no user-facing impact (test refactors, lint config changes, CI tweaks, formatting-only changes) can add the `no-changelog` label. The lint honors it. When unsure whether a change is user-facing, **add the fragment**. +## Documentation Updates — Update the Relevant Docs + +If your PR adds, changes, deprecates, or removes user-visible behavior, you **must** update the relevant documentation in `docs/`. CI will fail any PR whose changeset fragment is typed `Added`, `Changed`, `Deprecated`, or `Removed` without also modifying at least one file under `docs/` ([#3213](https://github.com/gsd-build/get-shit-done/issues/3213)). + +`Fixed` and `Security` fragments do not trigger this lint — bug fixes restore documented behavior, they do not introduce new behavior to document. (Edit the docs anyway if a fix corrects something the docs got wrong.) + +### Which docs to update + +| Change type | Required doc updates | +|---|---| +| New command or flag | `docs/COMMANDS.md`, `docs/FEATURES.md` | +| Changed command behavior or output | `docs/USER-GUIDE.md`, `docs/COMMANDS.md` | +| Configuration / schema change | `docs/CONFIGURATION.md` | +| Architectural change | `docs/ARCHITECTURE.md`, `docs/adr/` | +| Agent or skill change | `docs/AGENTS.md` | +| Removed command, flag, or workflow | All docs that referenced it | + +### Language policy + +All content in `docs/` and the root `README.md` **must be written in English**. English is the canonical source. The translated READMEs (`README.pt-BR.md`, `README.zh-CN.md`, `README.ja-JP.md`, `README.ko-KR.md`) are community-maintained translations and do not need to be updated by every PR. + +### CI enforcement + +The `Docs Required` workflow (`scripts/lint-docs-required.cjs`) reads the changeset fragments touched in the PR diff. If any has type `Added` / `Changed` / `Deprecated` / `Removed`, it requires at least one file under `docs/` to also appear in the diff. + +### Opt-outs (with paper trail) + +When a change genuinely has no user-facing documentation impact (infrastructure rewrite, internal refactor, test-only addition, CI fix), use one of: + +- **Label:** add the `no-docs` label to the PR. Leave a comment explaining why no docs update was needed. +- **Per-fragment marker:** add `` **on its own line** inside the body of each triggering changeset fragment (typically at the end). The reason is **required and must be non-empty** — a bare `` or `` is rejected (no audit trail = no exemption). The marker is extracted at parse time by `scripts/changeset/parse.cjs` and stripped from the body before the CHANGELOG.md and GitHub release-notes serializers see it — it leaves a paper trail in the source fragment without leaking into published release notes. Inline mentions of the marker syntax (e.g. inside backticks) are intentionally ignored; the parser only acts on a marker that occupies its own line. Both routes leave a paper trail; the label is global, the marker is per-fragment for mixed PRs. + +When unsure whether a change is user-facing, **update the docs**. + ## Testing Standards All tests use Node.js built-in test runner (`node:test`) and assertion library (`node:assert`). **Do not use Jest, Mocha, Chai, or any external test framework.** diff --git a/package.json b/package.json index 0a6ac5075..6443d2056 100644 --- a/package.json +++ b/package.json @@ -73,6 +73,7 @@ "lint:tests": "node scripts/lint-no-source-grep.cjs", "lint:pr-checks": "node scripts/lint-pr-check-project-dir.cjs", "lint:changeset": "node scripts/changeset/lint.cjs", + "lint:docs": "node scripts/lint-docs-required.cjs", "changeset": "node scripts/changeset/new.cjs", "changelog:render": "node scripts/changeset/cli.cjs render", "test": "node scripts/run-tests.cjs", diff --git a/scripts/changeset/parse.cjs b/scripts/changeset/parse.cjs index b35ceacff..4787b0cfc 100644 --- a/scripts/changeset/parse.cjs +++ b/scripts/changeset/parse.cjs @@ -9,9 +9,15 @@ * --- * * - * Returns { ok: true, fragment: { type, pr, body } } on success, + * Returns { ok: true, fragment: { type, pr, body, docsExempt } } on success, * { ok: false, reason: FRAGMENT_ERROR.X, detail } on failure. * + * `docsExempt` is `null` when the body contains no docs-exempt marker, or the + * trimmed reason string when the body contains `` + * (#3213). The marker is stripped from `body` at parse time so it never bleeds + * into the CHANGELOG.md or GitHub release-notes serializers, which append the + * `(#NNNN)` PR suffix verbatim to the body's last line. + * * The reason field is a frozen enum so tests assert on stable codes, * not free-text error messages (CONTRIBUTING.md: "Prohibited: Raw * Text Matching on Test Outputs"). @@ -27,6 +33,46 @@ const FRAGMENT_ERROR = Object.freeze({ const ALLOWED_TYPES = new Set(['Added', 'Changed', 'Deprecated', 'Removed', 'Fixed', 'Security']); +// HTML comment marking a fragment as exempt from the docs-required lint (#3213). +// Form: ``. The reason is the *required* human +// audit trail — without it the exemption has no paper-trail value, so a bare +// `` or empty `` is intentionally +// rejected (the colon and a non-whitespace first reason char are mandatory). +// +// Anchored with `^...$` + `m` flag so the marker only counts when it occupies +// its own line. Inline mentions inside paragraphs (e.g. backtick-wrapped +// syntax examples in documentation) are not matched — they cannot +// accidentally exempt a fragment. +// +// The trailing `\r?` consumes the CR character of a CRLF line terminator, +// which the `$` boundary (multiline mode) does not — so Windows-authored +// fragments produce the same `body` shape as LF-authored ones. The reason +// character class `[^\r\n>]` excludes `\r` for the same reason: a CRLF +// fragment's reason text never carries a trailing `\r`. +// +// Bounded character class `[^\r\n>]` keeps the regex linear-time — no +// catastrophic backtracking on adversarial input. The leading `\S` anchor +// inside the capture group forces at least one non-whitespace character in +// the reason; trailing whitespace before `-->` is consumed by the outer +// `[ \t]*-->` and is not part of the captured reason. +const DOCS_EXEMPT_RE = /^[ \t]*[ \t]*\r?$/im; + +function extractDocsExempt(body) { + const m = body.match(DOCS_EXEMPT_RE); + if (!m) return { docsExempt: null, body }; + const reason = (m[1] || '').trim(); + // Strip the marker line and tidy up the surrounding whitespace. The cleanup + // is CRLF-aware so Windows-authored fragments don't leave residual `\r` + // characters that would shift the `(#NNNN)` PR suffix to a blank line in + // the rendered CHANGELOG.md / GitHub release-notes bullet. + const cleaned = body + .replace(DOCS_EXEMPT_RE, '') + .replace(/[ \t\r]+$/gm, '') // strip trailing \r/spaces on each line + .replace(/(?:\r?\n){3,}/g, '\n\n') // collapse 3+ blank lines (CRLF-aware) + .replace(/[\r\n]+$/, ''); // strip every trailing line terminator + return { docsExempt: reason, body: cleaned }; +} + function parseFragment(src) { const fmMatch = src.match(/^---\r?\n([\s\S]*?)\r?\n---\r?\n([\s\S]*)$/); if (!fmMatch) return { ok: false, reason: FRAGMENT_ERROR.MISSING_FRONTMATTER }; @@ -49,12 +95,20 @@ function parseFragment(src) { } // Use trim() only for the emptiness check; preserve the body verbatim // (including significant leading/trailing whitespace, code blocks, etc.) - // so render → serialize round-trips exactly. Strip only a single trailing - // newline added by editors so byte-equality holds for typical fragments. + // so render → serialize round-trips exactly. Strip the single trailing + // line terminator added by editors so byte-equality holds for typical + // fragments. CRLF-aware: a Windows-authored fragment trims `\r\n` so the + // marker line in extractDocsExempt does not leave residual `\r` characters + // for downstream serializers to attach `(#NNNN)` to (#3213). if (!body.trim()) return { ok: false, reason: FRAGMENT_ERROR.EMPTY_BODY }; - const verbatimBody = body.endsWith('\n') ? body.slice(0, -1) : body; + let verbatimBody; + if (body.endsWith('\r\n')) verbatimBody = body.slice(0, -2); + else if (body.endsWith('\n')) verbatimBody = body.slice(0, -1); + else verbatimBody = body; + const { docsExempt, body: visibleBody } = extractDocsExempt(verbatimBody); + if (!visibleBody.trim()) return { ok: false, reason: FRAGMENT_ERROR.EMPTY_BODY }; - return { ok: true, fragment: { type: fields.type, pr, body: verbatimBody } }; + return { ok: true, fragment: { type: fields.type, pr, body: visibleBody, docsExempt } }; } -module.exports = { parseFragment, FRAGMENT_ERROR, ALLOWED_TYPES }; +module.exports = { parseFragment, extractDocsExempt, FRAGMENT_ERROR, ALLOWED_TYPES, DOCS_EXEMPT_RE }; diff --git a/scripts/lint-docs-required.cjs b/scripts/lint-docs-required.cjs new file mode 100755 index 000000000..752685b03 --- /dev/null +++ b/scripts/lint-docs-required.cjs @@ -0,0 +1,222 @@ +#!/usr/bin/env node +'use strict'; + +/** + * Docs-required lint (#3213). + * + * Mirrors scripts/changeset/lint.cjs. Pure verdict function + * evaluateLint({ changedFiles, fragments, labels, malformed }) returns + * { ok, reason, triggering } using the LINT_REASON enum. The CLI wrapper + * reads the PR diff (`git diff --name-only origin/${base}...HEAD`), parses + * each touched `.changeset/*.md` fragment, then calls evaluateLint. + * + * Tests assert on the structured verdict, never on free text. + */ + +const { parseFragment, FRAGMENT_ERROR } = require('./changeset/parse.cjs'); + +const LINT_REASON = Object.freeze({ + OK_NO_TRIGGERING_FRAGMENTS: 'ok_no_triggering_fragments', + OK_DOCS_UPDATED: 'ok_docs_updated', + OK_OPT_OUT_LABEL: 'ok_opt_out_label', + OK_FRAGMENTS_EXEMPT: 'ok_fragments_exempt', + FAIL_DOCS_MISSING: 'fail_docs_missing', + FAIL_MALFORMED_FRAGMENT: 'fail_malformed_fragment', +}); + +const OPT_OUT_LABEL = 'no-docs'; + +// Fragment types that require a docs update. `Fixed` and `Security` are +// bug-class — they describe regressions or vulnerabilities, not new +// behavior to document. +const TRIGGERING_TYPES = new Set(['Added', 'Changed', 'Deprecated', 'Removed']); + +const DOCS_PREFIX = 'docs/'; + +function isFragmentPath(file) { + return /^\.changeset\/[^/]+\.md$/.test(file) && !file.endsWith('/README.md'); +} + +function isDocsFile(file) { + return file.startsWith(DOCS_PREFIX); +} + +// Per-fragment escape hatch: parse.cjs extracts `` +// from the body into `fragment.docsExempt` (a non-empty reason string when the +// marker was present and well-formed; `null` otherwise). A non-empty audit +// trail is required — the lint defends in depth here too: even if a caller +// constructs a fragment with `docsExempt: ''`, that does not count as exempt. +function isExemptFragment(fragment) { + return typeof fragment.docsExempt === 'string' && fragment.docsExempt.trim().length > 0; +} + +/** + * Pure verdict — no fs, no git. + * + * Malformed fragments fail closed: a triggering fragment with bad frontmatter + * cannot silently bypass docs enforcement. The changeset-required lint only + * checks fragment _presence_, not _validity_, so docs lint takes responsibility + * for any fragment it tries to consume. + * + * @param {object} args + * @param {string[]} args.changedFiles - file paths changed in the PR + * @param {Array<{ path: string, type: string, body: string, docsExempt: string|null }>} args.fragments + * - parsed records for well-formed `.changeset/*.md` files in `changedFiles` + * @param {Array<{ path: string, reason: string }>} [args.malformed] + * - records for `.changeset/*.md` files that failed `parseFragment` + * @param {string[]} args.labels - PR labels + * @returns {{ ok: boolean, reason: string, triggering: string[], malformed?: Array<{path:string,reason:string}> }} + */ +function evaluateLint({ changedFiles, fragments, labels, malformed = [] }) { + if (malformed.length > 0) { + return { + ok: false, + reason: LINT_REASON.FAIL_MALFORMED_FRAGMENT, + triggering: [], + malformed, + }; + } + + const triggering = fragments.filter((f) => TRIGGERING_TYPES.has(f.type)); + const triggeringPaths = triggering.map((f) => f.path); + + if (triggering.length === 0) { + return { ok: true, reason: LINT_REASON.OK_NO_TRIGGERING_FRAGMENTS, triggering: [] }; + } + + // Per-fragment exempt path: every triggering fragment must carry the marker. + // Partial exemption fails closed — one un-marked Added fragment still requires docs. + if (triggering.every(isExemptFragment)) { + return { ok: true, reason: LINT_REASON.OK_FRAGMENTS_EXEMPT, triggering: triggeringPaths }; + } + + if (labels.includes(OPT_OUT_LABEL)) { + return { ok: true, reason: LINT_REASON.OK_OPT_OUT_LABEL, triggering: triggeringPaths }; + } + + if (changedFiles.some(isDocsFile)) { + return { ok: true, reason: LINT_REASON.OK_DOCS_UPDATED, triggering: triggeringPaths }; + } + + return { ok: false, reason: LINT_REASON.FAIL_DOCS_MISSING, triggering: triggeringPaths }; +} + +function readFragmentsFromDisk(changedFiles, rootDir) { + const fs = require('node:fs'); + const path = require('node:path'); + const fragments = []; + const malformed = []; + for (const rel of changedFiles) { + if (!isFragmentPath(rel)) continue; + const abs = path.join(rootDir, rel); + if (!fs.existsSync(abs)) continue; // fragment deleted in PR — skip + let src; + try { + src = fs.readFileSync(abs, 'utf8'); + } catch (e) { + malformed.push({ path: rel, reason: 'read_error', detail: e.code || e.message }); + continue; + } + const parsed = parseFragment(src); + if (!parsed.ok) { + malformed.push({ path: rel, reason: parsed.reason, detail: parsed.detail || null }); + continue; + } + fragments.push({ + path: rel, + type: parsed.fragment.type, + body: parsed.fragment.body, + docsExempt: parsed.fragment.docsExempt, + }); + } + return { fragments, malformed }; +} + +function main() { + const fs = require('node:fs'); + const cp = require('node:child_process'); + const path = require('node:path'); + + const rootDir = path.join(__dirname, '..'); + + const eventPath = process.env.GITHUB_EVENT_PATH; + let labels = []; + if (eventPath && fs.existsSync(eventPath)) { + try { + const event = JSON.parse(fs.readFileSync(eventPath, 'utf8')); + labels = (event.pull_request?.labels || []).map((l) => l.name); + } catch { /* fall through */ } + } + + const base = process.env.GITHUB_BASE_REF || 'main'; + let changedFiles = []; + try { + // execFileSync with argv — no shell, so a malicious GITHUB_BASE_REF + // cannot inject shell syntax. Git's own ref-name validator rejects + // any metacharacters it would otherwise interpret. + const out = cp.execFileSync( + 'git', + ['diff', '--name-only', `origin/${base}...HEAD`], + { encoding: 'utf8', cwd: rootDir }, + ); + changedFiles = out.split('\n').filter(Boolean); + } catch (e) { + process.stderr.write(`could not compute diff: ${e.message}\n`); + process.exit(2); + } + + const { fragments, malformed } = readFragmentsFromDisk(changedFiles, rootDir); + const verdict = evaluateLint({ changedFiles, fragments, labels, malformed }); + + if (process.argv.includes('--json')) { + process.stdout.write( + JSON.stringify({ ...verdict, changedFiles, fragments, malformed, labels }, null, 2) + '\n', + ); + } else if (verdict.ok) { + process.stdout.write(`ok docs-lint: ${verdict.reason}\n`); + } else if (verdict.reason === LINT_REASON.FAIL_MALFORMED_FRAGMENT) { + process.stderr.write(`\nERROR docs-lint: ${verdict.reason}\n`); + process.stderr.write( + `${malformed.length} changeset fragment(s) failed to parse — docs lint cannot consume them:\n`, + ); + for (const m of malformed) { + process.stderr.write(` ${m.path} (reason: ${m.reason}${m.detail ? `, detail: ${m.detail}` : ''})\n`); + } + process.stderr.write( + `\nFix the fragment frontmatter (\`type:\` + \`pr:\`) before this PR can pass.\n`, + ); + } else { + process.stderr.write(`\nERROR docs-lint: ${verdict.reason}\n`); + process.stderr.write( + `${verdict.triggering.length} changeset fragment(s) require documentation updates:\n`, + ); + for (const f of fragments.filter((f) => TRIGGERING_TYPES.has(f.type))) { + process.stderr.write(` ${f.path} (type: ${f.type})\n`); + } + process.stderr.write(`\nNo files under docs/ were modified in this PR.\n\n`); + process.stderr.write( + `Update the relevant docs/ file(s), or add the \`${OPT_OUT_LABEL}\` label if this change\n`, + ); + process.stderr.write( + `is genuinely internal-only (infrastructure, refactor, test-only). Per-fragment\n`, + ); + process.stderr.write( + `exemption via \`\` inside the fragment body also works.\n`, + ); + } + process.exit(verdict.ok ? 0 : 1); +} + +if (require.main === module) main(); + +module.exports = { + evaluateLint, + readFragmentsFromDisk, + LINT_REASON, + OPT_OUT_LABEL, + TRIGGERING_TYPES, + FRAGMENT_ERROR, + isFragmentPath, + isDocsFile, + isExemptFragment, +}; diff --git a/tests/changeset-new.test.cjs b/tests/changeset-new.test.cjs index 9bc3bd5ce..eb2adbddd 100644 --- a/tests/changeset-new.test.cjs +++ b/tests/changeset-new.test.cjs @@ -52,6 +52,7 @@ describe('changeset new: name generator + scaffold writer (#2975)', () => { type: 'Fixed', pr: 9999, body: 'this is a placeholder body that the contributor will replace.', + docsExempt: null, }); }); diff --git a/tests/changeset-parse.test.cjs b/tests/changeset-parse.test.cjs index ff3642d41..4c71721e0 100644 --- a/tests/changeset-parse.test.cjs +++ b/tests/changeset-parse.test.cjs @@ -5,7 +5,7 @@ const { test, describe } = require('node:test'); const assert = require('node:assert/strict'); const path = require('node:path'); -const { parseFragment, FRAGMENT_ERROR } = require(path.join(__dirname, '..', 'scripts', 'changeset', 'parse.cjs')); +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', () => { @@ -16,6 +16,7 @@ describe('changeset parse: fragment file → typed record (#2975)', () => { type: 'Fixed', pr: 2975, body: 'fix the thing.', + docsExempt: null, }); }); @@ -54,3 +55,140 @@ describe('changeset parse: fragment file → typed record (#2975)', () => { }); } }); + +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'); + 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 `` 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'); + assert.equal(out.docsExempt, null); + assert.match(out.body, /docs-exempt/); // unchanged — bare marker stays in body + }); + + test('extractDocsExempt REJECTS marker with empty reason ()', () => { + const out = extractDocsExempt('body\n'); + assert.equal(out.docsExempt, null); + }); + + test('extractDocsExempt REJECTS marker with whitespace-only reason', () => { + const out = extractDocsExempt('body\n'); + assert.equal(out.docsExempt, null); + }); + + test('extractDocsExempt is case-insensitive on the marker token', () => { + const out = extractDocsExempt('body\n'); + assert.equal(out.docsExempt, 'shouty reason'); + }); + + test('parseFragment surfaces docsExempt on the fragment record', () => { + const src = '---\ntype: Added\npr: 3213\n---\nbootstrap.\n\n\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\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_RE); + assert.match('', DOCS_EXEMPT_RE); + assert.doesNotMatch('docs-exempt: not in a comment', DOCS_EXEMPT_RE); + assert.doesNotMatch('', DOCS_EXEMPT_RE); // no colon + assert.doesNotMatch('', DOCS_EXEMPT_RE); // empty reason + assert.doesNotMatch('', 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: `` 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\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\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: `` documents the syntax.\n' + + '\n' + + '\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: /); + }); +}); diff --git a/tests/lint-docs-required.test.cjs b/tests/lint-docs-required.test.cjs new file mode 100644 index 000000000..adf8dc6b6 --- /dev/null +++ b/tests/lint-docs-required.test.cjs @@ -0,0 +1,374 @@ +'use strict'; +process.env.GSD_TEST_MODE = '1'; + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); + +const { + evaluateLint, + readFragmentsFromDisk, + LINT_REASON, + OPT_OUT_LABEL, + TRIGGERING_TYPES, + isFragmentPath, + isDocsFile, + isExemptFragment, +} = require(path.join(__dirname, '..', 'scripts', 'lint-docs-required.cjs')); + +// evaluateLint is pure over the resolved inputs (changedFiles, fragments, +// labels, malformed). Tests assert on the structured verdict: +// { ok, reason: LINT_REASON.X, triggering: string[], malformed? }. + +describe('docs-required lint: pure verdict (#3213)', () => { + test('LINT_REASON enum exposes the documented codes', () => { + assert.deepEqual( + Object.keys(LINT_REASON).sort(), + [ + 'FAIL_DOCS_MISSING', + 'FAIL_MALFORMED_FRAGMENT', + 'OK_DOCS_UPDATED', + 'OK_FRAGMENTS_EXEMPT', + 'OK_NO_TRIGGERING_FRAGMENTS', + 'OK_OPT_OUT_LABEL', + ].sort(), + ); + }); + + test('TRIGGERING_TYPES covers the four user-facing non-fix types', () => { + assert.deepEqual( + [...TRIGGERING_TYPES].sort(), + ['Added', 'Changed', 'Deprecated', 'Removed'].sort(), + ); + }); + + test('OPT_OUT_LABEL is no-docs (matches CONTRIBUTING)', () => { + assert.equal(OPT_OUT_LABEL, 'no-docs'); + }); + + test('OK_NO_TRIGGERING_FRAGMENTS when no fragments touched at all', () => { + const verdict = evaluateLint({ + changedFiles: ['bin/install.js'], + fragments: [], + labels: [], + }); + assert.equal(verdict.ok, true); + assert.equal(verdict.reason, LINT_REASON.OK_NO_TRIGGERING_FRAGMENTS); + assert.deepEqual(verdict.triggering, []); + }); + + test('OK_NO_TRIGGERING_FRAGMENTS for Fixed-only fragments (bug-class)', () => { + const verdict = evaluateLint({ + changedFiles: ['bin/install.js', '.changeset/silly-bears-dance.md'], + fragments: [ + { path: '.changeset/silly-bears-dance.md', type: 'Fixed', body: 'fix typo', docsExempt: null }, + ], + labels: [], + }); + assert.deepEqual(verdict, { + ok: true, + reason: LINT_REASON.OK_NO_TRIGGERING_FRAGMENTS, + triggering: [], + }); + }); + + test('OK_NO_TRIGGERING_FRAGMENTS for Security-only fragments', () => { + const verdict = evaluateLint({ + changedFiles: [], + fragments: [{ path: '.changeset/a.md', type: 'Security', body: 'cve', docsExempt: null }], + labels: [], + }); + assert.equal(verdict.ok, true); + assert.equal(verdict.reason, LINT_REASON.OK_NO_TRIGGERING_FRAGMENTS); + }); + + test('OK_DOCS_UPDATED when Added fragment ships alongside a docs/ change', () => { + const verdict = evaluateLint({ + changedFiles: ['.changeset/a.md', 'docs/COMMANDS.md'], + fragments: [{ path: '.changeset/a.md', type: 'Added', body: 'new cmd', docsExempt: null }], + labels: [], + }); + assert.equal(verdict.ok, true); + assert.equal(verdict.reason, LINT_REASON.OK_DOCS_UPDATED); + assert.deepEqual(verdict.triggering, ['.changeset/a.md']); + }); + + test('OK_DOCS_UPDATED for nested docs/ paths (docs/adr/, docs/agents/)', () => { + const verdict = evaluateLint({ + changedFiles: ['.changeset/a.md', 'docs/adr/0099-new.md'], + fragments: [{ path: '.changeset/a.md', type: 'Changed', body: '...', docsExempt: null }], + labels: [], + }); + assert.equal(verdict.reason, LINT_REASON.OK_DOCS_UPDATED); + }); + + for (const type of ['Added', 'Changed', 'Deprecated', 'Removed']) { + test(`FAIL_DOCS_MISSING when ${type} fragment has no docs/ change and no escape hatch`, () => { + const verdict = evaluateLint({ + changedFiles: ['.changeset/a.md', 'bin/install.js'], + fragments: [{ path: '.changeset/a.md', type, body: '...', docsExempt: null }], + labels: [], + }); + assert.equal(verdict.ok, false); + assert.equal(verdict.reason, LINT_REASON.FAIL_DOCS_MISSING); + assert.deepEqual(verdict.triggering, ['.changeset/a.md']); + }); + } + + test('OK_OPT_OUT_LABEL when no-docs label present overrides triggering fragments', () => { + const verdict = evaluateLint({ + changedFiles: ['.changeset/a.md', 'bin/install.js'], + fragments: [{ path: '.changeset/a.md', type: 'Added', body: '...', docsExempt: null }], + labels: ['no-docs'], + }); + assert.equal(verdict.ok, true); + assert.equal(verdict.reason, LINT_REASON.OK_OPT_OUT_LABEL); + }); + + test('per-fragment docsExempt reason exempts that fragment', () => { + const verdict = evaluateLint({ + changedFiles: ['.changeset/a.md', 'bin/install.js'], + fragments: [ + { path: '.changeset/a.md', type: 'Added', body: 'foo', docsExempt: 'internal-only' }, + ], + labels: [], + }); + assert.equal(verdict.ok, true); + assert.equal(verdict.reason, LINT_REASON.OK_FRAGMENTS_EXEMPT); + assert.deepEqual(verdict.triggering, ['.changeset/a.md']); + }); + + test('docsExempt empty string does NOT exempt — defense-in-depth (CodeRabbit finding)', () => { + // parse.cjs no longer produces empty-string docsExempt (the marker regex + // requires a non-empty reason). evaluateLint defends against any caller + // that constructs a fragment with `docsExempt: ''` directly — empty or + // whitespace-only reasons are not a valid audit trail. + const verdict = evaluateLint({ + changedFiles: ['.changeset/a.md', 'bin/install.js'], + fragments: [ + { path: '.changeset/a.md', type: 'Added', body: 'foo', docsExempt: '' }, + ], + labels: [], + }); + assert.equal(verdict.ok, false); + assert.equal(verdict.reason, LINT_REASON.FAIL_DOCS_MISSING); + }); + + test('docsExempt whitespace-only does NOT exempt — defense-in-depth', () => { + const verdict = evaluateLint({ + changedFiles: ['.changeset/a.md', 'bin/install.js'], + fragments: [ + { path: '.changeset/a.md', type: 'Added', body: 'foo', docsExempt: ' \t' }, + ], + labels: [], + }); + assert.equal(verdict.reason, LINT_REASON.FAIL_DOCS_MISSING); + }); + + test('partial exemption fails — one un-marked triggering fragment is enough to require docs', () => { + const verdict = evaluateLint({ + changedFiles: ['.changeset/a.md', '.changeset/b.md', 'bin/install.js'], + fragments: [ + { path: '.changeset/a.md', type: 'Added', body: 'foo', docsExempt: 'x' }, + { path: '.changeset/b.md', type: 'Changed', body: 'no marker here', docsExempt: null }, + ], + labels: [], + }); + assert.equal(verdict.ok, false); + assert.equal(verdict.reason, LINT_REASON.FAIL_DOCS_MISSING); + assert.deepEqual(verdict.triggering.sort(), ['.changeset/a.md', '.changeset/b.md']); + }); + + test('mixed Fixed + Added with no docs still fails — Added triggers', () => { + const verdict = evaluateLint({ + changedFiles: ['.changeset/a.md', '.changeset/b.md'], + fragments: [ + { path: '.changeset/a.md', type: 'Fixed', body: '...', docsExempt: null }, + { path: '.changeset/b.md', type: 'Added', body: '...', docsExempt: null }, + ], + labels: [], + }); + assert.equal(verdict.ok, false); + assert.equal(verdict.reason, LINT_REASON.FAIL_DOCS_MISSING); + assert.deepEqual(verdict.triggering, ['.changeset/b.md']); + }); +}); + +describe('docs-required lint: malformed fragments fail closed (#3213, Codex finding)', () => { + test('FAIL_MALFORMED_FRAGMENT when a touched fragment failed to parse', () => { + const verdict = evaluateLint({ + changedFiles: ['.changeset/bad.md'], + fragments: [], + labels: [], + malformed: [{ path: '.changeset/bad.md', reason: 'missing_frontmatter' }], + }); + assert.equal(verdict.ok, false); + assert.equal(verdict.reason, LINT_REASON.FAIL_MALFORMED_FRAGMENT); + assert.deepEqual(verdict.malformed, [{ path: '.changeset/bad.md', reason: 'missing_frontmatter' }]); + }); + + test('FAIL_MALFORMED_FRAGMENT outranks OK_DOCS_UPDATED — malformed must be fixed first', () => { + const verdict = evaluateLint({ + changedFiles: ['.changeset/bad.md', '.changeset/ok.md', 'docs/USER-GUIDE.md'], + fragments: [{ path: '.changeset/ok.md', type: 'Added', body: 'fine', docsExempt: null }], + labels: ['no-docs'], + malformed: [{ path: '.changeset/bad.md', reason: 'invalid_type', detail: 'Bogus' }], + }); + assert.equal(verdict.ok, false); + assert.equal(verdict.reason, LINT_REASON.FAIL_MALFORMED_FRAGMENT); + }); + + test('no-docs label cannot bypass FAIL_MALFORMED_FRAGMENT', () => { + const verdict = evaluateLint({ + changedFiles: ['.changeset/bad.md'], + fragments: [], + labels: ['no-docs'], + malformed: [{ path: '.changeset/bad.md', reason: 'missing_pr' }], + }); + assert.equal(verdict.reason, LINT_REASON.FAIL_MALFORMED_FRAGMENT); + }); + + test('malformed defaults to [] when omitted — back-compat with simple test inputs', () => { + const verdict = evaluateLint({ + changedFiles: [], + fragments: [], + labels: [], + }); + assert.equal(verdict.ok, true); + assert.equal(verdict.reason, LINT_REASON.OK_NO_TRIGGERING_FRAGMENTS); + }); +}); + +describe('docs-required lint: helpers', () => { + test('isFragmentPath accepts .changeset/.md, rejects README', () => { + assert.equal(isFragmentPath('.changeset/foo.md'), true); + assert.equal(isFragmentPath('.changeset/silly-bears-dance.md'), true); + assert.equal(isFragmentPath('.changeset/README.md'), false); + assert.equal(isFragmentPath('.changeset/nested/foo.md'), false); + assert.equal(isFragmentPath('docs/COMMANDS.md'), false); + assert.equal(isFragmentPath('bin/install.js'), false); + }); + + test('isDocsFile matches docs/ prefix only', () => { + assert.equal(isDocsFile('docs/COMMANDS.md'), true); + assert.equal(isDocsFile('docs/adr/0001-foo.md'), true); + assert.equal(isDocsFile('docs/agents/triage-labels.md'), true); + assert.equal(isDocsFile('docs'), false); // exact 'docs' without slash is not a file under docs/ + assert.equal(isDocsFile('CONTRIBUTING.md'), false); + assert.equal(isDocsFile('README.md'), false); + }); + + test('isExemptFragment checks docsExempt is a non-empty string, not body content', () => { + assert.equal(isExemptFragment({ docsExempt: 'reason' }), true); + assert.equal(isExemptFragment({ docsExempt: 'a' }), true); + // Empty/whitespace-only reason → no audit trail → not exempt. + assert.equal(isExemptFragment({ docsExempt: '' }), false); + assert.equal(isExemptFragment({ docsExempt: ' \t' }), false); + assert.equal(isExemptFragment({ docsExempt: null }), false); + assert.equal(isExemptFragment({ docsExempt: undefined }), false); + assert.equal(isExemptFragment({}), false); + // Body content is irrelevant — parse.cjs extracts the marker into docsExempt. + assert.equal( + isExemptFragment({ body: '', docsExempt: null }), + false, + ); + }); +}); + +describe('docs-required lint: readFragmentsFromDisk', () => { + function withTempRepo(fn) { + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-docs-lint-')); + try { + fs.mkdirSync(path.join(tmp, '.changeset'), { recursive: true }); + fn(tmp); + } finally { + fs.rmSync(tmp, { recursive: true, force: true }); + } + } + + test('returns { fragments, malformed } shape', () => { + withTempRepo((tmp) => { + const out = readFragmentsFromDisk([], tmp); + assert.ok('fragments' in out, 'has fragments'); + assert.ok('malformed' in out, 'has malformed'); + assert.deepEqual(out.fragments, []); + assert.deepEqual(out.malformed, []); + }); + }); + + test('parses valid fragments and skips non-fragment paths', () => { + withTempRepo((tmp) => { + fs.writeFileSync( + path.join(tmp, '.changeset', 'a.md'), + '---\ntype: Added\npr: 1\n---\nnew feature\n', + ); + fs.writeFileSync( + path.join(tmp, '.changeset', 'b.md'), + '---\ntype: Fixed\npr: 2\n---\nbug fix\n', + ); + const { fragments, malformed } = readFragmentsFromDisk( + ['.changeset/a.md', '.changeset/b.md', 'bin/x.js'], + tmp, + ); + assert.equal(fragments.length, 2); + assert.equal(fragments[0].path, '.changeset/a.md'); + assert.equal(fragments[0].type, 'Added'); + assert.equal(fragments[0].docsExempt, null); + assert.equal(fragments[1].type, 'Fixed'); + assert.deepEqual(malformed, []); + }); + }); + + test('skips deleted fragments (path in diff but file gone)', () => { + withTempRepo((tmp) => { + const { fragments, malformed } = readFragmentsFromDisk(['.changeset/deleted.md'], tmp); + assert.deepEqual(fragments, []); + assert.deepEqual(malformed, []); + }); + }); + + test('routes malformed fragments to the malformed list with typed reason', () => { + withTempRepo((tmp) => { + fs.writeFileSync(path.join(tmp, '.changeset', 'bad.md'), 'no frontmatter here\n'); + const { fragments, malformed } = readFragmentsFromDisk(['.changeset/bad.md'], tmp); + assert.deepEqual(fragments, []); + assert.equal(malformed.length, 1); + assert.equal(malformed[0].path, '.changeset/bad.md'); + assert.equal(malformed[0].reason, 'missing_frontmatter'); + }); + }); + + test('Added fragment with bad pr surfaces as malformed (Codex finding regression test)', () => { + withTempRepo((tmp) => { + fs.writeFileSync( + path.join(tmp, '.changeset', 'a.md'), + '---\ntype: Added\n---\nbody but no pr field\n', + ); + const { fragments, malformed } = readFragmentsFromDisk(['.changeset/a.md'], tmp); + assert.deepEqual(fragments, []); + assert.equal(malformed.length, 1); + assert.equal(malformed[0].reason, 'missing_pr'); + // End-to-end: feed straight into evaluateLint and confirm fail-closed. + const verdict = evaluateLint({ changedFiles: ['.changeset/a.md'], fragments, malformed, labels: [] }); + assert.equal(verdict.reason, LINT_REASON.FAIL_MALFORMED_FRAGMENT); + }); + }); + + test('extracts docs-exempt marker into typed field and strips it from body', () => { + withTempRepo((tmp) => { + fs.writeFileSync( + path.join(tmp, '.changeset', 'a.md'), + '---\ntype: Added\npr: 3\n---\nnew thing\n\n\n', + ); + const { fragments } = readFragmentsFromDisk(['.changeset/a.md'], tmp); + assert.equal(fragments.length, 1); + assert.equal(fragments[0].docsExempt, 'internal-only'); + // The marker no longer appears in the rendered body — renderers append + // `(#NNNN)` to body's last line, so the marker would otherwise leak into + // CHANGELOG.md / GitHub release notes. + assert.doesNotMatch(fragments[0].body, /docs-exempt/); + }); + }); +});