diff --git a/README.md b/README.md index 782e04c..a15da93 100644 --- a/README.md +++ b/README.md @@ -76,6 +76,26 @@ traceroot findings list --detector --since 7d --json | jq '.data[] traceroot findings get --trace 99224be337d725fd5e8f2e7b45dc22ef ``` +### Exit codes + +Every command exits with a class-specific code so scripts can branch on the kind +of failure — retry a network blip, re-authenticate, or give up on a missing +resource — without parsing prose. + +| Code | Class | JSON `code` | Meaning | +| ---- | ----- | ----------- | ------- | +| `0` | success | — | The command completed. | +| `1` | internal | `internal` | Unexpected/internal error (the default when nothing else fits). | +| `2` | usage | `usage` | Invalid arguments or options (bad flag value, unknown agent/skill, missing required input). | +| `3` | auth | `auth` | Authentication required or invalid: HTTP 401/403, or no local credentials. | +| `4` | not_found | `not_found` | The requested resource does not exist (HTTP 404). | +| `5` | network | `network` | Network failure or timeout — transient, so a retry may succeed. | + +On failure the human-readable message goes to stderr as `error: `. Under +`--json` the failure is written to stderr instead as exactly one line — +`{"error":{"code":"","message":""}}` — while stdout stays empty, so a +`jq` pipeline over stdout is never corrupted by an error. + ## Skills & agents Make your coding agent TraceRoot-aware without touching your application source. The diff --git a/src/agents/index.ts b/src/agents/index.ts index 5f74c71..3d320d5 100644 --- a/src/agents/index.ts +++ b/src/agents/index.ts @@ -1,6 +1,6 @@ import { homedir } from "node:os"; import { isAbsolute, relative, sep } from "node:path"; -import { CliError } from "../output.js"; +import { CliError, ExitCode } from "../output.js"; import { claudeAdapter } from "./claude.js"; import { codexAdapter } from "./codex.js"; import { genericAdapter } from "./generic.js"; @@ -19,7 +19,10 @@ export const AGENT_IDS: readonly AgentId[] = ALL_AGENTS.map((a) => a.id); export function requireAgent(id: string): AgentAdapter { const adapter = ALL_AGENTS.find((a) => a.id === id); if (adapter === undefined) { - throw new CliError(`Unknown agent '${id}'. Supported agents: ${AGENT_IDS.join(", ")}.`); + throw new CliError( + `Unknown agent '${id}'. Supported agents: ${AGENT_IDS.join(", ")}.`, + ExitCode.usage, + ); } return adapter; } diff --git a/src/agents/select.ts b/src/agents/select.ts index 2be65ba..7ef7ee0 100644 --- a/src/agents/select.ts +++ b/src/agents/select.ts @@ -1,4 +1,4 @@ -import { CliError } from "../output.js"; +import { CliError, ExitCode } from "../output.js"; import { type Prompt, dim, isInteractive, readLine } from "../prompt.js"; import { AGENT_IDS, requireAgent } from "./index.js"; import type { AgentAdapter, AgentId } from "./types.js"; @@ -40,6 +40,7 @@ export async function resolveAgentOrPrompt(input: ResolveAgentInput): Promise"); - throw new CliError(`request to ${base} failed: ${safe}`); + throw new CliError(`request to ${base} failed: ${safe}`, ExitCode.network); } } @@ -128,7 +146,10 @@ export function createApiClient(opts: ApiClientOptions): ApiClient { // api-key-free message naming the host and the timeout budget. function throwIfTimeout(err: unknown): void { if (opts.timeoutMs !== undefined && err instanceof Error && err.name === "TimeoutError") { - throw new CliError(`request to ${base} timed out after ${opts.timeoutMs / 1000}s`); + throw new CliError( + `request to ${base} timed out after ${opts.timeoutMs / 1000}s`, + ExitCode.network, + ); } } @@ -142,7 +163,10 @@ export function createApiClient(opts: ApiClientOptions): ApiClient { } catch { // Ignore unreadable / non-JSON error bodies. } - throw new CliError(detail ?? `request failed with status ${res.status}`); + throw new CliError( + detail ?? `request failed with status ${res.status}`, + exitCodeForStatus(res.status), + ); } /** Reads a JSON body, translating a body-phase timeout into the same message. */ diff --git a/src/cli.ts b/src/cli.ts index 7f58239..0f5d698 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -1,6 +1,6 @@ -import { Command, Option } from "commander"; +import { Command, CommanderError, Option } from "commander"; import { registerCommands } from "./commands/index.js"; -import { colorizeError, handlePipeError, reportError } from "./output.js"; +import { CliError, ExitCode, handlePipeError, reportError } from "./output.js"; import { getVersion } from "./version.js"; export function buildProgram(): Command { @@ -8,10 +8,17 @@ export function buildProgram(): Command { program.name("traceroot").description("TraceRoot command line interface").version(getVersion()); // Drop the implicit `help [command]` subcommand; `-h, --help` already covers it. program.helpCommand(false); - // Color commander's own errors (unknown command/option) the same red as the - // central error handler, so every error message is consistent. + // Make commander throw a CommanderError instead of calling process.exit, so + // its native failures (unknown option/command, missing option argument) reach + // the central handler in run() and follow the standard exit-code and `--json` + // envelope contract. Set before registerCommands so subcommands inherit it. + program.exitOverride(); + // Suppress commander's own error printing: with exitOverride the same message + // travels on the thrown CommanderError and run() reports it exactly once via + // reportError (consistent prose/JSON shape and color). Help output is + // unaffected — it goes through writeOut/writeErr, not outputError. program.configureOutput({ - outputError: (str, write) => write(colorizeError(str)), + outputError: () => {}, }); // Surface program-wide global flags (e.g. `--json`) in every subcommand's // `--help` under a "Global Options" section, so they're discoverable where @@ -53,13 +60,29 @@ export function buildProgram(): Command { .option("--json", "emit machine-readable JSON output for supported commands") .option("--timeout ", "per-request network timeout in milliseconds (default: 30000)"); registerCommands(program); + // Make the exit-code contract discoverable from `traceroot --help` so scripts + // know how to branch on failures (mirrors the README table). + program.addHelpText( + "after", + [ + "", + "Exit codes:", + " 0 success", + " 1 internal unexpected/internal error", + " 2 usage invalid arguments or options", + " 3 auth authentication required or invalid", + " 4 not_found the requested resource does not exist", + " 5 network network failure or timeout (retryable)", + "", + 'Under --json, failures also print {"error":{"code","message"}} to stderr.', + ].join("\n"), + ); // Root action: lets global flags parse without a subcommand, while still // rejecting an unrecognized operand so unknown-command handling is preserved. program.action((_opts, command: Command) => { const operands = command.args; if (operands.length > 0) { - command.error(`error: unknown command '${operands[0]}'`, { exitCode: 1 }); - return; + throw new CliError(`unknown command '${operands[0]}'`, ExitCode.usage); } // No subcommand given: show help and exit non-zero. Help goes to stderr // (per the output contract: human text never pollutes stdout). An explicit @@ -69,6 +92,19 @@ export function buildProgram(): Command { return program; } +/** + * Ends the process with `code` once stderr has drained. The empty write is + * queued behind any pending stderr chunks, so its completion callback fires only + * after the error text has fully reached a pipe/file — then it is safe to call + * `process.exit` without truncation. Exiting explicitly matters: a failed request + * can leave a pending socket (e.g. undici's connect timeout) holding the event + * loop open for seconds after the error was reported. + */ +function exitAfterStderrDrain(code: number): void { + process.exitCode = code; + process.stderr.write("", () => process.exit(code)); +} + export async function run(argv: string[]): Promise { // Exit cleanly when a downstream reader (e.g. `head`, `jq`) closes the pipe: // turn the resulting EPIPE into a quiet exit instead of a Node stack trace. @@ -77,10 +113,29 @@ export async function run(argv: string[]): Promise { try { await buildProgram().parseAsync(argv); } catch (err) { - const code = reportError(err); - process.exitCode = code; - if (code !== 0) { - process.exit(code); + // The central catch has no resolved Context, so detect `--json` straight from + // argv (the accepted approach) to pick the machine-readable error envelope. + const json = argv.includes("--json"); + if (err instanceof CommanderError) { + if (err.exitCode === 0) { + // `--help` / `--version`: normal output was already written; exit 0 + // naturally so stdout flushes on its own. + return; + } + if (err.code === "commander.help") { + // Help was already rendered (e.g. bare `traceroot`); the CommanderError + // message is only a placeholder, so add no error line. + exitAfterStderrDrain(err.exitCode); + return; + } + // A commander-native usage failure (unknown option, missing option + // argument, …). Its own printing is suppressed in buildProgram, so report + // it here exactly once, stripping commander's "error: " prefix (the + // reporter adds its own). + const message = err.message.replace(/^error: /, ""); + exitAfterStderrDrain(reportError(new CliError(message, ExitCode.usage), { json })); + return; } + exitAfterStderrDrain(reportError(err, { json })); } } diff --git a/src/commands/detectors/list.ts b/src/commands/detectors/list.ts index 256e1d6..f334ee1 100644 --- a/src/commands/detectors/list.ts +++ b/src/commands/detectors/list.ts @@ -1,6 +1,13 @@ import type { Command } from "commander"; import type { ApiClient, ListDetectorsParams } from "../../api/client.js"; -import { CliError, type Writers, defaultWriters, logProgress, writeJson } from "../../output.js"; +import { + CliError, + ExitCode, + type Writers, + defaultWriters, + logProgress, + writeJson, +} from "../../output.js"; import { createStyler } from "../../render/style.js"; import { renderTable } from "../../render/table.js"; import { formatTimestamp } from "../../util/index.js"; @@ -129,11 +136,13 @@ export function registerDetectorsList(detectors: Command): void { if (value !== undefined && bareDate.test(value)) { throw new CliError( `unexpected argument(s): ${strayJoined}.\n\nDid you mean to quote the timestamp?\n traceroot detectors list ${flag} "${value} ${strayJoined}"\n\nTimestamps with spaces must be passed as one shell argument.\nISO 8601 also works:\n traceroot detectors list ${flag} 2026-06-23T20:31:02Z\n traceroot detectors list ${flag} 2026-06-23T14:31:02-06:00`, + ExitCode.usage, ); } } throw new CliError( `unexpected argument(s): ${strayJoined}. 'detectors list' takes no positional arguments. If you meant a time filter, --from/--to take a single ISO 8601 timestamp with no spaces, e.g. --from 2026-06-23T14:29:54Z (or with an offset, 2026-06-23T14:29:54-06:00).`, + ExitCode.usage, ); } const opts = command.opts(); diff --git a/src/commands/findings/get.ts b/src/commands/findings/get.ts index 00d5ca2..e8d5df0 100644 --- a/src/commands/findings/get.ts +++ b/src/commands/findings/get.ts @@ -1,6 +1,6 @@ import type { Command } from "commander"; import type { ApiClient, FindingDetail } from "../../api/client.js"; -import { type Writers, CliError, defaultWriters, writeJson } from "../../output.js"; +import { CliError, ExitCode, type Writers, defaultWriters, writeJson } from "../../output.js"; import { createStyler } from "../../render/style.js"; import { formatTimestamp } from "../../util/index.js"; import { contextFromCommand, requireApiClient } from "../shared.js"; @@ -29,10 +29,10 @@ export async function runGet(deps: RunGetDeps): Promise { const hasTrace = traceId !== undefined && traceId.trim() !== ""; if (hasFinding && hasTrace) { - throw new CliError("provide either a finding id or --trace, not both"); + throw new CliError("provide either a finding id or --trace, not both", ExitCode.usage); } if (!hasFinding && !hasTrace) { - throw new CliError("provide a finding id, or --trace "); + throw new CliError("provide a finding id, or --trace ", ExitCode.usage); } const finding = hasFinding @@ -128,6 +128,7 @@ export function registerFindingsGet(findings: Command): void { if (command.args.length > 1) { throw new CliError( `unexpected argument(s): ${command.args.slice(1).join(" ")}. 'findings get' takes a single finding id (or use --trace).`, + ExitCode.usage, ); } const opts = command.opts(); diff --git a/src/commands/findings/list.ts b/src/commands/findings/list.ts index 880fe13..2c9d77d 100644 --- a/src/commands/findings/list.ts +++ b/src/commands/findings/list.ts @@ -1,6 +1,13 @@ import type { Command } from "commander"; import type { ApiClient, ListFindingsParams } from "../../api/client.js"; -import { type Writers, CliError, defaultWriters, logProgress, writeJson } from "../../output.js"; +import { + CliError, + ExitCode, + type Writers, + defaultWriters, + logProgress, + writeJson, +} from "../../output.js"; import { createStyler } from "../../render/style.js"; import { renderTable } from "../../render/table.js"; import { formatTimestamp } from "../../util/index.js"; @@ -144,11 +151,13 @@ export function registerFindingsList(findings: Command): void { if (value !== undefined && bareDate.test(value)) { throw new CliError( `unexpected argument(s): ${strayJoined}.\n\nDid you mean to quote the timestamp?\n traceroot findings list ${flag} "${value} ${strayJoined}"\n\nTimestamps with spaces must be passed as one shell argument.\nISO 8601 also works:\n traceroot findings list ${flag} 2026-06-23T20:31:02Z\n traceroot findings list ${flag} 2026-06-23T14:31:02-06:00`, + ExitCode.usage, ); } } throw new CliError( `unexpected argument(s): ${strayJoined}. 'findings list' takes no positional arguments. If you meant a time filter, --from/--to take a single ISO 8601 timestamp with no spaces, e.g. --from 2026-06-23T14:29:54Z (or with an offset, 2026-06-23T14:29:54-06:00).`, + ExitCode.usage, ); } const opts = command.opts(); diff --git a/src/commands/instrument.ts b/src/commands/instrument.ts index b67cf36..11d6354 100644 --- a/src/commands/instrument.ts +++ b/src/commands/instrument.ts @@ -5,6 +5,7 @@ import { displaySkillPath } from "../agents/index.js"; import { resolveAgentOrPrompt } from "../agents/select.js"; import { CliError, + ExitCode, type Writers, defaultWriters, logInfo, @@ -13,8 +14,8 @@ import { } from "../output.js"; import { confirm, dim, isInteractive, readLine, yellow } from "../prompt.js"; import { buildInstrumentPrompt } from "../prompts/instrumentPrompt.js"; -import { type RepoDetection, detectRepo } from "../repo/detect.js"; import { createStyler } from "../render/style.js"; +import { type RepoDetection, detectRepo } from "../repo/detect.js"; import { formatBytes } from "../util/index.js"; /** Default location for the generated prompt when neither --print nor --output is given. */ @@ -107,6 +108,7 @@ export async function runInstrument(deps: RunInstrumentDeps): Promise { } else { throw new CliError( `Missing required option --output.\nProvide a path to write the prompt, or use --print.\nExample:\n traceroot instrument --agent ${agent.id} --output ${DEFAULT_PROMPT_PATH}`, + ExitCode.usage, ); } diff --git a/src/commands/login.ts b/src/commands/login.ts index 68b7402..c25f300 100644 --- a/src/commands/login.ts +++ b/src/commands/login.ts @@ -8,7 +8,7 @@ import { } from "../api/client.js"; import { writeConfig as realWriteConfig } from "../config/manager.js"; import type { AuthSource } from "../config/resolve.js"; -import { CliError, type Writers, defaultWriters, logInfo, writeJson } from "../output.js"; +import { CliError, ExitCode, type Writers, defaultWriters, logInfo, writeJson } from "../output.js"; import { apiKeyLabel, identity } from "../render/identity.js"; import { createStyler } from "../render/style.js"; import { DEFAULT_HOST } from "./constants.js"; @@ -94,10 +94,10 @@ export async function runLogin(deps: LoginDeps): Promise { } else if (deps.isInteractive) { apiKey = (await deps.promptHidden("API key: ")).trim(); } else { - throw new CliError(MISSING_KEY); + throw new CliError(MISSING_KEY, ExitCode.auth); } if (apiKey === "") { - throw new CliError(MISSING_KEY); + throw new CliError(MISSING_KEY, ExitCode.auth); } let host: string; diff --git a/src/commands/shared.ts b/src/commands/shared.ts index 95d6aad..77148c9 100644 --- a/src/commands/shared.ts +++ b/src/commands/shared.ts @@ -1,7 +1,7 @@ import type { Command } from "commander"; import { type ApiClient, createApiClient } from "../api/client.js"; import { type Context, buildContext } from "../context.js"; -import { CliError } from "../output.js"; +import { CliError, ExitCode } from "../output.js"; /** Build the per-invocation Context from a command's merged (global+local) options. */ export function contextFromCommand(command: Command): Context { @@ -26,11 +26,13 @@ export function requireApiClient(ctx: Context): ApiClient { if (apiKey === undefined) { throw new CliError( "No API key found. Run `traceroot login`, or set TRACEROOT_API_KEY, or pass --api-key.", + ExitCode.auth, ); } if (host === undefined) { throw new CliError( "No host found. Run `traceroot login`, or set TRACEROOT_HOST_URL, or pass --host.", + ExitCode.auth, ); } return createApiClient({ host, apiKey, timeoutMs: ctx.timeoutMs }); diff --git a/src/commands/traces/list.ts b/src/commands/traces/list.ts index 22502ef..9dd9d9b 100644 --- a/src/commands/traces/list.ts +++ b/src/commands/traces/list.ts @@ -2,6 +2,7 @@ import type { Command } from "commander"; import type { ApiClient } from "../../api/client.js"; import { CliError, + ExitCode, type Writers, colorizeError, defaultWriters, @@ -196,6 +197,7 @@ function normalizeTimestamp(raw: string, flag: string, timeZone: string): string if (offH > 14 || offM > 59 || Number.isNaN(d.getTime()) || roundTrip !== `${date}T${time}`) { throw new CliError( `${flag} "${trimmed}": not a valid timestamp. Use ISO 8601, e.g. ${flag} 2026-06-23T17:30:00+05:30.`, + ExitCode.usage, ); } return d.toISOString(); @@ -259,6 +261,7 @@ function normalizeTimestamp(raw: string, flag: string, timeZone: string): string // explicit offset using a generic, valid example instead. throw new CliError( `${flag} "${trimmed}": not a valid local time (invalid date/time, or a nonexistent local time such as a DST gap). Use ISO 8601 with an explicit offset, e.g. ${flag} 2026-06-23T14:31:02-06:00.`, + ExitCode.usage, ); } @@ -275,6 +278,7 @@ function normalizeTimestamp(raw: string, flag: string, timeZone: string): string const suggestion = `${ys}-${ms}-${ds}T${hs}:${mins}:${ss}${sign}${offH}:${offM}`; throw new CliError( `${flag} "${trimmed}": timezone "${abbr}" doesn't match your local timezone (${timeZone} → ${localAbbr}). Use ISO 8601 with an explicit offset instead, e.g. ${flag} ${suggestion}.`, + ExitCode.usage, ); } @@ -296,6 +300,7 @@ function normalizeTimestamp(raw: string, flag: string, timeZone: string): string if (Number.isNaN(date.getTime()) || !hasValidCalendarDate(trimmed)) { throw new CliError( `${flag} must be a valid ISO 8601 timestamp, e.g. 2026-06-01 or 2026-06-01T13:00:00Z`, + ExitCode.usage, ); } return date.toISOString(); @@ -315,12 +320,12 @@ export function resolveTimeRange( ): TimeRange { const { since, from, to } = opts; if (since !== undefined && (from !== undefined || to !== undefined)) { - throw new CliError("--since cannot be combined with --from/--to"); + throw new CliError("--since cannot be combined with --from/--to", ExitCode.usage); } if (since !== undefined) { const startAfter = new Date(now() - parseDuration(since)); if (Number.isNaN(startAfter.getTime())) { - throw new CliError(`--since ${since} is too large`); + throw new CliError(`--since ${since} is too large`, ExitCode.usage); } return { startAfter: startAfter.toISOString(), sinceLabel: since }; } @@ -336,10 +341,11 @@ export function resolveTimeRange( if (range.startAfter === range.endBefore) { throw new CliError( "--from and --to resolve to the same time. The lower bound is inclusive and the upper bound is exclusive, so choose a later time for --to.", + ExitCode.usage, ); } if (range.startAfter > range.endBefore) { - throw new CliError("--from must resolve to an earlier time than --to"); + throw new CliError("--from must resolve to an earlier time than --to", ExitCode.usage); } } return range; @@ -355,11 +361,11 @@ export function parseLimit(raw: string | undefined): number | undefined { return undefined; } if (!/^\d+$/.test(raw)) { - throw new CliError("--limit must be a positive integer"); + throw new CliError("--limit must be a positive integer", ExitCode.usage); } const value = Number.parseInt(raw, 10); if (!Number.isInteger(value) || value < 1) { - throw new CliError("--limit must be a positive integer"); + throw new CliError("--limit must be a positive integer", ExitCode.usage); } return value; } @@ -514,7 +520,7 @@ export async function runList(deps: RunListDeps): Promise { export function onceOption(flag: string): (val: string, prev: string | undefined) => string { return (val: string, prev: string | undefined): string => { if (prev !== undefined) { - throw new CliError(`${flag} may only be given once`); + throw new CliError(`${flag} may only be given once`, ExitCode.usage); } return val; }; @@ -554,16 +560,19 @@ export function registerTracesList(traces: Command): void { const reconstructed = `${fromVal} ${strayJoined}`; throw new CliError( `unexpected argument(s): ${strayJoined}.\n\nDid you mean to quote the timestamp?\n traceroot traces list --from "${reconstructed}"\n\nTimestamps with spaces must be passed as one shell argument.\nISO 8601 also works:\n traceroot traces list --from 2026-06-23T20:31:02Z\n traceroot traces list --from 2026-06-23T14:31:02-06:00`, + ExitCode.usage, ); } if (toVal !== undefined && bareDate.test(toVal)) { const reconstructed = `${toVal} ${strayJoined}`; throw new CliError( `unexpected argument(s): ${strayJoined}.\n\nDid you mean to quote the timestamp?\n traceroot traces list --to "${reconstructed}"\n\nTimestamps with spaces must be passed as one shell argument.\nISO 8601 also works:\n traceroot traces list --to 2026-06-23T20:31:02Z\n traceroot traces list --to 2026-06-23T14:31:02-06:00`, + ExitCode.usage, ); } throw new CliError( `unexpected argument(s): ${strayJoined}. 'traces list' takes no positional arguments. If you meant a time filter, --from/--to take a single ISO 8601 timestamp with no spaces, e.g. --from 2026-06-23T14:29:54Z (or with an offset, 2026-06-23T14:29:54-06:00).`, + ExitCode.usage, ); } const opts = command.opts(); diff --git a/src/context.ts b/src/context.ts index 0396a77..133ea8f 100644 --- a/src/context.ts +++ b/src/context.ts @@ -4,7 +4,7 @@ import { loadEnvFileFromDisk, loadOptionalEnvFileFromDisk } from "./config/envFi import { readConfig } from "./config/manager.js"; import { type ResolvedAuth, resolveAuth } from "./config/resolve.js"; import type { Config } from "./config/schema.js"; -import { CliError } from "./output.js"; +import { CliError, ExitCode } from "./output.js"; /** Global flags parsed by the root program. */ export interface GlobalOptions { @@ -47,7 +47,10 @@ function resolveTimeoutMs(flag: string | undefined, env: NodeJS.ProcessEnv): num // so match the same digits-only rule `--limit` uses. const trimmed = raw.trim(); if (!/^\d+$/.test(trimmed) || Number.parseInt(trimmed, 10) <= 0) { - throw new CliError(`invalid timeout: ${raw} (expected a positive integer of milliseconds)`); + throw new CliError( + `invalid timeout: ${raw} (expected a positive integer of milliseconds)`, + ExitCode.usage, + ); } return Number.parseInt(trimmed, 10); } diff --git a/src/output.ts b/src/output.ts index a95f29f..15b8afc 100644 --- a/src/output.ts +++ b/src/output.ts @@ -70,6 +70,44 @@ export function logWarn(msg: string, w: Writers = defaultWriters): void { w.err.write(`warning: ${msg}\n`); } +/** + * Process exit codes by failure class. This is a stable, script-facing contract: + * scripts can branch on the code to decide whether to retry (network), re-auth + * (auth), give up (not-found), or fix their invocation (usage). Anything not + * classified stays `internal` (1). + */ +export const ExitCode = { + /** Bad invocation: unknown flag/argument, malformed value, missing required input. */ + usage: 2, + /** Authentication/authorization: HTTP 401/403, or missing local credentials. */ + auth: 3, + /** The requested resource does not exist: HTTP 404. */ + notFound: 4, + /** Network failure or timeout — transient; a retry may succeed. */ + network: 5, + /** Unexpected/internal failure. The default when nothing else applies. */ + internal: 1, +} as const; + +/** + * Maps a numeric exit code to its stable string code for the `--json` error + * envelope. Unknown codes (including the default 1) map to `"internal"`. + */ +export function exitCodeToString(exitCode: number): string { + switch (exitCode) { + case ExitCode.usage: + return "usage"; + case ExitCode.auth: + return "auth"; + case ExitCode.notFound: + return "not_found"; + case ExitCode.network: + return "network"; + default: + return "internal"; + } +} + /** An error carrying a process exit code. */ export class CliError extends Error { readonly exitCode: number; @@ -104,12 +142,25 @@ export function handlePipeError( } /** - * Reports an error to stderr (red when color is enabled) without a stack trace - * and returns the exit code (a {@link CliError}'s `exitCode`, else 1). Never - * writes to stdout. + * Reports an error to stderr without a stack trace and returns the exit code (a + * {@link CliError}'s `exitCode`, else {@link ExitCode.internal}). Never writes to + * stdout. In `opts.json` mode a single machine-readable line is written instead + * of prose: `{"error":{"code":"","message":""}}`, where `` is + * the stable string for the exit code. Otherwise the human-readable `error: + * ` line is written (red when color is enabled). */ -export function reportError(err: unknown, w: Writers = defaultWriters): number { +export function reportError( + err: unknown, + opts: { json?: boolean } = {}, + w: Writers = defaultWriters, +): number { const message = err instanceof Error ? err.message : String(err); - w.err.write(`${colorizeError(`error: ${message}`, w.err)}\n`); - return isCliError(err) ? err.exitCode : 1; + const code = isCliError(err) ? err.exitCode : ExitCode.internal; + if (opts.json) { + // One compact line to stderr; stdout stays empty (the output contract). + w.err.write(`${JSON.stringify({ error: { code: exitCodeToString(code), message } })}\n`); + } else { + w.err.write(`${colorizeError(`error: ${message}`, w.err)}\n`); + } + return code; } diff --git a/src/skills/registry.ts b/src/skills/registry.ts index 16c4607..3ad0984 100644 --- a/src/skills/registry.ts +++ b/src/skills/registry.ts @@ -1,4 +1,4 @@ -import { CliError } from "../output.js"; +import { CliError, ExitCode } from "../output.js"; /** The first-party skills the CLI knows how to install. This is the allowlist. */ export type BuiltinSkillName = "traceroot-instrument-repo" | "traceroot-quickstart"; @@ -43,7 +43,10 @@ export function isBuiltinSkillName(name: string): name is BuiltinSkillName { export function requireBuiltinSkill(name: string): BuiltinSkill { const skill = BUILTIN_SKILLS.find((s) => s.name === name); if (skill === undefined) { - throw new CliError(`Unknown skill '${name}'. Choose one of: ${builtinSkillNames()}.`); + throw new CliError( + `Unknown skill '${name}'. Choose one of: ${builtinSkillNames()}.`, + ExitCode.usage, + ); } return skill; } diff --git a/src/skills/select.ts b/src/skills/select.ts index 6a7c936..c0dd29a 100644 --- a/src/skills/select.ts +++ b/src/skills/select.ts @@ -1,9 +1,9 @@ -import { type Writers, CliError, logInfo } from "../output.js"; +import { CliError, ExitCode, type Writers, logInfo } from "../output.js"; import { type Prompt, dim, isInteractive, readLine } from "../prompt.js"; import { createStyler } from "../render/style.js"; import { - type BuiltinSkill, BUILTIN_SKILLS, + type BuiltinSkill, builtinSkillNames, requireBuiltinSkill, } from "./registry.js"; @@ -57,6 +57,7 @@ export async function resolveSkillOrPrompt(input: ResolveSkillInput): Promise.\nChoose one of: ${builtinSkillNames()}.\nExample:\n traceroot skills install traceroot-instrument-repo`, + ExitCode.usage, ); } diff --git a/src/util/index.ts b/src/util/index.ts index 6acaa06..e157543 100644 --- a/src/util/index.ts +++ b/src/util/index.ts @@ -1,4 +1,4 @@ -import { CliError } from "../output.js"; +import { CliError, ExitCode } from "../output.js"; const DURATION_UNITS_MS: Record = { s: 1_000, @@ -19,11 +19,12 @@ export function parseDuration(raw: string): number { if (match === null) { throw new CliError( `invalid duration: "${raw}" (expected a count and unit, e.g. 30m, 6h, 7d, 2w)`, + ExitCode.usage, ); } const count = Number.parseInt(match[1] as string, 10); if (count < 1) { - throw new CliError(`invalid duration: "${raw}" (must be a positive amount)`); + throw new CliError(`invalid duration: "${raw}" (must be a positive amount)`, ExitCode.usage); } return count * (DURATION_UNITS_MS[match[2] as string] as number); } diff --git a/tests/api/client.test.ts b/tests/api/client.test.ts index 6dc0133..36fc421 100644 --- a/tests/api/client.test.ts +++ b/tests/api/client.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; import { createApiClient } from "../../src/api/client.js"; -import { CliError } from "../../src/output.js"; +import { CliError, ExitCode } from "../../src/output.js"; import { createFakeFetch, errorResponse, jsonResponse } from "../helpers/fakeFetch.js"; const API_KEY = "tr_secret_LEAK"; @@ -116,6 +116,63 @@ describe("createApiClient", () => { }); }); +describe("HTTP status → exit-code class", () => { + async function exitCodeOf(status: number): Promise { + const { client } = clientWith(() => errorResponse(status, `status ${status}`)); + const err = await client.whoami().catch((e: unknown) => e); + expect(err).toBeInstanceOf(CliError); + return (err as CliError).exitCode; + } + + it("maps 401 and 403 to the auth exit code", async () => { + expect(await exitCodeOf(401)).toBe(ExitCode.auth); + expect(await exitCodeOf(403)).toBe(ExitCode.auth); + }); + + it("maps 404 to the not-found exit code", async () => { + expect(await exitCodeOf(404)).toBe(ExitCode.notFound); + }); + + it("maps a 500 to the internal exit code", async () => { + expect(await exitCodeOf(500)).toBe(ExitCode.internal); + }); + + it("maps a network failure to the network exit code", async () => { + const { client } = clientWith(() => { + throw new Error("connect ECONNREFUSED"); + }); + const err = await client.whoami().catch((e: unknown) => e); + expect((err as CliError).exitCode).toBe(ExitCode.network); + }); + + it("maps a request timeout to the network exit code", async () => { + const fetchImpl = (() => + Promise.reject(new DOMException("aborted", "TimeoutError"))) as typeof fetch; + const client = createApiClient({ + host: "https://h", + apiKey: API_KEY, + fetchImpl, + timeoutMs: 10, + }); + const err = await client.whoami().catch((e: unknown) => e); + expect(err).toBeInstanceOf(CliError); + expect((err as CliError).exitCode).toBe(ExitCode.network); + }); + + it("rejects a bad host at construction with the usage exit code", () => { + const fake = createFakeFetch(() => jsonResponse({})); + const err = ((): unknown => { + try { + createApiClient({ host: "not a url", apiKey: API_KEY, fetchImpl: fake.fetchImpl }); + } catch (e) { + return e; + } + })(); + expect(err).toBeInstanceOf(CliError); + expect((err as CliError).exitCode).toBe(ExitCode.usage); + }); +}); + describe("detector findings", () => { it("sends list filters as query params", async () => { const { client, calls } = clientWith(() => jsonResponse({ data: [], meta: {} })); diff --git a/tests/output.contract.test.ts b/tests/output.contract.test.ts index 9505f5f..424765e 100644 --- a/tests/output.contract.test.ts +++ b/tests/output.contract.test.ts @@ -1,4 +1,5 @@ import { spawnSync } from "node:child_process"; +import { mkdtempSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { fileURLToPath } from "node:url"; @@ -16,10 +17,16 @@ const isolatedEnv: NodeJS.ProcessEnv = { TRACEROOT_HOST_URL: "", }; +// A fresh empty working directory so the CLI's auto-discovered `.env` (a +// lowest-precedence credential source) can never pick up a developer's stray +// repo `.env` and flip a missing-credentials failure into a network one. +const isolatedCwd = mkdtempSync(join(tmpdir(), "traceroot-cli-contract-")); + function runIsolated(...args: string[]): { stdout: string; stderr: string; status: number | null } { const result = spawnSync(process.execPath, [binPath, ...args], { encoding: "utf8", env: isolatedEnv, + cwd: isolatedCwd, }); return { stdout: result.stdout, stderr: result.stderr, status: result.status }; } @@ -45,3 +52,100 @@ describe("output contract (spawned failing command)", () => { expect(stderr).not.toContain("\x1b["); }); }); + +describe("exit-code classes (spawned)", () => { + it("exits 2 (usage) on a validation error", () => { + // Bad --limit is a pure usage error, so it never needs credentials. + const { stdout, stderr, status } = runIsolated("traces", "list", "--limit", "banana"); + expect(status).toBe(2); + expect(stdout).toBe(""); + expect(stderr).toMatch(/^error: /); + }); + + it("exits 3 (auth) when credentials are missing", () => { + const { stdout, stderr, status } = runIsolated("status"); + expect(status).toBe(3); + expect(stdout).toBe(""); + expect(stderr).not.toBe(""); + }); + + it("human-mode stderr starts with `error: `", () => { + const { stderr } = runIsolated("status"); + expect(stderr.startsWith("error: ")).toBe(true); + }); + + it("under --json emits a single parseable error envelope to stderr, stdout empty", () => { + const { stdout, stderr, status } = runIsolated("--json", "status"); + expect(status).toBe(3); + expect(stdout).toBe(""); + // Exactly one line. + expect(stderr.trimEnd().includes("\n")).toBe(false); + const parsed = JSON.parse(stderr) as { error: { code: string; message: string } }; + expect(parsed.error.code).toBe("auth"); + expect(typeof parsed.error.message).toBe("string"); + expect(parsed.error.message).not.toBe(""); + }); + + it("under --json a usage error carries the `usage` code", () => { + const { stdout, stderr, status } = runIsolated("--json", "traces", "list", "--limit", "banana"); + expect(status).toBe(2); + expect(stdout).toBe(""); + const parsed = JSON.parse(stderr) as { error: { code: string } }; + expect(parsed.error.code).toBe("usage"); + }); +}); + +describe("commander-native failures follow the exit-code contract (spawned)", () => { + it("exits 2 with one `error: ` line on an unknown option", () => { + const { stdout, stderr, status } = runIsolated("traces", "list", "--bogusflag"); + expect(status).toBe(2); + expect(stdout).toBe(""); + expect(stderr).toMatch(/^error: unknown option '--bogusflag'/); + // Reported exactly once (no commander double-print). + expect(stderr.match(/unknown option/g)).toHaveLength(1); + }); + + it("exits 2 on an unknown command", () => { + const { stdout, stderr, status } = runIsolated("boguscmd"); + expect(status).toBe(2); + expect(stdout).toBe(""); + expect(stderr).toMatch(/^error: unknown command 'boguscmd'/); + }); + + it("exits 2 when an option's argument is missing", () => { + const { stdout, stderr, status } = runIsolated("traces", "list", "--limit"); + expect(status).toBe(2); + expect(stdout).toBe(""); + expect(stderr).toMatch(/^error: .*--limit.*argument missing/); + }); + + it("under --json an unknown option yields a single `usage` envelope on stderr", () => { + const { stdout, stderr, status } = runIsolated("--json", "traces", "list", "--bogusflag"); + expect(status).toBe(2); + expect(stdout).toBe(""); + expect(stderr.trimEnd().includes("\n")).toBe(false); + const parsed = JSON.parse(stderr) as { error: { code: string; message: string } }; + expect(parsed.error.code).toBe("usage"); + expect(parsed.error.message).toContain("--bogusflag"); + }); + + it("keeps --help exiting 0 with help on stdout", () => { + const { stdout, stderr, status } = runIsolated("--help"); + expect(status).toBe(0); + expect(stdout).toContain("Usage: traceroot"); + expect(stderr).toBe(""); + }); + + it("keeps subcommand --help exiting 0 with help on stdout", () => { + const { stdout, status } = runIsolated("traces", "list", "--help"); + expect(status).toBe(0); + expect(stdout).toContain("Usage: traceroot traces list"); + }); + + it("keeps --version exiting 0 with the version on stdout", () => { + const { stdout, stderr, status } = runIsolated("--version"); + expect(status).toBe(0); + expect(stdout.trim()).not.toBe(""); + expect(stderr).toBe(""); + }); +}); diff --git a/tests/output.test.ts b/tests/output.test.ts index 1f32d92..035d8c1 100644 --- a/tests/output.test.ts +++ b/tests/output.test.ts @@ -1,8 +1,10 @@ import { describe, expect, it } from "vitest"; import { CliError, + ExitCode, type Writers, colorEnabled, + exitCodeToString, handlePipeError, isCliError, logInfo, @@ -83,7 +85,7 @@ describe("log helpers", () => { describe("reportError", () => { it("returns the CliError exit code and writes the message to err with no stack", () => { const { w, out, err } = writers(); - const code = reportError(new CliError("boom", 2), w); + const code = reportError(new CliError("boom", 2), {}, w); expect(code).toBe(2); expect(err.data).toContain("boom"); expect(err.data).not.toContain("at "); @@ -92,7 +94,67 @@ describe("reportError", () => { it("returns 1 for a plain Error", () => { const { w } = writers(); - expect(reportError(new Error("plain"), w)).toBe(1); + expect(reportError(new Error("plain"), {}, w)).toBe(1); + }); + + it("writes human prose (not JSON) by default", () => { + const { w, err } = writers(); + reportError(new CliError("nope", 2), {}, w); + expect(err.data).toBe("error: nope\n"); + }); + + describe("json envelope", () => { + it("writes exactly one parseable line to err with the class code and message", () => { + const { w, out, err } = writers(); + const code = reportError(new CliError("no key", ExitCode.auth), { json: true }, w); + expect(code).toBe(ExitCode.auth); + // Single line, trailing newline only. + expect(err.data.endsWith("\n")).toBe(true); + expect(err.data.trimEnd().includes("\n")).toBe(false); + const parsed = JSON.parse(err.data) as { error: { code: string; message: string } }; + expect(parsed).toEqual({ error: { code: "auth", message: "no key" } }); + // stdout stays empty. + expect(out.data).toBe(""); + }); + + it("maps each exit-code class to its stable string code", () => { + const cases: Array<[number, string]> = [ + [ExitCode.usage, "usage"], + [ExitCode.auth, "auth"], + [ExitCode.notFound, "not_found"], + [ExitCode.network, "network"], + [ExitCode.internal, "internal"], + ]; + for (const [exitCode, expected] of cases) { + const { w, err } = writers(); + reportError(new CliError("x", exitCode), { json: true }, w); + expect((JSON.parse(err.data) as { error: { code: string } }).error.code).toBe(expected); + } + }); + + it("classifies a plain Error as internal", () => { + const { w, err } = writers(); + const code = reportError(new Error("boom"), { json: true }, w); + expect(code).toBe(ExitCode.internal); + expect((JSON.parse(err.data) as { error: { code: string } }).error.code).toBe("internal"); + }); + + it("emits no ANSI escape in the JSON envelope", () => { + const { w, err } = writers(false, true); + reportError(new CliError("boom", ExitCode.usage), { json: true }, w); + expect(err.data).not.toContain("\x1b["); + }); + }); +}); + +describe("exitCodeToString", () => { + it("maps known codes and defaults unknown ones to internal", () => { + expect(exitCodeToString(ExitCode.usage)).toBe("usage"); + expect(exitCodeToString(ExitCode.auth)).toBe("auth"); + expect(exitCodeToString(ExitCode.notFound)).toBe("not_found"); + expect(exitCodeToString(ExitCode.network)).toBe("network"); + expect(exitCodeToString(ExitCode.internal)).toBe("internal"); + expect(exitCodeToString(99)).toBe("internal"); }); }); @@ -126,7 +188,7 @@ describe("color application", () => { const { w, err } = writers(false, false); logProgress("dim text", w); logWarn("warn text", w); - reportError(new Error("err text"), w); + reportError(new Error("err text"), {}, w); expect(err.data).not.toContain("\x1b["); }); });