clean

Author: Hona

packages/opencode/src/lsp/client.ts

@@ -16,7 +16,8 @@ import { Filesystem } from "../util"
 const DIAGNOSTICS_DEBOUNCE_MS = 150
 const DIAGNOSTICS_POLL_MS = 500
 const DIAGNOSTICS_REQUEST_TIMEOUT_MS = 2_000
-const DIAGNOSTICS_WAIT_TIMEOUT_MS = 10_000
+const DIAGNOSTICS_WAIT_TIMEOUT_MS = 3_000
+const DIAGNOSTICS_PULL_WAIT_TIMEOUT_MS = 10_000
 const DIAGNOSTICS_SETTLE_MS = 1_500
 
 const log = Log.create({ service: "lsp.client" })
@@ -62,6 +63,7 @@ type ServerCapabilities = {
     | {
         change?: number
       }
+  diagnosticProvider?: unknown
   [key: string]: unknown
 }
 
@@ -210,6 +212,7 @@ export async function create(input: { serverID: string; server: LSPServer.Handle
     )
   })
   const syncKind = getSyncKind(initialized.capabilities)
+  const hasStaticPullDiagnostics = Boolean(initialized.capabilities?.diagnosticProvider)
 
   await connection.sendNotification("initialized", {})
 
@@ -257,10 +260,14 @@ export async function create(input: { serverID: string; server: LSPServer.Handle
   }
 
   async function requestDiagnostics(filePath: string) {
+    const registrations = [...diagnosticRegistrations.values()].filter(
+      (registration) => registration.registerOptions?.workspaceDiagnostics !== true,
+    )
+    if (!hasStaticPullDiagnostics && registrations.length === 0) return false
+
     const results = [await requestDiagnosticReport(filePath)]
     const identifiers = new Set(
-      [...diagnosticRegistrations.values()]
-        .filter((registration) => registration.registerOptions?.workspaceDiagnostics !== true)
+      registrations
         .map((registration) => registration.registerOptions?.identifier)
         .filter((identifier): identifier is string => Boolean(identifier)),
     )
@@ -374,40 +381,47 @@ export async function create(input: { serverID: string; server: LSPServer.Handle
         path.isAbsolute(request.path) ? request.path : path.resolve(input.directory, request.path),
       )
       log.info("waiting for diagnostics", { path: normalizedPath })
-      let unsub: () => void
+      let unsub: (() => void) | undefined
       let debounceTimer: ReturnType<typeof setTimeout> | undefined
-      let pushed = false
-      let firstHandledAt: number | undefined
+      let done = false
+      let resolvePushed: (() => void) | undefined
+      const timeout = hasStaticPullDiagnostics || diagnosticRegistrations.size > 0
+        ? DIAGNOSTICS_PULL_WAIT_TIMEOUT_MS
+        : DIAGNOSTICS_WAIT_TIMEOUT_MS
       return await withTimeout(
-        (async () => {
-          unsub = Bus.subscribe(Event.Diagnostics, (event) => {
-            if (event.properties.path !== normalizedPath || event.properties.serverID !== result.serverID) return
-            if (debounceTimer) clearTimeout(debounceTimer)
-            debounceTimer = setTimeout(() => {
-              pushed = true
-            }, DIAGNOSTICS_DEBOUNCE_MS)
-          })
-          await new Promise((resolve) => setTimeout(resolve, DIAGNOSTICS_DEBOUNCE_MS))
-          while (true) {
-            if (pushed) {
-              log.info("got diagnostics", { path: normalizedPath })
-              return
-            }
-            const handled = await requestDiagnostics(normalizedPath)
-            if (handled) {
-              firstHandledAt = firstHandledAt ?? Date.now()
-              if (Date.now() - firstHandledAt >= DIAGNOSTICS_SETTLE_MS) {
+        Promise.race([
+          new Promise<void>((resolve) => {
+            resolvePushed = resolve
+            unsub = Bus.subscribe(Event.Diagnostics, (event) => {
+              if (event.properties.path !== normalizedPath || event.properties.serverID !== result.serverID) return
+              if (debounceTimer) clearTimeout(debounceTimer)
+              debounceTimer = setTimeout(() => {
                 log.info("got diagnostics", { path: normalizedPath })
-                return
+                resolve()
+              }, DIAGNOSTICS_DEBOUNCE_MS)
+            })
+          }),
+          (async () => {
+            let firstHandledAt: number | undefined
+            while (!done) {
+              const handled = await requestDiagnostics(normalizedPath)
+              if (handled) {
+                firstHandledAt = firstHandledAt ?? Date.now()
+                if (Date.now() - firstHandledAt >= DIAGNOSTICS_SETTLE_MS) {
+                  log.info("got diagnostics", { path: normalizedPath })
+                  return
+                }
               }
+              await new Promise((resolve) => setTimeout(resolve, DIAGNOSTICS_POLL_MS))
             }
-            await new Promise((resolve) => setTimeout(resolve, DIAGNOSTICS_POLL_MS))
-          }
-        })(),
-        DIAGNOSTICS_WAIT_TIMEOUT_MS,
+          })(),
+        ]),
+        timeout,
       )
         .catch(() => {})
         .finally(() => {
+          done = true
+          resolvePushed?.()
           if (debounceTimer) clearTimeout(debounceTimer)
           unsub?.()
         })

packages/opencode/test/fixture/lsp/fake-lsp-server.js

@@ -3,6 +3,7 @@
 
 let nextId = 1
 let lastDidChange = null
+let diagnosticRequests = 0
 
 function encode(message) {
   const json = JSON.stringify(message)
@@ -78,10 +79,32 @@ function handle(raw) {
     lastDidChange = data.params
     return
   }
+  if (data.method === "textDocument/diagnostic") {
+    diagnosticRequests += 1
+    send({ jsonrpc: "2.0", id: data.id, result: null })
+    return
+  }
   if (data.method === "test/get-last-change") {
     send({ jsonrpc: "2.0", id: data.id, result: lastDidChange })
     return
   }
+  if (data.method === "test/get-diagnostic-request-count") {
+    send({ jsonrpc: "2.0", id: data.id, result: diagnosticRequests })
+    return
+  }
+  if (data.method === "test/publish-diagnostics") {
+    const uri = data.params && data.params.uri
+    if (!uri) return
+    send({
+      jsonrpc: "2.0",
+      method: "textDocument/publishDiagnostics",
+      params: {
+        uri,
+        diagnostics: (data.params && data.params.diagnostics) || [],
+      },
+    })
+    return
+  }
   if (data.method === "test/trigger") {
     const method = data.params && data.params.method
     if (method) sendRequest(method, {})

packages/opencode/test/lsp/client.test.ts

@@ -1,5 +1,6 @@
 import { describe, expect, test, beforeEach } from "bun:test"
 import path from "path"
+import { pathToFileURL } from "url"
 import { tmpdir } from "../fixture/fixture"
 import { LSPClient } from "../../src/lsp"
 import { LSPServer } from "../../src/lsp"
@@ -142,4 +143,36 @@ describe("LSPClient interop", () => {
       },
     })
   })
+
+  test("waitForDiagnostics avoids pull requests on push-only servers", async () => {
+    const handle = spawnFakeServer() as any
+    await using tmp = await tmpdir()
+    const file = path.join(tmp.path, "client.ts")
+    await Bun.write(file, "const x = 1\n")
+
+    await Instance.provide({
+      directory: tmp.path,
+      fn: async () => {
+        const client = await LSPClient.create({
+          serverID: "fake",
+          server: handle as unknown as LSPServer.Handle,
+          root: tmp.path,
+          directory: tmp.path,
+        })
+
+        const wait = client.waitForDiagnostics({ path: file })
+        await client.notify.open({ path: file })
+        await client.connection.sendNotification("test/publish-diagnostics", {
+          uri: pathToFileURL(file).href,
+          diagnostics: [],
+        })
+        await wait
+
+        const count = await client.connection.sendRequest<number>("test/get-diagnostic-request-count", {})
+        expect(count).toBe(0)
+
+        await client.shutdown()
+      },
+    })
+  })
 })