Fix tokio bash timeout buffered output capture

[Sewer56]

Summary

• keep bounded timeout behavior in tokio bash while preserving buffered output after grace expiration.
• refactor stdout/stderr drain tasks in llm-coding-tools-core/src/tools/bash/tokio_impl.rs to use a shared Arc<Mutex<Vec<u8>>> buffer so aborting the drain task cannot drop already-read bytes.
• retain the 100ms grace window for deterministic timeout latency and add grace_abort_retains_shared_pipe_buffer to verify buffered data survives the abort path.

Testing

• .cargo/verify.sh

[codecov]

Codecov Report

❌ Patch coverage is 93.33333% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.13%. Comparing base (4cac3b1) to head (f5ea4b8).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
...llm-coding-tools-core/src/tools/bash/tokio_impl.rs	93.33%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #42      +/-   ##
==========================================
+ Coverage   74.84%   75.13%   +0.28%     
==========================================
  Files          67       67              
  Lines        1980     2019      +39     
==========================================
+ Hits         1482     1517      +35     
- Misses        498      502       +4     

Flag	Coverage Δ
unittests	75.13% <93.33%> (+0.28%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...llm-coding-tools-core/src/tools/bash/tokio_impl.rs	95.69% <93.33%> (-4.31%)	⬇️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[coderabbitai]

Warning

Rate limit exceeded

@Sewer56 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 11 minutes and 45 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 9e4bc83 and f5ea4b8.

📒 Files selected for processing (1)

• src/llm-coding-tools-core/src/tools/bash/tokio_impl.rs

Walkthrough

Adds asynchronous pipe-draining infrastructure (PipeDrainTask, SharedPipeBuffer, spawn/take/await helpers), constants for drain behavior, and a helper to build timeout errors that include buffered stdout/stderr. Refactors execute_command to spawn concurrent drains for stdout/stderr, race process completion against a timeout, kill the process tree on timeout, and continue draining with a grace period before returning either aggregated BashOutput or a timeout error containing buffered output. Adds tests covering timeout buffered output, grace abort behavior, large-output handling, exit codes, and invalid working directories.

Possibly related PRs

• Sewer56/llm-coding-tools PR 9 — Implements the same concurrent stdout/stderr pipe draining, timeout-driven process-tree termination, and related tests that overlap strongly with this change.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: fixing timeout behavior in tokio bash to preserve buffered output.
Description check	✅ Passed	The PR description provides a clear summary, explains the technical refactoring, and specifies the testing approach, though it could be more comprehensive.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/tokio-timeout-buffered-output

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/llm-coding-tools-core/src/tools/bash/tokio_impl.rs`:
- Around line 35-44: The await_pipe_drain_task_with_grace function currently
aborts the JoinHandle<Vec<u8>> on timeout which drops the task-local Vec<u8>
buffer; remove the task.abort() call in the tokio::select! branch that handles
tokio::time::sleep(grace) so the code instead simply awaits the task (i.e.,
return task.await.unwrap_or_default()) to preserve any buffered output; if you
still need strict timeout semantics, refactor the drain task to write into a
shared/external buffer (rather than task-local state) so it can be read after
aborting.

---

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 829801c and d685824.

📒 Files selected for processing (1)

• src/llm-coding-tools-core/src/tools/bash/tokio_impl.rs

📜 Review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: Build and Test (Async/Tokio) (macos-latest, aarch64-apple-darwin, false)
• GitHub Check: Build and Test (Blocking) (windows-latest, x86_64-pc-windows-msvc, false)
• GitHub Check: Build and Test (Async/Tokio) (windows-latest, x86_64-pc-windows-msvc, false)
• GitHub Check: Build and Test (Async/Tokio) (ubuntu-latest, x86_64-unknown-linux-gnu, false)

🧰 Additional context used

📓 Path-based instructions (1)

src/**/*.rs

📄 CodeRabbit inference engine (src/AGENTS.md)

src/**/*.rs: Preallocate collections when size is known or estimable using String::with_capacity(estimated_len), Vec::with_capacity(count), or BufReader::with_capacity(size, reader)
Prefer &str / &[T] returns over owned types when lifetime allows
Use Cow<'_, str> for conditional ownership (e.g., String::from_utf8_lossy)
Use &'static str for compile-time constant strings
Reuse buffers by calling .clear() and reusing Vec/String instead of reallocating
Use const generics for compile-time branching (e.g., <const LINE_NUMBERS: bool>)
Use #[inline] on small, hot-path functions
Prefer core over std where possible (e.g., core::mem over std::mem)
Use memchr crate for fast byte searching over manual iteration
Keep modules under 500 lines (excluding tests); split if larger
Place use statements inside functions only for #[cfg] conditional compilation
Document public items with /// and add examples in docs where helpful
Use //! for module-level documentation
Use [TypeName] rustdoc links instead of backticks in documentation

Files:

• src/llm-coding-tools-core/src/tools/bash/tokio_impl.rs

🧬 Code graph analysis (1)

src/llm-coding-tools-core/src/tools/bash/tokio_impl.rs (1)

src/llm-coding-tools-core/src/tools/bash/blocking_impl.rs (1)

• execute_command (17-125)

🔇 Additional comments (3)

src/llm-coding-tools-core/src/tools/bash/tokio_impl.rs (3)

17-27: Good drain-task setup with preallocation.

This is a solid change: drains are decoupled and the buffer is preallocated for the expected pipe size.

As per coding guidelines, src/**/*.rs: “Preallocate collections when size is known or estimable using String::with_capacity(estimated_len), Vec::with_capacity(count), or BufReader::with_capacity(size, reader)”.

---

48-75: Timeout error construction is clear and useful.

Including buffered stdout/stderr in the timeout message is a practical UX improvement for debugging failed commands.

---

231-247: Nice regression test for buffered timeout output.

This test directly validates the intended behavior change and protects against regressions in timeout-output preservation.