fix(dsh-plugin): settle runner promises when the child never emits close

[MaxFreedomPollard]

Problem

browser_session { action: "start" } can hang forever even though the bsk child exits normally: the plugin's close handler never runs, and the plugin's own 120 s defaultTimeoutMs never surfaces either, because the timeout path only kills the child and then keeps waiting for the same close (#180).

Root cause

createBskRunner().run() resolves only on the child's close event. Node emits close only after every stdio pipe has reached EOF, and a pipe reaches EOF only when every process holding a copy of its write handle has closed it — not just bsk.

When bsk has to auto-spawn the daemon (ensure_daemon() → bsk daemon start → detached daemon), the daemon can end up holding those handles. On Windows, CreateProcess inherits every inheritable handle of the parent regardless of what the child's own stdio is set to — Rust's Command calls it with bInheritHandles = TRUE (the only way to change that is still unstable), and the stdio handles a child receives are themselves inheritable unless the child clears the flag, which Node does for itself via libuv's uv_disable_stdio_inheritance() and a Rust binary does not. So the daemon's own stdio is NUL exactly as spawn_detached_at intends, but it also carries copies of the plugin's stdout/stderr pipe handles, and it lives on until its own idle timeout. bsk prints its JSON and exits → exit fires → close never does → the promise never settles. Killing bsk on timeout changes nothing, because bsk is already gone.

This fits the report: the same bsk session start --json returns in ~230 ms from pwsh, from a standalone Node spawn, and from the host's own subprocess service, yet hangs from the plugin runner — the runner is the one caller that waits for close. If the above is right, it hangs exactly on the call that had to spawn the daemon; a quick check on a Windows box is bsk daemon stop, then browser_session start from the plugin (hangs), versus starting the daemon from a console first (returns).

On macOS/Linux the same spawn does not leak: Command puts /dev/null on fds 0–2 and everything else is O_CLOEXEC. I verified this here — the auto-spawned daemon has fds 0–2 on /dev/null and no pipe fds in lsof — which is why the hang is Windows-only in practice, while the runner's dependence on close is platform-independent.

Fix

Stop depending on close alone and settle deterministically in every case (the reporter's suggested direction 3):

• Normal exit: allow a short drain grace (250 ms) for close, then resolve with the output already captured. close normally follows exit in the same event-loop turn, so the grace is only ever paid when something else is still holding the pipes; the normal path is unchanged (measured below).
• Kill we initiated (timeout / abort): resolve on exit immediately — nothing left worth draining.
• Killed child that reports nothing at all: resolve once the SIGKILL grace has passed (KILL_GRACE_MS + 1 s), so the caller is always released.
• Timeout / abort landing after the process already exited: settle at once instead of killing a process that is gone.
• Once settled, detach the stdout/stderr collectors: anything arriving later belongs to whatever still holds the pipes, not to the finished command.

EXIT_DRAIN_GRACE_MS = 250 is a judgment call; happy to change it.

Tests

The existing FakeChild.kill() emitted close on every kill, so the suite was structurally unable to see this. Seven tests added (181 → 188):

• runner.test.ts: settles on timeout / on abort when close never fires; settles after the kill grace when neither exit nor close ever fires (fake timers, asserts the SIGINT → SIGKILL escalation); returns the captured output after a normal exit when close never fires; settles immediately on a timeout that lands after exit; still waits for close on a normal exit so late stdout is not lost.
• tools.test.ts: browser_session start returns the session through the real runner when the child emits exit but never close — the business path from [Bug][dsh-plugin] browser_session start hangs forever on Windows - bsk child exits fine, but plugin close event never fires; its own 120s timeout never surfaces #180.

Six of the seven fail against the current runner.ts (the seventh pins existing behaviour).

Verification

• pnpm lint (the CI chain: biome, stylelint, typecheck, vitest) — exit 0, 188/188.
• Real processes on macOS (sh children driven through createBskRunner), main (47ac947) vs this branch:

scenario	main	this branch
child exits 0, a grandchild keeps the pipes open 30 s	hung (>6 s guard)	settled in 254 ms, code: 0, stdout intact
child hangs, grandchild keeps the pipes, 300 ms timeout	hung (>6 s guard)	settled in 301 ms, timedOut: true
child exits 0, nothing holds the pipes	3 ms	3 ms
child hangs, nothing holds the pipes, 300 ms timeout	302 ms	302 ms

Not verified on Windows (no Windows machine here). The Windows mechanism above is derived from the Rust std and libuv sources; the fix itself does not depend on which process is holding the pipes.

Out of scope

The leak could also be closed at the source by restricting handle inheritance when ensure_daemon() / spawn_detached_at() spawn on Windows (PROC_THREAD_ATTRIBUTE_HANDLE_LIST via raw_attribute), so the daemon never receives the caller's pipe handles. I left that out: it is Windows-only code that CI (ubuntu) never compiles, and the runner needs to settle without close regardless.

Closes #180

[iuyo5678]

Thanks for the fix! I reproduced the issue on Windows and confirmed that this PR fixes the hang.

The bounded fallback makes sense. Before merging, I’d like to address two remaining concerns:

1. Resource cleanup: finish() removes the data listeners, but does not close the remaining stdio streams. When another process keeps those pipes open, resolving the promise alone may leave live handles behind. Could you add explicit cleanup for the fallback paths and a real-process test verifying that the runner does not keep the host alive?

2. Output completeness: After exit, the fixed 250ms deadline stops collecting output. Late data can still be buffered output from the original child, so the comment saying it must come from another process is too strong. Could you account for this and add a delayed, chunked-output test that exercises the deadline boundary?

Also, killFor() and killAll() still call killChild() directly, bypassing the new settlement deadline. It would be good to route those through the same bounded shutdown logic.

I’m happy to keep the underlying Windows handle-inheritance fix as a separate follow-up. The original hang is confirmed fixed; the remaining concerns are about reliable cleanup and preserving complete output.

[MaxFreedomPollard]

All three are in now, thanks for the careful read.

Cleanup: every settle path drops the data listeners, destroys our ends of stdin/stdout/stderr and unrefs the child, so a grandchild holding the pipes can't keep the host alive.

Output completeness: you were right that late bytes can be the child's own buffered output, so I've softened that comment. Each chunk now reopens the drain window instead of a fixed deadline, with a hard cap so it stays bounded if something keeps writing.

killFor() and killAll() now go through the same bounded shutdown as the timeout and abort paths rather than calling killChild() directly.

Tests: a real-process test that the host exits while a grandchild still holds the pipes, and a chunked-output test that exercises the drain window and the cap.