Skip to content

fix(core): keep stdout pure when -o json/yaml/csv is requested - #99

Closed
GregHolmes wants to merge 1 commit into
mainfrom
fix/output-format-json-purity
Closed

fix(core): keep stdout pure when -o json/yaml/csv is requested#99
GregHolmes wants to merge 1 commit into
mainfrom
fix/output-format-json-purity

Conversation

@GregHolmes

Copy link
Copy Markdown
Contributor

Closes #98. Branched off main, independent of the other open PRs.

The bug

Commands print status lines and human tables for people; the framework writes the serialised result to stdout. Under a machine-readable format both went to the same stream:

$ dg -o json projects | jq .
Fetching projects...        # stdout
Found 1 project(s):         # stdout
{ ... }                     # stdout, too late for jq

The payload was never missing — just buried. In dg -o json models the JSON started 37 KB into stdout, behind a Rich table.

Status routing keyed off agentic (a TTY/env heuristic) rather than "does something else own stdout", so an explicit -o json on a terminal still sent status to stdout. listen and speak escaped because they gate their own display on the format; the other eight commands didn't.

The fix

  • StatusConsole resolves its write target at write time: stderr when the active format is in MACHINE_FORMATS (json/yaml/csv), stdout otherwise. The shared console is one, so print_info/print_success/print_warning/print_panel/print_separator follow automatically.
  • stdout_console carries the payload; print_output now uses it, so the result is never diverted along with the status text.
  • get_status_console() replaces the private Console() in the eight affected commands — a one-line change each.

table is deliberately not a machine format: it's a human rendering and keeps stdout.

What I deliberately did not do

Wrapping handle() in redirect_stdout would have been a smaller diff, but dg speak streams audio through sys.stdout.buffer.write() and branches on sys.stdout.isatty() — a blanket redirect would corrupt piped audio and flip that branch. Routing at the console layer leaves sys.stdout untouched.

Verification

check result
dg -o json {models,projects,keys,usage,billing,requests,members,read} stdout parses as JSON ✓ (was polluted)
-o yaml / -o csv valid YAML ✓ / CSV header ✓
dg speak "..." | ffplay RIFF/WAV stream, no JSON mixed in ✓
dg -o json listen f.mp3 | jq unchanged ✓
human output in a real pty unchanged; status still on stdout ✓
-o json in a pty payload on stdout, status on stderr ✓
unit tests 1059 passed, 6 skipped (7 new regression tests)
live API smoke suite 68/68

The 5 print_output tests that asserted it writes via console were repointed to stdout_console — that coupling is precisely what this change fixes.

Note for review

Three files (output.py, projects/command.py, usage/command.py) are stored with CRLF line endings. My first pass normalised them to LF and ballooned the diff to ~1300 lines; I restored their original convention, so the diff is now 127 insertions / 31 deletions. Worth considering a .gitattributes to settle line endings repo-wide — mixed conventions make diffs like that easy to create by accident.

Closes #98.

Commands print status lines and human tables for people, while the framework
writes the serialised result to stdout. With a machine-readable format those
two collided on the same stream, so the JSON arrived after the human output:

  $ dg -o json projects | jq .
  Fetching projects...          <- stdout
  Found 1 project(s):           <- stdout
  { ... }                       <- stdout, too late for jq

The payload was never missing, just buried — in `dg -o json models` the JSON
started 37KB into stdout, behind a Rich table. `listen` and `speak` were
unaffected because they gate their own display on the format; the other eight
commands did not.

Status routing previously keyed off `agentic` (a TTY/env heuristic) rather than
"does something else own stdout", so an explicit `-o json` on a terminal still
sent status to stdout.

- Add `StatusConsole`, which resolves its target at write time: stderr when the
  active format is in MACHINE_FORMATS (json/yaml/csv), stdout otherwise. The
  shared `console` becomes one, so print_info/success/warning/panel/separator
  follow automatically.
- Add `stdout_console` for the payload and point `print_output` at it, so the
  result is never diverted along with the status text.
- Add `get_status_console()` and use it in read, models, projects, keys,
  billing, usage, requests and members in place of a private `Console()`.

`table` is deliberately not a machine format: it is a human rendering and keeps
stdout.

Deliberately avoided redirecting sys.stdout around handle(): `dg speak` streams
audio via `sys.stdout.buffer.write()` and branches on `sys.stdout.isatty()`, so
a blanket redirect would corrupt piped audio.

Verified: stdout is parseable JSON for all eight commands (plus valid YAML/CSV);
`dg speak | ffplay` still receives a RIFF/WAV stream with no JSON mixed in;
`dg -o json listen | jq` unchanged; human TTY output byte-for-byte as before,
with status now on stderr under -o json. 1059 unit tests pass (7 new regression
tests for the routing) and the 68-check live API smoke suite is green.
@GregHolmes

Copy link
Copy Markdown
Contributor Author

Superseded by #101 — closing.

#97 landed the per-command pattern (status_console for chrome, human rendering gated on get_output_format() == "default") across seven commands, so the central format-aware console here would now conflict with main rather than complement it. #101 finishes the sweep for keys, the one command #97 missed.

The idea from this PR worth keeping — a shared primitive rather than a Console(stderr=True) declared per command — is in #101 as get_status_console() in core, with all eight commands migrated to it.

