From c082309fe472979e4010f5314fa26ae55c3b6c81 Mon Sep 17 00:00:00 2001 From: weixiaoing <1537476031@qq.com> Date: Tue, 25 Aug 2026 22:18:37 +0800 Subject: [PATCH] fix: group imported worktree sessions under repository --- src-tauri/src/commands/conversations.rs | 121 +++++++++++++++++++++- src-tauri/src/commands/folders.rs | 130 ++++++++++++++++++++++-- 2 files changed, 236 insertions(+), 15 deletions(-) diff --git a/src-tauri/src/commands/conversations.rs b/src-tauri/src/commands/conversations.rs index 90641c2acb..d27e602843 100644 --- a/src-tauri/src/commands/conversations.rs +++ b/src-tauri/src/commands/conversations.rs @@ -859,14 +859,11 @@ pub(crate) async fn import_selected_from_summaries( }); let created = row.map(|r| r.deleted).unwrap_or(true); - // `add_folder` is the only fallible step here — `import_summaries` is + // Folder registration is the only fallible step here — `import_summaries` is // resilient (per-row failures are counted, never aborting the group), so // a partial failure still commits and reports its good rows and still // broadcasts the folder it created. - match folder_service::add_folder(conn, &target_path) - .await - .map_err(AppCommandError::from) - { + match crate::commands::folders::add_imported_folder_core(conn, &target_path).await { Ok(entry) => { let folder_id = entry.id; let (tally, _updated_ids, failed_in_group) = @@ -4899,6 +4896,27 @@ mod tests { } } + /// Run git with a self-contained identity so worktree tests never depend on + /// or modify the developer's global configuration. + fn git_run(dir: &std::path::Path, args: &[&str]) { + let output = std::process::Command::new("git") + .args(args) + .current_dir(dir) + .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("GIT_CONFIG_SYSTEM", "/dev/null") + .env("GIT_AUTHOR_NAME", "t") + .env("GIT_AUTHOR_EMAIL", "t@example.com") + .env("GIT_COMMITTER_NAME", "t") + .env("GIT_COMMITTER_EMAIL", "t@example.com") + .output() + .expect("spawn git"); + assert!( + output.status.success(), + "git {args:?} failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + } + #[test] fn scan_groups_normalized_path_variants_into_one_folder() { // A trailing-slash cwd variant must land in the same group as the bare @@ -5078,6 +5096,99 @@ mod tests { assert!(convs.iter().all(|c| c.folder_id == folder_rows[0].id)); } + #[tokio::test] + async fn batch_import_groups_existing_worktree_under_registered_root_idempotently() { + use sea_orm::EntityTrait; + + let db = fresh_in_memory_db().await; + let repo_dir = tempfile::tempdir().expect("tempdir"); + git_run(repo_dir.path(), &["init", "-q", "-b", "main"]); + git_run( + repo_dir.path(), + &["commit", "-q", "--allow-empty", "-m", "base"], + ); + let worktree_path = repo_dir.path().join("imported-wt"); + let worktree = worktree_path.to_str().expect("utf-8 path").to_string(); + git_run( + repo_dir.path(), + &["worktree", "add", "-q", "-b", "imported-wt", &worktree], + ); + + let root_path = repo_dir.path().to_str().expect("utf-8 path"); + // Git prints the canonical main-worktree path; register an equivalent + // non-canonical spelling to prove the parent lookup canonicalizes DB + // paths before matching. + let root_spelling = format!("{root_path}/."); + let root_id = seed_folder(&db, &root_spelling).await; + // Reproduce the pre-fix state: a previous import registered the linked + // worktree as a top-level folder because it used plain `add_folder`. + let legacy = folder_service::add_folder(&db.conn, &worktree) + .await + .expect("legacy worktree folder"); + assert_eq!( + folder_service::get_folder_by_id(&db.conn, legacy.id) + .await + .unwrap() + .unwrap() + .parent_id, + None + ); + + let summaries = || { + vec![scan_summary( + "worktree-session", + AgentType::Codex, + Some(&worktree), + at(0), + )] + }; + let selections = || vec![key_of(AgentType::Codex, "worktree-session")]; + + let first = import_selected_from_summaries( + &db.conn, + &EventEmitter::Noop, + summaries(), + selections(), + ) + .await + .expect("first import"); + assert_eq!(first.imported, 1); + assert_eq!(first.folders[0].folder_id, legacy.id, "row reconciled in place"); + assert_eq!( + folder_service::get_folder_by_id(&db.conn, legacy.id) + .await + .unwrap() + .unwrap() + .parent_id, + Some(root_id), + "linked worktree must group under the registered main worktree" + ); + + let second = import_selected_from_summaries( + &db.conn, + &EventEmitter::Noop, + summaries(), + selections(), + ) + .await + .expect("repeat import"); + assert_eq!(second.imported, 0); + assert_eq!(second.skipped, 1); + assert_eq!(second.folders[0].folder_id, legacy.id); + + let folders = crate::db::entities::folder::Entity::find() + .all(&db.conn) + .await + .unwrap(); + assert_eq!(folders.len(), 2, "repeat import must not duplicate folders"); + let conversations = conversation::Entity::find().all(&db.conn).await.unwrap(); + assert_eq!( + conversations.len(), + 1, + "repeat import must not duplicate the session" + ); + } + #[tokio::test] async fn batch_import_reuses_stored_path_for_trailing_slash_variant() { use sea_orm::EntityTrait; diff --git a/src-tauri/src/commands/folders.rs b/src-tauri/src/commands/folders.rs index 942796834a..c5a2e8714d 100644 --- a/src-tauri/src/commands/folders.rs +++ b/src-tauri/src/commands/folders.rs @@ -689,6 +689,65 @@ pub async fn open_folder_core( .ok_or_else(|| AppCommandError::not_found("Folder not found after add")) } +/// Register a folder discovered by the session importer, preserving git +/// worktree grouping when the repository's main worktree is already known. +/// +/// Import summaries only carry a cwd, not the source folder id passed by the +/// interactive worktree flows. Ask git for the repository's worktree list and, +/// when `path` is one of its linked worktrees, reuse the registered main +/// worktree as the source. Any git failure, ordinary repository, or unregistered +/// main worktree falls back to the historical top-level-folder behavior. +pub(crate) async fn add_imported_folder_core( + conn: &sea_orm::DatabaseConnection, + path: &str, +) -> Result { + if let Some(parent_id) = imported_worktree_parent(conn, path).await? { + return add_worktree_folder_with_parent(conn, path, Some(parent_id)).await; + } + + folder_service::add_folder(conn, path) + .await + .map_err(AppCommandError::from) +} + +/// Resolve the registered main-worktree folder for an imported linked +/// worktree. `git worktree list --porcelain` guarantees the main worktree is +/// listed first; matching the target against later entries keeps a normal repo +/// cwd top-level while still repairing a worktree row created by an older +/// importer with `parent_id = NULL`. +async fn imported_worktree_parent( + conn: &sea_orm::DatabaseConnection, + path: &str, +) -> Result, AppCommandError> { + let output = match crate::process::tokio_command("git") + .args(["worktree", "list", "--porcelain"]) + .current_dir(path) + .output() + .await + { + Ok(output) if output.status.success() => output, + _ => return Ok(None), + }; + + let worktrees = parse_worktrees(&String::from_utf8_lossy(&output.stdout)); + let Some((main_path, _)) = worktrees.first() else { + return Ok(None); + }; + let target = std::fs::canonicalize(path).unwrap_or_else(|_| PathBuf::from(path)); + let is_linked_worktree = worktrees.iter().skip(1).any(|(worktree_path, _)| { + std::fs::canonicalize(worktree_path).unwrap_or_else(|_| PathBuf::from(worktree_path)) + == target + }); + if !is_linked_worktree { + return Ok(None); + } + + let main = std::fs::canonicalize(main_path).unwrap_or_else(|_| PathBuf::from(main_path)); + Ok(folder_at_path(conn, &main) + .await? + .map(|folder| folder.parent_id.unwrap_or(folder.id))) +} + /// Open a freshly created git worktree directory as a folder, recording the /// *root* folder it descends from. Parents are flattened: a worktree created /// from another worktree still records the original root, so every worktree of a @@ -716,16 +775,28 @@ pub async fn open_worktree_folder_core( } else { None }; - let entry = folder_service::add_folder_with_parent(&db.conn, &path, parent_id) - .await - .map_err(AppCommandError::from)?; - seed_worktree_alias(db, entry.id, &path).await; + let entry = add_worktree_folder_with_parent(&db.conn, &path, parent_id).await?; folder_service::get_folder_by_id(&db.conn, entry.id) .await .map_err(AppCommandError::from)? .ok_or_else(|| AppCommandError::not_found("Folder not found after add")) } +/// Shared worktree registration path used by interactive creation and session +/// import. Keeping the authoritative parent write and branch-alias seeding here +/// makes re-registration idempotent and prevents the two entry points drifting. +async fn add_worktree_folder_with_parent( + conn: &sea_orm::DatabaseConnection, + path: &str, + parent_id: Option, +) -> Result { + let entry = folder_service::add_folder_with_parent(conn, path, parent_id) + .await + .map_err(AppCommandError::from)?; + seed_worktree_alias(conn, entry.id, path).await; + Ok(entry) +} + /// Default a worktree folder's alias to the branch checked out at `path`. /// /// Best-effort in both directions, and deliberately silent: @@ -739,11 +810,15 @@ pub async fn open_worktree_folder_core( /// - The write only lands when no alias is set yet (see /// [`folder_service::seed_folder_alias`]), so re-registering a worktree — a /// task retry, a re-created checkout — never overwrites a name the user chose. -async fn seed_worktree_alias(db: &AppDatabase, folder_id: i32, path: &str) { +async fn seed_worktree_alias( + conn: &sea_orm::DatabaseConnection, + folder_id: i32, + path: &str, +) { let Some(branch) = resolve_git_head(path).await.ok().and_then(|h| h.branch) else { return; }; - if let Err(e) = folder_service::seed_folder_alias(&db.conn, folder_id, &branch).await { + if let Err(e) = folder_service::seed_folder_alias(conn, folder_id, &branch).await { tracing::warn!("[folders] could not seed folder {folder_id}'s alias from git: {e}"); } } @@ -2865,7 +2940,7 @@ pub async fn resolve_worktree_folder_core( }; let canonical_wt = std::fs::canonicalize(&wt_path).unwrap_or_else(|_| PathBuf::from(&wt_path)); - let folder_id = folder_at_path(db, &canonical_wt).await?.map(|f| f.id); + let folder_id = folder_at_path(&db.conn, &canonical_wt).await?.map(|f| f.id); Ok(WorktreeResolution { path: Some(canonical_wt.to_string_lossy().to_string()), @@ -2879,10 +2954,10 @@ pub async fn resolve_worktree_folder_core( /// which is the one the rest of the app (and every conversation's `origin_cwd`) /// is written in terms of. async fn folder_at_path( - db: &AppDatabase, + conn: &sea_orm::DatabaseConnection, path: &Path, ) -> Result, AppCommandError> { - let folders = folder_service::list_all_folder_details(&db.conn) + let folders = folder_service::list_all_folder_details(conn) .await .map_err(AppCommandError::from)?; Ok(folders.into_iter().find(|f| { @@ -2981,7 +3056,7 @@ pub async fn git_remove_worktree_core( // Resolve the folder while the directory still exists — canonicalizing // its path afterwards would no longer match anything. - let wt_folder = folder_at_path(db, &canonical).await?; + let wt_folder = folder_at_path(&db.conn, &canonical).await?; // A to-do task mid-run or mid-merge is working inside this directory: // removing it (and `--force` would) yanks the tree out from under a live @@ -6866,6 +6941,41 @@ mod tests { ); } + #[tokio::test] + async fn add_imported_folder_core_keeps_non_worktree_paths_top_level() { + let db = fresh_in_memory_db().await; + let plain_dir = tempfile::tempdir().expect("plain tempdir"); + let plain = plain_dir.path().to_str().expect("utf-8 path"); + let plain_entry = add_imported_folder_core(&db.conn, plain) + .await + .expect("plain folder"); + assert_eq!( + folder_service::get_folder_by_id(&db.conn, plain_entry.id) + .await + .unwrap() + .unwrap() + .parent_id, + None, + "a non-git cwd keeps the legacy top-level behavior" + ); + + let repo_dir = tempfile::tempdir().expect("repo tempdir"); + git_run(repo_dir.path(), &["init", "-q", "-b", "main"]); + let repo = repo_dir.path().to_str().expect("utf-8 path"); + let repo_entry = add_imported_folder_core(&db.conn, repo) + .await + .expect("main worktree folder"); + assert_eq!( + folder_service::get_folder_by_id(&db.conn, repo_entry.id) + .await + .unwrap() + .unwrap() + .parent_id, + None, + "a repository's main worktree must remain top-level" + ); + } + #[tokio::test] async fn open_worktree_folder_core_flattens_nested_worktrees() { let db = fresh_in_memory_db().await;