Skip to content

fix(core): resolve spawn completion on exit, not only close (Windows detached-child hang)#29831

Open
Hona wants to merge 7 commits into
anomalyco:devfrom
Hona:fix/shell-detached-child-hang
Open

fix(core): resolve spawn completion on exit, not only close (Windows detached-child hang)#29831
Hona wants to merge 7 commits into
anomalyco:devfrom
Hona:fix/shell-detached-child-hang

Conversation

@Hona
Copy link
Copy Markdown
Member

@Hona Hona commented May 29, 2026

Summary

Fix shell commands hanging after their main process exits while a child still holds stdout/stderr open.

Fix

  • Track process exited separately from stream/group closed.
  • exitCode / isRunning resolve on exited; kill escalation (forceKillAfter) waits on closed so resistant descendants are still SIGKILLed.
  • ShellTool completion = joining the output reader (drains to EOF). For a normal command this returns the instant the streams end: full output, no wall-clock delay, same backpressure-synced guarantee as before. A detached child can hold the pipe open forever, so an idle watcher that resets on every chunk bounds only that case -- a command still producing output is never cut off.

Why no temp file / no fixed drain

close only fires once every holder of the stdio write-end closes; a detached grandchild that inherited the pipe keeps it open, so close never comes. We cannot control the grandchild's stdio, so completion must key off process exit. "Exited" does not mean "fully read" -- a small produced-but-unconsumed backlog can remain, which is why we join the reader. A fixed-duration drain truncates that backlog for slow consumers; the idle-reset watcher does not.

Tests

  • Detached child inheriting stdio no longer blocks exitCode (core spawner).
  • POSIX kill escalation still SIGKILLs a SIGTERM-resistant descendant after the parent exits, including non-zero parent exit (core spawner).
  • 1MB of fast output with delayed per-chunk metadata is fully captured after exit (shell tool).
  • A command that leaves a detached child holding stdio open returns promptly via the idle drain instead of hanging on the pipe or the command timeout (shell tool, POSIX).
  • Windows shell suite: 92/92.

Closes #24731
Closes #24784
Closes #20902
Closes #29822
Closes #25038
Closes #28697
Closes #25938

Refs #28654
Refs #22012

The shell tool hangs on Windows when a command spawns a detached child that
inherits stdio (e.g. a backgrounded web server or playwright-cli). The
cross-spawn spawner resolved process completion only on the child's `close`
event, which fires after *all* stdio streams close. A detached child keeps the
inherited pipe's write end open, so `close` is delayed indefinitely even
though the main process already exited -- and exitCode (and thus the shell
tool) never resolves.

Resolve on whichever of `exit`/`close` fires first. `exit` returns
promptly once the main process is gone (fixing the hang); `close` is kept as
the required backstop for failed spawns, where cross-spawn emits
spawn->error->close but never exit, so exit-only would hang that path forever.

Closes anomalyco#24731
Copilot AI review requested due to automatic review settings May 29, 2026 04:33
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a Windows hang in the shell tool when a command spawns a detached child that inherits stdio (e.g. playwright-cli). The CrossSpawnSpawner previously completed only on the child's "close" event, which can be delayed indefinitely if a detached child keeps the parent's stdio pipes open. The fix completes the exit Deferred on whichever of "exit" or "close" fires first, while preserving the "close" listener as a backstop for the ENOENT path where cross-spawn never emits "exit".

Changes:

  • Resolve the spawn completion Deferred in the "exit" handler in addition to the existing "close" handler, with end/exit guards so the value is consistent regardless of order.
  • Added an explanatory comment documenting why both events are needed.
  • Added a regression test (using fx.live) that spawns a detached daemon inheriting stdio and asserts exitCode resolves within 5s, reaping the daemon afterward.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
packages/core/src/cross-spawn-spawner.ts Complete on first of exit/close; keep close as ENOENT backstop.
packages/core/test/effect/cross-spawn-spawner.test.ts New regression test for detached child stdio inheritance (#24731); trailing blank lines added.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/core/test/effect/cross-spawn-spawner.test.ts Outdated
Hona added 6 commits May 29, 2026 14:58
…ed pipes

Replace the fixed 100ms post-exit drain (which truncated output when a slow
per-chunk consumer was still working through a backlog) with joining the output
reader fiber. For a normal command this returns the instant the stdio streams
reach EOF -- full output, no fixed delay, matching upstream's backpressure-synced
behavior. A detached child can hold the pipe open forever with no further output,
so an idle watcher that resets on every chunk bounds only that case; a command
still producing output is never cut off.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment