fix(scanners): close the CLI argument class across all fourteen CLIs

`KNOWN_OPEN` in cli-unknown-flag-rejection.test.mjs named two CLIs as still
carrying the argument-swallow defect. That number was the previous session's
field of view, not a measurement. Measuring all fourteen found **7** open on
the unknown-flag arm and **10** on a second arm the deferral note never
described.

Arm 1 — unknown flag: with no `else` branch, `--zzz` leaves no trace. exit 0,
full payload, a confident answer to a question the caller did not ask.

Arm 2 — the sharper one: `a === '--output-file' && args[i + 1]` asks only
whether a next token EXISTS, never whether it is a value. `manifest`,
`campaign-cli` and `knowledge-refresh-cli` each wrote a file literally named
`--json` into the caller's working directory when handed `--output-file
--json`, exit 0, with `--json` mode silently dropped. A wrong answer is bad;
an unintended file on disk is worse.

Two of the CLIs this catches were already in GUARDED and green on arm 1 while
arm 2 stood open a few lines away — the guard asserted one relation instead of
the invariant.

Fixed with a shared gate (`lib/cli-args.mjs`) that runs BEFORE each CLI's own
parse loop rather than replacing it: valid argv reaches the existing parser
byte-for-byte unchanged, so the byte-stability argument is structural rather
than empirical. `drift-cli`, `fix-cli` and `plugin-health-scanner` were
already correct on both arms and were moved into GUARDED instead of rewritten.
The three CLIs with a bespoke unknown-flag branch had it removed once the gate
made it unreachable.

Suite 1488 → 1531. Frozen v5.0.0 snapshots untouched; `self-audit
--check-readme` passed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014P6Rh59Mtj4uYrdYMCYZJE
This commit is contained in:
Kjell Tore Guttormsen 2026-08-09 21:42:00 +02:00
commit 182a37c1af
14 changed files with 337 additions and 45 deletions

View file

@ -0,0 +1,52 @@
/**
* Unit-level companion to `tests/scanners/cli-unknown-flag-rejection.test.mjs`.
* That file measures the real CLIs end-to-end; this one pins the decision table
* itself, including the cases no CLI happens to exercise today.
*/
import { test } from 'node:test';
import { strict as assert } from 'node:assert';
import { findArgError } from '../../scanners/lib/cli-args.mjs';
const SPEC = { boolean: ['--json', '--raw'], value: ['--output-file', '--context-window'] };
test('well-formed argv passes', () => {
assert.equal(findArgError([], SPEC), null);
assert.equal(findArgError(['--json'], SPEC), null);
assert.equal(findArgError(['--output-file', 'out.json', '--raw'], SPEC), null);
assert.equal(findArgError(['/some/target', '--json'], SPEC), null);
});
test('an unknown flag is named in the diagnostic', () => {
const err = findArgError(['--zzz'], SPEC);
assert.match(err, /unknown flag "--zzz"/, 'the caller must be able to find the offending token');
});
test('a value flag followed by another flag is rejected', () => {
const err = findArgError(['--output-file', '--json'], SPEC);
assert.match(err, /--output-file/);
assert.match(err, /--json/, 'both the flag and the thing mistaken for its value must appear');
});
test('a value flag with nothing after it is rejected', () => {
assert.match(findArgError(['--output-file'], SPEC), /nothing followed it/);
});
test('a value is never re-read as a flag', () => {
// Without the consume step, a value that looks like a positional is harmless,
// but a spec change could make this the difference between pass and reject.
assert.equal(findArgError(['--output-file', 'report.json'], SPEC), null);
});
test('positionals and subcommands pass through untouched', () => {
assert.equal(findArgError(['init', '/target'], SPEC), null);
});
test('the FIRST error is reported, not the last', () => {
// A caller fixing errors one at a time should see them in argv order.
assert.match(findArgError(['--zzz', '--output-file'], SPEC), /--zzz/);
});
test('a spec with no value flags still rejects unknown flags', () => {
assert.match(findArgError(['--nope'], { boolean: ['--json'] }), /unknown flag "--nope"/);
});

View file

@ -28,29 +28,43 @@
import { test } from 'node:test';
import { strict as assert } from 'node:assert';
import { spawn } from 'node:child_process';
import { readFile } from 'node:fs/promises';
import { resolve, dirname } from 'node:path';
import { readFile, mkdtemp, readdir, rm } from 'node:fs/promises';
import { resolve, dirname, join } from 'node:path';
import { tmpdir } from 'node:os';
import { fileURLToPath } from 'node:url';
const __dirname = dirname(fileURLToPath(import.meta.url));
const SCANNERS_DIR = resolve(__dirname, '..', '..', 'scanners');
/** CLIs this guard holds to the invariant, with the argv they need to get past required-arg checks. */
/**
* Every CLI in `scanners/` that reads `process.argv`, with the argv it needs to
* get past required-arg checks and where it has one a value-taking flag.
*
* KNOWN_OPEN is gone. It named two CLIs; measuring all fourteen (#57) found
* **seven** open on the unknown-flag arm and **ten** on the value arm, including
* two CLIs that were already in GUARDED and passing. The deferral list was a
* prediction from the previous session's field of view, never a measurement.
*/
const GUARDED = [
{ cli: 'campaign-cli.mjs', argv: [] },
{ cli: 'knowledge-refresh-cli.mjs', argv: [] },
{ cli: 'campaign-write-cli.mjs', argv: ['init'] },
{ cli: 'campaign-export-cli.mjs', argv: ['--repo', '.'] },
{ cli: 'campaign-cli.mjs', argv: [], valueFlag: '--output-file' },
{ cli: 'knowledge-refresh-cli.mjs', argv: [], valueFlag: '--output-file' },
{ cli: 'campaign-write-cli.mjs', argv: ['init'], valueFlag: '--output-file' },
{ cli: 'campaign-export-cli.mjs', argv: ['--repo', '.'], valueFlag: '--output-file' },
{ cli: 'drift-cli.mjs', argv: [], valueFlag: '--baseline' },
{ cli: 'fix-cli.mjs', argv: ['--dry-run'], valueFlag: '--output-file' },
{ cli: 'plugin-health-scanner.mjs', argv: [], valueFlag: '--output-file' },
{ cli: 'manifest.mjs', argv: [], valueFlag: '--output-file' },
{ cli: 'optimize-lens-cli.mjs', argv: [], valueFlag: '--output-file' },
{ cli: 'posture.mjs', argv: [], valueFlag: '--context-window' },
{ cli: 'scan-orchestrator.mjs', argv: [], valueFlag: '--output-file' },
{ cli: 'token-hotspots-cli.mjs', argv: [], valueFlag: '--output-file' },
{ cli: 'whats-active.mjs', argv: [], valueFlag: '--output-file' },
{ cli: 'self-audit.mjs', argv: [], valueFlag: null }, // no value-taking flag
];
/** Same defect, deferred to the v5.14 arg-handling chunk together with their positional-swallow arm. */
const KNOWN_OPEN = ['optimize-lens-cli.mjs', 'token-hotspots-cli.mjs'];
function run(cli, argv) {
function run(cli, argv, cwd = resolve(__dirname, '..', '..')) {
return new Promise((res) => {
const child = spawn(process.execPath, [resolve(SCANNERS_DIR, cli), ...argv], {
cwd: resolve(__dirname, '..', '..'),
});
const child = spawn(process.execPath, [resolve(SCANNERS_DIR, cli), ...argv], { cwd });
let stderr = '';
child.stderr.on('data', (d) => { stderr += d; });
child.stdout.on('data', () => {});
@ -77,15 +91,56 @@ for (const { cli, argv } of GUARDED) {
});
}
test('the deferred CLIs are still deferred, and still counted', () => {
assert.equal(
KNOWN_OPEN.length,
2,
'When the v5.14 arg-handling chunk closes optimize-lens-cli and token-hotspots-cli,\n' +
'move them from KNOWN_OPEN into GUARDED rather than deleting them — the count is\n' +
'the record of how wide the class was.',
);
});
/**
* Arm 2 the value arm, and the sharper of the two.
*
* `else if (a === '--output-file' && args[i + 1])` only asks whether a next token
* EXISTS, never whether it is a value. So `--output-file --json` takes `--json`
* as the filename: measured live (#57), `manifest`, `campaign-cli` and
* `knowledge-refresh-cli` each wrote a file literally named `--json` into the
* caller's working directory, exit 0, with `--json` mode silently dropped. A
* wrong answer is bad; writing an unintended file is worse.
*
* Two of the CLIs this catches were already in GUARDED and green on arm 1 the
* guard was passing on one arm while the other stood open a few lines away
* ([[guard-can-be-green-on-its-own-defect]]).
*/
for (const { cli, argv, valueFlag } of GUARDED) {
if (!valueFlag) continue;
test(`${cli} rejects ${valueFlag} followed by another flag instead of a value`, async () => {
const scratch = await mkdtemp(join(tmpdir(), 'config-audit-argsluk-'));
try {
const { code, stderr } = await run(cli, [...argv, valueFlag, '--json'], scratch);
assert.equal(
code,
3,
`${cli} treated "--json" as the value of ${valueFlag} (exit ${code}). The flag needs a\n` +
'value; the next flag is not one. stderr was: ' + JSON.stringify(stderr),
);
assert.deepEqual(
await readdir(scratch),
[],
`${cli} wrote a file into the caller's working directory named after a FLAG. A CLI that\n` +
'misreads its own argv must not leave anything on disk.',
);
} finally {
await rm(scratch, { recursive: true, force: true });
}
});
test(`${cli} rejects ${valueFlag} with no value at all`, async () => {
const { code } = await run(cli, [...argv, valueFlag]);
assert.equal(
code,
3,
`${cli} ignored a trailing ${valueFlag} (exit ${code}). Under the ux-rule every command\n` +
'invokes these CLIs as `--output-file <path> 2>/dev/null` — an empty <path> must fail\n' +
'loudly, not fall through to stdout the command has been told to discard.',
);
});
}
/**
* The caller arm. #45/#46/#47 all taught the same lesson: fixing a CLI does not fix the