meta-reviewing-cli-reviewing

CLI コードレビューパターン

クイックガイド： CLI コードをレビューするときは、SIGINT の処理、p.isCancel() のチェック、終了コードの定数、parseAsync() の使用、ユーザーへのフィードバック（スピナー、明確なエラー）を確認します。設定の階層、ヘルプテキストの品質、ドライランへの対応も確認してください。重要度（必須修正／修正推奨／あると望ましい）を区別し、各問題が重要な理由を説明します。

---

<critical_requirements>

重大：CLI コードをレビューする前に

すべてのコードはプロジェクトの規約に従う必要があります（CLAUDE.md の kebab-case、名前付きエクスポート、インポート順序、import type、名前付き定数）

（CLI のエントリーポイントに SIGINT（Ctrl+C）の処理があることを必ず確認してください）

（@clack/prompts の呼び出し後には必ず毎回 p.isCancel() が呼び出されていることを確認してください）

（終了コードに名前付き定数が使われていることを必ず確認してください。process.exit() 内のマジックナンバーはすべて指摘してください）

（非同期アクションには parse() ではなく parseAsync() が使われていることを必ず確認してください）

（コンソールへの出力やエラー処理の前にスピナーが停止されていることを必ず確認してください）

</critical_requirements>

---

自動検出： CLI のレビュー、CLI コードのチェック、CLI PR のレビュー、Commander.js のレビュー、@clack/prompts のレビュー、CLI の品質、CLI のエラー処理のレビュー、終了コードのレビュー

使用する場面：

• Commander.js で構築された CLI アプリケーションをレビューする場合
• @clack/prompts を使用した対話型プロンプトをレビューする場合
• CLI のエラー処理と終了コードのパターンを確認する場合
• CLI のユーザーエクスペリエンス（ヘルプテキスト、スピナー、フィードバック）を評価する場合
• CLI のテストが十分か確認する場合
• 設定管理のパターンをレビューする場合

使用しない場面：

• CLI コードを実装する場合（該当する CLI 実装スキルを使用してください）
• CLI 固有の懸念事項に関係しない一般的なコードレビューの場合
• バックエンド API のレビューの場合

対象となる主なパターン：

• CLI 固有のレビュー用チェックリスト
• 終了コードとシグナル処理の確認
• ユーザーエクスペリエンスのレビュー基準
• エラーメッセージの品質評価
• テストの十分性に関するチェックリスト
• 設定の階層のレビュー
• コマンドの構造と整理のレビュー
• CLI の問題の重要度分類

詳細なリソース：

• examples/core.md - レビュー出力形式の例、CLI テストのレビューパターン

---

基本方針

CLI の UX は重要です。 視覚的なフィードバックがあるウェブアプリとは異なり、CLI ツールはすべてテキストで情報を伝えます。わかりにくいエラーメッセージ、進捗インジケーターの欠如、予期しない終了は、ユーザーの信頼を損ないます。エンドユーザーの立場に寄り添ってレビューしてください。

CLI コードをレビューするときは：

• process.exit() に至るすべての経路で名前付き定数が使われていることを確認する
• すべての非同期処理に視覚的なフィードバック（スピナー）があることを確認する
• あらゆる場所でキャンセルが適切に処理されるようにする
• エラーメッセージに、何が失敗したのか、どうすれば解決できるのかが説明されていることを確認する
• ヘルプテキストが役立ち、例が含まれていることを確認する

厳しく指摘しない場面：

• 機能が正しく動作しているなら、ヘルプテキストの表現を理由に PR をブロックしない
• 500ms 未満で完了する処理にスピナーを必須としない
• 既存のパターンに沿っているなら、色の選択を細かく指摘しない
• CLI が十分にシンプルなら、詳細モードを要求しない

基本原則：

• 安全性を最優先： 終了コードとシグナル処理は必須です
• ユーザーへの共感： すべてのエラーで、ユーザーが解決できるよう案内する必要があります
• 一貫性： すべてのコマンドで同じパターンに従う必要があります
• テスト容易性： CLI コードはプロセスを起動せずにテストできる必要があります

---

主なパターン

パターン 1：CLI レビューチェックリスト

CLI コードレビューでは、毎回この包括的なチェックリストを使用してください。

エントリーポイントの確認

## 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

重要な理由： エントリーポイントの問題はすべてのコマンドに影響します。SIGINT の処理がないとユーザーはキャンセルできず、parseAsync がないとエラーが何も通知されないまま握りつぶされます。

---

パターン 2：終了コードのレビュー

すべての終了経路で名前付き定数が使われていることを確認してください。

終了コード確認チェックリスト

## 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);

重要な理由： マジックナンバーの終了コードは保守が困難です。CLI に依存するスクリプトには、予測可能で文書化された終了コードが必要です。

---

パターン 3：プロンプトのキャンセル処理レビュー

