diff --git a/crates/agent_ui/src/agent_panel.rs b/crates/agent_ui/src/agent_panel.rs index ffb13b4d7074ec57d320defb535016a5c8f09b12..b2c8eae7d62358aff0acbc1784cdd17e52c19a3c 100644 --- a/crates/agent_ui/src/agent_panel.rs +++ b/crates/agent_ui/src/agent_panel.rs @@ -86,7 +86,7 @@ use ui::{ use util::{ResultExt as _, debug_panic}; use workspace::{ CollaboratorId, DraggedSelection, DraggedTab, PathList, SerializedPathList, - ToggleWorkspaceSidebar, ToggleZoom, Workspace, WorkspaceId, WorkspaceSidebarDelegate, + ToggleWorkspaceSidebar, ToggleZoom, Workspace, WorkspaceId, dock::{DockPosition, Panel, PanelEvent}, }; @@ -109,39 +109,6 @@ impl MaxIdleRetainedThreads { } } -#[derive(Default)] -struct AgentPanelSidebarDelegate; - -impl WorkspaceSidebarDelegate for AgentPanelSidebarDelegate { - fn reconcile_group( - &self, - workspace: &mut Workspace, - group_key: &workspace::ProjectGroupKey, - window: &mut Window, - cx: &mut Context, - ) -> bool { - if workspace.project_group_key(cx) != *group_key { - return false; - } - - let Some(panel) = workspace.panel::(cx) else { - return false; - }; - - panel.update(cx, |panel, cx| { - if panel.pending_thread_loads > 0 { - return false; - } - if panel.draft_thread_ids(cx).is_empty() { - panel.create_thread(window, cx); - true - } else { - false - } - }) - } -} - #[derive(Serialize, Deserialize)] struct LastUsedAgent { agent: Agent, @@ -218,7 +185,6 @@ struct SerializedActiveThread { pub fn init(cx: &mut App) { cx.observe_new( |workspace: &mut Workspace, _window, _cx: &mut Context| { - workspace.set_sidebar_delegate(Arc::new(AgentPanelSidebarDelegate)); workspace .register_action(|workspace, action: &NewThread, window, cx| { if let Some(panel) = workspace.panel::(cx) { @@ -1022,10 +988,6 @@ impl AgentPanel { cx, ); }); - } else { - panel.update(cx, |panel, cx| { - panel.show_or_create_empty_draft(window, cx); - }); } panel })?; @@ -1358,30 +1320,16 @@ impl AgentPanel { .unwrap_or(false) } - /// Clear any active conversation while preserving a real empty draft. - /// Running non-draft threads are retained in the background. - pub fn clear_active_thread(&mut self, window: &mut Window, cx: &mut Context) { - self.show_or_create_empty_draft(window, cx); - } - - fn show_or_create_empty_draft(&mut self, window: &mut Window, cx: &mut Context) { - if self.active_thread_is_draft(cx) { - self.clear_overlay_state(); - self.refresh_base_view_subscriptions(window, cx); - self.serialize(cx); - cx.emit(AgentPanelEvent::ActiveViewChanged); - cx.notify(); - return; - } - - if let Some(draft_id) = self.draft_thread_ids(cx).into_iter().next() { - self.activate_retained_thread(draft_id, false, window, cx); - cx.notify(); - return; - } - - let id = self.create_thread(window, cx); - self.activate_retained_thread(id, false, window, cx); + /// Clear the active view, retaining any running thread in the background. + pub fn clear_base_view(&mut self, cx: &mut Context) { + let old_view = std::mem::replace(&mut self.base_view, BaseView::Uninitialized); + self.retain_running_thread(old_view, cx); + self.clear_overlay_state(); + self._thread_view_subscription = None; + self._active_thread_focus_subscription = None; + self._base_view_observation = None; + self.serialize(cx); + cx.emit(AgentPanelEvent::ActiveViewChanged); cx.notify(); } @@ -5510,8 +5458,8 @@ mod tests { ); }); - // Workspace B should restore its own agent type and seed an empty draft. - loaded_b.read_with(cx, |panel, cx| { + // Workspace B should restore its own agent type but have no active thread. + loaded_b.read_with(cx, |panel, _cx| { assert_eq!( panel.selected_agent, Agent::Custom { @@ -5520,12 +5468,8 @@ mod tests { "workspace B agent type should be restored" ); assert!( - panel.active_conversation_view().is_some(), - "workspace B should show an empty draft" - ); - assert!( - panel.active_thread_is_draft(cx), - "workspace B should seed a draft in an empty workspace" + panel.active_conversation_view().is_none(), + "workspace B should have no active thread when it had no prior conversation" ); }); } @@ -5587,14 +5531,10 @@ mod tests { .expect("panel load should succeed"); cx.run_until_parked(); - loaded.read_with(cx, |panel, cx| { + loaded.read_with(cx, |panel, _cx| { assert!( - panel.active_conversation_view().is_some(), - "empty workspaces should still show a draft after load" - ); - assert!( - panel.active_thread_is_draft(cx), - "thread without metadata should not be restored; the panel should fall back to a fresh draft" + panel.active_conversation_view().is_none(), + "thread without metadata should not be restored; the panel should have no active thread" ); }); } @@ -6265,69 +6205,6 @@ mod tests { }); } - #[gpui::test] - async fn test_clear_active_thread_creates_real_empty_draft(cx: &mut TestAppContext) { - let (panel, mut cx) = setup_panel(cx).await; - - let connection = StubAgentConnection::new(); - connection.set_next_prompt_updates(vec![acp::SessionUpdate::AgentMessageChunk( - acp::ContentChunk::new("done".into()), - )]); - open_thread_with_connection(&panel, connection, &mut cx); - send_message(&panel, &mut cx); - - panel.read_with(&cx, |panel, cx| { - assert!( - panel.draft_thread_ids(cx).is_empty(), - "sent thread should not leave any draft entries before clearing" - ); - }); - - panel.update_in(&mut cx, |panel, window, cx| { - panel.clear_active_thread(window, cx); - }); - cx.run_until_parked(); - - panel.read_with(&cx, |panel, cx| { - assert!(panel.active_thread_is_draft(cx)); - assert_eq!(panel.draft_thread_ids(cx).len(), 1); - }); - } - - #[gpui::test] - async fn test_clear_active_thread_reuses_retained_empty_draft(cx: &mut TestAppContext) { - let (panel, mut cx) = setup_panel(cx).await; - - let connection_a = StubAgentConnection::new(); - connection_a.set_next_prompt_updates(vec![acp::SessionUpdate::AgentMessageChunk( - acp::ContentChunk::new("done".into()), - )]); - open_thread_with_connection(&panel, connection_a, &mut cx); - send_message(&panel, &mut cx); - - panel.update_in(&mut cx, |panel, window, cx| { - panel.new_thread(&NewThread, window, cx); - }); - cx.run_until_parked(); - - let retained_draft_id = panel.read_with(&cx, |panel, cx| { - let ids = panel.draft_thread_ids(cx); - assert_eq!(ids.len(), 1); - ids[0] - }); - - panel.update_in(&mut cx, |panel, window, cx| { - panel.clear_active_thread(window, cx); - }); - cx.run_until_parked(); - - panel.read_with(&cx, |panel, cx| { - assert_eq!(panel.active_thread_id(cx), Some(retained_draft_id)); - assert!(panel.active_thread_is_draft(cx)); - assert_eq!(panel.draft_thread_ids(cx), vec![retained_draft_id]); - }); - } - #[gpui::test] async fn test_thread_target_local_project(cx: &mut TestAppContext) { init_test(cx); diff --git a/crates/sidebar/src/sidebar.rs b/crates/sidebar/src/sidebar.rs index bdb637c37832995fc6e3dd3bb8ff567ff995b868..e8e68a07ccfabddc8794ae851289395073579e2e 100644 --- a/crates/sidebar/src/sidebar.rs +++ b/crates/sidebar/src/sidebar.rs @@ -448,7 +448,6 @@ pub struct Sidebar { project_header_menu_ix: Option, _subscriptions: Vec, _draft_observations: Vec, - reconciling: bool, } impl Sidebar { @@ -476,21 +475,17 @@ impl Sidebar { this.sync_active_entry_from_active_workspace(cx); this.observe_draft_editors(cx); this.update_entries(cx); - this.reconcile_groups(window, cx); } MultiWorkspaceEvent::WorkspaceAdded(workspace) => { this.subscribe_to_workspace(workspace, window, cx); this.update_entries(cx); - this.reconcile_groups(window, cx); } MultiWorkspaceEvent::WorkspaceRemoved(_) => { this.update_entries(cx); - this.reconcile_groups(window, cx); } MultiWorkspaceEvent::ProjectGroupKeyUpdated { old_key, new_key } => { this.move_threads_for_key_change(old_key, new_key, cx); this.update_entries(cx); - this.reconcile_groups(window, cx); } }, ) @@ -521,7 +516,6 @@ impl Sidebar { this.subscribe_to_workspace(workspace, window, cx); } this.update_entries(cx); - this.reconcile_groups(window, cx); }); Self { @@ -546,7 +540,6 @@ impl Sidebar { project_header_menu_ix: None, _subscriptions: Vec::new(), _draft_observations: Vec::new(), - reconciling: false, } } @@ -620,19 +613,17 @@ impl Sidebar { cx.subscribe_in( &project, window, - |this, project, event, window, cx| match event { + |this, project, event, _window, cx| match event { ProjectEvent::WorktreeAdded(_) | ProjectEvent::WorktreeRemoved(_) | ProjectEvent::WorktreeOrderChanged => { this.observe_draft_editors(cx); this.update_entries(cx); - this.reconcile_groups(window, cx); } ProjectEvent::WorktreePathsChanged { old_worktree_paths } => { this.move_thread_paths(project, old_worktree_paths, cx); this.observe_draft_editors(cx); this.update_entries(cx); - this.reconcile_groups(window, cx); } _ => {} }, @@ -666,7 +657,6 @@ impl Sidebar { if let Ok(agent_panel) = view.clone().downcast::() { this.subscribe_to_agent_panel(&agent_panel, window, cx); this.update_entries(cx); - this.reconcile_groups(window, cx); } } }, @@ -784,29 +774,19 @@ impl Sidebar { cx.subscribe_in( agent_panel, window, - |this, _agent_panel, event: &AgentPanelEvent, window, cx| match event { + |this, _agent_panel, event: &AgentPanelEvent, _window, cx| match event { AgentPanelEvent::ActiveViewChanged => { - let resolved_pending_activation = - this.sync_active_entry_from_panel(_agent_panel, cx); - if resolved_pending_activation { - let active_workspace = this.active_workspace(cx); - if let Some(active_workspace) = active_workspace { - this.clear_empty_group_drafts(&active_workspace, cx); - } - } + this.sync_active_entry_from_panel(_agent_panel, cx); this.observe_draft_editors(cx); this.update_entries(cx); - this.reconcile_groups(window, cx); } AgentPanelEvent::ThreadFocused | AgentPanelEvent::RetainedThreadChanged => { this.sync_active_entry_from_panel(_agent_panel, cx); this.update_entries(cx); - this.reconcile_groups(window, cx); } AgentPanelEvent::MessageSentOrQueued { thread_id } => { this.record_thread_message_sent(thread_id); this.update_entries(cx); - this.reconcile_groups(window, cx); } }, ) @@ -1499,79 +1479,6 @@ impl Sidebar { }; } - /// Gives each workspace's sidebar delegate a chance to reconcile its - /// project group (e.g. create a draft when the group is empty). - /// - /// Called at the top of `render` so we have `Window` + `Context` - /// available for panel mutations. - fn reconcile_groups(&mut self, window: &mut Window, cx: &mut Context) { - if self.reconciling - || self.pending_thread_activation.is_some() - || !self.restoring_tasks.is_empty() - { - return; - } - self.reconciling = true; - - let Some(multi_workspace) = self.multi_workspace.upgrade() else { - self.reconciling = false; - return; - }; - - let empty_group_keys: Vec = self - .contents - .entries - .iter() - .filter_map(|entry| match entry { - ListEntry::ProjectHeader { - key, - has_threads: false, - .. - } => Some(key.clone()), - _ => None, - }) - .collect(); - - if empty_group_keys.is_empty() { - self.reconciling = false; - return; - } - - let mut did_reconcile = false; - - for key in &empty_group_keys { - let workspace = { - let mw = multi_workspace.read(cx); - let active = mw.workspace().clone(); - if active.read(cx).project_group_key(cx) == *key { - Some(active) - } else { - mw.workspace_for_paths(key.path_list(), key.host().as_ref(), cx) - } - }; - - let Some(workspace) = workspace else { - continue; - }; - - let Some(delegate) = workspace.read(cx).sidebar_delegate() else { - continue; - }; - - let changed = workspace.update(cx, |workspace, cx| { - delegate.reconcile_group(workspace, key, window, cx) - }); - - did_reconcile |= changed; - } - - if did_reconcile { - self.update_entries(cx); - } - - self.reconciling = false; - } - /// Rebuilds the sidebar's visible entries from already-cached state. fn update_entries(&mut self, cx: &mut Context) { let Some(multi_workspace) = self.multi_workspace.upgrade() else { @@ -1720,7 +1627,7 @@ impl Sidebar { let disclosure_id = SharedString::from(format!("disclosure-{ix}")); let group_name = SharedString::from(format!("{id_prefix}header-group-{ix}")); - let is_collapsed = self.is_group_collapsed(key, cx); + let is_collapsed = !has_threads || self.is_group_collapsed(key, cx); let (disclosure_icon, disclosure_tooltip) = if is_collapsed { (IconName::ChevronRight, "Expand Project") } else { @@ -1792,12 +1699,19 @@ impl Sidebar { IconButton::new(disclosure_id, disclosure_icon) .shape(ui::IconButtonShape::Square) .icon_size(IconSize::Small) - .icon_color(Color::Custom(cx.theme().colors().icon_muted.opacity(0.5))) - .tooltip(Tooltip::text(disclosure_tooltip)) - .on_click(cx.listener(move |this, _, window, cx| { - this.selection = None; - this.toggle_collapse(&key_for_toggle, window, cx); - })), + .icon_color(if has_threads { + Color::Custom(cx.theme().colors().icon_muted.opacity(0.5)) + } else { + Color::Custom(cx.theme().colors().icon_disabled) + }) + .when(has_threads, |this| { + this.tooltip(Tooltip::text(disclosure_tooltip)).on_click( + cx.listener(move |this, _, window, cx| { + this.selection = None; + this.toggle_collapse(&key_for_toggle, window, cx); + }), + ) + }), ) .child(label) .when_some( @@ -1845,7 +1759,7 @@ impl Sidebar { cx.stop_propagation(); }) .child(self.render_project_header_ellipsis_menu(ix, id_prefix, key, cx)) - .when(view_more_expanded && !is_collapsed, |this| { + .when(has_threads && view_more_expanded && !is_collapsed, |this| { this.child( IconButton::new( SharedString::from(format!( @@ -2369,9 +2283,13 @@ impl Sidebar { }; match entry { - ListEntry::ProjectHeader { key, .. } => { - let key = key.clone(); - self.toggle_collapse(&key, window, cx); + ListEntry::ProjectHeader { + key, has_threads, .. + } => { + if *has_threads { + let key = key.clone(); + self.toggle_collapse(&key, window, cx); + } } ListEntry::Thread(thread) => { let metadata = thread.metadata.clone(); @@ -2509,40 +2427,6 @@ impl Sidebar { .detach_and_log_err(cx); } - fn clear_empty_group_drafts(&mut self, workspace: &Entity, cx: &mut Context) { - let Some(multi_workspace) = self.multi_workspace.upgrade() else { - return; - }; - - let group_key = workspace.read(cx).project_group_key(cx); - let group_workspaces: Vec<_> = multi_workspace - .read(cx) - .workspaces() - .filter(|candidate| candidate.read(cx).project_group_key(cx) == group_key) - .cloned() - .collect(); - - for group_workspace in group_workspaces { - group_workspace.update(cx, |workspace, cx| { - let Some(panel) = workspace.panel::(cx) else { - return; - }; - - panel.update(cx, |panel, cx| { - let empty_draft_ids: Vec = panel - .draft_thread_ids(cx) - .into_iter() - .filter(|id| panel.editor_text(*id, cx).is_none()) - .collect(); - - for id in empty_draft_ids { - panel.remove_thread(id, cx); - } - }); - }); - } - } - fn activate_thread_locally( &mut self, metadata: &ThreadMetadata, @@ -2592,7 +2476,6 @@ impl Sidebar { self.observe_draft_editors(cx); } else { Self::load_agent_thread_in_workspace(workspace, metadata, true, window, cx); - self.clear_empty_group_drafts(workspace, cx); } self.update_entries(cx); @@ -2635,7 +2518,6 @@ impl Sidebar { workspace: workspace_for_entry.clone(), }); sidebar.record_thread_access(&target_session_id); - sidebar.clear_empty_group_drafts(&workspace_for_entry, cx); sidebar.update_entries(cx); }); } @@ -2919,15 +2801,19 @@ impl Sidebar { let Some(ix) = self.selection else { return }; match self.contents.entries.get(ix) { - Some(ListEntry::ProjectHeader { key, .. }) => { - let key = key.clone(); - if self.is_group_collapsed(&key, cx) { - self.set_group_expanded(&key, true, cx); - self.update_entries(cx); - } else if ix + 1 < self.contents.entries.len() { - self.selection = Some(ix + 1); - self.list_state.scroll_to_reveal_item(ix + 1); - cx.notify(); + Some(ListEntry::ProjectHeader { + key, has_threads, .. + }) => { + if *has_threads { + let key = key.clone(); + if self.is_group_collapsed(&key, cx) { + self.set_group_expanded(&key, true, cx); + self.update_entries(cx); + } else if ix + 1 < self.contents.entries.len() { + self.selection = Some(ix + 1); + self.list_state.scroll_to_reveal_item(ix + 1); + cx.notify(); + } } } _ => {} @@ -2943,11 +2829,15 @@ impl Sidebar { let Some(ix) = self.selection else { return }; match self.contents.entries.get(ix) { - Some(ListEntry::ProjectHeader { key, .. }) => { - let key = key.clone(); - if !self.is_group_collapsed(&key, cx) { - self.set_group_expanded(&key, false, cx); - self.update_entries(cx); + Some(ListEntry::ProjectHeader { + key, has_threads, .. + }) => { + if *has_threads { + let key = key.clone(); + if !self.is_group_collapsed(&key, cx) { + self.set_group_expanded(&key, false, cx); + self.update_entries(cx); + } } } Some(ListEntry::Thread(_) | ListEntry::ViewMore { .. }) => { @@ -2987,16 +2877,20 @@ impl Sidebar { }; if let Some(header_ix) = header_ix { - if let Some(ListEntry::ProjectHeader { key, .. }) = self.contents.entries.get(header_ix) + if let Some(ListEntry::ProjectHeader { + key, has_threads, .. + }) = self.contents.entries.get(header_ix) { - let key = key.clone(); - if self.is_group_collapsed(&key, cx) { - self.set_group_expanded(&key, true, cx); - } else { - self.selection = Some(header_ix); - self.set_group_expanded(&key, false, cx); + if *has_threads { + let key = key.clone(); + if self.is_group_collapsed(&key, cx) { + self.set_group_expanded(&key, true, cx); + } else { + self.selection = Some(header_ix); + self.set_group_expanded(&key, false, cx); + } + self.update_entries(cx); } - self.update_entries(cx); } } } @@ -3415,11 +3309,7 @@ impl Sidebar { }); if panel_shows_archived { panel.update(cx, |panel, cx| { - // Replace the archived thread with a - // tracked draft so the panel isn't left - // in Uninitialized state. - let id = panel.create_thread(window, cx); - panel.activate_retained_thread(id, false, window, cx); + panel.clear_base_view(cx); }); } } @@ -3449,27 +3339,20 @@ impl Sidebar { } } - // No neighbor or its workspace isn't open — fall back to a new - // draft. Use the group workspace (main project) rather than the - // active entry workspace, which may be a linked worktree that is - // about to be cleaned up or already removed. - let fallback_workspace = thread_folder_paths - .and_then(|folder_paths| { - let mw = self.multi_workspace.upgrade()?; - let mw = mw.read(cx); - let thread_workspace = mw.workspace_for_paths(folder_paths, None, cx)?; - let group_key = thread_workspace.read(cx).project_group_key(cx); - mw.workspace_for_paths(group_key.path_list(), None, cx) - }) - .or_else(|| { - self.multi_workspace - .upgrade() - .map(|mw| mw.read(cx).workspace().clone()) - }); - - if let Some(workspace) = fallback_workspace { - self.activate_workspace(&workspace, window, cx); - self.create_new_thread(&workspace, window, cx); + // No neighbor or its workspace isn't open — just clear the + // panel so the group is left empty. + if let Some(folder_paths) = thread_folder_paths { + let workspace = self + .multi_workspace + .upgrade() + .and_then(|mw| mw.read(cx).workspace_for_paths(folder_paths, None, cx)); + if let Some(workspace) = workspace { + if let Some(panel) = workspace.read(cx).panel::(cx) { + panel.update(cx, |panel, cx| { + panel.clear_base_view(cx); + }); + } + } } } @@ -4281,7 +4164,6 @@ impl Sidebar { } self.update_entries(cx); - self.reconcile_groups(window, cx); } /// Cleans, collapses whitespace, and truncates raw editor text diff --git a/crates/sidebar/src/sidebar_tests.rs b/crates/sidebar/src/sidebar_tests.rs index 104f835cc1393077557d8072f1c283ddbdbbc801..e2515719d59b378c1550dec08264a08d98300bb0 100644 --- a/crates/sidebar/src/sidebar_tests.rs +++ b/crates/sidebar/src/sidebar_tests.rs @@ -590,7 +590,7 @@ async fn test_single_workspace_no_threads(cx: &mut TestAppContext) { assert_eq!( visible_entries_as_strings(&_sidebar, cx), - vec!["v [my-project]", " [~ Draft]"] + vec!["v [my-project]"] ); } @@ -1496,10 +1496,10 @@ async fn test_keyboard_navigation_on_empty_list(cx: &mut TestAppContext) { cx.add_window_view(|window, cx| MultiWorkspace::test_new(project, window, cx)); let (sidebar, _panel) = setup_sidebar_with_agent_panel(&multi_workspace, cx); - // An empty project has the header and an auto-created draft. + // An empty project has only the header (no auto-created draft). assert_eq!( visible_entries_as_strings(&sidebar, cx), - vec!["v [empty-project]", " [~ Draft]"] + vec!["v [empty-project]"] ); // Focus sidebar — focus_in does not set a selection @@ -1510,17 +1510,17 @@ async fn test_keyboard_navigation_on_empty_list(cx: &mut TestAppContext) { cx.dispatch_action(SelectNext); assert_eq!(sidebar.read_with(cx, |s, _| s.selection), Some(0)); - // SelectNext advances to index 1 (draft entry) - cx.dispatch_action(SelectNext); - assert_eq!(sidebar.read_with(cx, |s, _| s.selection), Some(1)); - - // At the end (two entries), wraps back to first entry + // SelectNext with only one entry stays at index 0 cx.dispatch_action(SelectNext); assert_eq!(sidebar.read_with(cx, |s, _| s.selection), Some(0)); // SelectPrevious from first entry clears selection (returns to editor) cx.dispatch_action(SelectPrevious); assert_eq!(sidebar.read_with(cx, |s, _| s.selection), None); + + // SelectPrevious from None selects the last entry + cx.dispatch_action(SelectPrevious); + assert_eq!(sidebar.read_with(cx, |s, _| s.selection), Some(0)); } #[gpui::test] @@ -2443,7 +2443,7 @@ async fn test_confirm_on_historical_thread_preserves_historical_timestamp_and_or historical_entries_after, vec![ " Newer Historical Thread".to_string(), - " Older Historical Thread".to_string(), + " Older Historical Thread <== selected".to_string(), ], "activating an older historical thread should not reorder it ahead of a newer historical thread" ); @@ -3114,26 +3114,6 @@ async fn test_draft_title_updates_from_editor_text(cx: &mut TestAppContext) { cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let (sidebar, _panel) = setup_sidebar_with_agent_panel(&multi_workspace, cx); - // The reconciliation-created draft should show the default title. - let draft_title = sidebar.read_with(cx, |sidebar, _cx| { - sidebar - .contents - .entries - .iter() - .find_map(|entry| match entry { - ListEntry::Thread(thread) if thread.is_draft => { - Some(thread.metadata.display_title()) - } - _ => None, - }) - .expect("should have a draft entry") - }); - assert_eq!( - draft_title.as_ref(), - "New Agent Thread", - "draft should start with default title" - ); - // Create a new thread (activates the draft as base view and connects). let workspace = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone()); let panel = workspace.read_with(cx, |ws, cx| ws.panel::(cx).unwrap()); @@ -3189,26 +3169,6 @@ async fn test_draft_title_updates_across_two_groups(cx: &mut TestAppContext) { let panel_b = add_agent_panel(&workspace_b, cx); cx.run_until_parked(); - // Both groups should have reconciliation drafts. - let draft_titles: Vec<(SharedString, bool)> = sidebar.read_with(cx, |sidebar, _cx| { - sidebar - .contents - .entries - .iter() - .filter_map(|entry| match entry { - ListEntry::Thread(thread) if thread.is_draft => { - Some((thread.metadata.display_title(), false)) - } - _ => None, - }) - .collect() - }); - assert_eq!( - draft_titles.len(), - 2, - "should have two drafts, one per group" - ); - // Open a thread in each group's panel to get Connected state. let workspace_a = multi_workspace.read_with(cx, |mw, _cx| mw.workspaces().next().unwrap().clone()); @@ -5537,7 +5497,6 @@ async fn test_linked_worktree_threads_not_duplicated_across_groups(cx: &mut Test vec![ // "v [other, project]", - " [~ Draft]", "v [project]", " Worktree Thread {wt-feature-a}", ] @@ -6023,11 +5982,11 @@ async fn test_archive_thread_active_entry_management(cx: &mut TestAppContext) { }); cx.run_until_parked(); - // Should fall back to a draft on the same workspace. + // Archiving the active thread clears active_entry (no draft is created). sidebar.read_with(cx, |sidebar, _| { assert!( - matches!(&sidebar.active_entry, Some(ActiveEntry { workspace: ws, .. }) if ws == &workspace_b), - "expected Draft(workspace_b) after archiving active thread, got: {:?}", + sidebar.active_entry.is_none(), + "expected None after archiving active thread, got: {:?}", sidebar.active_entry, ); }); @@ -6340,9 +6299,9 @@ async fn test_unarchive_into_new_workspace_does_not_create_duplicate_real_thread #[gpui::test] async fn test_unarchive_into_existing_workspace_replaces_draft(cx: &mut TestAppContext) { - // When a workspace already exists with an empty draft (from - // reconcile_groups) and a thread is unarchived into it, the draft - // should be replaced — not kept alongside the loaded thread. + // When a workspace already exists with an empty draft and a thread + // is unarchived into it, the draft should be replaced — not kept + // alongside the loaded thread. agent_ui::test_support::init_test(cx); cx.update(|cx| { ThreadStore::init_global(cx); @@ -6372,19 +6331,12 @@ async fn test_unarchive_into_existing_workspace_replaces_draft(cx: &mut TestAppC let session_id = agent_ui::test_support::active_session_id(&panel, cx); cx.run_until_parked(); - // Archive the thread — this creates a draft to replace it. + // Archive the thread — the group is left empty (no draft created). sidebar.update_in(cx, |sidebar, window, cx| { sidebar.archive_thread(&session_id, window, cx); }); cx.run_until_parked(); - // Verify the draft exists before unarchive. - let entries = visible_entries_as_strings(&sidebar, cx); - assert!( - entries.iter().any(|e| e.contains("Draft")), - "expected a draft after archiving, got: {entries:?}" - ); - // Un-archive the thread. let thread_id = cx.update(|_, cx| { ThreadMetadataStore::global(cx) @@ -6416,88 +6368,6 @@ async fn test_unarchive_into_existing_workspace_replaces_draft(cx: &mut TestAppC ); } -#[gpui::test] -async fn test_pending_thread_activation_suppresses_reconcile_draft_creation( - cx: &mut TestAppContext, -) { - agent_ui::test_support::init_test(cx); - cx.update(|cx| { - cx.set_global(agent_ui::MaxIdleRetainedThreads(1)); - ThreadStore::init_global(cx); - ThreadMetadataStore::init_global(cx); - language_model::LanguageModelRegistry::test(cx); - prompt_store::init(cx); - }); - - let fs = FakeFs::new(cx.executor()); - fs.insert_tree("/project-a", serde_json::json!({ "src": {} })) - .await; - fs.insert_tree("/project-b", serde_json::json!({ "src": {} })) - .await; - cx.update(|cx| ::set_global(fs.clone(), cx)); - - let project_a = project::Project::test(fs.clone(), ["/project-a".as_ref()], cx).await; - let project_b = project::Project::test(fs.clone(), ["/project-b".as_ref()], cx).await; - - let (multi_workspace, cx) = - cx.add_window_view(|window, cx| MultiWorkspace::test_new(project_a.clone(), window, cx)); - let sidebar = setup_sidebar(&multi_workspace, cx); - - let workspace_b = multi_workspace.update_in(cx, |mw, window, cx| { - mw.test_add_workspace(project_b.clone(), window, cx) - }); - let panel_b = add_agent_panel(&workspace_b, cx); - cx.run_until_parked(); - - let preexisting_empty_draft_ids = panel_b.read_with(cx, |panel, cx| { - panel - .draft_thread_ids(cx) - .into_iter() - .filter(|id| panel.editor_text(*id, cx).is_none()) - .collect::>() - }); - if !preexisting_empty_draft_ids.is_empty() { - panel_b.update(cx, |panel, cx| { - for draft_id in &preexisting_empty_draft_ids { - panel.remove_thread(*draft_id, cx); - } - }); - cx.run_until_parked(); - } - - let project_b_key = project_b.read_with(cx, |project, cx| project.project_group_key(cx)); - - sidebar.update_in(cx, |sidebar, window, cx| { - assert!( - panel_b.read(cx).draft_thread_ids(cx).is_empty(), - "expected target panel to start without drafts after clearing setup state" - ); - - sidebar.pending_thread_activation = Some(ThreadId::new()); - sidebar.reconcile_groups(window, cx); - - assert!( - panel_b.read(cx).draft_thread_ids(cx).is_empty(), - "expected pending_thread_activation to suppress reconcile-driven fallback draft creation" - ); - - sidebar.pending_thread_activation = None; - sidebar.update_entries(cx); - sidebar.reconcile_groups(window, cx); - - let created_draft_ids = panel_b.read(cx).draft_thread_ids(cx); - assert_eq!( - created_draft_ids.len(), - 1, - "expected reconcile_groups to create a fallback draft again once the activation guard is cleared for the empty group {project_b_key:?}" - ); - assert!( - panel_b.read(cx).editor_text(created_draft_ids[0], cx).is_none(), - "expected the reconciled draft to be empty" - ); - }); -} - #[gpui::test] async fn test_unarchive_into_inactive_existing_workspace_does_not_leave_active_draft( cx: &mut TestAppContext, @@ -6876,10 +6746,12 @@ async fn test_unarchive_does_not_create_duplicate_real_thread_metadata(cx: &mut } #[gpui::test] -async fn test_switch_to_workspace_with_archived_thread_shows_draft(cx: &mut TestAppContext) { +async fn test_switch_to_workspace_with_archived_thread_shows_no_active_entry( + cx: &mut TestAppContext, +) { // When a thread is archived while the user is in a different workspace, - // the archiving code replaces the thread with a tracked draft in its - // panel. Switching back to that workspace should show the draft. + // the group is left empty (no draft is created). Switching back to that + // workspace should show no active entry. agent_ui::test_support::init_test(cx); cx.update(|cx| { ThreadStore::init_global(cx); @@ -6925,7 +6797,7 @@ async fn test_switch_to_workspace_with_archived_thread_shows_draft(cx: &mut Test cx.run_until_parked(); // Switch back to project-a. Its panel was cleared during archiving, - // so active_entry should be Draft. + // so active_entry should be None (no draft is created). let workspace_a = multi_workspace.read_with(cx, |mw, _| mw.workspaces().next().unwrap().clone()); multi_workspace.update_in(cx, |mw, window, cx| { @@ -6940,8 +6812,8 @@ async fn test_switch_to_workspace_with_archived_thread_shows_draft(cx: &mut Test sidebar.read_with(cx, |sidebar, _| { assert!( - matches!(&sidebar.active_entry, Some(ActiveEntry { workspace: ws, .. }) if ws == &workspace_a), - "expected Draft(workspace_a) after switching to workspace with archived thread, got: {:?}", + sidebar.active_entry.is_none(), + "expected no active entry after switching to workspace with archived thread, got: {:?}", sidebar.active_entry, ); }); @@ -7189,12 +7061,12 @@ async fn test_archive_last_thread_on_linked_worktree_does_not_create_new_thread_ } #[gpui::test] -async fn test_archive_last_thread_on_linked_worktree_with_no_siblings_creates_draft_on_main( +async fn test_archive_last_thread_on_linked_worktree_with_no_siblings_leaves_group_empty( cx: &mut TestAppContext, ) { // When a linked worktree thread is the ONLY thread in the project group - // (no threads on the main repo either), archiving it should create a - // draft on the main workspace, not the linked worktree workspace. + // (no threads on the main repo either), archiving it should leave the + // group empty with no active entry. agent_ui::test_support::init_test(cx); cx.update(|cx| { ThreadStore::init_global(cx); @@ -7299,12 +7171,12 @@ async fn test_archive_last_thread_on_linked_worktree_with_no_siblings_creates_dr "no entry should reference the archived worktree, got: {entries_after:?}" ); - // The active entry should be a draft on the main workspace. + // The active entry should be None — no draft is created. sidebar.read_with(cx, |s, _| { - assert_active_draft( - s, - &main_workspace, - "active entry should be a draft on the main workspace", + assert!( + s.active_entry.is_none(), + "expected no active entry after archiving the last thread, got: {:?}", + s.active_entry, ); }); } @@ -7378,12 +7250,6 @@ async fn test_unarchive_linked_worktree_thread_into_project_group_shows_only_res let _main_panel = add_agent_panel(&main_workspace, cx); cx.run_until_parked(); - let entries_before = visible_entries_as_strings(&sidebar, cx); - assert!( - entries_before.iter().any(|entry| entry.contains("Draft")), - "expected main workspace to start with a fallback draft, got entries: {entries_before:?}" - ); - let session_id = acp::SessionId::new(Arc::from("linked-worktree-unarchive")); let original_thread_id = ThreadId::new(); let main_paths = PathList::new(&[PathBuf::from("/project")]); @@ -8317,9 +8183,9 @@ async fn test_linked_worktree_workspace_reachable_after_adding_unrelated_project } #[gpui::test] -async fn test_startup_failed_restoration_shows_draft(cx: &mut TestAppContext) { - // Rule 4: When the app starts and the AgentPanel fails to restore its - // last thread (no metadata), a draft should appear in the sidebar. +async fn test_startup_failed_restoration_shows_no_draft(cx: &mut TestAppContext) { + // Empty project groups no longer auto-create drafts via reconciliation. + // A fresh startup with no restorable thread should show only the header. 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)); @@ -8328,10 +8194,10 @@ async fn test_startup_failed_restoration_shows_draft(cx: &mut TestAppContext) { let _workspace = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone()); let entries = visible_entries_as_strings(&sidebar, cx); - assert_eq!(entries.len(), 2, "should have header + draft: {entries:?}"); - assert!( - entries[1].contains("Draft"), - "second entry should be a draft: {entries:?}" + assert_eq!( + entries, + vec!["v [my-project]"], + "empty group should show only the header, no auto-created draft" ); } @@ -8365,49 +8231,6 @@ async fn test_startup_successful_restoration_no_spurious_draft(cx: &mut TestAppC }); } -#[gpui::test] -async fn test_delete_last_draft_in_empty_group_shows_placeholder(cx: &mut TestAppContext) { - // Deleting the last draft in a threadless group should - // leave a placeholder draft entry (not an empty group). - 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, cx); - - // Reconciliation creates a draft for the empty group. - let workspace = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone()); - let entries = visible_entries_as_strings(&sidebar, cx); - let draft_count = entries.iter().filter(|e| e.contains("Draft")).count(); - assert_eq!( - draft_count, 1, - "should start with 1 draft from reconciliation" - ); - - // Find and delete the draft. - let draft_thread_id = sidebar.read_with(cx, |_sidebar, cx| { - let panel = workspace.read(cx).panel::(cx).unwrap(); - panel - .read(cx) - .draft_thread_ids(cx) - .into_iter() - .next() - .unwrap() - }); - sidebar.update_in(cx, |sidebar, window, cx| { - sidebar.remove_draft(draft_thread_id, &workspace, window, cx); - }); - cx.run_until_parked(); - - // The group has no threads and no tracked drafts, so a - // placeholder draft should appear via reconciliation. - let entries = visible_entries_as_strings(&sidebar, cx); - let draft_count = entries.iter().filter(|e| e.contains("Draft")).count(); - assert_eq!( - draft_count, 1, - "placeholder draft should appear after deleting all tracked drafts" - ); -} - #[gpui::test] async fn test_project_header_click_restores_last_viewed(cx: &mut TestAppContext) { // Rule 9: Clicking a project header should restore whatever the @@ -8521,10 +8344,10 @@ async fn test_plus_button_reuses_empty_draft(cx: &mut TestAppContext) { cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); let (sidebar, _panel) = setup_sidebar_with_agent_panel(&multi_workspace, cx); - // Start: panel has 1 draft from set_active. + // Start: no drafts from reconciliation. let entries = visible_entries_as_strings(&sidebar, cx); let draft_count = entries.iter().filter(|e| e.contains("Draft")).count(); - assert_eq!(draft_count, 1, "should start with 1 draft"); + assert_eq!(draft_count, 0, "should start with 0 drafts"); let workspace = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone()); let simulate_plus_button = @@ -8532,7 +8355,17 @@ async fn test_plus_button_reuses_empty_draft(cx: &mut TestAppContext) { sidebar.create_new_thread(&workspace, window, cx); }; - // + click with empty draft: should reuse it, not create a new one. + // First + click: should create a draft. + sidebar.update_in(cx, |sidebar, window, cx| { + simulate_plus_button(sidebar, window, cx); + }); + cx.run_until_parked(); + + let entries = visible_entries_as_strings(&sidebar, cx); + let draft_count = entries.iter().filter(|e| e.contains("Draft")).count(); + assert_eq!(draft_count, 1, "first + click should create a draft"); + + // Second + click with empty draft: should reuse it, not create a new one. sidebar.update_in(cx, |sidebar, window, cx| { simulate_plus_button(sidebar, window, cx); }); @@ -8542,7 +8375,7 @@ async fn test_plus_button_reuses_empty_draft(cx: &mut TestAppContext) { let draft_count = entries.iter().filter(|e| e.contains("Draft")).count(); assert_eq!( draft_count, 1, - "+ click should reuse the existing empty draft, not create a new one" + "second + click should reuse the existing empty draft, not create a new one" ); // The draft should be active. diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 5e1e680545aa98818bbda6bfa27f05637e75f131..7aa9b45c52d82df4dcef8d363d3f8e39deaf3ed7 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -216,16 +216,6 @@ pub trait DebuggerProvider { fn active_thread_state(&self, cx: &App) -> Option; } -pub trait WorkspaceSidebarDelegate: Send + Sync { - fn reconcile_group( - &self, - workspace: &mut Workspace, - group_key: &ProjectGroupKey, - window: &mut Window, - cx: &mut Context, - ) -> bool; -} - /// Opens a file or directory. #[derive(Clone, PartialEq, Deserialize, JsonSchema, Action)] #[action(namespace = workspace)] @@ -1382,7 +1372,6 @@ pub struct Workspace { _panels_task: Option>>, sidebar_focus_handle: Option, multi_workspace: Option>, - sidebar_delegate: Option>, } impl EventEmitter for Workspace {} @@ -1811,7 +1800,6 @@ impl Workspace { removing: false, sidebar_focus_handle: None, multi_workspace, - sidebar_delegate: None, open_in_dev_container: false, _dev_container_task: None, } @@ -2472,14 +2460,6 @@ impl Workspace { self.multi_workspace = Some(multi_workspace); } - pub fn set_sidebar_delegate(&mut self, delegate: Arc) { - self.sidebar_delegate = Some(delegate); - } - - pub fn sidebar_delegate(&self) -> Option> { - self.sidebar_delegate.clone() - } - pub fn app_state(&self) -> &Arc { &self.app_state }