diff --git a/docs/superpowers/specs/2026-06-05-git-merge-review-rework-design.md b/docs/superpowers/specs/2026-06-05-git-merge-review-rework-design.md new file mode 100644 index 0000000..da6fa1f --- /dev/null +++ b/docs/superpowers/specs/2026-06-05-git-merge-review-rework-design.md @@ -0,0 +1,197 @@ +# 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 StartConflictMergeAsync(string taskId, string targetBranch); +Task GetMergeConflictsAsync(string taskId); +Task WriteConflictResolutionAsync(string taskId, string path, string resolvedContent); +Task 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:` (ours), `:3:` (theirs), `:1:` (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 Files); +public record ConflictFileDto(string Path, IReadOnlyList 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` or `Func`). 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`) 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).