agent_ui: Prevent race conditions inside thread metadata store (#52819)

Bennet Bo Fenner and Gaauwe Rombouts created

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: Gaauwe Rombouts <mail@grombouts.nl>

Change summary

crates/acp_thread/src/acp_thread.rs          |   4 
crates/agent_ui/src/agent_diff.rs            |   3 
crates/agent_ui/src/agent_panel.rs           |  23 -
crates/agent_ui/src/conversation_view.rs     |   6 
crates/agent_ui/src/thread_metadata_store.rs | 345 ++++++++++---------
crates/agent_ui/src/threads_archive_view.rs  |   1 
crates/sidebar/src/sidebar.rs                |   7 
crates/sidebar/src/sidebar_tests.rs          | 374 +++++++++++----------
8 files changed, 397 insertions(+), 366 deletions(-)

Detailed changes

crates/acp_thread/src/acp_thread.rs 🔗

@@ -1095,6 +1095,7 @@ pub enum AcpThreadEvent {
     AvailableCommandsUpdated(Vec<acp::AvailableCommand>),
     ModeUpdated(acp::SessionModeId),
     ConfigOptionsUpdated(Vec<acp::SessionConfigOption>),
+    WorkingDirectoriesUpdated,
 }
 
 impl EventEmitter<AcpThreadEvent> for AcpThread {}
@@ -1288,8 +1289,9 @@ impl AcpThread {
         self.work_dirs.as_ref()
     }
 
-    pub fn set_work_dirs(&mut self, work_dirs: PathList) {
+    pub fn set_work_dirs(&mut self, work_dirs: PathList, cx: &mut Context<Self>) {
         self.work_dirs = Some(work_dirs);
+        cx.emit(AcpThreadEvent::WorkingDirectoriesUpdated)
     }
 
     pub fn status(&self) -> ThreadStatus {

crates/agent_ui/src/agent_diff.rs 🔗

@@ -1415,7 +1415,8 @@ impl AgentDiff {
             | AcpThreadEvent::AvailableCommandsUpdated(_)
             | AcpThreadEvent::Retry(_)
             | AcpThreadEvent::ModeUpdated(_)
-            | AcpThreadEvent::ConfigOptionsUpdated(_) => {}
+            | AcpThreadEvent::ConfigOptionsUpdated(_)
+            | AcpThreadEvent::WorkingDirectoriesUpdated => {}
         }
     }
 

crates/agent_ui/src/agent_panel.rs 🔗

@@ -2030,27 +2030,8 @@ impl AgentPanel {
         }
 
         for thread in &root_threads {
-            thread.update(cx, |thread, _cx| {
-                thread.set_work_dirs(new_work_dirs.clone());
-            });
-        }
-
-        if let Some(metadata_store) =
-            crate::thread_metadata_store::ThreadMetadataStore::try_global(cx)
-        {
-            metadata_store.update(cx, |store, cx| {
-                for thread in &root_threads {
-                    let is_archived = store
-                        .entry(thread.read(cx).session_id())
-                        .map(|t| t.archived)
-                        .unwrap_or(false);
-                    let metadata = crate::thread_metadata_store::ThreadMetadata::from_thread(
-                        is_archived,
-                        thread,
-                        cx,
-                    );
-                    store.save(metadata, cx);
-                }
+            thread.update(cx, |thread, cx| {
+                thread.set_work_dirs(new_work_dirs.clone(), cx);
             });
         }
     }

crates/agent_ui/src/conversation_view.rs 🔗

@@ -202,7 +202,8 @@ impl Conversation {
                 | AcpThreadEvent::Refusal
                 | AcpThreadEvent::AvailableCommandsUpdated(_)
                 | AcpThreadEvent::ModeUpdated(_)
-                | AcpThreadEvent::ConfigOptionsUpdated(_) => {}
+                | AcpThreadEvent::ConfigOptionsUpdated(_)
+                | AcpThreadEvent::WorkingDirectoriesUpdated => {}
             }
         });
         self.subscriptions.push(subscription);
@@ -1457,6 +1458,9 @@ impl ConversationView {
                 // The watch task in ConfigOptionsView handles rebuilding selectors
                 cx.notify();
             }
+            AcpThreadEvent::WorkingDirectoriesUpdated => {
+                cx.notify();
+            }
         }
         cx.notify();
     }

crates/agent_ui/src/thread_metadata_store.rs 🔗

@@ -1,5 +1,6 @@
 use std::{path::Path, sync::Arc};
 
+use acp_thread::AcpThreadEvent;
 use agent::{ThreadStore, ZED_AGENT_ID};
 use agent_client_protocol as acp;
 use anyhow::Context as _;
@@ -138,49 +139,13 @@ impl From<&ThreadMetadata> for acp_thread::AgentSessionInfo {
     }
 }
 
