fix(bundle): send command errors to stderr so --json stdout stays parseable#3235
Merged
Merged
Conversation
…seable The bundle command group's _fail() helper is documented as printing 'to stderr', and the module contract is 'human logs go to stderr/console' while --json 'emits machine-readable data on stdout'. But it called console.print(), and the shared console writes to STDOUT, so every bundle error (every command routes through _fail) landed on stdout -- corrupting the JSON stream that --json consumers parse. Add a stderr-bound err_console to _console.py (its documented role as the single Console source) and use it in _fail. stdout now carries only the JSON payload. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes the specify bundle CLI output contract so that error/diagnostic messages are written to stderr, keeping stdout clean and parseable for --json consumers.
Changes:
- Introduces a dedicated Rich
err_consolebound to stderr in the shared console module. - Routes bundle command failures through
err_consolevia the_fail()helper instead of the stdout-boundconsole. - Adds a contract test ensuring
_fail()writes the error message to stderr and not stdout.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/_console.py |
Adds a stderr-bound Rich console (err_console) to centralize error/diagnostic output. |
src/specify_cli/commands/bundle/__init__.py |
Updates _fail() to print errors via err_console, preventing stdout JSON corruption under --json. |
tests/contract/test_bundle_cli.py |
Adds a regression test asserting _fail() writes to stderr and not stdout. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Low
Collaborator
|
Thank you! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The
bundlecommand group documents its output contract as:and its
_fail()helper's docstring says "Print an actionable error to stderr and exit non-zero." But_failcalledconsole.print(...), and the sharedconsole(_console.console = Console(highlight=False)) writes to stdout. Everybundlesubcommand routes failures through_fail(search,info,list,install,update,remove,validate,build,init), so under--jsonan error printedError: ...Rich text onto stdout, corrupting the JSON stream a consumer parses.Fix
Add a stderr-bound
err_consoleto_console.py(the module documented as the single source of RichConsoleinstances) and use it in_fail. stdout now carries only the JSON payload.Testing
uvx ruff checkclean; fulltests/contract/test_bundle_cli.pypasses (24 tests; the existing outside-project guidance test still passes).test_fail_writes_error_to_stderr_not_stdout: asserts_failwrites to stderr and not stdout. Fails before (message was on stdout), passes after._fail('boom')now writesError: boomto stderr with stdout empty.AI Disclosure
Found and fixed with Claude Code (Claude Opus 4.8) under my direction. AI traced
_fail→console→ stdout against the module's stated--json/stderr contract; I reproduced the stdout contamination, confirmed the existing suite still passes, and reviewed the diff before submitting.