summaryrefslogtreecommitdiffhomepage
path: root/packages/lsp/src/client.ts
diff options
context:
space:
mode:
authorAdam Malczewski <[email protected]>2026-06-25 18:11:17 +0900
committerAdam Malczewski <[email protected]>2026-06-25 18:11:17 +0900
commit7626c7f3adf940ee871c4fd2ba2d342f19d9d60b (patch)
tree17af00238fd8020bf27a482415f37fb79baf0b59 /packages/lsp/src/client.ts
parentc1bc7bfaaca7bdf4d9b2973f5dc88605217a7866 (diff)
downloaddispatch-7626c7f3adf940ee871c4fd2ba2d342f19d9d60b.tar.gz
dispatch-7626c7f3adf940ee871c4fd2ba2d342f19d9d60b.zip
fix(lsp): stop per-edit hangs on dead/slow servers (10s cap + skip + self-heal)
The LSP diagnostics path hung up to 60s per edit whenever a configured Ruby language server was dead or slow (the reported Steep langserver case): a killed/crashed server was never detected (stayed "connected" forever), servers were queried sequentially with a 60s budget each, and a corrupted-but-alive server (Steep's ~3h phantom-SyntaxError drift) had no recovery. Four fixes, all in packages/lsp/ (the tool-edit-file call site lowered to 10s): 1. Dead-process detection: SpawnedProcess.onExit (Bun proc.exited) + stdout-end defence flip the client to error, dispose the rpc, kill the proc. The manager re-spawns a fresh server after the 30s backoff. Dead servers are now skipped (0s) instead of polled for 60s. 2. Concurrent fan-out + 10s hard cap: new aggregateDiagnostics queries all matching servers at once, each capped at 10s. A non-responder is skipped with "LSP took too long (>10s), skipped — raise this to the user" instead of blocking the fast server's results. Replaces the vague "unusually long" warning (now structurally impossible: slow is always false). 3. Corruption self-heal: a detector flags a server re-emitting identical non-empty diagnostics despite the file changing; after 5 repeats the client is marked broken and re-spawned. Clean files never trip it. (Acknowledged false-positive risk on persistent unfixed errors; CLI type-check gate stays authoritative.) 4. sendRequest timeout: hover/definition/references cap at 10s so they can't hang the turn against a dead server; the initialize handshake keeps its 45s race. Verification: typecheck clean; 1573 tests pass (96 files), +15 new LSP tests (86 in packages/lsp); biome clean. No kernel/contract changes; onExit is internal to packages/lsp.
Diffstat (limited to 'packages/lsp/src/client.ts')
-rw-r--r--packages/lsp/src/client.ts153
1 files changed, 139 insertions, 14 deletions
diff --git a/packages/lsp/src/client.ts b/packages/lsp/src/client.ts
index 677a22a..ac7d025 100644
--- a/packages/lsp/src/client.ts
+++ b/packages/lsp/src/client.ts
@@ -4,13 +4,22 @@
* FileWatcher, and forwards matching disk changes.
*/
-import { DiagnosticsStore, type PublishDiagnosticsParams } from "./diagnostics.js";
+import { DiagnosticsStore, diagnosticKey, type PublishDiagnosticsParams } from "./diagnostics.js";
import { computeChangeRange } from "./diff.js";
import { FrameDecoder } from "./framing.js";
import { languageId as resolveLanguageId } from "./language.js";
import { JsonRpcConnection, type WriteFn } from "./rpc.js";
import { FileChangeType, WatchedFilesRegistry } from "./watched-files.js";
+/** Info delivered to an `onExit` handler when the child process terminates. */
+export interface ProcessExitInfo {
+ readonly code: number | null;
+ readonly signal?: string;
+}
+
+/** A handler registered to be called when the child process exits. */
+export type ProcessExitHandler = (info: ProcessExitInfo) => void;
+
export interface SpawnedProcess {
readonly stdin: { readonly write: (bytes: Uint8Array) => void };
readonly stdout:
@@ -22,6 +31,13 @@ export interface SpawnedProcess {
| undefined;
readonly pid: number | undefined;
readonly kill: () => void;
+ /**
+ * Register a handler fired when the child process exits (code|signal).
+ * Optional: when absent, death is detected via stdout-end instead. Wires
+ * Bun's `proc.exited` in production; tests invoke it directly to simulate
+ * a crash. Lets the client stop querying a dead server (no per-edit hang).
+ */
+ readonly onExit?: ((handler: ProcessExitHandler) => void) | undefined;
}
export type SpawnProcess = (
@@ -102,6 +118,18 @@ export class LanguageServerClient {
private openDocuments = new Map<string, { version: number; text: string }>();
/** Sync mode captured from the server's initialize capabilities: 1=Full, 2=Incremental. */
private textDocumentChange: 1 | 2 = 1;
+ /**
+ * Corruption detection: the last diagnostic-key set + synced text per URI.
+ * A healthy server's diagnostics change when the file changes; a corrupted
+ * one (e.g. Steep's ~3h phantom-SyntaxError drift) re-emits the identical
+ * non-empty set across edits. `staleRepeat` counts consecutive such repeats
+ * across URIs; at the threshold the client is marked broken (→ respawn).
+ */
+ private lastDiagSnapshot = new Map<string, { keys: Set<string>; text: string }>();
+ private staleRepeat = 0;
+ private static readonly STALE_REPEAT_THRESHOLD = 5;
+ /** Default timeout for outbound requests (hover/definition/references). */
+ private static readonly REQUEST_TIMEOUT_MS = 10_000;
constructor(deps: ClientDeps) {
this.deps = deps;
@@ -128,6 +156,12 @@ export class LanguageServerClient {
}
const proc = this.deps.spawn(this.deps.command as string[], spawnOpts);
this.process = proc;
+ // Detect process death so we stop querying a corpse (fixes the
+ // per-edit hang after a server is killed/crashes). onExit is the
+ // primary signal; stdout-end is the defence-in-depth fallback.
+ if (proc.onExit) {
+ proc.onExit((info) => this.handleExit(info));
+ }
const writeFn: WriteFn = (bytes) => proc.stdin.write(bytes);
const rpc = new JsonRpcConnection(writeFn);
@@ -158,8 +192,11 @@ export class LanguageServerClient {
for await (const chunk of source) {
this.handleBytes(chunk);
}
+ // stdout closed — the process is gone (defence-in-depth alongside onExit,
+ // which some edges never call). Idempotent via handleExit's guard.
+ this.handleExit({ code: null });
} catch {
- // process exited
+ this.handleExit({ code: null });
}
})();
}
@@ -172,6 +209,65 @@ export class LanguageServerClient {
});
}
+ /**
+ * The server process exited (onExit or stdout-end). Transition to a broken
+ * state so callers skip it and the manager re-spawns after backoff — instead
+ * of polling a corpse for the full timeout on every edit. Idempotent.
+ */
+ private handleExit(info: ProcessExitInfo): void {
+ if (this.state === "error" || this.state === "not-started") return;
+ const detail = info.signal !== undefined ? `signal ${info.signal}` : `code ${info.code ?? "?"}`;
+ this.markBroken(`language server process exited (${detail})`);
+ }
+
+ /**
+ * Mark this client permanently broken: kill the process if still alive
+ * (corruption case), dispose the rpc (rejects pending requests), and drop
+ * edge handles. The manager's status() observes state:"error" and re-spawns
+ * after the bounded backoff. Called on process death AND on corruption.
+ */
+ private markBroken(reason: string): void {
+ if (this.state === "error") return;
+ this.state = "error";
+ this.stateError = reason;
+ this.fileWatcherHandle?.close();
+ this.fileWatcherHandle = null;
+ this.process?.kill();
+ this.process = null;
+ this.rpc?.dispose();
+ this.rpc = null;
+ }
+
+ /**
+ * Detect a server stuck re-emitting identical non-empty diagnostics
+ * despite the file content changing between calls — the signature of a
+ * corrupted parse/type-check state (e.g. Steep's ~3h phantom-SyntaxError
+ * drift, where a fresh CLI reports green on the same project). After
+ * STALE_REPEAT_THRESHOLD consecutive such repeats, mark the client broken
+ * so it is skipped + re-spawned. A clean file (empty diagnostics) or a
+ * genuinely changing diagnostic set resets the counter. Note the
+ * tradeoff: a real, unfixed error on an untouched line also "stays the
+ * same across edits", so this can false-positive on a healthy server —
+ * the threshold is set conservatively and the CLI type-check gate remains
+ * authoritative either way.
+ */
+ private detectStaleDiagnostics(uri: string, text: string): void {
+ const merged = this.diagnostics.getMerged(uri);
+ const keys = new Set(merged.map((d) => diagnosticKey(d)));
+ const prev = this.lastDiagSnapshot.get(uri);
+ if (prev && keys.size > 0 && setsEqual(keys, prev.keys) && text !== prev.text) {
+ this.staleRepeat++;
+ } else {
+ this.staleRepeat = 0;
+ }
+ this.lastDiagSnapshot.set(uri, { keys, text });
+ if (this.staleRepeat >= LanguageServerClient.STALE_REPEAT_THRESHOLD) {
+ this.markBroken(
+ "language server emitting repeated stale diagnostics despite file changes — likely corrupted; restarting",
+ );
+ }
+ }
+
private handleBytes(chunk: Uint8Array): void {
const messages = this.decoder.decode(chunk);
for (const msg of messages) {
@@ -403,26 +499,37 @@ export class LanguageServerClient {
await this.open(filePath);
}
- const slowThreshold = 10_000;
const start = Date.now();
- // Poll until the server pushes diagnostics (even empty = done) or timeout.
- return new Promise((resolve) => {
+ // Poll until the server pushes diagnostics (even empty = done) or the
+ // per-server cap elapses (then we skip it — see aggregateDiagnostics).
+ const received = await new Promise<boolean>((resolve) => {
const check = () => {
const elapsed = Date.now() - start;
- const received = this.diagnostics.hasReceivedPush(uri);
- if (received || elapsed >= timeoutMs) {
- resolve({
- formatted: this.diagnostics.formatFiltered(uri, opts?.minSeverity),
- slow: elapsed > slowThreshold,
- timedOut: !received,
- });
+ const got = this.diagnostics.hasReceivedPush(uri);
+ if (got || elapsed >= timeoutMs) {
+ resolve(got);
return;
}
setTimeout(check, 100);
};
check();
});
+
+ // Only a server that actually pushed can be corruption-checked.
+ if (received) {
+ this.detectStaleDiagnostics(uri, opts?.text ?? "");
+ }
+
+ // `slow` is structurally false now: the per-server cap is 10s, so
+ // elapsed can never exceed the old "unusually long" threshold. That
+ // warning is superseded by the timeout→skip notice produced in
+ // aggregateDiagnostics. The field is kept for contract compatibility.
+ return {
+ formatted: this.diagnostics.formatFiltered(uri, opts?.minSeverity),
+ slow: false,
+ timedOut: !received,
+ };
}
getWatchedFilesRegistry(): WatchedFilesRegistry {
@@ -433,11 +540,21 @@ export class LanguageServerClient {
return this.diagnostics;
}
- async request(method: string, params?: unknown): Promise<unknown> {
+ /**
+ * Send a request (hover/definition/references/documentSymbol). Capped at
+ * REQUEST_TIMEOUT_MS so a dead/slow server can't hang the turn — the
+ * initialize handshake bypasses this (it calls rpc.sendRequest directly
+ * with its own 45s race).
+ */
+ async request(
+ method: string,
+ params?: unknown,
+ timeoutMs: number = LanguageServerClient.REQUEST_TIMEOUT_MS,
+ ): Promise<unknown> {
if (!this.rpc || this.state !== "connected") {
throw new Error("Client not connected");
}
- return this.rpc.sendRequest(method, params);
+ return this.rpc.sendRequest(method, params, timeoutMs);
}
shutdown(): void {
@@ -450,3 +567,11 @@ export class LanguageServerClient {
this.state = "not-started";
}
}
+
+function setsEqual<T>(a: Set<T>, b: Set<T>): boolean {
+ if (a.size !== b.size) return false;
+ for (const v of a) {
+ if (!b.has(v)) return false;
+ }
+ return true;
+}