yenru0/zed
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Simplify sidebar active state (#52799)

Browse Source 
This changes the terminology from "focused entry" to "active entry", and
adds 4 properties that rebuild_contents should maintain for the active
entry:

- We should always have an active_entry after rebuild_contents
- The active entry's workspace should always be == to the active
multiworkspace
- If there's a thread, the active entry should always reflect that
thread
- There should always be exactly 1 active entry

Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Closes #ISSUE

Release Notes:

- N/A
This commit is contained in:
Mikayla Maki authored and GitHub committed 2026-03-31 04:53:34 +00:00
1 parent c61ad775ce
commit 556dbd7352
2 files changed
+378 -204
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

crates/sidebar/src/sidebar.rs
+149 -83
View File
@@ -108,6 +108,44 @@ enum SidebarView {
Archive(Entity<ThreadsArchiveView>),
}
#[derive(Clone, Debug)]
enum ActiveEntry {
Thread {
session_id: acp::SessionId,
workspace: Entity<Workspace>,
},
Draft(Entity<Workspace>),
}
impl ActiveEntry {
fn workspace(&self) -> &Entity<Workspace> {
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<Workspace>,
is_active_draft: bool,
worktrees: Vec<WorktreeInfo>,
},
}
@@ -322,11 +359,8 @@ pub struct Sidebar {
///
/// Note: This is NOT the same as the active item.
selection: Option<usize>,
/// 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<acp::SessionId>,
/// Tracks which sidebar entry is currently active (highlighted).
active_entry: Option<ActiveEntry>,
hovered_thread_index: Option<usize>,
collapsed_groups: HashSet<PathList>,
expanded_groups: HashMap<PathList, usize>,
@@ -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,29 +477,16 @@ impl Sidebar {
cx.emit(workspace::SidebarEvent::SerializeNeeded);
}
fn active_entry_workspace(&self) -> Option<&Entity<Workspace>> {
self.active_entry.as_ref().map(|entry| entry.workspace())
}
fn is_active_workspace(&self, workspace: &Entity<Workspace>, cx: &App) -> bool {
self.multi_workspace
.upgrade()
.map_or(false, |mw| mw.read(cx).workspace() == 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 {
self.multi_workspace
.upgrade()
.and_then(|mw| {
let workspace = mw.read(cx).workspace();
workspace.read(cx).panel::<AgentPanel>(cx)
})
.map_or(false, |panel| panel.read(cx).active_thread_is_draft(cx))
}
fn subscribe_to_workspace(
&mut self,
workspace: &Entity<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::<AgentPanel>(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::<AgentPanel>(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<Self>,
) {
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::<Self>().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::<AgentPanel>(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<Self>,
) -> 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<Workspace>,
is_active_draft: bool,
is_active: bool,
worktrees: &[WorktreeInfo],
is_selected: bool,
cx: &mut Context<Self>,
) -> 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())
crates/sidebar/src/sidebar_tests.rs
+229 -121
View File
@@ -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<Workspace>, 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<MultiWorkspace>,
sidebar: &Entity<Sidebar>,
_sidebar: &Entity<Sidebar>,
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::<AgentPanel>(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::<AgentPanel>(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::<AgentPanel>(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();
let active_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::<AgentPanel>(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,
);
}
// 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::<AgentPanel>(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(())
}