diff --git a/src/authorship/rewrite.rs b/src/authorship/rewrite.rs index 5d9235c50b..c61a3e97ef 100644 --- a/src/authorship/rewrite.rs +++ b/src/authorship/rewrite.rs @@ -992,7 +992,24 @@ fn derive_mappings_from_range_diff( } _ => &base, }; - let range_diff_output = run_range_diff(repo, &base, old_tip, onto, new_tip)?; + let commit_limit = range_diff_commit_limit(); + let old_count = range_commit_count_up_to(repo, &base, old_tip, commit_limit); + let bounded_new_base = bounded_tip_range_base(repo, onto, new_tip, commit_limit); + let bounded_new_base = match (old_count, bounded_new_base) { + (Some(old), Some(new_base)) if old <= commit_limit => new_base, + _ => { + tracing::warn!( + old_tip, + new_tip, + old_count, + commit_limit, + "skipping range-diff mapping derivation because its inputs cannot be bounded" + ); + return Ok(Vec::new()); + } + }; + + let range_diff_output = run_range_diff(repo, &base, old_tip, &bounded_new_base, new_tip)?; let mut mappings = parse_range_diff_output(&range_diff_output); let merge_mappings = derive_merge_commit_mappings(repo, &base, old_tip, new_tip, &mappings)?; @@ -1046,6 +1063,71 @@ pub(crate) fn list_commits_in_range(repo: &Repository, base: &str, tip: &str) -> .unwrap_or_default() } +/// `git range-diff` retains every commit's patch while calculating its +/// pairwise matching costs. Bounding each input side prevents a divergent +/// ref move from making the daemon spawn a multi-GB range-diff child. +const MAX_RANGE_DIFF_COMMITS: u64 = 1000; + +fn range_diff_commit_limit() -> u64 { + #[cfg(feature = "test-support")] + if let Ok(raw) = std::env::var("GIT_AI_TEST_RANGE_DIFF_COMMIT_LIMIT") + && let Ok(limit) = raw.parse() + { + return limit; + } + + MAX_RANGE_DIFF_COMMITS +} + +/// Count at most `limit + 1` commits so the preflight itself has bounded work. +/// This is called once per range-diff side: two constant git spawns total, +/// independent of the number of commits in either range. +fn range_commit_count_up_to(repo: &Repository, base: &str, tip: &str, limit: u64) -> Option { + let mut args = repo.global_args_for_exec(); + args.extend([ + "rev-list".to_string(), + "--count".to_string(), + format!("--max-count={}", limit.saturating_add(1)), + format!("{}..{}", base, tip), + ]); + let output = exec_git_allow_nonzero(&args).ok()?; + if !output.status.success() { + return None; + } + String::from_utf8_lossy(&output.stdout).trim().parse().ok() +} + +/// Return a base that keeps `base..tip` at or below `limit` commits. If the +/// original range is larger, retain its newest bounded suffix. This preserves +/// rewritten stack commits at the tip while excluding long upstream history +/// that may be present when no precise landing hint is available. +fn bounded_tip_range_base(repo: &Repository, base: &str, tip: &str, limit: u64) -> Option { + let mut args = repo.global_args_for_exec(); + args.extend([ + "rev-list".to_string(), + "--topo-order".to_string(), + format!("--max-count={}", limit.saturating_add(1)), + format!("{}..{}", base, tip), + ]); + let output = exec_git_allow_nonzero(&args).ok()?; + if !output.status.success() { + return None; + } + + let output = String::from_utf8_lossy(&output.stdout); + let mut commits = output.lines(); + let bounded_base = commits.nth(limit.try_into().ok()?); + let Some(bounded_base) = bounded_base else { + return Some(base.to_string()); + }; + + matches!( + range_commit_count_up_to(repo, bounded_base, tip, limit), + Some(count) if count <= limit + ) + .then(|| bounded_base.to_string()) +} + fn run_range_diff( repo: &Repository, old_base: &str, @@ -1067,16 +1149,12 @@ fn run_range_diff( Ok(String::from_utf8_lossy(&output.stdout).trim().to_string()) } -/// Maximum number of unmatched old-range (`<`) commits buffered ahead of the -/// first matched pair. A genuine rebase squash absorbs a handful of commits -/// into the first surviving one, so small queues are drained into that match -/// as before. But when the count exceeds this bound the history is divergent -/// — e.g. a restack undo (`git reset --keep `), where the old -/// range spans every trunk commit landed since the branch forked — and the -/// whole queue is discarded, permanently. Without this, each bogus mapping -/// whose source commit has an authorship note becomes a full-root-tree diff -/// pair, and daemon memory scales as -/// (trunk commits since fork) × (lines modified on trunk). +/// Maximum number of consecutive unmatched old-range (`<`) commits that may +/// be treated as a squash into an adjacent matched commit. This applies both +/// before the first match and after a match: exceeding it discards the whole +/// fabricated run while retaining exact `=`/`!` mappings. Without the +/// post-match bound, moving a branch away from many commits on top of its +/// matched stack fabricated one full-root-tree diff pair per dropped commit. const MAX_PENDING_DROPPED_COMMITS: usize = 64; fn parse_range_diff_output(output: &str) -> Vec<(String, String)> { @@ -1105,19 +1183,14 @@ fn parse_range_diff_output(output: &str) -> Vec<(String, String)> { match status_char { '<' => { // Dropped commit (squashed into a later commit) - if !old_sha.chars().all(|c| c == '0') { - if let Some(new_sha) = previous_new_sha.as_ref() { - mappings.push((old_sha, new_sha.clone())); - } else if !pending_overflowed { - pending_dropped.push(old_sha); - if pending_dropped.len() > MAX_PENDING_DROPPED_COMMITS { - // Divergent history (e.g. restack undo), not a - // squash: discard the queue and stop collecting so - // the mapping count stays bounded. - pending_dropped.clear(); - pending_dropped.shrink_to_fit(); - pending_overflowed = true; - } + if !old_sha.chars().all(|c| c == '0') && !pending_overflowed { + pending_dropped.push(old_sha); + if pending_dropped.len() > MAX_PENDING_DROPPED_COMMITS { + // Divergent history, not a squash: discard the entire + // run and stop collecting until the next exact match. + pending_dropped.clear(); + pending_dropped.shrink_to_fit(); + pending_overflowed = true; } } } @@ -1130,10 +1203,14 @@ fn parse_range_diff_output(output: &str) -> Vec<(String, String)> { if old_sha.chars().all(|c| c == '0') || new_sha.chars().all(|c| c == '0') { continue; } - // Map any preceding dropped commits to this new commit (squash) + // Leading drops squash into this match. Drops after a prior + // match squash into that prior destination, preserving the + // parser's existing direction for small runs. + let dropped_destination = previous_new_sha.as_ref().unwrap_or(&new_sha); for dropped in pending_dropped.drain(..) { - mappings.push((dropped, new_sha.clone())); + mappings.push((dropped, dropped_destination.clone())); } + pending_overflowed = false; previous_new_sha = Some(new_sha.clone()); mappings.push((old_sha, new_sha)); } @@ -1144,6 +1221,12 @@ fn parse_range_diff_output(output: &str) -> Vec<(String, String)> { } } + if !pending_overflowed && let Some(new_sha) = previous_new_sha { + for dropped in pending_dropped { + mappings.push((dropped, new_sha.clone())); + } + } + mappings } @@ -1821,6 +1904,26 @@ Binary files a/image.png and b/image.png differ ); } + #[test] + fn test_parse_range_diff_output_discards_many_drops_after_match() { + let old_matched = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa".to_string(); + let new_matched = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string(); + let mut output = format!("1: {old_matched} = 1: {new_matched} matched\n"); + for i in 1..=MAX_PENDING_DROPPED_COMMITS + 1 { + let sha = format!("{i:040}"); + output.push_str(&format!( + "{}: {} < -: ---------------------------------------- dropped {}\n", + i + 1, + sha, + i + )); + } + + let mappings = parse_range_diff_output(&output); + + assert_eq!(mappings, vec![(old_matched, new_matched)]); + } + #[test] fn test_parse_range_diff_output_divergent_history_discards_pending() { // Regression: restack-undo (moving a branch tip backward to undo a diff --git a/tests/integration/rebase.rs b/tests/integration/rebase.rs index 2010c050b0..5e3e1f2e3b 100644 --- a/tests/integration/rebase.rs +++ b/tests/integration/rebase.rs @@ -2860,6 +2860,7 @@ fn test_rebase_same_file_then_ff_merge_preserves_attribution() { } const COMMITS_OVER_PENDING_DROPPED_LIMIT: usize = 200; +const COMMITS_JUST_OVER_PENDING_DROPPED_LIMIT: usize = 65; fn numbered_file_contents(prefix: &str, count: usize) -> String { (1..=count) @@ -2895,6 +2896,48 @@ fn leading_dropped_commits_before_first_match(range_diff: &str) -> usize { dropped } +fn dropped_commits_after_first_match(range_diff: &str) -> usize { + let mut saw_match = false; + let mut dropped = 0; + + for line in range_diff.lines() { + let mut parts = line.split_whitespace(); + let (_ordinal, _old_sha, Some(status)) = (parts.next(), parts.next(), parts.next()) else { + continue; + }; + + match status { + "=" | "!" => saw_match = true, + "<" if saw_match => dropped += 1, + _ => {} + } + } + + dropped +} + +fn shift_authorship_mapping_counts(daemon_log: &str) -> Vec { + daemon_log + .lines() + .filter_map(|line| { + line.split_once("shift_authorship_notes: ")? + .1 + .split_whitespace() + .next()? + .parse() + .ok() + }) + .collect() +} + +fn range_diff_spawn_count(spawn_log_path: &std::path::Path) -> usize { + std::fs::read_to_string(spawn_log_path) + .unwrap_or_default() + .lines() + .filter(|command| *command == "range-diff") + .count() +} + #[test] fn test_rebase_more_commits_than_pending_dropped_limit_preserves_authorship() { use std::fs; @@ -3038,6 +3081,234 @@ fn test_reset_keep_undo_rebase_discards_many_old_upstream_commits() { ); } +#[test] +fn test_reset_keep_move_discards_many_commits_after_matched_stack() { + use std::fs; + + let repo = TestRepo::new_with_daemon_env(&[("GIT_AI_DEBUG", "1")]); + + let mut base_file = repo.filename("base.txt"); + base_file.set_contents(crate::lines!["base"]); + repo.stage_all_and_commit("Initial commit").unwrap(); + let base_sha = repo.git(&["rev-parse", "HEAD"]).unwrap().trim().to_string(); + + repo.git(&["checkout", "-b", "feature"]).unwrap(); + let feature_path = repo.path().join("feature.txt"); + fs::write(&feature_path, "ai feature line\n").unwrap(); + repo.git_ai(&["checkpoint", "mock_ai", "feature.txt"]) + .unwrap(); + repo.git(&["add", "feature.txt"]).unwrap(); + repo.commit("feature commit").unwrap(); + let feature_commit = repo.git(&["rev-parse", "HEAD"]).unwrap().trim().to_string(); + let mut feature_file = repo.filename("feature.txt"); + feature_file.assert_committed_lines(crate::lines!["ai feature line".ai()]); + + repo.git(&["checkout", "-b", "feature-prime", &base_sha]) + .unwrap(); + repo.git(&["cherry-pick", &feature_commit]).unwrap(); + repo.git(&[ + "commit", + "--amend", + "-m", + "feature commit moved to new parent", + ]) + .unwrap(); + let feature_prime_tip = repo.git(&["rev-parse", "HEAD"]).unwrap().trim().to_string(); + feature_file.assert_committed_lines(crate::lines!["ai feature line".ai()]); + + repo.git(&["checkout", "feature"]).unwrap(); + let dropped_path = repo.path().join("dropped.txt"); + for commit_number in 1..=COMMITS_JUST_OVER_PENDING_DROPPED_LIMIT { + fs::write( + &dropped_path, + numbered_file_contents("ai dropped line", commit_number), + ) + .unwrap(); + repo.git_ai(&["checkpoint", "mock_ai", "dropped.txt"]) + .unwrap(); + repo.git(&["add", "dropped.txt"]).unwrap(); + repo.commit(&format!("dropped commit {commit_number}")) + .unwrap(); + } + let feature_tip = repo.git(&["rev-parse", "HEAD"]).unwrap().trim().to_string(); + let mut dropped_file = repo.filename("dropped.txt"); + dropped_file.assert_committed_lines(expected_ai_numbered_lines( + "ai dropped line", + COMMITS_JUST_OVER_PENDING_DROPPED_LIMIT, + )); + + let old_range = format!("{base_sha}..{feature_tip}"); + let new_range = format!("{base_sha}..{feature_prime_tip}"); + let range_diff = repo + .git(&[ + "range-diff", + "-s", + "--creation-factor=100", + &old_range, + &new_range, + ]) + .unwrap(); + let trailing_drops = dropped_commits_after_first_match(&range_diff); + assert!( + trailing_drops == COMMITS_JUST_OVER_PENDING_DROPPED_LIMIT, + "test setup must place {COMMITS_JUST_OVER_PENDING_DROPPED_LIMIT} drops after a match; got {trailing_drops}\n{range_diff}" + ); + + repo.sync_daemon(); + let mapping_count_baseline = + shift_authorship_mapping_counts(&repo.daemon_stderr_contents()).len(); + + repo.git(&["reset", "--keep", &feature_prime_tip]).unwrap(); + repo.sync_daemon(); + + assert!(!dropped_path.exists()); + feature_file.assert_committed_lines(crate::lines!["ai feature line".ai()]); + + let daemon_log = repo.daemon_stderr_contents(); + let mapping_counts = shift_authorship_mapping_counts(&daemon_log); + assert_eq!( + &mapping_counts[mapping_count_baseline..], + &[1], + "the move should map only the matched stack commit, not fabricate one mapping per dropped commit\n{daemon_log}" + ); +} + +#[test] +fn test_reset_keep_move_skips_range_diff_when_range_exceeds_limit() { + use std::fs; + + let log_dir = tempfile::tempdir().unwrap(); + let spawn_log_path = log_dir.path().join("spawns.log"); + let spawn_log = spawn_log_path.to_string_lossy().into_owned(); + let repo = TestRepo::new_with_daemon_env(&[ + ("GIT_AI_SPAWN_LOG", spawn_log.as_str()), + ("GIT_AI_TEST_RANGE_DIFF_COMMIT_LIMIT", "2"), + ]); + + let mut base_file = repo.filename("base.txt"); + base_file.set_contents(crate::lines!["base"]); + repo.stage_all_and_commit("Initial commit").unwrap(); + let base_sha = repo.git(&["rev-parse", "HEAD"]).unwrap().trim().to_string(); + + repo.git(&["checkout", "-b", "feature"]).unwrap(); + let feature_path = repo.path().join("feature.txt"); + fs::write(&feature_path, "ai feature line\n").unwrap(); + repo.git_ai(&["checkpoint", "mock_ai", "feature.txt"]) + .unwrap(); + repo.git(&["add", "feature.txt"]).unwrap(); + repo.commit("feature commit").unwrap(); + let feature_commit = repo.git(&["rev-parse", "HEAD"]).unwrap().trim().to_string(); + let mut feature_file = repo.filename("feature.txt"); + feature_file.assert_committed_lines(crate::lines!["ai feature line".ai()]); + + repo.git(&["checkout", "-b", "feature-prime", &base_sha]) + .unwrap(); + repo.git(&["cherry-pick", &feature_commit]).unwrap(); + repo.git(&[ + "commit", + "--amend", + "-m", + "feature commit moved to new parent", + ]) + .unwrap(); + let feature_prime_tip = repo.git(&["rev-parse", "HEAD"]).unwrap().trim().to_string(); + feature_file.assert_committed_lines(crate::lines!["ai feature line".ai()]); + + repo.git(&["checkout", "feature"]).unwrap(); + let dropped_path = repo.path().join("dropped.txt"); + for commit_number in 1..=3 { + fs::write( + &dropped_path, + numbered_file_contents("ai dropped line", commit_number), + ) + .unwrap(); + repo.git_ai(&["checkpoint", "mock_ai", "dropped.txt"]) + .unwrap(); + repo.git(&["add", "dropped.txt"]).unwrap(); + repo.commit(&format!("dropped commit {commit_number}")) + .unwrap(); + } + let mut dropped_file = repo.filename("dropped.txt"); + dropped_file.assert_committed_lines(expected_ai_numbered_lines("ai dropped line", 3)); + + repo.sync_daemon(); + let range_diff_baseline = range_diff_spawn_count(&spawn_log_path); + + repo.git(&["reset", "--keep", &feature_prime_tip]).unwrap(); + repo.sync_daemon(); + + assert!(!dropped_path.exists()); + feature_file.assert_committed_lines(crate::lines!["ai feature line".ai()]); + assert_eq!( + range_diff_spawn_count(&spawn_log_path), + range_diff_baseline, + "an oversized range must be rejected before spawning git range-diff" + ); +} + +#[test] +fn test_reset_keep_move_bounds_long_upstream_without_losing_authorship() { + use std::fs; + + let repo = TestRepo::new_with_daemon_env(&[("GIT_AI_TEST_RANGE_DIFF_COMMIT_LIMIT", "2")]); + + let mut base_file = repo.filename("base.txt"); + base_file.set_contents(crate::lines!["base"]); + repo.stage_all_and_commit("Initial commit").unwrap(); + let base_sha = repo.git(&["rev-parse", "HEAD"]).unwrap().trim().to_string(); + let default_branch = repo.current_branch(); + base_file.assert_committed_lines(crate::lines!["base".human()]); + + repo.git(&["checkout", "-b", "feature"]).unwrap(); + let feature_path = repo.path().join("feature.txt"); + fs::write(&feature_path, "ai feature line\n").unwrap(); + repo.git_ai(&["checkpoint", "mock_ai", "feature.txt"]) + .unwrap(); + repo.git(&["add", "feature.txt"]).unwrap(); + repo.commit("feature commit").unwrap(); + let feature_commit = repo.git(&["rev-parse", "HEAD"]).unwrap().trim().to_string(); + let mut feature_file = repo.filename("feature.txt"); + feature_file.assert_committed_lines(crate::lines!["ai feature line".ai()]); + + repo.git(&["checkout", &default_branch]).unwrap(); + let upstream_path = repo.path().join("upstream.txt"); + let mut upstream_file = repo.filename("upstream.txt"); + for commit_number in 1..=3 { + fs::write( + &upstream_path, + numbered_file_contents("upstream line", commit_number), + ) + .unwrap(); + repo.git(&["add", "upstream.txt"]).unwrap(); + repo.commit(&format!("upstream commit {commit_number}")) + .unwrap(); + upstream_file.assert_committed_lines( + (1..=commit_number) + .map(|i| format!("upstream line {i}").unattributed_human()) + .collect(), + ); + } + + repo.git(&["checkout", "-b", "feature-prime"]).unwrap(); + repo.git(&["cherry-pick", &feature_commit]).unwrap(); + let feature_prime_tip = repo.git(&["rev-parse", "HEAD"]).unwrap().trim().to_string(); + feature_file.assert_committed_lines(crate::lines!["ai feature line".ai()]); + repo.git(&["notes", "--ref=refs/notes/ai", "remove", &feature_prime_tip]) + .unwrap(); + assert!(repo.read_authorship_note(&feature_prime_tip).is_none()); + + repo.git(&["checkout", "feature"]).unwrap(); + repo.git(&["reset", "--keep", &feature_prime_tip]).unwrap(); + + assert_eq!( + repo.git(&["merge-base", &feature_commit, &feature_prime_tip]) + .unwrap() + .trim(), + base_sha + ); + feature_file.assert_committed_lines(crate::lines!["ai feature line".ai()]); +} + /// Test rebase with more than DIFF_TREE_STREAM_CHUNK_SIZE (50) rewritten commits. /// /// The streaming diff-tree path drains completed results in chunks of 50 and