From 22a4e26e02e51e8e81cf59d0e5d1b12acacd46a7 Mon Sep 17 00:00:00 2001 From: dan-claude-bot Date: Sun, 19 Jul 2026 20:33:49 +0000 Subject: [PATCH] fix: the arming guard names the section that ships, not the top one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round on #114. Both blockers were real and reproduced here. The re-arm and the older extraction guard contradicted each other: that guard asserted the TOP section extracts non-empty, and CONTRIBUTING now mandates a deliberately EMPTY `## Unreleased` on top of the stamp. The mandated ceremony tree was CI-red — #108's unshippability by another route. Keying to the top section was only ever a stand-in for "what release.yml publishes", so the assert now names that section: on a bare version the `## X.Y.Z` being shipped, on a `-dev` tree the newest stamped one. rig#67 retargeted the identical assert for the same reason. And a bare version with no matching stamped section — bumped, never stamped — passed every test and failed only after the merge, in release.yml's notes step, past the ship decision. Red here instead. Plus a double re-arm (two `## Unreleased` headings, the extracted section silently the empty first one) is now red. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 20 +++++- CONTRIBUTING.md | 15 +++-- test/release.test.ts | 143 ++++++++++++++++++++++++++++++++++++++----- 3 files changed, 158 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4128a73..f318102 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,8 +34,24 @@ actually cutting it, and this file starts there. top section must be `## Unreleased`. That is the distinction #108 had to collapse to make the ceremony shippable at all, recovered rather than reverted: the ceremony stays green at every step, and a disarmed - dev `main` goes red. box and rig carry the same fix - (heavy-duty/box#108, heavy-duty/rig#66). + dev `main` goes red. The re-arm also forced the older extraction guard + to move. It asserted that the **top** section extracts non-empty, which + the re-armed ceremony tree — a deliberately empty `## Unreleased` above + the stamp — makes false by construction: the re-arm and the guard would + have contradicted each other, and the next release PR would have been + unshippable for a second time, the way #108 was. Keying to the top + section was only ever a stand-in for "the section `release.yml` will + publish", so the assert now names that section directly — on a bare + version the `## X.Y.Z` being shipped, on a `-dev` tree the newest + stamped one. Existence is checked with it: a bare version with no + matching section is a bump that never stamped, which used to pass every + test and fail only *after* the merge, in `release.yml`'s notes step, + past the ship decision and leaving `main` with a minted, unreleased + version to repair by hand. A double re-arm — two `## Unreleased` + headings, the extracted section silently the empty one — is red too. + box and rig carry the same fix (heavy-duty/box#110, + heavy-duty/rig#67); rig#67 retargeted the identical assert for the + identical reason. ## 0.1.0 — 2026-07-19 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9e9170c..c7d3a7a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -58,10 +58,17 @@ on box#83's shape): and merged after has its entry land *inside the shipped section*, which git does cleanly, with no conflict to warn anyone (heavy-duty/rig#66 — it happened there). `test/release.test.ts` keys - this to the version: a stamped top section is legal while - `package.json` is bare, but the moment step 3's `-dev` bump lands, the - top section must be `## Unreleased` or CI is red. CI green on it, same - loop as any PR. + this to the version, and checks both halves of the stamp: + - while `package.json` is bare, the top section may be the stamp or the + re-armed `## Unreleased`, but a `## X.Y.Z` section for the version you + are shipping **must exist and extract non-empty** — a bump without a + stamp is red here rather than after the merge, in release.yml; + - the moment step 3's `-dev` bump lands, the top section must be + `## Unreleased` or CI is red. + + The empty `## Unreleased` this step adds is deliberately tolerated: what + must extract non-empty is the section that SHIPS, not the top one. CI + green on it, same loop as any PR. 2. **Merge. That's the ship decision — nothing else to do.** [release.yml](.github/workflows/release.yml) fires on the merged, `release`-labeled PR and asserts, in order, each fail-loud and creating diff --git a/test/release.test.ts b/test/release.test.ts index 5b1b3df..38f699c 100644 --- a/test/release.test.ts +++ b/test/release.test.ts @@ -151,22 +151,43 @@ describe("release-notes.sh", () => { // after it, the top section IS the stamped release. Demanding the literal // Unreleased (with an issue number inside it, rotting per release) made // the release PR unshippable by construction, invisible to fork - // rehearsals (a tag push runs release.yml, never ci.yml). Whatever the - // top section is called, the exact tool release.yml runs must extract it - // non-empty. - it("the real CHANGELOG.md's top section extracts", async () => { + // rehearsals (a tag push runs release.yml, never ci.yml). + // + // But keying the assert to the TOP section was only ever a stand-in for + // "the section release.yml will publish", and the re-arm (#113) breaks the + // stand-in: the ceremony PR now leaves a fresh, deliberately EMPTY + // `## Unreleased` on top of the section it just stamped, and an empty + // section is exactly what release-notes.sh refuses. Asserting the top + // section extracts would make the re-armed ceremony tree CI-red — #108's + // unshippability by another route, and the re-arm and the guard would + // contradict each other. So the assert retargets to the section that + // SHIPS, keyed on package.json the same way the arming rule below is + // (rig#67 made the identical move): + // + // version BARE — the tree is, or follows, the release of that version. + // `## ` is what release.yml extracts. It must + // exist and be non-empty. The top section is NOT + // constrained here; an empty re-armed Unreleased above + // it is correct. + // version -dev — nothing ships from this tree, and the top section is + // `## Unreleased`, legitimately empty between releases. + // Drift coverage retargets to the most recent STAMPED + // section, which release.yml did publish. Before the + // first release there is none, and that is not a fault. + it("the real CHANGELOG.md's SHIPPING section extracts (#113)", async () => { + const version = realVersion(); const changelog = readFileSync(join(ROOT, "CHANGELOG.md"), "utf8"); - const top = changelog.match(/^## (\S+)/m); - if (!top) throw new Error("CHANGELOG.md has no ## section at all"); - const r = await notes(top[1], join(ROOT, "CHANGELOG.md")); + const target = version.endsWith("-dev") ? firstStamped(changelog) : version; + if (!target) return; // greenfield -dev: nothing has shipped yet. + const r = await notes(target, join(ROOT, "CHANGELOG.md")); expect(r.code).toBe(0); expect(r.output.trim()).not.toBe(""); }); }); // --- the changelog is ARMED — the version says which state is legal -------- -// heavy-duty/rig#66. The section above proves the top section EXTRACTS; it -// deliberately does not care what the top section is CALLED, and cannot: +// heavy-duty/rig#66. The section above proves the SHIPPING section extracts; +// it deliberately does not care what the top section is CALLED, and cannot: // #108 relaxed exactly that, because the ceremony PR's own tree has a // stamped `## X.Y.Z` on top and a literal-Unreleased demand made the // release unshippable by construction. So nothing on main notices when @@ -189,6 +210,12 @@ describe("release-notes.sh", () => { // mandatory. Green through the whole ceremony; red on a disarmed `-dev` // main, which is the state the guard exists to name. +/** package.json's version — the fact the whole rule is keyed on. */ +function realVersion(): string { + return JSON.parse(readFileSync(join(ROOT, "package.json"), "utf8")) + .version as string; +} + /** The top `## ` section's token — `Unreleased`, or a stamped version. */ function topSection(changelog: string): string { const top = changelog.match(/^## (\S+)/m); @@ -196,29 +223,77 @@ function topSection(changelog: string): string { return top[1]; } +/** The newest stamped (non-Unreleased) section's token, or null if none. */ +function firstStamped(changelog: string): string | null { + for (const m of changelog.matchAll(/^## (\S+)/gm)) { + if (m[1] !== "Unreleased") return m[1]; + } + return null; +} + +/** Does `## ` appear as a section heading at all? */ +function hasSection(changelog: string, token: string): boolean { + return [...changelog.matchAll(/^## (\S+)/gm)].some((m) => m[1] === token); +} + /** null = armed. A string = why this (version, changelog) pair is illegal. */ function disarmedBecause(version: string, changelog: string): string | null { const top = topSection(changelog); + + // Idempotence: re-arming twice leaves two `## Unreleased` headings, and + // the section awk extracts is then the EMPTY first one — armed by the + // heading test, unpublishable in fact. One heading, always. + const unreleased = [...changelog.matchAll(/^## Unreleased\s*$/gm)].length; + if (unreleased > 1) { + return `the changelog carries ${unreleased} '## Unreleased' headings — the re-arm ran twice. Entries split across them, and the section release-notes.sh extracts is the empty first one.`; + } + if (version.endsWith("-dev")) { return top === "Unreleased" ? null : `version ${version} is a dev tree, so the top section must be '## Unreleased' — found '## ${top}'. The release ceremony stamps Unreleased into the shipped version and must re-add an empty one (heavy-duty/rig#66); without it the next PR's entry lands inside ${top}'s published notes, with no merge conflict to warn anyone.`; } - if (top === "Unreleased" || top === version) return null; - return `version ${version} is bare, so the top section must be '## Unreleased' (re-armed) or the matching '## ${version}' (the ceremony tree) — found '## ${top}'.`; + + if (top !== "Unreleased" && top !== version) { + return `version ${version} is bare, so the top section must be '## Unreleased' (re-armed) or the matching '## ${version}' (the ceremony tree) — found '## ${top}'.`; + } + + // A bare version is a SHIP claim: release.yml will extract `## ` + // and publish it. Every legal bare state has that section — the ceremony + // tree (stamped on top), the re-armed ceremony tree (stamped under an + // empty Unreleased), and main in the post-release window. Its absence is + // the half-ceremony: bumped, never stamped. That passed the heading rule + // alone and failed only AFTER merge, in release.yml's notes step — past + // the ship decision, leaving main with a minted, unreleased bare version + // the decide step then refuses on re-runs. Red here, one round earlier. + if (!hasSection(changelog, version)) { + return `version ${version} is bare — a ship claim — but there is no '## ${version}' section to publish. The ceremony bumped the version without stamping the changelog; release.yml's notes step would refuse AFTER the merge, past the ship decision.`; + } + + return null; } describe("the changelog is armed for the next entry (rig#66)", () => { + const work = mkdtempSync(join(tmpdir(), "cast-arming-")); const dated = (v: string) => `## ${v} — 2026-07-19`; const body = "\n\n- **An entry** — prose.\n"; const armed = `# Changelog\n\n## Unreleased${body}\n${dated("0.2.0")}${body}`; const stamped = `# Changelog\n\n${dated("0.2.0")}${body}`; + // The tree CONTRIBUTING step 1 actually mandates: the stamped section with + // a fresh, EMPTY `## Unreleased` above it. The `armed` fixture gives + // Unreleased a body and so never exercises this one. + const rearmed = `# Changelog\n\n## Unreleased\n\n${dated("0.2.0")}${body}`; + + /** Run the REAL release-notes.sh against a fixture changelog. */ + const extract = (name: string, changelog: string, ver: string) => { + const file = join(work, `${name}.md`); + writeFileSync(file, changelog); + return run("bash", [NOTES, ver, file]); + }; it("the REAL tree is armed — package.json and CHANGELOG.md agree", () => { - const version = JSON.parse(readFileSync(join(ROOT, "package.json"), "utf8")) - .version as string; const changelog = readFileSync(join(ROOT, "CHANGELOG.md"), "utf8"); - expect(disarmedBecause(version, changelog)).toBeNull(); + expect(disarmedBecause(realVersion(), changelog)).toBeNull(); }); // The ceremony, walked end to end. Every state green — this is the #108 @@ -231,6 +306,22 @@ describe("the changelog is armed for the next entry (rig#66)", () => { expect(disarmedBecause("0.2.0", armed)).toBeNull(); }); + // The state the whole re-arm turns on, and the one this guard is most at + // risk of contradicting: CONTRIBUTING's mandated tree, whose top section is + // an EMPTY `## Unreleased`. Asserted end to end — the arming rule passes it + // AND the exact tool release.yml runs extracts the section that ships. An + // assert aimed at the TOP section here is #108's unshippability again + // (rig#67 retargeted the same assert for the same reason). + it("the MANDATED ceremony tree — empty Unreleased over the stamp — is green end to end", async () => { + expect(disarmedBecause("0.2.0", rearmed)).toBeNull(); + const r = await extract("rearmed", rearmed, "0.2.0"); + expect(r.code).toBe(0); + expect(r.output).toContain("An entry"); + // And the empty top section is untouchable by release.yml, as intended. + const top = await extract("rearmed", rearmed, "Unreleased"); + expect(top.code).toBe(1); + }); + it("stays green through the ceremony: main in the post-release window", () => { // Merged, tagged, published — the -dev bump has not landed yet. expect(disarmedBecause("0.2.0", stamped)).toBeNull(); @@ -254,6 +345,30 @@ describe("the changelog is armed for the next entry (rig#66)", () => { ).toContain("the ceremony tree"); }); + // The half-ceremony: bumped, never stamped. Green under the heading rule + // alone — the top IS `## Unreleased`, re-armed and populated — and it fails + // only after the merge, in release.yml's notes step, past the ship + // decision. Asserted against the real tool so the post-merge failure this + // pre-empts is the actual one, not a paraphrase of it. + it("goes RED on the half-ceremony: bumped bare, changelog never stamped", async () => { + const half = `# Changelog\n\n## Unreleased${body}`; + const why = disarmedBecause("0.2.0", half); + expect(why).toContain("no '## 0.2.0' section"); + // Exactly what release.yml would have done instead, after the merge. + const r = await extract("half", half, "0.2.0"); + expect(r.code).toBe(1); + expect(r.output).toContain("no section for '0.2.0'"); + }); + + // Idempotence: re-arming an already-armed file is silently wrong, because + // the section awk extracts is then the empty first heading. + it("goes RED on a double re-arm — two Unreleased headings", () => { + const twice = `# Changelog\n\n## Unreleased\n\n## Unreleased${body}\n${dated("0.2.0")}${body}`; + expect(disarmedBecause("0.2.1-dev", twice)).toContain( + "2 '## Unreleased' headings", + ); + }); + it("refuses a changelog with no sections at all rather than passing it", () => { expect(() => disarmedBecause("0.2.1-dev", "# Changelog\n")).toThrow( "no ## section",