fix(scripts): run check-versions BEFORE release-plugin writes, not after

release-plugin.mjs --write wrote marketplace.json and the catalog README label
first, and only THEN ran check-versions via execFileSync (which throws on exit
1). A red catalog therefore left a half-applied release in the working tree —
exactly the state a parallel session has already been observed carrying to the
public remote.

Adds applyRelease() with an injected io seam so the ORDER is testable: runGate()
runs first, and any ERROR aborts with nothing written. The pre-flight reads the
ERROR set only, never failed/--strict — pre-bump the released plugin is SUPPOSED
to be WARN (catalog ref behind plugin.json), so a WARN gate would brick every
release. Verified against the real classifier, not synthetic data.

The post-write gate stays: pre-flight validates the old state, that one
validates the new state.

Known remaining hole, documented not built: --create-tag mints and pushes the
plugin tag before the pre-flight runs.

Tests 14 -> 19 (120 -> 125 across the six suites); check-versions 12 OK.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Ga5tZ3AgUxAcdLtWB8Kig

CLAUDE.md

@@ -27,8 +27,17 @@ their own Forgejo repositories under `https://git.fromaitochitta.com/open/`.
   tag first. On `--write` it bumps the catalog `ref` AND the catalog README's per-plugin `` `vX.Y.Z` ``
   label together (and `git add`s both on `--commit`). Because it only moves both to a verified, tagged,
   consistent version, `check-versions.mjs` is green by construction. Never hand-edit a `ref` or a