すべての @clack/prompts 呼び出しで、キャンセルを確認する必要があります。

キャンセル処理の監査

// 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);
}

レビューチェックリスト：

## 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

重要な理由： isCancel チェックが欠けていると、ユーザーが Ctrl+C を押したときに未定義の動作が発生します。コードは期待される文字列やブール値ではなく、Symbol 値を使って処理を続行します。

---

パターン 4：非同期処理のレビュー

すべての非同期処理について、スピナーとエラー処理を確認します。

スピナーの使用状況レビュー

// 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);
}

レビューチェックリスト：

## 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

重要な理由： ユーザーは処理中であることを把握する必要があります。何も表示されない処理は、壊れているように感じられます。

---

パターン 5：エラーメッセージの品質レビュー

エラーメッセージが具体的な対応につながるか評価します。

エラーメッセージの基準

## 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(`Config file not found: ${configPath}`);

// Good: Actionable error
p.log.error(`Config file not found: ${configPath}`);
p.log.info(`Run 'mycli init' to create one, or specify path with --config`);

重要な理由： 対応につながるエラーメッセージは、サポートの負担を軽減し、ユーザーエクスペリエンスを向上させます。ユーザーが行き詰まることがあってはなりません。

---

パターン 6：設定階層のレビュー

設定の解決が正しい優先順位に従っていることを確認します。

設定の優先順位チェックリスト

## Configuration Review

**Precedence Order (highest to lowest):**

1. [ ] CLI flags (--source, --config)
2. [ ] Environment variables (MYAPP_SOURCE)
3. [ ] Project config (.myapp/config.yaml in cwd)
4. [ ] Global config (~/.myapp/config.yaml)
5. [ ] Default values (hardcoded constants)

**Implementation:**

- [ ] resolveConfig function exists and follows precedence
- [ ] Empty flag values handled (--source "" should error or skip)
- [ ] Missing config files handled gracefully (no crash)
- [ ] Source/origin tracked for debugging (--verbose shows where value came from)

重要な理由： 設定の優先順位が正しくないと、ユーザーを混乱させます。環境変数がフラグより優先されるなら、それはバグです。

---

パターン 7：ヘルプテキストのレビュー

コマンドのドキュメントの品質を評価します。

ヘルプテキストのチェックリスト

## Help Text Review

**Command Documentation:**

- [ ] Description is clear and concise
- [ ] All options have descriptions
- [ ] Required vs optional options clear
- [ ] Default values documented in option descriptions

**Examples Section:**

- [ ] Common use cases shown
- [ ] Examples are copy-paste ready
- [ ] Complex options demonstrated

**Consistency:**

- [ ] Naming follows conventions (--dry-run not --dryRun)
- [ ] Short flags used for common options (-f for --force)
- [ ] Help shown after errors (showHelpAfterError enabled)

---

パターン 8：テストの十分性レビュー

CLI テストで重要なパスが網羅されていることを確認します。

CLI テストのチェックリスト

## Testing Review

**Command Testing:**

- [ ] Happy path tested for each command
- [ ] Invalid arguments tested (missing required, unknown options)
- [ ] --help output tested
- [ ] exitOverride() used to prevent process.exit in tests

**Prompt Testing:**

- [ ] @clack/prompts mocked properly
- [ ] Cancellation flow tested (isCancel returns true)
- [ ] Validation rejection tested
- [ ] Multiple selection paths tested

**File System Testing:**

- [ ] In-memory filesystem used for isolated tests
- [ ] Config loading tested (missing, invalid, valid)
- [ ] File write operations tested

**Exit Code Testing:**

- [ ] Success exits with 0
- [ ] Each error type returns correct non-zero code
- [ ] Cancellation exits with CANCELLED code

レビュー中に確認するテストコードのパターンについては、examples/core.md を参照してください。

---

パターン 9：コマンド構成のレビュー

コマンドの構成が論理的であることを確認します。

構成チェックリスト

## Command Structure Review

**Organization:**

- [ ] Related commands grouped as subcommands (config show, config set)
- [ ] Each command in separate file
- [ ] No god commands (> 200 lines)
- [ ] Shared utilities extracted to lib/

**Options:**

- [ ] Global options defined on parent (--verbose, --dry-run)
- [ ] optsWithGlobals() used to access parent options
- [ ] Option names consistent across commands

**Arguments:**

- [ ] Positional arguments have clear names
- [ ] Required vs optional arguments documented
- [ ] Argument validation happens early in action

---

パターン 10：セキュリティレビュー

CLI 固有のセキュリティ上の懸念がないか確認します。

セキュリティチェックリスト

## CLI Security Review

**Input Validation:**

- [ ] No shell injection (user input not concatenated into shell commands)
- [ ] File paths validated before operations
- [ ] URLs validated before fetch

**Secrets Handling:**

