fix(catalog): test-vouch main()'s finally, orphan-tag cleanup, precise CLAUDE.md wording

Order 20260912T222008Z-8793921535-from-.claude (Q3d), follow-up to the PM
re-measurement of Q3c (dd278ca).

R1 — the run-scoped push-token consumption was only test-vouched at the
runRelease() level; D3's own test called consume() a second time in the TEST
BODY ("mirrors main()'s own finally") and asserted on that call, never on
anything main() itself did. A PM agent deleted the `finally` line in main()
in a copy and every existing test stayed green (39/39) while the real CLI,
run end-to-end, pushed the tag, hit NOOP, and left the token behind.

Added a test that runs the actual CLI entry point as a real subprocess
(main() calls process.exit(), so it cannot run in-process without killing
the test runner) against an isolated plugin repo, a local bare "remote",
and its own HOME — the same scenario the PM agent used (tag pushed, then a
NOOP branch that is not the run's final line). Mutation proof: removed the
`finally` line -> new test went RED (40 pass, 1 fail) while D3 stayed GREEN,
confirming D3 does not cover this path -> restored -> GREEN.

Sub-fix required to make the new test possible on this machine: macOS's
os.tmpdir() resolves through /var/folders, a symlink to /private/var/folders.
release-plugin.mjs's self-invocation guard compares the literal argv[1] path
against import.meta.url (which Node resolves through symlinks), so a script
run from the unresolved path never satisfies the guard and main() silently
never executes (exit 0, zero output). makeTempRoot() now returns the
realpath of the created temp dir.

S (side finding) — a tag push that fails after the local `git tag -a`
succeeded left an orphan local tag behind; a retry then failed on git's own
"tag already exists" (exit 128) instead of going through the idempotent
tag-absent path --create-tag already relies on. Chose: delete the local tag
when its push fails (option a) rather than detect-and-explain the orphan
state (option b) — it reuses the existing idempotency property instead of
adding a second one. Red-first: new test failed (orphan tag survived) ->
wrapped the push in try/catch, `git tag -d` on failure, rethrow -> GREEN.

R2 — CLAUDE.md said "a failed push leaves the token intact for the retry",
which is imprecise: with --create-tag --write --commit --push, if the tag
push succeeds and the catalog push then fails, the token IS consumed
(main()'s finally fires because pushGate.pushed was already set true by the
earlier successful push) even though the run overall "failed". Corrected to
state the actual rule: the token survives only when the run makes zero
successful pushes. The usage-block comment at the top of release-plugin.mjs
does not carry the same imprecise claim, so it needed no change.

Verification:
- node --test scripts/release-plugin.test.mjs: 39 -> 41/41 (R1, S added)
- node --test scripts/*.test.mjs: 156 -> 158/158
- node scripts/check-versions.mjs: 0 ERROR (1 known WARN: claude-design;
  repo-mailbox now OK — externally re-tagged since Q3c, untouched here)
- git tag -l: unchanged (12 tags, no new ones — no tag/push/bump this session)
- mutation proof for R1: finally line removed -> RED (D3 stayed green) -> restored -> GREEN
- mutation proof for S: recorded in scripts/release-plugin.test.mjs history above (red-first)

Not done (out of scope, deliberately): no version bump, no tag, no push;
no files touched outside scripts/release-plugin.mjs, scripts/release-plugin.test.mjs, CLAUDE.md.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

CLAUDE.md

@@ -81,8 +81,10 @@ their own Forgejo repositories under `https://git.fromaitochitta.com/open/`.
     `mkdir -p ~/.claude/runtime/push-approvals && touch "~/.claude/runtime/push-approvals/$(pwd | sed 's|/|_|g')"`
     (`pwd` must be this catalog directory — tag-push and catalog-push share ONE token, one
     publish from the operator's perspective). The script consumes the token itself right after
-    a push succeeds, the same way `post-push-consume.sh` does for a direct push; a failed push
-    leaves the token intact for the retry. Covered by `pushAuthorisation`/`requirePushAuthorisation`/
+    a push succeeds, the same way `post-push-consume.sh` does for a direct push; the token stays
+    intact for a retry only when the run makes ZERO successful pushes — once any push in the run
+    succeeds (e.g. the tag push in `--create-tag --write --commit --push`), the token is spent even
+    if a later push in that same run then fails. Covered by `pushAuthorisation`/`requirePushAuthorisation`/
     `pushWithToken`/`consumeToken` in `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

scripts/release-plugin.mjs

@@ -335,7 +335,16 @@ export function runRelease({ args, catalogDir, mktPath, marketplace, pushGate })
     const tag = 'v' + target;
     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, 'push', 'origin', tag], { stdio: 'inherit' });
+    // Q3d/S: if the push itself fails (network, permissions, remote gone), delete the
+    // local tag we just made rather than leave an orphan behind — a retry after fixing
+    // the underlying problem must go through shouldCreateTag's tag-absent check again,
+    // not hit git's own "tag already exists" (exit 128).
+    try {
+      execFileSync('git', ['-C', obs.repoDir, 'push', 'origin', tag], { stdio: 'inherit' });
+    } catch (err) {
+      execFileSync('git', ['-C', obs.repoDir, 'tag', '-d', tag], { stdio: 'inherit' });
+      throw err;
+    }
     pushGate.pushed = true;
     obs = observePlugin(catalogDir, args.name);
   }

scripts/release-plugin.test.mjs

@@ -3,10 +3,11 @@
 // is exercised by the CLI against the live tree, not here.
 import { test } from 'node:test';
 import assert from 'node:assert/strict';
-import { execFileSync } from 'node:child_process';
-import { mkdtempSync, mkdirSync, writeFileSync as fsWriteFileSync, rmSync } from 'node:fs';
+import { execFileSync, spawnSync } from 'node:child_process';
+import { mkdtempSync, mkdirSync, writeFileSync as fsWriteFileSync, readFileSync as fsReadFileSync, existsSync, realpathSync, rmSync } from 'node:fs';
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';
+import { fileURLToPath } from 'node:url';
 import {
   planRelease, reconcileReadmeLabel, preflightErrors, applyRelease, shouldCreateTag,
   pushAuthorisation, requirePushAuthorisation, pushWithToken, consumeToken, createPushGate,
@@ -437,8 +438,16 @@ test('createPushGate: an unauthorised run must not create the tag or touch the c
 // 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).
 
+// realpathSync matters here: macOS's tmpdir() is under /var/folders, itself a symlink
+// to /private/var/folders. release-plugin.mjs's own self-invocation guard
+// (`pathToFileURL(process.argv[1]).href === import.meta.url`, main() below) compares the
+// literal argv[1] path against import.meta.url, which Node resolves through symlinks —
+// so a script run from the unresolved /var/folders path never satisfies the guard and
+// main() silently never runs (exit 0, zero output). Only matters for tests that spawn
+// the real CLI as a subprocess (R1); the other temp-repo tests call runRelease directly
+// and never hit this path-identity check at all.
 function makeTempRoot(prefix) {
-  return mkdtempSync(join(tmpdir(), prefix));
+  return realpathSync(mkdtempSync(join(tmpdir(), prefix)));
 }
 
 function initPluginRepo(repoDir, { version, remote } = {}) {
@@ -482,6 +491,42 @@ test('S1 (real git, no mocked ensure): runRelease does not create the tag when t
   }
 });
 
+// --- Q3d/S: a failed TAG PUSH used to leave a local orphan tag behind (the local
+// `git tag -a` succeeds, then `git push origin <tag>` fails), so a retry after fixing
+// the network/permission issue hit "tag already exists" (exit 128) instead of a clean
+// idempotent re-run. Order 20260912T222008Z-8793921535-from-.claude, decided: delete the
+// local tag when its push fails (option a) — this reuses shouldCreateTag's existing
+// tag-absent check to make the retry idempotent, the same property --create-tag already
+// relies on, rather than inventing a second "orphan tag" state to detect and explain.
+
+test('S: a failed tag push deletes the local orphan tag so a retry is not blocked by "tag already exists"', () => {
+  const root = makeTempRoot('release-plugin-s-');
+  try {
+    const repoDir = join(root, 'demo-plugin');
+    const catalogDir = join(root, 'catalog');
+    mkdirSync(catalogDir, { recursive: true });
+    initPluginRepo(repoDir, { version: '1.1.0' }); // no origin remote configured -> the push fails
+    execFileSync('git', ['-C', repoDir, 'tag', '-a', 'v1.0.0', '-m', 'v1.0.0']); // pre-existing unrelated tag
+
+    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: () => true,
+      unlink: () => { throw new Error('BUG: must not consume — no push succeeded'); },
+    });
+
+    assert.throws(() => 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,
+    }), /origin/);
+
+    const tags = execFileSync('git', ['-C', repoDir, 'tag', '--list', 'v*'], { encoding: 'utf8' }).trim().split('\n').filter(Boolean);
+    assert.deepEqual(tags, ['v1.0.0'], 'the orphan v1.1.0 tag must be gone once its push has failed');
+    assert.equal(pushGate.pushed, false, 'a failed push must not be recorded as 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 {
@@ -516,3 +561,61 @@ test('D3 (Q3c): the shared token is consumed once the tag push succeeds, even th
     rmSync(root, { recursive: true, force: true });
   }
 });
+
+// --- Q3d/R1: the consume-on-exit line is only wired into main() itself, not into
+// runRelease — D3's own test calls consume() a second time in the TEST BODY ("Mirrors
+// main()'s own finally") and asserts on that call, not on anything main() actually did.
+// A PM re-measurement proved this gap live: deleting the `finally` line from main()
+// left every runRelease-level test green (39/39) while the real CLI, run end-to-end,
+// pushed the tag, hit NOOP, and left the token behind. This test exercises main() the
+// only way that is possible without killing the test-runner process — main() calls
+// process.exit(), so it must run as a real subprocess, against an isolated plugin repo,
+// a local bare "remote", and its own HOME (so the token path resolves inside the temp
+// tree, never the operator's real ~/.claude). Order 20260912T222008Z-8793921535-from-.claude.
+
+test('R1 (main(), real subprocess): the token is gone after the CLI returns, once its tag push has succeeded', () => {
+  const root = makeTempRoot('release-plugin-r1-');
+  try {
+    const originDir = join(root, 'origin.git');
+    const repoDir = join(root, 'demo-plugin');
+    const catalogDir = join(root, 'catalog');
+    mkdirSync(join(catalogDir, 'scripts'), { recursive: true });
+    mkdirSync(join(catalogDir, '.claude-plugin'), { 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 yet; the catalog already pins v1.0.0, so once --create-tag mints +
+    // pushes it, planRelease resolves to NOOP — the exact PM-measured scenario (tag
+    // pushed, then a later branch that isn't the run's final line, token still used).
+
+    // main() locates the catalog from import.meta.url, not from an injected path — so the
+    // real script (and its check-versions.mjs import) must physically live inside the temp
+    // tree for "catalogDir" to resolve there instead of to this repo's own working tree.
+    const scriptSrc = fileURLToPath(new URL('./release-plugin.mjs', import.meta.url));
+    const cvSrc = fileURLToPath(new URL('./check-versions.mjs', import.meta.url));
+    const scriptDest = join(catalogDir, 'scripts', 'release-plugin.mjs');
+    fsWriteFileSync(scriptDest, fsReadFileSync(scriptSrc, 'utf8'));
+    fsWriteFileSync(join(catalogDir, 'scripts', 'check-versions.mjs'), fsReadFileSync(cvSrc, 'utf8'));
+
+    fsWriteFileSync(join(catalogDir, '.claude-plugin', 'marketplace.json'), JSON.stringify({
+      plugins: [{ name: 'demo-plugin', source: { source: 'url', url: originDir, ref: 'v1.0.0' }, description: 'd' }],
+    }, null, 2));
+    fsWriteFileSync(join(catalogDir, 'README.md'), '');
+
+    const tokenDir = join(root, '.claude', 'runtime', 'push-approvals');
+    mkdirSync(tokenDir, { recursive: true });
+    const tokenPath = join(tokenDir, catalogDir.split('/').join('_'));
+    fsWriteFileSync(tokenPath, '');
+
+    const result = spawnSync(process.execPath, [scriptDest, 'demo-plugin', '--create-tag', '--write'], {
+      env: { ...process.env, HOME: root }, encoding: 'utf8',
+    });
+
+    assert.equal(result.status, 0, `expected NOOP exit 0; got ${result.status}\nstdout: ${result.stdout}\nstderr: ${result.stderr}`);
+    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.ok(!existsSync(tokenPath), "main()'s finally must consume the token after a real, end-to-end run — not just after runRelease() returns inside a test's own mirrored consume() call");
+  } finally {
+    rmSync(root, { recursive: true, force: true });
+  }
+});