Skip to content

Commit 7919199

Browse files
Merge pull request #738 from corbitsdev/cl-7092-recognize-help-consistently-and-reject-flags-as-option
Recognize CLI help regardless of argument position
2 parents 6b68b94 + 7b55aaa commit 7919199

3 files changed

Lines changed: 111 additions & 19 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename
2121
live-identity rule. Context usage and `/cost` still work; `/cost`
2222
reports Codex cost as covered by ChatGPT subscription. Metered OpenAI
2323
API endpoints keep dollar estimates.
24+
- CLI `--help` / `-h` is recognized in any argument position. Value flags no
25+
longer swallow `--*` or `-h` as their option values.
2426

2527
## [0.3.11] - 2026-08-31
2628

src/config.test.ts

Lines changed: 94 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { describe, test, expect } from "bun:test";
22
import { mkdtemp, mkdir, writeFile, rm } from "node:fs/promises";
33
import { tmpdir } from "node:os";
4-
import { join } from "node:path";
4+
import { join, resolve } from "node:path";
55

66
import {
77
buildBifrostSource,
@@ -73,6 +73,18 @@ async function emptyCwd(): Promise<string> {
7373
return mkdtemp(join(tmpdir(), "ic-config-"));
7474
}
7575

76+
async function expectCliHelp(argv: readonly string[]): Promise<void> {
77+
try {
78+
await loadConfig([...argv], { globalSettingsPath: NO_SETTINGS });
79+
expect.unreachable("expected CliHelpError");
80+
} catch (err) {
81+
expect(err).toBeInstanceOf(CliHelpError);
82+
const help = err as CliHelpError;
83+
expect(help.exitCode).toBe(0);
84+
expect(help.message).toBe(CLI_HELP_TEXT);
85+
}
86+
}
87+
7688
describe("loadConfig", () => {
7789
test("resolves provider from the global settings file", async () => {
7890
const cwd = await emptyCwd();
@@ -726,21 +738,90 @@ describe("loadConfig", () => {
726738
});
727739

728740
test("--help throws CliHelpError with exitCode 0 and full help text", async () => {
729-
await expect(
730-
loadConfig(["--help"], { globalSettingsPath: NO_SETTINGS }),
731-
).rejects.toBeInstanceOf(CliHelpError);
732-
try {
733-
await loadConfig(["-h"], { globalSettingsPath: NO_SETTINGS });
734-
expect.unreachable("expected CliHelpError");
735-
} catch (err) {
736-
expect(err).toBeInstanceOf(CliHelpError);
737-
const help = err as CliHelpError;
738-
expect(help.exitCode).toBe(0);
739-
expect(help.message).toBe(CLI_HELP_TEXT);
740-
expect(help.message).toContain("resume");
741+
await expectCliHelp(["--help"]);
742+
await expectCliHelp(["-h"]);
743+
});
744+
745+
test("--help after flags throws CliHelpError", async () => {
746+
await expectCliHelp(["--force", "--help"]);
747+
await expectCliHelp(["--force", "-h"]);
748+
});
749+
750+
test("--help after a positional throws CliHelpError", async () => {
751+
await expectCliHelp(["ship it", "--help"]);
752+
await expectCliHelp(["ship", "it", "--help"]);
753+
});
754+
755+
test("--help after a bound flag value throws CliHelpError", async () => {
756+
await expectCliHelp(["--cwd", ".", "--help"]);
757+
await expectCliHelp(["--provider", "fireworks", "--help"]);
758+
});
759+
760+
test("exec --help throws CliHelpError", async () => {
761+
await expectCliHelp(["exec", "--help"]);
762+
await expectCliHelp(["exec", "--director", "--help"]);
763+
});
764+
765+
test("resume --pick --help throws CliHelpError", async () => {
766+
await expectCliHelp(["resume", "--pick", "--help"]);
767+
});
768+
769+
test("resume -h / --help throws CliHelpError instead of treating it as a session id", async () => {
770+
await expectCliHelp(["resume", "-h"]);
771+
await expectCliHelp(["resume", "--help"]);
772+
await expectCliHelp(["continue", "-h"]);
773+
});
774+
775+
test("value flags do not swallow --help / -h as their value", async () => {
776+
for (const flag of ["--provider", "--model", "--cwd", "--config", "--profile"] as const) {
777+
await expectCliHelp([flag, "--help"]);
778+
await expectCliHelp([flag, "-h"]);
741779
}
742780
});
743781

782+
test("value flags reject other flag-shaped tokens as values", async () => {
783+
await expect(
784+
loadConfig(["--provider", "--force"], { globalSettingsPath: NO_SETTINGS }),
785+
).rejects.toThrow("--provider requires a value");
786+
await expect(
787+
loadConfig(["--model", "--cwd"], { globalSettingsPath: NO_SETTINGS }),
788+
).rejects.toThrow("--model requires a value");
789+
await expect(
790+
loadConfig(["--cwd", "--tmp"], { globalSettingsPath: NO_SETTINGS }),
791+
).rejects.toThrow("--cwd requires a value");
792+
await expect(
793+
loadConfig(["exec", "--director", "--force", "ship it"], {
794+
globalSettingsPath: NO_SETTINGS,
795+
}),
796+
).rejects.toThrow("--director requires a value");
797+
});
798+
799+
test("value flags still error clearly when the value is omitted", async () => {
800+
await expect(loadConfig(["--provider"], { globalSettingsPath: NO_SETTINGS })).rejects.toThrow(
801+
"--provider requires a value",
802+
);
803+
await expect(loadConfig(["--model"], { globalSettingsPath: NO_SETTINGS })).rejects.toThrow(
804+
"--model requires a value",
805+
);
806+
await expect(loadConfig(["--cwd"], { globalSettingsPath: NO_SETTINGS })).rejects.toThrow(
807+
"--cwd requires a value",
808+
);
809+
await expect(loadConfig(["--config"], { globalSettingsPath: NO_SETTINGS })).rejects.toThrow(
810+
"--config requires a value",
811+
);
812+
await expect(loadConfig(["--profile"], { globalSettingsPath: NO_SETTINGS })).rejects.toThrow(
813+
"--profile requires a value",
814+
);
815+
});
816+
817+
test("value flags accept a POSIX path that starts with a single dash", async () => {
818+
const config = await loadConfig(["--cwd", "-my-dir", "do something"], {
819+
allowUnconfigured: true,
820+
globalSettingsPath: NO_SETTINGS,
821+
});
822+
expect(config.cwd).toBe(resolve("-my-dir"));
823+
});
824+
744825
test("rejects unknown flags", async () => {
745826
await expect(loadConfig(["--unknown"], { globalSettingsPath: NO_SETTINGS })).rejects.toThrow(
746827
/unrecognized flag/,

src/config/index.ts

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -555,6 +555,10 @@ export interface LoadConfigOptions {
555555
pricing?: PricingFetcherOptions;
556556
}
557557

558+
function isFlagToken(arg: string): boolean {
559+
return arg.startsWith("--") || arg === "-h";
560+
}
561+
558562
export async function loadConfig(
559563
argv: readonly string[],
560564
options?: LoadConfigOptions & { allowUnconfigured?: false },
@@ -567,6 +571,12 @@ export async function loadConfig(
567571
argv: readonly string[],
568572
options: LoadConfigOptions = {},
569573
): Promise<Config | UnconfiguredConfig> {
574+
// Help wins in any position, including after subcommands and immediately
575+
// after a value flag that would otherwise swallow the token as its value.
576+
if (argv.some((arg) => arg === "--help" || arg === "-h")) {
577+
throw new CliHelpError();
578+
}
579+
570580
const args = [...argv];
571581

572582
// Leading subcommand: `corbits exec "prompt"` (alias: `run`). Default is TUI.
@@ -589,7 +599,7 @@ export async function loadConfig(
589599
if (next === "--pick" || next === "--list") {
590600
resumeMode = "pick";
591601
args.shift();
592-
} else if (next !== undefined && !next.startsWith("--")) {
602+
} else if (next !== undefined && !isFlagToken(next)) {
593603
if (!isSessionId(next)) {
594604
throw new Error(
595605
`'${next}' is not a session id. Use a UUID session id or \`corbits resume\` to choose.`,
@@ -603,10 +613,6 @@ export async function loadConfig(
603613
}
604614
}
605615

606-
if (args[0] === "--help" || args[0] === "-h") {
607-
throw new CliHelpError();
608-
}
609-
610616
let cwd = process.cwd();
611617
let force = false;
612618
let dangerouslySkipPermissions = false;
@@ -626,7 +632,10 @@ export async function loadConfig(
626632
const positional: string[] = [];
627633

628634
const requireValue = (flag: string, value: string | undefined): string => {
629-
if (value === undefined) {
635+
// Flag-shaped tokens are never option values. `--provider --force` and a
636+
// trailing `--provider` both surface as a missing value rather than binding
637+
// the next flag (or accepting `--help`, which is already handled above).
638+
if (value === undefined || isFlagToken(value)) {
630639
throw new Error(`${flag} requires a value`);
631640
}
632641
return value;

0 commit comments

Comments
 (0)