diff --git a/packages/opencode/src/flag/flag.ts b/packages/opencode/src/flag/flag.ts index b11058b3405..c06c3cceba9 100644 --- a/packages/opencode/src/flag/flag.ts +++ b/packages/opencode/src/flag/flag.ts @@ -45,6 +45,7 @@ export namespace Flag { export const OPENCODE_EXPERIMENTAL_OXFMT = OPENCODE_EXPERIMENTAL || truthy("OPENCODE_EXPERIMENTAL_OXFMT") export const OPENCODE_EXPERIMENTAL_LSP_TY = truthy("OPENCODE_EXPERIMENTAL_LSP_TY") export const OPENCODE_EXPERIMENTAL_LSP_TOOL = OPENCODE_EXPERIMENTAL || truthy("OPENCODE_EXPERIMENTAL_LSP_TOOL") + export const OPENCODE_EXPERIMENTAL_LSP_DIAGNOSTICS_TIMEOUT_MS = number("OPENCODE_EXPERIMENTAL_LSP_DIAGNOSTICS_TIMEOUT_MS") export const OPENCODE_DISABLE_FILETIME_CHECK = truthy("OPENCODE_DISABLE_FILETIME_CHECK") export const OPENCODE_EXPERIMENTAL_PLAN_MODE = OPENCODE_EXPERIMENTAL || truthy("OPENCODE_EXPERIMENTAL_PLAN_MODE") export const OPENCODE_EXPERIMENTAL_MARKDOWN = truthy("OPENCODE_EXPERIMENTAL_MARKDOWN") diff --git a/packages/opencode/src/lsp/client.test.ts b/packages/opencode/src/lsp/client.test.ts new file mode 100644 index 00000000000..2c7bc618a9b --- /dev/null +++ b/packages/opencode/src/lsp/client.test.ts @@ -0,0 +1,46 @@ +import { afterEach, describe, expect, test } from "bun:test" +import { Flag } from "@/flag/flag" + +describe("LSP Client diagnostics timeout configuration", () => { + const TIMEOUT_KEY = + "OPENCODE_EXPERIMENTAL_LSP_DIAGNOSTICS_TIMEOUT_MS" as const + + afterEach(() => { + // Restore to the original module-load-time value (undefined when env var isn't set) + Object.defineProperty(Flag, TIMEOUT_KEY, { + value: undefined, + configurable: true, + writable: true, + }) + }) + + test("defaults to 10_000ms when flag is undefined", () => { + // Flag value is undefined at module load time (env var not set) + const timeout = Flag[TIMEOUT_KEY] ?? 10_000 + expect(timeout).toBe(10_000) + }) + + test("uses custom timeout when flag is set", () => { + Object.defineProperty(Flag, TIMEOUT_KEY, { + value: 5_000, + configurable: true, + writable: true, + }) + + const timeout = Flag[TIMEOUT_KEY] ?? 10_000 + expect(timeout).toBe(5_000) + }) + + test("falls back to 10_000ms when flag value is invalid", () => { + // number() helper returns undefined for 0, negative, or non-integer values + // so Flag[TIMEOUT_KEY] would be undefined, triggering the ?? fallback + Object.defineProperty(Flag, TIMEOUT_KEY, { + value: undefined, + configurable: true, + writable: true, + }) + + const timeout = Flag[TIMEOUT_KEY] ?? 10_000 + expect(timeout).toBe(10_000) + }) +}) diff --git a/packages/opencode/src/lsp/client.ts b/packages/opencode/src/lsp/client.ts index 8704b65acb5..2126b996cda 100644 --- a/packages/opencode/src/lsp/client.ts +++ b/packages/opencode/src/lsp/client.ts @@ -12,6 +12,7 @@ import { NamedError } from "@opencode-ai/util/error" import { withTimeout } from "../util/timeout" import { Instance } from "../project/instance" import { Filesystem } from "../util/filesystem" +import { Flag } from "@/flag/flag" const DIAGNOSTICS_DEBOUNCE_MS = 150 @@ -228,7 +229,7 @@ export namespace LSPClient { } }) }), - 3000, + Flag.OPENCODE_EXPERIMENTAL_LSP_DIAGNOSTICS_TIMEOUT_MS ?? 10_000, ) .catch(() => {}) .finally(() => { diff --git a/packages/opencode/src/lsp/index.test.ts b/packages/opencode/src/lsp/index.test.ts new file mode 100644 index 00000000000..5a660c42306 --- /dev/null +++ b/packages/opencode/src/lsp/index.test.ts @@ -0,0 +1,278 @@ +import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test" +import * as BusModule from "@/bus" +import { Config } from "../config/config" +import { LSPClient } from "./client" +import { LSPServer } from "./server" +import { Instance } from "../project/instance" +import { tmpdir } from "../../test/fixture/fixture" + +type ClientLike = Awaited> +type ServerLike = { + id: string + extensions: string[] + root: (file: string) => Promise + spawn: (root: string) => Promise<{ process: { kill: () => void } } | undefined> +} + +const filePath = "/project/file.ts" +const rootPath = "/project" + +let publishCalls: Array<[unknown, unknown]> +let createCalls: Array<{ serverID: string; server: unknown; root: string }> +let spawnCalls: string[] +let openCalls: string[] +let killCalls: string[] + +let configSpy: ReturnType +let createSpy: ReturnType +let publishSpy: ReturnType + +let originalServers: [string, unknown][] + +function makeClient(root: string): ClientLike { + return { + root, + serverID: "mock", + notify: { + async open(input: { path: string }) { + openCalls.push(input.path) + }, + }, + diagnostics: new Map(), + async waitForDiagnostics() {}, + async shutdown() {}, + connection: {} as any, + } as ClientLike +} + +function defer() { + let resolve: (value: T) => void = () => {} + let reject: (reason?: unknown) => void = () => {} + const promise = new Promise((res, rej) => { + resolve = res + reject = rej + }) + return { promise, resolve, reject } +} + +async function flush(times = 3) { + for (let i = 0; i < times; i++) { + await Promise.resolve() + await new Promise((resolve) => setTimeout(resolve, 0)) + } +} + +function setMockServers(server: ServerLike) { + const entries = Object.entries(LSPServer) + for (const [key, value] of entries) { + if (!value || typeof value !== "object") continue + if (!("id" in (value as any) && "root" in (value as any) && "spawn" in (value as any))) continue + delete (LSPServer as any)[key] + } + ;(LSPServer as any).Mock = server +} + +function restoreServers() { + for (const key of Object.keys(LSPServer)) { + delete (LSPServer as any)[key] + } + for (const [key, value] of originalServers) { + ;(LSPServer as any)[key] = value + } +} + +async function loadLSP() { + const mod = (await import(`./index?test=${Math.random().toString(36).slice(2)}`)) as { + LSP: { + Event: { Updated: unknown } + touchFile: (file: string) => Promise + } + } + return mod.LSP +} + +beforeEach(() => { + publishCalls = [] + createCalls = [] + spawnCalls = [] + openCalls = [] + killCalls = [] + + originalServers = Object.entries(LSPServer) + + const server: ServerLike = { + id: "mock", + extensions: [".ts"], + root: async () => rootPath, + spawn: async (root: string) => { + spawnCalls.push(root) + return { + process: { + kill() { + killCalls.push(root) + }, + }, + } + }, + } + setMockServers(server) + + configSpy = spyOn(Config, "get").mockImplementation(async () => ({ lsp: {} } as any)) + createSpy = spyOn(LSPClient, "create").mockImplementation(async (input: any) => { + createCalls.push(input) + return makeClient(input.root) + }) + publishSpy = spyOn(BusModule.Bus, "publish").mockImplementation((event: unknown, payload: unknown) => { + publishCalls.push([event, payload]) + return undefined as any + }) +}) + +afterEach(async () => { + publishSpy.mockRestore() + createSpy.mockRestore() + configSpy.mockRestore() + restoreServers() + await Instance.disposeAll() +}) + +describe("LSP non-blocking initialization", () => { + test("getClients returns immediately during initialization", async () => { + configSpy.mockImplementation( + () => + new Promise((resolve) => { + setTimeout(() => resolve({ lsp: {} } as any), 250) + }), + ) + + const LSP = await loadLSP() + const tmp = await tmpdir({ git: true }) + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const started = Date.now() + await LSP.touchFile(filePath) + expect(Date.now() - started).toBeLessThan(100) + }, + }) + + expect(createCalls.length).toBe(0) + expect(spawnCalls.length).toBe(0) + }) + + test("getClients returns [] when state is not cached", async () => { + const LSP = await loadLSP() + const tmp = await tmpdir({ git: true }) + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + await LSP.touchFile(filePath) + }, + }) + + expect(createCalls.length).toBe(0) + expect(spawnCalls.length).toBe(0) + expect(openCalls.length).toBe(0) + }) + + test("getClients returns ready clients after initialization completes", async () => { + const LSP = await loadLSP() + const tmp = await tmpdir({ git: true }) + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + await LSP.touchFile(filePath) + await flush() + await LSP.touchFile(filePath) + await flush() + await LSP.touchFile(filePath) + }, + }) + + expect(createCalls.length).toBe(1) + expect(openCalls).toEqual([filePath]) + }) + + test("Bus.publish(Event.Updated, {}) fires when client becomes ready", async () => { + const pending = defer() + createSpy.mockImplementation(async (input: any) => { + createCalls.push(input) + return pending.promise + }) + + const LSP = await loadLSP() + const tmp = await tmpdir({ git: true }) + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + await LSP.touchFile(filePath) + await flush() + await LSP.touchFile(filePath) + await flush() + expect(publishCalls.length).toBe(0) + pending.resolve(makeClient(rootPath)) + await flush() + }, + }) + + expect(publishCalls.length).toBe(1) + expect(publishCalls[0][0]).toBe(LSP.Event.Updated) + expect(publishCalls[0][1]).toEqual({}) + }) + + test("broken set prevents retries of failed servers", async () => { + createSpy.mockImplementation(async (input: any) => { + createCalls.push(input) + throw new Error("create failed") + }) + + const LSP = await loadLSP() + const tmp = await tmpdir({ git: true }) + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + await LSP.touchFile(filePath) + await flush() + await LSP.touchFile(filePath) + await flush() + await LSP.touchFile(filePath) + await flush() + }, + }) + + expect(createCalls.length).toBe(1) + expect(spawnCalls.length).toBe(1) + expect(killCalls.length).toBe(2) + }) + + test("spawning map prevents duplicate spawn attempts", async () => { + const pending = defer() + createSpy.mockImplementation(async (input: any) => { + createCalls.push(input) + return pending.promise + }) + + const LSP = await loadLSP() + const tmp = await tmpdir({ git: true }) + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + await LSP.touchFile(filePath) + await flush() + await Promise.all([LSP.touchFile(filePath), LSP.touchFile(filePath)]) + await flush() + pending.resolve(makeClient(rootPath)) + await flush() + }, + }) + + expect(createCalls.length).toBe(1) + expect(spawnCalls.length).toBe(1) + }) +}) diff --git a/packages/opencode/src/lsp/index.ts b/packages/opencode/src/lsp/index.ts index 9d7d30632ab..72eea368a75 100644 --- a/packages/opencode/src/lsp/index.ts +++ b/packages/opencode/src/lsp/index.ts @@ -18,6 +18,8 @@ export namespace LSP { Updated: BusEvent.define("lsp.updated", z.object({})), } + let cachedState: Awaited> | undefined + export const Range = z .object({ start: z.object({ @@ -144,7 +146,9 @@ export namespace LSP { ) export async function init() { - return state() + const current = await state() + cachedState = current + return current } export const Status = z @@ -174,8 +178,15 @@ export namespace LSP { }) } - async function getClients(file: string) { - const s = await state() + function getClients(file: string): LSPClient.Info[] { + if (!cachedState) { + void state().then((current) => { + cachedState = current + }) + return [] + } + const s = cachedState + const extension = path.parse(file).ext || file const result: LSPClient.Info[] = [] @@ -224,38 +235,42 @@ export namespace LSP { for (const server of Object.values(s.servers)) { if (server.extensions.length && !server.extensions.includes(extension)) continue - const root = await server.root(file) - if (!root) continue - if (s.broken.has(root + server.id)) continue - - const match = s.clients.find((x) => x.root === root && x.serverID === server.id) - if (match) { - result.push(match) - continue + const ready = s.clients.find( + (client) => + client.serverID === server.id && + (file === client.root || file.startsWith(client.root + path.sep)), + ) + if (ready) { + result.push(ready) } - const inflight = s.spawning.get(root + server.id) - if (inflight) { - const client = await inflight - if (!client) continue - result.push(client) - continue - } - - const task = schedule(server, root, root + server.id) - s.spawning.set(root + server.id, task) - - task.finally(() => { - if (s.spawning.get(root + server.id) === task) { - s.spawning.delete(root + server.id) - } - }) - - const client = await task - if (!client) continue - - result.push(client) - Bus.publish(Event.Updated, {}) + void server + .root(file) + .then((root) => { + if (!root) return + const key = root + server.id + if (s.broken.has(key)) return + + const match = s.clients.find((x) => x.root === root && x.serverID === server.id) + if (match) return + + if (s.spawning.has(key)) return + + const task = schedule(server, root, key) + .then((client) => { + if (client) Bus.publish(Event.Updated, {}) + return client + }) + .finally(() => { + if (s.spawning.get(key) === task) { + s.spawning.delete(key) + } + }) + s.spawning.set(key, task) + }) + .catch((err) => { + log.error(`Failed to resolve LSP root for ${server.id}`, { error: err }) + }) } return result @@ -276,7 +291,7 @@ export namespace LSP { export async function touchFile(input: string, waitForDiagnostics?: boolean) { log.info("touching file", { file: input }) - const clients = await getClients(input) + const clients = getClients(input) await Promise.all( clients.map(async (client) => { const wait = waitForDiagnostics ? client.waitForDiagnostics({ path: input }) : Promise.resolve() @@ -461,7 +476,7 @@ export namespace LSP { } async function run(file: string, input: (client: LSPClient.Info) => Promise): Promise { - const clients = await getClients(file) + const clients = getClients(file) const tasks = clients.map((x) => input(x)) return Promise.all(tasks) }