fix(output): keep stdout clean on failure and error on unknown subcommands by dspangen · Pull Request #78 · exploreomni/cli

dspangen · 2026-08-24T15:08:14Z

Failed invocations no longer look like data. Three fixes from the agent-session cost audit:

API errors (≥400) go to stderr as exactly one valid JSON envelope ({"error", "status", "body"}) — previously the error body landed on stdout, and cobra's extra Error: line made stderr unparseable.
No usage-block noise on runtime failures — usage still prints for unknown flags / wrong arg counts, but not after an API error.
Unknown subcommands are real errors: omni models list-branches (with or without --help) now exits 1 with a suggestion (Did you mean this? list) on stderr instead of dumping group help with exit 0.

Behavior changes to know: omni <group>, <group> <typo>, and <group> <typo> --help now exit 1 (bare-group help goes to stderr, like git); JSON-mode stderr is an envelope with the old bare API body under .body. Successful commands are byte-for-byte unchanged on stdout.

Details: contracts, design decisions, and verification

What changed

cmd/omni/output.go — split into outputResponseTo(stdout, stderr, ...). Failures write nothing to stdout; success and 204 still go to stdout. Non-zero exit unchanged.
internal/openapi/generate.go — leaf commands set cmd.SilenceUsage = true as the first statement of RunE (after flag parsing and arg validation, so those still print usage).
internal/openapi/generate.go — groups are built by NewGroupCommand (every generated tag group and the hand-written config group). subcommandSuggestions extends cobra's Levenshtein/prefix matching with a reverse-prefix pass, so the over-specified guess list-branches points at list (Levenshtein distance 9 — cobra alone suggests nothing).
Hand-written API commands (models create-branch, users set-attributes) set SilenceUsage the same way.
Docs: README, CLAUDE.md, and omni agent-help state the stream/exit contract and the envelope shape.

Contracts, and the decisions behind them

1. stderr holds exactly one JSON document per failed invocation

omni models list 2>err.json must produce a parseable file. Writing the API's body and letting cobra append Error: API returned HTTP 400 gave two documents, so outputResponse now emits a single envelope and returns a typed *apiError that executeAPICall uses to silence cobra's duplicate line:

{
  "error": "bad model id",
  "status": 400,
  "body": { "detail": "bad model id", "code": "INVALID" }
}

body carries the API's payload verbatim and is omitted when the response wasn't JSON (an HTML page from a proxy, say — its text becomes error). Rejected: writing the raw body alone, which drops the status code, the one thing that tells a 400 from a 500 when the body is terse. Human mode is unchanged: one Error: <detail> (HTTP N) line on stderr, no duplicate.

2. A non-JSON 2xx body is pass-through success

query run streams text/ndjson by default ("it cannot be parsed as a single JSON document", per the spec) and returns CSV or XLSX with a result type, so validating 2xx bodies as JSON would break real endpoints. An un-parseable payload is treated as data and written to stdout unchanged with exit 0. What is now enforced: the body is read in full before anything is written, so a truncated response exits non-zero with an empty stdout instead of leaving half a payload behind.

3. Bare group: help to stderr, exit 1

Cobra's common idiom (return cmd.Help()) writes help to stdout and exits 0, which is exactly the ambiguity being fixed. Help-to-stderr + non-zero exit matches git's behavior for an incomplete command and keeps the invariant that an empty stdout always means "no data".

4. A typo plus `--help` is still a typo

Cobra answers the help flag inside execute() before RunE and returns nil from Execute, so no Args/RunE hook can catch it. Group commands now carry a help func that checks cmd.Flags().Args(): with a leftover positional it prints the unknown-subcommand error to stderr and annotates the command, and main uses ExecuteC + openapi.UnknownSubcommand(cmd) to exit 1. Rejected: os.Exit(1) inside the help func (untestable, skips deferred cleanup). Note: renderHelp reproduces cobra's defaultHelpFunc because calling cmd.Help() there would recurse back through HelpFunc. Unaffected and covered by tests: omni models --help, omni models -h, omni help models, omni models list --help — all still stdout, exit 0, byte-for-byte identical output.

Verification

make build && make test — all six packages pass; go vet ./... clean.

