Skip to content

fix: correct exit codes, unknown subcommands, and table overflow - #39

Merged
patramsey merged 2 commits into
mainfrom
fix/cli-ux-audit
Aug 20, 2026
Merged

fix: correct exit codes, unknown subcommands, and table overflow#39
patramsey merged 2 commits into
mainfrom
fix/cli-ux-audit

Conversation

@patramsey

Copy link
Copy Markdown
Owner

An audit of the CLI surface — how the binary presents itself, rather than what
it asks the API for — turned up nine defects. All nine are fixed here.

Breaks scripted use

A mistyped subcommand succeeded. Cobra's legacyArgs only rejects unknown
commands on the root command; for a parent that itself has a parent it
returns nil, so cobra fell through to "not runnable", printed help, and
returned no error.

$ namecom domain regsiter example.com
<prints domain help>
$ echo $?
0

namecom domain regsiter foo.com && deploy deployed. Every group now goes
through cmdutil.GroupCmd, which rejects an unknown subcommand as a usage
error and reuses cobra's suggestion list. Invoking a group bare still prints
help and exits 0.

Invocation mistakes exited 1 instead of the documented 2. Two causes: the
validators in cmd/cmdutil/validate.go returned bare fmt.Errorf values, and
cobra checks required flags inside execute() — after SetFlagErrorFunc has
had its chance, with no hook of its own.

invocation before after
dns create example.com --type ZZZ --answer 1.2.3.4 1 2
dns create example.com (required flags missing) 1 2
dns list example.com --badflag 2 2
domain get (arg missing) 2 2

A script watching for exit 1 could not tell a typo from a server error.
Validators now build UsageError directly; cobra's own messages are
classified in one documented place, asserted by a test that fails loudly if an
upgrade rewords them.

--dry-run was silently ignored on reads. It promised to "print the API
request that would be sent without executing it", but only the 28 write paths
consult IsDryRunnamecom domain list --dry-run made a live authenticated
call and returned real account data. Rather than change what 38 read commands
do, the flag now describes what it actually does.

Terminal output

Tables overflowed the terminal. They rendered at natural width regardless
of it — domain list 113 columns, order list 99, dns list 87 — so in an
80-column pane the rounded borders wrapped into fragments. Trailing columns
are now dropped until the table fits, since callers order columns
most- to least-important, and a footer names what went rather than letting it
vanish:

╭───────────────────────────────────────────────────┬──────────────────────────╮
│ DOMAIN                                            │ EXPIRES                  │
├───────────────────────────────────────────────────┼──────────────────────────┤
│ loadtest-ff7fb52b-b51b-46c8-b254-6a557f053321.com │ 2027-03-01 (in 6 months) │
│ beers.army                                        │ 2034-03-01 (in 8 years)  │
╰───────────────────────────────────────────────────┴──────────────────────────╯
3 columns hidden (auto-renew, locked, privacy) — widen the terminal, pass --wide, or use -o json

The new --wide opts out. A pipe has no width to fit and keeps every column,
so scripts reading table output are unaffected.

Relative dates only spoke days. A domain paid through 2034 read in 2750 days. Days stay exact inside a quarter, where a renewal decision is actually
pending; past that the unit widens to months, then years.

Help and docs

  • Examples: moved from below the flag tables and the "see all global
    options" footer to directly under the usage line.
  • Command groups showed namecom domain [flags], an invocation that does
    nothing; they now show namecom domain <command>.
  • dns create --type omitted CAA, which the validator has always accepted.
  • Non-string flag defaults print unquoted — default 300, not default "300".
  • CLAUDE.md's command tree had drifted: no config, contact, status,
    open, or version, and vanity/ where the command is vanity-ns.

Tests

  • TestEveryGroupRejectsUnknownSubcommands walks the real command tree, so a
    group added later without GroupCmd fails rather than silently regressing.
  • TestClassifyCobraUsage pins the cobra message strings the exit-code
    mapping depends on.
  • TestTableFitsTerminalWidth covers dropping, --wide, and the unconstrained
    pipe case.
  • TestRelativeTimeWidensUnit pins the unit thresholds.

make test, make lint, and make build all pass.

An audit of the CLI surface turned up nine defects, all of them in how the
binary presents itself rather than in what it asks the API for.

Three of them break scripted use:

A mistyped subcommand succeeded. Cobra's legacyArgs only rejects unknown
commands on the root command — for a parent that itself has a parent it
returns nil, so cobra fell through to "not runnable", printed help, and
returned no error. `namecom domain regsiter example.com` exited 0, which
means `namecom domain regsiter foo.com && deploy` deployed. Every group is
now wrapped in cmdutil.GroupCmd, which rejects an unknown subcommand as a
usage error and reuses cobra's suggestion list. A group invoked bare still
prints help and exits 0, which is what browsing looks like.

Invocation mistakes exited 1 instead of the documented 2. Two causes: the
validators in cmd/cmdutil/validate.go returned bare fmt.Errorf values, and
cobra checks required flags inside execute(), after SetFlagErrorFunc has had
its chance and with no hook of its own. So `--type ZZZ` and a missing
`--answer` reported the same code as a 500 while `--badflag` beside them
reported 2. Validators now build UsageError directly; cobra's own messages
are classified in one documented place, asserted by a test that fails if an
upgrade rewords them.

--dry-run promised to "print the API request that would be sent without
executing it", but only the 28 write paths consult it — reads always called
the API. Rather than change what 38 commands do, the flag now says what it
does.

The rest are presentation. Tables were rendered at natural width with no
regard for the terminal: 113 columns for `domain list` against an 80-column
pane, where the rounded borders wrap into fragments. Trailing columns are
dropped until the table fits, a footer names what went, and --wide opts out;
a pipe has no width to fit and keeps everything. Relative dates widen their
unit past a quarter, because "in 2750 days" told a reader nothing. Examples
moved above the flag tables. Command groups no longer advertise themselves
as `namecom domain [flags]`. `dns create --type` lists CAA, which the
validator has always accepted. Non-string flag defaults print unquoted.

CLAUDE.md's command tree had drifted — no config, contact, status, open, or
version, and `vanity/` where the command is `vanity-ns`.
@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.83333% with 7 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/output/output.go 95.1% 2 Missing and 1 partial ⚠️
cmd/help.go 86.6% 1 Missing and 1 partial ⚠️
cmd/root.go 50.0% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

Codecov put patch coverage at 89.6%. Most of the miss was pre-existing
colour branches and Execute's os.Exit path, but three of the fixes in this
branch genuinely had no test: the Examples/Flags ordering, the command-group
usage line, and the non-string default formatting. All three are the kind of
thing that reverts silently when someone edits the template for an unrelated
reason.

The expiry-threshold test is the reason for the small refactor. Written the
obvious way it asserted on rendered ANSI prefixes — and lipgloss degrades
every style to a no-op off a TTY, so each expected prefix was the empty
string and all three cases passed without checking anything. expiryStyle is
split out of ExpiryDate so the thresholds can be compared as values instead.

Every new assertion was mutation-checked: reverting the week threshold, the
unit-widening threshold, the column dropping, the group usage line, the
default quoting, the required-flag prefix, the Examples ordering, and the
unknown-subcommand rejection each fail at least one test.
@patramsey
patramsey merged commit c582ade into main Aug 20, 2026
4 checks passed
@patramsey
patramsey deleted the fix/cli-ux-audit branch August 20, 2026 00:47
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