diff --git a/codex-rs/git-utils/src/lib.rs b/codex-rs/git-utils/src/lib.rs index 2bcd233648..ec0f4128b7 100644 --- a/codex-rs/git-utils/src/lib.rs +++ b/codex-rs/git-utils/src/lib.rs @@ -9,6 +9,7 @@ mod operations; mod platform; mod status; mod trust; +mod worktree; /// Git configuration that rejects implicitly discovered bare repositories while /// preserving repositories selected explicitly through `GIT_DIR` or `--git-dir`. @@ -51,3 +52,6 @@ pub use operations::git_config_override_env; pub use platform::create_symlink; pub use status::get_has_changes_in_repo; pub use trust::resolve_root_git_project_for_trust; +pub use worktree::RepositoryIdentity; +pub use worktree::linked_worktree_cwds; +pub use worktree::repository_identity; diff --git a/codex-rs/git-utils/src/worktree.rs b/codex-rs/git-utils/src/worktree.rs new file mode 100644 index 0000000000..863c4c5287 --- /dev/null +++ b/codex-rs/git-utils/src/worktree.rs @@ -0,0 +1,178 @@ +//! Resolves repository identity and sibling checkouts from validated Git metadata. +//! Administrative links must agree before a checkout participates in discovery. + +use std::collections::HashSet; +use std::ffi::OsStr; +use std::fs; +use std::path::Path; +use std::path::PathBuf; + +use codex_utils_absolute_path::AbsolutePathBuf; + +use crate::get_git_repo_root; + +/// The stable on-disk identity shared by a repository and its linked worktrees. +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub struct RepositoryIdentity { + /// Canonical shared Git administrative directory. + pub common_dir: AbsolutePathBuf, + /// Directory within the current checkout, preserved across linked worktrees. + pub relative_cwd: PathBuf, + /// Canonical root of the repository's primary checkout. + pub primary_root: AbsolutePathBuf, +} + +fn canonicalize_native(path: impl AsRef) -> Option { + AbsolutePathBuf::from_absolute_path(path) + .ok()? + .canonicalize() + .ok() + .map(AbsolutePathBuf::into_path_buf) +} + +/// Identifies a checkout without executing Git or trusting unchecked administrative links. +pub fn repository_identity(cwd: &Path) -> Option { + let canonical_cwd = canonicalize_native(cwd)?; + if !canonical_cwd.is_dir() { + return None; + } + + let checkout_root = canonicalize_native(get_git_repo_root(&canonical_cwd)?)?; + let relative_cwd = canonical_cwd + .strip_prefix(&checkout_root) + .ok()? + .to_path_buf(); + let git_entry = checkout_root.join(".git"); + let entry_type = fs::symlink_metadata(&git_entry).ok()?.file_type(); + if entry_type.is_symlink() { + return None; + } + + let common_dir = if entry_type.is_dir() { + canonicalize_native(&git_entry)? + } else if entry_type.is_file() { + let git_dir = read_git_path(&git_entry, &checkout_root, "gitdir:")?; + if !git_dir.is_dir() { + return None; + } + let common_dir = read_git_path(&git_dir.join("commondir"), &git_dir, "")?; + if !common_dir.is_dir() { + return None; + } + let registered_root = canonicalize_native(common_dir.join("worktrees"))?; + if git_dir.parent()? != registered_root { + return None; + } + let backlink = read_git_path(&git_dir.join("gitdir"), &git_dir, "")?; + if backlink != canonicalize_native(&git_entry)? { + return None; + } + common_dir + } else { + return None; + }; + + let primary_root = common_dir.parent()?.to_path_buf(); + let primary_git_entry = primary_root.join(".git"); + let primary_entry_type = fs::symlink_metadata(&primary_git_entry).ok()?.file_type(); + if !primary_entry_type.is_dir() + || primary_entry_type.is_symlink() + || canonicalize_native(primary_git_entry)? != common_dir + { + return None; + } + + Some(RepositoryIdentity { + common_dir: AbsolutePathBuf::from_absolute_path_checked(common_dir).ok()?, + relative_cwd, + primary_root: AbsolutePathBuf::from_absolute_path_checked(primary_root).ok()?, + }) +} + +/// Returns corresponding existing directories in the current, primary, and linked checkouts. +pub fn linked_worktree_cwds(cwd: &Path) -> Option> { + let identity = repository_identity(cwd)?; + let current_cwd = canonicalize_native(cwd)?; + let mut result = vec![cwd.to_path_buf()]; + let mut seen = HashSet::from([cwd.to_path_buf()]); + if seen.insert(current_cwd.clone()) { + result.push(current_cwd); + } + + append_linked_cwd(&mut result, &mut seen, &identity.primary_root, &identity); + + let worktrees = identity.common_dir.join("worktrees"); + if !worktrees.exists() { + return Some(result); + } + let worktrees = canonicalize_native(worktrees)?; + let mut registered: Vec<_> = fs::read_dir(&worktrees) + .ok()? + .filter_map(Result::ok) + .filter(|entry| entry.file_type().is_ok_and(|file_type| file_type.is_dir())) + .collect(); + registered.sort_by_key(std::fs::DirEntry::file_name); + + for entry in registered { + let Some(git_dir) = canonicalize_native(entry.path()) else { + continue; + }; + if git_dir.parent() != Some(worktrees.as_path()) { + continue; + } + let Some(git_file) = read_git_path(&git_dir.join("gitdir"), &git_dir, "") else { + continue; + }; + if git_file.file_name() != Some(OsStr::new(".git")) { + continue; + } + let Some(checkout_root) = git_file.parent() else { + continue; + }; + append_linked_cwd(&mut result, &mut seen, checkout_root, &identity); + } + + Some(result) +} + +fn append_linked_cwd( + result: &mut Vec, + seen: &mut HashSet, + checkout_root: &Path, + identity: &RepositoryIdentity, +) { + let Some(checkout_root) = canonicalize_native(checkout_root) else { + return; + }; + let Some(candidate) = canonicalize_native(checkout_root.join(&identity.relative_cwd)) else { + return; + }; + if !candidate.is_dir() || !candidate.starts_with(&checkout_root) { + return; + } + let Some(candidate_identity) = repository_identity(&candidate) else { + return; + }; + if candidate_identity.common_dir == identity.common_dir + && candidate_identity.relative_cwd == identity.relative_cwd + && seen.insert(candidate.clone()) + { + result.push(candidate); + } +} + +fn read_git_path(path: &Path, relative_to: &Path, prefix: &str) -> Option { + if !fs::symlink_metadata(path).ok()?.file_type().is_file() { + return None; + } + let contents = fs::read_to_string(path).ok()?; + let value = contents.trim().strip_prefix(prefix)?.trim(); + if value.is_empty() || value.contains('\n') || value.contains('\r') { + return None; + } + canonicalize_native(relative_to.join(value)) +} + +#[cfg(test)] +#[path = "worktree_tests.rs"] +mod tests; diff --git a/codex-rs/git-utils/src/worktree_tests.rs b/codex-rs/git-utils/src/worktree_tests.rs new file mode 100644 index 0000000000..b2b0ec4908 --- /dev/null +++ b/codex-rs/git-utils/src/worktree_tests.rs @@ -0,0 +1,97 @@ +//! Validates repository identity and discovery across linked checkout layouts. + +use super::*; +use pretty_assertions::assert_eq; + +fn repository_with_linked_checkout() -> (tempfile::TempDir, PathBuf, PathBuf) { + let root = tempfile::tempdir().expect("temporary repository"); + let primary = root.path().join("primary"); + let linked = root.path().join("linked"); + let admin = primary.join(".git").join("worktrees").join("linked"); + fs::create_dir_all(primary.join("nested")).expect("primary nested directory"); + fs::create_dir_all(linked.join("nested")).expect("linked nested directory"); + fs::create_dir_all(&admin).expect("worktree administrative directory"); + fs::write(primary.join(".git/HEAD"), "ref: refs/heads/main\n") + .expect("primary repository HEAD"); + fs::write(admin.join("commondir"), "../..\n").expect("common directory"); + fs::write( + admin.join("gitdir"), + format!("{}\n", linked.join(".git").display()), + ) + .expect("linked checkout backlink"); + fs::write( + linked.join(".git"), + format!("gitdir: {}\n", admin.display()), + ) + .expect("linked checkout git file"); + (root, primary, linked) +} + +#[test] +fn repository_identity_is_shared_by_primary_and_linked_checkouts() { + let (_root, primary, linked) = repository_with_linked_checkout(); + let primary_cwd = canonicalize_native(primary.join("nested")).expect("primary cwd"); + let linked_cwd = canonicalize_native(linked.join("nested")).expect("linked cwd"); + let expected = RepositoryIdentity { + common_dir: AbsolutePathBuf::from_absolute_path_checked( + canonicalize_native(primary.join(".git")).expect("common git directory"), + ) + .expect("absolute common directory"), + relative_cwd: PathBuf::from("nested"), + primary_root: AbsolutePathBuf::from_absolute_path_checked( + canonicalize_native(&primary).expect("primary checkout"), + ) + .expect("absolute primary checkout"), + }; + + assert_eq!(repository_identity(&primary_cwd), Some(expected.clone())); + assert_eq!(repository_identity(&linked_cwd), Some(expected)); + assert_eq!( + linked_worktree_cwds(&linked_cwd), + Some(vec![linked_cwd, primary_cwd]) + ); +} + +#[cfg(unix)] +#[test] +fn linked_worktree_discovery_preserves_logical_working_directory_aliases() { + let (root, primary, linked) = repository_with_linked_checkout(); + let alias = root.path().join("primary-alias"); + std::os::unix::fs::symlink(&primary, &alias).expect("checkout alias"); + let logical_cwd = alias.join("nested"); + let canonical_cwd = fs::canonicalize(primary.join("nested")).expect("primary cwd"); + let linked_cwd = fs::canonicalize(linked.join("nested")).expect("linked cwd"); + + assert_eq!( + linked_worktree_cwds(&logical_cwd), + Some(vec![logical_cwd, canonical_cwd, linked_cwd]) + ); +} + +#[test] +fn linked_worktree_discovery_rejects_mismatched_backlinks() { + let (_root, primary, linked) = repository_with_linked_checkout(); + let primary_cwd = canonicalize_native(primary.join("nested")).expect("primary cwd"); + let admin = primary.join(".git").join("worktrees").join("linked"); + fs::write( + admin.join("gitdir"), + primary.join(".git").display().to_string(), + ) + .expect("invalid worktree backlink"); + + assert_eq!(repository_identity(&linked.join("nested")), None); + assert_eq!(linked_worktree_cwds(&primary_cwd), Some(vec![primary_cwd])); +} + +#[cfg(unix)] +#[test] +fn linked_worktree_discovery_rejects_relative_directory_symlink_escapes() { + let (root, primary, linked) = repository_with_linked_checkout(); + let primary_cwd = fs::canonicalize(primary.join("nested")).expect("primary cwd"); + let outside = root.path().join("outside"); + fs::create_dir(&outside).expect("outside directory"); + fs::remove_dir(linked.join("nested")).expect("remove linked nested directory"); + std::os::unix::fs::symlink(&outside, linked.join("nested")).expect("escaping symlink"); + + assert_eq!(linked_worktree_cwds(&primary_cwd), Some(vec![primary_cwd])); +} diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 6a6dda6bc0..cfb45b2bfb 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -226,6 +226,7 @@ mod history_pagination; mod history_ui; mod input; mod loaded_threads; +mod managed_worktree_creation; mod misalignment_policy; mod model_defaults; mod new_session; @@ -634,6 +635,7 @@ pub(crate) struct App { tokio::sync::broadcast::Sender, dynamic_tool_tasks: HashMap)>, pending_startup_thread_start: bool, + pending_managed_worktree_creation: bool, /// Keeps protected screens quarantined until initialized chat receives genuine user input. startup_protected_input_boundary: bool, /// Keeps that boundary armed while a startup approval waits for the typing-idle timer. diff --git a/codex-rs/tui/src/app/agents_overview.rs b/codex-rs/tui/src/app/agents_overview.rs index d8167e3851..96bef76a94 100644 --- a/codex-rs/tui/src/app/agents_overview.rs +++ b/codex-rs/tui/src/app/agents_overview.rs @@ -281,6 +281,11 @@ impl App { rows, selected_thread_id, self.primary_thread_id.is_none(), + self.config.features.enabled(Feature::Worktrees) + && !crate::uses_remote_workspace_or_environment( + &self.app_server_target, + self.environment_manager.as_ref(), + ), self.app_event_tx.clone(), self.keymap.clone(), Arc::clone(&self.agents_overview.view_state), diff --git a/codex-rs/tui/src/app/agents_overview_tests.rs b/codex-rs/tui/src/app/agents_overview_tests.rs index 485a40fb7e..76a0e221ad 100644 --- a/codex-rs/tui/src/app/agents_overview_tests.rs +++ b/codex-rs/tui/src/app/agents_overview_tests.rs @@ -892,6 +892,72 @@ async fn agents_overview_reasoning_uses_existing_events_and_expires_with_attachm assert!(!app.agents_overview.activity.contains_key(&thread_id)); } +#[tokio::test] +async fn worktrees_overview_grouping_requires_feature() { + let mut app = make_test_app().await; + let root = tempfile::tempdir().unwrap(); + let primary = root.path().join("primary"); + let linked = root.path().join("linked"); + let linked_subdir = linked.join("subdir/child"); + let nested = primary.join("subdir"); + let nested_child = nested.join("child"); + let admin = primary.join(".git/worktrees/linked"); + std::fs::create_dir_all(&admin).unwrap(); + std::fs::create_dir_all(&linked_subdir).unwrap(); + std::fs::create_dir_all(nested.join(".git")).unwrap(); + std::fs::create_dir_all(&nested_child).unwrap(); + std::fs::write(primary.join(".git/HEAD"), "ref: refs/heads/main\n").unwrap(); + std::fs::write(nested.join(".git/HEAD"), "ref: refs/heads/main\n").unwrap(); + std::fs::write(admin.join("commondir"), "../..").unwrap(); + std::fs::write( + admin.join("gitdir"), + linked.join(".git").display().to_string(), + ) + .unwrap(); + std::fs::write(linked.join(".git"), format!("gitdir: {}", admin.display())).unwrap(); + let primary = dunce::canonicalize(primary).unwrap(); + let threads = [primary.clone(), linked, linked_subdir, nested_child] + .into_iter() + .map(|cwd| { + let mut thread = overview_thread( + ThreadId::new(), + /*parent_thread_id*/ None, + "Task", + ThreadStatus::Idle, + ); + thread.cwd = AbsolutePathBuf::from_absolute_path(cwd).unwrap(); + thread + }) + .collect::>(); + let grouped_heading = format!("{} 2", primary.join("").display()); + let width = 160; + for enabled in [false, true] { + app.config + .features + .set_enabled(Feature::Worktrees, enabled) + .unwrap(); + let view = app.agents_overview_view(threads.clone(), /*selected_thread_id*/ None); + app.chat_widget.show_bottom_pane_view(Box::new(view)); + let popup = render_bottom_popup(&app.chat_widget, width); + assert_eq!(popup.contains(&grouped_heading), enabled, "{popup}"); + if enabled { + let grouping = popup + .lines() + .map(|line| line.split('│').next().unwrap_or(line).trim_end()) + .collect::>() + .join("\n") + .replace( + &primary.join("").display().to_string(), + "/tmp/worktree-root/primary/", + ) + .replace('\\', "/"); + insta::with_settings!({snapshot_path => "../snapshots"}, { + insta::assert_snapshot!("agents_overview_worktree_grouping", grouping); + }); + } + } +} + #[tokio::test] async fn shared_overview_shows_only_root_sessions() { assert_eq!( @@ -949,6 +1015,7 @@ async fn shared_overview_shows_only_root_sessions() { view.rows.clone(), Some(first_root), /*exit_on_cancel*/ false, + /*worktrees_enabled*/ false, crate::app_event_sender::AppEventSender::new(event_tx), app.keymap.clone(), Arc::clone(&app.agents_overview.view_state), @@ -1190,6 +1257,7 @@ async fn filtered_dashboard_actions_use_configured_shortcuts() { .rows, Some(first), /*exit_on_cancel*/ false, + /*worktrees_enabled*/ false, crate::app_event_sender::AppEventSender::new(event_tx), app.keymap.clone(), Arc::clone(&app.agents_overview.view_state), diff --git a/codex-rs/tui/src/app/agents_overview_view.rs b/codex-rs/tui/src/app/agents_overview_view.rs index e955a51bc9..c2e4152023 100644 --- a/codex-rs/tui/src/app/agents_overview_view.rs +++ b/codex-rs/tui/src/app/agents_overview_view.rs @@ -41,6 +41,7 @@ use ratatui::text::Span; use ratatui::widgets::Clear; use ratatui::widgets::Paragraph; use ratatui::widgets::Widget; +use std::path::PathBuf; use std::sync::Arc; use std::sync::Mutex; use std::sync::MutexGuard; @@ -96,6 +97,33 @@ fn display_title(thread: &Thread) -> &str { title.trim().lines().next().unwrap_or("Untitled task") } +#[derive(Clone, Debug, Eq, PartialEq)] +struct AgentsOverviewProjectGroup { + key: (PathBuf, PathBuf), + heading: PathBuf, +} + +impl AgentsOverviewProjectGroup { + fn for_thread(thread: &Thread, worktrees_enabled: bool) -> Self { + if worktrees_enabled + && let Some(identity) = codex_git_utils::repository_identity(thread.cwd.as_path()) + { + Self { + key: ( + identity.common_dir.into_path_buf(), + identity.relative_cwd.clone(), + ), + heading: identity.primary_root.as_path().join(identity.relative_cwd), + } + } else { + Self { + key: (thread.cwd.to_path_buf(), PathBuf::new()), + heading: thread.cwd.to_path_buf(), + } + } + } +} + #[derive(Default)] pub(super) struct AgentsOverviewViewState { // Search and rename never borrow the new-task draft. @@ -148,6 +176,7 @@ impl AgentsOverviewViewState { pub(super) struct AgentsOverviewView { pub(super) rows: Vec, + project_groups: Vec, selected: usize, state: Arc>, exit_on_cancel: bool, @@ -163,6 +192,7 @@ impl AgentsOverviewView { rows: Vec, selected_thread_id: Option, exit_on_cancel: bool, + worktrees_enabled: bool, app_event_tx: AppEventSender, keymap: RuntimeKeymap, state: Arc>, @@ -183,8 +213,13 @@ impl AgentsOverviewView { }) .chain([("esc".to_string(), "tasks".to_string())]) .collect(); + let project_groups = rows + .iter() + .map(|row| AgentsOverviewProjectGroup::for_thread(&row.thread, worktrees_enabled)) + .collect(); let mut view = Self { rows, + project_groups, selected, state, exit_on_cancel, @@ -237,7 +272,7 @@ impl AgentsOverviewView { if !state.status_grouping { visible.sort_by_key(|index| { ( - &self.rows[*index].thread.cwd, + &self.project_groups[*index].key, std::cmp::Reverse(self.rows[*index].thread.updated_at), ) }); @@ -320,7 +355,7 @@ impl AgentsOverviewView { fn render_rows(&self, area: Rect, buf: &mut Buffer) { let mut offset = 0; - let mut previous_group: Option = None; + let mut previous_group_index: Option = None; let project_grouping = !self .state .lock() @@ -333,10 +368,12 @@ impl AgentsOverviewView { .unwrap_or_default(); let mut height = 2; while first > 0 { - let previous = &self.rows[visible[first - 1]]; - let current = &self.rows[visible[first]]; + let previous_index = visible[first - 1]; + let current_index = visible[first]; + let previous = &self.rows[previous_index]; + let current = &self.rows[current_index]; let group_changed = if project_grouping { - previous.thread.cwd != current.thread.cwd + self.project_groups[previous_index].key != self.project_groups[current_index].key } else { previous.group != current.group }; @@ -353,21 +390,30 @@ impl AgentsOverviewView { } let row = &self.rows[index]; let group = if project_grouping { - row.thread.cwd.display().to_string() + self.project_groups[index].heading.display().to_string() } else { row.group.label().to_string() }; - if previous_group.as_deref() != Some(group.as_str()) { - offset += u16::from(previous_group.is_some()); + let group_changed = previous_group_index.is_none_or(|previous_index| { + if project_grouping { + self.project_groups[previous_index].key != self.project_groups[index].key + } else { + self.rows[previous_index].group != row.group + } + }); + if group_changed { + offset += u16::from(previous_group_index.is_some()); if offset >= area.height { break; } let count = self .rows .iter() - .filter(|candidate| { + .enumerate() + .filter(|(candidate_index, candidate)| { if project_grouping { - candidate.thread.cwd == row.thread.cwd + self.project_groups[*candidate_index].key + == self.project_groups[index].key } else { candidate.group == row.group } @@ -376,7 +422,7 @@ impl AgentsOverviewView { Line::from(vec![group.clone().bold(), format!(" {count}").dim()]) .render(Rect::new(area.x, area.y + offset, area.width, 1), buf); offset += 1; - previous_group = Some(group); + previous_group_index = Some(index); } if offset >= area.height { break; diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 88b2316662..8ceaa3957c 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -34,6 +34,7 @@ impl App { && !matches!( &event, AppEvent::InsertHistoryCell(_) + | AppEvent::ManagedWorktreeCreated(_) | AppEvent::AppendMessageHistoryEntry { .. } | AppEvent::BeginInitialHistoryReplayBuffer | AppEvent::BeginThreadSwitchHistoryReplayBuffer @@ -91,7 +92,10 @@ impl App { } } AppEvent::StartManagedWorktree { mode, name } => { - self.start_managed_worktree(tui, app_server, mode, name).await; + self.start_managed_worktree(app_server, mode, name).await; + } + AppEvent::ManagedWorktreeCreated(created) => { + self.finish_managed_worktree(tui, app_server, *created).await; } AppEvent::ChangeWorkingDirectory { thread_id, diff --git a/codex-rs/tui/src/app/managed_worktree_creation.rs b/codex-rs/tui/src/app/managed_worktree_creation.rs new file mode 100644 index 0000000000..e245db7df6 --- /dev/null +++ b/codex-rs/tui/src/app/managed_worktree_creation.rs @@ -0,0 +1,258 @@ +//! Managed checkout creation and handoff from blocking Git work to the TUI event loop. +//! +//! Source-only blockers are checked before allocation. The completion event rechecks the +//! original session and reports a recovery path for every retained checkout. + +use super::*; +use crate::app_event::ManagedWorktreeCreated; +use crate::history_cell::McpInventoryLoadingCell as LoadingCell; +use codex_app_server_protocol::ThreadBackgroundTerminalsListParams; +use codex_app_server_protocol::ThreadBackgroundTerminalsListResponse as ListResponse; + +impl App { + pub(super) async fn start_managed_worktree( + &mut self, + app_server: &mut AppServerSession, + mode: crate::app_event::ManagedWorktreeMode, + name: Option, + ) { + if !self.config.features.enabled(Feature::Worktrees) { + self.chat_widget.add_error_message( + "Enable worktrees in /experimental to create a worktree.".to_string(), + ); + } else if self.config.active_project.is_untrusted() { + self.chat_widget.add_error_message( + "Cannot create a worktree from an explicitly untrusted source.".to_string(), + ); + } else if crate::uses_remote_workspace_or_environment( + &self.app_server_target, + self.environment_manager.as_ref(), + ) { + self.chat_widget.add_error_message( + "Managed worktrees are only supported for local sessions.".to_string(), + ); + } else if self + .primary_thread_id + .is_none_or(|thread_id| !self.chat_widget.can_change_working_directory(thread_id)) + { + self.chat_widget.add_error_message( + "Creating a worktree requires an idle primary session without queued input." + .to_string(), + ); + } else if self.pending_managed_worktree_creation { + self.chat_widget + .add_error_message("A worktree is already being created.".to_string()); + } else { + // These source-only checks must precede allocation. The transition repeats them + // after the background task, since the session can change while Git is running. + if self + .transcript_cells + .iter() + .any(|cell| cell.as_any().is::()) + { + return self.working_directory_error("MCP inventory is still loading."); + } + let Some(thread_id) = self.primary_thread_id else { + return self.working_directory_error( + "Creating a worktree requires an idle primary session without queued input.", + ); + }; + let agents = self.agent_navigation.ordered_threads(); + let closed_agents: HashSet<_> = agents + .iter() + .filter_map(|(id, agent)| agent.is_closed.then_some(*id)) + .collect(); + let active = self.thread_event_channels.iter().any(|(id, channel)| { + *id != thread_id + && !closed_agents.contains(id) + && !channel + .store + .try_lock() + .is_ok_and(|store| store.active_turn_id().is_none()) + }); + if active + || agents + .iter() + .any(|(id, agent)| *id != thread_id && agent.is_running) + { + return self.working_directory_error("Cannot change: another agent is running."); + } + let rollout = self.chat_widget.rollout_path(); + let has_rollout = rollout.as_deref().is_some_and(rollout_path_is_resumable); + if mode == crate::app_event::ManagedWorktreeMode::Fork + && !has_rollout + && (!self.chat_widget.token_usage().is_zero() + || self + .thread_event_channels + .get(&thread_id) + .is_some_and(|channel| { + channel.store.try_lock().map_or(/*default*/ true, |store| { + !store.turns.is_empty() || store.buffer.iter().any(|event| { + matches!(event, ThreadBufferedEvent::Notification(notification) + if matches!(notification.as_ref(), + ServerNotification::TurnStarted(_) + | ServerNotification::TurnCompleted(_))) + }) + }) + })) + { + return self.working_directory_error("Conversation history is not saved."); + } + let mut ids: HashSet<_> = self + .thread_event_channels + .keys() + .copied() + .filter(|id| !closed_agents.contains(id)) + .collect(); + ids.extend( + agents + .iter() + .filter_map(|(id, agent)| (!agent.is_closed).then_some(*id)), + ); + for tracked_id in ids { + let request = ClientRequest::ThreadBackgroundTerminalsList { + request_id: app_server.next_request_id(), + params: ThreadBackgroundTerminalsListParams { + thread_id: tracked_id.to_string(), + cursor: None, + limit: Some(1), + }, + }; + let result = app_server + .request_handle() + .request_typed::(request) + .await; + if !matches!(result, Ok(response) if response.data.is_empty()) { + return self.working_directory_error("Active background terminals block /cd."); + } + } + let setup = async { + let source = self + .rebuild_config_for_cwd(self.config.cwd.to_path_buf()) + .await + .map_err(|error| anyhow::anyhow!(error.to_string()))?; + anyhow::ensure!( + !source.active_project.is_untrusted(), + "Cannot create a worktree from an explicitly untrusted source." + ); + let host = crate::legacy_core::config::load_config_toml_with_layer_stack( + &self.config.codex_home, + /*cwd*/ None, + Vec::new(), + codex_config::ConfigLoadOptions::default(), + ) + .await?; + let settings = codex_worktree::WorktreeSettings::for_cli( + &self.config.codex_home, + host.config_toml.desktop.as_ref(), + )?; + let manager = codex_worktree::WorktreeManager::new(settings); + anyhow::Ok(manager) + } + .await; + match setup { + Ok(manager) => { + self.pending_managed_worktree_creation = true; + let source_cwd = self.config.cwd.clone(); + let sender = self.app_event_tx.clone(); + tokio::spawn(async move { + let create_cwd = source_cwd.to_path_buf(); + let result = tokio::task::spawn_blocking(move || { + manager + .create(&codex_worktree::CreateWorktree { + source_cwd: create_cwd, + base: None, + }) + .map(|checkout| (manager, checkout)) + .map_err(|error| error.to_string()) + }) + .await + .unwrap_or_else(|error| { + Err(format!("Worktree creation task failed: {error}")) + }); + sender.send(AppEvent::ManagedWorktreeCreated(Box::new( + ManagedWorktreeCreated { + source_thread_id: thread_id, + source_cwd, + mode, + name, + result, + }, + ))); + }); + } + Err(error) => self.chat_widget.add_error_message(error.to_string()), + } + } + } + + pub(super) async fn finish_managed_worktree( + &mut self, + tui: &mut tui::Tui, + app_server: &mut AppServerSession, + created: ManagedWorktreeCreated, + ) { + self.pending_managed_worktree_creation = false; + let (manager, checkout) = match created.result { + Ok(created) => created, + Err(error) => return self.working_directory_error(error), + }; + let can_continue = !self.reconnect.offline + && !self.chat_widget.has_misalignment_policy_violation() + && self.config.features.enabled(Feature::Worktrees) + && !crate::uses_remote_workspace_or_environment( + &self.app_server_target, + self.environment_manager.as_ref(), + ) + && self.primary_thread_id == Some(created.source_thread_id) + && !crate::session_resume::cwds_differ( + self.config.cwd.as_path(), + created.source_cwd.as_path(), + ) + && self + .chat_widget + .can_change_working_directory(created.source_thread_id); + if !can_continue { + return self.retained_worktree_error( + &checkout, + "Cannot continue into the new worktree while the source session changed or is unavailable.", + ); + } + match self + .rebuild_config_for_cwd(created.source_cwd.to_path_buf()) + .await + { + Ok(config) if !config.active_project.is_untrusted() => {} + Ok(_) | Err(_) => { + return self.retained_worktree_error( + &checkout, + "Cannot continue into the new worktree because source configuration changed.", + ); + } + } + if let Err(error) = self + .switch_to_managed_worktree( + tui, + app_server, + manager, + checkout.clone(), + created.mode, + created.name, + ) + .await + { + self.retained_worktree_error(&checkout, error.to_string()); + } + } + + fn retained_worktree_error( + &mut self, + checkout: &codex_worktree::ManagedWorktree, + reason: impl std::fmt::Display, + ) { + self.working_directory_error(format!( + "{reason} A checkout was retained at {}; remove it with `git worktree remove ` from the source repository if it is no longer needed.", + checkout.root.display() + )); + } +} diff --git a/codex-rs/tui/src/app/session_picker.rs b/codex-rs/tui/src/app/session_picker.rs index a1227f31cf..b671886628 100644 --- a/codex-rs/tui/src/app/session_picker.rs +++ b/codex-rs/tui/src/app/session_picker.rs @@ -9,14 +9,27 @@ impl App { tui: &mut tui::Tui, app_server: &mut AppServerSession, ) -> Result { - let picker_app_server = match crate::start_app_server_for_picker( - &self.config, - &self.app_server_target, - self.state_db.clone(), - self.environment_manager.clone(), - ) - .await - { + // This runs inside the active TUI event future. Starting a second embedded server + // inline can exhaust that thread's stack while initializing its auth manager. + let picker_config = self.config.clone(); + let picker_target = self.app_server_target.clone(); + let picker_state_db = self.state_db.clone(); + let picker_environment_manager = Arc::clone(&self.environment_manager); + let picker_app_server = tokio::spawn(async move { + crate::start_app_server_for_picker( + &picker_config, + &picker_target, + picker_state_db, + picker_environment_manager, + ) + .await + }) + .await; + let picker_app_server = match picker_app_server { + Ok(result) => result, + Err(err) => Err(err.into()), + }; + let picker_app_server = match picker_app_server { Ok(app_server) => app_server, Err(err) => { self.add_session_picker_error(format!("Failed to start TUI session picker: {err}")); @@ -26,6 +39,10 @@ impl App { }; let selection = crate::resume_picker::run_resume_picker_from_existing_session_with_app_server( + crate::uses_remote_workspace_or_environment( + &self.app_server_target, + &self.environment_manager, + ), tui, &self.config, &self.local_settings, diff --git a/codex-rs/tui/src/app/startup.rs b/codex-rs/tui/src/app/startup.rs index f0a0d34f81..b2edd17b07 100644 --- a/codex-rs/tui/src/app/startup.rs +++ b/codex-rs/tui/src/app/startup.rs @@ -690,6 +690,7 @@ See the Codex keymap documentation for supported actions and examples." dynamic_tool_status_updates, dynamic_tool_tasks: HashMap::new(), pending_startup_thread_start, + pending_managed_worktree_creation: false, startup_protected_input_boundary: true, startup_pending_protected_request: false, rate_limit_hard_stop_generation: 0, diff --git a/codex-rs/tui/src/app/test_support.rs b/codex-rs/tui/src/app/test_support.rs index 4b338b9faa..cc6f407c5a 100644 --- a/codex-rs/tui/src/app/test_support.rs +++ b/codex-rs/tui/src/app/test_support.rs @@ -80,6 +80,7 @@ pub(super) async fn make_test_app() -> App { dynamic_tool_status_updates: tokio::sync::broadcast::channel(/*capacity*/ 64).0, dynamic_tool_tasks: HashMap::new(), pending_startup_thread_start: false, + pending_managed_worktree_creation: false, startup_protected_input_boundary: false, startup_pending_protected_request: false, rate_limit_hard_stop_generation: 0, diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 045369ce10..a66ca91633 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -5539,6 +5539,7 @@ async fn make_test_app() -> App { dynamic_tool_status_updates: tokio::sync::broadcast::channel(/*capacity*/ 64).0, dynamic_tool_tasks: HashMap::new(), pending_startup_thread_start: false, + pending_managed_worktree_creation: false, startup_protected_input_boundary: false, startup_pending_protected_request: false, rate_limit_hard_stop_generation: 0, @@ -5625,6 +5626,7 @@ pub(super) async fn make_test_app_with_channels() -> ( dynamic_tool_status_updates: tokio::sync::broadcast::channel(/*capacity*/ 64).0, dynamic_tool_tasks: HashMap::new(), pending_startup_thread_start: false, + pending_managed_worktree_creation: false, startup_protected_input_boundary: false, startup_pending_protected_request: false, rate_limit_hard_stop_generation: 0, diff --git a/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs b/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs index e8ff9b153e..e9ebf9a357 100644 --- a/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs +++ b/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs @@ -41,6 +41,23 @@ use tokio_tungstenite::tungstenite::Message; pub(super) type RecordedRequests = Arc>>; pub(super) type RecordingAppServer = (AppServerSession, RecordedRequests, JoinHandle>); +async fn complete_managed_worktree_creation( + app: &mut App, + tui: &mut crate::tui::Tui, + server: &mut AppServerSession, + events: &mut tokio::sync::mpsc::UnboundedReceiver, +) -> Result<()> { + loop { + let event = tokio::time::timeout(Duration::from_secs(15), events.recv()) + .await? + .ok_or_else(|| color_eyre::eyre::eyre!("worktree creation event channel closed"))?; + if matches!(event, AppEvent::ManagedWorktreeCreated(_)) { + app.handle_event(tui, server, event).await?; + return Ok(()); + } + } +} + #[tokio::test] async fn same_thread_retry_keeps_subscription_and_restores_draft() -> Result<()> { let (mut app, codex_home) = make_history_test_app().await?; @@ -2947,6 +2964,7 @@ model_reasoning_effort = "low" }, ) .await?; + complete_managed_worktree_creation(&mut app, &mut tui, &mut server, &mut events).await?; assert_eq!(app.chat_widget.thread_id(), Some(original)); assert!(recorded_params(&requests, "thread/fork").is_empty()); let unused = manager @@ -2976,6 +2994,26 @@ model_reasoning_effort = "low" .split_once("; remove it with") .expect("cleanup marker"); insta::assert_snapshot!(error.replace(path, ""), @"■ Cannot fork into this worktree because developer instructions differ. Start a new conversation instead. An unused checkout was created at ; remove it with `git worktree remove ` from the source repository."); + let retained = std::iter::from_fn(|| events.try_recv().ok()) + .filter_map(|event| match event { + AppEvent::InsertHistoryCell(cell) => { + Some(lines_to_single_string(&cell.display_lines(/*width*/ 500))) + } + _ => None, + }) + .find(|message| message.contains("A checkout was retained at")) + .expect("post-allocation recovery message"); + let (_, with_path) = retained + .split_once("A checkout was retained at ") + .expect("retained checkout marker"); + let (path, _) = with_path + .split_once("; remove it with") + .expect("retained checkout cleanup marker"); + assert_eq!( + dunce::canonicalize(path)?, + dunce::canonicalize(&unused.root)? + ); + assert!(retained.contains("git worktree remove ")); fs::write( source.join(".codex/config.toml"), r#"developer_instructions = "committed policy" @@ -2994,8 +3032,63 @@ terminal_visualization_instructions = true }, ) .await?; + complete_managed_worktree_creation(&mut app, &mut tui, &mut server, &mut events).await?; assert_eq!(app.chat_widget.thread_id(), Some(original)); assert!(recorded_params(&requests, "thread/fork").is_empty()); + while events.try_recv().is_ok() {} + app.handle_event( + &mut tui, + &mut server, + AppEvent::StartManagedWorktree { + mode: ManagedWorktreeMode::New, + name: None, + }, + ) + .await?; + assert_eq!(app.chat_widget.thread_id(), Some(original)); + assert!(app.pending_managed_worktree_creation); + let created = loop { + let event = tokio::time::timeout(Duration::from_secs(15), events.recv()) + .await? + .ok_or_else(|| color_eyre::eyre::eyre!("worktree completion channel closed"))?; + if let AppEvent::ManagedWorktreeCreated(created) = event { + break created; + } + }; + let stale_checkout = created + .result + .as_ref() + .expect("created checkout") + .1 + .root + .clone(); + app.primary_thread_id = Some(ThreadId::new()); + app.handle_event( + &mut tui, + &mut server, + AppEvent::ManagedWorktreeCreated(created), + ) + .await?; + app.primary_thread_id = Some(original); + assert_eq!(app.chat_widget.thread_id(), Some(original)); + assert!(!app.pending_managed_worktree_creation); + let stale_error = std::iter::from_fn(|| events.try_recv().ok()) + .filter_map(|event| match event { + AppEvent::InsertHistoryCell(cell) => { + Some(lines_to_single_string(&cell.display_lines(/*width*/ 500))) + } + _ => None, + }) + .find(|message| message.contains("A checkout was retained at")) + .expect("stale creation recovery message"); + assert!(stale_error.contains(&stale_checkout.display().to_string())); + assert!(stale_error.contains("git worktree remove ")); + assert_eq!( + manager + .owner(&stale_checkout) + .map_err(|error| color_eyre::eyre::eyre!(error.to_string()))?, + None + ); for mode in [ManagedWorktreeMode::New, ManagedWorktreeMode::Fork] { requests.lock().expect("request recorder lock").clear(); let previous = app.chat_widget.thread_id(); @@ -3009,6 +3102,7 @@ terminal_visualization_instructions = true }, ) .await?; + complete_managed_worktree_creation(&mut app, &mut tui, &mut server, &mut events).await?; let replacement = app.chat_widget.thread_id().expect("replacement thread"); assert_ne!(Some(replacement), previous); assert_eq!( @@ -3105,6 +3199,10 @@ terminal_visualization_instructions = true ) .await?; let is_new = mode == ManagedWorktreeMode::New; + if is_new { + complete_managed_worktree_creation(&mut app, &mut tui, &mut server, &mut events) + .await?; + } assert_eq!(app.chat_widget.thread_id() != Some(unsaved), is_new); assert_eq!(recorded_params(&requests, "thread/fork").len(), 0); } diff --git a/codex-rs/tui/src/app/tests/turn_submission.rs b/codex-rs/tui/src/app/tests/turn_submission.rs index 606ccaeb62..5bdec41374 100644 --- a/codex-rs/tui/src/app/tests/turn_submission.rs +++ b/codex-rs/tui/src/app/tests/turn_submission.rs @@ -89,6 +89,51 @@ async fn worktree_creation_rejects_untrusted_source_before_allocation() -> Resul Ok(()) } +#[tokio::test] +async fn worktree_creation_rejects_running_agent_before_allocation() -> Result<()> { + let (mut app, mut events, _op_rx) = make_test_app_with_channels().await; + let home = tempfile::tempdir()?; + app.config.codex_home = home.path().to_path_buf().abs(); + app.config.features.enable(Feature::Worktrees)?; + app.config.active_project.trust_level = Some(codex_protocol::config_types::TrustLevel::Trusted); + let primary = ThreadId::new(); + app.primary_thread_id = Some(primary); + app.chat_widget + .handle_thread_session_quiet(test_thread_session(primary, app.config.cwd.to_path_buf())); + let child = ThreadId::new(); + app.agent_navigation.upsert( + child, /*agent_nickname*/ None, /*agent_role*/ None, /*is_closed*/ false, + ); + app.agent_navigation.set_running(child, /*is_running*/ true); + let mut tui = crate::tui::test_support::make_test_tui()?; + let mut server = Box::pin(crate::start_embedded_app_server_for_picker(&app.config)).await?; + while events.try_recv().is_ok() {} + + app.handle_event( + &mut tui, + &mut server, + AppEvent::StartManagedWorktree { + mode: crate::app_event::ManagedWorktreeMode::New, + name: None, + }, + ) + .await?; + + assert!(!app.pending_managed_worktree_creation); + assert!(!home.path().join("worktrees").exists()); + let message = std::iter::from_fn(|| events.try_recv().ok()) + .find_map(|event| match event { + AppEvent::InsertHistoryCell(cell) => { + Some(lines_to_single_string(&cell.display_lines(/*width*/ 100))) + } + _ => None, + }) + .expect("running agent message"); + insta::assert_snapshot!(message, @"■ Cannot change: another agent is running."); + server.shutdown().await?; + Ok(()) +} + #[tokio::test] async fn turn_start_failure_is_shown_without_exiting() -> Result<()> { let (mut app, mut app_event_rx, mut op_rx) = make_test_app_with_channels().await; diff --git a/codex-rs/tui/src/app/working_directory.rs b/codex-rs/tui/src/app/working_directory.rs index 481b2867ce..b4bd7fc00e 100644 --- a/codex-rs/tui/src/app/working_directory.rs +++ b/codex-rs/tui/src/app/working_directory.rs @@ -1,4 +1,4 @@ -//! Local directory transitions and managed worktrees with fresh or preserved conversation history. +//! Local directory transitions with fresh or preserved conversation history. use super::session_lifecycle::ThreadAttachPresentation; use super::*; @@ -9,80 +9,7 @@ use codex_app_server_protocol::ThreadBackgroundTerminalsListParams; use codex_app_server_protocol::ThreadBackgroundTerminalsListResponse as ListResponse; impl App { - pub(super) async fn start_managed_worktree( - &mut self, - tui: &mut tui::Tui, - app_server: &mut AppServerSession, - mode: crate::app_event::ManagedWorktreeMode, - name: Option, - ) { - if !self.config.features.enabled(Feature::Worktrees) { - self.chat_widget.add_error_message( - "Enable worktrees in /experimental to create a worktree.".to_string(), - ); - } else if self.config.active_project.is_untrusted() { - self.chat_widget.add_error_message( - "Cannot create a worktree from an explicitly untrusted source.".to_string(), - ); - } else if crate::uses_remote_workspace_or_environment( - &self.app_server_target, - self.environment_manager.as_ref(), - ) { - self.chat_widget.add_error_message( - "Managed worktrees are only supported for local sessions.".to_string(), - ); - } else if self - .primary_thread_id - .is_none_or(|thread_id| !self.chat_widget.can_change_working_directory(thread_id)) - { - self.chat_widget.add_error_message( - "Creating a worktree requires an idle primary session without queued input." - .to_string(), - ); - } else { - let setup = async { - let source = self - .rebuild_config_for_cwd(self.config.cwd.to_path_buf()) - .await - .map_err(|error| anyhow::anyhow!(error.to_string()))?; - anyhow::ensure!( - !source.active_project.is_untrusted(), - "Cannot create a worktree from an explicitly untrusted source." - ); - let host = crate::legacy_core::config::load_config_toml_with_layer_stack( - &self.config.codex_home, - /*cwd*/ None, - Vec::new(), - codex_config::ConfigLoadOptions::default(), - ) - .await?; - let settings = codex_worktree::WorktreeSettings::for_cli( - &self.config.codex_home, - host.config_toml.desktop.as_ref(), - )?; - let manager = codex_worktree::WorktreeManager::new(settings); - let checkout = manager.create(&codex_worktree::CreateWorktree { - source_cwd: self.config.cwd.to_path_buf(), - base: None, - })?; - anyhow::Ok((manager, checkout)) - } - .await; - match setup { - Ok((manager, checkout)) => { - if let Err(error) = self - .switch_to_managed_worktree(tui, app_server, manager, checkout, mode, name) - .await - { - self.chat_widget.add_error_message(error.to_string()); - } - } - Err(error) => self.chat_widget.add_error_message(error.to_string()), - } - } - } - - fn working_directory_error(&mut self, message: impl Into) { + pub(super) fn working_directory_error(&mut self, message: impl Into) { self.chat_widget.add_error_message(message.into()); } diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 0a7fb54d85..819b8ace8f 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -73,6 +73,22 @@ pub(crate) enum ManagedWorktreeMode { Fork, } +/// Checkout creation result returned from the blocking Git task to the TUI event loop. +#[derive(Debug)] +pub(crate) struct ManagedWorktreeCreated { + pub(crate) source_thread_id: ThreadId, + pub(crate) source_cwd: AbsolutePathBuf, + pub(crate) mode: ManagedWorktreeMode, + pub(crate) name: Option, + pub(crate) result: Result< + ( + codex_worktree::WorktreeManager, + codex_worktree::ManagedWorktree, + ), + String, + >, +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum ThreadGoalSetMode { ConfirmIfExists, @@ -400,6 +416,8 @@ pub(crate) enum AppEvent { mode: ManagedWorktreeMode, name: Option, }, + /// Continue a checkout transition after synchronous Git work finishes off-loop. + ManagedWorktreeCreated(Box), /// Change the working directory of the originating idle primary thread. ChangeWorkingDirectory { diff --git a/codex-rs/tui/src/app_server_session.rs b/codex-rs/tui/src/app_server_session.rs index e26acc7302..0c96354fdb 100644 --- a/codex-rs/tui/src/app_server_session.rs +++ b/codex-rs/tui/src/app_server_session.rs @@ -7,6 +7,7 @@ mod fs; mod history; mod models; mod rollout_history; +mod thread_list; #[cfg(test)] #[path = "app_server_session/collaboration_catalog_tests.rs"] @@ -1042,17 +1043,6 @@ impl AppServerSession { } } - pub(crate) async fn thread_list( - &mut self, - params: ThreadListParams, - ) -> Result { - let request_id = self.next_request_id(); - self.client - .request_typed(ClientRequest::ThreadList { request_id, params }) - .await - .wrap_err("thread/list failed during TUI session lookup") - } - /// Lists thread ids that the app server currently holds in memory. /// /// Used by `App::backfill_loaded_subagent_threads` to discover subagent threads that were diff --git a/codex-rs/tui/src/app_server_session/thread_list.rs b/codex-rs/tui/src/app_server_session/thread_list.rs new file mode 100644 index 0000000000..17a45f0280 --- /dev/null +++ b/codex-rs/tui/src/app_server_session/thread_list.rs @@ -0,0 +1,41 @@ +//! Session listing preserves grouped filters while tolerating older single-CWD daemons. + +use super::*; +use codex_app_server_protocol::ThreadListCwdFilter; + +impl AppServerSession { + pub(crate) async fn thread_list( + &mut self, + mut params: ThreadListParams, + ) -> Result { + loop { + let request_id = self.next_request_id(); + let response = self + .client + .request_typed(ClientRequest::ThreadList { + request_id, + params: params.clone(), + }) + .await; + if let Err(TypedRequestError::Server { source, .. }) = &response + && matches!( + source.code, + JSONRPC_INVALID_REQUEST | JSONRPC_INVALID_PARAMS + ) + && source.message.contains("invalid type: sequence") + && source.message.contains("expected a string") + && let Some(ThreadListCwdFilter::Many(cwds)) = ¶ms.cwd + && let Some(cwd) = cwds.first() + { + // Repository discovery puts the originally requested CWD first. + params.cwd = Some(ThreadListCwdFilter::One(cwd.clone())); + continue; + } + return response.wrap_err("thread/list failed during TUI session lookup"); + } + } +} + +#[cfg(test)] +#[path = "thread_list_tests.rs"] +mod tests; diff --git a/codex-rs/tui/src/app_server_session/thread_list_tests.rs b/codex-rs/tui/src/app_server_session/thread_list_tests.rs new file mode 100644 index 0000000000..7ce2c5509a --- /dev/null +++ b/codex-rs/tui/src/app_server_session/thread_list_tests.rs @@ -0,0 +1,77 @@ +//! Public-RPC compatibility keeps modern grouping and narrows only legacy array errors. + +use super::*; +use codex_app_server_protocol::JSONRPCMessage; +use futures::SinkExt; +use futures::StreamExt; +use pretty_assertions::assert_eq; +use serde_json::json; +use tokio::net::TcpListener; +use tokio_tungstenite::tungstenite::Message; + +#[tokio::test] +async fn thread_list_preserves_filters_and_retries_only_legacy_cwd_errors() -> Result<()> { + let legacy_message = "invalid type: sequence, expected a string"; + for (code, message, retry) in [ + (0, "", false), + (-32600, legacy_message, true), + (-32602, legacy_message, true), + (-32603, legacy_message, false), + (-32602, "invalid cursor", false), + ] { + let listener = TcpListener::bind("127.0.0.1:0").await?; + let endpoint = crate::resolve_remote_addr(&format!("ws://{}", listener.local_addr()?))?; + let server = tokio::spawn(async move { + let (stream, _) = listener.accept().await?; + let mut socket = tokio_tungstenite::accept_async(stream).await?; + let mut requests = Vec::new(); + while let Some(Ok(Message::Text(text))) = socket.next().await { + let JSONRPCMessage::Request(request) = serde_json::from_str(&text)? else { + continue; + }; + let mut reply = match request.method.as_str() { + "initialize" => json!({"result": {"userAgent": "thread-list-test/1.0"}}), + "thread/list" => { + requests.push(request.params.unwrap()); + if code != 0 && requests.len() == 1 { + json!({"error": {"code": code, "message": message}}) + } else { + json!({"result": {"data": [], "nextCursor": "next"}}) + } + } + method => panic!("unexpected request: {method}"), + }; + reply["id"] = json!(request.id); + socket.send(Message::Text(reply.to_string().into())).await?; + } + Ok::<_, color_eyre::Report>(requests) + }); + let params: ThreadListParams = serde_json::from_value(json!({ + "cwd": ["/requested/nested", "/linked/nested"], "cursor": "cursor", "limit": 3, + "sortKey": "created_at", "modelProviders": ["provider"], "archived": true, + "searchTerm": "needle" + }))?; + let mut expected = vec![serde_json::to_value(¶ms)?]; + if retry { + let mut fallback = expected[0].clone(); + fallback["cwd"] = json!("/requested/nested"); + expected.push(fallback); + } + let mut session = AppServerSession::new( + crate::connect_remote_app_server(endpoint).await?, + ThreadParamsMode::Embedded, + ); + let result = session.thread_list(params).await; + if code == 0 || retry { + assert_eq!( + serde_json::to_value(result?)?, + json!({"data": [], "nextCursor": "next", "backwardsCursor": null}) + ); + } else { + assert!(format!("{:#}", result.unwrap_err()).contains(message)); + } + session.shutdown().await?; + assert_eq!(server.await??, expected); + } + Ok(()) +} diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 09164fed9f..89a3746a13 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -38,6 +38,7 @@ use codex_app_server_protocol::AskForApproval; use codex_app_server_protocol::ConfigWarningNotification; use codex_app_server_protocol::GetAccountResponse; use codex_app_server_protocol::Thread as AppServerThread; +#[cfg(test)] use codex_app_server_protocol::ThreadListCwdFilter; use codex_app_server_protocol::ThreadListParams; use codex_app_server_protocol::ThreadSortKey as AppServerThreadSortKey; @@ -718,6 +719,7 @@ async fn lookup_session_target_with_app_server( } async fn lookup_latest_session_target_with_app_server( + uses_remote_filesystem: bool, app_server: &mut AppServerSession, config: &Config, cwd_filter: Option<&Path>, @@ -730,6 +732,7 @@ async fn lookup_latest_session_target_with_app_server( ] { let response = app_server .thread_list(latest_session_lookup_params( + uses_remote_filesystem, uses_remote_workspace, config, cwd_filter, @@ -757,6 +760,7 @@ enum LatestSessionLookupMode { } fn latest_session_lookup_params( + uses_remote_filesystem: bool, uses_remote_workspace: bool, config: &Config, cwd_filter: Option<&Path>, @@ -780,7 +784,13 @@ fn latest_session_lookup_params( project_id: None, parent_thread_id: None, ancestor_thread_id: None, - cwd: cwd_filter.map(|cwd| ThreadListCwdFilter::One(cwd.to_string_lossy().to_string())), + cwd: cwd_filter.map(|cwd| { + resume_picker::repository_cwd_filter( + cwd, + uses_remote_filesystem, + config.features.enabled(codex_features::Feature::Worktrees), + ) + }), use_state_db_only: match lookup_mode { LatestSessionLookupMode::StateDbOnly => true, LatestSessionLookupMode::ScanAndRepair => false, @@ -1366,6 +1376,10 @@ async fn run_ratatui_app( .run_until( &mut tui, lookup_latest_session_target_with_app_server( + uses_remote_workspace_or_environment( + &app_server_target, + &environment_manager, + ), startup_app_server, &config, filter_cwd, @@ -1393,6 +1407,7 @@ async fn run_ratatui_app( unreachable!("app server should be initialized for --fork picker"); }; match resume_picker::run_fork_picker_with_app_server( + uses_remote_workspace_or_environment(&app_server_target, &environment_manager), &mut tui, &config, &crate::local_settings::LocalSettings::from(&config), @@ -1461,6 +1476,7 @@ async fn run_ratatui_app( .run_until( &mut tui, lookup_latest_session_target_with_app_server( + uses_remote_workspace_or_environment(&app_server_target, &environment_manager), startup_app_server, &config, filter_cwd, @@ -1488,6 +1504,7 @@ async fn run_ratatui_app( unreachable!("app server should be initialized for --resume picker"); }; match resume_picker::run_resume_picker_with_app_server( + uses_remote_workspace_or_environment(&app_server_target, &environment_manager), &mut tui, &config, &crate::local_settings::LocalSettings::from(&config), @@ -2063,7 +2080,7 @@ fn should_show_bedrock_setup_wizard( mod daemon_startup_tests; #[cfg(test)] -mod tests { +pub(crate) mod tests { use super::*; use crate::legacy_core::config::ConfigBuilder; use crate::legacy_core::config::ConfigOverrides; @@ -2204,7 +2221,7 @@ requires_openai_auth = {requires_openai_auth} Ok(()) } - fn write_session_rollout( + pub(crate) fn write_session_rollout( codex_home: &Path, filename_ts: &str, meta_rfc3339: &str, @@ -2946,6 +2963,7 @@ requires_openai_auth = {requires_openai_auth} let cwd = temp_dir.path().join("project"); let params = latest_session_lookup_params( + /*uses_remote_filesystem*/ false, /*uses_remote_workspace*/ false, &config, Some(cwd.as_path()), @@ -2964,6 +2982,7 @@ requires_openai_auth = {requires_openai_auth} assert!(params.use_state_db_only); let scan_params = latest_session_lookup_params( + /*uses_remote_filesystem*/ false, /*uses_remote_workspace*/ false, &config, Some(cwd.as_path()), @@ -2987,6 +3006,7 @@ requires_openai_auth = {requires_openai_auth} }; let params = latest_session_lookup_params( + /*uses_remote_filesystem*/ false, target.uses_remote_workspace(), &config, Some(cwd.as_path()), @@ -3009,6 +3029,7 @@ requires_openai_auth = {requires_openai_auth} let config = build_config(&temp_dir).await?; let params = latest_session_lookup_params( + /*uses_remote_filesystem*/ true, /*uses_remote_workspace*/ true, &config, /*cwd_filter*/ None, @@ -3028,6 +3049,7 @@ requires_openai_auth = {requires_openai_auth} let config = build_config(&temp_dir).await?; let params = latest_session_lookup_params( + /*uses_remote_filesystem*/ true, /*uses_remote_workspace*/ true, &config, /*cwd_filter*/ None, @@ -3055,6 +3077,7 @@ requires_openai_auth = {requires_openai_auth} let cwd = Path::new("repo/on/server"); let params = latest_session_lookup_params( + /*uses_remote_filesystem*/ true, /*uses_remote_workspace*/ true, &config, Some(cwd), @@ -3101,11 +3124,24 @@ requires_openai_auth = {requires_openai_auth} async fn fork_last_filters_latest_session_by_cwd_unless_show_all() -> color_eyre::Result<()> { let temp_dir = TempDir::new()?; let project_cwd = temp_dir.path().join("project"); + let linked_cwd = temp_dir.path().join("linked-project"); let other_cwd = temp_dir.path().join("other-project"); - std::fs::create_dir_all(&project_cwd)?; + let admin = project_cwd.join(".git/worktrees/linked"); + std::fs::create_dir_all(&admin)?; + std::fs::create_dir_all(&linked_cwd)?; std::fs::create_dir_all(&other_cwd)?; + std::fs::write(project_cwd.join(".git/HEAD"), "ref: refs/heads/main\n")?; + std::fs::write(admin.join("commondir"), "../..\n")?; + std::fs::write( + admin.join("gitdir"), + linked_cwd.join(".git").display().to_string(), + )?; + std::fs::write( + linked_cwd.join(".git"), + format!("gitdir: {}", admin.display()), + )?; - let config = ConfigBuilder::default() + let mut config = ConfigBuilder::default() .codex_home(temp_dir.path().to_path_buf()) .harness_overrides(ConfigOverrides { cwd: Some(project_cwd.clone()), @@ -3122,6 +3158,14 @@ requires_openai_auth = {requires_openai_auth} model_provider, &project_cwd, )?; + let linked_thread_id = write_session_rollout( + temp_dir.path(), + "2025-01-02T11-00-00", + "2025-01-02T11:00:00Z", + "newer linked-worktree session", + model_provider, + &linked_cwd, + )?; let other_thread_id = write_session_rollout( temp_dir.path(), "2025-01-02T12-00-00", @@ -3141,7 +3185,24 @@ requires_openai_auth = {requires_openai_auth} /*uses_remote_workspace*/ false, /*remote_cwd_override*/ None, &config, /*show_all*/ false, ); + let disabled_target = lookup_latest_session_target_with_app_server( + /*uses_remote_filesystem*/ false, + &mut app_server, + &config, + filter_cwd, + /*include_non_interactive*/ false, + ) + .await? + .expect("expected current-checkout target with worktrees disabled"); + config + .features + .set_enabled(codex_features::Feature::Worktrees, /*enabled*/ true)?; + let filter_cwd = latest_session_cwd_filter( + /*uses_remote_workspace*/ false, /*remote_cwd_override*/ None, &config, + /*show_all*/ false, + ); let scoped_target = lookup_latest_session_target_with_app_server( + /*uses_remote_filesystem*/ false, &mut app_server, &config, filter_cwd, @@ -3154,6 +3215,7 @@ requires_openai_auth = {requires_openai_auth} /*show_all*/ true, ); let show_all_target = lookup_latest_session_target_with_app_server( + /*uses_remote_filesystem*/ false, &mut app_server, &config, show_all_filter_cwd, @@ -3163,7 +3225,8 @@ requires_openai_auth = {requires_openai_auth} .expect("expected global fork --last target"); app_server.shutdown().await?; - assert_eq!(scoped_target.thread_id, project_thread_id); + assert_eq!(disabled_target.thread_id, project_thread_id); + assert_eq!(scoped_target.thread_id, linked_thread_id); assert_eq!(show_all_target.thread_id, other_thread_id); Ok(()) } @@ -3200,6 +3263,7 @@ requires_openai_auth = {requires_openai_auth} )?; let target = lookup_latest_session_target_with_app_server( + /*uses_remote_filesystem*/ false, &mut app_server, &config, Some(project_cwd.as_path()), @@ -3340,6 +3404,10 @@ requires_openai_auth = {requires_openai_auth} )?; assert_eq!(config_cwd, None); + assert!(uses_remote_workspace_or_environment( + &target, + &environment_manager + )); let local_daemon = AppServerTarget::LocalDaemon { endpoint: RemoteAppServerEndpoint::UnixSocket { socket_path: AbsolutePathBuf::relative_to_current_dir("codex.sock")?, diff --git a/codex-rs/tui/src/resume_picker.rs b/codex-rs/tui/src/resume_picker.rs index 6ad1f8c8af..1ea7d49b9e 100644 --- a/codex-rs/tui/src/resume_picker.rs +++ b/codex-rs/tui/src/resume_picker.rs @@ -79,6 +79,7 @@ use uuid::Uuid; mod archive; mod page_loading; +use page_loading::PageCwdFilter; use page_loading::PageLoadMode; use page_loading::PaginationState; @@ -334,6 +335,7 @@ struct SessionPickerRunOptions { show_all: bool, filter_cwd: Option, local_filter_cwd: Option, + worktrees_enabled: bool, action: SessionPickerAction, launch_context: SessionPickerLaunchContext, provider_filter: ProviderFilter, @@ -362,6 +364,7 @@ struct SessionPickerRunOptions { /// 1. Provider, source, and eligible working-directory filtering at the backend. /// 2. Typed search filtering over loaded rows in the picker. pub async fn run_resume_picker_with_app_server( + uses_remote_filesystem: bool, tui: &mut Tui, config: &Config, local_settings: &crate::local_settings::LocalSettings, @@ -371,6 +374,7 @@ pub async fn run_resume_picker_with_app_server( ) -> Result { let archive_request_handle = app_server.request_handle(); run_resume_picker_with_launch_context( + uses_remote_filesystem, tui, config, local_settings, @@ -388,6 +392,7 @@ pub async fn run_resume_picker_with_app_server( reason = "keep local preferences separate while the legacy Config parameter is still required" )] pub async fn run_resume_picker_from_existing_session_with_app_server( + uses_remote_filesystem: bool, tui: &mut Tui, config: &Config, local_settings: &crate::local_settings::LocalSettings, @@ -398,6 +403,7 @@ pub async fn run_resume_picker_from_existing_session_with_app_server( current_thread_id: Option, ) -> Result { run_resume_picker_with_launch_context( + uses_remote_filesystem, tui, config, local_settings, @@ -415,6 +421,7 @@ pub async fn run_resume_picker_from_existing_session_with_app_server( reason = "keep local preferences separate while the legacy Config parameter is still required" )] async fn run_resume_picker_with_launch_context( + uses_remote_filesystem: bool, tui: &mut Tui, config: &Config, local_settings: &crate::local_settings::LocalSettings, @@ -432,13 +439,14 @@ async fn run_resume_picker_with_launch_context( uses_remote_workspace, app_server.remote_cwd_override(), ); - let local_filter_cwd = local_picker_cwd_filter(&cwd_filter, uses_remote_workspace); + let local_filter_cwd = local_picker_cwd_filter(&cwd_filter, uses_remote_filesystem); let provider_filter = picker_provider_filter(config, uses_remote_workspace); let runtime_keymap = picker_runtime_keymap(local_settings)?; let options = SessionPickerRunOptions { show_all, filter_cwd: cwd_filter, local_filter_cwd, + worktrees_enabled: config.features.enabled(codex_features::Feature::Worktrees), action: SessionPickerAction::Resume, launch_context, provider_filter, @@ -461,6 +469,7 @@ async fn run_resume_picker_with_launch_context( tui, options, spawn_app_server_page_loader( + uses_remote_filesystem, app_server, archive_request_handle, include_non_interactive, @@ -474,6 +483,7 @@ async fn run_resume_picker_with_launch_context( } pub async fn run_fork_picker_with_app_server( + uses_remote_filesystem: bool, tui: &mut Tui, config: &Config, local_settings: &crate::local_settings::LocalSettings, @@ -489,13 +499,14 @@ pub async fn run_fork_picker_with_app_server( uses_remote_workspace, app_server.remote_cwd_override(), ); - let local_filter_cwd = local_picker_cwd_filter(&cwd_filter, uses_remote_workspace); + let local_filter_cwd = local_picker_cwd_filter(&cwd_filter, uses_remote_filesystem); let provider_filter = picker_provider_filter(config, uses_remote_workspace); let runtime_keymap = picker_runtime_keymap(local_settings)?; let options = SessionPickerRunOptions { show_all, filter_cwd: cwd_filter, local_filter_cwd, + worktrees_enabled: config.features.enabled(codex_features::Feature::Worktrees), action: SessionPickerAction::Fork, launch_context: SessionPickerLaunchContext::Startup, provider_filter, @@ -518,6 +529,7 @@ pub async fn run_fork_picker_with_app_server( tui, options, spawn_app_server_page_loader( + uses_remote_filesystem, app_server, archive_request_handle, /*include_non_interactive*/ false, @@ -546,6 +558,7 @@ async fn run_session_picker_with_loader( options.action, ); state.local_filter_cwd = options.local_filter_cwd; + state.worktrees_enabled = options.worktrees_enabled; state.density = options.initial_density; state.view_persistence = options.view_persistence; state.pager_keymap = options.pager_keymap; @@ -633,9 +646,9 @@ fn raw_reasoning_visibility(config: &Config) -> RawReasoningVisibility { fn local_picker_cwd_filter( cwd_filter: &Option, - uses_remote_workspace: bool, + uses_remote_filesystem: bool, ) -> Option { - if uses_remote_workspace { + if uses_remote_filesystem { None } else { cwd_filter.clone() @@ -671,6 +684,7 @@ fn picker_cwd_filter( } fn spawn_app_server_page_loader( + uses_remote_filesystem: bool, app_server: AppServerSession, archive_request_handle: AppServerRequestHandle, include_non_interactive: bool, @@ -682,13 +696,22 @@ fn spawn_app_server_page_loader( tokio::spawn(async move { let mut app_server = app_server; + let mut page_cwd_filter = PageCwdFilter::default(); while let Some(request) = request_rx.recv().await { match request { PickerLoadRequest::Page(request) => { + let cwd_filter = page_cwd_filter.for_request( + request.cursor.as_ref(), + request.cwd_filter.as_deref(), + uses_remote_filesystem, + config.as_ref().is_some_and(|config| { + config.features.enabled(codex_features::Feature::Worktrees) + }), + ); let cursor = request.cursor.map(|PageCursor::AppServer(cursor)| cursor); let params = thread_list_params( cursor, - request.cwd_filter.as_deref(), + cwd_filter, request.status, request.provider_filter, request.sort_key, @@ -803,6 +826,8 @@ impl Drop for AltScreenGuard<'_> { } struct PickerState { + // Resolve local filesystem membership once per cwd for each page-loading cycle. + local_cwd_matches: HashMap, requester: FrameRequester, relative_time_reference: Option>, pagination: PaginationState, @@ -826,6 +851,7 @@ struct PickerState { status: SessionStatus, filter_cwd: Option, local_filter_cwd: Option, + worktrees_enabled: bool, toolbar_focus: ToolbarControl, density: SessionListDensity, launch_context: SessionPickerLaunchContext, @@ -927,6 +953,7 @@ impl SearchState { } #[derive(Clone)] +#[cfg_attr(test, derive(Debug, PartialEq))] struct Row { path: Option, preview: String, @@ -1021,6 +1048,8 @@ impl PickerState { filter_mode: SessionFilterMode::from_show_all(show_all, filter_cwd.as_deref()), status: SessionStatus::Active, local_filter_cwd: filter_cwd.clone(), + local_cwd_matches: HashMap::new(), + worktrees_enabled: false, filter_cwd, toolbar_focus: ToolbarControl::Filter, density: SessionListDensity::Comfortable, @@ -1407,6 +1436,7 @@ impl PickerState { self.filtered_rows.clear(); self.thread_history_modes.clear(); self.seen_rows.clear(); + self.local_cwd_matches.clear(); self.selected = 0; self.pending_page_down_target = None; self.frozen_footer_percent = None; @@ -1548,6 +1578,12 @@ impl PickerState { self.thread_history_modes.extend(history_modes); for row in rows { + if let Some(cwd) = row.cwd.as_ref() + && !self.local_cwd_matches.contains_key(cwd) + { + let matches = self.row_matches_local_cwd(&row); + self.local_cwd_matches.insert(cwd.clone(), matches); + } if let Some(seen_key) = row.seen_key() { if self.seen_rows.insert(seen_key) { self.all_rows.push(row); @@ -1603,9 +1639,17 @@ impl PickerState { } fn row_matches_filter(&self, row: &Row) -> bool { - if self.filter_mode == SessionFilterMode::All { - return true; - } + self.filter_mode == SessionFilterMode::All + || self.local_filter_cwd.is_none() + || row + .cwd + .as_ref() + .and_then(|cwd| self.local_cwd_matches.get(cwd)) + .copied() + .unwrap_or(false) + } + + fn row_matches_local_cwd(&self, row: &Row) -> bool { let Some(filter_cwd) = self.local_filter_cwd.as_ref() else { return true; }; @@ -1613,6 +1657,13 @@ impl PickerState { return false; }; paths_match(row_cwd, filter_cwd) + || (self.worktrees_enabled + && codex_git_utils::repository_identity(row_cwd) + .zip(codex_git_utils::repository_identity(filter_cwd)) + .is_some_and(|(row, filter)| { + row.common_dir == filter.common_dir + && row.relative_cwd == filter.relative_cwd + })) } fn set_query(&mut self, new_query: String) { @@ -1997,7 +2048,7 @@ fn row_from_app_server_thread(thread: Thread) -> Option { fn thread_list_params( cursor: Option, - cwd_filter: Option<&Path>, + cwd_filter: Option, status: SessionStatus, provider_filter: ProviderFilter, sort_key: ThreadSortKey, @@ -2020,12 +2071,31 @@ fn thread_list_params( project_id: None, parent_thread_id: None, ancestor_thread_id: None, - cwd: cwd_filter.map(|cwd| ThreadListCwdFilter::One(cwd.to_string_lossy().into_owned())), + cwd: cwd_filter, use_state_db_only, search_term: None, } } +pub(crate) fn repository_cwd_filter( + cwd: &Path, + uses_remote_filesystem: bool, + worktrees_enabled: bool, +) -> ThreadListCwdFilter { + if worktrees_enabled + && !uses_remote_filesystem + && let Some(cwds) = codex_git_utils::linked_worktree_cwds(cwd) + && cwds.len() > 1 + { + return ThreadListCwdFilter::Many( + cwds.into_iter() + .map(|cwd| cwd.to_string_lossy().into_owned()) + .collect(), + ); + } + ThreadListCwdFilter::One(cwd.to_string_lossy().into_owned()) +} + fn paths_match(a: &Path, b: &Path) -> bool { path_utils::paths_match_after_normalization(a, b) } @@ -2803,13 +2873,20 @@ fn render_comfortable_session_lines( .cwd .as_ref() .map(|path| format_directory_display(path, /*max_width*/ None)); + let show_cwd = state.filter_mode == SessionFilterMode::All + || (state.worktrees_enabled + && row + .cwd + .as_deref() + .zip(state.local_filter_cwd.as_deref()) + .is_some_and(|(row_cwd, filter_cwd)| !paths_match(row_cwd, filter_cwd))); let footer_lines = render_footer_lines( state.sort_key, &created, &updated, branch, cwd.as_deref(), - state.filter_mode == SessionFilterMode::All, + show_cwd, width, ); if let Some(style) = row_style { @@ -3576,6 +3653,22 @@ mod tests { .join("\n") } + fn render_picker_list(state: &PickerState, width: u16, height: u16) -> String { + use crate::custom_terminal::Terminal; + use crate::test_backend::VT100Backend; + + let backend = VT100Backend::new(width, height); + let mut terminal = Terminal::with_options(backend).expect("terminal"); + terminal.set_viewport_area(Rect::new(0, 0, width, height)); + { + let mut frame = terminal.get_frame(); + let area = frame.area(); + render_list(&mut frame, area, state); + } + terminal.flush().expect("flush"); + terminal.backend().to_string() + } + #[test] fn row_display_preview_prefers_thread_name() { let row = Row { @@ -3592,6 +3685,198 @@ mod tests { assert_eq!(row.display_preview(), "My session"); } + #[tokio::test] + async fn worktrees_filtering_requires_feature() { + let root = tempfile::tempdir().expect("temporary repository"); + let primary = root.path().join("primary"); + let linked = root.path().join("linked"); + let admin = primary.join(".git/worktrees/linked"); + std::fs::create_dir_all(&admin).unwrap(); + std::fs::create_dir_all(&linked).unwrap(); + std::fs::write(primary.join(".git/HEAD"), "ref: refs/heads/main\n").unwrap(); + std::fs::write(admin.join("commondir"), "../..").unwrap(); + std::fs::write( + admin.join("gitdir"), + linked.join(".git").display().to_string(), + ) + .unwrap(); + std::fs::write(linked.join(".git"), format!("gitdir: {}", admin.display())).unwrap(); + let primary = dunce::canonicalize(primary).unwrap(); + let linked = dunce::canonicalize(linked).unwrap(); + let mut state = PickerState::new( + FrameRequester::test_dummy(), + page_only_loader(|_| {}), + ProviderFilter::MatchDefault("openai".to_string()), + /*show_all*/ false, + Some(primary.clone()), + SessionPickerAction::Resume, + ); + let mut row = make_row("session.jsonl", "2025-01-01T00:00:00Z", "Linked session"); + row.cwd = Some(linked.clone()); + let single = ThreadListCwdFilter::One(primary.display().to_string()); + for enabled in [false, true] { + state.worktrees_enabled = enabled; + state.start_initial_load(); + state.ingest_page(page( + vec![row.clone()], + /*next_cursor*/ None, + /*num_scanned_files*/ 1, + /*reached_scan_cap*/ false, + )); + assert_eq!( + state.filtered_rows, + if enabled { vec![row.clone()] } else { vec![] } + ); + if enabled { + state + .pagination + .finish_load(state.next_request_token - 1) + .expect("initial page completed"); + state.relative_time_reference = + Some(parse_timestamp_str("2025-01-02T00:00:00Z").expect("fixed time")); + // The retained row was filtered using its real checkout path; use a stable + // display path so the snapshot is independent of the temporary directory. + state.filtered_rows[0].cwd = Some(PathBuf::from("/repo/linked")); + state.update_viewport(/*rows*/ 12, /*width*/ 100); + assert_snapshot!( + "resume_picker_linked_worktree", + render_picker_list(&state, /*width*/ 100, /*height*/ 12) + ); + } + assert_eq!( + repository_cwd_filter(&primary, /*uses_remote_filesystem*/ false, enabled), + if enabled { + ThreadListCwdFilter::Many(vec![ + primary.display().to_string(), + linked.display().to_string(), + ]) + } else { + single.clone() + } + ); + assert_eq!( + repository_cwd_filter(&primary, /*uses_remote_filesystem*/ true, enabled), + single + ); + } + let mut config = crate::legacy_core::config::ConfigBuilder::default() + .codex_home(root.path().to_path_buf()) + .build() + .await + .unwrap(); + config + .features + .set_enabled(codex_features::Feature::Worktrees, /*enabled*/ true) + .unwrap(); + for (filename_ts, timestamp, cwd) in [ + ("2025-01-02T10-00-00", "2025-01-02T10:00:00Z", &primary), + ("2025-01-02T11-00-00", "2025-01-02T11:00:00Z", &linked), + ] { + crate::tests::write_session_rollout( + root.path(), + filename_ts, + timestamp, + "picker session", + &config.model_provider_id, + cwd, + ) + .expect("persist CLI session"); + } + let app_server = crate::start_embedded_app_server_for_picker(&config) + .await + .expect("start local app-server"); + let request_handle = app_server.request_handle(); + let (bg_tx, mut bg_rx) = mpsc::unbounded_channel(); + let loader = spawn_app_server_page_loader( + /*uses_remote_filesystem*/ false, + app_server, + request_handle, + /*include_non_interactive*/ false, + RawReasoningVisibility::Hidden, + Some(config.clone()), + bg_tx, + ); + let mut live_picker = PickerState::new( + FrameRequester::test_dummy(), + loader, + ProviderFilter::MatchDefault(config.model_provider_id.clone()), + /*show_all*/ false, + Some(primary.clone()), + SessionPickerAction::Resume, + ); + live_picker.local_filter_cwd = Some(primary.clone()); + live_picker.worktrees_enabled = true; + live_picker.start_initial_load(); + let event = tokio::time::timeout(std::time::Duration::from_secs(10), bg_rx.recv()) + .await + .expect("app-server page response") + .expect("page event"); + live_picker + .handle_background_event(event) + .await + .expect("ingest app-server page"); + let linked_index = live_picker + .filtered_rows + .iter() + .position(|row| row.cwd.as_deref() == Some(linked.as_path())) + .unwrap_or_else(|| panic!("linked session missing from {:?}", live_picker.all_rows)); + live_picker.selected = linked_index; + let linked_id = live_picker.filtered_rows[linked_index] + .thread_id + .expect("listed linked session id"); + let accept = KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE); + assert!(matches!( + live_picker.handle_key(accept).await.expect("resume selection"), + Some(SessionSelection::Resume(SessionTarget { thread_id, .. })) if thread_id == linked_id + )); + live_picker.action = SessionPickerAction::Fork; + assert!(matches!( + live_picker.handle_key(accept).await.expect("fork selection"), + Some(SessionSelection::Fork(SessionTarget { thread_id, .. })) if thread_id == linked_id + )); + let mut local = crate::latest_session_lookup_params( + /*uses_remote_filesystem*/ false, + /*uses_remote_workspace*/ false, + &config, + Some(&primary), + /*include_non_interactive*/ false, + crate::LatestSessionLookupMode::StateDbOnly, + ); + assert!(matches!(local.cwd, Some(ThreadListCwdFilter::Many(_)))); + local.cwd = Some(single.clone()); + assert_eq!( + crate::latest_session_lookup_params( + /*uses_remote_filesystem*/ true, + /*uses_remote_workspace*/ false, + &config, + Some(&primary), + /*include_non_interactive*/ false, + crate::LatestSessionLookupMode::StateDbOnly, + ), + local + ); + std::fs::remove_file(linked.join(".git")).unwrap(); + state.set_query("Linked".to_string()); + assert_eq!(state.filtered_rows, vec![row.clone()]); + state.start_initial_load(); + state.ingest_page(page( + vec![row.clone()], + /*next_cursor*/ None, + /*num_scanned_files*/ 1, + /*reached_scan_cap*/ false, + )); + assert!(state.filtered_rows.is_empty()); + state.filter_mode = SessionFilterMode::All; + state.apply_filter(); + assert_eq!(state.filtered_rows, vec![row.clone()]); + state.filter_mode = SessionFilterMode::Cwd; + state.local_filter_cwd = + local_picker_cwd_filter(&Some(primary), /*uses_remote_filesystem*/ true); + assert_eq!(state.local_filter_cwd, None); + state.apply_filter(); + assert_eq!(state.filtered_rows, vec![row]); + } + #[test] fn local_picker_thread_list_params_include_cwd_filter() { let cwd_filter = picker_cwd_filter( @@ -3602,7 +3887,7 @@ mod tests { ); let params = thread_list_params( Some(String::from("cursor-1")), - cwd_filter.as_deref(), + cwd_filter.map(|cwd| ThreadListCwdFilter::One(cwd.to_string_lossy().into_owned())), SessionStatus::Active, ProviderFilter::MatchDefault(String::from("openai")), ThreadSortKey::UpdatedAt, @@ -3982,7 +4267,7 @@ mod tests { fn remote_thread_list_params_omit_provider_filter() { let params = thread_list_params( Some(String::from("cursor-1")), - Some(Path::new("repo/on/server")), + Some(ThreadListCwdFilter::One("repo/on/server".to_string())), SessionStatus::Active, ProviderFilter::Any, ThreadSortKey::UpdatedAt, @@ -4037,7 +4322,7 @@ mod tests { SessionPickerAction::Resume, ); state.local_filter_cwd = - local_picker_cwd_filter(&remote_cwd, /*uses_remote_workspace*/ true); + local_picker_cwd_filter(&remote_cwd, /*uses_remote_filesystem*/ true); state.start_initial_load(); @@ -4089,9 +4374,6 @@ mod tests { #[test] fn resume_table_snapshot() { - use crate::custom_terminal::Terminal; - use crate::test_backend::VT100Backend; - let loader = page_only_loader(|_| {}); let mut state = PickerState::new( FrameRequester::test_dummy(), @@ -4142,20 +4424,7 @@ mod tests { state.scroll_top = 0; state.update_viewport(/*rows*/ 12, /*width*/ 80); - let width: u16 = 80; - let height: u16 = 12; - let backend = VT100Backend::new(width, height); - let mut terminal = Terminal::with_options(backend).expect("terminal"); - terminal.set_viewport_area(Rect::new(0, 0, width, height)); - - { - let mut frame = terminal.get_frame(); - let area = frame.area(); - render_list(&mut frame, area, &state); - } - terminal.flush().expect("flush"); - - let snapshot = terminal.backend().to_string(); + let snapshot = render_picker_list(&state, /*width*/ 80, /*height*/ 12); assert_snapshot!("resume_picker_table", snapshot); } diff --git a/codex-rs/tui/src/resume_picker/archive_tests.rs b/codex-rs/tui/src/resume_picker/archive_tests.rs index abbf48935e..b453ee1a22 100644 --- a/codex-rs/tui/src/resume_picker/archive_tests.rs +++ b/codex-rs/tui/src/resume_picker/archive_tests.rs @@ -189,7 +189,9 @@ fn archived_status_preserves_directory_filter_and_hides_archive_shortcut() { assert_eq!(state.status, SessionStatus::Archived); let params = super::super::thread_list_params( /*cursor*/ None, - Some(std::path::Path::new("/tmp/project")), + Some(codex_app_server_protocol::ThreadListCwdFilter::One( + "/tmp/project".to_string(), + )), SessionStatus::Archived, ProviderFilter::Any, ThreadSortKey::UpdatedAt, diff --git a/codex-rs/tui/src/resume_picker/page_loading.rs b/codex-rs/tui/src/resume_picker/page_loading.rs index c580a61505..1a6da3d5e8 100644 --- a/codex-rs/tui/src/resume_picker/page_loading.rs +++ b/codex-rs/tui/src/resume_picker/page_loading.rs @@ -1,4 +1,36 @@ +//! Keeps a picker listing's pagination state and its checkout filter consistent across pages. + +use std::path::Path; + use super::PageCursor; +use super::repository_cwd_filter; +use codex_app_server_protocol::ThreadListCwdFilter; + +/// Holds the expanded checkout set for the cursor's entire listing cycle. +#[derive(Default)] +pub(super) struct PageCwdFilter { + current: Option, +} + +impl PageCwdFilter { + pub(super) fn for_request( + &mut self, + cursor: Option<&PageCursor>, + cwd: Option<&Path>, + uses_remote_filesystem: bool, + worktrees_enabled: bool, + ) -> Option { + if cursor.is_none() { + self.current = cwd + .map(|cwd| repository_cwd_filter(cwd, uses_remote_filesystem, worktrees_enabled)); + } + self.current.clone() + } +} + +#[cfg(test)] +#[path = "page_loading_tests.rs"] +mod tests; /// Selects whether thread listing trusts indexed metadata or uses the store's normal behavior. #[derive(Clone, Copy, Debug, Eq, PartialEq)] diff --git a/codex-rs/tui/src/resume_picker/page_loading_tests.rs b/codex-rs/tui/src/resume_picker/page_loading_tests.rs new file mode 100644 index 0000000000..92c07de0b4 --- /dev/null +++ b/codex-rs/tui/src/resume_picker/page_loading_tests.rs @@ -0,0 +1,82 @@ +//! Checks that a cursor keeps its original checkout filter until the listing restarts. + +use super::*; +use pretty_assertions::assert_eq; + +#[test] +fn linked_checkout_changes_only_affect_the_next_listing_cycle() { + let root = tempfile::tempdir().expect("temporary repository"); + let primary = root.path().join("primary"); + let linked = root.path().join("linked"); + let admin = primary.join(".git/worktrees/linked"); + std::fs::create_dir_all(primary.join(".git")).expect("primary git directory"); + std::fs::write(primary.join(".git/HEAD"), "ref: refs/heads/main\n").expect("primary HEAD"); + let primary = dunce::canonicalize(primary).expect("canonical primary"); + let single = Some(ThreadListCwdFilter::One( + primary.to_string_lossy().into_owned(), + )); + let mut filter = PageCwdFilter::default(); + assert_eq!( + filter.for_request( + /*cursor*/ None, + Some(&primary), + /*uses_remote_filesystem*/ false, + /*worktrees_enabled*/ true, + ), + single + ); + + std::fs::create_dir_all(&admin).expect("linked worktree registration"); + std::fs::create_dir_all(&linked).expect("linked checkout"); + std::fs::write(admin.join("commondir"), "../..\n").expect("common directory"); + std::fs::write( + admin.join("gitdir"), + linked.join(".git").display().to_string(), + ) + .expect("linked backlink"); + std::fs::write(linked.join(".git"), format!("gitdir: {}", admin.display())) + .expect("linked git file"); + let linked = dunce::canonicalize(linked).expect("canonical linked checkout"); + let cursor = PageCursor::AppServer("next-page".to_string()); + assert_eq!( + filter.for_request( + Some(&cursor), + Some(&primary), + /*uses_remote_filesystem*/ false, + /*worktrees_enabled*/ true, + ), + single + ); + let both = Some(ThreadListCwdFilter::Many(vec![ + primary.to_string_lossy().into_owned(), + linked.to_string_lossy().into_owned(), + ])); + assert_eq!( + filter.for_request( + /*cursor*/ None, + Some(&primary), + /*uses_remote_filesystem*/ false, + /*worktrees_enabled*/ true, + ), + both + ); + std::fs::remove_file(linked.join(".git")).expect("remove linked checkout registration"); + assert_eq!( + filter.for_request( + Some(&cursor), + Some(&primary), + /*uses_remote_filesystem*/ false, + /*worktrees_enabled*/ true, + ), + both + ); + assert_eq!( + filter.for_request( + /*cursor*/ None, + Some(&primary), + /*uses_remote_filesystem*/ false, + /*worktrees_enabled*/ true, + ), + single + ); +} diff --git a/codex-rs/tui/src/snapshots/codex_tui__app__agents_overview__tests__agents_overview_worktree_grouping.snap b/codex-rs/tui/src/snapshots/codex_tui__app__agents_overview__tests__agents_overview_worktree_grouping.snap new file mode 100644 index 0000000000..6bc3144270 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__app__agents_overview__tests__agents_overview_worktree_grouping.snap @@ -0,0 +1,29 @@ +--- +source: tui/src/app/agents_overview_tests.rs +assertion_line: 943 +expression: grouping +--- + Agent command center + 0 need input 0 working 4 ready + ──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── + /tmp/worktree-root/primary/ 2 + › ○ Task Ready + ○ Task Ready + + /tmp/worktree-root/primary/subdir/child 1 + ○ Task Ready + + /tmp/worktree-root/primary/subdir/child 1 + ○ Task Ready + + + + + + + + New task + +› Describe a new task + + enter create task ctrl+j newline esc tasks diff --git a/codex-rs/tui/src/snapshots/codex_tui__resume_picker__tests__resume_picker_linked_worktree.snap b/codex-rs/tui/src/snapshots/codex_tui__resume_picker__tests__resume_picker_linked_worktree.snap new file mode 100644 index 0000000000..12b962d6e9 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__resume_picker__tests__resume_picker_linked_worktree.snap @@ -0,0 +1,6 @@ +--- +source: tui/src/resume_picker.rs +expression: "render_picker_list(&state, 100, 12)" +--- +❯ Linked session + 1d ago ⌁ /repo/linked  no branch