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 <[email protected]>

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, &params["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 {

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())]);

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();