fix: Surface server errors in CLI events and unregister commands - #118
Merged
Conversation
CLI --json mode now outputs the full error dict and exits non-zero when
the server returns {"error": ...}. Text mode prints to stderr. Covers
both cmd_events (e.g., resume with nonexistent session) and
cmd_unregister (e.g., session not found).
Fixes #116
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
Summary
Clean fix for CLI error surfacing in cmd_events and cmd_unregister. Both commands now detect {"error": ...} responses from the server, print appropriately (JSON to stdout or message to stderr depending on mode), and exit non-zero. Tests cover all three paths well.
Verdict
APPROVE - Code is correct, well-tested, and addresses the linked issues.
Automated review by Claude Code
Owner
Author
|
No reviewer feedback to address. PR is approved and CI green. |
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
1 participant
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.
Summary
cmd_eventsandcmd_unregisternow detect and surface server-side{"error": ...}responses--jsonmode: outputs the full error dict to stdout for machine consumptionContext
Discovered during live testing of #115 — the CLI
--resume --jsonmode silently returned{"events": [], "next_cursor": null}instead of surfacing the server's{"error": "Session not found"}response.Note: The venv bootstrap task from #117 was already fixed by PR #109 (migration to uv), so only the CLI error surfacing is addressed here.
Fixes #116, fixes #117
Test plan
test_events_error_json_mode— JSON mode outputs error dict and exits 1test_events_error_text_mode— text mode prints to stderr and exits 1test_unregister_error_surfaces— unregister surfaces errors to stderr🤖 Generated with Claude Code