Skip to content

fix: 存在しないサブコマンドへの --help をエラー扱いにして exit 1 を返す#147

Merged
miya0001 merged 5 commits into
mainfrom
fix/unknown-command-help
Jul 15, 2026
Merged

fix: 存在しないサブコマンドへの --help をエラー扱いにして exit 1 を返す#147
miya0001 merged 5 commits into
mainfrom
fix/unknown-command-help

Conversation

@miya0001

@miya0001 miya0001 commented Jul 15, 2026

Copy link
Copy Markdown
Member

概要

geonic hello --help のような存在しないサブコマンドに対するヘルプ実行が、トップレベルのヘルプを表示して exit 0 で成功扱いになっていた。エラーメッセージを stderr に出力し、exit 1 を返すようにする。

原因

Commander は --help/-h を検出すると、残りの operand が既知のサブコマンドか検証する前に _outputHelpIfRequested でヘルプを表示して exit 0 する。ルート・グループ配下(geonic entities hello --help)の両方で発生していた。

実装

  • enforceKnownCommandHelp() を追加し、サブコマンドを持つ全コマンドの outputHelp をラップ。未知のサブコマンド名が operand に残っている場合は geonic help <unknown> と同じエラー形式で exit 1
  • operand 判定は cmd.args先頭トークンのみ(commander は operands を unknown オプションより前に並べるため、未知オプションの値を誤ってサブコマンド名と判定しない)
  • エラーメッセージはユーザーが入力したエイリアスをそのまま echo(models badsub'models badsub'。正規名 custom-data-models に置換しない)
  • unknown-command エラー出力を unknownCommandError() に統合し showHelp と共有

挙動

コマンド Before After
geonic hello --help トップレベルヘルプ / exit 0 Error: 'hello' is not a geonic command. / exit 1
geonic entities hello --help entities ヘルプ / exit 0 Error: 'entities hello' ... / exit 1
geonic models badsub --help models ヘルプ / exit 0 Error: 'models badsub' ... / exit 1
geonic entities --bogus json --help entities ヘルプ / exit 0 変更なし(値 "json" をコマンド名と誤認しない)
geonic --help / entities --help 等の正常系 ヘルプ / exit 0 変更なし

テスト

  • 単体テスト 7 件追加(--help/-h/ネスト/エイリアス echo/オプション値誤認防止/正常系リーフ/--help 先行)
  • lint / typecheck / unit 770 件パス

Summary by CodeRabbit

  • バグ修正
    • 存在しないサブコマンドで --help または -h を指定した場合、ヘルプ表示のみで成功終了していた挙動を修正し、エラーメッセージ出力のうえ終了コード 1 に変更。
    • 入力したコマンド名やエイリアスをエラーメッセージに適切に反映。
  • テスト
    • ヘルプ表示時の未知コマンド挙動を検証するテストを整理・追加。
  • ドキュメント
    • 未リリースの変更履歴に更新内容を記載。

miya0001 added 2 commits July 15, 2026 15:21
Commander は --help/-h を検出すると、残りの operand が既知のサブコマンドか
検証せずにヘルプを表示して exit 0 していた (例: geonic hello --help)。
サブコマンドを持つコマンドの outputHelp をラップし、未解決の operand が
残っている場合は 'geonic help <unknown>' と同じエラー形式で exit 1 にする。
- operand 判定を cmd.args 先頭トークンに限定し、未知オプションの値を
  サブコマンド名と誤認しないようにする (--bogus json --help 等)。
  副作用として --help が先行する場合 (geonic --help hello) は
  従来どおりヘルプ表示に戻る
- エラーメッセージはユーザーが入力したエイリアス (models, batch) を
  そのまま echo し、正規名に置換しない (rawArgs から復元)
- unknown-command エラー出力を unknownCommandError() に統合し、
  showHelp との重複を解消
- テストの spy ボイラープレートを runExpectingError/runExpectingHelp に
  集約し、既存テストも移行
@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: cc4de334-b977-4567-ae5c-3cb54322c161

📥 Commits

Reviewing files that changed from the base of the PR and between 4a98dbd and 4fbde3e.

📒 Files selected for processing (1)
  • tests/help.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/help.test.ts

📝 Walkthrough

Walkthrough

未知サブコマンドに対する --help の処理を見直し、stderr 出力と exit 1 によるエラー扱いへ統一しました。入力エイリアスの保持、未知オプション値の判定、ネストしたコマンドの検証を追加し、関連テストと変更履歴を更新しています。

Changes

未知サブコマンドのヘルプ処理

