feat(cli): --help that never starts a run, and unknown flags are refused

CLAUDE.md

@@ -20,12 +20,20 @@ The codebase is a small ESM-only Node.js CLI. `bin/twd-cli.js` and `src/index.js
 
 **`bin/twd-cli.js`**: CLI entry point. Parses `process.argv` for the `run` command via `src/parseArgs.js`, calls `runTests()`, and exits with code 0 (pass) or 1 (failure).
 
+Help is resolved **first**, before either parser runs and before the dynamic `import('../src/index.js')`: a bare `twd-cli`, `help` (optionally `help <command>`), `--help` or `-h` as the command, or `--help`/`-h` anywhere after `run`/`merge`. That ordering is the whole point — the imports are dynamic so that `merge` never loads puppeteer, and the help path has to preserve the property. Before this existed `run --help` was an unknown token the parser dropped, so it **ran the entire suite**. Help goes to stdout with exit 0 and leaves the process to drain instead of calling `process.exit`, so a piped stdout cannot truncate it. An unknown command is a usage error: stderr, exit 1.
+
 **`src/changedTests.js`**: `resolveChangedTitles(ref, cwd)` shells out to git (`execFileSync` with an argument array, never a string — a ref is user input) and returns the `it()` titles this branch added or changed, to feed the same filter path `--test` uses. `extractTitles(source)` is the pure half. Test files are identified by the **suffix** `*.twd.test.*`, never by directory: the examples use `src/twd-tests`, `app/twd-tests` and `src/twd-test` between them, and the suffix also keeps a project's Vitest suite — which uses `it()` too — from contributing titles. Diffs to the **working tree** (`git diff <base>`, no second ref) and adds untracked test files, so uncommitted work counts; in CI the tree is clean and this is identical to `<base> HEAD`.
 
 **`src/parseArgs.js`**: `parseRunArgs(argv)` returns `{ testFilters, changedSince, record, shard, reportDir, updateSnapshots, ci }`. Supports `--test` (repeatable substring filter), `--changed-since`, and the recording flags `--record`, `--record-dir`, `--record-speed`, `--record-pace`. Each accepts both `--flag value` and `--flag=value`; a value starting with `--` is refused, so `--test --record` cannot swallow the flag after it. The returned `record` object is passed to `runTests()` as `recordOverrides` and wins over the config file.
 
+`RUN_FLAGS` and `MERGE_FLAGS` are the exported list of what each parser recognises, and they do two jobs: `tests/usage.test.js` derives the expected `--help` contents from them, and the "Did you mean" suggestion picks from them. A flag added to a parser has to be added to its list and given a help line in `src/usage.js`, or the suite fails.
+
+Any `--`-prefixed token no branch claimed makes the parser **throw** — `twd-cli run: unknown option --tests`, a suggestion when one is close, and a pointer at `--help` — so the bin's existing catch prints it to stderr and exits 1 without running anything. This is what makes `--help` reliable rather than cosmetic: a flag the bin forgets to route fails loudly instead of running the suite, which is exactly what `run --help` used to do. The suggestion is a prefix match first (`--output` → `--out`), otherwise the nearest flag within **two** edits; the budget is tight on purpose because `--out` passed to `run` must not come back as "Did you mean --ci?". Positionals keep their old treatment: `merge` takes the first as `<dir>`, `run` ignores them.
+
 The numeric guards on the two recording flags differ **on purpose**, so do not harmonise them: `--record-pace` accepts `>= 0` because 0 is the documented way to turn pacing off, while `--record-speed` accepts `> 0` because a playback multiplier of 0 is meaningless. Grouping pace's 0 with negatives is what made `--record-pace 0` a silent no-op that fell back to the 300ms default while the same value in `twd.config.json` worked. A falsy override must survive the `{ ...config.record, ...recordOverrides }` merge in `src/index.js` for the same reason.
 
+**`src/usage.js`**: `globalUsage()`, `runUsage()`, `mergeUsage()` — one template string each, a shared header that states the two value forms once. Deliberately no formatting library. The blocks are read by agents pasting `--help` into a prompt as much as by people, so every flag is on its own line with its default.
+
 **`src/config.js`**: `loadConfig()` reads `twd.config.json` from `process.cwd()`, merges it with defaults (url, timeout, coverage, coverageDir, nycOutputDir, headless, puppeteerArgs, retryCount, protocolTimeout, maxFailures, chunkSize, record), and returns the merged config. Falls back to defaults if the file is missing or unparseable.
 
 `--changed-since` deliberately makes a zero-match run exit **0**: it is a query, and an empty result is a normal CI outcome. `--test` keeps its exit 1, because a filter you typed is an assertion and a typo must not look like a pass. For the same reason the "matched no tests" warning is raised only for filters the user actually typed — a computed title matching nothing is unactionable noise.
@@ -106,7 +114,7 @@ running with `if-no-files-found: error`.
 
 Tests are in `tests/` and use vitest, one file per `src/` module. The suite mocks `fs` to test config loading and mocks Puppeteer to test the run flow. Coverage is configured for `src/**/*.js` only.
 
-No test may require a real ffmpeg binary or a real browser: `node:child_process` and `page.screencast` are always mocked. `tests/runTests.test.js` mocks the two ffmpeg-spawning helpers but deliberately runs the **real** `watchRecorder` / `stopRecording`, because the hang they prevent only appears in the wiring — its recorder stand-ins are real `EventEmitter`s for that reason, since production is handed a `PassThrough`. Note that `vi.mock('fs')` auto-mocks `fs.statSync` to return `undefined`, so anything reading a `Stats` has to tolerate that.
+No test may require a real ffmpeg binary or a real browser: `node:child_process` and `page.screencast` are always mocked. The one file that spawns a real process is `tests/cli.test.js`, which runs `bin/twd-cli.js` under `node` with `execFile` to assert exit code and stream for real — the `run --help` bug was invisible to every unit in isolation. Every case in it returns before the dynamic import of `src/index.js`, so no browser is involved, and a regression to running the suite fails on exit code alone because there is no dev server. `tests/runTests.test.js` mocks the two ffmpeg-spawning helpers but deliberately runs the **real** `watchRecorder` / `stopRecording`, because the hang they prevent only appears in the wiring — its recorder stand-ins are real `EventEmitter`s for that reason, since production is handed a `PassThrough`. Note that `vi.mock('fs')` auto-mocks `fs.statSync` to return `undefined`, so anything reading a `Stats` has to tolerate that.
 
 ## Releases
 

README.md

@@ -33,6 +33,26 @@ Run tests with default configuration:
 npx twd-cli run
 ```
 
