Skip to content
Merged
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
122 changes: 122 additions & 0 deletions src/cli-error-report.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,3 +115,125 @@ describe('handleProgramParseError', () => {
expect(process.exitCode).toBe(2);
});
});

/**
* One envelope, one exit code, one stderr line for every usage mistake.
* Each case below is a real `webcmd` invocation that used to render
* differently: see fix/cli-usage-error-envelope.
*/
describe('usage error contract', () => {
const previousExitCode = process.exitCode;
const previousArgv = process.argv;
afterEach(() => {
process.exitCode = previousExitCode;
process.argv = previousArgv;
vi.restoreAllMocks();
});

/** Run argv through the local CLI exactly as runCli does, capturing all stderr. */
async function runLocal(argv: string[]): Promise<{ stderr: string; exitCode: number }> {
process.argv = ['node', 'webcmd', ...argv];
process.exitCode = undefined;
let stderr = '';
const write = vi.spyOn(process.stderr, 'write').mockImplementation(((chunk: string) => {
stderr += String(chunk);
return true;
}) as typeof process.stderr.write);
vi.spyOn(console, 'error').mockImplementation((...values: unknown[]) => {
stderr += `${values.map(String).join(' ')}\n`;
});
const program = createProgram('', '');
applyUnknownOptionContract(program);
try {
await program.parseAsync(argv, { from: 'user' });
} catch (err) {
handleProgramParseError(err);
} finally {
write.mockRestore();
}
return { stderr, exitCode: Number(process.exitCode ?? 0) };
}

function errorLines(stderr: string): string[] {
return stderr.split('\n').filter(line => line.startsWith('error: '));
}

const usageCases = [
{ name: 'unknown subcommand', argv: ['adapter', 'list', 'rest'], code: 'UNKNOWN_COMMAND', help: /valid subcommands for `webcmd adapter`/ },
{ name: 'unknown option', argv: ['adapter', 'path', 'x/y', '-q'], code: 'UNKNOWN_OPTION', help: undefined },
{ name: 'excess arguments', argv: ['list', 'extra'], code: 'EXCESS_ARGUMENTS', help: /usage: webcmd list/ },
];

it.each(usageCases)('$name exits 2 with one stderr line and no envelope for humans', async ({ argv, help }) => {
const { stderr, exitCode } = await runLocal(argv);
expect(exitCode).toBe(EXIT_CODES.USAGE_ERROR);
expect(errorLines(stderr)).toHaveLength(1);
expect(stderr).not.toContain('ok: false');
if (help) expect(stderr).toMatch(help);
});

it.each(usageCases)('$name renders a JSON envelope under --json', async ({ argv, code }) => {
const { stderr, exitCode } = await runLocal([...argv, '--json']);
expect(exitCode).toBe(EXIT_CODES.USAGE_ERROR);
expect(JSON.parse(stderr)).toMatchObject({
ok: false,
error: { code, exitCode: EXIT_CODES.USAGE_ERROR },
});
});

it('renders a YAML envelope for -f yaml', async () => {
const { stderr } = await runLocal(['adapter', 'list', 'rest', '-f', 'yaml']);
expect(yaml.load(stderr)).toMatchObject({ ok: false, error: { code: 'UNKNOWN_COMMAND', exitCode: 2 } });
});

it('keeps --help a display exit, not a usage error', async () => {
vi.spyOn(console, 'log').mockImplementation(() => undefined);
const { stderr, exitCode } = await runLocal(['adapter', '--help']);
expect(exitCode).toBe(0);
expect(stderr).toBe('');
});
});

