From 2662e7bd9894c0cb83110f9c450556c3bdd66f3c Mon Sep 17 00:00:00 2001 From: mika kuns Date: Mon, 10 Aug 2026 14:36:55 +0200 Subject: [PATCH] feat(worker): auto-rebase overlapping WaitingForReview branches after a merge After a merge lands on the target branch, TaskMergeService now rebases every other WaitingForReview task's active worktree onto the new tip -- but only when that branch actually touches a file the merge just changed, computed via GitService.GetChangedFileNamesAsync (both the merge's landed files and each candidate's own diff). A branch merely behind is left alone. On conflict or any other failure the rebase is aborted and the branch is left exactly as it was, with a WorkerLog warning naming the task; a successful rebase updates the worktree's recorded BaseCommit/HeadCommit and broadcasts WorktreeUpdated. --- src/ClaudeDo.Data/Git/GitService.cs | 36 ++++ .../Repositories/WorktreeRepository.cs | 11 ++ .../Lifecycle/TaskMergeService.cs | 92 +++++++++ .../Services/TaskMergeServiceTests.cs | 175 ++++++++++++++++++ 4 files changed, 314 insertions(+) diff --git a/src/ClaudeDo.Data/Git/GitService.cs b/src/ClaudeDo.Data/Git/GitService.cs index 651796b4..6a1e1831 100644 --- a/src/ClaudeDo.Data/Git/GitService.cs +++ b/src/ClaudeDo.Data/Git/GitService.cs @@ -389,6 +389,42 @@ public sealed class GitService .Count(s => s.Length > 0); } + /// Files that differ between two exact refs (2-dot, no merge-base resolution) -- used to see + /// what a target branch itself picked up since a task's fork point, as opposed to + /// 's 3-dot count of a branch's own changes. + public async Task> GetChangedFileNamesAsync( + string repoDir, string fromRef, string toRef, CancellationToken ct = default) + { + var (exitCode, stdout, _) = await RunGitAsync(repoDir, + ["diff", "--name-only", $"{fromRef}..{toRef}"], ct); + if (exitCode != 0) return Array.Empty(); + return stdout + .Split('\n', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries) + .Where(s => s.Length > 0) + .ToList(); + } + + /// + /// Rebases the branch checked out at onto . + /// On conflict or any other failure the rebase is aborted before returning, so the worktree is left + /// exactly as it was rather than stranded mid-rebase; ConflictFiles is best-effort and only + /// populated when the failure was an actual conflict. + /// + public async Task<(int ExitCode, string Stderr, IReadOnlyList ConflictFiles)> RebaseAsync( + string worktreePath, string ontoRef, CancellationToken ct = default) + { + var (exitCode, _, stderr) = await RunGitAsync(worktreePath, ["rebase", ontoRef], ct); + if (exitCode == 0) + return (0, stderr, Array.Empty()); + + List conflictFiles; + try { conflictFiles = await ListConflictedFilesAsync(worktreePath, ct); } + catch { conflictFiles = new(); } + + await RunGitAsync(worktreePath, ["rebase", "--abort"], ct); + return (exitCode, stderr, conflictFiles); + } + private static async Task<(int ExitCode, string Stdout, string Stderr)> RunGitAsync( string workDir, IEnumerable args, CancellationToken ct, string? stdinData = null, bool trimOutput = true) { diff --git a/src/ClaudeDo.Data/Repositories/WorktreeRepository.cs b/src/ClaudeDo.Data/Repositories/WorktreeRepository.cs index 04576c04..e5264932 100644 --- a/src/ClaudeDo.Data/Repositories/WorktreeRepository.cs +++ b/src/ClaudeDo.Data/Repositories/WorktreeRepository.cs @@ -45,6 +45,17 @@ public sealed class WorktreeRepository .SetProperty(w => w.MergeCommit, mergeCommit), ct); } + /// Records a successful auto-rebase: the branch's fork point moves to the rebased-onto + /// commit and its head moves to the new tip git rebase produced. + public async Task SetRebasedAsync(string taskId, string baseCommit, string headCommit, CancellationToken ct = default) + { + await _context.Worktrees + .Where(w => w.TaskId == taskId) + .ExecuteUpdateAsync(s => s + .SetProperty(w => w.BaseCommit, baseCommit) + .SetProperty(w => w.HeadCommit, headCommit), ct); + } + public async Task DeleteAsync(string taskId, CancellationToken ct = default) { await _context.Worktrees.Where(w => w.TaskId == taskId).ExecuteDeleteAsync(ct); diff --git a/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs b/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs index e98c1c05..1618bc89 100644 --- a/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs +++ b/src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs @@ -161,6 +161,89 @@ public sealed class TaskMergeService await _state.ApproveReviewAsync(task.Id, ct); } + /// + /// After a merge lands on , rebases every other WaitingForReview + /// task's active worktree in the same list onto the new tip -- but only ones that actually + /// touch a file the merge just changed. A branch merely behind (no overlap) is left alone: a + /// rebase is disruptive busywork when there's nothing to gain from it. Best-effort throughout -- + /// this runs after the merge that matters has already succeeded, so a failure here must never + /// surface as a failure of that merge. + /// + private async Task RebaseOthersAfterMergeAsync( + TaskEntity mergedTask, ListEntity list, string targetBranch, + string oldTargetTip, string newTargetTip, CancellationToken ct) + { + if (string.Equals(oldTargetTip, newTargetTip, StringComparison.Ordinal)) return; + + IReadOnlyList landedFiles; + try + { + landedFiles = await _git.GetChangedFileNamesAsync(list.WorkingDir!, oldTargetTip, newTargetTip, ct); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Auto-rebase: could not compute files landed by merging task {TaskId}", mergedTask.Id); + return; + } + if (landedFiles.Count == 0) return; + + List candidates; + using (var ctx = _dbFactory.CreateDbContext()) + { + var tasks = await new TaskRepository(ctx).GetByListIdAsync(list.Id, ct); + candidates = tasks.Where(t => t.Id != mergedTask.Id && t.Status == TaskStatus.WaitingForReview).ToList(); + } + + foreach (var candidate in candidates) + await RebaseOneIfOverlappingAsync(candidate, targetBranch, newTargetTip, landedFiles, ct); + } + + private async Task RebaseOneIfOverlappingAsync( + TaskEntity candidate, string targetBranch, string newTargetTip, + IReadOnlyList landedFiles, CancellationToken ct) + { + WorktreeEntity? wt; + using (var ctx = _dbFactory.CreateDbContext()) + wt = await new WorktreeRepository(ctx).GetByTaskIdAsync(candidate.Id, ct); + + if (wt is null || wt.State != WorktreeState.Active || !Directory.Exists(wt.Path)) return; + + IReadOnlyList ownFiles; + try + { + if (await _git.HasChangesAsync(wt.Path, includeUntracked: false, ct)) return; + ownFiles = await _git.GetChangedFileNamesAsync(wt.Path, wt.BaseCommit, "HEAD", ct); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Auto-rebase: could not inspect worktree for task {TaskId}", candidate.Id); + return; + } + + if (!landedFiles.Intersect(ownFiles, StringComparer.OrdinalIgnoreCase).Any()) return; // merely behind + + var (exitCode, stderr, conflictFiles) = await _git.RebaseAsync(wt.Path, targetBranch, ct); + if (exitCode != 0) + { + var detail = conflictFiles.Count > 0 ? $"conflicts in {string.Join(", ", conflictFiles)}" : stderr; + _logger.LogWarning( + "Auto-rebase of task {TaskId} branch {Branch} onto {Target} failed, left as-is: {Detail}", + candidate.Id, wt.BranchName, targetBranch, detail); + await _broadcaster.WorkerLog( + $"Auto-rebase failed for \"{candidate.Title}\" onto {targetBranch} — left as-is ({detail})", + WorkerLogLevel.Warn, DateTime.UtcNow); + return; + } + + var newHead = await _git.RevParseHeadAsync(wt.Path, ct); + using (var ctx = _dbFactory.CreateDbContext()) + await new WorktreeRepository(ctx).SetRebasedAsync(candidate.Id, newTargetTip, newHead, ct); + + await _broadcaster.WorktreeUpdated(candidate.Id); + _logger.LogInformation( + "Auto-rebased task {TaskId} branch {Branch} onto {Target}", candidate.Id, wt.BranchName, targetBranch); + } + public async Task MergeAsync( string taskId, string targetBranch, @@ -198,6 +281,7 @@ public sealed class TaskMergeService catch (Exception ex) { return Blocked($"failed to switch target branch: {ex.Message}"); } } + var oldTargetTip = await _git.RevParseHeadAsync(list.WorkingDir, ct); var (exitCode, stderr) = await _git.MergeNoFfAsync(list.WorkingDir, wt.BranchName, commitMessage, ct); if (exitCode != 0) { @@ -251,6 +335,7 @@ public sealed class TaskMergeService } await MarkWorktreeMergedAsync(taskId, mergeSha, ct); + await RebaseOthersAfterMergeAsync(task, list, targetBranch, oldTargetTip, mergeSha, ct); var verifyFailure = await RunVerifyGateAsync(verifyCommand, list.WorkingDir, ct); if (verifyFailure is not null) @@ -295,6 +380,10 @@ public sealed class TaskMergeService if (!await _git.IsMidMergeAsync(list.WorkingDir, ct)) return Blocked("repo is not mid-merge"); + // HEAD still points at the pre-merge tip here: a conflicted `git merge` never moves it, + // only MERGE_HEAD plus the working tree/index change. + var oldTargetTip = await _git.RevParseHeadAsync(list.WorkingDir, ct); + // Validate BEFORE staging: `git add` marks a conflicted path resolved regardless of // its content, so an unresolved file with markers still in it would otherwise get // staged (and committed) as-is. Check text content for markers first; binary files @@ -327,6 +416,9 @@ public sealed class TaskMergeService var mergeSha = await _git.RevParseHeadAsync(list.WorkingDir, ct); await MarkWorktreeMergedAsync(taskId, mergeSha, ct); + var targetBranch = await _git.GetCurrentBranchAsync(list.WorkingDir, ct); + await RebaseOthersAfterMergeAsync(task, list, targetBranch, oldTargetTip, mergeSha, ct); + var verifyFailure = await RunVerifyGateAsync(verifyCommand, list.WorkingDir, ct); if (verifyFailure is not null) { diff --git a/tests/ClaudeDo.Worker.Tests/Services/TaskMergeServiceTests.cs b/tests/ClaudeDo.Worker.Tests/Services/TaskMergeServiceTests.cs index 2abefead..da8d1f3a 100644 --- a/tests/ClaudeDo.Worker.Tests/Services/TaskMergeServiceTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Services/TaskMergeServiceTests.cs @@ -1149,6 +1149,181 @@ public class TaskMergeServiceTests : IDisposable var wt = await new WorktreeRepository(ctx).GetByTaskIdAsync(task.Id); Assert.Equal(WorktreeState.Merged, wt!.State); } + + [Fact] + public async Task MergeAsync_OverlappingWaitingForReviewBranch_GetsRebasedOntoNewTip() + { + if (!GitRepoFixture.IsGitAvailable()) return; + + var db = NewDb(); + var repo = NewRepo(); + GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main"); + + File.WriteAllText(Path.Combine(repo.RepoDir, "shared.txt"), "line1\nline2\nline3\n"); + GitRepoFixture.RunGit(repo.RepoDir, "add", "-A"); + GitRepoFixture.RunGit(repo.RepoDir, "commit", "-m", "seed shared file"); + var baseCommit = GitRepoFixture.RunGit(repo.RepoDir, "rev-parse", "HEAD").Trim(); + + var (list, taskA) = await SeedListAndTask(db, workingDir: repo.RepoDir, status: TaskStatus.Done); + var wtPathA = Path.Combine(Path.GetTempPath(), $"wt_{Guid.NewGuid():N}"); + _wtCleanups.Add((repo.RepoDir, wtPathA)); + GitRepoFixture.RunGit(repo.RepoDir, "worktree", "add", "-b", "claudedo/reba-a", wtPathA, baseCommit); + File.WriteAllText(Path.Combine(wtPathA, "shared.txt"), "line1-changed\nline2\nline3\n"); + GitRepoFixture.RunGit(wtPathA, "commit", "-am", "A changes line1"); + await SeedWorktree(db, taskA.Id, wtPathA, "claudedo/reba-a", baseCommit); + + // Task B is WaitingForReview and touches the same file A just changed, but a different + // line, so rebasing it onto the new tip should apply cleanly. + var taskB = new TaskEntity + { + Id = Guid.NewGuid().ToString(), + ListId = list.Id, + Title = "task-b", + Status = TaskStatus.WaitingForReview, + CreatedAt = DateTime.UtcNow, + }; + using (var ctx = db.CreateContext()) + await new TaskRepository(ctx).AddAsync(taskB); + + var wtPathB = Path.Combine(Path.GetTempPath(), $"wt_{Guid.NewGuid():N}"); + _wtCleanups.Add((repo.RepoDir, wtPathB)); + GitRepoFixture.RunGit(repo.RepoDir, "worktree", "add", "-b", "claudedo/reba-b", wtPathB, baseCommit); + File.WriteAllText(Path.Combine(wtPathB, "shared.txt"), "line1\nline2\nline3-changed\n"); + GitRepoFixture.RunGit(wtPathB, "commit", "-am", "B changes line3"); + await SeedWorktree(db, taskB.Id, wtPathB, "claudedo/reba-b", baseCommit); + + var (svc, proxy) = BuildService(db); + + var result = await svc.MergeAsync(taskA.Id, "main", removeWorktree: false, "Merge A", CancellationToken.None); + Assert.Equal(TaskMergeService.StatusMerged, result.Status); + + // B's worktree must now carry both changes -- rebased cleanly onto the new main tip. + Assert.Equal("line1-changed\nline2\nline3-changed\n", + File.ReadAllText(Path.Combine(wtPathB, "shared.txt")).Replace("\r\n", "\n")); + + var mainHead = GitRepoFixture.RunGit(repo.RepoDir, "rev-parse", "main").Trim(); + using var ctx2 = db.CreateContext(); + var wtB = await new WorktreeRepository(ctx2).GetByTaskIdAsync(taskB.Id); + Assert.Equal(mainHead, wtB!.BaseCommit); + Assert.Equal(GitRepoFixture.RunGit(wtPathB, "rev-parse", "HEAD").Trim(), wtB.HeadCommit); + + Assert.Contains(proxy.Calls, c => c.Method == "WorktreeUpdated" && c.Args[0] is string s && s == taskB.Id); + } + + [Fact] + public async Task MergeAsync_NonOverlappingWaitingForReviewBranch_IsLeftAlone() + { + if (!GitRepoFixture.IsGitAvailable()) return; + + var db = NewDb(); + var repo = NewRepo(); + GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main"); + var baseCommit = repo.BaseCommit; + + var (list, taskA) = await SeedListAndTask(db, workingDir: repo.RepoDir, status: TaskStatus.Done); + var wtPathA = Path.Combine(Path.GetTempPath(), $"wt_{Guid.NewGuid():N}"); + _wtCleanups.Add((repo.RepoDir, wtPathA)); + GitRepoFixture.RunGit(repo.RepoDir, "worktree", "add", "-b", "claudedo/noov-a", wtPathA, baseCommit); + File.WriteAllText(Path.Combine(wtPathA, "a-only.txt"), "a\n"); + GitRepoFixture.RunGit(wtPathA, "add", "-A"); + GitRepoFixture.RunGit(wtPathA, "commit", "-m", "A adds a-only.txt"); + await SeedWorktree(db, taskA.Id, wtPathA, "claudedo/noov-a", baseCommit); + + // Task B touches a completely different file -- merely behind, not worth disturbing. + var taskB = new TaskEntity + { + Id = Guid.NewGuid().ToString(), + ListId = list.Id, + Title = "task-b", + Status = TaskStatus.WaitingForReview, + CreatedAt = DateTime.UtcNow, + }; + using (var ctx = db.CreateContext()) + await new TaskRepository(ctx).AddAsync(taskB); + + var wtPathB = Path.Combine(Path.GetTempPath(), $"wt_{Guid.NewGuid():N}"); + _wtCleanups.Add((repo.RepoDir, wtPathB)); + GitRepoFixture.RunGit(repo.RepoDir, "worktree", "add", "-b", "claudedo/noov-b", wtPathB, baseCommit); + File.WriteAllText(Path.Combine(wtPathB, "b-only.txt"), "b\n"); + GitRepoFixture.RunGit(wtPathB, "add", "-A"); + GitRepoFixture.RunGit(wtPathB, "commit", "-m", "B adds b-only.txt"); + var headBBefore = GitRepoFixture.RunGit(wtPathB, "rev-parse", "HEAD").Trim(); + await SeedWorktree(db, taskB.Id, wtPathB, "claudedo/noov-b", baseCommit); + + var (svc, proxy) = BuildService(db); + + var result = await svc.MergeAsync(taskA.Id, "main", removeWorktree: false, "Merge A", CancellationToken.None); + Assert.Equal(TaskMergeService.StatusMerged, result.Status); + + // B is untouched: same head, same recorded base commit, no broadcast for B. + Assert.Equal(headBBefore, GitRepoFixture.RunGit(wtPathB, "rev-parse", "HEAD").Trim()); + using var ctx2 = db.CreateContext(); + var wtB = await new WorktreeRepository(ctx2).GetByTaskIdAsync(taskB.Id); + Assert.Equal(baseCommit, wtB!.BaseCommit); + Assert.DoesNotContain(proxy.Calls, c => c.Method == "WorktreeUpdated" && c.Args[0] is string s && s == taskB.Id); + } + + [Fact] + public async Task MergeAsync_OverlappingButConflictingRebase_LeavesBranchAsIsAndWarns() + { + if (!GitRepoFixture.IsGitAvailable()) return; + + var db = NewDb(); + var repo = NewRepo(); + GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main"); + + File.WriteAllText(Path.Combine(repo.RepoDir, "shared.txt"), "line1\nline2\nline3\n"); + GitRepoFixture.RunGit(repo.RepoDir, "add", "-A"); + GitRepoFixture.RunGit(repo.RepoDir, "commit", "-m", "seed shared file"); + var baseCommit = GitRepoFixture.RunGit(repo.RepoDir, "rev-parse", "HEAD").Trim(); + + var (list, taskA) = await SeedListAndTask(db, workingDir: repo.RepoDir, status: TaskStatus.Done); + var wtPathA = Path.Combine(Path.GetTempPath(), $"wt_{Guid.NewGuid():N}"); + _wtCleanups.Add((repo.RepoDir, wtPathA)); + GitRepoFixture.RunGit(repo.RepoDir, "worktree", "add", "-b", "claudedo/conf-a", wtPathA, baseCommit); + File.WriteAllText(Path.Combine(wtPathA, "shared.txt"), "line1\nA-line2\nline3\n"); + GitRepoFixture.RunGit(wtPathA, "commit", "-am", "A changes line2"); + await SeedWorktree(db, taskA.Id, wtPathA, "claudedo/conf-a", baseCommit); + + // Task B changes the exact same line differently -- rebasing must conflict. + var taskB = new TaskEntity + { + Id = Guid.NewGuid().ToString(), + ListId = list.Id, + Title = "task-b", + Status = TaskStatus.WaitingForReview, + CreatedAt = DateTime.UtcNow, + }; + using (var ctx = db.CreateContext()) + await new TaskRepository(ctx).AddAsync(taskB); + + var wtPathB = Path.Combine(Path.GetTempPath(), $"wt_{Guid.NewGuid():N}"); + _wtCleanups.Add((repo.RepoDir, wtPathB)); + GitRepoFixture.RunGit(repo.RepoDir, "worktree", "add", "-b", "claudedo/conf-b", wtPathB, baseCommit); + File.WriteAllText(Path.Combine(wtPathB, "shared.txt"), "line1\nB-line2\nline3\n"); + GitRepoFixture.RunGit(wtPathB, "commit", "-am", "B changes line2"); + var headBBefore = GitRepoFixture.RunGit(wtPathB, "rev-parse", "HEAD").Trim(); + await SeedWorktree(db, taskB.Id, wtPathB, "claudedo/conf-b", baseCommit); + + var (svc, proxy) = BuildService(db); + + var result = await svc.MergeAsync(taskA.Id, "main", removeWorktree: false, "Merge A", CancellationToken.None); + Assert.Equal(TaskMergeService.StatusMerged, result.Status); + + // B is left exactly as it was: same head commit, same content, no mid-rebase state, base + // commit unchanged in the DB, and a clear warning went out instead of silent partial work. + Assert.Equal(headBBefore, GitRepoFixture.RunGit(wtPathB, "rev-parse", "HEAD").Trim()); + Assert.Equal("line1\nB-line2\nline3\n", + File.ReadAllText(Path.Combine(wtPathB, "shared.txt")).Replace("\r\n", "\n")); + Assert.True(string.IsNullOrWhiteSpace(GitRepoFixture.RunGit(wtPathB, "status", "--porcelain"))); + + using var ctx2 = db.CreateContext(); + var wtB = await new WorktreeRepository(ctx2).GetByTaskIdAsync(taskB.Id); + Assert.Equal(baseCommit, wtB!.BaseCommit); + + Assert.Contains(proxy.Calls, c => c.Method == "WorkerLog" + && c.Args[0] is string s && s.Contains("Auto-rebase failed") && s.Contains("task-b")); + } } #region Test doubles