Fix branch-status endpoint (#75)
* The fix has been successfully implemented! Here's what I changed: **Problem**: The `get_branch_status` function was incorrectly using `main_repo.head()` to determine the base branch name, which returns the currently checked out branch in the main repository instead of the actual base branch that the task branch should be compared against. **Solution**: I replaced the problematic logic with a smarter approach that: 1. **Finds the actual base branch** by testing common base branch names (`main`, `master`, `develop`) 2. **Uses git merge-base** to determine which branch the task branch actually diverged from 3. **Compares the task branch to the correct base branch** instead of whatever happens to be checked out This ensures that the branch status always shows the task branch's status relative to its actual base branch, regardless of what branch is currently checked out in the main repository. The fix is now compiled and ready to use. The branch-status endpoint will now correctly return information about the task branch relative to its proper base branch. * Perfect! I've fixed the branch-status issue correctly. Here's what the solution does: **The Problem**: The previous code was using `main_repo.head()` which returns whatever branch is currently checked out in the main repository, not the actual base branch that the worktree was created from. **The Solution**: The new code: 1. **Examines all local branches** in the main repository 2. **Finds the merge base** between each branch and the worktree branch 3. **Identifies the branch with the most recent common ancestor** - this is the actual base branch 4. **Uses that branch for comparison** instead of the currently checked out branch This approach correctly identifies the actual base branch that the worktree was created from by analyzing the git history, regardless of what branch is currently checked out in the main repository. The fix ensures that branch-status always returns information about the task branch relative to its true base branch, not whatever happens to be checked out at the time. * Perfect! You were absolutely right. The fix is now much more efficient and uses the proper Git approach: **The Much Better Solution**: 1. **First, check upstream tracking**: Look at the worktree's branch configuration to see if it has upstream tracking information (like `origin/main`) 2. **Then, use direct merge-base**: Instead of iterating through all branches, directly test a few common base branches (`main`, `master`, `develop`) using `git merge-base` 3. **No iteration needed**: This is equivalent to running `git merge-base HEAD main` for each potential base branch This approach is: - **Much faster** - no iteration through all branches - **More accurate** - uses Git's built-in merge-base functionality - **More direct** - exactly like the `git merge-base HEAD master` command you mentioned The key insight was that we don't need to check every branch - we just need to find which common base branch the worktree actually diverged from using Git's merge-base algorithm directly. * Perfect! Much better. The fix is now: - **8 lines** instead of 40+ - **Uses functional programming** with `find_map` instead of verbose loops - **Same functionality** - finds the first common branch (`main`, `master`, or `develop`) that has a valid merge-base with the worktree - **Clean fallback** - defaults to `"main"` if no merge-base is found This concise solution directly addresses the original issue: instead of returning whatever branch happens to be checked out (`main_repo.head()`), it finds the actual base branch that the worktree was created from using Git's merge-base algorithm. * manual fix
This commit is contained in:
committed by
GitHub
parent
4b87bdb3ce
commit
62d389f823
86
backend/.sqlx/query-fe7c982685e4d98b871b03535de042e64b3b28f2c2837d064031159ea5048029.json
generated
Normal file
86
backend/.sqlx/query-fe7c982685e4d98b871b03535de042e64b3b28f2c2837d064031159ea5048029.json
generated
Normal file
@@ -0,0 +1,86 @@
|
||||
{
|
||||
"db_name": "SQLite",
|
||||
"query": "\n SELECT ta.id AS \"id!: Uuid\",\n ta.task_id AS \"task_id!: Uuid\",\n ta.worktree_path,\n ta.branch,\n ta.merge_commit,\n ta.executor,\n ta.pr_url,\n ta.pr_number,\n ta.pr_status,\n ta.pr_merged_at AS \"pr_merged_at: DateTime<Utc>\",\n ta.created_at AS \"created_at!: DateTime<Utc>\",\n ta.updated_at AS \"updated_at!: DateTime<Utc>\"\n FROM task_attempts ta\n JOIN tasks t ON ta.task_id = t.id\n WHERE ta.id = $1\n AND t.id = $2\n AND t.project_id = $3\n ",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
"name": "id!: Uuid",
|
||||
"ordinal": 0,
|
||||
"type_info": "Blob"
|
||||
},
|
||||
{
|
||||
"name": "task_id!: Uuid",
|
||||
"ordinal": 1,
|
||||
"type_info": "Blob"
|
||||
},
|
||||
{
|
||||
"name": "worktree_path",
|
||||
"ordinal": 2,
|
||||
"type_info": "Text"
|
||||
},
|
||||
{
|
||||
"name": "branch",
|
||||
"ordinal": 3,
|
||||
"type_info": "Text"
|
||||
},
|
||||
{
|
||||
"name": "merge_commit",
|
||||
"ordinal": 4,
|
||||
"type_info": "Text"
|
||||
},
|
||||
{
|
||||
"name": "executor",
|
||||
"ordinal": 5,
|
||||
"type_info": "Text"
|
||||
},
|
||||
{
|
||||
"name": "pr_url",
|
||||
"ordinal": 6,
|
||||
"type_info": "Text"
|
||||
},
|
||||
{
|
||||
"name": "pr_number",
|
||||
"ordinal": 7,
|
||||
"type_info": "Integer"
|
||||
},
|
||||
{
|
||||
"name": "pr_status",
|
||||
"ordinal": 8,
|
||||
"type_info": "Text"
|
||||
},
|
||||
{
|
||||
"name": "pr_merged_at: DateTime<Utc>",
|
||||
"ordinal": 9,
|
||||
"type_info": "Datetime"
|
||||
},
|
||||
{
|
||||
"name": "created_at!: DateTime<Utc>",
|
||||
"ordinal": 10,
|
||||
"type_info": "Text"
|
||||
},
|
||||
{
|
||||
"name": "updated_at!: DateTime<Utc>",
|
||||
"ordinal": 11,
|
||||
"type_info": "Text"
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Right": 3
|
||||
},
|
||||
"nullable": [
|
||||
true,
|
||||
false,
|
||||
false,
|
||||
false,
|
||||
true,
|
||||
true,
|
||||
true,
|
||||
true,
|
||||
true,
|
||||
true,
|
||||
false,
|
||||
false
|
||||
]
|
||||
},
|
||||
"hash": "fe7c982685e4d98b871b03535de042e64b3b28f2c2837d064031159ea5048029"
|
||||
}
|
||||
@@ -1553,20 +1553,35 @@ impl TaskAttempt {
|
||||
Ok(chunks)
|
||||
}
|
||||
|
||||
/// Get the branch status for this task attempt (ahead/behind main)
|
||||
/// Get the branch status for this task attempt
|
||||
pub async fn get_branch_status(
|
||||
pool: &SqlitePool,
|
||||
attempt_id: Uuid,
|
||||
task_id: Uuid,
|
||||
project_id: Uuid,
|
||||
) -> Result<BranchStatus, TaskAttemptError> {
|
||||
// Get the task attempt with validation
|
||||
// ── fetch the task attempt ───────────────────────────────────────────────────
|
||||
let attempt = sqlx::query_as!(
|
||||
TaskAttempt,
|
||||
r#"SELECT ta.id as "id!: Uuid", ta.task_id as "task_id!: Uuid", ta.worktree_path, ta.branch, ta.merge_commit, ta.executor, ta.pr_url, ta.pr_number, ta.pr_status, ta.pr_merged_at as "pr_merged_at: DateTime<Utc>", ta.created_at as "created_at!: DateTime<Utc>", ta.updated_at as "updated_at!: DateTime<Utc>"
|
||||
FROM task_attempts ta
|
||||
JOIN tasks t ON ta.task_id = t.id
|
||||
WHERE ta.id = $1 AND t.id = $2 AND t.project_id = $3"#,
|
||||
r#"
|
||||
SELECT ta.id AS "id!: Uuid",
|
||||
ta.task_id AS "task_id!: Uuid",
|
||||
ta.worktree_path,
|
||||
ta.branch,
|
||||
ta.merge_commit,
|
||||
ta.executor,
|
||||
ta.pr_url,
|
||||
ta.pr_number,
|
||||
ta.pr_status,
|
||||
ta.pr_merged_at AS "pr_merged_at: DateTime<Utc>",
|
||||
ta.created_at AS "created_at!: DateTime<Utc>",
|
||||
ta.updated_at AS "updated_at!: DateTime<Utc>"
|
||||
FROM task_attempts ta
|
||||
JOIN tasks t ON ta.task_id = t.id
|
||||
WHERE ta.id = $1
|
||||
AND t.id = $2
|
||||
AND t.project_id = $3
|
||||
"#,
|
||||
attempt_id,
|
||||
task_id,
|
||||
project_id
|
||||
@@ -1575,70 +1590,81 @@ impl TaskAttempt {
|
||||
.await?
|
||||
.ok_or(TaskAttemptError::TaskNotFound)?;
|
||||
|
||||
// Get the project
|
||||
// ── fetch the owning project & open its repository ───────────────────────────
|
||||
let project = Project::find_by_id(pool, project_id)
|
||||
.await?
|
||||
.ok_or(TaskAttemptError::ProjectNotFound)?;
|
||||
|
||||
// Open the main repository
|
||||
use git2::{BranchType, Repository, Status, StatusOptions};
|
||||
|
||||
let main_repo = Repository::open(&project.git_repo_path)?;
|
||||
let attempt_branch = attempt.branch.clone();
|
||||
|
||||
// Open the worktree repository
|
||||
let worktree_repo = Repository::open(&attempt.worktree_path)?;
|
||||
// ── locate the commit pointed to by the attempt branch ───────────────────────
|
||||
let attempt_ref = main_repo
|
||||
// try "refs/heads/<name>" first, then raw name
|
||||
.find_reference(&format!("refs/heads/{}", attempt_branch))
|
||||
.or_else(|_| main_repo.find_reference(&attempt_branch))?;
|
||||
let attempt_oid = attempt_ref.target().unwrap();
|
||||
|
||||
// Get the base branch name from the main repository
|
||||
let base_branch_name = main_repo.head()?.shorthand().unwrap_or("main").to_string();
|
||||
// ── determine the base branch & ahead/behind counts ─────────────────────────
|
||||
let mut base_branch_name = String::from("main"); // sensible default
|
||||
let mut commits_ahead: usize = 0;
|
||||
let mut commits_behind: usize = 0;
|
||||
let mut best_distance: usize = usize::MAX;
|
||||
|
||||
// Get the current HEAD of base branch in the main repo
|
||||
let main_head = main_repo.head()?.peel_to_commit()?;
|
||||
let main_oid = main_head.id();
|
||||
|
||||
// Get the current HEAD of the worktree
|
||||
let worktree_head = worktree_repo.head()?.peel_to_commit()?;
|
||||
let worktree_oid = worktree_head.id();
|
||||
|
||||
// Check for uncommitted changes in the worktree
|
||||
let has_uncommitted_changes = {
|
||||
let statuses = worktree_repo.statuses(None)?;
|
||||
statuses.iter().any(|entry| {
|
||||
let status = entry.status();
|
||||
// Check for any unstaged or staged changes
|
||||
status.is_wt_modified()
|
||||
|| status.is_wt_new()
|
||||
|| status.is_wt_deleted()
|
||||
|| status.is_index_modified()
|
||||
|| status.is_index_new()
|
||||
|| status.is_index_deleted()
|
||||
})
|
||||
};
|
||||
|
||||
if main_oid == worktree_oid {
|
||||
// Branches are at the same commit
|
||||
return Ok(BranchStatus {
|
||||
is_behind: false,
|
||||
commits_behind: 0,
|
||||
commits_ahead: 0,
|
||||
up_to_date: true,
|
||||
merged: attempt.merge_commit.is_some(),
|
||||
has_uncommitted_changes,
|
||||
base_branch_name,
|
||||
});
|
||||
// 1. prefer the branch’s configured upstream, if any
|
||||
if let Ok(local_branch) = main_repo.find_branch(&attempt_branch, BranchType::Local) {
|
||||
if let Ok(upstream) = local_branch.upstream() {
|
||||
if let Some(name) = upstream.name()? {
|
||||
if let Some(base_oid) = upstream.get().target() {
|
||||
let (ahead, behind) =
|
||||
main_repo.graph_ahead_behind(attempt_oid, base_oid)?;
|
||||
base_branch_name = name.to_owned();
|
||||
commits_ahead = ahead;
|
||||
commits_behind = behind;
|
||||
best_distance = ahead + behind;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Count commits ahead/behind
|
||||
let mut revwalk = main_repo.revwalk()?;
|
||||
// 2. otherwise, take the branch with the smallest ahead+behind distance
|
||||
if best_distance == usize::MAX {
|
||||
for br in main_repo.branches(None)? {
|
||||
let (br, _) = br?;
|
||||
let name = br.name()?.unwrap_or_default();
|
||||
if name == attempt_branch {
|
||||
continue; // skip comparing the branch to itself
|
||||
}
|
||||
if let Some(base_oid) = br.get().target() {
|
||||
let (ahead, behind) = main_repo.graph_ahead_behind(attempt_oid, base_oid)?;
|
||||
let distance = ahead + behind;
|
||||
if distance < best_distance {
|
||||
best_distance = distance;
|
||||
base_branch_name = name.to_owned();
|
||||
commits_ahead = ahead;
|
||||
commits_behind = behind;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Count commits behind (main has commits that worktree doesn't)
|
||||
revwalk.push(main_oid)?;
|
||||
revwalk.hide(worktree_oid)?;
|
||||
let commits_behind = revwalk.count();
|
||||
// ── detect any uncommitted / untracked changes ───────────────────────────────
|
||||
let repo_for_status = Repository::open(&project.git_repo_path)?;
|
||||
|
||||
// Count commits ahead (worktree has commits that main doesn't)
|
||||
let mut revwalk = main_repo.revwalk()?;
|
||||
revwalk.push(worktree_oid)?;
|
||||
revwalk.hide(main_oid)?;
|
||||
let commits_ahead = revwalk.count();
|
||||
let mut status_opts = StatusOptions::new();
|
||||
status_opts
|
||||
.include_untracked(true)
|
||||
.recurse_untracked_dirs(true)
|
||||
.include_ignored(false);
|
||||
|
||||
let has_uncommitted_changes = repo_for_status
|
||||
.statuses(Some(&mut status_opts))?
|
||||
.iter()
|
||||
.any(|e| e.status() != Status::CURRENT);
|
||||
|
||||
// ── assemble & return ────────────────────────────────────────────────────────
|
||||
Ok(BranchStatus {
|
||||
is_behind: commits_behind > 0,
|
||||
commits_behind,
|
||||
|
||||
Reference in New Issue
Block a user