fix(lsp): make transport recovery ownership-safe

packages/lsp/lsp-local/README.md

@@ -7,10 +7,10 @@ Namespace plugin (`name` / `inject` / `Config` / `apply`, no default export).
 ## What it does
 
 - Resolves every server-local setting before registration; an invalid mapping or registration conflict rolls back earlier entries, so a failed load leaves no provider routes.
-- Lazily single-flights one server process per `(server id, canonical workspace realpath)`. A live server error is not replayed; if the transport becomes dead between the pool's liveness check and a read-only query, the provider evicts it and retries that query once on a fresh process.
+- Lazily single-flights one server process per `(server id, canonical workspace realpath)`. A live server error is not replayed; if the selected pooled transport fails before or during a read-only query, the provider awaits its disposal and retries that query once on a fresh process.
 - Uses a compatibility-first **transient-open** sequence per query: canonicalize and read the source with Node APIs, `textDocument/didOpen` (version 1, full text), the requested request, then `textDocument/didClose` in `finally`. A failed or canceled `didOpen` write terminates the instance before the pool can reuse it. Documents close after each call, so the first version needs no `didChange`, content cache, or document LRU.
 - Serializes each source-read/open/query/close lifecycle through one abortable per-workspace queue so queued calls read current source only when their turn starts; distinct workspaces run in parallel.
-- After protocol shutdown fails, terminates the server's descendant tree through POSIX process-group signaling or synchronous Windows `taskkill /T /F`, with a direct-child fallback for teardown races.
+- After protocol shutdown fails, terminates the server's descendant tree through POSIX process-group signaling or synchronous Windows `taskkill /T /F`. Windows suppresses only taskkill's already-absent-tree result; command, permission, and other tree-kill failures remain visible.
 - Reads sources through Node filesystem APIs in the subprocess's host namespace — NOT `ctx.fs`, and emits no `fs/observed`: only the LSP result is model-visible, so a query does not satisfy read-before-write policy.
 
 ## Configuration

packages/lsp/lsp-local/src/connection.ts

