diff --git a/src/cli-error-report.test.ts b/src/cli-error-report.test.ts index 169ee970..8bcab0c7 100644 --- a/src/cli-error-report.test.ts +++ b/src/cli-error-report.test.ts @@ -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 ' 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 }, + }); + }); +}); diff --git a/src/cli-error-report.ts b/src/cli-error-report.ts new file mode 100644 index 00000000..2bf50f28 --- /dev/null +++ b/src/cli-error-report.ts @@ -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; +} diff --git a/src/cli.ts b/src/cli.ts index 3e1a0d61..56a2f18d 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -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 @@ -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'; @@ -2272,36 +2273,7 @@ export async function runCli(BUILTIN_CLIS: string, USER_CLIS: string): Promise> = { + '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(); @@ -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( @@ -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); } diff --git a/src/fetch/command.ts b/src/fetch/command.ts index 50865fea..d81acc34 100644 --- a/src/fetch/command.ts +++ b/src/fetch/command.ts @@ -2,6 +2,8 @@ import { Command } from 'commander'; import { cli, Strategy, type CommandArgs } from '../registry.js'; import { registerCommandToProgram } from '../commanderAdapter.js'; import { configureRootCommandSurface } from '../root-command-surface.js'; +import { applyUnknownOptionContract } from '../command-surface.js'; +import { handleProgramParseError } from '../cli-error-report.js'; import { ArgumentError } from '../errors.js'; import type { WebFetchOptions, WebFetchResult } from './client.js'; @@ -28,7 +30,14 @@ export async function runWebFetchCommand(argv: string[]): Promise { const program = configureRootCommandSurface(new Command('webcmd')) .option('--workspace ', 'Hosted workspace id/slug for the request'); registerCommandToProgram(program.command('web'), webFetchCommand); - await program.parseAsync(argv, { from: 'user' }); + // Same structural-error contract as runCli: this fast path skips cli.ts, but + // a usage mistake here must still exit 2 with the shared envelope. + applyUnknownOptionContract(program); + try { + await program.parseAsync(argv, { from: 'user' }); + } catch (err) { + handleProgramParseError(err); + } } function clientOptionsFromKwargs(kwargs: CommandArgs): WebFetchOptions { diff --git a/src/hosted/root-command-surface.test.ts b/src/hosted/root-command-surface.test.ts index f126fba4..f23663c4 100644 --- a/src/hosted/root-command-surface.test.ts +++ b/src/hosted/root-command-surface.test.ts @@ -2,7 +2,7 @@ import { Writable } from 'node:stream'; import { Command, CommanderError } from 'commander'; import { describe, expect, it, vi } from 'vitest'; import { createProgram } from '../cli.js'; -import { applyUnknownOptionContract, CommanderStructuralError } from '../command-surface.js'; +import { applyUnknownOptionContract, CommanderStructuralError, USAGE_ERROR_CODES } from '../command-surface.js'; import { formatRootHelp } from '../command-presentation.js'; import { HOSTED_ROOT_HELP } from '../completion-shared.js'; import { CliError } from '../errors.js'; @@ -114,7 +114,9 @@ async function runActualLocalRoot(argv: string[]): Promise { return { exitCode: Number(process.exitCode ?? 0), stdout, stderr }; } catch (error) { if (error instanceof CommanderStructuralError) { - return { exitCode: error.exitCode, stdout, stderr: error.output, errorCode: 'commander.unknownOption' }; + const commanderCode = Object.entries(USAGE_ERROR_CODES) + .find(([, code]) => code === error.envelope?.error.code)?.[0] ?? 'commander.unknownOption'; + return { exitCode: error.exitCode, stdout, stderr: error.output, errorCode: commanderCode }; } if (error instanceof CommanderError) { return { exitCode: error.exitCode, stdout, stderr, errorCode: error.code }; @@ -508,23 +510,17 @@ describe('hosted root preflight call order', () => { fetchImpl: async () => manifestResponse(), }); + // Usage errors are exit 2 and carry a `help:` line instead of a trailing + // UNKNOWN/exit-1 envelope. See fix/cli-usage-error-envelope. expect(local).toMatchObject({ - exitCode: 1, + exitCode: 2, stdout: '', - stderr: "error: too many arguments for 'list'. Expected 0 arguments but got 1.\n", + stderr: "error: too many arguments for 'list'. Expected 0 arguments but got 1.\nhelp: usage: webcmd list [options]\n", errorCode: 'commander.excessArguments', }); expect(hosted).toEqual({ handled: true, exitCode: local.exitCode }); expect(stdout.text()).toBe(local.stdout); - expect(stderr.text()).toBe([ - local.stderr.trimEnd(), - 'ok: false', - 'error:', - " code: UNKNOWN", - " message: 'error: too many arguments for ''list''. Expected 0 arguments but got 1.'", - ' exitCode: 1', - '', - ].join('\n')); + expect(stderr.text()).toBe(local.stderr); }); it.each([ @@ -555,6 +551,9 @@ describe('hosted root preflight call order', () => { 'help: valid flags for `webcmd list`: --tag, -f, --format, --json', '', ].join('\n')); + } else if (argv.join(' ') === 'list extra') { + // Excess arguments are a usage error: same bytes as local, no UNKNOWN envelope. + expect(stderr.text()).toBe(local.stderr); } else { expect(stderr.text()).toContain('ok: false\nerror:\n code: UNKNOWN\n'); } @@ -573,23 +572,17 @@ describe('hosted root preflight call order', () => { fetchImpl, }); + // Usage errors are exit 2 and carry a `help:` line instead of a trailing + // UNKNOWN/exit-1 envelope. See fix/cli-usage-error-envelope. expect(local).toMatchObject({ - exitCode: 1, + exitCode: 2, stdout: '', - stderr: "error: missing required argument 'shell'\n", + stderr: "error: missing required argument 'shell'\nhelp: usage: webcmd completion [options] \n", errorCode: 'commander.missingArgument', }); expect(hosted).toEqual({ handled: true, exitCode: local.exitCode }); expect(stdout.text()).toBe(local.stdout); - expect(stderr.text()).toBe([ - local.stderr.trimEnd(), - 'ok: false', - 'error:', - " code: UNKNOWN", - " message: 'error: missing required argument ''shell'''", - ' exitCode: 1', - '', - ].join('\n')); + expect(stderr.text()).toBe(local.stderr); expect(fetchImpl).not.toHaveBeenCalled(); }); @@ -669,8 +662,8 @@ describe('hosted root preflight call order', () => { '', ].join('\n')); } else { - expect(stderr.text()).toContain(local.stderr); - expect(stderr.text()).toContain('ok: false\nerror:\n code: UNKNOWN\n'); + // Usage errors: hosted reproduces the local bytes exactly, no UNKNOWN envelope. + expect(stderr.text()).toBe(local.stderr); } expect(fetchImpl).not.toHaveBeenCalled(); }); @@ -791,10 +784,16 @@ describe('hosted root preflight call order', () => { fetchImpl, }); - expect(local).toMatchObject({ exitCode: 1, stdout: '', stderr: "error: unknown command 'bogus'\n" }); + // Unknown subcommand is a usage error: exit 2, one line plus the valid + // subcommands, no trailing UNKNOWN envelope. See fix/cli-usage-error-envelope. + expect(local).toMatchObject({ + exitCode: 2, + stdout: '', + stderr: "error: unknown command 'bogus'\nhelp: valid subcommands for `webcmd github`: whoami, help\n", + }); expect(hosted).toEqual({ handled: true, exitCode: local.exitCode }); expect(stdout.text()).toBe(local.stdout); - expect(stderr.text()).toContain(`${local.stderr}ok: false\nerror:\n code: UNKNOWN\n`); + expect(stderr.text()).toBe("error: unknown command 'bogus'\nhelp: valid subcommands for `webcmd github`: whoami\n"); expect(fetchImpl).toHaveBeenCalledTimes(1); expect(String(fetchImpl.mock.calls[0]![0])).toBe('https://api.example.com/v1/manifest'); }); diff --git a/src/hosted/runner.test.ts b/src/hosted/runner.test.ts index 4a60b9cb..dac86bbf 100644 --- a/src/hosted/runner.test.ts +++ b/src/hosted/runner.test.ts @@ -1139,14 +1139,11 @@ describe('runHostedCli', () => { fetchImpl: async () => manifestResponse(), }); - expect(result).toEqual({ handled: true, exitCode: 1 }); + // Usage error: exit 2, one line plus the valid subcommands. + expect(result).toEqual({ handled: true, exitCode: 2 }); expect(stderr.text()).toBe([ "error: unknown command 'missing-command'", - 'ok: false', - 'error:', - " code: UNKNOWN", - " message: 'error: unknown command ''missing-command'''", - ' exitCode: 1', + 'help: valid subcommands for `webcmd github`: whoami', '', ].join('\n')); expect(stdout.text()).toBe(''); @@ -1164,14 +1161,11 @@ describe('runHostedCli', () => { fetchImpl: async () => new Response(JSON.stringify({ ok: true, manifest: requiredManifest }), { status: 200 }), }); - expect(result).toEqual({ handled: true, exitCode: 1 }); + // Usage error: exit 2, one line plus the usage restatement. + expect(result).toEqual({ handled: true, exitCode: 2 }); expect(stderr.text()).toBe([ "error: missing required argument 'account'", - 'ok: false', - 'error:', - " code: UNKNOWN", - " message: 'error: missing required argument ''account'''", - ' exitCode: 1', + 'help: usage: webcmd github whoami [options] ', '', ].join('\n')); expect(stdout.text()).toBe(''); @@ -1346,8 +1340,9 @@ describe('runHostedCli', () => { fetchImpl: async () => new Response(JSON.stringify({ ok: true, manifest: precedenceManifest }), { status: 200 }), }); - expect(result).toEqual({ handled: true, exitCode: 1 }); - expect(stderr.text()).toContain("error: missing required argument 'account'\nok: false\nerror:\n code: UNKNOWN\n"); + // Usage error: exit 2, one line plus the usage restatement. + expect(result).toEqual({ handled: true, exitCode: 2 }); + expect(stderr.text()).toContain("error: missing required argument 'account'\nhelp: usage: webcmd github whoami"); expect(stdout.text()).toBe(''); }); @@ -1390,28 +1385,32 @@ describe('runHostedCli', () => { { name: 'required named option before invalid format', tail: ['account', '-f', 'xml'], - exitCode: 1, + exitCode: 2, + usage: true, stderr: "error: required option '--token ' not specified\n", help: false, }, { name: 'required named option before invalid trace', tail: ['account', '--trace', 'always'], - exitCode: 1, + exitCode: 2, + usage: true, stderr: "error: required option '--token ' not specified\n", help: false, }, { name: 'required named option before invalid choice', tail: ['account', '--mode', 'bad'], - exitCode: 1, + exitCode: 2, + usage: true, stderr: "error: required option '--token ' not specified\n", help: false, }, { name: 'required positional before invalid format', tail: ['--token', 'secret', '-f', 'xml'], - exitCode: 1, + exitCode: 2, + usage: true, stderr: "error: missing required argument 'account'\n", help: false, }, @@ -1432,11 +1431,12 @@ describe('runHostedCli', () => { { name: 'ordinary excess positional', tail: ['account', 'extra', '--token', 'secret'], - exitCode: 1, + exitCode: 2, + usage: true, stderr: "error: too many arguments for 'whoami'. Expected 1 argument but got 2.\n", help: false, }, - ])('matches public Commander structural bytes and discovery order: $name', async ({ name, tail, exitCode, stderr: expectedStderr, help }) => { + ])('matches public Commander structural bytes and discovery order: $name', async ({ name, tail, exitCode, stderr: expectedStderr, help, usage }) => { const structuralManifest = manifestWithStructuralArguments(); const stdout = sink(); const stderr = sink(); @@ -1458,6 +1458,11 @@ describe('runHostedCli', () => { } else if (name === 'ordinary unknown option') { expect(stderr.text().startsWith(expectedStderr)).toBe(true); expect(stderr.text()).toContain('help: valid flags for'); + } else if (usage) { + // Usage error: the Commander line, a `help:` line, and no UNKNOWN envelope. + expect(stderr.text().startsWith(expectedStderr)).toBe(true); + expect(stderr.text()).toContain('help: usage: webcmd github whoami'); + expect(stderr.text()).not.toContain('ok: false'); } else { expect(stderr.text()).toContain(`${expectedStderr}ok: false\nerror:\n code: UNKNOWN\n`); } diff --git a/src/hosted/runner.ts b/src/hosted/runner.ts index 0425c0cf..09e4c33e 100644 --- a/src/hosted/runner.ts +++ b/src/hosted/runner.ts @@ -25,7 +25,7 @@ import { CliError, ConfigError, EXIT_CODES, toEnvelope } from '../errors.js'; import { getRequestedHelpFormat, renderStructuredHelp } from '../help.js'; import { enableVerbose } from '../logger.js'; import { findPackageRoot } from '../package-paths.js'; -import { errorEnvelopeFormat, formatErrorEnvelope, requestedFormatFromArgv, render as renderOutput } from '../output.js'; +import { errorEnvelopeFormat, formatErrorEnvelope, requestedFormatFromArgv, requestedMachineFormat, render as renderOutput } from '../output.js'; import { StreamWriteError, writeToStream } from '../stream-write.js'; import { PKG_VERSION } from '../version.js'; import { getCompletionScriptFast } from '../completion-fast.js'; @@ -138,6 +138,13 @@ export async function runHostedCli(argv: string[], opts: HostedRunnerOptions = { return { handled: true, exitCode: EXIT_CODES.USAGE_ERROR }; } if (err instanceof CommanderStructuralError) { + // Usage errors carry their own envelope; honour -f/--format and --json the + // same way the local CLI does instead of falling back to UNKNOWN/exit 1. + const usageFormat = requestedMachineFormat(argv); + if (usageFormat && err.envelope) { + await writeToStream(stderr, formatErrorEnvelope(err.envelope, { fmt: usageFormat })); + return { handled: true, exitCode: err.exitCode }; + } await writeToStream(stderr, err.output); if (err.appendErrorEnvelope) { await writeToStream(stderr, formatErrorEnvelope(toEnvelope(err), { @@ -448,7 +455,14 @@ async function dispatchHosted( await writeHostedHelp(stdout, args, data, renderHostedSiteHelp(manifest, site)); return; } - throw new CommanderCompatibleError(`error: unknown command '${commandName}'\n`, EXIT_CODES.GENERIC_ERROR, undefined, true); + // Same usage-error contract as the local CLI: exit 2 plus the valid + // subcommands for the site, not a trailing UNKNOWN/exit-1 envelope. + const known = hostedCommands(manifest).filter(entry => entry.site === site).map(entry => entry.name); + const help = known.length > 0 ? `help: valid subcommands for \`webcmd ${site}\`: ${known.join(', ')}\n` : ''; + throw new CommanderCompatibleError( + `error: unknown command '${commandName}'\n${help}`, + EXIT_CODES.USAGE_ERROR, + ); } if (isLocalOnlyHostedCommand(command)) { throw new ConfigError( @@ -461,7 +475,9 @@ async function dispatchHosted( parsed = parseHostedInvocation(command, args.slice(2)); } catch (error) { if (error instanceof CommanderStructuralError) { - throw new CommanderStructuralError(error.output, error.exitCode, true); + // A usage error carries its own envelope; only the legacy fallback needs + // the UNKNOWN envelope appended after the human bytes. + throw new CommanderStructuralError(error.output, error.exitCode, !error.envelope, error.envelope); } throw error; } @@ -1378,7 +1394,17 @@ function parseHostedCompletionSurface( }); } if (shell === undefined) { - throw new CommanderStructuralError("error: missing required argument 'shell'\n", 1, true); + // Hand-written mirror of the local `webcmd completion` usage error, which + // Commander raises there but not here (the shell arg is optional in this + // parse). Same bytes, same code, same exit status. + const message = "missing required argument 'shell'"; + const help = 'usage: webcmd completion [options] '; + throw new CommanderStructuralError( + `error: ${message}\nhelp: ${help}\n`, + EXIT_CODES.USAGE_ERROR, + false, + { ok: false, error: { code: 'MISSING_ARGUMENT', message, help, exitCode: EXIT_CODES.USAGE_ERROR } }, + ); } return { kind: 'run', shell }; } diff --git a/src/output.ts b/src/output.ts index de2b3f5b..5edbc1bf 100644 --- a/src/output.ts +++ b/src/output.ts @@ -110,6 +110,19 @@ export function requestedFormatFromArgv(argv: readonly string[]): string | undef return format; } +/** + * Machine format explicitly requested on the command line, if any. + * + * Structural failures render as an envelope only when the caller asked for a + * machine format; humans keep the short `error:`/`help:` lines. + */ +export function requestedMachineFormat(argv: readonly string[]): 'json' | 'yaml' | undefined { + const requested = requestedFormatFromArgv(argv)?.trim().toLowerCase(); + if (requested === 'json') return 'json'; + if (requested === 'yaml' || requested === 'yml') return 'yaml'; + return undefined; +} + /** Serialize the local error envelope without writing to process-global stderr. */ export function formatErrorEnvelope(envelope: ErrorEnvelope, opts: ErrorRenderOptions = {}): string { const repaired = withAdapterRepairHelp(envelope, opts.cmdName);