From 508b5f53e92a72bb3c1d4d9a735924f746844945 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Fri, 14 Aug 2026 15:14:06 +0200 Subject: [PATCH 01/14] feat(git): clone a repository into a fresh directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `clone_repository` runs `git clone -- ` on `Lane::Long`. The lane matters: a clone is network-bound and unbounded in duration, so on the interactive lane it would sit on one of four slots for however long the repo takes to fetch, and on the poll lane it would starve git status. `clone_dir_name` derives the directory git itself would create, so callers can prefill it. It drops the scheme and host before taking the last path segment — otherwise a hostless `https://` yields "https" as the name. Two guards up front, so bad input fails fast with our message instead of a confusing git one: `validate_clone_url` rejects empty and option-like URLs (the `--` separator already makes them harmless, but a clear error beats a silent oddity), and a non-empty target is refused before git runs. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01S63Q5kXgrw85F85QS1cntR --- crates/okena-git/CLAUDE.md | 1 + crates/okena-git/src/error.rs | 8 ++ crates/okena-git/src/lib.rs | 12 +- crates/okena-git/src/repository/clone.rs | 154 +++++++++++++++++++++++ crates/okena-git/src/repository/mod.rs | 2 + 5 files changed, 171 insertions(+), 6 deletions(-) create mode 100644 crates/okena-git/src/repository/clone.rs diff --git a/crates/okena-git/CLAUDE.md b/crates/okena-git/CLAUDE.md index e0bd6c1d8..17939076c 100644 --- a/crates/okena-git/CLAUDE.md +++ b/crates/okena-git/CLAUDE.md @@ -10,6 +10,7 @@ Git status, diff parsing, and worktree operations for project directories. | `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/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. | diff --git a/crates/okena-git/src/error.rs b/crates/okena-git/src/error.rs index 377cf72bf..e9182432c 100644 --- a/crates/okena-git/src/error.rs +++ b/crates/okena-git/src/error.rs @@ -35,6 +35,14 @@ 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), diff --git a/crates/okena-git/src/lib.rs b/crates/okena-git/src/lib.rs index 152060897..e702b020e 100644 --- a/crates/okena-git/src/lib.rs +++ b/crates/okena-git/src/lib.rs @@ -17,16 +17,16 @@ 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, + 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, 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, 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, + 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/clone.rs b/crates/okena-git/src/repository/clone.rs new file mode 100644 index 000000000..821dbd71f --- /dev/null +++ b/crates/okena-git/src/repository/clone.rs @@ -0,0 +1,154 @@ +//! Clone a remote repository into a fresh directory. + +use std::path::Path; + +use okena_core::process::{CommandSpec, Lane, command, run}; + +use super::{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(()) +} + +/// `git clone `. +/// +/// Runs on [`Lane::Long`]: a clone is network-bound and unbounded in duration, +/// so it must never occupy an interactive or poller slot. +pub fn clone_repository(url: &str, target_path: &Path) -> 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 = command("git"); + cmd.args(["clone", "--", url, target_str]); + let output = run(CommandSpec::from_command(&cmd) + .lane(Lane::Long) + .label("git clone"))?; + require_success(output) +} + +#[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 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..4177d1df3 100644 --- a/crates/okena-git/src/repository/mod.rs +++ b/crates/okena-git/src/repository/mod.rs @@ -17,6 +17,7 @@ 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 +32,7 @@ pub use branch::{ pub use ci::{ CiFetch, PrFetch, fetch_ci_checks, fetch_pr_info, has_github_remote, list_pull_requests, }; +pub use clone::{clone_dir_name, 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, From 4fe11bc897afe60ece35348a4ea9a1984725eb07 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Fri, 14 Aug 2026 15:14:14 +0200 Subject: [PATCH 02/14] feat(workspace): register a project whose directory is not there yet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A clone cannot follow `add_project`: the row has to exist while the checkout is still running, but seeding a layout or firing `on_project_open` would cd into a directory that is not there. `register_pending_project` is the clone counterpart of `register_worktree_project_deferred_hooks` — the row lands with no layout and no hooks. `finish_pending_project` seeds the layout and fires the hooks once the directory is real; `remove_pending_project` rolls the row back when the checkout fails, guarded like `remove_stale_worktree` so it never touches a row that belongs to an operation still in flight. `resolve_clone_target` joins the parent and the directory on the host that will do the cloning, not on the caller — a remote daemon need not share the client's path conventions. It also keeps the directory a NAME: separators and `..` are rejected, so the checkout cannot land outside the parent the user picked. The full `ProjectData` shape for new projects now lives in one `new_project_row` helper, and `add_project` reuses the existing `fire_project_open_hooks` instead of its own copy of that tail. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01S63Q5kXgrw85F85QS1cntR --- crates/okena-workspace/src/actions/project.rs | 352 +++++++++++++++--- 1 file changed, 308 insertions(+), 44 deletions(-) diff --git a/crates/okena-workspace/src/actions/project.rs b/crates/okena-workspace/src/actions/project.rs index 66f822a96..82e3dd328 100644 --- a/crates/okena-workspace/src/actions/project.rs +++ b/crates/okena-workspace/src/actions/project.rs @@ -10,6 +10,38 @@ 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, + } +} + #[derive(Clone)] pub struct ProjectDirectoryRenamePlan { project_id: String, @@ -142,6 +174,47 @@ 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()); + } + if directory == "." + || directory == ".." + || directory.contains(['/', '\\']) + || std::path::Path::new(directory).components().count() != 1 + { + 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,112 @@ 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(), - 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); + let layout = with_terminal.then(LayoutNode::new_terminal); + self.data + .projects + .push(new_project_row(id.clone(), name, path, layout, default_shell)); 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 @@ -3641,4 +3769,140 @@ 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()); + } + + #[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" + ); + } } From 6059e29a2f9603c567f61339f9fa637f492d62b5 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Fri, 14 Aug 2026 15:14:20 +0200 Subject: [PATCH 03/14] feat(core): add a clone-project action `CloneProject { url, parent_dir, directory, name }` carries the parent and the directory name separately so the receiving host joins them with its own separator; a client cannot predict a remote daemon's path shape. The `execute_action` arm clones and then adds the project, blocking end to end. The daemon intercepts this action ahead of `execute_action` and does the same work off the reactor (next commit); this path serves callers that drive `execute_action` directly, and it is what the new tests exercise against a real repository. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01S63Q5kXgrw85F85QS1cntR --- .../src/workspace/actions/execute/mod.rs | 8 + .../src/workspace/actions/execute/project.rs | 206 +++++++++++++++++- crates/okena-app/src/action_dispatch.rs | 11 + crates/okena-core/src/api.rs | 16 ++ web/src/api/types.ts | 1 + 5 files changed, 241 insertions(+), 1 deletion(-) 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..277821129 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, 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..6e7b1f4c6 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,47 @@ 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 +705,7 @@ mod hook_action_tests { } #[derive(Default)] - struct RecordingBackend { + pub(super) struct RecordingBackend { transport: Arc, next_id: AtomicUsize, shells: Mutex>>, @@ -1031,6 +1072,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}; 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-core/src/api.rs b/crates/okena-core/src/api.rs index 4c76e6f51..77cfedb6a 100644 --- a/crates/okena-core/src/api.rs +++ b/crates/okena-core/src/api.rs @@ -829,6 +829,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, @@ -1675,6 +1685,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/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 } From a82cc88519dcb98fa1fc2ce6af4ef54c38bd64d7 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Fri, 14 Aug 2026 15:14:28 +0200 Subject: [PATCH 04/14] feat(daemon): run the project clone off the reactor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same split as `CreateWorktree`, for the same reason and more so: a clone is network-bound and unbounded, so running it through the synchronous `execute_action` path would hold the workspace lock for the whole fetch and stall every other daemon action. The URL and the target resolve first with no lock held, so bad input fails the request outright instead of creating a row that vanishes a moment later. Then an optimistic row lands (no layout, so the client renders the creating placeholder), the clone runs on a blocking thread, and the fast mutations — seed layout, fire `on_project_open`, spawn PTYs — happen under a brief lock. Failure rolls the row back and toasts. A stale completion (the workspace was replaced by a session load) leaves the clone on disk, unlike the worktree path: it is a plain directory of the user's code, and deleting it unprompted is worse than leaving it. The reply carries `pending: true` — same contract as `CreateWorktree`, so callers know `path` does not exist yet. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01S63Q5kXgrw85F85QS1cntR --- crates/okena-daemon-core/src/command_loop.rs | 166 +++++++++++++++++++ 1 file changed, 166 insertions(+) diff --git a/crates/okena-daemon-core/src/command_loop.rs b/crates/okena-daemon-core/src/command_loop.rs index d2e454f9e..dfe0a26c9 100644 --- a/crates/okena-daemon-core/src/command_loop.rs +++ b/crates/okena-daemon-core/src/command_loop.rs @@ -2876,6 +2876,172 @@ 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)) => { + 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 clone_url = url.clone(); + tokio::task::spawn_local(async move { + let git = tokio::task::spawn_blocking(move || { + okena_git::clone_repository(&clone_url, &target) + }) + .await; + + // 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 => { + let msg = match result { + Ok(Err(e)) => e.to_string(), + Err(join) => { + format!("clone task failed: {join}") + } + 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( + format!("Clone failed: {msg}"), + )); + } + } + } + }); + // 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 From 56f8f617328cfcbb0ddf4bb1cde4f1a6005a690d Mon Sep 17 00:00:00 2001 From: David Matejka Date: Fri, 14 Aug 2026 15:14:36 +0200 Subject: [PATCH 05/14] feat(app): add a git source to the add-project dialog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `Source: Folder | Git` toggle. In Git mode the dialog asks for the URL, the parent to clone into, and the folder name to create inside it — the folder name is derived from the URL but stays editable, because the name a repository ships with is not always the one you want on disk. The fills chain: URL fills the folder name, the folder name fills the project name. A field holding anything other than what the dialog put there belongs to the user and stops being overwritten. Both are driven by `InputChangedEvent`, not by notify — the cursor blink notifies twice a second, and re-running a fill on those would keep resetting the caret. The path completion list now anchors to the input's painted bounds instead of a hardcoded offset; the offset was already approximate, and the extra rows in Git mode would have put the list in the wrong place. The creating placeholder tells a clone from a worktree, so a cloning project no longer claims to be setting up a worktree. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01S63Q5kXgrw85F85QS1cntR --- .../src/views/overlays/add_project_dialog.rs | 435 +++++++++++++----- .../src/views/panels/project_column.rs | 30 +- 2 files changed, 350 insertions(+), 115 deletions(-) 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..1ef571f71 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,26 @@ 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 +128,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 +155,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 +221,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 +233,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 +292,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 +315,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 +402,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 +496,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 +508,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 +562,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 +572,18 @@ 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 +594,30 @@ 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 +637,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/panels/project_column.rs b/crates/okena-app/src/views/panels/project_column.rs index 06f6c3ab4..eb4e4efa4 100644 --- a/crates/okena-app/src/views/panels/project_column.rs +++ b/crates/okena-app/src/views/panels/project_column.rs @@ -930,9 +930,23 @@ 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, 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 +955,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,7 +971,7 @@ 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), ) } @@ -1157,7 +1171,9 @@ 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(), cx) + .into_any_element(), ColumnContent::Empty => self.render_empty_state(cx).into_any_element(), }; From 7e0b01ef12da30de73feff2875c2ce5adeae34bd Mon Sep 17 00:00:00 2001 From: David Matejka Date: Fri, 14 Aug 2026 15:14:42 +0200 Subject: [PATCH 06/14] feat(cli): add project clone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `okena project clone [--into ] [--dir ] [--name ]`. `--into` defaults to the CWD and resolves against it, since the daemon does not share this process's working directory; `--dir` defaults to the name git would pick. Placement (`--hidden`, `--folder`) is shared with `project add` rather than copied — the two commands differ only in how the directory comes to exist. Like `worktree add`, the clone is optimistic: the id and path print immediately and a note goes to stderr, so a script does not `cd` into a path that is still being fetched. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01S63Q5kXgrw85F85QS1cntR --- Cargo.lock | 1 + crates/okena-cli/Cargo.toml | 3 + crates/okena-cli/src/CLAUDE.md | 2 +- crates/okena-cli/src/commands.rs | 104 +++++++++++++++++++++++++++++-- crates/okena-cli/src/lib.rs | 19 +++++- crates/okena-cli/src/parser.rs | 20 ++++++ crates/okena-cli/src/skill.md | 17 ++--- 7 files changed, 152 insertions(+), 14 deletions(-) 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-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..5efff5bbe 100644 --- a/crates/okena-cli/src/commands.rs +++ b/crates/okena-cli/src/commands.rs @@ -1150,7 +1150,103 @@ 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 +1255,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 +1268,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 +1287,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/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. From 550088a1804a61d24dced22487805262258c0d3e Mon Sep 17 00:00:00 2001 From: David Matejka Date: Fri, 14 Aug 2026 16:34:48 +0200 Subject: [PATCH 07/14] fix: make repository cloning recoverable Cancel an in-flight git command when daemon shutdown drops its task. Reconcile interrupted clone rows during startup, and reject Windows drive prefixes as clone directory names. --- crates/okena-core/src/process/bus.rs | 23 ++++ crates/okena-core/src/process/mod.rs | 7 +- crates/okena-daemon-core/src/command_loop.rs | 58 +++++++- crates/okena-git/src/lib.rs | 15 +- crates/okena-git/src/repository/clone.rs | 25 +++- crates/okena-git/src/repository/mod.rs | 5 +- crates/okena-workspace/src/actions/project.rs | 11 +- crates/okena-workspace/src/persistence.rs | 130 ++++++++++++++---- 8 files changed, 222 insertions(+), 52 deletions(-) diff --git a/crates/okena-core/src/process/bus.rs b/crates/okena-core/src/process/bus.rs index 382db4911..dd01bde8c 100644 --- a/crates/okena-core/src/process/bus.rs +++ b/crates/okena-core/src/process/bus.rs @@ -327,6 +327,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 +358,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. diff --git a/crates/okena-core/src/process/mod.rs b/crates/okena-core/src/process/mod.rs index 7914cc691..424abed0f 100644 --- a/crates/okena-core/src/process/mod.rs +++ b/crates/okena-core/src/process/mod.rs @@ -14,7 +14,9 @@ mod bus; -pub use bus::{CommandBus, CommandHandle, CommandSpec, Lane, current_lane, with_lane}; +pub use bus::{ + CommandBus, CommandCancellation, CommandHandle, CommandSpec, Lane, 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 @@ -460,9 +462,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 dfe0a26c9..03297c0f4 100644 --- a/crates/okena-daemon-core/src/command_loop.rs +++ b/crates/okena-daemon-core/src/command_loop.rs @@ -135,6 +135,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 { @@ -2934,7 +2954,27 @@ pub async fn daemon_command_loop( }; match registered { Err(e) => CommandResult::Err(e), - Ok((new_id, operation_epoch)) => { + Ok((new_id, operation_epoch)) => 'start_clone: { + let clone_command = match okena_git::start_clone_repository( + &url, &target, + ) { + 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(); @@ -2943,12 +2983,20 @@ pub async fn daemon_command_loop( let terminals = terminals.clone(); let app_settings = app_settings.clone(); let new_id_task = new_id.clone(); - let clone_url = url.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::clone_repository(&clone_url, &target) + 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 @@ -3018,7 +3066,9 @@ pub async fn daemon_command_loop( ws.remove_pending_project(&new_id_task); ws.notify_data(&mut cx); } - log::error!("clone-project: {url} failed: {msg}"); + log::error!( + "clone-project: {url} failed: {msg}" + ); if let Some(hm) = &hook_monitor { hm.push_toast(okena_state::Toast::error( format!("Clone failed: {msg}"), diff --git a/crates/okena-git/src/lib.rs b/crates/okena-git/src/lib.rs index e702b020e..ca520af49 100644 --- a/crates/okena-git/src/lib.rs +++ b/crates/okena-git/src/lib.rs @@ -20,13 +20,14 @@ pub use repository::{ 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, 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, - 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, validate_clone_url, verify_linked_worktree_fresh, verify_orphaned_worktree, + 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, list_branches, + list_branches_classified, list_linked_worktree_paths, list_pull_requests, merge_branch, + move_worktree, 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, 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/clone.rs b/crates/okena-git/src/repository/clone.rs index 821dbd71f..ea34a9165 100644 --- a/crates/okena-git/src/repository/clone.rs +++ b/crates/okena-git/src/repository/clone.rs @@ -2,7 +2,7 @@ use std::path::Path; -use okena_core::process::{CommandSpec, Lane, command, run}; +use okena_core::process::{CommandBus, CommandHandle, CommandSpec, Lane, command}; use super::{path_str, require_success}; use crate::error::{GitError, GitResult}; @@ -70,11 +70,11 @@ fn require_absent_clone_target(target_path: &Path) -> GitResult<()> { Ok(()) } -/// `git clone `. +/// 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. -pub fn clone_repository(url: &str, target_path: &Path) -> GitResult<()> { +pub fn start_clone_repository(url: &str, target_path: &Path) -> GitResult { let url = validate_clone_url(url)?; require_absent_clone_target(target_path)?; @@ -85,10 +85,21 @@ pub fn clone_repository(url: &str, target_path: &Path) -> GitResult<()> { let mut cmd = command("git"); cmd.args(["clone", "--", url, target_str]); - let output = run(CommandSpec::from_command(&cmd) - .lane(Lane::Long) - .label("git clone"))?; - require_success(output) + Ok(CommandBus::global().submit( + CommandSpec::from_command(&cmd) + .lane(Lane::Long) + .label("git clone"), + )) +} + +/// Wait for a clone submitted by [`start_clone_repository`]. +pub fn finish_clone_repository(handle: CommandHandle) -> GitResult<()> { + require_success(handle.wait()?) +} + +/// Submit and synchronously wait for `git clone `. +pub fn clone_repository(url: &str, target_path: &Path) -> GitResult<()> { + finish_clone_repository(start_clone_repository(url, target_path)?) } #[cfg(test)] diff --git a/crates/okena-git/src/repository/mod.rs b/crates/okena-git/src/repository/mod.rs index 4177d1df3..218e5b645 100644 --- a/crates/okena-git/src/repository/mod.rs +++ b/crates/okena-git/src/repository/mod.rs @@ -32,7 +32,10 @@ pub use branch::{ pub use ci::{ CiFetch, PrFetch, fetch_ci_checks, fetch_pr_info, has_github_remote, list_pull_requests, }; -pub use clone::{clone_dir_name, clone_repository, validate_clone_url}; +pub use clone::{ + clone_dir_name, clone_repository, finish_clone_repository, 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, diff --git a/crates/okena-workspace/src/actions/project.rs b/crates/okena-workspace/src/actions/project.rs index 82e3dd328..f35a11531 100644 --- a/crates/okena-workspace/src/actions/project.rs +++ b/crates/okena-workspace/src/actions/project.rs @@ -192,11 +192,10 @@ pub fn resolve_clone_target( if directory.is_empty() { return Err("Directory name is required".to_string()); } - if directory == "." - || directory == ".." - || directory.contains(['/', '\\']) - || std::path::Path::new(directory).components().count() != 1 - { + 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" )); @@ -3790,6 +3789,8 @@ mod gpui_tests { ); } assert!(super::resolve_clone_target("", "okena").is_err()); + #[cfg(windows)] + assert!(super::resolve_clone_target(r"C:\parent", "D:").is_err()); } #[test] diff --git a/crates/okena-workspace/src/persistence.rs b/crates/okena-workspace/src/persistence.rs index c9f3bee11..3147357a6 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 && !Path::new(&project.path).exists() + } + }) .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,30 +1043,30 @@ 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; + } + let checkout_exists = if p.worktree_info.is_some() { + worktree_checkout_path(p).exists() + } else { + Path::new(&p.path).exists() + }; + if !checkout_exists { + 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 @@ -2482,6 +2490,76 @@ mod tests { ); } + #[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() + )); + std::fs::create_dir(&checkout).expect("create clone target"); + 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(checkout).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 -> From b26e76c7399cf4d9d60e1a9e99d8006438c9f7ca Mon Sep 17 00:00:00 2001 From: David Matejka Date: Tue, 18 Aug 2026 14:23:31 +0200 Subject: [PATCH 08/14] style: apply rustfmt to the clone-project branch Formatting only, no behaviour change. `cargo fmt --all --check` reported these against the branch before any of the following commits touched it; separating them keeps the real diffs readable. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Xb6YVTnTpkMEZ8smJ1U8Sc --- .../src/workspace/actions/execute/project.rs | 12 ++- .../src/views/overlays/add_project_dialog.rs | 23 +++--- crates/okena-cli/src/commands.rs | 4 +- crates/okena-workspace/src/actions/project.rs | 28 ++++--- crates/okena-workspace/src/persistence.rs | 79 +++++++++++++++---- 5 files changed, 99 insertions(+), 47 deletions(-) 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 6e7b1f4c6..2dc856dc6 100644 --- a/crates/okena-app-core/src/workspace/actions/execute/project.rs +++ b/crates/okena-app-core/src/workspace/actions/execute/project.rs @@ -100,13 +100,11 @@ pub(super) fn clone_project( 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 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()); 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 1ef571f71..82d0f82f6 100644 --- a/crates/okena-app/src/views/overlays/add_project_dialog.rs +++ b/crates/okena-app/src/views/overlays/add_project_dialog.rs @@ -82,11 +82,9 @@ 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...")); + 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 @@ -576,8 +574,7 @@ impl Render for AddProjectDialog { .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)), + SimpleInput::new(&self.url_input).text_size(ui_text_md(cx)), ), )) }) @@ -605,12 +602,14 @@ impl Render for AddProjectDialog { ) // 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)), + 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( diff --git a/crates/okena-cli/src/commands.rs b/crates/okena-cli/src/commands.rs index 5efff5bbe..c7db71d35 100644 --- a/crates/okena-cli/src/commands.rs +++ b/crates/okena-cli/src/commands.rs @@ -1231,9 +1231,7 @@ pub fn cli_project_clone( // 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" - ); + eprintln!("clone started in the background; the path will exist once it completes"); } project_add_followups(&token, &project_id, hidden, folder, window) } diff --git a/crates/okena-workspace/src/actions/project.rs b/crates/okena-workspace/src/actions/project.rs index f35a11531..c53091e02 100644 --- a/crates/okena-workspace/src/actions/project.rs +++ b/crates/okena-workspace/src/actions/project.rs @@ -358,9 +358,13 @@ impl Workspace { let id = uuid::Uuid::new_v4().to_string(); let layout = with_terminal.then(LayoutNode::new_terminal); - self.data - .projects - .push(new_project_row(id.clone(), name, path, layout, default_shell)); + self.data.projects.push(new_project_row( + id.clone(), + name, + path, + layout, + default_shell, + )); self.data.project_order.push(id.clone()); self.data.add_project_hide_in_other_windows(&id, window_id); self.notify_data(cx); @@ -422,12 +426,12 @@ impl Workspace { #[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), - }, - ); + 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); @@ -462,7 +466,6 @@ impl Workspace { 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 @@ -3830,7 +3833,10 @@ mod gpui_tests { 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.layout.is_some(), + "layout seeded once the dir exists" + ); assert!(!project.is_creating); }); } diff --git a/crates/okena-workspace/src/persistence.rs b/crates/okena-workspace/src/persistence.rs index 3147357a6..47991aa91 100644 --- a/crates/okena-workspace/src/persistence.rs +++ b/crates/okena-workspace/src/persistence.rs @@ -2490,13 +2490,35 @@ 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() - )); - std::fs::create_dir(&checkout).expect("create clone target"); + 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; @@ -2518,15 +2540,46 @@ mod tests { !clone.is_creating, "completed clone clears its stale marker" ); - std::fs::remove_dir(checkout).expect("remove clone target"); + 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 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; @@ -2545,10 +2598,8 @@ mod tests { #[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 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; From 893d05455e8ce3c88ca7989a20f9811044e7b5e8 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Tue, 18 Aug 2026 14:24:03 +0200 Subject: [PATCH 09/14] fix(process): give every spawned command its own session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `git clone` of a private repo hung indefinitely with nothing in the log. The child was stopped (state T, `do_signal_stop`), not running: git asks for credentials on `/dev/tty` rather than stdin, and reading a terminal from a background process group raises SIGTTIN. The bus then blocked in `wait()` forever. `setsid` closes that off. A new session has no controlling terminal, so `/dev/tty` cannot be opened to prompt on in the first place. It also starts a new process group whose id is the child's pid, which is exactly the identity `ProcessTree::terminate` kills, so this replaces `process_group(0)` rather than fighting it. `setpgid` stays as a fallback so that invariant holds even if `setsid` ever fails. Applies to every bus command, which is the right scope: they all run with stdin on /dev/null and their output piped, so none of them can service a prompt anyway — they can only hang on one. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Xb6YVTnTpkMEZ8smJ1U8Sc --- crates/okena-core/src/process/bus.rs | 22 +++++++++++++++++++++- crates/okena-core/src/process/mod.rs | 19 +++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/crates/okena-core/src/process/bus.rs b/crates/okena-core/src/process/bus.rs index dd01bde8c..25f54657b 100644 --- a/crates/okena-core/src/process/bus.rs +++ b/crates/okena-core/src/process/bus.rs @@ -718,7 +718,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 424abed0f..9803f9c11 100644 --- a/crates/okena-core/src/process/mod.rs +++ b/crates/okena-core/src/process/mod.rs @@ -275,6 +275,25 @@ pub mod testing { #[cfg(test)] mod tests { + /// 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; From 695b065b6e030c83cf6d3e547a51956a2abcbc73 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Tue, 18 Aug 2026 14:24:26 +0200 Subject: [PATCH 10/14] fix(git): refuse interactive prompts when git talks to a remote MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cloning a private repo with no credential helper configured left git waiting on a username prompt it could never show. The previous commit stops that from hanging; this makes it fail with something worth reading instead. `network_command()` builds the git command for anything that reaches a remote, and every clone/fetch/push site now goes through it: - `GIT_TERMINAL_PROMPT=0` — git dies with "terminal prompts disabled". - `GIT_ASKPASS` / `SSH_ASKPASS` set to empty. Empty rather than unset is deliberate: git reads an empty value as "set but unusable" and skips askpass entirely, where unsetting would let it fall through to `core.askpass`. - `GCM_INTERACTIVE=never` for Git Credential Manager. - `GIT_SSH_COMMAND=ssh -o BatchMode=yes`, but only when the user has not set one of their own. BatchMode still authenticates through an agent; it turns passphrase and host-key prompts into failures rather than hangs. Fetch and push had the same exposure and are covered too, before anyone hit it there. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Xb6YVTnTpkMEZ8smJ1U8Sc --- crates/okena-git/CLAUDE.md | 3 ++- crates/okena-git/src/repository/branch.rs | 8 +++--- crates/okena-git/src/repository/clone.rs | 6 ++--- crates/okena-git/src/repository/mod.rs | 27 +++++++++++++++++++++ crates/okena-git/src/repository/worktree.rs | 6 ++--- 5 files changed, 39 insertions(+), 11 deletions(-) diff --git a/crates/okena-git/CLAUDE.md b/crates/okena-git/CLAUDE.md index 17939076c..8241f5027 100644 --- a/crates/okena-git/CLAUDE.md +++ b/crates/okena-git/CLAUDE.md @@ -8,7 +8,7 @@ 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. | @@ -20,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/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 index ea34a9165..15e651747 100644 --- a/crates/okena-git/src/repository/clone.rs +++ b/crates/okena-git/src/repository/clone.rs @@ -2,9 +2,9 @@ use std::path::Path; -use okena_core::process::{CommandBus, CommandHandle, CommandSpec, Lane, command}; +use okena_core::process::{CommandBus, CommandHandle, CommandSpec, Lane}; -use super::{path_str, require_success}; +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. @@ -83,7 +83,7 @@ pub fn start_clone_repository(url: &str, target_path: &Path) -> GitResult 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<()> { 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!( From fb4fd3d65851b5dace19c7e6c27b0baa2bcafe41 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Tue, 18 Aug 2026 14:24:43 +0200 Subject: [PATCH 11/14] fix(git): show git's own error line when a clone fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The failure toast read: Clone failed: git exited with status 128: Cloning into '/home/x/repo'... fatal: could not read Username for 'https://github.com': terminal prompts disabled Two lines of noise ahead of the one that says what went wrong. Git writes progress to stderr alongside errors, so the raw message opens with chatter and buries the cause. `GitError::user_detail()` picks out git's own error line — the last `fatal:` / `error:` / `remote: error:` — and drops the prefix, falling back to the exit status when nothing matches. The toast now carries "Clone failed" with that line as its detail, and the log keeps the untouched message. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Xb6YVTnTpkMEZ8smJ1U8Sc --- crates/okena-daemon-core/src/command_loop.rs | 23 ++++-- crates/okena-git/src/error.rs | 74 ++++++++++++++++++++ 2 files changed, 91 insertions(+), 6 deletions(-) diff --git a/crates/okena-daemon-core/src/command_loop.rs b/crates/okena-daemon-core/src/command_loop.rs index 03297c0f4..f0b81d94f 100644 --- a/crates/okena-daemon-core/src/command_loop.rs +++ b/crates/okena-daemon-core/src/command_loop.rs @@ -3043,10 +3043,18 @@ pub async fn daemon_command_loop( ws.notify_data(&mut cx); } result => { - let msg = match result { - Ok(Err(e)) => e.to_string(), + // 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) => { - format!("clone task failed: {join}") + let m = format!( + "clone task failed: {join}" + ); + (m.clone(), m) } Ok(Ok(())) => { unreachable!("success handled above") @@ -3070,9 +3078,12 @@ pub async fn daemon_command_loop( "clone-project: {url} failed: {msg}" ); if let Some(hm) = &hook_monitor { - hm.push_toast(okena_state::Toast::error( - format!("Clone failed: {msg}"), - )); + hm.push_toast( + okena_state::Toast::error( + "Clone failed", + ) + .with_detail(detail), + ); } } } diff --git a/crates/okena-git/src/error.rs b/crates/okena-git/src/error.rs index e9182432c..8443c8af0 100644 --- a/crates/okena-git/src/error.rs +++ b/crates/okena-git/src/error.rs @@ -48,5 +48,79 @@ pub enum GitError { 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) + }) +} + +#[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"); + } +} + /// Convenience alias for `Result`. pub type GitResult = Result; From bc807a655840a860e422cafa7245654de224881e Mon Sep 17 00:00:00 2001 From: David Matejka Date: Tue, 18 Aug 2026 14:35:15 +0200 Subject: [PATCH 12/14] fix(workspace): discard a clone interrupted mid-fetch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Startup recovery decided a mid-create project was finished purely from the target directory existing. That is not a safe test for a clone: `git clone` is not atomic, and one killed mid-fetch (daemon shutdown kills the process tree) leaves the directory behind holding a `.git` with an unborn HEAD. The row was then promoted to a normal project — layout seeded, terminal spawned — for a repo with no files in it. `is_complete_checkout` draws the line at HEAD resolving to a commit, which git writes only once the fetch has landed the branch it is about to check out. Both reconciliation sites now share one predicate; they have to agree, because a half-clone that is neither removed nor finished stays marked creating forever — the stuck-on-"cloning" state this recovery exists to prevent. Worktrees keep the existence test: `git worktree add` has no equivalent half-done state. Known gap: a clone killed during checkout, after HEAD is written, still passes. That window is far smaller than the fetch, and closing it properly means cloning to a temp directory and renaming on success. The directory itself is left on disk. Removing a directory of the user's code unprompted is worse than leaving it, matching how the stale-epoch path already reasons; the cost is that a retry reports the target as non-empty. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Xb6YVTnTpkMEZ8smJ1U8Sc --- crates/okena-git/src/lib.rs | 3 +- crates/okena-git/src/repository/clone.rs | 45 +++++++++++++++++++++++ crates/okena-git/src/repository/mod.rs | 4 +- crates/okena-workspace/src/persistence.rs | 29 +++++++++++---- 4 files changed, 71 insertions(+), 10 deletions(-) diff --git a/crates/okena-git/src/lib.rs b/crates/okena-git/src/lib.rs index ca520af49..3c5cf8972 100644 --- a/crates/okena-git/src/lib.rs +++ b/crates/okena-git/src/lib.rs @@ -22,7 +22,8 @@ pub use repository::{ 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, list_branches, + 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, project_path_in_worktree, push_branch, rebase_onto, remove_orphaned_worktree, remove_worktree, remove_worktree_fast, resolve_git_root_and_subdir, resolve_review_base, diff --git a/crates/okena-git/src/repository/clone.rs b/crates/okena-git/src/repository/clone.rs index 15e651747..ba5b9f836 100644 --- a/crates/okena-git/src/repository/clone.rs +++ b/crates/okena-git/src/repository/clone.rs @@ -97,6 +97,20 @@ 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 `. pub fn clone_repository(url: &str, target_path: &Path) -> GitResult<()> { finish_clone_repository(start_clone_repository(url, target_path)?) @@ -141,6 +155,37 @@ mod tests { ); } + #[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())); diff --git a/crates/okena-git/src/repository/mod.rs b/crates/okena-git/src/repository/mod.rs index c95f0c126..bee357425 100644 --- a/crates/okena-git/src/repository/mod.rs +++ b/crates/okena-git/src/repository/mod.rs @@ -34,8 +34,8 @@ pub use ci::{ CiFetch, PrFetch, fetch_ci_checks, fetch_pr_info, has_github_remote, list_pull_requests, }; pub use clone::{ - clone_dir_name, clone_repository, finish_clone_repository, start_clone_repository, - validate_clone_url, + clone_dir_name, clone_repository, finish_clone_repository, is_complete_checkout, + start_clone_repository, validate_clone_url, }; pub use paths::{ compute_target_paths, get_repo_common_dir, get_repo_root, normalize_path, diff --git a/crates/okena-workspace/src/persistence.rs b/crates/okena-workspace/src/persistence.rs index 47991aa91..4d7738925 100644 --- a/crates/okena-workspace/src/persistence.rs +++ b/crates/okena-workspace/src/persistence.rs @@ -978,7 +978,7 @@ pub(crate) fn sync_worktrees_with_backend_and_shell( if project.worktree_info.is_some() { !worktree_checkout_path(project).exists() } else { - project.is_creating && !Path::new(&project.path).exists() + project.is_creating && !interrupted_create_is_usable(project) } }) .map(|p| p.id.clone()) @@ -1055,12 +1055,7 @@ pub(crate) fn sync_worktrees_with_backend_and_shell( if !p.is_creating { continue; } - let checkout_exists = if p.worktree_info.is_some() { - worktree_checkout_path(p).exists() - } else { - Path::new(&p.path).exists() - }; - if !checkout_exists { + if !interrupted_create_is_usable(p) { continue; } if p.layout.is_none() { @@ -1072,6 +1067,26 @@ pub(crate) fn sync_worktrees_with_backend_and_shell( 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 From 2a39c81dfa2a8754da0046d5abbba34df926d96b Mon Sep 17 00:00:00 2001 From: David Matejka Date: Tue, 18 Aug 2026 14:37:17 +0200 Subject: [PATCH 13/14] feat(clone): report progress while a repository clones MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A clone of a large repo sat on "Cloning repository…" for minutes with no way to tell it from a hang. Git knows how far it has got; nothing carried that to the UI. The bus was the obstacle: it read each pipe with `read_to_end` and handed the output back only once the process exited, so progress arrived exactly when it stopped being useful. `CommandSpec::on_stderr_line` adds an opt-in sink fed as the bytes arrive. Lines break on `\r` as well as `\n` — progress tools rewrite one line with a carriage return, and splitting on `\n` alone would hold every update back to the end, which is the buffering this removes. Captured output is unchanged, and a line buffer that never breaks is capped rather than grown without bound. `git clone` then runs with `--progress`, which is required rather than cosmetic: git reports progress only when stderr is a terminal, and the bus always pipes it. `parse_clone_progress` reads the phase and percentage and ignores every other line git writes. `ProjectData.creating_progress` carries it to the UI and over the wire. Not persisted, for the reason `is_closing` is not: it describes a live process, and reloading a stale percentage after a restart would claim progress that nothing is making. `set_creating_progress` reports whether anything changed so an unchanged value costs no broadcast, and ignores a project that is no longer being created — progress comes off a reader thread and can land late, which must not resurrect the placeholder. Publishing is limited to one update per 250ms because each takes the workspace lock and pushes a snapshot to every client; 100% always goes through so a phase never appears to stall short of finishing. The project column shows the line under the placeholder, and the sidebar row replaces its generic "Creating…" with it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Xb6YVTnTpkMEZ8smJ1U8Sc --- crates/okena-app-core/src/remote_snapshot.rs | 1 + .../src/workspace/actions/execute/mod.rs | 1 + .../src/workspace/actions/execute/project.rs | 2 + .../src/workspace/actions/execute/session.rs | 1 + .../src/views/overlays/project_switcher.rs | 1 + .../src/views/panels/project_column.rs | 24 ++- crates/okena-cli/src/resolve.rs | 1 + crates/okena-core/src/api.rs | 7 + crates/okena-core/src/process/bus.rs | 98 ++++++++++- crates/okena-core/src/process/mod.rs | 71 +++++++- crates/okena-daemon-core/src/command_loop.rs | 58 +++++- crates/okena-daemon-core/src/daemon.rs | 3 + crates/okena-daemon-core/src/git_poll.rs | 1 + crates/okena-daemon-core/src/observers.rs | 1 + crates/okena-daemon-core/src/pty_loop.rs | 3 + crates/okena-daemon-core/src/soft_close.rs | 1 + .../src/worktree_close_watchdog.rs | 1 + crates/okena-git/src/lib.rs | 20 +-- crates/okena-git/src/repository/clone.rs | 165 +++++++++++++++++- crates/okena-git/src/repository/mod.rs | 48 ++++- crates/okena-mobile-ffi/src/client/manager.rs | 1 + crates/okena-state/src/workspace_data.rs | 10 ++ crates/okena-transport/src/client/state.rs | 2 + .../okena-views-sidebar/src/project_list.rs | 15 +- .../src/sidebar/from_project_test.rs | 1 + crates/okena-views-sidebar/src/sidebar/mod.rs | 5 + crates/okena-workspace/src/actions/focus.rs | 1 + crates/okena-workspace/src/actions/folder.rs | 2 + .../src/actions/layout/tests_gpui.rs | 2 + crates/okena-workspace/src/actions/project.rs | 3 + .../okena-workspace/src/actions/soft_close.rs | 1 + .../okena-workspace/src/actions/worktree.rs | 2 + crates/okena-workspace/src/persistence.rs | 2 + crates/okena-workspace/src/remote_apply.rs | 2 + crates/okena-workspace/src/state.rs | 24 +++ crates/okena-workspace/src/visibility.rs | 1 + 36 files changed, 554 insertions(+), 28 deletions(-) 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 277821129..6aa4db4a6 100644 --- a/crates/okena-app-core/src/workspace/actions/execute/mod.rs +++ b/crates/okena-app-core/src/workspace/actions/execute/mod.rs @@ -1294,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 2dc856dc6..72266811c 100644 --- a/crates/okena-app-core/src/workspace/actions/execute/project.rs +++ b/crates/okena-app-core/src/workspace/actions/execute/project.rs @@ -809,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, @@ -1276,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/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 eb4e4efa4..8f7fd0e5d 100644 --- a/crates/okena-app/src/views/panels/project_column.rs +++ b/crates/okena-app/src/views/panels/project_column.rs @@ -932,7 +932,12 @@ impl ProjectColumn { /// 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, cx: &mut Context) -> impl IntoElement { + 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 { ( @@ -973,6 +978,16 @@ impl ProjectColumn { .text_center() .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 { @@ -1172,7 +1187,11 @@ impl Render for ProjectColumn { } ColumnContent::Closing => self.render_closing_state(cx).into_any_element(), ColumnContent::Creating => self - .render_creating_state(project.worktree_info.is_some(), cx) + .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(), }; @@ -1280,6 +1299,7 @@ mod tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, } } 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-core/src/api.rs b/crates/okena-core/src/api.rs index 77cfedb6a..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 @@ -1370,6 +1376,7 @@ mod tests { }, is_creating: false, is_closing: false, + creating_progress: None, }], focused_project_id: Some("p1".into()), fullscreen_terminal: None, diff --git a/crates/okena-core/src/process/bus.rs b/crates/okena-core/src/process/bus.rs index 25f54657b..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, } } @@ -578,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 @@ -684,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>, diff --git a/crates/okena-core/src/process/mod.rs b/crates/okena-core/src/process/mod.rs index 9803f9c11..1b59bd011 100644 --- a/crates/okena-core/src/process/mod.rs +++ b/crates/okena-core/src/process/mod.rs @@ -15,7 +15,8 @@ mod bus; pub use bus::{ - CommandBus, CommandCancellation, CommandHandle, CommandSpec, Lane, current_lane, with_lane, + CommandBus, CommandCancellation, CommandHandle, CommandSpec, Lane, StderrSink, current_lane, + with_lane, }; /// Create a [`std::process::Command`] that does **not** flash a console @@ -275,6 +276,74 @@ 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. diff --git a/crates/okena-daemon-core/src/command_loop.rs b/crates/okena-daemon-core/src/command_loop.rs index f0b81d94f..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)] @@ -2956,7 +2961,51 @@ pub async fn daemon_command_loop( Err(e) => CommandResult::Err(e), Ok((new_id, operation_epoch)) => 'start_clone: { let clone_command = match okena_git::start_clone_repository( - &url, &target, + &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) => { @@ -6014,6 +6063,7 @@ mod tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, }; WorkspaceData { version: 1, @@ -6371,6 +6421,7 @@ mod tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, }; WorkspaceData { version: 1, @@ -7111,6 +7162,7 @@ mod tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, }; WorkspaceData { version: 1, @@ -7353,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()]); @@ -8830,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/src/lib.rs b/crates/okena-git/src/lib.rs index 3c5cf8972..1d77e7aeb 100644 --- a/crates/okena-git/src/lib.rs +++ b/crates/okena-git/src/lib.rs @@ -16,19 +16,19 @@ pub use diff::{ }; pub use error::{GitError, GitResult}; pub use repository::{ - BranchList, 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, + 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, 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, start_clone_repository, stash_changes, stash_pop, unstage_file, validate_clone_url, - verify_linked_worktree_fresh, verify_orphaned_worktree, + 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, 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/clone.rs b/crates/okena-git/src/repository/clone.rs index ba5b9f836..1f1156fa3 100644 --- a/crates/okena-git/src/repository/clone.rs +++ b/crates/okena-git/src/repository/clone.rs @@ -70,11 +70,61 @@ fn require_absent_clone_target(target_path: &Path) -> GitResult<()> { 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. -pub fn start_clone_repository(url: &str, target_path: &Path) -> GitResult { +/// +/// `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)?; @@ -84,11 +134,18 @@ pub fn start_clone_repository(url: &str, target_path: &Path) -> GitResult bool { gix::open(path).is_ok_and(|repo| repo.head_id().is_ok()) } -/// Submit and synchronously wait for `git clone `. +/// 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)?) + finish_clone_repository(start_clone_repository(url, target_path, |_| {})?) } #[cfg(test)] @@ -155,6 +213,103 @@ mod tests { ); } + #[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}; diff --git a/crates/okena-git/src/repository/mod.rs b/crates/okena-git/src/repository/mod.rs index bee357425..775be5175 100644 --- a/crates/okena-git/src/repository/mod.rs +++ b/crates/okena-git/src/repository/mod.rs @@ -34,8 +34,8 @@ pub use ci::{ CiFetch, PrFetch, fetch_ci_checks, fetch_pr_info, has_github_remote, list_pull_requests, }; pub use clone::{ - clone_dir_name, clone_repository, finish_clone_repository, is_complete_checkout, - start_clone_repository, validate_clone_url, + 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, @@ -164,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-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 c53091e02..13da4d0b9 100644 --- a/crates/okena-workspace/src/actions/project.rs +++ b/crates/okena-workspace/src/actions/project.rs @@ -39,6 +39,7 @@ fn new_project_row( last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, } } @@ -1261,6 +1262,7 @@ mod tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, } } @@ -1590,6 +1592,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/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 4d7738925..b69d4f393 100644 --- a/crates/okena-workspace/src/persistence.rs +++ b/crates/okena-workspace/src/persistence.rs @@ -1191,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(), @@ -1615,6 +1616,7 @@ mod tests { last_activity_at: None, is_creating: false, is_closing: false, + creating_progress: None, } } 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, } } From 60a5f400bc3ddc79fdd24806ee508d988037afb6 Mon Sep 17 00:00:00 2001 From: David Matejka Date: Tue, 18 Aug 2026 15:19:28 +0200 Subject: [PATCH 14/14] fix(git): move the error tests below the module's items `clippy::items_after_test_module` is denied in CI and the new `mod tests` landed above `GitResult`, breaking the build. Test module goes last. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Xb6YVTnTpkMEZ8smJ1U8Sc --- crates/okena-git/src/error.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/okena-git/src/error.rs b/crates/okena-git/src/error.rs index 8443c8af0..36de7a24a 100644 --- a/crates/okena-git/src/error.rs +++ b/crates/okena-git/src/error.rs @@ -80,6 +80,9 @@ fn git_failure_line(stderr: &str) -> Option { }) } +/// Convenience alias for `Result`. +pub type GitResult = Result; + #[cfg(test)] mod tests { use super::*; @@ -121,6 +124,3 @@ mod tests { assert_eq!(err.user_detail(), "invalid repository URL: -x"); } } - -/// Convenience alias for `Result`. -pub type GitResult = Result;