feat(worker): approve a unit parent like a single task, orchestrator only for legacy units
This commit is contained in:
+15
-4
@@ -12,6 +12,7 @@ using ClaudeDo.Worker.Hub;
|
|||||||
using ClaudeDo.Worker.Lifecycle;
|
using ClaudeDo.Worker.Lifecycle;
|
||||||
using ClaudeDo.Worker.Planning;
|
using ClaudeDo.Worker.Planning;
|
||||||
using ClaudeDo.Worker.Queue;
|
using ClaudeDo.Worker.Queue;
|
||||||
|
using ClaudeDo.Worker.Runner;
|
||||||
using ClaudeDo.Worker.State;
|
using ClaudeDo.Worker.State;
|
||||||
using ClaudeDo.Worker.Worktrees;
|
using ClaudeDo.Worker.Worktrees;
|
||||||
using Microsoft.EntityFrameworkCore;
|
using Microsoft.EntityFrameworkCore;
|
||||||
@@ -678,16 +679,23 @@ public sealed class ExternalMcpService
|
|||||||
{
|
{
|
||||||
// Approve is the single review+merge action — mirror the hub's ApproveReview
|
// Approve is the single review+merge action — mirror the hub's ApproveReview
|
||||||
// routing instead of only flipping the status (which left branches unmerged).
|
// routing instead of only flipping the status (which left branches unmerged).
|
||||||
bool hasChildren;
|
bool hasChildren, legacyUnit;
|
||||||
await using (var ctx = await _dbFactory.CreateDbContextAsync(cancellationToken))
|
await using (var ctx = await _dbFactory.CreateDbContextAsync(cancellationToken))
|
||||||
hasChildren = await ctx.Tasks.AnyAsync(t => t.ParentTaskId == taskId, cancellationToken);
|
|
||||||
|
|
||||||
if (hasChildren)
|
|
||||||
{
|
{
|
||||||
|
hasChildren = await ctx.Tasks.AnyAsync(t => t.ParentTaskId == taskId, cancellationToken);
|
||||||
|
legacyUnit = hasChildren && await UnitBranchService.HasUnabsorbedChildrenAsync(ctx, taskId, cancellationToken);
|
||||||
|
}
|
||||||
|
|
||||||
// Compute before the merge starts -- children are still Done with their own
|
// Compute before the merge starts -- children are still Done with their own
|
||||||
// pre-merge worktree/commit range at this point, so "did it contribute anything"
|
// pre-merge worktree/commit range at this point, so "did it contribute anything"
|
||||||
// reflects the review range the reviewer is about to approve.
|
// reflects the review range the reviewer is about to approve.
|
||||||
|
if (hasChildren)
|
||||||
emptyChildren = await GetEmptyDoneChildrenAsync(taskId, cancellationToken);
|
emptyChildren = await GetEmptyDoneChildrenAsync(taskId, cancellationToken);
|
||||||
|
|
||||||
|
// Absorbed children are already in the parent's unit branch: the parent merges like a
|
||||||
|
// single task. Only a legacy unit (a Done child still Active) takes the unit merge.
|
||||||
|
if (legacyUnit)
|
||||||
|
{
|
||||||
// externallyDriven: true — this call came from an MCP session, not the UI's
|
// externallyDriven: true — this call came from an MCP session, not the UI's
|
||||||
// Approve button. A unit-merge conflict must not auto-open the in-app resolver;
|
// Approve button. A unit-merge conflict must not auto-open the in-app resolver;
|
||||||
// the driving session resolves it via continue_merge/abort_merge instead.
|
// the driving session resolves it via continue_merge/abort_merge instead.
|
||||||
@@ -796,6 +804,9 @@ public sealed class ExternalMcpService
|
|||||||
{
|
{
|
||||||
if (child.Worktree is not null)
|
if (child.Worktree is not null)
|
||||||
{
|
{
|
||||||
|
// Absorbed into the unit branch (no merge commit): empty iff it never committed.
|
||||||
|
if (child.Worktree is { State: WorktreeState.Merged, MergeCommit: null })
|
||||||
|
return child.Worktree.HeadCommit is null;
|
||||||
if (!Directory.Exists(child.Worktree.Path)) return false;
|
if (!Directory.Exists(child.Worktree.Path)) return false;
|
||||||
var files = ParseDiffStatFileNames(
|
var files = ParseDiffStatFileNames(
|
||||||
await _git.DiffStatAsync(child.Worktree.Path, child.Worktree.BaseCommit, "HEAD", ct: ct));
|
await _git.DiffStatAsync(child.Worktree.Path, child.Worktree.BaseCommit, "HEAD", ct: ct));
|
||||||
|
|||||||
@@ -697,11 +697,13 @@ public sealed class WorkerHub : Microsoft.AspNetCore.SignalR.Hub
|
|||||||
public Task<MergeResultDto> ApproveReview(string taskId, string targetBranch, bool autostash = false)
|
public Task<MergeResultDto> ApproveReview(string taskId, string targetBranch, bool autostash = false)
|
||||||
=> HubGuard(async () =>
|
=> HubGuard(async () =>
|
||||||
{
|
{
|
||||||
bool hasChildren;
|
// Children already absorbed into the parent's unit branch need no unit merge -- the parent
|
||||||
|
// merges like a single task. Only legacy units (a Done child still Active) take the orchestrator.
|
||||||
|
bool legacyUnit;
|
||||||
await using (var ctx = await _dbFactory.CreateDbContextAsync(CancellationToken.None))
|
await using (var ctx = await _dbFactory.CreateDbContextAsync(CancellationToken.None))
|
||||||
hasChildren = await ctx.Tasks.AnyAsync(t => t.ParentTaskId == taskId, CancellationToken.None);
|
legacyUnit = await UnitBranchService.HasUnabsorbedChildrenAsync(ctx, taskId, CancellationToken.None);
|
||||||
|
|
||||||
if (hasChildren)
|
if (legacyUnit)
|
||||||
{
|
{
|
||||||
var unitResult = await _planningMergeOrchestrator.StartAsync(taskId, targetBranch ?? "", CancellationToken.None);
|
var unitResult = await _planningMergeOrchestrator.StartAsync(taskId, targetBranch ?? "", CancellationToken.None);
|
||||||
if (unitResult.Status == TaskMergeService.StatusBlocked)
|
if (unitResult.Status == TaskMergeService.StatusBlocked)
|
||||||
|
|||||||
@@ -134,7 +134,9 @@ public sealed class PlanningMergeOrchestrator : IActiveMergeState
|
|||||||
throw new InvalidOperationException("working tree has uncommitted changes");
|
throw new InvalidOperationException("working tree has uncommitted changes");
|
||||||
|
|
||||||
var idsToMerge = new List<string>();
|
var idsToMerge = new List<string>();
|
||||||
if (!isPlanning && parentHasWorktree)
|
// A planning parent owns a worktree too since the unit branch -- a mixed unit (absorbed
|
||||||
|
// children in it, legacy children beside it) must land it before the legacy children.
|
||||||
|
if (parentHasWorktree)
|
||||||
idsToMerge.Add(parentTaskId);
|
idsToMerge.Add(parentTaskId);
|
||||||
idsToMerge.AddRange(
|
idsToMerge.AddRange(
|
||||||
children
|
children
|
||||||
|
|||||||
Reference in New Issue
Block a user