fix(catalog): release-plugin.mjs stat-line pre-flight + no raw stacktrace on post-write fail

Q3e (order 20260913T051659Z-717911204-from-.claude), after Q3d. Live incident 13.09 (operator's own run): `release-plugin.mjs repo-mailbox --version 0.34.0 --create-tag --write --commit --push` tagged+pushed v0.34.0, wrote the catalog ref + README label, then crashed with a raw Node stacktrace because check-versions found the catalog's own stat line stale against the plugin's NEW badge (868 vs 927). Neither --commit nor --push of the catalog ran; the push token was correctly consumed (Q3c). Operator fixed the line and committed/pushed manually. D1 — the ordinary pre-flight (applyRelease -> check-versions) only ever inspects the OLD ref, so a stat-line drift the release itself is about to expose slipped straight through it into a pushed tag + a written, uncommitted catalog. New `preflightStatMismatches` compares the catalog's stat line against what the release is about to make current (the target ref's badge if that tag already exists, else the plugin's worktree README — exactly what --create-tag is about to tag), BEFORE any tag or write. Extracted the shared mismatch logic into check-versions.mjs as `statMismatchFindings` (pure refactor, classifyPlugin's own behavior unchanged) so both the post-hoc gate and this pre-release check use one rule. D2 — the post-write confirmation was a bare execFileSync, which throws on a non-zero exit. `reportPostWriteCheck` catches any failure (a real ERROR, or the subprocess dying) and reports exactly what is done (tag pushed y/n, files written) and what remains (commit/push), returning an exit code instead of an unhandled exception. No version bump, no tag, no push, no CLAUDE.md wording this session. Verification: - node --test scripts/release-plugin.test.mjs: 41/41 (Q3d) -> 50/50 (9 new: 3 preflightStatMismatches unit + 2 real-git D1 integration + 3 reportPostWriteCheck unit + 1 real-git D2 integration) - node --test scripts/*.test.mjs: 158/158 (Q3d) -> 167/167 - node scripts/check-versions.mjs: 0 ERROR (1 known WARN: claude-design, unrelated) - Mutation evidence (D1): commented out the new pre-flight call in runRelease -> exactly the new "D1 (Q3e, real git)" test went red (49 pass, 1 fail), all others stayed green; restored -> 50/50 again. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-14 06:48:48 +02:00 · 2026-09-14 06:48:48 +02:00 · b5b0e2b88e
commit b5b0e2b88e
parent 19908a88b7
3 changed files with 359 additions and 19 deletions
--- a/scripts/release-plugin.test.mjs
+++ b/scripts/release-plugin.test.mjs
@ -11,7 +11,7 @@ import { fileURLToPath } from 'node:url';
 import {
  planRelease, reconcileReadmeLabel, preflightErrors, applyRelease, shouldCreateTag,
  pushAuthorisation, requirePushAuthorisation, pushWithToken, consumeToken, createPushGate,
-  runRelease,
+  runRelease, preflightStatMismatches, reportPostWriteCheck,
 } from './release-plugin.mjs';
 import { classifyPlugin } from './check-versions.mjs';

@ -619,3 +619,224 @@ test('R1 (main(), real subprocess): the token is gone after the CLI returns, onc
    rmSync(root, { recursive: true, force: true });
  }
 });
