diff --git a/src/index.ts b/src/index.ts index e2cf663..5c7f5ce 100644 --- a/src/index.ts +++ b/src/index.ts @@ -11,7 +11,7 @@ import { createProjectCommand } from './commands/project.js'; import { createTestCommand } from './commands/test.js'; import { createUsageCommand } from './commands/usage.js'; import { ApiError, CLIError, RequestTimeoutError } from './lib/errors.js'; -import { Output, isOutputMode } from './lib/output.js'; +import { Output, isOutputMode, type OutputMode } from './lib/output.js'; import { rephraseUnknownOption } from './lib/render-error.js'; import { maybeEmitSkillNudge } from './lib/skill-nudge.js'; import { VERSION } from './version.js'; @@ -91,12 +91,49 @@ function applyExitOverrideDeep(cmd: Command): void { } applyExitOverrideDeep(program); -program.configureOutput({ - outputError(str, write) { - const rephrased = rephraseUnknownOption(str); - write(rephrased !== null ? `${rephrased}\n` : str); - }, -}); +/** + * Resolve the requested `--output` mode directly from argv. + * + * We cannot rely on `program.opts()` for this: a parse error can be thrown + * before Commander binds the global `--output` option (e.g. an unknown command, + * or `--output` placed after the failing token), yet the error renderer below + * runs at exactly that point. Scanning argv is order-independent and always + * reflects what the caller asked for. + */ +function outputModeFromArgv(argv: readonly string[]): OutputMode { + for (let i = 0; i < argv.length; i++) { + const a = argv[i]; + if (a === '--') break; // end-of-options marker + if (a === '--output') return isOutputMode(argv[i + 1]) ? (argv[i + 1] as OutputMode) : 'text'; + if (a?.startsWith('--output=')) { + const v = a.slice('--output='.length); + return isOutputMode(v) ? v : 'text'; + } + } + return 'text'; +} +const requestedMode = outputModeFromArgv(process.argv.slice(2)); + +// Commander writes parse errors (unknown option/command, missing argument, +// excess arguments) via `outputError` BEFORE throwing the CommanderError our +// catch block sees. In `--output json` mode we suppress that plain-text write +// and let the catch emit a JSON envelope instead, so EVERY error path honors +// the JSON contract a machine consumer relies on. Text mode keeps the friendly +// rephrasing. Applied to every command in the tree because subcommands do not +// inherit the root's output configuration. +function configureErrorOutput(cmd: Command): void { + cmd.configureOutput({ + outputError(str, write) { + if (requestedMode === 'json') return; // the catch block emits a JSON envelope + const rephrased = rephraseUnknownOption(str); + write(rephrased !== null ? `${rephrased}\n` : str); + }, + }); + for (const child of cmd.commands) { + configureErrorOutput(child); + } +} +configureErrorOutput(program); /** * Render a leaf command's full path (group + leaf), e.g. `test run` / @@ -213,6 +250,22 @@ try { ) { process.exit(0); } + // JSON mode: the plain-text write was suppressed in `configureErrorOutput`, + // so emit a structured envelope here. Parse errors are the VALIDATION_ERROR + // family (exit 5). `err.message` is the bare reason (e.g. "unknown option + // '--foo'"); strip any leading "error: " Commander may have prefixed. + if (requestedMode === 'json') { + const envelope = { + error: { + code: 'VALIDATION_ERROR', + message: err.message.replace(/^error:\s*/i, ''), + nextAction: 'Run `testsprite --help`, or `testsprite --help`, for usage.', + requestId: 'local', + details: { commanderCode: err.code }, + }, + }; + process.stderr.write(`${JSON.stringify(envelope, null, 2)}\n`); + } process.exit(5); } if (err instanceof CLIError) { diff --git a/test/cli.subprocess.test.ts b/test/cli.subprocess.test.ts index 8cbfa59..704fc7d 100644 --- a/test/cli.subprocess.test.ts +++ b/test/cli.subprocess.test.ts @@ -499,6 +499,59 @@ describe('project list subprocess', () => { }, 30_000); }); +describe('--output json honors the JSON contract for argument-parse errors too', () => { + // Previously Commander parse errors (unknown command/option, missing + // argument) printed plain text even under --output json, so a machine + // consumer that JSON.parse()'d stderr crashed on a malformed invocation. + // Now every error path emits a structured envelope. + + it('unknown command emits a JSON VALIDATION_ERROR envelope (exit 5)', async () => { + const result = await runCli(['--output', 'json', 'frobnicate'], {}); + expect(result.exitCode).toBe(5); + const parsed = JSON.parse(result.stderr) as { + error: { code: string; message: string; details: { commanderCode: string } }; + }; + expect(parsed.error.code).toBe('VALIDATION_ERROR'); + expect(parsed.error.message).toContain('frobnicate'); + expect(parsed.error.details.commanderCode).toBe('commander.unknownCommand'); + }, 30_000); + + it('unknown option on a subcommand emits JSON (deep config, not just the root)', async () => { + const result = await runCli(['--output', 'json', 'project', 'list', '--bogus'], {}); + expect(result.exitCode).toBe(5); + const parsed = JSON.parse(result.stderr) as { error: { code: string } }; + expect(parsed.error.code).toBe('VALIDATION_ERROR'); + }, 30_000); + + it('missing required argument on a nested subcommand emits JSON', async () => { + const result = await runCli(['--output', 'json', 'test', 'code', 'get'], {}); + expect(result.exitCode).toBe(5); + const parsed = JSON.parse(result.stderr) as { error: { message: string } }; + expect(parsed.error.message).toContain('test-id'); + }, 30_000); + + it('resolves --output even when it follows the failing token', async () => { + // The mode is scanned from argv, so json applies even though Commander + // errors on the unknown command before it would bind the global flag. + const result = await runCli(['frobnicate', '--output', 'json'], {}); + expect(result.exitCode).toBe(5); + expect(() => JSON.parse(result.stderr)).not.toThrow(); + }, 30_000); + + it('text mode (no --output) still prints the plain-text parse error', async () => { + const result = await runCli(['frobnicate'], {}); + expect(result.exitCode).toBe(5); + expect(result.stderr).toContain("unknown command 'frobnicate'"); + expect(result.stderr.trimStart().startsWith('{')).toBe(false); + }, 30_000); + + it('--help still exits 0 with human-readable text under --output json', async () => { + const result = await runCli(['--output', 'json', '--help'], {}); + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('Usage: testsprite'); + }, 30_000); +}); + describe('project get subprocess', () => { it('--output json returns the ยง6.1 Project shape', async () => { const result = await runCli(['--output', 'json', 'project', 'get', 'project_subproc'], {