Skip to content

Reap child when capture_output errors in CommandRunner::run - #401

Merged
Finesssee merged 1 commit into
mainfrom
fix/capture-output-reap-child
Aug 29, 2026
Merged

Reap child when capture_output errors in CommandRunner::run#401
Finesssee merged 1 commit into
mainfrom
fix/capture-output-reap-child

Conversation

@Finesssee

@Finesssee Finesssee commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator

Closes #400.

The early ? on capture_output in CommandRunner::run propagated errors immediately, dropping the Child without kill()/wait() — a zombie on Unix until parent exit. Now routes the error through finish_child (the same kill+wait+recover teardown used by every other path since #398/#399) before propagating.

Verification: cargo build, cargo clippy --all-targets -- -D warnings, and cargo test (1365 passed) all green on a clean origin/main + this patch. Note: the current repo checkout carries unrelated concurrent WIP in rust/src/logging.rs that fails to compile on its own; it is not part of this PR.

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of output-capture failures by safely cleaning up the running process before reporting an error.
    • Preserved existing behavior for successful command execution and output capture.

@coderabbitai

coderabbitai Bot commented Aug 29, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 40de7547-24fd-4a83-b392-4fcf9316f9f0

📥 Commits

Reviewing files that changed from the base of the PR and between a84ffb5 and c1f6e19.

📒 Files selected for processing (1)
  • rust/src/host/command_runner.rs

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

CommandRunner::run now cleans up the child process when output capture fails. Successful output capture and result construction remain unchanged.

Changes

Command runner cleanup

Layer / File(s) Summary
Capture failure cleanup
rust/src/host/command_runner.rs
Output-capture errors now trigger finish_child before CommandRunner::run returns the error.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to c1f6e

The change ensures a child process is cleaned up before a capture error is returned, preventing lingering processes without introducing an identified merge-blocking risk. It is merge-ready after normal checks and review.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: reaping the child process when output capture fails in CommandRunner::run.
Linked Issues check ✅ Passed The change satisfies issue #400 by routing capture_output failures through child cleanup before returning the original error, preventing unreaped Unix zombie processes.
Out of Scope Changes check ✅ Passed The seven added lines and one removed line directly implement the linked issue objective. No unrelated changes are reported.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/capture-output-reap-child

Comment @coderabbitai help to get the list of available commands.

@Finesssee
Finesssee merged commit f4375c3 into main Aug 29, 2026
2 of 3 checks passed
@Finesssee
Finesssee deleted the fix/capture-output-reap-child branch August 29, 2026 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

capture_output error path drops Child without kill/wait (zombie on Unix)

1 participant