describe('web fetch fast path usage errors', () => {
const previousExitCode = process.exitCode;
const previousArgv = process.argv;
afterEach(() => {
process.exitCode = previousExitCode;
process.argv = previousArgv;
vi.restoreAllMocks();
});

async function runFetch(argv: string[]): Promise<{ stderr: string; exitCode: number }> {
const { runWebFetchCommand } = await import('./fetch/command.js');
process.argv = ['node', 'webcmd', ...argv];
process.exitCode = undefined;
let stderr = '';
const write = vi.spyOn(process.stderr, 'write').mockImplementation(((chunk: string) => {
stderr += String(chunk);
return true;
}) as typeof process.stderr.write);
try {
await runWebFetchCommand(argv);
} finally {
write.mockRestore();
}
return { stderr, exitCode: Number(process.exitCode ?? 0) };
}

it('reports a missing required option once, as a usage error', async () => {
const { stderr, exitCode } = await runFetch(['web', 'fetch']);
expect(exitCode).toBe(EXIT_CODES.USAGE_ERROR);
expect(stderr.split('\n').filter(line => line.startsWith('error: '))).toHaveLength(1);
expect(stderr).toContain("error: required option '--url <value>' not specified");
expect(stderr).toContain('help: usage: webcmd web fetch');
});

it('honours --json on a missing required option', async () => {
const { stderr, exitCode } = await runFetch(['web', 'fetch', '--json']);
expect(exitCode).toBe(EXIT_CODES.USAGE_ERROR);
expect(JSON.parse(stderr)).toMatchObject({
ok: false,
error: { code: 'MISSING_OPTION', exitCode: EXIT_CODES.USAGE_ERROR },
});
});
});
47 changes: 47 additions & 0 deletions src/cli-error-report.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/**
* The single place a CLI failure becomes bytes on stderr plus an exit code.
*
* Both thrown `CliError`s and Commander's structural failures land here so a
* usage mistake always reports the same shape: one `error:` line (plus a
* `help:` line) for humans, or the machine envelope when `-f/--format` or
* `--json` asked for one.
*
* Lives outside cli.ts so the `web fetch` fast path in main.ts can reuse it
* without importing the full command tree.
*/
import { CommanderError } from 'commander';
import { CommanderStructuralError } from './command-surface.js';
import { toEnvelope } from './errors.js';
import { errorEnvelopeFormat, formatErrorEnvelope, requestedFormatFromArgv, requestedMachineFormat } from './output.js';

const COMMANDER_DISPLAY_CODES = new Set([
'commander.help',
'commander.helpDisplayed',
'commander.version',
]);

export function handleProgramParseError(err: unknown, stderr: NodeJS.WritableStream = process.stderr): void {
if (err instanceof CommanderStructuralError) {
const fmt = requestedMachineFormat(process.argv.slice(2));
stderr.write(fmt && err.envelope ? formatErrorEnvelope(err.envelope, { fmt }) : err.output);
process.exitCode = err.exitCode;
return;
}
if (err instanceof CommanderError && COMMANDER_DISPLAY_CODES.has(err.code)) {
process.exitCode = err.exitCode;
return;
}
reportCliError(err, stderr);
}

/** Render a thrown error as the shared envelope and set the exit code it carries. */
export function reportCliError(err: unknown, stderr: NodeJS.WritableStream = process.stderr): void {
const envelope = toEnvelope(err);
if (process.env.WEBCMD_DEBUG && err instanceof Error && err.stack) {
envelope.error.stack = err.stack;
}
stderr.write(formatErrorEnvelope(envelope, {
fmt: errorEnvelopeFormat(requestedFormatFromArgv(process.argv.slice(2))),
}));
process.exitCode = envelope.error.exitCode;
}
40 changes: 6 additions & 34 deletions src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import * as os from 'node:os';
import * as path from 'node:path';
import * as readline from 'node:readline/promises';
import { fileURLToPath, pathToFileURL } from 'node:url';
import { Command, CommanderError, Option } from 'commander';
import { Command, Option } from 'commander';
import { findPackageRoot, getBuiltEntryCandidates } from './package-paths.js';
import { type CliCommand, getRegistry } from './registry.js';
// Side-effect import: registers client-owned `web fetch` in the core registry
Expand All @@ -19,15 +19,16 @@ import './fetch/command.js';
import { commandListPresentation, filterCommandsByTag, toPresentableCommand } from './command-presentation.js';
import { configureCompletionCommandSurface, configureListCommandSurface, configurePluginInstallSurface, configurePluginListSurface, configurePluginSearchSurface } from './builtin-command-surface.js';
import { formatPluginSearchEmptyCopy, presentPluginSearch } from './plugin-search-presentation.js';
import { addOutputFormatOption, applyUnknownOptionContract, CommanderStructuralError, JSON_FORMAT_ALIAS_HELP, outputFormatIsExplicit, resolveCommandOutputFormat } from './command-surface.js';
import { render as renderOutput, formatErrorEnvelope, errorEnvelopeFormat, requestedFormatFromArgv } from './output.js';
import { addOutputFormatOption, applyUnknownOptionContract, JSON_FORMAT_ALIAS_HELP, outputFormatIsExplicit, resolveCommandOutputFormat } from './command-surface.js';
import { render as renderOutput } from './output.js';
import { handleProgramParseError } from './cli-error-report.js';
import { PKG_VERSION } from './version.js';
import { printCompletionScript } from './completion.js';
import { loadExternalClis, executeExternalCli, installExternalCli, registerExternalCli, isBinaryInstalled, formatExternalCliLabel } from './external.js';
import { addWebcmdSkills, listWebcmdSkills, removeWebcmdSkills, updateWebcmdSkill, type WebcmdSkillAddResult } from './skills.js';
import { registerAllCommands } from './commanderAdapter.js';
import { buildRootHelpPresentation, classifyAdapter, commanderCommandHelpData, installCommanderNamespaceStructuredHelp, installRootPresentationHelp, installStructuredHelp, leadingPositionalFromUsage, rootHelpData, type RootAdapterGroups } from './help.js';
import { EXIT_CODES, getErrorMessage, toEnvelope, BrowserConnectError, CliError, ArgumentError } from './errors.js';
import { EXIT_CODES, getErrorMessage, BrowserConnectError, CliError, ArgumentError } from './errors.js';
import { TargetError, type TargetErrorCode } from './browser/target-errors.js';
import { resolveTargetJs, getTextResolvedJs, getValueResolvedJs, getAttributesResolvedJs, selectResolvedJs, isAutocompleteResolvedJs, type ResolveOptions, type TargetMatchLevel } from './browser/target-resolver.js';
import { buildFindJs, buildSemanticFindJs, isFindError, type FindResult, type FindError, type SemanticFindOptions } from './browser/find.js';
Expand Down Expand Up @@ -2272,36 +2273,7 @@ export async function runCli(BUILTIN_CLIS: string, USER_CLIS: string): Promise<v
}
}

