-
Notifications
You must be signed in to change notification settings - Fork 7
fix(ci): require pre-merge CHANGELOG PR-reference for governed changes #705
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
cf50e4d
fix(ci): require pre-merge CHANGELOG PR-reference for governed changes
qnbs 5787825
docs: reference PR #705 in the CHANGELOG PR-admission gate entry
qnbs c1ab3a5
test: reduce duplication in checkPrChangelogReference regression tests
qnbs dec4127
docs: sync README test-count metrics after test-file refactor
qnbs 3ce40e9
fix(ci): scope CHANGELOG PR-reference check to actual bullet entries
qnbs b052825
fix(ci): close two review-found bypasses in the CHANGELOG PR-referenc…
qnbs e0a4ef2
fix(ci): strip comments before locating the [Unreleased] heading
qnbs 7cc65d9
fix(ci): reject malformed PR metadata and generalize bullet-continuat…
qnbs b02958d
refactor(ci): extract isIndentedContinuation to simplify extractBulle…
qnbs File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| name: PR CHANGELOG Reference Guard | ||
| on: | ||
| pull_request: | ||
|
qnbs marked this conversation as resolved.
|
||
| types: [opened, edited, synchronize, reopened] | ||
| branches: [main] | ||
| permissions: | ||
| contents: read | ||
| jobs: | ||
| check: | ||
| name: Require this PR's own number in CHANGELOG.md [Unreleased] before merge | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 3 | ||
| steps: | ||
| - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | ||
| with: | ||
| fetch-depth: 0 | ||
| persist-credentials: false | ||
| - name: Require [Unreleased] to reference this PR before merge | ||
| env: | ||
| GITHUB_EVENT_PATH: ${{ github.event_path }} | ||
| run: | | ||
| set -euo pipefail | ||
| BASE="${{ github.event.pull_request.base.sha }}" | ||
| mkdir -p /tmp/base-scripts | ||
| CHECKER=scripts/check-pr-changelog-reference.mjs | ||
| if git show "$BASE:scripts/check-pr-changelog-reference.mjs" > /tmp/base-scripts/check-pr-changelog-reference.mjs 2>/dev/null; then | ||
| CHECKER=/tmp/base-scripts/check-pr-changelog-reference.mjs | ||
|
qnbs marked this conversation as resolved.
|
||
| else | ||
| echo "::notice::check-pr-changelog-reference.mjs not found on base ref (bootstrap PR) — using this PR's own copy this one time." | ||
|
qnbs marked this conversation as resolved.
|
||
| fi | ||
| node "$CHECKER" | ||
|
qnbs marked this conversation as resolved.
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| export function isReferencedByPrLabel(prNumber: number, text: string): boolean; | ||
|
|
||
| export interface CheckPrChangelogReferenceInput { | ||
| prNumber: number; | ||
| prTitle: string | undefined | null; | ||
| changelog: string | undefined | null; | ||
| } | ||
|
|
||
| export type CheckPrChangelogReferenceReason = 'not-governed' | 'referenced' | 'missing-reference'; | ||
|
|
||
| export interface CheckPrChangelogReferenceResult { | ||
| ok: boolean; | ||
| reason: CheckPrChangelogReferenceReason; | ||
| } | ||
|
|
||
| export function checkPrChangelogReference( | ||
| input: CheckPrChangelogReferenceInput, | ||
| ): CheckPrChangelogReferenceResult; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,133 @@ | ||
| #!/usr/bin/env node | ||
| /** | ||
| * CI-only pre-merge admission gate: before a governed (feat|fix|perf) PR can merge, its own | ||
| * CHANGELOG.md [Unreleased] section must already reference this PR's real GitHub-assigned number | ||
| * as "PR #<N>". This closes the blind spot where scripts/check-doc-metrics.mjs's completeness | ||
| * check only fires AFTER squash-merge, once the commit is on main and its subject already carries | ||
| * "(#N)" — a gap that has recurred three times (#678->#679, #684->#685, #699->#700), each requiring | ||
| * a same-pattern follow-up PR to add the missing reference after the fact. | ||
| * | ||
| * Deliberately self-contained (no local imports, mirrors check-commit-attribution.mjs) so the | ||
| * base-ref self-grading copy in .github/workflows/pr-changelog-reference.yml never breaks on a | ||
| * missing transitive dependency (check-doc-metrics.mjs itself imports two further local modules | ||
| * that would also need copying and keeping in sync). | ||
| */ | ||
| import { readFileSync } from 'node:fs'; | ||
| import process from 'node:process'; | ||
| import { fileURLToPath } from 'node:url'; | ||
|
|
||
| // QNBS-v3: duplicated from check-doc-metrics.mjs's GOVERNED_COMMIT_TYPE (kept in sync manually, not via import) so this file has zero local dependencies — see file header. | ||
| const GOVERNED_COMMIT_TYPE = /^(?:feat|fix|perf)(?:\([^)]*\))?!?:\s*/i; | ||
|
|
||
| // QNBS-v3: strips comments from the WHOLE document before searching for the heading — a commented-out template containing a literal "## [Unreleased]" line earlier in the file would otherwise hijack the section boundary, since slicing off the opening "<!--" before comment-removal runs left the fake section's own content unstrippable. | ||
| function getUnreleasedSectionText(changelog) { | ||
| const withoutComments = changelog.replace(/<!--[\s\S]*?(?:-->|$)/g, ''); | ||
|
qnbs marked this conversation as resolved.
|
||
| const heading = /^## \[Unreleased\]\s*$/m.exec(withoutComments); | ||
| if (!heading) return ''; | ||
| const afterHeading = withoutComments.slice(heading.index + heading[0].length); | ||
| const nextHeading = afterHeading.search(/^##\s/m); | ||
| return nextHeading === -1 ? afterHeading : afterHeading.slice(0, nextHeading); | ||
| } | ||
|
|
||
| // QNBS-v3: duplicated from check-doc-metrics.mjs's splitUnreleasedEntries for the same self-containment reason — joins a bullet's own soft-wrapped continuation lines into one entry. | ||
| function extractBulletEntries(unreleasedSection) { | ||
| const entries = []; | ||
| let current = []; | ||
| const flush = () => { | ||
| if (current.length > 0) entries.push(current.join(' ')); | ||
| current = []; | ||
| }; | ||
| for (const rawLine of unreleasedSection.split('\n')) { | ||
| const line = rawLine.trim(); | ||
| if (/^-\s/.test(line)) { | ||
| flush(); | ||
| current.push(line); | ||
| } else if (/^#{1,6}\s/.test(line)) { | ||
| flush(); | ||
| } else if (current.length > 0 && line !== '') { | ||
|
cubic-dev-ai[bot] marked this conversation as resolved.
Outdated
|
||
| current.push(line); | ||
|
qnbs marked this conversation as resolved.
Outdated
qnbs marked this conversation as resolved.
Outdated
|
||
| } else if (line === '') { | ||
| flush(); | ||
| } | ||
| } | ||
| flush(); | ||
| return entries; | ||
| } | ||
|
|
||
| // QNBS-v3: exact "PR #NNN" grammar with a full trailing word boundary (rejects "PR #705alpha"/"PR #705_"), stricter than check-doc-metrics.mjs's post-merge bare "#NNN" matcher since pre-merge there is no squash-appended "(#NNN)" to anchor on. | ||
| export function isReferencedByPrLabel(prNumber, text) { | ||
| return new RegExp(`\\bPR\\s*#${prNumber}(?!\\w)`, 'i').test(text); | ||
| } | ||
|
|
||
| /** Pure decision function — kept separate from I/O so it is directly unit-testable. */ | ||
| export function checkPrChangelogReference({ prNumber, prTitle, changelog }) { | ||
| if (!GOVERNED_COMMIT_TYPE.test(prTitle ?? '')) { | ||
| return { ok: true, reason: 'not-governed' }; | ||
| } | ||
| // QNBS-v3: scoped to actual bullet entries, not the whole section — a PR number floating in prose or a sub-heading (not inside a real release-note bullet) must not count as documentation. | ||
| const unreleasedSection = getUnreleasedSectionText(changelog ?? ''); | ||
| const bulletEntries = extractBulletEntries(unreleasedSection); | ||
| return bulletEntries.some((entry) => isReferencedByPrLabel(prNumber, entry)) | ||
| ? { ok: true, reason: 'referenced' } | ||
|
qnbs marked this conversation as resolved.
|
||
| : { ok: false, reason: 'missing-reference' }; | ||
| } | ||
|
|
||
| function main() { | ||
| const eventPath = process.env.GITHUB_EVENT_PATH; | ||
| if (!eventPath) { | ||
| console.log('[check-pr-changelog-reference] no GITHUB_EVENT_PATH — skipping'); | ||
| process.exit(0); | ||
| } | ||
|
|
||
| let payload; | ||
| try { | ||
| payload = JSON.parse(readFileSync(eventPath, 'utf8')); | ||
| } catch (error) { | ||
| console.error( | ||
| `[check-pr-changelog-reference] cannot read event payload: ${error instanceof Error ? error.message : 'invalid JSON'}`, | ||
| ); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| const pr = payload.pull_request; | ||
| if (!pr) { | ||
| console.log('[check-pr-changelog-reference] not a pull_request event — skipping'); | ||
| process.exit(0); | ||
| } | ||
| if (typeof pr.number !== 'number') { | ||
| console.error( | ||
| '[check-pr-changelog-reference] pull_request event payload is missing a numeric "number" field', | ||
| ); | ||
| process.exit(1); | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| let changelog; | ||
| try { | ||
| changelog = readFileSync('CHANGELOG.md', 'utf8'); | ||
| } catch (error) { | ||
| console.error( | ||
| `[check-pr-changelog-reference] cannot read CHANGELOG.md: ${error instanceof Error ? error.message : String(error)}`, | ||
| ); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| const result = checkPrChangelogReference({ prNumber: pr.number, prTitle: pr.title, changelog }); | ||
| if (result.reason === 'not-governed') { | ||
| console.log( | ||
| '[check-pr-changelog-reference] PR title is not a governed feat/fix/perf change — skipping', | ||
| ); | ||
| process.exit(0); | ||
| } | ||
| if (!result.ok) { | ||
| console.error( | ||
| `[check-pr-changelog-reference] FAIL — CHANGELOG.md's [Unreleased] section does not yet reference "PR #${pr.number}". Add (or update) a bullet describing this change and reference it literally as "PR #${pr.number}" before merging.`, | ||
| ); | ||
| process.exit(1); | ||
| } | ||
| console.log(`[check-pr-changelog-reference] OK — [Unreleased] references PR #${pr.number}`); | ||
| } | ||
|
|
||
| // QNBS-v3: only run the CLI side-effect when invoked directly — checkPrChangelogReference stays importable from a unit test. | ||
| if (process.argv[1] === fileURLToPath(import.meta.url)) { | ||
|
qnbs marked this conversation as resolved.
|
||
| main(); | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.