vrr/zed
2
0
Fork 0
mirror of https://github.com/zed-industries/zed.git synced 2026-07-10 00:13:29 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 18

lsp_button: Fix missing server metadata after restart (#55162)

Browse source 
Restarting an LSP from the picker drops the server's metadata (version,
memory, "View Logs") even though the new server is healthy. Reproduces
with `vtsls`, `eslint`, `rust-analyzer`, and others; `vlsls` doesn't.

The button kept id-keyed state in
[`health_statuses`](https://github.com/zed-industries/zed/blob/main/crates/language_tools/src/lsp_button.rs#L159)
and
[`servers_per_buffer_abs_path`](https://github.com/zed-industries/zed/blob/main/crates/language_tools/src/lsp_button.rs#L161),
but never handled
[`LspStoreEvent::LanguageServerRemoved`](https://github.com/zed-industries/zed/blob/main/crates/project/src/lsp_store.rs#L4066)
— there was even a [stale
TODO](https://github.com/zed-industries/zed/blob/main/crates/language_tools/src/lsp_button.rs#L860)
claiming the event wasn't emitted (it's emitted from [five
sites](https://github.com/zed-industries/zed/blob/main/crates/project/src/lsp_store.rs#L11190)
in `lsp_store`). On restart the dead id lingered next to the new one in
both maps, and rendering picked the wrong entry.

Fix: handle `LanguageServerRemoved` and evict the dead id from both
maps. `binary_statuses` is left alone — it's name-keyed and shared
across restart cycles to drive the "Downloading… → Starting…" UX.

The cleanup is extracted to `LanguageServers::remove_server` so it's
directly unit-testable. Added four tests covering health eviction,
per-buffer eviction with empty-entry pruning, `binary_statuses`
preservation, and the full restart sequence.

[PR #50417](https://github.com/zed-industries/zed/pull/50417) attempted
this earlier with a `LanguageServerAdded` handler. @SomeoneToIgnore
flagged that as the wrong hook ([buffer
registration](https://github.com/zed-industries/zed/blob/main/crates/language_tools/src/lsp_button.rs#L919-L948)
is what populates state, not startup) and asked for tests. Handling
`LanguageServerRemoved` is a cleaner fit — it's the natural counterpart
to registration, and it also fixes the related case where stopping a
server (without restart) leaks state.

## Verification

Reproduced on `main` and verified the fix on this branch with `vtsls`
and `eslint` against a small TypeScript project.

| Before restart | After restart on `main` (bug) | After restart on this
branch (fix) |
|---|---|---|
| <img width="588" alt="vtsls row before restart — version, memory, View
Logs all visible"
src="https://github.com/user-attachments/assets/bdd2610b-cf8f-4ea1-bc45-f363b098c146"
/> | <img width="536" alt="vtsls row after restart on main — metadata
gone"
src="https://github.com/user-attachments/assets/66b3eefd-f801-4bfd-8b80-5f0e20fb3bc6"
/> | <img width="554" alt="vtsls row after restart on fix branch —
metadata persists"
src="https://github.com/user-attachments/assets/51d4df48-1223-4b21-856f-72d4c1eb2a6a"
/> |

Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Closes #53627

Release Notes:

- Fixed missing language server metadata in the LSP menu after restart.
</content>

---------

Co-authored-by: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com>
This commit is contained in:
Ankan Misra 2026-07-02 16:30:16 +05:30 • committed by GitHub
parent e5513539ec
commit 4a1cb2b1e2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
1 changed files with 179 additions and 1 deletions
Download patch file Download diff file Expand all files Collapse all files

180
crates/language_tools/src/lsp_button.rs
View file

@ -724,6 +724,19 @@ impl LanguageServers {
fn is_empty(&self) -> bool {
self.binary_statuses.is_empty() && self.health_statuses.is_empty()
}
/// Drop all id-keyed state for a server that has been removed (stopped or
/// reaching end-of-life via restart). `binary_statuses` is intentionally
/// preserved — it is keyed by name and shared across restart cycles to
/// drive the "Downloading… → Starting…" status UX.
fn remove_server(&mut self, server_id: LanguageServerId) {
self.health_statuses.remove(&server_id);
self.servers_per_buffer_abs_path
.retain(|_, servers_for_path| {
servers_for_path.servers.remove(&server_id);
!servers_for_path.servers.is_empty()
});
}
}
#[derive(Debug)]
@ -903,7 +916,6 @@ impl LspButton {
let mut updated = false;
// TODO `LspStore` is global and reports status from all language servers, even from the other windows.
// Also, we do not get "LSP removed" events so LSPs are never removed.
match e {
LspStoreEvent::LanguageServerUpdate {
language_server_id,
@ -993,6 +1005,12 @@ impl LspButton {
});
updated = true;
}
LspStoreEvent::LanguageServerRemoved(server_id) => {
self.server_state.update(cx, |state, _| {
state.language_servers.remove_server(*server_id);
});
updated = true;
}
_ => {}
};
@ -1406,3 +1424,163 @@ impl Render for LspButton {
)
}
}
#[cfg(test)]
mod tests {
use super::*;
fn server_id(n: usize) -> LanguageServerId {
LanguageServerId(n)
}
fn server_name(s: &str) -> LanguageServerName {
LanguageServerName(s.into())
}
fn health_status(name: &str) -> LanguageServerHealthStatus {
LanguageServerHealthStatus {
name: server_name(name),
health: Some((None, ServerHealth::Ok)),
}
}
fn servers_for_path(servers: &[(LanguageServerId, &str)]) -> ServersForPath {
ServersForPath {
servers: servers
.iter()
.map(|(id, name)| (*id, Some(server_name(name))))
.collect(),
worktree: None,
}
}
/// `remove_server` evicts the id from `health_statuses` so a restarted
/// server's new id renders without inheriting the old one's stale entry.
/// This is the regression test for #53627.
#[test]
fn remove_server_drops_health_entry_for_id() {
let mut state = LanguageServers::default();
state
.health_statuses
.insert(server_id(1), health_status("rust-analyzer"));
state
.health_statuses
.insert(server_id(2), health_status("typescript-language-server"));
state.remove_server(server_id(1));
assert!(!state.health_statuses.contains_key(&server_id(1)));
assert!(state.health_statuses.contains_key(&server_id(2)));
}
/// `remove_server` evicts the id from each per-buffer entry; entries that
/// become empty are dropped so the map does not grow unbounded across
/// many buffer opens/closes.
#[test]
fn remove_server_evicts_id_from_per_buffer_entries_and_drops_empty_entries() {
let mut state = LanguageServers::default();
let buffer_a = PathBuf::from("/project/a.rs");
let buffer_b = PathBuf::from("/project/b.rs");
state.servers_per_buffer_abs_path.insert(
buffer_a.clone(),
servers_for_path(&[(server_id(1), "rust-analyzer")]),
);
state.servers_per_buffer_abs_path.insert(
buffer_b.clone(),
servers_for_path(&[(server_id(1), "rust-analyzer"), (server_id(2), "typos-lsp")]),
);
state.remove_server(server_id(1));
assert!(
!state.servers_per_buffer_abs_path.contains_key(&buffer_a),
"buffer_a's entry held only the removed server, so the entry itself should be dropped",
);
let buffer_b_entry = state
.servers_per_buffer_abs_path
.get(&buffer_b)
.expect("buffer_b's entry has another server, so it must be retained");
assert!(!buffer_b_entry.servers.contains_key(&server_id(1)));
assert!(buffer_b_entry.servers.contains_key(&server_id(2)));
}
/// `binary_statuses` is keyed by name and intentionally shared across
/// restart cycles to drive the "Downloading… → Starting…" UX. Removing a
/// single server's id must not touch it.
#[test]
fn remove_server_does_not_touch_binary_statuses() {
let mut state = LanguageServers::default();
state.binary_statuses.insert(
server_name("rust-analyzer"),
LanguageServerBinaryStatus {
status: BinaryStatus::Starting,
message: None,
},
);
state.remove_server(server_id(1));
assert!(
state
.binary_statuses
.contains_key(&server_name("rust-analyzer")),
"binary_statuses is name-keyed and shared across restart cycles",
);
}
/// Simulates the full restart event sequence: remove old id, register
/// new id with same name, write health for the new id. After restart
/// only the new id should be visible — no leftover entry from the old
/// incarnation.
#[test]
fn restart_sequence_leaves_only_new_server_id() {
let mut state = LanguageServers::default();
let buffer = PathBuf::from("/project/main.rs");
let name = "rust-analyzer";
// Pre-restart: server v1 is registered for the buffer with health.
state
.servers_per_buffer_abs_path
.insert(buffer.clone(), servers_for_path(&[(server_id(1), name)]));
state
.health_statuses
.insert(server_id(1), health_status(name));
// Restart: old id is removed.
state.remove_server(server_id(1));
// New id registers for the same buffer.
let entry = state
.servers_per_buffer_abs_path
.entry(buffer.clone())
.or_insert_with(|| ServersForPath {
servers: HashMap::default(),
worktree: None,
});
entry.servers.insert(server_id(2), Some(server_name(name)));
// Health update for the new id arrives.
state
.health_statuses
.insert(server_id(2), health_status(name));
let entry = state
.servers_per_buffer_abs_path
.get(&buffer)
.expect("buffer must still be tracked");
assert_eq!(
entry.servers.keys().copied().collect::<Vec<_>>(),
vec![server_id(2)],
"exactly one server for this buffer — the new incarnation",
);
assert!(
!state.health_statuses.contains_key(&server_id(1)),
"the dead server's health entry must not linger",
);
assert!(
state.health_statuses.contains_key(&server_id(2)),
"the new server's health entry is present",
);
}
}