From f790f2820a6bd5d93322ffdb2bc51ffbb9f1718c Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 5 Oct 2026 02:08:35 -0400 Subject: [PATCH 1/5] feat(scripts): script the release notes and Release creation (release:notes) scripts/release-notes.mjs replaces the release skill's step 3a shell recipe. It generates the What's Changed list with releases/generate-notes (previous stable tag to main), appends the smoke-ledger line, any known issues the maintainer passes in, and a "Thanks for helping us improve" section crediting the community authors of every issue the listed PRs close (paginated closingIssuesReferences plus closing keywords in the body), excluding maintainers by permission and bots. Every gh/git call is checked and any failure aborts, so a failed permission lookup can never credit a maintainer and a rate limit can never publish a partial Thanks list. The default run is a preview; --draft creates a draft Release and --publish publishes, both with the bare x.y.z tag and --target main, and both refuse any version other than origin/main's. A preview of 2.9.0 reproduces its published notes. Closes #2550 Co-Authored-By: Claude Opus 5.5 Signed-off-by: cliffhall --- .claude/skills/project-structure/SKILL.md | 2 +- .claude/skills/release/SKILL.md | 135 +++--- AGENTS.md | 2 +- README.md | 2 +- package.json | 1 + scripts/release-notes.mjs | 347 +++++++++++++++ scripts/release-notes.test.mjs | 493 ++++++++++++++++++++++ 7 files changed, 908 insertions(+), 74 deletions(-) create mode 100644 scripts/release-notes.mjs create mode 100644 scripts/release-notes.test.mjs diff --git a/.claude/skills/project-structure/SKILL.md b/.claude/skills/project-structure/SKILL.md index 3185ad5de..49117c7a2 100644 --- a/.claude/skills/project-structure/SKILL.md +++ b/.claude/skills/project-structure/SKILL.md @@ -26,7 +26,7 @@ inspector/ ├── test-servers/ Composable MCP test servers + JSON configs used by tests and by hand ├── scripts/ Root build/verify tooling (install cascade, smokes, the verify:* guards) │ and maintainer-workflow helpers (the pr:*, board:*, advisory:*, -│ release:tag and action:resolve-pin aliases) +│ release:notes, release:tag and action:resolve-pin aliases) ├── docs/ Task-oriented guides (see docs/README-style index in the root README) ├── specification/ Design/build specifications └── AGENTS.md The rules contract — read this before changing anything diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index a2df1574f..e15bf0bec 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -1,6 +1,6 @@ --- name: release -description: "Cut an Inspector v2 release — two PRs and then a GitHub Release. PR 1 puts the npm audit, any fixes it forces, and the version bump on v2/main; PR 2 merges v2/main into main and is smoke-tested from the production build with a ledger artifact for the maintainers; the maintainer then tags and publishes through the GitHub UI. Also covers the v1 line and what the publish jobs gate on." +description: "Cut an Inspector v2 release — two PRs and then a GitHub Release. PR 1 puts the npm audit, any fixes it forces, and the version bump on v2/main; PR 2 merges v2/main into main and is smoke-tested from the production build with a ledger artifact for the maintainers; the maintainer then drafts the Release with a script and publishes it. Also covers the v1 line and what the publish jobs gate on." disable-model-invocation: true --- @@ -206,104 +206,97 @@ the artifact the maintainers approve the merge on. ## 3. Tag and publish the Release -### 3a. Draft the release notes +### 3a. Assemble the notes and draft the Release A release's notes have four parts, in this order: 1. **What's Changed.** GitHub's generated list of every PR since the previous tag. 2. **The smoke-ledger line**, linking the artifact from 2b. -3. **`## Known issue`**, only when there is one. It names the issue, who is - affected and the workaround. Deciding what counts as a known issue is a - maintainer judgment, so it is written by hand, never generated. +3. **`## Known issue`** (or `issues`), only when there is one. It names the + issue, who is affected and the workaround. Deciding what counts as a known + issue is a maintainer judgment, so it is written by hand and passed in, + never generated. 4. **`## Thanks for helping us improve`.** Credit to the community members whose issues the release addresses. GitHub adds everyone `@`-mentioned in a release body to that release's **Contributors** avatar strip, so the people credited here appear there too (confirmed on 2.9.0). -The generated list comes from the same API the UI's *Generate release notes* -button uses, so it can be produced without creating anything. The recipe below -builds parts 1 and 4. It reproduced 2.9.0's published Thanks section exactly. +**`npm run release:notes` assembles all four and creates the Release** +(`scripts/release-notes.mjs`, #2550). It derives the version from `origin/main` +after an explicit fetch, picks the previous **stable** tag itself (this repo +also carries `-rc.N`, `-hotfix`, `-amended` and `v2-alpha-1` tags, any of which +would drop changes from the list), and asks the same `releases/generate-notes` +API the UI's *Generate release notes* button uses. Run it twice, then publish in 3b: ```sh -REPO=modelcontextprotocol/inspector -git fetch origin main --tags -VERSION=$(git show origin/main:package.json | node -p "JSON.parse(require('fs').readFileSync(0)).version") -# The highest STABLE tag below VERSION. Whole-name match: this repo also has -# 2.0.0-rc.N, x.y.z-hotfix, x.y.z-amended and v2-alpha-1 tags, and a glob -# like [0-9]*.[0-9]*.[0-9]* would pick an RC as PREV and drop changes. -PREV=$( { git tag -l | grep -E '^[0-9]+\.[0-9]+\.[0-9]+$'; echo "$VERSION"; } \ - | sort -uV | grep -B1 -x "$VERSION" | head -1 ) -echo "$PREV → $VERSION" # sanity-check both - -# 1. What's Changed, exactly as the UI generates it. -gh api "repos/$REPO/releases/generate-notes" -f tag_name="$VERSION" \ - -f target_commitish=main -f previous_tag_name="$PREV" --jq .body > release-notes.md - -# 4. Reporter credit: the author of every issue a listed PR closes, minus -# maintainers (admin/maintain/write) and bots. -for pr in $(grep -oE 'pull/[0-9]+' release-notes.md | cut -d/ -f2 | sort -un); do - gh api graphql -F n="$pr" -f query='query($n:Int!){repository(owner:"modelcontextprotocol",name:"inspector"){pullRequest(number:$n){body closingIssuesReferences(first:20){nodes{number}}}}}' \ - --jq '.data.repository.pullRequest | ([.closingIssuesReferences.nodes[].number] + ([.body | scan("(?i)(?:closes|fixes|resolves) #([0-9]+)") | .[0] | tonumber])) | .[]' -done | sort -un | while read -r n; do - gh api graphql -F n="$n" -f query='query($n:Int!){repository(owner:"modelcontextprotocol",name:"inspector"){issueOrPullRequest(number:$n){... on Issue{number author{login __typename}}}}}' \ - --jq '.data.repository.issueOrPullRequest | select(.number and .author.__typename == "User") | "\(.author.login) \(.number)"' -done | while read -r who n; do - perm=$(gh api "repos/$REPO/collaborators/$who/permission" --jq .permission 2>/dev/null || echo none) - case "$perm" in admin|maintain|write) ;; *) echo "$who $n" ;; esac -done | awk '{ c[$1]++; l[$1] = l[$1] (l[$1] ? ", " : "") "#" $2 } - END { for (u in c) printf "%d\t%s\t%s\n", c[u], u, l[u] }' \ - | sort -t$'\t' -k1,1nr -k2,2f | awk -F'\t' '{ print "* @" $2 " (" $3 ")" }' > thanks.txt - -: > thanks.md # truncate first, so a rerun never keeps a stale section -[ -s thanks.txt ] && { printf '\n## Thanks for helping us improve\n\nThis release addresses issues reported by these community members. Thank you for taking the time to file them:\n\n'; cat thanks.txt; } >> thanks.md +ARGS=(--merge-branch v2/chore/milestone-merge-vX.Y.Z --ledger-url ) +# Add one --known-issue "" per known issue, if any. + +npm run release:notes -- "${ARGS[@]}" # 1. preview: prints the notes, creates nothing +npm run release:notes -- "${ARGS[@]}" --draft # 2. creates the Release as a DRAFT ``` -Then assemble `release-notes.md`, the ledger line, any known issue, and -`thanks.md`, and read the result before publishing. The rules behind the recipe: +Read the preview, then create the draft and read it again on the Releases page. +The preview prints the notes on stdout and its progress on stderr, so +`> notes.md` captures just the notes. + +The rules the helper applies: - **An issue counts when a listed PR closes it**, through either the manual - closing link (`closingIssuesReferences`) or a `Closes / Fixes / Resolves #N` - in the PR body. So an issue older than the release still counts when this - release closed it. + closing link (`closingIssuesReferences`, followed across every page) or a + closing keyword (`close`/`fix`/`resolve` in any tense, optional colon) before + a bare `#N` in the PR body. So an issue older than the release still counts + when this release closed it. A cross-repo `owner/repo#N` does not count. - **Maintainers and bots are excluded by permission, not by name.** A maintainer is anyone with `admin`, `maintain` or `write` on the repo. Bot authors are dropped, which covers the issues the SDK-watch and Dependabot sweeps file. On a public repo, anyone without a role reads as `read`, so they are credited. +- **One line per person, most issues first:** `* @user (#1, #2, …)`. - **"Addresses", not "fixes."** The credited issues include feature requests. -- **Leave the section out** when no community reporter remains, as with 2.1.0. - -The whole step, including creating the Release from these notes, is being -scripted as a tested helper in #2550. Until that lands, this recipe is the -procedure. - -### 3b. Tag and publish - -**Normally this is done by a maintainer through the GitHub UI**, after PR 2 has -merged: *Releases → Draft a new release → Choose a tag → type the bare `x.y.z` -→ Create new tag on publish*, with **Target: `main`**, then paste the notes from -3a and publish. Publishing the Release is what fires the `publish` and -`publish-github-container-registry` jobs. - -The same thing from the CLI, with the notes file from 3a: +- **The section is left out** when no community reporter remains, as with 2.1.0. +- ⚠️ **Any API failure aborts the whole run.** A failed permission lookup never + reads as "community", which could credit a maintainer, and a rate-limited PR + or issue lookup is never skipped. A partial Thanks list is never published, so + on a rate limit, wait for the reset and run it again. + +To regenerate an **older** release's notes (to check them, or after editing a +PR body), pass `--version x.y.z`, and optionally `--previous-tag`. This works +for preview only. `--draft` and `--publish` refuse any version other than the +one on `origin/main`, because `--target main` would attach it to the wrong tree. +Previewing `--version 2.9.0` with 2.9.0's ledger and known issue reproduces its +published notes exactly. + +### 3b. Publish the Release + +**Publishing is the deliberate maintainer action.** It fires `package` → +`publish` (npm) and `publish-github-container-registry`, so the helper never +does it implicitly. Publish the draft from 3a on the Releases page (*Edit → +Publish release*, with **Set as the latest release** checked), or from the CLI: ```sh -NOTES=release-notes-final.md # the assembled notes from 3a -gh release create "$VERSION" --target main --title "$VERSION" --notes-file "$NOTES" --latest +VERSION=$(git show origin/main:package.json | node -p "JSON.parse(require('fs').readFileSync(0)).version") +gh release edit "$VERSION" --repo modelcontextprotocol/inspector --draft=false --latest ``` -`--target main` and the bare `$VERSION` give the right target and tag by -construction. +A draft has no tag yet. GitHub creates the bare `x.y.z` tag **on publish**, at +`main`'s head at that moment, so publish only while `main` is still the merge +commit PR 2 landed. `npm run release:notes -- "${ARGS[@]}" --publish` creates and +publishes in one step, for when the notes were already reviewed in a preview. + +The helper passes the bare `$VERSION` as the tag and `--target main`, so the tag +name and the target are right by construction. -The equivalent by hand, for when the UI is not an option, is the script -(`scripts/release-tag.mjs`, #2558) — it derives the tag from the version that -just landed rather than taking one as input, since a hard-coded tag is either -already taken (so `git tag` aborts) or, worse, wrong: +If the tag has to exist before the Release (for example, to pin the commit +before drafting), push it first with `scripts/release-tag.mjs` (#2558). It +derives the tag from the version that just landed rather than taking one as +input, since a hard-coded tag is either already taken (so `git tag` aborts) or, +worse, wrong. A Release created afterwards attaches to that existing tag: ```sh npm run release:tag # dry run: prints what would be tagged npm run release:tag -- --push # tags origin/main's SHA and pushes the tag -# then draft & publish a GitHub Release for that tag → triggers `publish` +# then 3a and 3b; the Release attaches to this tag ``` ⚠️ **It tags `origin/main`, not your local `HEAD`.** `git checkout main && git @@ -359,10 +352,10 @@ is what 2.9.0 needed (#2551): the first run's `publish` passed a bare makes the re-tag abort, and later makes `git fetch --tags` refuse to clobber it. Confirm the remote tag is gone with `gh api repos/$REPO/git/ref/tags/$VERSION` (expect a 404). -4. **Recreate the Release at the new `main`.** Re-run the whole 3a recipe: +4. **Recreate the Release at the new `main`.** Re-run `npm run release:notes` (3a): What's Changed now includes the fix PRs, and a fix PR can close a - community-reported issue, so the Thanks section can change too. Keep the - hand-written parts (the ledger line and any known issue) as they were. + community-reported issue, so the Thanks section can change too. Pass the + same `--merge-branch`, `--ledger-url` and `--known-issue` arguments as before. 5. **Verify the fix before publishing again, wherever it can be verified.** Run the gate, and the smoke rows the fix touches, on the new tree. Only a path that exists solely inside a release run (like #2551's publish step) has the diff --git a/AGENTS.md b/AGENTS.md index 871d96850..2fa1795f5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -58,7 +58,7 @@ inspector/ ├── scripts/ Root build/verify tooling (install cascade, smokes, verify:* guards) │ plus repo automation run from CI (the dependency, alert + SDK sweeps) │ and maintainer-workflow helpers (the pr:*, board:*, advisory:*, -│ release:tag + action:resolve-pin aliases) +│ release:notes, release:tag + action:resolve-pin aliases) ├── docs/ Task-oriented guides ├── specification/ Design/build specifications └── .claude/skills/ The procedures (see the index above) diff --git a/README.md b/README.md index 3a691d61d..ca9912b16 100644 --- a/README.md +++ b/README.md @@ -59,7 +59,7 @@ inspector/ ├── scripts/ Root build/verify tooling (install cascade, smokes, the verify:* guards), │ repo automation run from CI (the dependency, Dependabot-alert and SDK sweeps), │ maintainer-workflow helpers (the pr:*, board:*, advisory:*, -│ release:tag and action:resolve-pin aliases) +│ release:notes, release:tag and action:resolve-pin aliases) │ and the Docker image's HEALTHCHECK probe ├── docs/ Task-oriented guides — see below ├── specification/ Design/build specifications diff --git a/package.json b/package.json index 20ec2694a..912efe0f6 100644 --- a/package.json +++ b/package.json @@ -62,6 +62,7 @@ "board:audit": "node scripts/board-audit.mjs", "board:recover": "node scripts/board-recover.mjs", "advisory:fork": "node scripts/advisory-fork.mjs", + "release:notes": "node scripts/release-notes.mjs", "release:tag": "node scripts/release-tag.mjs", "action:resolve-pin": "node scripts/action-pin-resolve.mjs", "validate": "npm run validate:guards && npm run validate:core && npm run validate:web && npm run validate:cli && npm run validate:tui && npm run validate:launcher", diff --git a/scripts/release-notes.mjs b/scripts/release-notes.mjs new file mode 100644 index 000000000..3d6b5db85 --- /dev/null +++ b/scripts/release-notes.mjs @@ -0,0 +1,347 @@ +#!/usr/bin/env node +// Assemble a release's notes and create the GitHub Release (#2550) — +// `npm run release:notes -- --merge-branch --ledger-url [...]`. +// The recipe the release skill's step 3a previously transcribed inline. +// +// The notes are four parts, in this order: GitHub's generated "What's +// Changed" list (the same `releases/generate-notes` API the UI button uses), +// the smoke-ledger line, a `## Known issue(s)` section from what the +// maintainer passes in (a judgment call, so never generated), and a +// `## Thanks for helping us improve` section crediting the community members +// whose issues the listed PRs close. GitHub adds everyone `@`-mentioned in a +// release body to its Contributors strip, so that section is what puts the +// reporters there. +// +// FAIL FAST. Every `gh`/`git` call is checked and a failure throws: a +// permission lookup that errors must never read as "community" (that could +// credit a maintainer), and a rate-limited PR or issue lookup must never be +// silently skipped (that would publish a partial Thanks list). A permission +// value outside the known set throws for the same reason. +// +// The default run is a PREVIEW that prints the notes and creates nothing. +// Publishing the Release triggers `package` → `publish` (npm) and the GHCR +// image, so it is never implicit: `--draft` creates a draft for review and +// `--publish` publishes. Both pass the bare `x.y.z` tag and `--target main`, +// and both refuse unless that version is the one on origin/main. + +import { spawnSync } from "node:child_process"; +import { parseArgs } from "node:util"; +import { versionFrom } from "./release-tag.mjs"; + +export const REPO = "modelcontextprotocol/inspector"; +const [OWNER, NAME] = REPO.split("/"); + +// What `collaborators/{user}/permission` reports in its `permission` field. +// A public repo reports `read` for anyone without a role, so a community +// member is `read` (or `triage`/`none`), and a maintainer is anyone above it. +const MAINTAINER_PERMISSIONS = new Set(["admin", "maintain", "write"]); +const COMMUNITY_PERMISSIONS = new Set(["triage", "read", "none"]); + +export const THANKS_LEAD_IN = + "This release addresses issues reported by these community members. Thank you for taking the time to file them:"; + +export function parseNotesArgs(argv) { + const { values } = parseArgs({ + args: argv, + options: { + "merge-branch": { type: "string" }, + "ledger-url": { type: "string" }, + "known-issue": { type: "string", multiple: true }, + version: { type: "string" }, + "previous-tag": { type: "string" }, + draft: { type: "boolean" }, + publish: { type: "boolean" }, + }, + }); + const mergeBranch = values["merge-branch"]; + const ledgerUrl = values["ledger-url"]; + if (!mergeBranch || !ledgerUrl) { + throw new Error("--merge-branch and --ledger-url are both required"); + } + if (!/^https:\/\/\S+$/.test(ledgerUrl)) { + throw new Error(`--ledger-url "${ledgerUrl}" is not an https URL`); + } + for (const [flag, tag] of [ + ["--version", values.version], + ["--previous-tag", values["previous-tag"]], + ]) { + if (tag !== undefined && !isStableTag(tag)) { + throw new Error(`${flag} "${tag}" is not a bare x.y.z`); + } + } + if (values.draft && values.publish) { + throw new Error("--draft and --publish are mutually exclusive"); + } + return { + mergeBranch, + ledgerUrl, + knownIssues: values["known-issue"] ?? [], + version: values.version, + previousTag: values["previous-tag"], + mode: values.publish ? "publish" : values.draft ? "draft" : "preview", + }; +} + +function run(spawn, cmd, args, input) { + const result = spawn(cmd, args, { encoding: "utf8", input }); + if (result.error) { + throw result.error; + } + if (result.status !== 0) { + throw new Error( + `${cmd} ${args.join(" ")} failed: ${(result.stderr ?? "").trim()}`, + ); + } + return (result.stdout ?? "").trim(); +} + +function graphql(spawn, query, variables) { + const args = ["api", "graphql", "-f", `query=${query}`]; + for (const [key, value] of Object.entries(variables)) { + // -F types a number as Int; -f keeps a cursor a String. + args.push(typeof value === "number" ? "-F" : "-f", `${key}=${value}`); + } + const response = JSON.parse(run(spawn, "gh", args)); + if (response.errors?.length) { + throw new Error(`gh api graphql: ${JSON.stringify(response.errors)}`); + } + return response.data; +} + +export function isStableTag(tag) { + return /^\d+\.\d+\.\d+$/.test(tag); +} + +function compareVersions(a, b) { + const pa = a.split(".").map(Number); + const pb = b.split(".").map(Number); + for (let i = 0; i < 3; i++) { + if (pa[i] !== pb[i]) return pa[i] - pb[i]; + } + return 0; +} + +/** + * The highest STABLE tag below `version`. Whole-name match: this repo also + * has 2.0.0-rc.N, x.y.z-hotfix, x.y.z-amended and v2-alpha-1 tags, and + * picking one of those as the previous tag would drop changes from the list. + */ +export function previousStableTag(tags, version) { + const below = tags + .filter(isStableTag) + .filter((tag) => compareVersions(tag, version) < 0) + .sort(compareVersions); + if (below.length === 0) { + throw new Error(`no stable x.y.z tag below ${version}`); + } + return below[below.length - 1]; +} + +/** Every PR of this repo the generated list links to, ascending. */ +export function pullNumbersFrom(generated) { + const pattern = new RegExp(`https://github\\.com/${REPO}/pull/(\\d+)`, "g"); + return [...new Set([...generated.matchAll(pattern)].map((m) => +m[1]))].sort( + (a, b) => a - b, + ); +} + +/** + * Issue numbers a PR body closes by keyword — GitHub's nine closing keywords, + * with its optional colon. A bare `#N` only, so a cross-repo `owner/repo#N` + * is not mistaken for one of ours. + */ +export function closingKeywordIssues(body) { + const pattern = /\b(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?):?\s+#(\d+)\b/gi; + return [...(body ?? "").matchAll(pattern)].map((m) => +m[1]); +} + +const CLOSING_QUERY = `query($n:Int!,$after:String){repository(owner:"${OWNER}",name:"${NAME}"){pullRequest(number:$n){body closingIssuesReferences(first:100,after:$after){pageInfo{hasNextPage endCursor} nodes{number repository{nameWithOwner}}}}}}`; + +/** Every issue of this repo one PR closes: manual links (paginated) + body keywords. */ +export function issuesClosedBy(spawn, pr) { + const numbers = new Set(); + let body; + let after; + for (;;) { + const variables = after === undefined ? { n: pr } : { n: pr, after }; + const pull = graphql(spawn, CLOSING_QUERY, variables).repository + .pullRequest; + if (!pull) { + throw new Error(`PR #${pr} not found`); + } + body ??= pull.body; + const refs = pull.closingIssuesReferences; + for (const node of refs.nodes) { + if (node.repository.nameWithOwner === REPO) numbers.add(node.number); + } + if (!refs.pageInfo.hasNextPage) break; + after = refs.pageInfo.endCursor; + } + for (const n of closingKeywordIssues(body)) numbers.add(n); + return numbers; +} + +const AUTHOR_QUERY = `query($n:Int!){repository(owner:"${OWNER}",name:"${NAME}"){issueOrPullRequest(number:$n){__typename ... on Issue{author{login __typename}}}}}`; + +/** + * The login to credit for issue `n`, or null when there is nobody to credit: + * the number is a PR rather than an issue, or the author is a bot (the + * SDK-watch and Dependabot sweeps) or a deleted account. + */ +export function creditableAuthor(spawn, n) { + const node = graphql(spawn, AUTHOR_QUERY, { n }).repository + .issueOrPullRequest; + if (!node) { + throw new Error(`#${n} not found`); + } + if (node.__typename !== "Issue") return null; + return node.author?.__typename === "User" ? node.author.login : null; +} + +/** True for a maintainer, false for a community member; throws otherwise. */ +export function isMaintainer(spawn, login) { + const permission = run(spawn, "gh", [ + "api", + `repos/${REPO}/collaborators/${login}/permission`, + "--jq", + ".permission", + ]); + if (MAINTAINER_PERMISSIONS.has(permission)) return true; + if (COMMUNITY_PERMISSIONS.has(permission)) return false; + throw new Error(`unexpected permission "${permission}" for @${login}`); +} + +/** Community reporter → the issues of theirs the listed PRs close. */ +export function collectReporters(spawn, pulls) { + const issues = new Set(); + for (const pr of pulls) { + for (const n of issuesClosedBy(spawn, pr)) issues.add(n); + } + const reporters = new Map(); + const maintainer = new Map(); + for (const n of [...issues].sort((a, b) => a - b)) { + const login = creditableAuthor(spawn, n); + if (login === null) continue; + if (!maintainer.has(login)) { + maintainer.set(login, isMaintainer(spawn, login)); + } + if (maintainer.get(login)) continue; + if (!reporters.has(login)) reporters.set(login, []); + reporters.get(login).push(n); + } + return reporters; +} + +/** One line per person, most issues first, then by name; "" when nobody. */ +export function formatThanks(reporters) { + if (reporters.size === 0) return ""; + const lines = [...reporters] + .sort( + ([a, ia], [b, ib]) => + ib.length - ia.length || + a.localeCompare(b, "en", { sensitivity: "base" }), + ) + .map( + ([login, nums]) => `* @${login} (${nums.map((n) => `#${n}`).join(", ")})`, + ); + return `## Thanks for helping us improve\n\n${THANKS_LEAD_IN}\n\n${lines.join("\n")}`; +} + +export function formatKnownIssues(knownIssues) { + if (knownIssues.length === 0) return ""; + const heading = knownIssues.length === 1 ? "Known issue" : "Known issues"; + return `## ${heading}\n\n${knownIssues.join("\n\n")}`; +} + +export function assembleNotes({ + generated, + mergeBranch, + ledgerUrl, + knownIssues, + thanks, +}) { + const ledger = `**Smoke test ledger for milestone branch**: [${mergeBranch}](${ledgerUrl})`; + const sections = [formatKnownIssues(knownIssues), thanks].filter(Boolean); + return [`${generated.trimEnd()}\n${ledger}`, ...sections].join("\n\n") + "\n"; +} + +export function main(argv = process.argv.slice(2), spawn = spawnSync) { + const args = parseNotesArgs(argv); + + // Tags first: a refspec-less fetch rewrites FETCH_HEAD, so the main fetch + // has to come last for FETCH_HEAD to be main (see release-tag.mjs on why + // FETCH_HEAD and not the origin/main tracking ref). + run(spawn, "git", ["fetch", "origin", "--tags"]); + run(spawn, "git", ["fetch", "origin", "main"]); + const sha = run(spawn, "git", ["rev-parse", "FETCH_HEAD"]); + const mainVersion = versionFrom( + run(spawn, "git", ["show", `${sha}:package.json`]), + ); + const version = args.version ?? mainVersion; + if (args.mode !== "preview" && version !== mainVersion) { + // --target main would attach this version's Release to another + // version's tree; regenerating an older release's notes is preview-only. + throw new Error( + `--${args.mode} creates ${version} on main, but origin/main is ${mainVersion}`, + ); + } + const previousTag = + args.previousTag ?? + previousStableTag(run(spawn, "git", ["tag", "-l"]).split("\n"), version); + console.error(`release notes: ${previousTag} → ${version}`); + + const generated = run(spawn, "gh", [ + "api", + `repos/${REPO}/releases/generate-notes`, + "-f", + `tag_name=${version}`, + "-f", + "target_commitish=main", + "-f", + `previous_tag_name=${previousTag}`, + "--jq", + ".body", + ]); + const pulls = pullNumbersFrom(generated); + console.error(`crediting reporters of issues closed by ${pulls.length} PRs`); + const notes = assembleNotes({ + generated, + mergeBranch: args.mergeBranch, + ledgerUrl: args.ledgerUrl, + knownIssues: args.knownIssues, + thanks: formatThanks(collectReporters(spawn, pulls)), + }); + + if (args.mode === "preview") { + process.stdout.write(notes); + console.error( + "preview only — nothing created; re-run with --draft (or --publish)", + ); + return notes; + } + const create = [ + "release", + "create", + version, + "--repo", + REPO, + "--target", + "main", + "--title", + version, + "--notes-file", + "-", + args.mode === "draft" ? "--draft" : "--latest", + ]; + const url = run(spawn, "gh", create, notes); + console.error( + args.mode === "draft" + ? `draft created: ${url} — review it, then publish (this triggers npm + GHCR)` + : `published: ${url}`, + ); + return notes; +} + +if (import.meta.url === `file://${process.argv[1]}`) { + main(); +} diff --git a/scripts/release-notes.test.mjs b/scripts/release-notes.test.mjs new file mode 100644 index 000000000..aedc57896 --- /dev/null +++ b/scripts/release-notes.test.mjs @@ -0,0 +1,493 @@ +// Tests for scripts/release-notes.mjs (#2550) — the issue-author mapping and +// its exclusions (maintainers by permission, bots, PRs, other repos), the +// paginated closing references, fail-fast on every API error, the note +// layout 2.9.0 shipped with, and that nothing is created without an explicit +// --draft / --publish. Run via `npm run test:scripts`. + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + REPO, + THANKS_LEAD_IN, + assembleNotes, + closingKeywordIssues, + formatKnownIssues, + formatThanks, + main, + parseNotesArgs, + previousStableTag, + pullNumbersFrom, +} from "./release-notes.mjs"; + +const BASE = [ + "--merge-branch", + "v2/chore/mm", + "--ledger-url", + "https://l.example/x", +]; +const SHA = "abc123"; +const PULL = (n) => `https://github.com/${REPO}/pull/${n}`; + +test("parseNotesArgs defaults to a preview and validates its inputs", () => { + assert.deepEqual(parseNotesArgs(BASE), { + mergeBranch: "v2/chore/mm", + ledgerUrl: "https://l.example/x", + knownIssues: [], + version: undefined, + previousTag: undefined, + mode: "preview", + }); + assert.equal(parseNotesArgs([...BASE, "--draft"]).mode, "draft"); + assert.equal(parseNotesArgs([...BASE, "--publish"]).mode, "publish"); + assert.deepEqual( + parseNotesArgs([...BASE, "--known-issue", "a", "--known-issue", "b"]) + .knownIssues, + ["a", "b"], + ); + assert.throws(() => parseNotesArgs([]), /both required/); + assert.throws( + () => parseNotesArgs(["--merge-branch", "b", "--ledger-url", "ftp://x"]), + /not an https URL/, + ); + assert.throws( + () => parseNotesArgs([...BASE, "--draft", "--publish"]), + /mutually exclusive/, + ); + assert.throws( + () => parseNotesArgs([...BASE, "--version", "v2.9.0"]), + /--version "v2.9.0" is not a bare x.y.z/, + ); + assert.throws( + () => parseNotesArgs([...BASE, "--previous-tag", "2.0.0-rc.1"]), + /--previous-tag/, + ); +}); + +test("previousStableTag skips RC, hotfix and v-prefixed tags", () => { + const tags = [ + "2.8.0", + "2.9.0", + "2.10.0-rc.1", + "2.9.1-hotfix", + "v2-alpha-1", + "2.10.0", + "", + ]; + assert.equal(previousStableTag(tags, "2.10.0"), "2.9.0"); + assert.equal(previousStableTag(tags, "2.9.0"), "2.8.0"); + assert.equal(previousStableTag(tags, "2.11.0"), "2.10.0"); + assert.throws(() => previousStableTag(tags, "1.0.0"), /no stable/); +}); + +test("pullNumbersFrom takes this repo's PR links only, deduplicated", () => { + const generated = [ + `* a by @x in ${PULL(12)}`, + `* b by @y in ${PULL(3)}`, + `* @y made their first contribution in ${PULL(3)}`, + "* c in https://github.com/other/repo/pull/7", + ].join("\n"); + assert.deepEqual(pullNumbersFrom(generated), [3, 12]); +}); + +test("closingKeywordIssues reads every closing keyword, never a cross-repo ref", () => { + const body = [ + "Closes #1", + "fixes: #2, Resolved #3", + "close #4 and FIXED #5", + "Refs #6", + "Closes other/repo#7", + "prefixes #8", + ].join("\n"); + assert.deepEqual(closingKeywordIssues(body), [1, 2, 3, 4, 5]); + assert.deepEqual(closingKeywordIssues(null), []); +}); + +test("formatThanks orders by issue count, then name case-insensitively", () => { + const reporters = new Map([ + ["zed", [5]], + ["Amy", [9]], + ["many", [1, 2, 3]], + ["bob", [4]], + ]); + assert.equal( + formatThanks(reporters), + [ + "## Thanks for helping us improve", + "", + THANKS_LEAD_IN, + "", + "* @many (#1, #2, #3)", + "* @Amy (#9)", + "* @bob (#4)", + "* @zed (#5)", + ].join("\n"), + ); + assert.equal(formatThanks(new Map()), ""); +}); + +test("formatKnownIssues pluralizes and is omitted when empty", () => { + assert.equal(formatKnownIssues([]), ""); + assert.equal(formatKnownIssues(["one"]), "## Known issue\n\none"); + assert.equal(formatKnownIssues(["a", "b"]), "## Known issues\n\na\n\nb"); +}); + +test("assembleNotes lays the parts out as 2.9.0 shipped them", () => { + const notes = assembleNotes({ + generated: "## What's Changed\n* x\n\n**Full Changelog**: c\n", + mergeBranch: "mm", + ledgerUrl: "https://l", + knownIssues: ["K"], + thanks: "## Thanks for helping us improve\n\nT", + }); + assert.equal( + notes, + [ + "## What's Changed", + "* x", + "", + "**Full Changelog**: c", + "**Smoke test ledger for milestone branch**: [mm](https://l)", + "", + "## Known issue", + "", + "K", + "", + "## Thanks for helping us improve", + "", + "T", + "", + ].join("\n"), + ); + assert.equal( + assembleNotes({ + generated: "G", + mergeBranch: "mm", + ledgerUrl: "https://l", + knownIssues: [], + thanks: "", + }), + "G\n**Smoke test ledger for milestone branch**: [mm](https://l)\n", + ); +}); + +// A fake `gh`/`git` over a small repo model. `failOn` makes the first call +// whose joined argv includes it fail, the way a rate limit or a 404 would. +function world({ + mainVersion = "2.10.0", + tags = ["2.8.0", "2.9.0", "2.10.0-rc.1"], + pulls = {}, + issues = {}, + perms = {}, + failOn, +} = {}) { + const calls = []; + const generated = [ + "## What's Changed", + ...Object.keys(pulls).map((n) => `* change by @dev in ${PULL(n)}`), + "", + "**Full Changelog**: https://example/compare", + ].join("\n"); + const ok = (stdout) => ({ status: 0, stdout, stderr: "" }); + const spawn = (cmd, args, opts) => { + calls.push({ cmd, args, input: opts?.input }); + const joined = args.join(" "); + if (failOn && joined.includes(failOn)) { + return { status: 1, stdout: "", stderr: "API rate limit exceeded" }; + } + if (cmd === "git") { + if (joined === "rev-parse FETCH_HEAD") return ok(`${SHA}\n`); + if (joined === `show ${SHA}:package.json`) + return ok(JSON.stringify({ version: mainVersion })); + if (joined === "tag -l") return ok(tags.join("\n")); + return ok(""); + } + assert.equal(cmd, "gh"); + if (args[1].endsWith("/releases/generate-notes")) return ok(generated); + if (args[0] === "release") return ok("https://github.com/r/releases/1"); + if (args[1] === "graphql") { + const vars = Object.fromEntries( + args + .filter((_, i) => args[i - 1] === "-F" || args[i - 1] === "-f") + .map((kv) => kv.split(/=(.*)/s).slice(0, 2)), + ); + const n = Number(vars.n); + if (vars.query.includes("closingIssuesReferences")) { + const pr = pulls[n]; + const pages = pr.pages ?? [pr.closing ?? []]; + const index = vars.after ? Number(vars.after) : 0; + const hasNextPage = index + 1 < pages.length; + return ok( + JSON.stringify({ + data: { + repository: { + pullRequest: { + body: pr.body ?? "", + closingIssuesReferences: { + pageInfo: { + hasNextPage, + endCursor: hasNextPage ? String(index + 1) : null, + }, + nodes: pages[index].map((ref) => + typeof ref === "number" + ? { number: ref, repository: { nameWithOwner: REPO } } + : ref, + ), + }, + }, + }, + }, + }), + ); + } + return ok( + JSON.stringify({ + data: { repository: { issueOrPullRequest: issues[n] ?? null } }, + }), + ); + } + const login = args[1].match(/collaborators\/([^/]+)\/permission/)[1]; + return ok(perms[login]); + }; + spawn.calls = calls; + return spawn; +} + +const user = (login) => ({ + __typename: "Issue", + author: { login, __typename: "User" }, +}); + +function quiet(t) { + const out = []; + t.mock.method(console, "error", () => {}); + t.mock.method(process.stdout, "write", (chunk) => { + out.push(chunk); + return true; + }); + return out; +} + +test("credits community reporters only — no maintainers, bots, PRs or other repos", (t) => { + const out = quiet(t); + const spawn = world({ + pulls: { + 10: { body: "Closes #1\nFixes #2", closing: [3] }, + 11: { + body: "Resolves #4, closes #5, closes #6, closes #1", + closing: [{ number: 7, repository: { nameWithOwner: "other/repo" } }], + }, + }, + issues: { + 1: user("reporter"), + 2: user("maint"), + 3: user("reporter"), + 4: { __typename: "Issue", author: { login: "bot", __typename: "Bot" } }, + 5: { __typename: "PullRequest" }, + 6: { __typename: "Issue", author: null }, + }, + perms: { reporter: "read", maint: "write" }, + }); + const notes = main(BASE, spawn); + assert.match(notes, /\* @reporter \(#1, #3\)\n$/); + assert.doesNotMatch(notes, /@maint|@bot|#5|#7/); + assert.equal(out.join(""), notes); + // Each person's permission is looked up once, however many issues they filed. + const permissionCalls = spawn.calls.filter((c) => + c.args[1]?.includes("/permission"), + ); + assert.deepEqual( + permissionCalls.map((c) => c.args[1].split("/")[4]), + ["reporter", "maint"], + ); + // The preview creates nothing. + assert.equal( + spawn.calls.some((c) => c.args[0] === "release"), + false, + ); +}); + +test("generate-notes is asked for the previous STABLE tag to main", (t) => { + quiet(t); + const spawn = world(); + main(BASE, spawn); + const call = spawn.calls.find((c) => + c.args[1]?.endsWith("/releases/generate-notes"), + ); + assert.deepEqual(call.args.slice(2), [ + "-f", + "tag_name=2.10.0", + "-f", + "target_commitish=main", + "-f", + "previous_tag_name=2.9.0", + "--jq", + ".body", + ]); + // The tags fetch precedes the main fetch, so FETCH_HEAD is main. + const fetches = spawn.calls.filter((c) => c.args[0] === "fetch"); + assert.deepEqual( + fetches.map((c) => c.args.join(" ")), + ["fetch origin --tags", "fetch origin main"], + ); +}); + +test("an explicit --version/--previous-tag regenerates an older release's notes", (t) => { + quiet(t); + const spawn = world({ mainVersion: "2.10.0" }); + main([...BASE, "--version", "2.8.0", "--previous-tag", "2.7.0"], spawn); + const call = spawn.calls.find((c) => + c.args[1]?.endsWith("/releases/generate-notes"), + ); + assert.ok(call.args.includes("tag_name=2.8.0")); + assert.ok(call.args.includes("previous_tag_name=2.7.0")); + assert.equal( + spawn.calls.some((c) => c.args.join(" ") === "tag -l"), + false, + ); +}); + +test("the Thanks section is omitted when no community reporter remains", (t) => { + quiet(t); + const notes = main( + BASE, + world({ + pulls: { 10: { body: "Closes #1" } }, + issues: { 1: user("maint") }, + perms: { maint: "admin" }, + }), + ); + assert.doesNotMatch(notes, /Thanks/); +}); + +test("closing references are followed across every page", (t) => { + quiet(t); + const spawn = world({ + pulls: { 10: { pages: [[1], [2], [3]] } }, + issues: { 1: user("a"), 2: user("b"), 3: user("c") }, + perms: { a: "read", b: "triage", c: "none" }, + }); + const notes = main(BASE, spawn); + assert.match(notes, /@a \(#1\)\n\* @b \(#2\)\n\* @c \(#3\)/); + const cursors = spawn.calls + .filter((c) => c.args.some((a) => a.includes("closingIssuesReferences"))) + .map((c) => c.args.find((a) => a.startsWith("after=")) ?? null); + assert.deepEqual(cursors, [null, "after=1", "after=2"]); +}); + +for (const [what, failOn] of [ + ["a permission lookup", "/permission"], + ["a closing-references lookup", "closingIssuesReferences"], + ["an issue-author lookup", "issueOrPullRequest"], + ["generate-notes", "generate-notes"], + ["the fetch", "fetch origin main"], +]) { + test(`a failed ${what} aborts — never a partial Thanks list`, (t) => { + quiet(t); + const spawn = world({ + pulls: { 10: { body: "Closes #1" } }, + issues: { 1: user("maybe-maint") }, + perms: { "maybe-maint": "read" }, + failOn, + }); + assert.throws( + () => main([...BASE, "--publish"], spawn), + /rate limit exceeded/, + ); + assert.equal( + spawn.calls.some((c) => c.args[0] === "release"), + false, + ); + }); +} + +test("an unknown permission value aborts rather than reading as community", (t) => { + quiet(t); + assert.throws( + () => + main( + BASE, + world({ + pulls: { 10: { body: "Closes #1" } }, + issues: { 1: user("who") }, + perms: { who: "" }, + }), + ), + /unexpected permission "" for @who/, + ); +}); + +test("GraphQL errors and missing nodes abort", (t) => { + quiet(t); + const missing = world({ pulls: { 10: { body: "Closes #99" } } }); + assert.throws(() => main(BASE, missing), /#99 not found/); + + const errors = world({ pulls: { 10: {} } }); + const inner = errors; + const spawn = (cmd, args, opts) => + args[1] === "graphql" + ? { status: 0, stdout: '{"errors":[{"message":"boom"}]}', stderr: "" } + : inner(cmd, args, opts); + assert.throws(() => main(BASE, spawn), /boom/); + + const noPull = (cmd, args, opts) => + args[1] === "graphql" + ? { + status: 0, + stdout: '{"data":{"repository":{"pullRequest":null}}}', + stderr: "", + } + : inner(cmd, args, opts); + assert.throws(() => main(BASE, noPull), /PR #10 not found/); +}); + +test("a spawn error is rethrown", () => { + const boom = new Error("ENOENT"); + assert.throws( + () => main(BASE, () => ({ error: boom })), + (e) => e === boom, + ); +}); + +test("--draft creates a draft at main under the bare tag, notes on stdin", (t) => { + quiet(t); + const spawn = world(); + const notes = main([...BASE, "--draft"], spawn); + const create = spawn.calls.find((c) => c.args[0] === "release"); + assert.deepEqual(create.args, [ + "release", + "create", + "2.10.0", + "--repo", + REPO, + "--target", + "main", + "--title", + "2.10.0", + "--notes-file", + "-", + "--draft", + ]); + assert.equal(create.input, notes); +}); + +test("--publish publishes as latest", (t) => { + quiet(t); + const spawn = world(); + main([...BASE, "--publish"], spawn); + const create = spawn.calls.find((c) => c.args[0] === "release"); + assert.equal(create.args.at(-1), "--latest"); + assert.equal(create.args.includes("--draft"), false); +}); + +test("creating a version other than origin/main's is refused", (t) => { + quiet(t); + const spawn = world({ mainVersion: "2.10.0" }); + assert.throws( + () => main([...BASE, "--version", "2.9.0", "--draft"], spawn), + /--draft creates 2\.9\.0 on main, but origin\/main is 2\.10\.0/, + ); + assert.equal( + spawn.calls.some((c) => c.cmd === "gh"), + false, + ); +}); From f792cb2940d9723beeb264e43109f19895b1b878 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 5 Oct 2026 06:21:01 -0400 Subject: [PATCH 2/5] fix(scripts): reject --previous-tag outside preview in release:notes A created Release always starts from the previous stable tag; a typo or stale override in --draft/--publish would ship the wrong range of changes and reporter credits. Make it preview-only, like --version, and say so in the release skill. Co-Authored-By: Claude Opus 5.5 Signed-off-by: cliffhall --- .claude/skills/release/SKILL.md | 5 +++-- scripts/release-notes.mjs | 6 ++++++ scripts/release-notes.test.mjs | 10 ++++++++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index e15bf0bec..0be30e3ee 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -262,8 +262,9 @@ The rules the helper applies: To regenerate an **older** release's notes (to check them, or after editing a PR body), pass `--version x.y.z`, and optionally `--previous-tag`. This works -for preview only. `--draft` and `--publish` refuse any version other than the -one on `origin/main`, because `--target main` would attach it to the wrong tree. +for preview only. `--draft` and `--publish` refuse `--previous-tag`, and any +version other than the one on `origin/main`, because the Release would carry +the wrong range of changes or attach to the wrong tree. Previewing `--version 2.9.0` with 2.9.0's ledger and known issue reproduces its published notes exactly. diff --git a/scripts/release-notes.mjs b/scripts/release-notes.mjs index 3d6b5db85..c1ab716cb 100644 --- a/scripts/release-notes.mjs +++ b/scripts/release-notes.mjs @@ -72,6 +72,12 @@ export function parseNotesArgs(argv) { if (values.draft && values.publish) { throw new Error("--draft and --publish are mutually exclusive"); } + // A Release created here always starts from the previous stable tag; an + // override (a typo, a stale value) would publish the wrong range of + // changes and credits, so it is a preview-only knob like --version. + if ((values.draft || values.publish) && values["previous-tag"]) { + throw new Error("--previous-tag is preview-only"); + } return { mergeBranch, ledgerUrl, diff --git a/scripts/release-notes.test.mjs b/scripts/release-notes.test.mjs index aedc57896..a6145e8fe 100644 --- a/scripts/release-notes.test.mjs +++ b/scripts/release-notes.test.mjs @@ -61,6 +61,16 @@ test("parseNotesArgs defaults to a preview and validates its inputs", () => { () => parseNotesArgs([...BASE, "--previous-tag", "2.0.0-rc.1"]), /--previous-tag/, ); + for (const mode of ["--draft", "--publish"]) { + assert.throws( + () => parseNotesArgs([...BASE, "--previous-tag", "2.8.0", mode]), + /--previous-tag is preview-only/, + ); + } + assert.equal( + parseNotesArgs([...BASE, "--previous-tag", "2.8.0"]).previousTag, + "2.8.0", + ); }); test("previousStableTag skips RC, hotfix and v-prefixed tags", () => { From 637f88b2dff2891ee4af3bc5d3a94752b98ee704 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 5 Oct 2026 06:27:10 -0400 Subject: [PATCH 3/5] docs(release): quote the release:notes argument placeholders An unquoted parses as a shell redirection, so the documented command failed when copied. Co-Authored-By: Claude Opus 5.5 Signed-off-by: cliffhall --- .claude/skills/release/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index 0be30e3ee..e2229b8c2 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -229,7 +229,7 @@ would drop changes from the list), and asks the same `releases/generate-notes` API the UI's *Generate release notes* button uses. Run it twice, then publish in 3b: ```sh -ARGS=(--merge-branch v2/chore/milestone-merge-vX.Y.Z --ledger-url ) +ARGS=(--merge-branch "v2/chore/milestone-merge-vX.Y.Z" --ledger-url "https://claude.ai/artifact/") # Add one --known-issue "" per known issue, if any. npm run release:notes -- "${ARGS[@]}" # 1. preview: prints the notes, creates nothing From c78e764b5618e29b1bf9af5e35e20fa316eef08e Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 5 Oct 2026 07:00:36 -0400 Subject: [PATCH 4/5] fix(scripts): ignore closing keywords GitHub ignores in release:notes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GitHub does not treat a closing keyword inside an HTML comment, fenced or inline code, or a blockquote as a closure, so neither should the reporter credit: a PR documenting `Closes #41`, or a template's leftover comment, must not credit #41's author. Mask those before the keyword scan. A preview of 2.9.0 still reproduces its notes. Co-Authored-By: Claude Opus 5.5 Signed-off-by: cliffhall --- scripts/release-notes.mjs | 19 ++++++++++++++++++- scripts/release-notes.test.mjs | 24 ++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/scripts/release-notes.mjs b/scripts/release-notes.mjs index c1ab716cb..ce4784204 100644 --- a/scripts/release-notes.mjs +++ b/scripts/release-notes.mjs @@ -158,7 +158,24 @@ export function pullNumbersFrom(generated) { */ export function closingKeywordIssues(body) { const pattern = /\b(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?):?\s+#(\d+)\b/gi; - return [...(body ?? "").matchAll(pattern)].map((m) => +m[1]); + return [...proseOf(body ?? "").matchAll(pattern)].map((m) => +m[1]); +} + +/** + * The body with every place GitHub ignores a closing keyword blanked out: + * HTML comments (PR templates leave `` behind), fenced + * code, inline code and blockquotes. A keyword quoted in any of those does + * not close the issue, so it must not credit the issue's author either. + */ +export function proseOf(body) { + return body + .replace(/|$)/g, " ") + .replace( + /^ {0,3}(`{3,}|~{3,})[^\n]*\n[\s\S]*?(?:^ {0,3}\1[^\n]*$|(?![\s\S]))/gm, + " ", + ) + .replace(/(`+)[\s\S]*?\1/g, " ") + .replace(/^ {0,3}>.*$/gm, " "); } const CLOSING_QUERY = `query($n:Int!,$after:String){repository(owner:"${OWNER}",name:"${NAME}"){pullRequest(number:$n){body closingIssuesReferences(first:100,after:$after){pageInfo{hasNextPage endCursor} nodes{number repository{nameWithOwner}}}}}}`; diff --git a/scripts/release-notes.test.mjs b/scripts/release-notes.test.mjs index a6145e8fe..62d35b3de 100644 --- a/scripts/release-notes.test.mjs +++ b/scripts/release-notes.test.mjs @@ -112,6 +112,30 @@ test("closingKeywordIssues reads every closing keyword, never a cross-repo ref", assert.deepEqual(closingKeywordIssues(null), []); }); +test("closingKeywordIssues ignores keywords GitHub ignores: code, quotes, comments", () => { + const body = [ + "Closes #1", + "", + "", + "Documents `Closes #4` and ``fixes #5``.", + "```sh", + "Closes #6", + "```", + "~~~", + "resolves #7", + "~~~", + "> Closes #8", + " > fixes #9", + "Resolves #10", + ].join("\n"); + assert.deepEqual(closingKeywordIssues(body), [1, 10]); + // An unterminated fence or comment swallows the rest, as GitHub renders it. + assert.deepEqual(closingKeywordIssues("Closes #1\n```\nCloses #2"), [1]); + assert.deepEqual(closingKeywordIssues("Closes #1\n` behind), fenced - * code, inline code and blockquotes. A keyword quoted in any of those does + * code, inline code and blockquotes. Four-space indented code is left + * alone: telling it from an indented list continuation needs a full + * Markdown parser, and masking a real `Closes` line would lose a credit. A keyword quoted in any of those does * not close the issue, so it must not credit the issue's author either. */ export function proseOf(body) { - return body - .replace(/|$)/g, " ") - .replace( - /^ {0,3}(`{3,}|~{3,})[^\n]*\n[\s\S]*?(?:^ {0,3}\1[^\n]*$|(?![\s\S]))/gm, - " ", - ) - .replace(/(`+)[\s\S]*?\1/g, " ") - .replace(/^ {0,3}>.*$/gm, " "); + return ( + body + .replace(/|$)/g, " ") + // A closing fence is the opening fence's character, at least as many, + // and nothing after it but spaces or tabs; anything else is content. + .replace( + /^ {0,3}((`|~)\2{2,})[^\n]*\n[\s\S]*?(?:^ {0,3}\1\2*[ \t]*$|(?![\s\S]))/gm, + " ", + ) + .replace(/(`+)[\s\S]*?\1/g, " ") + .replace(/^ {0,3}>.*$/gm, " ") + ); } const CLOSING_QUERY = `query($n:Int!,$after:String){repository(owner:"${OWNER}",name:"${NAME}"){pullRequest(number:$n){body closingIssuesReferences(first:100,after:$after){pageInfo{hasNextPage endCursor} nodes{number repository{nameWithOwner}}}}}}`; diff --git a/scripts/release-notes.test.mjs b/scripts/release-notes.test.mjs index 62d35b3de..356cabbce 100644 --- a/scripts/release-notes.test.mjs +++ b/scripts/release-notes.test.mjs @@ -134,6 +134,15 @@ test("closingKeywordIssues ignores keywords GitHub ignores: code, quotes, commen // An unterminated fence or comment swallows the rest, as GitHub renders it. assert.deepEqual(closingKeywordIssues("Closes #1\n```\nCloses #2"), [1]); assert.deepEqual(closingKeywordIssues("Closes #1\n