feat(claude-do): merge [D4] Restliche MCP-Long-Runner versorgen + Progress-Regel im
ClaudeDo-Task: 8e7f70d8-e821-45c8-835b-7743c201f599
This commit is contained in:
@@ -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).
|
||||
- `--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}`.
|
||||
- **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
@@ -703,7 +703,7 @@ public sealed class ExternalMcpService
|
||||
// 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;
|
||||
// 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;
|
||||
mergeStatus = parentDone ? TaskMergeService.StatusMerged : TaskMergeService.StatusConflict;
|
||||
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 " +
|
||||
"(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. " +
|
||||
"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)
|
||||
"Throws if there is no in-progress merge for the task; use abort_merge to cancel a paused merge instead." +
|
||||
McpToolDocs.ProgressHint)]
|
||||
public async Task<MergeContinuationResultDto> ContinueMerge(
|
||||
string taskId,
|
||||
CancellationToken cancellationToken = default,
|
||||
IProgress<ProgressNotificationValue>? progress = null)
|
||||
{
|
||||
taskId = await TaskIdResolver.ResolveAsync(_tasks, taskId, cancellationToken);
|
||||
var task = await _tasks.GetByIdAsync(taskId, cancellationToken)
|
||||
@@ -1053,7 +1057,7 @@ public sealed class ExternalMcpService
|
||||
|
||||
if (_planningMerge.HasActiveMerge(taskId))
|
||||
{
|
||||
await _planningMerge.ContinueAsync(taskId, cancellationToken);
|
||||
await _planningMerge.ContinueAsync(taskId, cancellationToken, progress);
|
||||
var parent = (await _tasks.GetByIdAsync(taskId, cancellationToken))!;
|
||||
if (parent.Status == TaskStatus.Done)
|
||||
{
|
||||
@@ -1080,7 +1084,7 @@ public sealed class ExternalMcpService
|
||||
}
|
||||
else
|
||||
{
|
||||
var r = await _merge.ContinueMergeAsync(taskId, cancellationToken);
|
||||
var r = await _merge.ContinueMergeAsync(taskId, cancellationToken, progress);
|
||||
if (r.Status == TaskMergeService.StatusMerged)
|
||||
{
|
||||
merged = true;
|
||||
@@ -1464,20 +1468,29 @@ public sealed class ExternalMcpService
|
||||
|
||||
[McpServerTool, Description(
|
||||
"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.")]
|
||||
public async Task<IReadOnlyList<WorktreeListItemDto>> ListWorktrees(CancellationToken cancellationToken)
|
||||
"the ClaudeDo database appear here, and headCommit is empty when the path is missing from disk." +
|
||||
McpToolDocs.ProgressHint)]
|
||||
public async Task<IReadOnlyList<WorktreeListItemDto>> ListWorktrees(
|
||||
CancellationToken cancellationToken = default,
|
||||
IProgress<ProgressNotificationValue>? progress = null)
|
||||
{
|
||||
var rows = await _maintenance.GetOverviewAsync(null, cancellationToken);
|
||||
var results = await Task.WhenAll(rows.Select(async row =>
|
||||
{
|
||||
var isDirty = row.PathExistsOnDisk && await TryGetIsDirtyAsync(row.Path, cancellationToken);
|
||||
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);
|
||||
}));
|
||||
// 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
|
||||
// while, so this reports on the same elapsed-time cadence as a single long git call rather
|
||||
// than per-row (rows finish out of order under Task.WhenAll, so there's no natural i/n).
|
||||
var results = await ProgressReporter.RunAsync(
|
||||
Task.WhenAll(rows.Select(async row =>
|
||||
{
|
||||
var isDirty = row.PathExistsOnDisk && await TryGetIsDirtyAsync(row.Path, cancellationToken);
|
||||
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;
|
||||
}
|
||||
|
||||
|
||||
@@ -492,7 +492,8 @@ public sealed class TaskMergeService
|
||||
CancellationToken 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);
|
||||
|
||||
@@ -558,7 +559,7 @@ public sealed class TaskMergeService
|
||||
var targetBranch = await _git.GetCurrentBranchAsync(list.WorkingDir, 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)
|
||||
{
|
||||
_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.State;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
using ModelContextProtocol;
|
||||
using TaskStatus = ClaudeDo.Data.Models.TaskStatus;
|
||||
|
||||
namespace ClaudeDo.Worker.Planning;
|
||||
@@ -57,7 +58,8 @@ public sealed class PlanningMergeOrchestrator : IActiveMergeState
|
||||
}
|
||||
|
||||
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;
|
||||
List<TaskEntity> children;
|
||||
@@ -121,7 +123,7 @@ public sealed class PlanningMergeOrchestrator : IActiveMergeState
|
||||
throw new InvalidOperationException($"Merge already in progress for {parentTaskId}.");
|
||||
|
||||
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>
|
||||
@@ -145,14 +147,15 @@ public sealed class PlanningMergeOrchestrator : IActiveMergeState
|
||||
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)
|
||||
throw new InvalidOperationException(
|
||||
"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 result = await _merge.ContinueMergeAsync(current, ct);
|
||||
var result = await _merge.ContinueMergeAsync(current, ct, progress);
|
||||
|
||||
if (result.Status == TaskMergeService.StatusConflict)
|
||||
{
|
||||
@@ -173,7 +176,7 @@ public sealed class PlanningMergeOrchestrator : IActiveMergeState
|
||||
await _broadcaster.PlanningSubtaskMerged(planningTaskId, current);
|
||||
|
||||
state.CurrentSubtaskId = null;
|
||||
await DrainAsync(planningTaskId, ct);
|
||||
await DrainAsync(planningTaskId, ct, progress);
|
||||
}
|
||||
|
||||
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.
|
||||
}
|
||||
|
||||
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;
|
||||
|
||||
@@ -229,7 +233,8 @@ public sealed class PlanningMergeOrchestrator : IActiveMergeState
|
||||
removeWorktree: true,
|
||||
commitMessage: "", // blank -> TaskMergeService builds the conventional default
|
||||
leaveConflictsInTree: true,
|
||||
ct);
|
||||
ct,
|
||||
progress);
|
||||
|
||||
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);
|
||||
}
|
||||
|
||||
// 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]
|
||||
public async Task ReviewTask_Approve_ParentWithChildren_ReportsEmptyChildByName()
|
||||
{
|
||||
@@ -1316,6 +1386,33 @@ public sealed class ExternalMcpServiceTests : IDisposable
|
||||
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 ────────────────────────────────────────────────────
|
||||
|
||||
[Fact]
|
||||
@@ -1711,6 +1808,52 @@ public sealed class ExternalMcpServiceTests : IDisposable
|
||||
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]
|
||||
public async Task ContinueMerge_MarkersStillPresent_ReportsConflicts()
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user