sidebar: Fix draft threads showing up in the sidebar in some cases (#52974)

Bennet Bo Fenner and Ben Brandt created 1 week ago

We were inserting threads with no entries into the sidebar, when you
clicked on New Thread, typed a message and then added a new project to
the current workspace.
We now skip persisting threads when they are empty.

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:

- N/A

---------

Co-authored-by: Ben Brandt <[e-mail]>

Change summary

crates/agent_ui/src/agent_panel.rs           | 37 ++++++++++++++++++++++
crates/agent_ui/src/thread_metadata_store.rs | 34 +++++++++++--------
2 files changed, 56 insertions(+), 15 deletions(-)

Detailed changes

crates/agent_ui/src/agent_panel.rs

@@ -1734,6 +1734,10 @@ impl AgentPanel {
             return;
         };
 
+        if thread_view.read(cx).thread.read(cx).entries().is_empty() {
+            return;
+        }
+
         self.background_threads
             .insert(thread_view.read(cx).id.clone(), conversation_view);
         self.cleanup_background_threads(cx);
@@ -4702,6 +4706,38 @@ mod tests {
         (panel, cx)
     }
 
+    #[gpui::test]
+    async fn test_empty_draft_thread_not_retained_when_navigating_away(cx: &mut TestAppContext) {
+        let (panel, mut cx) = setup_panel(cx).await;
+
+        let connection_a = StubAgentConnection::new();
+        open_thread_with_connection(&panel, connection_a, &mut cx);
+        let session_id_a = active_session_id(&panel, &cx);
+
+        panel.read_with(&cx, |panel, cx| {
+            let thread = panel.active_agent_thread(cx).unwrap();
+            assert!(
+                thread.read(cx).entries().is_empty(),
+                "newly opened draft thread should have no entries"
+            );
+            assert!(panel.background_threads.is_empty());
+        });
+
+        let connection_b = StubAgentConnection::new();
+        open_thread_with_connection(&panel, connection_b, &mut cx);
+
+        panel.read_with(&cx, |panel, _cx| {
+            assert!(
+                panel.background_threads.is_empty(),
+                "empty draft thread should not be retained in background_threads"
+            );
+            assert!(
+                !panel.background_threads.contains_key(&session_id_a),
+                "empty draft thread should not be keyed in background_threads"
+            );
+        });
+    }
+
     #[gpui::test]
     async fn test_running_thread_retained_when_navigating_away(cx: &mut TestAppContext) {
         let (panel, mut cx) = setup_panel(cx).await;
@@ -4813,6 +4849,7 @@ mod tests {
         // Open thread B — thread A goes to background.
         let connection_b = StubAgentConnection::new();
         open_thread_with_connection(&panel, connection_b, &mut cx);
+        send_message(&panel, &mut cx);
 
         let session_id_b = active_session_id(&panel, &cx);
 

crates/agent_ui/src/thread_metadata_store.rs

@@ -447,14 +447,9 @@ impl ThreadMetadataStore {
                 let weak_store = weak_store.clone();
                 move |thread, cx| {
                     weak_store
-                        .update(cx, |store, cx| {
+                        .update(cx, |store, _cx| {
                             let session_id = thread.session_id().clone();
                             store.session_subscriptions.remove(&session_id);
-                            if thread.entries().is_empty() {
-                                // Empty threads can be unloaded without ever being
-                                // durably persisted by the underlying agent.
-                                store.delete(session_id, cx);
-                            }
                         })
                         .ok();
                 }
@@ -545,6 +540,10 @@ impl ThreadMetadataStore {
             | AcpThreadEvent::Refusal
             | AcpThreadEvent::WorkingDirectoriesUpdated => {
                 let thread_ref = thread.read(cx);
+                if thread_ref.entries().is_empty() {
+                    return;
+                }
+
                 let existing_thread = self.threads.get(thread_ref.session_id());
                 let session_id = thread_ref.session_id().clone();
                 let title = thread_ref
@@ -1293,7 +1292,7 @@ mod tests {
     }
 
     #[gpui::test]
-    async fn test_empty_thread_metadata_deleted_when_thread_released(cx: &mut TestAppContext) {
+    async fn test_empty_thread_events_do_not_create_metadata(cx: &mut TestAppContext) {
         init_test(cx);
 
         let fs = FakeFs::new(cx.executor());
@@ -1323,11 +1322,16 @@ mod tests {
                 .entry_ids()
                 .collect::<Vec<_>>()
         });
-        assert_eq!(metadata_ids, vec![session_id]);
+        assert!(
+            metadata_ids.is_empty(),
+            "expected empty draft thread title updates to be ignored"
+        );
 
-        drop(thread);
-        cx.update(|_| {});
-        cx.run_until_parked();
+        cx.update(|cx| {
+            thread.update(cx, |thread, cx| {
+                thread.push_user_content_block(None, "Hello".into(), cx);
+            });
+        });
         cx.run_until_parked();
 
         let metadata_ids = cx.update(|cx| {
@@ -1336,10 +1340,7 @@ mod tests {
                 .entry_ids()
                 .collect::<Vec<_>>()
         });
-        assert!(
-            metadata_ids.is_empty(),
-            "expected empty draft thread metadata to be deleted on release"
-        );
+        assert_eq!(metadata_ids, vec![session_id]);
     }
 
     #[gpui::test]
@@ -1414,6 +1415,7 @@ mod tests {
 
         cx.update(|cx| {
             thread_without_worktree.update(cx, |thread, cx| {
+                thread.push_user_content_block(None, "content".into(), cx);
                 thread.set_title("No Project Thread".into(), cx).detach();
             });
         });
@@ -1434,6 +1436,7 @@ mod tests {
 
         cx.update(|cx| {
             thread_with_worktree.update(cx, |thread, cx| {
+                thread.push_user_content_block(None, "content".into(), cx);
                 thread.set_title("Project Thread".into(), cx).detach();
             });
         });
@@ -1489,6 +1492,7 @@ mod tests {
         // Set a title on the regular thread to trigger a save via handle_thread_update.
         cx.update(|cx| {
             regular_thread.update(cx, |thread, cx| {
+                thread.push_user_content_block(None, "content".into(), cx);
                 thread.set_title("Regular Thread".into(), cx).detach();
             });
         });