fix(worker): mark task Done on every successful merge path, not just approve
Generalizes the previous merge_task fix: the WaitingForReview->Done transition now lives in TaskMergeService.MergeAsync/ContinueMergeAsync, so the UI Merge button (WorkerHub.MergeTask), conflict-merge, continue-merge and the external MCP all land a merged task in Done. ApproveAndMergeAsync no longer double-approves. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -468,12 +468,6 @@ public sealed class ExternalMcpService
|
|||||||
|
|
||||||
if (result.Status == TaskMergeService.StatusMerged)
|
if (result.Status == TaskMergeService.StatusMerged)
|
||||||
{
|
{
|
||||||
// MergeAsync only flips the worktree to Merged; a merged task must also
|
|
||||||
// reach Done. If it was still awaiting review, approve it now (a Done task
|
|
||||||
// is already terminal and needs no transition).
|
|
||||||
if (task.Status == TaskStatus.WaitingForReview)
|
|
||||||
await _state.ApproveReviewAsync(taskId, cancellationToken);
|
|
||||||
|
|
||||||
string? mergeCommit = null;
|
string? mergeCommit = null;
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -85,6 +85,15 @@ public sealed class TaskMergeService
|
|||||||
await _broadcaster.WorktreeUpdated(taskId);
|
await _broadcaster.WorktreeUpdated(taskId);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private async Task ApproveIfWaitingForReviewAsync(TaskEntity task, CancellationToken ct)
|
||||||
|
{
|
||||||
|
// A merged worktree means the work is integrated, so the task must reach Done.
|
||||||
|
// MarkWorktreeMergedAsync only flips the worktree state; transition the task
|
||||||
|
// itself when it was still awaiting review (a Done task is already terminal).
|
||||||
|
if (task.Status == TaskStatus.WaitingForReview)
|
||||||
|
await _state.ApproveReviewAsync(task.Id, ct);
|
||||||
|
}
|
||||||
|
|
||||||
public async Task<MergeResult> MergeAsync(
|
public async Task<MergeResult> MergeAsync(
|
||||||
string taskId,
|
string taskId,
|
||||||
string targetBranch,
|
string targetBranch,
|
||||||
@@ -168,6 +177,7 @@ public sealed class TaskMergeService
|
|||||||
}
|
}
|
||||||
|
|
||||||
await MarkWorktreeMergedAsync(taskId, ct);
|
await MarkWorktreeMergedAsync(taskId, ct);
|
||||||
|
await ApproveIfWaitingForReviewAsync(task, ct);
|
||||||
|
|
||||||
_logger.LogInformation(
|
_logger.LogInformation(
|
||||||
"Merged task {TaskId} branch {Branch} into {Target} (remove worktree: {Remove})",
|
"Merged task {TaskId} branch {Branch} into {Target} (remove worktree: {Remove})",
|
||||||
@@ -187,7 +197,7 @@ public sealed class TaskMergeService
|
|||||||
|
|
||||||
public async Task<MergeResult> ContinueMergeAsync(string taskId, CancellationToken ct)
|
public async Task<MergeResult> ContinueMergeAsync(string taskId, CancellationToken ct)
|
||||||
{
|
{
|
||||||
var (_, list, wt) = await LoadMergeContextAsync(taskId, ct);
|
var (task, list, wt) = await LoadMergeContextAsync(taskId, ct);
|
||||||
|
|
||||||
if (wt is null) return Blocked("task has no worktree");
|
if (wt is null) return Blocked("task has no worktree");
|
||||||
if (wt.State != WorktreeState.Active) return Blocked($"worktree state is {wt.State}");
|
if (wt.State != WorktreeState.Active) return Blocked($"worktree state is {wt.State}");
|
||||||
@@ -205,6 +215,7 @@ public sealed class TaskMergeService
|
|||||||
catch (Exception ex) { return Blocked($"commit failed: {ex.Message}"); }
|
catch (Exception ex) { return Blocked($"commit failed: {ex.Message}"); }
|
||||||
|
|
||||||
await MarkWorktreeMergedAsync(taskId, ct);
|
await MarkWorktreeMergedAsync(taskId, ct);
|
||||||
|
await ApproveIfWaitingForReviewAsync(task, ct);
|
||||||
_logger.LogInformation("Continued merge of task {TaskId} branch {Branch}", taskId, wt.BranchName);
|
_logger.LogInformation("Continued merge of task {TaskId} branch {Branch}", taskId, wt.BranchName);
|
||||||
|
|
||||||
return new MergeResult(StatusMerged, Array.Empty<string>(), null);
|
return new MergeResult(StatusMerged, Array.Empty<string>(), null);
|
||||||
@@ -313,12 +324,8 @@ public sealed class TaskMergeService
|
|||||||
? await _git.GetCurrentBranchAsync(list.WorkingDir, ct)
|
? await _git.GetCurrentBranchAsync(list.WorkingDir, ct)
|
||||||
: targetBranch;
|
: targetBranch;
|
||||||
|
|
||||||
var merge = await MergeAsync(taskId, target, removeWorktree: false, $"Merge {wt.BranchName}", ct);
|
// MergeAsync transitions the task WaitingForReview -> Done on a successful merge.
|
||||||
if (merge.Status != StatusMerged)
|
return await MergeAsync(taskId, target, removeWorktree: false, $"Merge {wt.BranchName}", ct);
|
||||||
return merge;
|
|
||||||
|
|
||||||
var approve = await _state.ApproveReviewAsync(taskId, ct);
|
|
||||||
return approve.Ok ? merge : Blocked(approve.Reason ?? "approve failed");
|
|
||||||
}
|
}
|
||||||
|
|
||||||
private static MergeResult Blocked(string reason) =>
|
private static MergeResult Blocked(string reason) =>
|
||||||
|
|||||||
Reference in New Issue
Block a user