feat(worker): add preview_merge and preview_merge_set MCP tools
Give an autonomous reviewer a non-destructive merge-tree preview (status/conflicts/changedFileCount/behind) for a task's worktree branch, plus a file-overlap check across a batch of tasks so same-file collisions between sibling branches are visible before merging. MergeHelperDefault's Phase 4 now calls preview_merge_set before merging a batch.
This commit is contained in:
mika kuns
@@ -273,14 +273,18 @@ public static class PromptFiles
|
||||
Poll get_task until every task has left Queued and Running — WaitingForReview on success, Failed on error. Report progress as tasks land; do not poll silently for minutes.
|
||||
|
||||
## Phase 4 — Review and merge
|
||||
One task at a time, in the order the brief lists them.
|
||||
Before merging anything, call preview_merge_set with every surviving task's id (the same targetBranch you are about to merge into). It tells you, per task, whether a clean merge-tree preview is even possible (status/conflictFiles/changedFileCount/behind) and which files more than one of the tasks changed (overlaps). Read the overlaps: a file two tasks both touch is where a same-branch collision could happen. This is a HINT, not proof — it only catches same-file overlap, not a cross-file break (e.g. one task deletes a symbol another task's file still references), and a clean preview never guarantees the result compiles or passes tests. Use it to decide merge order and to know which pairs to look at extra carefully in step 1 below; it does not replace reading the diffs.
|
||||
|
||||
1. Inspect the change with get_task_diff (stat first, then the full diff if it is non-trivial) and sanity-check it against the task's title and description.
|
||||
One task at a time, in the order the brief lists them (adjust the order if the overlap check suggests a safer sequence).
|
||||
|
||||
1. Inspect the change with get_task_diff (stat first, then the full diff if it is non-trivial) and sanity-check it against the task's title and description. If preview_merge_set flagged this task in an overlap, also skim the diff of the other task(s) sharing that file.
|
||||
2. If the change looks wrong, incomplete, or risky, STOP and ask the user before merging — offer reject_rerun (with feedback) or skip.
|
||||
3. Otherwise merge with review_task(taskId, decision="approve", leaveConflictsInTree=true).
|
||||
- Clean merge → the task is Done; move on.
|
||||
- Conflict (markers left in the working tree, repoPath returned) → resolve it.
|
||||
|
||||
After each merge, main has moved — a clean preview_merge_set result from before this merge is now stale for the remaining tasks. If you are unsure whether an earlier overlap warning still matters, call preview_merge for the next task again before merging it.
|
||||
|
||||
Every branch in this run forked from the same base, so conflicts between them are the NORMAL case, not a failure. Resolve them and keep going; do not abandon the run because a merge conflicted.
|
||||
|
||||
Resolving a conflict:
|
||||
|
||||
@@ -34,7 +34,7 @@ Interfaces (e.g. `IQueueWaker`, `IPrimeClock`, `ITaskStateService`) live in an `
|
||||
- **OverrideSlotService** — owns `RunNow` / `ContinueTask`; goes through `TaskStateService.StartRunningAsync` (caller-driven, serialized by slot lock).
|
||||
- **StaleTaskRecovery** — startup-only service; calls `TaskStateService.RecoverStaleRunningAsync` to flip orphaned `Running` rows to `Failed`.
|
||||
- **External/*** — always-on MCP tools for general Claude sessions, scoped to *starting* and *observing* sessions (no worktree/merge, multi-turn, planning, or app-settings writes). Auth via optional `X-ClaudeDo-Key` header. Registered explicitly in `Program.cs`'s external app via `.WithTools<T>()`. Every optional/filter parameter across these tools must carry a C# default value (e.g. `string? status = null`) — the MCP schema only marks a parameter optional when it has one; nullability alone doesn't do it (`ExternalMcpToolSchemaTests` guards this by reflection). `ExternalMcpExceptionFilter.Wrap` is registered as a call-tool filter so `InvalidOperationException`/`ArgumentException` messages survive as `McpException` — otherwise the SDK's own catch-all replaces any non-`McpException` with a generic "An error occurred invoking 'X'." Organized by concern:
|
||||
- `ExternalMcpService` — task CRUD + execution: `ListTaskLists`, `ListTasks`, `GetTask`, `AddTask`, `AddSubtask`, `UpdateTask`, `UpdateTaskStatus` (`Idle` / `Queued` / `Cancelled` / `Done` — `Cancelled` goes through `TaskStateService.CancelAsync(..., allowFromIdle: true)`, the only caller that opts into cancelling from `Idle`; `PlanningChainCoordinator` relies on `Idle` staying a no-op there by default, since a child parked back to `Idle` mid-chain is a manual opt-out signal; `Done` goes through `TaskStateService.ForceSetStatusAsync` — same unconditional write the UI's "set status freely" affordance uses — but is refused here with an active-worktree task, since that would skip `review_task`'s merge), `GetTaskStatusValues`, `ReviewTask` (`approve` / `reject_rerun` / `reject_park` / `cancel` for a WaitingForReview task; approve is review+merge exactly like the hub's `ApproveReview` — unit merge for parents, worktree merge into optional `targetBranch` for childless tasks, conflicts reported in the `ReviewTaskResult`), `RunTaskNow`, `ContinueTask`, `CancelTask`, `DeleteTask`; worktree/git: `GetTaskWorktree`, `GetTaskDiff`, `MergeTask`, `ListWorktrees`, `CleanupTaskWorktree`
|
||||
- `ExternalMcpService` — task CRUD + execution: `ListTaskLists`, `ListTasks`, `GetTask`, `AddTask`, `AddSubtask`, `UpdateTask`, `UpdateTaskStatus` (`Idle` / `Queued` / `Cancelled` / `Done` — `Cancelled` goes through `TaskStateService.CancelAsync(..., allowFromIdle: true)`, the only caller that opts into cancelling from `Idle`; `PlanningChainCoordinator` relies on `Idle` staying a no-op there by default, since a child parked back to `Idle` mid-chain is a manual opt-out signal; `Done` goes through `TaskStateService.ForceSetStatusAsync` — same unconditional write the UI's "set status freely" affordance uses — but is refused here with an active-worktree task, since that would skip `review_task`'s merge), `GetTaskStatusValues`, `ReviewTask` (`approve` / `reject_rerun` / `reject_park` / `cancel` for a WaitingForReview task; approve is review+merge exactly like the hub's `ApproveReview` — unit merge for parents, worktree merge into optional `targetBranch` for childless tasks, conflicts reported in the `ReviewTaskResult`), `RunTaskNow`, `ContinueTask`, `CancelTask`, `DeleteTask`; worktree/git: `GetTaskWorktree`, `GetTaskDiff`, `MergeTask`, `PreviewMerge` (non-destructive `git merge-tree --write-tree` mergeability check for one task's worktree branch against `targetBranch`, default the repo's current branch — status/conflictFiles/changedFileCount plus `behind`; throws a clear error instead of TaskMergeService.PreviewAsync's silent "unavailable" when the task has no worktree, the worktree isn't Active, or the list's working dir is missing), `PreviewMergeSet` (same preview for a batch of task ids plus a file→tasks overlap report built from each task's own diff-stat — a same-file-name hint only, blind to cross-file collisions like the CS0103 case that motivated it; a task that fails to preview gets `error` set and is excluded from the overlap instead of aborting the batch), `ListWorktrees`, `CleanupTaskWorktree`
|
||||
- `BatchMcpTools` — best-effort batch variants that loop the `ExternalMcpService` single-entity methods (sequential — the scoped DbContext is not thread-safe; merge/review stay single-task): `BatchGetTasks`, `BatchAddTasks`, `BatchUpdateTaskStatus`, `BatchCancelTasks`, `BatchDeleteTasks`, `BatchSetMyDay`, `BatchCleanupTaskWorktrees`. Every tool returns a per-item result array ({ id/index, ok, error?, … }) — a failing item never aborts the rest — and rejects batches over 100 items.
|
||||
- `ListMcpTools` — `CreateList`, `UpdateList`, `DeleteList`
|
||||
- `ConfigMcpTools` — `GetListConfig`, `SetListConfig`, `GetTaskConfig`, `SetTaskConfig`
|
||||
|
||||
+109
@@ -51,6 +51,17 @@ public sealed record MergeContinuationResultDto(
|
||||
bool Merged, string TaskStatus, IReadOnlyList<string> Conflicts,
|
||||
string? RepoPath, string? Message);
|
||||
|
||||
public sealed record MergePreviewToolDto(
|
||||
string Status, IReadOnlyList<string> ConflictFiles, int ChangedFileCount, int Behind);
|
||||
|
||||
public sealed record MergePreviewSetEntryDto(
|
||||
string TaskId, string Status, IReadOnlyList<string> ConflictFiles, int ChangedFileCount, int Behind, string? Error);
|
||||
|
||||
public sealed record FileOverlapDto(string File, IReadOnlyList<string> TaskIds);
|
||||
|
||||
public sealed record MergePreviewSetResultDto(
|
||||
IReadOnlyList<MergePreviewSetEntryDto> Tasks, IReadOnlyList<FileOverlapDto> Overlaps);
|
||||
|
||||
public sealed record WorktreeListItemDto(
|
||||
string? TaskId, string Path, string Branch,
|
||||
string HeadCommit, bool IsDirty, bool MergedIntoMain);
|
||||
@@ -727,6 +738,104 @@ public sealed class ExternalMcpService
|
||||
return ToDto((await _tasks.GetByIdAsync(taskId, cancellationToken))!);
|
||||
}
|
||||
|
||||
[McpServerTool, Description(
|
||||
"Non-destructive merge preview for a task's worktree branch against targetBranch (default: the repo's " +
|
||||
"current branch), via `git merge-tree --write-tree` — does NOT touch the working tree, index, or HEAD. " +
|
||||
"status: 'clean' (mergeable; changedFileCount is the size of that merge) or 'conflict' (conflictFiles " +
|
||||
"lists the paths git would stop on). behind = commits on targetBranch not yet on this task's branch, so " +
|
||||
"you can spot a stale branch even when the preview itself is clean. " +
|
||||
"IMPORTANT: a clean preview says nothing about whether the merged result compiles or passes tests — git " +
|
||||
"can merge two changes cleanly (e.g. one file deletes a symbol another file still references) and still " +
|
||||
"break the build. " +
|
||||
"Throws a clear error if the task has no worktree, the worktree is not Active, or the list's working " +
|
||||
"directory is missing from disk.")]
|
||||
public async Task<MergePreviewToolDto> PreviewMerge(
|
||||
string taskId,
|
||||
string? targetBranch = null,
|
||||
CancellationToken cancellationToken = default)
|
||||
{
|
||||
var (preview, behind, _) = await PreviewMergeCoreAsync(taskId, targetBranch, cancellationToken);
|
||||
return new MergePreviewToolDto(preview.Status, preview.ConflictFiles, preview.ChangedFileCount, behind);
|
||||
}
|
||||
|
||||
[McpServerTool, Description(
|
||||
"Merge preview plus file-overlap check across several tasks at once, all previewed against the same " +
|
||||
"targetBranch (default: the repo's current branch). For each taskId returns the same fields as " +
|
||||
"preview_merge (status/conflictFiles/changedFileCount/behind; error is set instead if that task could not " +
|
||||
"be previewed, and it is then excluded from the overlap computation). overlaps lists, for each file " +
|
||||
"touched by MORE THAN ONE of the given tasks (via each task's own diff, not the merge preview itself), " +
|
||||
"which tasks touch it — passing a single taskId always yields an empty overlaps list. " +
|
||||
"IMPORTANT: file-name overlap is a HINT, not a guarantee of a real collision, and its absence is not a " +
|
||||
"guarantee of safety — two tasks touching different files entirely (e.g. one deletes a symbol, another " +
|
||||
"still references it elsewhere) can still collide, and this tool will not flag that case.")]
|
||||
public async Task<MergePreviewSetResultDto> PreviewMergeSet(
|
||||
IReadOnlyList<string> taskIds,
|
||||
string? targetBranch = null,
|
||||
CancellationToken cancellationToken = default)
|
||||
{
|
||||
if (taskIds is null || taskIds.Count == 0)
|
||||
throw new InvalidOperationException("taskIds must contain at least one task id.");
|
||||
|
||||
var entries = new List<MergePreviewSetEntryDto>();
|
||||
var filesByTask = new Dictionary<string, IReadOnlyList<string>>();
|
||||
|
||||
foreach (var taskId in taskIds)
|
||||
{
|
||||
try
|
||||
{
|
||||
var (preview, behind, changedFiles) = await PreviewMergeCoreAsync(taskId, targetBranch, cancellationToken);
|
||||
entries.Add(new MergePreviewSetEntryDto(
|
||||
taskId, preview.Status, preview.ConflictFiles, preview.ChangedFileCount, behind, null));
|
||||
filesByTask[taskId] = changedFiles;
|
||||
}
|
||||
catch (InvalidOperationException ex)
|
||||
{
|
||||
entries.Add(new MergePreviewSetEntryDto(
|
||||
taskId, TaskMergeService.PreviewUnavailable, Array.Empty<string>(), 0, 0, ex.Message));
|
||||
}
|
||||
}
|
||||
|
||||
var overlaps = filesByTask
|
||||
.SelectMany(kv => kv.Value.Select(f => (File: f, TaskId: kv.Key)))
|
||||
.GroupBy(x => x.File, StringComparer.OrdinalIgnoreCase)
|
||||
.Where(g => g.Select(x => x.TaskId).Distinct().Count() > 1)
|
||||
.Select(g => new FileOverlapDto(g.Key, g.Select(x => x.TaskId).Distinct().ToList()))
|
||||
.OrderBy(o => o.File, StringComparer.OrdinalIgnoreCase)
|
||||
.ToList();
|
||||
|
||||
return new MergePreviewSetResultDto(entries, overlaps);
|
||||
}
|
||||
|
||||
// Shared core for PreviewMerge/PreviewMergeSet: throws a clear InvalidOperationException instead of
|
||||
// TaskMergeService.PreviewAsync's silent "unavailable" status, and adds `behind` + the task's own
|
||||
// changed-file list (via diff-stat, not the merge-tree preview) for overlap detection.
|
||||
private async Task<(MergePreviewResult Preview, int Behind, IReadOnlyList<string> ChangedFiles)> PreviewMergeCoreAsync(
|
||||
string taskId, string? targetBranch, CancellationToken ct)
|
||||
{
|
||||
var (_, list, wt) = await LoadWorktreeContextAsync(taskId, ct);
|
||||
if (wt.State != WorktreeState.Active)
|
||||
throw new InvalidOperationException(
|
||||
$"Worktree state must be Active to preview a merge (current: {wt.State}).");
|
||||
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 ?? "", ct);
|
||||
if (preview.Status == TaskMergeService.PreviewUnavailable)
|
||||
throw new InvalidOperationException(
|
||||
"Merge preview unavailable for this task (worktree inactive or repo is not a git repository).");
|
||||
|
||||
var target = string.IsNullOrWhiteSpace(targetBranch)
|
||||
? await _git.GetCurrentBranchAsync(list.WorkingDir, ct)
|
||||
: targetBranch;
|
||||
var behind = await GitRevListCountAsync(list.WorkingDir, $"{wt.BranchName}..{target}", ct);
|
||||
|
||||
var changedFiles = Directory.Exists(wt.Path)
|
||||
? ParseDiffStatFileNames(await _git.DiffStatAsync(wt.Path, wt.BaseCommit, "HEAD", ct))
|
||||
: Array.Empty<string>();
|
||||
|
||||
return (preview, behind, changedFiles);
|
||||
}
|
||||
|
||||
[McpServerTool, Description(
|
||||
"List all ClaudeDo-tracked worktrees. " +
|
||||
"Each entry: taskId, path, branch, headCommit (empty if path missing on disk), " +
|
||||
|
||||
Reference in New Issue
Block a user