From 973dcd80fcbe2864ab2229d67f10180ded2e2bc7 Mon Sep 17 00:00:00 2001 From: Benjamin Carlsson Date: Tue, 8 Sep 2026 23:23:48 +0000 Subject: [PATCH] Show worktree owner details and add confirmed deletion (#43942) ## What changed - Display owner thread titles, relative update times, and archived or unavailable status in the managed worktree browser. Offer resume only for resumable owners, including those with compressed thread history. - Add a delete action with a confirmation dialog that defaults to Cancel and preserves thread history. - Restrict removal to managed worktrees in the current repository. Refuse the current checkout, including path aliases, and checkouts containing local changes, untracked files, or ignored files. ## Testing Add browser snapshots and tests for owner states, deletion confirmation, and compressed thread history. Add worktree removal coverage for unrelated checkouts, current-directory aliases, untracked and ignored files, and successful removal of a clean checkout. GitOrigin-RevId: e5867f1540d5d5bc8130acc76875e51e3d8b3b6a --- codex-rs/tui/src/app/event_dispatch.rs | 32 +++++ .../app/tests/session_lifecycle_requests.rs | 130 +++++++++++++++++- codex-rs/tui/src/app_event.rs | 12 ++ ...dget__tests__worktree_browser_actions.snap | 5 +- ..._worktree_browser_delete_confirmation.snap | 12 ++ ...twidget__tests__worktree_browser_list.snap | 6 +- ..._tests__worktree_browser_owner_states.snap | 14 ++ .../chatwidget/tests/worktree_picker_tests.rs | 88 +++++++++++- .../tui/src/chatwidget/worktree_picker.rs | 124 +++++++++++++++-- codex-rs/tui/src/worktree_browser.rs | 118 +++++++++++++++- codex-rs/worktree/src/lib.rs | 47 +++++++ codex-rs/worktree/tests/worktree.rs | 54 ++++++++ 12 files changed, 611 insertions(+), 31 deletions(-) create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__worktree_browser_delete_confirmation.snap create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__worktree_browser_owner_states.snap diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 6cda26334f..2c31b48c12 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -114,6 +114,7 @@ impl App { crate::worktree_browser::fetch( request, self.config.codex_home.to_path_buf(), + app_server.request_handle(), self.app_event_tx.clone(), ); } @@ -129,6 +130,37 @@ impl App { AppEvent::ShowManagedWorktreeActions { request, entry } => { self.chat_widget.show_managed_worktree_actions(request, entry); } + AppEvent::ConfirmManagedWorktreeRemoval { request, root } => { + self.chat_widget.confirm_managed_worktree_removal(request, root); + } + AppEvent::RemoveManagedWorktree { request, root } => { + if self.chat_widget.worktree_request_is_current(&request) + && !request.cwd.starts_with(&root) + { + let codex_home = self.config.codex_home.to_path_buf(); + let tx = self.app_event_tx.clone(); + tokio::spawn(async move { + let result = crate::worktree_browser::remove( + codex_home, + request.cwd, + root.clone(), + ) + .await + .map_err(|error| error.to_string()); + tx.send(AppEvent::ManagedWorktreeRemoved { root, result }); + }); + } + } + AppEvent::ManagedWorktreeRemoved { root, result } => match result { + Ok(()) => self.chat_widget.add_info_message( + format!("Removed worktree at {}. Thread history was kept.", root.display()), + /*hint*/ None, + ), + Err(error) => self.chat_widget.add_error_message(format!( + "Could not remove worktree at {}: {error}", + root.display() + )), + }, AppEvent::ChangeWorkingDirectory { thread_id, requested_cwd, 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 69acc3e1d1..8116703d11 100644 --- a/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs +++ b/codex-rs/tui/src/app/tests/session_lifecycle_requests.rs @@ -3058,8 +3058,9 @@ model_reasoning_effort = "low" .pop() .expect("unused checkout"); browser_entries.push(crate::worktree_browser::Entry { + root: unused.root.clone(), cwd: unused.cwd.clone(), - owner: None, + owner: crate::worktree_browser::Owner::None, }); assert_eq!( manager @@ -3131,8 +3132,9 @@ terminal_visualization_instructions = true .find(|checkout| checkout.cwd != unused.cwd) .expect("feature mismatch leaves an unused checkout"); browser_entries.push(crate::worktree_browser::Entry { + root: feature_mismatch_checkout.root, cwd: feature_mismatch_checkout.cwd, - owner: None, + owner: crate::worktree_browser::Owner::None, }); while events.try_recv().is_ok() {} app.handle_event( @@ -3202,8 +3204,9 @@ terminal_visualization_instructions = true None ); browser_entries.push(crate::worktree_browser::Entry { + root: stale_checkout, cwd: stale_cwd, - owner: None, + owner: crate::worktree_browser::Owner::None, }); for mode in [ManagedWorktreeMode::New, ManagedWorktreeMode::Fork] { requests.lock().expect("request recorder lock").clear(); @@ -3234,8 +3237,9 @@ terminal_visualization_instructions = true .find(|checkout| checkout.cwd.canonicalize().ok().as_ref() == Some(&cwd)) .expect("managed checkout"); browser_entries.push(crate::worktree_browser::Entry { + root: checkout.root.clone(), cwd: checkout.cwd.clone(), - owner: Some(replacement), + owner: crate::worktree_browser::Owner::Unavailable(replacement), }); assert!(checkout.root.starts_with(home.join("worktrees"))); assert!(!project_pool.exists()); @@ -3309,8 +3313,9 @@ terminal_visualization_instructions = true }) .map_err(|error| color_eyre::eyre::eyre!(error.to_string()))?; browser_entries.push(crate::worktree_browser::Entry { - cwd: unowned.cwd, - owner: None, + root: unowned.root.clone(), + cwd: unowned.cwd.clone(), + owner: crate::worktree_browser::Owner::None, }); browser_entries.sort_by(|left, right| left.cwd.cmp(&right.cwd)); for owner in [None, Some("not-a-thread-uuid")] { @@ -3326,6 +3331,45 @@ terminal_visualization_instructions = true browser_entries ); } + let missing_owner = ThreadId::new(); + let missing_checkout = manager + .create(&codex_worktree::CreateWorktree { + source_cwd: source.clone(), + base: None, + }) + .map_err(|error| color_eyre::eyre::eyre!(error.to_string()))?; + manager + .bind_thread(&missing_checkout.root, &missing_owner.to_string()) + .map_err(|error| color_eyre::eyre::eyre!(error.to_string()))?; + let (browser_tx, mut browser_rx) = tokio::sync::mpsc::unbounded_channel(); + crate::worktree_browser::fetch( + crate::worktree_browser::Request { + id: uuid::Uuid::new_v4(), + cwd: source.clone(), + thread_id: Some(original), + }, + home.clone(), + server.request_handle(), + crate::app_event_sender::AppEventSender::new(browser_tx), + ); + let Some(AppEvent::ManagedWorktreesLoaded { + result: Ok(entries), + .. + }) = tokio::time::timeout( + std::time::Duration::from_secs(/*secs*/ 5), + browser_rx.recv(), + ) + .await? + else { + panic!("browser must return annotated worktrees"); + }; + assert_eq!( + entries + .iter() + .find(|entry| entry.root == missing_checkout.root) + .map(|entry| &entry.owner), + Some(&crate::worktree_browser::Owner::Unavailable(missing_owner)) + ); app.start_fresh_session_with_summary_hint( &mut tui, &mut server, @@ -3360,6 +3404,80 @@ terminal_visualization_instructions = true assert_eq!(app.chat_widget.thread_id() != Some(unsaved), is_new); assert_eq!(recorded_params(&requests, "thread/fork").len(), 0); } + let saved_owner = entries + .iter() + .find_map(|entry| match &entry.owner { + crate::worktree_browser::Owner::Resumable(thread) => Some(thread.clone()), + _ => None, + }) + .expect("browser should resolve a saved owner"); + let saved_path = server + .thread_read(saved_owner.id, /*include_turns*/ false) + .await? + .path + .expect("saved rollout path"); + fs::OpenOptions::new() + .write(true) + .open(&saved_path)? + .set_times(std::fs::FileTimes::new().set_modified( + std::time::SystemTime::now() - std::time::Duration::from_secs(8 * 24 * 60 * 60), + ))?; + codex_rollout::spawn_rollout_compression_worker(home.clone()); + tokio::time::timeout(std::time::Duration::from_secs(10), async { + while saved_path.exists() || !saved_path.with_extension("jsonl.zst").is_file() { + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + } + }) + .await?; + let (browser_tx, mut browser_rx) = tokio::sync::mpsc::unbounded_channel(); + crate::worktree_browser::fetch( + crate::worktree_browser::Request { + id: uuid::Uuid::new_v4(), + cwd: source.clone(), + thread_id: Some(original), + }, + home.clone(), + server.request_handle(), + crate::app_event_sender::AppEventSender::new(browser_tx), + ); + let Some(AppEvent::ManagedWorktreesLoaded { + result: Ok(compressed_entries), + .. + }) = tokio::time::timeout(std::time::Duration::from_secs(10), browser_rx.recv()).await? + else { + panic!("browser must return compressed owner metadata"); + }; + assert!(compressed_entries.iter().any(|entry| matches!( + &entry.owner, + crate::worktree_browser::Owner::Resumable(thread) if thread.id == saved_owner.id + ))); + server.thread_archive(saved_owner.id).await?; + let (browser_tx, mut browser_rx) = tokio::sync::mpsc::unbounded_channel(); + crate::worktree_browser::fetch( + crate::worktree_browser::Request { + id: uuid::Uuid::new_v4(), + cwd: source.clone(), + thread_id: Some(original), + }, + home, + server.request_handle(), + crate::app_event_sender::AppEventSender::new(browser_tx), + ); + let Some(AppEvent::ManagedWorktreesLoaded { + result: Ok(entries), + .. + }) = tokio::time::timeout( + std::time::Duration::from_secs(/*secs*/ 10), + browser_rx.recv(), + ) + .await? + else { + panic!("browser must return archived owner metadata"); + }; + assert!(entries.iter().any(|entry| matches!( + &entry.owner, + crate::worktree_browser::Owner::Archived(thread) if thread.id == saved_owner.id + ))); server.shutdown().await?; proxy.await??; Ok(()) diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index b24a3dc439..6e481c0a04 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -460,6 +460,18 @@ pub(crate) enum AppEvent { request: crate::worktree_browser::Request, entry: crate::worktree_browser::Entry, }, + ConfirmManagedWorktreeRemoval { + request: crate::worktree_browser::Request, + root: PathBuf, + }, + RemoveManagedWorktree { + request: crate::worktree_browser::Request, + root: PathBuf, + }, + ManagedWorktreeRemoved { + root: PathBuf, + result: Result<(), String>, + }, /// Change the working directory of the originating idle primary thread. ChangeWorkingDirectory { diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__worktree_browser_actions.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__worktree_browser_actions.snap index 59abf1a3ed..34480af517 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__worktree_browser_actions.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__worktree_browser_actions.snap @@ -1,12 +1,13 @@ --- source: tui/src/chatwidget/tests/worktree_picker_tests.rs -assertion_line: 114 +assertion_line: 137 expression: "render_bottom_popup(&chat, 80)" --- - Worktree + Worktree: Fix login form /repo/worktree › 1. Resume owner thread 2. Copy working directory + 3. Delete worktree Press enter to confirm or esc to go back diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__worktree_browser_delete_confirmation.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__worktree_browser_delete_confirmation.snap new file mode 100644 index 0000000000..8a9e164c87 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__worktree_browser_delete_confirmation.snap @@ -0,0 +1,12 @@ +--- +source: tui/src/chatwidget/tests/worktree_picker_tests.rs +assertion_line: 170 +expression: "render_bottom_popup(&chat, 80)" +--- + Delete this worktree? + /repo/worktree + +› 1. Cancel + 2. Delete worktree Keeps thread history; may disrupt other sessions + + Press enter to confirm or esc to go back diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__worktree_browser_list.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__worktree_browser_list.snap index 61d7777386..8d04e3f545 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__worktree_browser_list.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__worktree_browser_list.snap @@ -1,12 +1,12 @@ --- source: tui/src/chatwidget/tests/worktree_picker_tests.rs -assertion_line: 100 +assertion_line: 120 expression: "render_bottom_popup(&chat, 80)" --- Managed worktrees - Select a worktree to resume its owner or copy its working directory + Select a worktree to resume, copy its path, or delete it -› /repo/worktree Owner: 00000000-0000-0000-0000-000000000001 +› Fix login form updated 2h ago · /repo/worktree Press enter to confirm or esc to go back diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__worktree_browser_owner_states.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__worktree_browser_owner_states.snap new file mode 100644 index 0000000000..1a1e08fae9 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__worktree_browser_owner_states.snap @@ -0,0 +1,14 @@ +--- +source: tui/src/chatwidget/tests/worktree_picker_tests.rs +assertion_line: 237 +expression: "render_bottom_popup(&chat, 80)" +--- + Managed worktrees + Select a worktree to resume, copy its path, or delete it + + +› Fix settings Archived · updated 1d ago · /repo/archived + /repo/missing Owner thread unavailable + /repo/empty No attached thread + + Press enter to confirm or esc to go back diff --git a/codex-rs/tui/src/chatwidget/tests/worktree_picker_tests.rs b/codex-rs/tui/src/chatwidget/tests/worktree_picker_tests.rs index ebfa30af68..c60e12bc0e 100644 --- a/codex-rs/tui/src/chatwidget/tests/worktree_picker_tests.rs +++ b/codex-rs/tui/src/chatwidget/tests/worktree_picker_tests.rs @@ -93,6 +93,8 @@ async fn slash_worktree_offers_current_or_new_conversation() { #[tokio::test] async fn worktree_browser_actions_and_stale_results() { use crate::worktree_browser::Entry; + use crate::worktree_browser::Owner; + use crate::worktree_browser::ThreadSummary; let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; let checkout = tempdir().unwrap(); std::fs::create_dir(checkout.path().join(".git")).unwrap(); @@ -106,8 +108,13 @@ async fn worktree_browser_actions_and_stale_results() { ); let owner = ThreadId::from_string("00000000-0000-0000-0000-000000000001").unwrap(); let entry = Entry { + root: PathBuf::from("/repo/worktree"), cwd: PathBuf::from("/repo/worktree"), - owner: Some(owner), + owner: Owner::Resumable(ThreadSummary { + id: owner, + title: "Fix login form".to_string(), + updated_at: chrono::Utc::now().timestamp() - 7_200, + }), }; chat.on_managed_worktrees_loaded(request, Ok(vec![entry.clone()])); assert_chatwidget_snapshot!( @@ -149,7 +156,7 @@ async fn worktree_browser_actions_and_stale_results() { chat.show_managed_worktree_actions( request, Entry { - owner: None, + owner: Owner::None, ..entry }, ); @@ -159,6 +166,24 @@ async fn worktree_browser_actions_and_stale_results() { panic!("copy action"); }; assert_matches!(chat.managed_worktree_action(&request, action), Some(AppEvent::CopySelection { text, .. }) if &*text == "/repo/worktree"); + chat.confirm_managed_worktree_removal(request.clone(), PathBuf::from("/repo/worktree")); + assert_chatwidget_snapshot!( + "worktree_browser_delete_confirmation", + render_bottom_popup(&chat, /*width*/ 80) + ); + chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); + assert_matches!(rx.try_recv(), Err(TryRecvError::Empty)); + chat.confirm_managed_worktree_removal(request, PathBuf::from("/repo/worktree")); + chat.handle_key_event(KeyEvent::from(KeyCode::Down)); + chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); + let AppEvent::ManagedWorktreeAction { + request: selected, + action, + } = rx.try_recv().unwrap() + else { + panic!("delete action"); + }; + assert_matches!(chat.managed_worktree_action(&selected, action), Some(AppEvent::RemoveManagedWorktree { root, .. }) if root.as_path() == std::path::Path::new("/repo/worktree")); let first = chat.request_managed_worktrees().unwrap(); chat.handle_key_event(KeyEvent::from(KeyCode::Esc)); chat.on_managed_worktrees_loaded(first, Ok(Vec::new())); @@ -171,3 +196,62 @@ async fn worktree_browser_actions_and_stale_results() { chat.on_managed_worktrees_loaded(current, Ok(Vec::new())); assert!(!chat.bottom_pane.has_active_view()); } + +#[tokio::test] +async fn worktree_browser_names_archived_and_missing_owners() { + use crate::worktree_browser::Entry; + use crate::worktree_browser::Owner; + use crate::worktree_browser::ThreadSummary; + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + let checkout = tempdir().unwrap(); + std::fs::create_dir(checkout.path().join(".git")).unwrap(); + std::fs::write(checkout.path().join(".git/HEAD"), "ref: refs/heads/main\n").unwrap(); + chat.config.cwd = AbsolutePathBuf::from_absolute_path(checkout.path()).unwrap(); + chat.set_feature_enabled(Feature::Worktrees, /*enabled*/ true); + let request = chat.request_managed_worktrees().unwrap(); + let owner = ThreadId::from_string("00000000-0000-0000-0000-000000000002").unwrap(); + chat.on_managed_worktrees_loaded( + request.clone(), + Ok(vec![ + Entry { + root: PathBuf::from("/repo/archived"), + cwd: PathBuf::from("/repo/archived"), + owner: Owner::Archived(ThreadSummary { + id: owner, + title: "Fix settings".to_string(), + updated_at: chrono::Utc::now().timestamp() - 86_400, + }), + }, + Entry { + root: PathBuf::from("/repo/missing"), + cwd: PathBuf::from("/repo/missing"), + owner: Owner::Unavailable(owner), + }, + Entry { + root: PathBuf::from("/repo/empty"), + cwd: PathBuf::from("/repo/empty"), + owner: Owner::None, + }, + ]), + ); + assert_chatwidget_snapshot!( + "worktree_browser_owner_states", + render_bottom_popup(&chat, /*width*/ 80) + ); + chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); + chat.show_managed_worktree_actions( + request, + Entry { + root: PathBuf::from("/repo/archived"), + cwd: PathBuf::from("/repo/archived"), + owner: Owner::Archived(ThreadSummary { + id: owner, + title: "Fix settings".to_string(), + updated_at: 0, + }), + }, + ); + let popup = render_bottom_popup(&chat, /*width*/ 80); + assert!(popup.contains("Worktree: Fix settings"), "popup: {popup}"); + assert!(!popup.contains("Resume owner")); +} diff --git a/codex-rs/tui/src/chatwidget/worktree_picker.rs b/codex-rs/tui/src/chatwidget/worktree_picker.rs index d9a8d3b067..7620aafdd1 100644 --- a/codex-rs/tui/src/chatwidget/worktree_picker.rs +++ b/codex-rs/tui/src/chatwidget/worktree_picker.rs @@ -4,6 +4,7 @@ use super::*; use crate::app_event::ManagedWorktreeMode; use crate::worktree_browser::Action; use crate::worktree_browser::Entry; +use crate::worktree_browser::Owner; use crate::worktree_browser::Request; const BROWSER_VIEW_ID: &str = "managed-worktrees"; @@ -158,7 +159,7 @@ impl ChatWidget { Some(request) } - fn worktree_request_is_current(&self, request: &Request) -> bool { + pub(crate) fn worktree_request_is_current(&self, request: &Request) -> bool { self.worktree_popup_request_id == Some(request.id) && request.cwd == self.config.cwd.as_path() && request.thread_id == self.thread_id @@ -194,7 +195,7 @@ impl ChatWidget { if entries.is_empty() { "No worktrees in this repository's configured pool" } else { - "Select a worktree to resume its owner or copy its working directory" + "Select a worktree to resume, copy its path, or delete it" } .to_string(), ), @@ -203,13 +204,38 @@ impl ChatWidget { .into_iter() .map(|entry| { let request = request.clone(); + let (name, description) = match &entry.owner { + Owner::None => ( + entry.cwd.display().to_string(), + "No attached thread".to_string(), + ), + Owner::Unavailable(_) => ( + entry.cwd.display().to_string(), + "Owner thread unavailable".to_string(), + ), + Owner::Archived(thread) | Owner::Resumable(thread) => { + let status = match &entry.owner { + Owner::Archived(_) => "Archived · ", + Owner::Resumable(_) => "", + Owner::None | Owner::Unavailable(_) => unreachable!(), + }; + ( + thread.title.clone(), + format!( + "{status}updated {} · {}", + worktree_updated_ago( + thread.updated_at, + chrono::Utc::now().timestamp() + ), + entry.cwd.display() + ), + ) + } + }; SelectionItem { - name: entry.cwd.display().to_string(), - search_value: Some(entry.cwd.display().to_string()), - description: Some(entry.owner.map_or_else( - || "No owner metadata".to_string(), - |owner| format!("Owner: {owner}"), - )), + name: name.clone(), + search_value: Some(format!("{name} {}", entry.cwd.display())), + description: Some(description), actions: vec![Box::new(move |tx| { tx.send(AppEvent::ShowManagedWorktreeActions { request: request.clone(), @@ -241,6 +267,10 @@ impl ChatWidget { label: "Worktree working directory".to_string(), format: crate::clipboard_copy::CopyFormat::PlainText, }, + Action::Remove(root) => AppEvent::RemoveManagedWorktree { + request: request.clone(), + root, + }, }) } @@ -249,7 +279,8 @@ impl ChatWidget { return; } let mut items = Vec::new(); - if let Some(owner) = entry.owner { + if let Owner::Resumable(thread) = &entry.owner { + let owner = thread.id; let request = request.clone(); items.push(SelectionItem { name: "Resume owner thread".to_string(), @@ -264,6 +295,7 @@ impl ChatWidget { }); } let cwd = entry.cwd.clone(); + let copy_request = request.clone(); items.push(SelectionItem { name: "Copy working directory".to_string(), is_disabled: entry.cwd.to_str().is_none(), @@ -274,19 +306,89 @@ impl ChatWidget { .then(|| "Path is not valid UTF-8".to_string()), actions: vec![Box::new(move |tx| { tx.send(AppEvent::ManagedWorktreeAction { - request: request.clone(), + request: copy_request.clone(), action: Action::Copy(cwd.clone()), }) })], dismiss_on_select: true, ..Default::default() }); + let can_delete = !request.cwd.starts_with(&entry.root); + let root = entry.root.clone(); + let delete_request = request; + items.push(SelectionItem { + name: "Delete worktree".to_string(), + is_disabled: !can_delete, + disabled_reason: (!can_delete) + .then(|| "Switch to another checkout before deleting this one".to_string()), + actions: vec![Box::new(move |tx| { + tx.send(AppEvent::ConfirmManagedWorktreeRemoval { + request: delete_request.clone(), + root: root.clone(), + }) + })], + dismiss_on_select: true, + ..Default::default() + }); + let title = match &entry.owner { + Owner::Archived(thread) | Owner::Resumable(thread) => { + format!("Worktree: {}", thread.title) + } + Owner::None | Owner::Unavailable(_) => "Worktree".to_string(), + }; self.bottom_pane.show_selection_view(SelectionViewParams { - title: Some("Worktree".to_string()), + title: Some(title), subtitle: Some(entry.cwd.display().to_string()), items, footer_hint: Some(standard_popup_hint_line()), ..Default::default() }); } + + pub(crate) fn confirm_managed_worktree_removal(&mut self, request: Request, root: PathBuf) { + if !self.worktree_request_is_current(&request) || request.cwd.starts_with(&root) { + return; + } + self.bottom_pane.show_selection_view(SelectionViewParams { + title: Some("Delete this worktree?".to_string()), + subtitle: Some(root.display().to_string()), + items: vec![ + SelectionItem { + name: "Cancel".to_string(), + actions: vec![], + dismiss_on_select: true, + ..Default::default() + }, + SelectionItem { + name: "Delete worktree".to_string(), + description: Some( + "Keeps thread history; may disrupt other sessions".to_string(), + ), + actions: vec![Box::new(move |tx| { + tx.send(AppEvent::ManagedWorktreeAction { + request: request.clone(), + action: Action::Remove(root.clone()), + }) + })], + dismiss_on_select: true, + ..Default::default() + }, + ], + footer_hint: Some(standard_popup_hint_line()), + ..Default::default() + }); + } +} + +fn worktree_updated_ago(updated_at: i64, now: i64) -> String { + let seconds = now.saturating_sub(updated_at).max(0); + if seconds < 60 { + "just now".to_string() + } else if seconds < 3_600 { + format!("{}m ago", seconds / 60) + } else if seconds < 86_400 { + format!("{}h ago", seconds / 3_600) + } else { + format!("{}d ago", seconds / 86_400) + } } diff --git a/codex-rs/tui/src/worktree_browser.rs b/codex-rs/tui/src/worktree_browser.rs index 723870af4d..aa3664d097 100644 --- a/codex-rs/tui/src/worktree_browser.rs +++ b/codex-rs/tui/src/worktree_browser.rs @@ -1,7 +1,12 @@ -//! Read-only discovery for the local worktree browser; ownership is not activity or exclusion. +//! Local worktree discovery and explicit removal; ownership is not activity or exclusion. use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; +use codex_app_server_client::AppServerRequestHandle; +use codex_app_server_protocol::ClientRequest; +use codex_app_server_protocol::RequestId; +use codex_app_server_protocol::ThreadReadParams; +use codex_app_server_protocol::ThreadReadResponse; use codex_protocol::ThreadId; use std::path::PathBuf; @@ -14,21 +19,98 @@ pub(crate) struct Request { #[derive(Clone, Debug, PartialEq, Eq)] pub(crate) struct Entry { + pub root: PathBuf, pub cwd: PathBuf, - pub owner: Option, + pub owner: Owner, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct ThreadSummary { + pub id: ThreadId, + pub title: String, + pub updated_at: i64, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) enum Owner { + None, + Unavailable(ThreadId), + Archived(ThreadSummary), + Resumable(ThreadSummary), } #[derive(Clone, Debug)] pub(crate) enum Action { Resume(ThreadId), Copy(PathBuf), + Remove(PathBuf), } -pub(crate) fn fetch(request: Request, codex_home: PathBuf, tx: AppEventSender) { +pub(crate) fn fetch( + request: Request, + codex_home: PathBuf, + app_server: AppServerRequestHandle, + tx: AppEventSender, +) { tokio::spawn(async move { - let result = list(codex_home, request.cwd.clone()) - .await - .map_err(|error| error.to_string()); + let result = async { + let mut entries = list(codex_home.clone(), request.cwd.clone()).await?; + let archive_root = codex_home.join(codex_rollout::ARCHIVED_SESSIONS_SUBDIR); + for entry in &mut entries { + let Owner::Unavailable(id) = entry.owner else { + continue; + }; + let response = app_server + .request_typed::(ClientRequest::ThreadRead { + request_id: RequestId::String(uuid::Uuid::new_v4().to_string()), + params: ThreadReadParams { + thread_id: id.to_string(), + include_turns: false, + }, + }) + .await; + if let Ok(response) = response { + let thread = response.thread; + let Some(path) = thread.path.as_ref() else { + continue; + }; + let Some(path) = codex_rollout::existing_rollout_path(path).await else { + continue; + }; + let archived = path.starts_with(&archive_root); + let title = thread + .name + .filter(|name| !name.trim().is_empty()) + .unwrap_or_else(|| { + thread + .preview + .lines() + .next() + .filter(|preview| !preview.trim().is_empty()) + .unwrap_or("Untitled conversation") + .to_string() + }); + let title = if title.chars().count() > 80 { + format!("{}…", title.chars().take(79).collect::()) + } else { + title + }; + let summary = ThreadSummary { + id, + title, + updated_at: thread.updated_at, + }; + entry.owner = if archived { + Owner::Archived(summary) + } else { + Owner::Resumable(summary) + }; + } + } + anyhow::Ok(entries) + } + .await + .map_err(|error| error.to_string()); tx.send(AppEvent::ManagedWorktreesLoaded { request, result }); }); } @@ -55,10 +137,32 @@ pub(crate) async fn list(codex_home: PathBuf, cwd: PathBuf) -> anyhow::Result anyhow::Result<()> { + let host = crate::legacy_core::config::load_config_toml_with_layer_stack( + &codex_home, + /*cwd*/ None, + Vec::new(), + codex_config::ConfigLoadOptions::default(), + ) + .await?; + let settings = + codex_worktree::WorktreeSettings::for_cli(&codex_home, host.config_toml.desktop.as_ref())?; + tokio::task::spawn_blocking(move || { + codex_worktree::WorktreeManager::new(settings).remove(&source_cwd, &root) + }) + .await? +} diff --git a/codex-rs/worktree/src/lib.rs b/codex-rs/worktree/src/lib.rs index 3162bd3c5a..e701a92ef8 100644 --- a/codex-rs/worktree/src/lib.rs +++ b/codex-rs/worktree/src/lib.rs @@ -280,6 +280,53 @@ impl WorktreeManager { metadata::owner(&checkout) } + /// Remove a currently registered checkout from this repository, refusing local changes. + pub fn remove(&self, source_cwd: &Path, root: &Path) -> Result<()> { + let checkout = self + .list(source_cwd)? + .into_iter() + .find(|checkout| checkout.root == root) + .with_context(|| { + format!( + "{} is not a managed worktree in this repository", + root.display() + ) + })?; + let source_cwd = dunce::canonicalize(source_cwd).with_context(|| { + format!("cannot resolve current directory {}", source_cwd.display()) + })?; + let checkout_root = dunce::canonicalize(&checkout.root) + .with_context(|| format!("cannot resolve worktree {}", checkout.root.display()))?; + if source_cwd.starts_with(&checkout_root) { + bail!("switch to another checkout before deleting the current worktree"); + } + let ignored = git_output( + &checkout.root, + GitOperation::WorkingTree, + [ + OsStr::new("ls-files"), + OsStr::new("--others"), + OsStr::new("--ignored"), + OsStr::new("--exclude-standard"), + OsStr::new("-z"), + ], + )?; + if !ignored.stdout.is_empty() { + bail!("worktree contains ignored local files; remove them before deleting it"); + } + git_output( + &checkout.source_root, + GitOperation::WorkingTree, + [ + OsStr::new("worktree"), + OsStr::new("remove"), + root.as_os_str(), + ], + )?; + remove_empty_bucket(root); + Ok(()) + } + fn managed_checkout(&self, checkout: &Path) -> Result { let managed_root = dunce::canonicalize(&self.settings.root).with_context(|| { format!( diff --git a/codex-rs/worktree/tests/worktree.rs b/codex-rs/worktree/tests/worktree.rs index 8f6c679674..8df546d915 100644 --- a/codex-rs/worktree/tests/worktree.rs +++ b/codex-rs/worktree/tests/worktree.rs @@ -80,6 +80,60 @@ fn create_worktree( }) } +#[test] +fn removing_managed_worktree_refuses_dirty_or_unrelated_checkouts() { + let fixture = RepositoryFixture::new(); + let manager = fixture.manager(); + let checkout = create_worktree(&manager, &fixture.repository, /*base*/ None) + .expect("create managed checkout"); + let other = RepositoryFixture::new(); + assert!(manager.remove(&other.repository, &checkout.root).is_err()); + assert!( + manager + .remove(&fixture.repository, &fixture.repository) + .is_err() + ); + let aliased_current = checkout + .root + .join("..") + .join(checkout.root.file_name().expect("managed checkout name")); + assert!( + manager + .remove(&aliased_current, &checkout.root) + .expect_err("cannot remove current checkout via alias") + .to_string() + .contains("current worktree") + ); + assert!(checkout.root.exists()); + fs::write(checkout.root.join("unsaved.txt"), "local work\n").expect("write untracked file"); + assert!(manager.remove(&fixture.repository, &checkout.root).is_err()); + assert!(checkout.root.join("unsaved.txt").exists()); + + fs::remove_file(checkout.root.join("unsaved.txt")).expect("remove untracked file"); + fs::write(fixture.repository.join(".git/info/exclude"), ".env\n") + .expect("ignore local settings file"); + fs::write(checkout.root.join(".env"), "local settings\n").expect("write ignored file"); + assert!( + manager + .remove(&fixture.repository, &checkout.root) + .expect_err("refuse ignored local files") + .to_string() + .contains("ignored local files") + ); + assert!(checkout.root.join(".env").exists()); + fs::remove_file(checkout.root.join(".env")).expect("remove ignored file"); + manager + .remove(&fixture.repository, &checkout.root) + .expect("remove clean checkout"); + assert!(!checkout.root.exists()); + assert!( + manager + .list(&fixture.repository) + .expect("list remaining") + .is_empty() + ); +} + fn git_output(repository: &Path, args: &[&str]) -> std::process::Output { Command::new("git") .current_dir(repository)