From cd8908dee9467f3e4e5818dbd88d93af2c7ea1eb Mon Sep 17 00:00:00 2001 From: tcerqueira Date: Mon, 29 Jun 2026 13:29:58 +0100 Subject: [PATCH 1/3] fix(cli): map Cliffy ValidationError to USAGE exit code Run the root command's `parse()` with `.noExit()` and funnel thrown `ValidationError`s (unknown/invalid/conflicting flags, missing values) through the CLI error contract so they exit with `ExitCode.USAGE` (2) and, under `--json`, emit the structured `{error:{code,message,...}}` envelope on stderr instead of dumping help to stdout. Previously a bad flag printed full help to stdout and a raw, ANSI-colored Cliffy error to stderr even under `--json`, corrupting machine-readable output. `.reset()` repoints the builder to the root command before `.noExit()`, since mounting/defining subcommands leaves it selecting a child. `--help`/`--version` still exit 0. Adds a token-free subprocess test for the exit-code contract and tightens the existing invalid-flag test to assert USAGE (2) plus a clean stdout. --- main.ts | 46 ++++++++++++++++++++++++---- tests/agent.test.ts | 9 +++--- tests/cli_contract.test.ts | 61 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 107 insertions(+), 9 deletions(-) create mode 100644 tests/cli_contract.test.ts diff --git a/main.ts b/main.ts index f3f7861..6f39783 100644 --- a/main.ts +++ b/main.ts @@ -1,8 +1,9 @@ -import { Command } from "@cliffy/command"; +import { Command, ValidationError } from "@cliffy/command"; import { greaterOrEqual, parse as semverParse } from "@std/semver"; import { sandboxCommand } from "./sandbox/mod.ts"; import { deployCommand } from "./deploy/mod.ts"; import { actionHandler, getApp, getOrg } from "./config.ts"; +import { error, ExitCode } from "./util.ts"; const MINIMUM_DENO_VERSION = "2.4.2"; if ( @@ -30,10 +31,45 @@ export type GlobalContext = { nonInteractive?: true; }; -if (Deno.env.has("DENO_DEPLOY_CLI_SANDBOX")) { - await sandboxCommand.parse(Deno.args); -} else { - await deployCommand.command("sandbox", sandboxCommand).parse(Deno.args); +// `.noExit()` makes Cliffy throw parse errors instead of exiting, so they can be +// routed through the CLI error contract (see `handleCliError`). It also stops +// `--help`/`--version` from exiting: Cliffy prints them and `parse()` returns, so +// the process still exits 0. `.reset()` first repoints the builder back to the +// root command (mounting/defining subcommands leaves it selecting a child), so +// `.noExit()` applies to the command we actually parse. +try { + if (Deno.env.has("DENO_DEPLOY_CLI_SANDBOX")) { + await sandboxCommand.reset().noExit().parse(Deno.args); + } else { + await deployCommand.command("sandbox", sandboxCommand).reset().noExit() + .parse(Deno.args); + } +} catch (e) { + handleCliError(e); +} + +/** + * Map an error thrown out of Cliffy's `parse()` onto the CLI error contract. + * `ValidationError` (unknown/invalid/conflicting flag, missing value) is a usage + * error and must exit with `ExitCode.USAGE` (2); anything else is generic. + */ +function handleCliError(e: unknown): never { + const context: GlobalContext = { + debug: Deno.args.includes("--debug"), + endpoint: "", + json: Deno.args.includes("--json") || Deno.args.includes("-j") + ? true + : undefined, + }; + + if (e instanceof ValidationError) { + error(context, e.message, { + code: ExitCode.USAGE, + errorCode: "VALIDATION_ERROR", + }); + } + + error(context, e instanceof Error ? e.message : String(e)); } export function createSwitchCommand( diff --git a/tests/agent.test.ts b/tests/agent.test.ts index 64b3db8..906bbf0 100644 --- a/tests/agent.test.ts +++ b/tests/agent.test.ts @@ -159,8 +159,8 @@ Deno.test("publish (default command) --json keeps stdout clean and emits an AUTH }); Deno.test("non-zero exit code matches taxonomy for invalid flag (USAGE=2)", async () => { - // Cliffy's ValidationError handler exits with code 1 by default; - // verify the agent can pattern-match on stderr text either way. + // Cliffy `ValidationError`s are now routed through the CLI error contract, so + // a bad flag value exits with USAGE (2) and keeps stdout clean. const res = await deployRaw( "create", "--dry-run", @@ -171,8 +171,9 @@ Deno.test("non-zero exit code matches taxonomy for invalid flag (USAGE=2)", asyn "--source", "invalid", ); - assert(res.code !== 0); - assertStringIncludes(res.stderr + res.stdout, "Invalid source"); + assertEquals(res.code, 2, `stderr: ${res.stderr}`); + assertEquals(res.stdout.trim(), "", `stdout should be empty: ${res.stdout}`); + assertStringIncludes(res.stderr, "Invalid source"); }); async function sandboxRaw(...args: string[]): Promise< diff --git a/tests/cli_contract.test.ts b/tests/cli_contract.test.ts new file mode 100644 index 0000000..78cebe7 --- /dev/null +++ b/tests/cli_contract.test.ts @@ -0,0 +1,61 @@ +import { assert, assertEquals } from "@std/assert"; +import { fromFileUrl } from "@std/path"; + +// These tests drive `main.ts` as a subprocess and assert the exit-code contract. +// Unlike the other suites here, they don't touch the backend, so no token is +// required: bad flags / `--help` / `--version` are resolved entirely by the +// argument parser before any command action runs. + +const MAIN_TS = fromFileUrl(new URL("../main.ts", import.meta.url)); + +async function runCli( + args: string[], + env: Record = {}, +): Promise<{ code: number; stdout: string; stderr: string }> { + const { code, stdout, stderr } = await new Deno.Command(Deno.execPath(), { + args: ["run", "-A", MAIN_TS, ...args], + env, + stdout: "piped", + stderr: "piped", + }).output(); + return { + code, + stdout: new TextDecoder().decode(stdout), + stderr: new TextDecoder().decode(stderr), + }; +} + +Deno.test("unknown flag exits with USAGE (2)", async () => { + const res = await runCli(["--does-not-exist"]); + assertEquals(res.code, 2, `stderr: ${res.stderr}`); +}); + +Deno.test("unknown flag with --json emits a USAGE envelope on stderr, clean stdout", async () => { + const res = await runCli(["--json", "--does-not-exist"]); + assertEquals(res.code, 2, `stderr: ${res.stderr}`); + assertEquals(res.stdout.trim(), "", `stdout should be empty: ${res.stdout}`); + const envelope = JSON.parse(res.stderr.trim().split("\n").pop()!); + assertEquals(envelope.error.code, "VALIDATION_ERROR"); + assert( + typeof envelope.error.message === "string" && + envelope.error.message.length > 0, + `expected a message; got: ${JSON.stringify(envelope)}`, + ); +}); + +Deno.test("--help exits 0", async () => { + const res = await runCli(["--help"]); + assertEquals(res.code, 0, `stderr: ${res.stderr}`); +}); + +Deno.test("--version exits 0", async () => { + const res = await runCli(["--version"]); + assertEquals(res.code, 0, `stderr: ${res.stderr}`); +}); + +Deno.test("unknown flag exits with USAGE (2) on the standalone sandbox root", async () => { + const res = await runCli(["--does-not-exist"], { + DENO_DEPLOY_CLI_SANDBOX: "1", + }); + assertEquals(res.code, 2, `stderr: ${res.stderr}`); +}); From 0d146b39e215205f54f9e53eea74932377a5cecd Mon Sep 17 00:00:00 2001 From: tcerqueira Date: Mon, 29 Jun 2026 13:30:17 +0100 Subject: [PATCH 2/3] fix(switch): emit JSON result and send status to stderr `switch` printed its confirmation to stdout via console.log and emitted nothing under `--json`, corrupting piped/machine-readable output. Route the human-readable message to stderr and, under `--json`, write a single `{ org, app }` result to stdout via `writeJsonResult` (app is null when the sandbox variant doesn't resolve an application). --- main.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/main.ts b/main.ts index 6f39783..2f3e7b1 100644 --- a/main.ts +++ b/main.ts @@ -3,7 +3,7 @@ import { greaterOrEqual, parse as semverParse } from "@std/semver"; import { sandboxCommand } from "./sandbox/mod.ts"; import { deployCommand } from "./deploy/mod.ts"; import { actionHandler, getApp, getOrg } from "./config.ts"; -import { error, ExitCode } from "./util.ts"; +import { error, ExitCode, writeJsonResult } from "./util.ts"; const MINIMUM_DENO_VERSION = "2.4.2"; if ( @@ -88,10 +88,14 @@ export function createSwitchCommand( app = out.app; } - console.log( - `Switched to organization '${org}'${ - app ? ` and application '${app}'` : "" - }.`, - ); + if (options.json) { + writeJsonResult({ org, app: app ?? null }); + } else { + console.error( + `Switched to organization '${org}'${ + app ? ` and application '${app}'` : "" + }.`, + ); + } })); } From 0abaf055130fa46babaf42040a48d015d191d38f Mon Sep 17 00:00:00 2001 From: tcerqueira Date: Mon, 29 Jun 2026 15:44:52 +0100 Subject: [PATCH 3/3] fix(cli): detect json mode for combined and =-form flags in error handler `handleCliError` picked the error output format with a `--json`/`-j` substring-free includes check, missing `--json=...` and combined short flags like `-jy`/`-yj`. A parse-time ValidationError in those invocations printed the human-readable ANSI error instead of the structured VALIDATION_ERROR envelope. Broaden detection to a parse-free heuristic matching `--json`, `--json=...`, `-j`, and combined short flags containing `j`. Adds a `-jy` envelope test case. --- main.ts | 14 +++++++++++--- tests/cli_contract.test.ts | 10 ++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/main.ts b/main.ts index 2f3e7b1..94d149f 100644 --- a/main.ts +++ b/main.ts @@ -57,9 +57,7 @@ function handleCliError(e: unknown): never { const context: GlobalContext = { debug: Deno.args.includes("--debug"), endpoint: "", - json: Deno.args.includes("--json") || Deno.args.includes("-j") - ? true - : undefined, + json: Deno.args.some(isJsonModeArg) ? true : undefined, }; if (e instanceof ValidationError) { @@ -72,6 +70,16 @@ function handleCliError(e: unknown): never { error(context, e instanceof Error ? e.message : String(e)); } +/** + * Best-effort `--json` detection without a parsed context, used by + * `handleCliError` to pick the error output format when `parse()` itself throws. + * Matches `--json`, `--json=...`, `-j`, and combined short flags like `-jy`. + */ +function isJsonModeArg(arg: string): boolean { + return arg === "-j" || arg === "--json" || arg.startsWith("--json=") || + /^-[a-z]*j[a-z]*$/.test(arg); +} + export function createSwitchCommand( handleApp: boolean, ): Command { diff --git a/tests/cli_contract.test.ts b/tests/cli_contract.test.ts index 78cebe7..1ac0d1a 100644 --- a/tests/cli_contract.test.ts +++ b/tests/cli_contract.test.ts @@ -43,6 +43,16 @@ Deno.test("unknown flag with --json emits a USAGE envelope on stderr, clean stdo ); }); +Deno.test("combined short flag -jy is detected as JSON mode for the error envelope", async () => { + // `-jy` bundles `-j` (json) and `-y` (non-interactive); a bad flag must still + // surface the structured envelope on stderr, not the human-readable error. + const res = await runCli(["-jy", "--does-not-exist"]); + assertEquals(res.code, 2, `stderr: ${res.stderr}`); + assertEquals(res.stdout.trim(), "", `stdout should be empty: ${res.stdout}`); + const envelope = JSON.parse(res.stderr.trim().split("\n").pop()!); + assertEquals(envelope.error.code, "VALIDATION_ERROR"); +}); + Deno.test("--help exits 0", async () => { const res = await runCli(["--help"]); assertEquals(res.code, 0, `stderr: ${res.stderr}`);