diff --git a/src/daemon.rs b/src/daemon.rs index 28755a10a..0032bd3db 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -4088,6 +4088,14 @@ impl ActorDaemonCoordinator { return Ok(None); }; + // Repo-creating commands (clone, init) have no meaningful carryover + // state — the target repo doesn't exist before the command runs, and the + // worktree hint may point to the CWD (a non-repo directory) rather than + // the newly created repo. + if matches!(command, "clone" | "init") { + return Ok(None); + } + let repo = discover_repository_in_path_no_git_exec(input.worktree)?; let stable_heads = stable_carryover_heads_for_command(&repo, &input, &parsed)?; diff --git a/src/daemon/trace_normalizer.rs b/src/daemon/trace_normalizer.rs index 1c0ca10c1..24a8185bd 100644 --- a/src/daemon/trace_normalizer.rs +++ b/src/daemon/trace_normalizer.rs @@ -585,6 +585,23 @@ impl TraceNormalizer { .and_then(|pending| argv_primary_command(&pending.raw_argv)) .is_some_and(|command| matches!(command.as_str(), "clone" | "init")); + // For clone/init the root process's def_repo carries the newly created + // repo path. Child processes (remote-https, index-pack, rev-list, …) + // inherit the parent CWD and their def_repo reports that CWD — not the + // clone destination. Once we've captured the root def_repo (first + // arrival), skip subsequent child def_repo events entirely to prevent + // overwriting the correct worktree, family, and sid lookup maps. + if prefer_def_repo_target { + let already_saw_def_repo = self + .state + .pending + .get(root_sid) + .is_some_and(|pending| pending.saw_def_repo); + if already_saw_def_repo { + return Ok(None); + } + } + // Trace2 `def_repo.repo` may point at a common-dir `.git` path for worktrees. // For normal in-repo commands we keep the start/cwd-derived worktree when available. // For clone/init the `def_repo` target is the repo we actually created and must win. @@ -921,10 +938,13 @@ impl TraceNormalizer { }; let mut candidates = Vec::new(); - if let Some(target) = target_from_argv.as_ref() { + // Prefer the def_repo target — it comes from git's own trace2 + // event and is always an absolute path. The argv-derived target + // may be relative and resolve against an unrelated ancestor repo. + if let Some(target) = target_from_def_repo.as_ref() { candidates.push(target.clone()); } - if let Some(target) = target_from_def_repo.as_ref() { + if let Some(target) = target_from_argv.as_ref() { let duplicate = candidates.iter().any(|existing| existing == target); if !duplicate { candidates.push(target.clone()); @@ -960,7 +980,7 @@ impl TraceNormalizer { if !resolved { // Keep the best available worktree hint even when family resolution fails. - if let Some(target) = target_from_argv.or(target_from_def_repo) { + if let Some(target) = target_from_def_repo.or(target_from_argv) { pending.worktree = Some(target); } if let Some((target, error)) = last_error { @@ -2122,6 +2142,69 @@ mod tests { ); } + #[test] + fn clone_child_def_repo_does_not_overwrite_root_worktree() { + // Real git trace2 output shows child processes (remote-https, index-pack) + // emit def_repo with the CWD as worktree, not the clone destination. + // The root process's def_repo has the correct newly-created repo path. + // Verify that child def_repo events don't clobber the root's worktree. + let backend = Arc::new(MockBackend::default()); + let mut normalizer = TraceNormalizer::new(backend); + let temp = tempfile::tempdir().expect("create tempdir"); + let cwd = temp.path().join("projects"); // non-repo CWD + let clone_dest = cwd.join("testing-git"); // the clone destination + fs::create_dir_all(clone_dest.join(".git")).expect("create clone git dir"); + + let root_sid = "20260327T000000.000000Z-Hdeadbeef-P00010000"; + let child_sid = format!("{}/20260327T000000.000001Z-Hdeadbeef-P00010001", root_sid); + + let start = serde_json::json!({ + "event": "start", + "sid": root_sid, + "ts": 1, + "argv": ["git", "clone", "https://github.com/svarlamov/testing-git"] + // No worktree or cwd — matches real trace2 start from non-repo dir + }); + // Root def_repo: correct clone destination + let root_def_repo = serde_json::json!({ + "event": "def_repo", + "sid": root_sid, + "ts": 2, + "worktree": clone_dest + }); + // Child def_repo from remote-https: reports CWD (parent), not destination + let child_def_repo = serde_json::json!({ + "event": "def_repo", + "sid": child_sid, + "ts": 3, + "worktree": cwd + }); + let exit = serde_json::json!({ + "event": "exit", + "sid": root_sid, + "ts": 4, + "code": 0 + }); + + assert!(normalizer.ingest_payload(&start).unwrap().is_none()); + assert!(normalizer.ingest_payload(&root_def_repo).unwrap().is_none()); + // Child def_repo must NOT overwrite the root worktree + assert!( + normalizer + .ingest_payload(&child_def_repo) + .unwrap() + .is_none() + ); + + let cmd = normalizer.ingest_payload(&exit).unwrap().unwrap(); + assert_eq!(cmd.primary_command.as_deref(), Some("clone")); + assert_eq!( + cmd.worktree.as_ref(), + Some(&clone_dest), + "clone worktree should be the destination, not the parent CWD" + ); + } + #[test] fn no_repo_routes_to_global_scope() { let backend = Arc::new(MockBackend::default()); diff --git a/tests/notes_sync_regression.rs b/tests/notes_sync_regression.rs index 9f8325090..98afafc15 100644 --- a/tests/notes_sync_regression.rs +++ b/tests/notes_sync_regression.rs @@ -248,6 +248,166 @@ worktree_test_wrappers! { } } +// Regression test: clone with an absolute target path from a non-repo CWD. +// Exercises the side-effect target resolution path where the clone target is +// specified as an absolute path (common in scripted / CI workflows and when +// the user types `git clone URL /some/absolute/path`). +worktree_test_wrappers! { + fn notes_sync_clone_absolute_target_from_non_repo_cwd_fetches_authorship_notes() { + if TestRepo::git_mode() == GitTestMode::Hooks { + return; + } + + let (local, upstream) = TestRepo::new_with_remote(); + + fs::write(local.path().join("abs-clone-seed.txt"), "seed\n") + .expect("failed to write seed file"); + local + .git_og(&["add", "abs-clone-seed.txt"]) + .expect("add should succeed"); + local + .git_og(&["commit", "-m", "seed commit"]) + .expect("seed commit should succeed"); + + let seed_sha = local + .git_og(&["rev-parse", "HEAD"]) + .expect("rev-parse should succeed") + .trim() + .to_string(); + + local + .git_og(&[ + "notes", + "--ref=ai", + "add", + "-m", + "abs-clone-note", + seed_sha.as_str(), + ]) + .expect("adding notes should succeed"); + local + .git_og(&["push", "-u", "origin", "HEAD"]) + .expect("pushing branch should succeed"); + local + .git_og(&["push", "origin", "refs/notes/ai"]) + .expect("pushing notes should succeed"); + + // Clone from a non-repo directory using an absolute target path. + let external_cwd = unique_temp_path("notes-sync-abs-target-cwd"); + let _ = fs::remove_dir_all(&external_cwd); + fs::create_dir_all(&external_cwd).expect("failed to create external cwd"); + + let clone_dir = unique_temp_path("notes-sync-abs-target-clone"); + let _ = fs::remove_dir_all(&clone_dir); + let clone_dir_str = clone_dir.to_string_lossy().to_string(); + let upstream_str = upstream.path().to_string_lossy().to_string(); + + local + .git_from_working_dir( + &external_cwd, + &["clone", upstream_str.as_str(), clone_dir_str.as_str()], + ) + .expect("clone with absolute target should succeed"); + + assert!( + clone_dir.exists(), + "clone target should exist at {}", + clone_dir.display() + ); + + let cloned_note = read_note_from_worktree(&clone_dir, &seed_sha, TestRepo::git_mode()); + assert!( + cloned_note.is_some(), + "cloned repository should have fetched authorship notes for commit {} (absolute target from non-repo CWD)", + seed_sha + ); + } +} + +// Regression test: clone with NO explicit target directory (implicit target +// derived from the source URL/path). This is the common user scenario: +// cd ~/projects && git clone https://github.com/user/repo +// In trace2, the root process emits def_repo with the correct clone destination, +// but child processes (remote-https, index-pack) emit def_repo with the CWD as +// worktree. The normalizer must prefer the root def_repo and ignore children. +worktree_test_wrappers! { + fn notes_sync_clone_implicit_target_from_non_repo_cwd_fetches_authorship_notes() { + if TestRepo::git_mode() == GitTestMode::Hooks { + return; + } + + let (local, upstream) = TestRepo::new_with_remote(); + + fs::write(local.path().join("implicit-seed.txt"), "seed\n") + .expect("failed to write seed file"); + local + .git_og(&["add", "implicit-seed.txt"]) + .expect("add should succeed"); + local + .git_og(&["commit", "-m", "seed commit"]) + .expect("seed commit should succeed"); + + let seed_sha = local + .git_og(&["rev-parse", "HEAD"]) + .expect("rev-parse should succeed") + .trim() + .to_string(); + + local + .git_og(&[ + "notes", + "--ref=ai", + "add", + "-m", + "implicit-clone-note", + seed_sha.as_str(), + ]) + .expect("adding notes should succeed"); + local + .git_og(&["push", "-u", "origin", "HEAD"]) + .expect("pushing branch should succeed"); + local + .git_og(&["push", "origin", "refs/notes/ai"]) + .expect("pushing notes should succeed"); + + // Clone from a non-repo CWD with NO explicit target — git derives the + // directory name from the source path (the upstream bare repo's basename). + let external_cwd = unique_temp_path("notes-sync-clone-implicit-cwd"); + let _ = fs::remove_dir_all(&external_cwd); + fs::create_dir_all(&external_cwd).expect("failed to create external cwd"); + + let upstream_str = upstream.path().to_string_lossy().to_string(); + // Derive the expected directory name the same way git does: basename of the source. + let expected_dir_name = Path::new(&upstream_str) + .file_name() + .expect("upstream path should have a filename") + .to_string_lossy() + .to_string(); + // Strip .git suffix if present (matches git's behavior) + let expected_dir_name = expected_dir_name + .strip_suffix(".git") + .unwrap_or(&expected_dir_name); + + local + .git_from_working_dir(&external_cwd, &["clone", upstream_str.as_str()]) + .expect("clone with implicit target should succeed"); + + let clone_dir = external_cwd.join(expected_dir_name); + assert!( + clone_dir.exists(), + "implicit clone target should exist at {}", + clone_dir.display() + ); + + let cloned_note = read_note_from_worktree(&clone_dir, &seed_sha, TestRepo::git_mode()); + assert!( + cloned_note.is_some(), + "cloned repository should have fetched authorship notes for commit {} (implicit target from non-repo CWD)", + seed_sha + ); + } +} + worktree_test_wrappers! { fn notes_sync_fetch_does_not_import_authorship_notes() { let mode = TestRepo::git_mode();