Files
ClaudeDo/docs/superpowers/specs/2026-06-05-git-merge-review-rework-design.md
mika kuns f4416ee1c3 docs(design): git tab merge & review rework — shared foundation + 3 layers
Design for simplifying single-task review/merge (Layer A), multi-worktree
batch merge cockpit (Layer B), and inline conflict resolver (Layer C),
with frozen shared contracts so B/C build in parallel worktrees.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2026-06-05 10:09:10 +02:00

9.9 KiB

Git Tab / Merge & Review Rework — Design

Date: 2026-06-05 Status: Approved

Goal

Make handling merges and reviews as simple as possible in the Terminal component's Git tab, and rework the diff viewers and worktree modals along the way. The work is split into three layers built across separate sessions, with a shared foundation that is built and pushed first so the parallel sessions branch from frozen contracts.

The user mostly trusts task output but wants the diff one click away for important work, and wants to land several independently-queued worktrees without per-task hopping or hand-resolving conflicts in an external editor.

Layers

  • Layer A — Review/merge cockpit (this session). Single-task review + merge UX in the Git tab; consolidate the four diff renderers into one DiffView.
  • Layer B — Multi-worktree merge cockpit (parallel session). Batch-merge N worktrees into one target, skip-and-continue, conflicts collected for resolution.
  • Layer C — Inline conflict resolver (parallel session). VSCode-style inline hunk resolver plus the worker-side conflict plumbing it needs.

They stack: A defines the single-task flow, B reuses it for many tasks, both funnel conflicts into C.

Shared foundation (built & pushed this session, before B/C branch)

Everything B and C depend on lands first on main. B and C branch from that commit.

1. One diff model + one DiffView control

Today there are four diff renderers and two parallel diff models:

  • DiffLinesView.axaml (used by DiffModalView)
  • the inline diff ItemsControl in WorktreeModalView.axaml
  • PlanningDiffView.axaml
  • their backing models: DiffFileViewModel/DiffLineViewModel (+ UnifiedDiffParser) vs WorktreeNodeViewModel/WorktreeDiffLineViewModel

Collapse into a single canonical diff model + parser + a DiffView UserControl. All diff rendering across the app goes through DiffView.

  • Model: DiffFileViewModel { Path, AddCount, DelCount, Lines }, DiffLineViewModel { OldNo, NewNo, Kind (Add|Del|Ctx|File|Hunk), Text }.
  • Parser: one static UnifiedDiffParser.Parse(rawUnifiedDiff) returning the model.
  • DiffView exposes a Files styled property (file list + selected-file lines), or a simpler Lines property for single-file use — Layer A decides the exact surface while building it, but the type names above are frozen so B and C can bind to them.

2. Frozen worker conflict contract

Added to IWorkerClient (and WorkerClient with stub bodies that throw NotSupportedException) plus new DTOs, so A and B compile against the interface while C provides the real worker-side implementation.

// IWorkerClient additions (signatures frozen this session)
Task<MergeResultDto>            StartConflictMergeAsync(string taskId, string targetBranch);
Task<MergeConflictsDto>         GetMergeConflictsAsync(string taskId);
Task                            WriteConflictResolutionAsync(string taskId, string path, string resolvedContent);
Task<MergeResultDto>            ContinueMergeAsync(string taskId);
Task                            AbortMergeAsync(string taskId);
  • StartConflictMergeAsync performs the merge with leaveConflictsInTree: true (the worker already supports this flag — used today by the planning orchestrator) and returns MergeResultDto with Status="conflict" and the conflict file list, leaving .git/MERGE_HEAD in place in the list's WorkingDir.
  • GetMergeConflictsAsync returns each conflicted file with ours/theirs/base content, read via git show :2:<path> (ours), :3:<path> (theirs), :1:<path> (base).
  • WriteConflictResolutionAsync writes resolved content to the file in WorkingDir and git adds it.
  • ContinueMergeAsync wraps the existing TaskMergeService.ContinueMergeAsync (git add -A → re-check git diff --name-only --diff-filter=Ugit commit).
  • AbortMergeAsync wraps the existing TaskMergeService.AbortMergeAsync (git merge --abort).

New DTOs (defined in the worker hub DTO file, mirrored client-side):

public record MergeConflictsDto(string TaskId, IReadOnlyList<ConflictFileDto> Files);
public record ConflictFileDto(string Path, IReadOnlyList<ConflictHunkDto> Hunks);
public record ConflictHunkDto(string Ours, string Theirs, string? Base);

Existing DTOs reused unchanged: MergeResultDto(Status, ConflictFiles, ErrorMessage), MergePreviewDto, MergeTargetsDto.

3. Conflict data model (UI)

ConflictFile { Path, Hunks[] }, ConflictHunk { Ours, Theirs, Base, Resolution }. Shaped so a future 3-way merge pane needs no model change (Layer C is the inline resolver now; the model leaves room for 3-way later).

4. Integration seams (delegates, wired by the integrator at merge)