- [ ] API keys/tokens not logged (even in verbose mode)
- [ ] Sensitive options not shown in help output
- [ ] Config files with secrets have appropriate permissions warning

**Dependency Injection:**

- [ ] Arguments not passed directly to child_process
- [ ] execa or similar used instead of exec when needed

---

<decision_framework>

判断フレームワーク

CLI の問題の重大度分類

Is this a safety/correctness issue?
├─ Missing SIGINT handler → MUST FIX
├─ Missing p.isCancel() check → MUST FIX
├─ Magic number exit code → MUST FIX
├─ parse() instead of parseAsync() → MUST FIX
├─ Missing error handling on async → MUST FIX
└─ NO → Is it a user experience issue?
    ├─ Missing spinner for >500ms operation → SHOULD FIX
    ├─ Unhelpful error message → SHOULD FIX
    ├─ Incorrect config precedence → SHOULD FIX
    ├─ Missing --help descriptions → SHOULD FIX
    └─ NO → Is it an enhancement?
        ├─ Could add --json output → NICE TO HAVE
        ├─ Could add more examples in help → NICE TO HAVE
        ├─ Could improve verbose logging → NICE TO HAVE
        └─ Style preference → DON'T MENTION

承認の判断フレームワーク

承認する場合：

• すべての SIGINT／キャンセル処理を確認済み
• すべての終了コードで名前付き定数を使用
• 非同期コマンドで parseAsync() を使用
• 非同期処理にエラー処理がある
• 重要なパスをテストで網羅している

変更を依頼する場合：

• p.isCancel() のチェックが欠けている（いずれかのプロンプト）
• process.exit() にマジックナンバーを使用
• 非同期アクションで parse() を使用
• 時間のかかる処理にスピナーがない
• 役に立たないエラーメッセージ

大幅な修正が必要な場合：

• エントリーポイントに SIGINT ハンドラーがない
• キャンセル処理が体系的に欠けている
• 終了コードの定数が定義されていない
• エラー処理のパターンが確立されていない
• セキュリティ脆弱性（シェルインジェクション）

</decision_framework>

---

<red_flags>

要注意事項

優先度の高い問題（必ず修正してください）：

• エントリーポイントに process.on("SIGINT", ...) がない
• プロンプト呼び出しの後に p.isCancel() を呼び出していない
• 名前付き定数ではなく、process.exit(1) または process.exit(0) を使用している
• 非同期アクションがあるのに、program.parseAsync() ではなく program.parse() を使用している
• エラーをログに出す前にスピナーを停止していない（出力が乱れる）
• シェルインジェクションの脆弱性がある（ユーザー入力を exec / spawn に渡している）

優先度が中程度の問題（修正することを推奨します）：

• 500msを超える可能性がある処理にスピナーがない
• エラーメッセージに失敗した内容は書かれているが、修正方法が書かれていない
• 破壊的な操作に --dry-run のサポートがない
• デバッグ用の詳細モードがない
• 設定の優先順位が正しくない（環境変数がフラグより優先される）
• プロンプトでのユーザー入力の検証がない
• showHelpAfterError(true) が設定されていない

よくある間違い：

• p.cancel() の後に process.exit() を呼び出し忘れる
• 親コマンドのオプションにアクセスするために optsWithGlobals() を使用していない
• スピナーを停止する前にログを出力している（出力が乱れる）
• フラグの値が空文字列の場合に対応していない
• YAML の解析エラーを捕捉していない
• .action() コールバック内の非同期エラーが握りつぶされる

注意点とエッジケース：

• Commander はオプションオブジェクト内で --my-option を myOption に自動変換します
• エラー経路でも Spinner.stop() を呼び出す必要があります
• p.isCancel() は undefined ではなく、Symbol 値に対して true を返します
• 非同期コンテキストでの process.exit() は、保留中の I/O の完了を待たない場合があります
• CI 環境では色が表示されない場合があります（NO_COLOR 環境変数を確認してください）
• 設定ファイルが存在していても、無効な YAML の場合があります

</red_flags>

---

<critical_reminders>

重要な確認事項

すべてのコードは CLAUDE.md のプロジェクト規約に従う必要があります

（CLI のエントリーポイントに SIGINT（Ctrl+C）の処理があることを必ず確認してください）

（すべての @clack/prompts 呼び出しの後に p.isCancel() が呼び出されていることを必ず確認してください）

（終了コードに名前付き定数が使われていることを必ず確認し、process.exit() 内のマジックナンバーをすべて指摘してください）

（非同期アクションには parse() ではなく parseAsync() が使われていることを必ず確認してください）

（コンソール出力やエラー処理を行う前にスピナーが停止されていることを必ず確認してください）

これらの問題を見逃すと、Ctrl+C でクラッシュしたり、終了コードが文書化されていなかったり、エラーが気づかれないまま握りつぶされたりする CLI になります。

</critical_reminders>