diff --git a/apps/staged/src-tauri/src/branches.rs b/apps/staged/src-tauri/src/branches.rs index 1bca3742..097b4faa 100644 --- a/apps/staged/src-tauri/src/branches.rs +++ b/apps/staged/src-tauri/src/branches.rs @@ -2353,6 +2353,366 @@ pub async fn delete_branch( store.delete_branch(&branch_id).map_err(|e| e.to_string()) } +/// Move a branch into another project, taking its notes, commits, reviews, +/// sessions and images with it. +#[tauri::command(rename_all = "camelCase")] +pub async fn move_branch( + store: tauri::State<'_, Mutex>>>, + executor: tauri::State<'_, Arc>, + registry: tauri::State<'_, Arc>, + branch_id: String, + target_project_id: String, +) -> Result { + let store = get_store(&store)?; + move_branch_impl(&store, &executor, ®istry, &branch_id, &target_project_id).await +} + +/// The `(github_repo, subpath)` identity of a repo inside a project, matching +/// `idx_project_repos_unique`'s `(project_id, github_repo, COALESCE(subpath, +/// ''))`: a NULL subpath and an empty one are the same repo. +fn repo_subpath_key(github_repo: &str, subpath: Option<&str>) -> String { + format!("{github_repo}\u{0}{}", subpath.unwrap_or("")) +} + +fn describe_repo(github_repo: &str, subpath: Option<&str>) -> String { + match subpath.filter(|s| !s.is_empty()) { + Some(subpath) => format!("{github_repo} ({subpath})"), + None => github_repo.to_string(), + } +} + +/// A validated branch move: what to rewrite, and where the worktree has to end +/// up, resolved against the database before anything is mutated. +#[derive(Debug)] +struct BranchMovePlan { + source_project: store::Project, + /// The branch's repo slug, resolved through `project_repos` or the source + /// project's primary. Names the local clone the worktree belongs to. + github_repo: String, + mv: store::BranchMove, +} + +/// Check every precondition on a move and resolve what it will rewrite. +/// +/// Split out from [`move_branch_impl`] because this is the whole of the move's +/// decision-making — which `project_repos` row travels, which is cloned, what +/// counts as a destination that already has the repo — and it needs nothing but +/// the store to run. +fn plan_branch_move( + store: &Arc, + branch_id: &str, + target_project_id: &str, +) -> Result { + let branch = store + .get_branch(branch_id) + .map_err(|e| e.to_string())? + .ok_or_else(|| format!("Branch not found: {branch_id}"))?; + if branch.project_id == target_project_id { + return Err("This branch is already in that project".to_string()); + } + + let source_project = store + .get_project(&branch.project_id) + .map_err(|e| e.to_string())? + .ok_or_else(|| format!("Project not found: {}", branch.project_id))?; + let target_project = store + .get_project(target_project_id) + .map_err(|e| e.to_string())? + .ok_or_else(|| format!("Project not found: {target_project_id}"))?; + + // Remote branches of a project share one Blox workspace, so moving one out + // would mean cross-workspace surgery on a filesystem we don't own. + for project in [&source_project, &target_project] { + if project.location == store::ProjectLocation::Remote { + return Err(format!( + "Project '{}' runs on a remote workspace; branches can only move between local projects", + project.name + )); + } + } + if branch.branch_type == store::BranchType::Remote { + return Err("Remote branches can't be moved between projects".to_string()); + } + + // The move pulls the worktree out from under anything running in it. + // [`apply_branch_move`] plans again under the branch's session launch lock, + // which is what keeps a session from starting between this check and the + // rename that acts on it. + if store + .has_running_session_for_branch(branch_id) + .map_err(|e| e.to_string())? + { + return Err("A session is running on this branch — wait for the running session to finish before moving it".to_string()); + } + + let source_repo = match &branch.project_repo_id { + Some(repo_id) => store.get_project_repo(repo_id).map_err(|e| e.to_string())?, + None => None, + }; + // Mirrors `resolve_branch_repo_slug`: a branch with no repo link falls back + // to its project's primary repo. + let (github_repo, subpath) = match &source_repo { + Some(repo) => (repo.github_repo.clone(), repo.subpath.clone()), + None => ( + project_primary_repo(&source_project)?.to_string(), + source_project.subpath.clone(), + ), + }; + + let key = repo_subpath_key(&github_repo, subpath.as_deref()); + if store + .list_project_repos(target_project_id) + .map_err(|e| e.to_string())? + .iter() + .any(|repo| repo_subpath_key(&repo.github_repo, repo.subpath.as_deref()) == key) + { + return Err(format!( + "'{}' already has {} attached", + target_project.name, + describe_repo(&github_repo, subpath.as_deref()) + )); + } + + // A `project_repos` row can be shared by several branches, so it only + // travels when this branch is the last one on it. + let placement = match &source_repo { + Some(repo) => { + let shared = store + .list_branches_for_project(&source_project.id) + .map_err(|e| e.to_string())? + .into_iter() + .any(|b| { + b.id != branch.id && b.project_repo_id.as_deref() == Some(repo.id.as_str()) + }); + if shared { + let mut clone = store::ProjectRepo::new( + target_project_id, + &repo.github_repo, + &repo.branch_name, + repo.subpath.clone(), + ); + clone.reason = repo.reason.clone(); + clone.head_repo = repo.head_repo.clone(); + store::RepoPlacement::Clone(clone) + } else { + store::RepoPlacement::Reparent { + repo_id: repo.id.clone(), + } + } + } + // Legacy branch with no repo link: materialize a row rather than carry + // the NULL across, which `resolve_branch_repo_slug` would resolve to the + // *destination's* primary repo — a wrong-repo read. + None => store::RepoPlacement::Clone(store::ProjectRepo::new( + target_project_id, + &github_repo, + &branch.branch_name, + subpath.clone(), + )), + }; + + // `workdirs.path` is the source of truth for where the worktree actually + // is — some branches still sit in the legacy non-project-scoped layout. + let workdir = match store + .get_workdir_for_branch(branch_id) + .map_err(|e| e.to_string())? + { + Some(wd) => { + let new_path = git::project_worktree_path_for( + target_project_id, + &github_repo, + &branch.branch_name, + ) + .map_err(|e| e.to_string())?; + Some(store::WorkdirMove { + workdir_id: wd.id, + old_path: wd.path, + new_path: new_path.to_string_lossy().to_string(), + }) + } + None => None, + }; + + Ok(BranchMovePlan { + mv: store::BranchMove { + branch_id: branch_id.to_string(), + source_project_id: source_project.id.clone(), + target_project_id: target_project_id.to_string(), + repo: placement, + workdir, + }, + source_project, + github_repo, + }) +} + +/// The body of [`move_branch`], shared with the web router. +/// +/// The move runs in three stages that have to agree with each other: the +/// worktree relocation on disk, the re-parent transaction, and the image files. +/// The first two go together in [`apply_branch_move`], under the branch's +/// session launch lock; a failed transaction puts the worktree back. +pub(crate) async fn move_branch_impl( + store: &Arc, + executor: &ActionExecutor, + registry: &ActionRegistry, + branch_id: &str, + target_project_id: &str, +) -> Result { + // Pre-flight, so an impossible move is refused before anything the branch is + // running gets stopped for it. [`apply_branch_move`] plans again under the + // launch lock, which is what makes the answer safe to act on. + plan_branch_move(store, branch_id, target_project_id)?; + + crate::actions::commands::stop_actions_for_branches(executor, registry, &[branch_id]); + + let source_project = { + let store = Arc::clone(store); + let branch_id = branch_id.to_string(); + let target_project_id = target_project_id.to_string(); + tauri::async_runtime::spawn_blocking(move || { + apply_branch_move(&store, &branch_id, &target_project_id) + }) + .await + .map_err(|e| format!("Failed to move branch: {e}"))?? + }; + + move_branch_image_files(store, branch_id, &source_project.id, target_project_id); + + // Cheaper to drop than to move, and it rebuilds on the next diff read. + if let Err(e) = crate::diff_cache::delete_branch_cache( + source_project.location, + &source_project.id, + branch_id, + ) { + log::warn!("Failed to clear diff cache for moved branch {branch_id}: {e}"); + } + + let updated = store + .get_branch(branch_id) + .map_err(|e| e.to_string())? + .ok_or_else(|| format!("Branch not found: {branch_id}"))?; + let workdir = store + .get_workdir_for_branch(branch_id) + .map_err(|e| e.to_string())? + .map(|w| w.path); + Ok(to_branch_with_workdir(updated, workdir)) +} + +/// Plan the move and apply it: the worktree on disk first, then the transaction +/// that re-parents the branch, with the rename undone if the transaction fails. +/// Returns the source project, which the caller still needs to relocate the +/// branch's images and drop its diff cache. +/// +/// Synchronous and holding the branch's session launch lock throughout, the way +/// [`crate::prs`] holds it around its queue-or-start decisions: the plan's "no +/// session is running on this branch" precondition is only worth acting on for +/// as long as no session can start underneath it, and two moves of the same +/// branch — the Tauri command and the web router are separate entry points — +/// would otherwise race the same rename. The plan is resolved in here for that +/// reason rather than handed in from outside the lock. +fn apply_branch_move( + store: &Arc, + branch_id: &str, + target_project_id: &str, +) -> Result { + let launch_lock = crate::session_commands::branch_session_launch_lock_for(branch_id); + let _guard = launch_lock.lock().unwrap(); + + let BranchMovePlan { + source_project, + github_repo, + mv, + } = plan_branch_move(store, branch_id, target_project_id)?; + + let repo_path = crate::paths::repos_dir() + .map(|d| d.join(&github_repo)) + .ok_or("Cannot determine clone path")?; + + // A branch whose setup never finished has nothing on disk to relocate — and + // neither does one whose move was interrupted between this rename and the + // commit below, because `new_path` is derived rather than stored: re-running + // the move finds the worktree already sitting where the transaction is about + // to record it, and completes. + let mut moved_on_disk = false; + if let Some(wd) = &mv.workdir { + let old_path = Path::new(&wd.old_path); + if old_path.exists() { + git::move_worktree(&repo_path, old_path, Path::new(&wd.new_path)) + .map_err(|e| format!("Failed to move worktree: {e}"))?; + moved_on_disk = true; + } + } + + if let Err(e) = store.move_branch_to_project(&mv) { + // Disk and database have to agree, so undo the half that landed. + if let (true, Some(wd)) = (moved_on_disk, &mv.workdir) { + if let Err(revert) = + git::move_worktree(&repo_path, Path::new(&wd.new_path), Path::new(&wd.old_path)) + { + log::warn!( + "Failed to move worktree for branch {branch_id} back to {}: {revert}", + wd.old_path + ); + // Reporting only the store error would read as "nothing changed" + // while the worktree sits where neither project expects it. + return Err(format!( + "{e} The worktree could not be moved back either, so it is now at {} instead of {}: {revert}", + wd.new_path, wd.old_path + )); + } + } + return Err(e.to_string()); + } + + Ok(source_project) +} + +/// Relocate a moved branch's image files into the destination project's +/// `images/` directory. +/// +/// Tolerant per entry, like [`crate::paths::migrate_directory_contents`]: a file +/// that won't move costs one broken attachment, not the whole move — and the +/// rows already point at the destination by the time this runs. +fn move_branch_image_files( + store: &Arc, + branch_id: &str, + source_project_id: &str, + target_project_id: &str, +) { + let images = match store.list_all_images_for_branch(branch_id) { + Ok(images) => images, + Err(e) => { + log::warn!("Cannot list images for moved branch {branch_id}: {e}"); + return; + } + }; + + for image in images { + let from = store::images::image_file_path(source_project_id, &image.id, &image.filename); + let to = store::images::image_file_path(target_project_id, &image.id, &image.filename); + let (Ok(from), Ok(to)) = (from, to) else { + continue; + }; + if !from.exists() { + continue; + } + if let Some(parent) = to.parent() { + if let Err(e) = std::fs::create_dir_all(parent) { + log::warn!("Cannot create image directory {}: {e}", parent.display()); + continue; + } + } + if let Err(e) = std::fs::rename(&from, &to) { + log::warn!( + "Failed to move image {} -> {}: {e}", + from.display(), + to.display() + ); + } + } +} + #[tauri::command(rename_all = "camelCase")] pub async fn rename_branch( store: tauri::State<'_, Mutex>>>, @@ -2991,4 +3351,200 @@ mod tests { "expected missing remote ref error, got: {err}" ); } + + // ── plan_branch_move ──────────────────────────────────────────────────── + + struct MoveFixture { + store: Arc, + source: store::Project, + target: store::Project, + repo: store::ProjectRepo, + branch: store::Branch, + } + + fn move_fixture() -> MoveFixture { + let store = Arc::new(Store::in_memory().unwrap()); + let source = store::Project::named("source").with_primary_repo("acme/widgets"); + let target = store::Project::named("target"); + store.create_project(&source).unwrap(); + store.create_project(&target).unwrap(); + + let repo = store::ProjectRepo::new(&source.id, "acme/widgets", "feature", None).primary(); + store.create_project_repo(&repo).unwrap(); + let branch = + store::Branch::new(&source.id, "feature", "origin/main").with_project_repo(&repo.id); + store.create_branch(&branch).unwrap(); + + MoveFixture { + store, + source, + target, + repo, + branch, + } + } + + #[test] + fn plan_move_carries_a_sole_branch_repo_row_across() { + let f = move_fixture(); + + let plan = plan_branch_move(&f.store, &f.branch.id, &f.target.id).unwrap(); + + match &plan.mv.repo { + store::RepoPlacement::Reparent { repo_id } => assert_eq!(repo_id, &f.repo.id), + other => panic!("expected the repo row to travel, got {other:?}"), + } + assert_eq!(plan.github_repo, "acme/widgets"); + // Nothing on disk yet, so there is no worktree to relocate. + assert!(plan.mv.workdir.is_none()); + } + + #[test] + fn plan_move_clones_a_repo_row_a_sibling_branch_shares() { + let f = move_fixture(); + let sibling = + store::Branch::new(&f.source.id, "other", "origin/main").with_project_repo(&f.repo.id); + f.store.create_branch(&sibling).unwrap(); + + let plan = plan_branch_move(&f.store, &f.branch.id, &f.target.id).unwrap(); + + match plan.mv.repo { + store::RepoPlacement::Clone(repo) => { + assert_ne!(repo.id, f.repo.id); + assert_eq!(repo.github_repo, "acme/widgets"); + assert_eq!(repo.project_id, f.target.id); + } + other => panic!("expected a cloned repo row, got {other:?}"), + } + } + + /// A NULL `project_repo_id` would resolve to the *destination's* primary + /// repo after the move, so the plan materializes a row instead. + #[test] + fn plan_move_materializes_a_row_for_a_branch_with_no_repo_link() { + let f = move_fixture(); + let legacy = store::Branch::new(&f.source.id, "legacy", "origin/main"); + f.store.create_branch(&legacy).unwrap(); + + let plan = plan_branch_move(&f.store, &legacy.id, &f.target.id).unwrap(); + + match plan.mv.repo { + store::RepoPlacement::Clone(repo) => { + assert_eq!(repo.github_repo, "acme/widgets"); + assert_eq!(repo.branch_name, "legacy"); + } + other => panic!("expected a materialized repo row, got {other:?}"), + } + } + + #[test] + fn plan_move_relocates_the_worktree_into_the_destination_project() { + let f = move_fixture(); + let workdir = store::Workdir::new(&f.source.id, "/wt/old/path").with_branch(&f.branch.id); + f.store.create_workdir(&workdir).unwrap(); + + let plan = plan_branch_move(&f.store, &f.branch.id, &f.target.id).unwrap(); + + let wd = plan.mv.workdir.expect("worktree should be relocated"); + assert_eq!(wd.workdir_id, workdir.id); + // The old path comes from the row, not a recomputed one — legacy-layout + // worktrees move from wherever they actually are. + assert_eq!(wd.old_path, "/wt/old/path"); + let expected = git::project_worktree_path_for(&f.target.id, "acme/widgets", "feature") + .unwrap() + .to_string_lossy() + .to_string(); + assert_eq!(wd.new_path, expected); + } + + #[test] + fn plan_move_rejects_a_remote_project_on_either_end() { + let f = move_fixture(); + let mut remote = store::Project::named("remote-target"); + remote.location = store::ProjectLocation::Remote; + f.store.create_project(&remote).unwrap(); + + let err = plan_branch_move(&f.store, &f.branch.id, &remote.id).unwrap_err(); + assert!(err.contains("remote workspace"), "unexpected error: {err}"); + + // …and the same when the branch is leaving a remote project. + let remote_repo = + store::ProjectRepo::new(&remote.id, "acme/other", "feature", None).primary(); + f.store.create_project_repo(&remote_repo).unwrap(); + let remote_branch = store::Branch::new(&remote.id, "feature", "origin/main") + .with_project_repo(&remote_repo.id); + f.store.create_branch(&remote_branch).unwrap(); + + let err = plan_branch_move(&f.store, &remote_branch.id, &f.target.id).unwrap_err(); + assert!(err.contains("remote workspace"), "unexpected error: {err}"); + } + + #[test] + fn plan_move_rejects_a_branch_with_a_session_running_on_it() { + let f = move_fixture(); + let session = store::Session::new_running("work", Path::new("/wt/old/path")) + .with_branch(&f.branch.id); + f.store.create_session(&session).unwrap(); + + let err = plan_branch_move(&f.store, &f.branch.id, &f.target.id).unwrap_err(); + + assert!( + err.contains("session is running"), + "unexpected error: {err}" + ); + } + + #[test] + fn plan_move_rejects_a_destination_that_already_has_the_repo() { + let f = move_fixture(); + f.store + .create_project_repo(&store::ProjectRepo::new( + &f.target.id, + "acme/widgets", + "main", + None, + )) + .unwrap(); + + let err = plan_branch_move(&f.store, &f.branch.id, &f.target.id).unwrap_err(); + + assert!( + err.contains("already has acme/widgets"), + "unexpected error: {err}" + ); + } + + /// `idx_project_repos_unique` coalesces the subpath, so a NULL subpath and + /// an empty one are the same repo — the check has to agree. + #[test] + fn plan_move_treats_a_null_and_an_empty_subpath_as_the_same_repo() { + let f = move_fixture(); + f.store + .create_project_repo(&store::ProjectRepo::new( + &f.target.id, + "acme/widgets", + "main", + Some(String::new()), + )) + .unwrap(); + + let err = plan_branch_move(&f.store, &f.branch.id, &f.target.id).unwrap_err(); + + assert!( + err.contains("already has acme/widgets"), + "unexpected error: {err}" + ); + } + + #[test] + fn plan_move_rejects_a_move_into_the_branchs_own_project() { + let f = move_fixture(); + + let err = plan_branch_move(&f.store, &f.branch.id, &f.source.id).unwrap_err(); + + assert!( + err.contains("already in that project"), + "unexpected error: {err}" + ); + } } diff --git a/apps/staged/src-tauri/src/git/mod.rs b/apps/staged/src-tauri/src/git/mod.rs index 23d61166..a0e8888d 100644 --- a/apps/staged/src-tauri/src/git/mod.rs +++ b/apps/staged/src-tauri/src/git/mod.rs @@ -50,9 +50,10 @@ pub use worktree::{ create_worktree_for_existing_branch_at_path, create_worktree_from_pr, create_worktree_from_pr_at_path, discard_worktree_changes, fetch_pr_head_sha, get_commits_since_base, get_full_commit_log, get_head_sha, get_parent_commit, - has_unpushed_commits, list_worktree_change_paths, list_worktrees, parse_branch_commit_line, - parse_worktree_status_paths, project_worktree_path_for, project_worktree_root_for, - remote_branch_exists, remove_worktree, reset_to_commit, set_upstream_to_origin, switch_branch, - update_branch_from_pr, worktree_path_for, BranchCommitFields, CommitInfo, UpdateFromPrResult, - WorktreeChangePaths, BRANCH_COMMIT_LOG_FORMAT, + has_unpushed_commits, list_worktree_change_paths, list_worktrees, move_worktree, + parse_branch_commit_line, parse_worktree_status_paths, project_worktree_path_for, + project_worktree_root_for, remote_branch_exists, remove_worktree, reset_to_commit, + set_upstream_to_origin, switch_branch, update_branch_from_pr, worktree_path_for, + BranchCommitFields, CommitInfo, UpdateFromPrResult, WorktreeChangePaths, + BRANCH_COMMIT_LOG_FORMAT, }; diff --git a/apps/staged/src-tauri/src/git/worktree.rs b/apps/staged/src-tauri/src/git/worktree.rs index a7cf975a..f1440ae1 100644 --- a/apps/staged/src-tauri/src/git/worktree.rs +++ b/apps/staged/src-tauri/src/git/worktree.rs @@ -184,6 +184,28 @@ pub fn create_worktree_for_existing_branch_at_path( Ok(worktree_path.to_path_buf()) } +/// Move an existing worktree to a new path. +/// +/// Goes through `git worktree move` rather than `fs::rename` because a +/// worktree is two pointers, not one directory: the `.git` gitfile inside the +/// worktree names the repo's admin directory, and that admin directory's +/// `gitdir` file names the worktree. A rename updates neither, leaving both +/// dangling; `git worktree move` rewrites both. +pub fn move_worktree(repo: &Path, from: &Path, to: &Path) -> Result<(), GitError> { + ensure_worktree_parent_exists(to)?; + ensure_worktree_absent(to)?; + + let from_str = from + .to_str() + .ok_or_else(|| GitError::InvalidPath(from.display().to_string()))?; + let to_str = to + .to_str() + .ok_or_else(|| GitError::InvalidPath(to.display().to_string()))?; + + cli::run(repo, &["worktree", "move", from_str, to_str])?; + Ok(()) +} + /// Remove a worktree and its associated branch. /// /// Removes the worktree directory, git worktree reference, and the local git branch. @@ -961,6 +983,39 @@ mod tests { assert!(paths.reset_required); } + /// The reason this goes through `git worktree move`: after the move the + /// worktree still has to be a working tree at its new path, which requires + /// both the gitfile and the repo's `gitdir` pointer to have been rewritten. + #[test] + fn move_worktree_leaves_a_working_tree_at_the_new_path() { + let repo = crate::test_utils::TempGitRepo::new(); + repo.write_file("tracked.txt", "base\n"); + repo.commit("base"); + + repo.run_git(&["branch", "feature"]); + + let name = repo.path().file_name().unwrap().to_str().unwrap(); + let parent = repo.path().parent().unwrap(); + let from = parent.join(format!("{name}-wt-from")); + let to = parent.join(format!("{name}-wt-to")); + create_worktree_for_existing_branch_at_path(repo.path(), "feature", &from).unwrap(); + + move_worktree(repo.path(), &from, &to).unwrap(); + + assert!(!from.exists()); + assert!(to.join("tracked.txt").exists()); + // A dangling gitfile or gitdir pointer fails both of these. + assert!(cli::run(&to, &["status", "--porcelain"]).is_ok()); + assert_eq!( + cli::run(&to, &["rev-parse", "--abbrev-ref", "HEAD"]) + .unwrap() + .trim(), + "feature" + ); + + let _ = std::fs::remove_dir_all(&to); + } + #[test] fn discard_worktree_changes_resets_tracked_and_removes_new_files() { let repo = crate::test_utils::TempGitRepo::new(); diff --git a/apps/staged/src-tauri/src/lib.rs b/apps/staged/src-tauri/src/lib.rs index cf460f89..1e7668b5 100644 --- a/apps/staged/src-tauri/src/lib.rs +++ b/apps/staged/src-tauri/src/lib.rs @@ -2229,6 +2229,7 @@ pub fn run() { branches::resume_workspace, branches::delete_branch, branches::rename_branch, + branches::move_branch, branches::get_blox_env, branches::get_workspace_info, branches::poll_workspace_status, diff --git a/apps/staged/src-tauri/src/store/branch_move.rs b/apps/staged/src-tauri/src/store/branch_move.rs new file mode 100644 index 00000000..838c6a3b --- /dev/null +++ b/apps/staged/src-tauri/src/store/branch_move.rs @@ -0,0 +1,707 @@ +//! Re-parenting a branch into a different project. +//! +//! Everything else a branch owns — commits, notes, reviews, sessions and their +//! messages, comments, reviewed files — hangs off `branch_id` and follows the +//! branch for free. Only four things carry a `project_id` of their own and have +//! to be rewritten in step with it: the branch row, its `project_repos` row, +//! its `workdirs` row, and its images. That rewrite is what lives here, in one +//! transaction, as `UPDATE`s and one `INSERT` only — the `AFTER DELETE` +//! triggers on the artifact tables garbage-collect sessions, so a +//! delete-and-reinsert would take transcripts with it. + +use rusqlite::{params, OptionalExtension}; + +use super::models::ProjectRepo; +use super::{now_timestamp, Store, StoreError}; + +/// Which `project_repos` row the moved branch points at once it lands. +/// +/// The relationship is N:1 — sibling branches can share one row — so the row +/// only travels when the moved branch is the last one on it. +#[derive(Debug, Clone)] +pub enum RepoPlacement { + /// Carry the branch's own row into the destination project. + Reparent { repo_id: String }, + /// Insert this clone in the destination and leave the source row for the + /// siblings still pointing at it. + Clone(ProjectRepo), +} + +impl RepoPlacement { + fn repo_id(&self) -> &str { + match self { + Self::Reparent { repo_id } => repo_id, + Self::Clone(repo) => &repo.id, + } + } +} + +/// The worktree relocation half of a move: which `workdirs` row to rewrite and +/// where to. Absent for a branch whose worktree was never set up. +#[derive(Debug, Clone)] +pub struct WorkdirMove { + pub workdir_id: String, + pub old_path: String, + pub new_path: String, +} + +/// Everything [`Store::move_branch_to_project`] rewrites, resolved by the +/// caller before any of it is applied. +#[derive(Debug, Clone)] +pub struct BranchMove { + pub branch_id: String, + pub source_project_id: String, + pub target_project_id: String, + pub repo: RepoPlacement, + pub workdir: Option, +} + +impl Store { + /// Move a branch into another project, rewriting every `project_id` that + /// points at the old one. + /// + /// The `branches(project_id, project_repo_id, branch_name)` and + /// `workdirs(project_id, path)` unique indexes are the backstop against a + /// destination that already holds this branch, so violations surface as + /// user-readable errors rather than raw SQLite text. + pub fn move_branch_to_project(&self, mv: &BranchMove) -> Result<(), StoreError> { + let mut conn = self.conn.lock().unwrap(); + let tx = conn.transaction()?; + let now = now_timestamp(); + + match &mv.repo { + RepoPlacement::Reparent { repo_id } => { + // Primary is re-elected per project below, so hand the row over + // unelected and let the destination decide. + tx.execute( + "UPDATE project_repos SET project_id = ?1, is_primary = 0, updated_at = ?2 + WHERE id = ?3", + params![mv.target_project_id, now, repo_id], + ) + .map_err(|e| duplicate_repo_error(&e))?; + } + RepoPlacement::Clone(repo) => { + tx.execute( + "INSERT INTO project_repos (id, project_id, github_repo, branch_name, subpath, + is_primary, reason, head_repo, created_at, updated_at) + VALUES (?1, ?2, ?3, ?4, ?5, 0, ?6, ?7, ?8, ?9)", + params![ + repo.id, + mv.target_project_id, + repo.github_repo, + repo.branch_name, + repo.subpath, + repo.reason, + repo.head_repo, + repo.created_at, + now, + ], + ) + .map_err(|e| duplicate_repo_error(&e))?; + } + } + + tx.execute( + "UPDATE branches SET project_id = ?1, project_repo_id = ?2, updated_at = ?3 + WHERE id = ?4", + params![mv.target_project_id, mv.repo.repo_id(), now, mv.branch_id], + ) + .map_err(|e| duplicate_branch_error(&e))?; + + if let Some(wd) = &mv.workdir { + tx.execute( + "UPDATE workdirs SET project_id = ?1, path = ?2, updated_at = ?3 WHERE id = ?4", + params![mv.target_project_id, wd.new_path, now, wd.workdir_id], + ) + .map_err(|e| duplicate_workdir_error(&e))?; + + // `sessions.working_dir` is an absolute-path snapshot taken when the + // session started, so it has to be rewritten by hand. Matching on + // the old worktree path rather than on the branch's artifacts is + // both narrower and wider in the right ways: a path under this + // worktree belongs to this branch and nothing else, and it catches + // sessions whose link to the branch runs through something this + // query would otherwise have to enumerate. The suffix is preserved + // so a session rooted at a repo subpath keeps it, separator and + // all — `substr` starts at the separator character itself. + let (old_slash, old_backslash) = session_dir_prefixes(&wd.old_path, cfg!(windows)); + tx.execute( + "UPDATE sessions + SET working_dir = ?1 || substr(working_dir, ?2), updated_at = ?3 + WHERE working_dir = ?4 + OR instr(working_dir, ?5) = 1 + OR instr(working_dir, ?6) = 1", + params![ + wd.new_path, + wd.old_path.chars().count() as i64 + 1, + now, + wd.old_path, + old_slash, + old_backslash, + ], + )?; + } + + // Branch-scoped images only: an image with no `branch_id` belongs to a + // project note and stays where it is. + tx.execute( + "UPDATE images SET project_id = ?1 WHERE branch_id = ?2", + params![mv.target_project_id, mv.branch_id], + )?; + + elect_primary_repo(&tx, &mv.source_project_id, now)?; + elect_primary_repo(&tx, &mv.target_project_id, now)?; + + tx.commit()?; + Ok(()) + } +} + +/// The prefixes a `sessions.working_dir` can start with when it lives under +/// `old_path`. Working dirs are stored via `Path::to_string_lossy`, so on +/// Windows the separator `PathBuf::join` inserts after the worktree root is +/// `\`, while a path that arrived as a string can still use `/`; both have to +/// match. On Unix the pair collapses to `/` alone: a backslash is an ordinary +/// filename character there, and matching it would move a sibling directory +/// that merely has one in its name. +fn session_dir_prefixes(old_path: &str, windows: bool) -> (String, String) { + let slash = format!("{old_path}/"); + let backslash = if windows { + format!("{old_path}\\") + } else { + slash.clone() + }; + (slash, backslash) +} + +/// Make sure a project has exactly one primary repo and that +/// `projects.github_repo`/`subpath` still denormalize it. +/// +/// Same shape as `remove_project_repo`'s re-election: the sitting primary keeps +/// the job, a project that just lost its primary promotes its oldest remaining +/// repo, and a project with no repos left denormalizes to NULL. +fn elect_primary_repo( + tx: &rusqlite::Transaction<'_>, + project_id: &str, + now: i64, +) -> Result<(), StoreError> { + let primary: Option<(String, String, Option)> = tx + .query_row( + "SELECT id, github_repo, subpath FROM project_repos + WHERE project_id = ?1 AND is_primary = 1 + ORDER BY created_at ASC LIMIT 1", + params![project_id], + |row| Ok((row.get(0)?, row.get(1)?, row.get(2)?)), + ) + .optional()?; + + let elected = match primary { + Some(existing) => Some(existing), + None => { + let next: Option<(String, String, Option)> = tx + .query_row( + "SELECT id, github_repo, subpath FROM project_repos + WHERE project_id = ?1 ORDER BY created_at ASC LIMIT 1", + params![project_id], + |row| Ok((row.get(0)?, row.get(1)?, row.get(2)?)), + ) + .optional()?; + if let Some((id, _, _)) = &next { + tx.execute( + "UPDATE project_repos SET is_primary = 1, updated_at = ?1 WHERE id = ?2", + params![now, id], + )?; + } + next + } + }; + + match elected { + Some((_, github_repo, subpath)) => tx.execute( + "UPDATE projects SET github_repo = ?1, subpath = ?2, updated_at = ?3 WHERE id = ?4", + params![github_repo, subpath, now, project_id], + )?, + None => tx.execute( + "UPDATE projects SET github_repo = NULL, subpath = NULL, updated_at = ?1 WHERE id = ?2", + params![now, project_id], + )?, + }; + Ok(()) +} + +/// Whether `err` is a constraint violation naming one of `needles`. +/// +/// The index or column name only comes back as message text, so the code has to +/// be checked too: without it any future error that happens to mention +/// `project_repos` would be rewritten into a confidently wrong explanation. +fn is_unique_violation(err: &rusqlite::Error, needles: &[&str]) -> bool { + match err { + rusqlite::Error::SqliteFailure(inner, Some(msg)) + if inner.code == rusqlite::ErrorCode::ConstraintViolation => + { + needles.iter().any(|needle| msg.contains(needle)) + } + _ => false, + } +} + +fn duplicate_repo_error(err: &rusqlite::Error) -> StoreError { + if is_unique_violation( + err, + &["idx_project_repos_unique", "project_repos.github_repo"], + ) { + return StoreError( + "The destination project already has this repository attached.".to_string(), + ); + } + StoreError(err.to_string()) +} + +fn duplicate_branch_error(err: &rusqlite::Error) -> StoreError { + if is_unique_violation(err, &["branches.branch_name"]) { + return StoreError( + "The destination project already tracks a branch with this name for this repository." + .to_string(), + ); + } + StoreError(err.to_string()) +} + +fn duplicate_workdir_error(err: &rusqlite::Error) -> StoreError { + if is_unique_violation(err, &["workdirs.path"]) { + return StoreError( + "The destination project already has a worktree at that path.".to_string(), + ); + } + StoreError(err.to_string()) +} + +#[cfg(test)] +mod tests { + use std::path::Path; + + use super::*; + use crate::store::models::{Branch, Image, Note, Project, Session, Workdir}; + + struct Fixture { + store: Store, + source: Project, + target: Project, + repo: ProjectRepo, + branch: Branch, + } + + /// A branch on its own `project_repos` row in `source`, plus an empty + /// `target` project to move it into. + fn fixture() -> Fixture { + let store = Store::in_memory().unwrap(); + let source = Project::named("source").with_primary_repo("acme/widgets"); + let target = Project::named("target"); + store.create_project(&source).unwrap(); + store.create_project(&target).unwrap(); + + let repo = ProjectRepo::new(&source.id, "acme/widgets", "feature", None).primary(); + store.create_project_repo(&repo).unwrap(); + + let branch = Branch::new(&source.id, "feature", "origin/main").with_project_repo(&repo.id); + store.create_branch(&branch).unwrap(); + + Fixture { + store, + source, + target, + repo, + branch, + } + } + + fn reparent(f: &Fixture, workdir: Option) -> BranchMove { + BranchMove { + branch_id: f.branch.id.clone(), + source_project_id: f.source.id.clone(), + target_project_id: f.target.id.clone(), + repo: RepoPlacement::Reparent { + repo_id: f.repo.id.clone(), + }, + workdir, + } + } + + #[test] + fn carries_the_branch_its_repo_row_and_its_images() { + let f = fixture(); + let image = Image::new( + Some(&f.branch.id), + &f.source.id, + "shot.png", + "image/png", + 12, + false, + ); + f.store.create_image(&image).unwrap(); + // A project-note image has no branch and must not follow. + let project_image = Image::new(None, &f.source.id, "note.png", "image/png", 12, false); + f.store.create_image(&project_image).unwrap(); + + f.store.move_branch_to_project(&reparent(&f, None)).unwrap(); + + let moved = f.store.get_branch(&f.branch.id).unwrap().unwrap(); + assert_eq!(moved.project_id, f.target.id); + assert_eq!(moved.project_repo_id.as_deref(), Some(f.repo.id.as_str())); + assert_eq!( + f.store + .get_project_repo(&f.repo.id) + .unwrap() + .unwrap() + .project_id, + f.target.id + ); + assert_eq!( + f.store.get_image(&image.id).unwrap().unwrap().project_id, + f.target.id + ); + assert_eq!( + f.store + .get_image(&project_image.id) + .unwrap() + .unwrap() + .project_id, + f.source.id + ); + } + + /// The point of moving via `UPDATE` rather than delete-and-reinsert: the + /// `AFTER DELETE` triggers would have taken the session transcript with it. + #[test] + fn keeps_the_branch_artifacts_and_their_sessions() { + let f = fixture(); + let session = Session::new_running("do the thing", Path::new("/tmp/wt")); + f.store.create_session(&session).unwrap(); + let note = Note::new(&f.branch.id, "Findings", "body").with_session(&session.id); + f.store.create_note(¬e).unwrap(); + + f.store.move_branch_to_project(&reparent(&f, None)).unwrap(); + + assert_eq!( + f.store.list_notes_for_branch(&f.branch.id).unwrap().len(), + 1 + ); + assert!(f.store.get_session(&session.id).unwrap().is_some()); + // The session resolves to the destination through the branch join. + assert_eq!( + f.store + .get_project_id_for_session(&session.id) + .unwrap() + .as_deref(), + Some(f.target.id.as_str()) + ); + } + + #[test] + fn rewrites_the_workdir_and_the_session_working_dirs_under_it() { + let f = fixture(); + let workdir = Workdir::new(&f.source.id, "/wt/source/acme-widgets--feature") + .with_branch(&f.branch.id); + f.store.create_workdir(&workdir).unwrap(); + + let at_root = Session::new_running("root", Path::new("/wt/source/acme-widgets--feature")); + let at_subpath = Session::new_running( + "sub", + Path::new("/wt/source/acme-widgets--feature/apps/web"), + ); + // A sibling path that merely shares the prefix's characters must not move. + let elsewhere = + Session::new_running("other", Path::new("/wt/source/acme-widgets--feature-two")); + for session in [&at_root, &at_subpath, &elsewhere] { + f.store.create_session(session).unwrap(); + } + + f.store + .move_branch_to_project(&reparent( + &f, + Some(WorkdirMove { + workdir_id: workdir.id.clone(), + old_path: workdir.path.clone(), + new_path: "/wt/target/acme-widgets--feature".to_string(), + }), + )) + .unwrap(); + + let moved = f.store.get_workdir(&workdir.id).unwrap().unwrap(); + assert_eq!(moved.project_id, f.target.id); + assert_eq!(moved.path, "/wt/target/acme-widgets--feature"); + + let working_dir = |id: &str| f.store.get_session(id).unwrap().unwrap().working_dir; + assert_eq!(working_dir(&at_root.id), "/wt/target/acme-widgets--feature"); + assert_eq!( + working_dir(&at_subpath.id), + "/wt/target/acme-widgets--feature/apps/web" + ); + assert_eq!( + working_dir(&elsewhere.id), + "/wt/source/acme-widgets--feature-two" + ); + } + + /// Windows records a `\` after the worktree root but can still hold `/` + /// in paths that arrived as strings; Unix must treat `\` as a filename + /// character, not a separator. + #[test] + fn session_prefixes_match_the_platforms_separators() { + assert_eq!( + session_dir_prefixes(r"C:\wt\x", true), + (r"C:\wt\x/".to_string(), r"C:\wt\x\".to_string()) + ); + assert_eq!( + session_dir_prefixes("/wt/x", false), + ("/wt/x/".to_string(), "/wt/x/".to_string()) + ); + } + + /// A sibling directory whose name happens to continue the worktree's with + /// a backslash is not under the worktree on Unix and must stay put. + #[cfg(not(windows))] + #[test] + fn leaves_a_backslash_named_sibling_alone_on_unix() { + let f = fixture(); + let workdir = Workdir::new(&f.source.id, "/wt/source/acme-widgets--feature") + .with_branch(&f.branch.id); + f.store.create_workdir(&workdir).unwrap(); + let sibling = + Session::new_running("other", Path::new(r"/wt/source/acme-widgets--feature\evil")); + f.store.create_session(&sibling).unwrap(); + + f.store + .move_branch_to_project(&reparent( + &f, + Some(WorkdirMove { + workdir_id: workdir.id.clone(), + old_path: workdir.path.clone(), + new_path: "/wt/target/acme-widgets--feature".to_string(), + }), + )) + .unwrap(); + + assert_eq!( + f.store + .get_session(&sibling.id) + .unwrap() + .unwrap() + .working_dir, + r"/wt/source/acme-widgets--feature\evil" + ); + } + + /// On Windows the session paths under the worktree continue with `\`, and + /// have to follow the move just as `/`-separated ones do. + #[cfg(windows)] + #[test] + fn rewrites_backslash_separated_session_paths_on_windows() { + let f = fixture(); + let workdir = Workdir::new(&f.source.id, r"C:\wt\source\acme-widgets--feature") + .with_branch(&f.branch.id); + f.store.create_workdir(&workdir).unwrap(); + let at_subpath = Session::new_running( + "sub", + Path::new(r"C:\wt\source\acme-widgets--feature\apps\web"), + ); + f.store.create_session(&at_subpath).unwrap(); + + f.store + .move_branch_to_project(&reparent( + &f, + Some(WorkdirMove { + workdir_id: workdir.id.clone(), + old_path: workdir.path.clone(), + new_path: r"C:\wt\target\acme-widgets--feature".to_string(), + }), + )) + .unwrap(); + + assert_eq!( + f.store + .get_session(&at_subpath.id) + .unwrap() + .unwrap() + .working_dir, + r"C:\wt\target\acme-widgets--feature\apps\web" + ); + } + + #[test] + fn clones_a_repo_row_that_sibling_branches_still_need() { + let f = fixture(); + let sibling = + Branch::new(&f.source.id, "other", "origin/main").with_project_repo(&f.repo.id); + f.store.create_branch(&sibling).unwrap(); + + let clone = ProjectRepo::new(&f.target.id, "acme/widgets", "feature", None); + f.store + .move_branch_to_project(&BranchMove { + repo: RepoPlacement::Clone(clone.clone()), + ..reparent(&f, None) + }) + .unwrap(); + + // The source row stays behind for the sibling. + let source_repo = f.store.get_project_repo(&f.repo.id).unwrap().unwrap(); + assert_eq!(source_repo.project_id, f.source.id); + assert_eq!( + f.store + .get_branch(&sibling.id) + .unwrap() + .unwrap() + .project_repo_id, + Some(f.repo.id.clone()) + ); + // …and the moved branch points at the clone in the destination. + let moved = f.store.get_branch(&f.branch.id).unwrap().unwrap(); + assert_eq!(moved.project_repo_id.as_deref(), Some(clone.id.as_str())); + assert_eq!( + f.store + .get_project_repo(&clone.id) + .unwrap() + .unwrap() + .project_id, + f.target.id + ); + } + + #[test] + fn promotes_the_arriving_repo_when_the_destination_had_none() { + let f = fixture(); + + f.store.move_branch_to_project(&reparent(&f, None)).unwrap(); + + let target_primary = f + .store + .get_primary_project_repo(&f.target.id) + .unwrap() + .unwrap(); + assert_eq!(target_primary.id, f.repo.id); + assert_eq!( + f.store + .get_project(&f.target.id) + .unwrap() + .unwrap() + .github_repo + .as_deref(), + Some("acme/widgets") + ); + // The source lost its only repo, so it denormalizes to NULL. + let source = f.store.get_project(&f.source.id).unwrap().unwrap(); + assert!(source.github_repo.is_none()); + assert!(f + .store + .get_primary_project_repo(&f.source.id) + .unwrap() + .is_none()); + } + + #[test] + fn leaves_the_destinations_own_primary_in_place() { + let f = fixture(); + let existing = ProjectRepo::new(&f.target.id, "acme/other", "main", None).primary(); + f.store.create_project_repo(&existing).unwrap(); + + f.store.move_branch_to_project(&reparent(&f, None)).unwrap(); + + assert_eq!( + f.store + .get_primary_project_repo(&f.target.id) + .unwrap() + .unwrap() + .id, + existing.id + ); + assert!( + !f.store + .get_project_repo(&f.repo.id) + .unwrap() + .unwrap() + .is_primary + ); + } + + #[test] + fn re_elects_the_sources_next_repo_when_the_primary_leaves() { + let f = fixture(); + let remaining = ProjectRepo::new(&f.source.id, "acme/other", "main", None); + f.store.create_project_repo(&remaining).unwrap(); + + f.store.move_branch_to_project(&reparent(&f, None)).unwrap(); + + assert_eq!( + f.store + .get_primary_project_repo(&f.source.id) + .unwrap() + .unwrap() + .id, + remaining.id + ); + assert_eq!( + f.store + .get_project(&f.source.id) + .unwrap() + .unwrap() + .github_repo + .as_deref(), + Some("acme/other") + ); + } + + /// The unique indexes are the backstop inside the transaction, and a + /// violation has to leave the branch where it was. + #[test] + fn rolls_back_when_the_destination_already_has_the_repo() { + let f = fixture(); + let conflict = ProjectRepo::new(&f.target.id, "acme/widgets", "main", None); + f.store.create_project_repo(&conflict).unwrap(); + + let err = f + .store + .move_branch_to_project(&reparent(&f, None)) + .unwrap_err(); + + assert!( + err.to_string().contains("already has this repository"), + "unexpected error: {err}" + ); + assert_eq!( + f.store + .get_branch(&f.branch.id) + .unwrap() + .unwrap() + .project_id, + f.source.id + ); + assert_eq!( + f.store + .get_project_repo(&f.repo.id) + .unwrap() + .unwrap() + .project_id, + f.source.id + ); + } + + /// Only a constraint violation may be rewritten into the friendly message: + /// the index name is matched in the error's text, so an unrelated failure + /// that happens to mention it has to keep its own words. + #[test] + fn explains_only_constraint_violations() { + let busy = rusqlite::Error::SqliteFailure( + rusqlite::ffi::Error::new(rusqlite::ffi::SQLITE_BUSY), + Some("database is locked writing idx_project_repos_unique".to_string()), + ); + let violation = rusqlite::Error::SqliteFailure( + rusqlite::ffi::Error::new(rusqlite::ffi::SQLITE_CONSTRAINT_UNIQUE), + Some("UNIQUE constraint failed: index 'idx_project_repos_unique'".to_string()), + ); + + assert!(duplicate_repo_error(&busy).to_string().contains("locked")); + assert!(duplicate_repo_error(&violation) + .to_string() + .contains("already has this repository")); + } +} diff --git a/apps/staged/src-tauri/src/store/images.rs b/apps/staged/src-tauri/src/store/images.rs index 0c59a1cd..bb279b66 100644 --- a/apps/staged/src-tauri/src/store/images.rs +++ b/apps/staged/src-tauri/src/store/images.rs @@ -53,6 +53,21 @@ impl Store { rows.collect::, _>>().map_err(Into::into) } + /// Every image row attached to a branch, session-scoped ones included. + /// + /// [`Self::list_images_for_branch`] is the timeline view and stops at + /// branch-level attachments; this is the whole set, for callers that have to + /// account for the files on disk (moving a branch between projects). + pub fn list_all_images_for_branch(&self, branch_id: &str) -> Result, StoreError> { + let conn = self.conn.lock().unwrap(); + let mut stmt = conn.prepare( + "SELECT id, branch_id, project_id, session_id, filename, mime_type, size_bytes, created_at + FROM images WHERE branch_id = ?1 ORDER BY created_at ASC", + )?; + let rows = stmt.query_map(params![branch_id], Self::row_to_image)?; + rows.collect::, _>>().map_err(Into::into) + } + /// Return a filename that is unique among images on the given branch (or project if no branch). /// If `filename` is already taken, appends ` 2`, ` 3`, … before the extension /// (e.g. `Screenshot.png` → `Screenshot 2.png`). diff --git a/apps/staged/src-tauri/src/store/mod.rs b/apps/staged/src-tauri/src/store/mod.rs index 024b1e5b..d1de4d8b 100644 --- a/apps/staged/src-tauri/src/store/mod.rs +++ b/apps/staged/src-tauri/src/store/mod.rs @@ -14,6 +14,7 @@ pub mod models; mod actions; +mod branch_move; mod branches; mod commits; pub mod images; @@ -37,6 +38,7 @@ mod migration_tests; mod tests; // Re-export all model types for backwards compatibility. +pub use branch_move::{BranchMove, RepoPlacement, WorkdirMove}; pub use models::*; pub use repo_badges::{fallback_short_name, next_hue}; diff --git a/apps/staged/src-tauri/src/web_server.rs b/apps/staged/src-tauri/src/web_server.rs index 55265eec..2a655977 100644 --- a/apps/staged/src-tauri/src/web_server.rs +++ b/apps/staged/src-tauri/src/web_server.rs @@ -1316,6 +1316,20 @@ async fn dispatch(command: &str, args: Value, state: &WebAppState) -> Result { + let store = get_store(store_mutex)?; + let branch_id: String = arg(&args, "branchId")?; + let target_project_id: String = arg(&args, "targetProjectId")?; + let moved = crate::branches::move_branch_impl( + &store, + action_executor, + action_registry, + &branch_id, + &target_project_id, + ) + .await?; + Ok(serde_json::to_value(moved).unwrap()) + } "get_blox_env" => Ok(serde_json::to_value(std::env::var("BLOX_ENV").ok()).unwrap()), // ===================================================================== // Workspace / Blox commands (Tier 3) diff --git a/apps/staged/src/lib/commands.ts b/apps/staged/src/lib/commands.ts index 1f50234d..c7cf3f9c 100644 --- a/apps/staged/src/lib/commands.ts +++ b/apps/staged/src/lib/commands.ts @@ -353,6 +353,24 @@ export function renameBranch(branchId: string, branchName: string): Promise { + const branch = await invokeCommand('move_branch', { branchId, targetProjectId }); + await Promise.all([ + invalidateCacheByCommand('list_projects'), + invalidateCacheByCommand('list_branches_for_project'), + invalidateCacheByCommand('list_project_repos'), + ]); + return branch; +} + /** Return the BLOX_ENV environment variable value, or null if unset. */ export function getBloxEnv(): Promise { return invokeCommand('get_blox_env'); diff --git a/apps/staged/src/lib/features/branches/BranchCard.svelte b/apps/staged/src/lib/features/branches/BranchCard.svelte index eb4aada9..649e1ecd 100644 --- a/apps/staged/src/lib/features/branches/BranchCard.svelte +++ b/apps/staged/src/lib/features/branches/BranchCard.svelte @@ -116,6 +116,7 @@ workspaceError?: string; onDelete?: () => void; onRename?: (branchName: string) => void | Promise; + onMove?: (targetProjectId: string) => void | Promise; onRetryWorktree?: () => void; } @@ -128,6 +129,7 @@ workspaceError, onDelete, onRename, + onMove, onRetryWorktree, }: Props = $props(); @@ -1927,6 +1929,7 @@ {remoteWorkspaceStatus} {onDelete} {onRename} + {onMove} onNoteCreated={() => loadTimeline()} onRebaseBranch={() => startBranchCommandPipeline('rebase')} onSquashCommits={() => startBranchCommandPipeline('squash')} diff --git a/apps/staged/src/lib/features/branches/BranchCardActionsBar.svelte b/apps/staged/src/lib/features/branches/BranchCardActionsBar.svelte index 7ded99ea..9984e99f 100644 --- a/apps/staged/src/lib/features/branches/BranchCardActionsBar.svelte +++ b/apps/staged/src/lib/features/branches/BranchCardActionsBar.svelte @@ -12,6 +12,7 @@ import { onMount, onDestroy } from 'svelte'; import GitBranch from '@lucide/svelte/icons/git-branch'; import Copy from '@lucide/svelte/icons/copy'; + import FolderInput from '@lucide/svelte/icons/folder-input'; import ExternalLink from '@lucide/svelte/icons/external-link'; import Trash2 from '@lucide/svelte/icons/trash-2'; import MoreVertical from '@lucide/svelte/icons/more-vertical'; @@ -31,6 +32,7 @@ import { agentState } from '../agents/agent.svelte'; import * as DropdownMenu from '$lib/components/ui/dropdown-menu'; import RenameBranchDialog from './RenameBranchDialog.svelte'; + import MoveBranchDialog from './MoveBranchDialog.svelte'; interface Props { branch: Branch; @@ -41,6 +43,7 @@ remoteWorkspaceStatus: string | null; onDelete?: () => void; onRename?: (branchName: string) => void | Promise; + onMove?: (targetProjectId: string) => void | Promise; onNoteCreated?: () => void; onRebaseBranch?: () => void; onSquashCommits?: () => void; @@ -59,6 +62,7 @@ remoteWorkspaceStatus, onDelete, onRename, + onMove, onNoteCreated, onRebaseBranch, onSquashCommits, @@ -95,6 +99,7 @@ }); let renameDialogOpen = $state(false); + let moveDialogOpen = $state(false); // More menu state let openerApps = $state([]); @@ -147,6 +152,10 @@ renameDialogOpen = true; } + function handleMoveFromMenu() { + moveDialogOpen = true; + } + const terminalAppIds = new Set([ 'terminal', 'warp', @@ -272,6 +281,11 @@ Rename Branch + {#if isLocal} + + Move to Project… + + {/if} onRebaseBranch?.()}> Rebase Branch @@ -290,6 +304,8 @@ + + + + + + + { + if (nextOpen) { + open = true; + } else { + requestClose(); + } + }} +> + + + + + + + + diff --git a/apps/staged/src/lib/features/branches/moveBranchTarget.test.ts b/apps/staged/src/lib/features/branches/moveBranchTarget.test.ts new file mode 100644 index 00000000..9615f0eb --- /dev/null +++ b/apps/staged/src/lib/features/branches/moveBranchTarget.test.ts @@ -0,0 +1,190 @@ +import { describe, expect, it } from 'vitest'; +import { + branchRepoIdentity, + filterMoveTargets, + isMoveTargetChecking, + moveTargetInvalidReason, + nextMoveTargetIndex, + repoKey, +} from './moveBranchTarget'; +import type { Project, ProjectRepo } from '../../types'; + +function project(overrides: Partial & { id: string; name: string }): Project { + return { + githubRepo: null, + location: 'local', + subpath: null, + createdAt: 0, + updatedAt: 0, + ...overrides, + }; +} + +function repo(overrides: Partial & { id: string; githubRepo: string }): ProjectRepo { + return { + projectId: 'p', + branchName: 'main', + subpath: null, + isPrimary: false, + reason: null, + headRepo: null, + createdAt: 0, + updatedAt: 0, + ...overrides, + }; +} + +describe('repoKey', () => { + it('treats a null and an empty subpath as the same repo, like the unique index', () => { + expect(repoKey('acme/widgets', null)).toBe(repoKey('acme/widgets', '')); + expect(repoKey('acme/widgets', 'apps/web')).not.toBe(repoKey('acme/widgets', null)); + }); +}); + +describe('branchRepoIdentity', () => { + it('prefers the branch’s own repo row', () => { + const identity = branchRepoIdentity( + repo({ id: 'r1', githubRepo: 'acme/widgets', subpath: 'apps/web' }), + project({ id: 'p1', name: 'Source', githubRepo: 'acme/other' }) + ); + + expect(identity).toEqual({ githubRepo: 'acme/widgets', subpath: 'apps/web' }); + }); + + it('falls back to the source project’s primary repo for a branch with no row', () => { + const identity = branchRepoIdentity( + null, + project({ id: 'p1', name: 'Source', githubRepo: 'acme/widgets', subpath: 'apps/web' }) + ); + + expect(identity).toEqual({ githubRepo: 'acme/widgets', subpath: 'apps/web' }); + }); + + it('has no identity when neither the row nor the project names a repo', () => { + expect(branchRepoIdentity(null, project({ id: 'p1', name: 'Empty' }))).toBeNull(); + }); +}); + +describe('filterMoveTargets', () => { + const alpha = project({ id: 'a', name: 'Alpha' }); + const beta = project({ id: 'b', name: 'Beta' }); + const repos = new Map([ + ['a', [repo({ id: 'r1', githubRepo: 'acme/widgets' })]], + ['b', [repo({ id: 'r2', githubRepo: 'other/gadgets', subpath: 'apps/web' })]], + ]); + + it('returns every candidate for an empty query', () => { + expect(filterMoveTargets([alpha, beta], repos, ' ')).toEqual([alpha, beta]); + }); + + it('matches on the project name', () => { + expect(filterMoveTargets([alpha, beta], repos, 'bet')).toEqual([beta]); + }); + + it('matches on an attached repo path, including its subpath', () => { + expect(filterMoveTargets([alpha, beta], repos, 'widgets')).toEqual([alpha]); + expect(filterMoveTargets([alpha, beta], repos, 'gadgets/apps/web')).toEqual([beta]); + }); + + it('matches nothing when neither the name nor a repo matches', () => { + expect(filterMoveTargets([alpha, beta], repos, 'zzz')).toEqual([]); + }); +}); + +describe('moveTargetInvalidReason', () => { + const branchRepo = { githubRepo: 'acme/widgets', subpath: null }; + + it('rejects a remote project, repos fetched or not', () => { + const remote = project({ id: 'r', name: 'Remote', location: 'remote' }); + + expect(moveTargetInvalidReason(remote, branchRepo, undefined)).toBe( + "Remote projects can't receive branches." + ); + expect(moveTargetInvalidReason(remote, branchRepo, [])).toBe( + "Remote projects can't receive branches." + ); + }); + + it('rejects a project that already has the branch’s repo', () => { + const target = project({ id: 't', name: 'Target' }); + + expect( + moveTargetInvalidReason(target, branchRepo, [repo({ id: 'r1', githubRepo: 'acme/widgets' })]) + ).toBe('Target already has acme/widgets attached.'); + }); + + it('names the subpath when the branch’s repo has one', () => { + const target = project({ id: 't', name: 'Target' }); + + expect( + moveTargetInvalidReason(target, { githubRepo: 'acme/widgets', subpath: 'apps/web' }, [ + repo({ id: 'r1', githubRepo: 'acme/widgets', subpath: 'apps/web' }), + ]) + ).toBe('Target already has acme/widgets (apps/web) attached.'); + }); + + it('counts an empty destination subpath as the same repo as a null one', () => { + const target = project({ id: 't', name: 'Target' }); + + expect( + moveTargetInvalidReason(target, branchRepo, [ + repo({ id: 'r1', githubRepo: 'acme/widgets', subpath: '' }), + ]) + ).toBe('Target already has acme/widgets attached.'); + }); + + it('accepts a project holding the same repo at a different subpath', () => { + const target = project({ id: 't', name: 'Target' }); + + expect( + moveTargetInvalidReason(target, branchRepo, [ + repo({ id: 'r1', githubRepo: 'acme/widgets', subpath: 'apps/web' }), + ]) + ).toBeNull(); + }); + + it('accepts a project with no overlapping repo', () => { + const target = project({ id: 't', name: 'Target' }); + + expect( + moveTargetInvalidReason(target, branchRepo, [repo({ id: 'r1', githubRepo: 'other/gadgets' })]) + ).toBeNull(); + }); +}); + +describe('nextMoveTargetIndex', () => { + it('starts at the first row when nothing is selected', () => { + expect(nextMoveTargetIndex(-1, 1, 3)).toBe(0); + expect(nextMoveTargetIndex(-1, -1, 3)).toBe(0); + }); + + it('steps within the list and stops at both ends', () => { + expect(nextMoveTargetIndex(0, 1, 3)).toBe(1); + expect(nextMoveTargetIndex(2, 1, 3)).toBe(2); + expect(nextMoveTargetIndex(1, -1, 3)).toBe(0); + expect(nextMoveTargetIndex(0, -1, 3)).toBe(0); + }); + + it('clamps a cursor left past the end by a narrowing query', () => { + expect(nextMoveTargetIndex(7, -1, 2)).toBe(1); + expect(nextMoveTargetIndex(7, 1, 2)).toBe(1); + }); + + it('selects nothing when the query matched nothing', () => { + expect(nextMoveTargetIndex(-1, 1, 0)).toBe(-1); + expect(nextMoveTargetIndex(3, -1, 0)).toBe(-1); + }); +}); + +describe('isMoveTargetChecking', () => { + it('waits on a local project whose repos have not been fetched', () => { + expect(isMoveTargetChecking(project({ id: 't', name: 'Target' }), undefined)).toBe(true); + expect(isMoveTargetChecking(project({ id: 't', name: 'Target' }), [])).toBe(false); + }); + + it('does not wait on a remote project — it is rejected either way', () => { + expect( + isMoveTargetChecking(project({ id: 'r', name: 'Remote', location: 'remote' }), undefined) + ).toBe(false); + }); +}); diff --git a/apps/staged/src/lib/features/branches/moveBranchTarget.ts b/apps/staged/src/lib/features/branches/moveBranchTarget.ts new file mode 100644 index 00000000..7f788418 --- /dev/null +++ b/apps/staged/src/lib/features/branches/moveBranchTarget.ts @@ -0,0 +1,107 @@ +/** + * Which projects a branch can move to, and why a chosen one can't take it. + * + * MoveBranchDialog's list and its disabled Move button both read from here, so + * the rules the backend enforces have exactly one frontend statement. + */ +import { matchesRepoSearch } from '../../shared/repoSearch'; +import { projectDisplayName } from '../../shared/utils'; +import type { Project, ProjectRepo } from '../../types'; + +/** The repo a branch travels with: `githubRepo` plus its optional subpath. */ +export interface BranchRepo { + githubRepo: string; + subpath: string | null; +} + +/** + * A repo's identity inside a project, keyed the way the backend's + * `idx_project_repos_unique` does — `(github_repo, COALESCE(subpath, ''))`, so a + * NULL subpath and an empty one are the same repo. + */ +export function repoKey(githubRepo: string, subpath: string | null | undefined): string { + return `${githubRepo}\x00${subpath ?? ''}`; +} + +/** + * The repo the branch moves with: its own `project_repos` row when it has one, + * else its project's denormalized primary — mirroring the backend's + * `resolve_branch_repo_slug` fallback. + */ +export function branchRepoIdentity( + repo: ProjectRepo | null | undefined, + sourceProject: Project | null | undefined +): BranchRepo | null { + if (repo) { + return { githubRepo: repo.githubRepo, subpath: repo.subpath }; + } + if (sourceProject?.githubRepo) { + return { githubRepo: sourceProject.githubRepo, subpath: sourceProject.subpath }; + } + return null; +} + +export function describeBranchRepo(repo: BranchRepo): string { + return repo.subpath ? `${repo.githubRepo} (${repo.subpath})` : repo.githubRepo; +} + +/** Match projects on their name or on any attached repo path. */ +export function filterMoveTargets( + candidates: Project[], + reposByProject: Map, + query: string +): Project[] { + const trimmed = query.trim(); + if (!trimmed) return candidates; + const lower = trimmed.toLowerCase(); + return candidates.filter((project) => { + if (projectDisplayName(project).toLowerCase().includes(lower)) return true; + return (reposByProject.get(project.id) ?? []).some((repo) => + matchesRepoSearch(repo.githubRepo, repo.subpath, trimmed) + ); + }); +} + +/** + * Why `target` can't receive the branch, or `null` when it can. + * + * `targetRepos` is `undefined` for a project whose repos haven't been fetched: + * the duplicate check would be a guess, so this returns `null` and callers gate + * on [`isMoveTargetChecking`] instead of letting the move through. + */ +export function moveTargetInvalidReason( + target: Project, + branchRepo: BranchRepo | null, + targetRepos: ProjectRepo[] | undefined +): string | null { + // Remote projects share one Blox workspace across their branches, so a move + // in would mean cross-workspace surgery on a filesystem we don't own. + if (target.location === 'remote') return "Remote projects can't receive branches."; + if (!targetRepos || !branchRepo) return null; + const key = repoKey(branchRepo.githubRepo, branchRepo.subpath); + if (targetRepos.some((repo) => repoKey(repo.githubRepo, repo.subpath) === key)) { + return `${projectDisplayName(target)} already has ${describeBranchRepo(branchRepo)} attached.`; + } + return null; +} + +/** + * The row an arrow key moves the keyboard cursor to, clamped into the list. + * + * `current` is derived from the selection rather than tracked alongside it, so + * it is `-1` while nothing is selected and can name a row the current query has + * filtered away; both have to land back inside the list rather than index off + * the end of it. Returns `-1` for an empty list, which selects nothing. + */ +export function nextMoveTargetIndex(current: number, delta: number, length: number): number { + if (length === 0) return -1; + return Math.min(Math.max(current + delta, 0), length - 1); +} + +/** Whether the target's repos still have to land before the move can be judged. */ +export function isMoveTargetChecking( + target: Project, + targetRepos: ProjectRepo[] | undefined +): boolean { + return target.location !== 'remote' && targetRepos === undefined; +} diff --git a/apps/staged/src/lib/features/projects/ProjectHome.svelte b/apps/staged/src/lib/features/projects/ProjectHome.svelte index 3eaa8965..ecce0caa 100644 --- a/apps/staged/src/lib/features/projects/ProjectHome.svelte +++ b/apps/staged/src/lib/features/projects/ProjectHome.svelte @@ -696,6 +696,26 @@ throw e; } } + + async function handleMoveBranch( + branchId: string, + sourceProjectId: string, + targetProjectId: string + ) { + try { + await commands.moveBranch(branchId, targetProjectId); + // A move reshapes both ends: the branch leaves one project's list and + // joins the other's, and each project's repos may have changed with it. + await Promise.all([ + projectsDataStore.refreshProject(sourceProjectId), + projectsDataStore.refreshProject(targetProjectId), + ]); + commands.invalidateBranchTimeline(branchId); + } catch (e) { + console.error('Failed to move branch:', e); + throw e; + } + } handleDeleteBranchRequest(branchId, project)} onRenameBranch={(branchId, branchName) => handleRenameBranch(branchId, project.id, branchName)} + onMoveBranch={(branchId, targetProjectId) => + handleMoveBranch(branchId, project.id, targetProjectId)} onProjectTitleElement={selectedProjectId ? setProjectTitleElement : undefined} onRepoSelected={(selection) => handleRepoSelected(project.id, selection)} onRetryWorktree={(branchId) => setupBranchWorktree(branchId, project.id)} diff --git a/apps/staged/src/lib/features/projects/ProjectRowContent.svelte b/apps/staged/src/lib/features/projects/ProjectRowContent.svelte new file mode 100644 index 00000000..3b1bf507 --- /dev/null +++ b/apps/staged/src/lib/features/projects/ProjectRowContent.svelte @@ -0,0 +1,227 @@ + + + +
+ {#if project.location === 'remote'} + + {:else if prStatus === 'merged'} + + {:else if prStatus === 'checks_failing'} + + {:else if prStatus === 'open'} + + {:else if prStatus === 'closed'} + + {:else if prStatus === 'conflict'} + + {:else if projectHasCodeChanges(branches)} + + {:else} + + {/if} +
+ {projectDisplayName(project)} +
+ {#if badges.length > 0} + + {#each badges as badge} + + {/each} + + {#if activity} + · + {activity} + {/if} + {:else} + {projectSubtitle(repoCount, sessionTypes, status.runActionPhase)} + {/if} +
+
+
+ + diff --git a/apps/staged/src/lib/features/projects/ProjectSection.svelte b/apps/staged/src/lib/features/projects/ProjectSection.svelte index d95e2b46..971d2267 100644 --- a/apps/staged/src/lib/features/projects/ProjectSection.svelte +++ b/apps/staged/src/lib/features/projects/ProjectSection.svelte @@ -59,6 +59,7 @@ workspaceErrors?: Map; onDeleteBranch?: (branchId: string) => void; onRenameBranch?: (branchId: string, branchName: string) => void | Promise; + onMoveBranch?: (branchId: string, targetProjectId: string) => void | Promise; onProjectTitleElement?: (element: HTMLHeadingElement | null) => void; onRepoSelected?: (selection: RepoPickerSelection) => void | Promise; onRetryWorktree?: (branchId: string) => void; @@ -74,6 +75,7 @@ workspaceErrors = new Map(), onDeleteBranch, onRenameBranch, + onMoveBranch, onProjectTitleElement, onRepoSelected, onRetryWorktree, @@ -532,6 +534,7 @@ workspaceError={workspaceErrors.get(branch.id)} onDelete={() => onDeleteBranch?.(branch.id)} onRename={(branchName) => onRenameBranch?.(branch.id, branchName)} + onMove={(targetProjectId) => onMoveBranch?.(branch.id, targetProjectId)} onRetryWorktree={() => onRetryWorktree?.(branch.id)} /> {/each} diff --git a/apps/staged/src/lib/features/projects/ProjectsSidebar.svelte b/apps/staged/src/lib/features/projects/ProjectsSidebar.svelte index 307d241a..c32756da 100644 --- a/apps/staged/src/lib/features/projects/ProjectsSidebar.svelte +++ b/apps/staged/src/lib/features/projects/ProjectsSidebar.svelte @@ -4,29 +4,14 @@ import { fade } from 'svelte/transition'; import House from '@lucide/svelte/icons/house'; import Plus from '@lucide/svelte/icons/plus'; - import Cloud from '@lucide/svelte/icons/cloud'; - import GitPullRequest from '@lucide/svelte/icons/git-pull-request'; - import GitPullRequestClosed from '@lucide/svelte/icons/git-pull-request-closed'; - import GitPullRequestDraft from '@lucide/svelte/icons/git-pull-request-draft'; import GitBranch from '@lucide/svelte/icons/git-branch'; - import Sprout from '@lucide/svelte/icons/sprout'; import FolderGit2 from '@lucide/svelte/icons/folder-git-2'; import Mail from '@lucide/svelte/icons/mail'; import Trash2 from '@lucide/svelte/icons/trash-2'; - import type { Project, WorkspaceStatus, RepoHomeItem } from '../../types'; + import type { RepoHomeItem } from '../../types'; import { goHome, navigation, selectProject, showAllRepos } from '../layout/navigation.svelte'; - import { - projectDisplayName, - aggregateProjectPrStatus, - projectHasCodeChanges, - projectSubtitle, - projectActivity, - } from '../../shared/utils'; - import RepoBadge from '../../shared/RepoBadge.svelte'; - import { repoBadgeStore } from '../../stores/repoBadges.svelte'; import { projectsDataStore } from '../../stores/projectsData.svelte'; import { projectRunActionsStore } from '../../stores/projectRunActions.svelte'; - import { projectStateStore } from '../../stores/projectState.svelte'; import Spinner from '../../shared/Spinner.svelte'; import SineWave from '../../shared/SineWave.svelte'; import StagedIcon from '../../shared/StagedIcon.svelte'; @@ -42,6 +27,7 @@ } from './projectsSidebarState.svelte'; import { viewport, watchViewport } from '../../shared/viewport.svelte'; import RepoCard from './RepoCard.svelte'; + import ProjectRowContent from './ProjectRowContent.svelte'; import SidebarFilterRow from './SidebarFilterRow.svelte'; import { projectFiltersStore } from './projectFilters.svelte'; import * as commands from '../../api/commands'; @@ -61,8 +47,6 @@ // state (width, scroll, drag) lives here or in projectsSidebarState. let projects = $derived(projectsDataStore.projects); let projectBranches = $derived(projectsDataStore.branchesByProject); - let reposByProject = $derived(projectsDataStore.reposByProject); - let repoCountsByProject = $derived(projectsDataStore.repoCountsByProject); let deletingProjectNames = $derived(projectsDataStore.deletingProjectNames); let loading = $derived(projectsDataStore.loading || !projectsDataStore.loaded); let error = $derived(projectsDataStore.error); @@ -287,37 +271,6 @@ } } - function repoCountForProject(project: Project): number { - return repoCountsByProject.get(project.id) ?? (project.githubRepo ? 1 : 0); - } - - function getProjectPrStatus( - projectId: string - ): 'merged' | 'open' | 'closed' | 'checks_failing' | 'conflict' | null { - const branches = projectBranches.get(projectId) || []; - return aggregateProjectPrStatus(branches); - } - - function getProjectWorkspaceStatus(projectId: string): WorkspaceStatus | null { - const branches = projectBranches.get(projectId) || []; - return branches.find((b) => b.workspaceStatus)?.workspaceStatus ?? null; - } - - function cloudStatusClass(status: WorkspaceStatus | null): string { - switch (status) { - case 'running': - return 'cloud-running'; - case 'starting': - return 'cloud-starting'; - case 'error': - return 'cloud-error'; - case 'stopped': - case 'suspended': - default: - return 'cloud-inactive'; - } - } - let resizing = $state(false); let resizeStartX = 0; let resizeStartWidth = SIDEBAR_DEFAULT_WIDTH; @@ -515,17 +468,6 @@ deletingProjectNames, projectBranches.get(project.id) || [] )} - {@const repoCount = repoCountForProject(project)} - {@const prStatus = getProjectPrStatus(project.id)} - {@const workspaceStatus = - project.location === 'remote' ? getProjectWorkspaceStatus(project.id) : null} - {@const sessionTypes = projectStateStore.getRunningSessionTypes(project.id)} - {@const repos = reposByProject.get(project.id) ?? []} - {@const badges = repos - .map((r) => repoBadgeStore.lookup(r.githubRepo, r.subpath)) - .filter((b): b is NonNullable => Boolean(b)) - .sort((a, b) => a.shortName.localeCompare(b.shortName))} - {@const activity = projectActivity(sessionTypes, status.runActionPhase)}