Skip to content

unit (windows) is red on dev: the V1 shell's wait never ends, and it is not the PTY bug #40

Description

@Edo771977

Split out of a wrong attribution in #29. That issue's comment and #37's both said this failure was the lost PTY exit; it is not, and nobody had checked the path.

The failure

unit (windows) has been red on every push to dev since 7f1b836b23 (run 36349524088), with exactly one failing test:

error: the shell never exited
(fail) loop waits while shell runs and starts after shell exits [16459.64ms]

packages/opencode/test/session/prompt.test.ts:1747. The command is sleep 0.2; the named wait that fails is awaitWithTimeout(Fiber.await(sh), "the shell never exited", "15 seconds"), so the shell fiber is still running fifteen seconds after a 200ms command. Measured on Linux the whole test is 1.8s.

Why it is not #29

There is no pseudoterminal in this path at all. SessionPrompt.shell → shellImpl (packages/opencode/src/session/prompt.ts:458) builds ChildProcess.make(sh, args, …) and spawns it through ChildProcessSpawner, provided by CrossSpawnSpawner.node (packages/opencode/src/tool/registry.ts:449). #pty and bun-pty appear nowhere in it. #29's defect cannot reach this test, and #29's fix will not turn this job green.

It is the same shape as #29 — a process that ends and a wait that is never released — which is presumably why it was misread.

The lead

packages/core/src/cross-spawn-spawner.ts. The exit signal is resolved only on close, never on exit:

proc.on("exit", (...args) => { exit = args })
proc.on("close", (...args) => {
  if (end) return
  end = true
  Deferred.doneUnsafe(signal, Exit.succeed(exit ?? args))
})

and handle.exitCode is Deferred.await(signal). close fires only once every stdio stream is closed, not when the child dies — so anything holding a pipe open holds the wait open, with the child long gone.

On Windows those pipes are opened as overlapped rather than pipe:

const pipe = (x) => (process.platform === "win32" && x === "pipe" ? "overlapped" : x)

stdio: overlapped requires the reader to keep reading for the handle to be released. The test's caller does read (Stream.runForEach(Stream.decodeText(handle.all), …)) and then awaits handle.exitCode, so the plausible readings are that one of stdout/stderr never reaches its end event under overlapped, or that the merge of the two leaves one side unread once the other has ended. Neither is measured yet — this issue is the diagnosis, not the cause.

Worth checking against the same file's release path, which treats win32 specially (if (process.platform === "win32") return yield* Effect.void) and kills through taskkill /T /F.

Why it matters beyond one test

While dev is red on this job, a genuine Windows regression in any pull request is indistinguishable from it. That cost was recorded on #29 and belongs here instead: three consecutive unit (windows) failures on #37 had three different causes and none of them was the change under review.

If the resolution is really about close versus exit, it is not test-only: every V1 shell invocation, every shell tool call and every mcp child process on Windows waits on the same deferred.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions