fix(cli): flush stdio before exiting so piped output survives - #1411
fix(cli): flush stdio before exiting so piped output survives#1411willwashburn wants to merge 2 commits into
Conversation
`agent-relay cloud session --json` (and any other command) could emit nothing at all — empty stdout *and* empty stderr — when stdout was a pipe rather than a TTY. Node's stdio writes are synchronous for files and POSIX TTYs but asynchronous for pipes on macOS, so a `process.exit()` in the same tick as the write discards whatever is still buffered. Interactively the same command printed fine, which made the failure look TTY-conditional. Add `exitAfterFlush()`, which drains stdout and stderr (bounded by a timeout, so a stalled reader can't wedge the exit) before calling `process.exit`, and route the three hard-exit paths through it: the top-level failure handler, the `CliExit` path in `runCli`, and `runSignalHandler`. Fixes #1371 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0129AoahEiYiRdbPGQjNpjdf
📝 WalkthroughWalkthroughThe patch adds bounded stdio flushing before CLI hard exits. It updates error and signal exit paths and adds regression coverage for non-TTY cloud session JSON output. ChangesCLI output and exit handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Poem Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/cli/src/cli/commands/cloud.test.ts (1)
426-429: 🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy liftAdd an end-to-end stdout assertion for the output-loss regression.
This test replaces
console.logand checks only its arguments. It does not verify that bytes reachprocess.stdoutbefore CLI termination. Add a child-process or real-stream test with non-TTY stdout that asserts the redacted JSON is received after the command exits.🤖 Prompt for 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. In `@packages/cli/src/cli/commands/cloud.test.ts` around lines 426 - 429, Add an end-to-end test for the CLI output path around the existing console.log spy, using a child process or real non-TTY process.stdout instead of mocking console.log. Execute the command, wait for it to exit, and assert that stdout contains the expected redacted JSON, verifying bytes are emitted before termination.
🤖 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 `@CHANGELOG.md`:
- Line 12: Update the changelog section heading from “## [Unreleased - Patch]”
to the standard “## [Unreleased]”, keeping the existing entry under the “###
Fixed” subsection.
---
Nitpick comments:
In `@packages/cli/src/cli/commands/cloud.test.ts`:
- Around line 426-429: Add an end-to-end test for the CLI output path around the
existing console.log spy, using a child process or real non-TTY process.stdout
instead of mocking console.log. Execute the command, wait for it to exit, and
assert that stdout contains the expected redacted JSON, verifying bytes are
emitted before termination.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ab25c0ec-605f-44d9-95b9-f46908bb0ca7
📒 Files selected for processing (7)
CHANGELOG.mdpackages/cli/src/cli/bootstrap.tspackages/cli/src/cli/commands/cloud.test.tspackages/cli/src/cli/index.tspackages/cli/src/cli/lib/exit.tspackages/cli/src/cli/lib/flush-stdio.test.tspackages/cli/src/cli/lib/flush-stdio.ts
|
|
||
| ### Fixed | ||
|
|
||
| - CLI output no longer disappears when stdout or stderr is a pipe instead of a terminal. Node's stdio writes are asynchronous for pipes on macOS, so exiting in the same tick as the write discarded whatever was still buffered — `agent-relay cloud session --json | parser` and `$(agent-relay …)` could come back with empty stdout _and_ empty stderr, hiding the payload and the error that explained the failure. Every hard-exit path now drains stdio first. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the standard unreleased heading.
Line 12 adds an unreleased entry, but the section heading is ## [Unreleased - Patch]. Change it to ## [Unreleased]. Keep this entry under ### Fixed.
As per coding guidelines, maintain the correctly leveled [Unreleased] heading.
🤖 Prompt for 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.
In `@CHANGELOG.md` at line 12, Update the changelog section heading from “##
[Unreleased - Patch]” to the standard “## [Unreleased]”, keeping the existing
entry under the “### Fixed” subsection.
Source: Coding guidelines
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2491e0b784
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| timer.unref?.(); | ||
|
|
||
| try { | ||
| stream.write('', () => settle()); |
There was a problem hiding this comment.
When a downstream reader closes a pipe while output is still pending (for example, a verbose command piped to head), the pending stdout write emits an asynchronous EPIPE error event. The callback and surrounding try/catch do not consume that event, and awaiting the flush gives Node time to treat it as unhandled, print a stack trace, and exit with code 1 instead of the command's requested code. This reproduces on Node 24 with a large process.stdout.write followed by this zero-length flush piped to head; install a temporary stream error handler during the flush or otherwise handle EPIPE explicitly.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CHANGELOG.md (1)
14-14: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winPlace this entry under
[Unreleased].The entry is currently below
## [11.3.1] - 2026-07-31, so it is recorded as part of the July 31, 2026 release. Move the existing bullet into a### Fixedsubsection under## [Unreleased].As per coding guidelines, curate
CHANGELOG.mdusing Keep a Changelog and SemVer with a correctly leveled[Unreleased]section.🤖 Prompt for 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. In `@CHANGELOG.md` at line 14, Move the existing CLI stdio-draining bullet from the 11.3.1 release section into a “### Fixed” subsection under “## [Unreleased]” in CHANGELOG.md, preserving its text and Keep a Changelog heading hierarchy.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@CHANGELOG.md`:
- Line 14: Move the existing CLI stdio-draining bullet from the 11.3.1 release
section into a “### Fixed” subsection under “## [Unreleased]” in CHANGELOG.md,
preserving its text and Keep a Changelog heading hierarchy.
2 participants
Summary
Fixes #1371 —
agent-relay cloud session --jsonemitting nothing when stdout is not a TTY.The command itself never gated on a TTY; the output was written and then thrown away. Node's
process.stdout/process.stderrwrites are synchronous for files and POSIX TTYs but asynchronous for pipes on macOS, so aprocess.exit()in the same tick as the write discards whatever is still buffered. Piping or$(...)-capturing a run that exits through one of those paths yields empty stdout and empty stderr — including the error text that would have explained the failure — while the identical run in a terminal prints fine. That asymmetry is what made the bug read as TTY-conditional.Changes:
packages/cli/src/cli/lib/flush-stdio.ts:flushStream/flushStdio/exitAfterFlush. A zero-length write is queued behind the pending chunks, so its completion callback is the signal that the earlier writes reached the OS. The wait is bounded (2s default,unref'd timer) so a stalled reader can never wedge the exit, and a throwing/destroyed stream resolves rather than hangs.exitAfterFlush: the top-level failure handler inindex.ts, theCliExitpath inrunCli(bootstrap.ts), andrunSignalHandler(lib/exit.ts).[Unreleased - Patch].No behavior change to the success path of
cloud session --jsonbeyond the guarantee — it never calledprocess.exit, so its output already survived; the silent-failure case is what this recovers.Test Plan
packages/cli/src/cli/lib/flush-stdio.test.ts— a fake stream that completes its write callback out-of-band provesexitAfterFlushdoes not exit until the drain lands, that the timeout releases a stalled stream, and that destroyed/undefined/throwing streams resolve. Platform-independent, so it holds on Linux CI.packages/cli/src/cli/commands/cloud.test.ts—cloud session --jsonwithprocess.stdout.isTTY = falseand no dependency overrides, asserting the productionconsole.logpath emits the JSON.cloud session --jsonpiped, redirected,$(...)-captured, and with stdin on/dev/null— JSON on stdout each time.npx vitest run packages/cli→ 888 passed, 11 skipped;tsc --noEmitclean;npm run lintunchanged (0 errors);prettier --checkclean.🤖 Generated with Claude Code
Generated by Claude Code