Repository navigation
fix(task): bound buffered command output - #11922
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughChangesFailure output buffering
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CmdLineRunner
participant ChildCommand
participant ProgressReport
participant ErrorHandler
CmdLineRunner->>ChildCommand: Execute command
ChildCommand->>ProgressReport: Stream stdout
ChildCommand->>CmdLineRunner: Stream stderr
CmdLineRunner->>CmdLineRunner: Retain bounded stdout tail
ChildCommand-->>CmdLineRunner: Return failure
CmdLineRunner->>ErrorHandler: Replay retained tail and truncation notice
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
Greptile SummaryThis PR bounds hidden command-output retention while preserving live stream handling and failure diagnostics.
Confidence Score: 5/5The PR appears safe to merge, with no concrete changed-code defect identified. The new buffer is enabled only for stdout hidden by progress reporting, bounds retained payload, truncates oversized UTF-8 lines safely, and leaves stderr visible through its existing live progress-report path. Important Files Changed
Reviews (1): Last reviewed commit: "fix(task): bound buffered command output" | Re-trigger Greptile |
Summary
Root cause
CmdLineRunnerappended every redacted stdout and stderr line tocombined_outputin both its synchronous and asynchronous execution paths. Most output modes never consumed that vector, including the default parallel prefix mode, so long-running services accumulated output for their entire lifetime. In a local reproduction with a continuously writing task and a second parallel task, mise RSS grew from about 374 MiB after one second to about 1.97 GiB after six seconds.The runner now creates a failure-output buffer only when stdout is hidden behind a progress reporter and therefore needs to be replayed on failure. That buffer keeps a UTF-8-safe 64 KiB tail and emits a truncation notice when older output is discarded.
Verification
mise exec -- cargo test --all-features cmd::tests(21 passed)mise run test:e2e e2e/tasks/test_task_output_bufferingmise run test:e2e e2e/tasks/test_task_parallel_replacing e2e/tasks/test_task_output_style_decoupling e2e/tasks/test_task_raw_output e2e/tasks/test_task_artifact_cache_output e2e/tasks/test_task_parallel_fail_fastmise exec -- cargo check --all-featuresmise exec -- cargo clippy --workspace --all-features --all-targets -- -D warningsmise exec -- cargo fmt --all -- --checkmise exec -- shellcheck e2e/tasks/test_task_output_bufferingmise exec -- shfmt -d e2e/tasks/test_task_output_bufferingmise run lintcould not run because itshktask requires a.gitdirectory, while this checkout is managed by Sapling without one; the underlying Rust, shell, and format checks above passed directly.Addresses #8786
AI-assisted — Tool: Codex; model: OpenAI/GPT-5; version: unavailable.
Summary by CodeRabbit
Bug Fixes
Tests