fix(catalog): release-plugin.mjs shares one push token across a whole run

Q3b (order 20260912T213049Z-5772222747) corrects two measured defects in
Q3's ea9bf7a: pushWithToken checked-and-consumed per call, so a single
`--create-tag --write --commit --push` run spent the operator's one-shot
token on the tag push and always saw blocked:true on the catalog push
right after (D1). And `git tag -a` ran before any token check at all, so
a blocked run left a local annotated tag behind, breaking the retry with
"tag already exists" (D2).

createPushGate replaces the per-push check-and-consume with a run-scoped
gate: ensure() checks the token once and every later call in the same run
reuses that result, consume() fires once after the run's last successful
push. main() calls ensure() before the tag write (not just before the
push) and consume() once at the end. pushWithToken is now a single-push
convenience wrapper over the same gate — its existing tests stay green
unmodified.

Red-first: `createPushGate` did not exist on ea9bf7a (import error),
proving both new tests were red before the fix. After:
node --test scripts/release-plugin.test.mjs -> 37/37 (35 + 2 new)
node --test scripts/*.test.mjs -> 154/154
node scripts/check-versions.mjs -> 0 ERROR (2 known WARN: claude-design, repo-mailbox)
Live D2 check: `release-plugin.mjs repo-mailbox --create-tag --write`
without a token -> BLOCKED, exit 1, no local tag created.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Kjell Tore Guttormsen 2026-09-12 23:57:07 +02:00
commit 5bc6c4ecbd
2 changed files with 105 additions and 16 deletions

View file

@ -187,14 +187,49 @@ export function consumeToken({ tokenPath, exists, unlink }) {
if (exists(tokenPath)) unlink(tokenPath); if (exists(tokenPath)) unlink(tokenPath);
} }
// Checks authorisation, then runs `push()`. Consumes the token only after `push()` // A run-scoped push-token gate (Q3b, order 20260912T213049Z-5772222747 — fixes two
// returns without throwing — if it throws (a real push failure), the exception // defects in Q3's per-call pushWithToken):
// propagates and the token is left intact for the retry. //
// D1: --create-tag --write --commit --push does TWO pushes (tag, then catalog) in ONE
// run. pushWithToken checked-and-consumed per call, so the tag push spent the operator's
// one-shot token and the catalog push right after always saw blocked:true. ensure()
// checks the token ONCE per run and every later call reuses that same result — one
// token covers every push the run makes.
//
// D2: `git tag -a` used to run before any token check at all, so a blocked run left a
// local annotated tag behind (a retry after the operator drops the token then fails
// with "tag already exists", exit 128). Callers must call ensure() BEFORE the first
// write this run intends to push toward — including a local tag meant to precede a
// later push — not only immediately before the `git push` itself.
//
// consume() deletes the token once, after the run's LAST push has succeeded; it is a
// no-op if ensure() was never authorised (nothing pushed) or already consumed.
export function createPushGate({ cwd, home, exists, unlink }) {
let auth = null;
let consumed = false;
return {
ensure() {
if (auth === null) auth = requirePushAuthorisation({ cwd, home, exists });
return auth;
},
consume() {
if (consumed) return;
if (auth && auth.authorised) consumeToken({ tokenPath: auth.tokenPath, exists, unlink });
consumed = true;
},
};
}
// Single-push convenience wrapper over createPushGate: checks, pushes, consumes for
// exactly one push. Consumes the token only after `push()` returns without throwing —
// if it throws (a real push failure), the exception propagates and the token is left
// intact for the retry.
export function pushWithToken({ cwd, home, exists, unlink, push }) { export function pushWithToken({ cwd, home, exists, unlink, push }) {
const auth = requirePushAuthorisation({ cwd, home, exists }); const gate = createPushGate({ cwd, home, exists, unlink });
const auth = gate.ensure();
if (!auth.authorised) return { pushed: false, blocked: true, message: auth.message, tokenPath: auth.tokenPath }; if (!auth.authorised) return { pushed: false, blocked: true, message: auth.message, tokenPath: auth.tokenPath };
push(); push();
consumeToken({ tokenPath: auth.tokenPath, exists, unlink }); gate.consume();
return { pushed: true, blocked: false, tokenPath: auth.tokenPath }; return { pushed: true, blocked: false, tokenPath: auth.tokenPath };
} }
@ -288,16 +323,20 @@ function main() {
// --create-tag: if the only thing missing is the tag, mint + push it first — but only // --create-tag: if the only thing missing is the tag, mint + push it first — but only
// under --write. Without it this is a dry-run and must publish nothing. // under --write. Without it this is a dry-run and must publish nothing.
// Shared across BOTH push sites in this run (tag push, catalog push) — one operator
// token authorises the whole publish, not each push individually (D1). ensure() is
// called before the FIRST write this run intends to push toward, including the local
// `git tag -a` below, which must not run before the check passes (D2).
const pushGate = createPushGate({ cwd: catalogDir, home: process.env.HOME, exists: existsSync, unlink: unlinkSync });
const tagStep = shouldCreateTag(args, obs, target); const tagStep = shouldCreateTag(args, obs, target);
if (tagStep === 'create') { if (tagStep === 'create') {
const auth = pushGate.ensure();
if (!auth.authorised) { console.error(auth.message); process.exit(1); }
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' });
const tagPush = pushWithToken({ execFileSync('git', ['-C', obs.repoDir, 'push', 'origin', tag], { stdio: 'inherit' });
cwd: catalogDir, home: process.env.HOME, exists: existsSync, unlink: unlinkSync,
push: () => execFileSync('git', ['-C', obs.repoDir, 'push', 'origin', tag], { stdio: 'inherit' }),
});
if (tagPush.blocked) { console.error(tagPush.message); process.exit(1); }
obs = observePlugin(catalogDir, args.name); obs = observePlugin(catalogDir, args.name);
} }
@ -349,15 +388,16 @@ function main() {
execFileSync('git', ['-C', catalogDir, 'commit', '-m', msg], { stdio: 'inherit' }); execFileSync('git', ['-C', catalogDir, 'commit', '-m', msg], { stdio: 'inherit' });
console.log(' ✓ committed the catalog'); console.log(' ✓ committed the catalog');
if (args.push) { if (args.push) {
const auth = pushGate.ensure();
if (!auth.authorised) { console.error(auth.message); process.exit(1); }
console.log(' → pushing'); console.log(' → pushing');
const catalogPush = pushWithToken({ execFileSync('git', ['-C', catalogDir, 'push', 'origin', 'HEAD'], { stdio: 'inherit' });
cwd: catalogDir, home: process.env.HOME, exists: existsSync, unlink: unlinkSync,
push: () => execFileSync('git', ['-C', catalogDir, 'push', 'origin', 'HEAD'], { stdio: 'inherit' }),
});
if (catalogPush.blocked) { console.error(catalogPush.message); process.exit(1); }
console.log(' ✓ pushed'); console.log(' ✓ pushed');
} }
} }
// Consume the shared token once, after the run's LAST successful push (D1) — a no-op
// if nothing this run pushed.
pushGate.consume();
process.exit(0); process.exit(0);
} }

View file

@ -5,7 +5,7 @@ import { test } from 'node:test';
import assert from 'node:assert/strict'; import assert from 'node:assert/strict';
import { import {
planRelease, reconcileReadmeLabel, preflightErrors, applyRelease, shouldCreateTag, planRelease, reconcileReadmeLabel, preflightErrors, applyRelease, shouldCreateTag,
pushAuthorisation, requirePushAuthorisation, pushWithToken, consumeToken, pushAuthorisation, requirePushAuthorisation, pushWithToken, consumeToken, createPushGate,
} from './release-plugin.mjs'; } from './release-plugin.mjs';
import { classifyPlugin } from './check-versions.mjs'; import { classifyPlugin } from './check-versions.mjs';
@ -363,3 +363,52 @@ test('consumeToken deletes the token when it is present', () => {
consumeToken({ tokenPath: '/x', exists: () => true, unlink: (p) => unlinked.push(p) }); consumeToken({ tokenPath: '/x', exists: () => true, unlink: (p) => unlinked.push(p) });
assert.deepEqual(unlinked, ['/x']); assert.deepEqual(unlinked, ['/x']);
}); });
// --- Q3b fix: D1 (shared token across BOTH pushes in one run) + D2 (checked before
// the tag write, not just before the push) — order 20260912T213049Z-5772222747 -----
//
// Q3's pushWithToken checked-and-consumed per call: with one token, the tag push consumed
// it before the catalog push ran, so inside a single `--create-tag --write --commit --push`
// run the catalog push always saw blocked:true. And `git tag -a` (pre-fix main():295) ran
// before ANY token check, so a blocked run left a local annotated tag behind — the retry
// after the operator drops the token then fails with "tag already exists" (exit 128).
// createPushGate fixes both: ONE ensure() shared across every push this run makes, checked
// before the FIRST write (including a local tag meant to precede a later push), consumed
// ONCE after the run's last push succeeds.
test('createPushGate: one token covers two pushes in the same run, consumed once at the end', () => {
let existsCalls = 0;
const exists = () => { existsCalls++; return true; };
const unlinked = [];
const gate = createPushGate({ cwd: '/c', home: '/h', exists, unlink: (p) => unlinked.push(p) });
const authForTag = gate.ensure();
assert.equal(authForTag.authorised, true, 'first push (tag) must be authorised by the one token');
// ... tag push happens here in main() ...
const authForCatalog = gate.ensure();
assert.equal(authForCatalog.authorised, true, "second push (catalog) must reuse the SAME token, not find it already consumed");
assert.equal(existsCalls, 1, 'the token is checked ONCE for the whole run, not once per push');
gate.consume();
assert.deepEqual(unlinked, [authForTag.tokenPath], "the token is consumed exactly once, after the run's last push");
gate.consume();
assert.deepEqual(unlinked, [authForTag.tokenPath], 'a second consume() must not double-unlink');
});
test('createPushGate: an unauthorised run must not create the tag or touch the catalog', () => {
let tagCreated = false;
let catalogWritten = false;
const gate = createPushGate({
cwd: '/c', home: '/h', exists: () => false,
unlink: () => { throw new Error('BUG: must not unlink without a token'); },
});
const auth = gate.ensure();
assert.equal(auth.authorised, false);
if (auth.authorised) tagCreated = true; // mirrors main(): `git tag -a` only runs past this check
if (auth.authorised) catalogWritten = true; // mirrors main(): applyRelease() only runs past this check
assert.equal(tagCreated, false, 'D2: the tag must not be created before the token check passes');
assert.equal(catalogWritten, false, 'no catalog change either — the run stops at the first blocked check');
gate.consume(); // must be safe even though ensure() never authorised anything
});