fix(scripts): --create-tag is a write and must obey --write

`--create-tag` minted AND PUSHED a plugin tag to a public remote without
`--write`, on the entry point CLAUDE.md documents as "dry-run by default".
The tag was public before the plan was even printed.

Extracts `shouldCreateTag(args, observed, target)` as a pure exported
predicate ('create' | 'dry-run' | 'skip') so the flag's write-ness is
testable, and gates minting on `--write`. Without it the CLI now reports
what it would mint, printed after the missing-tag blocker that points at
the flag.

Deliberately NOT placed behind the catalog-wide pre-flight: every
precondition it checks is local to the plugin being released
(plugin.json == target, badge agrees, tag absent), so the tag is correct
by construction. A red *other* plugin can only make the tag early, never
wrong, and the tag-absent check makes the retry idempotent. Gating it
would let plugin Y block the tagging of plugin X — the same over-coupling
that reading the ERROR set only (never `failed`) exists to avoid.

Tests 19 -> 25; 131/131 across the six suites. check-versions 12 OK.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019i3rnU2VNRdGcrUFMnRb6R
This commit is contained in:
Kjell Tore Guttormsen 2026-08-10 20:46:45 +02:00
commit ea8231f3c8
3 changed files with 87 additions and 9 deletions

View file

@ -23,8 +23,9 @@ their own Forgejo repositories under `https://git.fromaitochitta.com/open/`.
step strands users on the old version (the exact drift this helper exists to prevent). Run step strands users on the old version (the exact drift this helper exists to prevent). Run
`node scripts/release-plugin.mjs <plugin> [--version X.Y.Z]` — dry-run by default; it REFUSES unless `node scripts/release-plugin.mjs <plugin> [--version X.Y.Z]` — dry-run by default; it REFUSES unless
`plugin.json` == README badge == the target version AND the `vX.Y.Z` tag exists, then prints the `plugin.json` == README badge == the target version AND the `vX.Y.Z` tag exists, then prints the
planned bump. Apply with `--write [--commit] [--push]`; `--create-tag` mints+pushes a missing plugin planned bump. Apply with `--write [--commit] [--push]`; `--create-tag --write` mints+pushes a missing
tag first. On `--write` it bumps the catalog `ref` AND the catalog README's per-plugin `` `vX.Y.Z` `` plugin tag first (**`--create-tag` is a WRITE and obeys `--write`** — on a dry-run it only reports
what it would mint). 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, 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 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 + pre-flight/write step README label for a release — use this. Pure planner + label reconciler + pre-flight/write step
@ -36,8 +37,14 @@ their own Forgejo repositories under `https://git.fromaitochitta.com/open/`.
the public remote. The pre-flight reads the **ERROR set only**, never `failed`/`--strict`: pre-bump, 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 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 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 state, that one validates the new state. **`--create-tag` is deliberately NOT behind this gate**
the plugin tag *before* the pre-flight runs, so that irreversible side effect is still unprotected. (decided 2026-08-11): it mints and pushes the plugin tag *before* the pre-flight runs, but every
precondition it checks is local to that one plugin (`plugin.json` == target, badge agrees, tag
absent), so the tag is correct by construction. A red *other* plugin can only make the tag EARLY,
never WRONG, and the tag-absent check makes the retry idempotent — gating it would let plugin Y
block the *tagging* of plugin X, the same over-coupling that reading ERROR-only avoids. What WAS
closed is the worse half: `--create-tag` used to push on the documented dry-run path, with no
`--write` at all. It now requires `--write` (`shouldCreateTag`, tested).
- **Version-consistency gate:** run `node scripts/check-versions.mjs` before committing any `ref` - **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 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 real git tag (ERROR if dangling — breaks install), that `plugin.json` version == README

View file

