Skip to content

Preserve terminal stderr for subprocesses - #181

Open
hanoii wants to merge 1 commit into
ahoy-cli:masterfrom
hanoii:fix/preserve-child-stderr-tty
Open

Preserve terminal stderr for subprocesses#181
hanoii wants to merge 1 commit into
ahoy-cli:masterfrom
hanoii:fix/preserve-child-stderr-tty

Conversation

@hanoii

@hanoii hanoii commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Preserve the startup stderr stream for child commands.
  • Keep Ahoy's internal stderr drain pipe isolated from subprocesses.
  • Add regression coverage for stream selection and large stderr output.

Why this approach

Of the alternatives discussed in #180, this passes the original stderr directly to subprocesses. It preserves the exact inherited descriptor for terminals, files, and pipes without adding terminal detection or a new dependency.

Testing

  • go test -race ./...
  • bats tests (132 tests passed or skipped as platform-specific)
  • Manual PTY check confirmed /dev/fd/2 changes from a pipe to the terminal character device.

Closes #180

Summary by CodeRabbit

  • Bug Fixes

    • Improved subprocess error output handling so command errors continue to appear directly in the original terminal stream.
    • Prevented subprocess stderr from being delayed or blocked by internal output handling.
  • Tests

    • Added regression coverage confirming stderr remains connected to the stream available at application startup.
  • Documentation

    • Updated test descriptions to clarify expected stderr behaviour.

Capture stderr before Ahoy installs its internal drain pipe so child processes inherit the original terminal descriptor.

Closes ahoy-cli#180
@coderabbitai

coderabbitai Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: edcdbf89-de67-407a-b5eb-196dba4e335d

📥 Commits

Reviewing files that changed from the base of the PR and between 72810f4 and f275ecc.

📒 Files selected for processing (3)
  • ahoy.go
  • ahoy_test.go
  • tests/stderr-passthrough.bats

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


Walkthrough

Ahoy now captures stderr at startup for subprocesses. Child stderr bypasses Ahoy’s drain pipe, while Ahoy’s own stderr remains drained. Tests verify routing between startup and execution-time streams.

Changes

Stderr routing

Layer / File(s) Summary
Capture and route startup stderr
ahoy.go
appState stores the startup stderr stream. newAppState() initialises it. Subprocesses use the captured stream, with an os.Stderr fallback. The main comment documents the routing.
Validate stderr stream separation
ahoy_test.go, tests/stderr-passthrough.bats
Tests verify that child stderr reaches the startup stream and not the execution-time stream. Bats comments describe the direct, live stderr requirement and large-output checks.

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

Merge Risk: ⚪ Minimal · up to f275e

This change preserves child-process stderr behavior while isolating Ahoy's internal drain, with focused regression coverage; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: ocean

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title, "Preserve terminal stderr for subprocesses", clearly and concisely describes the primary change.
Linked Issues check ✅ Passed The implementation addresses issue #180 by capturing the startup stderr stream and assigning it to subprocesses. This prevents child processes from inheriting Ahoy's internal drain pipe and preserves …
Out of Scope Changes check ✅ Passed The code, tests, and comment updates all support the stderr-routing fix described in issue #180. No unrelated changes are present.
Full details: Linked Issues check

Explanation

The implementation addresses issue #180 by capturing the startup stderr stream and assigning it to subprocesses. This prevents child processes from inheriting Ahoy's internal drain pipe and preserves terminal access. The added regression test verifies stream selection, and the change retains direct stderr handling for large child output.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

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

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.

v3.0.0: subprocesses inherit a pipe as stderr, breaking interactive TUI commands (gum, Bubble Tea)

1 participant