From 4c52af2a658a9ba608ab48e6d0067cfc4da8be67 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Thu, 16 Apr 2026 15:50:06 -0400 Subject: [PATCH 1/3] test: add reproducer for write tool hanging on slow LSP initialize (#22872) Adds a failing regression test that reproduces the write tool hang reported in #22872. The write tool calls lsp.touchFile + lsp.diagnostics to enrich its output; if a matching LSP server spawns but never responds to the initialize request, the tool blocks on LSPClient.create's 45s withTimeout. The test configures a fake LSP server (hanging-lsp-server.js) that swallows every message and never replies, asserts the file is still written correctly, and checks the tool returns within 10s. On dev today the assertion fails with ~45s actual, proving the hang. The fix should make this green by bounding the diagnostic-enrichment tail. --- .../test/fixture/lsp/hanging-lsp-server.js | 44 ++++++++ .../opencode/test/tool/write-lsp-hang.test.ts | 104 ++++++++++++++++++ 2 files changed, 148 insertions(+) create mode 100644 packages/opencode/test/fixture/lsp/hanging-lsp-server.js create mode 100644 packages/opencode/test/tool/write-lsp-hang.test.ts diff --git a/packages/opencode/test/fixture/lsp/hanging-lsp-server.js b/packages/opencode/test/fixture/lsp/hanging-lsp-server.js new file mode 100644 index 0000000000..73b3cb2fee --- /dev/null +++ b/packages/opencode/test/fixture/lsp/hanging-lsp-server.js @@ -0,0 +1,44 @@ +// Fake LSP server that intentionally never responds to `initialize`. +// Used by tests that reproduce hangs in the LSP touchFile flow when an +// LSP server process spawns successfully but the handshake stalls. The +// process also ignores SIGTERM for a short period to surface any teardown +// issues, but exits cleanly on SIGKILL. + +let readBuffer = Buffer.alloc(0) + +function decodeFrames(buffer) { + const results = [] + let idx + while ((idx = buffer.indexOf("\r\n\r\n")) !== -1) { + const header = buffer.slice(0, idx).toString("utf8") + const m = /Content-Length:\s*(\d+)/i.exec(header) + const len = m ? parseInt(m[1], 10) : 0 + const bodyStart = idx + 4 + const bodyEnd = bodyStart + len + if (buffer.length < bodyEnd) break + results.push(buffer.slice(bodyStart, bodyEnd).toString("utf8")) + buffer = buffer.slice(bodyEnd) + } + return { messages: results, rest: buffer } +} + +process.stdin.on("data", (chunk) => { + readBuffer = Buffer.concat([readBuffer, chunk]) + const { messages, rest } = decodeFrames(readBuffer) + readBuffer = rest + // Swallow everything — including `initialize`. Never reply. + for (const _ of messages) { + // no-op + } +}) + +// Keep the process alive until parent terminates us or closes stdin. +const keepalive = setInterval(() => {}, 60_000) +process.stdin.on("end", () => { + clearInterval(keepalive) + process.exit(0) +}) +process.stdin.on("close", () => { + clearInterval(keepalive) + process.exit(0) +}) diff --git a/packages/opencode/test/tool/write-lsp-hang.test.ts b/packages/opencode/test/tool/write-lsp-hang.test.ts new file mode 100644 index 0000000000..a5a3f3f904 --- /dev/null +++ b/packages/opencode/test/tool/write-lsp-hang.test.ts @@ -0,0 +1,104 @@ +import { afterEach, describe, expect } from "bun:test" +import { Effect, Layer } from "effect" +import path from "path" +import fs from "fs/promises" +import { WriteTool } from "../../src/tool/write" +import { Instance } from "../../src/project/instance" +import { LSP } from "../../src/lsp" +import { AppFileSystem } from "@opencode-ai/shared/filesystem" +import { FileTime } from "../../src/file/time" +import { Bus } from "../../src/bus" +import { Format } from "../../src/format" +import { Truncate } from "../../src/tool" +import { Tool } from "../../src/tool" +import { Agent } from "../../src/agent/agent" +import { SessionID, MessageID } from "../../src/session/schema" +import * as CrossSpawnSpawner from "../../src/effect/cross-spawn-spawner" +import { provideTmpdirInstance } from "../fixture/fixture" +import { testEffect } from "../lib/effect" + +// Reproduces issue #22872 — the write tool hangs when an LSP server for the +// file's extension spawns successfully but never answers the `initialize` +// request. The fake LSP here swallows every message, mimicking pyright in +// the reporter's Docker container. If the write tool correctly bounds the +// diagnostic-enrichment tail (lsp.touchFile + lsp.diagnostics) the whole +// call should finish quickly, well before the 45s LSPClient.create timeout. + +const HANGING_SERVER = path.resolve(__dirname, "..", "fixture", "lsp", "hanging-lsp-server.js") + +const ctx = { + sessionID: SessionID.make("ses_test-write-lsp-hang"), + messageID: MessageID.make(""), + callID: "", + agent: "build", + abort: AbortSignal.any([]), + messages: [], + metadata: () => Effect.void, + ask: () => Effect.void, +} + +afterEach(async () => { + await Instance.disposeAll() +}) + +const it = testEffect( + Layer.mergeAll( + LSP.defaultLayer, + AppFileSystem.defaultLayer, + FileTime.defaultLayer, + Bus.layer, + Format.defaultLayer, + CrossSpawnSpawner.defaultLayer, + Truncate.defaultLayer, + Agent.defaultLayer, + ), +) + +const init = Effect.fn("WriteLspHangTest.init")(function* () { + const info = yield* WriteTool + return yield* info.init() +}) + +const run = Effect.fn("WriteLspHangTest.run")(function* ( + args: Tool.InferParameters, + next: Tool.Context = ctx, +) { + const tool = yield* init() + return yield* tool.execute(args, next) +}) + +describe("tool.write (LSP hang — issue #22872)", () => { + it.live( + "completes promptly when the LSP server for this extension never finishes initialize", + () => + provideTmpdirInstance( + (dir) => + Effect.gen(function* () { + const filepath = path.join(dir, "hello.hang") + const started = Date.now() + const result = yield* run({ filePath: filepath, content: "print('hi')" }) + const elapsed = Date.now() - started + + // On disk content is correct. + const content = yield* Effect.promise(() => fs.readFile(filepath, "utf-8")) + 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) + }), + { + config: { + lsp: { + "hang-ls": { + command: ["node", HANGING_SERVER], + extensions: [".hang"], + }, + }, + }, + }, + ), + 60_000, + ) +}) From 617c9841efe03d1661c7de38696cefea7e7cf122 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Thu, 16 Apr 2026 17:03:19 -0400 Subject: [PATCH 2/3] test: add reproducer for Npm.which-forever hang in Pyright spawn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a second reproducer covering the 'forever' branch of issue #22872: when Pyright.spawn calls Npm.which('pyright') and the npm registry is unreachable (sandboxed container), arborist.reify blocks indefinitely with no timeout. Changes: - Adds optional Info.spawnEffect alongside the existing async Info.spawn. spawnEffect returns an Effect that can yield from Npm.Service, making npm lookups injectable for tests. - Migrates Pyright to use spawnEffect, pulling the venv probing logic into a reusable pyrightVenvInitialization helper. The legacy async spawn stays for backwards compatibility. - Threads Npm.Service through LSP.layer so getClients captures a stable reference and uses it for any server that provides spawnEffect. - Adds test/tool/write-lsp-spawn-hang.test.ts — mocks Npm.Service.which with Effect.never and asserts the write tool still returns in < 10s. Fails today (hangs forever); the fix must bound the touchFile tail so the tool cannot wait on a wedged LSP spawn. The two reproducers now cover both hang branches: - write-lsp-hang.test.ts: 45s LSPClient.create initialize timeout - write-lsp-spawn-hang.test.ts: unbounded Npm.which arborist.reify --- packages/opencode/src/lsp/lsp.ts | 19 ++- packages/opencode/src/lsp/server.ts | 75 +++++++---- .../test/tool/write-lsp-spawn-hang.test.ts | 122 ++++++++++++++++++ 3 files changed, 186 insertions(+), 30 deletions(-) create mode 100644 packages/opencode/test/tool/write-lsp-spawn-hang.test.ts diff --git a/packages/opencode/src/lsp/lsp.ts b/packages/opencode/src/lsp/lsp.ts index d4d1e75634..c946ed2b0f 100644 --- a/packages/opencode/src/lsp/lsp.ts +++ b/packages/opencode/src/lsp/lsp.ts @@ -13,6 +13,7 @@ import { Process } from "../util" import { spawn as lspspawn } from "./launch" import { Effect, Layer, Context } from "effect" import { InstanceState } from "@/effect" +import { Npm as EffectNpm } from "@opencode-ai/shared/npm" const log = Log.create({ service: "lsp" }) @@ -160,6 +161,10 @@ export const layer = Layer.effect( Service, Effect.gen(function* () { const config = yield* Config.Service + // Resolve Npm.Service once at layer construction so per-call methods + // (getClients → server.spawnEffect) can capture a stable reference + // without propagating Npm.Service into the public LSP Interface. + const npm = yield* EffectNpm.Service const state = yield* InstanceState.make( Effect.fn("LSP.state")(function* () { @@ -229,9 +234,17 @@ export const layer = Layer.effect( const extension = path.parse(file).ext || file const result: LSPClient.Info[] = [] + const runSpawn = (server: LSPServer.Info, root: string): Promise => { + if (server.spawnEffect) { + return Effect.runPromise( + server.spawnEffect(root).pipe(Effect.provideService(EffectNpm.Service, npm)), + ) + } + return server.spawn(root) + } + async function schedule(server: LSPServer.Info, root: string, key: string) { - const handle = await server - .spawn(root) + const handle = await runSpawn(server, root) .then((value) => { if (!value) s.broken.add(key) return value @@ -504,7 +517,7 @@ export const layer = Layer.effect( }), ) -export const defaultLayer = layer.pipe(Layer.provide(Config.defaultLayer)) +export const defaultLayer = layer.pipe(Layer.provide(Config.defaultLayer), Layer.provide(EffectNpm.defaultLayer)) export namespace Diagnostic { const MAX_PER_FILE = 20 diff --git a/packages/opencode/src/lsp/server.ts b/packages/opencode/src/lsp/server.ts index 390c5f2428..e98d9e190a 100644 --- a/packages/opencode/src/lsp/server.ts +++ b/packages/opencode/src/lsp/server.ts @@ -1,6 +1,7 @@ import type { ChildProcessWithoutNullStreams } from "child_process" import path from "path" import os from "os" +import { Effect, Option } from "effect" import { Global } from "../global" import { Log } from "../util" import { text } from "node:stream/consumers" @@ -13,6 +14,7 @@ import { Process } from "../util" import { which } from "../util/which" import { Module } from "@opencode-ai/shared/util/module" import { spawn } from "./launch" +import { Npm as EffectNpm } from "@opencode-ai/shared/npm" import { Npm } from "../npm" const log = Log.create({ service: "lsp.server" }) @@ -61,6 +63,14 @@ export interface Info { global?: boolean root: RootFunction spawn(root: string): Promise + /** + * Optional Effect-based variant of `spawn`. When present, the LSP + * runtime uses this instead of `spawn` so the server body can yield + * from platform services like `Npm.Service.which`. Use for servers + * distributed via npm so tests can inject fakes. Existing `spawn` + * implementations continue to work unchanged. + */ + spawnEffect?(root: string): Effect.Effect } export const Deno: Info = { @@ -486,6 +496,9 @@ export const Pyright: Info = { extensions: [".py", ".pyi"], root: NearestRoot(["pyproject.toml", "setup.py", "setup.cfg", "requirements.txt", "Pipfile", "pyrightconfig.json"]), async spawn(root) { + // Legacy async entry point — kept so the Info interface stays + // backwards-compatible. The live runtime always prefers spawnEffect + // below; this path only runs if someone invokes spawn() directly. let binary = which("pyright-langserver") const args = [] if (!binary) { @@ -495,34 +508,42 @@ export const Pyright: Info = { binary = resolved } args.push("--stdio") - - const initialization: Record = {} - - const potentialVenvPaths = [process.env["VIRTUAL_ENV"], path.join(root, ".venv"), path.join(root, "venv")].filter( - (p): p is string => p !== undefined, - ) - for (const venvPath of potentialVenvPaths) { - const isWindows = process.platform === "win32" - const potentialPythonPath = isWindows - ? path.join(venvPath, "Scripts", "python.exe") - : path.join(venvPath, "bin", "python") - if (await Filesystem.exists(potentialPythonPath)) { - initialization["pythonPath"] = potentialPythonPath - break - } - } - - const proc = spawn(binary, args, { - cwd: root, - env: { - ...process.env, - }, - }) - return { - process: proc, - initialization, - } + const initialization = await pyrightVenvInitialization(root) + const proc = spawn(binary, args, { cwd: root, env: { ...process.env } }) + return { process: proc, initialization } }, + spawnEffect: (root) => + Effect.gen(function* () { + let binary = which("pyright-langserver") + if (!binary) { + if (Flag.OPENCODE_DISABLE_LSP_DOWNLOAD) return undefined + const npm = yield* EffectNpm.Service + const resolved = yield* npm.which("pyright") + if (Option.isNone(resolved)) return undefined + binary = resolved.value + } + const initialization = yield* Effect.promise(() => pyrightVenvInitialization(root)) + const proc = spawn(binary, ["--stdio"], { cwd: root, env: { ...process.env } }) + return { process: proc, initialization } + }), +} + +async function pyrightVenvInitialization(root: string): Promise> { + const initialization: Record = {} + const potentialVenvPaths = [process.env["VIRTUAL_ENV"], path.join(root, ".venv"), path.join(root, "venv")].filter( + (p): p is string => p !== undefined, + ) + for (const venvPath of potentialVenvPaths) { + const isWindows = process.platform === "win32" + const potentialPythonPath = isWindows + ? path.join(venvPath, "Scripts", "python.exe") + : path.join(venvPath, "bin", "python") + if (await Filesystem.exists(potentialPythonPath)) { + initialization["pythonPath"] = potentialPythonPath + break + } + } + return initialization } export const ElixirLS: Info = { diff --git a/packages/opencode/test/tool/write-lsp-spawn-hang.test.ts b/packages/opencode/test/tool/write-lsp-spawn-hang.test.ts new file mode 100644 index 0000000000..82f167270a --- /dev/null +++ b/packages/opencode/test/tool/write-lsp-spawn-hang.test.ts @@ -0,0 +1,122 @@ +import { afterEach, beforeAll, afterAll, describe, expect } from "bun:test" +import { Effect, Layer, Option } from "effect" +import path from "path" +import fs from "fs/promises" +import { Npm } from "@opencode-ai/shared/npm" +import { Config } from "../../src/config" +import { WriteTool } from "../../src/tool/write" +import { Instance } from "../../src/project/instance" +import * as LSP from "../../src/lsp/lsp" +import { AppFileSystem } from "@opencode-ai/shared/filesystem" +import { FileTime } from "../../src/file/time" +import { Bus } from "../../src/bus" +import { Format } from "../../src/format" +import { Truncate } from "../../src/tool" +import { Tool } from "../../src/tool" +import { Agent } from "../../src/agent/agent" +import { SessionID, MessageID } from "../../src/session/schema" +import * as CrossSpawnSpawner from "../../src/effect/cross-spawn-spawner" +import { provideTmpdirInstance } from "../fixture/fixture" +import { testEffect } from "../lib/effect" + +// Reproduces the "forever" branch of issue #22872 — in a sandboxed +// container with no network and no cached pyright binary, Pyright.spawn +// calls `Npm.Service.which("pyright")` which internally uses +// `arborist.reify()` with no timeout. If the npm registry is +// unreachable, that promise never resolves and the write tool blocks +// indefinitely. +// +// Here we mock Npm.Service so `which("pyright")` returns Effect.never, +// simulating the unbounded network block. The write tool must still +// return quickly for the fix to be correct — shortening the 45s +// LSPClient.create initialize timeout would NOT help this case, so +// the fix must bound the touchFile enrichment tail itself. + +const ctx = { + sessionID: SessionID.make("ses_test-write-lsp-spawn-hang"), + messageID: MessageID.make(""), + callID: "", + agent: "build", + abort: AbortSignal.any([]), + messages: [], + metadata: () => Effect.void, + ask: () => Effect.void, +} + +// Ensure pyright-langserver isn't picked up from the user's real PATH +// during the test — we want the spawn to fall through to Npm.which. +let savedPath: string | undefined +beforeAll(() => { + savedPath = process.env.PATH + process.env.PATH = "" +}) +afterAll(() => { + process.env.PATH = savedPath +}) + +afterEach(async () => { + await Instance.disposeAll() +}) + +const hangingNpm = Layer.mock(Npm.Service)({ + add: () => Effect.never, + install: () => Effect.never, + outdated: () => Effect.succeed(false), + which: () => Effect.never as unknown as Effect.Effect>, +}) + +// Build the LSP layer with the hanging Npm mock in place of the real one. +// LSP.defaultLayer pre-provides the real EffectNpm.defaultLayer which would +// shadow any outer provide, so we wire the mock directly into LSP.layer. +const lspWithHangingNpm = LSP.layer.pipe(Layer.provide(Config.defaultLayer), Layer.provide(hangingNpm)) + +const it = testEffect( + Layer.mergeAll( + lspWithHangingNpm, + AppFileSystem.defaultLayer, + FileTime.defaultLayer, + Bus.layer, + Format.defaultLayer, + CrossSpawnSpawner.defaultLayer, + Truncate.defaultLayer, + Agent.defaultLayer, + ), +) + +const init = Effect.fn("WriteLspSpawnHangTest.init")(function* () { + const info = yield* WriteTool + return yield* info.init() +}) + +const run = Effect.fn("WriteLspSpawnHangTest.run")(function* ( + args: Tool.InferParameters, + next: Tool.Context = ctx, +) { + const tool = yield* init() + return yield* tool.execute(args, next) +}) + +describe("tool.write (LSP spawn hang — issue #22872 forever branch)", () => { + it.live( + "completes promptly when Npm.Service.which hangs forever during LSP spawn", + () => + provideTmpdirInstance((dir) => + Effect.gen(function* () { + const filepath = path.join(dir, "hello.py") + const started = Date.now() + const result = yield* run({ filePath: filepath, content: "print('hi')" }) + const elapsed = Date.now() - started + + // File is on disk even though LSP spawn is wedged. + const content = yield* Effect.promise(() => fs.readFile(filepath, "utf-8")) + 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) + }), + ), + 15_000, + ) +}) From 69f7182f053698260aacc0bc6086caddb6f83f11 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Thu, 16 Apr 2026 17:36:04 -0400 Subject: [PATCH 3/3] fix: bound LSP diagnostic enrichment in write/edit/apply_patch (#22872) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- packages/opencode/src/lsp/client.ts | 8 +++++- packages/opencode/src/lsp/lsp.ts | 19 +++++++++++++- packages/opencode/src/tool/apply_patch.ts | 25 +++++++++++++------ packages/opencode/src/tool/edit.ts | 14 ++++++++--- packages/opencode/src/tool/write.ts | 18 ++++++++++--- .../opencode/test/tool/write-lsp-hang.test.ts | 8 +++--- .../test/tool/write-lsp-spawn-hang.test.ts | 7 +++--- 7 files changed, 77 insertions(+), 22 deletions(-) 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,