fix(windows): restore LSP and TUI coverage
This commit is contained in:
@@ -8,7 +8,7 @@
|
||||
*/
|
||||
|
||||
import type { ChildProcessByStdio } from 'node:child_process'
|
||||
import { spawn } from 'node:child_process'
|
||||
import { spawn, spawnSync } from 'node:child_process'
|
||||
import type { Readable, Writable } from 'node:stream'
|
||||
import { setImmediate as yieldToEventLoop } from 'node:timers/promises'
|
||||
import { encodeMessage, MessageDecoder } from './framing.ts'
|
||||
@@ -36,6 +36,105 @@ interface Pending {
|
||||
reject: (error: Error) => void
|
||||
}
|
||||
|
||||
/**
|
||||
* Write one JSON-RPC message to the child stdin.
|
||||
* @param stdin - the spawned server stdin.
|
||||
* @param message - the unencoded JSON-RPC message.
|
||||
* @param done - callback that reports asynchronous stream settlement.
|
||||
*/
|
||||
export type ConnectionWriter = (
|
||||
stdin: Writable,
|
||||
message: unknown,
|
||||
done: (error?: Error | null) => void,
|
||||
) => void
|
||||
|
||||
/** Host operations used to signal a detached process tree. */
|
||||
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. */
|
||||
readonly killChild: (signal: NodeJS.Signals) => void
|
||||
/** Terminate a Windows process tree by root pid. */
|
||||
readonly taskkill: (pid: number) => void
|
||||
}
|
||||
|
||||
/** Narrow taskkill runner result used by the Windows process-tree adapter. */
|
||||
export interface TaskkillResult {
|
||||
/** Process exit status, or null when spawning failed. */
|
||||
readonly status: number | null
|
||||
/** Spawn failure, when the executable could not run. */
|
||||
readonly error?: Error
|
||||
}
|
||||
|
||||
/** Invoke a command synchronously for the Windows taskkill adapter. */
|
||||
export type TaskkillRunner = (
|
||||
command: string,
|
||||
args: string[],
|
||||
options: { stdio: 'ignore' },
|
||||
) => TaskkillResult
|
||||
|
||||
/** Invoke the host process-signal primitive for a POSIX process group. */
|
||||
export type ProcessSignalRunner = (target: number, signal: NodeJS.Signals) => boolean
|
||||
|
||||
const processSignalRunner: ProcessSignalRunner = process.kill.bind(process)
|
||||
|
||||
const writeConnectionMessage: ConnectionWriter = (stdin, message, done) => {
|
||||
stdin.write(encodeMessage(message), done)
|
||||
}
|
||||
|
||||
/**
|
||||
* Terminate one Windows process tree and wait for taskkill to finish.
|
||||
* @param pid - root process id.
|
||||
* @param run - command runner; tests inject results without requiring Windows.
|
||||
*/
|
||||
export function taskkillProcessTree(
|
||||
pid: number,
|
||||
run: TaskkillRunner = spawnSync,
|
||||
): void {
|
||||
const result = run('taskkill', ['/PID', String(pid), '/T', '/F'], { stdio: 'ignore' })
|
||||
if (result.error !== undefined) throw result.error
|
||||
if (result.status !== 0) throw new Error(`taskkill exited with status ${String(result.status)}`)
|
||||
}
|
||||
|
||||
/**
|
||||
* Signal one POSIX process group through an injectable host primitive.
|
||||
* @param target - negative process-group id.
|
||||
* @param signal - requested signal.
|
||||
* @param run - host signal runner; tests inject it without touching real processes.
|
||||
*/
|
||||
export function signalProcessGroup(
|
||||
target: number,
|
||||
signal: NodeJS.Signals,
|
||||
run: ProcessSignalRunner = processSignalRunner,
|
||||
): void {
|
||||
run(target, signal)
|
||||
}
|
||||
|
||||
/**
|
||||
* Signal a detached process tree with platform-correct semantics and a direct-child fallback.
|
||||
* @param platform - host platform.
|
||||
* @param pid - detached root process id.
|
||||
* @param signal - requested termination signal.
|
||||
* @param operations - host operations.
|
||||
*/
|
||||
export function signalProcessTree(
|
||||
platform: NodeJS.Platform,
|
||||
pid: number,
|
||||
signal: NodeJS.Signals,
|
||||
operations: ProcessTreeOperations,
|
||||
): void {
|
||||
try {
|
||||
if (platform === 'win32') operations.taskkill(pid)
|
||||
else operations.signal(-pid, signal)
|
||||
} catch {
|
||||
try {
|
||||
operations.killChild(signal)
|
||||
} catch {
|
||||
// The direct child already exited; teardown remains idempotent.
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/** A live JSON-RPC endpoint bound to one child process. */
|
||||
export class LspConnection {
|
||||
private readonly child: ChildProcessByStdio<Writable, Readable, Readable>
|
||||
@@ -50,14 +149,16 @@ export class LspConnection {
|
||||
/**
|
||||
* @param spec - how to launch the server and answer its config requests.
|
||||
* @param onServerRequest - answers a server→client request; rejects to send an error response.
|
||||
* @param writer - message writer; tests inject callback failures without relying on OS pipe races.
|
||||
*/
|
||||
constructor(
|
||||
private readonly spec: ConnectionSpec,
|
||||
private readonly onServerRequest: (method: string, params: unknown) => Promise<unknown>,
|
||||
private readonly writer: ConnectionWriter = writeConnectionMessage,
|
||||
) {
|
||||
this.decoder = new MessageDecoder(spec.maxMessageBytes)
|
||||
// `detached` puts the server in its own process group so teardown can signal the WHOLE group
|
||||
// (via `process.kill(-pid)`), reaching helper processes a language server spawns (e.g. tsserver).
|
||||
// `detached` gives teardown a process-tree root: POSIX signals its negative process-group id,
|
||||
// while Windows passes the root pid to taskkill /T so helpers such as tsserver cannot outlive it.
|
||||
this.child = spawn(spec.command, [...spec.args], {
|
||||
cwd: spec.cwd,
|
||||
env: spec.env,
|
||||
@@ -94,6 +195,11 @@ export class LspConnection {
|
||||
return this.stderr.toString('utf8')
|
||||
}
|
||||
|
||||
/** Whether the transport has failed even if the child close event has not arrived yet. */
|
||||
get failed(): boolean {
|
||||
return this.closeReason !== undefined
|
||||
}
|
||||
|
||||
/**
|
||||
* Send a request and await its result.
|
||||
* @param method - the JSON-RPC method.
|
||||
@@ -147,23 +253,23 @@ export class LspConnection {
|
||||
return this.nextId
|
||||
}
|
||||
|
||||
/** Send SIGTERM to the server's process group (idempotent-safe; a dead group ignores it). */
|
||||
/** Request termination of the server's process tree. */
|
||||
terminate(): void {
|
||||
this.signalGroup('SIGTERM')
|
||||
this.signalTree('SIGTERM')
|
||||
}
|
||||
|
||||
/** Send SIGKILL to the server's process group. */
|
||||
/** Force termination of the server's process tree. */
|
||||
kill(): void {
|
||||
this.signalGroup('SIGKILL')
|
||||
this.signalTree('SIGKILL')
|
||||
}
|
||||
|
||||
/**
|
||||
* Wait until the owned process group has no members.
|
||||
* Wait until the owned process tree has exited.
|
||||
* @param signal - optional bound for the wait.
|
||||
* @returns `true` when the group exited, or `false` when the signal aborted first.
|
||||
* @returns `true` when the tree exited, or `false` when the signal aborted first.
|
||||
*/
|
||||
async waitForProcessGroupExit(signal?: AbortSignal): Promise<boolean> {
|
||||
while (this.processGroupAlive()) {
|
||||
async waitForProcessTreeExit(signal?: AbortSignal): Promise<boolean> {
|
||||
while (this.processTreeAlive()) {
|
||||
if (signal?.aborted) return false
|
||||
await yieldToEventLoop()
|
||||
}
|
||||
@@ -171,26 +277,21 @@ export class LspConnection {
|
||||
}
|
||||
|
||||
/**
|
||||
* Signal the whole process group (negative pid) so helper processes are reached; fall back to the
|
||||
* direct child if the group send fails. Never throws — teardown races process exit.
|
||||
* 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.
|
||||
*/
|
||||
private signalGroup(sig: NodeJS.Signals): void {
|
||||
private signalTree(sig: NodeJS.Signals): void {
|
||||
const pid = this.child.pid
|
||||
if (pid === undefined) return
|
||||
try {
|
||||
process.kill(-pid, sig)
|
||||
} catch {
|
||||
// The group is gone (already exited) or could not be signalled; try the direct child.
|
||||
try {
|
||||
this.child.kill(sig)
|
||||
} catch {
|
||||
// Already dead; nothing to signal.
|
||||
}
|
||||
}
|
||||
signalProcessTree(process.platform, pid, sig, {
|
||||
signal: signalProcessGroup,
|
||||
killChild: this.child.kill.bind(this.child),
|
||||
taskkill: taskkillProcessTree,
|
||||
})
|
||||
}
|
||||
|
||||
/** Whether the detached process group still has at least one member. */
|
||||
private processGroupAlive(): boolean {
|
||||
/** Whether the detached tree's root or POSIX process group is still alive. */
|
||||
private processTreeAlive(): boolean {
|
||||
const pid = this.child.pid
|
||||
/* v8 ignore next -- only an asynchronous spawn failure omits pid; its close path owns cleanup. */
|
||||
if (pid === undefined) return false
|
||||
@@ -218,7 +319,7 @@ export class LspConnection {
|
||||
// A framing/JSON failure corrupts the stream position irrecoverably: fail the instance and
|
||||
// SIGKILL the whole group so helper processes don't outlive the leader.
|
||||
this.fail(asError(error))
|
||||
this.signalGroup('SIGKILL')
|
||||
this.signalTree('SIGKILL')
|
||||
return
|
||||
}
|
||||
for (const message of messages) this.dispatch(message)
|
||||
@@ -293,7 +394,7 @@ export class LspConnection {
|
||||
reject(error)
|
||||
}
|
||||
try {
|
||||
this.child.stdin.write(encodeMessage(message), done)
|
||||
this.writer(this.child.stdin, message, done)
|
||||
/* v8 ignore start -- Node stream write failures are callback-delivered; this guards a
|
||||
nonconforming Writable implementation throwing synchronously. */
|
||||
} catch (error) {
|
||||
|
||||
@@ -2,9 +2,9 @@
|
||||
* 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 evicts a crashed process so a later query can replace it. Providers read sources
|
||||
* through Node APIs in the host namespace (not `ctx.fs`) and trust their configured servers — no
|
||||
* sandbox confinement.
|
||||
* 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`)
|
||||
* and trust their configured servers — no sandbox confinement.
|
||||
*
|
||||
* Namespace plugin (named exports, no default export). Lifecycle is effect-scoped: disposal
|
||||
* unregisters from `ctx.lsp` and tears down every live server.
|
||||
@@ -227,6 +227,14 @@ 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
|
||||
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)
|
||||
|
||||
@@ -17,7 +17,7 @@ import type {
|
||||
import { deadline } from '@deepseek-ai/dsh-timeout'
|
||||
import { abortable, abortError } from './abort.ts'
|
||||
import { LspConnection } from './connection.ts'
|
||||
import type { ConnectionSpec } from './connection.ts'
|
||||
import type { ConnectionSpec, ConnectionWriter } from './connection.ts'
|
||||
import type { HostSource } from './host.ts'
|
||||
import type { WireInitializeResult, WireServerCapabilities } from './protocol.ts'
|
||||
import {
|
||||
@@ -58,9 +58,10 @@ export class LspInstance {
|
||||
|
||||
/**
|
||||
* @param spec - the launch, initialize, and teardown parameters.
|
||||
* @param writer - optional connection writer used by transport conformance tests.
|
||||
*/
|
||||
constructor(private readonly spec: InstanceSpec) {
|
||||
this.connection = new LspConnection(spec, (method, params) => this.answerServerRequest(method, params))
|
||||
constructor(private readonly spec: InstanceSpec, writer?: ConnectionWriter) {
|
||||
this.connection = new LspConnection(spec, (method, params) => this.answerServerRequest(method, params), writer)
|
||||
this.ready = this.initialize()
|
||||
// A handshake rejection must not surface as an unhandled rejection before the first query awaits
|
||||
// it; queries attach the real handler.
|
||||
@@ -70,7 +71,7 @@ export class LspInstance {
|
||||
|
||||
/** Synchronous liveness check: true once the process has closed or the instance was disposed. */
|
||||
get dead(): boolean {
|
||||
return this.processClosed || this.disposed
|
||||
return this.processClosed || this.disposed || this.connection.failed
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -272,7 +273,7 @@ export class LspInstance {
|
||||
try {
|
||||
await this.gracefulShutdown(shutdownDeadline.signal)
|
||||
} catch {
|
||||
// Graceful shutdown failed or timed out; process-group cleanup below remains authoritative.
|
||||
// Graceful shutdown failed or timed out; process-tree cleanup below remains authoritative.
|
||||
} finally {
|
||||
shutdownDeadline[Symbol.dispose]()
|
||||
}
|
||||
@@ -286,20 +287,20 @@ export class LspInstance {
|
||||
await abortable(this.connection.closed, signal)
|
||||
}
|
||||
|
||||
/** SIGTERM the group, escalate after `killGraceMs`, then await leader and helper exit. */
|
||||
/** Terminate the tree, escalate after `killGraceMs`, then await leader and helper exit. */
|
||||
private async forceTerminate(): Promise<void> {
|
||||
this.connection.terminate()
|
||||
const graceDeadline = deadline(undefined, this.spec.killGraceMs, 'LSP_KILL_GRACE')
|
||||
let groupExited: boolean
|
||||
let treeExited: boolean
|
||||
try {
|
||||
groupExited = await this.connection.waitForProcessGroupExit(graceDeadline.signal)
|
||||
treeExited = await this.connection.waitForProcessTreeExit(graceDeadline.signal)
|
||||
} finally {
|
||||
graceDeadline[Symbol.dispose]()
|
||||
}
|
||||
if (!groupExited) this.connection.kill()
|
||||
if (!treeExited) this.connection.kill()
|
||||
await Promise.all([
|
||||
this.connection.closed,
|
||||
this.connection.waitForProcessGroupExit(),
|
||||
this.connection.waitForProcessTreeExit(),
|
||||
])
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user