CLI Code Review Patterns

Quick Guide: When reviewing CLI code, verify SIGINT handling, p.isCancel() checks, exit code constants, parseAsync() usage, and user feedback (spinners, clear errors). Check config hierarchy, help text quality, and dry-run support. Distinguish severity (Must Fix vs Should Fix vs Nice to Have) and explain WHY each issue matters.

<critical_requirements>

CRITICAL: Before Reviewing CLI Code

All code must follow project conventions in CLAUDE.md (kebab-case, named exports, import ordering, import type, named constants)

(You MUST verify SIGINT (Ctrl+C) handling exists in CLI entry point)

(You MUST verify p.isCancel() is called after EVERY @clack/prompts call)

(You MUST verify exit codes use named constants - flag ANY magic numbers in process.exit())

(You MUST verify parseAsync() is used for async actions, not parse())

(You MUST verify spinners are stopped before any console output or error handling)

</critical_requirements>

Auto-detection: review CLI, check CLI code, CLI PR review, Commander.js review, @clack/prompts review, CLI quality, CLI error handling review, exit codes review

When to use:

• Reviewing CLI applications built with Commander.js
• Reviewing interactive prompts using @clack/prompts
• Checking CLI error handling and exit code patterns
• Evaluating CLI user experience (help text, spinners, feedback)
• Verifying CLI testing adequacy
• Reviewing configuration management patterns

When NOT to use:

• When implementing CLI code (use the relevant CLI implementation skill)
• For general code review not specific to CLI concerns
• For backend API review

Key patterns covered:

• CLI-specific review checklist
• Exit code and signal handling verification
• User experience review criteria
• Error message quality assessment
• Testing adequacy checklist
• Configuration hierarchy review
• Command structure and organization review
• Severity classification for CLI issues

Detailed Resources:

• examples/core.md - Example review output format, CLI test review patterns

Philosophy

CLI UX is critical. Unlike web apps with visual feedback, CLI tools communicate entirely through text. Poor error messages, missing progress indicators, or unexpected exits destroy user trust. Review with empathy for the end user.

When reviewing CLI code:

• Verify all paths to process.exit() use named constants
• Check that every async operation has visual feedback (spinners)
• Ensure cancellation is handled gracefully everywhere
• Validate error messages explain WHAT failed and HOW to fix it
• Confirm help text is useful and examples are included

When NOT to be harsh:

• Don't block PRs for help text wording if functionality is correct
• Don't require spinners for operations under 500ms
• Don't nitpick color choices if they follow existing patterns
• Don't request verbose mode if the CLI is simple enough

Core principles:

• Safety First: Exit codes and signal handling are non-negotiable
• User Empathy: Every error should guide users to resolution
• Consistency: All commands should follow the same patterns
• Testability: CLI code should be testable without spawning processes

Core Patterns

Pattern 1: CLI Review Checklist

Use this comprehensive checklist for every CLI code review.

Entry Point Verification

## CLI Entry Point Review

**Signal Handling:**

- [ ] SIGINT handler exists in entry point
- [ ] SIGINT calls process.exit with EXIT_CODES.CANCELLED
- [ ] Other relevant signals handled (SIGTERM for containers)

**Command Registration:**

- [ ] Commands imported and registered cleanly
- [ ] parseAsync() used (not parse()) for async actions
- [ ] Global error handler with catch() on main()
- [ ] configureOutput() used for colored errors
- [ ] showHelpAfterError(true) enabled

**Global Options:**

- [ ] --dry-run supported for destructive operations
- [ ] --verbose supported for debug output
- [ ] --help generates useful output
- [ ] --version displays correct version

Why this matters: Entry point issues affect every command. Missing SIGINT handling leaves users unable to cancel, missing parseAsync swallows errors silently.

Pattern 2: Exit Code Review

Verify all exit paths use named constants.

Exit Code Verification Checklist

## Exit Codes Review

**Named Constants:**

- [ ] Exit codes defined as named constants (e.g., EXIT_CODES object)
- [ ] All exit codes have JSDoc descriptions
- [ ] Uses `as const` for type inference

**Usage Audit:**

- [ ] No magic numbers in process.exit() calls
- [ ] Correct exit code for each scenario:
  - Success: EXIT_CODES.SUCCESS (0)
  - General error: EXIT_CODES.ERROR (1)
  - Invalid args: EXIT_CODES.INVALID_ARGS (2)
  - User cancelled: EXIT_CODES.CANCELLED
  - Validation failed: EXIT_CODES.VALIDATION_ERROR

// Must Fix: Magic number exit code
process.exit(1); // What does 1 mean?

// Good: Named constant
process.exit(EXIT_CODES.VALIDATION_ERROR);

Why this matters: Magic exit codes are unmaintainable. Scripts that depend on your CLI need predictable, documented exit codes.

Pattern 3: Prompt Cancellation Review

Every @clack/prompts call must check for cancellation.

Cancellation Handling Audit

// Must Fix: Missing isCancel check
const name = await p.text({ message: "Name:" });
// User presses Ctrl+C - name is Symbol, code continues with garbage

// Good: Proper cancellation handling
const name = await p.text({ message: "Name:" });
if (p.isCancel(name)) {
  p.cancel("Setup cancelled");
  process.exit(EXIT_CODES.CANCELLED);
}

Review Checklist:

## Prompt Cancellation Review

For EACH @clack/prompts call (p.text, p.select, p.confirm, p.multiselect):

- [ ] p.isCancel() check immediately follows
- [ ] p.cancel() called with descriptive message
- [ ] process.exit() called with EXIT_CODES.CANCELLED
- [ ] No code executes after isCancel returns true

Why this matters: Missing isCancel checks cause undefined behavior when users press Ctrl+C. The code continues with a Symbol value instead of the expected string/boolean.

Pattern 4: Async Operation Review

Verify spinners and error handling for all async work.

Spinner Usage Review

// Must Fix: No feedback for long operation
const data = await fetchRemoteConfig(); // User sees nothing

// Good: Spinner with descriptive messages
const s = p.spinner();
s.start("Fetching configuration...");
try {
  const data = await fetchRemoteConfig();
  s.stop(`Loaded ${data.items.length} items`);
} catch (error) {
  s.stop("Failed to fetch configuration");
  p.log.error(error.message);
  process.exit(EXIT_CODES.NETWORK_ERROR);
}

Review Checklist:

## Async Operations Review

For EACH async operation (API calls, file operations, network):

- [ ] Spinner started with descriptive message
- [ ] Spinner stopped before any console output
- [ ] Spinner stopped before error logging
- [ ] Success message includes result info
- [ ] Error handling exists with appropriate exit code

Why this matters: Users need feedback that something is happening. Silent operations feel broken.

Pattern 5: Error Message Quality Review

Evaluate error messages for actionability.

Error Message Criteria

## Error Message Review

For EACH error path:

- [ ] Message explains WHAT failed (not just "Error")
- [ ] Message explains WHY it failed (permission, network, validation)
- [ ] Message suggests HOW to fix it (retry, check config, provide flag)
- [ ] Uses picocolors consistently (red for errors, yellow for warnings)
- [ ] Includes relevant context (file path, option name, command)

// Must Fix: Unhelpful error
p.log.error("Failed");

// Should Fix: Better but no resolution
p.log.error(`Confi