diff --git a/Cargo.lock b/Cargo.lock index 6994c9b01..2d172c7ce 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6093,6 +6093,7 @@ dependencies = [ "dirs 5.0.1", "okena-core", "okena-ext-updater", + "okena-git", "okena-remote-server", "okena-transport", "okena-workspace", diff --git a/crates/okena-app-core/src/remote_snapshot.rs b/crates/okena-app-core/src/remote_snapshot.rs index 9d8a53f1d..e2e91f0b0 100644 --- a/crates/okena-app-core/src/remote_snapshot.rs +++ b/crates/okena-app-core/src/remote_snapshot.rs @@ -73,6 +73,7 @@ pub fn build_api_project( .map(|(tid, e)| e.to_api(tid.clone())) .collect(), hooks: p.hooks.to_api(), + creating_progress: p.creating_progress.clone(), is_creating: p.is_creating, is_closing: p.is_closing, } diff --git a/crates/okena-app-core/src/workspace/actions/execute/mod.rs b/crates/okena-app-core/src/workspace/actions/execute/mod.rs index e12d43730..6aa4db4a6 100644 --- a/crates/okena-app-core/src/workspace/actions/execute/mod.rs +++ b/crates/okena-app-core/src/workspace/actions/execute/mod.rs @@ -484,6 +484,14 @@ pub fn execute_action( ActionRequest::AddProject { name, path } => { project::add_project(ws, window_id, name, path, backend, terminals, settings, cx) } + ActionRequest::CloneProject { + url, + parent_dir, + directory, + name, + } => project::clone_project( + ws, window_id, url, parent_dir, directory, name, backend, terminals, settings, cx, + ), ActionRequest::ReorderProjectInFolder { folder_id, project_id, @@ -1286,6 +1294,7 @@ mod reconnect_shell_tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, }; Workspace::new(WorkspaceData { version: 1, diff --git a/crates/okena-app-core/src/workspace/actions/execute/project.rs b/crates/okena-app-core/src/workspace/actions/execute/project.rs index f270af3d2..72266811c 100644 --- a/crates/okena-app-core/src/workspace/actions/execute/project.rs +++ b/crates/okena-app-core/src/workspace/actions/execute/project.rs @@ -82,6 +82,45 @@ pub(super) fn add_project( } } +/// Clone `url` into `parent_dir`/`directory`, then add the checkout as a project. +/// +/// Blocking end-to-end: the clone runs before the project row exists. The +/// daemon command loop intercepts `CloneProject` before this and does the same +/// work optimistically off the reactor; this path serves callers that drive +/// `execute_action` directly (tests, non-daemon hosts). +pub(super) fn clone_project( + ws: &mut Workspace, + window_id: WindowId, + url: String, + parent_dir: String, + directory: String, + name: String, + backend: &dyn TerminalBackend, + terminals: &TerminalsRegistry, + settings: &AppSettings, + cx: &mut impl WorkspaceCx, +) -> ActionResult { + let target = + match okena_workspace::actions::project::resolve_clone_target(&parent_dir, &directory) { + Ok(target) => target, + Err(error) => return ActionResult::Err(error), + }; + let name = okena_workspace::actions::project::clone_project_name(&name, &directory); + if let Err(error) = okena_git::clone_repository(&url, &target) { + return ActionResult::Err(error.to_string()); + } + add_project( + ws, + window_id, + name, + target.to_string_lossy().into_owned(), + backend, + terminals, + settings, + cx, + ) +} + pub(super) fn reorder_in_folder( ws: &mut Workspace, folder_id: String, @@ -664,7 +703,7 @@ mod hook_action_tests { } #[derive(Default)] - struct RecordingBackend { + pub(super) struct RecordingBackend { transport: Arc, next_id: AtomicUsize, shells: Mutex>>, @@ -770,6 +809,7 @@ mod hook_action_tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, }; Workspace::new(WorkspaceData { version: 1, @@ -1031,6 +1071,169 @@ mod hook_action_tests { } } +#[cfg(test)] +mod clone_project_tests { + use super::{ActionResult, clone_project}; + use crate::workspace::state::{WindowId, WindowState, Workspace, WorkspaceData}; + use okena_terminal::TerminalsRegistry; + use okena_workspace::context::WorkspaceCx; + use okena_workspace::hook_monitor::HookMonitor; + use okena_workspace::hooks::HookRunner; + use okena_workspace::settings::AppSettings; + use std::collections::HashMap; + use std::path::{Path, PathBuf}; + use std::process::Command; + + struct TestCx; + + impl WorkspaceCx for TestCx { + fn notify(&mut self) {} + fn refresh_views(&mut self) {} + fn hook_runner(&self) -> Option { + None + } + fn hook_monitor(&self) -> Option { + None + } + } + + fn empty_workspace() -> Workspace { + Workspace::new(WorkspaceData { + version: 1, + projects: vec![], + project_order: vec![], + service_panel_heights: HashMap::new(), + hook_panel_heights: HashMap::new(), + folders: vec![], + main_window: WindowState::default(), + extra_windows: Vec::new(), + }) + } + + fn git(dir: &Path, args: &[&str]) { + let output = Command::new("git") + .current_dir(dir) + .args(args) + .output() + .expect("run git"); + assert!( + output.status.success(), + "git {args:?} failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + } + + /// A source repository with one commit, cloneable over a plain path URL. + fn source_repo(fixture: &Path) -> PathBuf { + let source = fixture.join("source"); + std::fs::create_dir_all(&source).expect("create source dir"); + git(&source, &["init", "--initial-branch=main"]); + git(&source, &["config", "user.email", "test@example.com"]); + git(&source, &["config", "user.name", "Test"]); + std::fs::write(source.join("README.md"), b"hello").expect("write file"); + git(&source, &["add", "README.md"]); + git(&source, &["commit", "-m", "init"]); + source + } + + fn fixture_dir(name: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!("okena-clone-{name}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).expect("create fixture dir"); + dir + } + + #[test] + fn clone_project_checks_out_the_repo_and_registers_the_project() { + let fixture = fixture_dir("ok"); + let source = source_repo(&fixture); + let parent = fixture.join("parent"); + std::fs::create_dir_all(&parent).expect("create parent"); + + let mut workspace = empty_workspace(); + let result = clone_project( + &mut workspace, + WindowId::Main, + source.to_string_lossy().into_owned(), + parent.to_string_lossy().into_owned(), + "checkout".to_string(), + "My Clone".to_string(), + &super::hook_action_tests::RecordingBackend::default(), + &TerminalsRegistry::default(), + &AppSettings::default(), + &mut TestCx, + ); + + if let ActionResult::Err(error) = &result { + panic!("clone should succeed, got error: {error}"); + } + assert!( + parent.join("checkout/README.md").exists(), + "working tree checked out" + ); + let project = workspace + .projects() + .iter() + .find(|p| p.name == "My Clone") + .expect("project registered"); + assert_eq!( + Path::new(&project.path), + parent.join("checkout"), + "project points at the checkout" + ); + assert!(project.layout.is_some(), "terminal layout seeded"); + + let _ = std::fs::remove_dir_all(&fixture); + } + + #[test] + fn clone_project_registers_nothing_when_the_clone_fails() { + let fixture = fixture_dir("fail"); + let parent = fixture.join("parent"); + std::fs::create_dir_all(&parent).expect("create parent"); + + let mut workspace = empty_workspace(); + let result = clone_project( + &mut workspace, + WindowId::Main, + fixture.join("not-a-repo").to_string_lossy().into_owned(), + parent.to_string_lossy().into_owned(), + "checkout".to_string(), + "My Clone".to_string(), + &super::hook_action_tests::RecordingBackend::default(), + &TerminalsRegistry::default(), + &AppSettings::default(), + &mut TestCx, + ); + + assert!(matches!(result, ActionResult::Err(_))); + assert!(workspace.projects().is_empty(), "no row left behind"); + + let _ = std::fs::remove_dir_all(&fixture); + } + + #[test] + fn clone_project_rejects_a_directory_name_that_escapes_the_parent() { + let mut workspace = empty_workspace(); + let result = clone_project( + &mut workspace, + WindowId::Main, + "https://example.com/repo.git".to_string(), + "/tmp".to_string(), + "../escape".to_string(), + "Escape".to_string(), + &super::hook_action_tests::RecordingBackend::default(), + &TerminalsRegistry::default(), + &AppSettings::default(), + &mut TestCx, + ); + + // Rejected before any network access, so the test never touches git. + assert!(matches!(result, ActionResult::Err(_))); + assert!(workspace.projects().is_empty()); + } +} + #[cfg(all(test, feature = "gpui"))] mod set_show_in_overview_tests { use super::{ActionResult, apply_set_project_show_in_overview}; @@ -1074,6 +1277,7 @@ mod set_show_in_overview_tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, } } diff --git a/crates/okena-app-core/src/workspace/actions/execute/session.rs b/crates/okena-app-core/src/workspace/actions/execute/session.rs index a8040151f..ce3694c07 100644 --- a/crates/okena-app-core/src/workspace/actions/execute/session.rs +++ b/crates/okena-app-core/src/workspace/actions/execute/session.rs @@ -992,6 +992,7 @@ mod tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, } } diff --git a/crates/okena-app/src/action_dispatch.rs b/crates/okena-app/src/action_dispatch.rs index caab9e761..4558e825f 100644 --- a/crates/okena-app/src/action_dispatch.rs +++ b/crates/okena-app/src/action_dispatch.rs @@ -779,6 +779,17 @@ fn strip_remote_ids(action: ActionRequest, connection_id: &str) -> ActionRequest mode, }, ActionRequest::AddProject { name, path } => ActionRequest::AddProject { name, path }, + ActionRequest::CloneProject { + url, + parent_dir, + directory, + name, + } => ActionRequest::CloneProject { + url, + parent_dir, + directory, + name, + }, ActionRequest::ReorderProjectInFolder { folder_id, project_id, diff --git a/crates/okena-app/src/views/overlays/add_project_dialog.rs b/crates/okena-app/src/views/overlays/add_project_dialog.rs index 0e99ded43..82d0f82f6 100644 --- a/crates/okena-app/src/views/overlays/add_project_dialog.rs +++ b/crates/okena-app/src/views/overlays/add_project_dialog.rs @@ -5,8 +5,8 @@ use crate::remote_client::manager::RemoteConnectionManager; use crate::theme::theme; use crate::ui::tokens::{ui_text_md, ui_text_ms}; use crate::views::components::{ - PathAutoCompleteState, SimpleInput, SimpleInputState, button, input_container, labeled_input, - modal_backdrop, modal_content, modal_header, + PathAutoCompleteState, SimpleInput, SimpleInputState, button, dropdown_anchored_below, + input_container, labeled_input, modal_backdrop, modal_content, modal_header, }; use crate::workspace::state::{WindowId, Workspace}; use gpui::prelude::*; @@ -15,6 +15,7 @@ use gpui_component::v_flex; use okena_core::api::ActionRequest; use okena_transport::client::{ConnectionStatus, LOCAL_DAEMON_CONNECTION_ID}; use okena_ui::dialog_actions::dialog_actions; +use okena_ui::simple_input::InputChangedEvent; enum AddProjectTarget { Local, @@ -24,6 +25,15 @@ enum AddProjectTarget { }, } +/// Where the project's directory comes from. +#[derive(Clone, Copy, PartialEq, Eq)] +enum AddProjectSource { + /// A directory that already exists on the target host. + Folder, + /// A repository to clone into a directory that does not exist yet. + Git, +} + pub struct AddProjectDialog { workspace: Entity, /// Spawning window for the multi-window new-project visibility rule @@ -33,8 +43,22 @@ pub struct AddProjectDialog { window_id: WindowId, remote_manager: Option>, focus_handle: FocusHandle, + source: AddProjectSource, name_input: Entity, + /// Folder mode: the project directory. Git mode: the parent to clone into. path_input: Entity, + /// Window-absolute bounds of the path input, captured during paint so the + /// completion list can be anchored to it instead of a guessed offset. + path_input_bounds: Option>, + url_input: Entity, + /// Git mode: the directory name created inside the parent. + directory_input: Entity, + /// The last values this dialog derived into the directory / name inputs. A + /// field still holding its derived value counts as untouched and keeps + /// following the URL; once the user types their own, the auto-fill stops + /// overwriting it. + derived_directory: String, + derived_name: String, pending_name_value: Option, pending_path_value: Option, initial_focus_done: bool, @@ -58,6 +82,24 @@ impl AddProjectDialog { let name_input = cx.new(|cx| SimpleInputState::new(cx).placeholder("Enter project name...")); let path_input = cx.new(PathAutoCompleteState::new); + let url_input = + cx.new(|cx| SimpleInputState::new(cx).placeholder("https://github.com/user/repo.git")); + let directory_input = cx.new(|cx| SimpleInputState::new(cx).placeholder("Folder name...")); + + // Typing a URL fills in the directory and the name; editing the + // directory keeps the name in step. Subscribed to the change event, not + // to notify — the cursor blink notifies twice a second, and re-running a + // fill on those would keep resetting the caret to the end of the field. + cx.subscribe( + &url_input, + |this: &mut Self, _, _: &InputChangedEvent, cx| this.derive_from_url(cx), + ) + .detach(); + cx.subscribe( + &directory_input, + |this: &mut Self, _, _: &InputChangedEvent, cx| this.derive_name_from_directory(cx), + ) + .detach(); // Build targets list: Local (the implicit loopback local-daemon // connection) + connected remote connections. The local-daemon @@ -84,8 +126,14 @@ impl AddProjectDialog { window_id, remote_manager, focus_handle: cx.focus_handle(), + source: AddProjectSource::Folder, name_input, path_input, + path_input_bounds: None, + url_input, + directory_input, + derived_directory: String::new(), + derived_name: String::new(), pending_name_value: None, pending_path_value: None, initial_focus_done: false, @@ -105,23 +153,59 @@ impl AddProjectDialog { ) } - fn add_project(&mut self, _window: &mut Window, cx: &mut Context) { - let name = self.name_input.read(cx).value().to_string(); - let path = self.path_input.read(cx).value(cx); + fn is_git_source(&self) -> bool { + self.source == AddProjectSource::Git + } - if name.is_empty() || path.is_empty() { + /// Fill the directory (and, through it, the name) from the URL, for as long + /// as the user has not overridden them. + fn derive_from_url(&mut self, cx: &mut Context) { + let url = self.url_input.read(cx).value().to_string(); + let derived = okena_git::clone_dir_name(&url).unwrap_or_default(); + let current = self.directory_input.read(cx).value(); + // A field holding anything other than what we put there is the user's. + if current != self.derived_directory || current == derived { return; } + self.derived_directory = derived.clone(); + self.directory_input + .update(cx, |input, cx| input.set_value(derived, cx)); + } - // Resolve the target connection. "Local" is just the implicit loopback - // local-daemon connection; every project (local or remote) is added by - // dispatching `AddProject` to a daemon over the same mechanism — the GUI - // never mutates its read-only mirror directly. - let connection_id = match self.targets.get(self.selected_target) { + /// Keep the project name equal to the directory name until the user gives + /// the project a name of its own. + fn derive_name_from_directory(&mut self, cx: &mut Context) { + let directory = self.directory_input.read(cx).value().to_string(); + let current = self.name_input.read(cx).value(); + if current != self.derived_name || current == directory { + return; + } + self.derived_name = directory.clone(); + self.name_input + .update(cx, |input, cx| input.set_value(directory, cx)); + } + + /// Resolve the connection this dialog dispatches to. "Local" is just the + /// implicit loopback local-daemon connection; every project (local or + /// remote) is created by dispatching to a daemon over the same mechanism — + /// the GUI never mutates its read-only mirror directly. + fn selected_connection_id(&self) -> String { + match self.targets.get(self.selected_target) { Some(AddProjectTarget::Local) | None => LOCAL_DAEMON_CONNECTION_ID.to_string(), Some(AddProjectTarget::Remote { connection_id, .. }) => connection_id.clone(), + } + } + + fn submit(&mut self, cx: &mut Context) { + let action = match self.source { + AddProjectSource::Folder => self.folder_action(cx), + AddProjectSource::Git => self.clone_action(cx), + }; + let Some((action, name, path)) = action else { + return; }; + let connection_id = self.selected_connection_id(); if let Some(ref rm) = self.remote_manager { let connection_available = rm .read(cx) @@ -135,11 +219,11 @@ impl AddProjectDialog { window_id, &connection_id, &name, - Some(&path), + path.as_deref(), ); }); rm.update(cx, |rm, cx| { - rm.send_action(&connection_id, ActionRequest::AddProject { name, path }, cx); + rm.send_action(&connection_id, action, cx); }); } } @@ -147,12 +231,58 @@ impl AddProjectDialog { self.close(cx); } + /// The action plus the (name, path) the multi-window visibility queue + /// matches the materialized project on. + fn folder_action(&self, cx: &App) -> Option<(ActionRequest, String, Option)> { + let name = self.name_input.read(cx).value().trim().to_string(); + let path = self.path_input.read(cx).value(cx).trim().to_string(); + if name.is_empty() || path.is_empty() { + return None; + } + Some(( + ActionRequest::AddProject { + name: name.clone(), + path: path.clone(), + }, + name, + Some(path), + )) + } + + fn clone_action(&self, cx: &App) -> Option<(ActionRequest, String, Option)> { + let url = self.url_input.read(cx).value().trim().to_string(); + let parent_dir = self.path_input.read(cx).value(cx).trim().to_string(); + let directory = self.directory_input.read(cx).value().trim().to_string(); + let name = self.name_input.read(cx).value().trim().to_string(); + if url.is_empty() || parent_dir.is_empty() || directory.is_empty() || name.is_empty() { + return None; + } + Some(( + ActionRequest::CloneProject { + url, + parent_dir, + directory, + name: name.clone(), + }, + name, + // The target host joins the parent and the directory with ITS own + // separator, so the client cannot predict the final path. The + // visibility queue falls back to matching on the name alone. + None, + )) + } + fn open_folder_picker(&mut self, window: &mut Window, cx: &mut Context) { + let is_git = self.is_git_source(); let paths = cx.prompt_for_paths(gpui::PathPromptOptions { files: false, directories: true, multiple: false, - prompt: Some("Select project folder".into()), + prompt: Some(if is_git { + "Select the folder to clone into".into() + } else { + "Select project folder".into() + }), }); cx.spawn_in(window, async move |this, cx| { @@ -160,14 +290,21 @@ impl AddProjectDialog { && let Some(path) = selected_paths.first() { let path_str = path.to_string_lossy().to_string(); - let name_str = path - .file_name() - .map(|n| n.to_string_lossy().to_string()) - .unwrap_or_else(|| "Project".to_string()); + // Git mode picks the PARENT directory — the project's own name + // comes from the repository, not from the folder chosen here. + let name_str = if is_git { + None + } else { + Some( + path.file_name() + .map(|n| n.to_string_lossy().to_string()) + .unwrap_or_else(|| "Project".to_string()), + ) + }; this.update(cx, |this, cx| { this.pending_path_value = Some(path_str); - this.pending_name_value = Some(name_str); + this.pending_name_value = name_str; cx.notify(); }) .ok(); @@ -176,6 +313,41 @@ impl AddProjectDialog { .detach(); } + fn render_source_selector(&self, cx: &mut Context) -> impl IntoElement { + let t = theme(cx); + + div().flex().gap(px(6.0)).children( + [ + (AddProjectSource::Folder, "Folder"), + (AddProjectSource::Git, "Git"), + ] + .into_iter() + .map(|(source, label)| { + let is_selected = self.source == source; + div() + .id(ElementId::Name(format!("source-{label}").into())) + .px(px(10.0)) + .py(px(4.0)) + .text_size(ui_text_ms(cx)) + .rounded(px(4.0)) + .cursor_pointer() + .when(is_selected, |d| { + d.bg(rgb(t.border_active)).text_color(rgb(t.bg_primary)) + }) + .when(!is_selected, |d| { + d.bg(rgb(t.bg_secondary)) + .text_color(rgb(t.text_muted)) + .hover(|s| s.bg(rgb(t.bg_hover))) + }) + .child(label) + .on_click(cx.listener(move |this, _, _window, cx| { + this.source = source; + cx.notify(); + })) + }), + ) + } + fn render_target_selector(&self, cx: &mut Context) -> impl IntoElement { let t = theme(cx); @@ -228,85 +400,82 @@ impl AddProjectDialog { let selected_index = state.selected_index(); let scroll_handle = state.suggestions_scroll().clone(); - if suggestions.is_empty() { + let Some(bounds) = self.path_input_bounds.filter(|_| !suggestions.is_empty()) else { return div().into_any_element(); - } + }; - // Adjust top offset when target selector is visible - let top_offset = if self.targets.len() > 1 { 210.0 } else { 180.0 }; + dropdown_anchored_below( + bounds, + div() + .id("path-suggestions-container") + .occlude() + .w(bounds.size.width) + .bg(rgb(t.bg_primary)) + .border_1() + .border_color(rgb(t.border)) + .rounded(px(4.0)) + .shadow_xl() + .max_h(px(200.0)) + .overflow_y_scroll() + .track_scroll(&scroll_handle) + .on_mouse_down(MouseButton::Left, |_, _, cx| { + cx.stop_propagation(); + }) + .on_scroll_wheel(|_, _, cx| { + cx.stop_propagation(); + }) + .child( + v_flex().children(suggestions.iter().enumerate().map(|(i, suggestion)| { + let is_selected = i == selected_index; + let path_input = path_input.clone(); - div() - .absolute() - // Position below the path input inside the modal content - .top(px(top_offset)) - .left(px(20.0)) - .right(px(20.0)) - .id("path-suggestions-container") - .bg(rgb(t.bg_primary)) - .border_1() - .border_color(rgb(t.border)) - .rounded(px(4.0)) - .shadow_xl() - .max_h(px(200.0)) - .overflow_y_scroll() - .track_scroll(&scroll_handle) - .on_mouse_down(MouseButton::Left, |_, _, cx| { - cx.stop_propagation(); - }) - .on_scroll_wheel(|_, _, cx| { - cx.stop_propagation(); - }) - .child( - v_flex().children(suggestions.iter().enumerate().map(|(i, suggestion)| { - let is_selected = i == selected_index; - let path_input = path_input.clone(); - - div() - .id(ElementId::Name(format!("path-suggestion-{}", i).into())) - .px(px(8.0)) - .py(px(6.0)) - .cursor_pointer() - .when(is_selected, |d| d.bg(rgb(t.bg_selection))) - .hover(|s| s.bg(rgb(t.bg_hover))) - .flex() - .items_center() - .gap(px(8.0)) - .child( - svg() - .path(if suggestion.is_select_current { - "icons/check.svg" - } else if suggestion.is_directory { - "icons/folder.svg" - } else { - "icons/file.svg" - }) - .size(px(14.0)) - .text_color( - if suggestion.is_select_current || suggestion.is_directory { + div() + .id(ElementId::Name(format!("path-suggestion-{}", i).into())) + .px(px(8.0)) + .py(px(6.0)) + .cursor_pointer() + .when(is_selected, |d| d.bg(rgb(t.bg_selection))) + .hover(|s| s.bg(rgb(t.bg_hover))) + .flex() + .items_center() + .gap(px(8.0)) + .child( + svg() + .path(if suggestion.is_select_current { + "icons/check.svg" + } else if suggestion.is_directory { + "icons/folder.svg" + } else { + "icons/file.svg" + }) + .size(px(14.0)) + .text_color( + if suggestion.is_select_current || suggestion.is_directory { + rgb(t.border_active) + } else { + rgb(t.text_muted) + }, + ), + ) + .child( + div() + .text_size(ui_text_md(cx)) + .text_color(if suggestion.is_select_current { rgb(t.border_active) } else { - rgb(t.text_muted) - }, - ), - ) - .child( - div() - .text_size(ui_text_md(cx)) - .text_color(if suggestion.is_select_current { - rgb(t.border_active) - } else { - rgb(t.text_primary) - }) - .child(suggestion.display_name.clone()), - ) - .on_click(move |_, _window, cx| { - path_input.update(cx, |state, cx| { - state.select_and_complete(i, cx); - }); - }) - })), - ) - .into_any_element() + rgb(t.text_primary) + }) + .child(suggestion.display_name.clone()), + ) + .on_click(move |_, _window, cx| { + path_input.update(cx, |state, cx| { + state.select_and_complete(i, cx); + }); + }) + })), + ), + ) + .into_any_element() } } @@ -325,6 +494,10 @@ impl Render for AddProjectDialog { // Apply pending values from async operations if let Some(name_value) = self.pending_name_value.take() { + // The picker-derived name is an auto-fill like the URL-derived one, + // so record it as such — switching to Git afterwards then still lets + // the repository name take over. + self.derived_name = name_value.clone(); self.name_input .update(cx, |i, cx| i.set_value(&name_value, cx)); } @@ -333,14 +506,28 @@ impl Render for AddProjectDialog { .update(cx, |i, cx| i.set_value_quiet(&path_value, cx)); } + // Records the path input's bounds during paint. Deliberately does NOT + // notify — a notify from inside paint would re-render every frame; the + // stored bounds are read by the next render, which the keystroke that + // produced the suggestions triggers anyway. + let path_bounds_setter = { + let entity = cx.entity().downgrade(); + move |bounds, _: &mut Window, cx: &mut App| { + if let Some(entity) = entity.upgrade() { + entity.update(cx, |this, _| this.path_input_bounds = Some(bounds)); + } + } + }; + let is_remote = self.is_remote_target(); + let is_git = self.is_git_source(); let has_suggestions = !is_remote && self.path_input.read(cx).has_suggestions(); let has_multiple_targets = self.targets.len() > 1; - let path_label = if is_remote { - "Path:" - } else { - "Path (Tab to complete):" + let path_label = match (is_git, is_remote) { + (true, _) => "Clone into:", + (false, true) => "Path:", + (false, false) => "Path (Tab to complete):", }; modal_backdrop("add-project-backdrop", &t) @@ -373,6 +560,9 @@ impl Render for AddProjectDialog { .flex() .flex_col() .gap(px(12.0)) + .child( + labeled_input("Source:", &t).child(self.render_source_selector(cx)), + ) // Target selector (only when multiple targets available) .when(has_multiple_targets, |d| { d.child( @@ -380,13 +570,17 @@ impl Render for AddProjectDialog { .child(self.render_target_selector(cx)), ) }) - // Name input - .child(labeled_input("Name:", &t).child( - input_container(&t, None).child( - SimpleInput::new(&self.name_input).text_size(ui_text_md(cx)), - ), - )) - // Path input with auto-complete (or plain input for remote) + // Repository URL (git source only) + .when(is_git, |d| { + d.child(labeled_input("Repository URL:", &t).child( + input_container(&t, None).child( + SimpleInput::new(&self.url_input).text_size(ui_text_md(cx)), + ), + )) + }) + // Path input with auto-complete (or plain input for remote). + // Folder source: the project directory. Git source: the + // parent the clone lands in. .child( labeled_input(path_label, &t) .when(!is_remote, |d| d.child(self.path_input.clone())) @@ -397,8 +591,32 @@ impl Render for AddProjectDialog { .text_size(ui_text_md(cx)), ), ) - }), + }) + // Track the input's painted bounds so the + // completion list anchors to it in every layout. + .child( + canvas(path_bounds_setter, |_, _, _, _| {}) + .absolute() + .size_full(), + ), ) + // Target directory name (git source only) + .when(is_git, |d| { + d.child( + labeled_input("Folder name:", &t).child( + input_container(&t, None).child( + SimpleInput::new(&self.directory_input) + .text_size(ui_text_md(cx)), + ), + ), + ) + }) + // Name input + .child(labeled_input("Name:", &t).child( + input_container(&t, None).child( + SimpleInput::new(&self.name_input).text_size(ui_text_md(cx)), + ), + )) // Browse button (only for local target) .when(!is_remote, |d| { d.child( @@ -418,9 +636,9 @@ impl Render for AddProjectDialog { cx.listener(|this, _, _window, cx| { this.close(cx); }), - "Add", - cx.listener(|this, _, window, cx| { - this.add_project(window, cx); + if is_git { "Clone" } else { "Add" }, + cx.listener(|this, _, _window, cx| { + this.submit(cx); }), &t, )), diff --git a/crates/okena-app/src/views/overlays/project_switcher.rs b/crates/okena-app/src/views/overlays/project_switcher.rs index 135d2eddc..0896be06e 100644 --- a/crates/okena-app/src/views/overlays/project_switcher.rs +++ b/crates/okena-app/src/views/overlays/project_switcher.rs @@ -631,6 +631,7 @@ mod tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, } } diff --git a/crates/okena-app/src/views/panels/project_column.rs b/crates/okena-app/src/views/panels/project_column.rs index 06f6c3ab4..8f7fd0e5d 100644 --- a/crates/okena-app/src/views/panels/project_column.rs +++ b/crates/okena-app/src/views/panels/project_column.rs @@ -930,9 +930,28 @@ impl ProjectColumn { ) } - /// Render empty state for bookmark projects (no terminal) - fn render_creating_state(&self, cx: &mut Context) -> impl IntoElement { + /// Placeholder shown while the daemon is still materializing the project's + /// directory — a worktree checkout, or a clone of a remote repository. + fn render_creating_state( + &self, + is_worktree: bool, + progress: Option<&str>, + cx: &mut Context, + ) -> impl IntoElement { let t = theme(cx); + let (icon, title, detail) = if is_worktree { + ( + "icons/git-branch.svg", + "Setting up worktree\u{2026}", + "Fetching latest changes and creating the branch. Terminals will start automatically.", + ) + } else { + ( + "icons/refresh.svg", + "Cloning repository\u{2026}", + "Fetching the repository. Terminals will start automatically once the clone finishes.", + ) + }; v_flex() .items_center() .justify_center() @@ -941,15 +960,15 @@ impl ProjectColumn { .bg(rgb(t.bg_primary)) .child( svg() - .path("icons/git-branch.svg") + .path(icon) .size(px(48.0)) - .text_color(rgb(t.text_muted)) + .text_color(rgb(t.text_muted)), ) .child( div() .text_size(ui_text_xl(cx)) .text_color(rgb(t.text_secondary)) - .child("Setting up worktree\u{2026}") + .child(title), ) .child( div() @@ -957,8 +976,18 @@ impl ProjectColumn { .text_color(rgb(t.text_muted)) .max_w(px(240.0)) .text_center() - .child("Fetching latest changes and creating the branch. Terminals will start automatically.") + .child(detail), ) + // Only a clone reports progress; a worktree checkout is local and + // usually over before a percentage would be readable. + .when_some(progress, |d, progress: &str| { + d.child( + div() + .text_size(ui_text_ms(cx)) + .text_color(rgb(t.text_secondary)) + .child(progress.to_string()), + ) + }) } fn render_empty_state(&self, cx: &mut Context) -> impl IntoElement { @@ -1157,7 +1186,13 @@ impl Render for ProjectColumn { .into_any_element() } ColumnContent::Closing => self.render_closing_state(cx).into_any_element(), - ColumnContent::Creating => self.render_creating_state(cx).into_any_element(), + ColumnContent::Creating => self + .render_creating_state( + project.worktree_info.is_some(), + project.creating_progress.as_deref(), + cx, + ) + .into_any_element(), ColumnContent::Empty => self.render_empty_state(cx).into_any_element(), }; @@ -1264,6 +1299,7 @@ mod tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, } } diff --git a/crates/okena-cli/Cargo.toml b/crates/okena-cli/Cargo.toml index 0254640f6..c299f9a6c 100644 --- a/crates/okena-cli/Cargo.toml +++ b/crates/okena-cli/Cargo.toml @@ -6,6 +6,9 @@ license = "MIT" [dependencies] okena-core = { path = "../okena-core" } +# For `project clone`'s client-side directory-name derivation (already in the +# dependency graph via okena-workspace). +okena-git = { path = "../okena-git" } okena-transport = { path = "../okena-transport", features = ["client", "blocking-http"] } okena-workspace = { path = "../okena-workspace" } okena-remote-server = { path = "../okena-remote-server" } diff --git a/crates/okena-cli/src/CLAUDE.md b/crates/okena-cli/src/CLAUDE.md index c91bc6270..7b3a2fea1 100644 --- a/crates/okena-cli/src/CLAUDE.md +++ b/crates/okena-cli/src/CLAUDE.md @@ -22,7 +22,7 @@ handling untouched. - **Projects**: exact id, case-insensitive name, or absolute path (canonicalized). - **Terminals**: a bare terminal id, `/`, or `:` (DFS order). `` matches a `terminal_names` entry first, then falls back to a terminal id scoped to that project (so the id `term ls` shows for unnamed terminals also works after the `/`). -- **Windows** (`--window`): `"main"`, a full id, or a unique id prefix → resolved to the exact id put in the action's `window` field. The flag is `global` but only `project add/show/hide/focus` and `term focus/fullscreen` honor it; `dispatch` warns when any other command receives it. It must come **after** the subcommand (`okena term focus X --window main`) — the gate only engages when `args[1]` is a subcommand, so `--window` *before* the subcommand falls through to GUI launch. +- **Windows** (`--window`): `"main"`, a full id, or a unique id prefix → resolved to the exact id put in the action's `window` field. The flag is `global` but only `project add/clone/show/hide/focus` and `term focus/fullscreen` honor it; `dispatch` warns when any other command receives it. It must come **after** the subcommand (`okena term focus X --window main`) — the gate only engages when `args[1]` is a subcommand, so `--window` *before* the subcommand falls through to GUI launch. - **Layout `path`** for `term split`/`term tab` is resolved client-side from a terminal id (`resolve_terminal_path`), mirroring `okena_layout::LayoutNode::find_terminal_path` — agents never compute tree paths. `term tab` sends `in_group: false` (wrap-or-join, mirroring the UI), never `true` (which needs `path` to point at a Tabs node). ## Conventions diff --git a/crates/okena-cli/src/commands.rs b/crates/okena-cli/src/commands.rs index 97aa36bef..c7db71d35 100644 --- a/crates/okena-cli/src/commands.rs +++ b/crates/okena-cli/src/commands.rs @@ -1150,7 +1150,101 @@ pub fn cli_project_add( } }; println!("{project_id}"); + project_add_followups(&token, &project_id, hidden, folder, window) +} + +/// `okena project clone [--into ] [--dir ] [--name ] [--hidden] [--folder ]` +/// +/// Clones into `/`, where `--into` defaults to the CWD and `--dir` +/// to the name `git clone` would pick. The clone runs in the background on the +/// daemon (see the `pending` note below), so this returns as soon as the +/// project row exists. +pub fn cli_project_clone( + url: &str, + into: Option<&str>, + dir: Option<&str>, + name: Option<&str>, + hidden: bool, + folder: Option<&str>, + window: Option<&str>, +) -> i32 { + let directory = match dir + .map(|d| d.to_string()) + .or_else(|| okena_git::clone_dir_name(url)) + { + Some(d) => d, + None => { + eprintln!( + "Cannot derive a directory name from '{url}' — pass --dir to set one." + ); + return 1; + } + }; + // Resolve `--into` against the CWD so a relative parent means the same + // thing it would to `git clone`. The daemon is normally the same host, but + // it does not share this process's working directory. + let parent = into.map(std::path::PathBuf::from).unwrap_or_default(); + let parent = match std::path::absolute(if parent.as_os_str().is_empty() { + std::path::Path::new(".") + } else { + parent.as_path() + }) { + Ok(p) => p.to_string_lossy().into_owned(), + Err(e) => { + eprintln!("Cannot resolve target directory: {e}"); + return 1; + } + }; + let token = match ensure_token() { + Ok(t) => t, + Err(e) => { + eprintln!("{e}"); + return 1; + } + }; + let body = serde_json::json!({ + "action": "clone_project", + "url": url, + "parent_dir": parent, + "directory": directory, + "name": name.unwrap_or(&directory), + }); + let resp = match api_action(&token, &body.to_string()) { + Ok(r) => r, + Err(e) => { + eprintln!("{e}"); + return 1; + } + }; + let v: serde_json::Value = serde_json::from_str(&resp).unwrap_or(serde_json::Value::Null); + let project_id = match v.get("project_id").and_then(|x| x.as_str()) { + Some(id) => id.to_string(), + None => { + eprintln!("clone_project did not return a project_id.\n{resp}"); + return 1; + } + }; + print_response_ids(&resp, &["project_id", "path"]); + // The clone is OPTIMISTIC, same contract as `worktree add`: `pending: true` + // means the checkout is still running, so `path` does not exist on disk yet. + // On a later failure the row is removed from state (visible in `okena ls`) + // plus a toast. + if v.get("pending").and_then(|p| p.as_bool()).unwrap_or(false) { + eprintln!("clone started in the background; the path will exist once it completes"); + } + project_add_followups(&token, &project_id, hidden, folder, window) +} + +/// Post-add placement shared by `project add` and `project clone`: hide the +/// project and/or move it into a folder. Returns the process exit code. +fn project_add_followups( + token: &str, + project_id: &str, + hidden: bool, + folder: Option<&str>, + window: Option<&str>, +) -> i32 { // Follow-up: hide. if hidden { let mut hide_body = serde_json::json!({ @@ -1159,12 +1253,12 @@ pub fn cli_project_add( "show": false, }); if let Some(w) = window - && let Err(e) = apply_window(&mut hide_body, &token, w) + && let Err(e) = apply_window(&mut hide_body, token, w) { eprintln!("{e}"); return 1; } - if let Err(e) = api_action(&token, &hide_body.to_string()) { + if let Err(e) = api_action(token, &hide_body.to_string()) { eprintln!("Warning: failed to hide project: {e}"); return 1; } @@ -1172,7 +1266,7 @@ pub fn cli_project_add( // Follow-up: move into a folder. if let Some(folder_filter) = folder { - let state = match fetch_state(&token) { + let state = match fetch_state(token) { Ok(s) => s, Err(e) => { eprintln!("Warning: could not resolve folder: {e}"); @@ -1191,7 +1285,7 @@ pub fn cli_project_add( "project_id": project_id, "folder_id": folder_id, }); - if let Err(e) = api_action(&token, &move_body.to_string()) { + if let Err(e) = api_action(token, &move_body.to_string()) { eprintln!("Warning: failed to move project into folder: {e}"); return 1; } diff --git a/crates/okena-cli/src/lib.rs b/crates/okena-cli/src/lib.rs index 0788b5c5a..8e67f6fb5 100644 --- a/crates/okena-cli/src/lib.rs +++ b/crates/okena-cli/src/lib.rs @@ -65,6 +65,7 @@ fn command_uses_window(cmd: &Command) -> bool { Command::Project { cmd } => matches!( cmd, ProjectCmd::Add { .. } + | ProjectCmd::Clone { .. } | ProjectCmd::Show { .. } | ProjectCmd::Hide { .. } | ProjectCmd::Focus { .. } @@ -82,7 +83,7 @@ fn dispatch(cli: Cli) -> i32 { let window = cli.window.as_deref(); if cli.window.is_some() && !command_uses_window(&cli.command) { eprintln!( - "Warning: --window is ignored by this command. Only `project add/show/hide/focus` and `term focus/fullscreen` honor it." + "Warning: --window is ignored by this command. Only `project add/clone/show/hide/focus` and `term focus/fullscreen` honor it." ); } match cli.command { @@ -120,6 +121,22 @@ fn dispatch(cli: Cli) -> i32 { } => { commands::cli_project_add(&path, name.as_deref(), hidden, folder.as_deref(), window) } + ProjectCmd::Clone { + url, + into, + dir, + name, + hidden, + folder, + } => commands::cli_project_clone( + &url, + into.as_deref(), + dir.as_deref(), + name.as_deref(), + hidden, + folder.as_deref(), + window, + ), ProjectCmd::Rm { project } => commands::cli_project_rm(&project), ProjectCmd::Show { project } => commands::cli_project_show(&project, true, window), ProjectCmd::Hide { project } => commands::cli_project_show(&project, false, window), diff --git a/crates/okena-cli/src/parser.rs b/crates/okena-cli/src/parser.rs index 8264247e2..23af812ce 100644 --- a/crates/okena-cli/src/parser.rs +++ b/crates/okena-cli/src/parser.rs @@ -338,6 +338,26 @@ pub enum ProjectCmd { #[arg(long)] folder: Option, }, + /// Clone a git repository and add the checkout as a project + Clone { + /// Repository URL (anything `git clone` accepts) + url: String, + /// Parent directory to clone into (defaults to the CWD) + #[arg(long)] + into: Option, + /// Directory name inside the parent (defaults to the repo name) + #[arg(long)] + dir: Option, + /// Display name (defaults to the directory name) + #[arg(long)] + name: Option, + /// Add hidden (not shown in the overview) + #[arg(long)] + hidden: bool, + /// Move into a folder (by name or id) after adding + #[arg(long)] + folder: Option, + }, /// Remove a project (unlinks it from Okena; the folder on disk is kept) Rm { /// Project (id / name / path) diff --git a/crates/okena-cli/src/resolve.rs b/crates/okena-cli/src/resolve.rs index 0164b31af..0bfe3c6c3 100644 --- a/crates/okena-cli/src/resolve.rs +++ b/crates/okena-cli/src/resolve.rs @@ -318,6 +318,7 @@ mod tests { hooks: Default::default(), is_creating: false, is_closing: false, + creating_progress: None, } } diff --git a/crates/okena-cli/src/skill.md b/crates/okena-cli/src/skill.md index 92d111c90..7d601cb3a 100644 --- a/crates/okena-cli/src/skill.md +++ b/crates/okena-cli/src/skill.md @@ -41,7 +41,8 @@ okena key okena:0 ctrl-c # interrupt ## Manage the workspace -- Projects: `okena project add | rm | rename | color | focus | show | hide` +- Projects: `okena project add | clone | rm | rename | color | focus | show | hide` + (`clone` takes `--into ` (default CWD), `--dir `, `--name `) - Layout: `okena term new | close | rename | split | tab | focus | minimize | fullscreen` (`split h` = stacked top/bottom, `split v` = side by side left/right) - Worktrees: `okena worktree add [--new-branch] | rm` @@ -58,8 +59,8 @@ okena key okena:0 ctrl-c # interrupt `--keep-config` accepts the risk of an incompatible newer config. - Raw: `okena state` (full JSON), `okena action ''` (any ActionRequest). -Commands that create things (`term new/split/tab`, `project add`, `worktree add`, -`folder add`) print the new id to stdout. +Commands that create things (`term new/split/tab`, `project add`, `project clone`, +`worktree add`, `folder add`) print the new id to stdout. ## Gotchas @@ -70,11 +71,11 @@ Commands that create things (`term new/split/tab`, `project add`, `worktree add` - **`run --wait` assumes a POSIX-ish shell** (bash/zsh/sh) and a non-interactive command (it appends a completion marker). Don't use it for vim/REPLs. - **A bare `run` reports no completion or exit code** — only `run --wait` does. -- **`worktree add` is optimistic**: it prints the id + path and returns before the - checkout exists on disk (the reply carries `pending: true`). Don't `cd` into the - path immediately — poll `okena ls`/`okena state` until the worktree's terminal - appears; if creation fails the row disappears from state. -- **`--window` is honored only by** `project add/show/hide/focus` and +- **`worktree add` and `project clone` are optimistic**: they print the id + path + and return before the checkout exists on disk (the reply carries `pending: true`). + Don't `cd` into the path immediately — poll `okena ls`/`okena state` until the + project's terminal appears; if creation fails the row disappears from state. +- **`--window` is honored only by** `project add/clone/show/hide/focus` and `term focus/fullscreen`, and must come AFTER the subcommand; others just warn. - Default output is tab-separated (grep/awk friendly); add `--json` for structured. `okena ls --json` is a structured overview; `okena state` is the raw dump. diff --git a/crates/okena-core/src/api.rs b/crates/okena-core/src/api.rs index 4c76e6f51..ef45d0e41 100644 --- a/crates/okena-core/src/api.rs +++ b/crates/okena-core/src/api.rs @@ -370,6 +370,12 @@ pub struct ApiProject { /// closing". #[serde(default)] pub is_closing: bool, + /// How far the in-flight clone behind this project has got, e.g. + /// `Receiving objects: 42%`. Clients show it inside the creating + /// placeholder. serde-defaulted so older peers that omit it decode as + /// "creating, but no detail" rather than failing. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub creating_progress: Option, } /// Wire mirror of `okena_state::HookTerminalStatus` (which can't be referenced @@ -829,6 +835,16 @@ pub enum ActionRequest { name: String, path: String, }, + /// Clone `url` into `parent_dir`/`directory` and add the checkout as a + /// project. The parent and the directory name travel separately so the + /// receiving host joins them with ITS own separator — a remote daemon may + /// not share the caller's path conventions. + CloneProject { + url: String, + parent_dir: String, + directory: String, + name: String, + }, ReorderProjectInFolder { folder_id: String, project_id: String, @@ -1360,6 +1376,7 @@ mod tests { }, is_creating: false, is_closing: false, + creating_progress: None, }], focused_project_id: Some("p1".into()), fullscreen_terminal: None, @@ -1675,6 +1692,12 @@ mod tests { name: "My Project".into(), path: "/home/user/projects/my-project".into(), }, + ActionRequest::CloneProject { + url: "https://github.com/user/my-project.git".into(), + parent_dir: "/home/user/projects".into(), + directory: "my-project".into(), + name: "My Project".into(), + }, ActionRequest::ReorderProjectInFolder { folder_id: "f1".into(), project_id: "p1".into(), diff --git a/crates/okena-core/src/process/bus.rs b/crates/okena-core/src/process/bus.rs index 382db4911..8e62a1bf3 100644 --- a/crates/okena-core/src/process/bus.rs +++ b/crates/okena-core/src/process/bus.rs @@ -103,6 +103,31 @@ pub fn current_lane() -> Lane { CURRENT_LANE.with(|c| c.get()) } +/// A live feed of a command's stderr lines, delivered while it still runs. +/// +/// The bus otherwise hands back all output at once when the process exits, +/// which is fine for a verdict and useless for progress: a long `git clone` +/// reports where it has got to only as it goes. Held behind an `Arc` because +/// the reader thread owns a clone of it. +#[derive(Clone)] +pub struct StderrSink(Arc); + +impl StderrSink { + pub fn new(sink: impl Fn(&str) + Send + Sync + 'static) -> Self { + Self(Arc::new(sink)) + } + + fn emit(&self, line: &str) { + (self.0)(line); + } +} + +impl std::fmt::Debug for StderrSink { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("StderrSink") + } +} + /// A fully-described one-shot command. Built directly, or extracted from a /// configured [`std::process::Command`] via [`CommandSpec::from_command`]. #[derive(Debug, Clone)] @@ -120,6 +145,8 @@ pub struct CommandSpec { /// be killed at once via [`CommandBus::cancel_scope`] (e.g. on project /// close or app shutdown). pub scope: Option, + /// Optional live feed of stderr lines. See [`StderrSink`]. + pub stderr_sink: Option, } impl CommandSpec { @@ -133,6 +160,7 @@ impl CommandSpec { lane: current_lane(), label: None, scope: None, + stderr_sink: None, } } @@ -180,6 +208,13 @@ impl CommandSpec { self } + /// Report stderr lines to `sink` as they arrive, on top of capturing them. + /// For progress on commands too long to watch in silence. + pub fn on_stderr_line(mut self, sink: impl Fn(&str) + Send + Sync + 'static) -> Self { + self.stderr_sink = Some(StderrSink::new(sink)); + self + } + /// Extract a spec from an already-configured [`std::process::Command`]. /// /// This is what lets `safe_output(command("git").args(..).current_dir(..))` @@ -216,6 +251,7 @@ impl CommandSpec { lane: current_lane(), label: None, scope: None, + stderr_sink: None, } } @@ -327,6 +363,22 @@ pub struct CommandHandle { ctl: Arc, } +/// Cloneable cancellation capability for a submitted command. +/// +/// Keep this outside a blocking waiter when the caller must still be able to +/// stop the child process during async task cancellation or application +/// shutdown. +#[derive(Clone)] +pub struct CommandCancellation { + ctl: Arc, +} + +impl CommandCancellation { + pub fn cancel(&self) { + self.ctl.cancel(); + } +} + impl CommandHandle { /// Block until the command finishes, returning its captured output. Returns /// an `Other` error if the bus worker died, or `Interrupted` if cancelled. @@ -342,6 +394,13 @@ impl CommandHandle { pub fn cancel(&self) { self.ctl.cancel(); } + + /// Return a cancellation capability that can outlive this waiting handle. + pub fn cancellation(&self) -> CommandCancellation { + CommandCancellation { + ctl: self.ctl.clone(), + } + } } /// FIFO work queue shared by one lane's workers. @@ -555,7 +614,10 @@ fn spawn_and_collect(spec: &CommandSpec, ctl: &Arc) -> std::io::Resu // blocks on `write` forever while we wait for it to exit and never drain — // a classic deadlock, hit by e.g. a large `docker ps -a` or `git diff`. let out_reader = spawn_pipe_reader(child.stdout.take()); - let err_reader = spawn_pipe_reader(child.stderr.take()); + let err_reader = match spec.stderr_sink.clone() { + Some(sink) => spawn_streaming_pipe_reader(child.stderr.take(), sink), + None => spawn_pipe_reader(child.stderr.take()), + }; // Publish the kill handle so cancel()/cancel_scope() can reach the whole // process tree. The registration clears it before the OS identity can be @@ -661,6 +723,63 @@ fn spawn_pipe_reader( }) } +/// Longest run of bytes accepted as one line before it is flushed anyway, so a +/// stream that never breaks cannot grow the buffer without bound. +const MAX_SINK_LINE: usize = 8 * 1024; + +/// Like [`spawn_pipe_reader`], but also hands each line to `sink` as it +/// arrives rather than only at exit. +/// +/// Both `\r` and `\n` end a line: progress-reporting tools separate updates +/// with a carriage return so the line rewrites itself in place, and splitting +/// on `\n` alone would hold every update back until the very end — exactly the +/// buffering this exists to avoid. The full byte stream is still accumulated, +/// so the command's captured stderr is unchanged. +fn spawn_streaming_pipe_reader( + pipe: Option, + sink: StderrSink, +) -> std::thread::JoinHandle> { + std::thread::spawn(move || { + let mut all = Vec::new(); + let Some(mut pipe) = pipe else { + return all; + }; + let mut chunk = [0u8; 4096]; + let mut line = Vec::new(); + loop { + let read = match pipe.read(&mut chunk) { + Ok(0) | Err(_) => break, + Ok(read) => read, + }; + all.extend_from_slice(&chunk[..read]); + for &byte in &chunk[..read] { + if byte == b'\n' || byte == b'\r' { + emit_sink_line(&sink, &mut line); + } else { + line.push(byte); + if line.len() >= MAX_SINK_LINE { + emit_sink_line(&sink, &mut line); + } + } + } + } + emit_sink_line(&sink, &mut line); + all + }) +} + +/// Emit `line` if it holds anything, and clear it either way. +fn emit_sink_line(sink: &StderrSink, line: &mut Vec) { + if !line.is_empty() { + let text = String::from_utf8_lossy(line); + let text = text.trim(); + if !text.is_empty() { + sink.emit(text); + } + line.clear(); + } +} + fn wait_for_readers( stdout: &std::thread::JoinHandle>, stderr: &std::thread::JoinHandle>, @@ -695,7 +814,27 @@ impl ProcessTree { fn spawn(cmd: &mut std::process::Command) -> std::io::Result<(std::process::Child, Arc)> { use std::os::unix::process::CommandExt; - cmd.process_group(0); + // SAFETY: the closure runs in the forked child before exec and calls + // only async-signal-safe syscalls. + unsafe { + cmd.pre_exec(|| { + // `setsid` buys two things at once. A new session has no + // controlling terminal, so a child that tries to prompt on + // `/dev/tty` — git asking for credentials, ssh for a + // passphrase — fails instead of taking SIGTTIN and stopping + // forever where nothing can observe it. And a new session + // starts a new process group whose id is this pid, which is + // the identity `terminate` kills. + // + // It only fails if we are already a group leader, which a + // fresh fork is not; keep `setpgid` as the fallback anyway, + // because the group invariant below must hold either way. + if libc::setsid() == -1 && libc::setpgid(0, 0) == -1 { + return Err(std::io::Error::last_os_error()); + } + Ok(()) + }); + } let mut child = cmd.spawn()?; let process_group = match libc::pid_t::try_from(child.id()) { Ok(process_group) if process_group > 0 => process_group, diff --git a/crates/okena-core/src/process/mod.rs b/crates/okena-core/src/process/mod.rs index 7914cc691..1b59bd011 100644 --- a/crates/okena-core/src/process/mod.rs +++ b/crates/okena-core/src/process/mod.rs @@ -14,7 +14,10 @@ mod bus; -pub use bus::{CommandBus, CommandHandle, CommandSpec, Lane, current_lane, with_lane}; +pub use bus::{ + CommandBus, CommandCancellation, CommandHandle, CommandSpec, Lane, StderrSink, current_lane, + with_lane, +}; /// Create a [`std::process::Command`] that does **not** flash a console /// window on Windows. On other platforms this is identical to @@ -273,6 +276,93 @@ pub mod testing { #[cfg(test)] mod tests { + /// The point of the sink is arrival time, not content: lines must reach it + /// while the command still runs. Buffering them until exit would make + /// progress reporting useless, and that is what the plain reader does. + #[test] + fn stderr_lines_reach_the_sink_before_the_command_exits() { + use std::sync::{Arc, Mutex}; + + let seen: Arc>> = Arc::new(Mutex::new(Vec::new())); + let observed_early = Arc::new(Mutex::new(false)); + + let sink_seen = seen.clone(); + let sink_early = observed_early.clone(); + let output = super::run( + super::CommandSpec::new("sh") + .args([ + "-c", + // Marker, a pause, then exit. A sink that only fired at + // exit could not have seen the marker during the sleep. + "printf 'first\n' >&2; sleep 1; printf 'second\n' >&2", + ]) + .on_stderr_line(move |line| { + sink_seen.lock().expect("lock").push(line.to_string()); + if line == "first" { + *sink_early.lock().expect("lock") = true; + } + }), + ) + .expect("run sh"); + + assert!( + *observed_early.lock().expect("lock"), + "the first line must arrive before the command finishes" + ); + assert_eq!( + *seen.lock().expect("lock"), + vec!["first".to_string(), "second".to_string()] + ); + // Capture is unaffected. + assert_eq!(String::from_utf8_lossy(&output.stderr), "first\nsecond\n"); + } + + /// Progress tools rewrite one line with `\r`. Splitting on `\n` alone + /// would hold every update back until the command ended. + #[test] + fn carriage_returns_separate_progress_updates() { + use std::sync::{Arc, Mutex}; + + let seen: Arc>> = Arc::new(Mutex::new(Vec::new())); + let sink_seen = seen.clone(); + super::run( + super::CommandSpec::new("sh") + .args(["-c", "printf 'a: 10%%\rb: 50%%\rc: 100%%\n' >&2"]) + .on_stderr_line(move |line| { + sink_seen.lock().expect("lock").push(line.to_string()); + }), + ) + .expect("run sh"); + + assert_eq!( + *seen.lock().expect("lock"), + vec![ + "a: 10%".to_string(), + "b: 50%".to_string(), + "c: 100%".to_string() + ] + ); + } + + /// Every bus child must lead its own session, so it has no controlling + /// terminal to be stopped on. Without this a `git clone` that hits a + /// credential prompt takes SIGTTIN and hangs forever instead of failing. + /// + /// Session id equal to the pid is exactly what `setsid` produces, and it + /// is also the process-group identity the bus kills on cancel. + #[cfg(target_os = "linux")] + #[test] + fn a_bus_child_leads_its_own_session() { + let output = + super::run(super::CommandSpec::new("sh").args(["-c", "ps -o sid= -o pid= -p $$"])) + .expect("run ps"); + let text = String::from_utf8_lossy(&output.stdout); + let mut fields = text.split_whitespace(); + let sid: i64 = fields.next().and_then(|f| f.parse().ok()).expect("sid"); + let pid: i64 = fields.next().and_then(|f| f.parse().ok()).expect("pid"); + assert_eq!(sid, pid, "child should be a session leader, got {text:?}"); + } + use super::*; use std::sync::Mutex; use std::time::Duration; @@ -460,9 +550,10 @@ mod tests { fn cancel_kills_running_command() { let _g = guard(); let handle = CommandBus::global().submit(CommandSpec::new("sleep").arg("30")); + let cancellation = handle.cancellation(); // Give the worker a moment to spawn the child, then cancel. std::thread::sleep(Duration::from_millis(80)); - handle.cancel(); + cancellation.cancel(); let err = handle.wait().expect_err("cancelled"); assert_eq!(err.kind(), std::io::ErrorKind::Interrupted); } diff --git a/crates/okena-daemon-core/src/command_loop.rs b/crates/okena-daemon-core/src/command_loop.rs index d2e454f9e..1898c4fc4 100644 --- a/crates/okena-daemon-core/src/command_loop.rs +++ b/crates/okena-daemon-core/src/command_loop.rs @@ -39,7 +39,12 @@ use std::collections::{HashMap, HashSet}; use std::path::PathBuf; use std::sync::Arc; -use std::time::Duration; +use std::time::{Duration, Instant}; + +/// Shortest gap between two clone-progress publishes. Each one takes the +/// workspace lock and pushes a snapshot to every connected client, so the +/// limit is about their cost, not about how often git has something to say. +const CLONE_PROGRESS_INTERVAL: Duration = Duration::from_millis(250); use okena_app_core::remote_snapshot::build_state_response; #[cfg(test)] @@ -135,6 +140,26 @@ struct SettingsUpdateOutcome { committed: bool, } +struct CancelCommandOnDrop(Option); + +impl CancelCommandOnDrop { + fn new(cancellation: okena_core::process::CommandCancellation) -> Self { + Self(Some(cancellation)) + } + + fn disarm(&mut self) { + self.0 = None; + } +} + +impl Drop for CancelCommandOnDrop { + fn drop(&mut self) { + if let Some(cancellation) = self.0.take() { + cancellation.cancel(); + } + } +} + impl SettingsUpdateOutcome { fn uncommitted(result: CommandResult) -> Self { Self { @@ -2876,6 +2901,257 @@ pub async fn daemon_command_loop( } } + // ── Clone project: run the blocking git off the reactor ────── + // Same split as `CreateWorktree` below, for the same reason — + // only more so: `git clone` is network-bound and unbounded in + // duration, so holding the workspace lock for it would freeze + // the daemon for however long the repo takes to fetch. + // Register an optimistic row (no layout → the client renders + // the "creating" placeholder), clone with NO lock held, then + // seed the layout + fire on_project_open + spawn PTYs under a + // brief lock. On failure the row is rolled back and toasted. + ActionRequest::CloneProject { + url, + parent_dir, + directory, + name, + } => { + // Phase 0: validate + resolve the target, no lock held. An + // unusable URL or directory name fails the request outright + // instead of creating a row that vanishes a moment later. + let prepared = okena_git::validate_clone_url(&url) + .map_err(|e| e.to_string()) + .and_then(|_| { + okena_workspace::actions::project::resolve_clone_target( + &parent_dir, + &directory, + ) + }); + match prepared { + Err(e) => CommandResult::Err(e), + Ok(target) => { + let target_path = target.to_string_lossy().into_owned(); + let project_name = + okena_workspace::actions::project::clone_project_name( + &name, &directory, + ); + let app_settings = settings.lock().clone(); + let registered = { + let mut cx = DaemonWorkspaceCx::new( + &workspace_tick, + &hook_runner, + &hook_monitor, + ); + let mut ws = workspace.lock(); + let registered = ws.register_pending_project( + project_name, + target_path.clone(), + WindowId::Main, + &mut cx, + ); + // Mark creating only on success, so a rejected + // path claim propagates its own error. + if let Ok(id) = ®istered { + ws.mark_creating_project(id); + } + let operation_epoch = ws.data_replacement_epoch(); + registered.map(|id| (id, operation_epoch)) + }; + match registered { + Err(e) => CommandResult::Err(e), + Ok((new_id, operation_epoch)) => 'start_clone: { + let clone_command = match okena_git::start_clone_repository( + &url, + &target, + { + let workspace = workspace.clone(); + let workspace_tick = workspace_tick.clone(); + let hook_runner = hook_runner.clone(); + let hook_monitor = hook_monitor.clone(); + let project_id = new_id.clone(); + // git reports several updates a + // second and each publish takes the + // workspace lock and broadcasts a + // snapshot to every client, so rate + // limit. 100% always goes through so + // a phase never appears to stall + // short of finishing. + let last: parking_lot::Mutex> = + parking_lot::Mutex::new(None); + move |progress: okena_git::CloneProgress| { + { + let mut last = last.lock(); + let now = Instant::now(); + let due = progress.percent == 100 + || last.is_none_or(|at| { + now.duration_since(at) + >= CLONE_PROGRESS_INTERVAL + }); + if !due { + return; + } + *last = Some(now); + } + let mut cx = DaemonWorkspaceCx::new( + &workspace_tick, + &hook_runner, + &hook_monitor, + ); + let mut ws = workspace.lock(); + if ws.set_creating_progress( + &project_id, + progress.summary(), + ) { + ws.notify_data(&mut cx); + } + } + }, + ) { + Ok(command) => command, + Err(error) => { + let mut cx = DaemonWorkspaceCx::new( + &workspace_tick, + &hook_runner, + &hook_monitor, + ); + let mut ws = workspace.lock(); + ws.finish_creating_project(&new_id); + ws.remove_pending_project(&new_id); + ws.notify_data(&mut cx); + break 'start_clone CommandResult::Err( + error.to_string(), + ); + } + }; + let clone_cancellation = clone_command.cancellation(); + let workspace = workspace.clone(); + let workspace_tick = workspace_tick.clone(); + let hook_runner = hook_runner.clone(); + let hook_monitor = hook_monitor.clone(); + let backend = backend.clone(); + let terminals = terminals.clone(); + let app_settings = app_settings.clone(); + let new_id_task = new_id.clone(); + let cancel_on_drop = + CancelCommandOnDrop::new(clone_cancellation); + tokio::task::spawn_local(async move { + // `spawn_blocking` itself cannot be aborted. Keep a + // separate command-bus cancellation capability in + // the async task so dropping this future at daemon + // shutdown kills the git process tree and releases + // the blocking waiter. + let mut cancel_on_drop = cancel_on_drop; + let git = tokio::task::spawn_blocking(move || { + okena_git::finish_clone_repository(clone_command) + }) + .await; + cancel_on_drop.disarm(); + + // The workspace was replaced under us (session + // load / import): the row this clone belongs to + // is gone. Unlike a worktree checkout, the + // clone is left on disk — it is a plain + // directory of the user's code, and deleting it + // unprompted is worse than leaving it. + if workspace.lock().data_replacement_epoch() + != operation_epoch + { + log::info!( + "clone-project: ignoring stale completion for {new_id_task}" + ); + return; + } + + match git { + Ok(Ok(())) => { + let mut cx = DaemonWorkspaceCx::new( + &workspace_tick, + &hook_runner, + &hook_monitor, + ); + let mut ws = workspace.lock(); + // Seeds the layout, then fires on_project_open. + ws.finish_pending_project( + &new_id_task, + &app_settings.hooks, + &mut cx, + ); + // Clear creating BEFORE spawning — + // spawn_uninitialized_terminals no-ops + // while the project is creating. + ws.finish_creating_project(&new_id_task); + let _ = spawn_uninitialized_terminals( + &mut ws, + &new_id_task, + &*backend, + &terminals, + &app_settings, + None, + &mut cx, + ); + ws.notify_data(&mut cx); + } + result => { + // Two renderings: the whole + // thing for the log, git's own + // error line for the toast. + let (msg, detail) = match result { + Ok(Err(e)) => { + (e.to_string(), e.user_detail()) + } + Err(join) => { + let m = format!( + "clone task failed: {join}" + ); + (m.clone(), m) + } + Ok(Ok(())) => { + unreachable!("success handled above") + } + }; + // Roll the optimistic row back. Clear + // creating FIRST — remove_pending_project + // skips projects still marked creating. + { + let mut cx = DaemonWorkspaceCx::new( + &workspace_tick, + &hook_runner, + &hook_monitor, + ); + let mut ws = workspace.lock(); + ws.finish_creating_project(&new_id_task); + ws.remove_pending_project(&new_id_task); + ws.notify_data(&mut cx); + } + log::error!( + "clone-project: {url} failed: {msg}" + ); + if let Some(hm) = &hook_monitor { + hm.push_toast( + okena_state::Toast::error( + "Clone failed", + ) + .with_detail(detail), + ); + } + } + } + }); + // OPTIMISTIC reply, same contract as + // `CreateWorktree`: `pending: true` means the + // checkout is still running, so `path` does not + // exist on disk yet. + CommandResult::Ok(Some(serde_json::json!({ + "project_id": new_id, + "path": target_path, + "pending": true, + }))) + } + } + } + } + } + // ── Create worktree: run the blocking git off the reactor ──── // `git fetch` + `git worktree add` are network/disk-heavy (up to // seconds on a cold fetch). Routing them through the synchronous @@ -5787,6 +6063,7 @@ mod tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, }; WorkspaceData { version: 1, @@ -6144,6 +6421,7 @@ mod tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, }; WorkspaceData { version: 1, @@ -6884,6 +7162,7 @@ mod tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, }; WorkspaceData { version: 1, @@ -7126,6 +7405,7 @@ mod tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, } }; let parent = mk("p1", None, vec!["wt1".to_string()]); @@ -8603,6 +8883,7 @@ mod tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, }; let data = WorkspaceData { version: 1, diff --git a/crates/okena-daemon-core/src/daemon.rs b/crates/okena-daemon-core/src/daemon.rs index 41e2d8fe6..9d626f5c4 100644 --- a/crates/okena-daemon-core/src/daemon.rs +++ b/crates/okena-daemon-core/src/daemon.rs @@ -828,6 +828,7 @@ mod shutdown_tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, }; project .terminal_names @@ -918,6 +919,7 @@ mod shutdown_tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, }; project.hook_terminals.insert( "persistent-hook".to_string(), @@ -1001,6 +1003,7 @@ mod shutdown_tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, }); data.project_order.push("p1".to_string()); let workspace = Arc::new(Mutex::new(Workspace::new(data))); diff --git a/crates/okena-daemon-core/src/git_poll.rs b/crates/okena-daemon-core/src/git_poll.rs index 360aafe06..ceb4ec007 100644 --- a/crates/okena-daemon-core/src/git_poll.rs +++ b/crates/okena-daemon-core/src/git_poll.rs @@ -1121,6 +1121,7 @@ mod tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, }); data.project_order.push(id.to_string()); data.main_window.hidden_project_ids.insert(id.to_string()); diff --git a/crates/okena-daemon-core/src/observers.rs b/crates/okena-daemon-core/src/observers.rs index 9a7d9a724..106eba9eb 100644 --- a/crates/okena-daemon-core/src/observers.rs +++ b/crates/okena-daemon-core/src/observers.rs @@ -962,6 +962,7 @@ mod tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, }); okena_workspace::state::Workspace::new(data) } diff --git a/crates/okena-daemon-core/src/pty_loop.rs b/crates/okena-daemon-core/src/pty_loop.rs index bb268fd3c..3f08cccb3 100644 --- a/crates/okena-daemon-core/src/pty_loop.rs +++ b/crates/okena-daemon-core/src/pty_loop.rs @@ -980,6 +980,7 @@ mod tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, }; let child = ProjectData { id: "wt1".into(), @@ -1019,6 +1020,7 @@ mod tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, }; let mut workspace = Workspace::new(WorkspaceData { version: 1, @@ -1066,6 +1068,7 @@ mod tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, } } diff --git a/crates/okena-daemon-core/src/soft_close.rs b/crates/okena-daemon-core/src/soft_close.rs index 51cb2f83a..cb99bc3b4 100644 --- a/crates/okena-daemon-core/src/soft_close.rs +++ b/crates/okena-daemon-core/src/soft_close.rs @@ -163,6 +163,7 @@ mod tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, }; WorkspaceData { version: 1, diff --git a/crates/okena-daemon-core/src/worktree_close_watchdog.rs b/crates/okena-daemon-core/src/worktree_close_watchdog.rs index f967d8ed8..9065104c4 100644 --- a/crates/okena-daemon-core/src/worktree_close_watchdog.rs +++ b/crates/okena-daemon-core/src/worktree_close_watchdog.rs @@ -114,6 +114,7 @@ mod tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, }; project.hook_terminals.insert( terminal_id.into(), diff --git a/crates/okena-git/CLAUDE.md b/crates/okena-git/CLAUDE.md index e0bd6c1d8..8241f5027 100644 --- a/crates/okena-git/CLAUDE.md +++ b/crates/okena-git/CLAUDE.md @@ -8,8 +8,9 @@ Git status, diff parsing, and worktree operations for project directories. |------|---------| | `lib.rs` | `GitStatus` — cached git status. Tracks branch, dirty state, ahead/behind counts. PR/CI types (`PrInfo`, `PrState`, `CiCheck`, `CiStatus`, `CiCheckSummary`). `validate_git_ref`. Re-exports the `repository` API. | | `diff.rs` | Diff parsing — `DiffLine`, `DiffHunk`, `DiffResult`, `DiffMode` (unified/side-by-side). Parses `git diff` output into structured data. | -| `repository/` | Repository operations, split into submodules. `mod.rs` declares them, re-exports the public API (so `okena_git::repository::*` paths are unchanged), and holds shared private helpers (`require_success`, `path_str`, `head_branch_short`, `get_worktree_branches`) plus `#[cfg(test)] test_support` (shared `init_temp_repo` / `git_in`). | +| `repository/` | Repository operations, split into submodules. `mod.rs` declares them, re-exports the public API (so `okena_git::repository::*` paths are unchanged), and holds shared private helpers (`require_success`, `path_str`, `head_branch_short`, `get_worktree_branches`, `network_command`) plus `#[cfg(test)] test_support` (shared `init_temp_repo` / `git_in`). | | `repository/worktree.rs` | Worktree ops — `create_worktree`, `create_worktree_with_start_point`, `remove_worktree`, `remove_worktree_fast`, `list_git_worktrees`, stale-dir cleanup. Destructive ops take a freshly verified token: `VerifiedWorktree` (`verify_linked_worktree_fresh`) for tracked checkouts, `OrphanedWorktree` (`verify_orphaned_worktree` → `remove_orphaned_worktree`) for one whose metadata entry was pruned. | +| `repository/clone.rs` | Clone ops — `clone_repository` (runs on `Lane::Long`; a clone is network-bound and unbounded), `clone_dir_name` (the directory `git clone` would create, for prefilling), `validate_clone_url`. | | `repository/branch.rs` | Branch ops — list/classify (`BranchList`), checkout/create/delete/push, `get_default_branch`, rebase, merge, stash, per-file stage/unstage/discard. | | `repository/status.rs` | Working-tree status & diff stats — `StatusFetch`, `get_status`, `has_uncommitted_changes`, `get_current_branch`, `get_head_sha`, diff-stats, ahead/behind & unpushed counts. | | `repository/ci.rs` | CI/PR integration — `fetch_pr_info`, `fetch_ci_checks`, and the pure, unit-tested parsers `parse_ci_checks` / `parse_branch_ci`. Both return `PrFetch`/`CiFetch` so callers can tell "no PR / no checks" from a rate-limit refusal; `fetch_ci_checks` skips the request entirely while the upstream commit still matches a settled cached result. | @@ -19,5 +20,6 @@ Git status, diff parsing, and worktree operations for project directories. ## Key Patterns - **Cached status**: Git status is cached in-memory and populated by background polling. `get_git_status` is non-blocking (returns cached data or None). +- **Remote git is non-interactive**: anything touching a remote (clone, fetch, push) is built with `network_command()`, never `command("git")`. Git prompts on `/dev/tty`, so a background child would take SIGTTIN and hang forever instead of failing. - **Worktree workflow**: Worktrees are managed as lightweight branch checkouts alongside the main repo. - **Diff views**: UI for diffs lives in `crates/okena-views-git/src/diff_viewer/`. diff --git a/crates/okena-git/src/error.rs b/crates/okena-git/src/error.rs index 377cf72bf..36de7a24a 100644 --- a/crates/okena-git/src/error.rs +++ b/crates/okena-git/src/error.rs @@ -35,10 +35,92 @@ pub enum GitError { #[error("invalid git ref: {0}")] InvalidRef(String), + /// A clone URL is empty or looks like a CLI flag. + #[error("invalid repository URL: {0}")] + InvalidUrl(String), + + /// Clone target directory already exists and is not empty. + #[error("directory '{path}' already exists and is not empty")] + CloneTargetExists { path: PathBuf }, + /// Failed to parse structured output (JSON, etc.). #[error("parse error: {0}")] ParseError(String), } +impl GitError { + /// The one line worth putting in front of a user. + /// + /// Git writes progress to stderr alongside errors, so a failed command's + /// full message opens with chatter (`Cloning into '...'`) and buries the + /// cause several lines down. Pick out git's own error line instead; the + /// untouched message still goes to the log. + pub fn user_detail(&self) -> String { + match self { + GitError::GitExitError { status, stderr } => git_failure_line(stderr) + .unwrap_or_else(|| format!("git exited with status {status}")), + other => other.to_string(), + } + } +} + +/// The last line git marked as the failure, without its prefix. +/// +/// Last rather than first: when git reports several, the final one is the +/// operation's actual verdict. +fn git_failure_line(stderr: &str) -> Option { + const PREFIXES: [&str; 4] = ["fatal: ", "error: ", "remote: error: ", "warning: "]; + stderr.lines().rev().find_map(|line| { + let line = line.trim(); + PREFIXES + .iter() + .find_map(|prefix| line.strip_prefix(prefix)) + .filter(|rest| !rest.is_empty()) + .map(str::to_string) + }) +} + /// Convenience alias for `Result`. pub type GitResult = Result; + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn the_user_sees_gits_error_line_not_its_progress_chatter() { + let err = GitError::GitExitError { + status: 128, + stderr: "Cloning into '/tmp/repo'...\n fatal: could not read Username for 'https://github.com': terminal prompts disabled" + .to_string(), + }; + assert_eq!( + err.user_detail(), + "could not read Username for 'https://github.com': terminal prompts disabled" + ); + } + + #[test] + fn the_last_failure_line_wins() { + let err = GitError::GitExitError { + status: 1, + stderr: "error: failed to push some refs\nfatal: the remote end hung up".to_string(), + }; + assert_eq!(err.user_detail(), "the remote end hung up"); + } + + #[test] + fn stderr_with_no_recognisable_line_falls_back_to_the_status() { + let err = GitError::GitExitError { + status: 129, + stderr: "usage: git clone [] [--] ".to_string(), + }; + assert_eq!(err.user_detail(), "git exited with status 129"); + } + + #[test] + fn other_error_kinds_keep_their_own_message() { + let err = GitError::InvalidUrl("-x".to_string()); + assert_eq!(err.user_detail(), "invalid repository URL: -x"); + } +} diff --git a/crates/okena-git/src/lib.rs b/crates/okena-git/src/lib.rs index 152060897..1d77e7aeb 100644 --- a/crates/okena-git/src/lib.rs +++ b/crates/okena-git/src/lib.rs @@ -16,17 +16,19 @@ pub use diff::{ }; pub use error::{GitError, GitResult}; pub use repository::{ - BranchList, HeadSnapshot, OrphanedWorktree, VerifiedWorktree, checkout_local_branch, - checkout_remote_branch, compute_target_paths, count_ahead_behind, count_unpushed_commits, - create_and_checkout_branch, create_worktree, create_worktree_with_start_point, - delete_local_branch, delete_remote_branch, discard_file_changes, fetch_all, - fetch_and_fast_forward, get_available_branches_for_worktree, get_current_branch, - get_default_branch, get_head_snapshot, get_repo_common_dir, get_repo_root, - has_uncommitted_changes, list_branches, list_branches_classified, list_linked_worktree_paths, - list_pull_requests, merge_branch, move_worktree, project_path_in_worktree, push_branch, + BranchList, CloneProgress, HeadSnapshot, OrphanedWorktree, VerifiedWorktree, + checkout_local_branch, checkout_remote_branch, clone_dir_name, clone_repository, + compute_target_paths, count_ahead_behind, count_unpushed_commits, create_and_checkout_branch, + create_worktree, create_worktree_with_start_point, delete_local_branch, delete_remote_branch, + discard_file_changes, fetch_all, fetch_and_fast_forward, finish_clone_repository, + get_available_branches_for_worktree, get_current_branch, get_default_branch, get_head_snapshot, + get_repo_common_dir, get_repo_root, has_uncommitted_changes, is_complete_checkout, + list_branches, list_branches_classified, list_linked_worktree_paths, list_pull_requests, + merge_branch, move_worktree, parse_clone_progress, project_path_in_worktree, push_branch, rebase_onto, remove_orphaned_worktree, remove_worktree, remove_worktree_fast, - resolve_git_root_and_subdir, resolve_review_base, stage_file, stash_changes, stash_pop, - unstage_file, verify_linked_worktree_fresh, verify_orphaned_worktree, + resolve_git_root_and_subdir, resolve_review_base, stage_file, start_clone_repository, + stash_changes, stash_pop, unstage_file, validate_clone_url, verify_linked_worktree_fresh, + verify_orphaned_worktree, }; /// Validate that a git ref (branch name, commit hash, revision) doesn't look diff --git a/crates/okena-git/src/repository/branch.rs b/crates/okena-git/src/repository/branch.rs index e92a5de38..c3d619232 100644 --- a/crates/okena-git/src/repository/branch.rs +++ b/crates/okena-git/src/repository/branch.rs @@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize}; use okena_core::process::{command, safe_output}; -use super::{head_branch_short, path_str, require_success}; +use super::{head_branch_short, network_command, path_str, require_success}; use crate::error::{GitError, GitResult}; /// List all branches in a repository (local + remotes), deduplicating @@ -192,7 +192,7 @@ pub fn discard_file_changes(repo_path: &Path, file_path: &str) -> GitResult<()> /// Fetch from all remotes. pub fn fetch_all(path: &Path) -> GitResult<()> { let p = path_str(path)?; - let output = safe_output(command("git").args(["-C", p, "fetch", "--all"]))?; + let output = safe_output(network_command().args(["-C", p, "fetch", "--all"]))?; require_success(output) } @@ -225,7 +225,7 @@ pub fn delete_remote_branch(repo_path: &Path, branch: &str) -> GitResult<()> { crate::validate_git_ref(branch)?; let p = path_str(repo_path)?; let output = - safe_output(command("git").args(["-C", p, "push", "origin", "--delete", "--", branch]))?; + safe_output(network_command().args(["-C", p, "push", "origin", "--delete", "--", branch]))?; require_success(output) } @@ -233,7 +233,7 @@ pub fn delete_remote_branch(repo_path: &Path, branch: &str) -> GitResult<()> { pub fn push_branch(repo_path: &Path, branch: &str) -> GitResult<()> { crate::validate_git_ref(branch)?; let p = path_str(repo_path)?; - let output = safe_output(command("git").args(["-C", p, "push", "origin", "--", branch]))?; + let output = safe_output(network_command().args(["-C", p, "push", "origin", "--", branch]))?; require_success(output) } diff --git a/crates/okena-git/src/repository/clone.rs b/crates/okena-git/src/repository/clone.rs new file mode 100644 index 000000000..1f1156fa3 --- /dev/null +++ b/crates/okena-git/src/repository/clone.rs @@ -0,0 +1,365 @@ +//! Clone a remote repository into a fresh directory. + +use std::path::Path; + +use okena_core::process::{CommandBus, CommandHandle, CommandSpec, Lane}; + +use super::{network_command, path_str, require_success}; +use crate::error::{GitError, GitResult}; + +/// Validate that a clone URL cannot be read by git as an option. +/// +/// `git clone` is invoked with a `--` separator, so a leading `-` is already +/// harmless there; this rejects it anyway so the bad input surfaces as a clear +/// error instead of a confusing git failure. Empty URLs are rejected too. +pub fn validate_clone_url(url: &str) -> GitResult<&str> { + let trimmed = url.trim(); + if trimmed.is_empty() || trimmed.starts_with('-') { + return Err(GitError::InvalidUrl(url.to_string())); + } + Ok(trimmed) +} + +/// Derive the directory name `git clone ` would create. +/// +/// Mirrors git's own rule: take the last non-empty path segment and drop a +/// trailing `.git`. Handles both URL forms (`https://host/a/b.git`) and the +/// scp-like form (`git@host:a/b.git`). Returns `None` when nothing usable is +/// left, so callers can ask the user for a name instead of guessing. +pub fn clone_dir_name(url: &str) -> Option { + let url = url.trim(); + // Strip a fragment/query before splitting — `?ref=x` is not part of the name. + let url = url.split(['?', '#']).next().unwrap_or(url); + // For a `scheme://host/path` URL the name comes from the path, so drop the + // scheme and host first — otherwise a hostless `https://` would yield the + // scheme itself. Everything else (scp-like `git@host:a/b`, a local path) is + // already just a path. + let path = match url.split_once("://") { + Some((_scheme, rest)) => rest.split_once('/')?.1, + None => url, + }; + // Both `/` and `:` separate the repo from its host in the forms git accepts. + let name = path + .trim_end_matches('/') + .rsplit(['/', ':', '\\']) + .find(|segment| !segment.is_empty())?; + let name = name.strip_suffix(".git").unwrap_or(name); + let name = name.trim(); + if name.is_empty() || name == "." || name == ".." { + return None; + } + Some(name.to_string()) +} + +/// Reject a clone target that already holds something. +/// +/// `git clone` refuses a non-empty directory itself, but checking up-front +/// keeps the failure fast and the message ours. +fn require_absent_clone_target(target_path: &Path) -> GitResult<()> { + let occupied = match std::fs::read_dir(target_path) { + Ok(mut entries) => entries.next().is_some(), + // Not a directory (a file sits there) still counts as occupied. + Err(e) if e.kind() == std::io::ErrorKind::NotFound => false, + Err(_) => target_path.exists(), + }; + if occupied { + return Err(GitError::CloneTargetExists { + path: target_path.to_path_buf(), + }); + } + Ok(()) +} + +/// How far along a `git clone` is, as reported by git itself. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct CloneProgress { + /// The phase git names, e.g. `Receiving objects`. + pub phase: String, + /// Percent complete within that phase, 0-100. + pub percent: u8, +} + +impl CloneProgress { + /// One short line for a UI: `Receiving objects: 42%`. + pub fn summary(&self) -> String { + format!("{}: {}%", self.phase, self.percent) + } +} + +/// Read one line of `git clone --progress` output. +/// +/// The lines that carry progress look like `Receiving objects: 42% (52/123)`, +/// sometimes behind a `remote: ` prefix when the phase runs on the server. +/// Everything else git writes — the opening `Cloning into '...'`, warnings, +/// the final `done.` summaries — carries no percentage and is skipped, so +/// callers can feed it every line without filtering first. +pub fn parse_clone_progress(line: &str) -> Option { + let line = line.trim().strip_prefix("remote: ").unwrap_or(line.trim()); + let (phase, rest) = line.split_once(':')?; + let phase = phase.trim(); + // Guard against picking up a URL or a path with a colon in it. + if phase.is_empty() || !phase.chars().all(|c| c.is_ascii_alphabetic() || c == ' ') { + return None; + } + let rest = rest.trim_start(); + let digits: String = rest.chars().take_while(char::is_ascii_digit).collect(); + if digits.is_empty() || !rest[digits.len()..].starts_with('%') { + return None; + } + let percent: u8 = digits.parse().ok().filter(|p| *p <= 100)?; + Some(CloneProgress { + phase: phase.to_string(), + percent, + }) +} + +/// Submit `git clone ` to the process bus. +/// +/// Runs on [`Lane::Long`]: a clone is network-bound and unbounded in duration, +/// so it must never occupy an interactive or poller slot. +/// +/// `on_progress` is called from the bus reader thread for each progress update +/// git reports, so it must be cheap and must not block. +pub fn start_clone_repository( + url: &str, + target_path: &Path, + on_progress: impl Fn(CloneProgress) + Send + Sync + 'static, +) -> GitResult { + let url = validate_clone_url(url)?; + require_absent_clone_target(target_path)?; + + let target_str = path_str(target_path)?; + if let Some(parent) = target_path.parent() { + std::fs::create_dir_all(parent).map_err(GitError::CommandFailed)?; + } + + let mut cmd = network_command(); + // `--progress` is required, not cosmetic: git reports progress only when + // stderr is a terminal, and the bus always pipes it. + cmd.args(["clone", "--progress", "--", url, target_str]); + Ok(CommandBus::global().submit( + CommandSpec::from_command(&cmd) + .lane(Lane::Long) + .label("git clone") + .on_stderr_line(move |line| { + if let Some(progress) = parse_clone_progress(line) { + on_progress(progress); + } + }), + )) +} + +/// Wait for a clone submitted by [`start_clone_repository`]. +pub fn finish_clone_repository(handle: CommandHandle) -> GitResult<()> { + require_success(handle.wait()?) +} + +/// Whether a clone into `path` ran all the way to a checked-out commit. +/// +/// `git clone` is not atomic: killed mid-fetch it leaves the target directory +/// behind holding a `.git` whose HEAD points at a branch that does not exist +/// yet. That wreckage is indistinguishable from a finished clone by existence +/// alone, so startup recovery asks this instead — otherwise it promotes a +/// repo with no files in it to a normal-looking project. +/// +/// Resolving HEAD is the line between the two: git writes it only once the +/// fetch has landed the branch it is about to check out. +pub fn is_complete_checkout(path: &Path) -> bool { + gix::open(path).is_ok_and(|repo| repo.head_id().is_ok()) +} + +/// Submit and synchronously wait for `git clone `, +/// discarding progress. +pub fn clone_repository(url: &str, target_path: &Path) -> GitResult<()> { + finish_clone_repository(start_clone_repository(url, target_path, |_| {})?) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn derives_the_directory_name_git_would_use() { + let cases = [ + ("https://github.com/user/okena.git", "okena"), + ("https://github.com/user/okena", "okena"), + ("https://github.com/user/okena/", "okena"), + ("git@github.com:user/okena.git", "okena"), + ("ssh://git@host:2222/user/okena.git", "okena"), + ("/srv/repos/okena.git", "okena"), + ("https://host/user/okena.git?ref=main", "okena"), + (" https://host/user/okena.git ", "okena"), + ]; + for (url, expected) in cases { + assert_eq!(clone_dir_name(url).as_deref(), Some(expected), "url: {url}"); + } + } + + #[test] + fn rejects_urls_with_no_usable_name() { + for url in ["", " ", "/", "https://", "https://host/", "../"] { + assert_eq!(clone_dir_name(url), None, "url: {url}"); + } + } + + #[test] + fn rejects_option_like_and_empty_urls() { + assert!(validate_clone_url("--upload-pack=evil").is_err()); + assert!(validate_clone_url("-x").is_err()); + assert!(validate_clone_url(" ").is_err()); + assert_eq!( + validate_clone_url(" https://host/a.git ").ok(), + Some("https://host/a.git") + ); + } + + #[test] + fn reads_the_percentage_out_of_gits_progress_lines() { + let cases = [ + ("Receiving objects: 42% (52/123)", "Receiving objects", 42), + ( + "Resolving deltas: 100% (30/30), done.", + "Resolving deltas", + 100, + ), + ( + "remote: Enumerating objects: 7% (1/14)", + "Enumerating objects", + 7, + ), + ("Updating files: 0% (1/900)", "Updating files", 0), + ]; + for (line, phase, percent) in cases { + assert_eq!( + parse_clone_progress(line), + Some(CloneProgress { + phase: phase.to_string(), + percent + }), + "line: {line}" + ); + } + } + + #[test] + fn lines_without_a_percentage_are_not_progress() { + for line in [ + "Cloning into '/tmp/repo'...", + "fatal: could not read Username for 'https://github.com'", + "warning: redirecting to https://example.com/repo.git/", + "remote: Total 14 (delta 0), reused 0 (delta 0)", + "", + "https://example.com: unreachable", + ] { + assert_eq!(parse_clone_progress(line), None, "line: {line}"); + } + } + + #[test] + fn a_progress_update_renders_as_one_short_line() { + let progress = CloneProgress { + phase: "Receiving objects".to_string(), + percent: 42, + }; + assert_eq!(progress.summary(), "Receiving objects: 42%"); + } + + /// End-to-end over a real `git clone`: the parser above is only useful if + /// git actually emits these lines under the bus's piped stderr, which it + /// does only because of `--progress`. Uses a `file://` URL so the clone + /// goes through the real transport (a plain path would hardlink and skip + /// the transfer) without touching the network. + #[test] + fn a_real_clone_reports_progress() { + use crate::repository::test_support::{git_in, init_temp_repo}; + use std::sync::{Arc, Mutex}; + + let (_tmp, source) = init_temp_repo(); + for i in 0..20 { + std::fs::write(source.join(format!("file{i}.txt")), format!("{i}\n")).unwrap(); + } + git_in(&source, &["add", "."]); + git_in( + &source, + &["-c", "commit.gpgsign=false", "commit", "-m", "seed"], + ); + + let target = + std::env::temp_dir().join(format!("okena-clone-progress-{}", uuid::Uuid::new_v4())); + let seen: Arc>> = Arc::new(Mutex::new(Vec::new())); + let sink = seen.clone(); + + let url = format!("file://{}", source.display()); + let handle = start_clone_repository(&url, &target, move |progress| { + sink.lock().expect("lock").push(progress); + }) + .expect("start clone"); + finish_clone_repository(handle).expect("clone succeeds"); + + let seen = seen.lock().expect("lock"); + assert!( + !seen.is_empty(), + "git reported no progress; --progress or the parser regressed" + ); + assert!( + seen.iter().all(|p| p.percent <= 100 && !p.phase.is_empty()), + "malformed progress: {seen:?}" + ); + assert!(is_complete_checkout(&target), "clone should be complete"); + + let _ = std::fs::remove_dir_all(&target); + } + + #[test] + fn an_interrupted_clone_is_not_mistaken_for_a_finished_one() { + use crate::repository::test_support::{git_in, init_temp_repo}; + + // A finished checkout: HEAD resolves. + let (_tmp, repo) = init_temp_repo(); + std::fs::write(repo.join("file.txt"), "a\n").unwrap(); + git_in(&repo, &["add", "."]); + git_in( + &repo, + &["-c", "commit.gpgsign=false", "commit", "-m", "seed"], + ); + assert!(is_complete_checkout(&repo)); + + // What a clone killed mid-fetch leaves: a `.git` with an unborn HEAD. + let dir = std::env::temp_dir().join(format!("okena-partial-{}", uuid::Uuid::new_v4())); + std::fs::create_dir_all(&dir).unwrap(); + git_in(&dir, &["init"]); + assert!( + !is_complete_checkout(&dir), + "a repo with no commit is not a finished clone" + ); + + // Not a repo at all. + let plain = dir.join("plain"); + std::fs::create_dir_all(&plain).unwrap(); + assert!(!is_complete_checkout(&plain)); + + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn a_non_empty_target_is_rejected_before_git_runs() { + let dir = std::env::temp_dir().join(format!("okena-clone-test-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + + // Missing target is fine. + assert!(require_absent_clone_target(&dir).is_ok()); + + // Existing-but-empty is fine (git accepts it too). + std::fs::create_dir_all(&dir).unwrap(); + assert!(require_absent_clone_target(&dir).is_ok()); + + // Anything inside makes it occupied. + std::fs::write(dir.join("file"), b"x").unwrap(); + assert!(matches!( + require_absent_clone_target(&dir), + Err(GitError::CloneTargetExists { .. }) + )); + + let _ = std::fs::remove_dir_all(&dir); + } +} diff --git a/crates/okena-git/src/repository/mod.rs b/crates/okena-git/src/repository/mod.rs index 93b3ee030..775be5175 100644 --- a/crates/okena-git/src/repository/mod.rs +++ b/crates/okena-git/src/repository/mod.rs @@ -11,12 +11,14 @@ //! | [`ci`] | GitHub PR info + CI check parsing | //! | [`paths`] | repo-root resolution and worktree/project path computation | +use okena_core::process::command; use std::path::Path; use crate::error::{GitError, GitResult}; pub mod branch; pub mod ci; +pub mod clone; pub mod paths; pub mod status; pub mod worktree; @@ -31,6 +33,10 @@ pub use branch::{ pub use ci::{ CiFetch, PrFetch, fetch_ci_checks, fetch_pr_info, has_github_remote, list_pull_requests, }; +pub use clone::{ + CloneProgress, clone_dir_name, clone_repository, finish_clone_repository, is_complete_checkout, + parse_clone_progress, start_clone_repository, validate_clone_url, +}; pub use paths::{ compute_target_paths, get_repo_common_dir, get_repo_root, normalize_path, project_path_in_worktree, resolve_git_root_and_subdir, @@ -48,6 +54,32 @@ pub use worktree::{ verify_orphaned_worktree, }; +/// Build a `git` command for an operation that talks to a remote. +/// +/// Git asks for credentials on `/dev/tty`, not stdin, so redirecting stdin is +/// no defence: in a background process group that read raises SIGTTIN and the +/// child stops forever, leaving the caller blocked in `wait()` with nothing in +/// the log. Refuse every interactive prompt so a missing credential fails fast. +/// +/// The second half of the defence is in the command bus, which gives every +/// child its own session and so no controlling terminal to prompt on. +pub(crate) fn network_command() -> std::process::Command { + let mut cmd = command("git"); + // Empty `GIT_ASKPASS` is deliberate: git reads it as "set but unusable" and + // skips both `core.askpass` and `SSH_ASKPASS` instead of falling through. + cmd.env("GIT_TERMINAL_PROMPT", "0") + .env("GIT_ASKPASS", "") + .env("SSH_ASKPASS", "") + .env("GCM_INTERACTIVE", "never"); + // Only when the user has not chosen their own ssh command, so a custom + // `GIT_SSH_COMMAND` keeps working. BatchMode still authenticates via an + // agent; it only turns passphrase and host-key prompts into failures. + if std::env::var_os("GIT_SSH_COMMAND").is_none() { + cmd.env("GIT_SSH_COMMAND", "ssh -o BatchMode=yes"); + } + cmd +} + /// Run a git command and return `Ok(())` if it exits successfully, /// or `Err(GitExitError)` with the stderr message. pub(crate) fn require_success(output: std::process::Output) -> GitResult<()> { @@ -132,3 +164,47 @@ pub(crate) mod test_support { ); } } + +#[cfg(test)] +mod tests { + use super::*; + + /// A remote git command must refuse every interactive prompt. Without this + /// a clone of a private repo blocks on a credential prompt it can never + /// show, and the caller waits forever. + #[test] + fn network_commands_refuse_interactive_prompts() { + let cmd = network_command(); + let env: std::collections::HashMap<_, _> = cmd + .get_envs() + .filter_map(|(k, v)| Some((k.to_str()?.to_string(), v?.to_str()?.to_string()))) + .collect(); + + assert_eq!( + env.get("GIT_TERMINAL_PROMPT").map(String::as_str), + Some("0") + ); + // Set-but-empty, so git skips askpass instead of falling back to + // `core.askpass` / `SSH_ASKPASS`. + assert_eq!(env.get("GIT_ASKPASS").map(String::as_str), Some("")); + assert_eq!(env.get("SSH_ASKPASS").map(String::as_str), Some("")); + } + + /// A user who configured their own ssh command keeps it; we only fill in a + /// non-interactive default when the slot is free. + #[test] + fn a_user_ssh_command_is_left_alone() { + let ours = network_command() + .get_envs() + .find(|(k, _)| *k == "GIT_SSH_COMMAND") + .and_then(|(_, v)| v) + .map(|v| v.to_string_lossy().into_owned()); + + match std::env::var_os("GIT_SSH_COMMAND") { + // Nothing configured: we supply a batch-mode default. + None => assert_eq!(ours.as_deref(), Some("ssh -o BatchMode=yes")), + // Configured: we must not override it. + Some(_) => assert_eq!(ours, None), + } + } +} diff --git a/crates/okena-git/src/repository/worktree.rs b/crates/okena-git/src/repository/worktree.rs index 7d71711ea..9cba21aaa 100644 --- a/crates/okena-git/src/repository/worktree.rs +++ b/crates/okena-git/src/repository/worktree.rs @@ -5,7 +5,7 @@ use std::path::{Path, PathBuf}; use okena_core::process::{command, safe_output}; use super::branch::get_default_branch; -use super::{head_branch_short, path_str, require_success}; +use super::{head_branch_short, network_command, path_str, require_success}; use crate::error::{GitError, GitResult}; #[derive(Clone, Debug, PartialEq, Eq)] @@ -401,7 +401,7 @@ pub fn create_worktree( args.push(branch); args.push(target_str); if let Some(default_branch) = get_default_branch(repo_path) { - let _ = safe_output(command("git").args([ + let _ = safe_output(network_command().args([ "-C", repo_str, "fetch", @@ -464,7 +464,7 @@ pub fn fetch_and_fast_forward(repo_path: &Path, worktree_path: &Path, default_br let (Ok(repo_str), Ok(wt_str)) = (path_str(repo_path), path_str(worktree_path)) else { return; }; - match safe_output(command("git").args(["-C", repo_str, "fetch", "origin", default_branch])) { + match safe_output(network_command().args(["-C", repo_str, "fetch", "origin", default_branch])) { Ok(out) if out.status.success() => {} Ok(out) => { log::warn!( diff --git a/crates/okena-mobile-ffi/src/client/manager.rs b/crates/okena-mobile-ffi/src/client/manager.rs index 174328925..16ca38a2d 100644 --- a/crates/okena-mobile-ffi/src/client/manager.rs +++ b/crates/okena-mobile-ffi/src/client/manager.rs @@ -821,6 +821,7 @@ mod tests { hooks: Default::default(), is_creating: false, is_closing: false, + creating_progress: None, } } diff --git a/crates/okena-state/src/workspace_data.rs b/crates/okena-state/src/workspace_data.rs index b14b8ce77..db7727c38 100644 --- a/crates/okena-state/src/workspace_data.rs +++ b/crates/okena-state/src/workspace_data.rs @@ -290,6 +290,15 @@ pub struct ProjectData { /// project stranded "Closing…" forever. #[serde(skip)] pub is_closing: bool, + /// Latest progress line for an in-flight clone, e.g. `Receiving objects: + /// 42%`. Only ever set while `is_creating`; mirrored over the wire so thin + /// clients show the same thing as the desktop. + /// + /// Not persisted, for the same reason as `is_closing`: it describes a live + /// process, and reloading a stale percentage after a restart would claim + /// progress that nothing is making. + #[serde(skip)] + pub creating_progress: Option, } impl ProjectData { @@ -372,6 +381,7 @@ mod tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, } } diff --git a/crates/okena-transport/src/client/state.rs b/crates/okena-transport/src/client/state.rs index 3438db04e..1024bf990 100644 --- a/crates/okena-transport/src/client/state.rs +++ b/crates/okena-transport/src/client/state.rs @@ -214,6 +214,7 @@ mod tests { hooks: Default::default(), is_creating: false, is_closing: false, + creating_progress: None, } } @@ -380,6 +381,7 @@ mod tests { hooks: Default::default(), is_creating: false, is_closing: false, + creating_progress: None, }]); let sizes = collect_terminal_sizes(&state); assert_eq!(sizes.get("t1"), Some(&(120, 40))); diff --git a/crates/okena-views-sidebar/src/project_list.rs b/crates/okena-views-sidebar/src/project_list.rs index 36c9ecda2..913c7d13f 100644 --- a/crates/okena-views-sidebar/src/project_list.rs +++ b/crates/okena-views-sidebar/src/project_list.rs @@ -33,7 +33,7 @@ pub enum ProjectRowStyle { Worktree { is_orphan: bool, is_busy: bool, - busy_label: &'static str, + busy_label: String, }, /// Child under a group header: plain solid dot, no rename. GroupChild, @@ -230,8 +230,8 @@ impl Sidebar { // 6. Busy label (Worktree busy only) .when(is_busy, |d| { let label = match style { - ProjectRowStyle::Worktree { busy_label, .. } => *busy_label, - _ => "", + ProjectRowStyle::Worktree { busy_label, .. } => busy_label.clone(), + _ => String::new(), }; d.child( div() @@ -491,9 +491,14 @@ impl Sidebar { })); let busy_label = if is_creating { - "Creating\u{2026}" + // Prefer git's own percentage: a clone can run for minutes, and a + // bare "Creating…" gives no way to tell it apart from a hang. + project + .creating_progress + .clone() + .unwrap_or_else(|| "Creating\u{2026}".to_string()) } else { - "Closing\u{2026}" + "Closing\u{2026}".to_string() }; self.append_project_row_content( row, diff --git a/crates/okena-views-sidebar/src/sidebar/from_project_test.rs b/crates/okena-views-sidebar/src/sidebar/from_project_test.rs index 216802242..e5859401a 100644 --- a/crates/okena-views-sidebar/src/sidebar/from_project_test.rs +++ b/crates/okena-views-sidebar/src/sidebar/from_project_test.rs @@ -33,6 +33,7 @@ fn make_project(id: &str) -> ProjectData { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, } } diff --git a/crates/okena-views-sidebar/src/sidebar/mod.rs b/crates/okena-views-sidebar/src/sidebar/mod.rs index 65b1805f5..1e7ab2702 100644 --- a/crates/okena-views-sidebar/src/sidebar/mod.rs +++ b/crates/okena-views-sidebar/src/sidebar/mod.rs @@ -743,6 +743,10 @@ pub struct SidebarProjectInfo { pub is_closing: bool, /// True if this worktree is being created (git fetch + worktree add in progress) pub is_creating: bool, + /// How far the in-flight create has got, when it reports progress at all + /// (clones do, worktree checkouts do not). Replaces the generic + /// "Creating…" label so a long clone does not look stuck. + pub creating_progress: Option, /// Whether this project is itself a worktree pub is_worktree: bool, /// Whether this project is pinned (shows a pin marker; drives the pinned @@ -823,6 +827,7 @@ impl SidebarProjectInfo { // the user closed is a legitimate bookmark with layout None, and must // not render the "Setting up worktree…" placeholder. is_creating: project.is_creating, + creating_progress: project.creating_progress.clone(), is_worktree: project.worktree_info.is_some(), pinned: project.pinned, } diff --git a/crates/okena-workspace/src/actions/focus.rs b/crates/okena-workspace/src/actions/focus.rs index 92bfb298f..f334a4867 100644 --- a/crates/okena-workspace/src/actions/focus.rs +++ b/crates/okena-workspace/src/actions/focus.rs @@ -305,6 +305,7 @@ mod gpui_tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, } } diff --git a/crates/okena-workspace/src/actions/folder.rs b/crates/okena-workspace/src/actions/folder.rs index bcec0c369..27251aefa 100644 --- a/crates/okena-workspace/src/actions/folder.rs +++ b/crates/okena-workspace/src/actions/folder.rs @@ -236,6 +236,7 @@ mod tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, } } @@ -359,6 +360,7 @@ mod gpui_tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, } } diff --git a/crates/okena-workspace/src/actions/layout/tests_gpui.rs b/crates/okena-workspace/src/actions/layout/tests_gpui.rs index 03dff4e20..dc6ec03b9 100644 --- a/crates/okena-workspace/src/actions/layout/tests_gpui.rs +++ b/crates/okena-workspace/src/actions/layout/tests_gpui.rs @@ -36,6 +36,7 @@ fn make_project(id: &str) -> ProjectData { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, } } @@ -505,6 +506,7 @@ fn make_project_with_layout(id: &str, layout: LayoutNode) -> ProjectData { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, } } diff --git a/crates/okena-workspace/src/actions/project.rs b/crates/okena-workspace/src/actions/project.rs index 66f822a96..13da4d0b9 100644 --- a/crates/okena-workspace/src/actions/project.rs +++ b/crates/okena-workspace/src/actions/project.rs @@ -10,6 +10,39 @@ use crate::state::{LayoutNode, ProjectData, WindowId, Workspace}; use okena_core::theme::FolderColor; use std::collections::{HashMap, HashSet}; +/// A fresh, unparented project row — the one place the full `ProjectData` +/// shape is spelled out for newly created projects. +fn new_project_row( + id: String, + name: String, + path: String, + layout: Option, + default_shell: Option, +) -> ProjectData { + ProjectData { + id, + name, + path, + layout, + terminal_names: HashMap::new(), + hidden_terminals: HashMap::new(), + worktree_info: None, + worktree_ids: Vec::new(), + folder_color: FolderColor::default(), + hooks: HooksConfig::default(), + is_remote: false, + connection_id: None, + service_terminals: HashMap::new(), + default_shell, + hook_terminals: HashMap::new(), + pinned: false, + last_activity_at: None, + is_creating: false, + is_closing: false, + creating_progress: None, + } +} + #[derive(Clone)] pub struct ProjectDirectoryRenamePlan { project_id: String, @@ -142,6 +175,46 @@ fn expand_tilde(path: &str) -> String { path.to_string() } +/// Resolve a clone request's `parent_dir` + `directory` into one absolute path. +/// +/// `directory` is a NAME, not a path: separators and `..` are rejected so the +/// checkout cannot land outside the parent directory the user actually picked. +/// Runs on the host that will do the cloning, so `~` and the path separator are +/// resolved with that host's conventions — not the calling client's. +pub fn resolve_clone_target( + parent_dir: &str, + directory: &str, +) -> Result { + let parent = expand_tilde(parent_dir.trim()); + if parent.is_empty() { + return Err("Parent directory is required".to_string()); + } + let directory = directory.trim(); + if directory.is_empty() { + return Err("Directory name is required".to_string()); + } + let mut components = std::path::Path::new(directory).components(); + let is_plain_name = matches!(components.next(), Some(std::path::Component::Normal(_))) + && components.next().is_none(); + if directory.contains(['/', '\\']) || !is_plain_name { + return Err(format!( + "'{directory}' is not a valid directory name — it must be a single folder name" + )); + } + Ok(std::path::PathBuf::from(parent).join(directory)) +} + +/// Display name for a cloned project: the caller's, or the directory name when +/// the caller left it blank (`okena project clone` without `--name`). +pub fn clone_project_name(name: &str, directory: &str) -> String { + let name = name.trim(); + if name.is_empty() { + directory.trim().to_string() + } else { + name.to_string() + } +} + impl Workspace { /// Returns whether a project is hidden in the given window. /// @@ -285,57 +358,115 @@ impl Workspace { let default_shell: Option = None; let id = uuid::Uuid::new_v4().to_string(); - let project = ProjectData { - id: id.clone(), - name: name.clone(), - path: path.clone(), - layout: if with_terminal { - Some(LayoutNode::new_terminal()) - } else { - None - }, - terminal_names: HashMap::new(), - hidden_terminals: HashMap::new(), - worktree_info: None, - worktree_ids: Vec::new(), - folder_color: FolderColor::default(), - hooks: HooksConfig::default(), - is_remote: false, - connection_id: None, - service_terminals: HashMap::new(), + let layout = with_terminal.then(LayoutNode::new_terminal); + self.data.projects.push(new_project_row( + id.clone(), + name, + path, + layout, default_shell, - hook_terminals: HashMap::new(), - pinned: false, - last_activity_at: None, - is_creating: false, - is_closing: false, - }; - let project_hooks = project.hooks.clone(); - self.data.projects.push(project); + )); self.data.project_order.push(id.clone()); self.data.add_project_hide_in_other_windows(&id, window_id); self.notify_data(cx); - let folder = self.folder_for_project_or_parent(&id); - let folder_id = folder.map(|f| f.id.as_str()); - let folder_name = folder.map(|f| f.name.as_str()); - let runner = cx.hook_runner(); - let monitor = cx.hook_monitor(); - let hook_results = hooks::fire_on_project_open( - &project_hooks, - &id, - &name, - &path, - folder_id, - folder_name, - global_hooks, - runner.as_ref(), - monitor.as_ref(), - ); - self.register_hook_results(hook_results, cx); + self.fire_project_open_hooks(&id, global_hooks, cx); + Ok(id) + } + + /// Register a project row whose directory does NOT exist on disk yet. + /// + /// The clone counterpart of `register_worktree_project_deferred_hooks`: + /// the row appears immediately (so the user sees the project while the + /// clone runs) but gets no layout and fires no hooks, because both would + /// cd into a directory that is not there. The caller marks it creating, + /// runs the checkout, then calls `finish_pending_project` — or + /// `remove_pending_project` when the checkout fails. + pub fn register_pending_project( + &mut self, + name: String, + path: String, + window_id: WindowId, + cx: &mut impl WorkspaceCx, + ) -> Result { + let path = expand_tilde(&path); + self.ensure_project_path_claim_allowed(std::path::Path::new(&path))?; + + let id = uuid::Uuid::new_v4().to_string(); + // No layout: `column_content` renders the creating placeholder, and + // `spawn_uninitialized_terminals` has nothing to spawn into yet. The + // shell is detected in `finish_pending_project`, once the path is real. + self.data + .projects + .push(new_project_row(id.clone(), name, path, None, None)); + self.data.project_order.push(id.clone()); + self.data.add_project_hide_in_other_windows(&id, window_id); + self.notify_data(cx); Ok(id) } + /// Materialize a pending project once its directory exists: seed the + /// terminal layout and fire the deferred `on_project_open` hooks. + /// + /// Leaves an existing layout alone — a restored session may already carry + /// one, and re-seeding would drop its terminals. + pub fn finish_pending_project( + &mut self, + project_id: &str, + global_hooks: &HooksConfig, + cx: &mut impl WorkspaceCx, + ) { + let Some(project) = self.data.projects.iter_mut().find(|p| p.id == project_id) else { + return; + }; + if project.layout.is_none() { + project.layout = Some(LayoutNode::new_terminal()); + } + // Detect the WSL shell now that the path is real (`add_project` does + // this up-front from the path string; the check is the same either way). + #[cfg(windows)] + { + if project.default_shell.is_none() { + project.default_shell = okena_terminal::shell_config::parse_wsl_unc_path( + &project.path, + ) + .map(|(distro, _)| okena_terminal::shell_config::ShellType::Wsl { + distro: Some(distro), + }); + } + } + self.notify_data(cx); + self.fire_project_open_hooks(project_id, global_hooks, cx); + } + + /// Roll back a pending project row whose creation never completed. + /// + /// Mirrors `remove_stale_worktree` for plain (non-worktree) projects and + /// carries the same guard: a row still marked creating or closing belongs + /// to an in-flight operation and is left alone. Caller calls `notify_data`. + pub fn remove_pending_project(&mut self, project_id: &str) { + if self.lifecycle.is_closing(project_id) || self.lifecycle.is_creating(project_id) { + return; + } + // Worktree rows roll back through `remove_stale_worktree`, which also + // scrubs the parent's `worktree_ids`. + let is_plain_project = self + .data + .projects + .iter() + .any(|p| p.id == project_id && p.worktree_info.is_none()); + if !is_plain_project { + return; + } + + self.data.projects.retain(|p| p.id != project_id); + self.data.project_order.retain(|id| id != project_id); + for folder in &mut self.data.folders { + folder.project_ids.retain(|id| id != project_id); + } + self.data.delete_project_scrub_all_windows(project_id); + } + /// Remove hook terminal state restored without a matching live PTY. /// /// Returns the stale terminal ids so the caller can also tear down a @@ -1131,6 +1262,7 @@ mod tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, } } @@ -1460,6 +1592,7 @@ mod gpui_tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, } } @@ -3641,4 +3774,145 @@ mod gpui_tests { assert!(!ws.is_creating_project("wt1"), "creating flag cleared"); }); } + + #[test] + fn clone_target_joins_the_parent_and_the_directory() { + let target = super::resolve_clone_target("/home/user/projects", "okena").unwrap(); + assert_eq!(target, Path::new("/home/user/projects/okena")); + // Surrounding whitespace is the user's, not part of the path. + let target = super::resolve_clone_target(" /home/user/projects ", " okena ").unwrap(); + assert_eq!(target, Path::new("/home/user/projects/okena")); + } + + #[test] + fn clone_target_rejects_a_directory_that_is_not_a_plain_name() { + // A separator or `..` would put the checkout outside the parent the + // user picked. + for directory in ["../escape", "nested/dir", "a\\b", ".", "..", "", " "] { + assert!( + super::resolve_clone_target("/home/user", directory).is_err(), + "expected rejection for {directory:?}" + ); + } + assert!(super::resolve_clone_target("", "okena").is_err()); + #[cfg(windows)] + assert!(super::resolve_clone_target(r"C:\parent", "D:").is_err()); + } + + #[test] + fn clone_name_falls_back_to_the_directory() { + assert_eq!(super::clone_project_name("My Repo", "okena"), "My Repo"); + assert_eq!(super::clone_project_name(" ", "okena"), "okena"); + assert_eq!(super::clone_project_name("", " okena "), "okena"); + } + + #[gpui::test] + fn pending_project_gets_no_layout_until_it_is_finished(cx: &mut gpui::TestAppContext) { + let workspace = cx.new(|_cx| Workspace::new(make_workspace_data())); + + let id = workspace.update(cx, |ws: &mut Workspace, cx| { + let id = ws + .register_pending_project( + "Okena".to_string(), + "/tmp/okena-clone-target".to_string(), + WindowId::Main, + cx, + ) + .expect("registers"); + ws.mark_creating_project(&id); + id + }); + + workspace.read_with(cx, |ws: &Workspace, _cx| { + let project = ws.project(&id).expect("project exists"); + assert!(project.layout.is_none(), "no layout while the clone runs"); + assert!(project.is_creating, "creating flag mirrored onto the row"); + }); + + workspace.update(cx, |ws: &mut Workspace, cx| { + ws.finish_pending_project(&id, &HooksConfig::default(), cx); + ws.finish_creating_project(&id); + }); + + workspace.read_with(cx, |ws: &Workspace, _cx| { + let project = ws.project(&id).expect("project exists"); + assert!( + project.layout.is_some(), + "layout seeded once the dir exists" + ); + assert!(!project.is_creating); + }); + } + + #[gpui::test] + fn rolling_back_a_pending_project_drops_it_everywhere(cx: &mut gpui::TestAppContext) { + let mut data = make_workspace_data(); + data.folders = vec![crate::state::FolderData { + id: "f1".to_string(), + name: "Folder".to_string(), + project_ids: vec![], + folder_color: FolderColor::default(), + }]; + let workspace = cx.new(|_cx| Workspace::new(data)); + + let id = workspace.update(cx, |ws: &mut Workspace, cx| { + let id = ws + .register_pending_project( + "Okena".to_string(), + "/tmp/okena-clone-rollback".to_string(), + WindowId::Main, + cx, + ) + .expect("registers"); + ws.mark_creating_project(&id); + ws.move_project_to_folder(&id, "f1", None, cx); + id + }); + + // Still creating: the row belongs to an in-flight clone and stays put. + workspace.update(cx, |ws: &mut Workspace, _cx| ws.remove_pending_project(&id)); + workspace.read_with(cx, |ws: &Workspace, _cx| { + assert!(ws.project(&id).is_some(), "creating rows are not removed"); + }); + + workspace.update(cx, |ws: &mut Workspace, _cx| { + ws.finish_creating_project(&id); + ws.remove_pending_project(&id); + }); + workspace.read_with(cx, |ws: &Workspace, _cx| { + assert!(ws.project(&id).is_none()); + assert!(!ws.data().project_order.contains(&id)); + assert!(ws.data().folders[0].project_ids.is_empty()); + }); + } + + #[gpui::test] + fn a_pending_project_cannot_claim_a_path_reserved_by_a_worktree_create( + cx: &mut gpui::TestAppContext, + ) { + // A clone must inherit the same path-claim guard as any other project: + // a checkout that is still being created owns its root, so a clone + // cannot land inside it and race the checkout on the same directory. + let mut parent = make_project("parent"); + parent.worktree_ids = vec!["wt1".to_string()]; + let mut data = make_workspace_data(); + data.projects = vec![parent, make_worktree_project("wt1", "parent")]; + data.project_order = vec!["parent".to_string()]; + let workspace = cx.new(|_cx| Workspace::new(data)); + + let result = workspace.update(cx, |ws: &mut Workspace, cx| { + ws.mark_creating_project("wt1"); + ws.register_pending_project( + "Clash".to_string(), + "/tmp/worktrees/wt1/nested".to_string(), + WindowId::Main, + cx, + ) + }); + + assert!( + result.is_err(), + "clone target inside an in-flight worktree must be rejected" + ); + } } diff --git a/crates/okena-workspace/src/actions/soft_close.rs b/crates/okena-workspace/src/actions/soft_close.rs index 18f34630e..25974f0b2 100644 --- a/crates/okena-workspace/src/actions/soft_close.rs +++ b/crates/okena-workspace/src/actions/soft_close.rs @@ -616,6 +616,7 @@ mod tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, } } diff --git a/crates/okena-workspace/src/actions/worktree.rs b/crates/okena-workspace/src/actions/worktree.rs index cab4aa616..5becac184 100644 --- a/crates/okena-workspace/src/actions/worktree.rs +++ b/crates/okena-workspace/src/actions/worktree.rs @@ -568,6 +568,7 @@ impl Workspace { // optimistic (deferred-hooks) create still awaiting its checkout. is_creating: false, is_closing: false, + creating_progress: None, }; let new_project_hooks = project.hooks.clone(); @@ -749,6 +750,7 @@ impl Workspace { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, }; // Multi-window new-project visibility rule (PRD user story 14): diff --git a/crates/okena-workspace/src/persistence.rs b/crates/okena-workspace/src/persistence.rs index c9f3bee11..b69d4f393 100644 --- a/crates/okena-workspace/src/persistence.rs +++ b/crates/okena-workspace/src/persistence.rs @@ -956,11 +956,11 @@ pub(crate) fn migrate_workspace(mut data: WorkspaceData) -> WorkspaceData { data } -/// Remove stale worktree projects whose directories no longer exist on disk. +/// Remove stale worktrees and interrupted pending projects whose directories +/// do not exist on disk. /// -/// Worktrees are only added as projects explicitly by the user (via the worktree -/// list popover or the create worktree dialog). This function only cleans up -/// worktree projects that have become stale. +/// Ordinary projects remain untouched unless their persisted `is_creating` +/// marker proves that their creation was interrupted. #[cfg(test)] pub(crate) fn sync_worktrees(data: &mut WorkspaceData) -> Vec { sync_worktrees_with_backend_and_shell(data, SessionBackend::None, &ShellType::Default) @@ -974,8 +974,13 @@ pub(crate) fn sync_worktrees_with_backend_and_shell( let stale_ids: Vec = data .projects .iter() - .filter(|p| p.worktree_info.is_some()) - .filter(|p| !worktree_checkout_path(p).exists()) + .filter(|project| { + if project.worktree_info.is_some() { + !worktree_checkout_path(project).exists() + } else { + project.is_creating && !interrupted_create_is_usable(project) + } + }) .map(|p| p.id.clone()) .collect(); @@ -1019,6 +1024,9 @@ pub(crate) fn sync_worktrees_with_backend_and_shell( parent.worktree_ids.retain(|pid| pid != id); } } + if !stale_ids.is_empty() { + data.scrub_orphan_window_state(); + } let retained_terminal_ids: std::collections::HashSet = data .projects @@ -1035,35 +1043,50 @@ pub(crate) fn sync_worktrees_with_backend_and_shell( .collect(); stale_terminal_ids.retain(|session| !retained_terminal_ids.contains(&session.terminal_id)); - // Self-heal a worktree left mid-create by a daemon kill: optimistic create - // registers the row with layout:None before the git checkout, and the - // finalize (which seeds the layout + spawns the PTY) may not have persisted. - // If the checkout dir now EXISTS (so it isn't stale above), seed a terminal - // so it opens a shell instead of hanging on the "Setting up worktree…" - // placeholder forever. + // Self-heal a project left mid-create by a daemon kill: optimistic create + // registers the row with layout:None before the checkout, and the finalize + // (which seeds the layout + spawns the PTY) may not have persisted. If the + // checkout now exists, seed a terminal and clear the stale marker so startup + // materialization can finish it. // - // Gated on the persisted `is_creating` marker so we only touch worktrees - // genuinely interrupted mid-create. A worktree the user deliberately emptied - // (closed its last terminal -> layout:None bookmark) has is_creating == false - // and is left untouched — seeding a shell there would silently un-bookmark it - // and resurrect a shell on every restart. + // Gated on the persisted `is_creating` marker so a deliberate layout:None + // bookmark remains untouched. for p in data.projects.iter_mut() { - if p.is_creating - && p.worktree_info.is_some() - && p.layout.is_none() - && Path::new(&p.path).exists() - { + if !p.is_creating { + continue; + } + if !interrupted_create_is_usable(p) { + continue; + } + if p.layout.is_none() { p.layout = Some(LayoutNode::new_terminal()); - // Checkout exists and the layout is now seeded — the create is - // effectively finalized, so clear the marker (materialize spawns the - // PTY and the row renders as a normal worktree, not "creating"). - p.is_creating = false; } + p.is_creating = false; } stale_terminal_ids } +/// Whether what an interrupted create left on disk is worth keeping. +/// +/// A worktree checkout only has to exist — `git worktree add` either produces +/// one or does not. A clone is weaker: killed mid-fetch it leaves the target +/// directory behind with an unborn HEAD, so existence alone would promote a +/// repo containing no files to a normal-looking project. Demand a finished +/// checkout there. +/// +/// Both callers below must agree: the stale sweep removes the row when this is +/// false, and the self-heal finishes it when true. Split them and a half-clone +/// is neither removed nor finished — it stays marked creating forever, which is +/// the "stuck on cloning" state this recovery exists to prevent. +fn interrupted_create_is_usable(project: &ProjectData) -> bool { + if project.worktree_info.is_some() { + return worktree_checkout_path(project).exists(); + } + let path = Path::new(&project.path); + path.exists() && okena_git::is_complete_checkout(path) +} + pub(crate) fn worktree_checkout_path(project: &ProjectData) -> &Path { let path = project .worktree_info @@ -1168,6 +1191,7 @@ pub fn default_workspace() -> WorkspaceData { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, }], project_order: vec![project_id], service_panel_heights: HashMap::new(), @@ -1592,6 +1616,7 @@ mod tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, } } @@ -2482,6 +2507,127 @@ mod tests { ); } + /// Lay down what a finished `git clone` leaves: a repo whose HEAD resolves. + fn init_checked_out_repo(path: &std::path::Path) { + std::fs::create_dir_all(path).expect("create repo dir"); + let git = |args: &[&str]| { + let status = std::process::Command::new("git") + .arg("-C") + .arg(path) + .args(args) + .output() + .expect("run git"); + assert!( + status.status.success(), + "git {args:?}: {}", + String::from_utf8_lossy(&status.stderr) + ); + }; + git(&["init", "-q"]); + git(&["config", "user.email", "t@example.com"]); + git(&["config", "user.name", "t"]); + std::fs::write(path.join("file.txt"), "a\n").expect("write file"); + git(&["add", "."]); + git(&["-c", "commit.gpgsign=false", "commit", "-q", "-m", "seed"]); + } + + #[test] + fn sync_worktrees_finishes_mid_create_clone_when_target_exists() { + let checkout = + std::env::temp_dir().join(format!("okena-clone-recovery-{}", uuid::Uuid::new_v4())); + init_checked_out_repo(&checkout); + let mut clone = make_project("clone1"); + clone.path = checkout.to_string_lossy().into_owned(); + clone.layout = None; + clone.is_creating = true; + + let mut data = make_workspace(vec![clone], vec!["clone1"], vec![]); + sync_worktrees(&mut data); + + let clone = data + .projects + .iter() + .find(|project| project.id == "clone1") + .expect("completed clone kept"); + assert!( + clone.layout.is_some(), + "completed clone gets a terminal slot" + ); + assert!( + !clone.is_creating, + "completed clone clears its stale marker" + ); + std::fs::remove_dir_all(checkout).expect("remove clone target"); + } + + /// A clone killed mid-fetch leaves the directory behind with an unborn + /// HEAD. Existence alone would promote that empty repo to a normal + /// project, hiding a broken checkout behind a working-looking row. + #[test] + fn sync_worktrees_discards_a_clone_interrupted_mid_fetch() { + let wreckage = + std::env::temp_dir().join(format!("okena-partial-clone-{}", uuid::Uuid::new_v4())); + std::fs::create_dir_all(&wreckage).expect("create clone target"); + let status = std::process::Command::new("git") + .arg("-C") + .arg(&wreckage) + .args(["init", "-q"]) + .output() + .expect("run git init"); + assert!(status.status.success()); + + let mut clone = make_project("clone1"); + clone.path = wreckage.to_string_lossy().into_owned(); + clone.layout = None; + clone.is_creating = true; + + let mut data = make_workspace(vec![clone], vec!["clone1"], vec![]); + sync_worktrees(&mut data); + + assert!( + data.projects.is_empty(), + "a half-cloned repo is discarded, not finished" + ); + assert!(data.project_order.is_empty()); + + std::fs::remove_dir_all(wreckage).expect("remove clone target"); + } + + #[test] + fn sync_worktrees_removes_mid_create_clone_when_target_is_missing() { + let missing = + std::env::temp_dir().join(format!("okena-missing-clone-{}", uuid::Uuid::new_v4())); + let mut clone = make_project("clone1"); + clone.path = missing.to_string_lossy().into_owned(); + clone.layout = None; + clone.is_creating = true; + + let mut data = make_workspace(vec![clone], vec!["clone1"], vec![]); + data.main_window + .hidden_project_ids + .insert("clone1".to_string()); + sync_worktrees(&mut data); + + assert!(data.projects.is_empty()); + assert!(data.project_order.is_empty()); + assert!(!data.main_window.hidden_project_ids.contains("clone1")); + } + + #[test] + fn sync_worktrees_preserves_missing_plain_project_not_being_created() { + let missing = + std::env::temp_dir().join(format!("okena-missing-bookmark-{}", uuid::Uuid::new_v4())); + let mut project = make_project("bookmark"); + project.path = missing.to_string_lossy().into_owned(); + project.layout = None; + project.is_creating = false; + + let mut data = make_workspace(vec![project], vec!["bookmark"], vec![]); + sync_worktrees(&mut data); + + assert!(data.projects.iter().any(|project| project.id == "bookmark")); + } + #[test] fn sync_worktrees_leaves_deliberate_bookmark_untouched() { // A worktree the user deliberately emptied (closed its last terminal -> diff --git a/crates/okena-workspace/src/remote_apply.rs b/crates/okena-workspace/src/remote_apply.rs index 5386c02eb..ed1a2f8d9 100644 --- a/crates/okena-workspace/src/remote_apply.rs +++ b/crates/okena-workspace/src/remote_apply.rs @@ -260,6 +260,7 @@ pub fn apply_remote_snapshot( last_activity_at: api_project.last_activity_at, is_creating: api_project.is_creating, is_closing: api_project.is_closing, + creating_progress: api_project.creating_progress.clone(), }); } // Update the transient remote snapshot regardless of create/update path. @@ -555,6 +556,7 @@ mod tests { hooks: Default::default(), is_creating: false, is_closing: false, + creating_progress: None, } } diff --git a/crates/okena-workspace/src/state.rs b/crates/okena-workspace/src/state.rs index 779566107..ee5b40919 100644 --- a/crates/okena-workspace/src/state.rs +++ b/crates/okena-workspace/src/state.rs @@ -1548,9 +1548,31 @@ impl Workspace { self.lifecycle.finish_creating(project_id); if let Some(p) = self.data.projects.iter_mut().find(|p| p.id == project_id) { p.is_creating = false; + // Progress describes the operation, not the project: leaving the + // last percentage behind would keep claiming a clone is running. + p.creating_progress = None; } } + /// Record how far the in-flight create for `project_id` has got, e.g. + /// `Receiving objects: 42%`. + /// + /// Returns whether anything actually changed, so a caller driven by a + /// chatty progress stream can skip broadcasting an identical snapshot. + /// Ignores a project that is no longer being created: progress lines are + /// delivered from a reader thread and can land after the operation ended, + /// and one arriving late must not resurrect the placeholder. + pub fn set_creating_progress(&mut self, project_id: &str, summary: String) -> bool { + let Some(p) = self.data.projects.iter_mut().find(|p| p.id == project_id) else { + return false; + }; + if !p.is_creating || p.creating_progress.as_deref() == Some(summary.as_str()) { + return false; + } + p.creating_progress = Some(summary); + true + } + pub fn mark_worktree_removing(&mut self, path: &str) { self.lifecycle.mark_worktree_removing(path); } @@ -2610,6 +2632,7 @@ mod workspace_tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, } } @@ -3652,6 +3675,7 @@ mod gpui_tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, } } diff --git a/crates/okena-workspace/src/visibility.rs b/crates/okena-workspace/src/visibility.rs index 5d89392ca..603b3f1f7 100644 --- a/crates/okena-workspace/src/visibility.rs +++ b/crates/okena-workspace/src/visibility.rs @@ -241,6 +241,7 @@ mod tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, } } diff --git a/web/src/api/types.ts b/web/src/api/types.ts index b8028ae3d..7f30a6b31 100644 --- a/web/src/api/types.ts +++ b/web/src/api/types.ts @@ -307,6 +307,7 @@ export type ActionRequest = | { action: "git_discard_file"; project_id: string; file_path: string } | { action: "git_blame"; project_id: string; relative_path: string } | { action: "add_project"; name: string; path: string } + | { action: "clone_project"; url: string; parent_dir: string; directory: string; name: string } | { action: "reorder_project_in_folder"; folder_id: string; project_id: string; new_index: number } | { action: "set_project_color"; project_id: string; color: FolderColor } | { action: "set_folder_color"; folder_id: string; color: FolderColor }