From b1df103649086dc665644dd5d8bf4d7e5083ba83 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Wed, 7 Oct 2026 12:27:59 -0400 Subject: [PATCH 1/2] chore(scripts): script per-server release notes with a reporter Thanks section (release:notes) Add scripts/release-notes.mjs (npm run release:notes), modeled on the Inspector's helper and adapted to a seven-package release. It builds the Packages table at the merge SHA (registry-checked), one section per published package with its PRs and the community reporters it thanks, a Repository catch-all so no generated PR is dropped, GitHub's trailer, the ledger line and any known issues. Outside-PR authors are credited from a maintainer-written `Credit: @login` line. Preview by default; --draft creates the draft Release at the merge SHA; publishing stays manual. The release skill's step 5a uses the helper in place of the shell recipe, and 5b publishes the draft. The harvest instructions adopt the Credit line. Closes #5056 Co-Authored-By: Claude Opus 5.5 Signed-off-by: cliffhall --- .claude/skills/issue-triage/SKILL.md | 4 + .claude/skills/release/SKILL.md | 179 ++--- AGENTS.md | 2 +- RELEASING.md | 2 +- docs/contribution-model.md | 4 +- package.json | 1 + scripts/release-notes.mjs | 777 ++++++++++++++++++++++ scripts/release-notes.test.mjs | 940 +++++++++++++++++++++++++++ 8 files changed, 1798 insertions(+), 111 deletions(-) create mode 100644 scripts/release-notes.mjs create mode 100644 scripts/release-notes.test.mjs diff --git a/.claude/skills/issue-triage/SKILL.md b/.claude/skills/issue-triage/SKILL.md index ea2a40feb3..fd2c413ba1 100644 --- a/.claude/skills/issue-triage/SKILL.md +++ b/.claude/skills/issue-triage/SKILL.md @@ -397,6 +397,10 @@ For each PR marked for harvest, once the manifest is approved: does not approve it. The body states the problem in its own words (never just "see the PR"), links the PR as a prototype, and credits its author. Then score it in pass 1 like any other inflow. +- **The credit is a line of its own**, `Credit: @` (several logins + comma-separated), in the issue body or the comment. The release notes credit + the people on that line (`release` skill, 5a), and only when a maintainer + wrote it; a credit in prose is not read. - **A duplicate group gets one issue**, linking every PR in the group. ### 4. Close diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index 76c1de0dab..0c42d607e4 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -359,140 +359,103 @@ package, its version PR (2b or 2c) runs again first. ### 5a. Draft the release notes Draft these once the merge PR has merged, so `main` holds what is being -released. Drafting creates nothing on GitHub; the maintainer publishes (5b). - -The notes have five parts, in this order: - -1. **Packages.** What this release publishes and at which version. The tag is - the milestone's name, not a version, and seven packages carry seven - versions, so the notes are where a reader finds them. -2. **What's Changed.** GitHub's generated list of every PR since the previous - release. -3. **The release-ledger line**, linking the artifact from 4d. -4. **`## Known issues`**, only when there is one. It names the issue, who is +released. [`scripts/release-notes.mjs`](../../../scripts/release-notes.mjs) +(`npm run release:notes`) builds them; its header says why each part is +there. The notes are laid out per package, because a reader wants to know +what changed in the server they use: + +1. **`## Packages`.** Every package on the merge commit, its version, and + whether this release publishes it (the registry does not have that version + yet) or leaves it unchanged. The tag is the milestone's name, not a + version, so this table is where a reader finds the versions. +2. **One `## ` section per published package**: its + `### What's Changed` (the PRs that change files under its `src//`) and + its `### Thanks for helping us improve` (the community reporters of the + issues those PRs close). A PR that touches several servers is listed under + each. +3. **`## Repository`**: every PR no published section took (CI, docs, skills, + a server whose version did not change), so no PR in the generated list is + dropped. It carries its own Thanks. +4. GitHub's **New Contributors** block and **Full Changelog** link, verbatim. +5. **The release-ledger line**, linking the artifact from 4d. +6. **`## Known issues`**, only when you pass one. It names the issue, who is affected and the workaround. What counts as a known issue is a maintainer's judgment, so it is written by hand, never generated. -5. **`## 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** strip, so the people - credited here appear there too. -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, 2 and 5, in a scratch directory outside the checkout. +GitHub adds everyone `@`-mentioned in a release body to that release's +**Contributors** strip, so the people credited appear there too. + +Preview first. The default run prints the notes and creates nothing: ```sh REPO=modelcontextprotocol/servers -NOTES=$(mktemp -d) -git fetch origin main --tags - # The commit being released is the merge PR's merge commit, by SHA. `main` is # a moving name: if anything else landed on it since, "main" would be a tree -# the ledger never saw. Everything below targets the SHA. +# the ledger never saw. MERGE_SHA=$(gh pr view --repo "$REPO" --json mergeCommit --jq .mergeCommit.oid) || MERGE_SHA= [ -n "$MERGE_SHA" ] || echo "the merge PR has no merge commit: stop" >&2 -git rev-parse "$MERGE_SHA^{tree}" # must be the tree hash the ledger recorded - -# The previous release is the latest PUBLISHED Release, asked of GitHub rather -# than sorted out of the tag list: this repo's tags mix date stamps -# (2026.8.31), milestone names (vX.Y.Z) and older per-package tags, and no -# sort orders them correctly. -PREV=$(gh release view --repo "$REPO" --json tagName --jq .tagName) || PREV= -[ -n "$PREV" ] || echo "no previous release found: stop, do not guess one" >&2 -echo "$PREV → $MILESTONE" # sanity-check both - -# 1. Packages: each one's name and version on main, and whether this release -# publishes it. (HTTP 200 from the registry means that version is already -# there, so this release skips it.) -{ - echo "## Packages" - echo - echo "| Package | Version | Registry | In this release |" - echo "| --- | --- | --- | --- |" - node scripts/release-manifest.mjs | jq -r ' - (.npm | to_entries[] | [.value.name, .value.version, "npm"]), - (.pypi | to_entries[] | [.value.name, .value.version, "PyPI"]) | @tsv' \ - | while IFS=$'\t' read -r name version registry; do - case "$registry" in - npm) url="https://registry.npmjs.org/$(printf %s "$name" | sed 's|/|%2F|')/$version" ;; - PyPI) url="https://pypi.org/pypi/$name/$version/json" ;; - esac - code=$(curl -s -o /dev/null -w '%{http_code}' "$url") - case "$code" in - 200) state="unchanged" ;; - 404) state="**published**" ;; - *) state="UNKNOWN (HTTP $code): check by hand" ;; - esac - echo "| \`$name\` | \`$version\` | $registry | $state |" - done -} > "$NOTES/packages.md" - -# 2. What's Changed, exactly as the UI generates it. -gh api "repos/$REPO/releases/generate-notes" -f tag_name="$MILESTONE" \ - -f target_commitish="$MERGE_SHA" -f previous_tag_name="$PREV" --jq .body > "$NOTES/changed.md" - -# 5. 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]+' "$NOTES/changed.md" | cut -d/ -f2 | sort -un); do - gh api graphql -F n="$pr" -f query='query($n:Int!){repository(owner:"modelcontextprotocol",name:"servers"){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:"servers"){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 ")" }' > "$NOTES/thanks.txt" - -: > "$NOTES/thanks.md" # truncate first, so a rerun never keeps a stale section -[ -s "$NOTES/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 "$NOTES/thanks.txt"; } >> "$NOTES/thanks.md" - -# Assemble. Parts 3 and 4 are added by hand, between changed.md and thanks.md. -cat "$NOTES/packages.md" <(echo) "$NOTES/changed.md" "$NOTES/thanks.md" > "$NOTES/release-notes.md" -echo "$NOTES/release-notes.md" +git fetch origin main && git rev-parse "$MERGE_SHA^{tree}" # must be the tree hash the ledger recorded + +NOTES=$(mktemp) +npm run --silent release:notes -- --milestone "$MILESTONE" --merge-sha "$MERGE_SHA" \ + --merge-branch "v2/chore/$N-release-$MILESTONE" --ledger-url "" \ + > "$NOTES"; echo "EXIT=$?" ``` -Run the Packages step from a checkout of `$MERGE_SHA` (the verification -worktree, moved there with `git switch --detach "$MERGE_SHA"`, will do), since -it reads the manifests on disk. +Add `--known-issue ""` once per known issue. The helper reads the +manifests at `$MERGE_SHA` from GitHub, so it needs no checkout of that +commit. It takes a few minutes: it looks up every listed PR's files and +closing issues, and every reporter's permission. -Then add the ledger line and any known issue, and **read the result before -handing it over**. The rules behind the recipe: +**Read the preview before going on.** Then create the draft with the same +arguments plus `--draft`: +```sh +npm run --silent release:notes -- --milestone "$MILESTONE" --merge-sha "$MERGE_SHA" \ + --merge-branch "v2/chore/$N-release-$MILESTONE" --ledger-url "" --draft +``` + +`--draft` creates a **draft** Release tagged `$MILESTONE` at `$MERGE_SHA`. It +refuses a SHA that is not on `main`, and a tag that already has a Release or +draft (edit or delete that one instead). There is no `--publish`: publishing +starts `release.yml`, so it is the maintainer's act in 5b. + +The rules the helper applies: + +- **The previous release is the latest published Release**, asked of GitHub + rather than sorted out of the tag list: this repo's tags mix date stamps + (`2026.8.31`), milestone names (`vX.Y.Z`) and older per-package tags, and no + sort orders them. `--previous-tag` overrides it in a preview only. - **An issue counts when a listed PR closes it**, through either the explicit closing link (`closingIssuesReferences`, which every PR here gets from `addCloseIssueReferences`) or a `Closes / Fixes / Resolves #N` in the PR body. So an issue older than the release still counts when this release closed it. - **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. On a public repo, anyone without a role reads as - `read`, so they are credited. -- **Outside PRs are credited through their issue.** This repo closes an - outside PR and files an issue for a fix worth keeping, crediting the PR's - author in that issue. The recipe credits the issue's *author*, who is then a - maintainer and is excluded. Read the milestone's issues for such credits and - add those people to the Thanks section by hand. + maintainer is anyone with `admin`, `maintain` or `write` on the repo. Bots, + deleted accounts, PR numbers and other repos' issues are dropped. +- **Outside PRs are credited through a `Credit: @login` line.** This repo + closes an outside PR and files an issue for a fix worth keeping, crediting + the PR's author there (`issue-triage`, Harvest). The helper credits every + login on a `Credit:` line **a maintainer wrote**, in the issue's body or a + comment. A credit written as prose is not read: add the line. - **"Addresses", not "fixes."** The credited issues include feature requests. -- **Leave the section out** when no community reporter remains. -- **`UNKNOWN` in the Packages table is a stop**, not a row to publish: a - registry that did not answer says nothing about what will be released. +- **A Thanks subsection is left out** when no community reporter remains. +- **It fails fast.** A failed `gh` call, a GraphQL error, a missing PR or + issue, an unknown permission value, a generated line it does not recognize, + or a registry answer other than 200 or 404 aborts the run before anything is + created. Re-run it; never patch around a failure by hand. ### 5b. Tag and publish -**A maintainer does this**, after the merge PR has merged. Through the GitHub -UI: *Releases → Draft a new release → Choose a tag → type the milestone's name -(`vX.Y.Z`) → Create new tag on publish*. For **Target**, pick the merge commit -(`$MERGE_SHA` from 5a) under *Recent Commits*, not the branch `main`, then -paste the notes from 5a and publish. - -The same thing from the CLI, with the notes file from 5a: +**A maintainer does this**, after reading the draft 5a created. The draft +already carries the tag name (`vX.Y.Z`), the target (`$MERGE_SHA`, not the +branch `main`) and the notes; publishing creates the tag. Through the GitHub +UI: *Releases → the `vX.Y.Z` draft → Edit → Set as the latest release → +Publish release*. From the CLI: ```sh -gh release create "$MILESTONE" --repo modelcontextprotocol/servers \ - --target "$MERGE_SHA" --title "$MILESTONE" --notes-file "$NOTES/release-notes.md" --latest +gh release edit "$MILESTONE" --repo modelcontextprotocol/servers --draft=false --latest ``` The target is the SHA, so the tag lands on the commit the ledger verified, diff --git a/AGENTS.md b/AGENTS.md index 68d836d4d3..60ea150ff0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -54,7 +54,7 @@ servers/ ├── .changeset/ Pending changesets for the TypeScript servers, and the changesets config ├── scripts/ The pre-push gate (gate-lease, smoke-servers, validate-py, coverage-py), its guards (verify-*), │ the skills tooling, and release tooling (npm-publish-guard, prepare-python-release, -│ pack-and-verify, release-manifest) +│ pack-and-verify, release-manifest, release-notes) ├── docs/ Design documents; quality-gate.md is the gate's reference (stages, CI vs local, the lease); │ contribution-model.md holds the outside-PR backlog plan ├── .github/workflows/ typescript.yml, python.yml (per-package CI), release.yml (publishes on a GitHub Release), diff --git a/RELEASING.md b/RELEASING.md index 8dd2cedb9f..3bb8137ef2 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -83,7 +83,7 @@ A release ships one **milestone**. The step-by-step procedure, with its commands A milestone that touches both languages and needs an audit fix has all three. They are merged before the next step starts. 3. **The merge PR** takes `v2/main` into `main`. It is a pure merge: its branch is `origin/v2/main` with no commits of its own, and the merged tree equals `origin/v2/main`'s. Its body starts with `Part of #`, not `Closes`, because `main` is the default branch and a closing keyword there would close the issue before anything is published. It is merged with a merge commit, never squashed. 4. **The release ledger** is linked from the merge PR. It records, for the merge branch's tree: `npm run local:gate`; `npm run pack:verify` for each package; each server-facing issue in the milestone exercised through a client; and a targeted probe for each issue with no client surface. The maintainers approve the merge on it. A finding is fixed on `v2/main` and merged in again, never committed to the merge branch. -5. **A maintainer publishes a GitHub Release** targeting the merge commit on `main` (the commit the ledger verified, chosen by SHA rather than by the branch name, which may have moved): Releases → Draft a new release → create a tag named for the milestone (`vX.Y.Z`) → paste the drafted notes → Publish. The tag is a label, not a version. +5. **A maintainer publishes a GitHub Release** targeting the merge commit on `main` (the commit the ledger verified, chosen by SHA rather than by the branch name, which may have moved). [`scripts/release-notes.mjs`](scripts/release-notes.mjs) (`npm run release:notes`) writes the notes, one section per published package with its PRs and the community reporters it thanks, and with `--draft` creates the draft Release under a tag named for the milestone (`vX.Y.Z`). The maintainer reads the draft and publishes it. The tag is a label, not a version. 6. **The build jobs run, then the maintainer approves the `release` environment deployments.** Each package publishes if its version is not on the registry yet; the rest skip cleanly. 7. **The release issue is closed by hand**, and its card moves to Done. diff --git a/docs/contribution-model.md b/docs/contribution-model.md index bf581943e2..6a82da49b4 100644 --- a/docs/contribution-model.md +++ b/docs/contribution-model.md @@ -113,13 +113,15 @@ For each PR marked for harvest: then a search of open and closed issues), file nothing. The issue is the pointer in the close comment. Add the PR to that issue as a prototype in the same comment that credits its author, so the work is findable from the issue. + The credit is a line of its own, `Credit: @`: the release notes read + that line (`scripts/release-notes.mjs`), and only when a maintainer wrote it. - **Otherwise file one issue**, following `issue-create` for the labels (`v2`, exactly one type label, the server's `server-` scope label where one applies) and the board card, with one difference: **board it as `Incoming`, with no milestone.** Harvesting preserves a fix; it does not approve it. The issue then goes through triage's approval pass like any other inflow. The issue states the problem in its own words (not "see the PR"), links the PR - as a prototype, and credits its author. + as a prototype, and credits its author on a `Credit: @` line. - **A duplicate group** gets one issue, linking every PR in the group. ### Security fixes diff --git a/package.json b/package.json index 44af657abf..bdf58b2adc 100644 --- a/package.json +++ b/package.json @@ -44,6 +44,7 @@ "verify:dco": "node scripts/verify-dco.mjs", "smoke": "node scripts/smoke-servers.mjs", "pack:verify": "node scripts/pack-and-verify.mjs", + "release:notes": "node scripts/release-notes.mjs", "local:gate": "node scripts/gate-lease.mjs npm run local:gate:stages", "local:gate:stages": "npm run verify:install-fresh && npm run verify:dco && npm run validate && npm run coverage && npm run validate:py && npm run coverage:py && npm run verify:skills:cli && npm run smoke" }, diff --git a/scripts/release-notes.mjs b/scripts/release-notes.mjs new file mode 100644 index 0000000000..8e56ba361f --- /dev/null +++ b/scripts/release-notes.mjs @@ -0,0 +1,777 @@ +#!/usr/bin/env node +// Assemble a milestone release's notes, per server, and optionally create the +// draft GitHub Release (#5056) — +// `npm run release:notes -- --milestone vX.Y.Z --merge-sha --merge-branch --ledger-url [...]`. +// It replaces the shell recipe the release skill's step 5a carried, and is +// modeled on the MCP Inspector's helper (modelcontextprotocol/inspector#2550), +// adapted to a repository that publishes seven packages at seven versions. +// +// A reader of a release wants to know what changed in the server they use, so +// the notes are laid out per PACKAGE rather than as one flat list: +// +// ## Packages every package, its version, and whether +// this release publishes it (registry check) +// ## one per PUBLISHED package: its PRs, then +// ### What's Changed Thanks to the reporters of the issues +// ### Thanks for helping us improve those PRs close +// ## Repository every PR left over (CI, docs, skills, a +// server whose version did not change), so +// no PR in the generated list is dropped +// ## New Contributors / Full Changelog GitHub's own trailer, verbatim +// Release ledger: []() +// ## Known issues only when passed in: a maintainer's call +// +// The PR list is GitHub's generated one (`releases/generate-notes`, what the +// UI's button uses) from the latest PUBLISHED Release to the merge commit. A +// PR is bucketed by the files it changes: `src//` is that directory's +// package. Thanks credits each closed issue's author, plus anyone a +// maintainer names on a `Credit: @login` line in the issue's body or a comment +// (how an outside PR's author is credited once the PR is closed and an issue +// filed for the fix; see docs/contribution-model.md). Maintainers +// (admin/maintain/write), bots and deleted accounts are never credited. +// 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` call and registry request is checked and a failure +// throws before anything is created: a permission lookup that errors must +// never read as "community" (that could credit a maintainer), a rate-limited +// lookup must never drop a PR or a reporter, and a registry that does not +// answer says nothing about what this release publishes. A permission value or +// a generated-notes line outside the known set throws for the same reason. +// +// The default run is a PREVIEW that prints the notes and creates nothing. +// `--draft` creates a draft Release at the merge SHA under the milestone tag. +// There is deliberately no `--publish`: publishing starts `release.yml`, so it +// stays a maintainer's act on the draft (the release skill, step 5b). + +import { spawnSync } from "node:child_process"; +import { pathToFileURL } from "node:url"; +import { parseArgs } from "node:util"; +import { readProject } from "./prepare-python-release.mjs"; + +export const REPO = "modelcontextprotocol/servers"; +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_HEADING = "### Thanks for helping us improve"; +export const THANKS_LEAD_IN = + "This release addresses issues reported by these community members. Thank you for taking the time to file them:"; +export const REPOSITORY_HEADING = "## Repository"; + +/** + * @typedef {{ dir: string, name: string, version: string, registry: "npm" | "PyPI" }} Package + * @typedef {Package & { published: boolean }} CheckedPackage + * @typedef {{ pr: number, line: string }} Entry + * @typedef {{ heading: string, entries: Entry[] }} Section + * @typedef {(cmd: string, args: string[], options: object) => { status: number | null, stdout?: string, stderr?: string, error?: Error }} Spawn + */ + +/** + * @param {string[]} argv + */ +export function parseNotesArgs(argv) { + const { values } = parseArgs({ + args: argv, + options: { + milestone: { type: "string" }, + "merge-sha": { type: "string" }, + "merge-branch": { type: "string" }, + "ledger-url": { type: "string" }, + "known-issue": { type: "string", multiple: true }, + "previous-tag": { type: "string" }, + draft: { type: "boolean" }, + }, + }); + const milestone = values.milestone; + const mergeSha = values["merge-sha"]; + const mergeBranch = values["merge-branch"]; + const ledgerUrl = values["ledger-url"]; + if (!milestone || !mergeSha || !mergeBranch || !ledgerUrl) { + throw new Error( + "--milestone, --merge-sha, --merge-branch and --ledger-url are all required", + ); + } + // The tag is the milestone's name; a typo here becomes a tag on the release. + if (!/^v\d+\.\d+\.\d+$/.test(milestone)) { + throw new Error(`--milestone "${milestone}" is not vX.Y.Z`); + } + // A full SHA, never a branch name: `main` moves, and the Release must land + // on the commit the ledger verified. + if (!/^[0-9a-f]{40}$/.test(mergeSha)) { + throw new Error(`--merge-sha "${mergeSha}" is not a full 40-hex SHA`); + } + if (!/^https:\/\/\S+$/.test(ledgerUrl)) { + throw new Error(`--ledger-url "${ledgerUrl}" is not an https URL`); + } + // A Release created here always starts from the latest published Release; + // an override (a typo, a stale value) would publish the wrong range of + // changes and credits, so it is a preview-only knob. + if (values.draft && values["previous-tag"] !== undefined) { + throw new Error("--previous-tag is preview-only"); + } + return { + milestone, + mergeSha, + mergeBranch, + ledgerUrl, + knownIssues: values["known-issue"] ?? [], + previousTag: values["previous-tag"], + mode: values.draft ? "draft" : "preview", + }; +} + +/** + * @param {Spawn} spawn + * @param {string} cmd + * @param {string[]} args + * @param {string} [input] + */ +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(); +} + +/** + * @param {Spawn} spawn + * @param {string} query + * @param {Record} variables + */ +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 or an expression 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; +} + +const MANIFEST_QUERY = `query($expr:String!){repository(owner:"${OWNER}",name:"${NAME}"){object(expression:$expr){... on Tree{entries{name type object{... on Tree{entries{name object{... on Blob{text}}}}}}}}}}`; + +/** + * Every package under `src/` at `sha`, read from GitHub rather than a + * checkout, so the table describes the commit being released whatever is on + * disk. The same identities `release-manifest.mjs` reads, in directory order, + * npm before PyPI. + * + * @param {Spawn} spawn + * @param {string} sha + * @returns {Package[]} + */ +export function packagesAt(spawn, sha) { + const tree = graphql(spawn, MANIFEST_QUERY, { expr: `${sha}:src` }).repository + .object; + if (!tree?.entries) { + throw new Error(`no src/ tree at ${sha}`); + } + /** @type {Package[]} */ + const npm = []; + /** @type {Package[]} */ + const pypi = []; + const dirs = tree.entries + .filter((/** @type {{ type: string }} */ e) => e.type === "tree") + .sort( + (/** @type {{ name: string }} */ a, /** @type {{ name: string }} */ b) => + a.name.localeCompare(b.name, "en"), + ); + for (const dir of dirs) { + for (const file of dir.object.entries) { + if (file.name !== "package.json" && file.name !== "pyproject.toml") + continue; + const text = file.object?.text; + if (typeof text !== "string") { + throw new Error(`src/${dir.name}/${file.name} has no text at ${sha}`); + } + const { name, version } = + file.name === "package.json" ? JSON.parse(text) : readProject(text); + if (typeof name !== "string" || name === "") + throw new Error(`src/${dir.name}/${file.name} has no package name`); + if (typeof version !== "string" || version === "") + throw new Error(`src/${dir.name}/${file.name} has no version`); + (file.name === "package.json" ? npm : pypi).push({ + dir: dir.name, + name, + version, + registry: file.name === "package.json" ? "npm" : "PyPI", + }); + } + } + if (npm.length + pypi.length === 0) { + throw new Error(`no package manifests under src/ at ${sha}`); + } + return [...npm, ...pypi]; +} + +/** + * The registry URL that answers 200 when this exact version is already + * published, and 404 when it is not. + * + * @param {Package} pkg + */ +export function registryUrl(pkg) { + return pkg.registry === "npm" + ? `https://registry.npmjs.org/${pkg.name.replace("/", "%2F")}/${pkg.version}` + : `https://pypi.org/pypi/${pkg.name}/${pkg.version}/json`; +} + +/** + * Whether this release publishes each package: a version the registry does + * not have yet is published, one it has is skipped. Any other answer aborts. + * + * @param {Package[]} packages + * @param {typeof fetch} fetchImpl + * @returns {Promise} + */ +export async function checkRegistries(packages, fetchImpl) { + /** @type {CheckedPackage[]} */ + const checked = []; + for (const pkg of packages) { + const url = registryUrl(pkg); + const response = await fetchImpl(url, { method: "GET" }); + if (response.status !== 200 && response.status !== 404) { + throw new Error( + `${pkg.registry} answered HTTP ${response.status} for ${pkg.name}@${pkg.version} (${url}): UNKNOWN, not guessing`, + ); + } + checked.push({ ...pkg, published: response.status === 404 }); + } + return checked; +} + +/** @param {CheckedPackage[]} packages */ +export function formatPackages(packages) { + const rows = packages.map( + (p) => + `| \`${p.name}\` | \`${p.version}\` | ${p.registry} | ${p.published ? "**published**" : "unchanged"} |`, + ); + return [ + "## Packages", + "", + "| Package | Version | Registry | In this release |", + "| --- | --- | --- | --- |", + ...rows, + ].join("\n"); +} + +const PR_LINE = new RegExp( + `^\\* .* in https://github\\.com/${REPO}/pull/(\\d+)\\s*$`, +); + +/** + * Split GitHub's generated body into its PR entries and its trailer (the + * `## New Contributors` block and the `**Full Changelog**` line, kept + * verbatim). Headings, blank lines and HTML comments are dropped; any other + * line throws, so a format change on GitHub's side cannot silently drop a PR. + * + * @param {string} body + * @returns {{ entries: Entry[], trailer: string }} + */ +export function parseGenerated(body) { + /** @type {Entry[]} */ + const entries = []; + const seen = new Set(); + /** @type {string[]} */ + const contributors = []; + let changelog = ""; + let inContributors = false; + for (const raw of body.split("\n")) { + const line = raw.trimEnd(); + if (/^#{2,3} /.test(line)) { + inContributors = line === "## New Contributors"; + continue; + } + if (line === "" || /^$/.test(line)) continue; + if (line.startsWith("**Full Changelog**")) { + changelog = line; + continue; + } + if (inContributors && line.startsWith("* ")) { + contributors.push(line); + continue; + } + const match = PR_LINE.exec(line); + if (!match) { + throw new Error(`unrecognized line in the generated notes: ${line}`); + } + const pr = Number(match[1]); + if (seen.has(pr)) continue; + seen.add(pr); + entries.push({ pr, line }); + } + const trailer = [ + contributors.length + ? `## New Contributors\n\n${contributors.join("\n")}` + : "", + changelog, + ] + .filter(Boolean) + .join("\n\n"); + return { entries, trailer }; +} + +/** + * The package directories a PR's files fall under, of those given. + * + * @param {string[]} files + * @param {Set} dirs + * @returns {string[]} + */ +export function dirsTouched(files, dirs) { + const touched = new Set(); + for (const file of files) { + const match = /^src\/([^/]+)\//.exec(file); + if (match && dirs.has(match[1])) touched.add(match[1]); + } + return [...touched].sort(); +} + +/** + * @param {Spawn} spawn + * @param {number} pr + * @returns {string[]} + */ +export function filesOf(spawn, pr) { + const out = run(spawn, "gh", [ + "api", + "--paginate", + `repos/${REPO}/pulls/${pr}/files`, + "--jq", + ".[].filename", + ]); + return out === "" ? [] : out.split("\n"); +} + +/** + * One section per published package, in table order, then Repository with + * every entry no published section took. Each entry lands somewhere. + * + * @param {Entry[]} entries + * @param {CheckedPackage[]} packages + * @param {Map} touched PR → package directories it changes + * @returns {Section[]} + */ +export function bucket(entries, packages, touched) { + const published = packages.filter((p) => p.published); + /** @type {Section[]} */ + const sections = published.map((p) => ({ + heading: `## ${p.name} ${p.version}`, + entries: entries.filter((e) => (touched.get(e.pr) ?? []).includes(p.dir)), + })); + const placed = new Set(sections.flatMap((s) => s.entries.map((e) => e.pr))); + const rest = entries.filter((e) => !placed.has(e.pr)); + if (rest.length) + sections.push({ heading: REPOSITORY_HEADING, entries: rest }); + return sections; +} + +/** + * 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. + * + * @param {string | null | undefined} body + */ +export function closingKeywordIssues(body) { + const pattern = /\b(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?):?\s+#(\d+)\b/gi; + return [...proseOf(body ?? "").matchAll(pattern)].map((m) => Number(m[1])); +} + +/** + * The logins a `Credit: @a, @b` line names. One line per credit, at the start + * of a line, outside code and quotes, so a sentence that merely mentions + * someone credits nobody. + * + * @param {string | null | undefined} text + */ +export function creditLines(text) { + const logins = []; + for (const m of proseOf(text ?? "").matchAll(/^[ \t]*Credit:(.*)$/gim)) { + for (const at of m[1].matchAll( + /@([A-Za-z0-9](?:[A-Za-z0-9-]*[A-Za-z0-9])?)/g, + )) + logins.push(at[1]); + } + return logins; +} + +/** + * The text with every place GitHub ignores a closing keyword blanked out: + * HTML comments (PR templates leave `` behind), fenced + * 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 anyone either. + * + * @param {string} body + */ +export function proseOf(body) { + 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}}}}}}`; + +/** + * Every issue of this repo one PR closes: manual links (paginated) + body + * keywords. + * + * @param {Spawn} spawn + * @param {number} pr + * @returns {Set} + */ +export function issuesClosedBy(spawn, pr) { + const numbers = new Set(); + let body; + let after; + for (;;) { + /** @type {Record} */ + 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 ISSUE_QUERY = `query($n:Int!,$after:String){repository(owner:"${OWNER}",name:"${NAME}"){issueOrPullRequest(number:$n){__typename ... on Issue{author{login __typename} body comments(first:100,after:$after){pageInfo{hasNextPage endCursor} nodes{author{login __typename} body}}}}}}`; + +/** + * Who is up for credit on issue `n`, before the maintainer filter: its author, + * and every login a maintainer named on a `Credit:` line in the body or a + * comment. Empty when `n` is a PR, and a bot or deleted account is skipped. + * A `Credit:` line written by a non-maintainer is ignored, so a reporter + * cannot add names to the release notes. + * + * @param {Spawn} spawn + * @param {number} n + * @param {(login: string) => boolean} isMaintainerLogin + * @returns {string[]} + */ +export function candidatesFor(spawn, n, isMaintainerLogin) { + const found = []; + let after; + for (;;) { + /** @type {Record} */ + const variables = after === undefined ? { n } : { n, after }; + const node = graphql(spawn, ISSUE_QUERY, variables).repository + .issueOrPullRequest; + if (!node) { + throw new Error(`#${n} not found`); + } + if (node.__typename !== "Issue") return []; + // The body counts once, on the first page; every page adds its comments. + const texts = [...node.comments.nodes]; + if (after === undefined) { + if (node.author?.__typename === "User") found.push(node.author.login); + texts.unshift({ author: node.author, body: node.body }); + } + for (const { author, body } of texts) { + if (author?.__typename !== "User") continue; + const named = creditLines(body); + if (named.length && isMaintainerLogin(author.login)) found.push(...named); + } + if (!node.comments.pageInfo.hasNextPage) break; + after = node.comments.pageInfo.endCursor; + } + return [...new Set(found)]; +} + +/** + * True for a maintainer, false for a community member; throws otherwise. + * + * @param {Spawn} spawn + * @param {string} login + */ +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}`); +} + +/** + * Looks each PR, issue and login up once, however many sections share it. + * + * @param {Spawn} spawn + */ +export function creditLookup(spawn) { + /** @type {Map} */ + const maintainers = new Map(); + /** @type {Map>} */ + const closed = new Map(); + /** @type {Map} */ + const candidates = new Map(); + const isMaintainerLogin = (/** @type {string} */ login) => { + const key = login.toLowerCase(); + if (!maintainers.has(key)) maintainers.set(key, isMaintainer(spawn, login)); + return /** @type {boolean} */ (maintainers.get(key)); + }; + /** + * Community reporter → the issues of theirs this section's PRs close. + * + * @param {Entry[]} entries + * @returns {Map} + */ + return function reportersFor(entries) { + const issues = new Set(); + for (const { pr } of entries) { + if (!closed.has(pr)) closed.set(pr, issuesClosedBy(spawn, pr)); + for (const n of /** @type {Set} */ (closed.get(pr))) + issues.add(n); + } + /** @type {Map} */ + const reporters = new Map(); + for (const n of [...issues].sort((a, b) => a - b)) { + if (!candidates.has(n)) + candidates.set(n, candidatesFor(spawn, n, isMaintainerLogin)); + for (const login of /** @type {string[]} */ (candidates.get(n))) { + if (isMaintainerLogin(login)) continue; + const list = reporters.get(login) ?? []; + if (!list.includes(n)) list.push(n); + reporters.set(login, list); + } + } + return reporters; + }; +} + +/** + * One line per person, most issues first, then by name; "" when nobody. + * + * @param {Map} reporters + */ +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_HEADING}\n\n${THANKS_LEAD_IN}\n\n${lines.join("\n")}`; +} + +/** + * @param {Section} section + * @param {string} thanks + */ +export function formatSection(section, thanks) { + const changed = section.entries.length + ? section.entries.map((e) => e.line).join("\n") + : "No pull request in this range changed this package's directory."; + return [section.heading, `### What's Changed\n\n${changed}`, thanks] + .filter(Boolean) + .join("\n\n"); +} + +/** @param {string[]} knownIssues */ +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")}`; +} + +/** + * @param {{ packages: string, sections: string[], trailer: string, mergeBranch: string, ledgerUrl: string, knownIssues: string[] }} parts + */ +export function assembleNotes({ + packages, + sections, + trailer, + mergeBranch, + ledgerUrl, + knownIssues, +}) { + return ( + [ + packages, + ...sections, + trailer, + `Release ledger: [${mergeBranch}](${ledgerUrl})`, + formatKnownIssues(knownIssues), + ] + .filter(Boolean) + .join("\n\n") + "\n" + ); +} + +/** + * @param {string[]} [argv] + * @param {{ spawn?: Spawn, fetch?: typeof fetch }} [deps] + * @returns {Promise} the notes + */ +export async function main( + argv = process.argv.slice(2), + { spawn = spawnSync, fetch: fetchImpl = globalThis.fetch } = {}, +) { + const args = parseNotesArgs(argv); + + if (args.mode === "draft") { + // release.yml only checks that the tagged commit is on main; refuse here + // first, before a draft exists that someone could publish. + const status = run(spawn, "gh", [ + "api", + `repos/${REPO}/compare/main...${args.mergeSha}`, + "--jq", + ".status", + ]); + if (status !== "identical" && status !== "behind") { + throw new Error( + `${args.mergeSha} is not on main (compare status "${status}"): merge the milestone PR first`, + ); + } + // A second draft under the same tag is one more draft to publish by + // mistake: edit the existing one, or delete it, instead. + const tags = run(spawn, "gh", [ + "api", + "--paginate", + `repos/${REPO}/releases`, + "--jq", + ".[].tag_name", + ]).split("\n"); + if (tags.includes(args.milestone)) { + throw new Error( + `a Release (or draft) tagged ${args.milestone} already exists`, + ); + } + } + + // The previous release is the latest PUBLISHED Release, asked of GitHub + // rather than sorted out of the tag list: this repo's tags mix date stamps, + // milestone names and older per-package tags, and no sort orders them. + const previousTag = + args.previousTag ?? + run(spawn, "gh", [ + "release", + "view", + "--repo", + REPO, + "--json", + "tagName", + "--jq", + ".tagName", + ]); + if (!previousTag) { + throw new Error("no previous published Release found"); + } + console.error(`release notes: ${previousTag} → ${args.milestone}`); + + const packages = await checkRegistries( + packagesAt(spawn, args.mergeSha), + fetchImpl, + ); + + const { entries, trailer } = parseGenerated( + run(spawn, "gh", [ + "api", + `repos/${REPO}/releases/generate-notes`, + "-f", + `tag_name=${args.milestone}`, + "-f", + `target_commitish=${args.mergeSha}`, + "-f", + `previous_tag_name=${previousTag}`, + "--jq", + ".body", + ]), + ); + console.error(`bucketing ${entries.length} PRs by server`); + const dirs = new Set(packages.map((p) => p.dir)); + /** @type {Map} */ + const touched = new Map(); + for (const { pr } of entries) { + touched.set(pr, dirsTouched(filesOf(spawn, pr), dirs)); + } + + const reportersFor = creditLookup(spawn); + const sections = bucket(entries, packages, touched).map((section) => + formatSection(section, formatThanks(reportersFor(section.entries))), + ); + const notes = assembleNotes({ + packages: formatPackages(packages), + sections, + trailer, + mergeBranch: args.mergeBranch, + ledgerUrl: args.ledgerUrl, + knownIssues: args.knownIssues, + }); + + if (args.mode === "preview") { + process.stdout.write(notes); + console.error("preview only — nothing created; re-run with --draft"); + return notes; + } + const url = run( + spawn, + "gh", + [ + "release", + "create", + args.milestone, + "--repo", + REPO, + "--target", + args.mergeSha, + "--title", + args.milestone, + "--notes-file", + "-", + "--draft", + ], + notes, + ); + console.error( + `draft created: ${url} — review it, then publish it (this starts release.yml)`, + ); + return notes; +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) + main().catch((/** @type {Error} */ error) => { + console.error(`release-notes: ${error.message}`); + process.exitCode = 1; + }); diff --git a/scripts/release-notes.test.mjs b/scripts/release-notes.test.mjs new file mode 100644 index 0000000000..fcf9e74649 --- /dev/null +++ b/scripts/release-notes.test.mjs @@ -0,0 +1,940 @@ +// Tests for scripts/release-notes.mjs (#5056): per-server bucketing (one +// server, several, none), sections only for published packages, the +// Repository catch-all dropping no PR, per-section Thanks with every exclusion +// (maintainers by permission, bots, deleted accounts, PRs, other repos), the +// maintainer-only `Credit:` line, keyword parsing, pagination, fail-fast at +// every lookup, and that nothing is created without --draft. Run via +// `npm run test:scripts`. + +import assert from "node:assert/strict"; +import { test } from "node:test"; +import { + REPO, + THANKS_LEAD_IN, + assembleNotes, + bucket, + closingKeywordIssues, + creditLines, + dirsTouched, + formatKnownIssues, + formatPackages, + formatThanks, + main, + parseGenerated, + parseNotesArgs, + registryUrl, +} from "./release-notes.mjs"; + +const SHA = "a".repeat(40); +const BASE = [ + "--milestone", + "v2.0.0", + "--merge-sha", + SHA, + "--merge-branch", + "v2/chore/1-release-v2.0.0", + "--ledger-url", + "https://l.example/x", +]; +const PULL = (/** @type {number | string} */ n) => + `https://github.com/${REPO}/pull/${n}`; + +test("parseNotesArgs defaults to a preview and validates its inputs", () => { + assert.deepEqual(parseNotesArgs(BASE), { + milestone: "v2.0.0", + mergeSha: SHA, + mergeBranch: "v2/chore/1-release-v2.0.0", + ledgerUrl: "https://l.example/x", + knownIssues: [], + previousTag: undefined, + mode: "preview", + }); + assert.equal(parseNotesArgs([...BASE, "--draft"]).mode, "draft"); + assert.deepEqual( + parseNotesArgs([...BASE, "--known-issue", "a", "--known-issue", "b"]) + .knownIssues, + ["a", "b"], + ); + assert.throws(() => parseNotesArgs([]), /all required/); + const swap = (/** @type {string} */ flag, /** @type {string} */ value) => { + const argv = [...BASE]; + argv[argv.indexOf(flag) + 1] = value; + return argv; + }; + assert.throws( + () => parseNotesArgs(swap("--milestone", "2.0.0")), + /not vX\.Y\.Z/, + ); + assert.throws( + () => parseNotesArgs(swap("--merge-sha", "main")), + /not a full 40-hex SHA/, + ); + assert.throws( + () => parseNotesArgs(swap("--ledger-url", "ftp://x")), + /not an https URL/, + ); + assert.throws( + () => parseNotesArgs([...BASE, "--previous-tag", "2026.8.31", "--draft"]), + /--previous-tag is preview-only/, + ); + assert.equal( + parseNotesArgs([...BASE, "--previous-tag", "2026.8.31"]).previousTag, + "2026.8.31", + ); + // There is no --publish: publishing is the maintainer's act on the draft. + assert.throws(() => parseNotesArgs([...BASE, "--publish"]), /publish/); +}); + +test("registryUrl asks for the exact version on each registry", () => { + assert.equal( + registryUrl({ + dir: "memory", + name: "@modelcontextprotocol/server-memory", + version: "1.0.0", + registry: "npm", + }), + "https://registry.npmjs.org/@modelcontextprotocol%2Fserver-memory/1.0.0", + ); + assert.equal( + registryUrl({ + dir: "git", + name: "mcp-server-git", + version: "2026.10.1", + registry: "PyPI", + }), + "https://pypi.org/pypi/mcp-server-git/2026.10.1/json", + ); +}); + +test("formatPackages marks what this release publishes", () => { + assert.equal( + formatPackages([ + { + dir: "a", + name: "@s/a", + version: "1.0.0", + registry: "npm", + published: true, + }, + { + dir: "b", + name: "b", + version: "0.1", + registry: "PyPI", + published: false, + }, + ]), + [ + "## Packages", + "", + "| Package | Version | Registry | In this release |", + "| --- | --- | --- | --- |", + "| `@s/a` | `1.0.0` | npm | **published** |", + "| `b` | `0.1` | PyPI | unchanged |", + ].join("\n"), + ); +}); + +test("parseGenerated splits PR entries from GitHub's trailer", () => { + const body = [ + "", + "## What's Changed", + `* fix: a by @x in ${PULL(12)}`, + `* chore: b by @y in ${PULL(3)}`, + "", + "## New Contributors", + `* @y made their first contribution in ${PULL(3)}`, + "", + "**Full Changelog**: https://example/compare", + ].join("\n"); + assert.deepEqual(parseGenerated(body), { + entries: [ + { pr: 12, line: `* fix: a by @x in ${PULL(12)}` }, + { pr: 3, line: `* chore: b by @y in ${PULL(3)}` }, + ], + trailer: [ + "## New Contributors", + "", + `* @y made their first contribution in ${PULL(3)}`, + "", + "**Full Changelog**: https://example/compare", + ].join("\n"), + }); + // A category heading (a future .github/release.yml) is fine; a repeat is not + // listed twice. + assert.deepEqual( + parseGenerated(`### Fixes\n* a in ${PULL(1)}\n* a in ${PULL(1)}`).entries, + [{ pr: 1, line: `* a in ${PULL(1)}` }], + ); + assert.equal(parseGenerated(`* a in ${PULL(1)}`).trailer, ""); +}); + +test("parseGenerated throws on a line it does not know, rather than drop it", () => { + assert.throws( + () => + parseGenerated("## What's Changed\n* a in https://github.com/o/r/pull/1"), + /unrecognized line/, + ); + assert.throws( + () => parseGenerated("## What's Changed\nsomething new"), + /unrecognized line/, + ); +}); + +test("dirsTouched maps src// to known package directories only", () => { + const dirs = new Set(["git", "memory", "time"]); + assert.deepEqual(dirsTouched(["src/git/a.py"], dirs), ["git"]); + assert.deepEqual( + dirsTouched(["src/time/x", "src/git/y", "src/git/z", "README.md"], dirs), + ["git", "time"], + ); + assert.deepEqual( + dirsTouched(["scripts/x.mjs", "src/README.md", "src/unknown/x"], dirs), + [], + ); +}); + +test("bucket: one, several and no servers; unpublished and empty fall to Repository", () => { + const pkgs = [ + { dir: "git", name: "g", version: "2", registry: "PyPI", published: true }, + { dir: "time", name: "t", version: "2", registry: "PyPI", published: true }, + { + dir: "memory", + name: "m", + version: "1", + registry: "npm", + published: false, + }, + ]; + const e = (/** @type {number} */ pr) => ({ pr, line: `* ${pr}` }); + const entries = [e(1), e(2), e(3), e(4), e(5)]; + const touched = new Map([ + [1, ["git"]], + [2, ["git", "time"]], + [3, []], + [4, ["memory"]], + [5, ["memory", "time"]], + ]); + const sections = /** @type {const} */ (bucket(entries, pkgs, touched)).map( + (s) => [s.heading, s.entries.map((x) => x.pr)], + ); + assert.deepEqual(sections, [ + ["## g 2", [1, 2]], + ["## t 2", [2, 5]], + ["## Repository", [3, 4]], + ]); + // No leftovers → no Repository section; a published package with no PR + // still gets its section. + assert.deepEqual( + bucket([e(1)], pkgs, new Map([[1, ["git"]]])).map((s) => s.heading), + ["## g 2", "## t 2"], + ); +}); + +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("closingKeywordIssues ignores keywords GitHub ignores: code, quotes, comments", () => { + const body = [ + "Closes #1", + "", + "Documents `Closes #4` and ``fixes #5``.", + "```sh", + "Closes #6", + "```", + "> Closes #8", + "Resolves #10", + ].join("\n"); + assert.deepEqual(closingKeywordIssues(body), [1, 10]); + assert.deepEqual(closingKeywordIssues("Closes #1\n```\nCloses #2"), [1]); +}); + +test("creditLines reads `Credit:` lines only, outside code and quotes", () => { + assert.deepEqual( + creditLines( + [ + "Credit: @alice", + "credit: @bob, @carol-x", + "Thanks to @dave for the idea", // a mention is not a credit + "> Credit: @erin", + "`Credit: @frank`", + "```", + "Credit: @gina", + "```", + " Credit:@hank", + ].join("\n"), + ), + ["alice", "bob", "carol-x", "hank"], + ); + assert.deepEqual(creditLines(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: packages, sections, trailer, ledger, known issues", () => { + assert.equal( + assembleNotes({ + packages: "P", + sections: ["S1", "S2"], + trailer: "T", + mergeBranch: "mb", + ledgerUrl: "https://l", + knownIssues: ["K"], + }), + "P\n\nS1\n\nS2\n\nT\n\nRelease ledger: [mb](https://l)\n\n## Known issue\n\nK\n", + ); + assert.equal( + assembleNotes({ + packages: "P", + sections: [], + trailer: "", + mergeBranch: "mb", + ledgerUrl: "https://l", + knownIssues: [], + }), + "P\n\nRelease ledger: [mb](https://l)\n", + ); +}); + +// A fake `gh` over a small repo model, and a fake registry. `failOn` makes the +// first gh call whose joined argv includes it fail, the way a rate limit or a +// 404 would. +function world({ + packages = { + git: { file: "pyproject.toml", name: "mcp-server-git", version: "2.0.0" }, + time: { file: "pyproject.toml", name: "mcp-server-time", version: "2.0.0" }, + memory: { + file: "package.json", + name: "@modelcontextprotocol/server-memory", + version: "1.0.0", + }, + }, + onRegistry = ["mcp-server-time"], + /** @type {Record> }>} */ + pulls = {}, + /** @type {Record} */ + issues = {}, + /** @type {Record} */ + perms = {}, + /** @type {string | undefined} */ + failOn = undefined, + compare = "identical", + releases = ["2026.8.31"], +} = {}) { + /** @type {Array<{ cmd: string, args: string[], input?: string }>} */ + const calls = []; + const generated = [ + "## What's Changed", + ...Object.keys(pulls).map((n) => `* change ${n} by @dev in ${PULL(n)}`), + "", + "**Full Changelog**: https://example/compare", + ].join("\n"); + const ok = (/** @type {string} */ stdout) => ({ + status: 0, + stdout, + stderr: "", + }); + const tree = { + entries: [ + { name: "README.md", type: "blob", object: {} }, + ...Object.entries(packages).map(([dir, p]) => ({ + name: dir, + type: "tree", + object: { + entries: [ + { name: "README.md", object: { text: "" } }, + { + name: p.file, + object: { + text: + p.file === "package.json" + ? JSON.stringify({ name: p.name, version: p.version }) + : `[project]\nname = "${p.name}"\nversion = "${p.version}"\n`, + }, + }, + ], + }, + })), + ], + }; + /** + * @param {string} cmd + * @param {string[]} args + * @param {{ input?: string }} [opts] + */ + 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" }; + } + assert.equal(cmd, "gh"); + if (args[0] === "release" && args[1] === "view") return ok(releases[0]); + 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)), + ); + if (vars.query.includes("Blob")) { + assert.equal(vars.expr, `${SHA}:src`); + return ok(JSON.stringify({ data: { repository: { object: tree } } })); + } + const n = Number(vars.n); + const index = vars.after ? Number(vars.after) : 0; + if (vars.query.includes("closingIssuesReferences")) { + const pr = pulls[n]; + const pages = pr.pages ?? [pr.closing ?? []]; + 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, + ), + }, + }, + }, + }, + }), + ); + } + const issue = issues[n] ?? null; + let node = issue; + if (issue?.__typename === "Issue") { + const pages = issue.commentPages ?? [issue.comments ?? []]; + const hasNextPage = index + 1 < pages.length; + node = { + __typename: "Issue", + author: issue.author, + body: issue.body ?? "", + comments: { + pageInfo: { + hasNextPage, + endCursor: hasNextPage ? String(index + 1) : null, + }, + nodes: pages[index], + }, + }; + } + return ok( + JSON.stringify({ data: { repository: { issueOrPullRequest: node } } }), + ); + } + const files = args[2]?.match(/pulls\/(\d+)\/files$/); + if (files) return ok((pulls[Number(files[1])].files ?? []).join("\n")); + if (args[1].endsWith("/releases/generate-notes")) return ok(generated); + if (args[1].includes("/compare/")) return ok(compare); + if (args[2] === `repos/${REPO}/releases`) return ok(releases.join("\n")); + const login = args[1].match(/collaborators\/([^/]+)\/permission/)?.[1]; + assert.ok(login, `unexpected gh call: ${joined}`); + return ok(perms[login]); + }; + spawn.calls = calls; + /** @type {string[]} */ + const fetched = []; + /** @param {string} url */ + const fetch = async (url) => { + fetched.push(url); + const name = Object.values(packages).find((p) => + url.includes(p.name.replace("/", "%2F")), + )?.name; + return { status: name && onRegistry.includes(name) ? 200 : 404 }; + }; + return { spawn, fetch, fetched }; +} + +const user = (/** @type {string} */ login, extra = {}) => ({ + __typename: "Issue", + author: { login, __typename: "User" }, + ...extra, +}); +const said = (/** @type {string} */ login, /** @type {string} */ body) => ({ + author: { login, __typename: "User" }, + body, +}); + +/** @param {import("node:test").TestContext} t */ +function quiet(t) { + /** @type {string[]} */ + const out = []; + t.mock.method(console, "error", () => {}); + t.mock.method(process.stdout, "write", (/** @type {string} */ chunk) => { + out.push(chunk); + return true; + }); + return out; +} + +/** The text of one `## …` section of the notes. */ +function sectionOf(/** @type {string} */ notes, /** @type {string} */ heading) { + const start = notes.indexOf(`${heading}\n`); + assert.notEqual(start, -1, `no section ${heading}`); + const next = notes.indexOf("\n## ", start + heading.length); + return notes.slice(start, next === -1 ? undefined : next); +} + +test("notes: per-server sections with per-server Thanks, every PR placed", async (t) => { + const out = quiet(t); + const deps = world({ + pulls: { + 10: { files: ["src/git/a.py"], body: "Closes #1" }, + 11: { files: ["src/git/b.py", "src/memory/c.ts"], closing: [2] }, + 12: { files: [".github/workflows/x.yml"], body: "Fixes #3" }, + 13: { files: ["src/time/d.py"], body: "Closes #4" }, + }, + issues: { + 1: user("reporter"), + 2: user("reporter"), + 3: user("docs-person"), + 4: user("reporter"), + }, + perms: { reporter: "read", "docs-person": "triage" }, + }); + const notes = await main(BASE, deps); + assert.equal(out.join(""), notes); + + // time is already on the registry, so it is unchanged, gets no section, + // and its PR falls through to Repository rather than being dropped. + assert.match( + notes, + /\| `mcp-server-time` \| `2\.0\.0` \| PyPI \| unchanged \|/, + ); + assert.doesNotMatch(notes, /^## mcp-server-time/m); + assert.deepEqual(notes.match(/^## .*/gm), [ + "## Packages", + "## @modelcontextprotocol/server-memory 1.0.0", + "## mcp-server-git 2.0.0", + "## Repository", + ]); + + const git = sectionOf(notes, "## mcp-server-git 2.0.0"); + assert.match(git, /change 10 .*\n\* change 11 /); + assert.match(git, /\* @reporter \(#1, #2\)\n*$/); + const memory = sectionOf( + notes, + "## @modelcontextprotocol/server-memory 1.0.0", + ); + assert.match(memory, /change 11/); + assert.doesNotMatch(memory, /change 10/); + // The same reporter, credited per server with only that server's issues. + assert.match(memory, /\* @reporter \(#2\)\n*$/); + const repo = sectionOf(notes, "## Repository"); + assert.match(repo, /change 12[^\n]*\n\* change 13 /); + assert.match(repo, /\* @docs-person \(#3\)\n\* @reporter \(#4\)/); + + assert.match( + notes, + /\*\*Full Changelog\*\*: https:\/\/example\/compare\n\nRelease ledger: \[v2\/chore\/1-release-v2\.0\.0\]\(https:\/\/l\.example\/x\)\n$/, + ); + // Each PR's files, closing refs, each issue and each login: looked up once. + const count = (/** @type {string} */ s) => + deps.spawn.calls.filter((c) => c.args.join(" ").includes(s)).length; + assert.equal(count("pulls/11/files"), 1); + assert.equal(count("/permission"), 2); + assert.equal( + deps.spawn.calls.filter( + (c) => + c.args.join(" ").includes("issueOrPullRequest") && + c.args.includes("n=2"), + ).length, + 1, + ); + // The registries were asked about the exact versions on the merge SHA. + assert.deepEqual(deps.fetched, [ + "https://registry.npmjs.org/@modelcontextprotocol%2Fserver-memory/1.0.0", + "https://pypi.org/pypi/mcp-server-git/2.0.0/json", + "https://pypi.org/pypi/mcp-server-time/2.0.0/json", + ]); + // The preview creates nothing. + assert.equal( + deps.spawn.calls.some( + (c) => c.args[0] === "release" && c.args[1] === "create", + ), + false, + ); +}); + +test("Thanks excludes maintainers, bots, deleted accounts, PRs and other repos", async (t) => { + quiet(t); + const notes = await main( + BASE, + world({ + pulls: { + 10: { + files: ["src/git/a.py"], + body: "Closes #1\nFixes #2\nResolves #4, closes #5, closes #6", + closing: [ + 3, + { 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" }, + }), + ); + assert.match(notes, /\* @reporter \(#1, #3\)\n/); + assert.doesNotMatch(notes, /@maint|@bot|#5|#7/); +}); + +test("a maintainer's `Credit:` line credits an outside PR's author; anyone else's does not", async (t) => { + quiet(t); + const notes = await main( + BASE, + world({ + pulls: { + 10: { + files: ["src/git/a.py"], + body: "Closes #1\nCloses #2\nCloses #3", + }, + }, + issues: { + // Filed by a maintainer from a closed outside PR, crediting its author. + 1: user("maint", { body: "Prototype: #900.\n\nCredit: @pr-author" }), + // Credited later, in a maintainer's comment, across comment pages. + 2: user("reporter", { + commentPages: [ + [said("someone", "+1")], + [said("maint", "Credit: @other-author, @maint")], + ], + }), + // A non-maintainer cannot add names. + 3: user("reporter", { + body: "Credit: @sneaky", + comments: [said("reporter", "Credit: @sneaky2")], + }), + }, + perms: { + maint: "admin", + reporter: "read", + someone: "read", + "pr-author": "read", + "other-author": "none", + }, + }), + ); + const git = sectionOf(notes, "## mcp-server-git 2.0.0"); + assert.match(git, /\* @reporter \(#2, #3\)/); + assert.match(git, /\* @other-author \(#2\)/); + assert.match(git, /\* @pr-author \(#1\)/); + assert.doesNotMatch(git, /@maint|@sneaky|@someone/); +}); + +test("the Thanks subsection is left out when no community reporter remains", async (t) => { + quiet(t); + const notes = await main( + BASE, + world({ + pulls: { 10: { files: ["src/git/a.py"], body: "Closes #1" } }, + issues: { 1: user("maint") }, + perms: { maint: "maintain" }, + }), + ); + assert.doesNotMatch(notes, /Thanks/); + assert.match( + notes, + /## mcp-server-git 2\.0\.0\n\n### What's Changed\n\n\* change 10/, + ); +}); + +test("closing references are followed across every page", async (t) => { + quiet(t); + const deps = world({ + pulls: { 10: { files: ["src/git/a"], pages: [[1], [2], [3]] } }, + issues: { 1: user("a"), 2: user("b"), 3: user("c") }, + perms: { a: "read", b: "triage", c: "none" }, + }); + const notes = await main(BASE, deps); + assert.match(notes, /@a \(#1\)\n\* @b \(#2\)\n\* @c \(#3\)/); + const cursors = deps.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"]); +}); + +test("generate-notes runs from the latest published Release to the merge SHA", async (t) => { + quiet(t); + const deps = world(); + await main(BASE, deps); + const call = deps.spawn.calls.find((c) => + c.args[1]?.endsWith("/releases/generate-notes"), + ); + assert.deepEqual(call?.args.slice(2), [ + "-f", + "tag_name=v2.0.0", + "-f", + `target_commitish=${SHA}`, + "-f", + "previous_tag_name=2026.8.31", + "--jq", + ".body", + ]); + + const override = world(); + await main([...BASE, "--previous-tag", "2026.8.18"], override); + assert.ok( + override.spawn.calls.some((c) => + c.args.includes("previous_tag_name=2026.8.18"), + ), + ); + assert.equal( + override.spawn.calls.some((c) => c.args[0] === "release"), + false, + ); +}); + +for (const [what, failOn] of [ + ["the previous-release lookup", "release view"], + ["the manifest lookup", ":src"], + ["generate-notes", "generate-notes"], + ["a PR files lookup", "/files"], + ["a closing-references lookup", "closingIssuesReferences"], + ["an issue lookup", "issueOrPullRequest"], + ["a permission lookup", "/permission"], + ["the compare check", "/compare/"], + ["the existing-release check", `repos/${REPO}/releases --jq`], +]) { + test(`a failed ${what} aborts with nothing created`, async (t) => { + quiet(t); + const deps = world({ + pulls: { 10: { files: ["src/git/a"], body: "Closes #1" } }, + issues: { 1: user("maybe-maint") }, + perms: { "maybe-maint": "read" }, + failOn, + }); + await assert.rejects( + main([...BASE, "--draft"], deps), + /rate limit exceeded/, + ); + assert.equal( + deps.spawn.calls.some((c) => c.args[1] === "create"), + false, + ); + }); +} + +test("a registry answer other than 200/404 aborts — UNKNOWN is a stop", async (t) => { + quiet(t); + const deps = world(); + const fetch = async () => ({ status: 503 }); + await assert.rejects( + main([...BASE, "--draft"], { spawn: deps.spawn, fetch }), + /HTTP 503 .*UNKNOWN/, + ); + assert.equal( + deps.spawn.calls.some((c) => c.args[1] === "create"), + false, + ); +}); + +test("an unknown permission value aborts rather than reading as community", async (t) => { + quiet(t); + await assert.rejects( + main( + BASE, + world({ + pulls: { 10: { files: [], body: "Closes #1" } }, + issues: { 1: user("who") }, + perms: { who: "" }, + }), + ), + /unexpected permission "" for @who/, + ); +}); + +test("GraphQL errors and missing nodes abort", async (t) => { + quiet(t); + await assert.rejects( + main(BASE, world({ pulls: { 10: { files: [], body: "Closes #99" } } })), + /#99 not found/, + ); + + const inner = world({ pulls: { 10: { files: [] } } }); + const respond = (/** @type {string} */ stdout) => + /** @type {import("./release-notes.mjs").Spawn} */ ( + (cmd, args, opts) => + args[1] === "graphql" && + args.some((a) => a.includes("closingIssuesReferences")) + ? { status: 0, stdout, stderr: "" } + : inner.spawn(cmd, args, opts) + ); + await assert.rejects( + main(BASE, { + spawn: respond('{"errors":[{"message":"boom"}]}'), + fetch: inner.fetch, + }), + /boom/, + ); + await assert.rejects( + main(BASE, { + spawn: respond('{"data":{"repository":{"pullRequest":null}}}'), + fetch: inner.fetch, + }), + /PR #10 not found/, + ); +}); + +test("a manifest tree that is missing or malformed aborts", async (t) => { + quiet(t); + const inner = world(); + const tree = (/** @type {unknown} */ object) => + /** @type {import("./release-notes.mjs").Spawn} */ ( + (cmd, args, opts) => + args.some((a) => a.includes("Blob")) + ? { + status: 0, + stdout: JSON.stringify({ data: { repository: { object } } }), + stderr: "", + } + : inner.spawn(cmd, args, opts) + ); + const run = (/** @type {unknown} */ object) => + main(BASE, { spawn: tree(object), fetch: inner.fetch }); + await assert.rejects(run(null), /no src\/ tree/); + await assert.rejects(run({ entries: [] }), /no package manifests/); + const dir = (/** @type {unknown} */ file) => ({ + entries: [{ name: "x", type: "tree", object: { entries: [file] } }], + }); + await assert.rejects( + run(dir({ name: "package.json", object: null })), + /src\/x\/package\.json has no text/, + ); + await assert.rejects( + run(dir({ name: "package.json", object: { text: '{"version":"1"}' } })), + /has no package name/, + ); + await assert.rejects( + run(dir({ name: "package.json", object: { text: '{"name":"n"}' } })), + /has no version/, + ); +}); + +test("no previous published Release aborts", async (t) => { + quiet(t); + await assert.rejects( + main(BASE, world({ releases: [""] })), + /no previous published Release/, + ); +}); + +test("a spawn error is rethrown", async () => { + const boom = new Error("ENOENT"); + await assert.rejects( + main(BASE, { + spawn: () => ({ status: null, error: boom }), + fetch: world().fetch, + }), + (e) => e === boom, + ); +}); + +test("--draft creates a draft at the merge SHA under the milestone tag", async (t) => { + quiet(t); + const deps = world(); + const notes = await main([...BASE, "--draft"], deps); + const create = deps.spawn.calls.find((c) => c.args[1] === "create"); + assert.deepEqual(create?.args, [ + "release", + "create", + "v2.0.0", + "--repo", + REPO, + "--target", + SHA, + "--title", + "v2.0.0", + "--notes-file", + "-", + "--draft", + ]); + assert.equal(create?.input, notes); + // Never published, never marked latest: that is the maintainer's step. + assert.equal(create?.args.includes("--latest"), false); +}); + +test("--draft refuses a SHA that is not on main", async (t) => { + quiet(t); + for (const compare of ["ahead", "diverged"]) { + const deps = world({ compare }); + await assert.rejects( + main([...BASE, "--draft"], deps), + new RegExp(`not on main \\(compare status "${compare}"\\)`), + ); + assert.equal( + deps.spawn.calls.some((c) => c.args.join(" ").includes("generate-notes")), + false, + ); + } + const behind = world({ compare: "behind" }); + await main([...BASE, "--draft"], behind); + assert.ok(behind.spawn.calls.some((c) => c.args[1] === "create")); +}); + +test("--draft refuses when a Release or draft with that tag already exists", async (t) => { + quiet(t); + const deps = world({ releases: ["2026.8.31", "v2.0.0"] }); + await assert.rejects( + main([...BASE, "--draft"], deps), + /tagged v2\.0\.0 already exists/, + ); + assert.equal( + deps.spawn.calls.some((c) => c.args[1] === "create"), + false, + ); +}); From d3bd74d6d048eee09fccc28d7072d2947a2c3f63 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Wed, 7 Oct 2026 12:43:04 -0400 Subject: [PATCH 2/2] chore(scripts): one release-wide Thanks section, as the Inspector's; drop Credit lines Thanks is now a single `## Thanks for helping us improve` list at the end of the notes, laid out as the Inspector's release notes are, rather than a subsection per server. The maintainer-written `Credit: @login` convention is removed: outside-PR authors are added to the draft's Thanks by hand, as before, and the harvest docs are back to their v2/main wording. From Copilot's review: refuse a milestone tag that already exists (GitHub would use its commit, not --merge-sha), and count a rename's source path when bucketing a PR by server. Co-Authored-By: Claude Opus 5.5 Signed-off-by: cliffhall --- .claude/skills/issue-triage/SKILL.md | 4 - .claude/skills/release/SKILL.md | 30 ++-- RELEASING.md | 2 +- docs/contribution-model.md | 4 +- scripts/release-notes.mjs | 190 ++++++++++--------------- scripts/release-notes.test.mjs | 205 +++++++++++++-------------- 6 files changed, 193 insertions(+), 242 deletions(-) diff --git a/.claude/skills/issue-triage/SKILL.md b/.claude/skills/issue-triage/SKILL.md index fd2c413ba1..ea2a40feb3 100644 --- a/.claude/skills/issue-triage/SKILL.md +++ b/.claude/skills/issue-triage/SKILL.md @@ -397,10 +397,6 @@ For each PR marked for harvest, once the manifest is approved: does not approve it. The body states the problem in its own words (never just "see the PR"), links the PR as a prototype, and credits its author. Then score it in pass 1 like any other inflow. -- **The credit is a line of its own**, `Credit: @` (several logins - comma-separated), in the issue body or the comment. The release notes credit - the people on that line (`release` skill, 5a), and only when a maintainer - wrote it; a credit in prose is not read. - **A duplicate group gets one issue**, linking every PR in the group. ### 4. Close diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index 0c42d607e4..0b07c4b407 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -361,26 +361,27 @@ package, its version PR (2b or 2c) runs again first. Draft these once the merge PR has merged, so `main` holds what is being released. [`scripts/release-notes.mjs`](../../../scripts/release-notes.mjs) (`npm run release:notes`) builds them; its header says why each part is -there. The notes are laid out per package, because a reader wants to know +there. The changes are laid out per package, because a reader wants to know what changed in the server they use: 1. **`## Packages`.** Every package on the merge commit, its version, and whether this release publishes it (the registry does not have that version yet) or leaves it unchanged. The tag is the milestone's name, not a version, so this table is where a reader finds the versions. -2. **One `## ` section per published package**: its - `### What's Changed` (the PRs that change files under its `src//`) and - its `### Thanks for helping us improve` (the community reporters of the - issues those PRs close). A PR that touches several servers is listed under - each. +2. **One `## ` section per published package**, whose + `### What's Changed` lists the PRs that change files under its + `src//`. A PR that touches several servers is listed under each. 3. **`## Repository`**: every PR no published section took (CI, docs, skills, a server whose version did not change), so no PR in the generated list is - dropped. It carries its own Thanks. + dropped. 4. GitHub's **New Contributors** block and **Full Changelog** link, verbatim. 5. **The release-ledger line**, linking the artifact from 4d. 6. **`## Known issues`**, only when you pass one. It names the issue, who is affected and the workaround. What counts as a known issue is a maintainer's judgment, so it is written by hand, never generated. +7. **`## Thanks for helping us improve`**: one list for the whole release, as + the Inspector's release notes have it, crediting the community members + whose issues the listed PRs close, one line per person, most issues first. GitHub adds everyone `@`-mentioned in a release body to that release's **Contributors** strip, so the people credited appear there too. @@ -417,7 +418,9 @@ npm run --silent release:notes -- --milestone "$MILESTONE" --merge-sha "$MERGE_S `--draft` creates a **draft** Release tagged `$MILESTONE` at `$MERGE_SHA`. It refuses a SHA that is not on `main`, and a tag that already has a Release or -draft (edit or delete that one instead). There is no `--publish`: publishing +draft (edit or delete that one instead). Every run, preview included, refuses +a milestone tag that already exists: GitHub would build the notes and the +Release from that tag's commit, not `$MERGE_SHA`. There is no `--publish`: publishing starts `release.yml`, so it is the maintainer's act in 5b. The rules the helper applies: @@ -434,13 +437,14 @@ The rules the helper applies: - **Maintainers and bots are excluded by permission, not by name.** A maintainer is anyone with `admin`, `maintain` or `write` on the repo. Bots, deleted accounts, PR numbers and other repos' issues are dropped. -- **Outside PRs are credited through a `Credit: @login` line.** This repo +- **Outside PRs are credited through their issue, by hand.** This repo closes an outside PR and files an issue for a fix worth keeping, crediting - the PR's author there (`issue-triage`, Harvest). The helper credits every - login on a `Credit:` line **a maintainer wrote**, in the issue's body or a - comment. A credit written as prose is not read: add the line. + the PR's author in that issue (`issue-triage`, Harvest). The helper credits + the issue's *author*, who is then a maintainer and is excluded. Read the + milestone's issues for such credits and add those people to the draft's + Thanks section by hand before publishing. - **"Addresses", not "fixes."** The credited issues include feature requests. -- **A Thanks subsection is left out** when no community reporter remains. +- **The Thanks section is left out** when no community reporter remains. - **It fails fast.** A failed `gh` call, a GraphQL error, a missing PR or issue, an unknown permission value, a generated line it does not recognize, or a registry answer other than 200 or 404 aborts the run before anything is diff --git a/RELEASING.md b/RELEASING.md index 3bb8137ef2..fa33776762 100644 --- a/RELEASING.md +++ b/RELEASING.md @@ -83,7 +83,7 @@ A release ships one **milestone**. The step-by-step procedure, with its commands A milestone that touches both languages and needs an audit fix has all three. They are merged before the next step starts. 3. **The merge PR** takes `v2/main` into `main`. It is a pure merge: its branch is `origin/v2/main` with no commits of its own, and the merged tree equals `origin/v2/main`'s. Its body starts with `Part of #`, not `Closes`, because `main` is the default branch and a closing keyword there would close the issue before anything is published. It is merged with a merge commit, never squashed. 4. **The release ledger** is linked from the merge PR. It records, for the merge branch's tree: `npm run local:gate`; `npm run pack:verify` for each package; each server-facing issue in the milestone exercised through a client; and a targeted probe for each issue with no client surface. The maintainers approve the merge on it. A finding is fixed on `v2/main` and merged in again, never committed to the merge branch. -5. **A maintainer publishes a GitHub Release** targeting the merge commit on `main` (the commit the ledger verified, chosen by SHA rather than by the branch name, which may have moved). [`scripts/release-notes.mjs`](scripts/release-notes.mjs) (`npm run release:notes`) writes the notes, one section per published package with its PRs and the community reporters it thanks, and with `--draft` creates the draft Release under a tag named for the milestone (`vX.Y.Z`). The maintainer reads the draft and publishes it. The tag is a label, not a version. +5. **A maintainer publishes a GitHub Release** targeting the merge commit on `main` (the commit the ledger verified, chosen by SHA rather than by the branch name, which may have moved). [`scripts/release-notes.mjs`](scripts/release-notes.mjs) (`npm run release:notes`) writes the notes, one section per published package with its PRs and a Thanks section for the community members whose issues the release addresses, and with `--draft` creates the draft Release under a tag named for the milestone (`vX.Y.Z`). The maintainer reads the draft and publishes it. The tag is a label, not a version. 6. **The build jobs run, then the maintainer approves the `release` environment deployments.** Each package publishes if its version is not on the registry yet; the rest skip cleanly. 7. **The release issue is closed by hand**, and its card moves to Done. diff --git a/docs/contribution-model.md b/docs/contribution-model.md index 6a82da49b4..bf581943e2 100644 --- a/docs/contribution-model.md +++ b/docs/contribution-model.md @@ -113,15 +113,13 @@ For each PR marked for harvest: then a search of open and closed issues), file nothing. The issue is the pointer in the close comment. Add the PR to that issue as a prototype in the same comment that credits its author, so the work is findable from the issue. - The credit is a line of its own, `Credit: @`: the release notes read - that line (`scripts/release-notes.mjs`), and only when a maintainer wrote it. - **Otherwise file one issue**, following `issue-create` for the labels (`v2`, exactly one type label, the server's `server-` scope label where one applies) and the board card, with one difference: **board it as `Incoming`, with no milestone.** Harvesting preserves a fix; it does not approve it. The issue then goes through triage's approval pass like any other inflow. The issue states the problem in its own words (not "see the PR"), links the PR - as a prototype, and credits its author on a `Credit: @` line. + as a prototype, and credits its author. - **A duplicate group** gets one issue, linking every PR in the group. ### Security fixes diff --git a/scripts/release-notes.mjs b/scripts/release-notes.mjs index 8e56ba361f..913ad6f474 100644 --- a/scripts/release-notes.mjs +++ b/scripts/release-notes.mjs @@ -11,23 +11,22 @@ // // ## Packages every package, its version, and whether // this release publishes it (registry check) -// ## one per PUBLISHED package: its PRs, then -// ### What's Changed Thanks to the reporters of the issues -// ### Thanks for helping us improve those PRs close +// ## one per PUBLISHED package, with its +// ### What's Changed PRs // ## Repository every PR left over (CI, docs, skills, a // server whose version did not change), so // no PR in the generated list is dropped // ## New Contributors / Full Changelog GitHub's own trailer, verbatim // Release ledger: []() // ## Known issues only when passed in: a maintainer's call +// ## Thanks for helping us improve one release-wide list of the community +// members whose issues the PRs close, laid +// out as the Inspector's release notes are // // The PR list is GitHub's generated one (`releases/generate-notes`, what the // UI's button uses) from the latest PUBLISHED Release to the merge commit. A // PR is bucketed by the files it changes: `src//` is that directory's -// package. Thanks credits each closed issue's author, plus anyone a -// maintainer names on a `Credit: @login` line in the issue's body or a comment -// (how an outside PR's author is credited once the PR is closed and an issue -// filed for the fix; see docs/contribution-model.md). Maintainers +// package. Thanks credits each closed issue's author; maintainers // (admin/maintain/write), bots and deleted accounts are never credited. // GitHub adds everyone `@`-mentioned in a release body to its Contributors // strip, so that section is what puts the reporters there. @@ -58,7 +57,6 @@ const [OWNER, NAME] = REPO.split("/"); const MAINTAINER_PERMISSIONS = new Set(["admin", "maintain", "write"]); const COMMUNITY_PERMISSIONS = new Set(["triage", "read", "none"]); -export const THANKS_HEADING = "### Thanks for helping us improve"; export const THANKS_LEAD_IN = "This release addresses issues reported by these community members. Thank you for taking the time to file them:"; export const REPOSITORY_HEADING = "## Repository"; @@ -352,7 +350,9 @@ export function filesOf(spawn, pr) { "--paginate", `repos/${REPO}/pulls/${pr}/files`, "--jq", - ".[].filename", + // A rename's source is in previous_filename: a file moved out of a + // server's directory still changed that server. + ".[] | .filename, (.previous_filename // empty)", ]); return out === "" ? [] : out.split("\n"); } @@ -392,31 +392,13 @@ export function closingKeywordIssues(body) { return [...proseOf(body ?? "").matchAll(pattern)].map((m) => Number(m[1])); } -/** - * The logins a `Credit: @a, @b` line names. One line per credit, at the start - * of a line, outside code and quotes, so a sentence that merely mentions - * someone credits nobody. - * - * @param {string | null | undefined} text - */ -export function creditLines(text) { - const logins = []; - for (const m of proseOf(text ?? "").matchAll(/^[ \t]*Credit:(.*)$/gim)) { - for (const at of m[1].matchAll( - /@([A-Za-z0-9](?:[A-Za-z0-9-]*[A-Za-z0-9])?)/g, - )) - logins.push(at[1]); - } - return logins; -} - /** * The text with every place GitHub ignores a closing keyword blanked out: * HTML comments (PR templates leave `` behind), fenced * 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 anyone either. + * any of those does not close the issue, so it must not credit its author either. * * @param {string} body */ @@ -469,47 +451,25 @@ export function issuesClosedBy(spawn, pr) { return numbers; } -const ISSUE_QUERY = `query($n:Int!,$after:String){repository(owner:"${OWNER}",name:"${NAME}"){issueOrPullRequest(number:$n){__typename ... on Issue{author{login __typename} body comments(first:100,after:$after){pageInfo{hasNextPage endCursor} nodes{author{login __typename} body}}}}}}`; +const AUTHOR_QUERY = `query($n:Int!){repository(owner:"${OWNER}",name:"${NAME}"){issueOrPullRequest(number:$n){__typename ... on Issue{author{login __typename}}}}}`; /** - * Who is up for credit on issue `n`, before the maintainer filter: its author, - * and every login a maintainer named on a `Credit:` line in the body or a - * comment. Empty when `n` is a PR, and a bot or deleted account is skipped. - * A `Credit:` line written by a non-maintainer is ignored, so a reporter - * cannot add names to the release notes. + * 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 or a + * deleted account. * * @param {Spawn} spawn * @param {number} n - * @param {(login: string) => boolean} isMaintainerLogin - * @returns {string[]} + * @returns {string | null} */ -export function candidatesFor(spawn, n, isMaintainerLogin) { - const found = []; - let after; - for (;;) { - /** @type {Record} */ - const variables = after === undefined ? { n } : { n, after }; - const node = graphql(spawn, ISSUE_QUERY, variables).repository - .issueOrPullRequest; - if (!node) { - throw new Error(`#${n} not found`); - } - if (node.__typename !== "Issue") return []; - // The body counts once, on the first page; every page adds its comments. - const texts = [...node.comments.nodes]; - if (after === undefined) { - if (node.author?.__typename === "User") found.push(node.author.login); - texts.unshift({ author: node.author, body: node.body }); - } - for (const { author, body } of texts) { - if (author?.__typename !== "User") continue; - const named = creditLines(body); - if (named.length && isMaintainerLogin(author.login)) found.push(...named); - } - if (!node.comments.pageInfo.hasNextPage) break; - after = node.comments.pageInfo.endCursor; +export function creditableAuthor(spawn, n) { + const node = graphql(spawn, AUTHOR_QUERY, { n }).repository + .issueOrPullRequest; + if (!node) { + throw new Error(`#${n} not found`); } - return [...new Set(found)]; + if (node.__typename !== "Issue") return null; + return node.author?.__typename === "User" ? node.author.login : null; } /** @@ -531,49 +491,33 @@ export function isMaintainer(spawn, login) { } /** - * Looks each PR, issue and login up once, however many sections share it. + * Community reporter → the issues of theirs the listed PRs close. * * @param {Spawn} spawn + * @param {number[]} pulls + * @returns {Map} */ -export function creditLookup(spawn) { +export function collectReporters(spawn, pulls) { + const issues = new Set(); + for (const pr of pulls) { + for (const n of issuesClosedBy(spawn, pr)) issues.add(n); + } + /** @type {Map} */ + const reporters = new Map(); /** @type {Map} */ - const maintainers = new Map(); - /** @type {Map>} */ - const closed = new Map(); - /** @type {Map} */ - const candidates = new Map(); - const isMaintainerLogin = (/** @type {string} */ login) => { - const key = login.toLowerCase(); - if (!maintainers.has(key)) maintainers.set(key, isMaintainer(spawn, login)); - return /** @type {boolean} */ (maintainers.get(key)); - }; - /** - * Community reporter → the issues of theirs this section's PRs close. - * - * @param {Entry[]} entries - * @returns {Map} - */ - return function reportersFor(entries) { - const issues = new Set(); - for (const { pr } of entries) { - if (!closed.has(pr)) closed.set(pr, issuesClosedBy(spawn, pr)); - for (const n of /** @type {Set} */ (closed.get(pr))) - issues.add(n); + 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)); } - /** @type {Map} */ - const reporters = new Map(); - for (const n of [...issues].sort((a, b) => a - b)) { - if (!candidates.has(n)) - candidates.set(n, candidatesFor(spawn, n, isMaintainerLogin)); - for (const login of /** @type {string[]} */ (candidates.get(n))) { - if (isMaintainerLogin(login)) continue; - const list = reporters.get(login) ?? []; - if (!list.includes(n)) list.push(n); - reporters.set(login, list); - } - } - return reporters; - }; + if (maintainer.get(login)) continue; + const list = reporters.get(login) ?? []; + list.push(n); + reporters.set(login, list); + } + return reporters; } /** @@ -592,20 +536,15 @@ export function formatThanks(reporters) { .map( ([login, nums]) => `* @${login} (${nums.map((n) => `#${n}`).join(", ")})`, ); - return `${THANKS_HEADING}\n\n${THANKS_LEAD_IN}\n\n${lines.join("\n")}`; + return `## Thanks for helping us improve\n\n${THANKS_LEAD_IN}\n\n${lines.join("\n")}`; } -/** - * @param {Section} section - * @param {string} thanks - */ -export function formatSection(section, thanks) { +/** @param {Section} section */ +export function formatSection(section) { const changed = section.entries.length ? section.entries.map((e) => e.line).join("\n") : "No pull request in this range changed this package's directory."; - return [section.heading, `### What's Changed\n\n${changed}`, thanks] - .filter(Boolean) - .join("\n\n"); + return `${section.heading}\n\n### What's Changed\n\n${changed}`; } /** @param {string[]} knownIssues */ @@ -616,7 +555,7 @@ export function formatKnownIssues(knownIssues) { } /** - * @param {{ packages: string, sections: string[], trailer: string, mergeBranch: string, ledgerUrl: string, knownIssues: string[] }} parts + * @param {{ packages: string, sections: string[], trailer: string, mergeBranch: string, ledgerUrl: string, knownIssues: string[], thanks: string }} parts */ export function assembleNotes({ packages, @@ -625,6 +564,7 @@ export function assembleNotes({ mergeBranch, ledgerUrl, knownIssues, + thanks, }) { return ( [ @@ -633,6 +573,7 @@ export function assembleNotes({ trailer, `Release ledger: [${mergeBranch}](${ledgerUrl})`, formatKnownIssues(knownIssues), + thanks, ] .filter(Boolean) .join("\n\n") + "\n" @@ -650,6 +591,22 @@ export async function main( ) { const args = parseNotesArgs(argv); + // When the tag already exists, GitHub ignores target_commitish (and + // `--target`) in favor of the tag's commit, so the generated range and the + // draft could describe a commit other than --merge-sha. Refuse in every + // mode, before anything is generated. + const existing = run(spawn, "gh", [ + "api", + `repos/${REPO}/git/matching-refs/tags/${args.milestone}`, + "--jq", + ".[].ref", + ]).split("\n"); + if (existing.includes(`refs/tags/${args.milestone}`)) { + throw new Error( + `tag ${args.milestone} already exists: GitHub would use its commit, not ${args.mergeSha}`, + ); + } + if (args.mode === "draft") { // release.yml only checks that the tagged commit is on main; refuse here // first, before a draft exists that someone could publish. @@ -727,9 +684,15 @@ export async function main( touched.set(pr, dirsTouched(filesOf(spawn, pr), dirs)); } - const reportersFor = creditLookup(spawn); - const sections = bucket(entries, packages, touched).map((section) => - formatSection(section, formatThanks(reportersFor(section.entries))), + const sections = bucket(entries, packages, touched).map(formatSection); + console.error( + `crediting reporters of issues closed by ${entries.length} PRs`, + ); + const thanks = formatThanks( + collectReporters( + spawn, + entries.map((e) => e.pr), + ), ); const notes = assembleNotes({ packages: formatPackages(packages), @@ -738,6 +701,7 @@ export async function main( mergeBranch: args.mergeBranch, ledgerUrl: args.ledgerUrl, knownIssues: args.knownIssues, + thanks, }); if (args.mode === "preview") { diff --git a/scripts/release-notes.test.mjs b/scripts/release-notes.test.mjs index fcf9e74649..5cfaf197fc 100644 --- a/scripts/release-notes.test.mjs +++ b/scripts/release-notes.test.mjs @@ -1,9 +1,9 @@ // Tests for scripts/release-notes.mjs (#5056): per-server bucketing (one // server, several, none), sections only for published packages, the -// Repository catch-all dropping no PR, per-section Thanks with every exclusion -// (maintainers by permission, bots, deleted accounts, PRs, other repos), the -// maintainer-only `Credit:` line, keyword parsing, pagination, fail-fast at -// every lookup, and that nothing is created without --draft. Run via +// Repository catch-all dropping no PR, the release-wide Thanks section (laid +// out as the Inspector's) with every exclusion (maintainers by permission, +// bots, deleted accounts, PRs, other repos), keyword parsing, pagination, +// fail-fast at every lookup, and that nothing is created without --draft. Run via // `npm run test:scripts`. import assert from "node:assert/strict"; @@ -14,7 +14,6 @@ import { assembleNotes, bucket, closingKeywordIssues, - creditLines, dirsTouched, formatKnownIssues, formatPackages, @@ -259,26 +258,6 @@ test("closingKeywordIssues ignores keywords GitHub ignores: code, quotes, commen assert.deepEqual(closingKeywordIssues("Closes #1\n```\nCloses #2"), [1]); }); -test("creditLines reads `Credit:` lines only, outside code and quotes", () => { - assert.deepEqual( - creditLines( - [ - "Credit: @alice", - "credit: @bob, @carol-x", - "Thanks to @dave for the idea", // a mention is not a credit - "> Credit: @erin", - "`Credit: @frank`", - "```", - "Credit: @gina", - "```", - " Credit:@hank", - ].join("\n"), - ), - ["alice", "bob", "carol-x", "hank"], - ); - assert.deepEqual(creditLines(null), []); -}); - test("formatThanks orders by issue count, then name case-insensitively", () => { const reporters = new Map([ ["zed", [5]], @@ -289,7 +268,7 @@ test("formatThanks orders by issue count, then name case-insensitively", () => { assert.equal( formatThanks(reporters), [ - "### Thanks for helping us improve", + "## Thanks for helping us improve", "", THANKS_LEAD_IN, "", @@ -308,7 +287,7 @@ test("formatKnownIssues pluralizes and is omitted when empty", () => { assert.equal(formatKnownIssues(["a", "b"]), "## Known issues\n\na\n\nb"); }); -test("assembleNotes: packages, sections, trailer, ledger, known issues", () => { +test("assembleNotes: packages, sections, trailer, ledger, known issues, thanks", () => { assert.equal( assembleNotes({ packages: "P", @@ -317,8 +296,9 @@ test("assembleNotes: packages, sections, trailer, ledger, known issues", () => { mergeBranch: "mb", ledgerUrl: "https://l", knownIssues: ["K"], + thanks: "## Thanks for helping us improve\n\nX", }), - "P\n\nS1\n\nS2\n\nT\n\nRelease ledger: [mb](https://l)\n\n## Known issue\n\nK\n", + "P\n\nS1\n\nS2\n\nT\n\nRelease ledger: [mb](https://l)\n\n## Known issue\n\nK\n\n## Thanks for helping us improve\n\nX\n", ); assert.equal( assembleNotes({ @@ -328,6 +308,7 @@ test("assembleNotes: packages, sections, trailer, ledger, known issues", () => { mergeBranch: "mb", ledgerUrl: "https://l", knownIssues: [], + thanks: "", }), "P\n\nRelease ledger: [mb](https://l)\n", ); @@ -347,9 +328,9 @@ function world({ }, }, onRegistry = ["mcp-server-time"], - /** @type {Record> }>} */ + /** @type {Record, body?: string, closing?: number[], pages?: Array> }>} */ pulls = {}, - /** @type {Record} */ + /** @type {Record} */ issues = {}, /** @type {Record} */ perms = {}, @@ -357,6 +338,8 @@ function world({ failOn = undefined, compare = "identical", releases = ["2026.8.31"], + /** @type {string[]} */ + tagRefs = [], } = {}) { /** @type {Array<{ cmd: string, args: string[], input?: string }>} */ const calls = []; @@ -447,30 +430,24 @@ function world({ }), ); } - const issue = issues[n] ?? null; - let node = issue; - if (issue?.__typename === "Issue") { - const pages = issue.commentPages ?? [issue.comments ?? []]; - const hasNextPage = index + 1 < pages.length; - node = { - __typename: "Issue", - author: issue.author, - body: issue.body ?? "", - comments: { - pageInfo: { - hasNextPage, - endCursor: hasNextPage ? String(index + 1) : null, - }, - nodes: pages[index], - }, - }; - } return ok( - JSON.stringify({ data: { repository: { issueOrPullRequest: node } } }), + JSON.stringify({ + data: { repository: { issueOrPullRequest: issues[n] ?? null } }, + }), ); } const files = args[2]?.match(/pulls\/(\d+)\/files$/); - if (files) return ok((pulls[Number(files[1])].files ?? []).join("\n")); + if (files) { + // The --jq emits each file, then its rename source when it has one. + const { files: list = [], renamedFrom = {} } = pulls[Number(files[1])]; + return ok( + list + .flatMap((f) => (renamedFrom[f] ? [f, renamedFrom[f]] : [f])) + .join("\n"), + ); + } + if (args[1].includes("/git/matching-refs/tags/")) + return ok(tagRefs.join("\n")); if (args[1].endsWith("/releases/generate-notes")) return ok(generated); if (args[1].includes("/compare/")) return ok(compare); if (args[2] === `repos/${REPO}/releases`) return ok(releases.join("\n")); @@ -497,11 +474,6 @@ const user = (/** @type {string} */ login, extra = {}) => ({ author: { login, __typename: "User" }, ...extra, }); -const said = (/** @type {string} */ login, /** @type {string} */ body) => ({ - author: { login, __typename: "User" }, - body, -}); - /** @param {import("node:test").TestContext} t */ function quiet(t) { /** @type {string[]} */ @@ -522,7 +494,7 @@ function sectionOf(/** @type {string} */ notes, /** @type {string} */ heading) { return notes.slice(start, next === -1 ? undefined : next); } -test("notes: per-server sections with per-server Thanks, every PR placed", async (t) => { +test("notes: per-server sections, every PR placed, one release-wide Thanks", async (t) => { const out = quiet(t); const deps = world({ pulls: { @@ -554,26 +526,42 @@ test("notes: per-server sections with per-server Thanks, every PR placed", async "## @modelcontextprotocol/server-memory 1.0.0", "## mcp-server-git 2.0.0", "## Repository", + "## Thanks for helping us improve", ]); const git = sectionOf(notes, "## mcp-server-git 2.0.0"); - assert.match(git, /change 10 .*\n\* change 11 /); - assert.match(git, /\* @reporter \(#1, #2\)\n*$/); + assert.match(git, /change 10 .*\n\* change 11 [^\n]*\n*$/); const memory = sectionOf( notes, "## @modelcontextprotocol/server-memory 1.0.0", ); assert.match(memory, /change 11/); assert.doesNotMatch(memory, /change 10/); - // The same reporter, credited per server with only that server's issues. - assert.match(memory, /\* @reporter \(#2\)\n*$/); const repo = sectionOf(notes, "## Repository"); - assert.match(repo, /change 12[^\n]*\n\* change 13 /); - assert.match(repo, /\* @docs-person \(#3\)\n\* @reporter \(#4\)/); - assert.match( + repo, + /change 12[^\n]*\n\* change 13 [^\n]*\n\n\*\*Full Changelog/, + ); + + // One Thanks section for the whole release, last, as the Inspector lays it + // out: each reporter once, with every issue of theirs across all servers. + assert.ok( + notes.endsWith( + [ + "**Full Changelog**: https://example/compare", + "", + "Release ledger: [v2/chore/1-release-v2.0.0](https://l.example/x)", + "", + "## Thanks for helping us improve", + "", + THANKS_LEAD_IN, + "", + "* @reporter (#1, #2, #4)", + "* @docs-person (#3)", + "", + ].join("\n"), + ), notes, - /\*\*Full Changelog\*\*: https:\/\/example\/compare\n\nRelease ledger: \[v2\/chore\/1-release-v2\.0\.0\]\(https:\/\/l\.example\/x\)\n$/, ); // Each PR's files, closing refs, each issue and each login: looked up once. const count = (/** @type {string} */ s) => @@ -633,50 +621,7 @@ test("Thanks excludes maintainers, bots, deleted accounts, PRs and other repos", assert.doesNotMatch(notes, /@maint|@bot|#5|#7/); }); -test("a maintainer's `Credit:` line credits an outside PR's author; anyone else's does not", async (t) => { - quiet(t); - const notes = await main( - BASE, - world({ - pulls: { - 10: { - files: ["src/git/a.py"], - body: "Closes #1\nCloses #2\nCloses #3", - }, - }, - issues: { - // Filed by a maintainer from a closed outside PR, crediting its author. - 1: user("maint", { body: "Prototype: #900.\n\nCredit: @pr-author" }), - // Credited later, in a maintainer's comment, across comment pages. - 2: user("reporter", { - commentPages: [ - [said("someone", "+1")], - [said("maint", "Credit: @other-author, @maint")], - ], - }), - // A non-maintainer cannot add names. - 3: user("reporter", { - body: "Credit: @sneaky", - comments: [said("reporter", "Credit: @sneaky2")], - }), - }, - perms: { - maint: "admin", - reporter: "read", - someone: "read", - "pr-author": "read", - "other-author": "none", - }, - }), - ); - const git = sectionOf(notes, "## mcp-server-git 2.0.0"); - assert.match(git, /\* @reporter \(#2, #3\)/); - assert.match(git, /\* @other-author \(#2\)/); - assert.match(git, /\* @pr-author \(#1\)/); - assert.doesNotMatch(git, /@maint|@sneaky|@someone/); -}); - -test("the Thanks subsection is left out when no community reporter remains", async (t) => { +test("the Thanks section is left out when no community reporter remains", async (t) => { quiet(t); const notes = await main( BASE, @@ -749,6 +694,7 @@ for (const [what, failOn] of [ ["a permission lookup", "/permission"], ["the compare check", "/compare/"], ["the existing-release check", `repos/${REPO}/releases --jq`], + ["the existing-tag check", "matching-refs"], ]) { test(`a failed ${what} aborts with nothing created`, async (t) => { quiet(t); @@ -938,3 +884,46 @@ test("--draft refuses when a Release or draft with that tag already exists", asy false, ); }); + +test("an existing milestone tag aborts in every mode, before notes are generated", async (t) => { + quiet(t); + for (const mode of [[], ["--draft"]]) { + const deps = world({ + // A prefix match (v2.0.0-rc.1) is not the tag itself. + tagRefs: ["refs/tags/v2.0.0-rc.1", "refs/tags/v2.0.0"], + }); + await assert.rejects( + main([...BASE, ...mode], deps), + /tag v2\.0\.0 already exists: GitHub would use its commit/, + ); + assert.equal( + deps.spawn.calls.some( + (c) => + c.args.join(" ").includes("generate-notes") || c.args[1] === "create", + ), + false, + ); + } + const prefixOnly = world({ tagRefs: ["refs/tags/v2.0.0-rc.1"] }); + await main(BASE, prefixOnly); +}); + +test("a file renamed out of a server's directory still places the PR there", async (t) => { + quiet(t); + const notes = await main( + BASE, + world({ + pulls: { + 10: { + files: ["docs/moved.md"], + renamedFrom: { "docs/moved.md": "src/git/old.md" }, + }, + }, + }), + ); + assert.match( + notes, + /## mcp-server-git 2\.0\.0\n\n### What's Changed\n\n\* change 10/, + ); + assert.doesNotMatch(notes, /## Repository/); +});