From 80a053ed2a56f18875c389f2190cedcd4306f5fe Mon Sep 17 00:00:00 2001 From: Cameron Mcloughlin Date: Fri, 17 Apr 2026 14:43:50 +0100 Subject: [PATCH] sidebar: Fix sidebar thread times (#54173) Co-authored-by: Ben Brandt --- crates/agent_ui/src/thread_import.rs | 1 + crates/agent_ui/src/thread_metadata_store.rs | 48 +++++- crates/sidebar/src/sidebar.rs | 92 +++++------ crates/sidebar/src/sidebar_tests.rs | 165 ++++++++++++++----- 4 files changed, 209 insertions(+), 97 deletions(-) diff --git a/crates/agent_ui/src/thread_import.rs b/crates/agent_ui/src/thread_import.rs index 944813525b40ed0013972595f992bf96d1876fab..a8bd95916a7111afe4a7a75f11ff78f3547b4398 100644 --- a/crates/agent_ui/src/thread_import.rs +++ b/crates/agent_ui/src/thread_import.rs @@ -572,6 +572,7 @@ fn collect_importable_threads( title: session.title, updated_at: session.updated_at.unwrap_or_else(|| Utc::now()), created_at: session.created_at, + interacted_at: None, worktree_paths: WorktreePaths::from_folder_paths(&folder_paths), remote_connection: remote_connection.clone(), archived: true, diff --git a/crates/agent_ui/src/thread_metadata_store.rs b/crates/agent_ui/src/thread_metadata_store.rs index 0b99694159cd1c51bb7776336d6d7b2f6e03341d..475c4908d39dcb56f5c509c6e9b3c619b6543226 100644 --- a/crates/agent_ui/src/thread_metadata_store.rs +++ b/crates/agent_ui/src/thread_metadata_store.rs @@ -125,6 +125,7 @@ fn migrate_thread_metadata(cx: &mut App) -> Task> { }, updated_at: entry.updated_at, created_at: entry.created_at, + interacted_at: None, worktree_paths: WorktreePaths::from_folder_paths(&entry.folder_paths), remote_connection: None, archived: true, @@ -294,6 +295,9 @@ pub struct ThreadMetadata { pub title: Option, pub updated_at: DateTime, pub created_at: Option>, + /// When a user last interacted to send a message (including queueing). + /// Doesn't include the time when a queued message is fired. + pub interacted_at: Option>, pub worktree_paths: WorktreePaths, pub remote_connection: Option, pub archived: bool, @@ -750,6 +754,21 @@ impl ThreadMetadataStore { } } + pub fn update_interacted_at( + &mut self, + thread_id: &ThreadId, + time: DateTime, + cx: &mut Context, + ) { + if let Some(thread) = self.threads.get(thread_id) { + self.save_internal(ThreadMetadata { + interacted_at: Some(time), + ..thread.clone() + }); + cx.notify(); + }; + } + pub fn archive( &mut self, thread_id: ThreadId, @@ -1152,6 +1171,8 @@ impl ThreadMetadataStore { .and_then(|t| t.created_at) .unwrap_or_else(|| updated_at); + let interacted_at = existing_thread.and_then(|t| t.interacted_at); + let agent_id = thread_ref.connection().agent_id(); // Preserve project-dependent fields for archived threads. @@ -1187,6 +1208,7 @@ impl ThreadMetadataStore { agent_id, title, created_at: Some(created_at), + interacted_at, updated_at, worktree_paths, remote_connection, @@ -1290,6 +1312,9 @@ impl Domain for ThreadMetadataDb { SELECT archived_worktree_id FROM thread_archived_worktrees ); ), + sql!( + ALTER TABLE sidebar_threads ADD COLUMN interacted_at TEXT; + ), ]; } @@ -1306,7 +1331,7 @@ impl ThreadMetadataDb { } const LIST_QUERY: &str = "SELECT thread_id, session_id, agent_id, title, updated_at, \ - created_at, folder_paths, folder_paths_order, archived, main_worktree_paths, \ + created_at, interacted_at, folder_paths, folder_paths_order, archived, main_worktree_paths, \ main_worktree_paths_order, remote_connection \ FROM sidebar_threads \ WHERE session_id IS NOT NULL \ @@ -1339,6 +1364,7 @@ impl ThreadMetadataDb { .unwrap_or_default(); let updated_at = row.updated_at.to_rfc3339(); let created_at = row.created_at.map(|dt| dt.to_rfc3339()); + let interacted_at = row.interacted_at.map(|dt| dt.to_rfc3339()); let serialized = row.folder_paths().serialize(); let (folder_paths, folder_paths_order) = if row.folder_paths().is_empty() { (None, None) @@ -1362,14 +1388,15 @@ impl ThreadMetadataDb { let archived = row.archived; self.write(move |conn| { - let sql = "INSERT INTO sidebar_threads(thread_id, session_id, agent_id, title, updated_at, created_at, folder_paths, folder_paths_order, archived, main_worktree_paths, main_worktree_paths_order, remote_connection) \ - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12) \ + let sql = "INSERT INTO sidebar_threads(thread_id, session_id, agent_id, title, updated_at, created_at, interacted_at, folder_paths, folder_paths_order, archived, main_worktree_paths, main_worktree_paths_order, remote_connection) \ + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13) \ ON CONFLICT(thread_id) DO UPDATE SET \ session_id = excluded.session_id, \ agent_id = excluded.agent_id, \ title = excluded.title, \ updated_at = excluded.updated_at, \ created_at = excluded.created_at, \ + interacted_at = excluded.interacted_at, \ folder_paths = excluded.folder_paths, \ folder_paths_order = excluded.folder_paths_order, \ archived = excluded.archived, \ @@ -1383,6 +1410,7 @@ impl ThreadMetadataDb { i = stmt.bind(&title, i)?; i = stmt.bind(&updated_at, i)?; i = stmt.bind(&created_at, i)?; + i = stmt.bind(&interacted_at, i)?; i = stmt.bind(&folder_paths, i)?; i = stmt.bind(&folder_paths_order, i)?; i = stmt.bind(&archived, i)?; @@ -1534,6 +1562,7 @@ impl Column for ThreadMetadata { let (title, next): (String, i32) = Column::column(statement, next)?; let (updated_at_str, next): (String, i32) = Column::column(statement, next)?; let (created_at_str, next): (Option, i32) = Column::column(statement, next)?; + let (interacted_at_str, next): (Option, i32) = Column::column(statement, next)?; let (folder_paths_str, next): (Option, i32) = Column::column(statement, next)?; let (folder_paths_order_str, next): (Option, i32) = Column::column(statement, next)?; @@ -1556,6 +1585,12 @@ impl Column for ThreadMetadata { .transpose()? .map(|dt| dt.with_timezone(&Utc)); + let interacted_at = interacted_at_str + .as_deref() + .map(DateTime::parse_from_rfc3339) + .transpose()? + .map(|dt| dt.with_timezone(&Utc)); + let folder_paths = folder_paths_str .map(|paths| { PathList::deserialize(&util::path_list::SerializedPathList { @@ -1597,6 +1632,7 @@ impl Column for ThreadMetadata { }, updated_at, created_at, + interacted_at, worktree_paths, remote_connection, archived, @@ -1686,6 +1722,7 @@ mod tests { }, updated_at, created_at: Some(updated_at), + interacted_at: None, worktree_paths: WorktreePaths::from_folder_paths(&folder_paths), remote_connection: None, } @@ -1867,6 +1904,7 @@ mod tests { title: Some("First Thread".into()), updated_at: updated_time, created_at: Some(updated_time), + interacted_at: None, worktree_paths: WorktreePaths::from_folder_paths(&second_paths), remote_connection: None, archived: false, @@ -1950,6 +1988,7 @@ mod tests { title: Some("Existing Metadata".into()), updated_at: now - chrono::Duration::seconds(10), created_at: Some(now - chrono::Duration::seconds(10)), + interacted_at: None, worktree_paths: WorktreePaths::from_folder_paths(&project_a_paths), remote_connection: None, archived: false, @@ -2070,6 +2109,7 @@ mod tests { title: Some("Existing Metadata".into()), updated_at: existing_updated_at, created_at: Some(existing_updated_at), + interacted_at: None, worktree_paths: WorktreePaths::from_folder_paths(&project_paths), remote_connection: None, archived: false, @@ -2743,6 +2783,7 @@ mod tests { title: Some("Local Linked".into()), updated_at: now, created_at: Some(now), + interacted_at: None, worktree_paths: linked_worktree_paths.clone(), remote_connection: None, }; @@ -2755,6 +2796,7 @@ mod tests { title: Some("Remote Linked".into()), updated_at: now - chrono::Duration::seconds(1), created_at: Some(now - chrono::Duration::seconds(1)), + interacted_at: None, worktree_paths: linked_worktree_paths, remote_connection: Some(remote_a.clone()), }; diff --git a/crates/sidebar/src/sidebar.rs b/crates/sidebar/src/sidebar.rs index f0e7b326462c0f2c52730778eb75cd60078c62d3..04a850941b87789723538869a2e5fa319de306bf 100644 --- a/crates/sidebar/src/sidebar.rs +++ b/crates/sidebar/src/sidebar.rs @@ -33,6 +33,7 @@ use ui::utils::platform_title_bar_height; use serde::{Deserialize, Serialize}; use settings::Settings as _; +use std::cmp::Ordering; use std::collections::{HashMap, HashSet}; use std::mem; use std::path::{Path, PathBuf}; @@ -359,11 +360,7 @@ pub struct Sidebar { /// Updated only in response to explicit user actions (clicking a /// thread, confirming in the thread switcher, etc.) — never from /// background data changes. Used to sort the thread switcher popup. - thread_last_accessed: HashMap>, - /// Updated when the user presses a key to send or queue a message. - /// Used for sorting threads in the sidebar and as a secondary sort - /// key in the thread switcher. - thread_last_message_sent_or_queued: HashMap>, + thread_last_accessed: HashMap>, thread_switcher: Option>, _thread_switcher_subscriptions: Vec, pending_thread_activation: Option, @@ -458,7 +455,6 @@ impl Sidebar { hovered_thread_index: None, thread_last_accessed: HashMap::new(), - thread_last_message_sent_or_queued: HashMap::new(), thread_switcher: None, _thread_switcher_subscriptions: Vec::new(), pending_thread_activation: None, @@ -646,7 +642,7 @@ impl Sidebar { this.update_entries(cx); } AgentPanelEvent::MessageSentOrQueued { thread_id } => { - this.record_thread_message_sent(thread_id); + this.record_thread_message_sent_or_queued(thread_id, cx); this.update_entries(cx); } }, @@ -1138,8 +1134,8 @@ impl Sidebar { } threads.sort_by(|a, b| { - let a_time = self.display_time(&a.metadata); - let b_time = self.display_time(&b.metadata); + let a_time = Self::thread_display_time(&a.metadata); + let b_time = Self::thread_display_time(&b.metadata); b_time.cmp(&a_time) }); } else { @@ -1248,8 +1244,6 @@ impl Sidebar { notified_threads.retain(|id| current_thread_ids.contains(id)); self.thread_last_accessed - .retain(|id, _| current_session_ids.contains(id)); - self.thread_last_message_sent_or_queued .retain(|id, _| current_thread_ids.contains(id)); self.contents = SidebarContents { @@ -2217,7 +2211,7 @@ impl Sidebar { session_id: metadata.session_id.clone(), workspace: workspace.clone(), }); - self.record_thread_access(&metadata.session_id); + self.record_thread_access(&metadata.thread_id); if metadata.session_id.is_some() { self.pending_thread_activation = Some(metadata.thread_id); @@ -2286,7 +2280,7 @@ impl Sidebar { session_id: target_session_id.clone(), workspace: workspace_for_entry.clone(), }); - sidebar.record_thread_access(&target_session_id); + sidebar.record_thread_access(&metadata_thread_id); sidebar.update_entries(cx); }); } @@ -3316,22 +3310,37 @@ impl Sidebar { } } - fn record_thread_access(&mut self, session_id: &Option) { - if let Some(sid) = session_id { - self.thread_last_accessed.insert(sid.clone(), Utc::now()); - } + fn record_thread_access(&mut self, id: &ThreadId) { + self.thread_last_accessed.insert(*id, Utc::now()); + } + + fn record_thread_message_sent_or_queued( + &mut self, + thread_id: &agent_ui::ThreadId, + cx: &mut App, + ) { + let store = ThreadMetadataStore::global(cx); + store.update(cx, |store, cx| { + store.update_interacted_at(thread_id, Utc::now(), cx); + }) } - fn record_thread_message_sent(&mut self, thread_id: &agent_ui::ThreadId) { - self.thread_last_message_sent_or_queued - .insert(*thread_id, Utc::now()); + fn thread_display_time(metadata: &ThreadMetadata) -> DateTime { + metadata.interacted_at.unwrap_or(metadata.updated_at) } - fn display_time(&self, metadata: &ThreadMetadata) -> DateTime { - self.thread_last_message_sent_or_queued - .get(&metadata.thread_id) - .copied() - .unwrap_or(metadata.updated_at) + /// The sort order used by the ctrl-tab switcher + fn thread_cmp_for_switcher(&self, left: &ThreadMetadata, right: &ThreadMetadata) -> Ordering { + let sort_time = |x: &ThreadMetadata| { + self.thread_last_accessed + .get(&x.thread_id) + .copied() + .or(x.interacted_at) + .unwrap_or(x.updated_at) + }; + + // .reverse() = most recent first + sort_time(left).cmp(&sort_time(right)).reverse() } fn mru_threads_for_switcher(&self, cx: &App) -> Vec { @@ -3365,7 +3374,8 @@ impl Sidebar { }?; let notified = self.contents.is_thread_notified(&thread.metadata.thread_id); let timestamp: SharedString = - format_history_entry_timestamp(self.display_time(&thread.metadata)).into(); + format_history_entry_timestamp(Self::thread_display_time(&thread.metadata)) + .into(); Some(ThreadSwitcherEntry { session_id, title: thread.metadata.display_title(), @@ -3393,31 +3403,7 @@ impl Sidebar { }) .collect(); - entries.sort_by(|a, b| { - let a_accessed = self.thread_last_accessed.get(&a.session_id); - let b_accessed = self.thread_last_accessed.get(&b.session_id); - - match (a_accessed, b_accessed) { - (Some(a_time), Some(b_time)) => b_time.cmp(a_time), - (Some(_), None) => std::cmp::Ordering::Less, - (None, Some(_)) => std::cmp::Ordering::Greater, - (None, None) => { - let a_sent = self - .thread_last_message_sent_or_queued - .get(&a.metadata.thread_id); - let b_sent = self - .thread_last_message_sent_or_queued - .get(&b.metadata.thread_id); - - match (a_sent, b_sent) { - (Some(a_time), Some(b_time)) => b_time.cmp(a_time), - (Some(_), None) => std::cmp::Ordering::Less, - (None, Some(_)) => std::cmp::Ordering::Greater, - (None, None) => b.metadata.updated_at.cmp(&a.metadata.updated_at), - } - } - } - }); + entries.sort_by(|a, b| self.thread_cmp_for_switcher(&a.metadata, &b.metadata)); entries } @@ -3513,7 +3499,7 @@ impl Sidebar { mw.retain_active_workspace(cx); }); } - this.record_thread_access(&metadata.session_id); + this.record_thread_access(&metadata.thread_id); this.active_entry = Some(ActiveEntry { thread_id: metadata.thread_id, session_id: metadata.session_id.clone(), @@ -3631,7 +3617,7 @@ impl Sidebar { .title_bar_background .blend(color.panel_background.opacity(0.25)); - let timestamp = format_history_entry_timestamp(self.display_time(&thread.metadata)); + let timestamp = format_history_entry_timestamp(Self::thread_display_time(&thread.metadata)); let is_remote = thread.workspace.is_remote(cx); diff --git a/crates/sidebar/src/sidebar_tests.rs b/crates/sidebar/src/sidebar_tests.rs index e90f20de21cb437ba82d0f1ab02fd0d7651f9d3f..c4d2dde70fb98801ae5515668402d28ce81eabf6 100644 --- a/crates/sidebar/src/sidebar_tests.rs +++ b/crates/sidebar/src/sidebar_tests.rs @@ -3,7 +3,9 @@ use acp_thread::{AcpThread, PermissionOptions, StubAgentConnection}; use agent::ThreadStore; use agent_ui::{ ThreadId, - test_support::{active_session_id, open_thread_with_connection, send_message}, + test_support::{ + active_session_id, active_thread_id, open_thread_with_connection, send_message, + }, thread_metadata_store::{ThreadMetadata, WorktreePaths}, }; use chrono::DateTime; @@ -207,6 +209,7 @@ async fn save_n_test_threads( Some(format!("Thread {}", i + 1).into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, i).unwrap(), None, + None, project, cx, ) @@ -224,6 +227,7 @@ async fn save_test_thread_metadata( Some("Test".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), None, + None, project, cx, ) @@ -240,6 +244,7 @@ async fn save_named_thread_metadata( Some(SharedString::from(title.to_string())), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), None, + None, project, cx, ); @@ -335,6 +340,7 @@ fn save_thread_metadata( title: Option, updated_at: DateTime, created_at: Option>, + interacted_at: Option>, project: &Entity, cx: &mut TestAppContext, ) { @@ -354,6 +360,7 @@ fn save_thread_metadata( title, updated_at, created_at, + interacted_at, worktree_paths, archived: false, remote_connection, @@ -388,6 +395,7 @@ fn save_thread_metadata_with_main_paths( title: Some(title), updated_at, created_at: None, + interacted_at: None, worktree_paths: WorktreePaths::from_path_lists(main_worktree_paths, folder_paths).unwrap(), archived: false, remote_connection: None, @@ -631,6 +639,7 @@ async fn test_single_workspace_with_saved_threads(cx: &mut TestAppContext) { Some("Fix crash in project panel".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 3, 0, 0, 0).unwrap(), None, + None, &project, cx, ); @@ -640,6 +649,7 @@ async fn test_single_workspace_with_saved_threads(cx: &mut TestAppContext) { Some("Add inline diff view".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 2, 0, 0, 0).unwrap(), None, + None, &project, cx, ); @@ -672,6 +682,7 @@ async fn test_workspace_lifecycle(cx: &mut TestAppContext) { Some("Thread A1".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), None, + None, &project, cx, ); @@ -854,6 +865,7 @@ async fn test_visible_entries_as_strings(cx: &mut TestAppContext) { title: Some("Completed thread".into()), updated_at: Utc::now(), created_at: Some(Utc::now()), + interacted_at: None, archived: false, remote_connection: None, }, @@ -878,6 +890,7 @@ async fn test_visible_entries_as_strings(cx: &mut TestAppContext) { title: Some("Running thread".into()), updated_at: Utc::now(), created_at: Some(Utc::now()), + interacted_at: None, archived: false, remote_connection: None, }, @@ -902,6 +915,7 @@ async fn test_visible_entries_as_strings(cx: &mut TestAppContext) { title: Some("Error thread".into()), updated_at: Utc::now(), created_at: Some(Utc::now()), + interacted_at: None, archived: false, remote_connection: None, }, @@ -927,6 +941,7 @@ async fn test_visible_entries_as_strings(cx: &mut TestAppContext) { title: Some("Waiting thread".into()), updated_at: Utc::now(), created_at: Some(Utc::now()), + interacted_at: None, archived: false, remote_connection: None, }, @@ -952,6 +967,7 @@ async fn test_visible_entries_as_strings(cx: &mut TestAppContext) { title: Some("Notified thread".into()), updated_at: Utc::now(), created_at: Some(Utc::now()), + interacted_at: None, archived: false, remote_connection: None, }, @@ -1578,6 +1594,7 @@ async fn test_search_narrows_visible_threads_to_matches(cx: &mut TestAppContext) Some(title.into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, hour, 0, 0).unwrap(), None, + None, &project, cx, ); @@ -1629,6 +1646,7 @@ async fn test_search_matches_regardless_of_case(cx: &mut TestAppContext) { Some("Fix Crash In Project Panel".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), None, + None, &project, cx, ); @@ -1672,6 +1690,7 @@ async fn test_escape_clears_search_and_restores_full_list(cx: &mut TestAppContex Some(title.into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, hour, 0, 0).unwrap(), None, + None, &project, cx, ) @@ -1732,6 +1751,7 @@ async fn test_search_only_shows_workspace_headers_with_matches(cx: &mut TestAppC Some(title.into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, hour, 0, 0).unwrap(), None, + None, &project_a, cx, ) @@ -1756,6 +1776,7 @@ async fn test_search_only_shows_workspace_headers_with_matches(cx: &mut TestAppC Some(title.into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, hour, 0, 0).unwrap(), None, + None, &project_b, cx, ) @@ -1820,6 +1841,7 @@ async fn test_search_matches_workspace_name(cx: &mut TestAppContext) { Some(title.into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, hour, 0, 0).unwrap(), None, + None, &project_a, cx, ) @@ -1844,6 +1866,7 @@ async fn test_search_matches_workspace_name(cx: &mut TestAppContext) { Some(title.into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, hour, 0, 0).unwrap(), None, + None, &project_b, cx, ) @@ -1929,6 +1952,7 @@ async fn test_search_finds_threads_inside_collapsed_groups(cx: &mut TestAppConte Some("Important thread".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), None, + None, &project, cx, ); @@ -1980,6 +2004,7 @@ async fn test_search_then_keyboard_navigate_and_confirm(cx: &mut TestAppContext) Some(title.into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, hour, 0, 0).unwrap(), None, + None, &project, cx, ) @@ -2050,6 +2075,7 @@ async fn test_confirm_on_historical_thread_activates_workspace(cx: &mut TestAppC Some("Historical Thread".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 6, 1, 0, 0, 0).unwrap(), None, + None, &project, cx, ); @@ -2109,6 +2135,7 @@ async fn test_confirm_on_historical_thread_preserves_historical_timestamp_and_or Some("Newer Historical Thread".into()), newer_timestamp, Some(newer_timestamp), + None, &project, cx, ); @@ -2120,6 +2147,7 @@ async fn test_confirm_on_historical_thread_preserves_historical_timestamp_and_or Some("Older Historical Thread".into()), older_timestamp, Some(older_timestamp), + None, &project, cx, ); @@ -2230,6 +2258,7 @@ async fn test_confirm_on_historical_thread_in_new_project_group_opens_real_threa Some("Historical Thread in New Group".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 6, 1, 0, 0, 0).unwrap(), None, + None, &project_b, cx, ); @@ -2338,6 +2367,7 @@ async fn test_click_clears_selection_and_focus_in_restores_it(cx: &mut TestAppCo Some("Thread A".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 2, 0, 0, 0).unwrap(), None, + None, &project, cx, ); @@ -2347,6 +2377,7 @@ async fn test_click_clears_selection_and_focus_in_restores_it(cx: &mut TestAppCo Some("Thread B".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), None, + None, &project, cx, ); @@ -3140,6 +3171,7 @@ async fn test_two_worktree_workspaces_absorbed_when_main_added(cx: &mut TestAppC Some("Thread A".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), None, + None, &project_a, cx, ); @@ -3148,6 +3180,7 @@ async fn test_two_worktree_workspaces_absorbed_when_main_added(cx: &mut TestAppC Some("Thread B".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 1).unwrap(), None, + None, &project_b, cx, ); @@ -3998,6 +4031,7 @@ async fn test_activate_archived_thread_with_saved_paths_activates_matching_works title: Some("Archived Thread".into()), updated_at: Utc::now(), created_at: None, + interacted_at: None, worktree_paths: WorktreePaths::from_folder_paths(&PathList::new(&[PathBuf::from( "/project-b", )])), @@ -4066,6 +4100,7 @@ async fn test_activate_archived_thread_cwd_fallback_with_matching_workspace( title: Some("CWD Thread".into()), updated_at: Utc::now(), created_at: None, + interacted_at: None, worktree_paths: WorktreePaths::from_folder_paths(&PathList::new(&[ std::path::PathBuf::from("/project-b"), ])), @@ -4132,6 +4167,7 @@ async fn test_activate_archived_thread_no_paths_no_cwd_uses_active_workspace( title: Some("Contextless Thread".into()), updated_at: Utc::now(), created_at: None, + interacted_at: None, worktree_paths: WorktreePaths::default(), archived: false, remote_connection: None, @@ -4188,6 +4224,7 @@ async fn test_activate_archived_thread_saved_paths_opens_new_workspace(cx: &mut title: Some("New WS Thread".into()), updated_at: Utc::now(), created_at: None, + interacted_at: None, worktree_paths: WorktreePaths::from_folder_paths(&path_list_b), archived: false, remote_connection: None, @@ -4243,6 +4280,7 @@ async fn test_activate_archived_thread_reuses_workspace_in_another_window(cx: &m title: Some("Cross Window Thread".into()), updated_at: Utc::now(), created_at: None, + interacted_at: None, worktree_paths: WorktreePaths::from_folder_paths(&PathList::new(&[PathBuf::from( "/project-b", )])), @@ -4323,6 +4361,7 @@ async fn test_activate_archived_thread_reuses_workspace_in_another_window_with_t title: Some("Cross Window Thread".into()), updated_at: Utc::now(), created_at: None, + interacted_at: None, worktree_paths: WorktreePaths::from_folder_paths(&PathList::new(&[PathBuf::from( "/project-b", )])), @@ -4406,6 +4445,7 @@ async fn test_activate_archived_thread_prefers_current_window_for_matching_paths title: Some("Current Window Thread".into()), updated_at: Utc::now(), created_at: None, + interacted_at: None, worktree_paths: WorktreePaths::from_folder_paths(&PathList::new(&[PathBuf::from( "/project-a", )])), @@ -4540,6 +4580,7 @@ async fn test_archive_thread_uses_next_threads_own_workspace(cx: &mut TestAppCon Some("Thread 2".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 2, 0, 0, 0).unwrap(), None, + None, &main_project, cx, ); @@ -4552,6 +4593,7 @@ async fn test_archive_thread_uses_next_threads_own_workspace(cx: &mut TestAppCon Some("Thread 1".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), None, + None, &worktree_project, cx, ); @@ -4685,6 +4727,7 @@ async fn test_archive_last_worktree_thread_removes_workspace(cx: &mut TestAppCon Some("Main Thread".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 2, 0, 0, 0).unwrap(), None, + None, &main_project, cx, ); @@ -4696,6 +4739,7 @@ async fn test_archive_last_worktree_thread_removes_workspace(cx: &mut TestAppCon Some("Worktree Thread".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), None, + None, &worktree_project, cx, ); @@ -5160,6 +5204,7 @@ async fn test_restore_worktree_thread_uses_main_repo_project_group_key(cx: &mut Some("Worktree Thread C".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), None, + None, &worktree_project, cx, ); @@ -5306,6 +5351,7 @@ async fn test_archive_last_worktree_thread_not_blocked_by_remote_thread_at_same_ Some("Main Thread".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 2, 0, 0, 0).unwrap(), None, + None, &main_project, cx, ); @@ -5317,6 +5363,7 @@ async fn test_archive_last_worktree_thread_not_blocked_by_remote_thread_at_same_ Some("Local Worktree Thread".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), None, + None, &worktree_project, cx, ); @@ -5334,6 +5381,7 @@ async fn test_archive_last_worktree_thread_not_blocked_by_remote_thread_at_same_ title: Some("Remote Worktree Thread".into()), updated_at: chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), created_at: None, + interacted_at: None, worktree_paths: WorktreePaths::from_folder_paths(&PathList::new(&[PathBuf::from( "/wt-feature-a", )])), @@ -5473,6 +5521,7 @@ async fn test_linked_worktree_threads_not_duplicated_across_groups(cx: &mut Test Some("Worktree Thread".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), None, + None, &worktree_project, cx, ); @@ -5499,6 +5548,16 @@ async fn test_linked_worktree_threads_not_duplicated_across_groups(cx: &mut Test ); } +fn thread_id_for(session_id: &acp::SessionId, cx: &mut TestAppContext) -> ThreadId { + cx.read(|cx| { + ThreadMetadataStore::global(cx) + .read(cx) + .entry_by_session(session_id) + .map(|m| m.thread_id) + .expect("thread metadata should exist") + }) +} + #[gpui::test] async fn test_thread_switcher_ordering(cx: &mut TestAppContext) { let project = init_test_project_with_agent_panel("/my-project", cx).await; @@ -5507,7 +5566,7 @@ async fn test_thread_switcher_ordering(cx: &mut TestAppContext) { let (sidebar, panel) = setup_sidebar_with_agent_panel(&multi_workspace, cx); let switcher_ids = - |sidebar: &Entity, cx: &mut gpui::VisualTestContext| -> Vec { + |sidebar: &Entity, cx: &mut gpui::VisualTestContext| -> Vec { sidebar.read_with(cx, |sidebar, cx| { let switcher = sidebar .thread_switcher @@ -5517,13 +5576,13 @@ async fn test_thread_switcher_ordering(cx: &mut TestAppContext) { .read(cx) .entries() .iter() - .map(|e| e.session_id.clone()) + .map(|e| e.metadata.thread_id) .collect() }) }; let switcher_selected_id = - |sidebar: &Entity, cx: &mut gpui::VisualTestContext| -> acp::SessionId { + |sidebar: &Entity, cx: &mut gpui::VisualTestContext| -> ThreadId { sidebar.read_with(cx, |sidebar, cx| { let switcher = sidebar .thread_switcher @@ -5532,8 +5591,8 @@ async fn test_thread_switcher_ordering(cx: &mut TestAppContext) { let s = switcher.read(cx); s.selected_entry() .expect("should have selection") - .session_id - .clone() + .metadata + .thread_id }) }; @@ -5547,11 +5606,13 @@ 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); + let thread_id_c = active_thread_id(&panel, cx); save_thread_metadata( session_id_c.clone(), Some("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()), + None, &project, cx, ); @@ -5563,11 +5624,13 @@ 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); + let thread_id_b = active_thread_id(&panel, cx); save_thread_metadata( session_id_b.clone(), Some("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()), + None, &project, cx, ); @@ -5579,11 +5642,13 @@ 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); + let thread_id_a = active_thread_id(&panel, cx); save_thread_metadata( session_id_a.clone(), Some("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()), + None, &project, cx, ); @@ -5605,14 +5670,10 @@ async fn test_thread_switcher_ordering(cx: &mut TestAppContext) { // then B, then C. assert_eq!( switcher_ids(&sidebar, cx), - vec![ - session_id_a.clone(), - session_id_b.clone(), - session_id_c.clone() - ], + vec![thread_id_a, thread_id_b, thread_id_c,], ); // First ctrl-tab selects the second entry (B). - assert_eq!(switcher_selected_id(&sidebar, cx), session_id_b); + assert_eq!(switcher_selected_id(&sidebar, cx), thread_id_b); // Dismiss the switcher without confirming. sidebar.update_in(cx, |sidebar, _window, cx| { @@ -5639,7 +5700,7 @@ async fn test_thread_switcher_ordering(cx: &mut TestAppContext) { .update(cx, |s, cx| s.cycle_selection(cx)); }); cx.run_until_parked(); - assert_eq!(switcher_selected_id(&sidebar, cx), session_id_c); + assert_eq!(switcher_selected_id(&sidebar, cx), thread_id_c); assert!(sidebar.update(cx, |sidebar, _cx| sidebar.thread_last_accessed.is_empty())); @@ -5666,7 +5727,7 @@ async fn test_thread_switcher_ordering(cx: &mut TestAppContext) { .cloned() .collect::>(); assert_eq!(last_accessed.len(), 1); - assert!(last_accessed.contains(&session_id_c)); + assert!(last_accessed.contains(&thread_id_c)); assert!( is_active_session(&sidebar, &session_id_c), "active_entry should be Thread({session_id_c:?})" @@ -5680,11 +5741,7 @@ async fn test_thread_switcher_ordering(cx: &mut TestAppContext) { assert_eq!( switcher_ids(&sidebar, cx), - vec![ - session_id_c.clone(), - session_id_a.clone(), - session_id_b.clone() - ], + vec![thread_id_c, thread_id_a, thread_id_b], ); // Confirm on Thread A. @@ -5702,8 +5759,8 @@ async fn test_thread_switcher_ordering(cx: &mut TestAppContext) { .cloned() .collect::>(); assert_eq!(last_accessed.len(), 2); - assert!(last_accessed.contains(&session_id_c)); - assert!(last_accessed.contains(&session_id_a)); + assert!(last_accessed.contains(&thread_id_c)); + assert!(last_accessed.contains(&thread_id_a)); assert!( is_active_session(&sidebar, &session_id_a), "active_entry should be Thread({session_id_a:?})" @@ -5717,11 +5774,7 @@ async fn test_thread_switcher_ordering(cx: &mut TestAppContext) { assert_eq!( switcher_ids(&sidebar, cx), - vec![ - session_id_a.clone(), - session_id_c.clone(), - session_id_b.clone(), - ], + vec![thread_id_a, thread_id_c, thread_id_b,], ); sidebar.update_in(cx, |sidebar, _window, cx| { @@ -5745,9 +5798,9 @@ async fn test_thread_switcher_ordering(cx: &mut TestAppContext) { .cloned() .collect::>(); 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!(last_accessed.contains(&thread_id_c)); + assert!(last_accessed.contains(&thread_id_a)); + assert!(last_accessed.contains(&thread_id_b)); assert!( is_active_session(&sidebar, &session_id_b), "active_entry should be Thread({session_id_b:?})" @@ -5761,6 +5814,7 @@ async fn test_thread_switcher_ordering(cx: &mut TestAppContext) { Some("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()), + None, &project, cx, ); @@ -5779,16 +5833,12 @@ 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 thread_id_hist = thread_id_for(&session_id_hist, cx); let ids = switcher_ids(&sidebar, cx); assert_eq!( ids, - vec![ - session_id_b.clone(), - session_id_a.clone(), - session_id_c.clone(), - session_id_hist.clone() - ], + vec![thread_id_b, thread_id_a, thread_id_c, thread_id_hist], ); sidebar.update_in(cx, |sidebar, _window, cx| { @@ -5802,6 +5852,7 @@ async fn test_thread_switcher_ordering(cx: &mut TestAppContext) { Some("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()), + None, &project, cx, ); @@ -5814,15 +5865,16 @@ async fn test_thread_switcher_ordering(cx: &mut TestAppContext) { // Both historical threads have no access or message times. They should // appear after accessed threads, sorted by created_at (newest first). let session_id_old_hist = acp::SessionId::new(Arc::from("thread-old-historical")); + let thread_id_old_hist = thread_id_for(&session_id_old_hist, cx); let ids = switcher_ids(&sidebar, cx); assert_eq!( ids, vec![ - session_id_b, - session_id_a, - session_id_c, - session_id_hist, - session_id_old_hist, + thread_id_b, + thread_id_a, + thread_id_c, + thread_id_hist, + thread_id_old_hist, ], ); @@ -5844,6 +5896,7 @@ async fn test_archive_thread_keeps_metadata_but_hides_from_sidebar(cx: &mut Test Some("Thread To Archive".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), None, + None, &project, cx, ); @@ -6099,6 +6152,7 @@ async fn test_unarchive_first_thread_in_group_does_not_create_spurious_draft( title: Some("Unarchived Thread".into()), updated_at: Utc::now(), created_at: None, + interacted_at: None, worktree_paths: WorktreePaths::from_folder_paths(&path_list_b), archived: true, remote_connection: None, @@ -6191,6 +6245,7 @@ async fn test_unarchive_into_new_workspace_does_not_create_duplicate_real_thread title: Some("Unarchived Thread".into()), updated_at: Utc::now(), created_at: None, + interacted_at: None, worktree_paths: WorktreePaths::from_folder_paths(&path_list_b), archived: true, remote_connection: None, @@ -6416,6 +6471,7 @@ async fn test_unarchive_into_inactive_existing_workspace_does_not_leave_active_d title: Some("Restored In Inactive Workspace".into()), updated_at: Utc::now(), created_at: None, + interacted_at: None, worktree_paths: WorktreePaths::from_folder_paths(&PathList::new(&[ PathBuf::from("/project-b"), ])), @@ -6821,6 +6877,7 @@ async fn test_archived_threads_excluded_from_sidebar_entries(cx: &mut TestAppCon Some("Visible Thread".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 2, 0, 0, 0).unwrap(), None, + None, &project, cx, ); @@ -6831,6 +6888,7 @@ async fn test_archived_threads_excluded_from_sidebar_entries(cx: &mut TestAppCon Some("Archived Thread".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), None, + None, &project, cx, ); @@ -6976,6 +7034,7 @@ async fn test_archive_last_thread_on_linked_worktree_does_not_create_new_thread_ Some("Ochre Drift Thread".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 2, 0, 0, 0).unwrap(), None, + None, &worktree_project, cx, ); @@ -6987,6 +7046,7 @@ async fn test_archive_last_thread_on_linked_worktree_does_not_create_new_thread_ Some("Main Project Thread".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), None, + None, &main_project, cx, ); @@ -7142,6 +7202,7 @@ async fn test_archive_last_thread_on_linked_worktree_with_no_siblings_leaves_gro Some("Ochre Drift Thread".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 2, 0, 0, 0).unwrap(), None, + None, &worktree_project, cx, ); @@ -7258,6 +7319,7 @@ async fn test_unarchive_linked_worktree_thread_into_project_group_shows_only_res title: Some("Unarchived Linked Thread".into()), updated_at: Utc::now(), created_at: None, + interacted_at: None, worktree_paths: WorktreePaths::from_path_lists( main_paths.clone(), folder_paths.clone(), @@ -7441,6 +7503,7 @@ async fn test_archive_thread_on_linked_worktree_selects_sibling_thread(cx: &mut Some("Ochre Drift Thread".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 2, 0, 0, 0).unwrap(), None, + None, &worktree_project, cx, ); @@ -7452,6 +7515,7 @@ async fn test_archive_thread_on_linked_worktree_selects_sibling_thread(cx: &mut Some("Main Project Thread".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), None, + None, &main_project, cx, ); @@ -7919,6 +7983,7 @@ async fn test_legacy_thread_with_canonical_path_opens_main_repo_workspace(cx: &m title: Some("Legacy Main Thread".into()), updated_at: chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), created_at: None, + interacted_at: None, worktree_paths: WorktreePaths::from_folder_paths(&PathList::new(&[PathBuf::from( "/project", )])), @@ -8377,6 +8442,7 @@ async fn test_non_archive_thread_paths_migrate_on_worktree_add_and_remove(cx: &m Some("Historical 1".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), None, + None, &project, cx, ); @@ -8385,6 +8451,7 @@ async fn test_non_archive_thread_paths_migrate_on_worktree_add_and_remove(cx: &m Some("Historical 2".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 1).unwrap(), None, + None, &project, cx, ); @@ -8552,6 +8619,7 @@ async fn test_worktree_add_only_regroups_threads_for_changed_workspace(cx: &mut Some("Main Thread".into()), time_main, Some(time_main), + None, &main_project, cx, ); @@ -8560,6 +8628,7 @@ async fn test_worktree_add_only_regroups_threads_for_changed_workspace(cx: &mut Some("Worktree Thread".into()), time_wt, Some(time_wt), + None, &worktree_project, cx, ); @@ -8903,6 +8972,7 @@ mod property_test { title: Some(title), updated_at, created_at: None, + interacted_at: None, worktree_paths: WorktreePaths::from_path_lists(main_worktree_paths, path_list).unwrap(), archived: false, remote_connection: None, @@ -8957,7 +9027,15 @@ mod property_test { chrono::TimeZone::with_ymd_and_hms(&chrono::Utc, 2024, 1, 1, 0, 0, 0) .unwrap() + chrono::Duration::seconds(state.thread_counter as i64); - save_thread_metadata(session_id, Some(title), updated_at, None, &project, cx); + save_thread_metadata( + session_id, + Some(title), + updated_at, + None, + None, + &project, + cx, + ); } } Operation::SaveWorktreeThread { worktree_index } => { @@ -9776,6 +9854,7 @@ async fn test_remote_project_integration_does_not_briefly_render_as_separate_pro Some("Main Thread".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 0).unwrap(), None, + None, &project, cx, ); @@ -9801,6 +9880,7 @@ async fn test_remote_project_integration_does_not_briefly_render_as_separate_pro title: Some("Worktree Thread".into()), updated_at: chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 1, 0, 0, 1).unwrap(), created_at: None, + interacted_at: None, worktree_paths: WorktreePaths::from_path_lists( main_worktree_paths, PathList::new(&[PathBuf::from("/project-wt-1")]), @@ -10057,6 +10137,7 @@ async fn test_archive_removes_worktree_even_when_workspace_paths_diverge(cx: &mu Some("Main Thread".into()), chrono::TimeZone::with_ymd_and_hms(&Utc, 2024, 1, 2, 0, 0, 0).unwrap(), None, + None, &main_project, cx, ); @@ -10521,6 +10602,7 @@ async fn test_remote_archive_thread_with_active_connection( updated_at: chrono::TimeZone::with_ymd_and_hms(&chrono::Utc, 2024, 1, 1, 0, 0, 0) .unwrap(), created_at: None, + interacted_at: None, worktree_paths: WorktreePaths::from_path_lists( PathList::new(&[PathBuf::from("/project")]), PathList::new(&[PathBuf::from("/worktrees/project/feature-a/project")]), @@ -10639,6 +10721,7 @@ async fn test_remote_archive_thread_with_disconnected_remote( Some("Remote Thread".into()), chrono::TimeZone::with_ymd_and_hms(&chrono::Utc, 2024, 1, 1, 0, 0, 0).unwrap(), None, + None, &project, cx, );