fix(verdict-gate): read --repo=<dir> and refuse unknown options

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Kjell Tore Guttormsen 2026-10-26 13:44:11 +01:00
commit 703c508433
Signed by: ktg
SSH key fingerprint: SHA256:JakMjO6FTBBzN0Bhfj9saOoEjaFxlSdYuZQQpM/lF9Q
2 changed files with 85 additions and 5 deletions

View file

@ -4,7 +4,7 @@
// One command decides on the verdict written in review.md, so whatever stops a
// merge or a release reads a file and an exit code, never a model's summary:
//
// node lib/review/verdict-gate.mjs [--json] <review.md> [--repo <dir>]
// node lib/review/verdict-gate.mjs [--json] <review.md> [--repo <dir> | --repo=<dir>]
// 0 = ALLOW or WARN
// 1 = BLOCK
// 2 = could not decide: missing/unreadable file, not a review, no verdict,
@ -14,7 +14,8 @@
// WARN because a reviewer or engine did not run (`missing-reviewer:` /
// `unattributable-payload` in allow_blocked_by — operator decision
// 2026-10-06: a review not performed in full stops a merge), a
// `--repo` with no value, or, with `--repo <dir>`, an ALLOW/WARN
// `--repo` or `--repo=` with no value, any other option (M6), or,
// with `--repo <dir>`, an ALLOW/WARN
// review of another commit than the one being merged (reason
// `review-stale: …`: its `scope_sha_end` is not HEAD, or an ancestor
// of HEAD whose later commits change only the review lessons and the
@ -189,9 +190,11 @@ export function decideVerdictFile(filePath, { repo } = {}) {
if (isMain(import.meta.url)) {
const args = process.argv.slice(2);
const usage = 'Usage: verdict-gate.mjs [--json] <review.md> [--repo <dir>] (exactly one file)\n';
const usage = 'Usage: verdict-gate.mjs [--json] <review.md> [--repo <dir> | --repo=<dir>] (exactly one file)\n';
// --repo consumes its value before the file argument is collected, so the
// value is never read as a second file.
// value is never read as a second file. Any other option is refused (M6): a
// dropped --repo=<dir> or a misspelt flag would otherwise pass the gate
// without the check it asked for.
const files = [];
let repo;
for (let i = 0; i < args.length; i++) {
@ -203,7 +206,19 @@ if (isMain(import.meta.url)) {
process.exit(EXIT.UNDECIDED);
}
repo = v;
} else if (!a.startsWith('--')) {
} else if (a.startsWith('--repo=')) {
const v = a.slice('--repo='.length);
if (v === '') {
process.stderr.write(`verdict-gate: --repo= needs a value\n${usage}`);
process.exit(EXIT.UNDECIDED);
}
repo = v;
} else if (a === '--json') {
// read below
} else if (a.startsWith('--')) {
process.stderr.write(`verdict-gate: unknown option ${a}\n${usage}`);
process.exit(EXIT.UNDECIDED);
} else {
files.push(a);
}
}

View file

@ -625,3 +625,68 @@ test('[M11] a 64-hex name of a branch in a SHA-1 repository is stale with the re
assert.ok(r.stdout.includes(name), `the reason names the id as written: ${r.stdout}`);
} finally { fx.cleanup(); }
});
// ---- M6: --repo=<dir> is read, and an unknown option is refused ----
/** An ALLOW review in a throwaway directory; `fn(path)` gets its path. */
function withAllowReview(fn) {
const dir = mkdtempSync(join(tmpdir(), 'verdict-gate-m6-'));
try {
const p = join(dir, 'review.md');
writeFileSync(p, review({ verdict: 'ALLOW', severities: ['MINOR'] }));
return fn(p);
} finally {
rmSync(dir, { recursive: true, force: true });
}
}
function gateArgs(...argv) {
return spawnSync(process.execPath, [GATE, ...argv], { encoding: 'utf-8', env: childEnv() });
}
test('[M6] --repo=<dir> runs the stale check', () => {
const fx = gateRepo();
try {
fx.commit('fix: more work', { 'a.mjs': 'export const a = 2;\n' });
const p = join(fx.base, 'review.md');
writeFileSync(p, review({ verdict: 'ALLOW', severities: ['MINOR'], scopeShaEnd: fx.reviewed }));
assertStale(gateArgs('--json', p, `--repo=${fx.dir}`), /a\.mjs/);
} finally { fx.cleanup(); }
});
test('[M6] --repo= with no value exits 2', () => {
withAllowReview((p) => {
const r = gateArgs('--json', p, '--repo=');
assert.equal(r.status, 2, r.stdout + r.stderr);
assert.match(r.stderr, /--repo= needs a value/);
});
});
test('[M6] an unknown option exits 2', () => {
withAllowReview((p) => {
const r = gateArgs('--bogus', p);
assert.equal(r.status, 2, r.stdout + r.stderr);
assert.match(r.stderr, /unknown option --bogus/);
assert.match(r.stderr, /Usage: verdict-gate\.mjs/);
});
});
test('[M6] an unknown --x=value option exits 2', () => {
withAllowReview((p) => {
const r = gateArgs(p, '--x=value');
assert.equal(r.status, 2, r.stdout + r.stderr);
assert.match(r.stderr, /unknown option --x=value/);
assert.match(r.stderr, /Usage: verdict-gate\.mjs/);
});
});
test('[M6] --json is accepted while --jsonx is refused', () => {
withAllowReview((p) => {
const ok = gateArgs('--json', p);
assert.equal(ok.status, 0, ok.stdout + ok.stderr);
assert.equal(JSON.parse(ok.stdout).verdict, 'ALLOW');
const bad = gateArgs('--jsonx', p);
assert.equal(bad.status, 2, bad.stdout + bad.stderr);
assert.match(bad.stderr, /unknown option --jsonx/);
});
});