Compare commits

...

3 Commits

Author SHA1 Message Date
Kit Langton 69f7182f05 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.
2026-04-16 17:36:04 -04:00
Kit Langton 617c9841ef test: add reproducer for Npm.which-forever hang in Pyright spawn
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
2026-04-16 17:03:19 -04:00
Kit Langton 4c52af2a65 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.
2026-04-16 15:50:06 -04:00
9 changed files with 404 additions and 45 deletions
+7 -1
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(
+33 -3
View File
@@ -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<State>(
Effect.fn("LSP.state")(function* () {
@@ -229,10 +234,32 @@ export const layer = Layer.effect(
const extension = path.parse(file).ext || file
const result: LSPClient.Info[] = []
const runSpawn = (server: LSPServer.Info, root: string): Promise<LSPServer.Handle | undefined> => {
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)
// 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
})
@@ -241,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 })
@@ -504,7 +534,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
+48 -27
View File
@@ -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<Handle | undefined>
/**
* 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<Handle | undefined, never, EffectNpm.Service>
}
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<string, string> = {}
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<Record<string, string>> {
const initialization: Record<string, string> = {}
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 = {
+17 -8
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) => {
+11 -3
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}`
+15 -3
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)) {
@@ -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)
})
@@ -0,0 +1,106 @@
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<typeof WriteTool>,
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 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: {
lsp: {
"hang-ls": {
command: ["node", HANGING_SERVER],
extensions: [".hang"],
},
},
},
},
),
60_000,
)
})
@@ -0,0 +1,123 @@
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<Option.Option<string>>,
})
// 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<typeof WriteTool>,
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 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,
)
})