+### Getting help
+
+```bash
+npx twd-cli --help          # the commands
+npx twd-cli run --help      # every run option
+npx twd-cli merge --help
+```
+
+Help prints and exits `0` without launching a browser or reading your config.
+A flag the CLI does not know is refused before anything runs, with the closest
+match suggested, so a typo cannot quietly run the whole suite:
+
+```
+$ npx twd-cli run --tests "Login"
+twd-cli run: unknown option --tests
+
+Did you mean --test?
+Run `twd-cli run --help` to see every option.
+```
+
 ### Filtering tests
 
 Run only a subset of tests with the repeatable `--test` flag. Matching is

bin/twd-cli.js

@@ -3,15 +3,29 @@
 // runTests and runMerge are imported inside their branches, not here. A static
 // import of src/index.js pulls in puppeteer, so `twd-cli merge` — which never
 // opens a browser — would otherwise load the whole browser-automation graph
-// before it even looked at argv.
+// before it even looked at argv. The help paths below return for the same
+// reason: `run --help` used to run the entire suite.
 import { parseRunArgs, parseMergeArgs } from '../src/parseArgs.js';
+import { globalUsage, runUsage, mergeUsage } from '../src/usage.js';
 
-const command = process.argv[2];
+const [command, ...args] = process.argv.slice(2);
 
