feat(worker): report progress for continue_merge and unit-merge verify gate

continue_merge and the parent/children unit-merge drain (PlanningMergeOrchestrator)
re-run the post-merge verify gate but never forwarded their IProgress token into it,
so a slow verify command on either path went silent past Claude Code's 300s MCP
idle-abort even though D1-D3 already fixed this for merge_task/review_task's
childless path. list_worktrees also gets elapsed-time progress: many tracked
worktrees means many concurrent git subprocess spawns.

Worker CLAUDE.md's existing progress rule now points at ProgressReporter as the
one implementation instead of a fresh polling loop.
This commit is contained in:
Mika Kuns
2026-08-17 09:56:59 +02:00
parent 1341c5f4f3
commit 23aab26daf
5 changed files with 189 additions and 27 deletions
+1 -1
View File
@@ -228,4 +228,4 @@ list-only — there is no task-level override — and is written via `set_list_c
- The worker runs standalone — start it separately from the UI. Loopback only (127.0.0.1). - The worker runs standalone — start it separately from the UI. Loopback only (127.0.0.1).
- `--permission-mode auto` by default; legacy `bypassPermissions` settings map to `auto` at dispatch time. `acceptEdits`, `plan`, `default` pass through unchanged. - `--permission-mode auto` by default; legacy `bypassPermissions` settings map to `auto` at dispatch time. `acceptEdits`, `plan`, `default` pass through unchanged.
- Worktree branches follow `claudedo/{id}`. - Worktree branches follow `claudedo/{id}`.
- **An MCP tool that can run longer than ~5s reports progress.** Staying silent lets the MCP client abort after 300s idle while the worker keeps working — the caller sees an abort even though the operation is still running. - **An MCP tool that can run longer than ~5s reports progress.** Staying silent lets the MCP client abort after 300s idle while the worker keeps working — the caller sees an abort even though the operation is still running. `Lifecycle/ProgressReporter` is the one implementation (elapsed-time reporting via `RunAsync`, per-item `i/n` via `ReportItem`) — thread an `IProgress<ProgressNotificationValue>? progress = null` parameter through instead of writing another polling loop.
+30 -17
View File
@@ -703,7 +703,7 @@ public sealed class ExternalMcpService
// 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.
await _planningMerge.StartAsync(taskId, targetBranch ?? "", cancellationToken, externallyDriven: true); await _planningMerge.StartAsync(taskId, targetBranch ?? "", cancellationToken, externallyDriven: true, progress);
var parentDone = (await _tasks.GetByIdAsync(taskId, cancellationToken))!.Status == TaskStatus.Done; var parentDone = (await _tasks.GetByIdAsync(taskId, cancellationToken))!.Status == TaskStatus.Done;
mergeStatus = parentDone ? TaskMergeService.StatusMerged : TaskMergeService.StatusConflict; mergeStatus = parentDone ? TaskMergeService.StatusMerged : TaskMergeService.StatusConflict;
if (!parentDone) if (!parentDone)
@@ -1037,8 +1037,12 @@ public sealed class ExternalMcpService
"Finish an in-progress conflicted merge once you have resolved the conflict markers in the working tree " + "Finish an in-progress conflicted merge once you have resolved the conflict markers in the working tree " +
"(repoPath from merge_task/review_task). Pass the PARENT task id to continue a parent/children unit merge. " + "(repoPath from merge_task/review_task). Pass the PARENT task id to continue a parent/children unit merge. " +
"merged=false with conflicts listed means markers are still present — resolve them and call again. " + "merged=false with conflicts listed means markers are still present — resolve them and call again. " +
"Throws if there is no in-progress merge for the task; use abort_merge to cancel a paused merge instead.")] "Throws if there is no in-progress merge for the task; use abort_merge to cancel a paused merge instead." +
public async Task<MergeContinuationResultDto> ContinueMerge(string taskId, CancellationToken cancellationToken) McpToolDocs.ProgressHint)]
public async Task<MergeContinuationResultDto> ContinueMerge(
string taskId,
CancellationToken cancellationToken = default,
IProgress<ProgressNotificationValue>? progress = null)
{ {
taskId = await TaskIdResolver.ResolveAsync(_tasks, taskId, cancellationToken); taskId = await TaskIdResolver.ResolveAsync(_tasks, taskId, cancellationToken);
var task = await _tasks.GetByIdAsync(taskId, cancellationToken) var task = await _tasks.GetByIdAsync(taskId, cancellationToken)
@@ -1053,7 +1057,7 @@ public sealed class ExternalMcpService
if (_planningMerge.HasActiveMerge(taskId)) if (_planningMerge.HasActiveMerge(taskId))
{ {
await _planningMerge.ContinueAsync(taskId, cancellationToken); await _planningMerge.ContinueAsync(taskId, cancellationToken, progress);
var parent = (await _tasks.GetByIdAsync(taskId, cancellationToken))!; var parent = (await _tasks.GetByIdAsync(taskId, cancellationToken))!;
if (parent.Status == TaskStatus.Done) if (parent.Status == TaskStatus.Done)
{ {
@@ -1080,7 +1084,7 @@ public sealed class ExternalMcpService
} }
else else
{ {
var r = await _merge.ContinueMergeAsync(taskId, cancellationToken); var r = await _merge.ContinueMergeAsync(taskId, cancellationToken, progress);
if (r.Status == TaskMergeService.StatusMerged) if (r.Status == TaskMergeService.StatusMerged)
{ {
merged = true; merged = true;
@@ -1464,20 +1468,29 @@ public sealed class ExternalMcpService
[McpServerTool, Description( [McpServerTool, Description(
"Survey every worktree ClaudeDo tracks — use it to find leftovers to clean up. Only worktrees recorded in " + "Survey every worktree ClaudeDo tracks — use it to find leftovers to clean up. Only worktrees recorded in " +
"the ClaudeDo database appear here, and headCommit is empty when the path is missing from disk.")] "the ClaudeDo database appear here, and headCommit is empty when the path is missing from disk." +
public async Task<IReadOnlyList<WorktreeListItemDto>> ListWorktrees(CancellationToken cancellationToken) McpToolDocs.ProgressHint)]
public async Task<IReadOnlyList<WorktreeListItemDto>> ListWorktrees(
CancellationToken cancellationToken = default,
IProgress<ProgressNotificationValue>? progress = null)
{ {
var rows = await _maintenance.GetOverviewAsync(null, cancellationToken); var rows = await _maintenance.GetOverviewAsync(null, cancellationToken);
var results = await Task.WhenAll(rows.Select(async row => // One git status + one rev-parse per row, run concurrently -- with many tracked worktrees
{ // (this tool's whole point is surfacing leftovers nobody cleaned up) that can still take a
var isDirty = row.PathExistsOnDisk && await TryGetIsDirtyAsync(row.Path, cancellationToken); // while, so this reports on the same elapsed-time cadence as a single long git call rather
var headCommit = row.PathExistsOnDisk // than per-row (rows finish out of order under Task.WhenAll, so there's no natural i/n).
? (await TryRunGitAsync(row.Path, ["rev-parse", "HEAD"], cancellationToken) ?? "") var results = await ProgressReporter.RunAsync(
: ""; Task.WhenAll(rows.Select(async row =>
return new WorktreeListItemDto( {
row.TaskId, row.TaskNumber, row.Path, row.BranchName, headCommit, var isDirty = row.PathExistsOnDisk && await TryGetIsDirtyAsync(row.Path, cancellationToken);
isDirty, row.State == WorktreeState.Merged); var headCommit = row.PathExistsOnDisk
})); ? (await TryRunGitAsync(row.Path, ["rev-parse", "HEAD"], cancellationToken) ?? "")
: "";
return new WorktreeListItemDto(
row.TaskId, row.TaskNumber, row.Path, row.BranchName, headCommit,
isDirty, row.State == WorktreeState.Merged);
})),
ProgressReportInterval, progress, "surveying worktrees");
return results; return results;
} }
@@ -492,7 +492,8 @@ public sealed class TaskMergeService
CancellationToken ct) CancellationToken ct)
=> MergeAsync(taskId, targetBranch, removeWorktree, commitMessage, leaveConflictsInTree: false, ct); => MergeAsync(taskId, targetBranch, removeWorktree, commitMessage, leaveConflictsInTree: false, ct);
public async Task<MergeResult> ContinueMergeAsync(string taskId, CancellationToken ct) public async Task<MergeResult> ContinueMergeAsync(
string taskId, CancellationToken ct, IProgress<ProgressNotificationValue>? progress = null)
{ {
var (task, list, wt, verifyCommand) = await LoadMergeContextAsync(taskId, ct); var (task, list, wt, verifyCommand) = await LoadMergeContextAsync(taskId, ct);
@@ -558,7 +559,7 @@ public sealed class TaskMergeService
var targetBranch = await _git.GetCurrentBranchAsync(list.WorkingDir, ct); var targetBranch = await _git.GetCurrentBranchAsync(list.WorkingDir, ct);
await RebaseOthersAfterMergeAsync(task, list, targetBranch, oldTargetTip, mergeSha, ct); await RebaseOthersAfterMergeAsync(task, list, targetBranch, oldTargetTip, mergeSha, ct);
var verifyFailure = await RunVerifyGateAsync(verifyCommand, list.WorkingDir, ct); var verifyFailure = await RunVerifyGateAsync(verifyCommand, list.WorkingDir, ct, progress);
if (verifyFailure is not null) if (verifyFailure is not null)
{ {
_logger.LogWarning("Verify command failed after continuing merge of task {TaskId}: {Reason}", taskId, verifyFailure.ErrorMessage); _logger.LogWarning("Verify command failed after continuing merge of task {TaskId}: {Reason}", taskId, verifyFailure.ErrorMessage);
@@ -6,6 +6,7 @@ using ClaudeDo.Worker.Hub;
using ClaudeDo.Worker.Lifecycle; using ClaudeDo.Worker.Lifecycle;
using ClaudeDo.Worker.State; using ClaudeDo.Worker.State;
using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore;
using ModelContextProtocol;
using TaskStatus = ClaudeDo.Data.Models.TaskStatus; using TaskStatus = ClaudeDo.Data.Models.TaskStatus;
namespace ClaudeDo.Worker.Planning; namespace ClaudeDo.Worker.Planning;
@@ -57,7 +58,8 @@ public sealed class PlanningMergeOrchestrator : IActiveMergeState
} }
public async Task StartAsync( public async Task StartAsync(
string parentTaskId, string targetBranch, CancellationToken ct, bool externallyDriven = false) string parentTaskId, string targetBranch, CancellationToken ct, bool externallyDriven = false,
IProgress<ProgressNotificationValue>? progress = null)
{ {
string workingDir; string workingDir;
List<TaskEntity> children; List<TaskEntity> children;
@@ -121,7 +123,7 @@ public sealed class PlanningMergeOrchestrator : IActiveMergeState
throw new InvalidOperationException($"Merge already in progress for {parentTaskId}."); throw new InvalidOperationException($"Merge already in progress for {parentTaskId}.");
await _broadcaster.PlanningMergeStarted(parentTaskId, targetBranch); await _broadcaster.PlanningMergeStarted(parentTaskId, targetBranch);
await DrainAsync(parentTaskId, ct); await DrainAsync(parentTaskId, ct, progress);
} }
/// <summary>True when a unit merge for this parent is paused on a conflict (in-memory state).</summary> /// <summary>True when a unit merge for this parent is paused on a conflict (in-memory state).</summary>
@@ -145,14 +147,15 @@ public sealed class PlanningMergeOrchestrator : IActiveMergeState
return result; return result;
} }
public async Task ContinueAsync(string planningTaskId, CancellationToken ct) public async Task ContinueAsync(
string planningTaskId, CancellationToken ct, IProgress<ProgressNotificationValue>? progress = null)
{ {
if (!_states.TryGetValue(planningTaskId, out var state) || state.CurrentSubtaskId is null) if (!_states.TryGetValue(planningTaskId, out var state) || state.CurrentSubtaskId is null)
throw new InvalidOperationException( throw new InvalidOperationException(
"no in-progress merge to continue; if the worker was restarted during a conflict, use AbortPlanningMerge to reset the repository"); "no in-progress merge to continue; if the worker was restarted during a conflict, use AbortPlanningMerge to reset the repository");
var current = state.CurrentSubtaskId; var current = state.CurrentSubtaskId;
var result = await _merge.ContinueMergeAsync(current, ct); var result = await _merge.ContinueMergeAsync(current, ct, progress);
if (result.Status == TaskMergeService.StatusConflict) if (result.Status == TaskMergeService.StatusConflict)
{ {
@@ -173,7 +176,7 @@ public sealed class PlanningMergeOrchestrator : IActiveMergeState
await _broadcaster.PlanningSubtaskMerged(planningTaskId, current); await _broadcaster.PlanningSubtaskMerged(planningTaskId, current);
state.CurrentSubtaskId = null; state.CurrentSubtaskId = null;
await DrainAsync(planningTaskId, ct); await DrainAsync(planningTaskId, ct, progress);
} }
public async Task AbortAsync(string planningTaskId, CancellationToken ct) public async Task AbortAsync(string planningTaskId, CancellationToken ct)
@@ -213,7 +216,8 @@ public sealed class PlanningMergeOrchestrator : IActiveMergeState
// Parent remains WaitingForReview — Approve will restart the unit merge from scratch. // Parent remains WaitingForReview — Approve will restart the unit merge from scratch.
} }
private async Task DrainAsync(string planningTaskId, CancellationToken ct) private async Task DrainAsync(
string planningTaskId, CancellationToken ct, IProgress<ProgressNotificationValue>? progress = null)
{ {
if (!_states.TryGetValue(planningTaskId, out var state)) return; if (!_states.TryGetValue(planningTaskId, out var state)) return;
@@ -229,7 +233,8 @@ public sealed class PlanningMergeOrchestrator : IActiveMergeState
removeWorktree: true, removeWorktree: true,
commitMessage: "", // blank -> TaskMergeService builds the conventional default commitMessage: "", // blank -> TaskMergeService builds the conventional default
leaveConflictsInTree: true, leaveConflictsInTree: true,
ct); ct,
progress);
if (result.Status == TaskMergeService.StatusConflict) if (result.Status == TaskMergeService.StatusConflict)
{ {
@@ -694,6 +694,76 @@ public sealed class ExternalMcpServiceTests : IDisposable
Assert.Equal(WorktreeState.Merged, verify.Worktrees.Single(w => w.TaskId == childId).State); Assert.Equal(WorktreeState.Merged, verify.Worktrees.Single(w => w.TaskId == childId).State);
} }
// Regression coverage for the parent/children unit-merge path specifically: PlanningMergeOrchestrator
// used to drain every child's merge (and verify gate) without ever forwarding review_task's own
// progress token, so a multi-child approve with a slow verify command reported only the single
// "review_task started" tick and then went silent for as long as every child's gate took combined.
[Fact]
public async Task ReviewTask_Approve_ParentWithChildren_SlowVerifyCommand_ReportsProgress()
{
if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; }
var original = TaskMergeService.ProgressReportInterval;
TaskMergeService.ProgressReportInterval = TimeSpan.FromMilliseconds(50);
try
{
var repo = new GitRepoFixture();
_repos.Add(repo);
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 = "plan", CreatedAt = DateTime.UtcNow,
Status = TaskStatus.WaitingForReview, PlanningPhase = PlanningPhase.Finalized, Number = ++_numberSeed });
ctx.Tasks.Add(new TaskEntity { Id = childId, ListId = listId, Title = "child", CreatedAt = DateTime.UtcNow,
ParentTaskId = parentId, Status = TaskStatus.Done, SortOrder = 1, Number = ++_numberSeed });
var wtPath = Path.Combine(Path.GetTempPath(), $"wt_{Guid.NewGuid():N}");
_worktreeCleanups.Add((repo.RepoDir, wtPath));
var branch = $"claudedo/{childId[..8]}";
GitRepoFixture.RunGit(repo.RepoDir, "worktree", "add", "-b", branch, wtPath, repo.BaseCommit);
File.WriteAllText(Path.Combine(wtPath, "child.txt"), "c\n");
GitRepoFixture.RunGit(wtPath, "add", "child.txt");
GitRepoFixture.RunGit(wtPath, "commit", "-m", "add child.txt");
ctx.Worktrees.Add(new WorktreeEntity
{
TaskId = childId, Path = wtPath, BranchName = branch,
BaseCommit = repo.BaseCommit,
HeadCommit = GitRepoFixture.RunGit(wtPath, "rev-parse", "HEAD").Trim(),
State = WorktreeState.Active, CreatedAt = DateTime.UtcNow,
});
await new ListRepository(ctx).SetConfigAsync(new ListConfigEntity { ListId = listId, VerifyCommand = "dotnet build" });
await ctx.SaveChangesAsync();
}
var fakeVerify = new FakeVerifyCommandRunner
{
Result = new VerifyCommandResult(0, false, "ok"),
Delay = TimeSpan.FromMilliseconds(300),
};
var sut = BuildSut(CreateQueue(), fakeVerify);
var reports = new List<ProgressNotificationValue>();
var progress = new Progress<ProgressNotificationValue>(reports.Add);
var result = await sut.ReviewTask(
parentId, "approve", null, "main", cancellationToken: CancellationToken.None, progress: progress);
Assert.Equal(TaskMergeService.StatusMerged, result.MergeStatus);
Assert.Equal("Done", result.Task.Status);
await Task.Delay(200);
Assert.Contains(reports, r => r.Message != null && r.Message.Contains("verify gate running"));
}
finally
{
TaskMergeService.ProgressReportInterval = original;
}
}
[Fact] [Fact]
public async Task ReviewTask_Approve_ParentWithChildren_ReportsEmptyChildByName() public async Task ReviewTask_Approve_ParentWithChildren_ReportsEmptyChildByName()
{ {
@@ -1316,6 +1386,33 @@ public sealed class ExternalMcpServiceTests : IDisposable
Assert.Contains(rows, r => r.TaskId == task.Id); Assert.Contains(rows, r => r.TaskId == task.Id);
} }
// list_worktrees fans out one git status + one rev-parse per tracked worktree under
// Task.WhenAll -- no natural i/n since rows finish out of order, so it reports on the same
// elapsed-time cadence as CleanupTaskWorktree above.
[Fact]
public async Task ListWorktrees_ReportsProgressWhileSurveying()
{
if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; }
var original = ExternalMcpService.ProgressReportInterval;
ExternalMcpService.ProgressReportInterval = TimeSpan.Zero;
try
{
await SeedWorktreeAsync();
var sut = BuildSut(CreateQueue());
var progress = new SyncProgress<ProgressNotificationValue>();
var rows = await sut.ListWorktrees(CancellationToken.None, progress);
Assert.NotEmpty(rows);
Assert.NotEmpty(progress.Reports);
}
finally
{
ExternalMcpService.ProgressReportInterval = original;
}
}
// ── CleanupTaskWorktree ──────────────────────────────────────────────────── // ── CleanupTaskWorktree ────────────────────────────────────────────────────
[Fact] [Fact]
@@ -1711,6 +1808,52 @@ public sealed class ExternalMcpServiceTests : IDisposable
Assert.False(await new GitService().IsMidMergeAsync(list.WorkingDir!)); Assert.False(await new GitService().IsMidMergeAsync(list.WorkingDir!));
} }
// Regression coverage for the same class of incident as ReviewTask_ApproveWithSlowVerifyCommand
// above, but for the resume path: continue_merge re-runs the verify gate too, and until this
// fix it never forwarded its progress token into RunVerifyGateAsync at all.
[Fact]
public async Task ContinueMerge_SlowVerifyCommand_ReportsProgressWellBeforeIdleTimeout()
{
if (!GitAvailable) { Assert.True(true, "git not available -- skipping"); return; }
var original = TaskMergeService.ProgressReportInterval;
TaskMergeService.ProgressReportInterval = TimeSpan.FromMilliseconds(50);
try
{
var (task, list, _) = await SeedConflictingWorktreeAsync(TaskStatus.WaitingForReview);
var plainSut = BuildSut(CreateQueue());
await plainSut.ReviewTask(task.Id, "approve", null, null,
leaveConflictsInTree: true, CancellationToken.None);
File.WriteAllText(Path.Combine(list.WorkingDir!, "README.md"), "# resolved\n");
GitRepoFixture.RunGit(list.WorkingDir!, "add", "README.md");
using (var ctx = _db.CreateContext())
await new ListRepository(ctx).SetConfigAsync(new ListConfigEntity { ListId = list.Id, VerifyCommand = "dotnet build" });
var fakeVerify = new FakeVerifyCommandRunner
{
Result = new VerifyCommandResult(0, false, "ok"),
Delay = TimeSpan.FromMilliseconds(300),
};
var sut = BuildSut(CreateQueue(), fakeVerify);
var reports = new List<ProgressNotificationValue>();
var progress = new Progress<ProgressNotificationValue>(reports.Add);
var result = await sut.ContinueMerge(task.Id, CancellationToken.None, progress);
Assert.True(result.Merged);
Assert.Equal("Done", result.TaskStatus);
await Task.Delay(200);
Assert.Contains(reports, r => r.Message != null && r.Message.Contains("verify gate running"));
}
finally
{
TaskMergeService.ProgressReportInterval = original;
}
}
[Fact] [Fact]
public async Task ContinueMerge_MarkersStillPresent_ReportsConflicts() public async Task ContinueMerge_MarkersStillPresent_ReportsConflicts()
{ {