-impl ThreadMetadata {
-    pub fn from_thread(
-        is_archived: bool,
-        thread: &Entity<acp_thread::AcpThread>,
-        cx: &App,
-    ) -> Self {
-        let thread_ref = thread.read(cx);
-        let session_id = thread_ref.session_id().clone();
-        let title = thread_ref
-            .title()
-            .unwrap_or_else(|| DEFAULT_THREAD_TITLE.into());
-        let updated_at = Utc::now();
-
-        let agent_id = thread_ref.connection().agent_id();
-
-        let folder_paths = {
-            let project = thread_ref.project().read(cx);
-            let paths: Vec<Arc<Path>> = project
-                .visible_worktrees(cx)
-                .map(|worktree| worktree.read(cx).abs_path())
-                .collect();
-            PathList::new(&paths)
-        };
-
-        Self {
-            session_id,
-            agent_id,
-            title,
-            created_at: Some(updated_at), // handled by db `ON CONFLICT`
-            updated_at,
-            folder_paths,
-            archived: is_archived,
-        }
-    }
-}
-
 /// The store holds all metadata needed to show threads in the sidebar/the archive.
 ///
 /// Automatically listens to AcpThread events and updates metadata if it has changed.
 pub struct ThreadMetadataStore {
     db: ThreadMetadataDb,
     threads: HashMap<acp::SessionId, ThreadMetadata>,
-    threads_by_paths: HashMap<PathList, Vec<ThreadMetadata>>,
+    threads_by_paths: HashMap<PathList, HashSet<acp::SessionId>>,
     reload_task: Option<Shared<Task<()>>>,
     session_subscriptions: HashMap<acp::SessionId, Subscription>,
     pending_thread_ops_tx: smol::channel::Sender<DbOperation>,
@@ -189,14 +154,14 @@ pub struct ThreadMetadataStore {
 
 #[derive(Debug, PartialEq)]
 enum DbOperation {
-    Insert(ThreadMetadata),
+    Upsert(ThreadMetadata),
     Delete(acp::SessionId),
 }
 
 impl DbOperation {
     fn id(&self) -> &acp::SessionId {
         match self {
-            DbOperation::Insert(thread) => &thread.session_id,
+            DbOperation::Upsert(thread) => &thread.session_id,
             DbOperation::Delete(session_id) => session_id,
         }
     }
@@ -248,12 +213,12 @@ impl ThreadMetadataStore {
     }
 
     /// Returns all threads.
-    pub fn entries(&self) -> impl Iterator<Item = ThreadMetadata> + '_ {
-        self.threads.values().cloned()
+    pub fn entries(&self) -> impl Iterator<Item = &ThreadMetadata> + '_ {
+        self.threads.values()
     }
 
     /// Returns all archived threads.
-    pub fn archived_entries(&self) -> impl Iterator<Item = ThreadMetadata> + '_ {
+    pub fn archived_entries(&self) -> impl Iterator<Item = &ThreadMetadata> + '_ {
         self.entries().filter(|t| t.archived)
     }
 
@@ -261,13 +226,13 @@ impl ThreadMetadataStore {
     pub fn entries_for_path(
         &self,
         path_list: &PathList,
-    ) -> impl Iterator<Item = ThreadMetadata> + '_ {
+    ) -> impl Iterator<Item = &ThreadMetadata> + '_ {
         self.threads_by_paths
             .get(path_list)
             .into_iter()
             .flatten()
+            .filter_map(|s| self.threads.get(s))
             .filter(|s| !s.archived)
-            .cloned()
     }
 
     fn reload(&mut self, cx: &mut Context<Self>) -> Shared<Task<()>> {
@@ -291,7 +256,7 @@ impl ThreadMetadataStore {
                         this.threads_by_paths
                             .entry(row.folder_paths.clone())
                             .or_default()
-                            .push(row.clone());
+                            .insert(row.session_id.clone());
                         this.threads.insert(row.session_id.clone(), row);
                     }
 
@@ -310,19 +275,44 @@ impl ThreadMetadataStore {
         }
 
         for metadata in metadata {
-            self.pending_thread_ops_tx
-                .try_send(DbOperation::Insert(metadata))
-                .log_err();
+            self.save_internal(metadata);
         }
+        cx.notify();
     }
 
-    pub fn save(&mut self, metadata: ThreadMetadata, cx: &mut Context<Self>) {
+    #[cfg(any(test, feature = "test-support"))]
+    pub fn save_manually(&mut self, metadata: ThreadMetadata, cx: &mut Context<Self>) {
+        self.save(metadata, cx)
+    }
+
+    fn save(&mut self, metadata: ThreadMetadata, cx: &mut Context<Self>) {
         if !cx.has_flag::<AgentV2FeatureFlag>() {
             return;
         }
 
+        self.save_internal(metadata);
+        cx.notify();
+    }
+
+    fn save_internal(&mut self, metadata: ThreadMetadata) {
+        // If the folder paths have changed, we need to clear the old entry
+        if let Some(thread) = self.threads.get(&metadata.session_id)
+            && thread.folder_paths != metadata.folder_paths
+            && let Some(session_ids) = self.threads_by_paths.get_mut(&thread.folder_paths)
+        {
+            session_ids.remove(&metadata.session_id);
+        }
+
+        self.threads
+            .insert(metadata.session_id.clone(), metadata.clone());
+
+        self.threads_by_paths
+            .entry(metadata.folder_paths.clone())
+            .or_default()
+            .insert(metadata.session_id.clone());
+
         self.pending_thread_ops_tx
-            .try_send(DbOperation::Insert(metadata))
+            .try_send(DbOperation::Upsert(metadata))
             .log_err();
     }
 
@@ -345,13 +335,10 @@ impl ThreadMetadataStore {
         }
 
         if let Some(thread) = self.threads.get(session_id) {
-            self.save(
-                ThreadMetadata {
-                    archived,
-                    ..thread.clone()
-                },
-                cx,
-            );
+            self.save_internal(ThreadMetadata {
+                archived,
+                ..thread.clone()
+            });
             cx.notify();
         }
     }
@@ -361,9 +348,16 @@ impl ThreadMetadataStore {
             return;
         }
 
+        if let Some(thread) = self.threads.get(&session_id)
+            && let Some(session_ids) = self.threads_by_paths.get_mut(&thread.folder_paths)
+        {
+            session_ids.remove(&session_id);
+        }
+        self.threads.remove(&session_id);
         self.pending_thread_ops_tx
             .try_send(DbOperation::Delete(session_id))
             .log_err();
