fix: report agent explain file errors as json - #3023
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthrough
ChangesAgent explain error handling
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized change makes missing-file errors return structured JSON while preserving successful explanations, and no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@akbash-bot fix conflicts pls |
cb75c18 to
2c438d4
Compare
Greptile SummaryThe PR converts
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| src/cli/agent.rs | Converts fixture read errors into the CLI's structured ErrorResponse format and returns exit code 1. |
| tests/cli/agents.rs | Verifies missing fixture files produce parseable JSON on stderr, no stdout, and the expected failure status. |
| docs/next/CHANGELOG.md | Records the new structured error behavior for agent explanation fixture reads. |
Reviews (2): Last reviewed commit: "fix: report agent explain file errors as..." | Re-trigger Greptile
|
Done — rebased onto current master, resolved the changelog conflict, and all PR checks plus CodeRabbit and Greptile are green at 2c438d4. |
|
@akbash-bot fix conflict |
|
@akbash-bot the implementation review is clean. please rebase onto current master, resolve the changelog conflict, and rerun checks and both review bots. |
|
I rebased and resolved the changelog conflict locally, but I cannot update this PR head: |
2c438d4 to
565a2fb
Compare
Issue
A missing file passed to
herdr agent explain --fileprints a Rust I/O error instead of structured JSON. Shell wrappers cannot read an error code from that output.Problem
The local fixture read returned its operating-system error directly. The CLI's top-level error handling then printed the Rust debug representation.
How did we fix it?
Herdr now converts local fixture read failures into an
ErrorResponse. It writes oneagent_explain_file_read_failedJSON response to stderr and exits with code 1. Successful fixture and live-pane explanations are unchanged.Verification
The process test failed before because stderr could not be parsed as JSON. It now verifies the exact command, response ID and code, path context, output streams, and exit status. Formatting, clippy, Windows lint, 3,488 other Rust tests, integration assets, and maintenance checks passed.
live_server_holds_one_pty_master_fd_per_panestill fails on clean master because its replacement server does not appear.refs #3022