New tests: TestOutputResponseTo_StderrIsSingleJSONDocument (unmarshals the entire stderr capture, pretty + compact), _NonJSONErrorBodyStillJSON, _ErrorBodyGoesToStderr, _HumanErrorGoesToStderr, _NonJSONSuccessPassesThrough (ndjson + CSV), _ReadFailureWritesNothing, _SuccessGoesToStdout, TestAPIErrorTo_SingleJSONDocument, TestAPIErrorTo_OmitsMissingBody, TestBuildCommand_RuntimeErrorSilencesUsage, TestBuildCommand_FlagErrorKeepsUsage, TestGroupRunE_UnknownSubcommand / _NoSuggestions / _NoSubcommand / _HelpFlagStaysOnStdout, TestGroupHelp_UnknownSubcommandWithHelpFlag, TestGroupHelp_SubcommandHelpUnaffected, TestGenerateCommands_GroupsAreRunnable.

Binary sanity checks (stub API on localhost):

$ ./omni models list-branches            # exit=1, stdout 0 bytes
Error: unknown subcommand "list-branches" for "omni models"

Did you mean this?
	list

Run 'omni models --help' for a list of available subcommands

$ ./omni models list-branches --help     # exit=1, stdout 0 bytes (same error)
$ ./omni models list                     # stub returns 400; exit=1, stdout 0 bytes
$ ./omni models list 2>err.json; jq -r '.status, .body.code' err.json
400
INVALID

$ ./omni query wait                      # stub returns text/ndjson; exit=0, stderr empty
$ ./omni models --help                   # exit=0, 2431 bytes stdout — identical to pre-change

🤖 Generated with Claude Code

https://claude.ai/code/session_014TwwKSAsAGPBToNb4iUe5s

