fix(release): NOOP no longer drops a pending commit/push, token scope documented accurately
Three defects in release-plugin.mjs measured live by llm-security S4 during the v8.1.0 release (order 20260922T185613Z-5930828122-from-.claude): 1. The header/token comments claimed the tag-push and catalog-push "share ONE token" without qualifying the scope, which read as spanning a whole release however many separate invocations it took. The push-approval token is actually scoped to ONE script invocation (createPushGate consumes it as soon as that run's first push succeeds) — chosen over making the token survive across separate processes, since a persisted cross-invocation authorisation is exactly the kind of standing grant the one-shot design exists to avoid. Comments and the operator-facing BLOCKED message now say this, and recommend the combined `--create-tag --write --commit --push` as the one-token-one-release path. 2. `--write --commit` run after an earlier `--write`-only invocation reported NOOP and committed nothing: a fresh process re-reads marketplace.json off disk, sees the target ref already written (but uncommitted) by the prior run, and planRelease — which has no git access — cannot tell that apart from an already-released catalog. 3. NOOP returned before ever checking --push, so a pending write could also never be pushed by a follow-up invocation. Fixed by pendingCatalogChanges(), which checks the working tree for the plugin's catalog files; a NOOP verdict with --commit requested against a dirty tree now finishes the release (commit, push, Forgejo release object) via a shared finishPublish() instead of silently reporting "nothing to do". A genuinely clean NOOP is unchanged (still exits early, still never touches the push gate). Also isolates release-plugin.test.mjs's temp git fixtures from the machine-global pre-push hook (installed today, order 20260918T004628Z, live during this session) via a repo-local core.hooksPath override — those tests exercise this script's own token/NOOP/push logic, not that unrelated global CHANGELOG policy, and R-FJ1 specifically needs the tag-message fallback path a real CHANGELOG.md would short-circuit. TDD: BUG 2 and BUG 3 were written RED against the unmodified script (real git temp repos, two-invocation traces reproducing the measured scenario) before the fix; full suite 217/217, check-versions 0 ERROR (1 pre-existing WARN on llm-security's in-flight, unrelated v8.1.1 bump). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
1fd3743f46
commit
bb43a20376
SSH key fingerprint:
SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
2 changed files with 328 additions and 85 deletions
|
|
@ -13,7 +13,7 @@ import {
|
|||
pushAuthorisation, requirePushAuthorisation, pushWithToken, consumeToken, createPushGate,
|
||||
runRelease, preflightStatMismatches, reportPostWriteCheck,
|
||||
parseForgejoRepo, planForgejoRelease, ensureForgejoRelease,
|
||||
extractChangelogSection, releaseBodyFrom,
|
||||
extractChangelogSection, releaseBodyFrom, pendingCatalogChanges,
|
||||
} from './release-plugin.mjs';
|
||||
import { classifyPlugin } from './check-versions.mjs';
|
||||
|
||||
|
|
@ -367,8 +367,10 @@ test('shouldCreateTag: a null README badge is tolerated (badge-less plugin)', ()
|
|||
// gate. So the ONE script that pushes must require the SAME one-shot approval
|
||||
// token the gate checks, and consume it itself after a push actually succeeds
|
||||
// (post-push-consume.sh, a PostToolUse hook, never fires for a call the gate
|
||||
// never saw). Tag-push and catalog-push share ONE token: one publish from the
|
||||
// operator's point of view.
|
||||
// never saw). The token is scoped to ONE SCRIPT INVOCATION (see the top-of-file usage
|
||||
// note in release-plugin.mjs, corrected by order 20260922T185613Z-5930828122): every
|
||||
// push that ONE run makes shares it, which is why the combined
|
||||
// `--create-tag --write --commit --push` is the recommended one-token-one-release path.
|
||||
|
||||
test('pushAuthorisation computes the token path exactly like token_path() — only / becomes _', () => {
|
||||
// Deliberately includes '-' and '.' in the path to prove ONLY '/' is rewritten,
|
||||
|
|
@ -527,11 +529,21 @@ function makeTempRoot(prefix) {
|
|||
return realpathSync(mkdtempSync(join(tmpdir(), prefix)));
|
||||
}
|
||||
|
||||
// core.hooksPath is disabled REPO-LOCALLY (not globally): the machine-global pre-push
|
||||
// hook (installed 2026-09-22, order 20260918T004628Z-8837326001) refuses to push a
|
||||
// version tag whose CHANGELOG.md has no non-empty section for that version, on every
|
||||
// repository on this machine, including these throwaway temp fixtures. These tests are
|
||||
// exercising release-plugin.mjs's OWN token/NOOP/push logic, not that unrelated global
|
||||
// policy (which has its own suite in .claude) — a repo-local override isolates the SUT
|
||||
// from it without touching global config, matching the "in doubt, isolate the unit
|
||||
// under test" default rather than crafting a real CHANGELOG.md purely to appease a
|
||||
// hook some of these tests (R-FJ1) specifically exist to prove the ABSENCE of.
|
||||
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']);
|
||||
execFileSync('git', ['-C', repoDir, 'config', 'core.hooksPath', '/dev/null']);
|
||||
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'), '');
|
||||
|
|
@ -639,6 +651,180 @@ test('D3 (Q3c): the shared token is consumed once the tag push succeeds, even th
|
|||
}
|
||||
});
|
||||
|
||||
// --- Order 20260922T185613Z-5930828122-from-.claude: NOOP silently dropped a pending
|
||||
// commit/push (measured live by llm-security S4 during the v8.1.0 release, 22.09.2026).
|
||||
//
|
||||
// The real trace: `--create-tag --write` (no --commit) writes the bumped catalog ref to
|
||||
// disk but does not commit it. A SEPARATE, later invocation `--write --commit` starts a
|
||||
// fresh process, which re-reads marketplace.json off disk — already at the target ref —
|
||||
// so planRelease resolves NOOP, and the old code returned 0 right there (line ~719),
|
||||
// before ever looking at args.commit or args.push. The operator had to `git add`/commit
|
||||
// by hand, and the script never pushed the pending catalog change either.
|
||||
//
|
||||
// pendingCatalogChanges() lets runRelease tell "already released" apart from "an earlier
|
||||
// --write left this uncommitted", and finish the pending commit/push in that second case
|
||||
// instead of silently reporting "nothing to do".
|
||||
|
||||
test('BUG 2 (order 20260922T185613Z): --write --commit after a prior --write-only run must actually commit the pending catalog write', () => {
|
||||
const root = makeTempRoot('release-plugin-noop-commit-');
|
||||
try {
|
||||
const repoDir = join(root, 'demo-plugin');
|
||||
const catalogDir = join(root, 'catalog');
|
||||
mkdirSync(join(catalogDir, '.claude-plugin'), { recursive: true });
|
||||
initPluginRepo(repoDir, { version: '1.1.0' });
|
||||
execFileSync('git', ['-C', repoDir, 'tag', '-a', 'v1.0.0', '-m', 'v1.0.0']);
|
||||
execFileSync('git', ['-C', repoDir, 'tag', '-a', 'v1.1.0', '-m', 'v1.1.0']);
|
||||
|
||||
const mktPath = join(catalogDir, '.claude-plugin', 'marketplace.json');
|
||||
const marketplaceBefore = { plugins: [{ name: 'demo-plugin', source: { source: 'url', url: 'x', ref: 'v1.0.0' }, description: 'd' }] };
|
||||
fsWriteFileSync(mktPath, JSON.stringify(marketplaceBefore, null, 2));
|
||||
fsWriteFileSync(join(catalogDir, 'README.md'), '### [Demo Plugin](https://x/open/demo-plugin) `v1.0.0`\n');
|
||||
execFileSync('git', ['init', '-q', catalogDir]);
|
||||
execFileSync('git', ['-C', catalogDir, 'config', 'user.email', 'x@x.com']);
|
||||
execFileSync('git', ['-C', catalogDir, 'config', 'user.name', 'x']);
|
||||
execFileSync('git', ['-C', catalogDir, 'add', '.']);
|
||||
execFileSync('git', ['-C', catalogDir, 'commit', '-q', '-m', 'init catalog']);
|
||||
|
||||
const pushGate = createPushGate({ cwd: catalogDir, home: root, exists: () => false, unlink: () => {} });
|
||||
|
||||
// Invocation 1 mirrors `--write` alone (no --commit): writes the bumped ref, uncommitted.
|
||||
const code1 = runRelease({
|
||||
args: { name: 'demo-plugin', version: undefined, createTag: false, write: true, commit: false, push: false },
|
||||
catalogDir, mktPath, marketplace: marketplaceBefore, pushGate,
|
||||
runCheckVersions: () => '1 plugins — 1 OK, 0 WARN, 0 ERROR, 0 SKIP — verified 1/1\n',
|
||||
});
|
||||
assert.equal(code1, 0);
|
||||
assert.ok(fsReadFileSync(mktPath, 'utf8').includes('v1.1.0'), 'invocation 1 wrote the bumped ref to disk');
|
||||
const statusAfter1 = execFileSync('git', ['-C', catalogDir, 'status', '--porcelain'], { encoding: 'utf8' });
|
||||
assert.notEqual(statusAfter1.trim(), '', 'the write is uncommitted after invocation 1 — the real S4 scenario');
|
||||
|
||||
// Invocation 2 is a FRESH process re-reading marketplace.json off disk (already v1.1.0).
|
||||
const marketplaceOnDisk = JSON.parse(fsReadFileSync(mktPath, 'utf8'));
|
||||
const code2 = runRelease({
|
||||
args: { name: 'demo-plugin', version: undefined, createTag: false, write: true, commit: true, push: false },
|
||||
catalogDir, mktPath, marketplace: marketplaceOnDisk, pushGate,
|
||||
runCheckVersions: () => '1 plugins — 1 OK, 0 WARN, 0 ERROR, 0 SKIP — verified 1/1\n',
|
||||
});
|
||||
|
||||
assert.equal(code2, 0, 'finishing a pending release must succeed');
|
||||
const statusAfter2 = execFileSync('git', ['-C', catalogDir, 'status', '--porcelain'], { encoding: 'utf8' });
|
||||
assert.equal(statusAfter2.trim(), '', 'BUG 2: --write --commit after --create-tag --write must actually commit the pending write');
|
||||
const log = execFileSync('git', ['-C', catalogDir, 'log', '-1', '--pretty=%s'], { encoding: 'utf8' }).trim();
|
||||
assert.match(log, /demo-plugin/);
|
||||
} finally {
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('BUG 3 (order 20260922T185613Z): --write --commit --push after a prior --write-only run must actually push the pending catalog commit', () => {
|
||||
const root = makeTempRoot('release-plugin-noop-push-');
|
||||
try {
|
||||
const bare = join(root, 'catalog-origin.git');
|
||||
execFileSync('git', ['init', '-q', '--bare', bare]);
|
||||
|
||||
const repoDir = join(root, 'demo-plugin');
|
||||
const catalogDir = join(root, 'catalog');
|
||||
mkdirSync(join(catalogDir, '.claude-plugin'), { recursive: true });
|
||||
initPluginRepo(repoDir, { version: '1.1.0' });
|
||||
execFileSync('git', ['-C', repoDir, 'tag', '-a', 'v1.0.0', '-m', 'v1.0.0']);
|
||||
execFileSync('git', ['-C', repoDir, 'tag', '-a', 'v1.1.0', '-m', 'v1.1.0']);
|
||||
|
||||
const mktPath = join(catalogDir, '.claude-plugin', 'marketplace.json');
|
||||
const url = 'https://git.fromaitochitta.com/open/demo-plugin';
|
||||
const marketplaceBefore = { plugins: [{ name: 'demo-plugin', source: { source: 'url', url, ref: 'v1.0.0' }, description: 'd' }] };
|
||||
fsWriteFileSync(mktPath, JSON.stringify(marketplaceBefore, null, 2));
|
||||
fsWriteFileSync(join(catalogDir, 'README.md'), '### [Demo Plugin](https://x/open/demo-plugin) `v1.0.0`\n');
|
||||
execFileSync('git', ['init', '-q', catalogDir]);
|
||||
execFileSync('git', ['-C', catalogDir, 'config', 'user.email', 'x@x.com']);
|
||||
execFileSync('git', ['-C', catalogDir, 'config', 'user.name', 'x']);
|
||||
execFileSync('git', ['-C', catalogDir, 'remote', 'add', 'origin', bare]);
|
||||
execFileSync('git', ['-C', catalogDir, 'add', '.']);
|
||||
execFileSync('git', ['-C', catalogDir, 'commit', '-q', '-m', 'init catalog']);
|
||||
execFileSync('git', ['-C', catalogDir, 'push', '-q', 'origin', 'HEAD:refs/heads/main']);
|
||||
|
||||
const pushGate = createPushGate({ cwd: catalogDir, home: root, exists: () => true, unlink: () => {} });
|
||||
// Already exists so finishPublish's Forgejo step (publishes=true once args.push fires)
|
||||
// is a harmless NOOP, keeping this test's assertions focused on the catalog push.
|
||||
const forgejo = { listReleaseTags: () => ['v1.1.0'], createRelease: () => { throw new Error('must not be called — release already exists'); } };
|
||||
|
||||
const code1 = runRelease({
|
||||
args: { name: 'demo-plugin', version: undefined, createTag: false, write: true, commit: false, push: false },
|
||||
catalogDir, mktPath, marketplace: marketplaceBefore, pushGate, forgejo,
|
||||
runCheckVersions: () => '1 plugins — 1 OK, 0 WARN, 0 ERROR, 0 SKIP — verified 1/1\n',
|
||||
});
|
||||
assert.equal(code1, 0);
|
||||
|
||||
const marketplaceOnDisk = JSON.parse(fsReadFileSync(mktPath, 'utf8'));
|
||||
const code2 = runRelease({
|
||||
args: { name: 'demo-plugin', version: undefined, createTag: false, write: true, commit: true, push: true },
|
||||
catalogDir, mktPath, marketplace: marketplaceOnDisk, pushGate, forgejo,
|
||||
runCheckVersions: () => '1 plugins — 1 OK, 0 WARN, 0 ERROR, 0 SKIP — verified 1/1\n',
|
||||
});
|
||||
|
||||
assert.equal(code2, 0, 'finishing a pending release (commit+push) must succeed');
|
||||
const originLog = execFileSync('git', ['--git-dir', bare, 'log', '-1', '--pretty=%s', 'refs/heads/main'], { encoding: 'utf8' }).trim();
|
||||
assert.match(originLog, /demo-plugin/, 'BUG 3: NOOP must not skip the push of a pending catalog commit');
|
||||
} finally {
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('NOOP with a genuinely clean tree still reports nothing to do, even with --commit requested (no accidental commit, no push-gate touch)', () => {
|
||||
const root = makeTempRoot('release-plugin-noop-clean-');
|
||||
try {
|
||||
const repoDir = join(root, 'demo-plugin');
|
||||
const catalogDir = join(root, 'catalog');
|
||||
mkdirSync(join(catalogDir, '.claude-plugin'), { recursive: true });
|
||||
initPluginRepo(repoDir, { version: '1.0.0' });
|
||||
execFileSync('git', ['-C', repoDir, 'tag', '-a', 'v1.0.0', '-m', 'v1.0.0']);
|
||||
|
||||
const mktPath = join(catalogDir, '.claude-plugin', 'marketplace.json');
|
||||
const marketplace = { plugins: [{ name: 'demo-plugin', source: { source: 'url', url: 'x', ref: 'v1.0.0' }, description: 'd' }] };
|
||||
fsWriteFileSync(mktPath, JSON.stringify(marketplace, null, 2));
|
||||
fsWriteFileSync(join(catalogDir, 'README.md'), '### [Demo Plugin](https://x/open/demo-plugin) `v1.0.0`\n');
|
||||
execFileSync('git', ['init', '-q', catalogDir]);
|
||||
execFileSync('git', ['-C', catalogDir, 'config', 'user.email', 'x@x.com']);
|
||||
execFileSync('git', ['-C', catalogDir, 'config', 'user.name', 'x']);
|
||||
execFileSync('git', ['-C', catalogDir, 'add', '.']);
|
||||
execFileSync('git', ['-C', catalogDir, 'commit', '-q', '-m', 'init']);
|
||||
|
||||
const pushGate = createPushGate({
|
||||
cwd: catalogDir, home: root,
|
||||
exists: () => { throw new Error('BUG: a clean NOOP must never touch the push gate'); },
|
||||
unlink: () => { throw new Error('BUG: nothing to consume'); },
|
||||
});
|
||||
|
||||
const code = runRelease({
|
||||
args: { name: 'demo-plugin', version: undefined, createTag: false, write: true, commit: true, push: false },
|
||||
catalogDir, mktPath, marketplace, pushGate,
|
||||
});
|
||||
|
||||
assert.equal(code, 0);
|
||||
const log = execFileSync('git', ['-C', catalogDir, 'log', '--oneline'], { encoding: 'utf8' }).trim().split('\n');
|
||||
assert.equal(log.length, 1, 'no new commit was made for a genuinely clean NOOP');
|
||||
} finally {
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('pendingCatalogChanges: true when git status reports changes to the given paths', () => {
|
||||
const calls = [];
|
||||
const exec = (cmd, cmdArgs) => { calls.push(cmdArgs); return ' M .claude-plugin/marketplace.json\n'; };
|
||||
const dirty = pendingCatalogChanges({ catalogDir: '/cat', paths: ['/cat/.claude-plugin/marketplace.json', '/cat/README.md'] }, exec);
|
||||
assert.equal(dirty, true);
|
||||
assert.deepEqual(calls[0], ['-C', '/cat', 'status', '--porcelain', '--', '/cat/.claude-plugin/marketplace.json', '/cat/README.md']);
|
||||
});
|
||||
|
||||
test('pendingCatalogChanges: false on a clean tree', () => {
|
||||
const dirty = pendingCatalogChanges({ catalogDir: '/cat', paths: ['/cat/x'] }, () => '');
|
||||
assert.equal(dirty, false);
|
||||
});
|
||||
|
||||
test('pendingCatalogChanges: false (not a crash) when the exec call itself fails', () => {
|
||||
const dirty = pendingCatalogChanges({ catalogDir: '/cat', paths: ['/cat/x'] }, () => { throw new Error('not a git repository'); });
|
||||
assert.equal(dirty, false);
|
||||
});
|
||||
|
||||
// --- 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.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue