[GH-ISSUE #328] Bug: test_rebase_preserve_merges fails - Side branch commits lost during --rebase-merges #116

New issue

Closed

opened 2026-03-02 04:11:56 +03:00 by kerem · 0 comments

kerem commented

2026-03-02 04:11:56 +03:00

Owner

Originally created by @txf0096 on GitHub (Jan 7, 2026).
Original GitHub issue: https://github.com/git-ai-project/git-ai/issues/328

Bug: `test_rebase_preserve_merges` fails - Side branch commits lost during `--rebase-merges`

Summary

The walk_commits_to_base function only follows parent(0), causing it to miss commits from merged branches when using git rebase --rebase-merges. This results in AI authorship being lost for side branch commits.

How to Reproduce

Run the integration test:

cargo test test_rebase_preserve_merges -- --nocapture

Expected: Test passes with AI authorship preserved
Actual: Test fails with error:

Line 1: Expected AI author but got 'Test User'
Expected: ExpectedLine { contents: "// AI side", author_type: Ai }
Actual: "// AI side" attributed to Test User

Root Cause

In src/authorship/rebase_authorship.rs, the walk_commits_to_base function only traverses the first parent:

// Current implementation (BUGGY)
pub fn walk_commits_to_base(...) -> Result<Vec<String>, GitAiError> {
    let mut commits = Vec::new();
    let mut current = repository.find_commit(head.to_string())?;
    let base_str = base.to_string();

    while current.id().to_string() != base_str {
        commits.push(current.id().to_string());
        current = current.parent(0)?;  // ❌ Only follows first parent!
    }

    Ok(commits)
}

Problem Scenario

When rebasing a merge commit structure:

Initial (A)
    ├── Feature (D)
    └── Side (E)
         └── Merge (F)
             parent(0) = D
             parent(1) = E

Current code: F → D → A (collects [F, D]) ❌ Missing E!

During rebase, the authorship mapping becomes:

D → D' ✅ (feature.txt authorship preserved)
E → E' ❌ (side.txt authorship lost - E was never collected!)
F → F' ✅

Proposed Fix

Use BFS (Breadth-First Search) to traverse all parents:

pub fn walk_commits_to_base(
    repository: &Repository,
    head: &str,
    base: &str,
) -> Result<Vec<String>, crate::error::GitAiError> {
    use std::collections::{HashSet, VecDeque};

    let mut commits = Vec::new();
    let mut visited = HashSet::new();
    let mut queue = VecDeque::new();

    let base_str = base.to_string();
    let head_commit = repository.find_commit(head.to_string())?;

    queue.push_back(head_commit);

    while let Some(current) = queue.pop_front() {
        let commit_id = current.id().to_string();

        // Skip if we've reached the base
        if commit_id == base_str {
            continue;
        }

        // Skip if already visited
        if visited.contains(&commit_id) {
            continue;
        }

        visited.insert(commit_id.clone());
        commits.push(commit_id);

        // Add all parents to the queue (handles merge commits)
        let parent_count = current.parent_count().unwrap_or(0);
        for i in 0..parent_count {
            if let Ok(parent) = current.parent(i) {
                queue.push_back(parent);
            }
        }
    }

    Ok(commits)
}

With this fix: F → (D, E) → A (collects [F, D, E]) ✅ All commits collected!

Test Results

Before fix:

test test_rebase_preserve_merges ... FAILED

After fix:

test test_rebase_preserve_merges ... ok

Impact

Affected: Users using git rebase --rebase-merges with merge commits
Symptom: AI authorship lost for commits in merged branches
Scope: Only affects --rebase-merges, regular rebase is unaffected
Severity: High (data loss for AI authorship tracking)

Environment

Git version: 2.39.5
Rust version: stable
OS: macOS/Linux/Windows (all affected)

Additional Context

The --rebase-merges flag was introduced in Git 2.18 to preserve merge commit structure during rebase. Our current implementation assumes linear history (only parent(0)), which works for regular rebase but breaks with merge commits.

Originally created by @txf0096 on GitHub (Jan 7, 2026). Original GitHub issue: https://github.com/git-ai-project/git-ai/issues/328 # Bug: `test_rebase_preserve_merges` fails - Side branch commits lost during `--rebase-merges` ## Summary The `walk_commits_to_base` function only follows `parent(0)`, causing it to miss commits from merged branches when using `git rebase --rebase-merges`. This results in AI authorship being lost for side branch commits. ## How to Reproduce 1. Run the integration test: ```bash cargo test test_rebase_preserve_merges -- --nocapture ``` **Expected**: Test passes with AI authorship preserved **Actual**: Test fails with error: ``` Line 1: Expected AI author but got 'Test User' Expected: ExpectedLine { contents: "// AI side", author_type: Ai } Actual: "// AI side" attributed to Test User ``` ## Root Cause In `src/authorship/rebase_authorship.rs`, the `walk_commits_to_base` function only traverses the first parent: ```rust // Current implementation (BUGGY) pub fn walk_commits_to_base(...) -> Result<Vec<String>, GitAiError> { let mut commits = Vec::new(); let mut current = repository.find_commit(head.to_string())?; let base_str = base.to_string(); while current.id().to_string() != base_str { commits.push(current.id().to_string()); current = current.parent(0)?; // ❌ Only follows first parent! } Ok(commits) } ``` ### Problem Scenario When rebasing a merge commit structure: ``` Initial (A) ├── Feature (D) └── Side (E) └── Merge (F) parent(0) = D parent(1) = E ``` Current code: `F → D → A` (collects `[F, D]`) ❌ Missing `E`! During rebase, the authorship mapping becomes: - D → D' ✅ (feature.txt authorship preserved) - E → E' ❌ (side.txt authorship lost - E was never collected!) - F → F' ✅ ## Proposed Fix Use BFS (Breadth-First Search) to traverse all parents: ```rust pub fn walk_commits_to_base( repository: &Repository, head: &str, base: &str, ) -> Result<Vec<String>, crate::error::GitAiError> { use std::collections::{HashSet, VecDeque}; let mut commits = Vec::new(); let mut visited = HashSet::new(); let mut queue = VecDeque::new(); let base_str = base.to_string(); let head_commit = repository.find_commit(head.to_string())?; queue.push_back(head_commit); while let Some(current) = queue.pop_front() { let commit_id = current.id().to_string(); // Skip if we've reached the base if commit_id == base_str { continue; } // Skip if already visited if visited.contains(&commit_id) { continue; } visited.insert(commit_id.clone()); commits.push(commit_id); // Add all parents to the queue (handles merge commits) let parent_count = current.parent_count().unwrap_or(0); for i in 0..parent_count { if let Ok(parent) = current.parent(i) { queue.push_back(parent); } } } Ok(commits) } ``` With this fix: `F → (D, E) → A` (collects `[F, D, E]`) ✅ All commits collected! ## Test Results **Before fix:** ``` test test_rebase_preserve_merges ... FAILED ``` **After fix:** ``` test test_rebase_preserve_merges ... ok ``` ## Impact - **Affected**: Users using `git rebase --rebase-merges` with merge commits - **Symptom**: AI authorship lost for commits in merged branches - **Scope**: Only affects `--rebase-merges`, regular rebase is unaffected - **Severity**: High (data loss for AI authorship tracking) ## Environment - Git version: 2.39.5 - Rust version: stable - OS: macOS/Linux/Windows (all affected) ## Additional Context The `--rebase-merges` flag was introduced in Git 2.18 to preserve merge commit structure during rebase. Our current implementation assumes linear history (only `parent(0)`), which works for regular rebase but breaks with merge commits.