Make changelog assembly recoverable after a partial write

`--write` wrote CHANGELOG.md and then removed fragments one by one, so a
failure after the write left surviving fragments that a rerun spliced in
a second time.

The write is now staged. The assembled file goes into
changelog.d/.assembling/, the fragments are renamed in beside it, and
renaming the assembled file over CHANGELOG.md is the single commit
point. A run that fails rolls itself back when it can; otherwise the next
`npm run changelog` reads the staging directory and either restores the
fragments (assembled file still staged) or deletes the leftovers (batch
landed). `--check` reports a staged batch instead of reporting past it.

A failure-injection test makes every filesystem call in a `--write` fail
in turn, both as a one-off error and as a crash with no in-process
rollback. It asserts the retry produces exactly the uninterrupted
CHANGELOG.md, with each entry once and an empty directory. The test
fails against the previous write-then-delete and against delete-first.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

.gitignore

@@ -21,3 +21,6 @@ qa-artifacts/
 3d-models/
 pinokio/ENVIRONMENT
 pinokio/.installed
+
+# Transient: a release interrupted mid-assembly (npm run changelog recovers it)
+changelog.d/.assembling/

changelog.d/README.md

@@ -38,6 +38,13 @@ The assembler refuses a batch rather than guessing: a filename that is not
 `<pull request>-<slug>.md`, or two fragments claiming one pull request number,
 stops the run before anything is written and names the files.
 
+If `npm run changelog` fails part-way (a full disk, a killed process), run it
+again. The batch is staged in `changelog.d/.assembling/` and only lands when the
+assembled file replaces `CHANGELOG.md` in a single rename, so the rerun either
+finishes that batch or puts its fragments back first. It never adds an entry
+twice or drops one. `npm run changelog:check` names a staged batch, if one is
+left over, instead of reporting past it.
+
 A documentation claim that needs a regression test — the way
 `src/annotations/drawTool.test.mjs` asserts against `CHANGELOG.md` — should
 assert against `docs/CURRENT-STATE.md` or `README.md` instead. A pull request

docs/MAINTAINER_WORKFLOW.md

@@ -130,7 +130,9 @@ resolves the top of `CHANGELOG.md`. At release, `npm run changelog:check` previe
 the batch and its order; `npm run changelog` splices it into `CHANGELOG.md` and
 empties the directory. The assembler refuses a malformed filename or two fragments
 claiming one PR number before writing, and copies text through unchanged so the
-contributor's wording and credit ship.
+contributor's wording and credit ship. The write is staged in
+`changelog.d/.assembling/`, so if it fails part-way, rerunning `npm run changelog`
+finishes or rolls back the batch. It never duplicates or drops an entry.
 
 Return this concise record privately unless posting is authorized. Keep sensitive
 security evidence in the private report:

scripts/assemble-changelog.mjs

@@ -1,4 +1,4 @@
-import { readdir, readFile, writeFile, rm } from 'node:fs/promises';
+import * as fs from 'node:fs/promises';
 import path from 'node:path';
 import { fileURLToPath, pathToFileURL } from 'node:url';
 
@@ -14,8 +14,19 @@ import { fileURLToPath, pathToFileURL } from 'node:url';
  * `src/annotations/drawTool.test.mjs` reads `CHANGELOG.md` as prose and slices a
  * fixed window after a heading, so reflowing an entry here could move text
  * across that window.
+ *
+ * A write is staged so that a failure at any point leaves a rerun unable to
+ * duplicate or lose an entry. The assembled file is written into
+ * `changelog.d/.assembling/` first, the fragments are renamed in beside it, and
+ * renaming the assembled file over `CHANGELOG.md` is the single commit point.
+ * While the assembled file is still staged the batch has not landed, so
+ * recovery moves the fragments back; once it is gone the batch has landed, and
+ * the staged fragments are leftovers to delete. Either way they are out of the
+ * input directory, so they are never assembled twice.
  */
 export const FRAGMENT_DIR = 'changelog.d';
+export const STAGING_DIR = '.assembling';
+const STAGED_CHANGELOG = 'CHANGELOG.md';
 const FRAGMENT_NAME = /^(\d+)-[a-z0-9]+(?:-[a-z0-9]+)*\.md$/;
 const HEADING = '# Changelog\n';
 
@@ -56,36 +67,109 @@ export function assembleChangelog(changelog, fragments) {
   return `${HEADING}\n${batch}\n\n${entries}`;
 }
 
