fix(lsp): Enhanced detection (availability/support/capabilities) and timeout/error handling (#234)

* fix(lsp): enhance LSP server resolution logic and client timeout handling

* fix(lsp): enhance diagnostics handling and add workspace configuration support

* fix(lsp): implement pull diagnostics handling and caching mechanism

* fix(lsp): enhance diagnostics capabilities and logging in LSPClient

src/tools/lsp/client.test.ts

@@ -7,6 +7,9 @@ import {
   spyOn,
   test,
 } from 'bun:test';
+import { mkdirSync, rmSync, writeFileSync } from 'node:fs';
+import { join, resolve } from 'node:path';
+import { pathToFileURL } from 'node:url';
 
 // Mock spawn from bun
 mock.module('bun', () => ({
@@ -30,7 +33,13 @@ mock.module('bun', () => ({
   }),
 }));
 
-import { LSPClient, lspManager } from './client';
+import {
+  getDiagnosticsCapabilitySummary,
+  getWorkspaceConfiguration,
+  LSP_TIMEOUTS,
+  LSPClient,
+  lspManager,
+} from './client';
 
 describe('LSPServerManager', () => {
   let startSpy: any;
@@ -146,3 +155,396 @@ describe('LSPServerManager', () => {
     onSpy.mockRestore();
   });
 });
+
+describe('LSPClient diagnostics', () => {
+  const tempDir = join(process.cwd(), '.tmp-lsp-tests');
+  const tempFile = join(tempDir, 'index.ts');
+
+  beforeEach(async () => {
+    await lspManager.stopAll();
+    mkdirSync(tempDir, { recursive: true });
+    writeFileSync(tempFile, 'export const value = 1;\n');
+  });
+
+  afterEach(async () => {
+    rmSync(tempDir, { recursive: true, force: true });
+    await lspManager.stopAll();
+  });
+
+  test('diagnostics falls back to cached publishDiagnostics on timeout', async () => {
+    const originalRequestTimeout = LSP_TIMEOUTS.request;
+    LSP_TIMEOUTS.request = 10;
+
+    const client = new LSPClient(tempDir, {
+      id: 'test',
+      command: ['/bin/true'],
+      extensions: ['.ts'],
+    });
+
+    (client as any).supportsPullDiagnostics = true;
+
+    (client as any).connection = {
+      sendNotification: mock(),
+      sendRequest: mock((method: string) => {
+        if (method === 'textDocument/diagnostic') {
+          return new Promise(() => {});
+        }
+        return Promise.resolve({});
+      }),
+    };
+
+    (client as any).diagnosticsStore.set(pathToFileURL(tempFile).href, [
+      {
+        message: 'cached diagnostic',
+        range: {
+          start: { line: 0, character: 0 },
+          end: { line: 0, character: 1 },
+        },
+        severity: 1,
+      },
+    ]);
+
+    try {
+      const result = await client.diagnostics(tempFile);
+      expect(result.items).toHaveLength(1);
+      expect(result.items[0]?.message).toBe('cached diagnostic');
+    } finally {
+      LSP_TIMEOUTS.request = originalRequestTimeout;
+    }
+  });
+
+  test('diagnostics throws after timeout when no cached diagnostics exist', async () => {
+    const originalRequestTimeout = LSP_TIMEOUTS.request;
+    LSP_TIMEOUTS.request = 10;
+
+    const client = new LSPClient(tempDir, {
+      id: 'test',
+      command: ['/bin/true'],
+      extensions: ['.ts'],
+    });
+
+    (client as any).supportsPullDiagnostics = true;
+
+    (client as any).connection = {
+      sendNotification: mock(),
+      sendRequest: mock((method: string) => {
+        if (method === 'textDocument/diagnostic') {
+          return new Promise(() => {});
+        }
+        return Promise.resolve({});
+      }),
+    };
+
+    try {
+      await expect(client.diagnostics(tempFile)).rejects.toThrow(
+        'Unable to retrieve diagnostics',
+      );
+    } finally {
+      LSP_TIMEOUTS.request = originalRequestTimeout;
+    }
+  });
+
+  test('diagnostics uses cached publishDiagnostics for push-only servers', async () => {
+    const originalRequestTimeout = LSP_TIMEOUTS.request;
+    LSP_TIMEOUTS.request = 200;
+
+    const client = new LSPClient(tempDir, {
+      id: 'test',
+      command: ['/bin/true'],
+      extensions: ['.ts'],
+    });
+
+    const sendRequest = mock(() => Promise.resolve({}));
+
+    (client as any).connection = {
+      sendNotification: mock(
+        (method: string, params?: { textDocument?: { uri?: string } }) => {
+          if (method === 'textDocument/didOpen') {
+            setTimeout(() => {
+              (client as any).diagnosticsStore.set(params?.textDocument?.uri, [
+                {
+                  message: 'push diagnostic',
+                  range: {
+                    start: { line: 0, character: 0 },
+                    end: { line: 0, character: 1 },
+                  },
+                  severity: 2,
+                },
+              ]);
+            }, 10);
+          }
+        },
+      ),
+      sendRequest,
+    };
+
+    try {
+      const result = await client.diagnostics(tempFile);
+      expect(result.items).toHaveLength(1);
+      expect(result.items[0]?.message).toBe('push diagnostic');
+      expect(sendRequest).not.toHaveBeenCalledWith(
+        'textDocument/diagnostic',
+        expect.anything(),
+      );
+    } finally {
+      LSP_TIMEOUTS.request = originalRequestTimeout;
+    }
+  });
+
+  test('diagnostics handles pull full reports and stores resultId', async () => {
+    const client = new LSPClient(tempDir, {
+      id: 'test',
+      command: ['/bin/true'],
+      extensions: ['.ts'],
+    });
+
+    (client as any).supportsPullDiagnostics = true;
+    (client as any).connection = {
+      sendNotification: mock(),
+      sendRequest: mock((method: string) => {
+        if (method === 'textDocument/diagnostic') {
+          return Promise.resolve({
+            kind: 'full',
+            items: [
+              {
+                message: 'pull diagnostic',
+                range: {
+                  start: { line: 0, character: 0 },
+                  end: { line: 0, character: 1 },
+                },
+                severity: 1,
+              },
+            ],
+            resultId: 'r1',
+          });
+        }
+        return Promise.resolve({});
+      }),
+    };
+
+    const result = await client.diagnostics(tempFile);
+    expect(result.items).toHaveLength(1);
+    expect(result.items[0]?.message).toBe('pull diagnostic');
+    expect(
+      (client as any).diagnosticResultIds.get(pathToFileURL(tempFile).href),
+    ).toBe('r1');
+  });
+
+  test('diagnostics handles pull unchanged reports using cached diagnostics', async () => {
+    const client = new LSPClient(tempDir, {
+      id: 'test',
+      command: ['/bin/true'],
+      extensions: ['.ts'],
+    });
+
+    const uri = pathToFileURL(tempFile).href;
+    (client as any).supportsPullDiagnostics = true;
+    (client as any).diagnosticsStore.set(uri, [
+      {
+        message: 'cached from previous full',
+        range: {
+          start: { line: 0, character: 0 },
+          end: { line: 0, character: 1 },
+        },
+        severity: 2,
+      },
+    ]);
+    (client as any).diagnosticResultIds.set(uri, 'r1');
+    (client as any).connection = {
+      sendNotification: mock(),
+      sendRequest: mock((method: string) => {
+        if (method === 'textDocument/diagnostic') {
+          return Promise.resolve({ kind: 'unchanged', resultId: 'r2' });
+        }
+        return Promise.resolve({});
+      }),
+    };
+
+    const result = await client.diagnostics(tempFile);
+    expect(result.items).toHaveLength(1);
+    expect(result.items[0]?.message).toBe('cached from previous full');
+    expect((client as any).diagnosticResultIds.get(uri)).toBe('r2');
+  });
+
+  test('external file change invalidates cached diagnostics and waits for fresh publish', async () => {
+    const tempDir = join(process.cwd(), '.tmp-lsp-tests');
+    const tempFile = join(tempDir, 'index.ts');
+    mkdirSync(tempDir, { recursive: true });
+    writeFileSync(tempFile, 'const a = 1;\n');
+
+    const client = new LSPClient(tempDir, {
+      id: 'test',
+      command: ['/bin/true'],
+      extensions: ['.ts'],
+    });
+
+    const sendNotification = mock((method: string, params?: any) => {
+      if (method === 'textDocument/didOpen') {
+        // initial publish
+        (client as any).diagnosticsStore.set(params.textDocument.uri, [
+          {
+            message: 'initial',
+            range: {
+              start: { line: 0, character: 0 },
+              end: { line: 0, character: 1 },
+            },
+            severity: 2,
+          },
+        ]);
+      }
+    });
+
+    (client as any).connection = {
+      sendNotification,
+      sendRequest: mock(() => Promise.resolve({})),
+    };
+
+    try {
+      // first diagnostics returns initial cached one
+      (client as any).supportsPullDiagnostics = false;
+      const first = await client.diagnostics(tempFile);
+      expect(first.items[0].message).toBe('initial');
+
+      // mutate the file externally
+      writeFileSync(tempFile, 'const b = 2;\n');
+
+      // simulate server publishing new diagnostics after change
+      setTimeout(() => {
+        const uri = pathToFileURL(resolve(tempFile)).href;
+        (client as any).diagnosticsStore.set(uri, [
+          {
+            message: 'after',
+            range: {
+              start: { line: 0, character: 0 },
+              end: { line: 0, character: 1 },
+            },
+            severity: 2,
+          },
+        ]);
+      }, 20);
+
+      const second = await client.diagnostics(tempFile);
+      expect(second.items[0].message).toBe('after');
+    } finally {
+      rmSync(tempDir, { recursive: true, force: true });
+    }
+  });
+});
+
+describe('LSPClient initialize', () => {
+  test('advertises textDocument diagnostic capability', async () => {
+    const client = new LSPClient('/root', {
+      id: 'test',
+      command: ['/bin/true'],
+      extensions: ['.ts'],
+    });
+
+    const sendRequest = mock(() => Promise.resolve({}));
+    const sendNotification = mock();
+
+    (client as any).connection = {
+      sendRequest,
+      sendNotification,
+    };
+
+    await client.initialize();
+
+    expect((client as any).supportsPullDiagnostics).toBe(false);
+
+    expect(sendRequest).toHaveBeenCalledWith(
+      'initialize',
+      expect.objectContaining({
+        capabilities: expect.objectContaining({
+          textDocument: expect.objectContaining({
+            diagnostic: {},
+          }),
+        }),
+      }),
+    );
+    expect(sendNotification).toHaveBeenCalledWith('initialized', {});
+  });
+
+  test('stores negotiated pull diagnostics support from initialize response', async () => {
+    const client = new LSPClient('/root', {
+      id: 'test',
+      command: ['/bin/true'],
+      extensions: ['.ts'],
+    });
+
+    (client as any).connection = {
+      sendRequest: mock(() =>
+        Promise.resolve({
+          capabilities: {
+            diagnosticProvider: {
+              interFileDependencies: false,
+              workspaceDiagnostics: false,
+            },
+          },
+        }),
+      ),
+      sendNotification: mock(),
+    };
+
+    await client.initialize();
+
+    expect((client as any).supportsPullDiagnostics).toBe(true);
+    expect((client as any).diagnosticProvider).toEqual({
+      interFileDependencies: false,
+      workspaceDiagnostics: false,
+    });
+  });
+
+  test('returns null for unknown workspace configuration sections', () => {
+    expect(
+      getWorkspaceConfiguration([
+        { section: 'unknown' },
+        { section: 'json' },
+        {},
+      ]),
+    ).toEqual([null, { validate: { enable: true } }, null]);
+  });
+
+  test('summarizes pull-only diagnostics capabilities', () => {
+    expect(
+      getDiagnosticsCapabilitySummary({
+        diagnosticProvider: {
+          interFileDependencies: true,
+          workspaceDiagnostics: true,
+        },
+      }),
+    ).toEqual({
+      availableModes: ['pull', 'pull/full', 'pull/unchanged', 'workspace-pull'],
+      preferredMode: 'pull',
+      inferredTransport: 'pull',
+      pull: true,
+      pushObserved: false,
+      pullResultTracking: true,
+      workspaceDiagnostics: true,
+      interFileDependencies: true,
+      workspaceConfiguration: false,
+    });
+  });
+
+  test('summarizes hybrid diagnostics capabilities when push is observed', () => {
+    expect(
+      getDiagnosticsCapabilitySummary({
+        diagnosticProvider: {
+          interFileDependencies: false,
+          workspaceDiagnostics: false,
+        },
+        publishDiagnosticsObserved: true,
+        workspaceConfigurationRequested: true,
+      }),
+    ).toEqual({
+      availableModes: ['pull', 'pull/full', 'pull/unchanged', 'push'],
+      preferredMode: 'pull',
+      inferredTransport: 'hybrid',
+      pull: true,
+      pushObserved: true,
+      pullResultTracking: true,
+      workspaceDiagnostics: false,
+      interFileDependencies: false,
+      workspaceConfiguration: true,
+    });
+  });
+});

src/tools/lsp/client.ts

@@ -12,8 +12,129 @@ import {
   StreamMessageWriter,
 } from 'vscode-jsonrpc/node';
 import { log } from '../../utils/logger';
-import { getLanguageId } from './config';
-import type { Diagnostic, ResolvedServer } from './types';
+import { getLanguageId, resolveServerCommand } from './config';
+import type {
+  Diagnostic,
+  DocumentDiagnosticReport,
+  ResolvedServer,
+} from './types';
+
+const START_TIMEOUT_MS = 5_000;
+const REQUEST_TIMEOUT_MS = 5_000;
+const OPEN_FILE_DELAY_MS = 250;
+const INITIALIZE_DELAY_MS = 100;
+const DIAGNOSTIC_SETTLE_DELAY_MS = 250;
+
+export const LSP_TIMEOUTS = {
+  start: START_TIMEOUT_MS,
+  request: REQUEST_TIMEOUT_MS,
+  openFileDelay: OPEN_FILE_DELAY_MS,
+  initializeDelay: INITIALIZE_DELAY_MS,
+  diagnosticSettleDelay: DIAGNOSTIC_SETTLE_DELAY_MS,
+};
+
+interface DiagnosticProviderCapabilities {
+  identifier?: string;
+  interFileDependencies?: boolean;
+  workspaceDiagnostics?: boolean;
+}
+
+export function getDiagnosticsCapabilitySummary({
+  diagnosticProvider,
+  publishDiagnosticsObserved = false,
+  workspaceConfigurationRequested = false,
+}: {
+  diagnosticProvider?: DiagnosticProviderCapabilities | null;
+  publishDiagnosticsObserved?: boolean;
+  workspaceConfigurationRequested?: boolean;
+}): {
+  availableModes: string[];
+  preferredMode: 'push' | 'pull';
+  inferredTransport: 'push' | 'pull' | 'hybrid';
+  pull: boolean;
+  pushObserved: boolean;
+  pullResultTracking: boolean;
+  workspaceDiagnostics: boolean;
+  interFileDependencies: boolean;
+  workspaceConfiguration: boolean;
+} {
+  const pull = Boolean(diagnosticProvider);
+  const workspaceDiagnostics = Boolean(
+    diagnosticProvider?.workspaceDiagnostics,
+  );
+  const interFileDependencies = Boolean(
+    diagnosticProvider?.interFileDependencies,
+  );
+
+  const availableModes = [
+    ...(pull ? ['pull', 'pull/full', 'pull/unchanged'] : ['push']),
+    ...(workspaceDiagnostics ? ['workspace-pull'] : []),
+    ...(publishDiagnosticsObserved ? ['push'] : []),
+  ];
+
+  return {
+    availableModes: Array.from(new Set(availableModes)),
+    preferredMode: pull ? 'pull' : 'push',
+    inferredTransport:
+      pull && publishDiagnosticsObserved ? 'hybrid' : pull ? 'pull' : 'push',
+    pull,
+    pushObserved: publishDiagnosticsObserved,
+    pullResultTracking: pull,
+    workspaceDiagnostics,
+    interFileDependencies,
+    workspaceConfiguration: workspaceConfigurationRequested,
+  };
+}
+
+function withTimeout<T>(
+  promise: Promise<T>,
+  ms: number,
+  label: string,
+  onTimeout?: () => Promise<void> | void,
+): Promise<T> {
+  return new Promise((resolve, reject) => {
+    let settled = false;
+    const timer = setTimeout(() => {
+      if (settled) {
+        return;
+      }
+      settled = true;
+      void Promise.resolve(onTimeout?.()).catch(() => {});
+      reject(new Error(`${label} timeout after ${ms}ms`));
+    }, ms);
+
+    promise.then(
+      (value) => {
+        if (settled) {
+          return;
+        }
+        settled = true;
+        clearTimeout(timer);
+        resolve(value);
+      },
+      (error) => {
+        if (settled) {
+          return;
+        }
+        settled = true;
+        clearTimeout(timer);
+        reject(error);
+      },
+    );
+  });
+}
+
+export function getWorkspaceConfiguration(
+  items: Array<{ section?: string } | undefined>,
+): Array<unknown> {
+  return items.map((item) => {
+    if (item?.section === 'json') {
+      return { validate: { enable: true } };
+    }
+
+    return null;
+  });
+}
 
 interface ManagedClient {
   client: LSPClient;
@@ -204,6 +325,15 @@ export class LSPClient {
   private stderrBuffer: string[] = [];
   private processExited = false;
   private diagnosticsStore = new Map<string, Diagnostic[]>();
+  private diagnosticResultIds = new Map<string, string>();
+  private documents = new Map<
+    string,
+    { version: number; text: string; languageId: string }
+  >();
+  private diagnosticProvider: DiagnosticProviderCapabilities | null = null;
+  private publishDiagnosticsObserved = false;
+  private supportsPullDiagnostics = false;
+  private workspaceConfigurationRequested = false;
 
   constructor(
     private root: string,
@@ -211,13 +341,20 @@ export class LSPClient {
   ) {}
 
   async start(): Promise<void> {
+    const command = resolveServerCommand(this.server.command, this.root);
+    if (!command) {
+      throw new Error(
+        `Failed to resolve LSP server command: ${this.server.command.join(' ')}`,
+      );
+    }
+
     log('[lsp] LSPClient.start: spawning server', {
       server: this.server.id,
-      command: this.server.command.join(' '),
+      command: command.join(' '),
       root: this.root,
     });
 
-    this.proc = spawn(this.server.command, {
+    this.proc = spawn(command, {
       stdin: 'pipe',
       stdout: 'pipe',
       stderr: 'pipe',
@@ -281,6 +418,18 @@ export class LSPClient {
     this.connection.onNotification(
       'textDocument/publishDiagnostics',
       (params: { uri?: string; diagnostics?: Diagnostic[] }) => {
+        if (!this.publishDiagnosticsObserved) {
+          this.publishDiagnosticsObserved = true;
+          log('[lsp] diagnostics capabilities: publishDiagnostics observed', {
+            server: this.server.id,
+            ...getDiagnosticsCapabilitySummary({
+              diagnosticProvider: this.diagnosticProvider,
+              publishDiagnosticsObserved: this.publishDiagnosticsObserved,
+              workspaceConfigurationRequested:
+                this.workspaceConfigurationRequested,
+            }),
+          });
+        }
         if (params.uri) {
           this.diagnosticsStore.set(params.uri, params.diagnostics ?? []);
         }
@@ -290,13 +439,29 @@ export class LSPClient {
     this.connection.onRequest(
       'workspace/configuration',
       (params: { items?: unknown[] }) => {
-        const items = params.items ?? [];
-        return items.map((item: unknown) => {
-          const configItem = item as { section?: string };
-          if (configItem.section === 'json')
-            return { validate: { enable: true } };
-          return {};
-        });
+        if (!this.workspaceConfigurationRequested) {
+          this.workspaceConfigurationRequested = true;
+          log(
+            '[lsp] diagnostics capabilities: workspace configuration requested',
+            {
+              server: this.server.id,
+              sections: (params.items ?? []).map((item) =>
+                item && typeof item === 'object' && 'section' in item
+                  ? ((item as { section?: string }).section ?? null)
+                  : null,
+              ),
+              ...getDiagnosticsCapabilitySummary({
+                diagnosticProvider: this.diagnosticProvider,
+                publishDiagnosticsObserved: this.publishDiagnosticsObserved,
+                workspaceConfigurationRequested:
+                  this.workspaceConfigurationRequested,
+              }),
+            },
+          );
+        }
+        return getWorkspaceConfiguration(
+          (params.items ?? []) as Array<{ section?: string } | undefined>,
+        );
       },
     );
 
@@ -356,67 +521,143 @@ export class LSPClient {
     });
 
     const rootUri = pathToFileURL(this.root).href;
-    await this.connection.sendRequest('initialize', {
-      processId: process.pid,
-      rootUri,
-      rootPath: this.root,
-      workspaceFolders: [{ uri: rootUri, name: 'workspace' }],
-      capabilities: {
-        textDocument: {
-          hover: { contentFormat: ['markdown', 'plaintext'] },
-          definition: { linkSupport: true },
-          references: {},
-          documentSymbol: { hierarchicalDocumentSymbolSupport: true },
-          publishDiagnostics: {},
-          rename: {
-            prepareSupport: true,
-            prepareSupportDefaultBehavior: 1,
-            honorsChangeAnnotations: true,
+    const result = await withTimeout(
+      this.connection.sendRequest('initialize', {
+        processId: process.pid,
+        rootUri,
+        rootPath: this.root,
+        workspaceFolders: [{ uri: rootUri, name: 'workspace' }],
+        capabilities: {
+          textDocument: {
+            diagnostic: {},
+            hover: { contentFormat: ['markdown', 'plaintext'] },
+            definition: { linkSupport: true },
+            references: {},
+            documentSymbol: { hierarchicalDocumentSymbolSupport: true },
+            publishDiagnostics: {},
+            rename: {
+              prepareSupport: true,
+              prepareSupportDefaultBehavior: 1,
+              honorsChangeAnnotations: true,
+            },
+          },
+          workspace: {
+            symbol: {},
+            workspaceFolders: true,
+            configuration: true,
+            applyEdit: true,
+            workspaceEdit: { documentChanges: true },
           },
         },
-        workspace: {
-          symbol: {},
-          workspaceFolders: true,
-          configuration: true,
-          applyEdit: true,
-          workspaceEdit: { documentChanges: true },
-        },
-      },
-      ...this.server.initialization,
+        ...this.server.initialization,
+      }),
+      LSP_TIMEOUTS.request,
+      `LSP initialize (${this.server.id})`,
+    );
+
+    const capabilities =
+      result &&
+      typeof result === 'object' &&
+      'capabilities' in result &&
+      result.capabilities &&
+      typeof result.capabilities === 'object'
+        ? result.capabilities
+        : undefined;
+
+    this.diagnosticProvider =
+      capabilities && 'diagnosticProvider' in capabilities
+        ? (capabilities.diagnosticProvider as DiagnosticProviderCapabilities)
+        : null;
+    this.supportsPullDiagnostics = Boolean(this.diagnosticProvider);
+
+    log('[lsp] diagnostics capabilities negotiated', {
+      server: this.server.id,
+      diagnosticProvider: this.diagnosticProvider,
+      ...getDiagnosticsCapabilitySummary({
+        diagnosticProvider: this.diagnosticProvider,
+        publishDiagnosticsObserved: this.publishDiagnosticsObserved,
+        workspaceConfigurationRequested: this.workspaceConfigurationRequested,
+      }),
     });
-    this.connection.sendNotification('initialized');
-    await new Promise((r) => setTimeout(r, 300));
+
+    this.connection.sendNotification('initialized', {});
+    await new Promise((r) => setTimeout(r, LSP_TIMEOUTS.initializeDelay));
     log('[lsp] LSPClient.initialize: complete', { server: this.server.id });
   }
 
+  private async waitForPublishedDiagnostics(
+    uri: string,
+    timeoutMs = LSP_TIMEOUTS.request,
+  ): Promise<Diagnostic[] | undefined> {
+    const cachedDiagnostics = this.diagnosticsStore.get(uri);
+    if (cachedDiagnostics) {
+      return cachedDiagnostics;
+    }
+
+    const startedAt = Date.now();
+    while (Date.now() - startedAt < timeoutMs) {
+      await new Promise((r) => setTimeout(r, 100));
+      const diagnostics = this.diagnosticsStore.get(uri);
+      if (diagnostics) {
+        return diagnostics;
+      }
+    }
+
+    return this.diagnosticsStore.get(uri);
+  }
+
   async openFile(filePath: string): Promise<void> {
+    await this.ensureDocumentSynced(filePath);
+  }
+
+  private async ensureDocumentSynced(filePath: string): Promise<void> {
     const absPath = resolve(filePath);
-    if (this.openedFiles.has(absPath)) {
-      log('[lsp] openFile: already open, skipping', { filePath: absPath });
-      return;
-    }
+    const uri = pathToFileURL(absPath).href;
 
     const text = readFileSync(absPath, 'utf-8');
     const ext = extname(absPath);
     const languageId = getLanguageId(ext);
 
-    log('[lsp] openFile: opening document', {
-      filePath: absPath,
-      languageId,
-      size: text.length,
-    });
+    const existing = this.documents.get(uri);
 
-    this.connection?.sendNotification('textDocument/didOpen', {
-      textDocument: {
-        uri: pathToFileURL(absPath).href,
+    if (!existing) {
+      log('[lsp] ensureDocumentSynced: didOpen', {
+        filePath: absPath,
         languageId,
-        version: 1,
-        text,
-      },
-    });
-    this.openedFiles.add(absPath);
+        size: text.length,
+      });
+      this.connection?.sendNotification('textDocument/didOpen', {
+        textDocument: { uri, languageId, version: 1, text },
+      });
+      this.documents.set(uri, { version: 1, text, languageId });
+      this.openedFiles.add(absPath);
+      // allow server to settle
+      await new Promise((r) => setTimeout(r, LSP_TIMEOUTS.openFileDelay));
+      return;
+    }
 
-    await new Promise((r) => setTimeout(r, 1000));
+    if (existing.text !== text) {
+      const newVersion = existing.version + 1;
+      log('[lsp] ensureDocumentSynced: didChange', {
+        filePath: absPath,
+        languageId,
+        oldVersion: existing.version,
+        newVersion,
+        size: text.length,
+      });
+      this.connection?.sendNotification('textDocument/didChange', {
+        textDocument: { uri, version: newVersion },
+        contentChanges: [{ text }],
+      });
+      this.documents.set(uri, { version: newVersion, text, languageId });
+      // Invalidate cached publishDiagnostics so we wait for fresh results
+      this.diagnosticsStore.delete(uri);
+      this.diagnosticResultIds.delete(uri);
+      // allow server to settle after change
+      await new Promise((r) => setTimeout(r, LSP_TIMEOUTS.openFileDelay));
+    } else {
+      log('[lsp] ensureDocumentSynced: already synced', { filePath: absPath });
+    }
   }
 
   async definition(
@@ -426,10 +667,16 @@ export class LSPClient {
   ): Promise<unknown> {
     const absPath = resolve(filePath);
     await this.openFile(absPath);
-    return this.connection?.sendRequest('textDocument/definition', {
-      textDocument: { uri: pathToFileURL(absPath).href },
-      position: { line: line - 1, character },
-    });
+    return this.connection
+      ? withTimeout(
+          this.connection.sendRequest('textDocument/definition', {
+            textDocument: { uri: pathToFileURL(absPath).href },
+            position: { line: line - 1, character },
+          }),
+          LSP_TIMEOUTS.request,
+          `LSP definition (${this.server.id})`,
+        )
+      : undefined;
   }
 
   async references(
@@ -440,32 +687,88 @@ export class LSPClient {
   ): Promise<unknown> {
     const absPath = resolve(filePath);
     await this.openFile(absPath);
-    return this.connection?.sendRequest('textDocument/references', {
-      textDocument: { uri: pathToFileURL(absPath).href },
-      position: { line: line - 1, character },
-      context: { includeDeclaration },
-    });
+    return this.connection
+      ? withTimeout(
+          this.connection.sendRequest('textDocument/references', {
+            textDocument: { uri: pathToFileURL(absPath).href },
+            position: { line: line - 1, character },
+            context: { includeDeclaration },
+          }),
+          LSP_TIMEOUTS.request,
+          `LSP references (${this.server.id})`,
+        )
+      : undefined;
   }
 
   async diagnostics(filePath: string): Promise<{ items: Diagnostic[] }> {
     const absPath = resolve(filePath);
     const uri = pathToFileURL(absPath).href;
     await this.openFile(absPath);
-    await new Promise((r) => setTimeout(r, 500));
+    await new Promise((r) => setTimeout(r, LSP_TIMEOUTS.diagnosticSettleDelay));
 
-    try {
-      const result = await this.connection?.sendRequest(
-        'textDocument/diagnostic',
-        {
-          textDocument: { uri },
-        },
-      );
-      if (result && typeof result === 'object' && 'items' in result) {
-        return result as { items: Diagnostic[] };
+    log('[lsp] diagnostics mode selected', {
+      server: this.server.id,
+      filePath: absPath,
+      activeMode: this.supportsPullDiagnostics ? 'pull' : 'push',
+      ...getDiagnosticsCapabilitySummary({
+        diagnosticProvider: this.diagnosticProvider,
+        publishDiagnosticsObserved: this.publishDiagnosticsObserved,
+        workspaceConfigurationRequested: this.workspaceConfigurationRequested,
+      }),
+    });
+
+    if (this.supportsPullDiagnostics) {
+      try {
+        const result = this.connection
+          ? await withTimeout(
+              this.connection.sendRequest('textDocument/diagnostic', {
+                textDocument: { uri },
+                previousResultId: this.diagnosticResultIds.get(uri),
+              }),
+              LSP_TIMEOUTS.request,
+              `LSP diagnostics (${this.server.id})`,
+            )
+          : undefined;
+
+        const report = result as DocumentDiagnosticReport | undefined;
+        if (report?.kind === 'full') {
+          if (report.resultId) {
+            this.diagnosticResultIds.set(uri, report.resultId);
+          } else {
+            this.diagnosticResultIds.delete(uri);
+          }
+          this.diagnosticsStore.set(uri, report.items);
+          return { items: report.items };
+        }
+
+        if (report?.kind === 'unchanged') {
+          if (report.resultId) {
+            this.diagnosticResultIds.set(uri, report.resultId);
+          }
+          return { items: this.diagnosticsStore.get(uri) ?? [] };
+        }
+
+        if (result && typeof result === 'object' && 'items' in result) {
+          const legacyResult = result as { items: Diagnostic[] };
+          this.diagnosticsStore.set(uri, legacyResult.items);
+          return legacyResult;
+        }
+      } catch (error) {
+        log('[lsp] diagnostics: falling back to cached publishDiagnostics', {
+          server: this.server.id,
+          error: String(error),
+        });
       }
-    } catch {}
+    }
 
-    return { items: this.diagnosticsStore.get(uri) ?? [] };
+    const cachedDiagnostics = await this.waitForPublishedDiagnostics(uri);
+    if (cachedDiagnostics) {
+      return { items: cachedDiagnostics };
+    }
+
+    throw new Error(
+      `Unable to retrieve diagnostics from ${this.server.id}: request timed out or is unsupported.`,
+    );
   }
 
   async rename(
@@ -476,11 +779,17 @@ export class LSPClient {
   ): Promise<unknown> {
     const absPath = resolve(filePath);
     await this.openFile(absPath);
-    return this.connection?.sendRequest('textDocument/rename', {
-      textDocument: { uri: pathToFileURL(absPath).href },
-      position: { line: line - 1, character },
-      newName,
-    });
+    return this.connection
+      ? withTimeout(
+          this.connection.sendRequest('textDocument/rename', {
+            textDocument: { uri: pathToFileURL(absPath).href },
+            position: { line: line - 1, character },
+            newName,
+          }),
+          LSP_TIMEOUTS.request,
+          `LSP rename (${this.server.id})`,
+        )
+      : undefined;
   }
 
   isAlive(): boolean {
@@ -493,7 +802,11 @@ export class LSPClient {
     log('[lsp] LSPClient.stop: stopping', { server: this.server.id });
     try {
       if (this.connection) {
-        await this.connection.sendRequest('shutdown');
+        await withTimeout(
+          this.connection.sendRequest('shutdown'),
+          1_000,
+          `LSP shutdown (${this.server.id})`,
+        );
         this.connection.sendNotification('exit');
         this.connection.dispose();
       }
@@ -502,7 +815,13 @@ export class LSPClient {
     this.proc = null;
     this.connection = null;
     this.processExited = true;
+    this.diagnosticProvider = null;
+    this.publishDiagnosticsObserved = false;
+    this.supportsPullDiagnostics = false;
+    this.workspaceConfigurationRequested = false;
     this.diagnosticsStore.clear();
+    this.diagnosticResultIds.clear();
+    this.documents.clear();
     log('[lsp] LSPClient.stop: complete', { server: this.server.id });
   }
 }

src/tools/lsp/config.test.ts

@@ -89,9 +89,33 @@ describe('config', () => {
   });
 
   describe('findServerForExtension', () => {
-    test('should return found for .ts extension if installed', () => {
-      (existsSync as any).mockReturnValue(true);
-      const result = findServerForExtension('.ts');
+    test('should skip deno for .ts when project is not a deno workspace', () => {
+      whichSyncMock.mockImplementation((cmd: string) =>
+        cmd === 'typescript-language-server'
+          ? join('/usr/bin', 'typescript-language-server')
+          : null,
+      );
+      (existsSync as any).mockImplementation((path: string) =>
+        path.includes('bun.lock'),
+      );
+      const result = findServerForExtension(
+        '.ts',
+        '/workspace/project/src/index.ts',
+      );
+      expect(result.status).toBe('found');
+      if (result.status === 'found') {
+        expect(result.server.id).toBe('typescript');
+      }
+    });
+
+    test('should prefer deno for .ts in a deno workspace', () => {
+      whichSyncMock.mockImplementation((cmd: string) =>
+        cmd === 'deno' ? join('/usr/bin', 'deno') : null,
+      );
+      (existsSync as any).mockImplementation((path: string) =>
+        path.includes('deno.json'),
+      );
+      const result = findServerForExtension('.ts', '/workspace/app/src/mod.ts');
       expect(result.status).toBe('found');
       if (result.status === 'found') {
         expect(result.server.id).toBe('deno');
@@ -99,7 +123,9 @@ describe('config', () => {
     });
 
     test('should return found for .py extension if installed (prefers ty)', () => {
-      (existsSync as any).mockReturnValue(true);
+      whichSyncMock.mockImplementation((cmd: string) =>
+        cmd === 'ty' ? join('/usr/bin', 'ty') : null,
+      );
       const result = findServerForExtension('.py');
       expect(result.status).toBe('found');
       if (result.status === 'found') {
@@ -112,13 +138,39 @@ describe('config', () => {
       expect(result.status).toBe('not_configured');
     });
 
-    test('should return not_installed if server not in PATH', () => {
-      (existsSync as any).mockReturnValue(false);
-      const result = findServerForExtension('.ts');
+    test('should continue to later matching servers when earlier ones are unavailable', () => {
+      whichSyncMock.mockImplementation((cmd: string) =>
+        cmd === 'typescript-language-server'
+          ? join('/usr/bin', 'typescript-language-server')
+          : null,
+      );
+      (existsSync as any).mockImplementation((path: string) =>
+        path.includes('bun.lock'),
+      );
+
+      const result = findServerForExtension(
+        '.ts',
+        '/workspace/project/src/index.ts',
+      );
+
+      expect(result.status).toBe('found');
+      if (result.status === 'found') {
+        expect(result.server.id).toBe('typescript');
+      }
+    });
+
+    test('should return first applicable not_installed server if no match is launchable', () => {
+      (existsSync as any).mockImplementation((path: string) =>
+        path.includes('bun.lock'),
+      );
+      const result = findServerForExtension(
+        '.ts',
+        '/workspace/project/src/index.ts',
+      );
       expect(result.status).toBe('not_installed');
       if (result.status === 'not_installed') {
-        expect(result.server.id).toBe('deno');
-        expect(result.installHint).toContain('Install Deno');
+        expect(result.server.id).toBe('typescript');
+        expect(result.installHint).toContain('typescript-language-server');
       }
     });
   });

src/tools/lsp/config.ts

@@ -3,8 +3,9 @@
 
 import { existsSync } from 'node:fs';
 import { homedir } from 'node:os';
-import { join } from 'node:path';
+import { dirname, join, resolve } from 'node:path';
 import whichSync from 'which';
+import { log } from '../../utils';
 import { getAllUserLspConfigs, hasUserLspConfig } from './config-store';
 import {
   BUILTIN_SERVERS,
@@ -87,25 +88,76 @@ function buildMergedServers(): Map<string, MergedServerConfig> {
   return servers;
 }
 
-export function findServerForExtension(ext: string): ServerLookupResult {
-  const servers = buildMergedServers();
-
-  for (const [, config] of servers) {
-    if (config.extensions.includes(ext)) {
-      const server: ResolvedServer = {
-        id: config.id,
-        command: config.command,
-        extensions: config.extensions,
-        root: config.root,
-        env: config.env,
-        initialization: config.initialization,
-      };
+function getServerWorkspace(
+  config: MergedServerConfig,
+  filePath?: string,
+): string | undefined {
+  if (!filePath) {
+    return undefined;
+  }
 
-      if (isServerInstalled(config.command)) {
-        return { status: 'found', server };
-      }
+  if (!config.root) {
+    return dirname(resolve(filePath));
+  }
+
+  return config.root(filePath);
+}
+
+function shouldSkipServer(
+  config: MergedServerConfig,
+  filePath?: string,
+): boolean {
+  if (!filePath) {
+    return false;
+  }
 
-      return {
+  return (
+    config.id === 'deno' && getServerWorkspace(config, filePath) === undefined
+  );
+}
+
+function toResolvedServer(
+  config: MergedServerConfig,
+  command?: string[],
+): ResolvedServer {
+  return {
+    id: config.id,
+    command: command ?? config.command,
+    extensions: config.extensions,
+    root: config.root,
+    env: config.env,
+    initialization: config.initialization,
+  };
+}
+
+function findInstalledServer(
+  configs: MergedServerConfig[],
+  filePath?: string,
+): ServerLookupResult | undefined {
+  let firstNotInstalled: Extract<
+    ServerLookupResult,
+    { status: 'not_installed' }
+  > | null = null;
+
+  for (const config of configs) {
+    const workspace = getServerWorkspace(config, filePath);
+    const resolvedCommand = resolveServerCommand(
+      config.command,
+      workspace ?? (filePath ? dirname(resolve(filePath)) : undefined),
+    );
+    const server = toResolvedServer(config, resolvedCommand ?? undefined);
+
+    log(
+      `[LSP] Considering server for ${config.extensions.join(', ')}: ${config.id} with command ${config.command.join(' ')}`,
+    );
+
+    if (resolvedCommand) {
+      return { status: 'found', server };
+    }
+
+    if (!firstNotInstalled) {
+      log(`[LSP] Server ${config.id} not found in PATH or local node_modules`);
+      firstNotInstalled = {
         status: 'not_installed',
         server,
         installHint:
@@ -115,6 +167,38 @@ export function findServerForExtension(ext: string): ServerLookupResult {
     }
   }
 
+  return firstNotInstalled ?? undefined;
+}
+
+export function findServerForExtension(
+  ext: string,
+  filePath?: string,
+): ServerLookupResult {
+  const servers = [...buildMergedServers().values()].filter((config) =>
+    config.extensions.includes(ext),
+  );
+
+  if (servers.length === 0) {
+    log(`[LSP] No server config found for ${ext}`);
+    return { status: 'not_configured', extension: ext };
+  }
+
+  const candidateServers = servers.filter(
+    (config) => !shouldSkipServer(config, filePath),
+  );
+
+  if (candidateServers.length === 0) {
+    log(`[LSP] No applicable server config found for ${ext} at ${filePath}`);
+    return { status: 'not_configured', extension: ext };
+  }
+
+  const result = findInstalledServer(candidateServers, filePath);
+
+  if (result) {
+    return result;
+  }
+
+  log(`[LSP] No applicable server config found for ${ext}`);
   return { status: 'not_configured', extension: ext };
 }
 
@@ -122,21 +206,21 @@ export function getLanguageId(ext: string): string {
   return LANGUAGE_EXTENSIONS[ext] || 'plaintext';
 }
 
-export function isServerInstalled(command: string[]): boolean {
-  if (command.length === 0) return false;
+export function resolveServerCommand(
+  command: string[],
+  cwd?: string,
+): string[] | null {
+  if (command.length === 0) return null;
 
-  const cmd = command[0];
+  const [cmd, ...args] = command;
 
-  // Absolute paths
   if (cmd.includes('/') || cmd.includes('\\')) {
-    return existsSync(cmd);
+    return existsSync(cmd) ? command : null;
   }
 
   const isWindows = process.platform === 'win32';
   const ext = isWindows ? '.exe' : '';
 
-  // Check PATH using which (mirrors core's approach)
-  // Include ~/.config/opencode/bin in the search path
   const opencodeBin = join(homedir(), '.config', 'opencode', 'bin');
   const searchPath =
     (process.env.PATH ?? '') + (isWindows ? ';' : ':') + opencodeBin;
@@ -148,15 +232,21 @@ export function isServerInstalled(command: string[]): boolean {
   });
 
   if (result !== null) {
-    return true;
+    return [result, ...args];
   }
 
-  // Check local node_modules (where npm/yarn/pnpm install binaries)
-  const cwd = process.cwd();
-  const localBin = join(cwd, 'node_modules', '.bin', cmd);
-  if (existsSync(localBin) || existsSync(localBin + ext)) {
-    return true;
+  const localBinRoot = cwd ?? process.cwd();
+  const localBin = join(localBinRoot, 'node_modules', '.bin', cmd);
+  if (existsSync(localBin)) {
+    return [localBin, ...args];
+  }
+  if (existsSync(localBin + ext)) {
+    return [localBin + ext, ...args];
   }
 
-  return false;
+  return null;
+}
+
+export function isServerInstalled(command: string[]): boolean {
+  return resolveServerCommand(command) !== null;
 }

src/tools/lsp/types.ts

@@ -63,3 +63,18 @@ export type {
   SymbolInfo,
   DocumentSymbol,
 };
+
+export interface DocumentDiagnosticReportFull {
+  kind: 'full';
+  items: Diagnostic[];
+  resultId?: string;
+}
+
+export interface DocumentDiagnosticReportUnchanged {
+  kind: 'unchanged';
+  resultId?: string;
+}
+
+export type DocumentDiagnosticReport =
+  | DocumentDiagnosticReportFull
+  | DocumentDiagnosticReportUnchanged;

src/tools/lsp/utils.ts

@@ -106,7 +106,7 @@ export async function withLspClient<T>(
 ): Promise<T> {
   const absPath = resolve(filePath);
   const ext = extname(absPath);
-  const result = findServerForExtension(ext);
+  const result = findServerForExtension(ext, absPath);
 
   if (result.status !== 'found') {
     log('[lsp] withLspClient: server not found', {
@@ -121,6 +121,14 @@ export async function withLspClient<T>(
   // Fall back to file's directory if no root patterns match
   const root = findServerProjectRoot(absPath, server) ?? dirname(absPath);
 
+  log('[lsp] withLspClient: selected server', {
+    filePath: absPath,
+    extension: ext,
+    server: server.id,
+    command: server.command.join(' '),
+    root,
+  });
+
   log('[lsp] withLspClient: acquiring client', {
     filePath: absPath,
     server: server.id,