Uh oh!
There was an error while loading. Please reload this page.
fix(shell): drain stdout before reading output to avoid "(no output)" on exit 0 - #35543
Closed
chethanuk wants to merge 1 commit into
Closed
fix(shell): drain stdout before reading output to avoid "(no output)" on exit 0#35543chethanuk wants to merge 1 commit into
chethanuk wants to merge 1 commit into
Conversation
… on exit 0 The output reader was forked with forkScoped but only handle.exitCode was awaited, so the scope could close and interrupt the reader before buffered stdout/stderr drained to EOF, yielding "(no output)" on a clean exit 0 (anomalyco#35511). Capture the reader fiber and, on a clean exit, join it so the merged stream drains before the scope closes. Bound the best-effort drain with the abort/timeout races so a backgrounded child holding the pipe open cannot hang the tool; a clean exit is never relabelled aborted/terminated if the drain is cut short.
This was referenced Jul 14, 2026
Contributor
Automated PR Cleanup Thank you for contributing to opencode. Due to the high volume of PRs from users and AI agents, we periodically close older PRs using automated criteria so maintainers can focus review time on the most active and community-supported contributions. This PR was closed because it matched the following cleanup criteria:
PRs created within the last month are not affected by this cleanup. If you believe this PR was closed incorrectly, or if you are still actively working on it, please leave a comment explaining why it should be reopened. A maintainer can review and reopen it if appropriate. Thanks again for taking the time to contribute. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue for this PR
Closes#35511 (staged on fork against
dev).Type of change
What does this PR do?
The shell tool can return
(no output)with exit0on the first call, even when the command clearly printed to stdout.Root cause. The merged stdout+stderr reader runs in a
forkScopedfiber, but the main flow only awaitshandle.exitCode. When the process exits, the enclosingEffect.scopedblock closes and interrupts the reader fiber before it has drained the buffered chunks to EOF, so the assembledlistis empty and the tool reports(no output)despite a clean exit.Fix. Capture the reader fiber and, on the clean-exit branch,
Fiber.joinit so the merged stream drains to EOF before the scope closes. A backgrounded child can keep the stdout/stderr write end open past the parent's exit, so the join is bounded by the sameabort/timeoutraces to avoid hanging forever. Because the process already exited cleanly on this branch, a cut-short drain is not relabelled as aborted/terminated — that would contradict the clean exit code and print a misleading "terminated after exceeding timeout" message.Ordering that the fix guarantees:
How did you verify your code works?
bun run typecheck(tsgo,packages/opencode) — passes.bun test test/tool/shell.test.ts— 24 pass / 0 fail. Includes a new regression that runsecho hi10x and asserts exit0with output (andmetadata.output) never equal to(no output); this reliably reproduced the race before the fix.Screenshots / recordings
Not a UI change.
Checklist