const COMMANDER_DISPLAY_CODES = new Set([
'commander.help',
'commander.helpDisplayed',
'commander.version',
]);

export function handleProgramParseError(err: unknown, stderr: NodeJS.WritableStream = process.stderr): void {
if (err instanceof CommanderStructuralError) {
stderr.write(err.output);
process.exitCode = err.exitCode;
return;
}
if (err instanceof CommanderError && COMMANDER_DISPLAY_CODES.has(err.code)) {
process.exitCode = err.exitCode;
return;
}
reportCliError(err, stderr);
}

/** Render a thrown error as the shared envelope and set the exit code it carries. */
export function reportCliError(err: unknown, stderr: NodeJS.WritableStream = process.stderr): void {
const envelope = toEnvelope(err);
if (process.env.WEBCMD_DEBUG && err instanceof Error && err.stack) {
envelope.error.stack = err.stack;
}
stderr.write(formatErrorEnvelope(envelope, {
fmt: errorEnvelopeFormat(requestedFormatFromArgv(process.argv.slice(2))),
}));
process.exitCode = envelope.error.exitCode;
}
export { handleProgramParseError, reportCliError } from './cli-error-report.js';

// ── Helpers ─────────────────────────────────────────────────────────────────

Expand Down
104 changes: 86 additions & 18 deletions src/command-surface.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Command, CommanderError } from 'commander';
import { ArgumentError, CliError, EXIT_CODES } from './errors.js';
import { ArgumentError, CliError, EXIT_CODES, type ErrorEnvelope } from './errors.js';
import type { Arg, CliCommand, CommandArgs } from './registry.js';

/** Canonical output format names accepted by the shared renderer. */
Expand Down Expand Up @@ -54,12 +54,28 @@ export class CommanderStructuralError extends Error {
readonly output: string,
readonly exitCode: number,
readonly appendErrorEnvelope = false,
/** Machine-readable form of the same failure, when it is a usage mistake. */
readonly envelope?: ErrorEnvelope,
) {
super(output.trimEnd());
this.name = 'CommanderStructuralError';
}
}

/**
* Commander structural failure codes that are user usage mistakes, mapped onto
* the machine-readable `error.code` they report. Everything listed here exits
* with EXIT_CODES.USAGE_ERROR so a typo is distinguishable from a runtime fault.
*/
export const USAGE_ERROR_CODES: Readonly<Record<string, string>> = {
'commander.unknownCommand': 'UNKNOWN_COMMAND',
'commander.unknownOption': 'UNKNOWN_OPTION',
'commander.missingArgument': 'MISSING_ARGUMENT',
'commander.missingMandatoryOptionValue': 'MISSING_OPTION',
'commander.excessArguments': 'EXCESS_ARGUMENTS',
'commander.invalidArgument': 'INVALID_ARGUMENT',
};

