fix: bound LSP diagnostic enrichment in write/edit/apply_patch (#22872)

Issue #22872 reports the write tool hanging indefinitely after a file
is written. Two underlying causes, both in the post-write LSP
enrichment path:

1. LSPClient.create wraps the `initialize` request in a 45s
   withTimeout. If the spawned LSP process is wedged (happens with
   pyright under certain conditions), every write that matches that
   LSP blocks the tool for up to 45s even though the file is on disk.

2. Server.spawn for npm-distributed LSPs (pyright, tsserver,
   biome, ...) calls Npm.which, which internally uses arborist.reify
   with no timeout. In sandboxed containers with no network access
   this promise never resolves — the write tool hangs forever.

Fix applied at three layers of defense:

- write.ts / edit.ts / apply_patch.ts: wrap the touchFile +
  diagnostics tail in a 5s Effect.timeout with catch-to-empty.
  Diagnostics are a best-effort enrichment; they must not block the
  tool's return after the file is already written.
- lsp.ts schedule(): bound server.spawn with a 10s Promise.race
  timeout. On timeout the server is added to s.broken so subsequent
  touches short-circuit instantly instead of re-racing.
- client.ts: lower the `initialize` withTimeout from 45_000 to
  10_000. If a server hasn't responded to initialize in 10s it's
  wedged; 45s was punishing for no benefit.

Reproducer tests (added in earlier commits on this branch) now pass:
- write-lsp-hang.test.ts  (branch A, 45s initialize timeout)
- write-lsp-spawn-hang.test.ts  (branch B, forever Npm.which)
Both complete in ~5s.

Full opencode test suite: 1934 pass, 0 fail.
This commit is contained in:
Kit Langton 2026-04-16 17:36:04 -04:00
parent 617c9841ef
commit 69f7182f05
7 changed files with 77 additions and 22 deletions

View file

@ -113,7 +113,13 @@ export async function create(input: { serverID: string; server: LSPServer.Handle
},
},
}),
45_000,
// 10s budget for LSP `initialize`. Previously 45s, which is far
// longer than a healthy server needs and effectively froze tools
// that await this path (e.g. write → lsp.touchFile). Servers that
// genuinely need more time for first-boot indexing publish their
// progress via window/workDoneProgress after initialize returns.
// See issue #22872.
10_000,
).catch((err) => {
l.error("initialize error", { error: err })
throw new InitializeError(

View file

@ -244,8 +244,22 @@ export const layer = Layer.effect(
}
async function schedule(server: LSPServer.Info, root: string, key: string) {
const handle = await runSpawn(server, root)
// Bound server.spawn so an unresponsive provisioning step
// (e.g. pyright's Npm.which → arborist.reify with no network)
// cannot hang touchFile forever. A 10s budget is generous for
// a cold start and an eternity for a wedged one. See #22872.
const SPAWN_TIMEOUT_MS = 10_000
let spawnTimer: ReturnType<typeof setTimeout> | undefined
const timeoutP = new Promise<"__lsp_spawn_timeout__">((resolve) => {
spawnTimer = setTimeout(() => resolve("__lsp_spawn_timeout__"), SPAWN_TIMEOUT_MS)
})
const handle = await Promise.race([runSpawn(server, root), timeoutP])
.then((value) => {
if (value === "__lsp_spawn_timeout__") {
s.broken.add(key)
log.error(`LSP server ${server.id} spawn timed out after ${SPAWN_TIMEOUT_MS}ms`, { root })
return undefined
}
if (!value) s.broken.add(key)
return value
})
@ -254,6 +268,9 @@ export const layer = Layer.effect(
log.error(`Failed to spawn LSP server ${server.id}`, { error: err })
return undefined
})
.finally(() => {
if (spawnTimer) clearTimeout(spawnTimer)
})
if (!handle) return undefined
log.info("spawned lsp server", { serverID: server.id, root })

View file

@ -1,6 +1,7 @@
import z from "zod"
import * as path from "path"
import { Effect } from "effect"
import { Duration, Effect } from "effect"
import type * as LSPClient from "../lsp/client"
import * as Tool from "./tool"
import { Bus } from "../bus"
import { FileWatcher } from "../file/watcher"
@ -244,13 +245,21 @@ export const ApplyPatchTool = Tool.define(
yield* bus.publish(FileWatcher.Event.Updated, update)
}
// Notify LSP of file changes and collect diagnostics
for (const change of fileChanges) {
if (change.type === "delete") continue
const target = change.movePath ?? change.filePath
yield* lsp.touchFile(target, true)
}
const diagnostics = yield* lsp.diagnostics()
// Notify LSP of file changes and collect diagnostics. Best-effort;
// bounded at 5s total so a slow or wedged LSP cannot block the
// tool result after the patches have already been applied. See
// issue #22872 and write.ts for the same pattern.
const diagnostics = yield* Effect.gen(function* () {
for (const change of fileChanges) {
if (change.type === "delete") continue
const target = change.movePath ?? change.filePath
yield* lsp.touchFile(target, true)
}
return yield* lsp.diagnostics()
}).pipe(
Effect.timeout(Duration.seconds(5)),
Effect.catch(() => Effect.succeed({} as Record<string, LSPClient.Diagnostic[]>)),
)
// Generate output summary
const summaryLines = fileChanges.map((change) => {

View file

@ -5,9 +5,10 @@
import z from "zod"
import * as path from "path"
import { Effect } from "effect"
import { Duration, Effect } from "effect"
import * as Tool from "./tool"
import { LSP } from "../lsp"
import type * as LSPClient from "../lsp/client"
import { createTwoFilesPatch, diffLines } from "diff"
import DESCRIPTION from "./edit.txt"
import { File } from "../file"
@ -166,8 +167,15 @@ export const EditTool = Tool.define(
})
let output = "Edit applied successfully."
yield* lsp.touchFile(filePath, true)
const diagnostics = yield* lsp.diagnostics()
// LSP diagnostic enrichment is best-effort; see write.ts for
// rationale. Never block the tool on a slow or wedged LSP.
const diagnostics = yield* Effect.gen(function* () {
yield* lsp.touchFile(filePath, true)
return yield* lsp.diagnostics()
}).pipe(
Effect.timeout(Duration.seconds(5)),
Effect.catch(() => Effect.succeed({} as Record<string, LSPClient.Diagnostic[]>)),
)
const normalizedFilePath = AppFileSystem.normalizePath(filePath)
const block = LSP.Diagnostic.report(filePath, diagnostics[normalizedFilePath] ?? [])
if (block) output += `\n\nLSP errors detected in this file, please fix:\n${block}`

View file

@ -1,8 +1,9 @@
import z from "zod"
import * as path from "path"
import { Effect } from "effect"
import { Duration, Effect } from "effect"
import * as Tool from "./tool"
import { LSP } from "../lsp"
import type * as LSPClient from "../lsp/client"
import { createTwoFilesPatch } from "diff"
import DESCRIPTION from "./write.txt"
import { Bus } from "../bus"
@ -64,8 +65,19 @@ export const WriteTool = Tool.define(
yield* filetime.read(ctx.sessionID, filepath)
let output = "Wrote file successfully."
yield* lsp.touchFile(filepath, true)
const diagnostics = yield* lsp.diagnostics()
// LSP diagnostic enrichment is best-effort. If the LSP server is
// slow to spawn, slow to initialize, or wedged entirely (e.g. a
// pyright install hanging on network in a sandboxed container)
// we must not block the tool's return on it — the file is
// already on disk. Bound at 5s and fall back to an empty
// diagnostics set. See issue #22872.
const diagnostics = yield* Effect.gen(function* () {
yield* lsp.touchFile(filepath, true)
return yield* lsp.diagnostics()
}).pipe(
Effect.timeout(Duration.seconds(5)),
Effect.catch(() => Effect.succeed({} as Record<string, LSPClient.Diagnostic[]>)),
)
const normalizedFilepath = AppFileSystem.normalizePath(filepath)
let projectDiagnosticsCount = 0
for (const [file, issues] of Object.entries(diagnostics)) {

View file

@ -84,9 +84,11 @@ describe("tool.write (LSP hang — issue #22872)", () => {
expect(content).toBe("print('hi')")
expect(result.output).toContain("Wrote file successfully")
// Regression guard: touchFile/diagnostics must not block the tool
// on the 45s LSPClient.create initialize timeout.
expect(elapsed).toBeLessThan(10_000)
// Regression guard: touchFile/diagnostics must not block the
// tool on the LSP initialize timeout. The write tool wraps
// its enrichment tail in a 5s Effect.timeout, so the tool
// must return within roughly 5s regardless of LSP state.
expect(elapsed).toBeLessThan(7_000)
}),
{
config: {

View file

@ -112,9 +112,10 @@ describe("tool.write (LSP spawn hang — issue #22872 forever branch)", () => {
expect(content).toBe("print('hi')")
expect(result.output).toContain("Wrote file successfully")
// The LSP spawn path is now blocked forever (Npm.Service.which
// returns Effect.never). The write tool must not wait on it.
expect(elapsed).toBeLessThan(10_000)
// The LSP spawn path is blocked forever (Npm.Service.which
// returns Effect.never). The write tool's 5s enrichment
// timeout must win, so the tool returns within roughly 5s.
expect(elapsed).toBeLessThan(7_000)
}),
),
15_000,