diff --git a/crates/sidebar/src/sidebar.rs b/crates/sidebar/src/sidebar.rs index cd12dc852e4a25f244a5371b80aa2ce9fabad8f2..a2e0a50abc02b406a4fba5dd4ed2764295b14c34 100644 --- a/crates/sidebar/src/sidebar.rs +++ b/crates/sidebar/src/sidebar.rs @@ -108,6 +108,44 @@ enum SidebarView { Archive(Entity), } +#[derive(Clone, Debug)] +enum ActiveEntry { + Thread { + session_id: acp::SessionId, + workspace: Entity, + }, + Draft(Entity), +} + +impl ActiveEntry { + fn workspace(&self) -> &Entity { + match self { + ActiveEntry::Thread { workspace, .. } => workspace, + ActiveEntry::Draft(workspace) => workspace, + } + } + + fn is_active_thread(&self, session_id: &acp::SessionId) -> bool { + matches!(self, ActiveEntry::Thread { session_id: id, .. } if id == session_id) + } + + fn matches_entry(&self, entry: &ListEntry) -> bool { + match (self, entry) { + (ActiveEntry::Thread { session_id, .. }, ListEntry::Thread(thread)) => { + thread.metadata.session_id == *session_id + } + ( + ActiveEntry::Draft(workspace), + ListEntry::NewThread { + workspace: entry_workspace, + .. + }, + ) => workspace == entry_workspace, + _ => false, + } + } +} + #[derive(Clone, Debug)] struct ActiveThreadInfo { session_id: acp::SessionId, @@ -185,7 +223,6 @@ enum ListEntry { NewThread { path_list: PathList, workspace: Entity, - is_active_draft: bool, worktrees: Vec, }, } @@ -322,11 +359,8 @@ pub struct Sidebar { /// /// Note: This is NOT the same as the active item. selection: Option, - /// Derived from the active panel's thread in `rebuild_contents`. - /// Only updated when the panel returns `Some` — never cleared by - /// derivation, since the panel may transiently return `None` while - /// loading. User actions may write directly for immediate feedback. - focused_thread: Option, + /// Tracks which sidebar entry is currently active (highlighted). + active_entry: Option, hovered_thread_index: Option, collapsed_groups: HashSet, expanded_groups: HashMap, @@ -423,7 +457,7 @@ impl Sidebar { list_state: ListState::new(0, gpui::ListAlignment::Top, px(1000.)), contents: SidebarContents::default(), selection: None, - focused_thread: None, + active_entry: None, hovered_thread_index: None, collapsed_groups: HashSet::new(), expanded_groups: HashMap::new(), @@ -443,27 +477,14 @@ impl Sidebar { cx.emit(workspace::SidebarEvent::SerializeNeeded); } - fn is_active_workspace(&self, workspace: &Entity, cx: &App) -> bool { - self.multi_workspace - .upgrade() - .map_or(false, |mw| mw.read(cx).workspace() == workspace) + fn active_entry_workspace(&self) -> Option<&Entity> { + self.active_entry.as_ref().map(|entry| entry.workspace()) } - fn agent_panel_visible(&self, cx: &App) -> bool { - self.multi_workspace.upgrade().map_or(false, |mw| { - let workspace = mw.read(cx).workspace(); - AgentPanel::is_visible(&workspace, cx) - }) - } - - fn active_thread_is_draft(&self, cx: &App) -> bool { + fn is_active_workspace(&self, workspace: &Entity, cx: &App) -> bool { self.multi_workspace .upgrade() - .and_then(|mw| { - let workspace = mw.read(cx).workspace(); - workspace.read(cx).panel::(cx) - }) - .map_or(false, |panel| panel.read(cx).active_thread_is_draft(cx)) + .map_or(false, |mw| mw.read(cx).workspace() == workspace) } fn subscribe_to_workspace( @@ -543,7 +564,13 @@ impl Sidebar { .active_conversation_view() .is_some_and(|cv| cv.read(cx).parent_id(cx).is_none()); if is_new_draft { - this.focused_thread = None; + if let Some(active_workspace) = this + .multi_workspace + .upgrade() + .map(|mw| mw.read(cx).workspace().clone()) + { + this.active_entry = Some(ActiveEntry::Draft(active_workspace)); + } } this.observe_draft_editor(cx); this.update_entries(cx); @@ -677,24 +704,38 @@ impl Sidebar { let query = self.filter_editor.read(cx).text(cx); - let agent_panel_visible = self.agent_panel_visible(cx); - let active_thread_is_draft = self.active_thread_is_draft(cx); - - // Derive focused_thread from the active workspace's agent panel. - // Only update when the panel gives us a positive signal — if the - // panel returns None (e.g. still loading after a thread activation), - // keep the previous value so eager writes from user actions survive. - let panel_focused = active_workspace - .as_ref() - .and_then(|ws| ws.read(cx).panel::(cx)) - .and_then(|panel| { - panel + // Derive active_entry from the active workspace's agent panel. + // Draft is checked first because a conversation can have a session_id + // before any messages are sent. However, a thread that's still loading + // also appears as a "draft" (no messages yet), so when we already have + // an eager Thread write for this workspace we preserve it. A session_id + // on a non-draft is a positive Thread signal. The remaining case + // (conversation exists, not draft, no session_id) is a genuine + // mid-load — keep the previous value. + if let Some(active_ws) = &active_workspace { + if let Some(panel) = active_ws.read(cx).panel::(cx) { + if panel.read(cx).active_thread_is_draft(cx) + || panel.read(cx).active_conversation_view().is_none() + { + let preserving_thread = + matches!(&self.active_entry, Some(ActiveEntry::Thread { .. })) + && self.active_entry_workspace() == Some(active_ws); + if !preserving_thread { + self.active_entry = Some(ActiveEntry::Draft(active_ws.clone())); + } + } else if let Some(session_id) = panel .read(cx) .active_conversation_view() .and_then(|cv| cv.read(cx).parent_id(cx)) - }); - if panel_focused.is_some() && !active_thread_is_draft { - self.focused_thread = panel_focused; + { + self.active_entry = Some(ActiveEntry::Thread { + session_id, + workspace: active_ws.clone(), + }); + } + // else: conversation exists, not a draft, but no session_id + // yet — thread is mid-load. Keep previous value. + } } let previous = mem::take(&mut self.contents); @@ -966,10 +1007,9 @@ impl Sidebar { entries.push(thread.into()); } } else { - let is_draft_for_workspace = agent_panel_visible - && active_thread_is_draft - && self.focused_thread.is_none() - && is_active; + let is_draft_for_workspace = is_active + && matches!(&self.active_entry, Some(ActiveEntry::Draft(_))) + && self.active_entry_workspace() == Some(representative_workspace); project_header_indices.push(entries.len()); entries.push(ListEntry::ProjectHeader { @@ -989,11 +1029,9 @@ impl Sidebar { // Emit "New Thread" entries for threadless workspaces // and active drafts, right after the header. for (workspace, worktrees) in &threadless_workspaces { - let is_draft = is_draft_for_workspace && workspace == representative_workspace; entries.push(ListEntry::NewThread { path_list: path_list.clone(), workspace: workspace.clone(), - is_active_draft: is_draft, worktrees: worktrees.clone(), }); } @@ -1007,7 +1045,6 @@ impl Sidebar { entries.push(ListEntry::NewThread { path_list: path_list.clone(), workspace: representative_workspace.clone(), - is_active_draft: true, worktrees, }); } @@ -1033,10 +1070,9 @@ impl Sidebar { let is_promoted = thread.status == AgentThreadStatus::Running || thread.status == AgentThreadStatus::WaitingForConfirmation || notified_threads.contains(session_id) - || self - .focused_thread - .as_ref() - .is_some_and(|id| id == session_id); + || self.active_entry.as_ref().is_some_and(|active| { + active.matches_entry(&ListEntry::Thread(thread.clone())) + }); if is_promoted { promoted_threads.insert(session_id.clone()); } @@ -1135,6 +1171,11 @@ impl Sidebar { let is_group_header_after_first = ix > 0 && matches!(entry, ListEntry::ProjectHeader { .. }); + let is_active = self + .active_entry + .as_ref() + .is_some_and(|active| active.matches_entry(entry)); + let rendered = match entry { ListEntry::ProjectHeader { path_list, @@ -1143,7 +1184,7 @@ impl Sidebar { highlight_positions, has_running_threads, waiting_thread_count, - is_active, + is_active: is_active_group, } => self.render_project_header( ix, false, @@ -1153,11 +1194,11 @@ impl Sidebar { highlight_positions, *has_running_threads, *waiting_thread_count, - *is_active, + *is_active_group, is_selected, cx, ), - ListEntry::Thread(thread) => self.render_thread(ix, thread, is_selected, cx), + ListEntry::Thread(thread) => self.render_thread(ix, thread, is_active, is_selected, cx), ListEntry::ViewMore { path_list, is_fully_expanded, @@ -1165,13 +1206,12 @@ impl Sidebar { ListEntry::NewThread { path_list, workspace, - is_active_draft, worktrees, } => self.render_new_thread( ix, path_list, workspace, - *is_active_draft, + is_active, worktrees, is_selected, cx, @@ -1391,7 +1431,8 @@ impl Sidebar { .tooltip(Tooltip::text("Activate Workspace")) .on_click(cx.listener({ move |this, _, window, cx| { - this.focused_thread = None; + this.active_entry = + Some(ActiveEntry::Draft(workspace_for_open.clone())); if let Some(multi_workspace) = this.multi_workspace.upgrade() { multi_workspace.update(cx, |multi_workspace, cx| { multi_workspace.activate( @@ -1987,10 +2028,13 @@ impl Sidebar { return; }; - // Set focused_thread eagerly so the sidebar highlight updates + // Set active_entry eagerly so the sidebar highlight updates // immediately, rather than waiting for a deferred AgentPanel // event which can race with ActiveWorkspaceChanged clearing it. - self.focused_thread = Some(metadata.session_id.clone()); + self.active_entry = Some(ActiveEntry::Thread { + session_id: metadata.session_id.clone(), + workspace: workspace.clone(), + }); self.record_thread_access(&metadata.session_id); multi_workspace.update(cx, |multi_workspace, cx| { @@ -2010,6 +2054,7 @@ impl Sidebar { cx: &mut Context, ) { let target_session_id = metadata.session_id.clone(); + let workspace_for_entry = workspace.clone(); let activated = target_window .update(cx, |multi_workspace, window, cx| { @@ -2030,7 +2075,10 @@ impl Sidebar { .and_then(|sidebar| sidebar.downcast::().ok()) { target_sidebar.update(cx, |sidebar, cx| { - sidebar.focused_thread = Some(target_session_id.clone()); + sidebar.active_entry = Some(ActiveEntry::Thread { + session_id: target_session_id.clone(), + workspace: workspace_for_entry.clone(), + }); sidebar.record_thread_access(&target_session_id); sidebar.update_entries(cx); }); @@ -2296,7 +2344,11 @@ impl Sidebar { // nearest thread within the same project group. We never cross group // boundaries — if the group has no other threads, clear focus and open // a blank new thread in the panel instead. - if self.focused_thread.as_ref() == Some(session_id) { + if self + .active_entry + .as_ref() + .is_some_and(|e| e.is_active_thread(session_id)) + { let current_pos = self.contents.entries.iter().position(|entry| { matches!(entry, ListEntry::Thread(t) if &t.metadata.session_id == session_id) }); @@ -2360,7 +2412,12 @@ impl Sidebar { ThreadEntryWorkspace::Open(ws) => Some(ws.clone()), ThreadEntryWorkspace::Closed(_) => group_workspace, }; - self.focused_thread = Some(next_metadata.session_id.clone()); + if let Some(ref ws) = target_workspace { + self.active_entry = Some(ActiveEntry::Thread { + session_id: next_metadata.session_id.clone(), + workspace: ws.clone(), + }); + } self.record_thread_access(&next_metadata.session_id); if let Some(workspace) = target_workspace { @@ -2379,8 +2436,8 @@ impl Sidebar { } } } else { - self.focused_thread = None; if let Some(workspace) = &group_workspace { + self.active_entry = Some(ActiveEntry::Draft(workspace.clone())); if let Some(agent_panel) = workspace.read(cx).panel::(cx) { agent_panel.update(cx, |panel, cx| { panel.new_thread(&NewThread, window, cx); @@ -2543,11 +2600,13 @@ impl Sidebar { let weak_multi_workspace = self.multi_workspace.clone(); - let original_metadata = self - .focused_thread - .as_ref() - .and_then(|focused_id| entries.iter().find(|e| &e.session_id == focused_id)) - .map(|e| e.metadata.clone()); + let original_metadata = match &self.active_entry { + Some(ActiveEntry::Thread { session_id, .. }) => entries + .iter() + .find(|e| &e.session_id == session_id) + .map(|e| e.metadata.clone()), + _ => None, + }; let original_workspace = self .multi_workspace .upgrade() @@ -2569,7 +2628,10 @@ impl Sidebar { mw.activate(workspace.clone(), window, cx); }); } - this.focused_thread = Some(metadata.session_id.clone()); + this.active_entry = Some(ActiveEntry::Thread { + session_id: metadata.session_id.clone(), + workspace: workspace.clone(), + }); this.update_entries(cx); Self::load_agent_thread_in_workspace(workspace, metadata, false, window, cx); let focus = thread_switcher.focus_handle(cx); @@ -2585,7 +2647,10 @@ impl Sidebar { }); } this.record_thread_access(&metadata.session_id); - this.focused_thread = Some(metadata.session_id.clone()); + this.active_entry = Some(ActiveEntry::Thread { + session_id: metadata.session_id.clone(), + workspace: workspace.clone(), + }); this.update_entries(cx); Self::load_agent_thread_in_workspace(workspace, metadata, false, window, cx); this.dismiss_thread_switcher(cx); @@ -2602,7 +2667,12 @@ impl Sidebar { } } if let Some(metadata) = &original_metadata { - this.focused_thread = Some(metadata.session_id.clone()); + if let Some(original_ws) = &original_workspace { + this.active_entry = Some(ActiveEntry::Thread { + session_id: metadata.session_id.clone(), + workspace: original_ws.clone(), + }); + } this.update_entries(cx); if let Some(original_ws) = &original_workspace { Self::load_agent_thread_in_workspace( @@ -2651,7 +2721,10 @@ impl Sidebar { mw.activate(workspace.clone(), window, cx); }); } - self.focused_thread = Some(metadata.session_id.clone()); + self.active_entry = Some(ActiveEntry::Thread { + session_id: metadata.session_id.clone(), + workspace: workspace.clone(), + }); self.update_entries(cx); Self::load_agent_thread_in_workspace(&workspace, &metadata, false, window, cx); } @@ -2663,6 +2736,7 @@ impl Sidebar { &self, ix: usize, thread: &ThreadEntry, + is_active: bool, is_focused: bool, cx: &mut Context, ) -> AnyElement { @@ -2675,8 +2749,7 @@ impl Sidebar { let thread_workspace = thread.workspace.clone(); let is_hovered = self.hovered_thread_index == Some(ix); - let is_selected = self.agent_panel_visible(cx) - && self.focused_thread.as_ref() == Some(&metadata.session_id); + let is_selected = is_active; let is_running = matches!( thread.status, AgentThreadStatus::Running | AgentThreadStatus::WaitingForConfirmation @@ -2943,11 +3016,7 @@ impl Sidebar { return; }; - // Clear focused_thread immediately so no existing thread stays - // highlighted while the new blank thread is being shown. Without this, - // if the target workspace is already active (so ActiveWorkspaceChanged - // never fires), the previous thread's highlight would linger. - self.focused_thread = None; + self.active_entry = Some(ActiveEntry::Draft(workspace.clone())); multi_workspace.update(cx, |multi_workspace, cx| { multi_workspace.activate(workspace.clone(), window, cx); @@ -2968,14 +3037,11 @@ impl Sidebar { ix: usize, _path_list: &PathList, workspace: &Entity, - is_active_draft: bool, + is_active: bool, worktrees: &[WorktreeInfo], is_selected: bool, cx: &mut Context, ) -> AnyElement { - let is_active = - is_active_draft && self.agent_panel_visible(cx) && self.active_thread_is_draft(cx); - let label: SharedString = if is_active { self.active_draft_text(cx) .unwrap_or_else(|| DEFAULT_THREAD_TITLE.into()) diff --git a/crates/sidebar/src/sidebar_tests.rs b/crates/sidebar/src/sidebar_tests.rs index 53cac608c189449f28eb41e6166fbc2f5fcd568c..fb1519ec2c0f12cce023359084d566974685e2e5 100644 --- a/crates/sidebar/src/sidebar_tests.rs +++ b/crates/sidebar/src/sidebar_tests.rs @@ -30,6 +30,28 @@ fn init_test(cx: &mut TestAppContext) { }); } +#[track_caller] +fn assert_active_thread(sidebar: &Sidebar, session_id: &acp::SessionId, msg: &str) { + assert!( + sidebar + .active_entry + .as_ref() + .is_some_and(|e| e.is_active_thread(session_id)), + "{msg}: expected active_entry to be Thread({session_id:?}), got {:?}", + sidebar.active_entry, + ); +} + +#[track_caller] +fn assert_active_draft(sidebar: &Sidebar, workspace: &Entity, msg: &str) { + assert!( + matches!(&sidebar.active_entry, Some(ActiveEntry::Draft(ws)) if ws == workspace), + "{msg}: expected active_entry to be Draft for workspace {:?}, got {:?}", + workspace.entity_id(), + sidebar.active_entry, + ); +} + fn has_thread_entry(sidebar: &Sidebar, session_id: &acp::SessionId) -> bool { sidebar .contents @@ -1979,10 +2001,10 @@ async fn test_focused_thread_tracks_user_intent(cx: &mut TestAppContext) { // ── 1. Initial state: focused thread derived from active panel ───── sidebar.read_with(cx, |sidebar, _cx| { - assert_eq!( - sidebar.focused_thread.as_ref(), - Some(&session_id_a), - "The active panel's thread should be focused on startup" + assert_active_thread( + sidebar, + &session_id_a, + "The active panel's thread should be focused on startup", ); }); @@ -2005,10 +2027,10 @@ async fn test_focused_thread_tracks_user_intent(cx: &mut TestAppContext) { cx.run_until_parked(); sidebar.read_with(cx, |sidebar, _cx| { - assert_eq!( - sidebar.focused_thread.as_ref(), - Some(&session_id_a), - "After clicking a thread, it should be the focused thread" + assert_active_thread( + sidebar, + &session_id_a, + "After clicking a thread, it should be the focused thread", ); assert!( has_thread_entry(sidebar, &session_id_a), @@ -2060,10 +2082,10 @@ async fn test_focused_thread_tracks_user_intent(cx: &mut TestAppContext) { cx.run_until_parked(); sidebar.read_with(cx, |sidebar, _cx| { - assert_eq!( - sidebar.focused_thread.as_ref(), - Some(&session_id_b), - "Clicking a thread in another workspace should focus that thread" + assert_active_thread( + sidebar, + &session_id_b, + "Clicking a thread in another workspace should focus that thread", ); assert!( has_thread_entry(sidebar, &session_id_b), @@ -2078,10 +2100,10 @@ async fn test_focused_thread_tracks_user_intent(cx: &mut TestAppContext) { cx.run_until_parked(); sidebar.read_with(cx, |sidebar, _cx| { - assert_eq!( - sidebar.focused_thread.as_ref(), - Some(&session_id_a), - "Switching workspace should seed focused_thread from the new active panel" + assert_active_thread( + sidebar, + &session_id_a, + "Switching workspace should seed focused_thread from the new active panel", ); assert!( has_thread_entry(sidebar, &session_id_a), @@ -2104,10 +2126,10 @@ async fn test_focused_thread_tracks_user_intent(cx: &mut TestAppContext) { // This prevents running threads in background workspaces from causing // the selection highlight to jump around. sidebar.read_with(cx, |sidebar, _cx| { - assert_eq!( - sidebar.focused_thread.as_ref(), - Some(&session_id_a), - "Opening a thread in a non-active panel should not change focused_thread" + assert_active_thread( + sidebar, + &session_id_a, + "Opening a thread in a non-active panel should not change focused_thread", ); }); @@ -2117,10 +2139,10 @@ async fn test_focused_thread_tracks_user_intent(cx: &mut TestAppContext) { cx.run_until_parked(); sidebar.read_with(cx, |sidebar, _cx| { - assert_eq!( - sidebar.focused_thread.as_ref(), - Some(&session_id_a), - "Defocusing the sidebar should not change focused_thread" + assert_active_thread( + sidebar, + &session_id_a, + "Defocusing the sidebar should not change focused_thread", ); }); @@ -2135,10 +2157,10 @@ async fn test_focused_thread_tracks_user_intent(cx: &mut TestAppContext) { cx.run_until_parked(); sidebar.read_with(cx, |sidebar, _cx| { - assert_eq!( - sidebar.focused_thread.as_ref(), - Some(&session_id_b2), - "Switching workspace should seed focused_thread from the new active panel" + assert_active_thread( + sidebar, + &session_id_b2, + "Switching workspace should seed focused_thread from the new active panel", ); assert!( has_thread_entry(sidebar, &session_id_b2), @@ -2158,10 +2180,10 @@ async fn test_focused_thread_tracks_user_intent(cx: &mut TestAppContext) { cx.run_until_parked(); sidebar.read_with(cx, |sidebar, _cx| { - assert_eq!( - sidebar.focused_thread.as_ref(), - Some(&session_id_b2), - "Focusing the agent panel thread should set focused_thread" + assert_active_thread( + sidebar, + &session_id_b2, + "Focusing the agent panel thread should set focused_thread", ); assert!( has_thread_entry(sidebar, &session_id_b2), @@ -2199,10 +2221,11 @@ async fn test_new_thread_button_works_after_adding_folder(cx: &mut TestAppContex // The "New Thread" button should NOT be in "active/draft" state // because the panel has a thread with messages. - sidebar.read_with(cx, |sidebar, cx| { + sidebar.read_with(cx, |sidebar, _cx| { assert!( - !sidebar.active_thread_is_draft(cx), - "Panel has a thread with messages, so it should not be a draft" + matches!(&sidebar.active_entry, Some(ActiveEntry::Thread { .. })), + "Panel has a thread with messages, so active_entry should be Thread, got {:?}", + sidebar.active_entry, ); }); @@ -2229,11 +2252,12 @@ async fn test_new_thread_button_works_after_adding_folder(cx: &mut TestAppContex // The "New Thread" button must still be clickable (not stuck in // "active/draft" state). Verify that `active_thread_is_draft` is // false — the panel still has the old thread with messages. - sidebar.read_with(cx, |sidebar, cx| { + sidebar.read_with(cx, |sidebar, _cx| { assert!( - !sidebar.active_thread_is_draft(cx), + matches!(&sidebar.active_entry, Some(ActiveEntry::Thread { .. })), "After adding a folder the panel still has a thread with messages, \ - so active_thread_is_draft should be false" + so active_entry should be Thread, got {:?}", + sidebar.active_entry, ); }); @@ -2247,10 +2271,11 @@ async fn test_new_thread_button_works_after_adding_folder(cx: &mut TestAppContex // After creating a new thread, the panel should now be in draft // state (no messages on the new thread). - sidebar.read_with(cx, |sidebar, cx| { - assert!( - sidebar.active_thread_is_draft(cx), - "After creating a new thread the panel should be in draft state" + sidebar.read_with(cx, |sidebar, _cx| { + assert_active_draft( + sidebar, + &workspace, + "After creating a new thread active_entry should be Draft", ); }); } @@ -2301,14 +2326,59 @@ async fn test_cmd_n_shows_new_thread_entry(cx: &mut TestAppContext) { "After Cmd-N the sidebar should show a highlighted New Thread entry" ); - sidebar.read_with(cx, |sidebar, cx| { - assert!( - sidebar.focused_thread.is_none(), - "focused_thread should be cleared after Cmd-N" + sidebar.read_with(cx, |sidebar, _cx| { + assert_active_draft( + sidebar, + &workspace, + "active_entry should be Draft after Cmd-N", ); - assert!( - sidebar.active_thread_is_draft(cx), - "the new blank thread should be a draft" + }); +} + +#[gpui::test] +async fn test_draft_with_server_session_shows_as_draft(cx: &mut TestAppContext) { + let project = init_test_project_with_agent_panel("/my-project", cx).await; + let (multi_workspace, cx) = + cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + let (sidebar, panel) = setup_sidebar_with_agent_panel(&multi_workspace, &project, cx); + + let path_list = PathList::new(&[std::path::PathBuf::from("/my-project")]); + + // Create a saved thread so the workspace has history. + let connection = StubAgentConnection::new(); + connection.set_next_prompt_updates(vec![acp::SessionUpdate::AgentMessageChunk( + acp::ContentChunk::new("Done".into()), + )]); + open_thread_with_connection(&panel, connection, cx); + send_message(&panel, cx); + let saved_session_id = active_session_id(&panel, cx); + save_test_thread_metadata(&saved_session_id, path_list.clone(), cx).await; + cx.run_until_parked(); + + assert_eq!( + visible_entries_as_strings(&sidebar, cx), + vec!["v [my-project]", " Hello *"] + ); + + // Open a new draft thread via a server connection. This gives the + // conversation a parent_id (session assigned by the server) but + // no messages have been sent, so active_thread_is_draft() is true. + let draft_connection = StubAgentConnection::new(); + open_thread_with_connection(&panel, draft_connection, cx); + cx.run_until_parked(); + + assert_eq!( + visible_entries_as_strings(&sidebar, cx), + vec!["v [my-project]", " [+ New Thread]", " Hello *"], + "Draft with a server session should still show as [+ New Thread]" + ); + + let workspace = multi_workspace.read_with(cx, |mw, _cx| mw.workspace().clone()); + sidebar.read_with(cx, |sidebar, _cx| { + assert_active_draft( + sidebar, + &workspace, + "Draft with server session should be Draft, not Thread", ); }); } @@ -2437,14 +2507,11 @@ async fn test_cmd_n_shows_new_thread_entry_in_absorbed_worktree(cx: &mut TestApp a highlighted New Thread entry under the main repo header" ); - sidebar.read_with(cx, |sidebar, cx| { - assert!( - sidebar.focused_thread.is_none(), - "focused_thread should be cleared after Cmd-N" - ); - assert!( - sidebar.active_thread_is_draft(cx), - "the new blank thread should be a draft" + sidebar.read_with(cx, |sidebar, _cx| { + assert_active_draft( + sidebar, + &worktree_workspace, + "active_entry should be Draft after Cmd-N", ); }); } @@ -3894,8 +3961,8 @@ async fn test_activate_archived_thread_reuses_workspace_in_another_window(cx: &m "should activate the window that already owns the matching workspace" ); sidebar.read_with(cx_a, |sidebar, _| { - assert_eq!( - sidebar.focused_thread, None, + assert!( + !matches!(&sidebar.active_entry, Some(ActiveEntry::Thread { session_id: id, .. }) if id == &session_id), "source window's sidebar should not eagerly claim focus for a thread opened in another window" ); }); @@ -3970,16 +4037,16 @@ async fn test_activate_archived_thread_reuses_workspace_in_another_window_with_t "should activate the window that already owns the matching workspace" ); sidebar_a.read_with(cx_a, |sidebar, _| { - assert_eq!( - sidebar.focused_thread, None, + assert!( + !matches!(&sidebar.active_entry, Some(ActiveEntry::Thread { session_id: id, .. }) if id == &session_id), "source window's sidebar should not eagerly claim focus for a thread opened in another window" ); }); sidebar_b.read_with(cx_b, |sidebar, _| { - assert_eq!( - sidebar.focused_thread.as_ref(), - Some(&session_id), - "target window's sidebar should eagerly focus the activated archived thread" + assert_active_thread( + sidebar, + &session_id, + "target window's sidebar should eagerly focus the activated archived thread", ); }); } @@ -4031,10 +4098,10 @@ async fn test_activate_archived_thread_prefers_current_window_for_matching_paths "should keep activation in the current window when it already has a matching workspace" ); sidebar_a.read_with(cx_a, |sidebar, _| { - assert_eq!( - sidebar.focused_thread.as_ref(), - Some(&session_id), - "current window's sidebar should eagerly focus the activated archived thread" + assert_active_thread( + sidebar, + &session_id, + "current window's sidebar should eagerly focus the activated archived thread", ); }); assert_eq!( @@ -4188,13 +4255,13 @@ async fn test_archive_thread_uses_next_threads_own_workspace(cx: &mut TestAppCon // The sidebar should track T2 as the focused thread (derived from the // main panel's active view). - let focused = sidebar.read_with(cx, |s, _| s.focused_thread.clone()); - assert_eq!( - focused, - Some(thread2_session_id.clone()), - "focused thread should be Thread 2 before archiving: {:?}", - focused - ); + sidebar.read_with(cx, |s, _| { + assert_active_thread( + s, + &thread2_session_id, + "focused thread should be Thread 2 before archiving", + ); + }); // Archive thread 2. sidebar.update_in(cx, |sidebar, window, cx| { @@ -4816,6 +4883,7 @@ mod property_test { SaveWorktreeThread { worktree_index: usize }, DeleteThread { index: usize }, ToggleAgentPanel, + CreateDraftThread, AddWorkspace, OpenWorktreeAsWorkspace { worktree_index: usize }, RemoveWorkspace { index: usize }, @@ -4824,16 +4892,17 @@ mod property_test { } // Distribution (out of 20 slots): - // SaveThread: 5 slots (25%) - // SaveWorktreeThread: 2 slots (10%) - // DeleteThread: 2 slots (10%) - // ToggleAgentPanel: 2 slots (10%) - // AddWorkspace: 1 slot (5%) - // OpenWorktreeAsWorkspace: 1 slot (5%) - // RemoveWorkspace: 1 slot (5%) - // SwitchWorkspace: 2 slots (10%) - // AddLinkedWorktree: 4 slots (20%) - const DISTRIBUTION_SLOTS: u32 = 20; + // SaveThread: 5 slots (~23%) + // SaveWorktreeThread: 2 slots (~9%) + // DeleteThread: 2 slots (~9%) + // ToggleAgentPanel: 2 slots (~9%) + // CreateDraftThread: 2 slots (~9%) + // AddWorkspace: 1 slot (~5%) + // OpenWorktreeAsWorkspace: 1 slot (~5%) + // RemoveWorkspace: 1 slot (~5%) + // SwitchWorkspace: 2 slots (~9%) + // AddLinkedWorktree: 4 slots (~18%) + const DISTRIBUTION_SLOTS: u32 = 22; impl TestState { fn generate_operation(&self, raw: u32) -> Operation { @@ -4857,24 +4926,25 @@ mod property_test { workspace_index: extra % workspace_count, }, 9..=10 => Operation::ToggleAgentPanel, - 11 if !self.unopened_worktrees.is_empty() => Operation::OpenWorktreeAsWorkspace { + 11..=12 => Operation::CreateDraftThread, + 13 if !self.unopened_worktrees.is_empty() => Operation::OpenWorktreeAsWorkspace { worktree_index: extra % self.unopened_worktrees.len(), }, - 11 => Operation::AddWorkspace, - 12 if workspace_count > 1 => Operation::RemoveWorkspace { + 13 => Operation::AddWorkspace, + 14 if workspace_count > 1 => Operation::RemoveWorkspace { index: extra % workspace_count, }, - 12 => Operation::AddWorkspace, - 13..=14 => Operation::SwitchWorkspace { + 14 => Operation::AddWorkspace, + 15..=16 => Operation::SwitchWorkspace { index: extra % workspace_count, }, - 15..=19 if !self.main_repo_indices.is_empty() => { + 17..=21 if !self.main_repo_indices.is_empty() => { let main_index = self.main_repo_indices[extra % self.main_repo_indices.len()]; Operation::AddLinkedWorktree { workspace_index: main_index, } } - 15..=19 => Operation::SaveThread { + 17..=21 => Operation::SaveThread { workspace_index: extra % workspace_count, }, _ => unreachable!(), @@ -4910,7 +4980,7 @@ mod property_test { operation: Operation, state: &mut TestState, multi_workspace: &Entity, - sidebar: &Entity, + _sidebar: &Entity, cx: &mut gpui::VisualTestContext, ) { match operation { @@ -4936,7 +5006,7 @@ mod property_test { Operation::ToggleAgentPanel => { let workspace = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone()); let panel_open = - sidebar.read_with(cx, |sidebar, cx| sidebar.agent_panel_visible(cx)); + workspace.read_with(cx, |_, cx| AgentPanel::is_visible(&workspace, cx)); workspace.update_in(cx, |workspace, window, cx| { if panel_open { workspace.close_panel::(window, cx); @@ -4945,6 +5015,19 @@ mod property_test { } }); } + Operation::CreateDraftThread => { + let workspace = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone()); + let panel = + workspace.read_with(cx, |workspace, cx| workspace.panel::(cx)); + if let Some(panel) = panel { + let connection = StubAgentConnection::new(); + open_thread_with_connection(&panel, connection, cx); + cx.run_until_parked(); + } + workspace.update_in(cx, |workspace, window, cx| { + workspace.focus_panel::(window, cx); + }); + } Operation::AddWorkspace => { let path = state.next_workspace_path(); state @@ -5184,34 +5267,59 @@ mod property_test { anyhow::bail!("sidebar should still have an associated multi-workspace"); }; - let workspace = multi_workspace.read(cx).workspace(); - - // TODO: The focused_thread should _always_ be Some(item-in-the-list) after - // update_entries. If the activated workspace's agent panel has an active thread, - // this item should match the one in the list. There may be a slight delay where - // a thread is loading so the agent panel returns None initially, and the - // focused_thread is often optimistically set to the thread the agent panel is - // going to be. - if sidebar.agent_panel_visible(cx) && !sidebar.active_thread_is_draft(cx) { - let panel_active_session_id = - workspace - .read(cx) - .panel::(cx) - .and_then(|panel| { - panel - .read(cx) - .active_conversation_view() - .and_then(|cv| cv.read(cx).parent_id(cx)) - }); - if let Some(panel_session_id) = panel_active_session_id { - anyhow::ensure!( - sidebar.focused_thread.as_ref() == Some(&panel_session_id), - "agent panel is visible with active session {:?} but sidebar focused_thread is {:?}", - panel_session_id, - sidebar.focused_thread, - ); - } + let active_workspace = multi_workspace.read(cx).workspace(); + + // 1. active_entry must always be Some after rebuild_contents. + let entry = sidebar + .active_entry + .as_ref() + .ok_or_else(|| anyhow::anyhow!("active_entry must always be Some"))?; + + // 2. The entry's workspace must agree with the multi-workspace's + // active workspace. + anyhow::ensure!( + entry.workspace().entity_id() == active_workspace.entity_id(), + "active_entry workspace ({:?}) != active workspace ({:?})", + entry.workspace().entity_id(), + active_workspace.entity_id(), + ); + + // 3. The entry must match the agent panel's current state. + let panel = active_workspace.read(cx).panel::(cx).unwrap(); + if panel.read(cx).active_thread_is_draft(cx) { + anyhow::ensure!( + matches!(entry, ActiveEntry::Draft(_)), + "panel shows a draft but active_entry is {:?}", + entry, + ); + } else if let Some(session_id) = panel + .read(cx) + .active_conversation_view() + .and_then(|cv| cv.read(cx).parent_id(cx)) + { + anyhow::ensure!( + matches!(entry, ActiveEntry::Thread { session_id: id, .. } if id == &session_id), + "panel has session {:?} but active_entry is {:?}", + session_id, + entry, + ); } + + // 4. Exactly one entry in sidebar contents must be uniquely + // identified by the active_entry. + let matching_count = sidebar + .contents + .entries + .iter() + .filter(|e| entry.matches_entry(e)) + .count(); + anyhow::ensure!( + matching_count == 1, + "expected exactly 1 sidebar entry matching active_entry {:?}, found {}", + entry, + matching_count, + ); + Ok(()) }