diff --git a/src/authorship/rewrite.rs b/src/authorship/rewrite.rs index 14ff8e18ae..ab48de4c17 100644 --- a/src/authorship/rewrite.rs +++ b/src/authorship/rewrite.rs @@ -458,7 +458,31 @@ pub(crate) fn handle_non_fast_forward_rewrite_with_operation( onto: Option<&str>, operation: RewriteMetricOperation, ) -> Result { - let mappings = derive_mappings_from_range_diff(repo, old_tip, new_tip, onto)?; + handle_non_fast_forward_rewrite_with_additional_sources( + repo, + old_tip, + new_tip, + onto, + operation, + &[], + ) +} + +pub(crate) fn handle_non_fast_forward_rewrite_with_additional_sources( + repo: &Repository, + old_tip: &str, + new_tip: &str, + onto: Option<&str>, + operation: RewriteMetricOperation, + additional_sources: &[String], +) -> Result { + let mut mappings = derive_mappings_from_range_diff(repo, old_tip, new_tip, onto)?; + mappings.extend( + additional_sources + .iter() + .filter(|source| source.as_str() != new_tip) + .map(|source| (source.clone(), new_tip.to_string())), + ); if mappings.is_empty() { return Ok(RewriteOutcome::empty()); } @@ -476,6 +500,43 @@ pub(crate) fn handle_non_fast_forward_rewrite_with_operation( )) } +pub(crate) fn transplant_deltas_match( + repo: &Repository, + source_parent: &str, + source_commit: &str, + old_target: &str, + new_target: &str, +) -> Result { + fn exact_tree_diff( + repo: &Repository, + old_commit: &str, + new_commit: &str, + ) -> Result, GitAiError> { + let mut args = repo.global_args_for_exec(); + args.extend([ + "diff-tree".to_string(), + "-p".to_string(), + "-r".to_string(), + "--binary".to_string(), + "--full-index".to_string(), + "--no-commit-id".to_string(), + "--no-ext-diff".to_string(), + "--no-textconv".to_string(), + "--no-color".to_string(), + old_commit.to_string(), + new_commit.to_string(), + ]); + Ok(exec_git(&args)?.stdout) + } + + let source_delta = exact_tree_diff(repo, source_parent, source_commit)?; + if source_delta.is_empty() { + return Ok(false); + } + let target_delta = exact_tree_diff(repo, old_target, new_target)?; + Ok(source_delta == target_delta) +} + fn handle_squash_merge( repo: &Repository, source_head: &str, diff --git a/src/daemon.rs b/src/daemon.rs index 05c70745be..f9695f2123 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -2474,6 +2474,26 @@ struct PendingCherryPickNoCommit { head: String, } +#[derive(Debug, Clone)] +struct PendingGraphiteTransplant { + source_parent: String, + source_commit: String, +} + +fn is_graphite_temporary_commit(cmd: &crate::daemon::domain::NormalizedCommand) -> bool { + const MESSAGE: &str = "graphite (temporary): staged changes"; + + cmd.primary_command.as_deref() == Some("commit") + && (cmd + .invoked_args + .windows(2) + .any(|args| matches!(args[0].as_str(), "-m" | "--message") && args[1] == MESSAGE) + || cmd + .invoked_args + .iter() + .any(|arg| arg.strip_prefix("--message=") == Some(MESSAGE))) +} + #[derive(Debug, Clone)] #[allow(dead_code)] enum RecentReplayPrerequisite { @@ -2520,6 +2540,7 @@ pub struct ActorDaemonCoordinator { pending_cherry_pick_sources_by_worktree: Mutex>>, pending_cherry_pick_no_commit_by_worktree: Mutex>, pending_squash_merge_by_worktree: Mutex>, + pending_graphite_transplant_by_worktree: Mutex>, inflight_effects_by_family: Mutex>, /// Files with an in-flight AI edit (PreFileEdit received, PostFileEdit not yet completed). /// Outer key: family. Inner key: absolute file path string. Value: registration timestamp (nanos). @@ -2608,6 +2629,7 @@ impl ActorDaemonCoordinator { pending_cherry_pick_sources_by_worktree: Mutex::new(HashMap::new()), pending_cherry_pick_no_commit_by_worktree: Mutex::new(HashMap::new()), pending_squash_merge_by_worktree: Mutex::new(HashMap::new()), + pending_graphite_transplant_by_worktree: Mutex::new(HashMap::new()), inflight_effects_by_family: Mutex::new(HashMap::new()), pending_ai_edits_by_family: Mutex::new(HashMap::new()), family_sequencers_by_family: Mutex::new(HashMap::new()), @@ -2938,6 +2960,11 @@ impl ActorDaemonCoordinator { !pending.source_head.trim().is_empty() && !pending.onto.trim().is_empty() }); } + if let Ok(mut map) = self.pending_graphite_transplant_by_worktree.lock() { + map.retain(|_, pending| { + !pending.source_parent.trim().is_empty() && !pending.source_commit.trim().is_empty() + }); + } if let Ok(mut map) = self.queued_trace_payloads_by_root.lock() { map.retain(|_, count| *count > 0); } @@ -4623,6 +4650,55 @@ impl ActorDaemonCoordinator { Ok(map.remove(&Self::worktree_state_key(worktree))) } + fn set_pending_graphite_transplant_for_worktree( + &self, + worktree: &Path, + source_parent: String, + source_commit: String, + ) -> Result<(), GitAiError> { + let mut map = self + .pending_graphite_transplant_by_worktree + .lock() + .map_err(|_| { + GitAiError::Generic("pending Graphite transplant map lock poisoned".to_string()) + })?; + map.insert( + Self::worktree_state_key(worktree), + PendingGraphiteTransplant { + source_parent, + source_commit, + }, + ); + Ok(()) + } + + fn pending_graphite_transplant_for_worktree( + &self, + worktree: &Path, + ) -> Result, GitAiError> { + let map = self + .pending_graphite_transplant_by_worktree + .lock() + .map_err(|_| { + GitAiError::Generic("pending Graphite transplant map lock poisoned".to_string()) + })?; + Ok(map.get(&Self::worktree_state_key(worktree)).cloned()) + } + + fn clear_pending_graphite_transplant_for_worktree( + &self, + worktree: &Path, + ) -> Result<(), GitAiError> { + let mut map = self + .pending_graphite_transplant_by_worktree + .lock() + .map_err(|_| { + GitAiError::Generic("pending Graphite transplant map lock poisoned".to_string()) + })?; + map.remove(&Self::worktree_state_key(worktree)); + Ok(()) + } + fn resolve_heads_for_command( cmd: &crate::daemon::domain::NormalizedCommand, ) -> (String, String) { @@ -4780,6 +4856,11 @@ impl ActorDaemonCoordinator { .filter(|rc| is_valid_oid(&rc.new) && !is_zero_oid(&rc.new)) .map(|rc| rc.new.clone()) .next(); + let pending_graphite_transplant = if cmd.primary_command.as_deref() == Some("update-ref") { + self.pending_graphite_transplant_for_worktree(worktree)? + } else { + None + }; // If we have a pending original head from a failed rebase, use it as old_tip // with the branch ref update as new_tip. This handles rebase --skip/--continue @@ -4851,12 +4932,26 @@ impl ActorDaemonCoordinator { crate::authorship::rewrite::RewriteMetricOperation::Rebase, )? } else if cmd.primary_command.as_deref() == Some("update-ref") { - crate::authorship::rewrite::handle_non_fast_forward_rewrite_with_operation( + let additional_sources = if let Some(pending) = pending_graphite_transplant.as_ref() + && is_ancestor_commit(&repo, old_tip, &pending.source_parent) + && crate::authorship::rewrite::transplant_deltas_match( + &repo, + &pending.source_parent, + &pending.source_commit, + old_tip, + new_tip, + )? { + vec![pending.source_commit.clone()] + } else { + Vec::new() + }; + crate::authorship::rewrite::handle_non_fast_forward_rewrite_with_additional_sources( &repo, old_tip, new_tip, rewrite_onto.as_deref(), crate::authorship::rewrite::RewriteMetricOperation::UpdateRef, + &additional_sources, )? } else { crate::authorship::rewrite::handle_non_fast_forward_rewrite_with_operation( @@ -4885,6 +4980,9 @@ impl ActorDaemonCoordinator { ); } } + if cmd.primary_command.as_deref() == Some("update-ref") && !collapsed.is_empty() { + self.clear_pending_graphite_transplant_for_worktree(worktree)?; + } Ok(()) } @@ -5559,6 +5657,18 @@ impl ActorDaemonCoordinator { ) })?; + if is_graphite_temporary_commit(cmd) + && let Some(source_parent) = base + .as_ref() + .filter(|base| is_valid_oid(base) && !is_zero_oid(base)) + { + self.set_pending_graphite_transplant_for_worktree( + worktree.as_ref(), + source_parent.clone(), + new_head.clone(), + )?; + } + if cmd.primary_command.as_deref() == Some("commit") && let Some(pending) = self .take_pending_cherry_pick_no_commit_for_worktree( diff --git a/tests/integration/graphite.rs b/tests/integration/graphite.rs index 905291547f..64914b2415 100644 --- a/tests/integration/graphite.rs +++ b/tests/integration/graphite.rs @@ -560,7 +560,6 @@ fn test_gt_modify_restacks_children_preserves_attribution() { } #[test] -#[ignore = "fixed by stacked implementation PR"] fn test_gt_modify_into_downstack_preserves_attribution() { require_gt!(); let repo = TestRepo::new(); @@ -640,7 +639,6 @@ fn test_gt_modify_into_downstack_preserves_attribution() { } #[test] -#[ignore = "fixed by stacked implementation PR"] fn test_graphite_modify_into_plumbing_preserves_transplanted_attribution() { let repo = TestRepo::new(); setup_initial_commit(&repo); @@ -755,6 +753,86 @@ fn test_graphite_modify_into_plumbing_preserves_transplanted_attribution() { ]); } +#[test] +fn test_graphite_temporary_commit_does_not_attribute_a_different_rewrite() { + let repo = TestRepo::new(); + setup_initial_commit(&repo); + + repo.git(&["switch", "-c", "parent-branch"]).unwrap(); + let parent_path = repo.path().join("parent.txt"); + fs::write(&parent_path, "alpha\nbravo\ncharlie\n").unwrap(); + repo.git_ai(&["checkpoint", "mock_ai", "parent.txt"]) + .unwrap(); + repo.stage_all_and_commit("parent").unwrap(); + + let parent_base = repo + .git(&["rev-parse", "HEAD^"]) + .unwrap() + .trim() + .to_string(); + let mut parent_file = repo.filename("parent.txt"); + parent_file.assert_committed_lines(crate::lines!["alpha".ai(), "bravo".ai(), "charlie".ai(),]); + + // Prepare a different target tree before creating the Graphite temporary + // commit. Its delta must not receive the temporary commit's AI note. + repo.git(&["switch", "-c", "different-rewrite"]).unwrap(); + repo.git_ai(&["checkpoint", "human", "parent.txt"]).unwrap(); + fs::write(&parent_path, "alpha\ndifferent target rewrite\ncharlie\n").unwrap(); + repo.stage_all_and_commit("different rewrite").unwrap(); + let different_tree = repo + .git(&["rev-parse", "HEAD^{tree}"]) + .unwrap() + .trim() + .to_string(); + parent_file.assert_committed_lines(crate::lines![ + "alpha".ai(), + "different target rewrite".unattributed_human(), + "charlie".ai(), + ]); + + repo.git(&["switch", "parent-branch"]).unwrap(); + repo.git(&["switch", "-c", "child-branch"]).unwrap(); + repo.git_ai(&["checkpoint", "human", "parent.txt"]).unwrap(); + fs::write(&parent_path, "alpha\nAI transplant candidate\ncharlie\n").unwrap(); + repo.git_ai(&["checkpoint", "mock_ai", "parent.txt"]) + .unwrap(); + repo.git(&["add", "--all"]).unwrap(); + repo.git(&[ + "commit", + "-m", + "graphite (temporary): staged changes", + "--allow-empty", + ]) + .unwrap(); + parent_file.assert_committed_lines(crate::lines![ + "alpha".ai(), + "AI transplant candidate".ai(), + "charlie".ai(), + ]); + + let rewritten_parent = repo + .git(&[ + "commit-tree", + &different_tree, + "-p", + &parent_base, + "-m", + "parent", + ]) + .unwrap() + .trim() + .to_string(); + repo.git(&["update-ref", "refs/heads/parent-branch", &rewritten_parent]) + .unwrap(); + + repo.git(&["switch", "parent-branch"]).unwrap(); + parent_file.assert_committed_lines(crate::lines![ + "alpha".ai(), + "different target rewrite".unattributed_human(), + "charlie".ai(), + ]); +} + // =========================================================================== // Group 3: gt squash — Squash commits in branch // ===========================================================================