fix(work): surface captured output on a headless agent timeout instead of discarding it - #398
Conversation
…d of discarding it run_headless's timeout branch reported only "<agent> timed out after Ns" and threw away the child's already-captured stdout/stderr, even though run_captured keeps whatever the process wrote before being killed. Every timeout was a black box with no way to tell a genuinely stuck agent apart from one that finished real work and hung afterward (observed live: an opencode run completed and pushed 4 commits, then hung — the failure comment posted to the item said only 'timed out', giving no hint that work had actually happened).
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughHeadless timeout failures now include captured stdout or stderr diagnostics. Output is limited to the final 2,000 UTF-8-safe characters. Tests cover capture, truncation, multibyte text, and source selection. ChangesHeadless timeout diagnostics
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/agent_launch.rs`:
- Around line 312-325: Update diagnostic_suffix to select stdout whenever
captured.stdout is non-empty, without calling trim on either stream; otherwise
select stderr when non-empty, preserve the no-output fallback, and pass the
selected original stream directly to tail_str.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9b840f4d-5e3f-401b-9333-62a6060baaa8
📒 Files selected for processing (1)
src/agent_launch.rs
…ead of trimming it trim() dropped whitespace-only stdout to empty (wrongly falling back to stderr) and shaved real trailing content off before the tail_str slice. Select on emptiness of the untrimmed stream and pass the original stream to tail_str. Addresses CodeRabbit review comment on PR #398.
Summary
run_headless's timeout branch (src/agent_launch.rs) discarded the child's already-captured stdout/stderr and reported only"<agent> timed out after Ns"— even thoughrun_capturedretains whatever the process wrote before being killed.agentflare work <item> --agent opencoderun completed all of its actual work (4 commits, pushed to a branch) and then hung on something afterward; the timeout failure comment posted to the item said only "timed out", with nothing distinguishing it from a run that made zero progress. Had to manually dig through git branches to discover real work existed.Failedmessage, so the item comment/CLI output actually shows what the agent was doing when it was killed.Test plan
run_captured_keeps_output_written_before_a_timeout_kill— proves output written before a kill survives inCaptured, which the fix depends ontail_strunit tests, including a UTF-8 multi-byte boundary casediagnostic_suffixunit tests: empty-output case, stdout-preferred case, stderr-fallback caseagent_launch::suite still passes (20/20)Summary by CodeRabbit