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
4 changes: 2 additions & 2 deletions src/commands/agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Command } from 'commander';
import type { CommonOptions as FactoryCommonOptions } from '../lib/client-factory.js';
import { CLIError, localValidationError } from '../lib/errors.js';
import type { OutputMode } from '../lib/output.js';
import { GLOBAL_OPTS_HINT, Output } from '../lib/output.js';
import { GLOBAL_OPTS_HINT, Output, resolveOutputMode } from '../lib/output.js';
import { promptText } from '../lib/prompt.js';
import {
type AgentTarget,
Expand Down Expand Up @@ -757,7 +757,7 @@ function resolveCommonOptions(command: Command): CommonOptions {
const globals = command.optsWithGlobals() as Partial<CommonOptions>;
return {
profile: globals.profile ?? 'default',
output: globals.output ?? 'text',
output: resolveOutputMode(globals.output),
endpointUrl: globals.endpointUrl,
debug: globals.debug ?? false,
verbose: globals.verbose ?? false,
Expand Down
4 changes: 2 additions & 2 deletions src/commands/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
import { loadConfig } from '../lib/config.js';
import { emitDeprecationNotice } from '../lib/deprecate.js';
import type { OutputMode } from '../lib/output.js';
import { GLOBAL_OPTS_HINT, Output } from '../lib/output.js';
import { GLOBAL_OPTS_HINT, Output, resolveOutputMode } from '../lib/output.js';
import { promptSecret } from '../lib/prompt.js';

export interface MeResponse {
Expand Down Expand Up @@ -318,7 +318,7 @@ function resolveCommonOptions(command: Command): CommonOptions {
};
return {
profile: globals.profile ?? 'default',
output: globals.output ?? 'text',
output: resolveOutputMode(globals.output),
endpointUrl: globals.endpointUrl,
debug: globals.debug ?? false,
verbose: globals.verbose ?? false,
Expand Down
4 changes: 2 additions & 2 deletions src/commands/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { Command } from 'commander';
import type { CommonOptions as FactoryCommonOptions } from '../lib/client-factory.js';
import { emitDeprecationNotice } from '../lib/deprecate.js';
import { CLIError } from '../lib/errors.js';
import { GLOBAL_OPTS_HINT, Output } from '../lib/output.js';
import { GLOBAL_OPTS_HINT, Output, resolveOutputMode } from '../lib/output.js';
import type { AuthDeps, MeResponse } from './auth.js';
import { runConfigure, runWhoami } from './auth.js';
import type { AgentDeps, AgentFs, InstallResult } from './agent.js';
Expand Down Expand Up @@ -391,7 +391,7 @@ function resolveCommonOptions(command: Command): CommonOptions {
};
return {
profile: globals.profile ?? 'default',
output: globals.output ?? 'text',
output: resolveOutputMode(globals.output),
endpointUrl: globals.endpointUrl,
debug: globals.debug ?? false,
verbose: globals.verbose ?? false,
Expand Down
8 changes: 2 additions & 6 deletions src/commands/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
import { ApiError } from '../lib/errors.js';
import type { FetchImpl } from '../lib/http.js';
import type { HttpClient } from '../lib/http.js';
import { GLOBAL_OPTS_HINT, Output, type OutputMode } from '../lib/output.js';
import { GLOBAL_OPTS_HINT, Output, resolveOutputMode, type OutputMode } from '../lib/output.js';
import { assertNotLocal } from '../lib/target-url.js';
import { assertIdempotencyKey } from '../lib/validate.js';
import {
Expand Down Expand Up @@ -488,13 +488,9 @@ function resolveCommonOptions(command: Command): CommonOptions {
requestTimeout?: string;
};
// P2-8: validate --output before allowing silent fallback to 'text'.
const rawOutput = globals.output;
if (rawOutput !== undefined && rawOutput !== 'json' && rawOutput !== 'text') {
throw localValidationError('--output must be one of: json, text');
}
return {
profile: globals.profile ?? 'default',
output: (globals.output as OutputMode | undefined) ?? 'text',
output: resolveOutputMode(globals.output),
endpointUrl: globals.endpointUrl,
debug: globals.debug ?? false,
verbose: globals.verbose ?? false,
Expand Down
8 changes: 2 additions & 6 deletions src/commands/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import {
import { REQUEST_TIMEOUT_DEFAULT_MS, REQUEST_TIMEOUT_MAX_MS } from '../lib/http.js';
import type { FetchImpl } from '../lib/http.js';
import type { HttpClient } from '../lib/http.js';
import { GLOBAL_OPTS_HINT, Output, type OutputMode } from '../lib/output.js';
import { GLOBAL_OPTS_HINT, Output, resolveOutputMode, type OutputMode } from '../lib/output.js';
import {
fetchSinglePage,
paginate,
Expand Down Expand Up @@ -7670,13 +7670,9 @@ function resolveCommonOptions(command: Command): CommonOptions {
// P2-8: validate --output before allowing silent fallback to 'text'.
// An invalid value (e.g. `--output yaml`) must exit 5 with a clear error
// rather than silently treating the request as text mode.
const rawOutput = globals.output;
if (rawOutput !== undefined && rawOutput !== 'json' && rawOutput !== 'text') {
throw localValidationError('output', 'must be one of: json, text', ['json', 'text']);
}
return {
profile: globals.profile ?? 'default',
output: (globals.output as OutputMode | undefined) ?? 'text',
output: resolveOutputMode(globals.output),
dryRun: globals.dryRun ?? false,
endpointUrl: globals.endpointUrl,
debug: globals.debug ?? false,
Expand Down
4 changes: 2 additions & 2 deletions src/commands/usage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
import { loadConfig } from '../lib/config.js';
import { resolvePortalBase } from '../lib/facade.js';
import type { FetchImpl } from '../lib/http.js';
import { GLOBAL_OPTS_HINT, Output, type OutputMode } from '../lib/output.js';
import { GLOBAL_OPTS_HINT, Output, resolveOutputMode, type OutputMode } from '../lib/output.js';

/**
* Usage/balance response from `/me` (when the backend supplies it) or a future
Expand Down Expand Up @@ -216,7 +216,7 @@ function resolveCommonOptions(command: Command): CommonOptions {
};
return {
profile: globals.profile ?? 'default',
output: globals.output ?? 'text',
output: resolveOutputMode(globals.output),
endpointUrl: globals.endpointUrl,
debug: globals.debug ?? false,
verbose: globals.verbose ?? false,
Expand Down
33 changes: 32 additions & 1 deletion src/lib/output.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { Output, isOutputMode } from './output.js';
import { Output, isOutputMode, resolveOutputMode } from './output.js';
import { ApiError } from './errors.js';

describe('isOutputMode', () => {
it('accepts json and text', () => {
Expand All @@ -15,6 +16,36 @@ describe('isOutputMode', () => {
});
});

describe('resolveOutputMode', () => {
it('returns the mode verbatim for valid values', () => {
expect(resolveOutputMode('json')).toBe('json');
expect(resolveOutputMode('text')).toBe('text');
});

it('defaults to text when the flag is omitted (undefined)', () => {
expect(resolveOutputMode(undefined)).toBe('text');
});

it('throws a typed VALIDATION_ERROR (exit 5) instead of silently falling back to text', () => {
// The footgun this guards against: an agent that asks for `--output json`
// but mistypes it would otherwise receive a text payload and fail to parse
// it as JSON with no signal. Every command group must reject, not coerce.
for (const bad of ['josn', 'yaml', 'JSON', 'Text', '']) {
let caught: unknown;
try {
resolveOutputMode(bad);
} catch (err) {
caught = err;
}
expect(caught).toBeInstanceOf(ApiError);
const apiErr = caught as ApiError;
expect(apiErr.code).toBe('VALIDATION_ERROR');
expect(apiErr.exitCode).toBe(5);
expect(apiErr.nextAction).toContain('must be one of: json, text');
}
});
});

describe('Output', () => {
let logSpy: ReturnType<typeof vi.spyOn>;
let errorSpy: ReturnType<typeof vi.spyOn>;
Expand Down
22 changes: 22 additions & 0 deletions src/lib/output.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { localValidationError } from './errors.js';

export type OutputMode = 'json' | 'text';

/**
Expand All @@ -13,6 +15,26 @@ export function isOutputMode(value: unknown): value is OutputMode {
return value === 'json' || value === 'text';
}

/**
* Resolve a raw `--output` flag value to a concrete {@link OutputMode}.
*
* `undefined` (flag omitted) resolves to the default `'text'`. Any other
* value that is not `'json'` or `'text'` throws a typed VALIDATION_ERROR
* (exit 5) with an actionable message.
*
* The alternative — silently falling back to `'text'` — is a footgun for the
* CLI's primary consumer (coding agents): a caller that asks for
* `--output json` but mistypes it (`--output josn`) would otherwise receive a
* human-readable text payload and fail to parse it as JSON, with no signal as
* to why. Every command group routes its global-option resolution through this
* helper so the validation is uniform.
*/
export function resolveOutputMode(raw: unknown): OutputMode {
if (raw === undefined) return 'text';
if (raw === 'json' || raw === 'text') return raw;
throw localValidationError('output', 'must be one of: json, text', ['json', 'text']);
}

export interface OutputStreams {
/**
* Line-oriented stdout writer. Each call is one logical line; the
Expand Down
27 changes: 27 additions & 0 deletions test/cli.subprocess.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,33 @@ describe('project list subprocess', () => {
}, 30_000);
});

describe('invalid --output is rejected uniformly (exit 5)', () => {
// Regression: previously only `test` and `project` validated `--output`;
// `auth`, `usage`, `agent`, and `init` silently coerced an unknown value to
// text mode. An agent that asked for `--output json` but mistyped it then
// received a text payload it could not parse, with no signal as to why. Every
// command group now routes through resolveOutputMode (exit 5 on bad input).

// Note: when `--output` itself is the invalid value, the requested mode is
// unusable for the error envelope, so it is rendered in text mode (the catch
// block in index.ts falls back to text for an unrecognised --output).

it('agent list --output josn exits 5 with an actionable message (offline command)', async () => {
const result = await runCli(['--output', 'josn', 'agent', 'list'], {});
expect(result.exitCode).toBe(5);
expect(result.stderr).toContain('must be one of: json, text');
}, 30_000);

it('auth status --output yaml exits 5 before any network call', async () => {
const result = await runCli(['--output', 'yaml', 'auth', 'status'], {
TESTSPRITE_API_KEY: 'sk-subproc',
TESTSPRITE_API_URL: baseUrl,
});
expect(result.exitCode).toBe(5);
expect(result.stderr).toContain('must be one of: json, text');
}, 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'], {
Expand Down