diff --git a/packages/opencode/src/lsp/client.ts b/packages/opencode/src/lsp/client.ts index 59a64ca1ed..64545657fd 100644 --- a/packages/opencode/src/lsp/client.ts +++ b/packages/opencode/src/lsp/client.ts @@ -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( diff --git a/packages/opencode/src/lsp/lsp.ts b/packages/opencode/src/lsp/lsp.ts index c946ed2b0f..f32dffc040 100644 --- a/packages/opencode/src/lsp/lsp.ts +++ b/packages/opencode/src/lsp/lsp.ts @@ -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 | 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 }) diff --git a/packages/opencode/src/tool/apply_patch.ts b/packages/opencode/src/tool/apply_patch.ts index 7da7dd255c..14ecec0784 100644 --- a/packages/opencode/src/tool/apply_patch.ts +++ b/packages/opencode/src/tool/apply_patch.ts @@ -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)), + ) // Generate output summary const summaryLines = fileChanges.map((change) => { diff --git a/packages/opencode/src/tool/edit.ts b/packages/opencode/src/tool/edit.ts index 62b96cba82..84f2e3b24e 100644 --- a/packages/opencode/src/tool/edit.ts +++ b/packages/opencode/src/tool/edit.ts @@ -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)), + ) 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}` diff --git a/packages/opencode/src/tool/write.ts b/packages/opencode/src/tool/write.ts index c5871eb0ef..ed3075f0e4 100644 --- a/packages/opencode/src/tool/write.ts +++ b/packages/opencode/src/tool/write.ts @@ -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)), + ) const normalizedFilepath = AppFileSystem.normalizePath(filepath) let projectDiagnosticsCount = 0 for (const [file, issues] of Object.entries(diagnostics)) { diff --git a/packages/opencode/test/tool/write-lsp-hang.test.ts b/packages/opencode/test/tool/write-lsp-hang.test.ts index a5a3f3f904..ae87b27367 100644 --- a/packages/opencode/test/tool/write-lsp-hang.test.ts +++ b/packages/opencode/test/tool/write-lsp-hang.test.ts @@ -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: { diff --git a/packages/opencode/test/tool/write-lsp-spawn-hang.test.ts b/packages/opencode/test/tool/write-lsp-spawn-hang.test.ts index 82f167270a..d55c0ef815 100644 --- a/packages/opencode/test/tool/write-lsp-spawn-hang.test.ts +++ b/packages/opencode/test/tool/write-lsp-spawn-hang.test.ts @@ -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,