From 0a52f80824a2f1e9b48dbdc615d63453eaaaf1f9 Mon Sep 17 00:00:00 2001 From: Bruno Moreira Date: Thu, 7 May 2026 05:17:16 -0300 Subject: [PATCH] acp_thread: Clear running_turn when prompt task drops tx (#55562) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `AcpThread::status` is purely `running_turn.is_some()`. The cleanup that takes `running_turn` sat below the early-return guard that fires when the prompt response oneshot resolves to `Err(Cancelled)` (the inner `send_task` was dropped before `tx.send`). Any code path that drops the in-flight `send_task` therefore left the panel stuck in `Generating`. Reordered so cleanup runs before the dropped-tx guard; the same-turn invariant is preserved. Related to #47928 (partial — that issue also has an upstream `claude-agent-acp` component this PR does not address). 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 Release Notes: - Fixed agent panel staying in a generating state when the underlying prompt task was cancelled before completing --- crates/acp_thread/src/acp_thread.rs | 72 +++++++++++++++++++++++++++-- 1 file changed, 67 insertions(+), 5 deletions(-) diff --git a/crates/acp_thread/src/acp_thread.rs b/crates/acp_thread/src/acp_thread.rs index 2c448d343075b6c1688847b6605d2ebcecf82f6f..769131a8e0d276a5ba00e16602f9d388498b2396 100644 --- a/crates/acp_thread/src/acp_thread.rs +++ b/crates/acp_thread/src/acp_thread.rs @@ -2294,10 +2294,6 @@ impl AcpThread { this.project .update(cx, |project, cx| project.set_agent_location(None, cx)); } - let Ok(response) = response else { - // tx dropped, just return - return Ok(None); - }; let is_same_turn = this .running_turn @@ -2306,11 +2302,18 @@ impl AcpThread { // If the user submitted a follow up message, running_turn might // already point to a different turn. Therefore we only want to - // take the task if it's the same turn. + // take the task if it's the same turn. We do this before the + // dropped-tx guard below so the panel exits its generating + // state even when the send_task is cancelled before tx.send(). if is_same_turn { this.running_turn.take(); } + let Ok(response) = response else { + // tx dropped, just return + return Ok(None); + }; + match response { Ok(r) => { Self::flush_streaming_text(&mut this.streaming_text_buffer, cx); @@ -5517,4 +5520,63 @@ mod tests { ); }); } + + /// Regression test: if the inner send_task is cancelled before it can + /// fire `tx.send(...)` (e.g. because the underlying future was dropped), + /// the outer task observes `rx.await` returning `Err(Cancelled)` and + /// must still clear `running_turn` so the panel transitions out of + /// `Generating`. Without this, the agent thread is wedged in the + /// loading state until Zed restarts. + #[gpui::test] + async fn test_running_turn_cleared_when_send_task_dropped(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + + // Handler hangs forever so the spawn at run_turn is parked inside + // `f(this, cx).await` with `tx` still alive but unsent. + let connection = Rc::new(FakeAgentConnection::new().on_user_message( + |_params, _thread, _cx| { + async move { futures::future::pending::>().await } + .boxed_local() + }, + )); + + let thread = cx + .update(|cx| { + connection.new_session(project, PathList::new(&[Path::new(path!("/test"))]), cx) + }) + .await + .unwrap(); + + let request = thread.update(cx, |thread, cx| thread.send_raw("hello", cx)); + cx.run_until_parked(); + + assert_eq!( + thread.read_with(cx, |t, _| t.status()), + ThreadStatus::Generating, + "thread should be generating while the handler is parked" + ); + + // Replace the in-flight send_task with a no-op. Dropping the original + // Task cancels its inner future, which drops `tx` without ever calling + // `tx.send(...)`. This mirrors the production scenario where the + // send_task future is cancelled before completion. + thread.update(cx, |thread, _| { + thread.running_turn.as_mut().unwrap().send_task = Task::ready(()); + }); + + let result = request.await; + assert!( + matches!(result, Ok(None)), + "outer task should resolve to Ok(None) on dropped tx, got {result:?}" + ); + + assert_eq!( + thread.read_with(cx, |t, _| t.status()), + ThreadStatus::Idle, + "running_turn must be cleared even when tx was dropped without send" + ); + } }