ClaudeDo/docs/superpowers/specs/2026-06-05-git-merge-review-rework-design.md

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

```csharp
// 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 add`s it.
- `ContinueMergeAsync` wraps the existing `TaskMergeService.ContinueMergeAsync`
  (`git add -A` → re-check `git diff --name-only --diff-filter=U` → `git commit`).
- `AbortMergeAsync` wraps the existing `TaskMergeService.AbortMergeAsync`
  (`git merge --abort`).

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

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