From 99dddba6b85b9a356d47d54bb1c38f6cc4e7dcd4 Mon Sep 17 00:00:00 2001 From: restlessronin <88921269+restlessronin@users.noreply.github.com> Date: Fri, 18 Sep 2026 05:46:01 +0000 Subject: [PATCH] agent_ui: Stop the sidebar creating two terminals for one new entry (#62151) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Objective Starting a new terminal thread from the agent sidebar creates **two** terminals instead of one, in a project that has no terminals yet. With terminals already open, only one is created. ## Solution `Sidebar::create_new_terminal` calls `AgentPanel::new_terminal` and then focuses the panel. The spawn is async, so when `focus_panel` activates the panel, `base_view` is still `Uninitialized` — and `ensure_thread_initialized` reads that as an empty panel and spawns its own "initial" terminal on top of the one already in flight. Once terminals exist they are restored at panel load, so `base_view` is already `Terminal` and the second spawn never happens. That's why the bug only appears on a project with no terminals. `restore_terminal` already guarded against this by marking the spawn pending before calling `spawn_terminal`. This moves that marker into `spawn_terminal` itself, so every spawn path arms it, and drops the now-redundant assignment in `restore_terminal`. The marker is cleared exactly where it already was: when the terminal lands, when the spawn fails, and when the terminal is closed. ## Testing - Added `test_new_terminal_prevents_initial_terminal_creation`, which reproduces the sidebar's spawn-then-focus sequence. It fails without the fix (the panel ends up with two terminals) and passes with it. - `cargo test -p agent_ui --lib` — 405 passed. - `cargo test -p sidebar --lib` — 142 passed. - `./script/clippy -p agent_ui` — clean. - Tested on macOS only. The race is platform-independent, but I can't exercise the Linux/Windows terminal spawn paths. ## 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 adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [ X] Tests cover the new/changed behavior - [ X] Performance impact has been considered and is acceptable --- Release Notes: - Fixed the agent panel creating two terminals when starting a new terminal thread in a project with no existing terminals --------- Co-authored-by: Claude Opus 5 --- crates/agent_ui/src/agent_panel.rs | 118 +++++++++++++++++++++++++---- 1 file changed, 103 insertions(+), 15 deletions(-) diff --git a/crates/agent_ui/src/agent_panel.rs b/crates/agent_ui/src/agent_panel.rs index e9ebe0749e..4efc64b652 100644 --- a/crates/agent_ui/src/agent_panel.rs +++ b/crates/agent_ui/src/agent_panel.rs @@ -1169,6 +1169,8 @@ pub struct AgentPanel { retained_threads: HashMap>, terminals: HashMap, pending_terminal_spawn: Option, + #[cfg(test)] + test_terminal_spawn_gate: Option>, new_thread_menu_handle: PopoverMenuHandle, agent_panel_menu_handle: PopoverMenuHandle, _extension_subscription: Option, @@ -1582,6 +1584,8 @@ impl AgentPanel { retained_threads: HashMap::default(), terminals: HashMap::default(), pending_terminal_spawn: None, + #[cfg(test)] + test_terminal_spawn_gate: None, new_thread_menu_handle: PopoverMenuHandle::default(), agent_panel_menu_handle: PopoverMenuHandle::default(), @@ -2073,11 +2077,14 @@ impl AgentPanel { window: &mut Window, cx: &mut Context, ) { + // The spawn is async, so the panel keeps its current base view until + // the terminal lands. Marking the spawn as pending stops + // `ensure_thread_initialized` from treating a still-uninitialized panel + // as empty and spawning a second, "initial" terminal on activation. + self.pending_terminal_spawn = Some(terminal_id); let terminal_working_directory = working_directory.clone(); let init_command = Self::terminal_init_command(run_init_command, cx); - let terminal_task = self.project.update(cx, |project, cx| { - project.create_terminal_shell(working_directory, cx) - }); + let terminal_task = self.create_terminal_shell(working_directory, cx); let workspace = self.workspace.clone(); let workspace_id = self.workspace_id; let project = self.project.downgrade(); @@ -2128,6 +2135,27 @@ impl AgentPanel { .detach_and_log_err(cx); } + fn create_terminal_shell( + &mut self, + working_directory: Option, + cx: &mut Context, + ) -> Task>> { + // A real shell ties the spawn's timing to the host, so a test that needs + // to observe the panel mid-spawn installs a gate and gets a display-only + // terminal when it releases it. + #[cfg(test)] + if let Some(gate) = self.test_terminal_spawn_gate.take() { + return cx.spawn(async move |this, cx| { + gate.await.ok(); + this.update(cx, |this, cx| this.build_display_only_terminal(cx)) + }); + } + + self.project.update(cx, |project, cx| { + project.create_terminal_shell(working_directory, cx) + }) + } + fn terminal_init_command(run_init_command: bool, cx: &App) -> Option { run_init_command .then(|| AgentSettings::get_global(cx).terminal_init_command.clone()) @@ -2452,7 +2480,6 @@ impl AgentPanel { return; } - self.pending_terminal_spawn = Some(metadata.terminal_id); let working_directory = self.terminal_restore_working_directory(&metadata, workspace, cx); let initial_title = Self::terminal_restore_initial_title(&metadata); self.spawn_terminal( @@ -6797,6 +6824,21 @@ impl AgentPanel { ) } + #[cfg(any(test, feature = "test-support"))] + fn build_display_only_terminal(&self, cx: &mut Context) -> Entity { + let settings = TerminalSettings::get_global(cx).clone(); + let path_style = self.project.read(cx).path_style(cx); + let builder = terminal::TerminalBuilder::new_display_only( + settings.cursor_shape, + settings.alternate_scroll, + settings.max_scroll_history_lines, + cx.entity_id().as_u64(), + cx.background_executor(), + path_style, + ); + cx.new(|cx| builder.subscribe(cx)) + } + #[cfg(any(test, feature = "test-support"))] fn insert_display_only_terminal( &mut self, @@ -6813,17 +6855,7 @@ impl AgentPanel { cx: &mut Context, ) -> Result<()> { let init_command = Self::terminal_init_command(run_init_command, cx); - let settings = TerminalSettings::get_global(cx).clone(); - let path_style = self.project.read(cx).path_style(cx); - let builder = terminal::TerminalBuilder::new_display_only( - settings.cursor_shape, - settings.alternate_scroll, - settings.max_scroll_history_lines, - cx.entity_id().as_u64(), - cx.background_executor(), - path_style, - ); - let terminal = cx.new(|cx| builder.subscribe(cx)); + let terminal = self.build_display_only_terminal(cx); let terminal_for_init_command = terminal.clone(); let terminal_view = cx.new(|cx| { let mut view = TerminalView::new( @@ -7674,6 +7706,62 @@ mod tests { }); } + #[gpui::test] + async fn test_new_terminal_prevents_initial_terminal_creation(cx: &mut TestAppContext) { + let (panel, mut cx) = setup_panel(cx).await; + + let (terminal_landed, spawn_gate) = futures::channel::oneshot::channel::<()>(); + + // `create_new_terminal` in the sidebar calls `new_terminal` and then + // focuses the panel, which activates it while the spawn is still in + // flight. The gate keeps the spawn in flight for as long as the test + // wants, instead of racing a real shell. + panel.update_in(&mut cx, |panel, window, cx| { + panel.test_terminal_spawn_gate = Some(spawn_gate); + panel.new_terminal(None, AgentThreadSource::AgentPanel, window, cx); + assert!( + panel.pending_terminal_spawn.is_some(), + "a new terminal spawn should be marked pending until it lands" + ); + panel.set_active(true, window, cx); + }); + cx.run_until_parked(); + + panel.read_with(&cx, |panel, cx| { + assert!( + panel.terminals(cx).is_empty(), + "no terminal should land while the spawn is held" + ); + assert!( + panel.pending_terminal_spawn.is_some(), + "the in-flight spawn should still be pending after activation" + ); + }); + + terminal_landed + .send(()) + .expect("the panel should still be waiting on the terminal spawn"); + cx.run_until_parked(); + + panel.read_with(&cx, |panel, cx| { + let terminals = panel.terminals(cx); + assert_eq!( + terminals.len(), + 1, + "activation while a new terminal is spawning should not create a second terminal" + ); + assert_eq!( + panel.active_terminal_id(), + terminals.first().map(|terminal| terminal.id), + "the terminal that was spawned should be the active one" + ); + assert!( + panel.pending_terminal_spawn.is_none(), + "the pending marker should be cleared once the terminal lands" + ); + }); + } + #[gpui::test] async fn test_restored_terminal_runs_init_command_once(cx: &mut TestAppContext) { let (panel, mut cx) = setup_panel(cx).await;