Skip to content

fix(cli): suppress Cliffy Command dump on ValidationError (swamp-club#171)#1229

Merged
stack72 merged 1 commit intomainfrom
fix/cli-validation-error-output
Apr 27, 2026
Merged

fix(cli): suppress Cliffy Command dump on ValidationError (swamp-club#171)#1229
stack72 merged 1 commit intomainfrom
fix/cli-validation-error-output

Conversation

@stack72
Copy link
Copy Markdown
Contributor

@stack72 stack72 commented Apr 27, 2026

Summary

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 on line 1.

This recognizes ValidationError at the presentation boundary and treats it the same as UserError — log just the message, never the full object. Replaces the fragile isCliffyMissingArgError string-match helper with an instanceof ValidationError check; the string match is now dead code (Cliffy throws ValidationError for missing-arg cases too).

Before / after

swamp workflow run namespace-debug --inputs podModel=foo

log mode json mode
Before 310 lines already clean (4-line stack)
After 1 line clean (no stack)
12:11:15.805 FTL error Error: 'Unknown option "--inputs". Did you mean option "--input"?'

Other paths verified to stay clean:

  • Missing required arg
  • Bad input syntax
  • Missing input file
  • Unknown subcommand (already handled by unknownCommandErrorHandler)
  • --json mode (still emits structured {error} JSON, no leaked internals)

Test plan

  • deno check passes
  • deno lint passes
  • deno fmt passes
  • deno run test — 4928 passed, 0 failed
  • deno run compile produces a working binary
  • Re-ran the issue's exact reproduction commands against the compiled binary; line counts dropped from 310 / 310 / 288 to 1 / 1 / 1
  • Confirmed clean cases (missing required, bad syntax, unknown subcommand, --json mode) remain clean
  • Two new regression tests in error_output_test.ts synthesize a Cliffy-shaped ValidationError with a circular cmd payload and assert that Command {, settings:, and [Circular never leak into rendered output (covers both log and json mode)

Closes swamp-club#171

…#171)

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
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CLI UX Review

Blocking

None.

Suggestions

None.

Verdict

PASS — pure UX improvement with no regressions.

This PR fixes a real and severe error-rendering bug. When Cliffy throws a ValidationError for an unknown flag, unknown option, or missing required argument, the old code fell through to logger.fatal("{error}", err), which dumped the entire Cliffy Command tree via Deno.inspect (~310 lines, including circular refs). The actionable message (e.g., Did you mean option "--input"?) was buried on line 1.

After this change, ValidationError is treated identically to UserError at the presentation boundary: only err.message is logged, never the object. The interaction with unknownCommandErrorHandler is correctly preserved — that handler already exits for "Unknown command" errors before they reach renderError; all other ValidationErrors re-throw and now render cleanly.

Log mode: 310 lines → 1 clean line with the Cliffy hint visible.
JSON mode: stack trace suppressed for validation errors (correct — stack traces on typos are noise for pipe consumers).

The replacement of the fragile startsWith("Missing argument(s):") string match with instanceof ValidationError is strictly more correct and covers the full class of Cliffy validation failures.

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Clean, well-scoped bug fix. The change correctly replaces a fragile string-match helper (isCliffyMissingArgError) with a proper instanceof ValidationError check at the presentation boundary. This handles all Cliffy validation failures (unknown options, missing args, etc.) rather than just the one string pattern that was previously matched.

No Blocking Issues

Suggestions

  1. Minor: The test comments are slightly verbose relative to the project's "default to no comments" convention (e.g., lines 105–106, 119–120 in the test file), but given these are regression tests for a specific issue, the context is useful for future readers. Not worth changing.

What looks good

  • DDD alignment: The fix stays cleanly in the presentation layer — framework errors (ValidationError) are caught at the rendering boundary without leaking into or from the domain layer. UserError remains the domain-level error concept.
  • Import boundary: No libswamp internal path violations. @cliffy/command is already used elsewhere in the codebase (e.g., unknown_command_handler.ts).
  • Test coverage: Three new/updated tests cover the exact regression (circular-ref Command dump in both log and JSON modes) plus the existing missing-arg case migrated to ValidationError.
  • Security: The fix improves the security posture by preventing internal framework details from leaking to end users.
  • Minimal blast radius: Only two files changed, dead code removed, no unrelated refactoring.

@stack72 stack72 merged commit 2b3d12e into main Apr 27, 2026
10 checks passed
@stack72 stack72 deleted the fix/cli-validation-error-output branch April 27, 2026 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant