From 519bfbe6b36d1b1e2476d06624c9b4e255a5d141 Mon Sep 17 00:00:00 2001 From: mika kuns Date: Thu, 4 Jun 2026 16:09:44 +0200 Subject: [PATCH] feat(merge): fold parent branch into tree-merge for improvement parents Co-Authored-By: Claude Sonnet 4.6 --- .../Planning/PlanningMergeOrchestrator.cs | 76 ++++++---- .../Planning/TreeMergeTests.cs | 140 ++++++++++++++++++ 2 files changed, 187 insertions(+), 29 deletions(-) create mode 100644 tests/ClaudeDo.Worker.Tests/Planning/TreeMergeTests.cs diff --git a/src/ClaudeDo.Worker/Planning/PlanningMergeOrchestrator.cs b/src/ClaudeDo.Worker/Planning/PlanningMergeOrchestrator.cs index 809db11..d4c0042 100644 --- a/src/ClaudeDo.Worker/Planning/PlanningMergeOrchestrator.cs +++ b/src/ClaudeDo.Worker/Planning/PlanningMergeOrchestrator.cs @@ -22,6 +22,7 @@ public sealed class PlanningMergeOrchestrator { public required string TargetBranch { get; init; } public required Queue RemainingSubtaskIds { get; init; } + public required bool IsPlanning { get; init; } public string? CurrentSubtaskId { get; set; } } @@ -43,32 +44,40 @@ public sealed class PlanningMergeOrchestrator _logger = logger; } - public async Task StartAsync(string planningTaskId, string targetBranch, CancellationToken ct) + public async Task StartAsync(string parentTaskId, string targetBranch, CancellationToken ct) { string workingDir; List children; + bool isPlanning; + bool parentHasWorktree; using (var ctx = _dbFactory.CreateDbContext()) { - var planning = await ctx.Tasks + var parent = await ctx.Tasks .Include(t => t.List) + .Include(t => t.Worktree) .Include(t => t.Children).ThenInclude(c => c.Worktree) - .SingleOrDefaultAsync(t => t.Id == planningTaskId, ct) - ?? throw new KeyNotFoundException($"Planning task '{planningTaskId}' not found."); - workingDir = planning.List.WorkingDir + .SingleOrDefaultAsync(t => t.Id == parentTaskId, ct) + ?? throw new KeyNotFoundException($"Planning task '{parentTaskId}' not found."); + workingDir = parent.List.WorkingDir ?? throw new InvalidOperationException("List has no working directory."); - children = planning.Children.OrderBy(c => c.SortOrder).ToList(); + children = parent.Children.OrderBy(c => c.SortOrder).ToList(); + isPlanning = parent.PlanningPhase != PlanningPhase.None; + parentHasWorktree = parent.Worktree is { State: WorktreeState.Active }; } - foreach (var c in children) + if (isPlanning) { - if (c.Status != TaskStatus.Done) - throw new InvalidOperationException($"subtask {c.Id} is not Done (status {c.Status})"); - if (c.Worktree is null) - throw new InvalidOperationException($"subtask {c.Id} has no worktree"); - if (c.Worktree.State != WorktreeState.Active && c.Worktree.State != WorktreeState.Merged) - throw new InvalidOperationException( - $"subtask {c.Id} worktree state is {c.Worktree.State}"); + foreach (var c in children) + { + if (c.Status != TaskStatus.Done) + throw new InvalidOperationException($"subtask {c.Id} is not Done (status {c.Status})"); + if (c.Worktree is null) + throw new InvalidOperationException($"subtask {c.Id} has no worktree"); + if (c.Worktree.State != WorktreeState.Active && c.Worktree.State != WorktreeState.Merged) + throw new InvalidOperationException( + $"subtask {c.Id} worktree state is {c.Worktree.State}"); + } } if (await _git.IsMidMergeAsync(workingDir, ct)) @@ -76,17 +85,22 @@ public sealed class PlanningMergeOrchestrator if (await _git.HasChangesAsync(workingDir, ct)) throw new InvalidOperationException("working tree has uncommitted changes"); - var queue = new Queue( + var idsToMerge = new List(); + if (!isPlanning && parentHasWorktree) + idsToMerge.Add(parentTaskId); + idsToMerge.AddRange( children - .Where(c => c.Worktree!.State == WorktreeState.Active) + .Where(c => c.Status == TaskStatus.Done && c.Worktree is { State: WorktreeState.Active }) .Select(c => c.Id)); - var state = new State { TargetBranch = targetBranch, RemainingSubtaskIds = queue }; - if (!_states.TryAdd(planningTaskId, state)) - throw new InvalidOperationException($"Merge already in progress for {planningTaskId}."); + var queue = new Queue(idsToMerge); - await _broadcaster.PlanningMergeStarted(planningTaskId, targetBranch); - await DrainAsync(planningTaskId, ct); + var state = new State { TargetBranch = targetBranch, RemainingSubtaskIds = queue, IsPlanning = isPlanning }; + if (!_states.TryAdd(parentTaskId, state)) + throw new InvalidOperationException($"Merge already in progress for {parentTaskId}."); + + await _broadcaster.PlanningMergeStarted(parentTaskId, targetBranch); + await DrainAsync(parentTaskId, ct); } public async Task ContinueAsync(string planningTaskId, CancellationToken ct) @@ -167,7 +181,7 @@ public sealed class PlanningMergeOrchestrator } state.CurrentSubtaskId = null; - await FinalizePlanningDoneAsync(planningTaskId, ct); + await FinalizeParentDoneAsync(planningTaskId, state.IsPlanning, ct); await _broadcaster.PlanningCompleted(planningTaskId); } finally @@ -176,16 +190,20 @@ public sealed class PlanningMergeOrchestrator } } - private async Task FinalizePlanningDoneAsync(string planningTaskId, CancellationToken ct) + private async Task FinalizeParentDoneAsync(string parentTaskId, bool isPlanning, CancellationToken ct) { using var ctx = _dbFactory.CreateDbContext(); - var planning = await ctx.Tasks.SingleOrDefaultAsync(t => t.Id == planningTaskId, ct); - if (planning is null) return; - planning.Status = TaskStatus.Done; - planning.FinishedAt = DateTime.UtcNow; + var parent = await ctx.Tasks.SingleOrDefaultAsync(t => t.Id == parentTaskId, ct); + if (parent is null) return; + parent.Status = TaskStatus.Done; + parent.FinishedAt = DateTime.UtcNow; await ctx.SaveChangesAsync(ct); - try { await _aggregator.CleanupIntegrationBranchAsync(planningTaskId, ct); } - catch (Exception ex) { _logger.LogWarning(ex, "integration branch cleanup failed"); } + // Only planning builds an integration branch via the aggregator; skip cleanup otherwise. + if (isPlanning) + { + try { await _aggregator.CleanupIntegrationBranchAsync(parentTaskId, ct); } + catch (Exception ex) { _logger.LogWarning(ex, "integration branch cleanup failed"); } + } } } diff --git a/tests/ClaudeDo.Worker.Tests/Planning/TreeMergeTests.cs b/tests/ClaudeDo.Worker.Tests/Planning/TreeMergeTests.cs new file mode 100644 index 0000000..b3b03b0 --- /dev/null +++ b/tests/ClaudeDo.Worker.Tests/Planning/TreeMergeTests.cs @@ -0,0 +1,140 @@ +using ClaudeDo.Data; +using ClaudeDo.Data.Git; +using ClaudeDo.Data.Models; +using ClaudeDo.Worker.Hub; +using ClaudeDo.Worker.Lifecycle; +using ClaudeDo.Worker.Planning; +using ClaudeDo.Worker.Tests.Infrastructure; +using Microsoft.AspNetCore.SignalR; +using Microsoft.Extensions.Logging.Abstractions; +using TaskStatus = ClaudeDo.Data.Models.TaskStatus; + +namespace ClaudeDo.Worker.Tests.Planning; + +file sealed class TreeMergeRecordingHubClients : IHubClients +{ + public TreeMergeRecordingClientProxy Proxy { get; } = new(); + public IClientProxy All => Proxy; + public IClientProxy AllExcept(IReadOnlyList excludedConnectionIds) => Proxy; + public IClientProxy Client(string connectionId) => Proxy; + public IClientProxy Clients(IReadOnlyList connectionIds) => Proxy; + public IClientProxy Group(string groupName) => Proxy; + public IClientProxy GroupExcept(string groupName, IReadOnlyList excludedConnectionIds) => Proxy; + public IClientProxy Groups(IReadOnlyList groupNames) => Proxy; + public IClientProxy User(string userId) => Proxy; + public IClientProxy Users(IReadOnlyList userIds) => Proxy; +} + +file sealed class TreeMergeRecordingClientProxy : IClientProxy +{ + public List<(string Method, object?[] Args)> Calls { get; } = new(); + public Task SendCoreAsync(string method, object?[] args, CancellationToken cancellationToken = default) + { + Calls.Add((method, args)); + return Task.CompletedTask; + } +} + +file sealed class TreeMergeFakeHubContext : IHubContext +{ + public TreeMergeRecordingHubClients RecordingClients { get; } = new(); + public IHubClients Clients => RecordingClients; + public IGroupManager Groups => throw new NotImplementedException(); +} + +public sealed class TreeMergeTests : IDisposable +{ + private readonly List _dbs = new(); + private readonly List _repos = new(); + private readonly List<(string repoDir, string wtPath)> _wtCleanups = new(); + + private DbFixture NewDb() { var d = new DbFixture(); _dbs.Add(d); return d; } + private GitRepoFixture NewRepo() { var r = new GitRepoFixture(); _repos.Add(r); return r; } + + public void Dispose() + { + foreach (var (repo, wt) in _wtCleanups) + try { GitRepoFixture.RunGit(repo, "worktree", "remove", "--force", wt); } catch { } + foreach (var d in _dbs) try { d.Dispose(); } catch { } + foreach (var r in _repos) try { r.Dispose(); } catch { } + } + + [Fact] + public async Task ImprovementParent_foldsOwnBranch_thenChild_andMarksDone() + { + if (!GitRepoFixture.IsGitAvailable()) { Assert.True(true, "git not available"); return; } + var db = NewDb(); + var repo = NewRepo(); + GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main"); + + var listId = Guid.NewGuid().ToString(); + var parentId = Guid.NewGuid().ToString(); + var childId = Guid.NewGuid().ToString(); + + using (var ctx = db.CreateContext()) + { + ctx.Lists.Add(new ListEntity { Id = listId, Name = "L", WorkingDir = repo.RepoDir, CreatedAt = DateTime.UtcNow }); + ctx.Tasks.Add(new TaskEntity { Id = parentId, ListId = listId, Title = "Parent", + Status = TaskStatus.WaitingForReview, PlanningPhase = PlanningPhase.None, SortOrder = 0, CreatedAt = DateTime.UtcNow }); + ctx.Tasks.Add(new TaskEntity { Id = childId, ListId = listId, Title = "Child", + Status = TaskStatus.Done, ParentTaskId = parentId, SortOrder = 1, CreatedAt = DateTime.UtcNow }); + await ctx.SaveChangesAsync(); + + var parentWt = SeedWorktree(repo, parentId, repo.BaseCommit, "parent.txt", "parent work"); + var childWt = SeedWorktree(repo, childId, parentWt.head, "child.txt", "child work"); + ctx.Worktrees.Add(MakeRow(parentId, parentWt)); + ctx.Worktrees.Add(MakeRow(childId, childWt)); + await ctx.SaveChangesAsync(); + } + + var (orch, calls) = BuildOrchestrator(db); + await orch.StartAsync(parentId, "main", CancellationToken.None); + + using var verify = db.CreateContext(); + Assert.Equal(TaskStatus.Done, verify.Tasks.Single(t => t.Id == parentId).Status); + Assert.Equal(WorktreeState.Merged, verify.Worktrees.Single(w => w.TaskId == parentId).State); + Assert.Equal(WorktreeState.Merged, verify.Worktrees.Single(w => w.TaskId == childId).State); + Assert.Contains(calls, c => c.Method == "PlanningSubtaskMerged" && (string)c.Args[1]! == parentId); + Assert.Contains(calls, c => c.Method == "PlanningSubtaskMerged" && (string)c.Args[1]! == childId); + Assert.Contains(calls, c => c.Method == "PlanningCompleted"); + Assert.True(File.Exists(Path.Combine(repo.RepoDir, "parent.txt"))); + Assert.True(File.Exists(Path.Combine(repo.RepoDir, "child.txt"))); + } + + private (string path, string branch, string head) SeedWorktree( + GitRepoFixture repo, string taskId, string baseCommit, string file, string content) + { + var wtPath = Path.Combine(Path.GetTempPath(), $"wt_{Guid.NewGuid():N}"); + _wtCleanups.Add((repo.RepoDir, wtPath)); + var branch = $"claudedo/{taskId.Replace("-", "")}"; + GitRepoFixture.RunGit(repo.RepoDir, "worktree", "add", "-b", branch, wtPath, baseCommit); + File.WriteAllText(Path.Combine(wtPath, file), content); + GitRepoFixture.RunGit(wtPath, "add", file); + GitRepoFixture.RunGit(wtPath, "commit", "-m", $"add {file}"); + var head = GitRepoFixture.RunGit(wtPath, "rev-parse", "HEAD").Trim(); + return (wtPath, branch, head); + } + + private static WorktreeEntity MakeRow(string taskId, (string path, string branch, string head) wt) + => new() { TaskId = taskId, Path = wt.path, BranchName = wt.branch, BaseCommit = "x", + HeadCommit = wt.head, State = WorktreeState.Active, CreatedAt = DateTime.UtcNow }; + + private (PlanningMergeOrchestrator orch, List<(string Method, object?[] Args)> calls) BuildOrchestrator(DbFixture db) + { + var fakeHub = new TreeMergeFakeHubContext(); + var spy = fakeHub.RecordingClients.Proxy; + var broadcaster = new HubBroadcaster(fakeHub); + var git = new GitService(); + var factory = db.CreateFactory(); + var merge = new TaskMergeService( + factory, git, broadcaster, + NullLogger.Instance); + var aggregator = new PlanningAggregator( + factory, git, + NullLogger.Instance); + var orch = new PlanningMergeOrchestrator( + factory, merge, aggregator, broadcaster, git, + NullLogger.Instance); + return (orch, spy.Calls); + } +}