Derive changelog PR links from the squash commit

AGENTS.md

@@ -100,7 +100,7 @@ on `master`. CI runs on every PR in the stack.
 
 ## Testing and changelog
 
-**Add a changelog fragment** `changes/<description>.<category>.md` for any user-facing change (bug fix, feature, or breaking change) instead of editing `CHANGELOG.md`; see [changes/README.md](changes/README.md) for categories and format. Validate with `yarn changelog:check`. Since the PR number is not known yet, open the PR as a draft with the placeholder link `https://github.com/rescript-lang/rescript/pull/XXXX`, then replace it with the real link, amend, push again, and mark the PR ready for review (`gh pr ready`).
+**Add a changelog fragment** `changes/<description>.<category>.md` for any user-facing change (bug fix, feature, or breaking change) instead of editing `CHANGELOG.md`; see [changes/README.md](changes/README.md) for categories and format. Validate with `yarn changelog:check`. Fragments need no PR link: the release adds it from the squash commit that merged the fragment.
 
 For compiler or standard-library changes, run `make test`. For syntax changes,
 also run `make test-syntax`; use `make test-syntax-roundtrip` when parsing or

changes/README.md

@@ -3,10 +3,12 @@
 Add a unique `<description>.<category>.md` file for each change that needs release notes. For example, `fix-watcher-recovery.fix.md`:
 
 ```markdown
-- Recover blocked dependents after a failed build. https://github.com/rescript-lang/rescript/pull/8667
+- Recover blocked dependents after a failed build.
 ```
 
-The PR number is only known once the PR exists, so open it as a draft with the placeholder link `https://github.com/rescript-lang/rescript/pull/XXXX`, replace the placeholder with the real link, amend and push again, then mark the PR ready for review. The changelog check fails while the placeholder remains. Each file may contain multiple bullets in the same category. Indent wrapped lines by two spaces; each bullet ends with one or more PR links.
+Each file may contain multiple bullets in the same category. Indent wrapped lines by two spaces.
+
+A bullet needs no PR link: `yarn changelog:release` appends the link of the pull request that added the fragment, taken from the subject of its squash commit (`... (#8667)`). Pull requests are therefore squash-merged, keeping the `(#N)` GitHub adds to the title. A bullet may still end with explicit PR links, for example for a change spanning several pull requests, and those are kept as written. After each push to `master`, the changelog check fails if a fragment without a link cannot be traced to its pull request; add the link to that fragment.
 
 | Category | Section |
 | --- | --- |

changes/changelog-derived-pr-links.internal.md

@@ -0,0 +1 @@
+- Fragments no longer need a PR link; the release adds it from the squash commit that merged the fragment.

scripts/changelog.js

@@ -19,6 +19,7 @@ export const placeholderLink =
 const trailingPrLinks = new RegExp(
   `${prLink.source}(?:\\s+${prLink.source})*$`,
 );
