From 46a2b259a71828e7fcdeab1f67ef675304e2ca14 Mon Sep 17 00:00:00 2001 From: Ogulcan Celik Date: Fri, 19 Jun 2026 18:59:43 +0300 Subject: [PATCH] fix: defer api worktree operations refs #686 refs #657 refs #662 refs https://github.com/ogulcancelik/herdr/discussions/687 --- src/app/api.rs | 18 +- src/app/api/worktrees.rs | 996 ++++++++++++++++++++++-------- src/app/api/worktrees/deferred.rs | 566 +++++++++++++++++ src/app/mod.rs | 8 + src/app/runtime.rs | 14 + src/app/worktrees.rs | 125 +++- src/events.rs | 30 +- src/server/headless.rs | 9 + src/ui/dialogs.rs | 84 ++- src/worktree.rs | 135 ++++ tests/cli_wrapper.rs | 70 +++ 11 files changed, 1764 insertions(+), 291 deletions(-) create mode 100644 src/app/api/worktrees/deferred.rs diff --git a/src/app/api.rs b/src/app/api.rs index 6dc48953..cbd6866c 100644 --- a/src/app/api.rs +++ b/src/app/api.rs @@ -91,12 +91,12 @@ impl App { } if let AppEvent::WorktreeAddFinished(result) = ev { - self.handle_worktree_add_finished(result); + self.handle_worktree_add_finished(*result); return; } if let AppEvent::WorktreeRemoveFinished(result) = ev { - self.handle_worktree_remove_finished(result); + self.handle_worktree_remove_finished(*result); return; } @@ -778,11 +778,21 @@ impl App { } Method::WorktreeList(params) => return self.handle_worktree_list(request.id, params), Method::WorktreeCreate(params) => { - return self.handle_worktree_create(request.id, params); + let _ = params; + return responses::encode_error( + request.id, + "invalid_request", + "worktree.create is handled asynchronously by the app runtime", + ); } Method::WorktreeOpen(params) => return self.handle_worktree_open(request.id, params), Method::WorktreeRemove(params) => { - return self.handle_worktree_remove(request.id, params); + let _ = params; + return responses::encode_error( + request.id, + "invalid_request", + "worktree.remove is handled asynchronously by the app runtime", + ); } Method::TabList(params) => return self.handle_tab_list(request.id, params), Method::TabGet(target) => return self.handle_tab_get(request.id, target), diff --git a/src/app/api/worktrees.rs b/src/app/api/worktrees.rs index abfa0c1e..62a46fdb 100644 --- a/src/app/api/worktrees.rs +++ b/src/app/api/worktrees.rs @@ -1,14 +1,15 @@ use std::path::{Path, PathBuf}; -use std::time::{SystemTime, UNIX_EPOCH}; use crate::api::schema::{ - EventData, EventEnvelope, EventKind, ResponseResult, WorktreeCreateParams, WorktreeInfo, - WorktreeListParams, WorktreeOpenParams, WorktreeRemoveParams, WorktreeSourceInfo, + EventData, EventEnvelope, EventKind, ResponseResult, WorktreeInfo, WorktreeListParams, + WorktreeOpenParams, WorktreeSourceInfo, }; use crate::app::App; use super::responses::{encode_error, encode_success}; +mod deferred; + struct ApiFailure { code: &'static str, message: String, @@ -71,99 +72,6 @@ impl App { ) } - pub(super) fn handle_worktree_create( - &mut self, - id: String, - params: WorktreeCreateParams, - ) -> String { - let branch = params - .branch - .unwrap_or_else(|| { - let seed = SystemTime::now() - .duration_since(UNIX_EPOCH) - .map(|duration| duration.as_micros().min(u128::from(u64::MAX)) as u64) - .unwrap_or(0); - crate::worktree::generated_branch_slug(seed) - }) - .trim() - .to_string(); - if branch.is_empty() { - return encode_error(id, "invalid_request", "branch is required"); - } - let base = params.base.unwrap_or_else(|| "HEAD".into()); - let mut source = match self.resolve_worktree_source(params.workspace_id, params.cwd) { - Ok(source) => source, - Err(err) => return encode_error(id, err.code, err.message), - }; - let checkout_path = match params.path { - Some(path) => match absolute_user_path(&path) { - Ok(path) => path, - Err(err) => return encode_error(id, err.code, err.message), - }, - None => crate::worktree::default_checkout_path( - &self.state.worktree_directory, - &source.repo_name, - &branch, - ), - }; - - if let Some(parent_dir) = checkout_path.parent() { - if let Err(err) = std::fs::create_dir_all(parent_dir) { - return encode_error(id, "worktree_create_failed", err.to_string()); - } - } - - let command = crate::worktree::build_worktree_add_new_branch_command( - &source.source_checkout_path, - &checkout_path, - &branch, - &base, - ); - if let Err(err) = crate::worktree::run_worktree_command(&command) { - return encode_error(id, "worktree_create_failed", err); - } - if let Err(err) = self.ensure_source_parent_membership(&mut source, true) { - return encode_error(id, err.code, err.message); - } - - let ws_idx = match self.create_workspace_with_options(checkout_path.clone(), params.focus) { - Ok(ws_idx) => ws_idx, - Err(err) => { - return encode_error( - id, - "worktree_open_failed", - format!("created worktree but failed to open workspace: {err}"), - ); - } - }; - self.mark_worktree_membership(&source, ws_idx, checkout_path, true, false); - if let Some(label) = params.label { - if let Some(ws) = self.state.workspaces.get_mut(ws_idx) { - ws.set_custom_name(label); - } - } - self.state.mark_session_dirty(); - self.emit_workspace_open_events(ws_idx); - - let worktree = self - .worktree_info_for_workspace(ws_idx) - .expect("created worktree workspace should have worktree info"); - self.emit_worktree_created_event(ws_idx, worktree.clone()); - encode_success( - id, - ResponseResult::WorktreeCreated { - workspace: self.workspace_info(ws_idx), - tab: self - .tab_info(ws_idx, 0) - .expect("new worktree workspace should have an initial tab"), - root_pane: self - .root_pane_info(ws_idx, 0) - .expect("new worktree workspace should have an initial root pane"), - worktree, - }, - ) - } - pub(super) fn handle_worktree_open( &mut self, id: String, @@ -263,106 +171,6 @@ impl App { ) } - pub(super) fn handle_worktree_remove( - &mut self, - id: String, - params: WorktreeRemoveParams, - ) -> String { - let Some(ws_idx) = self.parse_workspace_id(¶ms.workspace_id) else { - return encode_error( - id, - "workspace_not_found", - format!("workspace {} not found", params.workspace_id), - ); - }; - let Some(space) = self - .state - .workspaces - .get(ws_idx) - .and_then(|ws| ws.worktree_space().cloned()) - else { - return encode_error( - id, - "not_linked_worktree", - "workspace is not a Herdr-managed worktree checkout", - ); - }; - if !space.is_linked_worktree { - return encode_error( - id, - "not_linked_worktree", - "workspace is not a linked worktree checkout", - ); - } - - #[cfg(windows)] - { - if !params.force - && crate::worktree::checkout_has_dirty_files(&space.checkout_path).unwrap_or(false) - { - return encode_error( - id, - "dirty_worktree_requires_force", - crate::worktree::worktree_dirty_remove_message(&space.checkout_path), - ); - } - } - - #[cfg(windows)] - self.shutdown_workspace_terminal_runtimes_for_worktree_remove(ws_idx); - - let command = crate::worktree::build_worktree_remove_command( - &space.repo_root, - &space.checkout_path, - params.force, - ); - let workspace_snapshot = self.workspace_info(ws_idx); - let worktree = self.worktree_info_for_membership(&space, None); - if let Err(err) = crate::worktree::run_worktree_command(&command) { - let code = if !params.force && crate::worktree::is_dirty_worktree_remove_error(&err) { - "dirty_worktree_requires_force" - } else { - "worktree_remove_failed" - }; - return encode_error(id, code, err); - } - - let workspace_id = self.public_workspace_id(ws_idx); - let path = space.checkout_path.display().to_string(); - let still_same_linked_worktree = self.state.workspaces[ws_idx] - .worktree_space() - .is_some_and(|current| { - current.is_linked_worktree && current.checkout_path == space.checkout_path - }); - if still_same_linked_worktree { - self.state.selected = ws_idx; - self.state.close_selected_workspace(); - self.shutdown_detached_terminal_runtimes(); - self.emit_event(EventEnvelope { - event: EventKind::WorkspaceClosed, - data: EventData::WorkspaceClosed { - workspace_id: workspace_id.clone(), - workspace: Some(workspace_snapshot.clone()), - }, - }); - } - self.emit_worktree_removed_event( - workspace_id.clone(), - Some(workspace_snapshot), - worktree, - params.force, - ); - - encode_success( - id, - ResponseResult::WorktreeRemoved { - workspace_id, - path, - forced: params.force, - }, - ) - } - fn resolve_worktree_source( &mut self, workspace_id: Option, @@ -910,7 +718,15 @@ fn worktree_membership( #[cfg(test)] mod tests { use super::*; - use crate::api::schema::{ErrorResponse, Request, SuccessResponse}; + use std::time::{SystemTime, UNIX_EPOCH}; + + use crate::api::schema::{ + ErrorResponse, Request, SuccessResponse, WorktreeCreateParams, WorktreeRemoveParams, + }; + use crate::events::{ + ApiWorktreeAddRequest, ApiWorktreeRemoveRequest, AppEvent, WorktreeAddResult, + WorktreeRemoveResult, + }; use crate::{config::Config, workspace::Workspace}; fn unique_temp_path(name: &str) -> PathBuf { @@ -979,6 +795,74 @@ mod tests { app } + fn wait_for_app_event(app: &mut App) -> AppEvent { + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); + loop { + if let Ok(event) = app.event_rx.try_recv() { + return event; + } + assert!( + std::time::Instant::now() < deadline, + "timed out waiting for app event" + ); + std::thread::sleep(std::time::Duration::from_millis(20)); + } + } + + fn install_event_plugin(app: &mut App, name: &str, event: &str) -> PathBuf { + let plugin_root = unique_temp_path(name); + std::fs::create_dir_all(&plugin_root).unwrap(); + let manifest_path = plugin_root.join("herdr-plugin.toml"); + std::fs::write(&manifest_path, format!("id = 'example.{name}'\n")).unwrap(); + app.state.installed_plugins.insert( + format!("example.{name}"), + crate::api::schema::InstalledPluginInfo { + plugin_id: format!("example.{name}"), + name: name.into(), + version: "0.1.0".into(), + min_herdr_version: "0.7.0".into(), + description: None, + manifest_path: manifest_path.display().to_string(), + plugin_root: plugin_root.display().to_string(), + enabled: true, + platforms: None, + build: Vec::new(), + actions: Vec::new(), + events: vec![crate::api::schema::PluginManifestEventHook { + on: event.into(), + platforms: None, + command: vec!["sh".into(), "-c".into(), "true".into()], + }], + panes: Vec::new(), + link_handlers: Vec::new(), + source: crate::api::schema::PluginSourceInfo::default(), + warnings: Vec::new(), + }, + ); + plugin_root + } + + fn response_channel() -> ( + std::sync::mpsc::Sender, + std::sync::mpsc::Receiver, + ) { + std::sync::mpsc::channel() + } + + fn run_deferred_api_request(app: &mut App, request: Request) -> String { + let (respond_to, response_rx) = response_channel(); + assert!(app.handle_deferred_worktree_api_request(request, respond_to)); + if let Ok(response) = response_rx.recv_timeout(std::time::Duration::from_millis(50)) { + return response; + } + + let event = wait_for_app_event(app); + app.handle_internal_event(event); + response_rx + .recv_timeout(std::time::Duration::from_secs(2)) + .expect("deferred API request should respond after completion event") + } + #[tokio::test] async fn api_worktree_create_opens_workspace_and_marks_membership() { let repo = create_committed_repo("api-worktree-create-repo"); @@ -992,15 +876,19 @@ mod tests { app.state.active = Some(0); app.state.selected = 0; app.state.worktree_directory = worktree_root.clone(); + let workspace_id = app.state.workspaces[0].id.clone(); - let response = app.handle_api_request(Request { - id: "req".into(), - method: crate::api::schema::Method::WorktreeCreate(WorktreeCreateParams { - workspace_id: Some(app.state.workspaces[0].id.clone()), - branch: Some("worktree/api-create".into()), - ..WorktreeCreateParams::default() - }), - }); + let response = run_deferred_api_request( + &mut app, + Request { + id: "req".into(), + method: crate::api::schema::Method::WorktreeCreate(WorktreeCreateParams { + workspace_id: Some(workspace_id), + branch: Some("worktree/api-create".into()), + ..WorktreeCreateParams::default() + }), + }, + ); let success: SuccessResponse = serde_json::from_str(&response).unwrap(); let ResponseResult::WorktreeCreated { @@ -1073,6 +961,205 @@ mod tests { let _ = std::fs::remove_dir_all(repo); } + #[tokio::test] + async fn deferred_api_worktree_create_preserves_event_and_plugin_context() { + let repo = create_committed_repo("api-worktree-create-deferred-repo"); + let worktree_root = unique_temp_path("api-worktree-create-deferred-root"); + let event_hub = crate::api::EventHub::default(); + let mut app = test_app_with_event_hub(event_hub.clone()); + let mut parent = Workspace::test_new("main"); + parent.identity_cwd = repo.clone(); + app.state.workspaces = vec![parent]; + app.state.ensure_test_terminals(); + app.state.active = Some(0); + app.state.selected = 0; + app.state.worktree_directory = worktree_root.clone(); + let plugin_root = install_event_plugin(&mut app, "deferred-create", "worktree.created"); + let (respond_to, response_rx) = response_channel(); + + assert!(app.handle_deferred_worktree_api_request( + Request { + id: "req".into(), + method: crate::api::schema::Method::WorktreeCreate(WorktreeCreateParams { + workspace_id: Some(app.state.workspaces[0].id.clone()), + branch: Some("worktree/api-create-deferred".into()), + ..WorktreeCreateParams::default() + }), + }, + respond_to, + )); + assert!(response_rx.try_recv().is_err()); + + let event = wait_for_app_event(&mut app); + app.handle_internal_event(event); + let response = response_rx + .recv_timeout(std::time::Duration::from_secs(2)) + .expect("deferred worktree create should respond after completion event"); + let success: SuccessResponse = serde_json::from_str(&response).unwrap(); + let ResponseResult::WorktreeCreated { + workspace, + worktree, + .. + } = success.result + else { + panic!("expected worktree_created response"); + }; + let event_kinds = event_hub + .events_after(0) + .into_iter() + .map(|(_, event)| event.event) + .collect::>(); + assert_eq!( + &event_kinds[event_kinds.len() - 4..], + &[ + EventKind::WorkspaceCreated, + EventKind::TabCreated, + EventKind::PaneCreated, + EventKind::WorktreeCreated, + ] + ); + assert_eq!( + workspace + .worktree + .as_ref() + .map(|worktree| worktree.checkout_path.as_str()), + Some(worktree.path.as_str()) + ); + assert!(app.state.plugin_command_logs.iter().any(|log| { + log.event.as_deref() == Some("worktree.created") + && log.status == crate::api::schema::PluginCommandStatus::Running + })); + + for (_, runtime) in app.terminal_runtimes.drain() { + runtime.shutdown(); + } + let _ = std::fs::remove_dir_all(worktree_root); + let _ = std::fs::remove_dir_all(repo); + let _ = std::fs::remove_dir_all(plugin_root); + } + + #[test] + fn deferred_api_worktree_create_failure_clears_pending_checkout() { + let repo = create_committed_repo("api-worktree-create-failure-repo"); + let worktree_root = unique_temp_path("api-worktree-create-failure-root"); + let branch = "foo"; + run_git(&repo, &["branch", branch]); + let mut app = test_app(); + let mut parent = Workspace::test_new("main"); + parent.identity_cwd = repo.clone(); + let parent_id = parent.id.clone(); + app.state.workspaces = vec![parent]; + app.state.ensure_test_terminals(); + app.state.worktree_directory = worktree_root.clone(); + let request = || Request { + id: "req".into(), + method: crate::api::schema::Method::WorktreeCreate(WorktreeCreateParams { + workspace_id: Some(parent_id.clone()), + branch: Some(branch.into()), + ..WorktreeCreateParams::default() + }), + }; + + let (first_tx, first_rx) = response_channel(); + assert!(app.handle_deferred_worktree_api_request(request(), first_tx)); + let event = wait_for_app_event(&mut app); + app.handle_internal_event(event); + let response = first_rx + .recv_timeout(std::time::Duration::from_secs(2)) + .expect("failed create should respond"); + let error: ErrorResponse = serde_json::from_str(&response).unwrap(); + assert_eq!(error.error.code, "worktree_create_failed"); + assert!(error + .error + .message + .contains("fatal: a branch named 'foo' already exists")); + assert!(app.pending_api_worktree_creates.is_empty()); + + let (second_tx, second_rx) = response_channel(); + assert!(app.handle_deferred_worktree_api_request(request(), second_tx)); + assert!(second_rx.try_recv().is_err()); + let event = wait_for_app_event(&mut app); + app.handle_internal_event(event); + let response = second_rx + .recv_timeout(std::time::Duration::from_secs(2)) + .expect("retry should reach git instead of pending guard"); + let error: ErrorResponse = serde_json::from_str(&response).unwrap(); + assert_eq!(error.error.code, "worktree_create_failed"); + assert_ne!(error.error.code, "worktree_operation_in_progress"); + + let _ = std::fs::remove_dir_all(worktree_root); + let _ = std::fs::remove_dir_all(repo); + } + + #[tokio::test] + async fn deferred_api_worktree_create_completes_after_source_workspace_changes() { + let event_hub = crate::api::EventHub::default(); + let mut app = test_app_with_event_hub(event_hub.clone()); + let repo = create_committed_repo("api-worktree-create-changed-source-repo"); + let checkout = unique_temp_path("api-worktree-create-changed-source-checkout"); + std::fs::create_dir_all(&checkout).unwrap(); + let checkout_key = crate::worktree::canonical_or_original(&checkout); + let mut source = Workspace::test_new("source"); + source.identity_cwd = repo.clone(); + let source_id = source.id.clone(); + app.state.workspaces = vec![source]; + app.pending_api_worktree_creates + .insert(checkout_key.clone(), 9); + app.state.workspaces[0].worktree_space = Some(crate::workspace::WorktreeSpaceMembership { + key: "other-key".into(), + label: "other".into(), + repo_root: "/repo/other".into(), + checkout_path: "/repo/other".into(), + is_linked_worktree: false, + }); + let (respond_to, response_rx) = response_channel(); + + app.handle_api_worktree_add_finished(WorktreeAddResult { + path: checkout.clone(), + api_request: Some(ApiWorktreeAddRequest { + id: "req".into(), + operation_id: 9, + checkout_key, + source_workspace_id: Some(source_id), + source_existing_membership: None, + source_checkout_path: repo.clone(), + source_repo_root: repo.clone(), + repo_key: "repo-key".into(), + repo_name: "herdr".into(), + label: None, + focus: false, + respond_to, + }), + result: Ok(()), + }); + + let response = response_rx + .recv_timeout(std::time::Duration::from_secs(2)) + .expect("changed-source create completion should respond"); + let success: SuccessResponse = serde_json::from_str(&response).unwrap(); + assert!(matches!( + success.result, + ResponseResult::WorktreeCreated { .. } + )); + assert!(event_hub + .events_after(0) + .into_iter() + .any(|(_, event)| event.event == EventKind::WorktreeCreated)); + assert_eq!(app.state.workspaces.len(), 3); + assert_eq!( + app.state.workspaces[0] + .worktree_space() + .map(|membership| membership.label.as_str()), + Some("other") + ); + + for (_, runtime) in app.terminal_runtimes.drain() { + runtime.shutdown(); + } + let _ = std::fs::remove_dir_all(checkout); + let _ = std::fs::remove_dir_all(repo); + } + #[tokio::test] async fn api_worktree_create_from_cwd_emits_parent_with_membership() { let repo = create_committed_repo("api-worktree-create-cwd-repo"); @@ -1082,14 +1169,17 @@ mod tests { app.state.worktree_directory = worktree_root.clone(); app.state.default_shell = test_shell().into(); - let response = app.handle_api_request(Request { - id: "req".into(), - method: crate::api::schema::Method::WorktreeCreate(WorktreeCreateParams { - cwd: Some(repo.display().to_string()), - branch: Some("worktree/api-create-cwd".into()), - ..WorktreeCreateParams::default() - }), - }); + let response = run_deferred_api_request( + &mut app, + Request { + id: "req".into(), + method: crate::api::schema::Method::WorktreeCreate(WorktreeCreateParams { + cwd: Some(repo.display().to_string()), + branch: Some("worktree/api-create-cwd".into()), + ..WorktreeCreateParams::default() + }), + }, + ); let success: SuccessResponse = serde_json::from_str(&response).unwrap(); let ResponseResult::WorktreeCreated { worktree, .. } = success.result else { panic!("expected worktree_created response"); @@ -1128,14 +1218,17 @@ mod tests { let repo = create_committed_repo("api-worktree-create-invalid-cwd-repo"); let mut app = test_app(); - let response = app.handle_api_request(Request { - id: "req".into(), - method: crate::api::schema::Method::WorktreeCreate(WorktreeCreateParams { - cwd: Some(repo.display().to_string()), - branch: Some(" ".into()), - ..WorktreeCreateParams::default() - }), - }); + let response = run_deferred_api_request( + &mut app, + Request { + id: "req".into(), + method: crate::api::schema::Method::WorktreeCreate(WorktreeCreateParams { + cwd: Some(repo.display().to_string()), + branch: Some(" ".into()), + ..WorktreeCreateParams::default() + }), + }, + ); let error: ErrorResponse = serde_json::from_str(&response).unwrap(); assert_eq!(error.error.code, "invalid_request"); @@ -1168,16 +1261,20 @@ mod tests { fn raw_api_worktree_create_rejects_relative_path_override() { let repo = create_committed_repo("api-worktree-relative-path-repo"); let mut app = app_with_parent(&repo); + let workspace_id = app.state.workspaces[0].id.clone(); - let response = app.handle_api_request(Request { - id: "req".into(), - method: crate::api::schema::Method::WorktreeCreate(WorktreeCreateParams { - workspace_id: Some(app.state.workspaces[0].id.clone()), - branch: Some("worktree/relative".into()), - path: Some("relative-checkout".into()), - ..WorktreeCreateParams::default() - }), - }); + let response = run_deferred_api_request( + &mut app, + Request { + id: "req".into(), + method: crate::api::schema::Method::WorktreeCreate(WorktreeCreateParams { + workspace_id: Some(workspace_id), + branch: Some("worktree/relative".into()), + path: Some("relative-checkout".into()), + ..WorktreeCreateParams::default() + }), + }, + ); let error: ErrorResponse = serde_json::from_str(&response).unwrap(); assert_eq!(error.error.code, "invalid_request"); @@ -1189,14 +1286,17 @@ mod tests { fn raw_api_worktree_create_rejects_relative_cwd() { let mut app = test_app(); - let response = app.handle_api_request(Request { - id: "req".into(), - method: crate::api::schema::Method::WorktreeCreate(WorktreeCreateParams { - cwd: Some("relative-repo".into()), - branch: Some("worktree/relative-cwd".into()), - ..WorktreeCreateParams::default() - }), - }); + let response = run_deferred_api_request( + &mut app, + Request { + id: "req".into(), + method: crate::api::schema::Method::WorktreeCreate(WorktreeCreateParams { + cwd: Some("relative-repo".into()), + branch: Some("worktree/relative-cwd".into()), + ..WorktreeCreateParams::default() + }), + }, + ); let error: ErrorResponse = serde_json::from_str(&response).unwrap(); assert_eq!(error.error.code, "invalid_request"); @@ -1595,25 +1695,31 @@ mod tests { app.state.workspaces.push(child); app.state.ensure_test_terminals(); - let response = app.handle_api_request(Request { - id: "req".into(), - method: crate::api::schema::Method::WorktreeRemove(WorktreeRemoveParams { - workspace_id: child_id.clone(), - force: false, - }), - }); + let response = run_deferred_api_request( + &mut app, + Request { + id: "req".into(), + method: crate::api::schema::Method::WorktreeRemove(WorktreeRemoveParams { + workspace_id: child_id.clone(), + force: false, + }), + }, + ); let error: ErrorResponse = serde_json::from_str(&response).unwrap(); assert_eq!(error.error.code, "dirty_worktree_requires_force"); assert!(checkout.exists()); assert_eq!(app.state.workspaces.len(), 2); - let response = app.handle_api_request(Request { - id: "req".into(), - method: crate::api::schema::Method::WorktreeRemove(WorktreeRemoveParams { - workspace_id: child_id, - force: true, - }), - }); + let response = run_deferred_api_request( + &mut app, + Request { + id: "req".into(), + method: crate::api::schema::Method::WorktreeRemove(WorktreeRemoveParams { + workspace_id: child_id, + force: true, + }), + }, + ); let success: SuccessResponse = serde_json::from_str(&response).unwrap(); let ResponseResult::WorktreeRemoved { forced, path, .. } = success.result else { panic!("expected worktree_removed response"); @@ -1660,13 +1766,16 @@ mod tests { app.state.active = Some(0); app.state.selected = 0; - let response = app.handle_api_request(Request { - id: "req".into(), - method: crate::api::schema::Method::WorktreeRemove(WorktreeRemoveParams { - workspace_id: child_id.clone(), - force: false, - }), - }); + let response = run_deferred_api_request( + &mut app, + Request { + id: "req".into(), + method: crate::api::schema::Method::WorktreeRemove(WorktreeRemoveParams { + workspace_id: child_id.clone(), + force: false, + }), + }, + ); let success: SuccessResponse = serde_json::from_str(&response).unwrap(); assert!(matches!( success.result, @@ -1706,4 +1815,373 @@ mod tests { let _ = std::fs::remove_dir_all(repo); } + + #[test] + fn deferred_api_worktree_remove_preserves_event_and_plugin_context() { + let repo = create_committed_repo("api-worktree-remove-deferred-repo"); + let checkout = unique_temp_path("api-worktree-remove-deferred-checkout"); + run_git( + &repo, + &[ + "worktree", + "add", + "--quiet", + "-b", + "worktree/api-remove-deferred", + checkout.to_str().unwrap(), + "HEAD", + ], + ); + + let event_hub = crate::api::EventHub::default(); + let mut app = test_app_with_event_hub(event_hub.clone()); + let mut child = Workspace::test_new("child"); + child.identity_cwd = checkout.clone(); + child.worktree_space = Some(crate::workspace::WorktreeSpaceMembership { + key: crate::workspace::git_space_metadata(&repo).unwrap().key, + label: "api-worktree-remove-deferred-repo".into(), + repo_root: repo.clone(), + checkout_path: checkout.clone(), + is_linked_worktree: true, + }); + let child_id = child.id.clone(); + app.state.workspaces.push(child); + app.state.ensure_test_terminals(); + app.state.active = Some(0); + app.state.selected = 0; + let plugin_root = install_event_plugin(&mut app, "deferred-remove", "worktree.removed"); + let (respond_to, response_rx) = response_channel(); + + assert!(app.handle_deferred_worktree_api_request( + Request { + id: "req".into(), + method: crate::api::schema::Method::WorktreeRemove(WorktreeRemoveParams { + workspace_id: child_id.clone(), + force: false, + }), + }, + respond_to, + )); + assert!(response_rx.try_recv().is_err()); + + let event = wait_for_app_event(&mut app); + app.handle_internal_event(event); + let response = response_rx + .recv_timeout(std::time::Duration::from_secs(2)) + .expect("deferred worktree remove should respond after completion event"); + let success: SuccessResponse = serde_json::from_str(&response).unwrap(); + assert!(matches!( + success.result, + ResponseResult::WorktreeRemoved { .. } + )); + assert_eq!( + event_hub + .events_after(0) + .into_iter() + .map(|(_, event)| event.event) + .collect::>(), + vec![EventKind::WorkspaceClosed, EventKind::WorktreeRemoved] + ); + assert!(app.state.plugin_command_logs.iter().any(|log| { + log.event.as_deref() == Some("worktree.removed") + && log.status == crate::api::schema::PluginCommandStatus::Running + })); + assert!(app.state.workspaces.is_empty()); + + let _ = std::fs::remove_dir_all(repo); + let _ = std::fs::remove_dir_all(plugin_root); + } + + #[test] + fn deferred_api_worktree_remove_rejects_duplicate_in_flight_request() { + let repo = create_committed_repo("api-worktree-remove-duplicate-repo"); + let checkout = unique_temp_path("api-worktree-remove-duplicate-checkout"); + run_git( + &repo, + &[ + "worktree", + "add", + "--quiet", + "-b", + "worktree/api-remove-duplicate", + checkout.to_str().unwrap(), + "HEAD", + ], + ); + + let mut app = test_app(); + let mut child = Workspace::test_new("child"); + child.identity_cwd = checkout.clone(); + child.worktree_space = Some(crate::workspace::WorktreeSpaceMembership { + key: crate::workspace::git_space_metadata(&repo).unwrap().key, + label: "api-worktree-remove-duplicate-repo".into(), + repo_root: repo.clone(), + checkout_path: checkout.clone(), + is_linked_worktree: true, + }); + let child_id = child.id.clone(); + app.state.workspaces.push(child); + app.state.ensure_test_terminals(); + let (first_tx, _first_rx) = response_channel(); + let (second_tx, second_rx) = response_channel(); + + assert!(app.handle_deferred_worktree_api_request( + Request { + id: "first".into(), + method: crate::api::schema::Method::WorktreeRemove(WorktreeRemoveParams { + workspace_id: child_id.clone(), + force: false, + }), + }, + first_tx, + )); + assert!(app.handle_deferred_worktree_api_request( + Request { + id: "second".into(), + method: crate::api::schema::Method::WorktreeRemove(WorktreeRemoveParams { + workspace_id: child_id, + force: false, + }), + }, + second_tx, + )); + let response = second_rx + .recv_timeout(std::time::Duration::from_secs(2)) + .expect("duplicate request should respond immediately"); + let error: ErrorResponse = serde_json::from_str(&response).unwrap(); + assert_eq!(error.error.code, "worktree_operation_in_progress"); + + let event = wait_for_app_event(&mut app); + app.handle_internal_event(event); + let _ = std::fs::remove_dir_all(repo); + } + + #[test] + fn deferred_api_worktree_remove_rejects_duplicate_checkout_path_in_flight_request() { + let repo = create_committed_repo("api-worktree-remove-duplicate-path-repo"); + let checkout = unique_temp_path("api-worktree-remove-duplicate-path-checkout"); + run_git( + &repo, + &[ + "worktree", + "add", + "--quiet", + "-b", + "worktree/api-remove-duplicate-path", + checkout.to_str().unwrap(), + "HEAD", + ], + ); + + let mut app = test_app(); + let membership = crate::workspace::WorktreeSpaceMembership { + key: crate::workspace::git_space_metadata(&repo).unwrap().key, + label: "api-worktree-remove-duplicate-path-repo".into(), + repo_root: repo.clone(), + checkout_path: checkout.clone(), + is_linked_worktree: true, + }; + let mut first = Workspace::test_new("first"); + first.identity_cwd = checkout.clone(); + first.worktree_space = Some(membership.clone()); + let first_id = first.id.clone(); + let mut second = Workspace::test_new("second"); + second.identity_cwd = checkout.clone(); + second.worktree_space = Some(membership); + let second_id = second.id.clone(); + app.state.workspaces = vec![first, second]; + app.state.ensure_test_terminals(); + let (first_tx, _first_rx) = response_channel(); + let (second_tx, second_rx) = response_channel(); + + assert!(app.handle_deferred_worktree_api_request( + Request { + id: "first".into(), + method: crate::api::schema::Method::WorktreeRemove(WorktreeRemoveParams { + workspace_id: first_id, + force: true, + }), + }, + first_tx, + )); + assert!(app.handle_deferred_worktree_api_request( + Request { + id: "second".into(), + method: crate::api::schema::Method::WorktreeRemove(WorktreeRemoveParams { + workspace_id: second_id, + force: true, + }), + }, + second_tx, + )); + let response = second_rx + .recv_timeout(std::time::Duration::from_secs(2)) + .expect("duplicate path request should respond immediately"); + let error: ErrorResponse = serde_json::from_str(&response).unwrap(); + assert_eq!(error.error.code, "worktree_operation_in_progress"); + + let event = wait_for_app_event(&mut app); + app.handle_internal_event(event); + let _ = std::fs::remove_dir_all(repo); + } + + #[test] + fn deferred_api_worktree_create_rejects_checkout_with_remove_in_flight() { + let repo = create_committed_repo("api-worktree-create-remove-in-flight-repo"); + let checkout = unique_temp_path("api-worktree-create-remove-in-flight-checkout"); + let mut app = test_app(); + app.pending_api_worktree_remove_paths + .insert(crate::worktree::canonical_or_original(&checkout), 7); + let (respond_to, response_rx) = response_channel(); + + assert!(app.handle_deferred_worktree_api_request( + Request { + id: "req".into(), + method: crate::api::schema::Method::WorktreeCreate(WorktreeCreateParams { + workspace_id: None, + cwd: Some(repo.display().to_string()), + branch: Some("worktree/create-remove-in-flight".into()), + base: None, + path: Some(checkout.display().to_string()), + label: None, + focus: false, + }), + }, + respond_to, + )); + + let response = response_rx + .recv_timeout(std::time::Duration::from_secs(2)) + .expect("create should reject checkout with remove in flight"); + let error: ErrorResponse = serde_json::from_str(&response).unwrap(); + assert_eq!(error.error.code, "worktree_operation_in_progress"); + assert!(app.event_rx.try_recv().is_err()); + let _ = std::fs::remove_dir_all(repo); + } + + #[test] + fn deferred_api_worktree_remove_rejects_checkout_with_create_in_flight() { + let repo = create_committed_repo("api-worktree-remove-create-in-flight-repo"); + let checkout = unique_temp_path("api-worktree-remove-create-in-flight-checkout"); + run_git( + &repo, + &[ + "worktree", + "add", + "--quiet", + "-b", + "worktree/api-remove-create-in-flight", + checkout.to_str().unwrap(), + "HEAD", + ], + ); + + let mut app = test_app(); + let mut child = Workspace::test_new("child"); + child.identity_cwd = checkout.clone(); + child.worktree_space = Some(crate::workspace::WorktreeSpaceMembership { + key: crate::workspace::git_space_metadata(&repo).unwrap().key, + label: "api-worktree-remove-create-in-flight-repo".into(), + repo_root: repo.clone(), + checkout_path: checkout.clone(), + is_linked_worktree: true, + }); + let child_id = child.id.clone(); + app.state.workspaces.push(child); + app.state.ensure_test_terminals(); + app.pending_api_worktree_creates + .insert(crate::worktree::canonical_or_original(&checkout), 7); + let (respond_to, response_rx) = response_channel(); + + assert!(app.handle_deferred_worktree_api_request( + Request { + id: "req".into(), + method: crate::api::schema::Method::WorktreeRemove(WorktreeRemoveParams { + workspace_id: child_id, + force: false, + }), + }, + respond_to, + )); + + let response = response_rx + .recv_timeout(std::time::Duration::from_secs(2)) + .expect("remove should reject checkout with create in flight"); + let error: ErrorResponse = serde_json::from_str(&response).unwrap(); + assert_eq!(error.error.code, "worktree_operation_in_progress"); + assert!(app.event_rx.try_recv().is_err()); + let remove = crate::worktree::build_worktree_remove_command(&repo, &checkout, true); + let _ = crate::worktree::run_worktree_command(&remove); + let _ = std::fs::remove_dir_all(repo); + } + + #[test] + fn deferred_api_worktree_remove_emits_removed_after_workspace_changes() { + let event_hub = crate::api::EventHub::default(); + let mut app = test_app_with_event_hub(event_hub.clone()); + let checkout = PathBuf::from("/repo/herdr-issue"); + let mut child = Workspace::test_new("child"); + child.worktree_space = Some(crate::workspace::WorktreeSpaceMembership { + key: "repo-key".into(), + label: "herdr".into(), + repo_root: "/repo/herdr".into(), + checkout_path: checkout.clone(), + is_linked_worktree: true, + }); + let child_id = child.id.clone(); + app.state.workspaces.push(child); + let workspace_snapshot = app.workspace_info(0); + let worktree_snapshot = app + .worktree_info_for_membership(app.state.workspaces[0].worktree_space().unwrap(), None); + app.pending_api_worktree_removes.insert(child_id.clone(), 7); + app.pending_api_worktree_remove_paths + .insert(crate::worktree::canonical_or_original(&checkout), 7); + app.state.workspaces[0].worktree_space = Some(crate::workspace::WorktreeSpaceMembership { + key: "repo-key".into(), + label: "herdr".into(), + repo_root: "/repo/herdr".into(), + checkout_path: "/repo/other".into(), + is_linked_worktree: true, + }); + let (respond_to, response_rx) = response_channel(); + + app.handle_api_worktree_remove_finished(WorktreeRemoveResult { + workspace_id: child_id, + path: checkout.clone(), + workspace: Some(Box::new(workspace_snapshot)), + worktree: Some(Box::new(worktree_snapshot)), + forced: true, + api_request: Some(ApiWorktreeRemoveRequest { + id: "req".into(), + operation_id: 7, + checkout_key: crate::worktree::canonical_or_original(&checkout), + respond_to, + }), + result: Ok(()), + }); + + let response = response_rx + .recv_timeout(std::time::Duration::from_secs(2)) + .expect("changed-workspace remove completion should respond"); + let success: SuccessResponse = serde_json::from_str(&response).unwrap(); + assert!(matches!( + success.result, + ResponseResult::WorktreeRemoved { .. } + )); + assert_eq!( + event_hub + .events_after(0) + .into_iter() + .map(|(_, event)| event.event) + .collect::>(), + vec![EventKind::WorktreeRemoved] + ); + assert_eq!(app.state.workspaces.len(), 1); + assert_eq!( + app.state.workspaces[0] + .worktree_space() + .map(|membership| membership.checkout_path.as_path()), + Some(Path::new("/repo/other")) + ); + } } diff --git a/src/app/api/worktrees/deferred.rs b/src/app/api/worktrees/deferred.rs new file mode 100644 index 00000000..19794a6e --- /dev/null +++ b/src/app/api/worktrees/deferred.rs @@ -0,0 +1,566 @@ +use std::path::Path; +use std::time::{SystemTime, UNIX_EPOCH}; + +use crate::api::schema::{ + EventData, EventEnvelope, EventKind, Request, ResponseResult, WorktreeCreateParams, + WorktreeRemoveParams, +}; +use crate::app::App; +use crate::events::{ApiWorktreeAddRequest, ApiWorktreeRemoveRequest, AppEvent}; + +use super::super::responses::{encode_error, encode_success}; +use super::{absolute_user_path, WorktreeSource}; + +impl App { + pub(crate) fn handle_deferred_worktree_api_request( + &mut self, + request: Request, + respond_to: std::sync::mpsc::Sender, + ) -> bool { + match request.method { + crate::api::schema::Method::WorktreeCreate(params) => { + self.start_api_worktree_create(request.id, params, respond_to); + true + } + crate::api::schema::Method::WorktreeRemove(params) => { + self.start_api_worktree_remove(request.id, params, respond_to); + true + } + _ => false, + } + } + + fn send_api_response(respond_to: std::sync::mpsc::Sender, response: String) { + let _ = respond_to.send(response); + } + + fn next_api_worktree_operation_id(&mut self) -> u64 { + let id = self.next_api_worktree_operation_id; + self.next_api_worktree_operation_id = self.next_api_worktree_operation_id.saturating_add(1); + id + } + + fn api_create_source_workspace_idx(&self, api: &ApiWorktreeAddRequest) -> Option { + let Some(source_workspace_id) = api.source_workspace_id.as_ref() else { + return self.find_parent_workspace_by_key(&api.repo_key); + }; + let Some(ws_idx) = self + .state + .workspaces + .iter() + .position(|ws| &ws.id == source_workspace_id) + else { + return self.find_parent_workspace_by_key(&api.repo_key); + }; + let workspace = &self.state.workspaces[ws_idx]; + if let Some(expected) = api.source_existing_membership.as_ref() { + if workspace.worktree_space() == Some(expected) { + return Some(ws_idx); + } + return self.find_parent_workspace_by_key(&api.repo_key); + } + + if let Some(current) = workspace.worktree_space() { + let expected = crate::workspace::WorktreeSpaceMembership { + key: api.repo_key.clone(), + label: api.repo_name.clone(), + repo_root: api.source_repo_root.clone(), + checkout_path: api.source_checkout_path.clone(), + is_linked_worktree: false, + }; + if current == &expected { + return Some(ws_idx); + } + return self.find_parent_workspace_by_key(&api.repo_key); + } + let git_space = workspace.git_space().cloned().or_else(|| { + workspace + .resolved_identity_cwd_from(&self.state.terminals, &self.terminal_runtimes) + .as_deref() + .and_then(crate::workspace::git_space_metadata) + }); + if git_space.is_some_and(|space| { + !space.is_linked_worktree + && space.key == api.repo_key + && crate::worktree::canonical_or_original(&space.repo_root) + == crate::worktree::canonical_or_original(&api.source_repo_root) + }) { + Some(ws_idx) + } else { + self.find_parent_workspace_by_key(&api.repo_key) + } + } + + fn start_api_worktree_create( + &mut self, + id: String, + params: WorktreeCreateParams, + respond_to: std::sync::mpsc::Sender, + ) { + let branch = params + .branch + .unwrap_or_else(|| { + let seed = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|duration| duration.as_micros().min(u128::from(u64::MAX)) as u64) + .unwrap_or(0); + crate::worktree::generated_branch_slug(seed) + }) + .trim() + .to_string(); + if branch.is_empty() { + Self::send_api_response( + respond_to, + encode_error(id, "invalid_request", "branch is required"), + ); + return; + } + let base = params.base.unwrap_or_else(|| "HEAD".into()); + let source = match self.resolve_worktree_source(params.workspace_id, params.cwd) { + Ok(source) => source, + Err(err) => { + Self::send_api_response(respond_to, encode_error(id, err.code, err.message)); + return; + } + }; + let checkout_path = match params.path { + Some(path) => match absolute_user_path(&path) { + Ok(path) => path, + Err(err) => { + Self::send_api_response(respond_to, encode_error(id, err.code, err.message)); + return; + } + }, + None => crate::worktree::default_checkout_path( + &self.state.worktree_directory, + &source.repo_name, + &branch, + ), + }; + let checkout_key = crate::worktree::canonical_or_original(&checkout_path); + if self + .pending_api_worktree_creates + .contains_key(&checkout_key) + || self + .pending_api_worktree_remove_paths + .contains_key(&checkout_key) + { + Self::send_api_response( + respond_to, + encode_error( + id, + "worktree_operation_in_progress", + "worktree operation is already in progress for this checkout", + ), + ); + return; + } + let operation_id = self.next_api_worktree_operation_id(); + self.pending_api_worktree_creates + .insert(checkout_key.clone(), operation_id); + + let command = crate::worktree::build_worktree_add_new_branch_command( + &source.source_checkout_path, + &checkout_path, + &branch, + &base, + ); + let parent_dir = checkout_path.parent().map(Path::to_path_buf); + let source_workspace_id = source + .workspace_idx + .and_then(|idx| self.state.workspaces.get(idx).map(|ws| ws.id.clone())); + let source_existing_membership = source_workspace_id.as_ref().and_then(|workspace_id| { + self.state + .workspaces + .iter() + .find(|ws| &ws.id == workspace_id) + .and_then(|ws| ws.worktree_space().cloned()) + }); + let api_request = ApiWorktreeAddRequest { + id, + operation_id, + checkout_key, + source_workspace_id, + source_existing_membership, + source_checkout_path: source.source_checkout_path, + source_repo_root: source.source_repo_root, + repo_key: source.repo_key, + repo_name: source.repo_name, + label: params.label, + focus: params.focus, + respond_to, + }; + let path = checkout_path; + let event_tx = self.event_tx.clone(); + std::thread::spawn(move || { + let result = if let Some(parent_dir) = parent_dir { + std::fs::create_dir_all(&parent_dir) + .map_err(|err| err.to_string()) + .and_then(|()| crate::worktree::run_worktree_command(&command)) + } else { + crate::worktree::run_worktree_command(&command) + }; + let _ = event_tx.blocking_send(AppEvent::WorktreeAddFinished(Box::new( + crate::events::WorktreeAddResult { + path, + api_request: Some(api_request), + result, + }, + ))); + }); + } + + fn start_api_worktree_remove( + &mut self, + id: String, + params: WorktreeRemoveParams, + respond_to: std::sync::mpsc::Sender, + ) { + let Some(ws_idx) = self.parse_workspace_id(¶ms.workspace_id) else { + Self::send_api_response( + respond_to, + encode_error( + id, + "workspace_not_found", + format!("workspace {} not found", params.workspace_id), + ), + ); + return; + }; + let Some(space) = self + .state + .workspaces + .get(ws_idx) + .and_then(|ws| ws.worktree_space().cloned()) + else { + Self::send_api_response( + respond_to, + encode_error( + id, + "not_linked_worktree", + "workspace is not a Herdr-managed worktree checkout", + ), + ); + return; + }; + if !space.is_linked_worktree { + Self::send_api_response( + respond_to, + encode_error( + id, + "not_linked_worktree", + "workspace is not a linked worktree checkout", + ), + ); + return; + } + + #[cfg(windows)] + { + if !params.force + && crate::worktree::checkout_has_dirty_files(&space.checkout_path).unwrap_or(false) + { + Self::send_api_response( + respond_to, + encode_error( + id, + "dirty_worktree_requires_force", + crate::worktree::worktree_dirty_remove_message(&space.checkout_path), + ), + ); + return; + } + } + + let workspace_internal_id = self.state.workspaces[ws_idx].id.clone(); + let checkout_key = crate::worktree::canonical_or_original(&space.checkout_path); + if self + .pending_api_worktree_removes + .contains_key(&workspace_internal_id) + || self + .pending_api_worktree_remove_paths + .contains_key(&checkout_key) + || self + .pending_api_worktree_creates + .contains_key(&checkout_key) + { + Self::send_api_response( + respond_to, + encode_error( + id, + "worktree_operation_in_progress", + "worktree operation is already in progress for this checkout", + ), + ); + return; + } + + if Self::should_shutdown_workspace_terminal_runtimes_for_worktree_remove(params.force) { + self.shutdown_workspace_terminal_runtimes_for_worktree_remove(ws_idx); + } + + let operation_id = self.next_api_worktree_operation_id(); + self.pending_api_worktree_removes + .insert(workspace_internal_id.clone(), operation_id); + self.pending_api_worktree_remove_paths + .insert(checkout_key.clone(), operation_id); + let workspace_snapshot = self.workspace_info(ws_idx); + let worktree = self.worktree_info_for_membership(&space, None); + let command = crate::worktree::build_worktree_remove_command( + &space.repo_root, + &space.checkout_path, + params.force, + ); + let api_request = ApiWorktreeRemoveRequest { + id, + operation_id, + checkout_key, + respond_to, + }; + let repo_root = space.repo_root; + let path = space.checkout_path; + let force = params.force; + let event_tx = self.event_tx.clone(); + std::thread::spawn(move || { + let result = crate::worktree::run_worktree_remove_command_with_recovery( + &command, &repo_root, &path, force, + ); + let _ = event_tx.blocking_send(AppEvent::WorktreeRemoveFinished(Box::new( + crate::events::WorktreeRemoveResult { + workspace_id: workspace_internal_id, + path, + workspace: Some(Box::new(workspace_snapshot)), + worktree: Some(Box::new(worktree)), + forced: force, + api_request: Some(api_request), + result, + }, + ))); + }); + } + + pub(crate) fn handle_api_worktree_add_finished( + &mut self, + mut result: crate::events::WorktreeAddResult, + ) { + let Some(api) = result.api_request.take() else { + return; + }; + let checkout_key = api.checkout_key.clone(); + let operation_matches = self + .pending_api_worktree_creates + .get(&checkout_key) + .is_some_and(|operation_id| *operation_id == api.operation_id); + if !operation_matches { + Self::send_api_response( + api.respond_to, + encode_error( + api.id, + "stale_worktree_operation", + "worktree create completed after the operation was superseded", + ), + ); + return; + } + self.pending_api_worktree_creates.remove(&checkout_key); + + if let Err(err) = result.result { + Self::send_api_response( + api.respond_to, + encode_error(api.id, "worktree_create_failed", err), + ); + return; + } + + let source_workspace_idx = self.api_create_source_workspace_idx(&api); + let mut source = WorktreeSource { + workspace_idx: source_workspace_idx, + source_checkout_path: api.source_checkout_path, + source_repo_root: api.source_repo_root, + repo_key: api.repo_key, + repo_name: api.repo_name, + }; + if let Err(err) = self.ensure_source_parent_membership(&mut source, true) { + Self::send_api_response(api.respond_to, encode_error(api.id, err.code, err.message)); + return; + } + + let (ws_idx, created_workspace) = + if let Some(ws_idx) = self.open_workspace_idx_for_checkout(&result.path) { + if api.focus { + self.state.switch_workspace(ws_idx); + } + (ws_idx, false) + } else { + match self.create_workspace_with_options(result.path.clone(), api.focus) { + Ok(ws_idx) => (ws_idx, true), + Err(err) => { + Self::send_api_response( + api.respond_to, + encode_error( + api.id, + "worktree_open_failed", + format!("created worktree but failed to open workspace: {err}"), + ), + ); + return; + } + } + }; + + self.mark_worktree_membership( + &source, + ws_idx, + result.path.clone(), + true, + !created_workspace, + ); + if let Some(label) = api.label { + if let Some(ws) = self.state.workspaces.get_mut(ws_idx) { + ws.set_custom_name(label); + } + } + self.state.mark_session_dirty(); + if created_workspace { + self.emit_workspace_open_events(ws_idx); + } + let Some(worktree) = self.worktree_info_for_workspace(ws_idx) else { + Self::send_api_response( + api.respond_to, + encode_error( + api.id, + "worktree_open_failed", + "created worktree but failed to record workspace membership", + ), + ); + return; + }; + self.emit_worktree_created_event(ws_idx, worktree.clone()); + let tab_idx = self.state.workspaces[ws_idx].active_tab; + let response = encode_success( + api.id, + ResponseResult::WorktreeCreated { + workspace: self.workspace_info(ws_idx), + tab: self + .tab_info(ws_idx, tab_idx) + .expect("created worktree workspace should have an active tab"), + root_pane: self + .root_pane_info(ws_idx, tab_idx) + .expect("created worktree workspace should have an active root pane"), + worktree, + }, + ); + Self::send_api_response(api.respond_to, response); + } + + pub(crate) fn handle_api_worktree_remove_finished( + &mut self, + mut result: crate::events::WorktreeRemoveResult, + ) { + let Some(api) = result.api_request.take() else { + return; + }; + let operation_matches = self + .pending_api_worktree_removes + .get(&result.workspace_id) + .is_some_and(|operation_id| *operation_id == api.operation_id) + && self + .pending_api_worktree_remove_paths + .get(&api.checkout_key) + .is_some_and(|operation_id| *operation_id == api.operation_id); + if !operation_matches { + Self::send_api_response( + api.respond_to, + encode_error( + api.id, + "stale_worktree_operation", + "worktree remove completed after the operation was superseded", + ), + ); + return; + } + self.pending_api_worktree_removes + .remove(&result.workspace_id); + self.pending_api_worktree_remove_paths + .remove(&api.checkout_key); + + if let Err(message) = result.result { + let code = + if !result.forced && crate::worktree::is_dirty_worktree_remove_error(&message) { + "dirty_worktree_requires_force" + } else { + "worktree_remove_failed" + }; + Self::send_api_response(api.respond_to, encode_error(api.id, code, message)); + return; + } + + let mut workspace_id = result.workspace_id.clone(); + let mut workspace_snapshot = result.workspace.as_deref().cloned(); + let mut worktree = result.worktree.as_deref().cloned(); + if let Some(ws_idx) = self + .state + .workspaces + .iter() + .position(|ws| ws.id == result.workspace_id) + { + let current_matches = + self.state.workspaces[ws_idx] + .worktree_space() + .is_some_and(|space| { + space.is_linked_worktree && space.checkout_path == result.path + }); + if current_matches { + workspace_id = self.public_workspace_id(ws_idx); + workspace_snapshot.get_or_insert_with(|| self.workspace_info(ws_idx)); + if worktree.is_none() { + worktree = self.state.workspaces[ws_idx] + .worktree_space() + .cloned() + .map(|space| self.worktree_info_for_membership(&space, None)); + } + self.state.selected = ws_idx; + self.state.close_selected_workspace(); + self.shutdown_detached_terminal_runtimes(); + self.emit_event(EventEnvelope { + event: EventKind::WorkspaceClosed, + data: EventData::WorkspaceClosed { + workspace_id: workspace_id.clone(), + workspace: workspace_snapshot.clone(), + }, + }); + } else if let Some(snapshot) = workspace_snapshot.as_ref() { + workspace_id = snapshot.workspace_id.clone(); + } + } else if let Some(snapshot) = workspace_snapshot.as_ref() { + workspace_id = snapshot.workspace_id.clone(); + } + + let Some(worktree) = worktree else { + Self::send_api_response( + api.respond_to, + encode_error( + api.id, + "worktree_remove_failed", + "removed worktree but lost worktree snapshot", + ), + ); + return; + }; + self.emit_worktree_removed_event( + workspace_id.clone(), + workspace_snapshot, + worktree, + result.forced, + ); + let response = encode_success( + api.id, + ResponseResult::WorktreeRemoved { + workspace_id, + path: result.path.display().to_string(), + forced: result.forced, + }, + ); + Self::send_api_response(api.respond_to, response); + } +} diff --git a/src/app/mod.rs b/src/app/mod.rs index a24267e5..0b944cdf 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -109,6 +109,10 @@ pub struct App { pub(crate) git_refresh_in_flight: bool, pub(crate) git_refresh_due_after_in_flight: bool, pub(crate) git_status_cache: HashMap, + pub(crate) pending_api_worktree_creates: HashMap, + pub(crate) pending_api_worktree_removes: HashMap, + pub(crate) pending_api_worktree_remove_paths: HashMap, + pub(crate) next_api_worktree_operation_id: u64, pub(crate) last_sidebar_divider_click: Option, pub(crate) last_pane_click: Option, pub(crate) next_resize_poll: Instant, @@ -687,6 +691,10 @@ impl App { git_refresh_in_flight: false, git_refresh_due_after_in_flight: false, git_status_cache: HashMap::new(), + pending_api_worktree_creates: HashMap::new(), + pending_api_worktree_removes: HashMap::new(), + pending_api_worktree_remove_paths: HashMap::new(), + next_api_worktree_operation_id: 1, last_sidebar_divider_click: None, last_pane_click: None, next_resize_poll: Instant::now() + RESIZE_POLL_INTERVAL, diff --git a/src/app/runtime.rs b/src/app/runtime.rs index 6ef9a786..e3d5bc92 100644 --- a/src/app/runtime.rs +++ b/src/app/runtime.rs @@ -68,6 +68,20 @@ impl App { crate::api::schema::Method::ServerStop(_) | crate::api::schema::Method::ServerLiveHandoff(_) ); + if matches!( + &msg.request.method, + crate::api::schema::Method::WorktreeCreate(_) + | crate::api::schema::Method::WorktreeRemove(_) + ) { + self.drain_all_internal_events(); + let deferred_changed = + self.handle_deferred_worktree_api_request(msg.request, msg.respond_to); + if !skip_default_workspace { + changed |= self.ensure_default_workspace(); + } + self.sync_prefix_input_source(previous_mode); + return changed | deferred_changed; + } let response = self.handle_api_request(msg.request); if !skip_default_workspace { changed |= self.ensure_default_workspace(); diff --git a/src/app/worktrees.rs b/src/app/worktrees.rs index 7fcfe9d9..e1c30a77 100644 --- a/src/app/worktrees.rs +++ b/src/app/worktrees.rs @@ -547,10 +547,13 @@ impl App { } else { crate::worktree::run_worktree_command(&command) }; - let _ = event_tx.blocking_send(AppEvent::WorktreeAddFinished(WorktreeAddResult { - path, - result, - })); + let _ = event_tx.blocking_send(AppEvent::WorktreeAddFinished(Box::new( + WorktreeAddResult { + path, + api_request: None, + result, + }, + ))); }); } @@ -604,14 +607,15 @@ impl App { return; }; - #[cfg(windows)] - if let Some(ws_idx) = self - .state - .workspaces - .iter() - .position(|ws| ws.id == workspace_id) - { - self.shutdown_workspace_terminal_runtimes_for_worktree_remove(ws_idx); + if Self::should_shutdown_workspace_terminal_runtimes_for_worktree_remove(force) { + if let Some(ws_idx) = self + .state + .workspaces + .iter() + .position(|ws| ws.id == workspace_id) + { + self.shutdown_workspace_terminal_runtimes_for_worktree_remove(ws_idx); + } } let (workspace_snapshot, worktree_snapshot) = self @@ -633,20 +637,28 @@ impl App { tracing::info!(workspace_id = %workspace_id, path = %path.display(), force, "starting git worktree remove"); let event_tx = self.event_tx.clone(); std::thread::spawn(move || { - let result = crate::worktree::run_worktree_command(&command); - let _ = - event_tx.blocking_send(AppEvent::WorktreeRemoveFinished(WorktreeRemoveResult { + let result = crate::worktree::run_worktree_remove_command_with_recovery( + &command, &repo_root, &path, force, + ); + let _ = event_tx.blocking_send(AppEvent::WorktreeRemoveFinished(Box::new( + WorktreeRemoveResult { workspace_id, path, workspace: workspace_snapshot, worktree: worktree_snapshot, forced: force, + api_request: None, result, - })); + }, + ))); }); } pub(crate) fn handle_worktree_add_finished(&mut self, result: WorktreeAddResult) { + if result.api_request.is_some() { + self.handle_api_worktree_add_finished(result); + return; + } let Some(create) = &mut self.state.worktree_create else { return; }; @@ -741,6 +753,10 @@ impl App { } } pub(crate) fn handle_worktree_remove_finished(&mut self, result: WorktreeRemoveResult) { + if result.api_request.is_some() { + self.handle_api_worktree_remove_finished(result); + return; + } let Some(remove) = &mut self.state.worktree_remove else { return; }; @@ -823,7 +839,12 @@ impl App { } } - #[cfg(windows)] + pub(crate) fn should_shutdown_workspace_terminal_runtimes_for_worktree_remove( + force: bool, + ) -> bool { + force || cfg!(windows) + } + pub(crate) fn shutdown_workspace_terminal_runtimes_for_worktree_remove( &mut self, ws_idx: usize, @@ -833,7 +854,7 @@ impl App { tracing::debug!( workspace_index = ws_idx, terminal_id = %terminal_id, - "shutting down terminal runtime before Windows worktree removal" + "shutting down terminal runtime before worktree removal" ); runtime.shutdown(); } @@ -1446,6 +1467,7 @@ mod tests { app.handle_worktree_add_finished(WorktreeAddResult { path: checkout.clone(), + api_request: None, result: Ok(()), }); @@ -1539,6 +1561,7 @@ mod tests { app.handle_worktree_add_finished(WorktreeAddResult { path: checkout.clone(), + api_request: None, result: Ok(()), }); @@ -1594,6 +1617,7 @@ mod tests { let event = wait_for_worktree_event(&mut app); match event { AppEvent::WorktreeAddFinished(result) => { + let result = *result; assert_eq!(result.path, checkout); assert_eq!(result.result, Ok(())); } @@ -1607,6 +1631,55 @@ mod tests { let _ = std::fs::remove_dir_all(repo); } + #[test] + fn start_worktree_add_existing_branch_clears_creating_and_shows_fatal_error() { + let repo = create_committed_repo("app-worktree-add-existing-branch-repo"); + let worktree_root = unique_temp_path("app-worktree-add-existing-branch-root"); + let branch = "foo"; + let checkout = crate::worktree::default_checkout_path(&worktree_root, "herdr", branch); + run_git(&repo, &["branch", branch]); + let mut app = app_for_worktree_tests(); + app.state.worktree_directory = worktree_root.clone(); + app.state.name_input = branch.into(); + app.state.worktree_create = Some(WorktreeCreateState { + source_workspace_id: "source".into(), + source_checkout_path: repo.clone(), + source_existing_membership: None, + source_repo_root: repo.clone(), + repo_key: "repo-key".into(), + repo_name: "herdr".into(), + branch: branch.into(), + checkout_path: checkout.clone(), + error: None, + creating: false, + }); + + app.start_worktree_add(); + + assert!(app + .state + .worktree_create + .as_ref() + .is_some_and(|create| create.creating)); + let event = wait_for_worktree_event(&mut app); + match event { + AppEvent::WorktreeAddFinished(result) => { + app.handle_worktree_add_finished(*result); + } + other => panic!("unexpected event: {other:?}"), + } + + let create = app.state.worktree_create.as_ref().unwrap(); + assert!(!create.creating); + let error = create.error.as_deref().unwrap(); + assert!(error.contains("Preparing worktree")); + assert!(error.contains("fatal: a branch named 'foo' already exists")); + assert!(!checkout.exists()); + + let _ = std::fs::remove_dir_all(worktree_root); + let _ = std::fs::remove_dir_all(repo); + } + #[test] fn open_new_worktree_dialog_supports_standalone_bare_repo_source() { let repo = create_committed_repo("app-worktree-dialog-bare-origin"); @@ -1641,6 +1714,7 @@ mod tests { let event = wait_for_worktree_event(&mut app); match event { AppEvent::WorktreeAddFinished(result) => { + let result = *result; assert_eq!(result.path, checkout); assert_eq!(result.result, Ok(())); } @@ -1700,6 +1774,7 @@ mod tests { let event = wait_for_worktree_event(&mut app); match event { AppEvent::WorktreeAddFinished(result) => { + let result = *result; assert_eq!(result.path, checkout); assert_eq!(result.result, Ok(())); } @@ -1735,6 +1810,7 @@ mod tests { workspace: None, worktree: None, forced: false, + api_request: None, result: Err( "fatal: '/w/herdr/dirty' contains modified or untracked files, use --force to delete it" .into(), @@ -1766,6 +1842,7 @@ mod tests { workspace: None, worktree: None, forced: false, + api_request: None, result: Err("fatal: '/w/herdr/missing' is not a working tree".into()), }); @@ -1819,6 +1896,7 @@ mod tests { workspace: Some(Box::new(workspace_snapshot.clone())), worktree: Some(Box::new(worktree_snapshot)), forced: true, + api_request: None, result: Ok(()), }); @@ -1882,6 +1960,7 @@ mod tests { let safe_event = wait_for_worktree_event(&mut app); match safe_event { AppEvent::WorktreeRemoveFinished(result) => { + let result = *result; assert_eq!(result.workspace_id, workspace_id); assert_eq!(result.path, checkout); assert!(result.result.is_err()); @@ -1900,6 +1979,7 @@ mod tests { let force_event = wait_for_worktree_event(&mut app); match force_event { AppEvent::WorktreeRemoveFinished(result) => { + let result = *result; assert_eq!(result.workspace_id, workspace_id); assert_eq!(result.path, checkout); assert_eq!(result.result, Ok(())); @@ -1929,4 +2009,13 @@ mod tests { let _ = std::fs::remove_dir_all(repo); } + + #[test] + fn worktree_remove_runtime_shutdown_policy_preserves_windows_safe_remove() { + assert_eq!( + App::should_shutdown_workspace_terminal_runtimes_for_worktree_remove(false), + cfg!(windows) + ); + assert!(App::should_shutdown_workspace_terminal_runtimes_for_worktree_remove(true)); + } } diff --git a/src/events.rs b/src/events.rs index 558d826e..d9405989 100644 --- a/src/events.rs +++ b/src/events.rs @@ -9,12 +9,37 @@ use crate::detect::{Agent, AgentState}; use crate::layout::PaneId; use crate::workspace::{GitStatusCacheEntry, WorkspaceGitStatus}; +#[derive(Debug)] +pub struct ApiWorktreeAddRequest { + pub id: String, + pub operation_id: u64, + pub checkout_key: std::path::PathBuf, + pub source_workspace_id: Option, + pub source_existing_membership: Option, + pub source_checkout_path: std::path::PathBuf, + pub source_repo_root: std::path::PathBuf, + pub repo_key: String, + pub repo_name: String, + pub label: Option, + pub focus: bool, + pub respond_to: std::sync::mpsc::Sender, +} + #[derive(Debug)] pub struct WorktreeAddResult { pub path: std::path::PathBuf, + pub api_request: Option, pub result: Result<(), String>, } +#[derive(Debug)] +pub struct ApiWorktreeRemoveRequest { + pub id: String, + pub operation_id: u64, + pub checkout_key: std::path::PathBuf, + pub respond_to: std::sync::mpsc::Sender, +} + #[derive(Debug)] pub struct WorktreeRemoveResult { pub workspace_id: String, @@ -22,6 +47,7 @@ pub struct WorktreeRemoveResult { pub workspace: Option>, pub worktree: Option>, pub forced: bool, + pub api_request: Option, pub result: Result<(), String>, } @@ -125,7 +151,7 @@ pub enum AppEvent { error: Option, }, /// Background `git worktree add` completed. - WorktreeAddFinished(WorktreeAddResult), + WorktreeAddFinished(Box), /// Background `git worktree remove` completed. - WorktreeRemoveFinished(WorktreeRemoveResult), + WorktreeRemoveFinished(Box), } diff --git a/src/server/headless.rs b/src/server/headless.rs index aeda3a3b..35e17efe 100644 --- a/src/server/headless.rs +++ b/src/server/headless.rs @@ -2611,6 +2611,15 @@ impl HeadlessServer { }; self.sync_foreground_client_state(); + if matches!( + &msg.request.method, + api::schema::Method::WorktreeCreate(_) | api::schema::Method::WorktreeRemove(_) + ) { + let deferred_changed = self + .app + .handle_deferred_worktree_api_request(msg.request, msg.respond_to); + return changed | deferred_changed; + } let response = if matches!( &msg.request.method, api::schema::Method::ServerReloadConfig(_) diff --git a/src/ui/dialogs.rs b/src/ui/dialogs.rs index 31275282..51e5fb15 100644 --- a/src/ui/dialogs.rs +++ b/src/ui/dialogs.rs @@ -2,7 +2,7 @@ use ratatui::{ layout::{Constraint, Layout, Rect}, style::{Modifier, Style}, text::{Line, Span}, - widgets::{Clear, Paragraph}, + widgets::{Clear, Paragraph, Wrap}, Frame, }; @@ -12,6 +12,9 @@ use super::widgets::{ }; use crate::app::{state::WorktreeOpenState, AppState, Mode}; +const NEW_LINKED_WORKTREE_POPUP_WIDTH: u16 = 68; +const NEW_LINKED_WORKTREE_POPUP_HEIGHT: u16 = 12; + fn truncate_text(text: &str, max_width: usize) -> String { let len = text.chars().count(); if len <= max_width { @@ -126,7 +129,12 @@ pub(super) fn render_rename_overlay(app: &AppState, frame: &mut Frame, area: Rec } pub(crate) fn new_linked_worktree_inner_rect(area: Rect) -> Option { - centered_popup_rect(area, 68, 10).map(|popup| { + centered_popup_rect( + area, + NEW_LINKED_WORKTREE_POPUP_WIDTH, + NEW_LINKED_WORKTREE_POPUP_HEIGHT, + ) + .map(|popup| { Rect::new( popup.x + 1, popup.y + 1, @@ -240,10 +248,16 @@ pub(super) fn render_new_linked_worktree_overlay(app: &AppState, frame: &mut Fra }; super::dim_background(frame, area); - let Some(inner) = render_modal_shell(frame, area, 68, 10, &app.palette) else { + let Some(inner) = render_modal_shell( + frame, + area, + NEW_LINKED_WORKTREE_POPUP_WIDTH, + NEW_LINKED_WORKTREE_POPUP_HEIGHT, + &app.palette, + ) else { return; }; - if inner.height < 7 { + if inner.height < 9 { return; } @@ -253,7 +267,7 @@ pub(super) fn render_new_linked_worktree_overlay(app: &AppState, frame: &mut Fra Constraint::Length(1), Constraint::Length(1), Constraint::Length(1), - Constraint::Length(1), + Constraint::Length(3), Constraint::Length(1), Constraint::Min(0), ]) @@ -293,7 +307,9 @@ pub(super) fn render_new_linked_worktree_overlay(app: &AppState, frame: &mut Fra ); } else if let Some(error) = &create.error { frame.render_widget( - Paragraph::new(format!(" {error}")).style(Style::default().fg(app.palette.red)), + Paragraph::new(format!(" {error}")) + .style(Style::default().fg(app.palette.red)) + .wrap(Wrap { trim: false }), rows[5], ); } @@ -755,9 +771,13 @@ pub(crate) fn confirm_close_button_rects(inner: Rect) -> (Rect, Rect) { #[cfg(test)] mod tests { - use crate::{app::AppState, workspace::Workspace}; + use crate::{ + app::{state::WorktreeCreateState, AppState}, + workspace::Workspace, + }; + use ratatui::{backend::TestBackend, layout::Rect, Terminal}; - use super::confirm_close_overlay_text; + use super::{confirm_close_overlay_text, render_new_linked_worktree_overlay}; #[test] fn confirm_close_text_reports_parent_group_scope() { @@ -786,4 +806,52 @@ mod tests { assert_eq!(title, "Close worktree group?"); assert_eq!(detail, "main — 2 workspaces, 2 panes"); } + + #[test] + fn new_worktree_error_renders_fatal_stderr_line() { + let mut app = AppState::test_new(); + app.name_input = "foo".into(); + app.worktree_create = Some(WorktreeCreateState { + source_workspace_id: "source".into(), + source_checkout_path: "/repo/herdr".into(), + source_existing_membership: None, + source_repo_root: "/repo/herdr".into(), + repo_key: "repo-key".into(), + repo_name: "herdr".into(), + branch: "foo".into(), + checkout_path: "/repo/.worktrees/herdr/foo".into(), + error: Some( + "Preparing worktree (new branch 'foo')\nfatal: a branch named 'foo' already exists" + .into(), + ), + creating: false, + }); + + let mut terminal = + Terminal::new(TestBackend::new(100, 30)).expect("test terminal should initialize"); + terminal + .draw(|frame| render_new_linked_worktree_overlay(&app, frame, Rect::new(0, 0, 100, 30))) + .expect("new worktree overlay should render"); + let rendered = terminal + .backend() + .buffer() + .content() + .iter() + .map(|cell| cell.symbol()) + .collect::(); + + assert!(rendered.contains("fatal: a branch named 'foo' already exists")); + } + + #[test] + fn new_worktree_hit_test_geometry_matches_modal_size() { + let area = Rect::new(0, 0, 100, 30); + let inner = super::new_linked_worktree_inner_rect(area).unwrap(); + let (create, cancel) = super::new_linked_worktree_button_rects(inner); + + assert_eq!(inner.width, super::NEW_LINKED_WORKTREE_POPUP_WIDTH - 2); + assert_eq!(inner.height, super::NEW_LINKED_WORKTREE_POPUP_HEIGHT - 2); + assert_eq!(create.y, inner.y + inner.height - 1); + assert_eq!(cancel.y, inner.y + inner.height - 1); + } } diff --git a/src/worktree.rs b/src/worktree.rs index d345f337..f2f1dffe 100644 --- a/src/worktree.rs +++ b/src/worktree.rs @@ -183,6 +183,11 @@ pub(crate) fn is_dirty_worktree_remove_error(message: &str) -> bool { && lower.contains("use --force to delete it") } +pub(crate) fn is_not_working_tree_remove_error(message: &str) -> bool { + let lower = message.to_ascii_lowercase(); + lower.contains("is not a working tree") || lower.contains("is not a worktree") +} + #[cfg(windows)] pub(crate) fn worktree_dirty_remove_message(path: &Path) -> String { format!( @@ -261,6 +266,82 @@ pub(crate) fn run_worktree_command(command: &WorktreeCommand) -> Result<(), Stri }) } +pub(crate) fn run_worktree_remove_command_with_recovery( + command: &WorktreeCommand, + repo_root: &Path, + path: &Path, + force: bool, +) -> Result<(), String> { + match run_worktree_command(command) { + Ok(()) => Ok(()), + Err(err) if force && is_not_working_tree_remove_error(&err) => { + if worktree_list_contains_path(repo_root, path)? { + return Err(err); + } + if path.exists() { + if !leftover_worktree_checkout_matches_repo(repo_root, path) { + return Err(err); + } + std::fs::remove_dir_all(path).map_err(|remove_err| { + format!( + "{err}; failed to remove leftover checkout {}: {remove_err}", + path.display() + ) + })?; + } + Ok(()) + } + Err(err) => Err(err), + } +} + +fn leftover_worktree_checkout_matches_repo(repo_root: &Path, path: &Path) -> bool { + let git_file = path.join(".git"); + let Ok(content) = std::fs::read_to_string(&git_file) else { + return false; + }; + let Some(gitdir) = content.trim().strip_prefix("gitdir:") else { + return false; + }; + let gitdir = PathBuf::from(gitdir.trim()); + let gitdir = if gitdir.is_absolute() { + gitdir + } else { + path.join(gitdir) + }; + let Some(worktrees_dir) = git_common_worktrees_dir(repo_root) else { + return false; + }; + canonical_or_original(&gitdir).starts_with(canonical_or_original(&worktrees_dir)) +} + +fn git_common_worktrees_dir(repo_root: &Path) -> Option { + let output = std::process::Command::new("git") + .arg("-C") + .arg(repo_root) + .args(["rev-parse", "--git-common-dir"]) + .output() + .ok()?; + + if !output.status.success() { + return None; + } + + let stdout = String::from_utf8_lossy(&output.stdout); + let common_dir = stdout.trim(); + if common_dir.is_empty() { + None + } else { + let common_dir = PathBuf::from(common_dir); + let common_dir = if common_dir.is_absolute() { + common_dir + } else { + repo_root.join(common_dir) + }; + Some(common_dir.join("worktrees")) + } +} + pub(crate) fn parse_worktree_list_porcelain(output: &str) -> Vec { let mut entries = Vec::new(); let mut path: Option = None; @@ -351,6 +432,13 @@ pub(crate) fn list_existing_worktrees(repo_root: &Path) -> Result Result { + let expected = canonical_or_original(path); + Ok(list_existing_worktrees(repo_root)? + .into_iter() + .any(|entry| canonical_or_original(&entry.path) == expected)) +} + #[cfg(test)] mod tests { use super::*; @@ -695,4 +783,51 @@ prunable stale let _ = std::fs::remove_dir_all(repo); } + + #[test] + fn forced_worktree_remove_recovers_leftover_unregistered_checkout() { + let repo = create_committed_repo("worktree-recovery-repo"); + let checkout = unique_temp_path("worktree-recovery-checkout"); + let branch = "worktree/recovery"; + + let add = build_worktree_add_new_branch_command(&repo, &checkout, branch, "HEAD"); + run_worktree_command(&add).unwrap(); + let remove = build_worktree_remove_command(&repo, &checkout, true); + run_worktree_command(&remove).unwrap(); + std::fs::create_dir_all(&checkout).unwrap(); + let stale_admin_dir = git_common_worktrees_dir(&repo).unwrap().join("stale"); + std::fs::write( + checkout.join(".git"), + format!("gitdir: {}\n", stale_admin_dir.display()), + ) + .unwrap(); + std::fs::write(checkout.join("leftover"), "leftover\n").unwrap(); + + run_worktree_remove_command_with_recovery(&remove, &repo, &checkout, true).unwrap(); + + assert!(!checkout.exists()); + let _ = std::fs::remove_dir_all(repo); + } + + #[test] + fn forced_worktree_remove_recovery_keeps_unrelated_replacement_directory() { + let repo = create_committed_repo("worktree-recovery-unrelated-repo"); + let checkout = unique_temp_path("worktree-recovery-unrelated-checkout"); + let branch = "worktree/recovery-unrelated"; + + let add = build_worktree_add_new_branch_command(&repo, &checkout, branch, "HEAD"); + run_worktree_command(&add).unwrap(); + let remove = build_worktree_remove_command(&repo, &checkout, true); + run_worktree_command(&remove).unwrap(); + std::fs::create_dir_all(&checkout).unwrap(); + std::fs::write(checkout.join("unrelated"), "do not delete\n").unwrap(); + + let err = run_worktree_remove_command_with_recovery(&remove, &repo, &checkout, true) + .expect_err("unrelated replacement directory should not be removed"); + + assert!(is_not_working_tree_remove_error(&err)); + assert!(checkout.join("unrelated").exists()); + let _ = std::fs::remove_dir_all(checkout); + let _ = std::fs::remove_dir_all(repo); + } } diff --git a/tests/cli_wrapper.rs b/tests/cli_wrapper.rs index 53d91c43..1870f670 100644 --- a/tests/cli_wrapper.rs +++ b/tests/cli_wrapper.rs @@ -2007,6 +2007,76 @@ fn worktree_management_commands_work() { cleanup_spawned_herdr(herdr, base); } +#[test] +fn forced_worktree_remove_terminates_processes_inside_checkout() { + let base = unique_test_dir(); + let config_home = base.join("config"); + let runtime_dir = base.join("runtime"); + let socket_path = runtime_dir.join("herdr.sock"); + let repo = base.join("repo"); + let checkout = base.join("checkout-with-process"); + create_committed_repo(&repo); + + let herdr = spawn_herdr(&config_home, &runtime_dir, &socket_path); + wait_for_socket(&socket_path, Duration::from_secs(5)); + + let created = run_cli_json( + &socket_path, + &[ + "worktree", + "create", + "--cwd", + repo.to_str().unwrap(), + "--branch", + "worktree/force-process", + "--path", + checkout.to_str().unwrap(), + "--json", + ], + ); + let child_workspace_id = created["result"]["workspace"]["workspace_id"] + .as_str() + .unwrap() + .to_string(); + let pane_id = created["result"]["root_pane"]["pane_id"] + .as_str() + .unwrap() + .to_string(); + + let pid_file = base.join("worktree-remove-force.pid"); + let command = format!( + "python3 -c 'import os,time,pathlib; pathlib.Path(r\"{}\").write_text(str(os.getpid())); time.sleep(1000)'", + pid_file.display() + ); + let ran = run_cli(&socket_path, &["pane", "run", &pane_id, &command]); + assert!( + ran.status.success(), + "stderr: {}", + String::from_utf8_lossy(&ran.stderr) + ); + let pid = wait_for_pid_file(&pid_file, Duration::from_secs(5)).unwrap_or_else(|err| { + panic!("failed to read pane child pid: {err}"); + }); + assert!(process_exists(pid), "child process was not running"); + + let removed = run_cli_json( + &socket_path, + &[ + "worktree", + "remove", + "--workspace", + &child_workspace_id, + "--force", + "--json", + ], + ); + assert_eq!(removed["result"]["type"], "worktree_removed"); + assert!(wait_for_pid_exit(pid, Duration::from_secs(3))); + assert!(!checkout.exists()); + + cleanup_spawned_herdr(herdr, base); +} + #[test] fn worktree_open_existing_checkout_by_path_and_branch() { let base = unique_test_dir();