From d30f9cc72a25d4f3d59f50a7766b6b032e3c7329 Mon Sep 17 00:00:00 2001 From: Benjamin Carlsson Date: Sun, 6 Sep 2026 20:38:57 +0000 Subject: [PATCH] Include linked worktrees in TUI session discovery (#43279) ## Why Directory-scoped session lookup misses conversations in linked worktrees of the same repository. Managed worktree creation also runs synchronous Git work on the TUI event loop. ## What changed - With `worktrees` enabled for local sessions, include corresponding directories across linked checkouts in resume and fork pickers and `--last` lookup. Group them together in the agents overview and show checkout paths for picker entries from another worktree. - Validate shared Git metadata and backlinks, preserve relative working directories and logical path aliases, and reject directory symlink escapes. Keep the expanded directory filter stable across pagination until the listing restarts. - Fall back to the originally requested directory when an older daemon rejects an array-valued `cwd` filter. - Create managed worktrees on a blocking worker, check transition blockers before allocation, and revalidate the source session before switching. Report retained checkout paths and cleanup instructions if the transition cannot complete. - Start the in-session picker server in a separate task to avoid exhausting the TUI event future's stack. ## Testing Add coverage for linked-checkout discovery, feature gating, picker selection and rendering, project grouping, pagination stability, and legacy daemon fallback. Extend worktree lifecycle tests to cover running-agent blockers and recovery after the source session changes during creation. GitOrigin-RevId: 4a8a36c0dba3d32d72600f2edf8258a29fb4cb07 --- codex-rs/git-utils/src/lib.rs | 4 + codex-rs/git-utils/src/worktree.rs | 178 ++++++++++ codex-rs/git-utils/src/worktree_tests.rs | 97 +++++ codex-rs/tui/src/app.rs | 2 + codex-rs/tui/src/app/agents_overview.rs | 5 + codex-rs/tui/src/app/agents_overview_tests.rs | 68 ++++ codex-rs/tui/src/app/agents_overview_view.rs | 68 +++- codex-rs/tui/src/app/event_dispatch.rs | 6 +- .../tui/src/app/managed_worktree_creation.rs | 258 ++++++++++++++ codex-rs/tui/src/app/session_picker.rs | 33 +- codex-rs/tui/src/app/startup.rs | 1 + codex-rs/tui/src/app/test_support.rs | 1 + codex-rs/tui/src/app/tests.rs | 2 + .../app/tests/session_lifecycle_requests.rs | 98 ++++++ codex-rs/tui/src/app/tests/turn_submission.rs | 45 +++ codex-rs/tui/src/app/working_directory.rs | 77 +--- codex-rs/tui/src/app_event.rs | 18 + codex-rs/tui/src/app_server_session.rs | 12 +- .../tui/src/app_server_session/thread_list.rs | 41 +++ .../app_server_session/thread_list_tests.rs | 77 ++++ codex-rs/tui/src/lib.rs | 80 ++++- codex-rs/tui/src/resume_picker.rs | 331 ++++++++++++++++-- .../tui/src/resume_picker/archive_tests.rs | 4 +- .../tui/src/resume_picker/page_loading.rs | 32 ++ .../src/resume_picker/page_loading_tests.rs | 82 +++++ ...ts__agents_overview_worktree_grouping.snap | 29 ++ ..._tests__resume_picker_linked_worktree.snap | 6 + 27 files changed, 1511 insertions(+), 144 deletions(-) create mode 100644 codex-rs/git-utils/src/worktree.rs create mode 100644 codex-rs/git-utils/src/worktree_tests.rs create mode 100644 codex-rs/tui/src/app/managed_worktree_creation.rs create mode 100644 codex-rs/tui/src/app_server_session/thread_list.rs create mode 100644 codex-rs/tui/src/app_server_session/thread_list_tests.rs create mode 100644 codex-rs/tui/src/resume_picker/page_loading_tests.rs create mode 100644 codex-rs/tui/src/snapshots/codex_tui__app__agents_overview__tests__agents_overview_worktree_grouping.snap create mode 100644 codex-rs/tui/src/snapshots/codex_tui__resume_picker__tests__resume_picker_linked_worktree.snap 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