fix(cli): emit JSON envelope for Commander parse errors under --output json

src/index.ts

@@ -12,7 +12,7 @@ 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 { rephraseUnknownOption } from './lib/render-error.js';
+import { renderCommanderError, rephraseUnknownOption } from './lib/render-error.js';
 import { maybeEmitSkillNudge } from './lib/skill-nudge.js';
 import { VERSION } from './version.js';
 
@@ -77,27 +77,32 @@ program.addCommand(createTestCommand());
 program.addCommand(createAgentCommand({}));
 program.addCommand(createUsageCommand());
 
-// Propagate exitOverride to every subcommand in the tree.
-// Commander's addCommand() does NOT inherit exitOverride from the parent,
-// so commands built externally (createTestCommand, createProjectCommand, …)
-// and attached via addCommand() still have _exitCallback = null and call
-// process.exit directly. Recursively set exitOverride so CommanderError
-// bubbles up to our catch block for every leaf subcommand.
+// Buffer Commander error messages instead of writing immediately. The catch
+// block re-emits in the correct format (JSON or text) once the requested
+// output mode is known. Safe because applyExitOverrideDeep ensures every
+// command throws CommanderError rather than calling process.exit directly.
+let pendingCommanderErrorMsg: string | null = null;
+
+// Propagate exitOverride AND the buffered outputError config to every
+// subcommand in the tree. Commander's addCommand() does NOT inherit either
+// from the parent, so commands built externally (createTestCommand, etc.) and
+// attached via addCommand() still have _exitCallback = null and a default
+// outputError that writes immediately. Both must be applied after the full
+// command tree is assembled so every leaf subcommand behaves consistently.
 function applyExitOverrideDeep(cmd: Command): void {
   cmd.exitOverride();
+  cmd.configureOutput({
+    outputError(str, _write) {
+      const rephrased = rephraseUnknownOption(str);
+      pendingCommanderErrorMsg = rephrased !== null ? `${rephrased}\n` : str;
+    },
+  });
   for (const child of cmd.commands) {
     applyExitOverrideDeep(child);
   }
 }
 applyExitOverrideDeep(program);
 
-program.configureOutput({
-  outputError(str, write) {
-    const rephrased = rephraseUnknownOption(str);
-    write(rephrased !== null ? `${rephrased}\n` : str);
-  },
-});
-
 /**
  * Render a leaf command's full path (group + leaf), e.g. `test run` /
  * `auth whoami`, by walking parents up to (but not including) the root program.
@@ -192,9 +197,8 @@ try {
     process.exit(err.exitCode);
   }
   if (err instanceof CommanderError) {
-    // Commander already wrote the error message (via configureOutput) or the
-    // help/version text to stdout. Map exit codes per the CLI taxonomy:
-    //   help / version  → 0  (user asked for it — no error)
+    // Map exit codes per the CLI taxonomy:
+    //   help / version  → 0  (user asked for it; Commander already wrote the text)
     //   parse errors    → 5  (VALIDATION_ERROR family: missing arg, invalid
     //                         option, unknown command, etc.)
     //
@@ -213,6 +217,23 @@ try {
     ) {
       process.exit(0);
     }
+    // For parse errors, write the buffered message in the correct format.
+    // rawMode from program.opts() is reliable when --output was parsed before
+    // the error. When the error occurs first (e.g. `testsprite badcmd --output
+    // json`), rawMode is the default 'text'; scan argv as a best-effort
+    // fallback so machine consumers still receive a JSON envelope.
+    const commanderMode = (() => {
+      if (rawMode === 'json') return 'json' as const;
+      for (let i = 2; i < process.argv.length; i++) {
+        const arg = process.argv[i]!;
+        if (arg === '--output' && process.argv[i + 1] === 'json') return 'json' as const;
+        if (arg === '--output=json') return 'json' as const;
+      }
+      return mode;
+    })();
+    process.stderr.write(
+      renderCommanderError(pendingCommanderErrorMsg, err.message, commanderMode),
+    );
     process.exit(5);
   }
   if (err instanceof CLIError) {

src/lib/render-error.test.ts

@@ -7,7 +7,7 @@
  * subcommand name.
  */
 import { describe, expect, it } from 'vitest';