+const repositoryUrl = "https://github.com/rescript-lang/rescript";
 
 export function parseFragment(name, content) {
   const match = /^([a-z0-9][a-z0-9-]*)\.([a-z]+)\.md$/.exec(name);
@@ -37,21 +38,74 @@ export function parseFragment(name, content) {
   const invalid = bullets.some(
     bullet =>
       !/^- \S/.test(bullet) ||
-      !trailingPrLinks.test(bullet) ||
       bullet
         .split("\n")
         .slice(1)
         .some(line => line && !/^ {2}\S/.test(line)),
   );
   if (!text || invalid) {
     throw new Error(
-      `${name}: expected Markdown bullets ending with PR links (indent continuation lines by two spaces)`,
+      `${name}: expected Markdown bullets (indent continuation lines by two spaces)`,
     );
   }
-  const pr = Math.max(
-    ...[...text.matchAll(new RegExp(prLink, "g"))].map(link => Number(link[1])),
+  const links = [...text.matchAll(new RegExp(prLink, "g"))].map(link =>
+    Number(link[1]),
   );
-  return { name, category: match[2], text, pr };
+  // A bullet without a link gets the link of the pull request that added the
+  // fragment when the release is prepared.
+  const unlinked = bullets.some(bullet => !trailingPrLinks.test(bullet));
+  const pr = links.length ? Math.max(...links) : null;
+  return { name, category: match[2], text, pr, unlinked };
+}
+
+// The pull request that added a fragment, from the subject of the squash
+// commit that added it ("... (#1234)"). Rebase merges and merge commits carry
+// no such subject, which is why pull requests are squash-merged.
+export function addedByPullRequest(root, name) {
+  const subjects = execFileSync(
+    "git",
+    ["log", "--diff-filter=A", "--format=%s", "--", `changes/${name}`],
+    { cwd: root, encoding: "utf8" },
+  )
+    .split("\n")
+    .filter(Boolean);
+  // The most recent addition, in case an earlier fragment had the same name.
+  const match = /\(#([1-9]\d*)\)\s*$/.exec(subjects[0] ?? "");
+  return match ? Number(match[1]) : null;
+}
+
+// Appends the link of the pull request that added the fragment to each bullet
+// without one.
+export function linkFragment(fragment, pullRequestOf) {
+  if (!fragment.unlinked) return fragment;
+  const pr = pullRequestOf(fragment.name);
+  if (!pr) {
+    throw new Error(
+      `changes/${fragment.name}: no PR link, and the commit that added it does not name its pull request; add the link to the fragment`,
+    );
+  }
+  const text = fragment.text
+    .split(/\n(?=- )/)
+    .map(bullet =>
+      trailingPrLinks.test(bullet)
+        ? bullet
+        : `${bullet} ${repositoryUrl}/pull/${pr}`,
+    )
+    .join("\n");
+  return {
+    ...fragment,
+    text,
+    pr: Math.max(pr, fragment.pr ?? 0),
+    unlinked: false,
+  };
+}
+
+// What the release will write for a fragment: linked once its pull request is
+// merged, and as written before that.
+export function previewFragment(fragment, pullRequestOf) {
+  if (!fragment.unlinked) return fragment;
+  const pr = pullRequestOf(fragment.name);
+  return pr ? linkFragment(fragment, () => pr) : fragment;
 }
 
 export async function readFragments(root) {
@@ -76,12 +130,17 @@ export async function readFragments(root) {
   return fragments;
 }
 
-// Newest PR first, matching the order of existing CHANGELOG.md entries.
+// Newest PR first, matching the order of existing CHANGELOG.md entries. A
+// fragment without a link yet belongs to an unmerged pull request, so it comes
+// first.
 function categoryNotes(fragments, category) {
+  const order = fragment => fragment.pr ?? Number.POSITIVE_INFINITY;
   return fragments
     .filter(fragment => fragment.category === category)
     .sort(
-      (a, b) => b.pr - a.pr || (a.name < b.name ? -1 : a.name > b.name ? 1 : 0),
+      (a, b) =>
+        Math.sign(order(b) - order(a) || 0) ||
+        (a.name < b.name ? -1 : a.name > b.name ? 1 : 0),
     )
     .map(fragment => fragment.text);
 }
@@ -217,7 +276,11 @@ export function checkPullRequest(files, pullRequest) {
   }
 }
 
-export async function run(command, root = process.cwd()) {
+export async function run(
+  command,
+  root = process.cwd(),
+  pullRequestOf = name => addedByPullRequest(root, name),
+) {
   if (!["check", "preview", "release", "open"].includes(command)) {
     throw new Error(
       "Usage: node scripts/changelog.js check|preview|release|open",
@@ -245,11 +308,19 @@ export async function run(command, root = process.cwd()) {
         files.push({ status: diff[i], name: diff[i + 1] });
       }
       checkPullRequest(files, pullRequest);
+    } else if (process.env.GITHUB_EVENT_NAME === "push") {
+      // Once merged, every fragment must resolve to its pull request, so a
+      // merge without "(#N)" in its subject fails here rather than at release.
+      for (const fragment of fragments) linkFragment(fragment, pullRequestOf);
     }
     return;
   }
   if (command === "preview") {
-    process.stdout.write(renderFragments(fragments));
+    process.stdout.write(
+      renderFragments(
+        fragments.map(fragment => previewFragment(fragment, pullRequestOf)),
+      ),
+    );
     return;
   }
   const { version, repository } = JSON.parse(
@@ -261,13 +332,16 @@ export async function run(command, root = process.cwd()) {
     await fs.writeFile(target, openSection(changelog, version));
     return;
   }
+  const linked = fragments.map(fragment =>
+    linkFragment(fragment, pullRequestOf),
+  );
   const tagged =
-    fragments.length > 0 &&
+    linked.length > 0 &&
     isFinalized(changelog, version) &&
     releaseTagExists(repository.url, version);
   await fs.writeFile(
     target,
-    prepareRelease(changelog, version, fragments, { tagged }),
+    prepareRelease(changelog, version, linked, { tagged }),
   );
   for (const fragment of fragments) {
     await fs.unlink(path.join(root, "changes", fragment.name));

scripts/changelog.test.js

@@ -1,15 +1,19 @@
 import assert from "node:assert/strict";
+import { execFileSync } from "node:child_process";
 import fs from "node:fs/promises";
 import os from "node:os";
 import path from "node:path";
 import test from "node:test";
 import {
+  addedByPullRequest,
   categories,
   checkPullRequest,
+  linkFragment,
   openSection,
   parseFragment,
   placeholderLink,
   prepareRelease,
+  previewFragment,
   readFragments,
   renderFragments,
   run,
@@ -26,8 +30,8 @@ test("validates category names and bullet content", () => {
     ["recovery.bugfix.md", note],
     ["recovery.md", note],
     ["recovery.fix.md", ""],
-    ["recovery.fix.md", "- Missing link"],
     ["recovery.fix.md", `Heading\n${note}`],
+    ["recovery.fix.md", "- Bad\ncontinuation"],
   ]) {
     assert.throws(() => parseFragment(name, content));
   }
@@ -217,6 +221,121 @@ test("validates before mutation, sorts fragments, consumes only on release", asy
   await assert.rejects(run("release", root));
 });
 
+test("links bullets without a link to the pull request that added them", () => {
+  const fragment = parseFragment(
+    "recovery.fix.md",
+    "- Fix recovery\n  after errors.\n- Fix formatting. https://github.com/rescript-lang/rescript/pull/7",
+  );
+  assert.equal(fragment.unlinked, true);
+  assert.equal(fragment.pr, 7);
+  const linked = linkFragment(fragment, () => 42);
+  assert.equal(
+    linked.text,
+    "- Fix recovery\n  after errors. https://github.com/rescript-lang/rescript/pull/42\n- Fix formatting. https://github.com/rescript-lang/rescript/pull/7",
+  );
+  assert.equal(linked.pr, 42);
+  const plain = parseFragment("plain.fix.md", "- Fix recovery.");
+  assert.equal(plain.pr, null);
+  assert.equal(
+    linkFragment(parseFragment("linked.fix.md", note), () => {
+      throw new Error("not looked up");
+    }).text,
+    note,
+  );
+  assert.throws(
+    () => linkFragment(plain, () => null),
+    /changes\/plain\.fix\.md: no PR link/,
+  );
+});
+
+test("previews merged fragments with their links, newest PR first", () => {
+  const numbers = { "a.fix.md": 10, "b.fix.md": 30, "c.fix.md": null };
+  const fragments = [
+    parseFragment("a.fix.md", "- Older."),
+    parseFragment("b.fix.md", "- Newer."),
+    parseFragment("c.fix.md", "- Not merged yet."),
+    parseFragment(
+      "d.fix.md",
+      "- Linked. https://github.com/rescript-lang/rescript/pull/20",
+    ),
+  ].map(fragment => previewFragment(fragment, name => numbers[name]));
+  assert.ok(
+    renderFragments(fragments).includes(
+      [
+        "- Not merged yet.",
+        "- Newer. https://github.com/rescript-lang/rescript/pull/30",
+        "- Linked. https://github.com/rescript-lang/rescript/pull/20",
+        "- Older. https://github.com/rescript-lang/rescript/pull/10",
+      ].join("\n"),
+    ),
+  );
+});
+
+test("releases fragments without a link with their pull request's link", async t => {
+  const root = await fs.mkdtemp(path.join(os.tmpdir(), "rescript-changelog-"));
+  t.after(() => fs.rm(root, { recursive: true, force: true }));
+  await fs.mkdir(path.join(root, "changes"));
+  await fs.writeFile(
+    path.join(root, "package.json"),
+    JSON.stringify({ version: "13.0.0" }),
+  );
+  await fs.writeFile(path.join(root, "CHANGELOG.md"), changelog);
+  await fs.writeFile(path.join(root, "changes", "old.fix.md"), note);
+  await fs.writeFile(path.join(root, "changes", "new.fix.md"), "- New fix.");
+  await fs.writeFile(path.join(root, "changes", "lost.fix.md"), "- Lost.");
+  const numbers = { "new.fix.md": 9000, "lost.fix.md": null };
+  await assert.rejects(
+    run("release", root, name => numbers[name]),
+    /changes\/lost\.fix\.md/,
+  );
+  assert.equal(
+    await fs.readFile(path.join(root, "CHANGELOG.md"), "utf8"),
+    changelog,
+  );
+  await fs.unlink(path.join(root, "changes", "lost.fix.md"));
+  await run("release", root, name => numbers[name]);
+  assert.ok(
+    (await fs.readFile(path.join(root, "CHANGELOG.md"), "utf8")).includes(
+      `${categories.fix}\n\n- New fix. https://github.com/rescript-lang/rescript/pull/9000\n${note}\n`,
+    ),
+  );
+});
+
+test("finds the pull request in the subject of the commit that added a fragment", async t => {
+  const root = await fs.mkdtemp(path.join(os.tmpdir(), "rescript-changelog-"));
+  t.after(() => fs.rm(root, { recursive: true, force: true }));
+  const git = (...args) =>
+    execFileSync("git", args, {
+      cwd: root,
+      env: {
+        ...process.env,
+        GIT_AUTHOR_NAME: "a",
+        GIT_AUTHOR_EMAIL: "a@example.com",
+        GIT_COMMITTER_NAME: "a",
+        GIT_COMMITTER_EMAIL: "a@example.com",
+      },
+    });
+  git("init", "-q");
+  await fs.mkdir(path.join(root, "changes"));
+  const commit = async (name, subject) => {
+    await fs.writeFile(path.join(root, "changes", name), "- Note.");
+    git("add", ".");
+    git("commit", "-q", "-m", subject);
+  };
+  await commit("squashed.fix.md", "Fix recovery (#123)");
+  await commit("rebased.fix.md", "Fix formatting");
+  assert.equal(addedByPullRequest(root, "squashed.fix.md"), 123);
+  assert.equal(addedByPullRequest(root, "rebased.fix.md"), null);
+  assert.equal(addedByPullRequest(root, "missing.fix.md"), null);
+  // A later edit by another pull request does not change who added it.
+  await fs.writeFile(
+    path.join(root, "changes", "squashed.fix.md"),
+    "- Edited.",
+  );
+  git("commit", "-q", "-am", "Reword (#456)");
+  assert.equal(addedByPullRequest(root, "squashed.fix.md"), 123);
+});
+
 test("preview groups categories without consuming fragments", () => {
   const fragments = [parseFragment("recovery.fix.md", note)];
   const preview = renderFragments(fragments);