From 1dbebefa94ae132a3ca9c0fe1f959a475031ec1a Mon Sep 17 00:00:00 2001 From: lleviathe <95321986+lleviathe@users.noreply.github.com> Date: Mon, 9 Nov 2026 02:01:48 +0400 Subject: [PATCH] fix(lsp): end the earlier task when a progress token begins again (#2199) Some servers reuse one `$/progress` token for concurrent work (PHPantom sends begin, begin, end, end). The second begin overwrote the first task, which never got a done event and stayed at 0% in the background tasks popup. A begin on a token with a live task now ends that task first. The progress map keeps one entry per token, so busy status and idle waits count as before. Co-authored-by: lleviathe <95321986+lleviathe@users.noreply.github.com> --- crates/ide-lsp/src/handler.rs | 20 +++++- crates/ide-lsp/tests/mock_server.rs | 105 ++++++++++++++++++++++++++++ src/workbench/tasks.test.tsx | 23 ++++++ 3 files changed, 146 insertions(+), 2 deletions(-) diff --git a/crates/ide-lsp/src/handler.rs b/crates/ide-lsp/src/handler.rs index ec098020b..20fae6b01 100644 --- a/crates/ide-lsp/src/handler.rs +++ b/crates/ide-lsp/src/handler.rs @@ -51,6 +51,7 @@ impl ClientHandler { let message = value.get("message").and_then(Value::as_str).map(str::to_owned); let fraction = value.get("percentage").and_then(Value::as_f64).map(|p| (p / 100.0).clamp(0.0, 1.0) as f32); let cancellable = value.get("cancellable").and_then(Value::as_bool); + let mut replaced = None; let event = { let mut progress = self.server.progress.lock(); match value.get("kind").and_then(Value::as_str) { @@ -64,7 +65,19 @@ impl ClientHandler { let task = TaskId::next(); let cancel = (cancellable == Some(true)).then(|| self.cancel_handle(task, ¶ms["token"])); let cancellable = cancel.is_some(); - progress.insert(token, Progress { task, title: title.clone(), message: message.clone(), cancel }); + // A token carries one `begin`, yet servers reuse one for concurrent work + // (PHPantom: begin, begin, end, end). The task it replaces ends now, or no + // `end` would ever reach it. + replaced = progress + .insert(token, Progress { task, title: title.clone(), message: message.clone(), cancel }) + .map(|p| IdeEvent::Task { + id: p.task, + title: p.title, + message: p.message, + fraction: None, + done: true, + cancellable: false, + }); IdeEvent::Task { id: task, title, message, fraction, done: false, cancellable } } Some("report") => { @@ -102,7 +115,10 @@ impl ClientHandler { _ => return, } }; - let ended = matches!(&event, IdeEvent::Task { done: true, .. }); + let ended = replaced.is_some() || matches!(&event, IdeEvent::Task { done: true, .. }); + if let Some(replaced) = replaced { + self.server.emit(mgr.sink(), replaced); + } self.server.emit(mgr.sink(), event); self.server.refresh_busy_status(mgr.sink()); if ended { diff --git a/crates/ide-lsp/tests/mock_server.rs b/crates/ide-lsp/tests/mock_server.rs index 4abe3d289..05a866391 100644 --- a/crates/ide-lsp/tests/mock_server.rs +++ b/crates/ide-lsp/tests/mock_server.rs @@ -1306,6 +1306,111 @@ async fn progress_titled_with_an_executable_path_shows_the_program_name() { assert_eq!(titles, ["cargo check"; 3]); } +/// PHPantom reuses a fixed token for concurrent requests: begin, begin, end, end. +/// Every task it started ends, and the server reads ready afterwards. +#[tokio::test(flavor = "multi_thread")] +async fn a_progress_token_begun_again_ends_the_task_it_replaces() { + let h = harness(&[("mock", incremental_caps())]); + h.open("a.mk", "x"); + h.wait_status("mock", ServerStatus::Ready).await; + let peer = h.launcher.state("mock").peer(); + let progress = + |token: &str, kind: &str, title: &str| json!({ "token": token, "value": { "kind": kind, "title": title } }); + for kind in ["begin", "begin", "end", "end"] { + peer.notify("$/progress", progress("find_references", kind, "Finding references")).unwrap(); + } + // Notifications are handled in order: once this task ended, so did everything above. + peer.notify("$/progress", progress("later", "begin", "Indexing")).unwrap(); + peer.notify("$/progress", progress("later", "end", "Indexing")).unwrap(); + eventually(|| { + h.events.lock().iter().any(|e| matches!(e, IdeEvent::Task { title, done: true, .. } if title == "Indexing")) + }) + .await; + let (mut begun, mut ended) = (Vec::new(), Vec::new()); + for e in h.events.lock().iter() { + if let IdeEvent::Task { id, title, done, .. } = e + && title == "Finding references" + { + if *done { ended.push(*id) } else { begun.push(*id) } + } + } + assert_eq!(begun.len(), 2); + for id in &begun { + assert_eq!(ended.iter().filter(|e| *e == id).count(), 1, "every task ends once: {begun:?} {ended:?}"); + } + h.wait_status("mock", ServerStatus::Ready).await; +} + +#[tokio::test(flavor = "multi_thread")] +async fn replacing_progress_keeps_cancellation_reports_and_indexing_on_the_new_task() { + let h = harness(&[("mock", incremental_caps())]); + let (doc, _) = h.open("a.mk", "target"); + h.wait_status("mock", ServerStatus::Ready).await; + let s = h.launcher.state("mock"); + let peer = s.peer(); + let begin = |title| json!({ "token": 7, "value": { "kind": "begin", "title": title, "cancellable": true } }); + let task_of = |title: &str| { + h.events.lock().iter().find_map(|e| match e { + IdeEvent::Task { id, title: t, done: false, .. } if t == title => Some(*id), + _ => None, + }) + }; + peer.notify("$/progress", begin("Old work")).unwrap(); + h.wait_status("mock", ServerStatus::Busy).await; + let old = task_of("Old work").unwrap(); + let mgr = h.mgr.clone(); + let search = tokio::spawn(async move { mgr.references(doc, Pos::new(0, 1), true, true, Default::default()).await }); + eventually(|| !s.requested("textDocument/references").is_empty()).await; + + peer.notify("$/progress", begin("New work")).unwrap(); + peer.notify( + "$/progress", + json!({ "token": 7, "value": { "kind": "report", "message": "Halfway", "percentage": 50 } }), + ) + .unwrap(); + eventually(|| { + h.events.lock().iter().any(|e| matches!(e, IdeEvent::Task { title, message: Some(m), fraction: Some(f), done: false, .. } if title == "New work" && m == "Halfway" && *f == 0.5)) + }) + .await; + let new = task_of("New work").unwrap(); + assert_ne!(old, new); + assert!(!ide_core::task::cancel(old), "a replaced task cannot cancel its successor"); + assert!(ide_core::task::cancel(new)); + eventually(|| !s.notified("window/workDoneProgress/cancel").is_empty()).await; + assert_eq!(s.notified("window/workDoneProgress/cancel"), [json!({ "token": 7 })]); + assert_eq!(h.statuses("mock").last(), Some(&ServerStatus::Busy)); + assert!(!search.is_finished(), "replacement must not release a search waiting for idle"); + let refs = h.mgr.references(doc, Pos::new(0, 1), true, false, Default::default()).await.unwrap(); + assert_eq!(refs.incomplete, Some(ReferencesIncomplete::Indexing)); + + for _ in 0..2 { + peer.notify("$/progress", json!({ "token": 7, "value": { "kind": "end" } })).unwrap(); + } + // A later task is a barrier for both end notifications. + peer.notify("$/progress", json!({ "token": "barrier", "value": { "kind": "begin", "title": "Barrier" } })).unwrap(); + peer.notify("$/progress", json!({ "token": "barrier", "value": { "kind": "end" } })).unwrap(); + eventually(|| { + h.events.lock().iter().any(|e| matches!(e, IdeEvent::Task { title, done: true, .. } if title == "Barrier")) + }) + .await; + h.wait_status("mock", ServerStatus::Ready).await; + let refs = tokio::time::timeout(Duration::from_secs(5), search).await.unwrap().unwrap().unwrap(); + assert_eq!(refs.incomplete, None); + assert!(!ide_core::task::cancel(new)); + let events = h.events.lock(); + for id in [old, new] { + assert_eq!( + events.iter().filter(|e| matches!(e, IdeEvent::Task { id: t, done: true, .. } if *t == id)).count(), + 1 + ); + } + assert!( + !events + .iter() + .any(|e| matches!(e, IdeEvent::Task { id, message: Some(m), .. } if *id == old && m == "Halfway")) + ); +} + #[tokio::test(flavor = "multi_thread")] async fn cancelling_a_cancellable_progress_task_asks_the_server_to_stop() { let h = harness(&[("mock", incremental_caps())]); diff --git a/src/workbench/tasks.test.tsx b/src/workbench/tasks.test.tsx index 12f088dbd..c659293bf 100644 --- a/src/workbench/tasks.test.tsx +++ b/src/workbench/tasks.test.tsx @@ -78,6 +78,29 @@ describe("task queues", () => { }); describe("cancelling", () => { + it.each([false, true])("keeps a replacement task when the old completion arrives (delayed: %s)", (delayed) => { + vi.useFakeTimers(); + const uninstall = installTaskEvents(); + const base = { + type: "task" as const, + title: "Finding references", + message: null, + fraction: null, + cancellable: false, + }; + emitLocalIdeEvent({ ...base, id: 51, done: false }); + if (delayed) vi.advanceTimersByTime(600); + emitLocalIdeEvent({ ...base, id: 52, done: false }); + emitLocalIdeEvent({ ...base, id: 51, done: true }); + vi.advanceTimersByTime(600); + expect(useTasks.getState().tasks.map((t) => t.id)).toEqual([52]); + emitLocalIdeEvent({ ...base, id: 52, done: true }); + emitLocalIdeEvent({ ...base, id: 52, done: true }); + vi.advanceTimersByTime(600); + expect(useTasks.getState().tasks).toEqual([]); + uninstall(); + }); + it("shows Cancelling… until the task ends, then removes it without the finished bar", () => { vi.useFakeTimers(); const cancel = vi.fn();