+        cx.notify();
     }
 
     fn new(db: ThreadMetadataDb, cx: &mut Context<Self>) -> Self {
@@ -397,7 +391,7 @@ impl ThreadMetadataStore {
 
             weak_store
                 .update(cx, |this, cx| {
-                    let subscription = cx.subscribe(&thread_entity, Self::handle_thread_update);
+                    let subscription = cx.subscribe(&thread_entity, Self::handle_thread_event);
                     this.session_subscriptions
                         .insert(thread.session_id().clone(), subscription);
                 })
@@ -406,9 +400,9 @@ impl ThreadMetadataStore {
         .detach();
 
         let (tx, rx) = smol::channel::unbounded();
-        let _db_operations_task = cx.spawn({
+        let _db_operations_task = cx.background_spawn({
             let db = db.clone();
-            async move |this, cx| {
+            async move {
                 while let Ok(first_update) = rx.recv().await {
                     let mut updates = vec![first_update];
                     while let Ok(update) = rx.try_recv() {
@@ -417,7 +411,7 @@ impl ThreadMetadataStore {
                     let updates = Self::dedup_db_operations(updates);
                     for operation in updates {
                         match operation {
-                            DbOperation::Insert(metadata) => {
+                            DbOperation::Upsert(metadata) => {
                                 db.save(metadata).await.log_err();
                             }
                             DbOperation::Delete(session_id) => {
@@ -425,8 +419,6 @@ impl ThreadMetadataStore {
                             }
                         }
                     }
-
-                    this.update(cx, |this, cx| this.reload(cx)).ok();
                 }
             }
         });
@@ -455,10 +447,10 @@ impl ThreadMetadataStore {
         ops.into_values().collect()
     }
 
-    fn handle_thread_update(
+    fn handle_thread_event(
         &mut self,
         thread: Entity<acp_thread::AcpThread>,
-        event: &acp_thread::AcpThreadEvent,
+        event: &AcpThreadEvent,
         cx: &mut Context<Self>,
     ) {
         // Don't track subagent threads in the sidebar.
@@ -467,26 +459,62 @@ impl ThreadMetadataStore {
         }
 
         match event {
-            acp_thread::AcpThreadEvent::NewEntry
-            | acp_thread::AcpThreadEvent::TitleUpdated
-            | acp_thread::AcpThreadEvent::EntryUpdated(_)
-            | acp_thread::AcpThreadEvent::EntriesRemoved(_)
-            | acp_thread::AcpThreadEvent::ToolAuthorizationRequested(_)
-            | acp_thread::AcpThreadEvent::ToolAuthorizationReceived(_)
-            | acp_thread::AcpThreadEvent::Retry(_)
-            | acp_thread::AcpThreadEvent::Stopped(_)
-            | acp_thread::AcpThreadEvent::Error
-            | acp_thread::AcpThreadEvent::LoadError(_)
-            | acp_thread::AcpThreadEvent::Refusal => {
-                let is_archived = self
-                    .threads
-                    .get(thread.read(cx).session_id())
-                    .map(|t| t.archived)
-                    .unwrap_or(false);
-                let metadata = ThreadMetadata::from_thread(is_archived, &thread, cx);
+            AcpThreadEvent::NewEntry
+            | AcpThreadEvent::TitleUpdated
+            | AcpThreadEvent::EntryUpdated(_)
+            | AcpThreadEvent::EntriesRemoved(_)
+            | AcpThreadEvent::ToolAuthorizationRequested(_)
+            | AcpThreadEvent::ToolAuthorizationReceived(_)
+            | AcpThreadEvent::Retry(_)
+            | AcpThreadEvent::Stopped(_)
+            | AcpThreadEvent::Error
+            | AcpThreadEvent::LoadError(_)
+            | AcpThreadEvent::Refusal
+            | AcpThreadEvent::WorkingDirectoriesUpdated => {
+                let thread_ref = thread.read(cx);
+                let existing_thread = self.threads.get(thread_ref.session_id());
+                let session_id = thread_ref.session_id().clone();
+                let title = thread_ref
+                    .title()
+                    .unwrap_or_else(|| DEFAULT_THREAD_TITLE.into());
+
+                let updated_at = Utc::now();
+
+                let created_at = existing_thread
+                    .and_then(|t| t.created_at)
+                    .unwrap_or_else(|| updated_at);
+
+                let agent_id = thread_ref.connection().agent_id();
+
+                let folder_paths = {
+                    let project = thread_ref.project().read(cx);
+                    let paths: Vec<Arc<Path>> = project
+                        .visible_worktrees(cx)
+                        .map(|worktree| worktree.read(cx).abs_path())
+                        .collect();
+                    PathList::new(&paths)
+                };
+
+                let archived = existing_thread.map(|t| t.archived).unwrap_or(false);
+
+                let metadata = ThreadMetadata {
+                    session_id,
+                    agent_id,
+                    title,
+                    created_at: Some(created_at),
+                    updated_at,
+                    folder_paths,
+                    archived,
+                };
+
                 self.save(metadata, cx);
             }
-            _ => {}
+            AcpThreadEvent::TokenUsageUpdated
+            | AcpThreadEvent::SubagentSpawned(_)
+            | AcpThreadEvent::PromptCapabilitiesUpdated
+            | AcpThreadEvent::AvailableCommandsUpdated(_)
+            | AcpThreadEvent::ModeUpdated(_)
+            | AcpThreadEvent::ConfigOptionsUpdated(_) => {}
         }
     }
 }
@@ -559,6 +587,7 @@ impl ThreadMetadataDb {
                            agent_id = excluded.agent_id, \
                            title = excluded.title, \
                            updated_at = excluded.updated_at, \
+                           created_at = excluded.created_at, \
                            folder_paths = excluded.folder_paths, \
                            folder_paths_order = excluded.folder_paths_order, \
                            archived = excluded.archived";
@@ -688,6 +717,17 @@ mod tests {
         }
     }
 
+    fn init_test(cx: &mut TestAppContext) {
+        cx.update(|cx| {
+            let settings_store = settings::SettingsStore::test(cx);
+            cx.set_global(settings_store);
+            cx.update_flags(true, vec!["agent-v2".to_string()]);
+            ThreadMetadataStore::init_global(cx);
+            ThreadStore::init_global(cx);
+        });
+        cx.run_until_parked();
+    }
+
     #[gpui::test]
     async fn test_store_initializes_cache_from_database(cx: &mut TestAppContext) {
         let first_paths = PathList::new(&[Path::new("/project-a")]);
@@ -756,12 +796,7 @@ mod tests {
 
     #[gpui::test]
     async fn test_store_cache_updates_after_save_and_delete(cx: &mut TestAppContext) {
-        cx.update(|cx| {
-            let settings_store = settings::SettingsStore::test(cx);
-            cx.set_global(settings_store);
-            cx.update_flags(true, vec!["agent-v2".to_string()]);
-            ThreadMetadataStore::init_global(cx);
-        });
+        init_test(cx);
 
         let first_paths = PathList::new(&[Path::new("/project-a")]);
         let second_paths = PathList::new(&[Path::new("/project-b")]);
@@ -881,10 +916,7 @@ mod tests {
 
     #[gpui::test]
     async fn test_migrate_thread_metadata_migrates_only_missing_threads(cx: &mut TestAppContext) {
-        cx.update(|cx| {
-            ThreadStore::init_global(cx);
-            ThreadMetadataStore::init_global(cx);
-        });
+        init_test(cx);
 
         let project_a_paths = PathList::new(&[Path::new("/project-a")]);
         let project_b_paths = PathList::new(&[Path::new("/project-b")]);
@@ -959,7 +991,7 @@ mod tests {
 
         let list = cx.update(|cx| {
             let store = ThreadMetadataStore::global(cx);
-            store.read(cx).entries().collect::<Vec<_>>()
+            store.read(cx).entries().cloned().collect::<Vec<_>>()
         });
 
         assert_eq!(list.len(), 3);
@@ -999,10 +1031,7 @@ mod tests {
     async fn test_migrate_thread_metadata_noops_when_all_threads_already_exist(
         cx: &mut TestAppContext,
     ) {
-        cx.update(|cx| {
-            ThreadStore::init_global(cx);
-            ThreadMetadataStore::init_global(cx);
-        });
+        init_test(cx);
 
         let project_paths = PathList::new(&[Path::new("/project-a")]);
         let existing_updated_at = Utc::now();
@@ -1047,7 +1076,7 @@ mod tests {
 
         let list = cx.update(|cx| {
             let store = ThreadMetadataStore::global(cx);
-            store.read(cx).entries().collect::<Vec<_>>()
+            store.read(cx).entries().cloned().collect::<Vec<_>>()
         });
 
         assert_eq!(list.len(), 1);
@@ -1058,10 +1087,7 @@ mod tests {
     async fn test_migrate_thread_metadata_archives_beyond_five_most_recent_per_project(
         cx: &mut TestAppContext,
     ) {
-        cx.update(|cx| {
-            ThreadStore::init_global(cx);
-            ThreadMetadataStore::init_global(cx);
-        });
+        init_test(cx);
 
         let project_a_paths = PathList::new(&[Path::new("/project-a")]);
         let project_b_paths = PathList::new(&[Path::new("/project-b")]);
@@ -1110,7 +1136,7 @@ mod tests {
 
         let list = cx.update(|cx| {
             let store = ThreadMetadataStore::global(cx);
-            store.read(cx).entries().collect::<Vec<_>>()
+            store.read(cx).entries().cloned().collect::<Vec<_>>()
         });
 
         assert_eq!(list.len(), 10);
@@ -1149,13 +1175,7 @@ mod tests {
 
     #[gpui::test]
     async fn test_empty_thread_metadata_deleted_when_thread_released(cx: &mut TestAppContext) {
-        cx.update(|cx| {
-            let settings_store = settings::SettingsStore::test(cx);
-            cx.set_global(settings_store);
-            cx.update_flags(true, vec!["agent-v2".to_string()]);
-            ThreadStore::init_global(cx);
-            ThreadMetadataStore::init_global(cx);
-        });
+        init_test(cx);
 
         let fs = FakeFs::new(cx.executor());
         let project = Project::test(fs, None::<&Path>, cx).await;
@@ -1205,13 +1225,7 @@ mod tests {
 
     #[gpui::test]
     async fn test_nonempty_thread_metadata_preserved_when_thread_released(cx: &mut TestAppContext) {
-        cx.update(|cx| {
-            let settings_store = settings::SettingsStore::test(cx);
-            cx.set_global(settings_store);
-            cx.update_flags(true, vec!["agent-v2".to_string()]);
-            ThreadStore::init_global(cx);
-            ThreadMetadataStore::init_global(cx);
-        });
+        init_test(cx);
 
         let fs = FakeFs::new(cx.executor());
         let project = Project::test(fs, None::<&Path>, cx).await;
@@ -1257,13 +1271,7 @@ mod tests {
 
     #[gpui::test]
     async fn test_subagent_threads_excluded_from_sidebar_metadata(cx: &mut TestAppContext) {
-        cx.update(|cx| {
-            let settings_store = settings::SettingsStore::test(cx);
-            cx.set_global(settings_store);
-            cx.update_flags(true, vec!["agent-v2".to_string()]);
-            ThreadStore::init_global(cx);
-            ThreadMetadataStore::init_global(cx);
-        });
+        init_test(cx);
 
         let fs = FakeFs::new(cx.executor());
         let project = Project::test(fs, None::<&Path>, cx).await;
@@ -1321,7 +1329,7 @@ mod tests {
         // List all metadata from the store cache.
         let list = cx.update(|cx| {
             let store = ThreadMetadataStore::global(cx);
-            store.read(cx).entries().collect::<Vec<_>>()
+            store.read(cx).entries().cloned().collect::<Vec<_>>()
         });
 
         // The subagent thread should NOT appear in the sidebar metadata.
@@ -1342,7 +1350,7 @@ mod tests {
         let now = Utc::now();
 
         let operations = vec![
-            DbOperation::Insert(make_metadata(
+            DbOperation::Upsert(make_metadata(
                 "session-1",
                 "First Thread",
                 now,
@@ -1369,12 +1377,12 @@ mod tests {
         let new_metadata = make_metadata("session-1", "New Title", later, PathList::default());
 
         let deduped = ThreadMetadataStore::dedup_db_operations(vec![
-            DbOperation::Insert(old_metadata),
-            DbOperation::Insert(new_metadata.clone()),
+            DbOperation::Upsert(old_metadata),
+            DbOperation::Upsert(new_metadata.clone()),
         ]);
 
         assert_eq!(deduped.len(), 1);
-        assert_eq!(deduped[0], DbOperation::Insert(new_metadata));
+        assert_eq!(deduped[0], DbOperation::Upsert(new_metadata));
     }
 
     #[test]
@@ -1384,23 +1392,18 @@ mod tests {
         let metadata1 = make_metadata("session-1", "First Thread", now, PathList::default());
         let metadata2 = make_metadata("session-2", "Second Thread", now, PathList::default());
         let deduped = ThreadMetadataStore::dedup_db_operations(vec![
-            DbOperation::Insert(metadata1.clone()),
-            DbOperation::Insert(metadata2.clone()),
+            DbOperation::Upsert(metadata1.clone()),
+            DbOperation::Upsert(metadata2.clone()),
         ]);
 
         assert_eq!(deduped.len(), 2);
-        assert!(deduped.contains(&DbOperation::Insert(metadata1)));
-        assert!(deduped.contains(&DbOperation::Insert(metadata2)));
+        assert!(deduped.contains(&DbOperation::Upsert(metadata1)));
+        assert!(deduped.contains(&DbOperation::Upsert(metadata2)));
     }
 
     #[gpui::test]
     async fn test_archive_and_unarchive_thread(cx: &mut TestAppContext) {
-        cx.update(|cx| {
-            let settings_store = settings::SettingsStore::test(cx);
-            cx.set_global(settings_store);
-            cx.update_flags(true, vec!["agent-v2".to_string()]);
-            ThreadMetadataStore::init_global(cx);
-        });
+        init_test(cx);
 
         let paths = PathList::new(&[Path::new("/project-a")]);
         let now = Utc::now();
@@ -1486,12 +1489,7 @@ mod tests {
 
     #[gpui::test]
     async fn test_entries_for_path_excludes_archived(cx: &mut TestAppContext) {
-        cx.update(|cx| {
-            let settings_store = settings::SettingsStore::test(cx);
-            cx.set_global(settings_store);
-            cx.update_flags(true, vec!["agent-v2".to_string()]);
-            ThreadMetadataStore::init_global(cx);
-        });
+        init_test(cx);
 
         let paths = PathList::new(&[Path::new("/project-a")]);
         let now = Utc::now();
@@ -1551,12 +1549,7 @@ mod tests {
 
     #[gpui::test]
     async fn test_save_all_persists_multiple_threads(cx: &mut TestAppContext) {
-        cx.update(|cx| {
-            let settings_store = settings::SettingsStore::test(cx);
-            cx.set_global(settings_store);
-            cx.update_flags(true, vec!["agent-v2".to_string()]);
-            ThreadMetadataStore::init_global(cx);
-        });
+        init_test(cx);
 
         let paths = PathList::new(&[Path::new("/project-a")]);
         let now = Utc::now();
@@ -1604,12 +1597,7 @@ mod tests {
 
     #[gpui::test]
     async fn test_archived_flag_persists_across_reload(cx: &mut TestAppContext) {
-        cx.update(|cx| {
-            let settings_store = settings::SettingsStore::test(cx);
-            cx.set_global(settings_store);
-            cx.update_flags(true, vec!["agent-v2".to_string()]);
-            ThreadMetadataStore::init_global(cx);
-        });
+        init_test(cx);
 
         let paths = PathList::new(&[Path::new("/project-a")]);
         let now = Utc::now();
@@ -1668,12 +1656,7 @@ mod tests {
 
     #[gpui::test]
     async fn test_archive_nonexistent_thread_is_noop(cx: &mut TestAppContext) {
-        cx.update(|cx| {
-            let settings_store = settings::SettingsStore::test(cx);
-            cx.set_global(settings_store);
-            cx.update_flags(true, vec!["agent-v2".to_string()]);
-            ThreadMetadataStore::init_global(cx);
-        });
+        init_test(cx);
 
         cx.run_until_parked();
 
@@ -1695,4 +1678,38 @@ mod tests {
             assert_eq!(store.archived_entries().count(), 0);
         });
     }
+
+    #[gpui::test]
+    async fn test_save_followed_by_archiving_without_parking(cx: &mut TestAppContext) {
+        init_test(cx);
+
+        let paths = PathList::new(&[Path::new("/project-a")]);
+        let now = Utc::now();
+        let metadata = make_metadata("session-1", "Thread 1", now, paths);
+        let session_id = metadata.session_id.clone();
+
+        cx.update(|cx| {
+            let store = ThreadMetadataStore::global(cx);
+            store.update(cx, |store, cx| {
+                store.save(metadata.clone(), cx);
+                store.archive(&session_id, cx);
+            });
+        });
+
+        cx.run_until_parked();
+
+        cx.update(|cx| {
+            let store = ThreadMetadataStore::global(cx);
+            let store = store.read(cx);
+
+            let entries: Vec<ThreadMetadata> = store.entries().cloned().collect();
+            pretty_assertions::assert_eq!(
+                entries,
+                vec![ThreadMetadata {
+                    archived: true,
+                    ..metadata
+                }]
+            );
+        });
+    }
 }

crates/agent_ui/src/threads_archive_view.rs 🔗

@@ -214,6 +214,7 @@ impl ThreadsArchiveView {
             .archived_entries()
             .sorted_by_cached_key(|t| t.created_at.unwrap_or(t.updated_at))
             .rev()
+            .cloned()
             .collect::<Vec<_>>();
 
         let query = self.filter_editor.read(cx).text(cx).to_lowercase();

crates/sidebar/src/sidebar.rs 🔗

@@ -824,6 +824,7 @@ impl Sidebar {
                     let mut workspace_rows = thread_store
                         .read(cx)
                         .entries_for_path(&ws_path_list)
+                        .cloned()
                         .peekable();
                     if workspace_rows.peek().is_none() {
                         let worktrees =
@@ -872,7 +873,11 @@ impl Sidebar {
                     });
 
                 for worktree_path_list in linked_worktree_queries {
-                    for row in thread_store.read(cx).entries_for_path(&worktree_path_list) {
+                    for row in thread_store
+                        .read(cx)
+                        .entries_for_path(&worktree_path_list)
+                        .cloned()
+                    {
                         if !seen_session_ids.insert(row.session_id.clone()) {
                             continue;
                         }

crates/sidebar/src/sidebar_tests.rs 🔗

@@ -92,10 +92,10 @@ async fn save_n_test_threads(count: u32, path_list: &PathList, cx: &mut gpui::Vi
             acp::SessionId::new(Arc::from(format!("thread-{}", i))),
             format!("Thread {}", i + 1).into(),
             chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, i).unwrap(),
+            None,
             path_list.clone(),
             cx,
         )
-        .await;
     }
     cx.run_until_parked();
 }
@@ -109,10 +109,10 @@ async fn save_test_thread_metadata(
         session_id.clone(),
         "Test".into(),
         chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(),
+        None,
         path_list,
         cx,
     )
-    .await;
 }
 
 async fn save_named_thread_metadata(
@@ -125,17 +125,18 @@ async fn save_named_thread_metadata(
         acp::SessionId::new(Arc::from(session_id)),
         SharedString::from(title.to_string()),
         chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(),
+        None,
         path_list.clone(),
         cx,
-    )
-    .await;
+    );
     cx.run_until_parked();
 }
 
-async fn save_thread_metadata(
+fn save_thread_metadata(
     session_id: acp::SessionId,
     title: SharedString,
     updated_at: DateTime<Utc>,
+    created_at: Option<DateTime<Utc>>,
     path_list: PathList,
     cx: &mut TestAppContext,
 ) {
@@ -144,12 +145,12 @@ async fn save_thread_metadata(
         agent_id: agent::ZED_AGENT_ID.clone(),
         title,
         updated_at,
-        created_at: None,
+        created_at,
         folder_paths: path_list,
         archived: false,
     };
     cx.update(|cx| {
-        ThreadMetadataStore::global(cx).update(cx, |store, cx| store.save(metadata, cx))
+        ThreadMetadataStore::global(cx).update(cx, |store, cx| store.save_manually(metadata, cx))
     });
     cx.run_until_parked();
 }
@@ -407,19 +408,19 @@ async fn test_single_workspace_with_saved_threads(cx: &mut TestAppContext) {
         acp::SessionId::new(Arc::from("thread-1")),
         "Fix crash in project panel".into(),
         chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 3, 0, 0, 0).unwrap(),
+        None,
         path_list.clone(),
         cx,
-    )
-    .await;
+    );
 
     save_thread_metadata(
         acp::SessionId::new(Arc::from("thread-2")),
         "Add inline diff view".into(),
         chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 2, 0, 0, 0).unwrap(),
-        path_list.clone(),
+        None,
+        path_list,
         cx,
-    )
-    .await;
+    );
     cx.run_until_parked();
 
     multi_workspace.update_in(cx, |_, _window, cx| cx.notify());
@@ -449,10 +450,10 @@ async fn test_workspace_lifecycle(cx: &mut TestAppContext) {
         acp::SessionId::new(Arc::from("thread-a1")),
         "Thread A1".into(),
         chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(),
-        path_list.clone(),
+        None,
+        path_list,
         cx,
-    )
-    .await;
+    );
     cx.run_until_parked();
 
     multi_workspace.update_in(cx, |_, _window, cx| cx.notify());
@@ -1331,10 +1332,10 @@ async fn test_search_narrows_visible_threads_to_matches(cx: &mut TestAppContext)
             acp::SessionId::new(Arc::from(id)),
             title.into(),
             chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, hour, 0, 0).unwrap(),
+            None,
             path_list.clone(),
             cx,
-        )
-        .await;
+        );
     }
     cx.run_until_parked();
 
@@ -1379,10 +1380,10 @@ async fn test_search_matches_regardless_of_case(cx: &mut TestAppContext) {
         acp::SessionId::new(Arc::from("thread-1")),
         "Fix Crash In Project Panel".into(),
         chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(),
-        path_list.clone(),
+        None,
+        path_list,
         cx,
-    )
-    .await;
+    );
     cx.run_until_parked();
 
     // Lowercase query matches mixed-case title.
@@ -1422,10 +1423,10 @@ async fn test_escape_clears_search_and_restores_full_list(cx: &mut TestAppContex
             acp::SessionId::new(Arc::from(id)),
             title.into(),
             chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, hour, 0, 0).unwrap(),
+            None,
             path_list.clone(),
             cx,
         )
-        .await;
     }
     cx.run_until_parked();
 
@@ -1474,10 +1475,10 @@ async fn test_search_only_shows_workspace_headers_with_matches(cx: &mut TestAppC
             acp::SessionId::new(Arc::from(id)),
             title.into(),
             chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, hour, 0, 0).unwrap(),
+            None,
             path_list_a.clone(),
             cx,
         )
-        .await;
     }
 
     // Add a second workspace.
@@ -1496,10 +1497,10 @@ async fn test_search_only_shows_workspace_headers_with_matches(cx: &mut TestAppC
             acp::SessionId::new(Arc::from(id)),
             title.into(),
             chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, hour, 0, 0).unwrap(),
+            None,
             path_list_b.clone(),
             cx,
         )
-        .await;
     }
     cx.run_until_parked();
 
@@ -1556,10 +1557,10 @@ async fn test_search_matches_workspace_name(cx: &mut TestAppContext) {
             acp::SessionId::new(Arc::from(id)),
             title.into(),
             chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, hour, 0, 0).unwrap(),
+            None,
             path_list_a.clone(),
             cx,
         )
-        .await;
     }
 
     // Add a second workspace.
@@ -1578,10 +1579,10 @@ async fn test_search_matches_workspace_name(cx: &mut TestAppContext) {
             acp::SessionId::new(Arc::from(id)),
             title.into(),
             chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, hour, 0, 0).unwrap(),
+            None,
             path_list_b.clone(),
             cx,
         )
-        .await;
     }
     cx.run_until_parked();
 
@@ -1662,10 +1663,10 @@ async fn test_search_finds_threads_hidden_behind_view_more(cx: &mut TestAppConte
             acp::SessionId::new(Arc::from(format!("thread-{}", i))),
             title.into(),
             chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, i).unwrap(),
+            None,
             path_list.clone(),
             cx,
         )
-        .await;
     }
     cx.run_until_parked();
 
@@ -1706,10 +1707,10 @@ async fn test_search_finds_threads_inside_collapsed_groups(cx: &mut TestAppConte
         acp::SessionId::new(Arc::from("thread-1")),
         "Important thread".into(),
         chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(),
-        path_list.clone(),
+        None,
+        path_list,
         cx,
-    )
-    .await;
+    );
     cx.run_until_parked();
 
     // User focuses the sidebar and collapses the group using keyboard:
@@ -1752,10 +1753,10 @@ async fn test_search_then_keyboard_navigate_and_confirm(cx: &mut TestAppContext)
             acp::SessionId::new(Arc::from(id)),
             title.into(),
             chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, hour, 0, 0).unwrap(),
+            None,
             path_list.clone(),
             cx,
         )
-        .await;
     }
     cx.run_until_parked();
 
@@ -1814,10 +1815,10 @@ async fn test_confirm_on_historical_thread_activates_workspace(cx: &mut TestAppC
         acp::SessionId::new(Arc::from("hist-1")),
         "Historical Thread".into(),
         chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 6, 1, 0, 0, 0).unwrap(),
-        path_list.clone(),
+        None,
+        path_list,
         cx,
-    )
-    .await;
+    );
     cx.run_until_parked();
     multi_workspace.update_in(cx, |_, _window, cx| cx.notify());
     cx.run_until_parked();
@@ -1867,19 +1868,19 @@ async fn test_click_clears_selection_and_focus_in_restores_it(cx: &mut TestAppCo
         acp::SessionId::new(Arc::from("t-1")),
         "Thread A".into(),
         chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 2, 0, 0, 0).unwrap(),
+        None,
         path_list.clone(),
         cx,
-    )
-    .await;
+    );
 
     save_thread_metadata(
         acp::SessionId::new(Arc::from("t-2")),
         "Thread B".into(),
         chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(),
-        path_list.clone(),
+        None,
+        path_list,
         cx,
-    )
-    .await;
+    );
 
     cx.run_until_parked();
     multi_workspace.update_in(cx, |_, _window, cx| cx.notify());
@@ -4226,22 +4227,22 @@ async fn test_archive_thread_uses_next_threads_own_workspace(cx: &mut TestAppCon
         thread2_session_id.clone(),
         "Thread 2".into(),
         chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 2, 0, 0, 0).unwrap(),
+        None,
         PathList::new(&[std::path::PathBuf::from("/project")]),
         cx,
-    )
-    .await;
+    );
 
     // Save thread 1's metadata with the worktree path and an older timestamp so
     // it sorts below thread 2. archive_thread will find it as the "next" candidate.
     let thread1_session_id = acp::SessionId::new(Arc::from("thread1-worktree-session"));
     save_thread_metadata(
-        thread1_session_id.clone(),
+        thread1_session_id,
         "Thread 1".into(),
         chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(),
+        None,
         PathList::new(&[std::path::PathBuf::from("/wt-feature-a")]),
         cx,
-    )
-    .await;
+    );
 
     cx.run_until_parked();
 
@@ -4439,26 +4440,14 @@ async fn test_thread_switcher_ordering(cx: &mut TestAppContext) {
     open_thread_with_connection(&panel, connection_c, cx);
     send_message(&panel, cx);
     let session_id_c = active_session_id(&panel, cx);
-    cx.update(|_, cx| {
-        ThreadMetadataStore::global(cx).update(cx, |store, cx| {
-            store.save(
-                ThreadMetadata {
-                    session_id: session_id_c.clone(),
-                    agent_id: agent::ZED_AGENT_ID.clone(),
-                    title: "Thread C".into(),
-                    updated_at: chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0)
-                        .unwrap(),
-                    created_at: Some(
-                        chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(),
-                    ),
-                    folder_paths: path_list.clone(),
-                    archived: false,
-                },
-                cx,
-            )
-        })
-    });
-    cx.run_until_parked();
+    save_thread_metadata(
+        session_id_c.clone(),
+        "Thread C".into(),
+        chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(),
+        Some(chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap()),
+        path_list.clone(),
+        cx,
+    );
 
     let connection_b = StubAgentConnection::new();
     connection_b.set_next_prompt_updates(vec![acp::SessionUpdate::AgentMessageChunk(
@@ -4467,26 +4456,14 @@ async fn test_thread_switcher_ordering(cx: &mut TestAppContext) {
     open_thread_with_connection(&panel, connection_b, cx);
     send_message(&panel, cx);
     let session_id_b = active_session_id(&panel, cx);
-    cx.update(|_, cx| {
-        ThreadMetadataStore::global(cx).update(cx, |store, cx| {
-            store.save(
-                ThreadMetadata {
-                    session_id: session_id_b.clone(),
-                    agent_id: agent::ZED_AGENT_ID.clone(),
-                    title: "Thread B".into(),
-                    updated_at: chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 2, 0, 0, 0)
-                        .unwrap(),
-                    created_at: Some(
-                        chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 2, 0, 0, 0).unwrap(),
-                    ),
-                    folder_paths: path_list.clone(),
-                    archived: false,
-                },
-                cx,
-            )
-        })
-    });
-    cx.run_until_parked();
+    save_thread_metadata(
+        session_id_b.clone(),
+        "Thread B".into(),
+        chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 2, 0, 0, 0).unwrap(),
+        Some(chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 2, 0, 0, 0).unwrap()),
+        path_list.clone(),
+        cx,
+    );
 
     let connection_a = StubAgentConnection::new();
     connection_a.set_next_prompt_updates(vec![acp::SessionUpdate::AgentMessageChunk(
@@ -4495,26 +4472,14 @@ async fn test_thread_switcher_ordering(cx: &mut TestAppContext) {
     open_thread_with_connection(&panel, connection_a, cx);
     send_message(&panel, cx);
     let session_id_a = active_session_id(&panel, cx);
-    cx.update(|_, cx| {
-        ThreadMetadataStore::global(cx).update(cx, |store, cx| {
-            store.save(
-                ThreadMetadata {
-                    session_id: session_id_a.clone(),
-                    agent_id: agent::ZED_AGENT_ID.clone(),
-                    title: "Thread A".into(),
-                    updated_at: chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 3, 0, 0, 0)
-                        .unwrap(),
-                    created_at: Some(
-                        chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 3, 0, 0, 0).unwrap(),
-                    ),
-                    folder_paths: path_list.clone(),
-                    archived: false,
-                },
-                cx,
-            )
-        })
-    });
-    cx.run_until_parked();
+    save_thread_metadata(
+        session_id_a.clone(),
+        "Thread A".into(),
+        chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 3, 0, 0, 0).unwrap(),
+        Some(chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 3, 0, 0, 0).unwrap()),
+        path_list.clone(),
+        cx,
+    );
 
     // All three threads are now live. Thread A was opened last, so it's
     // the one being viewed. Opening each thread called record_thread_access,
@@ -4569,6 +4534,8 @@ async fn test_thread_switcher_ordering(cx: &mut TestAppContext) {
     cx.run_until_parked();
     assert_eq!(switcher_selected_id(&sidebar, cx), session_id_c);
 
+    assert!(sidebar.update(cx, |sidebar, _cx| sidebar.thread_last_accessed.is_empty()));
+
     // Confirm on Thread C.
     sidebar.update_in(cx, |sidebar, window, cx| {
         let switcher = sidebar.thread_switcher.as_ref().unwrap();
@@ -4585,7 +4552,23 @@ async fn test_thread_switcher_ordering(cx: &mut TestAppContext) {
         );
     });
 
-    // Re-open switcher: Thread C is now most-recently-accessed.
+    sidebar.update(cx, |sidebar, _cx| {
+        let last_accessed = sidebar
+            .thread_last_accessed
+            .keys()
+            .cloned()
+            .collect::<Vec<_>>();
+        assert_eq!(last_accessed.len(), 1);
+        assert!(last_accessed.contains(&session_id_c));
+        assert!(
+            sidebar
+                .active_entry
+                .as_ref()
+                .expect("active_entry should be set")
+                .is_active_thread(&session_id_c)
+        );
+    });
+
     sidebar.update_in(cx, |sidebar, window, cx| {
         sidebar.on_toggle_thread_switcher(&ToggleThreadSwitcher::default(), window, cx);
     });
@@ -4600,34 +4583,90 @@ async fn test_thread_switcher_ordering(cx: &mut TestAppContext) {
         ],
     );
 
+    // Confirm on Thread A.
+    sidebar.update_in(cx, |sidebar, window, cx| {
+        let switcher = sidebar.thread_switcher.as_ref().unwrap();
+        let focus = switcher.focus_handle(cx);
+        focus.dispatch_action(&menu::Confirm, window, cx);
+    });
+    cx.run_until_parked();
+
+    sidebar.update(cx, |sidebar, _cx| {
+        let last_accessed = sidebar
+            .thread_last_accessed
+            .keys()
+            .cloned()
+            .collect::<Vec<_>>();
+        assert_eq!(last_accessed.len(), 2);
+        assert!(last_accessed.contains(&session_id_c));
+        assert!(last_accessed.contains(&session_id_a));
+        assert!(
+            sidebar
+                .active_entry
+                .as_ref()
+                .expect("active_entry should be set")
+                .is_active_thread(&session_id_a)
+        );
+    });
+
+    sidebar.update_in(cx, |sidebar, window, cx| {
+        sidebar.on_toggle_thread_switcher(&ToggleThreadSwitcher::default(), window, cx);
+    });
+    cx.run_until_parked();
+
+    assert_eq!(
+        switcher_ids(&sidebar, cx),
+        vec![
+            session_id_a.clone(),
+            session_id_c.clone(),
+            session_id_b.clone(),
+        ],
+    );
+
     sidebar.update_in(cx, |sidebar, _window, cx| {
-        sidebar.dismiss_thread_switcher(cx);
+        let switcher = sidebar.thread_switcher.as_ref().unwrap();
+        switcher.update(cx, |switcher, cx| switcher.cycle_selection(cx));
     });
     cx.run_until_parked();
 
-    // ── 3. Add a historical thread (no last_accessed_at, no message sent) ──
-    // This thread was never opened in a panel — it only exists in metadata.
-    cx.update(|_, cx| {
-        ThreadMetadataStore::global(cx).update(cx, |store, cx| {
-            store.save(
-                ThreadMetadata {
-                    session_id: acp::SessionId::new(Arc::from("thread-historical")),
-                    agent_id: agent::ZED_AGENT_ID.clone(),
-                    title: "Historical Thread".into(),
-                    updated_at: chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 6, 1, 0, 0, 0)
-                        .unwrap(),
-                    created_at: Some(
-                        chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 6, 1, 0, 0, 0).unwrap(),
-                    ),
-                    folder_paths: path_list.clone(),
-                    archived: false,
-                },
-                cx,
-            )
-        })
+    // Confirm on Thread B.
+    sidebar.update_in(cx, |sidebar, window, cx| {
+        let switcher = sidebar.thread_switcher.as_ref().unwrap();
+        let focus = switcher.focus_handle(cx);
+        focus.dispatch_action(&menu::Confirm, window, cx);
     });
     cx.run_until_parked();
 
+    sidebar.update(cx, |sidebar, _cx| {
+        let last_accessed = sidebar
+            .thread_last_accessed
+            .keys()
+            .cloned()
+            .collect::<Vec<_>>();
+        assert_eq!(last_accessed.len(), 3);
+        assert!(last_accessed.contains(&session_id_c));
+        assert!(last_accessed.contains(&session_id_a));
+        assert!(last_accessed.contains(&session_id_b));
+        assert!(
+            sidebar
+                .active_entry
+                .as_ref()
+                .expect("active_entry should be set")
+                .is_active_thread(&session_id_b)
+        );
+    });
+
+    // ── 3. Add a historical thread (no last_accessed_at, no message sent) ──
+    // This thread was never opened in a panel — it only exists in metadata.
+    save_thread_metadata(
+        acp::SessionId::new(Arc::from("thread-historical")),
+        "Historical Thread".into(),
+        chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 6, 1, 0, 0, 0).unwrap(),
+        Some(chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 6, 1, 0, 0, 0).unwrap()),
+        path_list.clone(),
+        cx,
+    );
+
     sidebar.update_in(cx, |sidebar, window, cx| {
         sidebar.on_toggle_thread_switcher(&ToggleThreadSwitcher::default(), window, cx);
     });
@@ -4642,13 +4681,14 @@ async fn test_thread_switcher_ordering(cx: &mut TestAppContext) {
     // last_message_sent_or_queued. So for the accessed threads (tier 1) the
     // sort key is last_accessed_at; for Historical Thread (tier 3) it's created_at.
     let session_id_hist = acp::SessionId::new(Arc::from("thread-historical"));
+
     let ids = switcher_ids(&sidebar, cx);
     assert_eq!(
         ids,
         vec![
-            session_id_c.clone(),
-            session_id_a.clone(),
             session_id_b.clone(),
+            session_id_a.clone(),
+            session_id_c.clone(),
             session_id_hist.clone()
         ],
     );
@@ -4659,26 +4699,14 @@ async fn test_thread_switcher_ordering(cx: &mut TestAppContext) {
     cx.run_until_parked();
 
     // ── 4. Add another historical thread with older created_at ─────────
-    cx.update(|_, cx| {
-        ThreadMetadataStore::global(cx).update(cx, |store, cx| {
-            store.save(
-                ThreadMetadata {
-                    session_id: acp::SessionId::new(Arc::from("thread-old-historical")),
-                    agent_id: agent::ZED_AGENT_ID.clone(),
-                    title: "Old Historical Thread".into(),
-                    updated_at: chrono::TimeZone::with_ymd_and_hms(&Utc, 2023, 6, 1, 0, 0, 0)
-                        .unwrap(),
-                    created_at: Some(
-                        chrono::TimeZone::with_ymd_and_hms(&Utc, 2023, 6, 1, 0, 0, 0).unwrap(),
-                    ),
-                    folder_paths: path_list.clone(),
-                    archived: false,
-                },
-                cx,
-            )
-        })
-    });
-    cx.run_until_parked();
+    save_thread_metadata(
+        acp::SessionId::new(Arc::from("thread-old-historical")),
+        "Old Historical Thread".into(),
+        chrono::TimeZone::with_ymd_and_hms(&Utc, 2023, 6, 1, 0, 0, 0).unwrap(),
+        Some(chrono::TimeZone::with_ymd_and_hms(&Utc, 2023, 6, 1, 0, 0, 0).unwrap()),
+        path_list,
+        cx,
+    );
 
     sidebar.update_in(cx, |sidebar, window, cx| {
         sidebar.on_toggle_thread_switcher(&ToggleThreadSwitcher::default(), window, cx);
@@ -4692,9 +4720,9 @@ async fn test_thread_switcher_ordering(cx: &mut TestAppContext) {
     assert_eq!(
         ids,
         vec![
-            session_id_c.clone(),
-            session_id_a.clone(),
-            session_id_b.clone(),
+            session_id_b,
+            session_id_a,
+            session_id_c,
             session_id_hist,
             session_id_old_hist,
         ],
@@ -4719,10 +4747,10 @@ async fn test_archive_thread_keeps_metadata_but_hides_from_sidebar(cx: &mut Test
         acp::SessionId::new(Arc::from("thread-to-archive")),
         "Thread To Archive".into(),
         chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(),
-        path_list.clone(),
+        None,
+        path_list,
         cx,
-    )
-    .await;
+    );
     cx.run_until_parked();
 
     multi_workspace.update_in(cx, |_, _window, cx| cx.notify());
@@ -4771,22 +4799,25 @@ async fn test_archived_threads_excluded_from_sidebar_entries(cx: &mut TestAppCon
         acp::SessionId::new(Arc::from("visible-thread")),
         "Visible Thread".into(),
         chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 2, 0, 0, 0).unwrap(),
+        None,
         path_list.clone(),
         cx,
-    )
-    .await;
+    );
+
+    let archived_thread_session_id = acp::SessionId::new(Arc::from("archived-thread"));
+    save_thread_metadata(
+        archived_thread_session_id.clone(),
+        "Archived Thread".into(),
+        chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(),
+        None,
+        path_list,
+        cx,
+    );
 
     cx.update(|_, cx| {
-        let metadata = ThreadMetadata {
-            session_id: acp::SessionId::new(Arc::from("archived-thread")),
-            agent_id: agent::ZED_AGENT_ID.clone(),
-            title: "Archived Thread".into(),
-            updated_at: chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(),
-            created_at: None,
-            folder_paths: path_list.clone(),
-            archived: true,
-        };
-        ThreadMetadataStore::global(cx).update(cx, |store, cx| store.save(metadata, cx));
+        ThreadMetadataStore::global(cx).update(cx, |store, cx| {
+            store.archive(&archived_thread_session_id, cx)
+        })
     });
     cx.run_until_parked();
 
@@ -4962,18 +4993,7 @@ mod property_test {
         let updated_at = chrono::TimeZone::with_ymd_and_hms(&chrono::Utc, 2024, 1, 1, 0, 0, 0)
             .unwrap()
             + chrono::Duration::seconds(state.thread_counter as i64);
-        let metadata = ThreadMetadata {
-            session_id,
-            agent_id: agent::ZED_AGENT_ID.clone(),
-            title,
-            updated_at,
-            created_at: None,
-            folder_paths: path_list,
-            archived: false,
-        };
-        cx.update(|_, cx| {
-            ThreadMetadataStore::global(cx).update(cx, |store, cx| store.save(metadata, cx));
-        });
+        save_thread_metadata(session_id, title, updated_at, None, path_list, cx);
     }
 
     async fn perform_operation(