Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1,243 changes: 865 additions & 378 deletions src/authorship/rebase_authorship.rs

Large diffs are not rendered by default.

18 changes: 18 additions & 0 deletions src/daemon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2629,6 +2629,17 @@ fn processed_rebase_new_heads(repository: &Repository) -> Result<HashSet<String>
Ok(out)
}

/// Check whether `ancestor` is an ancestor of `descendant` using
/// `git merge-base --is-ancestor`.
fn is_ancestor_commit(repository: &Repository, ancestor: &str, descendant: &str) -> bool {
let mut args = repository.global_args_for_exec();
args.push("merge-base".to_string());
args.push("--is-ancestor".to_string());
args.push(ancestor.to_string());
args.push(descendant.to_string());
crate::git::repository::exec_git(&args).is_ok()
}

fn maybe_rebase_mappings_from_repository(
repository: &Repository,
old_head: &str,
Expand Down Expand Up @@ -5607,6 +5618,7 @@ impl ActorDaemonCoordinator {
&& !is_zero_oid(new_head)
{
if let Ok(repository) = repository_for_rewrite_context(cmd, "reset_rewrite")
&& !is_ancestor_commit(&repository, new_head, old_head)
{
if let Some((original_commits, new_commits)) =
maybe_rebase_mappings_from_repository(
Expand Down Expand Up @@ -5912,8 +5924,14 @@ impl ActorDaemonCoordinator {
if reference.starts_with("refs/heads/")
&& !old.is_empty()
&& !new.is_empty()
&& old != new
&& is_valid_oid(old)
&& !is_zero_oid(old)
&& is_valid_oid(new)
&& !is_zero_oid(new)
&& let Ok(repository) =
repository_for_rewrite_context(cmd, "update_ref_rewrite")
&& !is_ancestor_commit(&repository, new, old)
&& let Some((original_commits, new_commits)) =
maybe_rebase_mappings_from_repository(
&repository,
Expand Down
10 changes: 4 additions & 6 deletions src/daemon/analyzers/history.rs
Original file line number Diff line number Diff line change
Expand Up @@ -500,13 +500,11 @@ fn parse_update_ref_heads(
// The command has already executed; read the old value from the
// reflog entry that recorded this update-ref.
let worktree = cmd.worktree.as_deref()?;
resolve_worktree_head_reflog_old_oid_for_new_head(worktree, &new_oid)
.ok()
.flatten()
resolve_reflog_old_oid_for_ref_new_oid_in_worktree(worktree, &ref_name, &new_oid)
.or_else(|| {
resolve_reflog_old_oid_for_ref_new_oid_in_worktree(
worktree, &ref_name, &new_oid,
)
resolve_worktree_head_reflog_old_oid_for_new_head(worktree, &new_oid)
.ok()
.flatten()
})
})
.filter(|oid| !oid.is_empty() && !is_zero_oid(oid))?;
Expand Down
45 changes: 33 additions & 12 deletions src/git/repository.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3094,15 +3094,27 @@ pub fn exec_git_stdin_with_profile(

let mut child = cmd.spawn().map_err(GitAiError::IoError)?;

if let Some(mut stdin) = child.stdin.take() {
use std::io::Write;
if let Err(e) = stdin.write_all(stdin_data) {
return Err(GitAiError::IoError(e));
}
}
// Write stdin in a separate thread to avoid deadlock: if we write all stdin
// before reading stdout, the child's stdout pipe buffer can fill up, causing
// the child to block on write, which prevents it from consuming more stdin,
// which blocks our write_all. Writing concurrently avoids this.
let stdin_handle = child.stdin.take().map(|mut stdin| {
let data = stdin_data.to_vec();
std::thread::spawn(move || {
use std::io::Write;
stdin.write_all(&data)
})
});

let output = child.wait_with_output().map_err(GitAiError::IoError)?;

if let Some(handle) = stdin_handle
&& let Err(e) = handle.join().expect("stdin writer thread panicked")
&& e.kind() != std::io::ErrorKind::BrokenPipe
{
return Err(GitAiError::IoError(e));
}

if !output.status.success() {
let code = output.status.code();
let stderr = String::from_utf8_lossy(&output.stderr).to_string();
Expand Down Expand Up @@ -3159,15 +3171,24 @@ pub fn exec_git_stdin_with_env_with_profile(

let mut child = cmd.spawn().map_err(GitAiError::IoError)?;

if let Some(mut stdin) = child.stdin.take() {
use std::io::Write;
if let Err(e) = stdin.write_all(stdin_data) {
return Err(GitAiError::IoError(e));
}
}
// Write stdin in a separate thread to avoid deadlock (see exec_git_stdin_with_profile).
let stdin_handle = child.stdin.take().map(|mut stdin| {
let data = stdin_data.to_vec();
std::thread::spawn(move || {
use std::io::Write;
stdin.write_all(&data)
})
});

let output = child.wait_with_output().map_err(GitAiError::IoError)?;

if let Some(handle) = stdin_handle
&& let Err(e) = handle.join().expect("stdin writer thread panicked")
&& e.kind() != std::io::ErrorKind::BrokenPipe
{
return Err(GitAiError::IoError(e));
}

if !output.status.success() {
let code = output.status.code();
let stderr = String::from_utf8_lossy(&output.stderr).to_string();
Expand Down
177 changes: 177 additions & 0 deletions tests/integration/rebase.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1517,6 +1517,181 @@ fn test_rebase_preserves_custom_attributes_from_config() {
feature_file.assert_lines_and_blame(crate::lines!["// AI feature code".ai()]);
}

/// Regression test: prompt metrics (accepted_lines) must update per commit, not be frozen
/// from the initial state. When commit 1 has 2 AI lines and commit 2 adds 2 more
/// (total 4), the rebased notes should reflect different accepted_lines.
#[test]
fn test_rebase_prompt_metrics_update_per_commit() {
let repo = TestRepo::new();
let default_branch = repo.current_branch();

// Initial setup
let mut base_file = repo.filename("base.txt");
base_file.set_contents(crate::lines!["base content"]);
repo.stage_all_and_commit("Initial").unwrap();

// Create feature branch
repo.git(&["checkout", "-b", "feature"]).unwrap();

// Commit 1: add 2 AI lines
let mut ai_file = repo.filename("feature.txt");
ai_file.set_contents(crate::lines!["line1".ai(), "line2".ai()]);
let commit1 = repo.stage_all_and_commit("AI commit 1 - 2 lines").unwrap();

// Commit 2: add 2 more AI lines (total 4)
ai_file.set_contents(crate::lines![
"line1".ai(),
"line2".ai(),
"line3".ai(),
"line4".ai()
]);
let commit2 = repo.stage_all_and_commit("AI commit 2 - 4 lines").unwrap();

// Verify pre-rebase: commit 1 has 2 accepted, commit 2 has 4
let note1 = repo
.read_authorship_note(&commit1.commit_sha)
.expect("commit 1 should have note");
let log1 = AuthorshipLog::deserialize_from_string(&note1).expect("parse note 1");
let note2 = repo
.read_authorship_note(&commit2.commit_sha)
.expect("commit 2 should have note");
let log2 = AuthorshipLog::deserialize_from_string(&note2).expect("parse note 2");

let pre_accepted_1: u32 = log1
.metadata
.prompts
.values()
.map(|p| p.accepted_lines)
.sum();
let pre_accepted_2: u32 = log2
.metadata
.prompts
.values()
.map(|p| p.accepted_lines)
.sum();
assert!(
pre_accepted_1 < pre_accepted_2,
"precondition: commit 2 ({}) should have more accepted_lines than commit 1 ({})",
pre_accepted_2,
pre_accepted_1
);

// Advance default branch
repo.git(&["checkout", &default_branch]).unwrap();
let mut other_file = repo.filename("other.txt");
other_file.set_contents(crate::lines!["other"]);
repo.stage_all_and_commit("Main advances").unwrap();

// Rebase feature
repo.git(&["checkout", "feature"]).unwrap();
repo.git(&["rebase", &default_branch]).unwrap();

// Get rebased commit SHAs
let rebased_tip = repo.git(&["rev-parse", "HEAD"]).unwrap().trim().to_string();
let rebased_parent = repo
.git(&["rev-parse", "HEAD~1"])
.unwrap()
.trim()
.to_string();

// Verify post-rebase: metrics should differ between the two commits
let rebased_note1 = repo
.read_authorship_note(&rebased_parent)
.expect("rebased commit 1 should have note");
let rebased_log1 =
AuthorshipLog::deserialize_from_string(&rebased_note1).expect("parse rebased note 1");
let rebased_note2 = repo
.read_authorship_note(&rebased_tip)
.expect("rebased commit 2 should have note");
let rebased_log2 =
AuthorshipLog::deserialize_from_string(&rebased_note2).expect("parse rebased note 2");

let post_accepted_1: u32 = rebased_log1
.metadata
.prompts
.values()
.map(|p| p.accepted_lines)
.sum();
let post_accepted_2: u32 = rebased_log2
.metadata
.prompts
.values()
.map(|p| p.accepted_lines)
.sum();

assert!(
post_accepted_1 < post_accepted_2,
"regression: rebased commit 2 ({}) should have more accepted_lines than commit 1 ({}). \
If equal, the fast path is freezing metrics across commits.",
post_accepted_2,
post_accepted_1
);
}

/// Regression test: attributions should survive a delete-recreate cycle within a rebase.
/// If a file is deleted in commit N and recreated in commit N+1, the recreated file
/// should inherit attributions from the pre-deletion state via positional diff transfer.
#[test]
fn test_rebase_file_delete_recreate_preserves_attribution() {
let repo = TestRepo::new();
let default_branch = repo.current_branch();

// Initial setup
let mut base_file = repo.filename("base.txt");
base_file.set_contents(crate::lines!["base content"]);
repo.stage_all_and_commit("Initial").unwrap();

// Create feature branch with AI file
repo.git(&["checkout", "-b", "feature"]).unwrap();
let mut ai_file = repo.filename("feature.txt");
ai_file.set_contents(crate::lines!["line1".ai(), "line2".ai(), "line3".ai()]);
repo.stage_all_and_commit("Add AI file").unwrap();

// Delete the file
repo.git(&["rm", "feature.txt"]).unwrap();
repo.stage_all_and_commit("Delete AI file").unwrap();

// Recreate the file with same content
ai_file.set_contents(crate::lines!["line1".ai(), "line2".ai(), "line3".ai()]);
let recreate_commit = repo.stage_all_and_commit("Recreate AI file").unwrap();

// Verify pre-rebase: recreated file has attributions
let pre_note = repo
.read_authorship_note(&recreate_commit.commit_sha)
.expect("recreated commit should have note");
let pre_log = AuthorshipLog::deserialize_from_string(&pre_note).expect("parse pre note");
assert!(
!pre_log.attestations.is_empty(),
"precondition: recreated file should have attestations"
);

// Advance default branch
repo.git(&["checkout", &default_branch]).unwrap();
let mut other_file = repo.filename("other.txt");
other_file.set_contents(crate::lines!["other"]);
repo.stage_all_and_commit("Main advances").unwrap();

// Rebase feature
repo.git(&["checkout", "feature"]).unwrap();
repo.git(&["rebase", &default_branch]).unwrap();

// Check rebased tip (the recreate commit)
let rebased_sha = repo.git(&["rev-parse", "HEAD"]).unwrap().trim().to_string();
let rebased_note = repo
.read_authorship_note(&rebased_sha)
.expect("rebased recreate commit should have note");
let rebased_log =
AuthorshipLog::deserialize_from_string(&rebased_note).expect("parse rebased note");

assert!(
!rebased_log.attestations.is_empty(),
"regression: file recreated after deletion should still have attestations after rebase"
);

// Verify the AI attribution itself survived
ai_file.assert_lines_and_blame(crate::lines!["line1".ai(), "line2".ai(), "line3".ai()]);
}

crate::reuse_tests_in_worktree!(
test_rebase_no_conflicts_identical_trees,
test_rebase_with_different_trees,
Expand All @@ -1543,6 +1718,8 @@ crate::reuse_tests_in_worktree!(
test_rebase_exec,
test_rebase_preserve_merges,
test_rebase_commit_splitting,
test_rebase_prompt_metrics_update_per_commit,
test_rebase_file_delete_recreate_preserves_attribution,
);

crate::reuse_tests_in_worktree_with_attrs!(
Expand Down
Loading
Loading