diff options
| author | Adam Malczewski <[email protected]> | 2026-06-25 18:11:17 +0900 |
|---|---|---|
| committer | Adam Malczewski <[email protected]> | 2026-06-25 18:11:17 +0900 |
| commit | 7626c7f3adf940ee871c4fd2ba2d342f19d9d60b (patch) | |
| tree | 17af00238fd8020bf27a482415f37fb79baf0b59 /packages/lsp/src/aggregate.ts | |
| parent | c1bc7bfaaca7bdf4d9b2973f5dc88605217a7866 (diff) | |
| download | dispatch-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/aggregate.ts')
| -rw-r--r-- | packages/lsp/src/aggregate.ts | 80 |
1 files changed, 80 insertions, 0 deletions
diff --git a/packages/lsp/src/aggregate.ts b/packages/lsp/src/aggregate.ts new file mode 100644 index 0000000..b044e21 --- /dev/null +++ b/packages/lsp/src/aggregate.ts @@ -0,0 +1,80 @@ +/** + * Concurrent multi-server diagnostics aggregation. + * + * Queries every matching language server AT ONCE (not one-at-a-time), each + * capped at `timeoutMs`. A server that doesn't push diagnostics within the cap + * is SKIPPED with a per-server notice rather than blocking the others — so one + * slow/dead server (e.g. a corrupted Steep) can't hold up the fast one's + * (ruby-lsp) results for the full timeout on every edit. + * + * The only I/O here is `client.waitForDiagnostics` (injected via `getClient`), + * so this is unit-testable with a fake client and no real process. + */ + +import type { LanguageServerClient } from "./client.js"; + +export interface AggregateServer { + readonly id: string; + readonly name: string; + readonly root: string; +} + +export interface AggregateOpts { + /** Post-edit buffer; when omitted the server reads from disk. */ + readonly text?: string | undefined; + /** Only include diagnostics with severity ≤ this (1=Error, 2=Warning). */ + readonly minSeverity?: number | undefined; +} + +export interface AggregateResult { + /** Merged diagnostics tagged by source + a per-skipped-server notice. */ + readonly formatted: string; + /** True if at least one server was skipped for exceeding the cap. */ + readonly timedOut: boolean; +} + +/** + * Query `servers` concurrently, each capped at `timeoutMs`. Returns merged + * diagnostics tagged by source (`[name]\n…`) and, for any server that did not + * respond in time, a `⚠️ [name] LSP took too long (>Ns), skipped — please raise + * this to the user.` notice. Never rejects: a client error yields an empty + * contribution for that server. + */ +export async function aggregateDiagnostics( + getClient: (id: string, root: string) => LanguageServerClient | undefined, + servers: readonly AggregateServer[], + absolutePath: string, + timeoutMs: number, + opts: AggregateOpts, +): Promise<AggregateResult> { + const entries = await Promise.all( + servers.map(async (server) => { + const client = getClient(server.id, server.root); + if (!client) return null; + const waitOpts: { text?: string; timeoutMs: number; minSeverity?: number } = { timeoutMs }; + if (opts.text !== undefined) waitOpts.text = opts.text; + if (opts.minSeverity !== undefined) waitOpts.minSeverity = opts.minSeverity; + const result = await client.waitForDiagnostics(absolutePath, waitOpts); + return { server, result }; + }), + ); + + const parts: string[] = []; + let timedOut = false; + const capSeconds = Math.round(timeoutMs / 1000); + + for (const entry of entries) { + if (!entry) continue; + const { server, result } = entry; + if (result.timedOut) { + timedOut = true; + parts.push( + `⚠️ [${server.name}] LSP took too long (>${capSeconds}s), diagnostics skipped — please raise this to the user.`, + ); + } else if (result.formatted) { + parts.push(`[${server.name}]\n${result.formatted}`); + } + } + + return { formatted: parts.join("\n\n"), timedOut }; +} |