export function visibleCommandFlags(command: Command): string[] {
const flags: string[] = [];
const seen = new Set<string>();
Expand All @@ -82,12 +98,33 @@ export function commandInvocationPath(command: Command): string {
return names.join(' ');
}

export function formatUnknownOptionError(err: CommanderError, command: Command): string {
const flags = visibleCommandFlags(command);
function visibleSubcommandNames(command: Command): string[] {
try {
return command.createHelp().visibleCommands(command).map(child => child.name());
} catch {
return command.commands.map(child => child.name());
}
}

/** The `help:` body offered alongside a structural `error:` line — no prefix, no newline. */
export function structuralHelpText(code: string, command: Command): string | undefined {
const path = commandInvocationPath(command);
const help = flags.length > 0 ? `help: valid flags for \`${path}\`: ${flags.join(', ')}\n` : '';
if (code === 'commander.unknownOption') {
const flags = visibleCommandFlags(command);
return flags.length > 0 ? `valid flags for \`${path}\`: ${flags.join(', ')}` : undefined;
}
if (code === 'commander.unknownCommand') {
const names = visibleSubcommandNames(command);
return names.length > 0 ? `valid subcommands for \`${path}\`: ${names.join(', ')}` : undefined;
}
return `usage: ${path} ${command.usage()}`.trimEnd();
}

/** Human-readable rendering of one Commander structural failure. */
export function formatStructuralError(err: CommanderError, command: Command): string {
const help = structuralHelpText(err.code, command);
const message = err.message.replace(/^error:\s*/i, '');
return `error: ${message}\n${help}`;
return `error: ${message}\n${help ? `help: ${help}\n` : ''}`;
}

export function structuralErrorFromCommander(
Expand All @@ -96,25 +133,56 @@ export function structuralErrorFromCommander(
capturedStderr = '',
opts: { appendErrorEnvelope?: boolean; includeCapturedStderrForUnknownOption?: boolean } = {},
): CommanderStructuralError {
if (error.code === 'commander.unknownOption') {
const output = `${opts.includeCapturedStderrForUnknownOption ? capturedStderr : ''}${formatUnknownOptionError(error, command)}`;
return new CommanderStructuralError(output, EXIT_CODES.USAGE_ERROR);
const code = USAGE_ERROR_CODES[error.code];
if (!code) {
return new CommanderStructuralError(
capturedStderr || `${error.message}\n`,
error.exitCode,
opts.appendErrorEnvelope === true,
);
}
const prefix = error.code === 'commander.unknownOption' && opts.includeCapturedStderrForUnknownOption
? capturedStderr
: '';
const help = structuralHelpText(error.code, command);
const envelope: ErrorEnvelope = {
ok: false,
error: {
code,
message: error.message.replace(/^error:\s*/i, ''),
...(help ? { help } : {}),
exitCode: EXIT_CODES.USAGE_ERROR,
},
};
return new CommanderStructuralError(
capturedStderr || `${error.message}\n`,
error.exitCode,
opts.appendErrorEnvelope === true,
`${prefix}${formatStructuralError(error, command)}`,
EXIT_CODES.USAGE_ERROR,
// `appendErrorEnvelope` is the legacy hosted fallback that tacks an
// UNKNOWN/exit-1 envelope onto the human bytes. A usage error carries its
// own envelope, so that fallback must not fire on top of it.
false,
envelope,
);
}

/** Walk argv to the leaf command Commander would have been parsing. */
/**
* Route every Commander structural failure through the shared envelope path.
*
* Commander's default `outputError` writes to stderr *before* `exitOverride`
* throws, so capturing `writeErr` here is what keeps the caller from printing
* the same line a second time. Output we do not own is replayed verbatim.
*/
export function applyUnknownOptionContract(command: Command): void {
command.exitOverride((err) => {
if (err.code === 'commander.unknownOption') {
throw structuralErrorFromCommander(err, command);
}
throw err;
});
let captured = '';
command
.configureOutput({ writeErr: (value: string) => { captured += value; } })
.exitOverride((err) => {
const replay = captured;
captured = '';
if (USAGE_ERROR_CODES[err.code]) throw structuralErrorFromCommander(err, command);
if (replay) process.stderr.write(replay);
throw err;
});
for (const child of command.commands) applyUnknownOptionContract(child);
}

Expand Down
Loading
Loading