@@ -52,7 +52,7 @@ export type ConnectionWriter = (
 export interface ProcessTreeOperations {
   /** Signal a POSIX process group. */
   readonly signal: (target: number, signal: NodeJS.Signals) => void
-  /** Signal the direct child when group/tree signalling is unavailable. */
+  /** Signal the direct child when POSIX group signaling is unavailable. */
   readonly killChild: (signal: NodeJS.Signals) => void
   /** Terminate a Windows process tree by root pid. */
   readonly taskkill: (pid: number) => void
@@ -78,6 +78,9 @@ export type ProcessSignalRunner = (target: number, signal: NodeJS.Signals) => bo
 
 const processSignalRunner: ProcessSignalRunner = process.kill.bind(process)
 
+/** taskkill status for "process not found": the requested process tree is already absent. */
+const TASKKILL_TREE_NOT_FOUND_STATUS = 128
+
 const writeConnectionMessage: ConnectionWriter = (stdin, message, done) => {
   stdin.write(encodeMessage(message), done)
 }
@@ -93,6 +96,7 @@ export function taskkillProcessTree(
 ): void {
   const result = run('taskkill', ['/PID', String(pid), '/T', '/F'], { stdio: 'ignore' })
   if (result.error !== undefined) throw result.error
+  if (result.status === TASKKILL_TREE_NOT_FOUND_STATUS) return
   if (result.status !== 0) throw new Error(`taskkill exited with status ${String(result.status)}`)
 }
 
@@ -130,7 +134,8 @@ export async function waitForTreeExit(
 }
 
 /**
- * Signal a detached process tree with platform-correct semantics and a direct-child fallback.
+ * Signal a detached process tree with platform-correct semantics. POSIX falls back to the direct
+ * child; Windows requires taskkill to reach the full tree.
  * @param platform - host platform.
  * @param pid - detached root process id.
  * @param signal - requested termination signal.
@@ -142,9 +147,12 @@ export function signalProcessTree(
   signal: NodeJS.Signals,
   operations: ProcessTreeOperations,
 ): void {
+  if (platform === 'win32') {
+    operations.taskkill(pid)
+    return
+  }
   try {
-    if (platform === 'win32') operations.taskkill(pid)
-    else operations.signal(-pid, signal)
+    operations.signal(-pid, signal)
   } catch {
     try {
       operations.killChild(signal)
@@ -219,6 +227,15 @@ export class LspConnection {
     return this.closeReason !== undefined
   }
 
+  /**
+   * Test whether a caught error is this connection's retained fatal transport cause.
+   * @param error - error caught by the instance or provider.
+   * @returns `true` only when this connection produced that exact failure.
+   */
+  failedWith(error: unknown): boolean {
+    return this.closeReason === error
+  }
+
   /**
    * Send a request and await its result.
    * @param method - the JSON-RPC method.
@@ -291,10 +308,7 @@ export class LspConnection {
     return await waitForTreeExit(this.processTreeAlive.bind(this), signal)
   }
 
-  /**
-   * Signal the whole process tree so helper processes are reached; fall back to the direct child if
-   * tree signaling fails. Never throws because teardown races process exit.
-   */
+  /** Signal the whole process tree. */
   private signalTree(sig: NodeJS.Signals): void {
     const pid = this.child.pid
     if (pid === undefined) return

packages/lsp/lsp-local/src/index.ts

@@ -2,8 +2,8 @@
  * Generic stdio language-server backend for `ctx.lsp`. One plugin instance configures a named table
  * of server commands and registers one isolated provider for each entry. Every provider lazily
  * single-flights one server process per canonical workspace realpath, serves transient-open queries
- * through it, and replaces a transport that dies between a pool liveness check and the next
- * read-only query. Providers read sources through Node APIs in the host namespace (not `ctx.fs`)
+ * through it, and replaces a selected transport that fails before or during the next read-only
+ * query. Providers read sources through Node APIs in the host namespace (not `ctx.fs`)
  * and trust their configured servers — no sandbox confinement.
  *
  * Namespace plugin (named exports, no default export). Lifecycle is effect-scoped: disposal
@@ -224,16 +224,20 @@ class LocalLspProvider implements LspProvider {
       try {
         return await instance.query(request, source, signal)
       } catch (error) {
-        // A child can die after the pre-query liveness check but before or during the next write.
-        // Queries are read-only, so replace a newly failed transport once and retry transparently.
-        if (!instance.dead) throw error
+        // A selected child can have died while idle or fail during the next write. Queries are
+        // read-only, so replace that transport once and retry transparently.
+        if (!instance.isTransportFailure(error)) throw error
+        await instance.dispose()
         this.evictIfCurrent(workspace, instance)
         this.assertActive(signal)
         instance = this.instanceFor(workspace)
         return await instance.query(request, source, signal)
       } finally {
-        // Drop a crashed slot only when it still owns this instance; a replacement must survive.
-        if (instance.dead) this.evictIfCurrent(workspace, instance)
+        // Reach quiescence before dropping a dead slot; a replacement must survive this ownership check.
+        if (instance.dead) {
+          await instance.dispose()
+          this.evictIfCurrent(workspace, instance)
+        }
       }
     })
   }

packages/lsp/lsp-local/src/instance.ts

@@ -83,6 +83,15 @@ export class LspInstance {
     return this.processClosed || this.disposed || this.connection.failed
   }
 
+  /**
+   * Test whether a caught query error came from this instance's transport.
+   * @param error - error caught by the provider.
+   * @returns `true` only for the connection's retained fatal transport cause.
+   */
+  isTransportFailure(error: unknown): boolean {
+    return this.connection.failedWith(error)
+  }
+
   /**
    * Run one query through the serialized queue.
    * @param request - the resolved provider query.
@@ -94,7 +103,12 @@ export class LspInstance {
     // Serialize behind prior work, but observe abort DURING the queue wait too: if an earlier query
     // hangs (e.g. a signal-less seam caller), a later tool's timeout must still be able to give up
     // rather than block on the shared tail forever.
-    const run = abortable(this.queue, signal).then(() => this.runQuery(request, source, signal))
+    const run = abortable(this.queue, signal)
+      .then(() => this.runQuery(request, source, signal))
+      .catch(async (error: unknown) => {
+        if (this.isTransportFailure(error)) await this.startTeardown()
+        throw error
+      })
     // Keep the tail alive regardless of this query's outcome so the next caller still serializes. The
     // tail follows the ACTUAL prior work (this.queue), not the abortable view, so a caller giving up
     // on the wait does not deserialize the queue.

packages/lsp/lsp-local/tests/connection.spec.ts

@@ -65,6 +65,12 @@ describe('LspConnection', () => {
     await expect(conn.request('textDocument/hover', {})).rejects.toThrow(/server refused the request/)
   })
 
+  it('treats signaling an already-closed child as a teardown race', async () => {
+    const conn = connectScript('')
+    await conn.closed
+    expect(() => { conn.kill() }).not.toThrow()
+  })
+
   it('answers a server workspace/configuration request from static config', async () => {
     const seen: SeenRequest[] = []
     const conn = connect(
@@ -272,23 +278,27 @@ describe('process-tree signaling', () => {
     expect(operations.signal).toHaveBeenCalledWith(-42, 'SIGKILL')
   })
 
-  it('falls back to the direct child and tolerates an already-dead child', () => {
+  it('surfaces a Windows taskkill failure without downgrading to the direct child', () => {
     const fallback = fakeProcessTreeOperations()
     vi.mocked(fallback.taskkill).mockImplementation(() => { throw new Error('taskkill unavailable') })
-    signalProcessTree('win32', 42, 'SIGTERM', fallback)
-    expect(fallback.killChild).toHaveBeenCalledWith('SIGTERM')
-
-    const gone = fakeProcessTreeOperations()
-    vi.mocked(gone.signal).mockImplementation(() => { throw new Error('group gone') })
-    vi.mocked(gone.killChild).mockImplementation(() => { throw new Error('child gone') })
-    expect(() => { signalProcessTree('linux', 42, 'SIGKILL', gone) }).not.toThrow()
+    expect(() => { signalProcessTree('win32', 42, 'SIGTERM', fallback) }).toThrow(/taskkill unavailable/)
+    expect(fallback.killChild).not.toHaveBeenCalled()
   })
 
-  it('runs taskkill for the full tree and rejects command failures', () => {
+  it('tolerates a POSIX tree-signaling race after the direct child is already gone', () => {
+    const posixGone = fakeProcessTreeOperations()
+    vi.mocked(posixGone.signal).mockImplementation(() => { throw new Error('group gone') })
+    vi.mocked(posixGone.killChild).mockImplementation(() => { throw new Error('child gone') })
+    expect(() => { signalProcessTree('linux', 42, 'SIGKILL', posixGone) }).not.toThrow()
+  })
+
+  it('runs taskkill for the full tree, accepts an absent tree, and rejects command failures', () => {
     const success: TaskkillRunner = vi.fn(() => ({ status: 0 }))
     taskkillProcessTree(42, success)
     expect(success).toHaveBeenCalledWith('taskkill', ['/PID', '42', '/T', '/F'], { stdio: 'ignore' })
 
+    expect(() => { taskkillProcessTree(42, () => ({ status: 128 })) }).not.toThrow()
+
     const spawnFailure = new Error('cannot spawn taskkill')
     expect(() => { taskkillProcessTree(42, () => ({ status: null, error: spawnFailure })) }).toThrow(spawnFailure)
     expect(() => { taskkillProcessTree(42, () => ({ status: 1 })) }).toThrow(/status 1/)

packages/lsp/lsp-local/tests/instance.spec.ts

@@ -216,6 +216,17 @@ describe('LspInstance query and abort', () => {
     expect(instance.dead).toBe(true)
   })
 
+  it('awaits process exit before rejecting a request write failure', async () => {
+    const instance = makeInstance({}, {
+      shutdownTimeoutMs: 100,
+      killGraceMs: 100,
+    }, failingWriter('textDocument/definition'))
+    // The pid is observed only to prove the owned subprocess reached quiescence before rejection.
+    const pid = (instance as unknown as { connection: { pid: number } }).connection.pid
+    await expect(run(instance, 'goToDefinition')).rejects.toThrow(/fixture textDocument\/definition failure/)
+    expect(processAlive(pid)).toBe(false)
+  })
+
   it('rejects when the server lacks the operation capability', async () => {
     const instance = makeInstance({ LSP_FAKE_CAPS: JSON.stringify({ definitionProvider: false }), LSP_FAKE_DEF: 'null' })
     await expect(run(instance, 'goToDefinition')).rejects.toThrow(/does not support goToDefinition/)

packages/lsp/lsp-local/tests/lifecycle.spec.ts

@@ -122,9 +122,15 @@ describe('lsp-local end to end over a fake server', () => {
     await ctx.fiber.dispose()
   })
 
-  it('rejects a non-utf-16 position encoding at initialize', async () => {
-    const ctx = await mount({ LSP_FAKE_ENCODING: 'utf-8', LSP_FAKE_DEF: 'null' })
+  it('rejects a non-utf-16 position encoding at initialize without retrying', async () => {
+    const marker = join(root, 'initialize-rejection-exit.log')
+    const ctx = await mount({
+      LSP_FAKE_ENCODING: 'utf-8',
+      LSP_FAKE_DEF: 'null',
+      LSP_FAKE_EXIT_MARKER: marker,
+    })
     await expect(ctx.lsp.query(query('goToDefinition'))).rejects.toThrow(/unsupported position encoding/)
+    expect(await readFile(marker, 'utf8')).toBe('EXIT\nCLEAN\n')
     await ctx.fiber.dispose()
   })