Files
ClaudeDo/docs/superpowers/specs/2026-06-04-approve-merge-and-conflict-preview-design.md
2026-06-04 23:07:25 +02:00

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 ac9bae9 deliberately consolidated. All new controls go in its existing MERGE & WORKTREE block (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 Merged and existing auto-cleanup handles disposal (matches the single-task merge default removeWorktree:false).
  • Applies to: standalone leaf tasks with an active worktree. A WaitingForReview task with no active worktree (e.g. ran in a sandbox, or an improvement parent whose children own the worktrees) is just marked Done — current behavior preserved. Planning parents keep "Merge All Subtasks".

Acceptance (restated)

  1. Approve a clean-merging task → worktree merged into target, worktree Merged, task Done.
  2. Approve a conflicting task → task not Done, conflict surfaced.
  3. 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> from repoDir. merge-tree computes the merge base itself and writes only loose objects — it does not touch the working tree, index, or refs.
  • Exit code 0Clean = true, no conflict files.
  • Exit code 1Clean = 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) → return MergeResult(StatusMerged, [], null) ("approved, nothing to merge").
  • Active worktreeMergeAsync(taskId, target, removeWorktree:false, "Merge {branch}", ct). On StatusMerged_state.ApproveReviewAsync(taskId) then return the merged result. On StatusConflict/StatusBlocked → return as-is; do not flip status (task stays WaitingForReview).

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, when entity.Worktree != null and the task is not a planning parent, call GetMergeTargetsAsync(taskId) and set SelectedMergeTarget = DefaultBranch (fixes the standalone-task gap where targets were never loaded).
  • Mergeability indicator properties: MergePreviewText (string), MergeIsClean / MergeIsConflict (bool, for color). Compute via PreviewMergeAsync when the merge section is shown for an Active worktree; recompute on SelectedMergeTarget change. If worktree state is Merged/Discarded/Kept, show that label instead of probing. Text examples: "Merges cleanly · 7 files" / "Conflicts in a.cs, b.cs" / "Mergeability unknown".
  • Approve (ApproveReviewAsync): pass SelectedMergeTarget ?? ""; inspect result — on "conflict" set the conflict indicator + a short notice ("Approve blocked — resolve conflicts first"); success path relies on the existing TaskUpdated broadcast 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) when MergeIsClean, red (BloodBrush) when MergeIsConflict, 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=false with the expected file in ConflictFiles.
  • ApproveAndMergeAsync: clean worktree → returns merged, task is Done, worktree state Merged. Conflicting worktree → returns conflict, task still WaitingForReview, worktree still Active, target branch unmodified (HEAD unchanged, no MERGE_HEAD).
  • No-worktree WaitingForReview task → returns merged, task Done.

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).