From be44e310766557ee50337c08b66666cdf65d7746 Mon Sep 17 00:00:00 2001 From: stack72 Date: Mon, 27 Apr 2026 14:12:11 +0200 Subject: [PATCH] fix(cli): suppress Cliffy Command dump on ValidationError (swamp-club#171) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the CLI hit a Cliffy ValidationError (unknown flag, unknown option, missing required arg), renderError fell through to logger.fatal('{error}', err), which dumped the entire Cliffy Command tree (~300 lines, with circular refs) via Deno.inspect. The useful "did you mean" hint was buried. Recognize ValidationError at the rendering boundary and treat it like UserError — log just the message, never the full object. Replaces the fragile isCliffyMissingArgError string-match helper with an instanceof check; the string match is now dead code (Cliffy throws ValidationError for missing-arg cases too). Before: 310 lines for `swamp workflow run namespace-debug --inputs foo=bar` After: 1 line — `Error: 'Unknown option "--inputs". Did you mean option "--input"?'` Closes swamp-club#171 --- src/presentation/output/error_output.ts | 17 ++--- src/presentation/output/error_output_test.ts | 80 +++++++++++++++++++- 2 files changed, 81 insertions(+), 16 deletions(-) diff --git a/src/presentation/output/error_output.ts b/src/presentation/output/error_output.ts index 666e0cc3..48acde9c 100644 --- a/src/presentation/output/error_output.ts +++ b/src/presentation/output/error_output.ts @@ -17,21 +17,13 @@ // You should have received a copy of the GNU Affero General Public License // along with Swamp. If not, see . +import { ValidationError } from "@cliffy/command"; import { getSwampLogger } from "../../infrastructure/logging/logger.ts"; import { UserError } from "../../domain/errors.ts"; import type { OutputMode } from "./output.ts"; const logger = getSwampLogger(["error"]); -/** - * Returns true if the error is a Cliffy missing-argument error. - * These are plain Error objects with the message pattern "Missing argument(s): ..." - * and should be rendered without a stack trace. - */ -function isCliffyMissingArgError(err: Error): boolean { - return err.message.startsWith("Missing argument(s):"); -} - /** * Builds the JSON error object for structured output. * Format: { error: string, stack?: string } @@ -40,7 +32,8 @@ function isCliffyMissingArgError(err: Error): boolean { export function buildErrorJson(err: Error): Record { const data: Record = { error: err.message }; if ( - !(err instanceof UserError) && !isCliffyMissingArgError(err) && err.stack + !(err instanceof UserError) && !(err instanceof ValidationError) && + err.stack ) { const stackLines = err.stack.split("\n").filter((line) => line.trim().startsWith("at ") @@ -54,7 +47,7 @@ export function buildErrorJson(err: Error): Record { /** * Renders an error via LogTape at fatal level. - * UserError instances and Cliffy missing-argument errors log just the message (no stack trace). + * UserError instances and Cliffy ValidationErrors log just the message (no stack trace). * Other errors log the full Error object (including stack trace via Deno.inspect). * * In JSON mode, also writes the error as JSON to stdout so pipe consumers @@ -68,7 +61,7 @@ export function renderError(error: unknown, outputMode?: OutputMode): void { console.log(JSON.stringify(buildErrorJson(err), null, 2)); } - if (err instanceof UserError || isCliffyMissingArgError(err)) { + if (err instanceof UserError || err instanceof ValidationError) { logger.fatal("Error: {message}", { message: err.message }); } else { logger.fatal("{error}", { error: err }); diff --git a/src/presentation/output/error_output_test.ts b/src/presentation/output/error_output_test.ts index 6ca3cf08..1f17e4c1 100644 --- a/src/presentation/output/error_output_test.ts +++ b/src/presentation/output/error_output_test.ts @@ -18,6 +18,7 @@ // along with Swamp. If not, see . import { assertEquals, assertStringIncludes } from "@std/assert"; +import { ValidationError } from "@cliffy/command"; import { initializeLogging } from "../../infrastructure/logging/logger.ts"; import { buildErrorJson, renderError } from "./error_output.ts"; import { UserError } from "../../domain/errors.ts"; @@ -73,13 +74,13 @@ Deno.test("renderError wraps non-Error values in Error", () => { } }); -Deno.test("renderError logs Cliffy missing argument error without stack trace", () => { +Deno.test("renderError logs Cliffy ValidationError missing argument without stack trace", () => { const logs: string[] = []; const originalError = console.error; console.error = (...args: unknown[]) => logs.push(args.join(" ")); try { - const error = new Error("Missing argument(s): extension"); + const error = new ValidationError("Missing argument(s): extension"); renderError(error); assertEquals(logs.length, 1); @@ -91,6 +92,75 @@ Deno.test("renderError logs Cliffy missing argument error without stack trace", } }); +Deno.test("renderError suppresses Cliffy Command dump on ValidationError (issue #171)", () => { + const logs: string[] = []; + const originalError = console.error; + console.error = (...args: unknown[]) => logs.push(args.join(" ")); + + try { + const error = new ValidationError( + 'Unknown option "--inputs". Did you mean option "--input"?', + ); + // Mimic Cliffy's real failure: ValidationError carries a `cmd` payload + // that points at the parsed Command tree (with circular refs). + const fakeCmd: Record = { + settings: { name: "swamp", description: "AI Native Automation CLI" }, + }; + fakeCmd.cmd = fakeCmd; + Object.assign(error, { cmd: fakeCmd, exitCode: 2 }); + + renderError(error); + + assertEquals(logs.length, 1); + assertStringIncludes( + logs[0], + 'Unknown option "--inputs". Did you mean option "--input"?', + ); + // The bug was a 300-line dump of Cliffy internals — none of these markers + // should leak into user-facing output. + assertEquals(logs[0].includes("Command {"), false); + assertEquals(logs[0].includes("settings:"), false); + assertEquals(logs[0].includes("[Circular"), false); + assertEquals(logs[0].includes(" at "), false); + } finally { + console.error = originalError; + } +}); + +Deno.test("renderError: json mode emits clean JSON for ValidationError (issue #171)", () => { + const stdoutLogs: string[] = []; + const originalLog = console.log; + const originalError = console.error; + console.log = (...args: unknown[]) => stdoutLogs.push(args.join(" ")); + console.error = () => {}; + + try { + const error = new ValidationError( + 'Unknown option "--inputs". Did you mean option "--input"?', + ); + const fakeCmd: Record = { settings: { name: "swamp" } }; + fakeCmd.cmd = fakeCmd; + Object.assign(error, { cmd: fakeCmd, exitCode: 2 }); + + renderError(error, "json"); + + assertEquals(stdoutLogs.length, 1); + const parsed = JSON.parse(stdoutLogs[0]); + assertEquals( + parsed.error, + 'Unknown option "--inputs". Did you mean option "--input"?', + ); + // ValidationError should never produce a stack in JSON output. + assertEquals(parsed.stack, undefined); + // And the raw payload must not leak. + assertEquals(stdoutLogs[0].includes("Command {"), false); + assertEquals(stdoutLogs[0].includes("[Circular"), false); + } finally { + console.log = originalLog; + console.error = originalError; + } +}); + Deno.test("renderError uses fatal level for all errors", () => { const logs: string[] = []; const originalError = console.error; @@ -186,8 +256,10 @@ Deno.test("buildErrorJson: system Error includes stack", () => { assertStringIncludes(result.stack!, "at "); }); -Deno.test("buildErrorJson: Cliffy missing arg error has no stack", () => { - const result = buildErrorJson(new Error("Missing argument(s): extension")); +Deno.test("buildErrorJson: Cliffy ValidationError has no stack", () => { + const result = buildErrorJson( + new ValidationError("Missing argument(s): extension"), + ); assertEquals(result.error, "Missing argument(s): extension"); assertEquals(result.stack, undefined); });