-if (command === 'run') {
+const USAGE = { run: runUsage, merge: mergeUsage };
+const isHelp = (token) => token === '--help' || token === '-h';
+
+// Help is decided here, before either parser runs and before any dynamic
+// import, so asking for it never reads a config, touches git or launches a
+// browser. Success text goes to stdout with exit 0; the process is left to
+// drain rather than exited, so a piped stdout cannot truncate it.
+if (command === undefined || command === 'help' || isHelp(command)) {
+  const topic = command === 'help' ? args[0] : undefined;
+  console.log((USAGE[topic] ?? globalUsage)());
+} else if (USAGE[command] && args.some(isHelp)) {
+  console.log(USAGE[command]());
+} else if (command === 'run') {
   try {
     const { testFilters, changedSince, record, shard, reportDir, updateSnapshots, ci } =
-      parseRunArgs(process.argv.slice(3));
+      parseRunArgs(args);
     const { runTests } = await import('../src/index.js');
     const hasFailures = await runTests({
       testFilters,
@@ -31,7 +45,7 @@ if (command === 'run') {
   }
 } else if (command === 'merge') {
   try {
-    const { dir, out } = parseMergeArgs(process.argv.slice(3));
+    const { dir, out } = parseMergeArgs(args);
     const { runMerge } = await import('../src/mergeCommand.js');
     const hasFailures = runMerge({ dir, out });
     process.exit(hasFailures ? 1 : 0);
@@ -42,54 +56,10 @@ if (command === 'run') {
     process.exit(1);
   }
 } else {
-  console.log(`
-twd-cli - Test runner for TWD tests
-
-Usage:
-  npx twd-cli run                  Run all tests
-  npx twd-cli run --test "<name>"  Run only tests whose "suite > test" path
-                                   contains <name> (case-insensitive).
-                                   Repeatable; multiple --test values are OR'd.
-  npx twd-cli run --changed-since <ref>
-                                   Run only the tests this branch added or
-                                   changed, relative to <ref>
-  npx twd-cli run --record         Record the run to a video file
-  npx twd-cli run --shard 2/4      (beta) Run only this shard's slice of the
-                                   suite and write a report to ./.twd/run
-  npx twd-cli merge <dir>          (beta) Merge shard reports from <dir> into
-                                   one report, exit 1 if the run failed
-
-Examples:
-  npx twd-cli run --test "shows error"
-  npx twd-cli run --test "Login" --test "Signup"
-  npx twd-cli run --shard 2/4
-  npx twd-cli run --record --changed-since origin/main
-  npx twd-cli merge .twd/shards
-
-Options:
-  --test "<name>"        Filter tests by "suite > test" path (repeatable, OR'd)
-  --changed-since <ref>  Run only the tests this branch added or changed since
-                         <ref>, worked out from git. Unions with --test. A
-                         branch that changed no tests prints one line and
-                         exits 0 — an empty result is not a failure. Needs the
-                         base branch in the clone: in GitHub Actions set
-                         fetch-depth: 0 on actions/checkout.
-  --shard <i>/<n>        (beta) Run slice i of n. Each shard discovers the
-                         whole suite and takes every nth test, so the count
-                         never has to be known in advance. Implies a report.
-                         Which tests land in which shard may change.
-  --report-dir <path>    Where to write the shard report (default ./.twd/run)
-  --out <path>           merge only: where to write the merged report
-                         (default ./.twd/merged-run.json)
-  --record               Record the run to a video file (requires ffmpeg)
-  --record-dir <path>    Output directory (default ./twd-artifacts)
-  --record-speed <n>     Playback speed, e.g. 0.5 for half speed
-  --record-pace <ms>     Slow the run itself (default 300). 0 disables pacing
-
-  These three only set values. Recording still has to be turned on with
-  --record or "record": { "enabled": true } in twd.config.json.
-
-  Create a twd.config.json file in your project root to customize settings.
-  `);
-  process.exit(command ? 1 : 0);
+  // A command we do not know is a usage error, so it belongs on stderr with
+  // exit 1. Printing it on stdout, as this used to, let a script mistake the
+  // usage block for a successful run's output.
+  console.error(`twd-cli: unknown command '${command}'`);
+  console.error(globalUsage());
+  process.exitCode = 1;
 }

src/parseArgs.js

@@ -1,5 +1,22 @@
 import { parseShardSpec } from './shard.js';
 
+// Every flag each parser recognises. src/usage.js has to describe all of
+// them, and tests/usage.test.js checks that it does.
+export const RUN_FLAGS = [
+  '--test',
+  '--changed-since',
+  '--shard',
+  '--report-dir',
+  '--update-snapshots',
+  '--ci',
+  '--record',
+  '--record-dir',
+  '--record-speed',
+  '--record-pace',
+];
+
+export const MERGE_FLAGS = ['--out'];
+
 // Reads a flag's value in either `--flag value` or `--flag=value` form, and
 // reports how many tokens it consumed. Shared by both parsers.
 function readValue(argv, token, prefix, index) {
@@ -14,6 +31,63 @@ function readValue(argv, token, prefix, index) {
   return { value: token.slice(prefix.length + 1), consumed: 1 };
 }
 
+// Levenshtein distance. Inputs are flag names, so the plain O(n*m) table.
+function editDistance(a, b) {
+  let prev = Array.from({ length: b.length + 1 }, (_, j) => j);
+  for (let i = 1; i <= a.length; i++) {
+    const curr = [i];
+    for (let j = 1; j <= b.length; j++) {
+      curr[j] = Math.min(
+        prev[j] + 1,
+        curr[j - 1] + 1,
+        prev[j - 1] + (a[i - 1] === b[j - 1] ? 0 : 1),
+      );
+    }
+    prev = curr;
+  }
+  return prev[b.length];
+}
+
+// The known flag a typo most likely meant, or null. A prefix relation wins
+// (`--output` → `--out`, `--test-filter` → `--test`), longest match first;
+// otherwise the nearest flag within two edits (`--tests`, `--changed_since`).
+// A wrong pick costs nothing, since the error already names the offending
+// token, but `--out` must not turn into "Did you mean --ci?" — hence the
+// tight edit budget rather than a generous one.
+function closestFlag(name, known) {
+  if (name.length >= 4) {
+    const related = known
+      .filter((flag) => flag.startsWith(name) || name.startsWith(flag))
+      .sort((a, b) => b.length - a.length);
+    if (related.length) return related[0];
+  }
+  let best = null;
+  let bestDistance = Infinity;
+  for (const flag of known) {
+    const distance = editDistance(name, flag);
+    if (distance < bestDistance) {
+      best = flag;
+      bestDistance = distance;
+    }
+  }
+  return bestDistance <= 2 ? best : null;
+}
+
+// Every `--`-prefixed token no branch claimed ends up here, and the parser
+// throws rather than run. This is what makes --help reliable instead of
+// cosmetic: it used to be that `run --help` ran the whole suite because the
+// unknown token was dropped without a word, and a typo like `--tests` still
+// does the same today without this. The `=value` half is stripped so the
+// message names the flag the caller typed, not the value they gave it.
+function unknownOptionsError(command, tokens, known) {
+  const names = tokens.map((token) => token.split('=')[0]);
+  const suggestions = [...new Set(names.map((name) => closestFlag(name, known)).filter(Boolean))];
+  const lines = [`twd-cli ${command}: unknown option ${names.join(', ')}`, ''];
+  if (suggestions.length) lines.push(`Did you mean ${suggestions.join(', ')}?`);
+  lines.push(`Run \`twd-cli ${command} --help\` to see every option.`);
+  return new Error(lines.join('\n'));
+}
+
 export function parseRunArgs(argv) {
   const testFilters = [];
   const record = {};
@@ -26,6 +100,7 @@ export function parseRunArgs(argv) {
   // decided in twd-js, which is the only side that has fetched the reference.
   let updateSnapshots = false;
   let ci = false;
+  const unknown = [];
 
   for (let i = 0; i < argv.length; i++) {
     const token = argv[i];
@@ -76,17 +151,22 @@ export function parseRunArgs(argv) {
         record.pace = parsed;
       }
       i += consumed - 1;
+    } else if (token.startsWith('--')) {
+      unknown.push(token);
     }
   }
 
+  if (unknown.length) throw unknownOptionsError('run', unknown, RUN_FLAGS);
+
   return { testFilters, changedSince, record, shard, reportDir, updateSnapshots, ci };
 }
 
 // `twd-cli merge <dir> [--out <path>]`. The directory is the first positional
-// token; anything after the first is ignored.
+// token; further positionals are ignored, `--`-prefixed strays are refused.
 export function parseMergeArgs(argv) {
   let dir = null;
   let out = null;
+  const unknown = [];
 
   for (let i = 0; i < argv.length; i++) {
     const token = argv[i];
@@ -95,10 +175,14 @@ export function parseMergeArgs(argv) {
       const { value, consumed } = readValue(argv, token, '--out', i);
       if (value !== undefined) out = value;
       i += consumed - 1;
-    } else if (!token.startsWith('--') && dir === null) {
+    } else if (token.startsWith('--')) {
+      unknown.push(token);
+    } else if (dir === null) {
       dir = token;
     }
   }
 
+  if (unknown.length) throw unknownOptionsError('merge', unknown, MERGE_FLAGS);
+
   return { dir, out };
 }