Files
ClaudeDo/docs/superpowers/plans/2026-06-05-git-merge-review-foundation-layerA.md
2026-06-05 10:15:52 +02:00

433 lines
21 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Git Merge/Review — Shared Foundation + Layer A Implementation Plan
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
**Goal:** Build the shared worker conflict contract (so parallel Layer B/C sessions branch from frozen interfaces) and rework the Git tab into a single Approve+merge cockpit.
**Architecture:** Phase 0 adds the conflict-resolution contract to `IWorkerClient`/`WorkerClient` (real `_hub.InvokeAsync` bodies — the worker hub methods are implemented later by Layer C; calls simply fail at runtime until then) plus client-side DTOs and test-fake updates, then commits + pushes so B and C branch from it. Phase A reworks `WorkConsole.axaml`'s Git tab and routes single-task merge/approve conflicts into a `RequestConflictResolution` seam (wired to Layer C's resolver by the integrator at merge time).
**Tech Stack:** .NET 8, Avalonia 12 (Fluent), CommunityToolkit.Mvvm, SignalR, xUnit. Build individual csproj with `-c Release` (`.slnx` needs .NET 9; a running Worker locks `Debug`).
**Reference spec:** `docs/superpowers/specs/2026-06-05-git-merge-review-rework-design.md`
**Note on the canonical diff renderer:** the unified diff model/control already exists — `DiffFileViewModel`/`DiffLineViewModel`/`UnifiedDiffParser` (in `src/ClaudeDo.Ui/ViewModels/Modals/`) rendered by `DiffLinesView` (`src/ClaudeDo.Ui/Views/Controls/DiffLinesView.axaml`). `DiffModalView` and `PlanningDiffView` already use it. So "consolidate diff renderers" for this scope is just verifying that (Task A.3); migrating `WorktreeModalView`'s bespoke diff onto `DiffLinesView` is Layer B's job.
---
## File Structure
**Phase 0 (foundation — pushed before B/C branch):**
- Modify `src/ClaudeDo.Ui/Services/Interfaces/IWorkerClient.cs` — 5 new method signatures.
- Modify `src/ClaudeDo.Ui/Services/WorkerClient.cs` — 5 `InvokeAsync` bodies + 3 new DTO records.
- Modify `tests/ClaudeDo.Ui.Tests/StubWorkerClient.cs` — 5 new `virtual` no-op methods.
- Modify `tests/ClaudeDo.Worker.Tests/UiVm/TasksIslandViewModelPlanningTests.cs` — 5 new methods on `FakeWorkerClient`.
**Phase A (Layer A — this session, after foundation commit):**
- Modify `src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs``RequestConflictResolution` seam; route Approve/Merge conflicts into it.
- Modify `src/ClaudeDo.Ui/Views/Islands/Detail/WorkConsole.axaml` — fuse REVIEW + MERGE sections into one cockpit block.
- Test: `tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandPlanningTests.cs` (or a sibling test file in the same folder).
---
## Phase 0 — Shared Foundation
### Task 0.1: Add the conflict contract (interface + client + DTOs)
**Files:**
- Modify: `src/ClaudeDo.Ui/Services/Interfaces/IWorkerClient.cs`
- Modify: `src/ClaudeDo.Ui/Services/WorkerClient.cs`
- [ ] **Step 1: Add the 5 method signatures to `IWorkerClient`**
In `src/ClaudeDo.Ui/Services/Interfaces/IWorkerClient.cs`, after the existing
`Task CancelReviewAsync(string taskId);` line (line 45), add:
```csharp
// ── Conflict resolution (worker hub side implemented by Layer C) ──
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);
```
- [ ] **Step 2: Add the 3 DTO records to `WorkerClient.cs`**
In `src/ClaudeDo.Ui/Services/WorkerClient.cs`, immediately after line 534
(`public record MergeTargetsDto(...)`), add:
```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);
```
- [ ] **Step 3: Add the 5 client method bodies to `WorkerClient.cs`**
In `src/ClaudeDo.Ui/Services/WorkerClient.cs`, right after the `MergeTaskAsync`
method (ends at line 270), add:
```csharp
public Task<MergeResultDto> StartConflictMergeAsync(string taskId, string targetBranch)
=> _hub.InvokeAsync<MergeResultDto>("StartConflictMerge", taskId, targetBranch);
public Task<MergeConflictsDto> GetMergeConflictsAsync(string taskId)
=> _hub.InvokeAsync<MergeConflictsDto>("GetMergeConflicts", taskId);
public Task WriteConflictResolutionAsync(string taskId, string path, string resolvedContent)
=> _hub.InvokeAsync("WriteConflictResolution", taskId, path, resolvedContent);
public Task<MergeResultDto> ContinueMergeAsync(string taskId)
=> _hub.InvokeAsync<MergeResultDto>("ContinueMerge", taskId);
public Task AbortMergeAsync(string taskId)
=> _hub.InvokeAsync("AbortMerge", taskId);
```
- [ ] **Step 4: Build the UI project**
Run: `dotnet build src/ClaudeDo.Ui/ClaudeDo.Ui.csproj -c Release`
Expected: build FAILS — the two test projects won't compile yet, but the UI project
itself should succeed. If the UI project reports "does not implement interface member"
it means a body is missing; fix before continuing. (Test projects are fixed in 0.2.)
### Task 0.2: Update the hand-rolled test fakes
**Files:**
- Modify: `tests/ClaudeDo.Ui.Tests/StubWorkerClient.cs`
- Modify: `tests/ClaudeDo.Worker.Tests/UiVm/TasksIslandViewModelPlanningTests.cs`
- [ ] **Step 1: Add 5 virtual no-ops to `StubWorkerClient`**
In `tests/ClaudeDo.Ui.Tests/StubWorkerClient.cs`, after the `MergeTaskAsync` override
(line 57), add:
```csharp
public virtual Task<MergeResultDto> StartConflictMergeAsync(string taskId, string targetBranch) => Task.FromResult(new MergeResultDto("conflict", System.Array.Empty<string>(), null));
public virtual Task<MergeConflictsDto> GetMergeConflictsAsync(string taskId) => Task.FromResult(new MergeConflictsDto(taskId, System.Array.Empty<ConflictFileDto>()));
public virtual Task WriteConflictResolutionAsync(string taskId, string path, string resolvedContent) => Task.CompletedTask;
public virtual Task<MergeResultDto> ContinueMergeAsync(string taskId) => Task.FromResult(new MergeResultDto("merged", System.Array.Empty<string>(), null));
public virtual Task AbortMergeAsync(string taskId) => Task.CompletedTask;
```
- [ ] **Step 2: Add 5 methods to `FakeWorkerClient`**
In `tests/ClaudeDo.Worker.Tests/UiVm/TasksIslandViewModelPlanningTests.cs`, after the
`MergeTaskAsync` method (line 47), add:
```csharp
public Task<MergeResultDto> StartConflictMergeAsync(string taskId, string targetBranch) => Task.FromResult(new MergeResultDto("conflict", System.Array.Empty<string>(), null));
public Task<MergeConflictsDto> GetMergeConflictsAsync(string taskId) => Task.FromResult(new MergeConflictsDto(taskId, System.Array.Empty<ConflictFileDto>()));
public Task WriteConflictResolutionAsync(string taskId, string path, string resolvedContent) => Task.CompletedTask;
public Task<MergeResultDto> ContinueMergeAsync(string taskId) => Task.FromResult(new MergeResultDto("merged", System.Array.Empty<string>(), null));
public Task AbortMergeAsync(string taskId) => Task.CompletedTask;
```
- [ ] **Step 3: Build both test projects**
Run: `dotnet build tests/ClaudeDo.Ui.Tests/ClaudeDo.Ui.Tests.csproj -c Release && dotnet build tests/ClaudeDo.Worker.Tests/ClaudeDo.Worker.Tests.csproj -c Release`
Expected: both BUILD succeed.
- [ ] **Step 4: Run the UI test suite to confirm green baseline**
Run: `dotnet test tests/ClaudeDo.Ui.Tests/ClaudeDo.Ui.Tests.csproj -c Release`
Expected: PASS (no behavior changed yet).
### Task 0.3: Commit and push the foundation
- [ ] **Step 1: Commit**
```bash
git add src/ClaudeDo.Ui/Services/Interfaces/IWorkerClient.cs src/ClaudeDo.Ui/Services/WorkerClient.cs tests/ClaudeDo.Ui.Tests/StubWorkerClient.cs tests/ClaudeDo.Worker.Tests/UiVm/TasksIslandViewModelPlanningTests.cs
git commit -m "feat(ui): add conflict-resolution worker contract (foundation for merge rework)"
```
- [ ] **Step 2: Push so Layer B/C can branch from this commit**
Run: `git push`
Expected: pushed to `main`. (First push to git.kuns.dev may fail auth — retry once.)
**This commit is the branch point for the Layer B and Layer C kickoff prompts.**
---
## Phase A — Layer A Review/Merge Cockpit
### Task A.1: Conflict-resolution seam + route Approve/Merge conflicts into it (TDD)
**Files:**
- Modify: `src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs`
- Test: `tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandConflictSeamTests.cs` (new)
- [ ] **Step 1: Write the failing test**
Create `tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandConflictSeamTests.cs`. Mirror
the VM-construction harness used in
`tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandPlanningTests.cs` (same folder) —
construct `DetailsIslandViewModel` exactly as that file does, including its
`StubWorkerClient` subclass pattern. The test:
```csharp
[Fact]
public async Task ApproveReview_OnConflict_InvokesConflictResolutionSeam()
{
string? resolvedTaskId = null;
string? resolvedTarget = null;
// Construct the VM as in DetailsIslandPlanningTests, with a worker stub whose
// ApproveReviewAsync returns a conflict result:
// public override Task<MergeResultDto?> ApproveReviewAsync(string id, string target)
// => Task.FromResult<MergeResultDto?>(new MergeResultDto("conflict", new[]{"a.cs"}, null));
var vm = CreateVm(/* worker stub above */);
vm.RequestConflictResolution = (taskId, target) =>
{
resolvedTaskId = taskId; resolvedTarget = target;
return System.Threading.Tasks.Task.CompletedTask;
};
// assign a task in WaitingForReview + a SelectedMergeTarget = "main" via the same
// helpers DetailsIslandPlanningTests uses.
await vm.ApproveReviewCommand.ExecuteAsync(null);
Assert.Equal(/* the seeded task id */, resolvedTaskId);
Assert.Equal("main", resolvedTarget);
}
```
- [ ] **Step 2: Run the test to verify it fails**
Run: `dotnet test tests/ClaudeDo.Ui.Tests/ClaudeDo.Ui.Tests.csproj -c Release --filter ApproveReview_OnConflict_InvokesConflictResolutionSeam`
Expected: FAIL — `RequestConflictResolution` property does not exist (compile error).
- [ ] **Step 3: Add the seam property**
In `src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs`, beside the other
view-wired delegates (`ShowDiffModal`, `ShowMergeModal` around line 387390), add:
```csharp
// Invoked when a single-task merge/approve hits a conflict. Wired by the
// integrator to Layer C's conflict resolver. Args: (taskId, targetBranch).
public Func<string, string, System.Threading.Tasks.Task>? RequestConflictResolution { get; set; }
```
- [ ] **Step 4: Route the Approve conflict branch into the seam**
In `ApproveReviewAsync` (around line 1453), replace the conflict branch body so it
prefers the seam, falling back to the current preview-text behavior:
```csharp
var result = await _worker.ApproveReviewAsync(Task.Id, SelectedMergeTarget ?? "");
if (result?.Status == "conflict")
{
if (RequestConflictResolution is not null)
{
await RequestConflictResolution(Task.Id, SelectedMergeTarget ?? "");
}
else
{
var (text, _, _) = MergePreviewPresenter.Describe(
new MergePreviewDto("conflict", result.ConflictFiles, 0));
MergePreviewText = text; MergeIsClean = false; MergeIsConflict = true;
}
}
```
- [ ] **Step 5: Route the manual Merge conflict branch into the seam**
In `MergeAsync` (around line 1170), apply the same pattern to its conflict branch:
```csharp
var result = await _worker.MergeTaskAsync(Task.Id, SelectedMergeTarget ?? "", false, "Merge task");
if (result.Status == "conflict")
{
if (RequestConflictResolution is not null)
{
await RequestConflictResolution(Task.Id, SelectedMergeTarget ?? "");
}
else
{
var (text, _, _) = MergePreviewPresenter.Describe(
new MergePreviewDto("conflict", result.ConflictFiles, 0));
MergePreviewText = text; MergeIsClean = false; MergeIsConflict = true;
}
}
else
{
await RefreshMergePreviewAsync();
}
```
- [ ] **Step 6: Run the test to verify it passes**
Run: `dotnet test tests/ClaudeDo.Ui.Tests/ClaudeDo.Ui.Tests.csproj -c Release --filter ApproveReview_OnConflict_InvokesConflictResolutionSeam`
Expected: PASS.
- [ ] **Step 7: Run the full UI suite (no regressions)**
Run: `dotnet test tests/ClaudeDo.Ui.Tests/ClaudeDo.Ui.Tests.csproj -c Release`
Expected: PASS.
- [ ] **Step 8: Commit**
```bash
git add src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandConflictSeamTests.cs
git commit -m "feat(ui): route single-task merge conflicts into a resolution seam"
```
### Task A.2: Fuse the Git tab into one Approve+merge cockpit
**Files:**
- Modify: `src/ClaudeDo.Ui/Views/Islands/Detail/WorkConsole.axaml`
- [ ] **Step 1: Replace the two Git-tab sections with one cockpit block**
In `src/ClaudeDo.Ui/Views/Islands/Detail/WorkConsole.axaml`, replace the entire Git
`ScrollViewer` body (lines 255313 — the `<!-- Git: ... -->` block containing the
separate `REVIEW` `StackPanel` and the `MERGE & WORKTREE` `StackPanel`) with a single
cockpit where Approve sits with the merge target/preview/actions. Keep the existing
control class names (`section-label`, `field-label`, `btn`, `btn accent`, `meta`) and
the existing bindings (`SelectedMergeTarget`, `MergeTargetBranches`, `MergePreviewText`,
`MergeIsClean`, `MergeIsConflict`, `ShowMergePreviewMuted`, `OpenDiffCommand`,
`ApproveReviewCommand`, `MergeCommand`, `ShowSingleMerge`, `OpenWorktreeCommand`,
`ReviewCombinedDiffCommand`, `MergeAllCommand`, `CanMergeAll`, `MergeAllDisabledReason`,
`MergeAllError`):
```xml
<!-- Git: one Approve + merge cockpit -->
<ScrollViewer IsVisible="{Binding IsGitTab}" Padding="14,10">
<StackPanel Spacing="12" IsVisible="{Binding ShowMergeSection}">
<TextBlock Classes="section-label" Text="MERGE" />
<StackPanel Spacing="4">
<TextBlock Classes="field-label" Text="Target branch" />
<ComboBox ItemsSource="{Binding MergeTargetBranches}"
SelectedItem="{Binding SelectedMergeTarget, Mode=TwoWay}"
HorizontalAlignment="Stretch" />
</StackPanel>
<StackPanel Spacing="0">
<TextBlock Classes="meta" Text="{Binding MergePreviewText}" TextWrapping="Wrap"
Foreground="{DynamicResource MossBrush}"
IsVisible="{Binding MergeIsClean}" />
<TextBlock Classes="meta" Text="{Binding MergePreviewText}" TextWrapping="Wrap"
Foreground="{DynamicResource BloodBrush}"
IsVisible="{Binding MergeIsConflict}" />
<TextBlock Classes="meta" Text="{Binding MergePreviewText}" TextWrapping="Wrap"
Foreground="{DynamicResource TextMuteBrush}"
IsVisible="{Binding ShowMergePreviewMuted}" />
</StackPanel>
<!-- Primary action: Approve flows straight into the merge.
Approve is the review-gated path; the plain Merge button covers
already-reviewed / kept worktrees. -->
<WrapPanel Orientation="Horizontal">
<Button Classes="btn accent" Content="Approve &amp; Merge" Margin="0,0,8,8"
Command="{Binding ApproveReviewCommand}"
IsVisible="{Binding IsWaitingForReview}" />
<Button Classes="btn accent" Content="Merge" Margin="0,0,8,8"
Command="{Binding MergeCommand}"
IsVisible="{Binding ShowSingleMerge}" />
<Button Classes="btn" Content="Open Diff" Margin="0,0,8,8"
Command="{Binding OpenDiffCommand}" />
<Button Classes="btn" Margin="0,0,8,8"
Command="{Binding OpenWorktreeCommand}">
<StackPanel Orientation="Horizontal" Spacing="5">
<TextBlock Text="Worktree" />
<PathIcon Data="{StaticResource Icon.ArrowOut}" Width="11" Height="11" />
</StackPanel>
</Button>
<Button Classes="btn" Content="Review Combined Diff" Margin="0,0,8,8"
Command="{Binding ReviewCombinedDiffCommand}" />
<Button Classes="btn accent" Content="Merge All Subtasks" Margin="0,0,0,8"
Command="{Binding MergeAllCommand}"
IsEnabled="{Binding CanMergeAll}"
ToolTip.Tip="{Binding MergeAllDisabledReason}" />
</WrapPanel>
<TextBlock Text="{Binding MergeAllError}"
Foreground="{DynamicResource BloodBrush}"
TextWrapping="Wrap"
IsVisible="{Binding MergeAllError,
Converter={x:Static ObjectConverters.IsNotNull}}" />
</StackPanel>
</ScrollViewer>
```
Note: the cockpit now shows whenever `ShowMergeSection` is true. `ShowMergeSection`
(DetailsIslandViewModel line 161) must be true while `IsWaitingForReview` so the
Approve button appears. Check its expression in Step 2.
- [ ] **Step 2: Verify `ShowMergeSection` covers the review state**
Read `src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs` line 161. If
`ShowMergeSection` is false while `IsWaitingForReview` (e.g. it requires a non-review
state), widen it to also be true when `IsWaitingForReview && WorktreePath != null`, and
ensure `OnPropertyChanged(nameof(ShowMergeSection))` already fires on the relevant state
transitions (it is notified via `NotifySessionSections`). Make the minimal change needed
so the Approve button is visible in review state. If it already covers review, change
nothing.
- [ ] **Step 3: Build the app project**
Run: `dotnet build src/ClaudeDo.App/ClaudeDo.App.csproj -c Release`
Expected: BUILD succeeds (pulls in Ui + Data).
- [ ] **Step 4: Visual verification (manual — flag for the user)**
This is an AXAML layout change with no automated coverage. Launch the app, open a task
in `WaitingForReview`, open the Git tab, and confirm: the single MERGE block shows the
target combo, the colored preview line, an "Approve & Merge" button (review state), and
the diff/worktree/combined/merge-all actions. **Explicitly tell the user this needs a
visual pass — do not claim it works without running it.**
- [ ] **Step 5: Commit**
```bash
git add src/ClaudeDo.Ui/Views/Islands/Detail/WorkConsole.axaml src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs
git commit -m "feat(ui): fuse git tab into one approve+merge cockpit"
```
### Task A.3: Verify diff-renderer consolidation
**Files:** none modified (verification only).
- [ ] **Step 1: Confirm DiffModal + Planning already use the canonical renderer**
Run: `rg -l "DiffLinesView" src/ClaudeDo.Ui/Views`
Expected: matches in `Modals/DiffModalView.axaml` and `Planning/PlanningDiffView.axaml`.
If `PlanningDiffView.axaml` does NOT use `DiffLinesView`, change its diff `ItemsControl`
to a `<controls:DiffLinesView Lines="{Binding SelectedFile.Lines}" />` (matching
`DiffModalView.axaml`'s usage) and rebuild the App project. If both already use it, this
task is a no-op — record that and move on. (`WorktreeModalView`'s bespoke diff is
intentionally left for Layer B.)
---
## Self-Review
- **Spec coverage:** Foundation contract (spec §"Frozen worker conflict contract") →
Task 0.1. Test fakes (spec parallel-boundaries row) → Task 0.2. Branch point (spec
§"built & pushed this session") → Task 0.3. Layer A cockpit + Approve/merge flow
together (spec §"Layer A") → Task A.2. Single-task approve-on-conflict opens resolver
via seam (spec §"Layer A" + §"integration seams") → Task A.1. Diff consolidation
(spec §"One diff model") → Task A.3. Output-footer feedback unchanged → not touched
(correct). No spec requirement left unmapped for this session's scope.
- **Placeholder scan:** none — every code step has concrete code; the only "mirror the
existing harness" reference (Task A.1 Step 1) points at a real file with a working
pattern, not a TODO.
- **Type consistency:** `MergeConflictsDto`/`ConflictFileDto`/`ConflictHunkDto` and the
5 method names match between `IWorkerClient` (0.1 Step 1), `WorkerClient` (0.1 Steps
23), and both fakes (0.2). The seam `RequestConflictResolution` is
`Func<string,string,Task>?` everywhere (A.1 Steps 1, 35). DTO field names match the
spec.
---
## Integration notes (for the integrator merging A + B + C)
- Wire `DetailsIslandViewModel.RequestConflictResolution` and Layer B's equivalent
callback to Layer C's `ConflictResolverViewModel` factory + `ShowConflictResolver`
dialog delegate.
- Layer C implements the worker hub methods `StartConflictMerge`, `GetMergeConflicts`,
`WriteConflictResolution`, `ContinueMerge`, `AbortMerge`; the client side from Task
0.1 already calls them by name.