/meta-reviewing-cli-reviewing

name: meta-reviewing-cli-reviewing
description: CLI code review — exit codes, cancellation and signals, non-TTY behaviour, the stdout/stderr split, error text, config precedence, subprocess safety and test adequacy. Load when reviewing a command-line tool's commands, entry point, prompts, output or tests, whichever parser or prompt library it uses.

CLI Review Patterns

> **Quick Guide:** A CLI's contract is its exit code, its two output streams and what it does without a terminal — scripts and CI branch on those and never read the wording. Review every terminating path for the code it produces, every prompt for its cancel path and its non-interactive twin, and every write for the stream it lands on. Three Node facts decide verdicts: `process.exit()` abandons writes still pending to a pipe, so piped output arrives truncated; installing a `SIGINT` listener removes Node's default exit with 130; and while a prompt holds the terminal in raw mode, Ctrl+C is a keypress rather than a signal.

**Detailed Resources:**

• [examples/core.md](examples/core.md) — before-and-after code for each pattern, a spawned contract test, and a worked review
• [reference.md](reference.md) — review checklists by area, exit-code conventions, the shapes a prompt cancel takes

---

<critical_requirements>

Before reviewing CLI code

**Trace every terminating path to the exit code it produces.** Scripts and CI branch on the number, not the message, so a failure that exits 0 — a catch block that only logs, a cancelled prompt that returns — reports success to every `&&` chained after it.

**Find out what the prompt library does on Ctrl+C, then check that path at every prompt.** A prompt holds the terminal in raw mode, where Ctrl+C is a keypress rather than `SIGINT`, so no signal handler sees it; the library returns a cancel value, rejects, or drops the field, and an unchecked call carries that into the next line as data.

**Check that every prompted value has a flag or config twin, and that a prompt reached without a TTY fails with a usage error naming it.** CI has no terminal on stdin, so an unguarded prompt waits for input that never arrives, or a component that needs raw mode throws.

**Check which stream every write lands on.** stdout is the payload a pipe consumer parses; progress, warnings and errors belong on stderr, and spinners and colour only where their stream is a TTY — one spinner frame on stdout corrupts the consumer's input.

**Check that every spinner is stopped before anything else is written to the terminal, on the error path as well as on success.** A spinner redraws its line on a timer, so a log line or error written while it animates can be overwritten or split by its frames, and the message the user needed arrives garbled.

**Check that the entry point awaits async command handlers inside one top-level handler that turns any failure into a message and an exit code.** A handler the parser does not await rejects outside that path, so Node prints a raw stack trace and exits 1 whatever the failure was.

</critical_requirements>

---

**Auto-detection:** review CLI, CLI PR review, command-line tool review, exit code, process.exitCode, process.exit, SIGINT, SIGTERM, Ctrl+C handling, process.stdin.isTTY, process.stdout.isTTY, setRawMode, NO_COLOR, stdout vs stderr, --json output, --no-input, --dry-run, child_process, execFile, shell injection, config precedence, CLI tests

**Applies to:**

• Reviewing a command, the entry point, or how commands are dispatched
• Reviewing prompts, confirmations and key-driven terminal UI
• Reviewing output for piped, CI and `--json` callers
• Reviewing config resolution, error text and help
• Reviewing code that spawns processes or handles credentials
• Judging whether a CLI's tests reach its contracts

**Handled elsewhere:**

• Severity definitions, feedback style and the approve-or-request-changes decision — the general review process owns those; this skill says only where CLI defects land
• Choosing the contract — which flags, exit codes and output modes a feature should have is decided when it is specified, and review checks the code against that decision
• A given parser's, prompt library's or terminal renderer's API — this skill names the behaviour to check, and the library's own documentation names the call that provides it

---

<philosophy>

Philosophy

A CLI has three callers. A person at a terminal sees colour, spinners and prompts. A script piping the output sees stdout and the exit code, and nothing else. CI has no terminal at all: no prompt can be answered and no keypress arrives. Code that works when its author runs it by hand fails the other two silently, which is why these checks are about codes, streams and TTYs before wording.

Weigh findings by caller. A defect a script or CI can observe blocks, and so does output garbled on the terminal; one that otherwise costs only a person at a terminal is a Should Fix that travels with the approval; help wording and colour choices that match the existing commands are not findings. Don't require a spinner for an operation under 500 ms, or a verbose mode in a CLI simple enough to need none.

</philosophy>

---

<decision_framework>

Which checks a diff needs

• **The entry point or dispatch** — Patterns 1 and 6
• **A command's action** — Patterns 1, 5 and 7
• **Prompts, confirmations or key-driven UI** — Patterns 2 and 4
• **Long-running work, or anything that writes files or spawns children** — Patterns 3 and 9
• **Logging, progress or a `--json` mode** — Pattern 5
• **Config loading** — Pattern 8
• **Tests** — Pattern 10, against the contracts the other patterns name

Severity for CLI findings

Does it expose the user — input reaching a shell, a secret in ps output, history or logs?
├─ YES → MUST FIX
└─ NO → Can a script or CI observe it — wrong exit code, polluted or truncated stdout, a hang?
    ├─ YES → MUST FIX
    │   ├─ A failure or cancel that exits 0, or an exit code written a