+/**
+ * Finish or roll back a batch an earlier `--write` left in the staging
+ * directory. A no-op when there is none.
+ *
+ * @returns {Promise<'none'|'rolled-back'|'cleaned-up'>}
+ */
+export async function recoverInterruptedBatch(root, io = fs) {
+  const directory = path.join(root, FRAGMENT_DIR);
+  const staging = path.join(directory, STAGING_DIR);
+  let staged;
+  try {
+    staged = await io.readdir(staging);
+  } catch (error) {
+    if (error.code === 'ENOENT') return 'none';
+    throw error;
+  }
+  if (!staged.includes(STAGED_CHANGELOG)) {
+    // The assembled file was renamed over CHANGELOG.md: the batch landed.
+    await io.rm(staging, { recursive: true, force: true });
+    return 'cleaned-up';
+  }
+  // The batch never reached CHANGELOG.md, so every fragment goes back.
+  const pending = new Set(await io.readdir(directory));
+  const fragments = staged.filter((name) => name !== STAGED_CHANGELOG);
+  const clash = fragments.filter((name) => pending.has(name));
+  if (clash.length) {
+    throw new Error(
+      `Cannot roll back ${FRAGMENT_DIR}/${STAGING_DIR}/: ${clash.join(', ')}` +
+        ` also exist in ${FRAGMENT_DIR}/ — resolve by hand`,
+    );
+  }
+  for (const name of fragments) {
+    await io.rename(path.join(staging, name), path.join(directory, name));
+  }
+  await io.rm(staging, { recursive: true, force: true });
+  return 'rolled-back';
+}
+
 /**
  * Report or apply the pending fragments in `<root>/changelog.d`.
  *
  * The batch is validated in full before anything is written, so a malformed
  * filename or a duplicate pull request number leaves the directory untouched.
+ * `--write` first recovers any batch an interrupted run left staged.
  *
  * @param {string} root repository root
  * @param {'--check'|'--write'} mode `--check` reports and changes nothing
+ * @param {typeof fs} [io] filesystem calls, replaceable for failure injection
  * @returns {Promise<{ consumed: string[], changelog: string }>}
  */