-  README label for a release — use this. Pure planner + label reconciler covered by
-  `scripts/release-plugin.test.mjs`.
+  README label for a release — use this. Pure planner + label reconciler + pre-flight/write step
+  covered by `scripts/release-plugin.test.mjs`.
+- **Pre-flight gate (`--write` runs `check-versions` BEFORE it writes):** the helper calls `runGate()`
+  first and aborts with exit 1 — **nothing written** — if ANY plugin is ERROR, not just the one being
+  released (`check-versions`' exit code is catalog-wide). Previously the gate ran *after* both writes,
+  so a red catalog left a half-applied release in the working tree for a parallel session to carry to
+  the public remote. The pre-flight reads the **ERROR set only**, never `failed`/`--strict`: pre-bump,
+  the plugin being released is *supposed* to be WARN (catalog `ref` behind `plugin.json`), so gating on
+  WARN would brick every release. The post-write gate at the end stays — pre-flight validates the old
+  state, that one validates the new state. **Known remaining hole:** `--create-tag` mints and pushes
+  the plugin tag *before* the pre-flight runs, so that irreversible side effect is still unprotected.
 - **Version-consistency gate:** run `node scripts/check-versions.mjs` before committing any `ref`
   change. For each plugin it checks (against the sibling repo) that the catalog `ref` resolves to a
   real git tag (ERROR if dangling — breaks install), that `plugin.json` version == README

scripts/release-plugin.mjs

@@ -27,7 +27,7 @@ import { readFileSync, writeFileSync, existsSync } from 'node:fs';
 import { execFileSync } from 'node:child_process';
 import { join, dirname } from 'node:path';
 import { fileURLToPath, pathToFileURL } from 'node:url';
-import { normalizeVersion } from './check-versions.mjs';
+import { normalizeVersion, runGate } from './check-versions.mjs';
 
 // --- Pure planner (unit under test) -----------------------------------------
 
@@ -105,6 +105,46 @@ export function reconcileReadmeLabel(readmeText, name, newRef) {
   return changed ? out.join('\n') : null;
 }
 
+// --- Pre-flight gate + write step (unit under test via injected io) ----------
+
+// Which plugins does check-versions call ERROR right now? Catalog-wide on purpose: one red
+// plugin blocks every bump, because check-versions' exit code is global — a bump committed
+// on top of someone else's ERROR ships a catalog that cannot pass its own gate.
+//
+// Reads the ERROR set explicitly and NEVER `failed`/`hasWarn`: pre-bump, the plugin being
+// released is SUPPOSED to be WARN (catalog ref behind plugin.json). Gating on WARN would
+// brick every release.
+export function preflightErrors(gateResult) {
+  return (gateResult?.results ?? []).filter(r => r.status === 'ERROR').map(r => r.name);
+}
+
+// Run the gate FIRST, then write. The old order wrote both files and only then ran the gate
+// (which throws on exit 1), leaving a half-applied release in the working tree for a parallel
+// session to carry to the public remote. `io` is injected so the ORDER is testable.
+export function applyRelease({ plan, catalogDir, mktPath, readmePath }, io) {
+  const errors = preflightErrors(io.runGate(catalogDir));
+  if (errors.length > 0) return { verdict: 'BLOCKED', preflightErrors: errors, writes: [], readme: null };
+
+  const writes = [];
+  io.writeFileSync(mktPath, JSON.stringify(plan.newMarketplace, null, 2) + '\n', 'utf8');
+  writes.push(mktPath);
+
+  // Keep the human-facing catalog README label in lock-step with the ref (gated by check-versions).
+  let readme;
+  try {
+    const newReadme = reconcileReadmeLabel(io.readFileSync(readmePath, 'utf8'), plan.name, plan.newRef);
+    if (newReadme !== null) {
+      io.writeFileSync(readmePath, newReadme, 'utf8');
+      writes.push(readmePath);
+      readme = 'written';
+    } else {
+      readme = 'unchanged';
+    }
+  } catch { readme = 'missing'; }
+
+  return { verdict: 'WROTE', preflightErrors: [], writes, readme };
+}
+
 // --- I/O shell --------------------------------------------------------------
 
 function gitTags(repoDir) {
@@ -180,22 +220,23 @@ function main() {
     process.exit(0);
   }
 
-  writeFileSync(mktPath, JSON.stringify(plan.newMarketplace, null, 2) + '\n', 'utf8');
-  console.log(`  ✓ wrote ${mktPath} (ref ${plan.currentRef} -> ${plan.newRef})`);
-
-  // Keep the human-facing catalog README label in lock-step with the ref (gated by check-versions).
   const readmePath = join(catalogDir, 'README.md');
-  try {
-    const newReadme = reconcileReadmeLabel(readFileSync(readmePath, 'utf8'), plan.name, plan.newRef);
-    if (newReadme !== null) {
-      writeFileSync(readmePath, newReadme, 'utf8');
-      console.log(`  ✓ updated README label (${plan.name} -> ${plan.newRef})`);
-    } else {
-      console.log(`  · README label already ${plan.newRef} (or no heading found)`);
-    }
-  } catch { console.log('  · no catalog README to update'); }
+  const applied = applyRelease({ plan, catalogDir, mktPath, readmePath }, { readFileSync, writeFileSync, runGate });
 
-  // Confirm the gate is green for this plugin after the write.
+  if (applied.verdict === 'BLOCKED') {
+    console.log('  ✗ pre-flight check-versions is RED — nothing written.');
+    for (const n of applied.preflightErrors) console.log(`      ERROR: ${n}`);
+    console.log('    Fix every ERROR (any plugin — the gate exit code is catalog-wide), then re-run.');
+    process.exit(1);
+  }
+
+  console.log(`  ✓ wrote ${mktPath} (ref ${plan.currentRef} -> ${plan.newRef})`);
+  if (applied.readme === 'written') console.log(`  ✓ updated README label (${plan.name} -> ${plan.newRef})`);
+  else if (applied.readme === 'unchanged') console.log(`  · README label already ${plan.newRef} (or no heading found)`);
+  else console.log('  · no catalog README to update');
+
+  // Confirm the gate is green for this plugin AFTER the write — the pre-flight validated the
+  // old state, this validates the new one. Different jobs; the redundancy is only apparent.
   const gate = execFileSync('node', [join(catalogDir, 'scripts', 'check-versions.mjs')], { cwd: catalogDir, encoding: 'utf8' });
   const line = gate.split('\n').find(l => l.includes(args.name)) ?? '';
   console.log(`  check-versions: ${line.trim() || '(no line)'}`);

scripts/release-plugin.test.mjs

@@ -3,7 +3,7 @@
 // is exercised by the CLI against the live tree, not here.
 import { test } from 'node:test';
 import assert from 'node:assert/strict';
-import { planRelease, reconcileReadmeLabel } from './release-plugin.mjs';
+import { planRelease, reconcileReadmeLabel, preflightErrors, applyRelease } from './release-plugin.mjs';
 import { classifyPlugin } from './check-versions.mjs';
 
 const marketplace = () => ({
@@ -131,3 +131,82 @@ test('reconcileReadmeLabel returns null when the plugin has no heading', () => {
   const readme = '### [Other](https://x/open/other) `v1.0.0`';
   assert.equal(reconcileReadmeLabel(readme, 'ghost', 'v2.0.0'), null);
 });
+
+// --- pre-flight gate: check-versions must run BEFORE the writes, not after -----
+//
+// The old order wrote marketplace.json + the README label first and only THEN ran the
+// gate (which throws on exit 1) — leaving a half-applied release in the working tree
+// that a parallel session could carry to the public remote. These tests pin the
+// ORDER, so they must assert on writes-not-taken, not just on a verdict string.
+
+const gateResult = (statuses) => {
+  const results = Object.entries(statuses).map(([name, status]) => ({ name, status, findings: [] }));
+  return {
+    results,
+    hasError: results.some(r => r.status === 'ERROR'),
+    hasWarn: results.some(r => r.status === 'WARN'),
+    failed: results.some(r => r.status === 'ERROR'),
+  };
+};
+
+const fakeIo = (gate, readmeText = '### [Alpha](https://x/open/alpha) `v1.0.0`') => {
+  const writes = [];
+  return {
+    writes,
+    runGate: () => gate,
+    readFileSync: () => readmeText,
+    writeFileSync: (p) => { writes.push(p); },
+  };
+};
+
+const paths = { catalogDir: '/cat', mktPath: '/cat/.claude-plugin/marketplace.json', readmePath: '/cat/README.md' };
+
+test('preflightErrors names every ERROR plugin, catalog-wide (not just the target)', () => {
+  assert.deepEqual(preflightErrors(gateResult({ alpha: 'WARN', beta: 'ERROR', gamma: 'ERROR' })), ['beta', 'gamma']);
+  assert.deepEqual(preflightErrors(gateResult({ alpha: 'OK', beta: 'WARN', gamma: 'SKIP' })), []);
+});
+
+test('pre-flight ERROR aborts BEFORE any file is written (ordering, not just verdict)', () => {
+  const plan = planRelease({ marketplace: marketplace(), name: 'alpha', observed: observed() });
+  const io = fakeIo(gateResult({ alpha: 'WARN', beta: 'ERROR' })); // ERROR on a DIFFERENT plugin
+  const r = applyRelease({ plan, ...paths }, io);
+  assert.equal(r.verdict, 'BLOCKED');
+  assert.deepEqual(r.preflightErrors, ['beta']);
+  assert.deepEqual(io.writes, [], 'no file may be written when the gate is red');
+});
+
+test('pre-bump WARN is the NORMAL state and must NOT block the release', () => {
+  const plan = planRelease({ marketplace: marketplace(), name: 'alpha', observed: observed() });
+  // Real classifier, pre-bump: catalog ref still v1.0.0 while plugin.json is 1.1.0.
+  // This is exactly what a release looks like before it is applied — it MUST be WARN,
+  // or gating on `failed`/`--strict` would brick every release.
+  const pre = classifyPlugin({
+    name: 'alpha', catalogRef: plan.currentRef,
+    pluginVersion: '1.1.0', readmeBadge: '1.1.0', tags: ['v1.0.0', 'v1.1.0'],
+  });
+  assert.equal(pre.status, 'WARN');
+
+  const io = fakeIo({ results: [pre], hasError: false, hasWarn: true, failed: false });
+  const r = applyRelease({ plan, ...paths }, io);
+  assert.equal(r.verdict, 'WROTE');
+  assert.deepEqual(io.writes, [paths.mktPath, paths.readmePath]);
+});
+
+test('green pre-flight writes the ref and reports an already-correct README label as unchanged', () => {
+  const plan = planRelease({ marketplace: marketplace(), name: 'alpha', observed: observed() });
+  const io = fakeIo(gateResult({ alpha: 'OK' }), '### [Alpha](https://x/open/alpha) `v1.1.0`');
+  const r = applyRelease({ plan, ...paths }, io);
+  assert.equal(r.verdict, 'WROTE');
+  assert.equal(r.readme, 'unchanged');
+  assert.deepEqual(io.writes, [paths.mktPath]);
+});
+
+test('a missing catalog README does not abort the ref bump', () => {
+  const plan = planRelease({ marketplace: marketplace(), name: 'alpha', observed: observed() });
+  const io = fakeIo(gateResult({ alpha: 'OK' }));
+  io.readFileSync = () => { throw new Error('ENOENT'); };
+  const r = applyRelease({ plan, ...paths }, io);
+  assert.equal(r.verdict, 'WROTE');
+  assert.equal(r.readme, 'missing');
+  assert.deepEqual(io.writes, [paths.mktPath]);
+});