Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 106 additions & 0 deletions ix-cli/src/cli/__tests__/common-option-validation.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
import { Command } from "commander";
import { describe, expect, it } from "vitest";

import { registerOssCommands } from "../register/oss.js";
import { validateCliOptions } from "../options.js";

async function parseInvalid(args: string[]): Promise<unknown> {
const program = new Command();
program.name("ix").exitOverride();
registerOssCommands(program);
try {
await program.parseAsync(args, { from: "user" });
return undefined;
} catch (error) {
return error;
}
}

describe("common CLI option validation", () => {
it("declares every documented enum as Commander choices", () => {
const program = new Command();
program.name("ix");
registerOssCommands(program);
const pending = [...program.commands];

while (pending.length > 0) {
const command = pending.shift()!;
pending.push(...command.commands);
for (const option of command.options) {
const group = option.description.match(/\(([^()]*(?:\|)[^()]*)\)/)?.[1];
if (!group) continue;
expect(option.argChoices, `${command.name()} ${option.long}`).toEqual(
group.split("|").map((choice) => choice.trim()),
);
}
}
});

it("does not make a later Pro command's help text an implicit runtime contract", async () => {
const program = new Command();
program.name("ix");
registerOssCommands(program);
let received: string | undefined;
program
.command("pro-test")
.option("--mode <mode>", "Presentation mode (short|full)")
.action((options: { mode?: string }) => { received = options.mode; });

await program.parseAsync(["pro-test", "--mode", "custom"], { from: "user" });

expect(received).toBe("custom");
});

it.each([
[["doctor", "--format", "yaml"], "--format"],
[["inventory", "--kind", "file", "--limit", "1e3"], "--limit"],
[["rank", "--by", "dependents", "--kind", "class", "--top", "10abc"], "--top"],
[["patches", "--limit", "-1"], "--limit"],
[["search", "term", "--as-of", "abc"], "--as-of"],
[["search", "term", "--as-of", "1e3"], "--as-of"],
[["map", "--level", "nope"], "--level"],
[["map", "--min-confidence", "1.1"], "--min-confidence"],
[["map", "--sort", "newest"], "--sort"],
[["savings", "--model", "unknown"], "--model"],
] as const)("rejects %j before running the command", async (args, option) => {
const error = await parseInvalid([...args]);

expect(error).toMatchObject({ code: "commander.invalidArgument" });
expect(String((error as Error).message)).toContain(option);
});

it.each([
["doctor", "--format", "json"],
["map", "--format", "silent"],
["map", "--min-confidence", "0.75"],
["subsystems", "--offset", "0"],
// 0 is this flag's own default, so rejecting it was incoherent.
["smells", "--orphan-max-connections", "0"],
["smells", "--weak-max-neighbors", "0"],
] as const)("accepts the documented value in %j", (command, option, value) => {
const program = new Command();
program.name("ix");
registerOssCommands(program);
const action = program.commands.find((candidate) => candidate.name() === command)!;
action.parseOptions([option, value]);

expect(() => validateCliOptions(action)).not.toThrow();
});

// The regression this hook shipped with: it read every option's *default*
// through `command.opts()`, so a command whose own default fell outside the
// rule could not be run at all. `ix smells` defaults
// --orphan-max-connections to "0" and died on `ix smells` with no arguments.
it.each(["smells", "map", "subsystems", "rank", "inventory", "patches", "doctor", "status", "context"])(
"runs %s on its defaults alone",
(command) => {
const program = new Command();
program.name("ix");
registerOssCommands(program);
const action = program.commands.find((candidate) => candidate.name() === command)!;
action.parseOptions([]);

expect(() => validateCliOptions(action)).not.toThrow();
},
);
});
2 changes: 1 addition & 1 deletion ix-cli/src/cli/commands/map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ export function registerMapCommand(program: Command): void {
program
.command("map [path]")
.description("Map the architectural hierarchy of a codebase")
.option("--format <fmt>", "Output format (text|json|llm)", "text")
.option("--format <fmt>", "Output format (text|json|llm|silent)", "text")
.option("--level <n>", "Show only regions at this level (1=finest, higher=coarser)")
.option("--min-confidence <n>", "Only show regions above this confidence threshold (0-1)", "0")
.option("--max-items <n>", "Max items to show per section in text output (default: 10)", "10")
Expand Down
66 changes: 65 additions & 1 deletion ix-cli/src/cli/options.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { InvalidArgumentError } from "commander";
import { InvalidArgumentError, type Command, type Option } from "commander";

/**
* Parse a flag that must be a positive integer, rejecting anything else.
Expand All @@ -23,6 +23,70 @@ function parsePositiveInt(value: string, example: string): number {
return parsed;
}

function parseNonNegativeInt(value: string, example: string): number {
const normalized = value.trim();
const reject = () =>
new InvalidArgumentError(`must be a non-negative integer (for example, ${example})`);
if (!/^\+?\d+$/.test(normalized)) throw reject();

const parsed = Number(normalized);
if (!Number.isSafeInteger(parsed)) throw reject();
return parsed;
}

function invalidOption(option: Option, detail: string): InvalidArgumentError {
return new InvalidArgumentError(`option '${option.long}' ${detail}`);
}

/**
* Validate common option domains before a command can contact the backend.
*
* Most commands read numeric options with `parseInt` and route any unrecognized
* output format through their text branch. That turns typos into a different,
* successful request. Keeping this at the root command covers OSS and optional
* Pro commands consistently, including the long-lived MCP runner.
*/
export function validateCliOptions(command: Command): void {
const values = command.opts();

for (const option of command.options) {
const name = option.attributeName();
// Only what the caller actually typed. A default is the command author's
// own choice, and checking it turns a mismatch into a command nobody can
// run: `ix smells` defaults `--orphan-max-connections` to 0, which the
// `<n>` rule below rejected, so the command failed before its action ever
// started -- on no arguments at all. Anything the author ships as a
// default is by definition a value the command accepts.
if (command.getOptionValueSource(name) !== "cli") continue;
const value = values[name];
if (value === undefined || value === null) continue;

if (typeof value === "string") {
if (option.long === "--as-of") {
try { parseRevisionOption(value); }
catch { throw invalidOption(option, "must be a positive integer"); }
}

if (option.long === "--min-confidence") {
const parsed = Number(value);
if (!/^(?:0(?:\.\d+)?|1(?:\.0+)?)$/.test(value.trim()) || parsed < 0 || parsed > 1) {
throw invalidOption(option, "must be a number from 0 to 1");
}
} else if (option.flags.includes("<n>")) {
// Non-negative, not positive. 0 is a documented value for several of
// these -- `--offset 0`, `--orphan-max-connections 0` (its own
// default), `--weak-max-neighbors 0` -- and none of the typos this
// exists to catch survive either rule: `1e3`, `10abc`, `0x10`, `-5`
// and `abc` are all rejected. The flags that genuinely mean "at least
// one" declare `parsePositiveInt` as their own commander parser
// (`--pick`), so commander still rejects 0 for them at parse time.
try { parseNonNegativeInt(value, "0 or 10"); }
catch { throw invalidOption(option, "must be a non-negative integer"); }
}
}
}
}

export function parsePickOption(value: string): number {
return parsePositiveInt(value, "1 or 2");
}
Expand Down
29 changes: 29 additions & 0 deletions ix-cli/src/cli/register/oss.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import { registerSavingsCommand } from "../commands/savings.js";
import { registerPatchesCommand } from "../commands/patches.js";
import { registerMcpCommand } from "../commands/mcp.js";
import { registerContextCommand } from "../commands/context.js";
import { validateCliOptions } from "../options.js";

const PRO_COMMANDS: { name: string; desc: string }[] = [
{ name: "briefing", desc: "Session-resume briefing" },
Expand Down Expand Up @@ -65,6 +66,28 @@ const ADVANCED_COMMANDS = [
"init", "ingest",
];

const DEFAULT_FORMAT_CHOICES = ["text", "json", "llm"];
const OPTION_CHOICES: Record<string, Record<string, string[]>> = {
query: { depth: ["shallow", "standard", "deep"], format: ["text", "json"] },
map: { format: [...DEFAULT_FORMAT_CHOICES, "silent"], sort: ["importance", "confidence", "size", "alpha"] },
subsystems: { sort: ["importance", "confidence", "size", "alpha"] },
savings: { model: ["opus", "sonnet", "haiku", "gpt-4o"] },
context: { depth: ["compact", "standard", "full", "shallow", "deep"] },
};

function configureOssOptionChoices(root: Command): void {
const visit = (command: Command): void => {
const commandChoices = OPTION_CHOICES[command.name()] ?? {};
for (const option of command.options) {
const choices = commandChoices[option.attributeName()]
?? (option.long === "--format" ? DEFAULT_FORMAT_CHOICES : undefined);
if (choices) option.choices(choices);
}
for (const child of command.commands) visit(child);
};
visit(root);
}

export function registerOssCommands(program: Command): void {
registerQueryCommand(program);
registerIngestCommand(program);
Expand Down Expand Up @@ -105,6 +128,12 @@ export function registerOssCommands(program: Command): void {
registerMcpCommand(program);
registerContextCommand(program);

configureOssOptionChoices(program);

program.hook("preAction", (_thisCommand, actionCommand) => {
validateCliOptions(actionCommand);
});

// Hide advanced commands from default help
const advancedSet = new Set(ADVANCED_COMMANDS);
for (const cmd of program.commands) {
Expand Down
Loading