-export async function assembleFragments(root, mode) {
+export async function assembleFragments(root, mode, io = fs) {
   if (!['--check', '--write'].includes(mode)) {
     throw new Error(
       'Usage: node scripts/assemble-changelog.mjs --check|--write',
     );
   }
   const directory = path.join(root, FRAGMENT_DIR);
+  const staging = path.join(directory, STAGING_DIR);
   const changelogFile = path.join(root, 'CHANGELOG.md');
-  const consumed = orderFragmentNames(await readdir(directory));
+  if (mode === '--write') {
+    await recoverInterruptedBatch(root, io);
+  }
+  const names = await io.readdir(directory);
+  if (names.includes(STAGING_DIR)) {
+    throw new Error(
+      `${FRAGMENT_DIR}/${STAGING_DIR}/ holds a batch an interrupted release` +
+        ' left behind — `npm run changelog` finishes or rolls it back',
+    );
+  }
+  const consumed = orderFragmentNames(names);
   const bodies = [];
   for (const name of consumed) {
-    bodies.push(await readFile(path.join(directory, name), 'utf8'));
+    bodies.push(await io.readFile(path.join(directory, name), 'utf8'));
   }
   const changelog = assembleChangelog(
-    await readFile(changelogFile, 'utf8'),
+    await io.readFile(changelogFile, 'utf8'),
     bodies,
   );
   if (mode === '--write' && consumed.length) {
-    await writeFile(changelogFile, changelog, 'utf8');
+    try {
-    for (const name of consumed) await rm(path.join(directory, name));
+      await io.mkdir(staging);
+      await io.writeFile(
+        path.join(staging, STAGED_CHANGELOG),
+        changelog,
+        'utf8',
+      );
+      for (const name of consumed) {
+        await io.rename(path.join(directory, name), path.join(staging, name));
+      }
+      await io.rename(path.join(staging, STAGED_CHANGELOG), changelogFile);
+      await io.rm(staging, { recursive: true, force: true });
+    } catch (error) {
+      try {
+        await recoverInterruptedBatch(root, io);
+      } catch {
+        throw new Error(
+          `${error.message} — the batch is staged in ${FRAGMENT_DIR}/${STAGING_DIR}/,` +
+            ' and the next `npm run changelog` finishes or rolls it back',
+          { cause: error },
+        );
+      }
+      throw error;
+    }
   }
   return { consumed, changelog };
 }

src/tooling/changelogFragments.test.mjs

@@ -1,5 +1,6 @@
 import test from 'node:test';
 import assert from 'node:assert/strict';
+import * as fs from 'node:fs/promises';
 import {
   mkdtemp,
   mkdir,
@@ -14,9 +15,11 @@ import { fileURLToPath } from 'node:url';
 
 import {
   FRAGMENT_DIR,
+  STAGING_DIR,
   assembleChangelog,
   assembleFragments,
   orderFragmentNames,
+  recoverInterruptedBatch,
 } from '../../scripts/assemble-changelog.mjs';
 
 const EXISTING =
@@ -154,6 +157,144 @@ test('a mode that is neither check nor write is refused', async (t) => {
   await assert.rejects(assembleFragments(root, '--force'), /--check\|--write/);
 });
 
+// Failure injection. Every filesystem call the write path makes is a point
+// where a release can die: a full disk, a permissions error, a killed process.
+
+const IO_CALLS = ['readdir', 'readFile', 'mkdir', 'writeFile', 'rename', 'rm'];
+const BATCH = {
+  '20-second.md': '- Second (someone, #20).',
+  '100-first.md': '- First (someone, #100).',
+  '7-third.md': '- Third (someone, #7).',
+};
+
+/**
+ * The real filesystem, except that `call` throws the `nth` time it runs. With
+ * `crash`, every later call throws as well, the way a killed process makes no
+ * further calls: the in-process rollback cannot run either, so the next
+ * invocation is the only thing that can recover.
+ */
+function failing(call, nth, { crash = false } = {}) {
+  let seen = 0;
+  let dead = false;
+  const io = { ...fs };
+  for (const name of IO_CALLS) {
+    io[name] = async (...args) => {
+      if (dead) throw new Error('process already gone');
+      if (name === call && ++seen === nth) {
+        dead = crash;
+        throw new Error(`injected ${call} failure #${nth}`);
+      }
+      return fs[name](...args);
+    };
+  }
+  return io;
+}
+
+/** How many times one uninterrupted `--write` makes each filesystem call. */
+async function countCalls(t) {
+  const root = await fixture(t, BATCH);
+  const counts = Object.fromEntries(IO_CALLS.map((name) => [name, 0]));
+  const io = { ...fs };
+  for (const name of IO_CALLS) {
+    io[name] = (...args) => {
+      counts[name] += 1;
+      return fs[name](...args);
+    };
+  }
+  await assembleFragments(root, '--write', io);
+  return {
+    counts,
+    expected: await readFile(path.join(root, 'CHANGELOG.md'), 'utf8'),
+  };
+}
+
+test('a write that fails at any call never lets a retry duplicate or lose an entry', async (t) => {
+  const { counts, expected } = await countCalls(t);
+  // The write path itself: one mkdir, one writeFile, a rename per fragment plus
+  // the commit rename, and the cleanup rm. Fewer would mean a step went missing.
+  assert.ok(counts.rename >= Object.keys(BATCH).length + 1);
+  for (const crash of [false, true]) {
+    for (const call of IO_CALLS) {
+      for (let nth = 1; nth <= counts[call]; nth += 1) {
+        const label = `${crash ? 'crash' : 'error'} at ${call} #${nth}`;
+        const root = await fixture(t, BATCH);
+        await assert.rejects(
+          assembleFragments(root, '--write', failing(call, nth, { crash })),
+          /injected|already gone/,
+          label,
+        );
+        await assembleFragments(root, '--write');
+        const changelog = await readFile(
+          path.join(root, 'CHANGELOG.md'),
+          'utf8',
+        );
+        assert.equal(changelog, expected, label);
+        for (const pr of [100, 20, 7, 1]) {
+          assert.equal(
+            changelog.split(`(someone, #${pr}).`).length - 1,
+            1,
+            `${label}: #${pr} appears exactly once`,
+          );
+        }
+        assert.deepEqual(
+          await readdir(path.join(root, FRAGMENT_DIR)),
+          ['README.md'],
+          label,
+        );
+      }
+    }
+  }
+});
+
+test('an error before the commit rolls the batch back in the same run', async (t) => {
+  const root = await fixture(t, BATCH);
+  // The second rename has already moved one fragment into the staging directory.
+  await assert.rejects(
+    assembleFragments(root, '--write', failing('rename', 2)),
+    /injected rename failure #2/,
+  );
+  assert.equal(
+    await readFile(path.join(root, 'CHANGELOG.md'), 'utf8'),
+    EXISTING,
+  );
+  assert.deepEqual(
+    (await readdir(path.join(root, FRAGMENT_DIR))).sort(),
+    [...Object.keys(BATCH), 'README.md'].sort(),
+  );
+});
+
+test('check mode reports a batch an interrupted release left staged', async (t) => {
+  const root = await fixture(t, BATCH);
+  await assert.rejects(
+    assembleFragments(root, '--write', failing('rename', 2, { crash: true })),
+  );
+  await assert.rejects(
+    assembleFragments(root, '--check'),
+    new RegExp(`${STAGING_DIR}/ holds a batch an interrupted release`),
+  );
+  assert.equal(await recoverInterruptedBatch(root), 'rolled-back');
+  assert.equal(await recoverInterruptedBatch(root), 'none');
+});
+
+test('recovery refuses to roll a fragment back over a file of the same name', async (t) => {
+  const root = await fixture(t, BATCH);
+  await assert.rejects(
+    assembleFragments(root, '--write', failing('rename', 2, { crash: true })),
+  );
+  await writeFile(
+    path.join(root, FRAGMENT_DIR, '100-first.md'),
+    '- Rewritten.',
+  );
+  await assert.rejects(
+    assembleFragments(root, '--write'),
+    /Cannot roll back .*100-first\.md/,
+  );
+  assert.equal(
+    await readFile(path.join(root, 'CHANGELOG.md'), 'utf8'),
+    EXISTING,
+  );
+});
+
 // The two below run against the repository itself, so a fragment that the
 // release could not assemble fails `npm test` in the pull request that adds it
 // rather than at release time, when its author has moved on.