Layer / File(s) Summary
未知コマンドエラーの共通化
src/commands/help.ts
未知コマンドのメッセージ出力と process.exit(1) を共通関数へ集約し、既存の検出処理から利用するよう変更しました。
ヘルプ表示時の未知コマンド検証
src/commands/help.ts, src/cli.ts
入力されたコマンド名・エイリアスを記録し、outputHelp 実行時に未解決オペランドを検出する処理を全サブコマンドへ適用しました。
ヘルプ挙動の回帰検証
tests/help.test.ts, CHANGELOG.md
未知コマンド、エイリアス、オプション値、ヘルプ位置、空文字入力をテストし、変更内容を変更履歴へ記録しました。

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant createProgram
  participant enforceKnownCommandHelp
  participant Commander
  participant unknownCommandError
  createProgram->>enforceKnownCommandHelp: プログラムへ検証処理を登録
  createProgram->>Commander: argv を解析して --help を処理
  Commander->>enforceKnownCommandHelp: 未解決オペランドを確認
  enforceKnownCommandHelp->>unknownCommandError: コマンドパスを渡してエラー終了
Loading

Poem

ぴょんと跳ねればヘルプが咲く
知らない名には赤い鐘
エイリアスも耳を澄まし
stderr へ月影を運ぶ
exit one で道を示す
うさぎはにっこり、検証完了 🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 変更内容の主旨である、存在しないサブコマンドの --help をエラー扱いにして exit 1 にする点を正確に表しています。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/unknown-command-help

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed: private package registry requires authentication. Disable ESLint in CodeRabbit settings or use public packages.


Comment @coderabbitai help to get the list of available commands.

miya0001 added 2 commits July 15, 2026 15:22
rawArgs の文字列スキャンは、グローバルオプション (--service 等) の値が
コマンド名/エイリアスと衝突すると誤ったトークンを echo していた
(例: geonic --service custom-data-models models badsub --help が
'custom-data-models badsub' と表示)。

preSubcommand フックで dispatch 時に commander 自身が解決した operand
(parent.args[0]) を WeakMap に記録する方式へ変更。オプション値は
parseOptions で消費済みのため衝突しない。あわせて:

- 空文字 operand を unknown 扱いしない (truthiness ガード)
- 回帰テスト 3 件追加 (オプション値衝突 / サブグループで --help 先行 /
  空文字 operand)

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
tests/help.test.ts (1)

21-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

spy の復元を finally で保証してください。

parse や assertion が失敗すると mockRestore() に到達せず、後続テストの stdout、stderr、process.exit が汚染されます。両ヘルパーの復元処理を try/finally に移してください。

Also applies to: 697-711

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/help.test.ts` around lines 21 - 37, Update runExpectingError and the
other helper at the referenced location so console.error and process.exit spies
are always restored in a finally block. Keep parsing, output collection, and
assertions within the protected try section, ensuring restoration occurs even
when parsing or an assertion fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/help.test.ts`:
- Around line 21-37: Update runExpectingError and the other helper at the
referenced location so console.error and process.exit spies are always restored
in a finally block. Keep parsing, output collection, and assertions within the
protected try section, ensuring restoration occurs even when parsing or an
assertion fails.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 86b5e5a6-9563-4176-a201-3dbef35b1e24

📥 Commits

Reviewing files that changed from the base of the PR and between b565b47 and 4a98dbd.

📒 Files selected for processing (4)
  • CHANGELOG.md
  • src/cli.ts
  • src/commands/help.ts
  • tests/help.test.ts

@miya0001

Copy link
Copy Markdown
Member Author

@coderabbitai 対応しました (4fbde3e)。runExpectingError / runExpectingHelp の両ヘルパーで、parse・出力収集・assertion を try 節に、spy の復元 (mockRestore) を finally 節に移し、assertion 失敗時でも console.error / process.exit / process.stdout.write の spy が必ず復元されるようにしました。

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
git log --oneline -5
git show 4fbde3e --stat
git show 4fbde3e -- tests/help.test.ts | head -150

Length of output: 2969


Confirmed. The commit 4fbde3e correctly moves the spy restoration into finally blocks for both runExpectingError and runExpectingHelp, so errSpy/exitSpy/writeSpy are now always restored even if an assertion inside try throws. This resolves the leak I flagged.

Thanks for the quick fix!

@miya0001
miya0001 merged commit d75b0bd into main Jul 15, 2026
7 checks passed
@miya0001
miya0001 deleted the fix/unknown-command-help branch July 15, 2026 07:00
miya0001 added a commit that referenced this pull request Jul 15, 2026
* test: models.feature の @wip を解除し一意制約の E2E シナリオを追加 (geonicdb#1268 追従)

- @wip の理由「モデル作成に tenantId が必要」は e2e hooks の
  e2e_test テナント + tenant_admin 整備で解消済みのため全シナリオを有効化
- get/delete シナリオの ID 指定を Mongo id → type に修正
  (API はモデルを type でキーする)
- 新規シナリオ: 制約宣言 → models get table の可読表示 →
  重複エンティティ exit 1 + 違反制約名 + ヒント → [] で全削除後に重複可
- devDependency geonicdb を一意制約実装済み main (913ceecf) に更新

* docs: CHANGELOG の PR 番号を修正 (#147#148)
@miya0001 miya0001 mentioned this pull request Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant