diff --git a/app/src/pane_group/mod.rs b/app/src/pane_group/mod.rs index 1d036d4292c..b92f3d7eb4e 100644 --- a/app/src/pane_group/mod.rs +++ b/app/src/pane_group/mod.rs @@ -995,6 +995,24 @@ pub struct PaneGroup { /// Tab-level custom title set via the rename-tab flow. custom_title: Option, + + /// Terminal panes whose persisted scrollback/AI-conversation restoration + /// was deferred at startup (see + /// `FeatureFlag::LazyBackgroundTabScrollbackRestore`), keyed by the + /// pane's [`PaneId`]. Kept around so a pane that's closed or persisted + /// before ever being activated round-trips its original snapshot + /// unchanged, rather than a partially-restored one. + pending_lazy_terminal_restorations: HashMap, +} + +/// Deferred restoration payload for a single terminal pane. See +/// [`PaneGroup::pending_lazy_terminal_restorations`]. +pub(crate) struct PendingLazyTerminalRestoration { + /// The original, unmodified snapshot this pane was restored from. Used + /// to round-trip persistence for a pane that's never activated. + original_snapshot: TerminalPaneSnapshot, + restored_blocks: Option>, + conversation_restoration: Option, } /// A cloud orchestration parent whose direct children (per the server's @@ -1554,6 +1572,8 @@ impl PaneGroup { model_event_sender: Option>, deferred_panes: &mut Vec<(PaneId, LeafSnapshot)>, pending_ambient_restorations: &mut Vec<(AmbientAgentTaskId, PaneId)>, + is_active_tab: bool, + pending_lazy_terminal_restorations: &mut HashMap, ) -> anyhow::Result<(PaneData, InitialFocus)> { match root { PaneNodeSnapshot::Leaf(leaf) => Self::restore_pane_leaf( @@ -1567,6 +1587,8 @@ impl PaneGroup { model_event_sender, deferred_panes, pending_ambient_restorations, + is_active_tab, + pending_lazy_terminal_restorations, ), PaneNodeSnapshot::Branch(pane) => { let mut len = 0; @@ -1598,6 +1620,8 @@ impl PaneGroup { model_event_sender.clone(), deferred_panes, pending_ambient_restorations, + is_active_tab, + pending_lazy_terminal_restorations, ) { Ok((child, child_focus)) => { len += child.len(); @@ -1621,7 +1645,11 @@ impl PaneGroup { } } - /// Restores a single leaf pane from a snapshot. + /// Restores a single leaf pane from a snapshot. Non-active-tab terminal + /// leaves keep an eager shell (so a restored window doesn't change when + /// its tabs' processes start) but skip laying out their restored + /// scrollback/AI conversation text, since that's the expensive part and + /// isn't needed until the tab is actually shown. #[allow(clippy::too_many_arguments)] fn restore_pane_leaf( leaf: LeafSnapshot, @@ -1635,6 +1663,8 @@ impl PaneGroup { #[cfg_attr(not(feature = "local_fs"), allow(unused_variables, clippy::ptr_arg))] deferred_panes: &mut Vec<(PaneId, LeafSnapshot)>, pending_ambient_restorations: &mut Vec<(AmbientAgentTaskId, PaneId)>, + is_active_tab: bool, + pending_lazy_terminal_restorations: &mut HashMap, ) -> anyhow::Result<(PaneData, InitialFocus)> { let custom_vertical_tabs_title = leaf.custom_vertical_tabs_title.clone(); let result = match leaf.contents { @@ -1676,6 +1706,7 @@ impl PaneGroup { let startup_directory = terminal_snapshot .cwd + .clone() .map(PathBuf::from) .filter(|path| path.is_dir()); @@ -1718,14 +1749,52 @@ impl PaneGroup { }, ) }; + + // Background tabs (not the initially-active tab) still spawn their shell + // eagerly, but skip the comparatively expensive work of laying out restored + // scrollback/AI conversation text until the tab is actually activated. The + // original snapshot is retained so persistence round-trips even if the tab is + // closed without ever being activated. See `PendingLazyTerminalRestoration`. + let defer_scrollback_restoration = !is_active_tab + && FeatureFlag::LazyBackgroundTabScrollbackRestore.is_enabled() + && (block_list.is_some() || conversation_restoration.is_some()); + + let (block_list_for_session, deferred_restored_blocks) = + if defer_scrollback_restoration { + (None, block_list.cloned()) + } else { + (block_list, None) + }; + let (conversation_restoration_for_session, deferred_conversation_restoration) = + if defer_scrollback_restoration { + (None, conversation_restoration) + } else { + (conversation_restoration, None) + }; + let deferred_original_snapshot = + defer_scrollback_restoration.then(|| terminal_snapshot.clone()); + // Give a still-pending pane a usable tab title/subtitle before its + // blocks are ever fed into the live model, sourced from the same + // deferred data `materialize_lazy_tab_restorations` will apply later. + let pending_title_hint = deferred_restored_blocks.as_ref().and_then(|items| { + items.iter().rev().find_map(|item| match item { + SerializedBlockListItem::Command { block } + if block.start_ts.is_some() && block.completed_ts.is_some() => + { + block.plain_text_command_preview() + } + _ => None, + }) + }); + let (terminal_view, terminal_manager) = PaneGroup::create_session( startup_directory, HashMap::new(), uuid.0.as_slice(), IsSharedSessionCreator::No, resources, - block_list, - conversation_restoration, + block_list_for_session, + conversation_restoration_for_session, user_default_shell_unsupported_banner_model_handle, view_size, model_event_sender.clone(), @@ -1734,6 +1803,12 @@ impl PaneGroup { ctx, ); + if let Some(hint) = pending_title_hint { + terminal_view.update(ctx, |view, _| { + view.set_pending_restoration_title_hint(Some(hint)); + }); + } + let terminal_view_id = terminal_view.id(); let pane_data = TerminalPane::new( @@ -1748,6 +1823,17 @@ impl PaneGroup { let pane_id = terminal_pane_id.into(); pane_contents.insert(pane_id, Box::new(pane_data)); + if let Some(original_snapshot) = deferred_original_snapshot { + pending_lazy_terminal_restorations.insert( + pane_id, + PendingLazyTerminalRestoration { + original_snapshot, + restored_blocks: deferred_restored_blocks, + conversation_restoration: deferred_conversation_restoration, + }, + ); + } + if let Some(llm_override) = &terminal_snapshot.llm_model_override && let Ok(llm_id) = serde_json::from_str::(llm_override) { @@ -2185,25 +2271,39 @@ impl PaneGroup { // should inherit the active-session marker. let visible_leaf_is_active_session = pane_id.as_terminal_pane_id() == self.active_session_id(app); - let mut contents = match self.pane_contents.get(&snapshot_pane_id) { - Some(pane) => pane.as_pane().snapshot(app), - None => { - // Create a new pane uuid if we have a bug where we didn't save it - // properly. This approach will allow us to keep the uniqueness constraints - // intact so we don't fail to save the snapshot. - report_error!("Failed to get session data for pane, so used a new uuid"); - LeafContents::Terminal(TerminalPaneSnapshot { - uuid: Uuid::new_v4().as_bytes().to_vec(), - cwd: None, - is_active: visible_leaf_is_active_session, - is_read_only: false, - shell_launch_data: None, - input_config: Some(InputConfig::new(app)), - llm_model_override: None, - active_profile_id: None, - conversation_ids_to_restore: Vec::new(), - active_conversation_id: None, - }) + let mut contents = if let Some(pending) = self + .pending_lazy_terminal_restorations + .get(&snapshot_pane_id) + { + // This pane's scrollback/AI-conversation restoration was deferred + // (see `FeatureFlag::LazyBackgroundTabScrollbackRestore`) and never + // materialized. Round-trip the exact snapshot it was restored from + // rather than the live (partially-restored) pane state, so quitting + // without ever activating this tab doesn't lose its history. + LeafContents::Terminal(pending.original_snapshot.clone()) + } else { + match self.pane_contents.get(&snapshot_pane_id) { + Some(pane) => pane.as_pane().snapshot(app), + None => { + // Create a new pane uuid if we have a bug where we didn't save it + // properly. This approach will allow us to keep the uniqueness constraints + // intact so we don't fail to save the snapshot. + report_error!( + "Failed to get session data for pane, so used a new uuid" + ); + LeafContents::Terminal(TerminalPaneSnapshot { + uuid: Uuid::new_v4().as_bytes().to_vec(), + cwd: None, + is_active: visible_leaf_is_active_session, + is_read_only: false, + shell_launch_data: None, + input_config: Some(InputConfig::new(app)), + llm_model_override: None, + active_profile_id: None, + conversation_ids_to_restore: Vec::new(), + active_conversation_id: None, + }) + } } }; @@ -3223,6 +3323,7 @@ impl PaneGroup { transitively_shared_child_panes: HashMap::new(), child_agent_origin: None, custom_title: None, + pending_lazy_terminal_restorations: HashMap::new(), }; // Notify any restored panes that they belong to this pane group. @@ -3441,6 +3542,12 @@ impl PaneGroup { /// Constructs a new [`PaneGroup`] with a layout that adheres /// to the specification of the provided [`PanesLayout`]. + /// + /// `is_active_tab` should be `false` when constructing a tab other than + /// the window's initially-active tab during session restoration, so a + /// `PanesLayout::Snapshot` can skip its most expensive restoration work + /// for tabs the user won't see at launch. It has no effect on other + /// layout kinds. #[allow(clippy::too_many_arguments)] pub fn new_with_panes_layout( tips_completed: ModelHandle, @@ -3449,16 +3556,19 @@ impl PaneGroup { panes_layout: PanesLayout, block_lists: Arc>>, model_event_sender: Option>, + is_active_tab: bool, ctx: &mut ViewContext, ) -> Self { let unsupported_banner_model_handle = user_default_shell_unsupported_banner_model_handle.clone(); let model_event_sender_clone = model_event_sender.clone(); - // Shared container so pending ambient restorations collected inside the + // Shared containers so pending restorations collected inside the // layout closure can be accessed after `new_internal` returns. let pending_ambient = Rc::new(RefCell::new(Vec::new())); let pending_ambient_for_closure = pending_ambient.clone(); + let pending_lazy_terminal = Rc::new(RefCell::new(HashMap::new())); + let pending_lazy_terminal_for_closure = pending_lazy_terminal.clone(); let initial_layout = move |resources, pane_contents: &mut HashMap>, @@ -3479,6 +3589,7 @@ impl PaneGroup { PanesLayout::Snapshot(panes_snapshot) => { let mut deferred_panes = Vec::new(); let mut pending_restorations = Vec::new(); + let mut pending_lazy_terminal_restorations = HashMap::new(); let result = Self::restore_pane_tree( *panes_snapshot, block_lists, @@ -3490,6 +3601,8 @@ impl PaneGroup { model_event_sender_clone.clone(), &mut deferred_panes, &mut pending_restorations, + is_active_tab, + &mut pending_lazy_terminal_restorations, ) .unwrap_or_else(|err| { log::warn!("Error restoring pane tree: {err:#}"); @@ -3506,6 +3619,8 @@ impl PaneGroup { }); *pending_ambient_for_closure.borrow_mut() = pending_restorations; + *pending_lazy_terminal_for_closure.borrow_mut() = + pending_lazy_terminal_restorations; Self::process_deferred_panes(deferred_panes, result, pane_contents, ctx) } @@ -3546,9 +3661,72 @@ impl PaneGroup { pane_group.register_pending_ambient_restorations(pending, ctx); } + pane_group.pending_lazy_terminal_restorations = pending_lazy_terminal.take(); + pane_group } + /// Applies any background-tab scrollback/AI conversation restoration + /// that was deferred at startup (see + /// `pending_lazy_terminal_restorations`). Called the first time this + /// pane group's tab is activated; a no-op once nothing is pending. + pub fn materialize_lazy_tab_restorations(&mut self, ctx: &mut ViewContext) { + if self.pending_lazy_terminal_restorations.is_empty() { + return; + } + + let pending = std::mem::take(&mut self.pending_lazy_terminal_restorations); + for (pane_id, restoration) in pending { + // The pane may have been closed (or replaced, e.g. by a child-agent + // swap) before ever being activated; nothing further to do. + let Some(terminal_view) = self.terminal_view_from_pane_id(pane_id, ctx) else { + continue; + }; + + let has_conversation_restoration = restoration.conversation_restoration.is_some(); + + let has_restored_command_blocks = restoration + .restored_blocks + .as_ref() + .is_some_and(|blocks| !blocks.is_empty()) + && terminal_view + .as_ref(ctx) + .model + .lock() + .block_list_mut() + .apply_deferred_restored_blocks( + restoration.restored_blocks.as_deref().unwrap_or_default(), + ); + + if let Some(conversation_restoration) = restoration.conversation_restoration { + terminal_view.update(ctx, |view, ctx| { + view.restore_conversations_on_view_creation(conversation_restoration, ctx); + }); + } + + // Mirrors the separator inserted for a pane restored eagerly (see + // `create_terminal_view_surface`'s post-wire step). + #[cfg(feature = "local_fs")] + if has_restored_command_blocks || has_conversation_restoration { + terminal_view + .as_ref(ctx) + .model + .lock() + .block_list_mut() + .append_session_restoration_separator_to_block_list(false); + } + #[cfg(not(feature = "local_fs"))] + let _ = (has_restored_command_blocks, has_conversation_restoration); + } + + // Tab titles/subtitles are computed from this pane group's live state at + // render time (e.g. `TerminalView::last_completed_command_text`), but + // nothing above notifies the workspace to re-render with it — emit the + // same events every other state-changing method on this type emits. + ctx.emit(Event::TerminalViewStateChanged); + ctx.emit(Event::AppStateChanged); + } + pub fn new_from_existing_pane( pane: Box, tips_completed: ModelHandle, @@ -4423,6 +4601,30 @@ impl PaneGroup { pane_content } + /// Takes any pending lazy scrollback/AI-conversation restoration for + /// `pane_id`, if it was never materialized. A pane moved out of this + /// group via `remove_pane_for_move` must have its entry transferred with + /// this and `insert_pending_lazy_terminal_restoration`, rather than left + /// behind or dropped, so it's still applied on activation, or still + /// round-tripped if the pane is never activated, in its new home. + pub(crate) fn take_pending_lazy_terminal_restoration( + &mut self, + pane_id: PaneId, + ) -> Option { + self.pending_lazy_terminal_restorations.remove(&pane_id) + } + + /// Re-registers a pending lazy restoration for `pane_id` in this group. + /// See `take_pending_lazy_terminal_restoration`. + pub(crate) fn insert_pending_lazy_terminal_restoration( + &mut self, + pane_id: PaneId, + restoration: PendingLazyTerminalRestoration, + ) { + self.pending_lazy_terminal_restorations + .insert(pane_id, restoration); + } + pub fn notebook_pane_by_pane_id(&self, pane_id: Option) -> Option<&NotebookPane> { self.downcast_pane_by_id(pane_id?) } @@ -5590,6 +5792,9 @@ impl PaneGroup { // Drop any transitive-share tracking entry for this pane so the // map doesn't accumulate stale ids. self.forget_transitively_shared_pane(pane_id); + // A pane can be closed before its deferred restoration was ever + // materialized; drop its entry so the payload doesn't linger. + self.pending_lazy_terminal_restorations.remove(&pane_id); ctx.notify(); ctx.emit(Event::TerminalViewStateChanged); @@ -7762,11 +7967,16 @@ impl PaneGroup { // When user clicked on the close tab button, we should wind down the existing panes // by deleting all the saved blocks in each pane from the database. - pub fn clean_up_panes(&self, ctx: &mut ViewContext) { + pub fn clean_up_panes(&mut self, ctx: &mut ViewContext) { for pane in self.pane_contents.values() { let pane = pane.as_pane(); pane.detach(self, DetachType::Closed, ctx); } + // This is the real permanent-discard path for a closed tab (reached from + // `UndoCloseStack` once its grace period expires); `cleanup_closed_pane` + // covers per-pane undo instead. Any pane never activated before this + // point still holds its deferred restoration payload. + self.pending_lazy_terminal_restorations.clear(); } fn clean_up_pane(&self, pane_id: PaneId, ctx: &mut ViewContext) { diff --git a/app/src/pane_group/mod_tests.rs b/app/src/pane_group/mod_tests.rs index 5edf5dbfc8c..3e87a7dbda8 100644 --- a/app/src/pane_group/mod_tests.rs +++ b/app/src/pane_group/mod_tests.rs @@ -258,6 +258,7 @@ fn mock_pane_group(app: &mut App, options: MockOptions) -> ViewHandle options.layout, block_lists, None, + true, ctx, ) }); @@ -3806,6 +3807,7 @@ fn test_focused_pane_is_synchronized_with_application_focus() { panes_layout, block_lists, None, + true, ctx, ) }); @@ -3951,3 +3953,430 @@ fn test_undo_close_keeps_a_file_pane_watching_its_file() { }); }); } + +/// APP-5257: restoring a `PanesLayout::Snapshot` for a tab other than the window's +/// initially-active one, under `FeatureFlag::LazyBackgroundTabScrollbackRestore`, must: +/// - still create the terminal session/shell eagerly (unaffected by the flag), +/// - defer applying the persisted block into the live terminal view until the tab is +/// activated, +/// - round-trip the exact original snapshot if the tab is closed/persisted before ever +/// being activated, and +/// - apply the deferred restoration once `materialize_lazy_tab_restorations` runs (as +/// `Workspace::set_active_tab_index` does on activation). +#[test] +fn test_lazy_background_tab_scrollback_restore() { + use crate::terminal::model::block::SerializedBlock; + + let _flag = FeatureFlag::LazyBackgroundTabScrollbackRestore.override_enabled(true); + + App::test((), |mut app| async move { + initialize_app(&mut app); + + let uuid = Uuid::new_v4().as_bytes().to_vec(); + let restored_block = + SerializedBlock::new_for_test(b"echo restored".to_vec(), b"restored\n".to_vec()); + let mut block_lists = HashMap::new(); + block_lists.insert( + PaneUuid(uuid.clone()), + vec![SerializedBlockListItem::Command { + block: Box::new(restored_block), + }], + ); + + let original_snapshot = TerminalPaneSnapshot { + uuid: uuid.clone(), + cwd: None, + shell_launch_data: None, + is_active: true, + is_read_only: false, + input_config: None, + llm_model_override: None, + active_profile_id: None, + conversation_ids_to_restore: Vec::new(), + active_conversation_id: None, + }; + let root = PaneNodeSnapshot::Leaf(LeafSnapshot { + is_focused: true, + custom_vertical_tabs_title: None, + contents: LeafContents::Terminal(original_snapshot.clone()), + }); + + let tips_model = app.add_model(|_| TipsCompleted::default()); + let (_, pane_group) = + app.add_window_with_bounds(WindowStyle::NotStealFocus, WindowBounds::Default, |ctx| { + let banner = ctx.add_model(|_| BannerState::default()); + PaneGroup::new_with_panes_layout( + tips_model, + banner, + ServerApiProvider::as_ref(ctx).get(), + PanesLayout::Snapshot(Box::new(root)), + Arc::new(block_lists), + None, + false, // is_active_tab: simulates a background tab at startup. + ctx, + ) + }); + + let pane_id = pane_group.read(&app, |panes, _| { + panes.pane_ids().next().expect("should have one pane") + }); + + let has_restored_command = |app: &App| { + pane_group.read(app, |panes, ctx| { + let terminal_view = panes + .terminal_view_from_pane_id(pane_id, ctx) + .expect("terminal pane should have a view"); + let model = terminal_view.as_ref(ctx).model.lock(); + model + .block_list() + .blocks() + .iter() + .any(|block| block.command_to_string().contains("echo restored")) + }) + }; + + // The session/shell is created eagerly regardless of the flag: a terminal view + // exists, but the deferred block hasn't been applied yet. + assert!( + !has_restored_command(&app), + "background tab shouldn't have its restored block applied before activation" + ); + + // Snapshotting before activation must round-trip the exact original snapshot, so a + // never-activated tab's history isn't lost if the user quits. + pane_group.read(&app, |panes, ctx| { + let snapshot = panes.snapshot(ctx); + let PaneNodeSnapshot::Leaf(leaf) = snapshot else { + panic!("expected a leaf snapshot"); + }; + let LeafContents::Terminal(restored) = leaf.contents else { + panic!("expected terminal leaf contents"); + }; + assert_eq!( + restored, original_snapshot, + "snapshot should round-trip unchanged before activation" + ); + }); + + // Activating the tab materializes the deferred restoration. + pane_group.update(&mut app, |panes, ctx| { + panes.materialize_lazy_tab_restorations(ctx); + }); + + assert!( + has_restored_command(&app), + "activation should apply the deferred restored block" + ); + + // Materializing again is a no-op (idempotent): the pending map was drained. + pane_group.update(&mut app, |panes, ctx| { + panes.materialize_lazy_tab_restorations(ctx); + }); + assert!(has_restored_command(&app)); + }); +} + +/// APP-5257: a background tab's title must not fall back to a generic +/// placeholder just because its scrollback restoration is deferred, and the +/// title after activation must match what the eager (flag-off) restoration +/// path shows for the identical block — not merely "non-empty" or "cached +/// from before materialization", which a weaker assertion could pass on +/// while the real tab bar (fed by a live re-scan after materialization, +/// not by the pending hint alone) shows something else entirely. +#[test] +fn test_lazy_background_tab_reports_a_title_while_pending_and_after_materializing() { + use crate::terminal::model::block::SerializedBlock; + + fn restored_root(uuid: Vec) -> PaneNodeSnapshot { + PaneNodeSnapshot::Leaf(LeafSnapshot { + is_focused: true, + custom_vertical_tabs_title: None, + contents: LeafContents::Terminal(TerminalPaneSnapshot { + uuid, + cwd: None, + shell_launch_data: None, + is_active: true, + is_read_only: false, + input_config: None, + llm_model_override: None, + active_profile_id: None, + conversation_ids_to_restore: Vec::new(), + active_conversation_id: None, + }), + }) + } + + App::test((), |mut app| async move { + initialize_app(&mut app); + + // The eager (flag-off, `is_active_tab: true`) reference: applies the + // identical block through the normal, always-worked restoration path. + let eager_uuid = Uuid::new_v4().as_bytes().to_vec(); + let eager_root = restored_root(eager_uuid.clone()); + let mut eager_block_lists = HashMap::new(); + eager_block_lists.insert( + PaneUuid(eager_uuid), + vec![SerializedBlockListItem::Command { + block: Box::new(SerializedBlock::new_for_test( + b"uname -a".to_vec(), + b"Darwin\n".to_vec(), + )), + }], + ); + let tips_model = app.add_model(|_| TipsCompleted::default()); + let (_, eager_pane_group) = + app.add_window_with_bounds(WindowStyle::NotStealFocus, WindowBounds::Default, |ctx| { + let banner = ctx.add_model(|_| BannerState::default()); + PaneGroup::new_with_panes_layout( + tips_model, + banner, + ServerApiProvider::as_ref(ctx).get(), + PanesLayout::Snapshot(Box::new(eager_root)), + Arc::new(eager_block_lists), + None, + true, // is_active_tab: eager reference restores immediately. + ctx, + ) + }); + let eager_pane_id = eager_pane_group.read(&app, |panes, _| { + panes.pane_ids().next().expect("should have one pane") + }); + let eager_title = eager_pane_group.read(&app, |panes, ctx| { + panes + .terminal_view_from_pane_id(eager_pane_id, ctx) + .expect("terminal pane should have a view") + .as_ref(ctx) + .last_completed_command_text() + }); + assert_eq!( + eager_title.as_deref(), + Some("uname -a"), + "sanity check: the eager reference path should show the command-derived title" + ); + + // The deferred (flag-on, `is_active_tab: false`) case under test. + let _flag = FeatureFlag::LazyBackgroundTabScrollbackRestore.override_enabled(true); + let deferred_uuid = Uuid::new_v4().as_bytes().to_vec(); + let deferred_root = restored_root(deferred_uuid.clone()); + let mut deferred_block_lists = HashMap::new(); + deferred_block_lists.insert( + PaneUuid(deferred_uuid), + vec![SerializedBlockListItem::Command { + block: Box::new(SerializedBlock::new_for_test( + b"uname -a".to_vec(), + b"Darwin\n".to_vec(), + )), + }], + ); + let tips_model = app.add_model(|_| TipsCompleted::default()); + let (_, pane_group) = + app.add_window_with_bounds(WindowStyle::NotStealFocus, WindowBounds::Default, |ctx| { + let banner = ctx.add_model(|_| BannerState::default()); + PaneGroup::new_with_panes_layout( + tips_model, + banner, + ServerApiProvider::as_ref(ctx).get(), + PanesLayout::Snapshot(Box::new(deferred_root)), + Arc::new(deferred_block_lists), + None, + false, // is_active_tab: simulates a background tab at startup. + ctx, + ) + }); + + let pane_id = pane_group.read(&app, |panes, _| { + panes.pane_ids().next().expect("should have one pane") + }); + let title = |app: &App| { + pane_group.read(app, |panes, ctx| { + panes + .terminal_view_from_pane_id(pane_id, ctx) + .expect("terminal pane should have a view") + .as_ref(ctx) + .last_completed_command_text() + }) + }; + + assert_eq!( + title(&app).as_deref(), + eager_title.as_deref(), + "a pending background tab should report the same command-derived title the eager \ + path shows, not a generic placeholder" + ); + + pane_group.update(&mut app, |panes, ctx| { + panes.materialize_lazy_tab_restorations(ctx); + }); + + assert_eq!( + title(&app).as_deref(), + eager_title.as_deref(), + "after materializing, the title must still match the eager path's title — asserting \ + merely that a hint value survives is not enough, since the real tab bar re-derives \ + this from the live block list after materialization" + ); + }); +} + +/// APP-5257: permanently closing a background pane before it was ever +/// activated must drop its pending lazy restoration entry, rather than +/// leaking the stashed blocks/conversation payload. +#[test] +fn test_closing_pending_pane_drops_its_lazy_restoration() { + use crate::terminal::model::block::SerializedBlock; + + let _flag = FeatureFlag::LazyBackgroundTabScrollbackRestore.override_enabled(true); + + App::test((), |mut app| async move { + initialize_app(&mut app); + + let uuid = Uuid::new_v4().as_bytes().to_vec(); + let restored_block = + SerializedBlock::new_for_test(b"echo restored".to_vec(), b"restored\n".to_vec()); + let mut block_lists = HashMap::new(); + block_lists.insert( + PaneUuid(uuid.clone()), + vec![SerializedBlockListItem::Command { + block: Box::new(restored_block), + }], + ); + + let root = PaneNodeSnapshot::Leaf(LeafSnapshot { + is_focused: true, + custom_vertical_tabs_title: None, + contents: LeafContents::Terminal(TerminalPaneSnapshot { + uuid: uuid.clone(), + cwd: None, + shell_launch_data: None, + is_active: true, + is_read_only: false, + input_config: None, + llm_model_override: None, + active_profile_id: None, + conversation_ids_to_restore: Vec::new(), + active_conversation_id: None, + }), + }); + + let tips_model = app.add_model(|_| TipsCompleted::default()); + let (_, pane_group) = + app.add_window_with_bounds(WindowStyle::NotStealFocus, WindowBounds::Default, |ctx| { + let banner = ctx.add_model(|_| BannerState::default()); + PaneGroup::new_with_panes_layout( + tips_model, + banner, + ServerApiProvider::as_ref(ctx).get(), + PanesLayout::Snapshot(Box::new(root)), + Arc::new(block_lists), + None, + false, // is_active_tab: simulates a background tab at startup. + ctx, + ) + }); + + let pane_id = pane_group.read(&app, |panes, _| { + panes.pane_ids().next().expect("should have one pane") + }); + + pane_group.read(&app, |panes, _| { + assert!( + panes + .pending_lazy_terminal_restorations + .contains_key(&pane_id), + "restoring a background tab should stash a pending entry for its pane" + ); + }); + + pane_group.update(&mut app, |panes, ctx| { + panes.cleanup_closed_pane(pane_id, ctx); + }); + + pane_group.read(&app, |panes, _| { + assert!( + !panes + .pending_lazy_terminal_restorations + .contains_key(&pane_id), + "permanently closing a never-activated pane must drop its pending restoration, \ + not leak the stashed blocks/conversation payload" + ); + }); + }); +} + +/// APP-5257: `clean_up_panes` (not `cleanup_closed_pane`) is the real +/// teardown path a closed tab reaches once `UndoCloseStack` discards it +/// (its grace period expired, or the user disabled undo-close entirely). +/// It must also release any never-activated pane's pending restoration. +#[test] +fn test_clean_up_panes_drops_pending_lazy_restorations() { + use crate::terminal::model::block::SerializedBlock; + + let _flag = FeatureFlag::LazyBackgroundTabScrollbackRestore.override_enabled(true); + + App::test((), |mut app| async move { + initialize_app(&mut app); + + let uuid = Uuid::new_v4().as_bytes().to_vec(); + let restored_block = + SerializedBlock::new_for_test(b"echo restored".to_vec(), b"restored\n".to_vec()); + let mut block_lists = HashMap::new(); + block_lists.insert( + PaneUuid(uuid.clone()), + vec![SerializedBlockListItem::Command { + block: Box::new(restored_block), + }], + ); + + let root = PaneNodeSnapshot::Leaf(LeafSnapshot { + is_focused: true, + custom_vertical_tabs_title: None, + contents: LeafContents::Terminal(TerminalPaneSnapshot { + uuid: uuid.clone(), + cwd: None, + shell_launch_data: None, + is_active: true, + is_read_only: false, + input_config: None, + llm_model_override: None, + active_profile_id: None, + conversation_ids_to_restore: Vec::new(), + active_conversation_id: None, + }), + }); + + let tips_model = app.add_model(|_| TipsCompleted::default()); + let (_, pane_group) = + app.add_window_with_bounds(WindowStyle::NotStealFocus, WindowBounds::Default, |ctx| { + let banner = ctx.add_model(|_| BannerState::default()); + PaneGroup::new_with_panes_layout( + tips_model, + banner, + ServerApiProvider::as_ref(ctx).get(), + PanesLayout::Snapshot(Box::new(root)), + Arc::new(block_lists), + None, + false, // is_active_tab: simulates a background tab at startup. + ctx, + ) + }); + + pane_group.read(&app, |panes, _| { + assert!( + !panes.pending_lazy_terminal_restorations.is_empty(), + "restoring a background tab should stash a pending entry for its pane" + ); + }); + + pane_group.update(&mut app, |panes, ctx| { + panes.clean_up_panes(ctx); + }); + + pane_group.read(&app, |panes, _| { + assert!( + panes.pending_lazy_terminal_restorations.is_empty(), + "the real tab-close teardown path must drop pending restorations for panes \ + that were never activated, not just cleanup_closed_pane's per-pane path" + ); + }); + }); +} diff --git a/app/src/terminal/model/block/serialized_block.rs b/app/src/terminal/model/block/serialized_block.rs index c0226744949..bb62fc6b624 100644 --- a/app/src/terminal/model/block/serialized_block.rs +++ b/app/src/terminal/model/block/serialized_block.rs @@ -262,6 +262,73 @@ impl SerializedBlock { }; has_block_failed(self.exit_code, block_state) } + + /// A best-effort plain-text approximation of this block's command line, + /// stripping ANSI/OSC escape sequences from the raw styled bytes. This is + /// a lossy shortcut, not the full ANSI-aware parsing `restore_block` + /// performs; it exists only to give a pane a usable tab title while its + /// real restoration is still deferred (see + /// `FeatureFlag::LazyBackgroundTabScrollbackRestore`), before its blocks + /// have ever been fed through the real terminal model. + pub fn plain_text_command_preview(&self) -> Option { + if !self.did_execute { + return None; + } + let text = strip_ansi_escapes_for_preview(&self.stylized_command); + (!text.is_empty()).then_some(text) + } +} + +/// Strips ANSI CSI/OSC escape sequences from `bytes` and returns the first +/// non-blank line of the remaining printable text. Deliberately not a full +/// VTE parser (which requires a live grid to interpret correctly); good +/// enough for a short preview string, not for faithful rendering. +fn strip_ansi_escapes_for_preview(bytes: &[u8]) -> String { + let mut result = String::new(); + let mut i = 0; + while i < bytes.len() { + let byte = bytes[i]; + if byte == 0x1b { + match bytes.get(i + 1) { + Some(b'[') => { + // CSI: skip to and past the final byte in 0x40..=0x7e. + i += 2; + while i < bytes.len() && !(0x40..=0x7e).contains(&bytes[i]) { + i += 1; + } + i = (i + 1).min(bytes.len()); + } + Some(b']') => { + // OSC: skip to and past the BEL or ST (ESC \) terminator. + i += 2; + while i < bytes.len() && bytes[i] != 0x07 { + if bytes[i] == 0x1b && bytes.get(i + 1) == Some(&b'\\') { + i += 2; + break; + } + i += 1; + } + if i < bytes.len() && bytes[i] == 0x07 { + i += 1; + } + } + _ => i += 2, + } + continue; + } + if byte == b'\r' || byte == b'\n' { + if !result.trim().is_empty() { + break; + } + i += 1; + continue; + } + if byte.is_ascii_graphic() || byte == b' ' || byte == b'\t' { + result.push(byte as char); + } + i += 1; + } + result.trim().to_string() } /// We should only be serializing a block that has finished. diff --git a/app/src/terminal/model/block/serialized_block_tests.rs b/app/src/terminal/model/block/serialized_block_tests.rs index f16e9326e82..9382c349cba 100644 --- a/app/src/terminal/model/block/serialized_block_tests.rs +++ b/app/src/terminal/model/block/serialized_block_tests.rs @@ -61,3 +61,59 @@ fn from_json_accepts_integer_array_bytes() { assert_eq!(block.stylized_command, b"echo hello"); assert_eq!(block.stylized_output, b"hello world"); } + +/// APP-5257: `plain_text_command_preview` is used to give a background tab a +/// usable title before its real restoration has ever run. It must handle +/// plain commands, ones wrapped in CSI (color) escapes, and ones interleaved +/// with an OSC (e.g. title-setting) escape sequence, matching what a shell +/// with syntax highlighting or a title-setting hook would actually emit. +#[test] +fn plain_text_command_preview_strips_plain_command() { + let block = SerializedBlock::new_for_test(b"pwd".to_vec(), b"/home/user\n".to_vec()); + assert_eq!(block.plain_text_command_preview().as_deref(), Some("pwd")); +} + +#[test] +fn plain_text_command_preview_strips_csi_color_codes() { + let stylized = b"\x1b[32mecho\x1b[0m TAB2_MARKER_BRAVO".to_vec(); + let block = SerializedBlock::new_for_test(stylized, Vec::new()); + assert_eq!( + block.plain_text_command_preview().as_deref(), + Some("echo TAB2_MARKER_BRAVO") + ); +} + +#[test] +fn plain_text_command_preview_strips_osc_title_sequence() { + // A shell configured to set the terminal title to the running command + // (a common preexec hook) could plausibly interleave an OSC sequence + // with the echoed command bytes. + let stylized = b"\x1b]0;uname -a\x07uname -a".to_vec(); + let block = SerializedBlock::new_for_test(stylized, Vec::new()); + assert_eq!( + block.plain_text_command_preview().as_deref(), + Some("uname -a") + ); +} + +#[test] +fn plain_text_command_preview_takes_only_the_first_line() { + let block = SerializedBlock::new_for_test(b"echo one\necho two".to_vec(), Vec::new()); + assert_eq!( + block.plain_text_command_preview().as_deref(), + Some("echo one") + ); +} + +#[test] +fn plain_text_command_preview_is_none_for_a_block_that_never_executed() { + let mut block = SerializedBlock::new_for_test(b"pwd".to_vec(), Vec::new()); + block.did_execute = false; + assert_eq!(block.plain_text_command_preview(), None); +} + +#[test] +fn plain_text_command_preview_is_none_for_empty_command() { + let block = SerializedBlock::new_for_test(Vec::new(), Vec::new()); + assert_eq!(block.plain_text_command_preview(), None); +} diff --git a/app/src/terminal/model/blocks.rs b/app/src/terminal/model/blocks.rs index fee0885186a..0453831b862 100644 --- a/app/src/terminal/model/blocks.rs +++ b/app/src/terminal/model/blocks.rs @@ -999,7 +999,7 @@ impl BlockList { } #[cfg(feature = "local_fs")] - pub(in crate::terminal) fn append_session_restoration_separator_to_block_list( + pub fn append_session_restoration_separator_to_block_list( &mut self, is_historical_conversation_restoration: bool, ) { @@ -1013,6 +1013,64 @@ impl BlockList { ); } + /// Applies scrollback blocks whose restoration was deferred at startup. + /// No-ops if the live session is no longer pristine (a command has + /// already started in it): the restored content is strictly older than + /// anything the live session could have produced, and this list only + /// supports appending, so splicing it in now would force-finish and + /// misorder real, possibly still-running, output. Returns whether the + /// blocks were applied. + pub fn apply_deferred_restored_blocks( + &mut self, + restored_blocks: &[SerializedBlockListItem], + ) -> bool { + let valid_blocks: Vec<&SerializedBlock> = restored_blocks + .iter() + .filter_map(|item| match item { + SerializedBlockListItem::Command { block } + if block.start_ts.is_some() && block.completed_ts.is_some() => + { + Some(block.as_ref()) + } + _ => None, + }) + .collect(); + + if valid_blocks.is_empty() { + return false; + } + + // A still-bootstrapping session's active block can already be + // `started()` (e.g. the script-execution stage), which is expected + // and harmless to finish here. Only bail once bootstrapping is done + // and a real post-bootstrap command has genuinely started. + if self.is_bootstrapping_precmd_done() && self.active_block().started() { + report_error!( + "Dropping deferred scrollback restoration: session is no longer pristine" + ); + return false; + } + + self.is_restored_session = true; + self.restored_session_ts = valid_blocks.last().and_then(|block| block.completed_ts); + + self.finish_active_block_before_followup_append(); + + let mut processor = Processor::new(); + for block in valid_blocks { + // Tag as `RestoreBlocks` (matching eager restoration via + // `initialize`), not `PostBootstrapPrecmd` like + // `append_followup_shared_session_scrollback` uses: this content + // genuinely comes from a previous session, so it should render + // with the same "restored" treatment eager restoration gives it. + self.restore_block(block, BootstrapStage::RestoreBlocks, &mut processor); + } + + self.ensure_active_block_after_shared_session_scrollback(); + self.event_proxy.send_wakeup_event(); + true + } + /// Inserts an inline banner _before_ the provided block_index. pub fn insert_inline_banner_before_block( &mut self, diff --git a/app/src/terminal/model/blocks_tests.rs b/app/src/terminal/model/blocks_tests.rs index f5c5a9f179a..72999fd83c9 100644 --- a/app/src/terminal/model/blocks_tests.rs +++ b/app/src/terminal/model/blocks_tests.rs @@ -715,6 +715,113 @@ pub fn test_restore_completed_blocks() { )); } +/// APP-5257: `apply_deferred_restored_blocks` backfills scrollback into an +/// already-bootstrapped, live block list (mirroring how a background tab's +/// deferred restoration is applied once it's activated). The already-active +/// (empty, never-typed-into) block must not linger as visible clutter, the +/// restored blocks must land ahead of a fresh active block, and each restored +/// block must fire a `Restored` completion event just like eager restoration +/// does. +#[test] +pub fn test_apply_deferred_restored_blocks_after_bootstrap() { + let (events_tx, events_rx) = async_channel::unbounded(); + let channel_event_proxy = ChannelEventListener::builder_for_test() + .with_terminal_events_tx(events_tx) + .build(); + + // Simulates a background tab whose scrollback restoration was deferred at + // startup: the session still bootstraps eagerly (with no restored + // blocks), leaving a live, empty, ready-for-input block list. + let mut block_list = new_bootstrapped_block_list(None, None, channel_event_proxy); + let blocks_before_restore = block_list.blocks().len(); + drain_terminal_events(&events_rx); + + let serialized_block: SerializedBlockListItem = + SerializedBlock::new_for_test("i am".into(), "restored".into()).into(); + let deferred_blocks = [serialized_block.clone(), serialized_block]; + + block_list.apply_deferred_restored_blocks(&deferred_blocks); + + assert_eq!(block_list.blocks().len(), blocks_before_restore + 3); + assert!(block_list.blocks()[blocks_before_restore].is_restored()); + assert!(block_list.blocks()[blocks_before_restore + 1].is_restored()); + assert!( + block_list.blocks()[blocks_before_restore - 1].is_empty(&TranscriptScope::Terminal), + "the previously-active empty block must not become visible clutter" + ); + assert!( + !block_list.active_block().finished(), + "a fresh active block should be ready for input after materializing" + ); + assert!(block_list.is_restored_session()); + + let mut block_completed_events = Vec::new(); + while let Ok(event) = events_rx.try_recv() { + if let Event::AfterBlockCompleted(block_completed_type) = event { + block_completed_events.push(block_completed_type); + } + } + assert_eq!(block_completed_events.len(), 2); + assert!(matches!( + block_completed_events[0].block_type, + BlockType::Restored + )); + assert!(matches!( + block_completed_events[1].block_type, + BlockType::Restored + )); +} + +/// A second call after the first is fed no new blocks and must be a no-op. +#[test] +pub fn test_apply_deferred_restored_blocks_is_a_noop_when_given_no_blocks() { + let mut block_list = + new_bootstrapped_block_list(None, None, ChannelEventListener::new_for_test()); + let blocks_before = block_list.blocks().len(); + + block_list.apply_deferred_restored_blocks(&[]); + + assert_eq!(block_list.blocks().len(), blocks_before); + assert!(!block_list.is_restored_session()); +} + +/// APP-5257: a background tab's session stays live while its restoration is +/// deferred, so it can produce genuine content (e.g. via synced input) +/// before ever being activated. Applying the deferred (strictly older) +/// blocks in that case must not force-finish or reorder the live, currently +/// running command — the restoration must be dropped instead. +#[test] +pub fn test_apply_deferred_restored_blocks_does_not_corrupt_a_live_running_command() { + let mut block_list = + new_bootstrapped_block_list(None, None, ChannelEventListener::new_for_test()); + + block_list.start_active_block(); + input_string(&mut block_list, "sleep 100"); + block_list.preexec(Default::default()); + assert!(block_list.active_block().started()); + assert!(!block_list.active_block().finished()); + + let serialized_block: SerializedBlockListItem = + SerializedBlock::new_for_test("i am".into(), "restored".into()).into(); + + let applied = block_list.apply_deferred_restored_blocks(&[serialized_block]); + + assert!( + !applied, + "deferred restoration must be dropped rather than corrupt a live, running command" + ); + assert!( + !block_list.active_block().finished(), + "the live command must not be force-finished by a deferred restoration attempt" + ); + assert_eq!( + block_list.active_block().command_to_string(), + "sleep 100", + "the live command's content must be untouched" + ); + assert!(!block_list.is_restored_session()); +} + #[test] pub fn test_restore_blocks_with_local_status() { let (events_tx, events_rx) = async_channel::unbounded(); diff --git a/app/src/terminal/view.rs b/app/src/terminal/view.rs index ef31ba4e7c0..7745d905b77 100644 --- a/app/src/terminal/view.rs +++ b/app/src/terminal/view.rs @@ -2870,6 +2870,12 @@ pub struct TerminalView { // we want to keep the title as the conversation title, so we should ignore the model event setting the title after bootstrapping finishes ignore_next_set_title_event: bool, + /// A tab-title fallback for a pane whose scrollback restoration was + /// deferred (see `FeatureFlag::LazyBackgroundTabScrollbackRestore`), + /// computed once from the still-pending blocks so the tab has a usable + /// label before those blocks are ever fed into this view's live model. + pending_restoration_title_hint: Option, + cli_subagent_views: HashMap>, cli_subagent_controller: ModelHandle, use_agent_footer: ViewHandle, @@ -4444,6 +4450,7 @@ impl TerminalView { current_repo_path: None, terminal_title: Default::default(), ignore_next_set_title_event: false, + pending_restoration_title_hint: None, cli_subagent_views: Default::default(), cli_subagent_controller, use_agent_footer: use_agent_button_bar, diff --git a/app/src/terminal/view/load_ai_conversation.rs b/app/src/terminal/view/load_ai_conversation.rs index 7eb53b6e5f8..905b1b001d0 100644 --- a/app/src/terminal/view/load_ai_conversation.rs +++ b/app/src/terminal/view/load_ai_conversation.rs @@ -633,7 +633,7 @@ impl TerminalView { /// In this case, we expect shell command blocks to already exist in the terminal model, since they must /// be restored before bootstrapping finishes. /// Then we need to order the AI blocks correctly relative to shell commands that exist in the model. - pub(super) fn restore_conversations_on_view_creation( + pub(crate) fn restore_conversations_on_view_creation( &mut self, conversation_restoration: ConversationRestorationInNewPaneType, ctx: &mut ViewContext, diff --git a/app/src/terminal/view/tab_metadata.rs b/app/src/terminal/view/tab_metadata.rs index 2dd1c96c462..bb9c76a9359 100644 --- a/app/src/terminal/view/tab_metadata.rs +++ b/app/src/terminal/view/tab_metadata.rs @@ -43,23 +43,32 @@ impl TerminalView { } pub fn last_completed_command_text(&self) -> Option { - let model = self.model.lock(); - model.block_list().blocks().iter().rev().find_map(|block| { - if block.finished() - && !block.is_background() - && !block.is_static() - && (block.bootstrap_stage().is_done() || block.is_restored()) - { - let cmd = block.command_to_string(); - if cmd.trim().is_empty() { - None + let from_live_blocks = { + let model = self.model.lock(); + model.block_list().blocks().iter().rev().find_map(|block| { + if block.finished() + && !block.is_background() + && !block.is_static() + && (block.bootstrap_stage().is_done() || block.is_restored()) + { + let cmd = block.command_to_string(); + if cmd.trim().is_empty() { + None + } else { + Some(cmd) + } } else { - Some(cmd) + None } - } else { - None - } - }) + }) + }; + from_live_blocks.or_else(|| self.pending_restoration_title_hint.clone()) + } + + /// Sets the tab-title fallback shown while this pane's scrollback + /// restoration is deferred. See `pending_restoration_title_hint`. + pub(crate) fn set_pending_restoration_title_hint(&mut self, hint: Option) { + self.pending_restoration_title_hint = hint; } pub fn terminal_title_text(&self) -> String { diff --git a/app/src/workspace/view.rs b/app/src/workspace/view.rs index 0d0874bc6e0..9894c5e1c8a 100644 --- a/app/src/workspace/view.rs +++ b/app/src/workspace/view.rs @@ -3954,10 +3954,15 @@ impl Workspace { .enumerate() .for_each(|(tab_index, saved_tab)| { let custom_title = saved_tab.custom_title.clone(); - self.add_tab_with_pane_layout( + // Insert without activating: activating every restored tab as + // it's inserted would materialize its deferred scrollback + // restoration immediately, defeating the point of deferring it. + // The intended active tab is activated once, after this loop. + self.add_restored_tab( PanesLayout::Snapshot(Box::new(saved_tab.root.clone())), block_lists.clone(), custom_title, + tab_index == active_tab_index, ctx, ); self.tabs[tab_index].default_directory_color = @@ -5417,6 +5422,18 @@ impl Workspace { self.active_tab_index = index; + // Apply any scrollback/AI conversation restoration that was deferred + // for this tab at startup (see + // `FeatureFlag::LazyBackgroundTabScrollbackRestore`), before anything + // below reads this tab's pane content (title, focus, etc.). A no-op + // once the tab has been activated once. + if let Some(tab) = self.tabs.get(index) { + let pane_group = tab.pane_group.clone(); + pane_group.update(ctx, |pane_group, ctx| { + pane_group.materialize_lazy_tab_restorations(ctx); + }); + } + // The range selection's anchor is the active tab, so any change to // the active tab makes the existing selection stale; clear it. self.clear_tab_multi_selection(ctx); @@ -12039,9 +12056,15 @@ impl Workspace { else { return false; }; + let pending_restoration = pane_group.update(ctx, |pg, _| { + pg.take_pending_lazy_terminal_restoration(pane_id) + }); source_pane_group.update(ctx, |pg, ctx| { pg.re_adopt_child_agent_pane(pane_content, origin.conversation_id, ctx); + if let Some(pending_restoration) = pending_restoration { + pg.insert_pending_lazy_terminal_restoration(pane_id, pending_restoration); + } }); true @@ -12823,6 +12846,7 @@ impl Workspace { panes_layout, block_lists, self.model_event_sender.clone(), + true, ctx, ); if let Some(title) = custom_tab_title { @@ -12891,6 +12915,46 @@ impl Workspace { } } + /// Appends a tab restored from a persisted window snapshot, without + /// activating it and without the interactive-placement/inheritance rules + /// `add_tab_with_pane_layout` applies (irrelevant when bulk-restoring a + /// whole window's tabs in their original order, which sets that state + /// explicitly from each tab's own snapshot). The caller must activate + /// the intended tab once, after every tab has been inserted. + fn add_restored_tab( + &mut self, + panes_layout: PanesLayout, + block_lists: Arc>>, + custom_tab_title: Option, + is_active_tab: bool, + ctx: &mut ViewContext, + ) { + let new_pane_group = ctx.add_typed_action_view(|ctx| { + let mut pane_group = PaneGroup::new_with_panes_layout( + self.tips_completed.clone(), + self.user_default_shell_unsupported_banner_model_handle + .clone(), + self.server_api.clone(), + panes_layout, + block_lists, + self.model_event_sender.clone(), + is_active_tab, + ctx, + ); + if let Some(title) = custom_tab_title { + pane_group.set_title(&title, ctx); + } + pane_group + }); + + ctx.subscribe_to_view(&new_pane_group, move |me, pane_group, event, ctx| { + me.handle_file_tree_event(pane_group, event, ctx) + }); + + self.tab_mru_order.push(new_pane_group.id()); + self.tabs.push(TabData::new(new_pane_group)); + } + pub fn add_tab_from_existing_pane( &mut self, pane: Box, @@ -16583,20 +16647,28 @@ impl Workspace { } => { // If an editor tab is dropped into a new position in the workspace tab group, // create a new pane and insert it into the group. - let pane = if let ActionOrigin::EditorTab(editor_tab_index) = origin { - pane_group.update(ctx, |pane_group, ctx| { - pane_group.remove_editor_tab_for_move( - *pane_id, - *editor_tab_index, - ctx, - ) - }) - } else { - // Otherwise, move the existing pane's contents into the workspace tab group. - pane_group.update(ctx, |pane_group, ctx| { - pane_group.remove_pane_for_move(pane_id, ctx) - }) - }; + let (pane, pending_restoration) = + if let ActionOrigin::EditorTab(editor_tab_index) = origin { + let pane = pane_group.update(ctx, |pane_group, ctx| { + pane_group.remove_editor_tab_for_move( + *pane_id, + *editor_tab_index, + ctx, + ) + }); + (pane, None) + } else { + // Otherwise, move the existing pane's contents into the workspace tab group. + let pane = pane_group.update(ctx, |pane_group, ctx| { + pane_group.remove_pane_for_move(pane_id, ctx) + }); + let pending_restoration = + pane_group.update(ctx, |pane_group, _| { + pane_group + .take_pending_lazy_terminal_restoration(*pane_id) + }); + (pane, pending_restoration) + }; if let Some(pane) = pane { // `index`/`group` are already resolved by @@ -16615,6 +16687,18 @@ impl Workspace { ctx, ); + if let Some(pending_restoration) = pending_restoration + && let Some(new_pane_group) = + self.get_pane_group_view(self.active_tab_index).cloned() + { + new_pane_group.update(ctx, |pane_group, _| { + pane_group.insert_pending_lazy_terminal_restoration( + *pane_id, + pending_restoration, + ); + }); + } + // If the setting is enabled, preserve the color of the original pane's // tab for the newly created tab. if *TabSettings::as_ref(ctx).preserve_active_tab_color.value() @@ -16813,9 +16897,18 @@ impl Workspace { if let Some(pane) = pane_group.update(ctx, |pane_group, ctx| { pane_group.remove_pane_for_move(pane_id, ctx) }) { + let pending_restoration = pane_group.update(ctx, |pane_group, _| { + pane_group.take_pending_lazy_terminal_restoration(*pane_id) + }); self.set_active_tab_index(*tab_idx, ctx); self.active_tab_pane_group().update(ctx, |pane_group, ctx| { - pane_group.add_pane_as_hidden(pane, *hidden_pane_preview_direction, ctx) + pane_group.add_pane_as_hidden(pane, *hidden_pane_preview_direction, ctx); + if let Some(pending_restoration) = pending_restoration { + pane_group.insert_pending_lazy_terminal_restoration( + *pane_id, + pending_restoration, + ); + } }); } } diff --git a/app/src/workspace/view_tests.rs b/app/src/workspace/view_tests.rs index 7a0ff65bf58..f11c7026ada 100644 --- a/app/src/workspace/view_tests.rs +++ b/app/src/workspace/view_tests.rs @@ -341,6 +341,139 @@ fn restored_workspace( workspace } +/// APP-5257: an end-to-end regression covering the actual +/// `Workspace::configure_new_workspace` restore loop (not just +/// `PaneGroup::new_with_panes_layout` in isolation). A prior version of this +/// fix appeared to work at the `PaneGroup` level, but every restored tab was +/// still activated as it was inserted (`add_tab_with_pane_layout`'s +/// `activate_tab_internal` call), silently materializing every tab's +/// deferred restoration during the loop and reclaiming nothing. This test +/// fails if that regresses. +#[test] +fn test_restoring_a_window_defers_background_tab_scrollback_through_the_real_loop() { + use crate::terminal::model::block::SerializedBlock; + + let _flag = FeatureFlag::LazyBackgroundTabScrollbackRestore.override_enabled(true); + + App::test((), |mut app| async move { + initialize_app(&mut app); + + let active_uuid = uuid::Uuid::new_v4().as_bytes().to_vec(); + let background_uuid = uuid::Uuid::new_v4().as_bytes().to_vec(); + + let terminal_snapshot = |uuid: Vec| TerminalPaneSnapshot { + uuid, + cwd: None, + shell_launch_data: None, + is_active: true, + is_read_only: false, + input_config: None, + llm_model_override: None, + active_profile_id: None, + conversation_ids_to_restore: Vec::new(), + active_conversation_id: None, + }; + let tab_snapshot = |uuid: Vec| TabSnapshot { + custom_title: None, + root: PaneNodeSnapshot::Leaf(LeafSnapshot { + is_focused: true, + custom_vertical_tabs_title: None, + contents: LeafContents::Terminal(terminal_snapshot(uuid)), + }), + default_directory_color: None, + selected_color: SelectedTabColor::Unset, + left_panel: None, + right_panel: None, + group_id: None, + pinned: false, + }; + + let window_snapshot = WindowSnapshot { + tabs: vec![ + tab_snapshot(active_uuid.clone()), + tab_snapshot(background_uuid.clone()), + ], + active_tab_index: 0, + team_uid: None, + bounds: None, + fullscreen_state: Default::default(), + quake_mode: false, + universal_search_width: None, + warp_ai_width: None, + voltron_width: None, + warp_drive_index_width: None, + left_panel_open: false, + vertical_tabs_panel_open: false, + left_panel_width: None, + right_panel_width: None, + agent_management_filters: None, + tab_groups: Vec::new(), + }; + + let restored_block = + SerializedBlock::new_for_test(b"echo restored".to_vec(), b"restored\n".to_vec()); + let mut block_lists = HashMap::new(); + block_lists.insert( + PaneUuid(background_uuid), + vec![SerializedBlockListItem::Command { + block: Box::new(restored_block), + }], + ); + + let global_resource_handles = GlobalResourceHandles::mock(&mut app); + let (_, workspace) = app.add_window(WindowStyle::NotStealFocus, |ctx| { + Workspace::new( + global_resource_handles, + None, + NewWorkspaceSource::Restored { + window_snapshot, + block_lists: Arc::new(block_lists), + }, + ctx, + ) + }); + + let has_restored_command = |app: &App, tab_index: usize| { + workspace.read(app, |workspace, ctx| { + let pane_group = workspace.tabs[tab_index].pane_group.as_ref(ctx); + let pane_id = pane_group + .pane_ids() + .next() + .expect("tab should have a pane"); + let terminal_view = pane_group + .terminal_view_from_pane_id(pane_id, ctx) + .expect("terminal pane should have a view"); + let model = terminal_view.as_ref(ctx).model.lock(); + model + .block_list() + .blocks() + .iter() + .any(|block| block.command_to_string().contains("echo restored")) + }) + }; + + // The critical assertion: restoring the whole window through the real + // loop must NOT have materialized the background tab's (tab 1) + // deferred restoration, even though every tab's `add_restored_tab` + // call ran during that same loop. + assert!( + !has_restored_command(&app, 1), + "the real configure_new_workspace loop must defer, not eagerly apply, a \ + background tab's restoration" + ); + + // Activating the background tab through the public API must apply it. + workspace.update(&mut app, |workspace, ctx| { + workspace.set_active_tab_index(1, ctx); + }); + assert!( + has_restored_command(&app, 1), + "activating the tab through the real workspace API must materialize its \ + deferred restoration" + ); + }); +} + fn transferred_tab_workspace( app: &mut App, vertical_tabs_panel_open: bool, diff --git a/crates/warp_features/src/lib.rs b/crates/warp_features/src/lib.rs index 7c47d071df3..b6e385a5f38 100644 --- a/crates/warp_features/src/lib.rs +++ b/crates/warp_features/src/lib.rs @@ -966,6 +966,14 @@ pub enum FeatureFlag { /// Requires `OzHandoff` to also be enabled; a no-op for local runs and when /// `--no-snapshot` is set. Off by default while the coordinator rolls out. PeriodicHandoffCheckpoints, + + /// When restoring a window's tabs at startup, skips feeding persisted + /// scrollback blocks and AI conversation history into the terminal + /// session of any tab that isn't the initially-active tab. The shell + /// still starts eagerly for every tab; only the (comparatively + /// expensive) text layout of restoring history is deferred until the + /// tab is first activated, at which point it's applied in full. + LazyBackgroundTabScrollbackRestore, } static FLAG_STATES: [AtomicBool; cardinality::()] =