diff --git a/scripts/changes.md b/scripts/changes.md index fbb46cd51..bdc22ea76 100644 --- a/scripts/changes.md +++ b/scripts/changes.md @@ -18,6 +18,24 @@ - The assertion block at the end of `compiledLoaderProbeSource`, whenever upstream changes loader caching. +## 2026-09-21 - Reject changes to released changelog sections (#1884) + +### What changed + +- `scripts/check-pr-changelog.mjs` compares committed CHANGELOG sections against the PR merge base, rejecting released additions, edits and deletions with their path, line and section. + +### Why + +- `scripts/check-pr-changelog.mjs` previously accepted any changed changelog filename, including entries that could never appear in a future release. Only the existing Unreleased block's release stamp may introduce a new released section. + +### Why an extension could not handle it + +- `scripts/check-pr-changelog.mjs` runs in CI, outside the agent runtime. + +### Expected merge conflict zones + +- LOW: `scripts/check-pr-changelog.mjs` fact collection and verdict composition. + ## 2026-09-21 - run-workspaces gains --parallel with prefixed lanes and shared signal forwarding (senpi#1895) ### What changed diff --git a/scripts/check-pr-changelog.mjs b/scripts/check-pr-changelog.mjs index 349130a67..07b061ca6 100644 --- a/scripts/check-pr-changelog.mjs +++ b/scripts/check-pr-changelog.mjs @@ -31,7 +31,42 @@ function isChangelogChange(path) { return CHANGELOG_PATTERN.test(path); } -export function checkPrChangelog({ changedFiles, labels, trackerPolicy }) { +function releasedChangelogViolation({ path, before, after }) { + const [previous, current] = [before, after].map((text) => { + const headings = [...text.matchAll(/^## \[([^\]]+)\].*$/gm)]; + return headings.map((heading, index) => ({ + name: heading[1], + line: text.slice(0, heading.index).split("\n").length, + text: text.slice(heading.index, headings[index + 1]?.index ?? text.length), + body: text.slice(heading.index + heading[0].length, headings[index + 1]?.index ?? text.length), + })); + }); + const misplaced = current.findIndex((section, index) => index > 0 && section.name === "Unreleased"); + if (misplaced > 0) + return `${path}:${current[misplaced].line}: released section [${current[misplaced - 1].name}] interrupted`; + const oldReleased = previous.filter((section) => section.name !== "Unreleased"); + const newReleased = current.filter((section) => section.name !== "Unreleased"); + const unreleased = previous.find((section) => section.name === "Unreleased"); + // Release tooling renames the existing Unreleased block, then inserts an empty next cycle. + if ( + newReleased.length === oldReleased.length + 1 && + unreleased?.body === newReleased[0]?.body && + !oldReleased.some((section) => section.name === newReleased[0].name) + ) newReleased.shift(); + for (let index = 0; index < Math.max(oldReleased.length, newReleased.length); index += 1) { + const oldSection = oldReleased[index]; + const newSection = newReleased[index]; + if (oldSection?.text === newSection?.text) continue; + const oldLines = oldSection?.text.split("\n") ?? []; + const newLines = newSection?.text.split("\n") ?? []; + let offset = 0; + while (offset < Math.min(oldLines.length, newLines.length) && oldLines[offset] === newLines[offset]) offset += 1; + const section = newSection ?? oldSection; + return `${path}:${section.line + offset}: released section [${section.name}] changed`; + } +} + +export function checkPrChangelog({ changedFiles, labels, trackerPolicy, changelogChanges = [] }) { const normalizedLabels = (labels ?? []).map((label) => label.trim()).filter(Boolean); const hasNoChangelogLabel = normalizedLabels.includes(NO_CHANGELOG_LABEL); const changelogFiles = changedFiles.filter(isChangelogChange); @@ -61,9 +96,13 @@ export function checkPrChangelog({ changedFiles, labels, trackerPolicy }) { // required package CHANGELOG.md entry. const audit = trackerPolicy == null ? null : auditChangesMdCoverage({ changedFiles, trackerPolicy }); const uncovered = audit ? audit.uncovered.map((item) => item.path) : []; + const violation = changelogChanges.map(releasedChangelogViolation).find(Boolean); let pass; let reason; - if (audit && uncovered.length > 0) { + if (violation) { + pass = false; + reason = violation; + } else if (audit && uncovered.length > 0) { pass = false; reason = summarizeUncovered(audit.uncovered); } else if (audit) { @@ -108,6 +147,19 @@ function collectPrFacts(base) { const pin = readUpstreamPin(UPSTREAM_PIN_PATH); ensureCommitExists(pin.sha); const { changedFiles, renames, deletions } = resolvePrNameStatus(base); + const mergeBase = runGit(["merge-base", base, "HEAD"], "resolving PR merge base").trim(); + const baseFiles = filesInCommit(mergeBase); + const headFiles = filesInCommit("HEAD"); + const changelogChanges = changedFiles.filter(isChangelogChange) + .filter((path) => !renames.some((rename) => rename.from === path && isChangelogChange(rename.to))) + .map((path) => { + const oldPath = renames.find((rename) => rename.to === path)?.from ?? path; + return { + path, + before: baseFiles.has(oldPath) ? runGit(["show", `${mergeBase}:${oldPath}`], `reading base ${oldPath}`) : "", + after: headFiles.has(path) ? runGit(["show", `HEAD:${path}`], `reading HEAD ${path}`) : "", + }; + }); const pinChanged = changedFiles.includes(UPSTREAM_PIN_PATH); const upstreamTree = filesInCommit(pin.sha); const upstreamRenames = renames.filter((rename) => upstreamTree.has(rename.from)); @@ -130,6 +182,7 @@ function collectPrFacts(base) { } return { changedFiles, + changelogChanges, trackerPolicy: { forkOnly, trackerDiffs, @@ -196,16 +249,18 @@ export function main(argv) { let changedFiles; let trackerPolicy; + let changelogChanges; try { const facts = collectPrFacts(args.base); changedFiles = facts.changedFiles; trackerPolicy = facts.trackerPolicy; + changelogChanges = facts.changelogChanges; } catch (error) { console.error(`changelog-gate: ERROR - ${error.message}`); return 1; } - const result = checkPrChangelog({ changedFiles, labels: args.labels, trackerPolicy }); + const result = checkPrChangelog({ changedFiles, labels: args.labels, trackerPolicy, changelogChanges }); const verdict = result.pass ? "PASS" : "FAIL"; console.log(`changelog-gate: ${verdict} - ${result.reason}`); if (!result.pass) { @@ -216,7 +271,8 @@ export function main(argv) { console.log(` missing changes.md coverage: ${path}`); } console.log( - "Add an entry under ## [Unreleased] in the affected package CHANGELOG.md, " + + "Restore any changed released sections. " + + "Add an entry under ## [Unreleased] in the affected package CHANGELOG.md, " + `apply the '${NO_CHANGELOG_LABEL}' label if this change is not user-facing, ` + "or cover the change in its exact nearest changes.md tracker.", ); diff --git a/scripts/check-pr-changelog.test.mjs b/scripts/check-pr-changelog.test.mjs index e04a8bb71..9dbd6497c 100644 --- a/scripts/check-pr-changelog.test.mjs +++ b/scripts/check-pr-changelog.test.mjs @@ -1,7 +1,13 @@ #!/usr/bin/env node import assert from "node:assert/strict"; +import { spawnSync } from "node:child_process"; +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; import { describe, it } from "node:test"; +import { fileURLToPath } from "node:url"; import { checkPrChangelog } from "./check-pr-changelog.mjs"; +import { CHANGELOGS, reAddUnreleasedSections, stampChangelogs } from "./release-changelog.mjs"; describe("check-pr-changelog gate", () => { it("fails when runtime package source changes without a changelog entry", () => { @@ -124,3 +130,65 @@ describe("check-pr-changelog gate", () => { assert.equal(result.pass, true); }); }); + +// #1884: drive the real CLI over committed diffs, including the actual release transformation. +it("keeps released changelog sections immutable through the PR gate CLI", async (t) => { + const root = mkdtempSync(join(tmpdir(), "changelog-gate-1884-")); + t.after(() => rmSync(root, { recursive: true, force: true })); + const env = { ...process.env, GIT_CONFIG_GLOBAL: join(root, "gitconfig"), GIT_CONFIG_NOSYSTEM: "1" }; + writeFileSync(env.GIT_CONFIG_GLOBAL, ""); + const git = (...args) => { + const result = spawnSync("git", args, { cwd: root, env, encoding: "utf8", timeout: 30_000 }); + assert.equal(result.status, 0, result.stderr); + return result.stdout.trim(); + }; + git("init", "-q"); + git("config", "user.name", "Fixture"); + git("config", "user.email", "fixture@example.invalid"); + const original = "# Changelog\n\n## [Unreleased]\n\n### Fixed\n\n- pending\n\n## [2026.9.20] - 2026-09-20\n\n### Fixed\n\n- published\n"; + for (const file of CHANGELOGS) { + mkdirSync(dirname(join(root, file)), { recursive: true }); + writeFileSync(join(root, file), original); + } + git("add", ...CHANGELOGS); + git("commit", "-qm", "upstream fixture"); + mkdirSync(join(root, ".github")); + writeFileSync(join(root, ".github/upstream.json"), JSON.stringify({ sha: git("rev-parse", "HEAD") })); + git("add", ".github/upstream.json"); + git("commit", "-qm", "base fixture"); + const base = git("rev-parse", "HEAD"); + const file = "packages/coding-agent/CHANGELOG.md"; + const cli = fileURLToPath(new URL("./check-pr-changelog.mjs", import.meta.url)); + const check = (text, expected, labels = "") => { + if (text === null) rmSync(join(root, file)); + else writeFileSync(join(root, file), text); + git("add", file); + git("commit", "--allow-empty", "-qm", "scenario fixture"); + const result = spawnSync(process.execPath, [cli, "--base", base, "--labels", labels], { + cwd: root, env, encoding: "utf8", timeout: 30_000, + }); + assert.equal(result.status, expected, result.stdout + result.stderr); + if (expected === 1) assert.match(result.stdout + result.stderr, /packages\/coding-agent\/CHANGELOG\.md:\d+.*2026\.9\.20/); + }; + for (const [name, text, labels] of [ + ["addition", `${original}- misplaced\n`, ""], + ["modification", original.replace("- published", "- changed"), ""], + ["deletion", original.replace("- published\n", ""), ""], + ["deleted file", null, ""], + ["Unreleased below a release", `${original}## [Unreleased]\n- misplaced\n`, ""], + ["label cannot bypass", `${original}- misplaced\n`, "no-changelog"], + ]) await t.test(name, () => check(text, 1, labels)); + await t.test("Unreleased entry", () => check(original.replace("- pending", "- new\n- pending"), 0)); + writeFileSync(join(root, file), original); + const cwd = process.cwd(); + const captured = new Map(); + try { + process.chdir(root); + stampChangelogs("2026.9.21", "2026-09-21", false, captured, () => {}, () => {}); + await t.test("release stamp", () => check(readFileSync(file, "utf8"), 0)); + reAddUnreleasedSections("2026.9.21", "2026-09-21", false, captured, () => {}, () => {}); + await t.test("next cycle", () => check(readFileSync(file, "utf8"), 0)); + } finally { + process.chdir(cwd); + } +});