Files
ClaudeDo/tests/ClaudeDo.Worker.Tests/Planning/PlanningMergeOrchestratorTests.cs
T
mika kuns f205843020 fix(worker): propagate unit-merge failures instead of reporting success
A child merge that came back blocked/verify_failed/untracked_collision during a
parent/children unit merge used to vanish: DrainAsync only logged it server-side,
PlanningMergeAborted carried no reason, and ApproveReview/review_task always
reported StatusMerged for a task with children regardless of the real outcome,
so a failed unit merge left the parent stuck with no visible error.

- PlanningMergeOrchestrator.StartAsync/ContinueAsync/DrainAsync now return a
  PlanningMergeResult(Status, Reason) instead of void, and PlanningMergeAborted
  carries that reason to the UI.
- WorkerHub.ApproveReview and ExternalMcpService.ReviewTask's approve branch
  propagate the real status/reason for a parent with children instead of
  hardcoding "merged" (or masking a non-conflict failure as "conflict").
- StartAsync now requires the parent to already be WaitingForReview for
  improvement parents too, not just planning ones, so a stale caller can no
  longer trigger a partial child merge.
- HasActiveMerge now also covers the window between the last child merging and
  FinalizeParentDoneAsync completing, closing a gap where a concurrent Cancel
  could race the parent's own approve-to-Done transition.
- IslandsShellViewModel.OnPlanningMergeAborted flashes the reason via
  FlashFooterError instead of only clearing the external-merge banner.
2026-08-20 15:02:17 +02:00