+
+// --- Q3e: release-plugin.mjs left a HALF release (measured live 13.09, operator's own
+// run) — `--create-tag --write --commit --push` tagged + pushed v0.34.0, wrote the
+// catalog ref + README label, then crashed with a raw Node stacktrace from the post-write
+// execFileSync in runRelease because check-versions found the catalog's stat line stale
+// against the plugin's NEW badge. Neither --commit nor --push of the catalog ran; the
+// push token was (correctly, per Q3c) already consumed. Two defects, order
+// 20260913T051659Z-717911204-from-.claude:
+//
+// D1 — the ordinary pre-flight (applyRelease -> check-versions) only ever inspects the
+// OLD ref, so a stat-line drift the release itself is about to expose slips straight
+// through it. Fix: compare the catalog's stat line against what the release is ABOUT TO
+// MAKE current (the target ref's badge if that tag already exists, else the plugin's
+// worktree README — exactly what --create-tag is about to tag) BEFORE any tag or write.
+//
+// D2 — the post-write confirmation call used a bare execFileSync, which THROWS on a
+// non-zero exit — an unhandled exception over a release that had already tagged, pushed,
+// and written files but never committed. Fix: catch it, report exactly what is done and
+// what remains, return an exit code instead of letting the exception propagate.
+
+test('preflightStatMismatches: catalog stat line stale vs. the badge the release is about to make current', () => {
+  const catalogReadmeText = [
+    '### [Demo Plugin](https://x/open/demo-plugin) `v0.33.1`',
+    '',
+    '3 hooks · 868 selftest checks · [Full documentation →](x)',
+  ].join('\n');
+  const statSourceReadmeText = '![selftest_checks](https://img.shields.io/badge/selftest__checks-927-blue)';
+  const msgs = preflightStatMismatches({ catalogReadmeText, statSourceReadmeText, name: 'demo-plugin' });
+  assert.equal(msgs.length, 1);
+  assert.match(msgs[0], /868 selftest check/);
+  assert.match(msgs[0], /927/);
+});
+
+test('preflightStatMismatches: agreeing badge -> no mismatch', () => {
+  const catalogReadmeText = '### [Demo Plugin](https://x/open/demo-plugin) `v0.33.1`\n\n868 selftest checks · [Full documentation →](x)';
+  const statSourceReadmeText = '![selftest_checks](https://img.shields.io/badge/selftest__checks-868-blue)';
+  assert.deepEqual(preflightStatMismatches({ catalogReadmeText, statSourceReadmeText, name: 'demo-plugin' }), []);
+});
+
+test('preflightStatMismatches: a missing README on either side is "nothing to check", not a block', () => {
+  assert.deepEqual(preflightStatMismatches({ catalogReadmeText: null, statSourceReadmeText: 'x', name: 'demo' }), []);
+  assert.deepEqual(preflightStatMismatches({ catalogReadmeText: 'x', statSourceReadmeText: null, name: 'demo' }), []);
+});
+
+test('D1 (Q3e, real git): a stale catalog stat line for the RELEASED plugin blocks BEFORE any tag is created or any file is written', () => {
+  const root = makeTempRoot('release-plugin-d1-');
+  try {
+    const repoDir = join(root, 'demo-plugin');
+    const catalogDir = join(root, 'catalog');
+    mkdirSync(join(catalogDir, '.claude-plugin'), { recursive: true });
+    initPluginRepo(repoDir, { version: '1.1.0' }); // no v1.1.0 tag yet -> worktree README is the stat source
+
+    fsWriteFileSync(join(repoDir, 'README.md'), '![selftest_checks](https://img.shields.io/badge/selftest__checks-927-blue)');
+    execFileSync('git', ['-C', repoDir, 'add', 'README.md']);
+    execFileSync('git', ['-C', repoDir, 'commit', '-q', '-m', 'bump badge']);
+
+    const mktPath = join(catalogDir, '.claude-plugin', 'marketplace.json');
+    const marketplaceBefore = { plugins: [{ name: 'demo-plugin', source: { source: 'url', url: 'x', ref: 'v1.0.0' }, description: 'd' }] };
+    const mktTextBefore = JSON.stringify(marketplaceBefore, null, 2);
+    fsWriteFileSync(mktPath, mktTextBefore);
+    const readmeBefore = [
+      '### [Demo Plugin](https://x/open/demo-plugin) `v1.0.0`',
+      '',
+      '868 selftest checks · [Full documentation →](x)',
+      '',
+    ].join('\n');
+    fsWriteFileSync(join(catalogDir, 'README.md'), readmeBefore);
+
+    const pushGate = createPushGate({
+      cwd: catalogDir, home: root,
+      exists: () => { throw new Error('BUG: must not check the push token before the stat pre-flight'); },
+      unlink: () => { throw new Error('BUG: must not consume — nothing was pushed'); },
+    });
+
+    const logs = [];
+    const origLog = console.log;
+    console.log = (...a) => logs.push(a.join(' '));
+    let code;
+    try {
+      code = runRelease({
+        args: { name: 'demo-plugin', version: '1.1.0', createTag: true, write: true, commit: false, push: false },
+        catalogDir, mktPath, marketplace: marketplaceBefore, pushGate,
+      });
+    } finally {
+      console.log = origLog;
+    }
+
+    assert.notEqual(code, 0, 'a stale catalog stat line must not report success');
+    const output = logs.join('\n');
+    assert.match(output, /868 selftest check/);
+    assert.match(output, /927/);
+
+    const tags = execFileSync('git', ['-C', repoDir, 'tag', '--list', 'v*'], { encoding: 'utf8' }).trim().split('\n').filter(Boolean);
+    assert.deepEqual(tags, [], 'no tag may be created before the stat pre-flight passes');
+    assert.equal(fsReadFileSync(mktPath, 'utf8'), mktTextBefore, 'the catalog ref must not be written either');
+    assert.equal(fsReadFileSync(join(catalogDir, 'README.md'), 'utf8'), readmeBefore, 'the catalog README must be untouched');
+  } finally {
+    rmSync(root, { recursive: true, force: true });
+  }
+});
+
+test('D1 (Q3e, real git, known-negative): an agreeing stat line does not block the ordinary flow', () => {
+  const root = makeTempRoot('release-plugin-d1-neg-');
+  try {
+    const repoDir = join(root, 'demo-plugin');
+    const catalogDir = join(root, 'catalog');
+    mkdirSync(join(catalogDir, '.claude-plugin'), { recursive: true });
+    initPluginRepo(repoDir, { version: '1.1.0' });
+    fsWriteFileSync(join(repoDir, 'README.md'), '![selftest_checks](https://img.shields.io/badge/selftest__checks-868-blue)');
+    execFileSync('git', ['-C', repoDir, 'add', 'README.md']);
+    execFileSync('git', ['-C', repoDir, 'commit', '-q', '-m', 'add badge']);
+    execFileSync('git', ['-C', repoDir, 'tag', '-a', 'v1.0.0', '-m', 'v1.0.0']);
+
+    const mktPath = join(catalogDir, '.claude-plugin', 'marketplace.json');
+    const marketplace = { plugins: [{ name: 'demo-plugin', source: { source: 'url', url: 'x', ref: 'v1.0.0' }, description: 'd' }] };
+    fsWriteFileSync(mktPath, JSON.stringify(marketplace, null, 2));
+    fsWriteFileSync(join(catalogDir, 'README.md'), [
+      '### [Demo Plugin](https://x/open/demo-plugin) `v1.0.0`',
+      '',
+      '868 selftest checks · [Full documentation →](x)',
+      '',
+    ].join('\n'));
+
+    const pushGate = createPushGate({ cwd: catalogDir, home: root, exists: () => false, unlink: () => {} });
+
+    const code = runRelease({
+      args: { name: 'demo-plugin', version: '1.1.0', createTag: false, write: false, commit: false, push: false },
+      catalogDir, mktPath, marketplace, pushGate,
+    });
+
+    // v1.1.0 has no tag -> planRelease BLOCKs on the missing-tag precondition, same as
+    // ever; the point of this test is only that the stat pre-flight itself did NOT fire.
+    assert.equal(code, 1);
+  } finally {
+    rmSync(root, { recursive: true, force: true });
+  }
+});
+
+// --- Q3e/D2: the post-write confirmation must never surface as an unhandled exception ---
+
+test('reportPostWriteCheck: green check-versions -> unchanged, informational, ok', () => {
+  const applied = { writes: ['/cat/.claude-plugin/marketplace.json'], readme: 'written' };
+  const r = reportPostWriteCheck(
+    { name: 'demo-plugin', applied, tagged: true, willPush: true },
+    () => '✓ OK    demo-plugin\n\n1 plugins — 1 OK, 0 WARN, 0 ERROR, 0 SKIP — verified 1/1\n',
+  );
+  assert.equal(r.ok, true);
+  assert.match(r.message, /demo-plugin/);
+});
+
+test('reportPostWriteCheck: a failing check-versions becomes ONE precise message, not a thrown exception', () => {
+  const applied = { writes: ['/cat/.claude-plugin/marketplace.json', '/cat/README.md'], readme: 'written' };
+  const failing = () => {
+    const err = new Error('Command failed');
+    err.status = 1;
+    err.stdout = '✗ ERROR demo-plugin\n        catalog says 868 selftest check but the plugin\'s badge says 927 (catalog stat line is stale)\n\n1 plugins — 0 OK, 0 WARN, 1 ERROR, 0 SKIP — verified 1/1\n';
+    throw err;
+  };
+  let r;
+  assert.doesNotThrow(() => { r = reportPostWriteCheck({ name: 'demo-plugin', applied, tagged: true, willPush: true }, failing); });
+  assert.equal(r.ok, false);
+  assert.equal(r.exitCode, 1);
+  assert.match(r.message, /HALF DONE/);
+  assert.match(r.message, /tag pushed: yes/);
+  assert.match(r.message, /catalog files written: yes/);
+  assert.match(r.message, /NOT done: commit, push/);
+  assert.match(r.message, /868 selftest check/);
+});
+
+test('reportPostWriteCheck: NOT done omits push when --push was not requested', () => {
+  const applied = { writes: ['/cat/.claude-plugin/marketplace.json'], readme: 'unchanged' };
+  const failing = () => { const err = new Error('fail'); err.status = 1; err.stdout = ''; throw err; };
+  const r = reportPostWriteCheck({ name: 'demo-plugin', applied, tagged: false, willPush: false }, failing);
+  assert.equal(r.ok, false);
+  assert.match(r.message, /tag pushed: no/);
+  assert.match(r.message, /NOT done: commit$/m);
+});
+
+test('D2 (Q3e, real git): a post-write check-versions failure reports precisely and returns non-zero — no thrown exception reaches the caller', () => {
+  const root = makeTempRoot('release-plugin-d2-');
+  try {
+    const repoDir = join(root, 'demo-plugin');
+    const catalogDir = join(root, 'catalog');
+    mkdirSync(join(catalogDir, '.claude-plugin'), { recursive: true });
+    initPluginRepo(repoDir, { version: '1.1.0' });
+    // Both the OLD ref and the NEW target need a real tag, or the ordinary pre-flight
+    // (dangling-ref check on the OLD ref) blocks the release for an unrelated reason —
+    // that is not what this test is about; D2 is about the POST-write check failing.
+    execFileSync('git', ['-C', repoDir, 'tag', '-a', 'v1.0.0', '-m', 'v1.0.0']);
+    execFileSync('git', ['-C', repoDir, 'tag', '-a', 'v1.1.0', '-m', 'v1.1.0']);
+
+    const mktPath = join(catalogDir, '.claude-plugin', 'marketplace.json');
+    const marketplace = { plugins: [{ name: 'demo-plugin', source: { source: 'url', url: 'x', ref: 'v1.0.0' }, description: 'd' }] };
+    fsWriteFileSync(mktPath, JSON.stringify(marketplace, null, 2));
+    fsWriteFileSync(join(catalogDir, 'README.md'), '### [Demo Plugin](https://x/open/demo-plugin) `v1.0.0`\n');
+
+    const pushGate = createPushGate({ cwd: catalogDir, home: root, exists: () => false, unlink: () => {} });
+
+    let code;
+    let threw = false;
+    try {
+      code = runRelease({
+        args: { name: 'demo-plugin', version: '1.1.0', createTag: false, write: true, commit: true, push: false },
+        catalogDir, mktPath, marketplace, pushGate,
+        // Injected: simulates check-versions.mjs dying — the real subprocess call is
+        // exercised by R1 elsewhere; this proves runRelease never lets it throw upward.
+        runCheckVersions: () => { const err = new Error('Command failed: node check-versions.mjs'); err.status = 1; err.stdout = '✗ ERROR demo-plugin\n'; throw err; },
+      });
+    } catch {
+      threw = true;
+    }
+
+    assert.equal(threw, false, 'runRelease must never let the post-write check throw upward');
+    assert.notEqual(code, 0);
+    // The ref+label write already happened (pre-flight was green on the OLD state); the
+    // point of D2 is that the run stops cleanly there instead of crashing mid-commit.
+    assert.ok(fsReadFileSync(mktPath, 'utf8').includes('v1.1.0'), 'the ref write is not rolled back — D2 only stops what has not happened yet (commit)');
+  } finally {
+    rmSync(root, { recursive: true, force: true });
+  }
+});