A's and B's cockpits hold a RequestConflictResolution(string taskId) callback (an Action<string> or Func<string, Task>). They never reference Layer C's resolver types. The integrator connects these callbacks to C's ConflictResolverViewModel factory when merging the three branches together.

Parallel boundaries (verified disjoint)

Area A (this session) B (parallel) C (parallel)
DiffView + diff model/parser builds reuses reuses
WorkConsole.axaml / DetailsIslandViewModel owns
DiffModalView + PlanningDiffView migrates to DiffView
WorktreesOverviewModalView/VM + WorktreeModalView owns
WorkerHub / TaskMergeService / GitService owns
New ConflictResolverView/VM + conflict UI model owns
IWorkerClient / WorkerClient adds frozen stubs + DTOs reuses MergeTaskAsync fills stub bodies
Test fakes (IWorkerClient) in both test projects adds new no-op methods makes them functional if needed

The only file C and A both touch is WorkerClient.cs (C replaces the stub bodies A wrote). Contained; reconciled at integration. Everything else is disjoint.

Layer A — review/merge cockpit (this session)

  • The Git tab becomes the single Approve + merge surface. Approve and the merge target / preview / diff flow together as one block (no separate REVIEW vs MERGE & WORKTREE sections).
  • Continue (reject → requeue with feedback) and Reset (reject → idle) stay in the Output tab footer — unchanged.
  • The diff is shown via the unified DiffView opened as a modal from the cockpit. No inline diff recap in the tab (the island is too small).
  • On a single-task Approve that conflicts: instead of today's auto-abort, call StartConflictMergeAsync and fire RequestConflictResolution(taskId). This leaves the main checkout mid-merge until the user resolves or aborts (behavior change, intended). The callback is inert until Layer C is merged; the integrator wires it.
  • Migrate DiffModalView and PlanningDiffView onto the new DiffView.

Behavior change accepted

Today MergeTask/ApproveReview use leaveConflictsInTree: false (auto-abort on conflict). Under this design, an Approve that conflicts leaves the merge in progress and opens the resolver. The mid-merge guard (IsMidMergeAsync) still prevents a second concurrent merge.

Layer B — multi-worktree merge cockpit (parallel)

  • Rework WorktreesOverviewModalView/WorktreesOverviewModalViewModel into a batch-merge cockpit: list mergeable worktrees, select N, choose one target branch (single target — 99% of the time everything goes to the same branch), "Merge all".
  • Skip-and-continue: client-side loop calling the existing MergeTaskAsync(taskId, target, removeWorktree, msg) per selected task. Clean merges apply; conflicting ones are collected (existing MergeTaskAsync auto-aborts on conflict, leaving the tree clean) into a "needs resolution" list with live progress.
  • Each conflict row exposes a Resolve action → RequestConflictResolution(taskId) (wired to Layer C at integration).
  • Per-task diff via the shared DiffView; migrate WorktreeModalView's inline diff onto it.
  • B touches no worker files — keeps it parallel-safe.

Layer C — inline conflict resolver (parallel)

Worker side

Implement the five frozen contract methods:

  • Add hub methods StartConflictMerge, GetMergeConflicts, WriteConflictResolution, ContinueMerge, AbortMerge in WorkerHub.
  • StartConflictMerge calls the existing TaskMergeService.MergeAsync overload with leaveConflictsInTree: true.
  • ContinueMerge / AbortMerge wrap the existing TaskMergeService.ContinueMergeAsync / AbortMergeAsync (currently service-level only, not hub-exposed).
  • GetMergeConflicts reads ours/theirs/base per conflicted file via git show :2:/:3:/:1:; add the GitService helpers needed.
  • WriteConflictResolution writes the resolved content to WorkingDir and stages it.
  • Fill the WorkerClient stub bodies (real SignalR InvokeAsync calls).
  • Update the hand-rolled IWorkerClient fakes in both test projects.

UI

  • New ConflictResolverView + ConflictResolverViewModel. Per conflict hunk, show ours vs theirs stacked, with buttons Accept Current / Accept Incoming / Accept Both / Edit manually plus a free-text box for the merged result of that hunk.
  • When every file's hunks are resolved → ContinueMergeAsync(taskId)MergeResultDto (merged closes the resolver; conflict means not fully resolved, stay open).
  • AbortMergeAsync(taskId) cancels and aborts the merge.
  • Expose a factory (Func<string, ConflictResolverViewModel>) the integrator wires to A's and B's RequestConflictResolution callbacks.

Build / test

.slnx needs .NET 9; on .NET 8 build individual csproj with -c Release (a running Worker locks Debug). Run the relevant test projects. No tests that spawn the real claude CLI. Keep en.json/de.json localization keys in parity.

Out of scope

  • Full 3-way synchronized merge editor (model leaves room; not built now).
  • Per-task differing merge targets in the batch (single target only).
  • Any CI/PR tooling (direct push-to-main workflow).