882 lines
40 KiB
C#

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.State;
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 OrchestratorRecordingHubClients : IHubClients
{
public OrchestratorRecordingClientProxy Proxy { get; } = new();
public IClientProxy All => Proxy;
public IClientProxy AllExcept(IReadOnlyList<string> excludedConnectionIds) => Proxy;
public IClientProxy Client(string connectionId) => Proxy;
public IClientProxy Clients(IReadOnlyList<string> connectionIds) => Proxy;
public IClientProxy Group(string groupName) => Proxy;
public IClientProxy GroupExcept(string groupName, IReadOnlyList<string> excludedConnectionIds) => Proxy;
public IClientProxy Groups(IReadOnlyList<string> groupNames) => Proxy;
public IClientProxy User(string userId) => Proxy;
public IClientProxy Users(IReadOnlyList<string> userIds) => Proxy;
}
file sealed class OrchestratorRecordingClientProxy : 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 OrchestratorFakeHubContext : IHubContext<WorkerHub>
{
public OrchestratorRecordingHubClients RecordingClients { get; } = new();
public IHubClients Clients => RecordingClients;
public IGroupManager Groups => throw new NotImplementedException();
}
public sealed class PlanningMergeOrchestratorTests : IDisposable
{
private readonly List<DbFixture> _dbs = new();
private readonly List<GitRepoFixture> _repos = new();
private readonly List<(string repoDir, string wtPath)> _wtCleanups = new();
private int _numberSeed;
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 StartAsync_AllChildrenMergeCleanly_MarksPlanningDoneAndEmitsCompleted()
{
var db = NewDb();
var repo = NewRepo();
GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main");
var (parentId, subA, subB) = await SeedPlanningWithTwoNonConflictingChildrenAsync(db, repo);
var (orch, calls) = BuildOrchestrator(db);
var result = await orch.StartAsync(parentId, "main", CancellationToken.None);
Assert.Equal(TaskMergeService.StatusMerged, result.Status);
Assert.Null(result.Reason);
using var ctx = db.CreateContext();
var planning = ctx.Tasks.Single(t => t.Id == parentId);
Assert.Equal(TaskStatus.Done, planning.Status);
Assert.NotNull(planning.FinishedAt);
Assert.Equal(WorktreeState.Merged, ctx.Worktrees.Single(w => w.TaskId == subA).State);
Assert.Equal(WorktreeState.Merged, ctx.Worktrees.Single(w => w.TaskId == subB).State);
Assert.Contains(calls, c => c.Method == "PlanningMergeStarted");
Assert.Equal(2, calls.Count(c => c.Method == "PlanningSubtaskMerged"));
Assert.Contains(calls, c => c.Method == "PlanningCompleted" && (string)c.Args[0]! == parentId);
}
private async Task<(string parentId, string subA, string subB)> SeedPlanningWithTwoNonConflictingChildrenAsync(
DbFixture db, GitRepoFixture repo)
{
using var ctx = db.CreateContext();
var listId = Guid.NewGuid().ToString();
ctx.Lists.Add(new ListEntity
{
Id = listId, Name = "test", CreatedAt = DateTime.UtcNow,
WorkingDir = repo.RepoDir,
});
var parentId = Guid.NewGuid().ToString();
ctx.Tasks.Add(new TaskEntity
{
Id = parentId, ListId = listId, Title = "plan", CreatedAt = DateTime.UtcNow,
Status = TaskStatus.WaitingForReview, PlanningPhase = PlanningPhase.Finalized, SortOrder = 0,
Number = ++_numberSeed,
});
var subA = Guid.NewGuid().ToString();
var subB = Guid.NewGuid().ToString();
ctx.Tasks.Add(new TaskEntity
{
Id = subA, ListId = listId, Title = "child A", CreatedAt = DateTime.UtcNow,
ParentTaskId = parentId, Status = TaskStatus.Done, SortOrder = 1,
Number = ++_numberSeed,
});
ctx.Tasks.Add(new TaskEntity
{
Id = subB, ListId = listId, Title = "child B", CreatedAt = DateTime.UtcNow,
ParentTaskId = parentId, Status = TaskStatus.Done, SortOrder = 2,
Number = ++_numberSeed,
});
await ctx.SaveChangesAsync();
SeedWorktree(ctx, repo, subA, "fileA.txt", "content A");
SeedWorktree(ctx, repo, subB, "fileB.txt", "content B");
await ctx.SaveChangesAsync();
return (parentId, subA, subB);
}
[Fact]
public async Task ContinueAsync_AfterConflict_ResumesRemainingMergesAndCompletes()
{
var db = NewDb();
var repo = NewRepo();
GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main");
var (parentId, subA, subB, subC) = await SeedPlanningThreeChildrenMiddleConflictsAsync(db, repo);
var (orch, spy) = BuildOrchestrator(db);
var startResult = await orch.StartAsync(parentId, "main", CancellationToken.None);
Assert.Equal(TaskMergeService.StatusConflict, startResult.Status);
Assert.Contains(spy, c => c.Method == "PlanningSubtaskMerged" && (string)c.Args[1]! == subA);
Assert.Contains(spy, c => c.Method == "PlanningMergeConflict" && (string)c.Args[1]! == subB);
File.WriteAllText(Path.Combine(repo.RepoDir, "README.md"), "resolved\n");
var continueResult = await orch.ContinueAsync(parentId, CancellationToken.None);
Assert.Equal(TaskMergeService.StatusMerged, continueResult.Status);
using var ctx = db.CreateContext();
Assert.Equal(TaskStatus.Done, ctx.Tasks.Single(t => t.Id == parentId).Status);
Assert.Equal(WorktreeState.Merged, ctx.Worktrees.Single(w => w.TaskId == subB).State);
Assert.Equal(WorktreeState.Merged, ctx.Worktrees.Single(w => w.TaskId == subC).State);
Assert.Contains(spy, c => c.Method == "PlanningSubtaskMerged" && (string)c.Args[1]! == subB);
Assert.Contains(spy, c => c.Method == "PlanningSubtaskMerged" && (string)c.Args[1]! == subC);
Assert.Contains(spy, c => c.Method == "PlanningCompleted");
}
private async Task<(string parentId, string subA, string subB, string subC)> SeedPlanningThreeChildrenMiddleConflictsAsync(
DbFixture db, GitRepoFixture repo)
{
File.WriteAllText(Path.Combine(repo.RepoDir, "README.md"), "# main change\n");
GitRepoFixture.RunGit(repo.RepoDir, "commit", "-am", "main change README");
using var ctx = db.CreateContext();
var listId = Guid.NewGuid().ToString();
ctx.Lists.Add(new ListEntity
{
Id = listId, Name = "test", CreatedAt = DateTime.UtcNow, WorkingDir = repo.RepoDir,
});
var parentId = Guid.NewGuid().ToString();
ctx.Tasks.Add(new TaskEntity
{
Id = parentId, ListId = listId, Title = "plan", CreatedAt = DateTime.UtcNow,
Status = TaskStatus.WaitingForReview, PlanningPhase = PlanningPhase.Finalized, SortOrder = 0,
Number = ++_numberSeed,
});
var subA = Guid.NewGuid().ToString();
var subB = Guid.NewGuid().ToString();
var subC = Guid.NewGuid().ToString();
ctx.Tasks.AddRange(
new TaskEntity { Id = subA, ListId = listId, Title = "A", CreatedAt = DateTime.UtcNow, ParentTaskId = parentId, Status = TaskStatus.Done, SortOrder = 1, Number = ++_numberSeed },
new TaskEntity { Id = subB, ListId = listId, Title = "B", CreatedAt = DateTime.UtcNow, ParentTaskId = parentId, Status = TaskStatus.Done, SortOrder = 2, Number = ++_numberSeed },
new TaskEntity { Id = subC, ListId = listId, Title = "C", CreatedAt = DateTime.UtcNow, ParentTaskId = parentId, Status = TaskStatus.Done, SortOrder = 3, Number = ++_numberSeed }
);
await ctx.SaveChangesAsync();
SeedWorktreeWithFile(ctx, repo, subA, "fileA.txt", "A\n");
SeedWorktreeWithFile(ctx, repo, subB, "README.md", "branch change\n");
SeedWorktreeWithFile(ctx, repo, subC, "fileC.txt", "C\n");
await ctx.SaveChangesAsync();
return (parentId, subA, subB, subC);
}
private void SeedWorktreeWithFile(ClaudeDoDbContext ctx, GitRepoFixture repo, string taskId, string filename, string content)
=> SeedWorktree(ctx, repo, taskId, filename, content);
private void SeedWorktree(ClaudeDoDbContext ctx, GitRepoFixture repo, string taskId, string filename, string content)
{
var wtPath = Path.Combine(Path.GetTempPath(), $"wt_{Guid.NewGuid():N}");
_wtCleanups.Add((repo.RepoDir, wtPath));
var branch = $"claudedo/{taskId[..8]}";
GitRepoFixture.RunGit(repo.RepoDir, "worktree", "add", "-b", branch, wtPath, repo.BaseCommit);
File.WriteAllText(Path.Combine(wtPath, filename), content);
GitRepoFixture.RunGit(wtPath, "add", filename);
GitRepoFixture.RunGit(wtPath, "commit", "-m", $"add {filename}");
var head = GitRepoFixture.RunGit(wtPath, "rev-parse", "HEAD").Trim();
ctx.Worktrees.Add(new WorktreeEntity
{
TaskId = taskId,
Path = wtPath,
BranchName = branch,
BaseCommit = repo.BaseCommit,
HeadCommit = head,
DiffStat = null,
State = WorktreeState.Active,
CreatedAt = DateTime.UtcNow,
});
}
[Fact]
public async Task AbortAsync_AfterConflict_RestoresCleanRepoAndClearsState()
{
var db = NewDb();
var repo = NewRepo();
GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main");
var (parentId, subA, subB, _) = await SeedPlanningThreeChildrenMiddleConflictsAsync(db, repo);
var (orch, spy) = BuildOrchestrator(db);
await orch.StartAsync(parentId, "main", CancellationToken.None);
await orch.AbortAsync(parentId, CancellationToken.None);
using var ctx = db.CreateContext();
// Planning stays in Planned — NOT flipped to Done.
Assert.Equal(PlanningPhase.Finalized, ctx.Tasks.Single(t => t.Id == parentId).PlanningPhase);
// Earlier successful merge stays merged.
Assert.Equal(WorktreeState.Merged, ctx.Worktrees.Single(w => w.TaskId == subA).State);
// Conflicted subtask's worktree stays Active (abort doesn't flip it).
Assert.Equal(WorktreeState.Active, ctx.Worktrees.Single(w => w.TaskId == subB).State);
Assert.Contains(spy, c => c.Method == "PlanningMergeAborted" && (string)c.Args[0]! == parentId);
// Repo no longer mid-merge.
var git = new GitService();
Assert.False(await git.IsMidMergeAsync(repo.RepoDir, CancellationToken.None));
}
// ─── externallyDriven (MCP-driven merges must not auto-open the UI resolver) ────────────
[Fact]
public async Task StartAsync_ExternallyDriven_ConflictBroadcastFlagsExternallyDriven()
{
var db = NewDb();
var repo = NewRepo();
GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main");
var (parentId, _, _, _) = await SeedPlanningThreeChildrenMiddleConflictsAsync(db, repo);
var (orch, spy) = BuildOrchestrator(db);
await orch.StartAsync(parentId, "main", CancellationToken.None, externallyDriven: true);
var conflictCall = Assert.Single(spy, c => c.Method == "PlanningMergeConflict");
Assert.True((bool)conflictCall.Args[3]!);
}
[Fact]
public async Task StartAsync_DefaultNotExternallyDriven_ConflictBroadcastFlagsFalse()
{
var db = NewDb();
var repo = NewRepo();
GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main");
var (parentId, _, _, _) = await SeedPlanningThreeChildrenMiddleConflictsAsync(db, repo);
var (orch, spy) = BuildOrchestrator(db);
await orch.StartAsync(parentId, "main", CancellationToken.None);
var conflictCall = Assert.Single(spy, c => c.Method == "PlanningMergeConflict");
Assert.False((bool)conflictCall.Args[3]!);
}
[Fact]
public async Task GetActiveExternalConflictsAsync_ExternallyDrivenAndMidMerge_ReturnsEntry()
{
var db = NewDb();
var repo = NewRepo();
GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main");
var (parentId, _, subB, _) = await SeedPlanningThreeChildrenMiddleConflictsAsync(db, repo);
var (orch, _) = BuildOrchestrator(db);
await orch.StartAsync(parentId, "main", CancellationToken.None, externallyDriven: true);
var active = await orch.GetActiveExternalConflictsAsync(CancellationToken.None);
var entry = Assert.Single(active);
Assert.Equal(parentId, entry.PlanningTaskId);
Assert.Equal(subB, entry.SubtaskId);
}
[Fact]
public async Task GetActiveExternalConflictsAsync_UiDriven_ReturnsEmpty()
{
var db = NewDb();
var repo = NewRepo();
GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main");
var (parentId, _, _, _) = await SeedPlanningThreeChildrenMiddleConflictsAsync(db, repo);
var (orch, _) = BuildOrchestrator(db);
await orch.StartAsync(parentId, "main", CancellationToken.None); // externallyDriven defaults false
var active = await orch.GetActiveExternalConflictsAsync(CancellationToken.None);
Assert.Empty(active);
}
/// <summary>
/// The in-memory ExternallyDriven flag alone must never be trusted: if the repo was already
/// taken out of mid-merge through some other path (e.g. the driving Claude session died and
/// something else ran `git merge --abort` directly, without going through AbortAsync), the
/// query must not report a phantom external conflict. Proves the MERGE_HEAD coupling.
/// </summary>
[Fact]
public async Task GetActiveExternalConflictsAsync_StaleStateRepoNoLongerMidMerge_SelfHeals()
{
var db = NewDb();
var repo = NewRepo();
GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main");
var (parentId, _, _, _) = await SeedPlanningThreeChildrenMiddleConflictsAsync(db, repo);
var (orch, _) = BuildOrchestrator(db);
await orch.StartAsync(parentId, "main", CancellationToken.None, externallyDriven: true);
GitRepoFixture.RunGit(repo.RepoDir, "merge", "--abort");
var active = await orch.GetActiveExternalConflictsAsync(CancellationToken.None);
Assert.Empty(active);
}
// ─── Stateless abort (post-restart recovery) ───────────────────────────
/// <summary>
/// Worker restarted while a conflict was paused: _states is empty but the list repo is
/// still mid-merge. AbortAsync must abort the dangling merge, broadcast PlanningMergeAborted,
/// and leave the parent in WaitingForReview so a fresh Approve can retry.
/// </summary>
[Fact]
public async Task AbortAsync_NoState_RepoMidMerge_AbortsAndBroadcasts()
{
var db = NewDb();
var repo = NewRepo();
GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main");
var (parentId, _, subB, _) = await SeedPlanningThreeChildrenMiddleConflictsAsync(db, repo);
// Drive orch1 into the conflict pause — repo is now mid-merge.
var (orch1, _) = BuildOrchestrator(db);
await orch1.StartAsync(parentId, "main", CancellationToken.None);
// Simulate restart: fresh orchestrator has no in-memory state.
var (orch2, spy) = BuildOrchestrator(db);
await orch2.AbortAsync(parentId, CancellationToken.None);
var git = new GitService();
Assert.False(await git.IsMidMergeAsync(repo.RepoDir, CancellationToken.None));
using var ctx = db.CreateContext();
Assert.Equal(TaskStatus.WaitingForReview, ctx.Tasks.Single(t => t.Id == parentId).Status);
Assert.Contains(spy, c => c.Method == "PlanningMergeAborted" && (string)c.Args[0]! == parentId);
}
/// <summary>
/// No in-memory state and repo is clean — nothing to abort. Must throw a clear error.
/// </summary>
[Fact]
public async Task AbortAsync_NoState_RepoNotMidMerge_ThrowsClear()
{
var db = NewDb();
var repo = NewRepo();
GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main");
var (parentId, _, _) = await SeedPlanningWithTwoNonConflictingChildrenAsync(db, repo);
var (orch, _) = BuildOrchestrator(db);
var ex = await Assert.ThrowsAsync<InvalidOperationException>(
() => orch.AbortAsync(parentId, CancellationToken.None));
Assert.Contains("no in-progress merge", ex.Message);
}
private (PlanningMergeOrchestrator orch, List<(string Method, object?[] Args)> calls) BuildOrchestrator(DbFixture db)
{
var fakeHub = new OrchestratorFakeHubContext();
var spy = fakeHub.RecordingClients.Proxy;
var broadcaster = new HubBroadcaster(fakeHub);
var git = new GitService();
var factory = db.CreateFactory();
var built = TaskStateServiceBuilder.Build(factory);
var merge = new TaskMergeService(
factory, git, broadcaster,
built.State,
new VerifyCommandRunner(),
NullLogger<TaskMergeService>.Instance);
var aggregator = new PlanningAggregator(
factory, git,
NullLogger<PlanningAggregator>.Instance);
var orch = new PlanningMergeOrchestrator(
factory, merge, aggregator, broadcaster, git,
built.State,
NullLogger<PlanningMergeOrchestrator>.Instance);
return (orch, spy.Calls);
}
[Fact]
public async Task StartAsync_SubtaskStillRunning_ThrowsWithoutSideEffects()
{
var db = NewDb();
var repo = NewRepo();
GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main");
var (parentId, runningSub) = await SeedPlanningWithOneRunningChildAsync(db, repo);
var (orch, spy) = BuildOrchestrator(db);
var ex = await Assert.ThrowsAsync<InvalidOperationException>(
() => orch.StartAsync(parentId, "main", CancellationToken.None));
Assert.Contains(runningSub, ex.Message);
using var ctx = db.CreateContext();
Assert.Equal(PlanningPhase.Finalized, ctx.Tasks.Single(t => t.Id == parentId).PlanningPhase);
Assert.Empty(spy);
}
[Fact]
public async Task StartAsync_DirtyRepo_ThrowsWithoutSideEffects()
{
var db = NewDb();
var repo = NewRepo();
GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main");
var (parentId, _, _) = await SeedPlanningWithTwoNonConflictingChildrenAsync(db, repo);
// Modify a tracked file (real uncommitted change).
File.WriteAllText(Path.Combine(repo.RepoDir, "README.md"), "unstaged\n");
var (orch, _) = BuildOrchestrator(db);
var ex = await Assert.ThrowsAsync<InvalidOperationException>(
() => orch.StartAsync(parentId, "main", CancellationToken.None));
Assert.Contains("uncommitted", ex.Message);
}
[Fact]
public async Task StartAsync_UntrackedFileInRepo_DoesNotThrow()
{
var db = NewDb();
var repo = NewRepo();
GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main");
var (parentId, subA, subB) = await SeedPlanningWithTwoNonConflictingChildrenAsync(db, repo);
// An untracked file (e.g. left behind by a concurrent session) must not block the merge.
File.WriteAllText(Path.Combine(repo.RepoDir, "untracked.txt"), "stray\n");
var (orch, _) = BuildOrchestrator(db);
await orch.StartAsync(parentId, "main", CancellationToken.None);
using var ctx = db.CreateContext();
Assert.Equal(TaskStatus.Done, ctx.Tasks.Single(t => t.Id == parentId).Status);
Assert.Equal(WorktreeState.Merged, ctx.Worktrees.Single(w => w.TaskId == subA).State);
Assert.Equal(WorktreeState.Merged, ctx.Worktrees.Single(w => w.TaskId == subB).State);
}
[Fact]
public async Task StartAsync_IdempotentRestart_SkipsAlreadyMergedWorktrees()
{
var db = NewDb();
var repo = NewRepo();
GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main");
var (parentId, subA, subB) = await SeedPlanningWithTwoNonConflictingChildrenAsync(db, repo);
using (var setup = db.CreateContext())
{
var wt = setup.Worktrees.Single(w => w.TaskId == subA);
wt.State = WorktreeState.Merged;
await setup.SaveChangesAsync();
}
var (orch, spy) = BuildOrchestrator(db);
await orch.StartAsync(parentId, "main", CancellationToken.None);
Assert.DoesNotContain(spy, c => c.Method == "PlanningSubtaskMerged" && (string)c.Args[1]! == subA);
Assert.Contains(spy, c => c.Method == "PlanningSubtaskMerged" && (string)c.Args[1]! == subB);
Assert.Contains(spy, c => c.Method == "PlanningCompleted");
}
private async Task<(string parentId, string runningChild)> SeedPlanningWithOneRunningChildAsync(
DbFixture db, GitRepoFixture repo)
{
using var ctx = db.CreateContext();
var listId = Guid.NewGuid().ToString();
ctx.Lists.Add(new ListEntity
{
Id = listId, Name = "test", CreatedAt = DateTime.UtcNow, WorkingDir = repo.RepoDir,
});
var parentId = Guid.NewGuid().ToString();
ctx.Tasks.Add(new TaskEntity
{
Id = parentId, ListId = listId, Title = "plan", CreatedAt = DateTime.UtcNow,
Status = TaskStatus.Idle, PlanningPhase = PlanningPhase.Finalized, SortOrder = 0,
Number = ++_numberSeed,
});
var running = Guid.NewGuid().ToString();
ctx.Tasks.Add(new TaskEntity
{
Id = running, ListId = listId, Title = "still running",
CreatedAt = DateTime.UtcNow, ParentTaskId = parentId,
Status = TaskStatus.Running, SortOrder = 1,
Number = ++_numberSeed,
});
await ctx.SaveChangesAsync();
SeedWorktreeWithFile(ctx, repo, running, "fileR.txt", "R\n");
await ctx.SaveChangesAsync();
return (parentId, running);
}
// ─── ApproveReview routing ──────────────────────────────────────────────
/// <summary>
/// Improvement parent (PlanningPhase.None) in WaitingForReview with two Done children
/// that each have an Active worktree → orchestrator merges both and marks the parent Done.
/// This mirrors the ApproveReview hub path for a parent-with-children.
/// </summary>
[Fact]
public async Task StartAsync_ImprovementParentInWaitingForReview_MergesBothChildrenAndLeavesDone()
{
var db = NewDb();
var repo = NewRepo();
GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main");
var (parentId, subA, subB) = await SeedImprovementParentWithTwoDoneChildrenAsync(db, repo);
var (orch, calls) = BuildOrchestrator(db);
await orch.StartAsync(parentId, "main", CancellationToken.None);
using var ctx = db.CreateContext();
var parent = ctx.Tasks.Single(t => t.Id == parentId);
Assert.Equal(TaskStatus.Done, parent.Status);
Assert.NotNull(parent.FinishedAt);
Assert.Equal(WorktreeState.Merged, ctx.Worktrees.Single(w => w.TaskId == subA).State);
Assert.Equal(WorktreeState.Merged, ctx.Worktrees.Single(w => w.TaskId == subB).State);
Assert.Contains(calls, c => c.Method == "PlanningMergeStarted");
Assert.Equal(2, calls.Count(c => c.Method == "PlanningSubtaskMerged"));
Assert.Contains(calls, c => c.Method == "PlanningCompleted" && (string)c.Args[0]! == parentId);
}
private async Task<(string parentId, string subA, string subB)> SeedImprovementParentWithTwoDoneChildrenAsync(
DbFixture db, GitRepoFixture repo)
{
using var ctx = db.CreateContext();
var listId = Guid.NewGuid().ToString();
ctx.Lists.Add(new ListEntity
{
Id = listId, Name = "test", CreatedAt = DateTime.UtcNow,
WorkingDir = repo.RepoDir,
});
var parentId = Guid.NewGuid().ToString();
// Improvement parent: PlanningPhase.None, status WaitingForReview (after children finished)
ctx.Tasks.Add(new TaskEntity
{
Id = parentId, ListId = listId, Title = "improve", CreatedAt = DateTime.UtcNow,
Status = TaskStatus.WaitingForReview, PlanningPhase = PlanningPhase.None, SortOrder = 0,
Number = ++_numberSeed,
});
var subA = Guid.NewGuid().ToString();
var subB = Guid.NewGuid().ToString();
ctx.Tasks.Add(new TaskEntity
{
Id = subA, ListId = listId, Title = "child A", CreatedAt = DateTime.UtcNow,
ParentTaskId = parentId, Status = TaskStatus.Done, SortOrder = 1,
Number = ++_numberSeed,
});
ctx.Tasks.Add(new TaskEntity
{
Id = subB, ListId = listId, Title = "child B", CreatedAt = DateTime.UtcNow,
ParentTaskId = parentId, Status = TaskStatus.Done, SortOrder = 2,
Number = ++_numberSeed,
});
await ctx.SaveChangesAsync();
SeedWorktree(ctx, repo, subA, "fileA.txt", "content A");
SeedWorktree(ctx, repo, subB, "fileB.txt", "content B");
await ctx.SaveChangesAsync();
return (parentId, subA, subB);
}
/// <summary>
/// Guard (a): StartAsync now requires the parent to be WaitingForReview up front, for
/// improvement parents as much as planning ones. Before this guard existed, a parent that had
/// already left WaitingForReview (e.g. cancelled by a race, or a stale second Approve click)
/// still had its children merged during the drain, only to have the final ApproveReviewAsync
/// refuse at the very end — by then the child worktrees were already unrecoverably merged.
/// The fixed behaviour rejects up front: nothing gets touched.
/// </summary>
[Fact]
public async Task StartAsync_ParentNotWaitingForReview_ThrowsWithoutMergingChildren()
{
var db = NewDb();
var repo = NewRepo();
GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main");
// Improvement parent (PlanningPhase.None) seeded as Cancelled.
var (parentId, subA, subB) = await SeedCancelledParentWithDoneChildrenAsync(db, repo);
var (orch, calls) = BuildOrchestrator(db);
var ex = await Assert.ThrowsAsync<InvalidOperationException>(
() => orch.StartAsync(parentId, "main", CancellationToken.None));
Assert.Contains("not WaitingForReview", ex.Message);
using var ctx = db.CreateContext();
Assert.Equal(TaskStatus.Cancelled, ctx.Tasks.Single(t => t.Id == parentId).Status);
Assert.Empty(calls);
// Children must stay untouched — the guard rejects before any merge is attempted.
Assert.Equal(WorktreeState.Active, ctx.Worktrees.Single(w => w.TaskId == subA).State);
Assert.Equal(WorktreeState.Active, ctx.Worktrees.Single(w => w.TaskId == subB).State);
}
private async Task<(string parentId, string subA, string subB)> SeedCancelledParentWithDoneChildrenAsync(
DbFixture db, GitRepoFixture repo)
{
using var ctx = db.CreateContext();
var listId = Guid.NewGuid().ToString();
ctx.Lists.Add(new ListEntity
{
Id = listId, Name = "test", CreatedAt = DateTime.UtcNow,
WorkingDir = repo.RepoDir,
});
var parentId = Guid.NewGuid().ToString();
ctx.Tasks.Add(new TaskEntity
{
Id = parentId, ListId = listId, Title = "improve", CreatedAt = DateTime.UtcNow,
Status = TaskStatus.Cancelled, PlanningPhase = PlanningPhase.None, SortOrder = 0,
Number = ++_numberSeed,
});
var subA = Guid.NewGuid().ToString();
var subB = Guid.NewGuid().ToString();
ctx.Tasks.Add(new TaskEntity
{
Id = subA, ListId = listId, Title = "child A", CreatedAt = DateTime.UtcNow,
ParentTaskId = parentId, Status = TaskStatus.Done, SortOrder = 1,
Number = ++_numberSeed,
});
ctx.Tasks.Add(new TaskEntity
{
Id = subB, ListId = listId, Title = "child B", CreatedAt = DateTime.UtcNow,
ParentTaskId = parentId, Status = TaskStatus.Done, SortOrder = 2,
Number = ++_numberSeed,
});
await ctx.SaveChangesAsync();
SeedWorktree(ctx, repo, subA, "fileA.txt", "content A");
SeedWorktree(ctx, repo, subB, "fileB.txt", "content B");
await ctx.SaveChangesAsync();
return (parentId, subA, subB);
}
// ─── Unit-merge failure propagation (blocked/verify_failed/untracked_collision) ─────────
/// <summary>
/// A child whose branch has no common history with the target branch makes the underlying
/// `git merge --no-ff` refuse outright (no conflict markers at all) — TaskMergeService reports
/// this as StatusBlocked, not StatusConflict. Before this fix that outcome vanished: DrainAsync
/// only logged it server-side and broadcast a bare PlanningMergeAborted, and ApproveReview
/// always returned StatusMerged regardless. Now StartAsync must surface the real status/reason,
/// and the broadcast must carry that reason.
/// </summary>
[Fact]
public async Task StartAsync_ChildMergeBlocked_ReturnsBlockedResultAndBroadcastsReason()
{
var db = NewDb();
var repo = NewRepo();
GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main");
var (parentId, _) = await SeedImprovementParentWithOneUnrelatedHistoryChildAsync(db, repo);
var (orch, spy) = BuildOrchestrator(db);
var result = await orch.StartAsync(parentId, "main", CancellationToken.None);
Assert.Equal(TaskMergeService.StatusBlocked, result.Status);
Assert.False(string.IsNullOrWhiteSpace(result.Reason));
using var ctx = db.CreateContext();
// The parent was never finalized — it stays wherever it was (WaitingForReview here).
Assert.Equal(TaskStatus.WaitingForReview, ctx.Tasks.Single(t => t.Id == parentId).Status);
var abortedCall = Assert.Single(spy, c => c.Method == "PlanningMergeAborted");
Assert.Equal(parentId, (string)abortedCall.Args[0]!);
Assert.Equal(result.Reason, (string?)abortedCall.Args[1]);
Assert.DoesNotContain(spy, c => c.Method == "PlanningCompleted");
}
private async Task<(string parentId, string subA)> SeedImprovementParentWithOneUnrelatedHistoryChildAsync(
DbFixture db, GitRepoFixture repo)
{
using var ctx = db.CreateContext();
var listId = Guid.NewGuid().ToString();
ctx.Lists.Add(new ListEntity
{
Id = listId, Name = "test", CreatedAt = DateTime.UtcNow,
WorkingDir = repo.RepoDir,
});
var parentId = Guid.NewGuid().ToString();
ctx.Tasks.Add(new TaskEntity
{
Id = parentId, ListId = listId, Title = "improve", CreatedAt = DateTime.UtcNow,
Status = TaskStatus.WaitingForReview, PlanningPhase = PlanningPhase.None, SortOrder = 0,
Number = ++_numberSeed,
});
var subA = Guid.NewGuid().ToString();
ctx.Tasks.Add(new TaskEntity
{
Id = subA, ListId = listId, Title = "child A", CreatedAt = DateTime.UtcNow,
ParentTaskId = parentId, Status = TaskStatus.Done, SortOrder = 1,
Number = ++_numberSeed,
});
await ctx.SaveChangesAsync();
SeedWorktreeUnrelatedHistory(ctx, repo, subA, "fileA.txt", "content A");
await ctx.SaveChangesAsync();
return (parentId, subA);
}
/// <summary>
/// Seeds a worktree on a branch with no common ancestor with the target branch, so
/// `git merge --no-ff` refuses with "refusing to merge unrelated histories" — a real,
/// deterministic StatusBlocked trigger with no conflict files, as opposed to the
/// files.Count > 0 path the other fixtures exercise.
/// </summary>
private void SeedWorktreeUnrelatedHistory(ClaudeDoDbContext ctx, GitRepoFixture repo, string taskId, string filename, string content)
{
var wtPath = Path.Combine(Path.GetTempPath(), $"wt_{Guid.NewGuid():N}");
_wtCleanups.Add((repo.RepoDir, wtPath));
var branch = $"claudedo/{taskId[..8]}";
const string emptyTreeSha = "4b825dc642cb6eb9a060e54bf8d69288fbee4904";
var orphanRoot = GitRepoFixture.RunGit(repo.RepoDir, "commit-tree", emptyTreeSha, "-m", "orphan root").Trim();
GitRepoFixture.RunGit(repo.RepoDir, "branch", branch, orphanRoot);
GitRepoFixture.RunGit(repo.RepoDir, "worktree", "add", wtPath, branch);
File.WriteAllText(Path.Combine(wtPath, filename), content);
GitRepoFixture.RunGit(wtPath, "add", filename);
GitRepoFixture.RunGit(wtPath, "commit", "-m", $"add {filename}");
var head = GitRepoFixture.RunGit(wtPath, "rev-parse", "HEAD").Trim();
ctx.Worktrees.Add(new WorktreeEntity
{
TaskId = taskId,
Path = wtPath,
BranchName = branch,
BaseCommit = orphanRoot,
HeadCommit = head,
DiffStat = null,
State = WorktreeState.Active,
CreatedAt = DateTime.UtcNow,
});
}
// ─── Guard (b): HasActiveMerge must span the finalize window ─────────────────────────────
/// <summary>
/// Once the last child has merged, DrainAsync clears CurrentSubtaskId before calling
/// FinalizeParentDoneAsync (which flips the parent to Done). Before this fix, HasActiveMerge
/// went false in that window, so TaskStateService.CancelAsync's "a merge is in progress"
/// guard stopped protecting the parent for the whole duration of the finalize call. This test
/// observes HasActiveMerge from inside ApproveReviewAsync (the call FinalizeParentDoneAsync
/// makes) to prove the window is now covered, and that the flag clears once the drain returns.
/// </summary>
[Fact]
public async Task Drain_FinalizeWindow_HasActiveMergeStaysTrueUntilFinalizeCompletes()
{
var db = NewDb();
var repo = NewRepo();
GitRepoFixture.RunGit(repo.RepoDir, "branch", "-m", "main");
var (parentId, _, _) = await SeedImprovementParentWithTwoDoneChildrenAsync(db, repo);
var fakeHub = new OrchestratorFakeHubContext();
var broadcaster = new HubBroadcaster(fakeHub);
var git = new GitService();
var factory = db.CreateFactory();
var built = TaskStateServiceBuilder.Build(factory);
var merge = new TaskMergeService(
factory, git, broadcaster, built.State, new VerifyCommandRunner(), NullLogger<TaskMergeService>.Instance);
var aggregator = new PlanningAggregator(factory, git, NullLogger<PlanningAggregator>.Instance);
PlanningMergeOrchestrator? orchRef = null;
bool? activeDuringApprove = null;
var observingState = new ApproveObservingTaskStateService(built.State, () =>
{
activeDuringApprove = orchRef!.HasActiveMerge(parentId);
});
var orch = new PlanningMergeOrchestrator(
factory, merge, aggregator, broadcaster, git, observingState, NullLogger<PlanningMergeOrchestrator>.Instance);
orchRef = orch;
var result = await orch.StartAsync(parentId, "main", CancellationToken.None);
Assert.Equal(TaskMergeService.StatusMerged, result.Status);
Assert.True(activeDuringApprove, "HasActiveMerge must still be true while FinalizeParentDoneAsync's ApproveReviewAsync runs.");
Assert.False(orch.HasActiveMerge(parentId), "state must be cleared once the drain (incl. finalize) fully completes.");
}
}
/// <summary>Test-only decorator that invokes a callback right before delegating
/// ApproveReviewAsync — everything else passes straight through to the real service.</summary>
file sealed class ApproveObservingTaskStateService : ITaskStateService
{
private readonly ITaskStateService _inner;
private readonly Action _onApprove;
public ApproveObservingTaskStateService(ITaskStateService inner, Action onApprove)
{
_inner = inner;
_onApprove = onApprove;
}
public Task<TransitionResult> ApproveReviewAsync(string taskId, CancellationToken ct)
{
_onApprove();
return _inner.ApproveReviewAsync(taskId, ct);
}
public Task<TransitionResult> EnqueueAsync(string taskId, CancellationToken ct) => _inner.EnqueueAsync(taskId, ct);
public Task<TransitionResult> StartRunningAsync(string taskId, DateTime startedAt, CancellationToken ct) => _inner.StartRunningAsync(taskId, startedAt, ct);
public Task<TransitionResult> CompleteAsync(string taskId, DateTime finishedAt, string? result, CancellationToken ct) => _inner.CompleteAsync(taskId, finishedAt, result, ct);
public Task<TransitionResult> SubmitForReviewAsync(string taskId, DateTime finishedAt, string? result, CancellationToken ct) => _inner.SubmitForReviewAsync(taskId, finishedAt, result, ct);
public Task<TransitionResult> SubmitInteractiveForReviewAsync(string taskId, DateTime finishedAt, CancellationToken ct) => _inner.SubmitInteractiveForReviewAsync(taskId, finishedAt, ct);
public Task<TransitionResult> SubmitForChildrenAsync(string taskId, DateTime finishedAt, string? result, CancellationToken ct) => _inner.SubmitForChildrenAsync(taskId, finishedAt, result, ct);
public Task<TransitionResult> FailAsync(string taskId, DateTime finishedAt, string? error, CancellationToken ct, string failureReason = "error", int? turnsUsed = null, int? maxTurns = null)
=> _inner.FailAsync(taskId, finishedAt, error, ct, failureReason, turnsUsed, maxTurns);
public Task<TransitionResult> CancelAsync(string taskId, DateTime finishedAt, CancellationToken ct, bool allowFromIdle = false) => _inner.CancelAsync(taskId, finishedAt, ct, allowFromIdle);
public Task<TransitionResult> ResetToIdleAsync(string taskId, CancellationToken ct) => _inner.ResetToIdleAsync(taskId, ct);
public Task<TransitionResult> RejectToQueueAsync(string taskId, string feedback, CancellationToken ct) => _inner.RejectToQueueAsync(taskId, feedback, ct);
public Task<TransitionResult> RejectToIdleAsync(string taskId, CancellationToken ct) => _inner.RejectToIdleAsync(taskId, ct);
public Task<TransitionResult> ClearReviewFeedbackAsync(string taskId, CancellationToken ct) => _inner.ClearReviewFeedbackAsync(taskId, ct);
public Task<TransitionResult> ForceSetStatusAsync(string taskId, TaskStatus status, CancellationToken ct) => _inner.ForceSetStatusAsync(taskId, status, ct);
public Task<TransitionResult> StartPlanningAsync(string parentId, CancellationToken ct) => _inner.StartPlanningAsync(parentId, ct);
public Task<TransitionResult> FinalizePlanningAsync(string parentId, CancellationToken ct) => _inner.FinalizePlanningAsync(parentId, ct);
public Task<TransitionResult> BlockOnAsync(string taskId, string predecessorTaskId, CancellationToken ct) => _inner.BlockOnAsync(taskId, predecessorTaskId, ct);
public Task<TransitionResult> UnblockAsync(string taskId, CancellationToken ct) => _inner.UnblockAsync(taskId, ct);
public Task<TransitionResult> SetDependsOnAsync(string taskId, string? dependsOnTaskId, CancellationToken ct) => _inner.SetDependsOnAsync(taskId, dependsOnTaskId, ct);
public Task TryAdvanceParentAsync(string parentId) => _inner.TryAdvanceParentAsync(parentId);
public Task<int> RecoverStaleRunningAsync(string reason, CancellationToken ct) => _inner.RecoverStaleRunningAsync(reason, ct);
}