refactor(daemon): require explicit serverId on all daemon ops

Move all server matching logic to the extension/CLI side. The daemon no longer calls pickServer() — it receives an explicit serverId (or serverIds[] for diagnostics) and uses it directly for cache lookup and server spawning. Key changes: - request op requires serverId: string - diagnostics op requires serverIds: string[] — daemon fans out in parallel via Promise.allSettled and returns grouped map - formatDiagnostics() handles grouped results with per-server headers when multiple servers contribute (single-server omits header) - CLI picks servers locally before calling daemon helpers - New pickDiagnosticServers() in extension returns all available, non-disabled servers matching the file extension This makes multi-server diagnostics (e.g., typescript-language-server + oxlint) work naturally — the extension decides which servers to query, the daemon just executes.
2026-05-04 07:39:03 -04:00
parent d24e2e94f4
commit b9808a8b1f
5 changed files with 117 additions and 41 deletions
--- a/cli.ts
+++ b/cli.ts
@@ -2,7 +2,8 @@
 import * as path from "node:path";
 import { startClientForFile } from "./src/client.ts";
 import { isLspCommand, listCommands, runCommand } from "./src/commands.ts";
-import { pickServer } from "./src/root.ts";
+import { pickServer, isServerAvailable } from "./src/root.ts";
+import { servers } from "./server.ts";
 import {
  daemonDiagnostics,
  daemonRequest,
@@ -104,14 +105,20 @@ async function runViaDaemon(
  const filePath = path.resolve(fileArg);
  let result: unknown;
  if (cmdArg === "diagnostics") {
-    result = await daemonDiagnostics(filePath);
+    // Pick All Available Servers For Diagnostics
+    const ext = path.extname(filePath).replace(/^\./, "");
+    const serverIds = servers
+      .filter((s) => s.match.includes(ext) && isServerAvailable(s))
+      .map((s) => s.id);
+    result = await daemonDiagnostics(filePath, serverIds);
  } else if (cmdArg in methodMap) {
+    const server = pickServer(filePath);
    // References Default - Match commands.ts: include declaration unless
    // caller explicitly overrode `context`.
    if (cmdArg === "references" && !("context" in params)) {
      params.context = { includeDeclaration: true };
    }
-    result = await daemonRequest(filePath, methodMap[cmdArg], params);
+    result = await daemonRequest(filePath, server.id, methodMap[cmdArg], params);
  } else {
    process.stderr.write(
      `Unknown command "${cmdArg}". Known: ${listCommands().join(", ")}\n`,
--- a/index.ts
+++ b/index.ts
@@ -12,7 +12,7 @@ import {
  daemonRequest,
  daemonStatus,
 } from "./src/daemonClient.ts";
-import { pickServer } from "./src/root.ts";
+import { pickServer, isServerAvailable } from "./src/root.ts";
 import { servers } from "./server.ts";
 import {
  ServerNotFoundError,
@@ -201,17 +201,8 @@ function formatDocumentSymbols(result: unknown): string {
 }

 // Format Diagnostics - Turn diagnostic messages into readable text.
-function formatDiagnostics(result: unknown, limit = 20): string {
-  if (
-    !result ||
-    typeof result !== "object" ||
-    !("diagnostics" in result)
-  ) {
-    return "(no diagnostics)";
-  }
-  const diags = (result as any).diagnostics;
-  if (!Array.isArray(diags) || diags.length === 0) return "(no diagnostics)";
-
+// Format Single Server Diagnostics - Renders one server's diagnostics list.
+function formatServerDiagnostics(diags: any[], limit: number): string {
  const severityNames: Record<number, string> = {
    1: "Error",
    2: "Warning",
@@ -236,11 +227,45 @@ function formatDiagnostics(result: unknown, limit = 20): string {
    .join("\n");

  if (diags.length > limit) {
-    return `${formatted}\n\n... and ${diags.length - limit} more diagnostics (showing first ${limit})`;
+    return `${formatted}\n\n... and ${diags.length - limit} more (showing first ${limit})`;
  }
  return formatted;
 }

+// Format Diagnostics - Handles the grouped result map from the daemon:
+// { [serverId]: { uri, diagnostics[] } }. Single-server results omit the
+// header to avoid noise.
+function formatDiagnostics(result: unknown, limit = 20): string {
+  if (!result || typeof result !== "object") return "(no diagnostics)";
+
+  const grouped = result as Record<string, any>;
+  const serverIds = Object.keys(grouped);
+  if (serverIds.length === 0) return "(no diagnostics)";
+
+  // Collect Servers With Diagnostics
+  const sections: { id: string; diags: any[] }[] = [];
+  for (const id of serverIds) {
+    const entry = grouped[id];
+    const diags = entry?.diagnostics;
+    if (Array.isArray(diags) && diags.length > 0) {
+      sections.push({ id, diags });
+    }
+  }
+
+  if (sections.length === 0) return "(no diagnostics)";
+
+  // Single Server - Skip header for brevity.
+  if (sections.length === 1) {
+    return formatServerDiagnostics(sections[0].diags, limit);
+  }
+
+  // Multiple Servers - Group with headers.
+  const perServer = Math.max(5, Math.floor(limit / sections.length));
+  return sections
+    .map((s) => `## ${s.id}\n${formatServerDiagnostics(s.diags, perServer)}`)
+    .join("\n\n");
+}
+
 // Is Expected Error - Returns true if the error is an expected condition
 // (unsupported file type or missing server binary) that should be
 // suppressed rather than surfaced to the user.
@@ -281,7 +306,7 @@ async function runLsp(
        `LSP server "${server.id}" is disabled. Use /lsp-enable ${server.id} to re-enable.`,
      );
    }
-    return await daemonRequest(filePath, method, params);
+    return await daemonRequest(filePath, server.id, method, params);
  } catch (error) {
    if (isExpectedError(error)) {
      return undefined;
@@ -300,19 +325,26 @@ async function runLsp(
  }
 }

-// Run LSP Diagnostics - Diagnostics arrive as a notification, so the
-// daemon has a dedicated op that waits for the next publish. Expected
-// errors (unsupported file type, missing binary) are suppressed.
+// Pick Diagnostic Servers - Returns all available, non-disabled servers
+// matching the file's extension. Used for fan-out diagnostics.
+function pickDiagnosticServers(filePath: string): string[] {
+  const ext = path.extname(filePath).replace(/^\./, "");
+  return servers
+    .filter((s) => s.match.includes(ext) && isServerAvailable(s) && !disabledServers.has(s.id))
+    .map((s) => s.id);
+}
+
+// Run LSP Diagnostics - Fans out to all matching servers in a single
+// daemon call. Returns the grouped result map or undefined if no servers.
 async function runDiagnostics(filePath: string): Promise<unknown> {
  try {
-    return await daemonDiagnostics(filePath, 1500);
+    const serverIds = pickDiagnosticServers(filePath);
+    if (serverIds.length === 0) return undefined;
+    return await daemonDiagnostics(filePath, serverIds, 1500);
  } catch (error) {
    if (isExpectedError(error)) {
      return undefined;
    }
-    // Daemon-wrapped errors (plain Error with expected message) are also
-    // expected — the daemon catches pickServer() throws and returns them
-    // as string error messages.
    if (
      error instanceof Error &&
      (error.message.includes("No LSP server registered") ||
@@ -492,9 +524,12 @@ export default function (pi: ExtensionAPI) {

    try {
      const absolutePath = path.resolve(ctx.cwd, filePath);
-      // daemonDiagnostics triggers getOrCreateEntry + syncFile in the daemon.
-      // We don't await it — just fire and forget so the server starts warming up.
-      void daemonDiagnostics(absolutePath).catch(() => {});
+      // Warm Diagnostic Servers - Fire-and-forget so servers are ready by
+      // the time an LSP tool is called.
+      const serverIds = pickDiagnosticServers(absolutePath);
+      if (serverIds.length > 0) {
+        void daemonDiagnostics(absolutePath, serverIds).catch(() => {});
+      }
    } catch {
      // Silently ignore — unsupported file type, missing binary, etc.
    }
--- a/src/daemon.ts
+++ b/src/daemon.ts
@@ -6,8 +6,9 @@ import * as fs from "node:fs";
 import * as net from "node:net";
 import * as path from "node:path";
 import { LspClient } from "./client.ts";
-import { findRoot, pickServer, pathToUri } from "./root.ts";
+import { findRoot, pathToUri } from "./root.ts";
 import type { ServerConfig } from "./types.ts";
+import { servers } from "../server.ts";
 import {
  logPath,
  socketPath,
@@ -50,14 +51,25 @@ function log(...args: unknown[]) {
  );
 }

-// Get Or Create Entry - Looks up the cached client for a file, spawning a
-// fresh LspClient if needed. The returned entry is guaranteed to have its
-// `ready` promise resolved before the caller uses it.
+// Find Server By ID - Looks up a ServerConfig from the registry by ID.
+// Throws if the ID is not registered.
+function findServerById(serverId: string): ServerConfig {
+  const server = servers.find((s) => s.id === serverId);
+  if (!server) {
+    throw new Error(`Unknown server ID: "${serverId}". Registered: ${servers.map((s) => s.id).join(", ")}`);
+  }
+  return server;
+}
+
+// Get Or Create Entry - Looks up the cached client for a server+file,
+// spawning a fresh LspClient if needed. The returned entry is guaranteed
+// to have its `ready` promise resolved before the caller uses it.
 async function getOrCreateEntry(
  filePath: string,
+  serverId: string,
  launch: LaunchContext,
 ): Promise<ClientEntry> {
-  const server = pickServer(filePath);
+  const server = findServerById(serverId);
  const rootDir = findRoot(filePath, server.rootMarkers);
  const key = `${server.id}::${rootDir}`;
  const existing = entries.get(key);
@@ -161,7 +173,7 @@ async function handle(req: DaemonRequest): Promise<DaemonResponse> {
    switch (req.op) {
      case "request": {
        const filePath = path.resolve(req.file);
-        const entry = await getOrCreateEntry(filePath, req.launch);
+        const entry = await getOrCreateEntry(filePath, req.serverId, req.launch);
        const { uri } = await syncFile(entry, filePath);
        bumpIdle(entry);
        const result = await entry.client.sendRequest(
@@ -172,15 +184,27 @@ async function handle(req: DaemonRequest): Promise<DaemonResponse> {
      }
      case "diagnostics": {
        const filePath = path.resolve(req.file);
-        const entry = await getOrCreateEntry(filePath, req.launch);
-        const { uri, changed } = await syncFile(entry, filePath);
-        bumpIdle(entry);
-        if (changed) entry.client.clearDiagnostics(uri);
-        const result = await entry.client.waitForDiagnostics(
-          uri,
-          req.timeoutMs ?? 1500,
+        const timeoutMs = req.timeoutMs ?? 1500;
+
+        // Fan-Out - Run diagnostics against all requested servers in
+        // parallel. Individual failures are captured, not thrown.
+        const results: Record<string, unknown> = {};
+        const settled = await Promise.allSettled(
+          req.serverIds.map(async (serverId) => {
+            const entry = await getOrCreateEntry(filePath, serverId, req.launch);
+            const { uri, changed } = await syncFile(entry, filePath);
+            bumpIdle(entry);
+            if (changed) entry.client.clearDiagnostics(uri);
+            const diag = await entry.client.waitForDiagnostics(uri, timeoutMs);
+            return { serverId, diag };
+          }),
        );
-        return { id: req.id, ok: true, result };
+        for (const outcome of settled) {
+          if (outcome.status === "fulfilled") {
+            results[outcome.value.serverId] = outcome.value.diag;
+          }
+        }
+        return { id: req.id, ok: true, result: results };
      }
      case "status": {
        const result = {
--- a/src/daemonClient.ts
+++ b/src/daemonClient.ts
@@ -22,6 +22,7 @@ function unwrap(resp: DaemonResponse): unknown {
 // daemon injects textDocument.uri from `file`, so callers omit it.
 export async function daemonRequest(
  file: string,
+  serverId: string,
  method: string,
  params: Record<string, unknown>,
 ): Promise<unknown> {
@@ -29,6 +30,7 @@ export async function daemonRequest(
    await sendOnce({
      op: "request",
      file,
+      serverId,
      method,
      params,
      launch: buildLaunchContext(),
@@ -38,14 +40,18 @@ export async function daemonRequest(

 // Wait For Diagnostics - Diagnostics arrive as a notification, not a
 // response, so the daemon has a dedicated op that awaits the next publish.
+// Accepts an array of server IDs; daemon fans out in parallel and returns
+// a grouped map: { [serverId]: { uri, diagnostics[] } }.
 export async function daemonDiagnostics(
  file: string,
+  serverIds: string[],
  timeoutMs = 1500,
 ): Promise<unknown> {
  return unwrap(
    await sendOnce({
      op: "diagnostics",
      file,
+      serverIds,
      timeoutMs,
      launch: buildLaunchContext(),
    }),
--- a/src/daemonProtocol.ts
+++ b/src/daemonProtocol.ts
@@ -38,6 +38,7 @@ export type DaemonRequest =
      id: number;
      op: "request";
      file: string;
+      serverId: string;
      method: string;
      params: Record<string, unknown>;
      launch: LaunchContext;
@@ -46,6 +47,7 @@ export type DaemonRequest =
      id: number;
      op: "diagnostics";
      file: string;
+      serverIds: string[];
      timeoutMs?: number;
      launch: LaunchContext;
    }
@@ -57,6 +59,7 @@ export type DaemonRequestWithoutId =
  | {
      op: "request";
      file: string;
+      serverId: string;
      method: string;
      params: Record<string, unknown>;
      launch: LaunchContext;
@@ -64,6 +67,7 @@ export type DaemonRequestWithoutId =
  | {
      op: "diagnostics";
      file: string;
+      serverIds: string[];
      timeoutMs?: number;
      launch: LaunchContext;
    }