…mands Three ways a failed invocation used to look like data on stdout: - HTTP >=400 bodies were pretty-printed to stdout, contradicting the documented contract and mixing error JSON into pipes. They now go to stderr; stdout gets nothing. - Generated commands never set SilenceUsage, so a runtime failure (e.g. "API returned HTTP 400") dragged the whole usage block along behind the message. SilenceUsage is now set on entry to RunE, which is after flag parsing and arg validation, so genuine usage errors still print usage. - Tag groups had no RunE, so `omni models list-branches` printed the group help and exited 0. Groups now share a GroupRunE: an unknown subcommand is a cobra-style error with suggestions and a --help hint, and a bare group prints its help to stderr and exits 1. Suggestions extend cobra's Levenshtein/prefix matching with a reverse-prefix pass, so an over-specified guess like `models list-branches` points at `list` (distance 9 — cobra alone suggests nothing). Human mode already wrote its error to stderr; the CLI now suppresses cobra's follow-up error line there, since HumanError already prints the detail and the status code. Constraint: stdout must stay parseable — an empty stdout means "no data", never "parse this" Constraint: flag-parse errors must keep printing usage; only post-RunE failures suppress it Rejected: bare group prints help to stdout and exits 0 (cobra's `return cmd.Help()` idiom) | indistinguishable from a successful response when piped Rejected: printing the suggestion block ourselves before returning the error | puts the headline last; cobra's own unknown-command errors carry suggestions in the message Confidence: high Scope-risk: moderate Directive: outputResponseTo takes explicit streams so tests can assert stdout is empty — don't reintroduce os.Stdout writes in the >=400 branch Not-tested: TTY/human-mode rendering of the group help on stderr with colors enabled Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014TwwKSAsAGPBToNb4iUe5s

…rors too Review follow-ups on the stream-hygiene work: - stderr wasn't valid JSON in JSON mode. The API's error body was written, then cobra appended "Error: API returned HTTP 400", so `2>err.json` captured two documents. outputResponse now emits one envelope — {"error": <detail>, "status": <code>, "body": <API payload>} — and returns a typed *apiError that main uses to silence cobra's duplicate line. "body" is omitted when the response wasn't JSON (an HTML page from a proxy). - The 2xx contract is now explicit and enforced rather than incidental. The body is read in full before anything is written, so a truncated read exits non-zero with an empty stdout; a non-JSON 2xx body is passed through to stdout as success, which `query run` requires — it streams text/ndjson by default and returns CSV/XLSX with a result type. - `omni models list-branches --help` still printed group help on stdout and exited 0, because cobra answers the help flag before RunE. Group commands are now built by NewGroupCommand, which installs a help func that treats a leftover positional arg as the same unknown-subcommand error and marks the command so main (via ExecuteC + UnknownSubcommand) exits 1. Plain `omni <group> --help`, `-h`, `omni help <group>`, and subcommand help are byte-for-byte unchanged. Constraint: stderr must hold exactly one JSON document per failed invocation in JSON mode Constraint: query run streams text/ndjson — non-JSON 2xx bodies must stay a success path Rejected: printing the raw API body to stderr and dropping the status line | status appears nowhere else; agents lose the 400-vs-500 distinction Rejected: validating 2xx bodies as JSON before writing stdout | would break ndjson/CSV endpoints that legitimately return non-JSON Rejected: os.Exit(1) inside the help func | untestable, and skips deferred cleanup Confidence: high Scope-risk: moderate Directive: renderHelp reproduces cobra's defaultHelpFunc on purpose — calling cmd.Help() there recurses back through HelpFunc into groupHelpFunc Directive: main must keep using ExecuteC; Execute returns nil for the help path, which is where the typo+--help exit code comes from Not-tested: a 2xx body larger than memory (the body is now buffered in full before writing) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014TwwKSAsAGPBToNb4iUe5s

n8agrin

Claude findings worth addressing:

Review of [PR #78](#78) complete — 8 findings reported, all CONFIRMED:

Correctness (4):

[config_commands.go:69](cmd/omni/config_commands.go:69) — none of the 9 hand-written config subcommands set SilenceUsage, so runtime errors still dump the full usage block, contradicting the PR's own new CLAUDE.md contract.
[generate.go:119](internal/openapi/generate.go:119) — a bare group command (omni models) prints its help to stderr and lets cobra print a duplicate Error: ... requires a subcommand line.
[output.go:43](cmd/omni/output.go:43) — a body-read failure returns a plain error instead of *apiError, so it skips the JSON envelope entirely, breaking the "one JSON document on stderr" contract.
[output.go:76](cmd/omni/output.go:76) — a literal null error body is treated as valid JSON and embedded as "body": null instead of omitted.

Cleanup (2): duplicate JSON parsing in jsonBody/extractErrorDetail ([output.go:85](cmd/omni/output.go:85)); response bodies double-read on the success path ([output.go:68](cmd/omni/output.go:68)).

Conventions (2): two new doc comments exceed the user's global "3 sentences max" rule ([output.go:30](cmd/omni/output.go:30), [generate.go:85](internal/openapi/generate.go:85)).

Two other candidates (the apiError type being a "pure boolean flag," and the suggestion-matching being unnecessarily duplicated) were investigated and refuted — both have legitimate reasons for their current design.

Copilot

Pull request overview

Improves CLI failure handling so API errors and invalid subcommands do not contaminate stdout.

Changes:

Routes API failures to a structured stderr envelope.
Adds strict group/subcommand error handling and suppresses runtime usage noise.
Documents and tests the new stream and exit-code contracts.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
README.md	Documents output behavior.
CLAUDE.md	Records CLI output contracts.
cmd/omni/agent_help.go	Updates agent guidance.
cmd/omni/branch_commands.go	Suppresses runtime usage output.
cmd/omni/config_commands.go	Applies group error handling.
cmd/omni/main.go	Handles typed API and help errors.
cmd/omni/output.go	Separates success and failure streams.
cmd/omni/output_test.go	Tests response stream behavior.
cmd/omni/user_commands.go	Suppresses runtime usage output.
internal/openapi/generate.go	Adds group and suggestion handling.
internal/openapi/generate_test.go	Tests generated command behavior.
internal/output/output.go	Adds structured API error envelopes.
internal/output/output_test.go	Tests envelope formatting.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

 	if format == config.FormatHuman {
-		return output.Human(resp.Body)
+		return output.HumanTo(stdout, bytes.NewReader(data))
+	}
+	return output.JSONTo(stdout, bytes.NewReader(data), compact)


+	// Same unknown-subcommand handling as the generated groups.
+	configCmd := openapi.NewGroupCommand("config", "Manage CLI configuration profiles")


 	if format == config.FormatHuman {
-		return output.Human(resp.Body)
+		return output.HumanTo(stdout, bytes.NewReader(data))
+	}
+	return output.JSONTo(stdout, bytes.NewReader(data), compact)


…config errors Addresses the review of the stream-hygiene work: - All nine hand-written `config` leaves set SilenceUsage at RunE entry, so a runtime failure isn't buried under the usage block — the same contract the generated API commands already follow. - outputResponseTo formats the bytes it already buffered (new JSONBytes / HumanBytes) instead of handing the helpers a fresh reader, so a large NDJSON/CSV/XLSX export isn't read and allocated twice; the error path validates the body once and reuses the result for both the envelope and the detail. Pretty-printing now uses json.Indent rather than unmarshal + MarshalIndent, which also stops HTML-escaping the API's payload. - A non-JSON 2xx body goes to stdout verbatim: no re-indenting and no appended newline, so a redirect to disk reproduces the response byte for byte. - A bare group (`omni models`) prints its help to stderr and silences cobra's duplicate "Error: ... requires a subcommand" line. Exit code stays 1. - A body that fails mid-read still emits the JSON envelope and returns *apiError, so JSON-mode stderr stays a single parseable document. - An error body of literal `null` omits "body" from the envelope and falls back to the "HTTP <status>" detail — a JSON null carries nothing an omitted field doesn't. Constraint: stderr must hold exactly one JSON document per failed invocation Constraint: 2xx non-JSON payloads are data and must not be modified in transit Rejected: keep MarshalIndent for pretty output | parses the body a second time Rejected: a new exported error type for the bare-group case | GroupRunE already holds the command, so setting SilenceErrors there is the same pattern with less API surface Confidence: high Scope-risk: narrow Directive: JSON detection lives in outputResponseTo — the output helpers keep their own non-JSON fallbacks for direct callers; don't collapse the two Not-tested: real multi-hundred-MB export (allocation win is by inspection) Not-tested: interactive prompts in `config init` / `config delete` Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014TwwKSAsAGPBToNb4iUe5s

Only conflict was in internal/openapi/generate_test.go: both branches appended tests at the same points. Kept both sides — this branch's group/unknown-subcommand tests and main's --schema, canonicalName, flagLookupKey, and query-param-rename tests. Confidence: high Scope-risk: narrow

dspangen · 2026-08-25T15:56:22Z

All of these were addressed.

dspangen and others added 2 commits August 24, 2026 12:24

dspangen force-pushed the fix/stream-hygiene branch from f56cc90 to 553472a Compare August 24, 2026 16:25

dspangen requested a review from n8agrin August 24, 2026 16:48

n8agrin requested a balanced review from Copilot August 24, 2026 22:23

n8agrin reviewed Aug 24, 2026

View reviewed changes

Copilot started reviewing on behalf of n8agrin August 24, 2026 22:24 View session

Copilot AI reviewed Aug 24, 2026

View reviewed changes

dspangen and others added 2 commits August 24, 2026 21:46

n8agrin approved these changes Aug 25, 2026

View reviewed changes

dspangen merged commit e4e695e into main Aug 25, 2026
2 checks passed

dspangen deleted the fix/stream-hygiene branch August 25, 2026 16:46

Navigation Menu

Uh oh!

fix(output): keep stdout clean on failure and error on unknown subcommands - #78

fix(output): keep stdout clean on failure and error on unknown subcommands#78
dspangen merged 4 commits into
mainfrom
fix/stream-hygiene

dspangen commented Aug 24, 2026 •

edited

Loading

Uh oh!

n8agrin left a comment •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

dspangen commented Aug 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		// Same unknown-subcommand handling as the generated groups.
		configCmd := openapi.NewGroupCommand("config", "Manage CLI configuration profiles")

Uh oh!

Conversation

dspangen commented Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changed

Contracts, and the decisions behind them

1. stderr holds exactly one JSON document per failed invocation

2. A non-JSON 2xx body is pass-through success

3. Bare group: help to stderr, exit 1

4. A typo plus --help is still a typo

Verification

Uh oh!

n8agrin left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

dspangen commented Aug 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dspangen commented Aug 24, 2026 •

edited

Loading

4. A typo plus `--help` is still a typo

n8agrin left a comment •

edited

Loading