Skip to content

Commit 3a717b3

Browse files
potbsisyphus-dev-ai
andcommitted
fix(lsp): add poll retry for push-only diagnostics servers (#1522)
Closes #1522 ## Summary - After pull diagnostics fails, poll diagnosticsStore every 200ms for up to 3s - Fixes timing race where push diagnostics arrive after pull fails but before store read - Returns immediately if push arrives during poll window - Falls back to empty after timeout ceiling (same behavior as before, just with retry) - Servers like Godot that only support push diagnostics now work correctly Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <[email protected]>
1 parent d88449b commit 3a717b3

2 files changed

Lines changed: 154 additions & 1 deletion

File tree

src/tools/lsp/client.test.ts

Lines changed: 142 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { mkdtempSync, rmSync, writeFileSync } from "node:fs"
2-
import { join } from "node:path"
2+
import { join, resolve } from "node:path"
33
import { tmpdir } from "node:os"
4+
import { pathToFileURL } from "node:url"
45

56
import { describe, it, expect, spyOn, mock, beforeEach, afterEach } from "bun:test"
67

@@ -257,4 +258,144 @@ describe("LSPClient", () => {
257258
}
258259
})
259260
})
261+
262+
describe("diagnostics", () => {
263+
it("polls diagnosticsStore when pull fails and push arrives during poll window", async () => {
264+
// #given
265+
const dir = mkdtempSync(join(tmpdir(), "lsp-diagnostics-test-"))
266+
const filePath = join(dir, "test.ts")
267+
writeFileSync(filePath, "const a = 1\n")
268+
269+
const server: ResolvedServer = {
270+
id: "typescript",
271+
command: ["typescript-language-server", "--stdio"],
272+
extensions: [".ts"],
273+
priority: 0,
274+
}
275+
276+
const client = new LSPClient(dir, server)
277+
278+
// Mock sendRequest to throw (pull fails)
279+
const sendRequestSpy = spyOn(
280+
client as unknown as { sendRequest: (m: string, p?: unknown) => Promise<unknown> },
281+
"sendRequest"
282+
)
283+
sendRequestSpy.mockImplementation(async () => {
284+
throw new Error("pull not supported")
285+
})
286+
287+
// Mock openFile to avoid actual file operations
288+
const openFileSpy = spyOn(client, "openFile")
289+
openFileSpy.mockImplementation(async () => {})
290+
291+
try {
292+
// #when
293+
const expectedDiagnostics = [{ range: { start: { line: 0, character: 0 }, end: { line: 0, character: 5 } }, message: "error" }]
294+
const absPath = resolve(filePath)
295+
const uri = pathToFileURL(absPath).href
296+
297+
// Schedule push notification to populate store after 200ms
298+
setTimeout(() => {
299+
;(client as unknown as { diagnosticsStore: Map<string, unknown> }).diagnosticsStore.set(uri, expectedDiagnostics)
300+
}, 200)
301+
302+
const result = await client.diagnostics(filePath)
303+
304+
// #then
305+
expect(result.items).toEqual(expectedDiagnostics)
306+
} finally {
307+
sendRequestSpy.mockRestore()
308+
openFileSpy.mockRestore()
309+
rmSync(dir, { recursive: true, force: true })
310+
}
311+
})
312+
313+
it("returns empty array when pull fails and no push arrives within timeout", async () => {
314+
// #given
315+
const dir = mkdtempSync(join(tmpdir(), "lsp-diagnostics-timeout-test-"))
316+
const filePath = join(dir, "test.ts")
317+
writeFileSync(filePath, "const a = 1\n")
318+
319+
const server: ResolvedServer = {
320+
id: "typescript",
321+
command: ["typescript-language-server", "--stdio"],
322+
extensions: [".ts"],
323+
priority: 0,
324+
}
325+
326+
const client = new LSPClient(dir, server)
327+
328+
// Mock sendRequest to throw (pull fails)
329+
const sendRequestSpy = spyOn(
330+
client as unknown as { sendRequest: (m: string, p?: unknown) => Promise<unknown> },
331+
"sendRequest"
332+
)
333+
sendRequestSpy.mockImplementation(async () => {
334+
throw new Error("pull not supported")
335+
})
336+
337+
// Mock openFile to avoid actual file operations
338+
const openFileSpy = spyOn(client, "openFile")
339+
openFileSpy.mockImplementation(async () => {})
340+
341+
try {
342+
// #when
343+
const result = await client.diagnostics(filePath)
344+
345+
// #then
346+
expect(result.items).toEqual([])
347+
} finally {
348+
sendRequestSpy.mockRestore()
349+
openFileSpy.mockRestore()
350+
rmSync(dir, { recursive: true, force: true })
351+
}
352+
})
353+
354+
it("returns pull result immediately without polling when pull succeeds", async () => {
355+
// #given
356+
const dir = mkdtempSync(join(tmpdir(), "lsp-diagnostics-pull-success-test-"))
357+
const filePath = join(dir, "test.ts")
358+
writeFileSync(filePath, "const a = 1\n")
359+
360+
const server: ResolvedServer = {
361+
id: "typescript",
362+
command: ["typescript-language-server", "--stdio"],
363+
extensions: [".ts"],
364+
priority: 0,
365+
}
366+
367+
const client = new LSPClient(dir, server)
368+
369+
const pullDiagnostics = [{ range: { start: { line: 0, character: 0 }, end: { line: 0, character: 5 } }, message: "pull error" }]
370+
371+
// Mock sendRequest to return diagnostics (pull succeeds)
372+
const sendRequestSpy = spyOn(
373+
client as unknown as { sendRequest: (m: string, p?: unknown) => Promise<unknown> },
374+
"sendRequest"
375+
)
376+
sendRequestSpy.mockImplementation(async () => ({
377+
items: pullDiagnostics,
378+
}))
379+
380+
// Mock openFile to avoid actual file operations
381+
const openFileSpy = spyOn(client, "openFile")
382+
openFileSpy.mockImplementation(async () => {})
383+
384+
try {
385+
// #when
386+
const startTime = Date.now()
387+
const result = await client.diagnostics(filePath)
388+
const elapsed = Date.now() - startTime
389+
390+
// #then
391+
expect(result.items).toEqual(pullDiagnostics)
392+
// Should return quickly without waiting for poll timeout
393+
expect(elapsed).toBeLessThan(1000)
394+
} finally {
395+
sendRequestSpy.mockRestore()
396+
openFileSpy.mockRestore()
397+
rmSync(dir, { recursive: true, force: true })
398+
}
399+
})
400+
})
260401
})

src/tools/lsp/lsp-client.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,18 @@ export class LSPClient extends LSPClientConnection {
105105
}
106106
} catch {}
107107

108+
const POLL_INTERVAL_MS = 200
109+
const POLL_TIMEOUT_MS = 3000
110+
const startTime = Date.now()
111+
112+
while (Date.now() - startTime < POLL_TIMEOUT_MS) {
113+
const items = this.diagnosticsStore.get(uri)
114+
if (items && items.length > 0) {
115+
return { items }
116+
}
117+
await new Promise((r) => setTimeout(r, POLL_INTERVAL_MS))
118+
}
119+
108120
return { items: this.diagnosticsStore.get(uri) ?? [] }
109121
}
110122

0 commit comments

Comments
 (0)