@GregHolmes GregHolmes closed this Aug 18, 2026
GregHolmes added a commit that referenced this pull request Aug 18, 2026
…sweep) (#101)

Last command in the `-o json` sweep started by #97, plus the review
follow-ups that came out of it.

#97 fixed **requests, read, models, projects, members, usage, billing**
— but `keys` wasn't in that sweep, so it was left as the one account
command whose stdout still broke pipes:

```console
$ dg -o json keys | jq -r '.keys[0].key_id'
jq: parse error: Invalid numeric literal at line 1, column 9
```

`Fetching API keys...` and the Rich table were going to stdout ahead of
the JSON.

## The keys fix — same pattern as the merged seven

- `status_console` for progress, errors and the empty-state notice, so
chrome never touches stdout
- Human rendering gated on `get_output_format() == "default"`: the list
table, created-key details, key details, and both dry-run summaries

The created key's **secret is unaffected** — it already travels in
`KeysResult.created_key.key`, so json/yaml/csv callers still receive it;
only the human echo is suppressed. There's a test asserting exactly
that.

## Review follow-ups

Reviewing the above turned up four more defects. All predate this
branch, but three of them sit in code it touches and all four make "`-o
json` keeps stdout parseable" true only in the narrow case, so they're
fixed here rather than deferred.

**`dg keys --create --dry-run` never ran.** `handle` read `project_id`
and `dry_run` with `.get()` and then forwarded `**kwargs` alongside
them, so every argument arrived twice and Python raised `got multiple
values for argument 'project_id'` before the body started — swallowed
into an error result, exit 0. The dry-run gating added here was
unreachable, and this PR's original "known limitation" note described
behaviour that path never had. Popping both fixes it; fixing only
`dry_run` just exposes the `project_id` collision behind it.

**Failures exited 0.** `main.py` caught `SystemExit` and discarded the
code, so no command could ever signal failure — including the auth-guard
failure that already raised `SystemExit(1)`. `if dg -o json keys; then`
took the success branch on a failed call. The code is now carried
through the post-run notifications and re-raised, and `BaseCommand` maps
status to exit code in one table, matching the contract already
published in `llms-full.txt` (`0 = success, 1 = error, 2 = user
interrupt`): `error` → 1, `cancelled` → 2, everything else → 0. Exiting
0 on failure violated that documented contract, so this restores
published behaviour rather than introducing a new one — no docs change
needed.

**`-o yaml` and `-o csv` silently deleted user data.** Both printed
through Rich, which treats square brackets as style markup and removes
them — a key comment of `[ci] runner` came out as `runner`, no error.
Rich also hard-wrapped at the console width, injecting newlines into the
middle of a csv field. Both now write the payload verbatim (markup,
highlighting and wrapping off), still via `console` so `--quiet` keeps
working. The JSON path was already safe (`print_json` escapes rather
than interprets) and is untouched.

**`dg keys --delete ID` without `--yes` deleted nothing and blamed the
user.** `BaseCommand.confirm` returns its default whenever any parameter
came from the command line, and `--delete KEY_ID` is itself such a
parameter — so the prompt was unreachable and the command always
returned `Cancelled by user` without asking or calling the API. It now
prompts on stderr when someone is there to answer (stderr, so `-o json`
stdout stays clean), and returns a usage error naming `--yes` when
nobody is.

Also: `get_status_console()` in core replaces the eight per-command
`Console(stderr=True)` declarations. A per-command console silently
missed core's agentic no-color settings, and a new command reaching for
a bare `Console()` is exactly how `keys` regressed in the first place —
this makes it correct by default.

## Verification

| check | result |
|---|---|
| `dg -o json keys \| jq -r '.keys[0].key_id'` | returns the id ✓ (was a
parse error) |
| all 8 commands under `-o json` | keys, projects, models, usage,
requests, members, billing, read — all valid JSON on stdout ✓ (models is
77KB, so the wrapping fix holds at size) |
| `-o yaml` / `-o csv` for keys | valid, and bracketed values now
survive verbatim ✓ |
| chrome under `-o json` | still on stderr ✓ |
| human output | unchanged under a pty ✓ |
| `keys --create --dry-run` | reports `dry_run`, calls nothing ✓ (was a
TypeError) |
| `keys --delete ID` without `--yes` | usage error naming `--yes`, exit
1, no API call ✓ |
| exit codes | error → 1, cancelled → 2,
success/dry_run/`--help`/`--version` → 0, unknown command → 2 ✓ —
matches documented contract |
| tests | **1089 passed** (22 new), `ruff check` and `mypy` clean |

## Still open in #98

**#98 stays open for one item:** the promise that stdout auto-switches
to JSON *when piped*. `dg keys | jq` still gets the Rich table, because
the auto-switch (`setup_output`) sits behind `is_agentic()`, which needs
3+ soft signals; a plain pipe from an interactive shell scores 1.
Lowering that threshold changes what `dg <anything> | less` does for
every command, so it's a product decision rather than a bug fix —
deliberately not made here.

## Note on #99

#99 fixed the stdout bug with a central format-aware console before #97
landed. Now that #97 is merged, #99 is redundant and would conflict, so
I've closed it. Its one good idea — a shared status-console primitive
instead of per-command declarations — is implemented here as
`get_status_console()`.
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.

-o json is not honoured by most commands: human output precedes JSON on stdout, breaking pipes

1 participant