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

21 KiB
Raw Blame History

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.csRequestConflictResolution 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:

    // ── 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:

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:

    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:

    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:

    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
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:

[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:

    // 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:

            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:

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

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