Preserve agent-coord-bounded output on bounded exits

[justin808]

Why

agent-coord-bounded captured child stdout/stderr into temporary files, but timeout and interrupt exits could return without replaying those captured streams. That made degraded coordination reads harder to diagnose because the underlying command output was lost.

Closes #4320.

What changed

• Added one flush_captured_output helper for the bounded wrapper.
• Reused it in normal completion, timeout, and SIGINT/SIGTERM interrupt paths.
• Added regression coverage for partial stdout/stderr preservation on timeout and interrupted exits.

Validation

• ruby .agents/skills/pr-batch/bin/agent-coord-bounded-test.rb — 13 runs, 72 assertions, 0 failures.
• BUNDLE_GEMFILE=Gemfile bundle exec rubocop .agents/skills/pr-batch/bin/agent-coord-bounded .agents/skills/pr-batch/bin/agent-coord-bounded-test.rb
• .agents/bin/agent-workflow-seam-doctor
• .agents/bin/validate --changed
• git diff --check origin/main...HEAD
• Pre-push hook passed branch Ruby lint and markdown-link checks.
• Codex autoreview: clean.
• Claude second review: NO_FINDINGS.

Batch D notes

This PR only handles the claimed #4320 lane. Issue #4322 could not be claimed because private coordination repeatedly returned write-contention state, and #4323 depends on the unresolved #4322/#4320 follow-up surface, so those lanes remain separate blocked/UNKNOWN dispositions.

[coderabbitai]

Warning

Review limit reached

@justin808, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 3 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 90901fad-5882-47e9-8035-994ae9a284ce

📥 Commits

Reviewing files that changed from the base of the PR and between e03a4ad and fd664b6.

📒 Files selected for processing (2)

• .agents/skills/pr-batch/bin/agent-coord-bounded
• .agents/skills/pr-batch/bin/agent-coord-bounded-test.rb

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/batch-d-agent-tools-4320-4322

---

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.}

[greptile-apps]

Greptile Summary

This PR preserves captured agent-coord-bounded output across more exit paths. The main changes are:

• Added a shared helper for replaying captured stdout and stderr.
• Reused the helper on normal completion, timeout, and interrupt exits.
• Added tests for partial stdout and stderr on timeout and interrupted exits.

Confidence Score: 4/5

The timeout cleanup path can still miss output from helper processes.

• Direct-child output is now replayed on the changed exit paths.
• Process-group helpers can outlive the direct child during timeout cleanup.
• The new replay can read the tempfiles before those helpers finish writing.

.agents/skills/pr-batch/bin/agent-coord-bounded

Important Files Changed

Filename	Overview
.agents/skills/pr-batch/bin/agent-coord-bounded	Adds shared captured-output replay across normal, timeout, and interrupt exits; helper-process output can still be missed during timeout cleanup.
.agents/skills/pr-batch/bin/agent-coord-bounded-test.rb	Adds tests for direct-child partial output on timeout and interrupt exits.

_{Reviews (1): Last reviewed commit: "Preserve agent-coord output on bounded e..." | Re-trigger Greptile}

[claude]

Review

Summary: This PR extracts a flush_captured_output(stdout_file, stderr_file) helper in agent-coord-bounded and calls it from all three exit paths (normal completion, timeout, interrupted/SIGINT/TERM/HUP), so captured child stdout/stderr is no longer dropped on the timeout and interrupt code paths. Test coverage was added for partial output preservation on both the timeout and interrupted-exit paths.

Findings: None. The extraction is mechanically equivalent to the pre-existing flush-before-exit at the end of the happy path — each new call site flushes before its exit(...), and the outer ensure (stdout_file&.close! / stderr_file&.close!) only runs after the read/print completes, so there's no use-after-close or leak.

One pre-existing (not introduced by this diff) narrow race worth a passing mention: interrupted_signal is checked once inside the polling loop (line 122) and again just after it (line 142). If a signal arrives in the same ~50ms window the child exits normally via waitpid2, the post-loop check could report exit code 128+signal instead of the real exit status, since interrupted_signal would already be set. Output would still be flushed correctly either way — only the reported exit code could be off in that edge case. Not worth blocking on since it's unrelated to this PR's scope (output preservation) and pre-dates this change.

Nice use of a shared helper to eliminate the duplicated rewind/read/print sequence — good cleanup along with the fix. Test changes (bumping timeout from 0.2s→1s, asserting on real files instead of File::NULL) look like reasonable flakiness/coverage improvements rather than regressions.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[claude]

Code review

What this PR does: agent-coord-bounded captures the wrapped agent-coord command's stdout/stderr into tempfiles and previously only replayed them on normal completion — a timeout or SIGINT/SIGTERM interrupt exit dropped whatever output had already been captured. This PR adds a flush_captured_output helper reused across all three exit paths, and adds process_group_alive?/wait_for_process_group_exit so terminate_process_group waits for the whole process group (not just the direct child) to exit before giving up — which matters because helper processes reacting to TERM may still be writing output. Regression tests cover partial-output preservation on both timeout and interrupt.

Overall assessment: The core logic is sound. I traced the deadline math in terminate_process_group/wait_for_process_group_exit, the three flush_captured_output call sites (mutually exclusive, no double-flush or skip), and the new tests (including the process-group-helper trap/exit scenario) — all check out and match the stated intent. No correctness bugs found. Two minor, non-blocking notes left as inline comments:

• .agents/skills/pr-batch/bin/agent-coord-bounded:59-72 — the new group-liveness polling widens (by up to ~1s) a pre-existing, very-low-probability pid-reuse race in terminate_process_group's KILL branch. Flagging for awareness, not blocking given this is internal tooling.
• .agents/skills/pr-batch/bin/agent-coord-bounded:165-169 — the interrupted_signal handling block is now duplicated (with the added flush_captured_output line) at two call sites; consider extracting a small helper to avoid future divergence.

Nice addition of the test_timeout_replays_helper_output_before_killing_process_group test — it directly exercises the scenario this PR fixes (a TERM-trapping helper that writes output after the direct child has already exited).