Skip to content

Commit d75b0bd

Browse files
authored
fix: 存在しないサブコマンドへの --help をエラー扱いにして exit 1 を返す (#147)
* fix: 存在しないサブコマンドへの --help をエラー扱いにして exit 1 を返す Commander は --help/-h を検出すると、残りの operand が既知のサブコマンドか 検証せずにヘルプを表示して exit 0 していた (例: geonic hello --help)。 サブコマンドを持つコマンドの outputHelp をラップし、未解決の operand が 残っている場合は 'geonic help <unknown>' と同じエラー形式で exit 1 にする。 * fix: --help エラー処理のレビュー指摘対応 - operand 判定を cmd.args 先頭トークンに限定し、未知オプションの値を サブコマンド名と誤認しないようにする (--bogus json --help 等)。 副作用として --help が先行する場合 (geonic --help hello) は 従来どおりヘルプ表示に戻る - エラーメッセージはユーザーが入力したエイリアス (models, batch) を そのまま echo し、正規名に置換しない (rawArgs から復元) - unknown-command エラー出力を unknownCommandError() に統合し、 showHelp との重複を解消 - テストの spy ボイラープレートを runExpectingError/runExpectingHelp に 集約し、既存テストも移行 * docs: CHANGELOG に #147 のエントリを追加 * fix: typed トークンの復元を rawArgs スキャンから preSubcommand フックに置換 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) * test: レビュー指摘対応 — テストヘルパーの spy 復元を try/finally で保証
1 parent b565b47 commit d75b0bd

4 files changed

Lines changed: 179 additions & 23 deletions

File tree

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@
77

88
## [Unreleased]
99

10+
### 2026-07-15
11+
- **Fix**: 存在しないサブコマンドへの `--help` (`geonic hello --help` 等) がヘルプ表示 + exit 0 で成功扱いになっていた問題を修正 — `geonic help <unknown>` と同じエラーを stderr に出力し exit 1 を返す (#147)
12+
- エラーメッセージはユーザーが入力したエイリアス表記をそのまま echo する(`models badsub``custom-data-models badsub` に置換しない)
13+
- 未知オプションの値はサブコマンド名と誤認しない(`geonic entities --bogus json --help` は従来通りヘルプ表示)
14+
1015
## [0.18.0] - 2026-07-15
1116

1217
### 2026-07-15

src/cli.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { registerRulesCommand } from "./commands/rules.js";
1515
import { registerModelsCommand } from "./commands/models.js";
1616
import { registerCatalogCommand } from "./commands/catalog.js";
1717
import { registerHealthCommand, registerVersionCommand } from "./commands/health.js";
18-
import { registerHelpCommand } from "./commands/help.js";
18+
import { enforceKnownCommandHelp, registerHelpCommand } from "./commands/help.js";
1919
import { registerCliCommand } from "./commands/cli.js";
2020
import { addAttrsSubcommands } from "./commands/attrs.js";
2121

@@ -62,5 +62,7 @@ export function createProgram(): Command {
6262
addAttrsSubcommands(hiddenAttrs);
6363
program.addCommand(hiddenAttrs, { hidden: true });
6464

65+
enforceKnownCommandHelp(program);
66+
6567
return program;
6668
}

src/commands/help.ts

Lines changed: 64 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import chalk from "chalk";
2-
import type { Command, Help, Option } from "commander";
2+
import type { Command, Help, HelpContext, Option } from "commander";
33

44
interface Example {
55
description: string;
@@ -220,6 +220,12 @@ export function formatCommandDetails(
220220
return lines.join("\n");
221221
}
222222

223+
function unknownCommandError(attempted: string): never {
224+
console.error(chalk.red(`Error: '${attempted}' is not a geonic command.`));
225+
console.error(`\nSee 'geonic help' for available commands.`);
226+
process.exit(1);
227+
}
228+
223229
function showHelp(program: Command, args: string[]): void {
224230
if (args.length === 0) {
225231
console.log(formatTopLevelHelp(program));
@@ -232,12 +238,7 @@ function showHelp(program: Command, args: string[]): void {
232238
for (let i = 0; i < args.length; i++) {
233239
const found = findCommand(current, args[i]);
234240
if (!found) {
235-
const attempted = args.slice(0, i + 1).join(" ");
236-
console.error(
237-
chalk.red(`Error: '${attempted}' is not a geonic command.`),
238-
);
239-
console.error(`\nSee 'geonic help' for available commands.`);
240-
process.exit(1);
241+
unknownCommandError(args.slice(0, i + 1).join(" "));
241242
}
242243
resolved.push(found);
243244
current = found;
@@ -249,6 +250,62 @@ function showHelp(program: Command, args: string[]): void {
249250
console.log(formatCommandDetails(program, target, path));
250251
}
251252

253+
export function enforceKnownCommandHelp(program: Command): void {
254+
// Commander prints help and exits 0 when --help/-h is present, even if the
255+
// remaining operand is not a known subcommand (e.g. `geonic hello --help`).
256+
// Wrap outputHelp() on every command group so that case errors with exit 1,
257+
// matching `geonic help <unknown>`. Call after all commands are registered.
258+
259+
// Token the user actually typed to invoke each command (name or alias),
260+
// recorded at dispatch time so error messages echo e.g. "models" as typed,
261+
// not the canonical name "custom-data-models".
262+
const typedNames = new WeakMap<Command, string>();
263+
264+
// Path from the program down to cmd, using the recorded typed tokens.
265+
const typedCommandPath = (cmd: Command): string[] => {
266+
const path: string[] = [];
267+
let current: Command | null = cmd;
268+
while (current && current.parent) {
269+
path.unshift(typedNames.get(current) ?? current.name());
270+
current = current.parent;
271+
}
272+
return path;
273+
};
274+
275+
const visit = (cmd: Command): void => {
276+
if (cmd.commands.length > 0) {
277+
// preSubcommand fires on dispatch, before commander's --help
278+
// short-circuit; parent.args[0] is the operand commander resolved the
279+
// subcommand from. Hooks are not inherited, so attach per group.
280+
cmd.hook("preSubcommand", (parent, subCommand) => {
281+
const token = parent.args[0];
282+
if (token !== undefined) {
283+
typedNames.set(subCommand, token);
284+
}
285+
});
286+
287+
const originalOutputHelp = cmd.outputHelp.bind(cmd);
288+
// Same parameter type as Command.outputHelp (including deprecated overload)
289+
cmd.outputHelp = (
290+
contextOptions?: HelpContext | ((str: string) => string),
291+
): void => {
292+
// cmd.args lists operands first, then unparsed options such as --help.
293+
// Only a leading non-empty, non-dash token is a genuine operand — a
294+
// non-dash token found later may be the value of an unrecognized option.
295+
const operand = cmd.args[0];
296+
if (operand && !operand.startsWith("-") && !findCommand(cmd, operand)) {
297+
unknownCommandError([...typedCommandPath(cmd), operand].join(" "));
298+
}
299+
originalOutputHelp(contextOptions as HelpContext | undefined);
300+
};
301+
}
302+
for (const sub of cmd.commands) {
303+
visit(sub);
304+
}
305+
};
306+
visit(program);
307+
}
308+
252309
export function registerHelpCommand(program: Command): void {
253310
// Disable Commander's built-in help command
254311
program.addHelpCommand(false);

tests/help.test.ts

Lines changed: 107 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,30 @@ function findCommand(program: ReturnType<typeof createProgram>, ...names: string
1616
return current;
1717
}
1818

19+
// Parses argv expecting the unknown-command error path: captures stderr,
20+
// asserts process.exit(1), and returns the stripped stderr output.
21+
async function runExpectingError(argv: string[]): Promise<string> {
22+
const prog = createProgram();
23+
prog.exitOverride();
24+
const errSpy = vi.spyOn(console, "error").mockImplementation(() => {});
25+
const exitSpy = vi.spyOn(process, "exit").mockImplementation(() => {
26+
throw new Error("process.exit");
27+
});
28+
try {
29+
try {
30+
await prog.parseAsync(["node", "geonic", ...argv]);
31+
} catch {
32+
// expected
33+
}
34+
const output = stripAnsi(errSpy.mock.calls.map((c) => c[0]).join("\n"));
35+
expect(exitSpy).toHaveBeenCalledWith(1);
36+
return output;
37+
} finally {
38+
errSpy.mockRestore();
39+
exitSpy.mockRestore();
40+
}
41+
}
42+
1943
describe("help", () => {
2044
const program = createProgram();
2145

@@ -609,21 +633,8 @@ describe("help", () => {
609633
});
610634

611635
it("shows error for invalid command name in help", async () => {
612-
const prog = createProgram();
613-
prog.exitOverride();
614-
const errSpy = vi.spyOn(console, "error").mockImplementation(() => {});
615-
const exitSpy = vi.spyOn(process, "exit").mockImplementation(() => {
616-
throw new Error("process.exit");
617-
});
618-
try {
619-
await prog.parseAsync(["node", "geonic", "help", "nonexistent"]);
620-
} catch {
621-
// expected
622-
}
623-
const output = errSpy.mock.calls.map((c) => c[0]).join("\n");
624-
expect(stripAnsi(output)).toContain("'nonexistent' is not a geonic command");
625-
errSpy.mockRestore();
626-
exitSpy.mockRestore();
636+
const output = await runExpectingError(["help", "nonexistent"]);
637+
expect(output).toContain("'nonexistent' is not a geonic command");
627638
});
628639

629640
it("shows top-level help when help is called with no args", async () => {
@@ -683,6 +694,87 @@ describe("help", () => {
683694
});
684695
});
685696

697+
describe("unknown subcommand with --help", () => {
698+
// Parses argv expecting help to be shown: captures stdout and returns it,
699+
// asserting no error was printed.
700+
async function runExpectingHelp(argv: string[]): Promise<string> {
701+
const prog = createProgram();
702+
prog.exitOverride();
703+
const writeSpy = vi.spyOn(process.stdout, "write").mockImplementation(() => true);
704+
const errSpy = vi.spyOn(console, "error").mockImplementation(() => {});
705+
try {
706+
try {
707+
await prog.parseAsync(["node", "geonic", ...argv]);
708+
} catch {
709+
// exitOverride throws on help
710+
}
711+
const output = stripAnsi(writeSpy.mock.calls.map((c) => c[0]).join(""));
712+
expect(errSpy).not.toHaveBeenCalled();
713+
return output;
714+
} finally {
715+
writeSpy.mockRestore();
716+
errSpy.mockRestore();
717+
}
718+
}
719+
720+
it("errors for unknown top-level command with --help", async () => {
721+
const output = await runExpectingError(["hello", "--help"]);
722+
expect(output).toContain("'hello' is not a geonic command");
723+
expect(output).toContain("geonic help");
724+
});
725+
726+
it("errors for unknown top-level command with -h", async () => {
727+
const output = await runExpectingError(["hello", "-h"]);
728+
expect(output).toContain("'hello' is not a geonic command");
729+
});
730+
731+
it("errors for unknown nested subcommand with --help", async () => {
732+
const output = await runExpectingError(["entities", "hello", "--help"]);
733+
expect(output).toContain("'entities hello' is not a geonic command");
734+
});
735+
736+
it("still shows help for a leaf command with an argument and --help", async () => {
737+
const output = await runExpectingHelp(["entities", "get", "urn:x", "--help"]);
738+
expect(output).toContain("geonic entities get");
739+
});
740+
741+
it("echoes the alias the user typed, not the canonical command name", async () => {
742+
const output = await runExpectingError(["models", "badsub", "--help"]);
743+
expect(output).toContain("'models badsub' is not a geonic command");
744+
expect(output).not.toContain("custom-data-models");
745+
});
746+
747+
it("echoes the typed alias even when an option value collides with a command name", async () => {
748+
// The --service value must not be mistaken for the typed command token
749+
const output = await runExpectingError([
750+
"--service", "custom-data-models", "models", "badsub", "--help",
751+
]);
752+
expect(output).toContain("'models badsub' is not a geonic command");
753+
expect(output).not.toContain("custom-data-models badsub");
754+
});
755+
756+
it("does not mistake an unknown option's value for a subcommand", async () => {
757+
// "--bogus json" must not be reported as `geonic entities json`
758+
const output = await runExpectingHelp(["entities", "--bogus", "json", "--help"]);
759+
expect(output).toContain("geonic entities");
760+
});
761+
762+
it("shows top-level help when --help precedes a non-command token", async () => {
763+
const output = await runExpectingHelp(["--help", "hello"]);
764+
expect(output).toContain("AVAILABLE COMMANDS");
765+
});
766+
767+
it("shows group help when --help precedes a non-command token in a subgroup", async () => {
768+
const output = await runExpectingHelp(["entities", "--help", "badsub"]);
769+
expect(output).toContain("geonic entities");
770+
});
771+
772+
it("does not report an empty-string operand as an unknown command", async () => {
773+
const output = await runExpectingHelp(["entities", "", "--help"]);
774+
expect(output).toContain("geonic entities");
775+
});
776+
});
777+
686778
describe("no-args handler", () => {
687779
it("shows help when geonic is run with no arguments", async () => {
688780
const prog = createProgram();

0 commit comments

Comments
 (0)