chore(claude-do): MCP: review_task/merge_task senden Progress während des Veri
## Symptom (real aufgetreten, 2026-08-11) Ein `review_task(decision="approve")` über MCP lief >5 Min. Claude Codes MCP-Client brach den Call ab mit: > MCP server "claudedo" tool "review_task" sent no response or progress for 300s; aborting. Ergebnis: der Merge war **schon gelaufen und comitted** (`0e12be4`, `mergeCommit` am Worktree gesetzt), aber der Task blieb auf `WaitingForReview` hängen — ClaudeDo-Task: d8199f1f-3df3-447f-8de2-e7aca9ec5064
This commit is contained in:
mika kuns
+28
-14
@@ -15,6 +15,7 @@ using ClaudeDo.Worker.Queue;
|
||||
using ClaudeDo.Worker.State;
|
||||
using ClaudeDo.Worker.Worktrees;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
using ModelContextProtocol;
|
||||
using ModelContextProtocol.Server;
|
||||
using TaskStatus = ClaudeDo.Data.Models.TaskStatus;
|
||||
|
||||
@@ -644,7 +645,7 @@ public sealed class ExternalMcpService
|
||||
"means the merge stopped on conflicts, with the files listed. emptyChildren (parent approve only) lists " +
|
||||
"the Done children about to be unit-merged whose own review range contributed nothing (e.g. a child that " +
|
||||
"reported CLAUDEDO_BLOCKED and committed no code) — check it before trusting that every child actually " +
|
||||
"delivered something." + McpToolDocs.LeanTaskRef + McpToolDocs.TaskNumberHint)]
|
||||
"delivered something." + McpToolDocs.LeanTaskRef + McpToolDocs.TaskNumberHint + McpToolDocs.ProgressHint)]
|
||||
public async Task<ReviewTaskResult> ReviewTask(
|
||||
string taskId,
|
||||
[Description("'approve', 'reject_rerun', 'reject_park' or 'cancel'.")]
|
||||
@@ -660,8 +661,15 @@ public sealed class ExternalMcpService
|
||||
"in the working tree (repoPath in the result) so you can resolve them and call continue_merge, " +
|
||||
"or abort_merge to cancel.")]
|
||||
bool leaveConflictsInTree = false,
|
||||
CancellationToken cancellationToken = default)
|
||||
CancellationToken cancellationToken = default,
|
||||
IProgress<ProgressNotificationValue>? progress = null)
|
||||
{
|
||||
// First report fires before any git/verify work starts -- an approve that has to wait on
|
||||
// the per-repo merge gate (another merge/verify already running) must not sit silent long
|
||||
// enough to trip Claude Code's ~300s MCP idle-silence abort before RunVerifyGateAsync's own
|
||||
// periodic reports even begin.
|
||||
progress?.Report(new ProgressNotificationValue { Progress = 0, Message = "review_task started" });
|
||||
|
||||
taskId = await TaskIdResolver.ResolveAsync(_tasks, taskId, cancellationToken);
|
||||
var task = await _tasks.GetByIdAsync(taskId, cancellationToken)
|
||||
?? throw new InvalidOperationException($"Task {taskId} not found.");
|
||||
@@ -702,7 +710,7 @@ public sealed class ExternalMcpService
|
||||
}
|
||||
else
|
||||
{
|
||||
var r = await _merge.ApproveAndMergeAsync(taskId, targetBranch ?? "", leaveConflictsInTree, cancellationToken);
|
||||
var r = await _merge.ApproveAndMergeAsync(taskId, targetBranch ?? "", leaveConflictsInTree, cancellationToken, progress);
|
||||
if (r.Status == TaskMergeService.StatusBlocked)
|
||||
throw new InvalidOperationException(r.ErrorMessage ?? "approve failed");
|
||||
mergeStatus = r.Status;
|
||||
@@ -947,7 +955,7 @@ public sealed class ExternalMcpService
|
||||
[McpServerTool, Description(
|
||||
"Merge a Done task's worktree branch into targetBranch. For a task still in WaitingForReview prefer " +
|
||||
"review_task, which merges as part of approving. merged=true carries the new mergeCommit SHA; on conflict " +
|
||||
"merged=false and conflicts lists the affected files.")]
|
||||
"merged=false and conflicts lists the affected files." + McpToolDocs.ProgressHint)]
|
||||
public async Task<MergeTaskResultDto> MergeTask(
|
||||
string taskId,
|
||||
string targetBranch = "main",
|
||||
@@ -961,8 +969,11 @@ public sealed class ExternalMcpService
|
||||
"leave the conflict markers in the working tree at repoPath (conflictsInTree=true) so you can " +
|
||||
"resolve them there and call continue_merge, or abort_merge to cancel.")]
|
||||
bool leaveConflictsInTree = false,
|
||||
CancellationToken cancellationToken = default)
|
||||
CancellationToken cancellationToken = default,
|
||||
IProgress<ProgressNotificationValue>? progress = null)
|
||||
{
|
||||
progress?.Report(new ProgressNotificationValue { Progress = 0, Message = "merge_task started" });
|
||||
|
||||
taskId = await TaskIdResolver.ResolveAsync(_tasks, taskId, cancellationToken);
|
||||
var task = await _tasks.GetByIdAsync(taskId, cancellationToken)
|
||||
?? throw new InvalidOperationException($"Task {taskId} not found.");
|
||||
@@ -988,7 +999,7 @@ public sealed class ExternalMcpService
|
||||
|
||||
var commitMessage = $"Merge task branch for: {task.Title}";
|
||||
var result = await _merge.MergeAsync(
|
||||
taskId, targetBranch, removeWorktree: false, commitMessage, leaveConflictsInTree, cancellationToken);
|
||||
taskId, targetBranch, removeWorktree: false, commitMessage, leaveConflictsInTree, cancellationToken, progress);
|
||||
|
||||
if (result.Status == TaskMergeService.StatusMerged)
|
||||
{
|
||||
@@ -1226,15 +1237,16 @@ public sealed class ExternalMcpService
|
||||
"branch can be far behind yet touch nothing the target changed, or barely behind yet collide on the one " +
|
||||
"file that matters (always empty for a worktree-less handler task, which has no fork point). Throws if the " +
|
||||
"task has neither an active worktree nor a handler commit range, or the list's working directory is missing " +
|
||||
"from disk.")]
|
||||
"from disk." + McpToolDocs.ProgressHint)]
|
||||
public async Task<MergePreviewToolDto> PreviewMerge(
|
||||
string taskId,
|
||||
[Description("Branch to preview against; defaults to the repo's current branch.")]
|
||||
string? targetBranch = null,
|
||||
CancellationToken cancellationToken = default)
|
||||
CancellationToken cancellationToken = default,
|
||||
IProgress<ProgressNotificationValue>? progress = null)
|
||||
{
|
||||
taskId = await TaskIdResolver.ResolveAsync(_tasks, taskId, cancellationToken);
|
||||
var (preview, behind, _, isEmpty, staleFiles, _) = await PreviewMergeCoreAsync(taskId, targetBranch, runVerify: true, cancellationToken);
|
||||
var (preview, behind, _, isEmpty, staleFiles, _) = await PreviewMergeCoreAsync(taskId, targetBranch, runVerify: true, cancellationToken, progress);
|
||||
return new MergePreviewToolDto(preview.Status, preview.ConflictFiles, preview.ChangedFileCount, behind, isEmpty,
|
||||
preview.VerifyExitCode, preview.VerifyDurationMs, preview.VerifyOutputTail, staleFiles);
|
||||
}
|
||||
@@ -1251,7 +1263,7 @@ public sealed class ExternalMcpService
|
||||
"collide unflagged, and as with preview_merge a clean result does not mean the merge builds. " +
|
||||
"runVerify=false (default) never builds — set it true to also run each task's list's verify command in a " +
|
||||
"scratch worktree per entry (same fields as preview_merge); this can take a long time across many tasks, " +
|
||||
"since builds run one at a time.")]
|
||||
"since builds run one at a time." + McpToolDocs.ProgressHint)]
|
||||
public async Task<MergePreviewSetResultDto> PreviewMergeSet(
|
||||
IReadOnlyList<string> taskIds,
|
||||
[Description("Branch to preview every task against; defaults to the repo's current branch.")]
|
||||
@@ -1259,7 +1271,8 @@ public sealed class ExternalMcpService
|
||||
[Description("true: also run the verify command (if configured) for each task, one build at a time. " +
|
||||
"false (default): no builds, however many tasks are given.")]
|
||||
bool runVerify = false,
|
||||
CancellationToken cancellationToken = default)
|
||||
CancellationToken cancellationToken = default,
|
||||
IProgress<ProgressNotificationValue>? progress = null)
|
||||
{
|
||||
if (taskIds is null || taskIds.Count == 0)
|
||||
throw new InvalidOperationException("taskIds must contain at least one task id.");
|
||||
@@ -1274,7 +1287,7 @@ public sealed class ExternalMcpService
|
||||
{
|
||||
try
|
||||
{
|
||||
var (preview, behind, changedFiles, isEmpty, staleFiles, number) = await PreviewMergeCoreAsync(taskId, targetBranch, runVerify, cancellationToken);
|
||||
var (preview, behind, changedFiles, isEmpty, staleFiles, number) = await PreviewMergeCoreAsync(taskId, targetBranch, runVerify, cancellationToken, progress);
|
||||
entries.Add(new MergePreviewSetEntryDto(
|
||||
taskId, preview.Status, preview.ConflictFiles, preview.ChangedFileCount, behind, null, isEmpty,
|
||||
preview.VerifyExitCode, preview.VerifyDurationMs, preview.VerifyOutputTail, staleFiles, number));
|
||||
@@ -1343,7 +1356,8 @@ public sealed class ExternalMcpService
|
||||
// HandlerBaseCommit..HandlerHeadCommit range, reporting a synthetic "clean" preview of that
|
||||
// range's own diff-stat instead of throwing "has no worktree".
|
||||
private async Task<(MergePreviewResult Preview, int Behind, IReadOnlyList<string> ChangedFiles, bool IsEmpty, IReadOnlyList<string> StaleFiles, int Number)> PreviewMergeCoreAsync(
|
||||
string taskId, string? targetBranch, bool runVerify, CancellationToken ct)
|
||||
string taskId, string? targetBranch, bool runVerify, CancellationToken ct,
|
||||
IProgress<ProgressNotificationValue>? progress = null)
|
||||
{
|
||||
using var ctx = _dbFactory.CreateDbContext();
|
||||
var task = await new TaskRepository(ctx).GetByIdAsync(taskId, ct)
|
||||
@@ -1360,7 +1374,7 @@ public sealed class ExternalMcpService
|
||||
if (string.IsNullOrWhiteSpace(list.WorkingDir) || !Directory.Exists(list.WorkingDir))
|
||||
throw new InvalidOperationException("The list's working directory no longer exists.");
|
||||
|
||||
var preview = await _merge.PreviewAsync(taskId, targetBranch ?? "", runVerify, ct);
|
||||
var preview = await _merge.PreviewAsync(taskId, targetBranch ?? "", runVerify, ct, progress);
|
||||
if (preview.Status == TaskMergeService.PreviewUnavailable)
|
||||
throw new InvalidOperationException(
|
||||
"Merge preview unavailable for this task (worktree inactive or repo is not a git repository).");
|
||||
|
||||
Reference in New Issue
Block a user