From 5b63a2b95cd77bc4ef8d4c14cdef68abf158430a Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Wed, 28 Oct 2026 15:06:45 +0800 Subject: [PATCH] fix(lsp): Drop language servers that leave the list from the status bar (#125) Removing a language server (disabled in settings via stop_unlisted, or a fallback detached when its preferred server comes back) only emitted a stopped state, and the status bar list only refreshed for unknown servers, so the removed server stayed until the project was reopened. Both paths now go through forget(), which drops the server and emits IdeEvent::LanguageServerRemoved; the frontend drops the entry and ignores stale lsp_servers results that still contain it. Stopped servers no longer count towards "+N". Claude-Session: https://claude.ai/code/session_01VUYsdXA2nkF5v6zsC93dpn --- crates/ide-core/src/event.rs | 5 ++++ crates/ide-lsp/src/lib.rs | 12 ++++++-- crates/ide-lsp/tests/mock_server.rs | 24 +++++++++++++-- src/bindings.ts | 5 ++++ src/features/code-insight/servers.test.ts | 36 +++++++++++++++++++++++ src/features/code-insight/servers.ts | 20 +++++++++++-- 6 files changed, 96 insertions(+), 6 deletions(-) diff --git a/crates/ide-core/src/event.rs b/crates/ide-core/src/event.rs index 9db7dfb0..bebd8422 100644 --- a/crates/ide-core/src/event.rs +++ b/crates/ide-core/src/event.rs @@ -67,6 +67,11 @@ pub enum IdeEvent { status: ServerStatus, message: Option, }, + /// A stopped language server left the project's list (disabled in settings, or a + /// fallback that a preferred server took over from again). + LanguageServerRemoved { + server: ServerId, + }, /// A language server asked clients to re-request one kind of data for its /// open documents (`workspace/inlayHint/refresh`, …), e.g. after indexing. LspRefresh { diff --git a/crates/ide-lsp/src/lib.rs b/crates/ide-lsp/src/lib.rs index d313d417..bde52c93 100644 --- a/crates/ide-lsp/src/lib.rs +++ b/crates/ide-lsp/src/lib.rs @@ -1931,7 +1931,7 @@ impl LspManager { tracing::info!("{}: shadowed by an enabled server, stopping", s.name); self.stop_instance(s, true).await; } - self.servers.lock().retain(|s| !detached.iter().any(|d| d.id == s.id)); + self.forget(&detached); } /// Stops and forgets the servers that none of their languages uses any more. @@ -1949,10 +1949,18 @@ impl LspManager { self.stop_instance(s, true).await; } let gone = |s: &Arc| unlisted.iter().any(|u| u.id == s.id); - self.servers.lock().retain(|s| !gone(s)); for d in self.docs.lock().values_mut() { d.servers.retain(|a| !gone(&a.server)); } + self.forget(&unlisted); + } + + /// Drops stopped servers from the list and tells the UI they are gone. + fn forget(&self, gone: &[Arc]) { + self.servers.lock().retain(|s| !gone.iter().any(|g| g.id == s.id)); + for s in gone { + self.sink.emit(ide_core::IdeEvent::LanguageServerRemoved { server: s.id }); + } } pub fn set_apply_edit_hook(&self, hook: Box bool + Send + Sync>) { diff --git a/crates/ide-lsp/tests/mock_server.rs b/crates/ide-lsp/tests/mock_server.rs index 4f0da19c..d782be7c 100644 --- a/crates/ide-lsp/tests/mock_server.rs +++ b/crates/ide-lsp/tests/mock_server.rs @@ -1,7 +1,7 @@ //! Protocol tests against in-memory mock language servers. use ide_core::event::ServerStatus; -use ide_core::{DocId, FileEdit, IdeEvent, LanguageId, Pos, Range, Severity, WorkspaceEdit}; +use ide_core::{DocId, FileEdit, IdeEvent, LanguageId, Pos, Range, ServerId, Severity, WorkspaceEdit}; use ide_jsonrpc::{Handler, NoopHandler, Peer}; use ide_language::{LanguageConfig, LanguageRegistry, LanguageServerConfig}; use ide_lsp::types::*; @@ -9,7 +9,7 @@ use ide_lsp::{LaunchError, Launcher, LspManager}; use ide_text::AppliedChange; use parking_lot::Mutex; use serde_json::{Value, json}; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; use std::sync::Arc; use std::sync::atomic::{AtomicUsize, Ordering}; @@ -275,6 +275,22 @@ impl Harness { .collect() } + /// Whether every instance of `name` was reported as removed from the server list. + fn removed(&self, name: &str) -> bool { + let events = self.events.lock(); + let ids: HashSet = events + .iter() + .filter_map(|e| match e { + IdeEvent::LanguageServer { server, name: n, .. } if n == name => Some(*server), + _ => None, + }) + .collect(); + !ids.is_empty() + && ids.iter().all(|id| { + events.iter().any(|e| matches!(e, IdeEvent::LanguageServerRemoved { server } if server == id)) + }) + } + async fn wait_status(&self, name: &str, status: ServerStatus) { eventually(|| self.statuses(name).last() == Some(&status)).await; } @@ -847,6 +863,8 @@ async fn settings_changes_reach_servers_without_restarts() { h.mgr.reconfigure().await; assert_eq!(h.mgr.servers().iter().map(|s| s.name.as_str()).collect::>(), ["mock"]); assert_eq!(h.statuses("extra").last(), Some(&ServerStatus::Stopped)); + assert!(h.removed("extra")); + assert!(!h.removed("mock")); disabled.lock().clear(); h.mgr.reconfigure().await; h.wait_status("extra", ServerStatus::Ready).await; @@ -899,6 +917,7 @@ async fn a_fallback_leaves_when_the_preferred_server_is_enabled_again() { h.wait_status("alt", ServerStatus::Ready).await; eventually(|| h.launcher.state("alt").docs.lock().contains_key(&uri)).await; assert_eq!(h.mgr.servers().iter().map(|s| s.name.as_str()).collect::>(), ["alt"]); + assert!(h.removed("main")); // Enabled again: the fallback closes the document and stops. disabled.lock().clear(); @@ -907,6 +926,7 @@ async fn a_fallback_leaves_when_the_preferred_server_is_enabled_again() { eventually(|| h.launcher.state("main").docs.lock().contains_key(&uri)).await; assert!(!h.launcher.state("alt").notified("textDocument/didClose").is_empty()); assert_eq!(h.statuses("alt").last(), Some(&ServerStatus::Stopped)); + assert!(h.removed("alt")); assert_eq!(h.mgr.servers().iter().map(|s| s.name.as_str()).collect::>(), ["main"]); assert!(h.mgr.hover(doc, Pos::new(0, 0), Default::default()).await.is_ok()); assert_eq!(h.launcher.state("alt").requested("textDocument/hover").len(), 0); diff --git a/src/bindings.ts b/src/bindings.ts index 5ca5aa18..acde503a 100644 --- a/src/bindings.ts +++ b/src/bindings.ts @@ -3088,6 +3088,11 @@ export type IdeEvent = { type: "indexState"; indexing: boolean; files: number; symbols: number } | /** Git status / branch information should be refreshed. */ { type: "vcsChanged"; root: IpcPath } | { type: "languageServer"; server: ServerId; name: string; status: ServerStatus; message: string | null } | +/** + * A stopped language server left the project's list (disabled in settings, or a + * fallback that a preferred server took over from again). + */ +{ type: "languageServerRemoved"; server: ServerId } | /** * A language server asked clients to re-request one kind of data for its * open documents (`workspace/inlayHint/refresh`, …), e.g. after indexing. diff --git a/src/features/code-insight/servers.test.ts b/src/features/code-insight/servers.test.ts index 4ad01d45..e23f5c20 100644 --- a/src/features/code-insight/servers.test.ts +++ b/src/features/code-insight/servers.test.ts @@ -39,6 +39,36 @@ describe("server status", () => { await new Promise((r) => setTimeout(r, 0)); expect(useServers.getState().servers.get(id)).toMatchObject({ status: "missing", message: "install jdtls" }); }); + + it("drops a server the backend removed, even if an older snapshot still lists it", async () => { + useProject.setState({ project: { name: "p", root: "/p", isGit: false, settings: {} } as never }); + dispose = installServerEvents(); + const info = (id: number, name: string): ServerInfo => ({ + id: id as ServerId, + name, + languages: ["python"], + root: "/p", + status: "ready", + pid: null, + }); + pending.splice(0).forEach((resolve) => resolve([info(11, "basedpyright"), info(12, "pyright")])); + await vi.waitFor(() => expect(useServers.getState().servers.size).toBe(2)); + expect(serversSummary([...useServers.getState().servers.values()])?.text).toBe("basedpyright +1"); + + // The fallback stops and leaves the list; a snapshot taken before that answers late. + const status = (id: number, name: string, status: ServerInfo["status"]) => + emitLocalIdeEvent({ type: "languageServer", server: id as ServerId, name, status, message: null }); + status(13, "ty", "starting"); + status(12, "pyright", "stopped"); + emitLocalIdeEvent({ type: "languageServerRemoved", server: 12 as ServerId }); + expect(useServers.getState().servers.has(12 as ServerId)).toBe(false); + pending.splice(0).forEach((resolve) => resolve([info(11, "basedpyright"), info(12, "pyright"), info(13, "ty")])); + await vi.waitFor(() => expect(useServers.getState().servers.get(13 as ServerId)?.languages).toEqual(["python"])); + expect([...useServers.getState().servers.keys()]).toEqual([11, 13]); + + emitLocalIdeEvent({ type: "languageServerRemoved", server: 13 as ServerId }); + expect(serversSummary([...useServers.getState().servers.values()])?.text).toBe("basedpyright"); + }); }); describe("serversSummary", () => { @@ -54,4 +84,10 @@ describe("serversSummary", () => { expect(serversSummary([entry(1, "vtsls", "missing"), entry(2, "typescript-language-server", "ready")])?.tone).toBe("ok"); expect(serversSummary([entry(1, "vtsls", "missing")])?.tone).toBe("error"); }); + it("doesn't count stopped servers next to running ones", () => { + const text = (...servers: ReturnType[]) => serversSummary(servers)?.text; + expect(text(entry(1, "basedpyright", "ready"), entry(2, "pyright", "stopped"))).toBe("basedpyright"); + expect(text(entry(1, "basedpyright", "ready"), entry(2, "ty", "starting"))).toBe("basedpyright +1"); + expect(serversSummary([entry(2, "pyright", "stopped")])).toMatchObject({ text: "pyright", tone: "idle" }); + }); }); diff --git a/src/features/code-insight/servers.ts b/src/features/code-insight/servers.ts index 4cbf940a..1da19ec0 100644 --- a/src/features/code-insight/servers.ts +++ b/src/features/code-insight/servers.ts @@ -1,4 +1,4 @@ -/** Language server status: `lsp_servers` snapshot + `languageServer` events. */ +/** Language server status: `lsp_servers` snapshot + `languageServer` / `languageServerRemoved` events. */ import { getAction } from "@/platform/actions"; import { create } from "zustand"; import type { ServerId, ServerInfo, ServerStatus } from "@/bindings"; @@ -26,6 +26,8 @@ const notifiedMissing = new Set(); /** Event count at each server's latest event, so a snapshot taken earlier doesn't undo it. */ let eventSeq = 0; const lastEvent = new Map(); +/** Event count when each server left the list, so an older snapshot doesn't bring it back. */ +const removedAt = new Map(); function notifyMissing(e: ServerEntry) { if (notifiedMissing.has(e.name)) return; @@ -66,6 +68,7 @@ export async function refreshServers() { const prev = useServers.getState().servers; const map = new Map(); for (const s of list) { + if ((removedAt.get(s.id) ?? 0) > since) continue; const p = prev.get(s.id); const newer = p && (lastEvent.get(s.id) ?? 0) > since; map.set(s.id, { @@ -104,6 +107,16 @@ export function installServerEvents(): () => void { upsertServer({ id: e.server, name: e.name, status: e.status, message: e.message }); if (!known) void refreshServers(); }); + const offRemoved = onIdeEvent("languageServerRemoved", (e) => { + removedAt.set(e.server, ++eventSeq); + lastEvent.delete(e.server); + if (!useServers.getState().servers.has(e.server)) return; + useServers.setState((s) => { + const servers = new Map(s.servers); + servers.delete(e.server); + return { servers }; + }); + }); let root = useProject.getState().project?.root ?? null; void refreshServers(); const unsub = useProject.subscribe((s) => { @@ -115,6 +128,7 @@ export function installServerEvents(): () => void { }); return () => { off(); + offRemoved(); unsub(); }; } @@ -148,7 +162,9 @@ export function serversSummary(servers: readonly ServerEntry[]): { text: string; ); const busy = servers.filter((s) => s.status === "starting" || s.status === "busy"); const tone = bad.length ? "error" : busy.length ? "busy" : servers.some((s) => s.status === "ready") ? "ok" : "idle"; - const names = [...new Set(servers.map((s) => s.name))]; + // A stopped server (one a preferred server took over from) isn't worth naming next to running ones. + const active = servers.filter((s) => s.status !== "stopped"); + const names = [...new Set((active.length ? active : servers).map((s) => s.name))]; const text = names.length === 1 ? names[0]! : `${names[0]} +${names.length - 1}`; return { text, tone }; }