fix(catalog): release-plugin.mjs consumes the push token on ANY exit after a push
Q3c (RETTELSE av Q3b `5bc6c4e`, ordre 20260912T220453Z-5021715764-from-.claude, etter PM-re-måling på frossen klone 13.09 00:04). TDD: to nye røde tester skrevet først (kjørt mot5bc6c4e— SyntaxError: `runRelease` fantes ikke, se release-plugin.test.mjs sin `Q3c fix` header for detaljer), deretter fiksen. D3 (regresjon i Q3b) — `pushGate.consume()` sto nederst i main(), etter fire tidligere process.exit()-kall (BLOCKED, NOOP, dry-run, rød pre-flight) som en kjøring kan treffe ETTER at en tag-push allerede har lyktes. Live-verifisert FØR fiksen (node -e med try/finally rundt process.exit(1)): finally kjører IKKE når process.exit() kalles inne i try — Node terminerer før stack-avvikling. Derfor holder ikke et enkelt try/finally rundt den gamle main()-kroppen. Fiksen: `main()`s gren-logikk er flyttet til en eksportert `runRelease()` som returnerer en exit-kode i stedet for å kalle process.exit() noe sted etter at en push kan ha skjedd. `main()` kaller process.exit() nøyaktig ÉN gang, etter en `finally` som kjører `if (pushGate.pushed) pushGate.consume()`. `pushGate` fikk et nytt `pushed`-felt, satt til true rett etter hver vellykkede `git push` (tag-push og katalog-push). Dette er det eneste stedet igjen å resonnere om konsum — færrest utgangsstier, som ordren ba om. S1 (svakhet i D2-testen fra Q3b) — den gamle testen satte `tagCreated` fra `auth.authorised` i TESTEN SELV og asserterte på egen variabel; den beviste ingenting om den faktiske main()-stien. Nye tester kjører `runRelease` mot ekte midlertidige git-repoer (ingen mocket git): - S1: uten token → `git tag -l` uendret, exit ≠ 0, `pushGate.pushed === false`. Mutasjonsbevis: flyttet `git tag -a` over token-sjekken → testen ble RØD → gjenopprettet original rekkefølge → GRØNN igjen. - D3: med token, tag mangler men katalogen pinner allerede versjonen (NOOP- scenario) → ekte tag mintes+pushes til et lokalt bare-remote, run returnerer 0, `pushGate.pushed === true`, tokenet er FYSISK BORTE etter — reproduserer PM-agentens live-probe (tag pushet, NOOP, token fortsatt der FØR fiksen). Ordrens D1/D2-krav fra Q3b står uendret (delt token, sjekk før `git tag -a`) — ikke rørt av denne fiksen, kun konsum-tidspunktet. Verifisering (kommandoer kjørt, tall gjengitt her): - `node --test scripts/release-plugin.test.mjs` → 39/39, 0 fail (opp fra 37). - `node --test scripts/*.test.mjs` → 156/156, 0 fail (opp fra 154). - `node scripts/check-versions.mjs` → 0 ERROR, 2 kjente WARN (claude-design, repo-mailbox — urørt, utenfor scope). - Re-kjørt etter `git add` — uendret. Kun `scripts/release-plugin.mjs` og `scripts/release-plugin.test.mjs` rørt. Ingen versjonsbump, ingen tag, ingen push (forbudt i ordren). CLAUDE.md- ordlyden («consumed after the push actually succeeds») er nå sann på alle utgangsstier og krevde ingen endring. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
5bc6c4ecbd
commit
dd278ca70e
2 changed files with 154 additions and 24 deletions
|
|
@ -208,6 +208,11 @@ export function createPushGate({ cwd, home, exists, unlink }) {
|
||||||
let auth = null;
|
let auth = null;
|
||||||
let consumed = false;
|
let consumed = false;
|
||||||
return {
|
return {
|
||||||
|
// Set by a caller (runRelease) right after a `git push` it issued actually succeeds.
|
||||||
|
// Q3c/D3: consume() must fire whenever this is true, on EVERY exit from the run, not
|
||||||
|
// only the run's final line — a run can push a tag and then resolve to BLOCKED/NOOP/
|
||||||
|
// dry-run/red-pre-flight afterwards, and each of those used to skip consume() entirely.
|
||||||
|
pushed: false,
|
||||||
ensure() {
|
ensure() {
|
||||||
if (auth === null) auth = requirePushAuthorisation({ cwd, home, exists });
|
if (auth === null) auth = requirePushAuthorisation({ cwd, home, exists });
|
||||||
return auth;
|
return auth;
|
||||||
|
|
@ -309,34 +314,29 @@ function parseArgs(argv) {
|
||||||
return out;
|
return out;
|
||||||
}
|
}
|
||||||
|
|
||||||
function main() {
|
// The body of main(), minus the top-level plumbing (read marketplace.json off disk,
|
||||||
const args = parseArgs(process.argv.slice(2));
|
// build the pushGate) and minus the final process.exit call — pulled out so it is
|
||||||
if (!args.name) {
|
// testable against a real temp git repo (Q3c/S1) and so it can return an exit code
|
||||||
console.error('usage: release-plugin.mjs <name> [--version X.Y.Z] [--create-tag] [--write] [--commit] [--push]');
|
// instead of calling process.exit at each branch (Q3c/D3, see main() below for why).
|
||||||
process.exit(2);
|
export function runRelease({ args, catalogDir, mktPath, marketplace, pushGate }) {
|
||||||
}
|
|
||||||
const catalogDir = join(dirname(fileURLToPath(import.meta.url)), '..');
|
|
||||||
const mktPath = join(catalogDir, '.claude-plugin', 'marketplace.json');
|
|
||||||
const marketplace = JSON.parse(readFileSync(mktPath, 'utf8'));
|
|
||||||
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 — 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
|
// pushGate is shared across BOTH push sites in this run (tag push, catalog push) — one
|
||||||
// token authorises the whole publish, not each push individually (D1). ensure() is
|
// operator token authorises the whole publish, not each push individually (D1). ensure()
|
||||||
// called before the FIRST write this run intends to push toward, including the local
|
// 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).
|
// `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();
|
const auth = pushGate.ensure();
|
||||||
if (!auth.authorised) { console.error(auth.message); process.exit(1); }
|
if (!auth.authorised) { console.error(auth.message); return 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' });
|
||||||
execFileSync('git', ['-C', obs.repoDir, 'push', 'origin', tag], { stdio: 'inherit' });
|
execFileSync('git', ['-C', obs.repoDir, 'push', 'origin', tag], { stdio: 'inherit' });
|
||||||
|
pushGate.pushed = true;
|
||||||
obs = observePlugin(catalogDir, args.name);
|
obs = observePlugin(catalogDir, args.name);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -350,14 +350,18 @@ function main() {
|
||||||
console.log(` (dry-run) --create-tag would mint + push v${target} in ${obs.repoDir} — re-run with --write.`);
|
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);
|
// Q3c/D3: every branch below returns instead of calling process.exit. A tag push above
|
||||||
if (plan.verdict === 'NOOP') { console.log(' ✓ catalog already pins this version — nothing to do.'); process.exit(0); }
|
// may already have set pushGate.pushed = true, and process.exit() called from inside a
|
||||||
|
// try does not run its finally (verified live) — so main() must be the only place that
|
||||||
|
// calls process.exit, after its finally has had a chance to consume the token.
|
||||||
|
if (plan.verdict === 'BLOCKED') return 1;
|
||||||
|
if (plan.verdict === 'NOOP') { console.log(' ✓ catalog already pins this version — nothing to do.'); return 0; }
|
||||||
|
|
||||||
// READY
|
// READY
|
||||||
if (!args.write) {
|
if (!args.write) {
|
||||||
console.log(` ✓ ready — would bump catalog ref + README label and commit:\n ${plan.commitSubject}`);
|
console.log(` ✓ ready — would bump catalog ref + README label and commit:\n ${plan.commitSubject}`);
|
||||||
console.log(' (dry-run) re-run with --write [--commit] [--push] to apply.');
|
console.log(' (dry-run) re-run with --write [--commit] [--push] to apply.');
|
||||||
process.exit(0);
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
const readmePath = join(catalogDir, 'README.md');
|
const readmePath = join(catalogDir, 'README.md');
|
||||||
|
|
@ -367,7 +371,7 @@ function main() {
|
||||||
console.log(' ✗ pre-flight check-versions is RED — nothing written.');
|
console.log(' ✗ pre-flight check-versions is RED — nothing written.');
|
||||||
for (const n of applied.preflightErrors) console.log(` ERROR: ${n}`);
|
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.');
|
console.log(' Fix every ERROR (any plugin — the gate exit code is catalog-wide), then re-run.');
|
||||||
process.exit(1);
|
return 1;
|
||||||
}
|
}
|
||||||
|
|
||||||
console.log(` ✓ wrote ${mktPath} (ref ${plan.currentRef} -> ${plan.newRef})`);
|
console.log(` ✓ wrote ${mktPath} (ref ${plan.currentRef} -> ${plan.newRef})`);
|
||||||
|
|
@ -389,16 +393,38 @@ function main() {
|
||||||
console.log(' ✓ committed the catalog');
|
console.log(' ✓ committed the catalog');
|
||||||
if (args.push) {
|
if (args.push) {
|
||||||
const auth = pushGate.ensure();
|
const auth = pushGate.ensure();
|
||||||
if (!auth.authorised) { console.error(auth.message); process.exit(1); }
|
if (!auth.authorised) { console.error(auth.message); return 1; }
|
||||||
console.log(' → pushing');
|
console.log(' → pushing');
|
||||||
execFileSync('git', ['-C', catalogDir, 'push', 'origin', 'HEAD'], { stdio: 'inherit' });
|
execFileSync('git', ['-C', catalogDir, 'push', 'origin', 'HEAD'], { stdio: 'inherit' });
|
||||||
|
pushGate.pushed = true;
|
||||||
console.log(' ✓ pushed');
|
console.log(' ✓ pushed');
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
// Consume the shared token once, after the run's LAST successful push (D1) — a no-op
|
return 0;
|
||||||
// if nothing this run pushed.
|
}
|
||||||
pushGate.consume();
|
|
||||||
process.exit(0);
|
function main() {
|
||||||
|
const args = parseArgs(process.argv.slice(2));
|
||||||
|
if (!args.name) {
|
||||||
|
console.error('usage: release-plugin.mjs <name> [--version X.Y.Z] [--create-tag] [--write] [--commit] [--push]');
|
||||||
|
process.exit(2);
|
||||||
|
}
|
||||||
|
const catalogDir = join(dirname(fileURLToPath(import.meta.url)), '..');
|
||||||
|
const mktPath = join(catalogDir, '.claude-plugin', 'marketplace.json');
|
||||||
|
const marketplace = JSON.parse(readFileSync(mktPath, 'utf8'));
|
||||||
|
const pushGate = createPushGate({ cwd: catalogDir, home: process.env.HOME, exists: existsSync, unlink: unlinkSync });
|
||||||
|
|
||||||
|
let exitCode;
|
||||||
|
try {
|
||||||
|
exitCode = runRelease({ args, catalogDir, mktPath, marketplace, pushGate });
|
||||||
|
} finally {
|
||||||
|
// Q3c/D3 (order 20260912T220453Z-5021715764): consume the shared token on ANY exit
|
||||||
|
// from this run once at least one push has succeeded — not just the final line, which
|
||||||
|
// four earlier exits (BLOCKED plan, NOOP, dry-run, red pre-flight) used to bypass
|
||||||
|
// entirely. This is the only place consume() is called from.
|
||||||
|
if (pushGate.pushed) pushGate.consume();
|
||||||
|
}
|
||||||
|
process.exit(exitCode);
|
||||||
}
|
}
|
||||||
|
|
||||||
if (process.argv[1] && pathToFileURL(process.argv[1]).href === import.meta.url) {
|
if (process.argv[1] && pathToFileURL(process.argv[1]).href === import.meta.url) {
|
||||||
|
|
|
||||||
|
|
@ -3,9 +3,14 @@
|
||||||
// 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 { execFileSync } from 'node:child_process';
|
||||||
|
import { mkdtempSync, mkdirSync, writeFileSync as fsWriteFileSync, rmSync } from 'node:fs';
|
||||||
|
import { tmpdir } from 'node:os';
|
||||||
|
import { join } from 'node:path';
|
||||||
import {
|
import {
|
||||||
planRelease, reconcileReadmeLabel, preflightErrors, applyRelease, shouldCreateTag,
|
planRelease, reconcileReadmeLabel, preflightErrors, applyRelease, shouldCreateTag,
|
||||||
pushAuthorisation, requirePushAuthorisation, pushWithToken, consumeToken, createPushGate,
|
pushAuthorisation, requirePushAuthorisation, pushWithToken, consumeToken, createPushGate,
|
||||||
|
runRelease,
|
||||||
} from './release-plugin.mjs';
|
} from './release-plugin.mjs';
|
||||||
import { classifyPlugin } from './check-versions.mjs';
|
import { classifyPlugin } from './check-versions.mjs';
|
||||||
|
|
||||||
|
|
@ -412,3 +417,102 @@ test('createPushGate: an unauthorised run must not create the tag or touch the c
|
||||||
assert.equal(catalogWritten, false, 'no catalog change either — the run stops at the first blocked check');
|
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
|
gate.consume(); // must be safe even though ensure() never authorised anything
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// --- Q3c fix: D3 (token consumed on ANY exit once a push has succeeded) + S1
|
||||||
|
// (a real main()-path test, not a mirrored-variable one) — order
|
||||||
|
// 20260912T220453Z-5021715764-from-.claude ------------------------------------
|
||||||
|
//
|
||||||
|
// D3: Q3b's pushGate.consume() sat at the very bottom of main(), after four earlier
|
||||||
|
// process.exit() calls (BLOCKED plan, NOOP, dry-run, red pre-flight) that a run can hit
|
||||||
|
// AFTER a tag push already succeeded. process.exit() called from inside a try does NOT
|
||||||
|
// run its finally — verified live (node -e with a try/finally around process.exit(1)
|
||||||
|
// prints nothing from the finally) — so main() cannot just wrap the old body in
|
||||||
|
// try/finally as-is. The fix extracts the branching logic into `runRelease`, which
|
||||||
|
// returns an exit code instead of calling process.exit anywhere past the point a push
|
||||||
|
// might occur; main() alone calls process.exit, exactly once, after a finally that runs
|
||||||
|
// `if (pushGate.pushed) pushGate.consume()`. That is the ONLY exit point once a push may
|
||||||
|
// have happened — the fewest paths the order asked for.
|
||||||
|
//
|
||||||
|
// These two tests exercise the REAL git plumbing (temp repos, no mocked git calls) so
|
||||||
|
// they run against runRelease itself, not a copy of its logic — the exact weakness S1
|
||||||
|
// found in the superseded D2 test (it asserted on a variable the test set itself).
|
||||||
|
|
||||||
|
function makeTempRoot(prefix) {
|
||||||
|
return mkdtempSync(join(tmpdir(), prefix));
|
||||||
|
}
|
||||||
|
|
||||||
|
function initPluginRepo(repoDir, { version, remote } = {}) {
|
||||||
|
mkdirSync(join(repoDir, '.claude-plugin'), { recursive: true });
|
||||||
|
execFileSync('git', ['init', '-q', repoDir]);
|
||||||
|
execFileSync('git', ['-C', repoDir, 'config', 'user.email', 'x@x.com']);
|
||||||
|
execFileSync('git', ['-C', repoDir, 'config', 'user.name', 'x']);
|
||||||
|
if (remote) execFileSync('git', ['-C', repoDir, 'remote', 'add', 'origin', remote]);
|
||||||
|
fsWriteFileSync(join(repoDir, '.claude-plugin', 'plugin.json'), JSON.stringify({ version }));
|
||||||
|
fsWriteFileSync(join(repoDir, 'README.md'), '');
|
||||||
|
execFileSync('git', ['-C', repoDir, 'add', '.']);
|
||||||
|
execFileSync('git', ['-C', repoDir, 'commit', '-q', '-m', 'init']);
|
||||||
|
}
|
||||||
|
|
||||||
|
test('S1 (real git, no mocked ensure): runRelease does not create the tag when the push token is missing', () => {
|
||||||
|
const root = makeTempRoot('release-plugin-s1-');
|
||||||
|
try {
|
||||||
|
const catalogDir = join(root, 'catalog');
|
||||||
|
const repoDir = join(root, 'demo-plugin');
|
||||||
|
mkdirSync(catalogDir, { recursive: true });
|
||||||
|
initPluginRepo(repoDir, { version: '1.1.0' });
|
||||||
|
execFileSync('git', ['-C', repoDir, 'tag', '-a', 'v1.0.0', '-m', 'v1.0.0']);
|
||||||
|
|
||||||
|
const marketplace = { plugins: [{ name: 'demo-plugin', source: { source: 'url', url: 'x', ref: 'v1.0.0' }, description: 'd' }] };
|
||||||
|
const pushGate = createPushGate({
|
||||||
|
cwd: catalogDir, home: root, exists: () => false,
|
||||||
|
unlink: () => { throw new Error('BUG: must not consume without a push'); },
|
||||||
|
});
|
||||||
|
|
||||||
|
const code = runRelease({
|
||||||
|
args: { name: 'demo-plugin', createTag: true, write: true, commit: false, push: false, version: undefined },
|
||||||
|
catalogDir, mktPath: join(catalogDir, '.claude-plugin', 'marketplace.json'), marketplace, pushGate,
|
||||||
|
});
|
||||||
|
|
||||||
|
assert.notEqual(code, 0, 'an unauthorised tag push must not report success');
|
||||||
|
const tags = execFileSync('git', ['-C', repoDir, 'tag', '--list', 'v*'], { encoding: 'utf8' }).trim().split('\n').filter(Boolean);
|
||||||
|
assert.deepEqual(tags, ['v1.0.0'], 'no new tag may be created when the push token is missing');
|
||||||
|
assert.equal(pushGate.pushed, false, 'kjent-negativ: no push succeeded, so nothing must be marked pushed');
|
||||||
|
} finally {
|
||||||
|
rmSync(root, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
test('D3 (Q3c): the shared token is consumed once the tag push succeeds, even though the run then hits NOOP', () => {
|
||||||
|
const root = makeTempRoot('release-plugin-d3-');
|
||||||
|
try {
|
||||||
|
const originDir = join(root, 'origin.git');
|
||||||
|
const repoDir = join(root, 'demo-plugin');
|
||||||
|
const catalogDir = join(root, 'catalog');
|
||||||
|
mkdirSync(catalogDir, { recursive: true });
|
||||||
|
execFileSync('git', ['init', '-q', '--bare', originDir]);
|
||||||
|
initPluginRepo(repoDir, { version: '1.0.0', remote: originDir });
|
||||||
|
execFileSync('git', ['-C', repoDir, 'push', '-q', 'origin', 'HEAD:refs/heads/main']);
|
||||||
|
// No v1.0.0 tag exists yet — the catalog already pins v1.0.0 (as if it was bumped by
|
||||||
|
// hand before the tag was ever cut), so --create-tag has real work to do even though
|
||||||
|
// planRelease will resolve to NOOP once the tag exists.
|
||||||
|
|
||||||
|
const marketplace = { plugins: [{ name: 'demo-plugin', source: { source: 'url', url: originDir, ref: 'v1.0.0' }, description: 'd' }] };
|
||||||
|
const unlinked = [];
|
||||||
|
const pushGate = createPushGate({ cwd: catalogDir, home: root, exists: () => true, unlink: (p) => unlinked.push(p) });
|
||||||
|
|
||||||
|
const code = runRelease({
|
||||||
|
args: { name: 'demo-plugin', createTag: true, write: true, commit: false, push: false, version: undefined },
|
||||||
|
catalogDir, mktPath: join(catalogDir, '.claude-plugin', 'marketplace.json'), marketplace, pushGate,
|
||||||
|
});
|
||||||
|
// Mirrors main()'s own finally — the only place consume() is called from.
|
||||||
|
if (pushGate.pushed) pushGate.consume();
|
||||||
|
|
||||||
|
assert.equal(code, 0, 'catalog already pins v1.0.0 once the tag exists -> NOOP, exit 0');
|
||||||
|
const tags = execFileSync('git', ['-C', repoDir, 'tag', '--list', 'v*'], { encoding: 'utf8' }).trim().split('\n').filter(Boolean);
|
||||||
|
assert.deepEqual(tags, ['v1.0.0'], '--create-tag minted + pushed the tag before the NOOP verdict was even computed');
|
||||||
|
assert.equal(pushGate.pushed, true, 'runRelease must record the push even on a branch that is not the final line');
|
||||||
|
assert.deepEqual(unlinked, [pushGate.ensure().tokenPath], 'the token must be gone — a NOOP exit must not leave a used token behind (D3 regression)');
|
||||||
|
} finally {
|
||||||
|
rmSync(root, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue