fix(broker): stop worker stderr from rendering inside agent xterm#931
Merged
Conversation
The pty_worker subprocess is spawned with `stderr: Stdio::piped()` and
read by `spawn_worker_reader(..., parse_json=false, ...)`. Any non-JSON
line from that reader (i.e. every stderr line, including
`tracing::info!`/`warn!` output) was wrapped as a `worker_stream` event
and forwarded to dashboards, which append `worker_stream` chunks to the
agent's xterm buffer. Tracing logs like the idle watchdog therefore
rendered on top of the CLI's input prompt, breaking the agent UI.
Two changes:
- `spawn_worker_reader`: when `parse_json` is false (the stderr reader),
keep writing the line to the worker log file but skip the
`worker_stream` fallback. Stderr is for diagnostics, not PTY output.
- `pty_worker` idle watchdog: downgrade the two
`"watchdog: no PTY output …"` lines from `info`/`warn` to `debug` so
they stay out of the log file under the default `RUST_LOG=info`
filter. The `send_frame("agent_idle" | "agent_blocked_on_send", …)`
calls are unchanged, so dashboards still receive the state events.
Contributor
|
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 (2)
📝 WalkthroughWalkthroughThis PR reduces logging noise and filters stream event emission in worker output handling. PTY watchdog timeout conditions downgrade from warn/info to debug severity, and stderr processing is modified to prevent non-stdout content from being forwarded as worker_stream events, preserving only PTY-protocol output for client streams. ChangesWorker Output Handling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
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
stderr: Stdio::piped()(crates/broker/src/worker.rs:380-382) and read byspawn_worker_reader(..., parse_json=false, ...). The fallback branch (worker.rs:961) was wrapping every non-JSON line as{"type":"worker_stream","payload":{"stream":"stderr","chunk": line}}and forwarding it to dashboards.worker_streamchunk as PTY output and append it to the agent's xterm buffer. The result: tracing logs from the pty_worker (e.g. the idle watchdog"watchdog: no PTY output for Ns — marking idle") render inside the agent terminal, on top of the CLI's input prompt.RUST_LOG=infofilter.Changes
crates/broker/src/worker.rs— inspawn_worker_reader, skip theworker_streamfallback whenparse_jsonis false. The line is still written to the per-worker log file, so debugging context is preserved.crates/broker/src/pty_worker.rs— downgrade the two idle-watchdog tracing lines frominfo/warntodebug. Thesend_frame("agent_idle" | "agent_blocked_on_send", …)calls are unchanged, so dashboards still receive the state transitions.Test plan
cargo build -p agent-relay-brokerNO_OUTPUT_EXIT_TIMEOUT, and confirm nowatchdog: …text appears in the xterm buffer.agent_idle/agent_blocked_on_sendevents still arrive in the dashboard (state UI still updates).cargo test -p agent-relay-broker(no behavioural change expected for existing tests).