Use Commander for CLI parsing and hierarchical help

README.md

@@ -33,6 +33,16 @@ Commands return JSON envelopes an agent can branch on. Customer surface is
 content (`app`, `media`, `compose`), playlists, and screens. Full reference:
 [https://screenrig.ai/docs/cli.md](https://screenrig.ai/docs/cli.md).
 
+Discover commands progressively with `screenrig --help`, `screenrig screen --help`,
+and `screenrig screen assign --help`. Deeper groups work the same way:
+`screenrig comment show --help`. `screenrig help screen assign` is equivalent.
+Add `--json` for structured child command paths, invocation syntax, and option types;
+help runs without configuration or authentication. Command-specific options follow
+that command, for example `screenrig screen update ID --name Lobby --if-match 1`.
+Global options such as `--json` may appear before or after the command. Use
+`--name=VALUE` for a value starting with a dash, and `--` before option-like file
+names. Duplicate options are rejected.
+
 Choose by what the page is:
 
 - Already have the file: `media upload`, then a playlist and `screen assign`.
@@ -69,6 +79,11 @@ command.
 
 ## Develop
 
+Commander 14 owns argument parsing, command selection, and help rendering while
+supporting Node 20.11. The command tree feeds both human and JSON help. The adapter
+keeps parser diagnostics inside the CLI error envelope and only forwards explicit
+options to handlers, including the existing `no-*` boolean flags.
+
 ```sh
 npm ci
 npm run vendor:check

package.json

@@ -95,6 +95,7 @@
   "dependencies": {
     "@napi-rs/canvas": "^1.0.7",
     "ajv": "8.18.0",
-    "ajv-formats": "3.0.1"
+    "ajv-formats": "3.0.1",
+    "commander": "14.0.3"
   }
 }

src/adapters/protocol.test.ts

@@ -1,3 +1,4 @@
+import { commandHelp } from "../help.js";
 import assert from "node:assert/strict";
 import { readFileSync } from "node:fs";
 import { dirname, join } from "node:path";
@@ -859,12 +860,21 @@ test("opaque agent comments are optional on GET resources, dedicated comment rou
   assert.match(screenComments, /parameters: \[\{ \$ref: "#\/components\/parameters\/IdempotencyKey" \}\]/);
   assert.doesNotMatch(screenComments, /IfMatch/);
 
-  assert.match(commands, /comment show screen <id>/);
-  assert.match(commands, /comment show playlist <id> \[--page PAGE_ID\]/);
-  assert.match(commands, /comment set screen <id> \(--json-value JSON \| --file FILE\)/);
-  assert.match(commands, /comment set playlist <id> \[--page PAGE_ID\] \(--json-value JSON \| --file FILE\)/);
-  assert.match(commands, /comment delete screen <id>/);
-  assert.match(commands, /comment delete playlist <id> \[--page PAGE_ID\]/);
+  for (const action of ["show", "set", "delete"]) {
+    for (const target of ["screen", "playlist"]) {
+      const help = commandHelp(["comment", action, target]);
+      assert.match(help.synopsis[0]!, new RegExp(`comment ${action} ${target} \\[options\\] <id>`));
+      const options = new Map(help.options.map((option) => [option.name, option]));
+      assert.equal(options.has("--page"), target === "playlist");
+      if (target === "playlist") assert.equal(options.get("--page")?.type, "value");
+      for (const name of ["--json-value", "--file"]) {
+        assert.equal(options.has(name), action === "set");
+        if (action === "set") assert.equal(options.get(name)?.type, "value");
+      }
+      assert.equal(options.has("--if-match"), false);
+      assert.equal(options.has("--value-base64"), false);
+    }
+  }
   assert.doesNotMatch(commands, /comment\/screen\/:id/);
   assert.doesNotMatch(commands, /--value-base64.*comment/);
 });

src/argv.ts

@@ -1,47 +1,44 @@
-import { BOOLEAN_FLAGS, COMMAND_SPECS, GLOBAL_FLAGS } from "./command-spec.js";
+import { CommanderError } from "commander";
+import { commandPath, createCommandTree, invocationFlags } from "./command-tree.js";
+import { describeHelp, type HelpDocument } from "./help.js";
 import { usageError } from "./problems.js";
 
 export interface ParsedArgs {
   command: string[];
   flags: Record<string, string | boolean>;
   positionals: string[];
+  help?: HelpDocument;
 }
 
 export function parseArgv(argv: string[]): ParsedArgs {
-  const flags: Record<string, string | boolean> = Object.create(null) as Record<string, string | boolean>;
-  const positionals: string[] = [];
-  const known = new Set<string>([...GLOBAL_FLAGS, ...COMMAND_SPECS.flatMap((spec) => spec.flags)]);
-  for (let i = 0; i < argv.length; i += 1) {
-    const arg = argv[i]!;
-    if (arg === "--") {
-      positionals.push(...argv.slice(i + 1));
-      break;
-    }
-    if (!arg.startsWith("-") || arg === "-") {
-      positionals.push(arg);
-      continue;
-    }
-    const normalized = arg === "-h" ? "--help" : arg === "-V" ? "--version" : arg;
-    if (!normalized.startsWith("--")) throw usageError("Unsupported short option. Use --help to discover supported options.");
-    const eq = normalized.indexOf("=");
-    const name = normalized.slice(2, eq < 0 ? undefined : eq);
-    // Do not echo unknown names or values: either can contain credentials.
-    if (!known.has(name)) throw usageError("Unsupported option. Use --help to discover supported options.");
-    if (Object.hasOwn(flags, name)) throw usageError(`--${name} may be supplied only once.`);
-    if (BOOLEAN_FLAGS.has(name)) {
-      if (eq >= 0) throw usageError(`--${name} takes no value.`);
-      flags[name] = true;
-      continue;
-    }
-    const value = eq >= 0 ? normalized.slice(eq + 1) : argv[i + 1];
-    if (value === undefined || (eq < 0 && value.startsWith("-") && !/^-\d/.test(value))) {
-      throw usageError(`--${name} requires a value.`);
-    }
-    if (value.length === 0 && name !== "value-base64") throw usageError(`--${name} requires a value.`);
-    flags[name] = value;
-    if (eq < 0) i += 1;
+  // Commander handles tokenization, option scope, command paths, aliases,
+  // positional arity, and option conflicts. It must never exit this process.
+  const end = argv.indexOf("--");
+  const options = end < 0 ? argv : argv.slice(0, end);
+  for (const spellings of [["--help", "-h"], ["--version", "-V"]]) {
+    if (options.filter((arg) => spellings.includes(arg)).length > 1) throw usageError(`${spellings[0]} may be supplied only once.`);
   }
-  return { command: positionals.slice(0, 2), flags, positionals };
+  const tree = createCommandTree(options);
+  try {
+    tree.root.parse(argv, { from: "user" });
+  } catch (error) {
+    if (!(error instanceof CommanderError) || !(tree.helpRequested() || tree.versionRequested())) throw error;
+  }
+  const selected = tree.selected();
+  const flags = invocationFlags(selected);
+  if (tree.versionRequested()) flags.version = true;
+  if (tree.helpRequested()) {
+    flags.help = true;
+    const help = describeHelp(selected);
+    return { command: help.path.slice(0, 2), positionals: help.path, flags, help };
+  }
+  // These paired inputs are domain requirements, not parser syntax.
+  const has = (name: string) => Object.hasOwn(flags, name);
+  if (has("target-width") !== has("target-height")) throw usageError("Provide both --target-width and --target-height.");
+  const path = commandPath(selected);
+  if (path.join(" ") === "playlist import" && has("update") !== has("if-match")) throw usageError("playlist import --update and --if-match must be supplied together.");
+  const positionals = [...path, ...selected.args];
+  return { command: positionals.slice(0, 2), positionals, flags };
 }
 
 export function flagString(flags: Record<string, string | boolean>, name: string): string | undefined {

src/cli.test.ts

@@ -1,3 +1,4 @@
+import { commandHelp, CREDIT_HELP } from "./help.js";
 import assert from "node:assert/strict";
 import { createCipheriv, createHash, createPublicKey, diffieHellman, generateKeyPairSync, hkdfSync } from "node:crypto";
 import { PassThrough } from "node:stream";
@@ -6,7 +7,7 @@ import { mkdir, open, readFile, rename, chmod, stat, writeFile, rm } from "node:
 import path from "node:path";
 import { fileURLToPath } from "node:url";
 import { test } from "node:test";
-import { CLI_VERSION, EVENT_STREAM_BACKOFF_CAP_MS, EVENT_STREAM_BACKOFF_MS, USAGE, formatEventLine } from "./commands.js";
+import { CLI_VERSION, EVENT_STREAM_BACKOFF_CAP_MS, EVENT_STREAM_BACKOFF_MS, formatEventLine } from "./commands.js";
 import { run, type CliRuntime } from "./main.js";
 import { FakeTransport, memoryBackend } from "./transport/fake.js";
 import { ExitCode } from "./exit-codes.js";
@@ -3185,7 +3186,7 @@ test("feedback refuses a --command that could carry an argument value", async ()
       { fs: fsLike },
     );
     assert.equal(valueless.code, 2, valueless.stdout);
-    assert.match(JSON.parse(valueless.stdout).error.detail as string, /--command requires a value/);
+    assert.match(JSON.parse(valueless.stdout).error.detail as string, /--command requires a value|--json may be supplied only once/);
 
     // --no-context suppresses the diagnostic envelope entirely.
     const quiet = await withRuntime(
@@ -3589,7 +3590,7 @@ test("media upload warns on a low-information filename without blocking the uplo
 });
 
 test("customer-facing credit copy is fail-open until 1 Jan 2027", () => {
-  const documents: Array<[string, string]> = [["USAGE", USAGE]];
+  const documents: Array<[string, string]> = [["media generate help", CREDIT_HELP]];
   for (const [name, text] of documents) {
     assert.match(text, /nonnegative/, `${name} must say remaining is nonnegative`);
     assert.match(text, /credits_low/, `${name} must name the live credits_low warning`);
@@ -3611,7 +3612,7 @@ test("customer-facing credit copy is fail-open until 1 Jan 2027", () => {
     assert.doesNotMatch(text, /\$30 \/ 1M/, `${name} must not name the vendor image-output rate as the price`);
     assert.doesNotMatch(text, /OpenRouter/, `${name} must not name OpenRouter`);
   }
-  assert.match(USAGE, /feedback list \[--kind bug\|feature\]/);
+  assert.match(commandHelp(["feedback", "list"]).usage, /--kind <bug\|feature>/);
 
   // Maintainer AGENTS.md and README navigation do not duplicate the CLI help
   // rate card. Behavior and customer command help are validated above.
@@ -3942,7 +3943,7 @@ test("screen toast posts the closed write body and does not echo the text", asyn
 });
 
 test("screen toast defaults omitted --level to info", async () => {
-  assert.match(USAGE, /screen toast <id> --text TEXT \[--level info\] \[--duration-ms MS\]/);
+  assert.match(commandHelp(["screen", "toast"]).usage, /screen toast \[options\] <id>/);
   const transport = memoryBackend();
   const configDir = await testTemp("toast-default-level-");
   const fsLike = { mkdir, open, rename, rm, chmod, stat, homedir: () => configDir, env: { XDG_CONFIG_HOME: configDir } };
@@ -3991,7 +3992,7 @@ test("screen toast rejects invalid level, text, and duration before calling the
   try {
     for (const [argv, detail] of [
       [["screen", "toast"], /requires <id>/],
-      [["screen", "toast", "scr_1", "--level", "info"], /requires <id>/],
+      [["screen", "toast", "scr_1", "--level", "info"], /requires --text/],
       [["screen", "toast", "scr_1", "--level", "INFO", "--text", "Lobby closed"], /error, alert, or info/],
       [["screen", "toast", "scr_1", "--level", "warn", "--text", "Lobby closed"], /error, alert, or info/],
       [["screen", "toast", "scr_1", "--level", "info", "--text", "a\n\n\nb"], /1 to 120 characters/],
@@ -4023,7 +4024,7 @@ test("screen toast rejects invalid level, text, and duration before calling the
       { fs: fsLike },
     );
     assert.equal(valuelessText.code, ExitCode.Usage);
-    assert.match(JSON.parse(valuelessText.stdout).error.detail as string, /--text requires a value/);
+    assert.match(JSON.parse(valuelessText.stdout).error.detail as string, /--text requires a value|--json may be supplied only once/);
 
     const cancel = await withRuntime(["--json", "screen", "toast-cancel", "scr_1"], transport, { fs: fsLike });
     assert.equal(cancel.code, ExitCode.Usage);