Skip to content

fix: settle shell completion on process exit instead of pipe EOF - #44601

Open
linsmod wants to merge 2 commits into
anomalyco:devfrom
linsmod:fix/shell-exit-completion
Open

fix: settle shell completion on process exit instead of pipe EOF#44601
linsmod wants to merge 2 commits into
anomalyco:devfrom
linsmod:fix/shell-exit-completion

Conversation

@linsmod

@linsmodlinsmod commented Aug 24, 2026

Copy link
Copy Markdown

Issue for this PR

Closes#20902

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

The bash/shell tool reports a command as done only when the spawned child's stdio pipes fully close (close event). When a command leaves a background descendant alive (gradle daemon, adb server, dev server, Start-Process children), that descendant inherits the pipe and keeps it open after the direct child exits, so the tool never sees EOF and blocks until timeout/abort. Same root cause as #22012, #24784, #29822, #32504, #37838, #42524, #43910.

It's two problems stacked:

  1. cross-spawn-spawner.ts settles exitCode from close instead of exit.
  2. Callers still gate completion on stream EOF — AppProcess.run (Effect.all([collectStream, exitCode])) and the V1 ShellTool stream reader both wait for the pipe to close.

Changes:

  • Settle exitCode from the exit event, keep close as a fallback.
  • process.ts: new collectBounded — reads the stream concurrently (avoids a pipe-full deadlock), waits for process exit, then a short 2s drain window before returning captured output. A shared accumulator preserves partial output if the window expires.
  • opencode/src/tool/shell.ts: after the direct child exits, drain briefly then interrupt the stream-reader fiber so the effect scope closes.
  • tui/src/util/transcript.ts: /copy now renders captured output for aborted/error tool parts.

Completion is now anchored to the direct process exiting instead of pipe EOF, so a surviving grandchild can't block the tool; the bounded drain still captures all in-flight output.

Related fix attempt #42275 only covers the first layer and explicitly leaves callers waiting for stream completion; this PR closes that gap. The background-shell features #40005 / #33310 are intentionally out of scope (managed background jobs remain explicitly opt-in).

How did you verify your code works?

  • bun typecheck: @opencode-ai/core, opencode, @opencode-ai/tui all pass.
  • bun test: process (32), tool-bash, cross-spawn-spawner (incl. new regression: exitCode resolves on direct-child exit while a descendant holds the pipe open), shell tool (66, incl. new regression: foreground exits but a descendant holds the pipe 6s → tool returns in < 8s), transcript (20, incl. aborted-tool output), processor-effect (17).
  • --single build passes the smoke test.
  • Reproduced the real hang (gradle + adb install on Windows): output completed with "Success", the tool part stayed running ~6 min until abort, then ended as error: "Tool execution aborted" with no output in /copy. The same command returns promptly in an interactive pwsh because stdout is a console handle, not a pipe.

Screenshots / recordings

N/A — no UI change.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

@github-actionsgithub-actionsBot added needs:title needs:compliance This means the issue will auto-close after 2 hours. labels Aug 24, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Hey! Your PR title fix(core,opencode,tui): settle shell completion on process exit instead of pipe EOF doesn't follow conventional commit format.

Please update it to start with one of:

  • feat: or feat(scope): new feature
  • fix: or fix(scope): bug fix
  • docs: or docs(scope): documentation changes
  • chore: or chore(scope): maintenance tasks
  • refactor: or refactor(scope): code refactoring
  • test: or test(scope): adding or updating tests

Where scope is the package name (e.g., app, desktop, opencode).

See CONTRIBUTING.md for details.

@github-actions

Copy link
Copy Markdown
Contributor

The following comment was made by an LLM, it may be inaccurate:

Found a related PR:

PR #42275fix(core): resolve child process exit signal on exit event

This PR is directly related but incomplete compared to PR #44601. According to the PR description itself, #42275 only fixes the first layer (spawner exitCode settles on exit) but leaves callers still waiting for stream completion, so continuously-outputting descendants still hang. PR #44601 builds on this by adding the collectBounded layer to bound stream collection and prevent the hang at the caller level.

No other duplicate PRs found for this specific issue.

@linsmodlinsmod changed the title fix(core,opencode,tui): settle shell completion on process exit instead of pipe EOFfix: settle shell completion on process exit instead of pipe EOFAug 24, 2026
@github-actionsgithub-actionsBot removed the needs:compliance This means the issue will auto-close after 2 hours. label Aug 24, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Thanks for updating your PR! It now meets our contributing guidelines. 👍

Sign up for freeto 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.

bash tool hangs when command spawns background child processes

1 participant

@linsmod