Bug: LSP server stdout has no error listener — one transport failure crashes the whole host process

[FermiHart]

Bug: LSP server stdout has no error listener — one transport failure crashes the whole host process

Package: @deepseek-ai/dsh-lsp-stdio
Severity: medium-high (one language server's pipe failure takes down every session in the process)
Confidence: the stdin/stdout asymmetry is certain; the triggering OS error is platform/timing-dependent (EPIPE/EBADF-class reads)

Problem

packages/lsp/lsp-stdio/src/connection.ts deliberately attaches an error handler to the child's stdin and documents why:

// Child stdin can fail while the process itself remains alive (for example, a server closes fd
// 0). Treat that as a fatal connection error so pending requests reject immediately instead of
// waiting for a process-close event that may never arrive.
this.stdin.on('error', (error) => { this.fail(error) })
this.handle.stdout.on('data', (chunk: Buffer) => { this.onStdout(chunk) })

The stdout stream gets only a data handler — no error handler. In Node, an 'error' event on a stream with no listener is raised as an uncaught exception.

I verified the exposure chain end to end:

1. The subprocess seam hands back the raw child.stdout for pipe mode with nothing attached: packages/subprocess/subprocess-local/src/spawn.ts (stdout: outMode === 'pipe' ? child.stdout ?? undefined : undefined; collectStream only wraps collect-mode streams).
2. No shipped code installs a process-level safety net for this class of failure: app-boot registers only unhandledRejection (packages/boot/app-boot/src/index.ts), not uncaughtException. A stream 'error' becomes exactly the unhandled kind.
3. The framing decoder failures this module's documentation promises to contain all route through fail() → failAll() → pool eviction — but that containment never runs, because the crash happens at the raw stream layer below it.

Failure scenario

A language server closes its stdout fd while staying alive briefly (or a tree-kill escalation races the reader; most plausible on Windows or under load). Node emits 'error' (EPIPE/EBADF) on child.stdout, no listener exists → uncaught exception → the entire harness process dies, taking every other live session, agent, and language-server connection with it. The exact outcome LspConnection.fail() / instance teardown exists to contain.

Suggested fix

Mirror the stdin treatment:

this.handle.stdout.on('error', (error) => { this.fail(error) })

so a read-side failure degrades to failing one connection (rejecting its pending requests and evicting it from the pool) instead of killing the host. One line, same semantics as the existing stdin handler.

[argszero]

Verified against v0.1.1-rc.2 source — the asymmetry is exact, and your one-line fix is the right minimal change. I traced the surrounding lifecycle to confirm nothing else needs to move with it.

Source confirmation (packages/lsp/lsp-stdio/src/connection.ts)

• The stdin error handler at line 130 carries the documenting comment: a child that closes fd 0 while staying alive must fail the connection so pending requests reject immediately instead of waiting for a process-close event that may never arrive. The stdout stream at line 131 gets only a data handler. The same intent applies to a read-side failure (server closes its stdout fd while staying alive, or a kill-escalation race): the fatal-cause path (fail, then failAll, then pool eviction) exists and is designed for exactly this, but is unreachable from a raw stream error event with no listener.
• app-boot registers only unhandledRejection (lines 646-647), not uncaughtException — so a stream error on stdout is not a contained unhandled-rejection but a hard process exit. Your exposure chain holds.
• The subprocess seam hands back the raw child stdout for pipe mode (subprocess-local spawn, stdout exposed when outMode is pipe) — nothing downstream attaches a read-side error handler for the LSP pipe.

One-line fix: mirror the stdin line — attach the same error-to-fail handler on stdout next to the data handler. Same semantics as the documented stdin handler; the fail path is idempotent against the later close path (the closeReason latch at lines 113-116 means a second fail after close is a no-op, so no double-eviction concern).

Regression test shape: attach a fake subprocess handle whose stdout is a PassThrough; emit an error on it; assert the connection failed flag flips and pending requests reject with that error before any close event. Also assert a subsequent close does not double-evict.

Family: this is the second member of the connection-lifecycle family I documented with #4059 (the upgrade-socket error listener removed inside the close callback, packages/host/webserver/src/index.ts:202-204 — a late ECONNRESET with zero listeners terminates the whole dsh web process). Same root shape: a stream or socket whose error path is unlistened, converting a contained transport failure into a whole-process crash. The #4059 fix there is also about two lines (keep the listener plus a destroyed guard); together they suggest a sweep — any other stdout/stderr pipe consumers that attach only a data handler without a sibling error handler are candidates for the same crash class.

Nice catch on the stdin/stdout asymmetry — the existing comment at lines 127-129 states the design intent, so the fix is strictly finishing the intended behavior.

[Jstn-1g]

I implemented and verified a minimal reference fix for this report:

• Reference commit: Jstn-1g@b0d2b78
• Reference branch: https://github.com/Jstn-1g/deepseek-harness/tree/reference/discussion-4073-lsp-stdout-error

The production change attaches the missing stdout error listener beside the existing stdin listener and routes it through the same retained first-cause fail(error) path. Pending and later requests reject with that exact cause; the existing LspInstance.dead and provider teardown/retry path remains the sole cleanup owner.

The deterministic regression first failed on the unhandled emitted stdout error, then passed with the fix. It also verifies that pending, future, and post-close requests retain the original error. Validation passed: 22/22 focused connection tests; 151 passed / 2 skipped related tests; 100% statements, branches, functions, and lines for connection.ts; full typecheck, lint, doc-sync (28/28), translation pairing, and diff integrity.