-import { rephraseUnknownOption } from './render-error.js';
+import { renderCommanderError, rephraseUnknownOption } from './render-error.js';
 
 describe('rephraseUnknownOption', () => {
   it('rephrases --dry-run placed after subcommand', () => {
@@ -88,3 +88,60 @@ describe('rephraseUnknownOption', () => {
     expect(result).toContain('testsprite --endpoint-url <value> <subcommand>');
   });
 });
+
+describe('renderCommanderError', () => {
+  it('json mode: emits VALIDATION_ERROR envelope', () => {
+    const out = renderCommanderError("error: unknown command 'foo'\n", 'fallback', 'json');
+    const parsed = JSON.parse(out) as {
+      error: { code: string; message: string; requestId: string; nextAction: string };
+    };
+    expect(parsed.error.code).toBe('VALIDATION_ERROR');
+    expect(parsed.error.message).toBe("error: unknown command 'foo'");
+    expect(parsed.error.requestId).toBe('local');
+    expect(typeof parsed.error.nextAction).toBe('string');
+    expect(parsed.error.nextAction.length).toBeGreaterThan(0);
+  });
+
+  it('json mode: uses fallback when pendingMsg is null', () => {
+    const out = renderCommanderError(null, 'missing required argument', 'json');
+    const parsed = JSON.parse(out) as { error: { message: string } };
+    expect(parsed.error.message).toBe('missing required argument');
+  });
+
+  it('json mode: trims trailing newline from message', () => {
+    const out = renderCommanderError('error: bad option\n', 'bad', 'json');
+    const parsed = JSON.parse(out) as { error: { message: string } };
+    expect(parsed.error.message).toBe('error: bad option');
+  });
+
+  it('json mode: output is valid JSON ending with newline', () => {
+    const out = renderCommanderError('error: something', 'fallback', 'json');
+    expect(out.endsWith('\n')).toBe(true);
+    expect(() => JSON.parse(out)).not.toThrow();
+  });
+
+  it('text mode: returns pendingMsg as-is', () => {
+    const out = renderCommanderError('error: bad command\n', 'bad', 'text');
+    expect(out).toBe('error: bad command\n');
+  });
+
+  it('text mode: synthesizes from fallback when pendingMsg is null', () => {
+    const out = renderCommanderError(null, 'missing required argument', 'text');
+    expect(out).toContain('missing required argument');
+  });
+
+  it('json mode: rephrased global-flag message is embedded in envelope', () => {
+    const rephrased = rephraseUnknownOption("error: unknown option '--output'")!;
+    const out = renderCommanderError(`${rephrased}\n`, 'unknown option', 'json');
+    const parsed = JSON.parse(out) as { error: { code: string; message: string } };
+    expect(parsed.error.code).toBe('VALIDATION_ERROR');
+    expect(parsed.error.message).toContain('--output');
+    expect(parsed.error.message).toContain('global flag');
+  });
+
+  it('json mode: envelope has exactly the expected top-level key', () => {
+    const out = renderCommanderError('error: test', 'test', 'json');
+    const parsed = JSON.parse(out) as Record<string, unknown>;
+    expect(Object.keys(parsed)).toEqual(['error']);
+  });
+});

src/lib/render-error.ts

@@ -4,6 +4,7 @@
  * Extracted so the output-interceptor rephrasing logic (P10) is
  * unit-testable without spawning a full CLI process.
  */
+import type { OutputMode } from './output.js';
 
 /**
  * Global flags that belong before the subcommand, not after it.
@@ -29,6 +30,44 @@ const GLOBAL_FLAG_ARITY: Record<string, 'boolean' | 'value'> = {
  * it is an ordinary unknown flag that should fall through to the original
  * Commander output.
  */
+/**
+ * Format a Commander parse-error message for the requested output mode.
+ * Returns the string to write to stderr; the caller writes it.
+ *
+ * pendingMsg: message captured by configureOutput.outputError before the
+ *             CommanderError was thrown (may already be rephrased by
+ *             rephraseUnknownOption). Null only when Commander threw without
+ *             first calling outputError (should not happen for parse errors).
+ * fallbackMsg: err.message from CommanderError, used when pendingMsg is null.
+ * mode: the output mode resolved from --output (or argv fallback).
+ */
+export function renderCommanderError(
+  pendingMsg: string | null,
+  fallbackMsg: string,
+  mode: OutputMode,
+): string {
+  const rawMsg = (pendingMsg ?? fallbackMsg).trim();
+  if (mode === 'json') {
+    return (
+      JSON.stringify(
+        {
+          error: {
+            code: 'VALIDATION_ERROR',
+            message: rawMsg,
+            nextAction: 'Run testsprite --help or testsprite <command> --help for usage.',
+            requestId: 'local',
+          },
+        },
+        null,
+        2,
+      ) + '\n'
+    );
+  }
+  // Text mode: emit the buffered message as-is (already formatted by Commander
+  // or rephrased by rephraseUnknownOption). Synthesize when null.
+  return pendingMsg ?? `${rawMsg}\n`;
+}
+
 export function rephraseUnknownOption(raw: string): string | null {
   // Commander emits: "error: unknown option '--foo'"
   const match = /unknown option\s+'--([^']+)'/.exec(raw);

test/cli.subprocess.test.ts

@@ -1110,4 +1110,64 @@ describe('[fix-5] Commander parse errors → exit 5; help/version → exit 0', (
     expect(result.exitCode).toBe(0);
     expect(result.stdout.trim()).toBeTruthy(); // version string on stdout
   }, 30_000);
+
+  it('--output json: missing required arg emits JSON VALIDATION_ERROR envelope, not plain text', async () => {
+    // `test result` requires a positional <test-id>. With --output json, the
+    // CommanderError must be rendered as a machine-readable JSON envelope so
+    // a coding agent parsing stderr does not receive an unexpected plain-text
+    // error and crash its JSON.parse.
+    const result = await runCli(['--output', 'json', 'test', 'result'], {
+      TESTSPRITE_API_KEY: 'sk-subproc',
+      TESTSPRITE_API_URL: baseUrl,
+    });
+    expect(result.exitCode).toBe(5);
+    let parsed: { error: { code: string; message: string; requestId: string } };
+    expect(() => {
+      parsed = JSON.parse(result.stderr) as typeof parsed;
+    }).not.toThrow();
+    expect(parsed!.error.code).toBe('VALIDATION_ERROR');
+    expect(parsed!.error.requestId).toBe('local');
+    expect(parsed!.error.message).toBeTruthy();
+  }, 30_000);
+
+  it('--output json: unknown subcommand emits JSON envelope (--output before bad arg)', async () => {
+    // --output json appears before the unknown subcommand, so Commander parses
+    // it and program.opts().output is 'json' when the error fires.
+    const result = await runCli(['--output', 'json', 'test', 'not-a-real-subcommand'], {
+      TESTSPRITE_API_KEY: 'sk-subproc',
+      TESTSPRITE_API_URL: baseUrl,
+    });
+    expect(result.exitCode).toBe(5);
+    let parsed: { error: { code: string } };
+    expect(() => {
+      parsed = JSON.parse(result.stderr) as typeof parsed;
+    }).not.toThrow();
+    expect(parsed!.error.code).toBe('VALIDATION_ERROR');
+  }, 30_000);
+
+  it('--output json after bad arg: argv fallback detects json mode and emits envelope', async () => {
+    // The error fires before --output json is parsed, so program.opts().output
+    // is the default 'text'. The argv fallback scan must still detect json mode.
+    const result = await runCli(['test', 'not-a-real-subcommand', '--output', 'json'], {
+      TESTSPRITE_API_KEY: 'sk-subproc',
+      TESTSPRITE_API_URL: baseUrl,
+    });
+    expect(result.exitCode).toBe(5);
+    let parsed: { error: { code: string } };
+    expect(() => {
+      parsed = JSON.parse(result.stderr) as typeof parsed;
+    }).not.toThrow();
+    expect(parsed!.error.code).toBe('VALIDATION_ERROR');
+  }, 30_000);
+
+  it('text mode: Commander parse error still emits plain text (no regression)', async () => {
+    const result = await runCli(['test', 'result'], {
+      TESTSPRITE_API_KEY: 'sk-subproc',
+      TESTSPRITE_API_URL: baseUrl,
+    });
+    expect(result.exitCode).toBe(5);
+    // Must NOT be a JSON envelope in text mode.
+    expect(() => JSON.parse(result.stderr)).toThrow();
+    expect(result.stderr).toContain('test-id');
+  }, 30_000);
 });