@ -18,7 +18,8 @@
// //
// Usage: // Usage:
// node scripts/release-plugin.mjs <name> [--version X.Y.Z] # dry-run: print the plan // node scripts/release-plugin.mjs <name> [--version X.Y.Z] # dry-run: print the plan
// node scripts/release-plugin.mjs <name> --create-tag # create+push the missing vX.Y.Z plugin tag first // node scripts/release-plugin.mjs <name> --create-tag --write # create+push the missing vX.Y.Z plugin tag first
// # (--create-tag is a WRITE: without --write it only reports)
// node scripts/release-plugin.mjs <name> --write # write the bumped catalog ref // node scripts/release-plugin.mjs <name> --write # write the bumped catalog ref
// node scripts/release-plugin.mjs <name> --write --commit # + git commit the catalog // node scripts/release-plugin.mjs <name> --write --commit # + git commit the catalog
// node scripts/release-plugin.mjs <name> --write --commit --push # + push (you own the push window) // node scripts/release-plugin.mjs <name> --write --commit --push # + push (you own the push window)
@ -118,6 +119,27 @@ export function preflightErrors(gateResult) {
return (gateResult?.results ?? []).filter(r => r.status === 'ERROR').map(r => r.name); return (gateResult?.results ?? []).filter(r => r.status === 'ERROR').map(r => r.name);
} }
// --create-tag mints AND PUSHES a tag to a public remote — the one genuinely irreversible
// side effect here — so it is a WRITE and must obey --write. It used to fire on the
// documented dry-run entry point, publishing the tag before the plan was even printed.
//
// Deliberately NOT gated on the catalog-wide pre-flight: every precondition below is
// local to this plugin (plugin.json == target, badge agrees, tag absent), so the minted
// tag is correct by construction. A red OTHER plugin can only make the tag EARLY, never
// WRONG — and `!tags.includes(newRef)` makes the retry idempotent once that plugin is
// fixed. Gating on it would let plugin Y block the tagging of plugin X: the same
// over-coupling that `preflightErrors` reading ERROR-only (never `failed`) exists to avoid.
//
// Returns 'create' (mint + push), 'dry-run' (would, but no --write), or 'skip'.
export function shouldCreateTag(args, observed, target) {
if (!args?.createTag || !target) return 'skip';
if (observed?.tags === null || observed?.tags === undefined) return 'skip';
if (observed.tags.includes('v' + target)) return 'skip';
if (observed.pluginVersion !== target) return 'skip';
if (observed.readmeBadge !== null && observed.readmeBadge !== observed.pluginVersion) return 'skip';
return args.write ? 'create' : 'dry-run';
}
// Run the gate FIRST, then write. The old order wrote both files and only then ran the gate // 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 // (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. // session to carry to the public remote. `io` is injected so the ORDER is testable.
@ -195,9 +217,10 @@ function main() {
let obs = observePlugin(catalogDir, args.name); let obs = observePlugin(catalogDir, args.name);
const target = normalizeVersion(args.version ?? obs.pluginVersion ?? ''); const target = normalizeVersion(args.version ?? obs.pluginVersion ?? '');
// --create-tag: if the only thing missing is the tag, mint + push it first. // --create-tag: if the only thing missing is the tag, mint + push it first — but only
if (args.createTag && target && obs.tags !== null && !obs.tags.includes('v' + target) // under --write. Without it this is a dry-run and must publish nothing.
&& obs.pluginVersion === target && (obs.readmeBadge === null || obs.readmeBadge === obs.pluginVersion)) { const tagStep = shouldCreateTag(args, obs, target);
if (tagStep === 'create') {
const tag = 'v' + target; const tag = 'v' + target;
console.log(`→ creating annotated tag ${tag} in ${obs.repoDir}`); console.log(`→ creating annotated tag ${tag} in ${obs.repoDir}`);
execFileSync('git', ['-C', obs.repoDir, 'tag', '-a', tag, '-m', `${args.name} ${tag}`], { stdio: 'inherit' }); execFileSync('git', ['-C', obs.repoDir, 'tag', '-a', tag, '-m', `${args.name} ${tag}`], { stdio: 'inherit' });
@ -209,6 +232,11 @@ function main() {
console.log(`\nrelease-plugin: ${plan.name} ${plan.currentRef ?? '?'} -> ${plan.newRef ?? '?'} [${plan.verdict}]`); console.log(`\nrelease-plugin: ${plan.name} ${plan.currentRef ?? '?'} -> ${plan.newRef ?? '?'} [${plan.verdict}]`);
if (plan.blockers.length) { for (const b of plan.blockers) console.log(`${b}`); } if (plan.blockers.length) { for (const b of plan.blockers) console.log(`${b}`); }
// Printed AFTER the blockers: the missing-tag blocker points at --create-tag, and this is
// the answer to "I did pass it" — the flag is a write, so it waited for --write.
if (tagStep === 'dry-run') {
console.log(` (dry-run) --create-tag would mint + push v${target} in ${obs.repoDir} — re-run with --write.`);
}
if (plan.verdict === 'BLOCKED') process.exit(1); if (plan.verdict === 'BLOCKED') process.exit(1);
if (plan.verdict === 'NOOP') { console.log(' ✓ catalog already pins this version — nothing to do.'); process.exit(0); } if (plan.verdict === 'NOOP') { console.log(' ✓ catalog already pins this version — nothing to do.'); process.exit(0); }

View file

@ -3,7 +3,7 @@
// is exercised by the CLI against the live tree, not here. // is exercised by the CLI against the live tree, not here.
import { test } from 'node:test'; import { test } from 'node:test';
import assert from 'node:assert/strict'; import assert from 'node:assert/strict';
import { planRelease, reconcileReadmeLabel, preflightErrors, applyRelease } from './release-plugin.mjs'; import { planRelease, reconcileReadmeLabel, preflightErrors, applyRelease, shouldCreateTag } from './release-plugin.mjs';
import { classifyPlugin } from './check-versions.mjs'; import { classifyPlugin } from './check-versions.mjs';
const marketplace = () => ({ const marketplace = () => ({
@ -210,3 +210,46 @@ test('a missing catalog README does not abort the ref bump', () => {
assert.equal(r.readme, 'missing'); assert.equal(r.readme, 'missing');
assert.deepEqual(io.writes, [paths.mktPath]); assert.deepEqual(io.writes, [paths.mktPath]);
}); });
// --- shouldCreateTag: --create-tag is a WRITE, so it must obey --write --------
//
// `--create-tag` mints AND PUSHES a tag to a public remote — the one genuinely
// irreversible side effect in this helper. It used to fire on the documented
// dry-run entry point (`<name> --create-tag`, no --write), which contradicts
// "dry-run by default": the tag was already public before the plan was printed.
// The decision (2026-08-11) was to gate it on --write and leave the catalog-wide
// pre-flight where it is — that gate can only prevent an EARLY tag, never a WRONG
// one, since the preconditions below already make the tag correct by construction.
const tagArgs = (o = {}) => ({ createTag: true, write: true, ...o });
test('shouldCreateTag: --create-tag --write on a consistent plugin with no tag → create', () => {
assert.equal(shouldCreateTag(tagArgs(), observed({ tags: ['v1.0.0'] }), '1.1.0'), 'create');
});
test('shouldCreateTag: --create-tag WITHOUT --write never pushes (dry-run stays dry)', () => {
assert.equal(shouldCreateTag(tagArgs({ write: false }), observed({ tags: ['v1.0.0'] }), '1.1.0'), 'dry-run');
});
test('shouldCreateTag: no --create-tag → skip, even with --write', () => {
assert.equal(shouldCreateTag(tagArgs({ createTag: false }), observed({ tags: ['v1.0.0'] }), '1.1.0'), 'skip');
});
test('shouldCreateTag: tag already exists → skip (retry after a red gate is idempotent)', () => {
assert.equal(shouldCreateTag(tagArgs(), observed({ tags: ['v1.0.0', 'v1.1.0'] }), '1.1.0'), 'skip');
});
test('shouldCreateTag: skips when the plugin is not internally consistent', () => {
// plugin.json behind the target — planRelease would BLOCK anyway; never mint for it.
assert.equal(shouldCreateTag(tagArgs(), observed({ pluginVersion: '1.0.0', readmeBadge: '1.0.0', tags: ['v1.0.0'] }), '1.1.0'), 'skip');
// README badge disagrees with plugin.json
assert.equal(shouldCreateTag(tagArgs(), observed({ readmeBadge: '1.0.0', tags: ['v1.0.0'] }), '1.1.0'), 'skip');
// no target version resolved
assert.equal(shouldCreateTag(tagArgs(), observed({ tags: ['v1.0.0'] }), null), 'skip');
// plugin repo absent (gitTags returned null) — nothing to tag
assert.equal(shouldCreateTag(tagArgs(), observed({ tags: null }), '1.1.0'), 'skip');
});
test('shouldCreateTag: a null README badge is tolerated (badge-less plugin)', () => {
assert.equal(shouldCreateTag(tagArgs(), observed({ readmeBadge: null, tags: ['v1.0.0'] }), '1.1.0'), 'create');
});