8.7 KiB
Approve = Merge → Done, plus Conflict Preview — Design
Date: 2026-06-04 Status: Approved (autonomous — user on break, authorized to continue) Author: brainstormed from issue "Make merge/diff real"
Problem
Approving a WaitingForReview task flips it straight to Done
(TaskStateService.ApproveReviewAsync) and never merges its worktree — the
worktree stays Active. The user approved three component tasks expecting them
to merge; none did. Separately, there is no way to see whether a task's
worktree merges cleanly before acting, and a standalone task has no direct
Merge button (single-task merge is only reachable from inside the Diff
modal).
What is already real (verified): WorkerHub.MergeTask → TaskMergeService.MergeAsync
performs a real git merge --no-ff, aborts on conflict, and marks the worktree
Merged. Open Diff opens a real in-app diff. Merge All Subtasks
(planning) is real. So the gaps are narrow.
Scope decisions (autonomous)
- Tab location: keep the single "Session" tab that the recent commit
ac9bae9deliberately consolidated. All new controls go in its existingMERGE & WORKTREEblock (WorkConsole.axaml:196). Do not re-introduce a separate "Actions" tab. - Approve target: Approve merges into the UI-selected merge target
(
SelectedMergeTarget); when blank, the worker resolves to the repo's current branch. - On conflict: task stays in
WaitingForReview(no new status). The conflict is surfaced inline. No automatic state change to a "blocked" status. - Worktree removal on approve: do not remove — merge marks the worktree
Mergedand existing auto-cleanup handles disposal (matches the single-task merge defaultremoveWorktree:false). - Applies to: standalone leaf tasks with an active worktree. A
WaitingForReviewtask with no active worktree (e.g. ran in a sandbox, or an improvement parent whose children own the worktrees) is just markedDone— current behavior preserved. Planning parents keep "Merge All Subtasks".
Acceptance (restated)
- Approve a clean-merging task → worktree merged into target, worktree
Merged, taskDone. - Approve a conflicting task → task not
Done, conflict surfaced. - Opening a Done/WaitingForReview task shows clean/conflict status without
mutating the tree (use
git merge-tree, not a real merge).
Architecture
Three layers, each single-purpose; the only new cross-dependency is
TaskMergeService → ITaskStateService (one-way; verify no DI cycle).
1. GitService — non-destructive conflict probe (ClaudeDo.Data)
New method:
public sealed record MergePreview(bool Supported, bool Clean, IReadOnlyList<string> ConflictFiles);
public async Task<MergePreview> PreviewMergeAsync(
string repoDir, string targetBranch, string sourceBranch, CancellationToken ct = default)
- Runs
git merge-tree --write-tree --name-only <target> <source>fromrepoDir.merge-treecomputes the merge base itself and writes only loose objects — it does not touch the working tree, index, or refs. - Exit code
0→Clean = true, no conflict files. - Exit code
1→Clean = false; conflicted paths are the lines after the first (tree-OID) line, up to the first blank line. - Any other outcome (e.g. git too old → "unknown option") →
Supported = false(UI shows "mergeability unknown").
New helper for the "· N files" count (clean case):
git diff --name-only <target>...<source> (three-dot = changes on source since
the merge base); count non-empty lines. May reuse/extend existing diff helpers.
2. TaskMergeService — preview + approve orchestration (ClaudeDo.Worker)
Inject ITaskStateService (verify PlanningChainCoordinator has no back-edge to
TaskMergeService; if a cycle exists, fall back to orchestrating in the hub).
public sealed record MergePreviewResult(string Status, IReadOnlyList<string> ConflictFiles, int ChangedFileCount);
// Status: "clean" | "conflict" | "unavailable"
public Task<MergePreviewResult> PreviewAsync(string taskId, string targetBranch, CancellationToken ct);
public Task<MergeResult> ApproveAndMergeAsync(string taskId, string targetBranch, CancellationToken ct);
PreviewAsync: load context. If no active worktree → "unavailable". Resolve
targetBranch (blank → current branch). Call GitService.PreviewMergeAsync; map
Supported=false → "unavailable", else clean/conflict (+ ChangedFileCount on
clean).
ApproveAndMergeAsync: load context; require task.Status == WaitingForReview
(else Blocked). Resolve target (blank → current branch).
- No active worktree →
_state.ApproveReviewAsync(taskId)→ returnMergeResult(StatusMerged, [], null)("approved, nothing to merge"). - Active worktree →
MergeAsync(taskId, target, removeWorktree:false, "Merge {branch}", ct). OnStatusMerged→_state.ApproveReviewAsync(taskId)then return the merged result. OnStatusConflict/StatusBlocked→ return as-is; do not flip status (task staysWaitingForReview).
TaskStateService.ApproveReviewAsync is unchanged (still the sole Status writer;
still runs OnChildTerminalAsync).
3. WorkerHub — signatures (ClaudeDo.Worker)
public record MergePreviewDto(string Status, IReadOnlyList<string> ConflictFiles, int ChangedFileCount);
public Task<MergePreviewDto> PreviewMerge(string taskId, string targetBranch); // new
public Task<MergeResultDto> ApproveReview(string taskId, string targetBranch); // CHANGED: was void(taskId)
ApproveReview returns the orchestration result so the UI can react to conflicts.
MergeTask / GetMergeTargets unchanged.
4. UI (ClaudeDo.Ui)
IWorkerClient (+ WorkerClient + both test-project fakes — see memory:
changing IWorkerClient breaks hand-rolled fakes):
- Change
Task ApproveReviewAsync(string)→Task<MergeResultDto?> ApproveReviewAsync(string taskId, string targetBranch). - Add
Task<MergePreviewDto?> PreviewMergeAsync(string taskId, string targetBranch). - Add
Task<MergeResultDto> MergeTaskAsync(...)to the interface (already on the concrete client) so the single-task Merge button can use_worker.
DetailsIslandViewModel:
- Load merge targets whenever a worktree exists. In
BindAsync, whenentity.Worktree != nulland the task is not a planning parent, callGetMergeTargetsAsync(taskId)and setSelectedMergeTarget = DefaultBranch(fixes the standalone-task gap where targets were never loaded). - Mergeability indicator properties:
MergePreviewText(string),MergeIsClean/MergeIsConflict(bool, for color). Compute viaPreviewMergeAsyncwhen the merge section is shown for an Active worktree; recompute onSelectedMergeTargetchange. If worktree state isMerged/Discarded/Kept, show that label instead of probing. Text examples: "Merges cleanly · 7 files" / "Conflicts in a.cs, b.cs" / "Mergeability unknown". - Approve (
ApproveReviewAsync): passSelectedMergeTarget ?? ""; inspect result — on"conflict"set the conflict indicator + a short notice ("Approve blocked — resolve conflicts first"); success path relies on the existingTaskUpdatedbroadcast to refresh. - Single-task Merge (
MergeCommand):MergeTaskAsync(taskId, SelectedMergeTarget ?? "", removeWorktree:false, "Merge task"); on"conflict"show the conflict indicator. Shown for non-planning tasks with an active worktree (planning parents keep "Merge All Subtasks").
WorkConsole.axaml (Session tab, MERGE & WORKTREE block):
- Add a status line above the button row bound to
MergePreviewText, colored green (MossBrush) whenMergeIsClean, red (BloodBrush) whenMergeIsConflict, muted otherwise. Use existing tokens/classes only. - Add a Merge button (
MergeCommand) beside Open Diff for the single-task path.
Testing (git-backed, no real Claude)
In ClaudeDo.Worker.Tests (real temp git repos + real SQLite), and/or
ClaudeDo.Data.Tests for the pure git probe:
GitService.PreviewMergeAsync: clean branches →Clean=true; a real edit-conflict on the same lines →Clean=falsewith the expected file inConflictFiles.ApproveAndMergeAsync: clean worktree → returnsmerged, task isDone, worktree stateMerged. Conflicting worktree → returnsconflict, task stillWaitingForReview, worktree stillActive, target branch unmodified (HEAD unchanged, noMERGE_HEAD).- No-worktree
WaitingForReviewtask → returnsmerged, taskDone.
Out of scope
External difftools, new task statuses, auto-removing worktrees on approve,
re-splitting the console into separate tabs, conflict resolution UI (the existing
ContinueMerge/AbortMerge paths remain as-is for mid-merge cases).