docs(merge): add approve-merge + conflict-preview implementation plan
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
mika kuns
@@ -0,0 +1,994 @@
|
|||||||
|
# Approve = Merge → Done + Conflict Preview — 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:** Approving a `WaitingForReview` task merges its worktree into the target branch first and only marks the task `Done` on a clean merge; conflicts keep it in review and are surfaced. Add a non-destructive "merges cleanly / conflicts" indicator and a direct single-task Merge button.
|
||||||
|
|
||||||
|
**Architecture:** A new `GitService.PreviewMergeAsync` probes mergeability via `git merge-tree --write-tree` (no working-tree mutation). `TaskMergeService` gains `PreviewAsync` and `ApproveAndMergeAsync` (merge first, then delegate the `Done` flip to `ITaskStateService`). `WorkerHub` exposes `PreviewMerge` and a result-returning `ApproveReview(taskId, targetBranch)`. The UI loads merge targets whenever a worktree exists, shows the preview, and reacts to conflict results.
|
||||||
|
|
||||||
|
**Tech Stack:** .NET 8, Avalonia, EF Core/SQLite, SignalR, xUnit with real git (`GitRepoFixture`) and real SQLite (`DbFixture`).
|
||||||
|
|
||||||
|
**Conventions for the implementer:**
|
||||||
|
- Use the **sonnet** model.
|
||||||
|
- **Stage files explicitly by path** — never `git add -A` (parallel sessions leave unrelated WIP).
|
||||||
|
- Build with `-c Release` (a running Worker locks `Debug` output).
|
||||||
|
- Conventional Commit messages: `type(scope): description`.
|
||||||
|
- New UI strings use **plain English literals** to match the surrounding merge controls (no `loc:Tr`) — this avoids Localization.Tests parity churn.
|
||||||
|
- Ignore anything under `.claude/worktrees/` — those are stale worktrees, not the build tree.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## File map
|
||||||
|
|
||||||
|
| File | Change |
|
||||||
|
|------|--------|
|
||||||
|
| `src/ClaudeDo.Data/Git/GitService.cs` | Add `MergePreview` record + `PreviewMergeAsync` + `CountChangedFilesAsync` |
|
||||||
|
| `src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs` | Inject `ITaskStateService`; add `MergePreviewResult` + `PreviewAsync` + `ApproveAndMergeAsync` |
|
||||||
|
| `src/ClaudeDo.Worker/Hub/WorkerHub.cs` | Add `MergePreviewDto` + `PreviewMerge`; change `ApproveReview` to `(taskId, targetBranch) → MergeResultDto` |
|
||||||
|
| `src/ClaudeDo.Ui/Services/Interfaces/IWorkerClient.cs` | Change `ApproveReviewAsync`; add `PreviewMergeAsync`, `MergeTaskAsync` |
|
||||||
|
| `src/ClaudeDo.Ui/Services/WorkerClient.cs` | Implement the above; add UI `MergePreviewDto` record |
|
||||||
|
| `src/ClaudeDo.Ui/ViewModels/Islands/MergePreviewPresenter.cs` | New pure presenter (text + color flags) |
|
||||||
|
| `src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs` | Load targets for worktree tasks; preview props; approve conflict handling; `MergeCommand` |
|
||||||
|
| `src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs` | Update the list-level approve call to new signature |
|
||||||
|
| `src/ClaudeDo.Ui/Views/Islands/Detail/WorkConsole.axaml` | Mergeability status line + Merge button |
|
||||||
|
| `tests/ClaudeDo.Worker.Tests/Runner/GitServicePreviewMergeTests.cs` | New — git-backed preview tests |
|
||||||
|
| `tests/ClaudeDo.Worker.Tests/Services/TaskMergeServiceTests.cs` | Update `BuildService`; add preview + approve-merge tests |
|
||||||
|
| `tests/ClaudeDo.Worker.Tests/UiVm/TasksIslandViewModelPlanningTests.cs` | Update `FakeWorkerClient` |
|
||||||
|
| `tests/ClaudeDo.Ui.Tests/StubWorkerClient.cs` | Update fake |
|
||||||
|
| `tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandPlanningTests.cs` | Update the `ApproveReviewAsync` override |
|
||||||
|
| `tests/ClaudeDo.Ui.Tests/ViewModels/MergePreviewPresenterTests.cs` | New — presenter unit tests |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 1: GitService non-destructive merge probe
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Modify: `src/ClaudeDo.Data/Git/GitService.cs`
|
||||||
|
- Test: `tests/ClaudeDo.Worker.Tests/Runner/GitServicePreviewMergeTests.cs` (create)
|
||||||
|
|
||||||
|
Behaviour verified on git 2.50: `git merge-tree --write-tree --name-only <target> <source>` exits `0` when clean (stdout = a single tree-OID line) and `1` on conflict (stdout = tree-OID line, then conflicted file names, then a blank line, then informational messages). It writes only loose objects — the working tree, index, and refs are untouched.
|
||||||
|
|
||||||
|
- [ ] **Step 1: Write the failing tests**
|
||||||
|
|
||||||
|
Create `tests/ClaudeDo.Worker.Tests/Runner/GitServicePreviewMergeTests.cs`:
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
using ClaudeDo.Data.Git;
|
||||||
|
using ClaudeDo.Worker.Tests.Infrastructure;
|
||||||
|
|
||||||
|
namespace ClaudeDo.Worker.Tests.Runner;
|
||||||
|
|
||||||
|
public class GitServicePreviewMergeTests : IDisposable
|
||||||
|
{
|
||||||
|
private readonly List<GitRepoFixture> _repos = new();
|
||||||
|
private GitRepoFixture NewRepo() { var r = new GitRepoFixture(); _repos.Add(r); return r; }
|
||||||
|
public void Dispose() { foreach (var r in _repos) try { r.Dispose(); } catch { } }
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task PreviewMergeAsync_NonConflicting_ReportsCleanWithChangedCount()
|
||||||
|
{
|
||||||
|
if (!GitRepoFixture.IsGitAvailable()) return;
|
||||||
|
var repo = NewRepo();
|
||||||
|
var git = new GitService();
|
||||||
|
var baseBranch = await git.GetCurrentBranchAsync(repo.RepoDir);
|
||||||
|
|
||||||
|
GitRepoFixture.RunGit(repo.RepoDir, "checkout", "-b", "feature");
|
||||||
|
File.WriteAllText(Path.Combine(repo.RepoDir, "newfile.txt"), "x\n");
|
||||||
|
GitRepoFixture.RunGit(repo.RepoDir, "add", "-A");
|
||||||
|
GitRepoFixture.RunGit(repo.RepoDir, "commit", "-m", "feat");
|
||||||
|
GitRepoFixture.RunGit(repo.RepoDir, "checkout", baseBranch);
|
||||||
|
|
||||||
|
var preview = await git.PreviewMergeAsync(repo.RepoDir, baseBranch, "feature", CancellationToken.None);
|
||||||
|
|
||||||
|
Assert.True(preview.Supported);
|
||||||
|
Assert.True(preview.Clean);
|
||||||
|
Assert.Empty(preview.ConflictFiles);
|
||||||
|
|
||||||
|
var count = await git.CountChangedFilesAsync(repo.RepoDir, baseBranch, "feature", CancellationToken.None);
|
||||||
|
Assert.Equal(1, count);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task PreviewMergeAsync_Conflicting_ReportsFilesAndDoesNotMutateTree()
|
||||||
|
{
|
||||||
|
if (!GitRepoFixture.IsGitAvailable()) return;
|
||||||
|
var repo = NewRepo();
|
||||||
|
var git = new GitService();
|
||||||
|
var baseBranch = await git.GetCurrentBranchAsync(repo.RepoDir);
|
||||||
|
|
||||||
|
GitRepoFixture.RunGit(repo.RepoDir, "checkout", "-b", "feature");
|
||||||
|
File.WriteAllText(Path.Combine(repo.RepoDir, "README.md"), "# from feature\n");
|
||||||
|
GitRepoFixture.RunGit(repo.RepoDir, "add", "-A");
|
||||||
|
GitRepoFixture.RunGit(repo.RepoDir, "commit", "-m", "feat readme");
|
||||||
|
GitRepoFixture.RunGit(repo.RepoDir, "checkout", baseBranch);
|
||||||
|
File.WriteAllText(Path.Combine(repo.RepoDir, "README.md"), "# from base\n");
|
||||||
|
GitRepoFixture.RunGit(repo.RepoDir, "add", "-A");
|
||||||
|
GitRepoFixture.RunGit(repo.RepoDir, "commit", "-m", "base readme");
|
||||||
|
|
||||||
|
var headBefore = GitRepoFixture.RunGit(repo.RepoDir, "rev-parse", "HEAD").Trim();
|
||||||
|
|
||||||
|
var preview = await git.PreviewMergeAsync(repo.RepoDir, baseBranch, "feature", CancellationToken.None);
|
||||||
|
|
||||||
|
Assert.True(preview.Supported);
|
||||||
|
Assert.False(preview.Clean);
|
||||||
|
Assert.Contains("README.md", preview.ConflictFiles);
|
||||||
|
|
||||||
|
// Non-destructive: HEAD unchanged, no mid-merge state.
|
||||||
|
Assert.Equal(headBefore, GitRepoFixture.RunGit(repo.RepoDir, "rev-parse", "HEAD").Trim());
|
||||||
|
Assert.False(await git.IsMidMergeAsync(repo.RepoDir));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Run the tests, verify they fail to compile**
|
||||||
|
|
||||||
|
Run: `dotnet test tests/ClaudeDo.Worker.Tests/ClaudeDo.Worker.Tests.csproj -c Release --filter FullyQualifiedName~GitServicePreviewMergeTests`
|
||||||
|
Expected: build error — `PreviewMergeAsync`/`CountChangedFilesAsync` do not exist.
|
||||||
|
|
||||||
|
- [ ] **Step 3: Implement the probe**
|
||||||
|
|
||||||
|
In `src/ClaudeDo.Data/Git/GitService.cs`, add this record just under `namespace ClaudeDo.Data.Git;`:
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
public sealed record MergePreview(bool Supported, bool Clean, IReadOnlyList<string> ConflictFiles);
|
||||||
|
```
|
||||||
|
|
||||||
|
Add these methods inside the `GitService` class (e.g. after `ListConflictedFilesAsync`):
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
/// <summary>
|
||||||
|
/// Non-destructive mergeability probe via `git merge-tree --write-tree`. Writes only
|
||||||
|
/// loose objects — the working tree, index, and refs are left untouched.
|
||||||
|
/// </summary>
|
||||||
|
public async Task<MergePreview> PreviewMergeAsync(
|
||||||
|
string repoDir, string targetBranch, string sourceBranch, CancellationToken ct = default)
|
||||||
|
{
|
||||||
|
var (exitCode, stdout, _) = await RunGitAsync(repoDir,
|
||||||
|
["merge-tree", "--write-tree", "--name-only", targetBranch, sourceBranch], ct);
|
||||||
|
|
||||||
|
if (exitCode == 0)
|
||||||
|
return new MergePreview(true, true, Array.Empty<string>());
|
||||||
|
|
||||||
|
if (exitCode == 1)
|
||||||
|
{
|
||||||
|
// stdout: <tree-oid>\n<file>\n...\n\n<informational messages>
|
||||||
|
var lines = stdout.Split('\n');
|
||||||
|
var files = new List<string>();
|
||||||
|
for (int i = 1; i < lines.Length; i++)
|
||||||
|
{
|
||||||
|
var line = lines[i].TrimEnd('\r');
|
||||||
|
if (string.IsNullOrWhiteSpace(line)) break;
|
||||||
|
files.Add(line.Trim());
|
||||||
|
}
|
||||||
|
return new MergePreview(true, false, files);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Any other exit (e.g. git too old: "unknown option --write-tree").
|
||||||
|
return new MergePreview(false, false, Array.Empty<string>());
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>Count of files that differ on <paramref name="sourceBranch"/> since its merge base with the target.</summary>
|
||||||
|
public async Task<int> CountChangedFilesAsync(
|
||||||
|
string repoDir, string targetBranch, string sourceBranch, CancellationToken ct = default)
|
||||||
|
{
|
||||||
|
var (exitCode, stdout, _) = await RunGitAsync(repoDir,
|
||||||
|
["diff", "--name-only", $"{targetBranch}...{sourceBranch}"], ct);
|
||||||
|
if (exitCode != 0) return 0;
|
||||||
|
return stdout
|
||||||
|
.Split('\n', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries)
|
||||||
|
.Count(s => s.Length > 0);
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 4: Run the tests, verify they pass**
|
||||||
|
|
||||||
|
Run: `dotnet test tests/ClaudeDo.Worker.Tests/ClaudeDo.Worker.Tests.csproj -c Release --filter FullyQualifiedName~GitServicePreviewMergeTests`
|
||||||
|
Expected: PASS (2 tests).
|
||||||
|
|
||||||
|
- [ ] **Step 5: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add src/ClaudeDo.Data/Git/GitService.cs tests/ClaudeDo.Worker.Tests/Runner/GitServicePreviewMergeTests.cs
|
||||||
|
git commit -m "feat(git): add non-destructive merge-tree conflict probe"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 2: TaskMergeService preview + approve-merge orchestration
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Modify: `src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs`
|
||||||
|
- Test: `tests/ClaudeDo.Worker.Tests/Services/TaskMergeServiceTests.cs`
|
||||||
|
|
||||||
|
`ApproveAndMergeAsync` merges first (reusing `MergeAsync`, `removeWorktree:false`) and only then delegates the `Done` flip to `ITaskStateService.ApproveReviewAsync` (the sole owner of Status writes). Conflicts/blocks return without flipping status. No DI cycle: `TaskStateService` and `PlanningChainCoordinator` do not depend on `TaskMergeService`.
|
||||||
|
|
||||||
|
- [ ] **Step 1: Update `BuildService` and add failing tests**
|
||||||
|
|
||||||
|
In `tests/ClaudeDo.Worker.Tests/Services/TaskMergeServiceTests.cs`, replace the `BuildService` helper so it also constructs a real `TaskStateService` (existing merge tests still pass — they only inspect the merge service's own broadcaster proxy):
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
private static (TaskMergeService svc, MergeRecordingClientProxy proxy) BuildService(DbFixture db)
|
||||||
|
{
|
||||||
|
var fakeHub = new MergeRecordingHubContext();
|
||||||
|
var broadcaster = new HubBroadcaster(fakeHub);
|
||||||
|
var state = TaskStateServiceBuilder.Build(db.CreateFactory()).State;
|
||||||
|
var svc = new TaskMergeService(
|
||||||
|
db.CreateFactory(),
|
||||||
|
new GitService(),
|
||||||
|
broadcaster,
|
||||||
|
state,
|
||||||
|
NullLogger<TaskMergeService>.Instance);
|
||||||
|
return (svc, fakeHub.Proxy);
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Add these tests to the class:
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
[Fact]
|
||||||
|
public async Task PreviewAsync_CleanWorktree_ReturnsClean()
|
||||||
|
{
|
||||||
|
if (!GitRepoFixture.IsGitAvailable()) return;
|
||||||
|
var repo = NewRepo();
|
||||||
|
var db = NewDb();
|
||||||
|
var (list, task) = await SeedListAndTask(db, repo.RepoDir, TaskStatus.WaitingForReview);
|
||||||
|
|
||||||
|
var wtMgr = BuildWorktreeManager(db);
|
||||||
|
var wtCtx = await wtMgr.CreateAsync(task, list, CancellationToken.None);
|
||||||
|
_wtCleanups.Add((repo.RepoDir, wtCtx.WorktreePath));
|
||||||
|
File.WriteAllText(Path.Combine(wtCtx.WorktreePath, "added.txt"), "x\n");
|
||||||
|
await wtMgr.CommitIfChangedAsync(wtCtx, task, list, CancellationToken.None);
|
||||||
|
|
||||||
|
var (svc, _) = BuildService(db);
|
||||||
|
var target = await new GitService().GetCurrentBranchAsync(repo.RepoDir);
|
||||||
|
|
||||||
|
var preview = await svc.PreviewAsync(task.Id, target, CancellationToken.None);
|
||||||
|
|
||||||
|
Assert.Equal(TaskMergeService.PreviewClean, preview.Status);
|
||||||
|
Assert.True(preview.ChangedFileCount >= 1);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task PreviewAsync_Conflict_ReturnsConflictFiles()
|
||||||
|
{
|
||||||
|
if (!GitRepoFixture.IsGitAvailable()) return;
|
||||||
|
var repo = NewRepo();
|
||||||
|
var db = NewDb();
|
||||||
|
var (list, task) = await SeedListAndTask(db, repo.RepoDir, TaskStatus.WaitingForReview);
|
||||||
|
|
||||||
|
var wtMgr = BuildWorktreeManager(db);
|
||||||
|
var wtCtx = await wtMgr.CreateAsync(task, list, CancellationToken.None);
|
||||||
|
_wtCleanups.Add((repo.RepoDir, wtCtx.WorktreePath));
|
||||||
|
File.WriteAllText(Path.Combine(wtCtx.WorktreePath, "README.md"), "# from worktree\n");
|
||||||
|
await wtMgr.CommitIfChangedAsync(wtCtx, task, list, CancellationToken.None);
|
||||||
|
|
||||||
|
File.WriteAllText(Path.Combine(repo.RepoDir, "README.md"), "# from main\n");
|
||||||
|
GitRepoFixture.RunGit(repo.RepoDir, "add", "-A");
|
||||||
|
GitRepoFixture.RunGit(repo.RepoDir, "commit", "-m", "main edit");
|
||||||
|
|
||||||
|
var (svc, _) = BuildService(db);
|
||||||
|
var target = await new GitService().GetCurrentBranchAsync(repo.RepoDir);
|
||||||
|
|
||||||
|
var preview = await svc.PreviewAsync(task.Id, target, CancellationToken.None);
|
||||||
|
|
||||||
|
Assert.Equal(TaskMergeService.PreviewConflict, preview.Status);
|
||||||
|
Assert.Contains("README.md", preview.ConflictFiles);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task PreviewAsync_NoActiveWorktree_ReturnsUnavailable()
|
||||||
|
{
|
||||||
|
var db = NewDb();
|
||||||
|
var (_, task) = await SeedListAndTask(db, workingDir: "/tmp", status: TaskStatus.WaitingForReview);
|
||||||
|
var (svc, _) = BuildService(db);
|
||||||
|
|
||||||
|
var preview = await svc.PreviewAsync(task.Id, "main", CancellationToken.None);
|
||||||
|
|
||||||
|
Assert.Equal(TaskMergeService.PreviewUnavailable, preview.Status);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task ApproveAndMergeAsync_CleanWorktree_MergesAndMarksDone()
|
||||||
|
{
|
||||||
|
if (!GitRepoFixture.IsGitAvailable()) return;
|
||||||
|
var repo = NewRepo();
|
||||||
|
var db = NewDb();
|
||||||
|
var (list, task) = await SeedListAndTask(db, repo.RepoDir, TaskStatus.WaitingForReview);
|
||||||
|
|
||||||
|
var wtMgr = BuildWorktreeManager(db);
|
||||||
|
var wtCtx = await wtMgr.CreateAsync(task, list, CancellationToken.None);
|
||||||
|
_wtCleanups.Add((repo.RepoDir, wtCtx.WorktreePath));
|
||||||
|
File.WriteAllText(Path.Combine(wtCtx.WorktreePath, "added.txt"), "new\n");
|
||||||
|
await wtMgr.CommitIfChangedAsync(wtCtx, task, list, CancellationToken.None);
|
||||||
|
|
||||||
|
var (svc, _) = BuildService(db);
|
||||||
|
var target = await new GitService().GetCurrentBranchAsync(repo.RepoDir);
|
||||||
|
|
||||||
|
var result = await svc.ApproveAndMergeAsync(task.Id, target, CancellationToken.None);
|
||||||
|
|
||||||
|
Assert.Equal(TaskMergeService.StatusMerged, result.Status);
|
||||||
|
using var ctx = db.CreateContext();
|
||||||
|
var updated = await new TaskRepository(ctx).GetByIdAsync(task.Id);
|
||||||
|
Assert.Equal(TaskStatus.Done, updated!.Status);
|
||||||
|
var wt = await new WorktreeRepository(ctx).GetByTaskIdAsync(task.Id);
|
||||||
|
Assert.Equal(WorktreeState.Merged, wt!.State);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task ApproveAndMergeAsync_Conflict_LeavesTaskWaitingForReview()
|
||||||
|
{
|
||||||
|
if (!GitRepoFixture.IsGitAvailable()) return;
|
||||||
|
var repo = NewRepo();
|
||||||
|
var db = NewDb();
|
||||||
|
var (list, task) = await SeedListAndTask(db, repo.RepoDir, TaskStatus.WaitingForReview);
|
||||||
|
|
||||||
|
var wtMgr = BuildWorktreeManager(db);
|
||||||
|
var wtCtx = await wtMgr.CreateAsync(task, list, CancellationToken.None);
|
||||||
|
_wtCleanups.Add((repo.RepoDir, wtCtx.WorktreePath));
|
||||||
|
File.WriteAllText(Path.Combine(wtCtx.WorktreePath, "README.md"), "# from worktree\n");
|
||||||
|
await wtMgr.CommitIfChangedAsync(wtCtx, task, list, CancellationToken.None);
|
||||||
|
|
||||||
|
File.WriteAllText(Path.Combine(repo.RepoDir, "README.md"), "# from main\n");
|
||||||
|
GitRepoFixture.RunGit(repo.RepoDir, "add", "-A");
|
||||||
|
GitRepoFixture.RunGit(repo.RepoDir, "commit", "-m", "main edit");
|
||||||
|
var headBefore = GitRepoFixture.RunGit(repo.RepoDir, "rev-parse", "HEAD").Trim();
|
||||||
|
|
||||||
|
var (svc, _) = BuildService(db);
|
||||||
|
var target = await new GitService().GetCurrentBranchAsync(repo.RepoDir);
|
||||||
|
|
||||||
|
var result = await svc.ApproveAndMergeAsync(task.Id, target, CancellationToken.None);
|
||||||
|
|
||||||
|
Assert.Equal(TaskMergeService.StatusConflict, result.Status);
|
||||||
|
Assert.Contains("README.md", result.ConflictFiles);
|
||||||
|
|
||||||
|
using var ctx = db.CreateContext();
|
||||||
|
var updated = await new TaskRepository(ctx).GetByIdAsync(task.Id);
|
||||||
|
Assert.Equal(TaskStatus.WaitingForReview, updated!.Status);
|
||||||
|
var wt = await new WorktreeRepository(ctx).GetByTaskIdAsync(task.Id);
|
||||||
|
Assert.Equal(WorktreeState.Active, wt!.State);
|
||||||
|
Assert.Equal(headBefore, GitRepoFixture.RunGit(repo.RepoDir, "rev-parse", "HEAD").Trim());
|
||||||
|
Assert.False(await new GitService().IsMidMergeAsync(repo.RepoDir));
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task ApproveAndMergeAsync_NoWorktree_MarksDone()
|
||||||
|
{
|
||||||
|
var db = NewDb();
|
||||||
|
var (_, task) = await SeedListAndTask(db, workingDir: "/tmp", status: TaskStatus.WaitingForReview);
|
||||||
|
var (svc, _) = BuildService(db);
|
||||||
|
|
||||||
|
var result = await svc.ApproveAndMergeAsync(task.Id, "main", CancellationToken.None);
|
||||||
|
|
||||||
|
Assert.Equal(TaskMergeService.StatusMerged, result.Status);
|
||||||
|
using var ctx = db.CreateContext();
|
||||||
|
var updated = await new TaskRepository(ctx).GetByIdAsync(task.Id);
|
||||||
|
Assert.Equal(TaskStatus.Done, updated!.Status);
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Run the tests, verify they fail**
|
||||||
|
|
||||||
|
Run: `dotnet test tests/ClaudeDo.Worker.Tests/ClaudeDo.Worker.Tests.csproj -c Release --filter FullyQualifiedName~TaskMergeServiceTests`
|
||||||
|
Expected: build error — `ITaskStateService` ctor arg, `PreviewAsync`, `ApproveAndMergeAsync`, `PreviewClean/PreviewConflict/PreviewUnavailable` do not exist.
|
||||||
|
|
||||||
|
- [ ] **Step 3: Implement in TaskMergeService**
|
||||||
|
|
||||||
|
In `src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs`:
|
||||||
|
|
||||||
|
Add `using ClaudeDo.Worker.State;` to the usings.
|
||||||
|
|
||||||
|
Add the preview-result record beside `MergeTargets`:
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
public sealed record MergePreviewResult(
|
||||||
|
string Status,
|
||||||
|
IReadOnlyList<string> ConflictFiles,
|
||||||
|
int ChangedFileCount);
|
||||||
|
```
|
||||||
|
|
||||||
|
Add the status constants beside the existing `StatusMerged` etc.:
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
public const string PreviewClean = "clean";
|
||||||
|
public const string PreviewConflict = "conflict";
|
||||||
|
public const string PreviewUnavailable = "unavailable";
|
||||||
|
```
|
||||||
|
|
||||||
|
Add the field and constructor param (inject `ITaskStateService`):
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
private readonly ITaskStateService _state;
|
||||||
|
|
||||||
|
public TaskMergeService(
|
||||||
|
IDbContextFactory<ClaudeDoDbContext> dbFactory,
|
||||||
|
GitService git,
|
||||||
|
HubBroadcaster broadcaster,
|
||||||
|
ITaskStateService state,
|
||||||
|
ILogger<TaskMergeService> logger)
|
||||||
|
{
|
||||||
|
_dbFactory = dbFactory;
|
||||||
|
_git = git;
|
||||||
|
_broadcaster = broadcaster;
|
||||||
|
_state = state;
|
||||||
|
_logger = logger;
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Add the two methods (e.g. after `GetTargetsAsync`):
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
public async Task<MergePreviewResult> PreviewAsync(string taskId, string targetBranch, CancellationToken ct)
|
||||||
|
{
|
||||||
|
var (_, list, wt) = await LoadMergeContextAsync(taskId, ct);
|
||||||
|
|
||||||
|
if (wt is null || wt.State != WorktreeState.Active)
|
||||||
|
return new MergePreviewResult(PreviewUnavailable, Array.Empty<string>(), 0);
|
||||||
|
if (string.IsNullOrWhiteSpace(list.WorkingDir) || !await _git.IsGitRepoAsync(list.WorkingDir, ct))
|
||||||
|
return new MergePreviewResult(PreviewUnavailable, Array.Empty<string>(), 0);
|
||||||
|
|
||||||
|
var target = string.IsNullOrWhiteSpace(targetBranch)
|
||||||
|
? await _git.GetCurrentBranchAsync(list.WorkingDir, ct)
|
||||||
|
: targetBranch;
|
||||||
|
|
||||||
|
var preview = await _git.PreviewMergeAsync(list.WorkingDir, target, wt.BranchName, ct);
|
||||||
|
if (!preview.Supported)
|
||||||
|
return new MergePreviewResult(PreviewUnavailable, Array.Empty<string>(), 0);
|
||||||
|
if (!preview.Clean)
|
||||||
|
return new MergePreviewResult(PreviewConflict, preview.ConflictFiles, 0);
|
||||||
|
|
||||||
|
var count = await _git.CountChangedFilesAsync(list.WorkingDir, target, wt.BranchName, ct);
|
||||||
|
return new MergePreviewResult(PreviewClean, Array.Empty<string>(), count);
|
||||||
|
}
|
||||||
|
|
||||||
|
public async Task<MergeResult> ApproveAndMergeAsync(string taskId, string targetBranch, CancellationToken ct)
|
||||||
|
{
|
||||||
|
var (task, list, wt) = await LoadMergeContextAsync(taskId, ct);
|
||||||
|
|
||||||
|
if (task.Status != TaskStatus.WaitingForReview)
|
||||||
|
return Blocked("task is not waiting for review");
|
||||||
|
|
||||||
|
// No worktree to merge (sandbox run, or an improvement parent whose children own
|
||||||
|
// the worktrees) — approve straight to Done.
|
||||||
|
if (wt is null || wt.State != WorktreeState.Active)
|
||||||
|
{
|
||||||
|
var done = await _state.ApproveReviewAsync(taskId, ct);
|
||||||
|
return done.Ok
|
||||||
|
? new MergeResult(StatusMerged, Array.Empty<string>(), null)
|
||||||
|
: Blocked(done.Reason ?? "approve failed");
|
||||||
|
}
|
||||||
|
|
||||||
|
var target = string.IsNullOrWhiteSpace(targetBranch)
|
||||||
|
? await _git.GetCurrentBranchAsync(list.WorkingDir, ct)
|
||||||
|
: targetBranch;
|
||||||
|
|
||||||
|
var merge = await MergeAsync(taskId, target, removeWorktree: false, $"Merge {wt.BranchName}", ct);
|
||||||
|
if (merge.Status != StatusMerged)
|
||||||
|
return merge; // conflict or blocked — leave the task in WaitingForReview
|
||||||
|
|
||||||
|
var approve = await _state.ApproveReviewAsync(taskId, ct);
|
||||||
|
return approve.Ok ? merge : Blocked(approve.Reason ?? "approve failed");
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 4: Run the tests, verify they pass**
|
||||||
|
|
||||||
|
Run: `dotnet test tests/ClaudeDo.Worker.Tests/ClaudeDo.Worker.Tests.csproj -c Release --filter FullyQualifiedName~TaskMergeServiceTests`
|
||||||
|
Expected: PASS (all existing + 6 new).
|
||||||
|
|
||||||
|
- [ ] **Step 5: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add src/ClaudeDo.Worker/Lifecycle/TaskMergeService.cs tests/ClaudeDo.Worker.Tests/Services/TaskMergeServiceTests.cs
|
||||||
|
git commit -m "feat(worker): approve merges worktree before marking task done"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 3: WorkerHub — PreviewMerge + result-returning ApproveReview
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Modify: `src/ClaudeDo.Worker/Hub/WorkerHub.cs`
|
||||||
|
|
||||||
|
This is SignalR wiring (no unit test); verify by building the Worker.
|
||||||
|
|
||||||
|
- [ ] **Step 1: Add the DTO**
|
||||||
|
|
||||||
|
Beside the existing `MergeResultDto`/`MergeTargetsDto` records (around line 56):
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
public record MergePreviewDto(string Status, IReadOnlyList<string> ConflictFiles, int ChangedFileCount);
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Add `PreviewMerge` and replace `ApproveReview`**
|
||||||
|
|
||||||
|
Add a `PreviewMerge` method beside `GetMergeTargets`:
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
public Task<MergePreviewDto> PreviewMerge(string taskId, string targetBranch)
|
||||||
|
=> HubGuard(async () =>
|
||||||
|
{
|
||||||
|
var p = await _mergeService.PreviewAsync(taskId, targetBranch ?? "", CancellationToken.None);
|
||||||
|
return new MergePreviewDto(p.Status, p.ConflictFiles, p.ChangedFileCount);
|
||||||
|
});
|
||||||
|
```
|
||||||
|
|
||||||
|
Replace the existing `ApproveReview` method (currently lines ~383-387, delegating to `_state.ApproveReviewAsync`) with:
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
public Task<MergeResultDto> ApproveReview(string taskId, string targetBranch)
|
||||||
|
=> HubGuard(async () =>
|
||||||
|
{
|
||||||
|
var r = await _mergeService.ApproveAndMergeAsync(taskId, targetBranch ?? "", CancellationToken.None);
|
||||||
|
if (r.Status == TaskMergeService.StatusBlocked)
|
||||||
|
throw new HubException(r.ErrorMessage ?? "approve failed");
|
||||||
|
return new MergeResultDto(r.Status, r.ConflictFiles, r.ErrorMessage);
|
||||||
|
});
|
||||||
|
```
|
||||||
|
|
||||||
|
(Conflicts are returned, not thrown, so the UI can display the conflicting files; only hard blocks throw.)
|
||||||
|
|
||||||
|
- [ ] **Step 3: Build the Worker, verify green**
|
||||||
|
|
||||||
|
Run: `dotnet build src/ClaudeDo.Worker/ClaudeDo.Worker.csproj -c Release`
|
||||||
|
Expected: Build succeeded. (DI resolves the new `ITaskStateService` dependency of `TaskMergeService` automatically — it is already registered.)
|
||||||
|
|
||||||
|
- [ ] **Step 4: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add src/ClaudeDo.Worker/Hub/WorkerHub.cs
|
||||||
|
git commit -m "feat(worker): expose PreviewMerge hub method and merge-on-approve"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 4: UI client + interface + test fakes
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Modify: `src/ClaudeDo.Ui/Services/Interfaces/IWorkerClient.cs`
|
||||||
|
- Modify: `src/ClaudeDo.Ui/Services/WorkerClient.cs`
|
||||||
|
- Modify: `src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs` (caller at line 648)
|
||||||
|
- Modify: `tests/ClaudeDo.Worker.Tests/UiVm/TasksIslandViewModelPlanningTests.cs` (`FakeWorkerClient`)
|
||||||
|
- Modify: `tests/ClaudeDo.Ui.Tests/StubWorkerClient.cs`
|
||||||
|
- Modify: `tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandPlanningTests.cs` (override)
|
||||||
|
|
||||||
|
Note: `DetailsIslandViewModel.ApproveReviewAsync` (line 1368) is updated in Task 5, not here — but the interface change forces it to compile, so Task 5 must follow before the Ui project builds. To keep this task self-contained and green on its own, update that call site here too (the conflict-handling logic lands in Task 5).
|
||||||
|
|
||||||
|
- [ ] **Step 1: Add the UI DTO**
|
||||||
|
|
||||||
|
In `src/ClaudeDo.Ui/Services/WorkerClient.cs`, beside the existing `MergeResultDto`/`MergeTargetsDto` records (lines 521-522):
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
public record MergePreviewDto(string Status, IReadOnlyList<string> ConflictFiles, int ChangedFileCount);
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Update the interface**
|
||||||
|
|
||||||
|
In `src/ClaudeDo.Ui/Services/Interfaces/IWorkerClient.cs`, replace `Task ApproveReviewAsync(string taskId);` (line 40) with:
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
Task<MergeResultDto?> ApproveReviewAsync(string taskId, string targetBranch);
|
||||||
|
Task<MergePreviewDto?> PreviewMergeAsync(string taskId, string targetBranch);
|
||||||
|
Task<MergeResultDto> MergeTaskAsync(string taskId, string targetBranch, bool removeWorktree, string commitMessage);
|
||||||
|
```
|
||||||
|
|
||||||
|
(`MergeTaskAsync` already exists on the concrete `WorkerClient` — this only adds it to the interface.)
|
||||||
|
|
||||||
|
- [ ] **Step 3: Update the concrete client**
|
||||||
|
|
||||||
|
In `src/ClaudeDo.Ui/Services/WorkerClient.cs`, replace the existing `ApproveReviewAsync` (line ~389) and add `PreviewMergeAsync`. Mirror the existing `GetMergeTargetsAsync` pattern (it uses the `TryInvokeAsync<T>` helper which returns `null` when disconnected):
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
public Task<MergeResultDto?> ApproveReviewAsync(string taskId, string targetBranch)
|
||||||
|
=> TryInvokeAsync<MergeResultDto>("ApproveReview", taskId, targetBranch);
|
||||||
|
|
||||||
|
public Task<MergePreviewDto?> PreviewMergeAsync(string taskId, string targetBranch)
|
||||||
|
=> TryInvokeAsync<MergePreviewDto>("PreviewMerge", taskId, targetBranch);
|
||||||
|
```
|
||||||
|
|
||||||
|
Ensure the existing `public async Task<MergeResultDto> MergeTaskAsync(...)` signature matches the interface exactly (params: `string taskId, string targetBranch, bool removeWorktree, string commitMessage`). Leave its body as-is.
|
||||||
|
|
||||||
|
- [ ] **Step 4: Update the two callers**
|
||||||
|
|
||||||
|
`src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs` line 648 — the list-level quick approve has no merge-target selector, so it merges into the repo's current branch (empty string resolves server-side):
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
try { await _worker.ApproveReviewAsync(row.Id, ""); }
|
||||||
|
```
|
||||||
|
|
||||||
|
`src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs` line 1368 — update to the new signature for now (full conflict handling is added in Task 5):
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
try { await _worker.ApproveReviewAsync(Task.Id, SelectedMergeTarget ?? ""); }
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 5: Update the three test fakes**
|
||||||
|
|
||||||
|
`tests/ClaudeDo.Ui.Tests/StubWorkerClient.cs` line 53 — replace and add:
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
public virtual Task<MergeResultDto?> ApproveReviewAsync(string taskId, string targetBranch) => Task.FromResult<MergeResultDto?>(null);
|
||||||
|
public virtual Task<MergePreviewDto?> PreviewMergeAsync(string taskId, string targetBranch) => Task.FromResult<MergePreviewDto?>(null);
|
||||||
|
public virtual Task<MergeResultDto> MergeTaskAsync(string taskId, string targetBranch, bool removeWorktree, string commitMessage) => Task.FromResult(new MergeResultDto("merged", System.Array.Empty<string>(), null));
|
||||||
|
```
|
||||||
|
|
||||||
|
`tests/ClaudeDo.Worker.Tests/UiVm/TasksIslandViewModelPlanningTests.cs` line 45 (`FakeWorkerClient`) — replace and add:
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
public Task<MergeResultDto?> ApproveReviewAsync(string taskId, string targetBranch) => Task.FromResult<MergeResultDto?>(null);
|
||||||
|
public Task<MergePreviewDto?> PreviewMergeAsync(string taskId, string targetBranch) => Task.FromResult<MergePreviewDto?>(null);
|
||||||
|
public Task<MergeResultDto> MergeTaskAsync(string taskId, string targetBranch, bool removeWorktree, string commitMessage) => Task.FromResult(new MergeResultDto("merged", System.Array.Empty<string>(), null));
|
||||||
|
```
|
||||||
|
|
||||||
|
(Confirm whether `FakeWorkerClient` already implements `MergeTaskAsync`; if so, only change `ApproveReviewAsync` and add `PreviewMergeAsync`. Add `using` for the DTO namespace if needed — same namespace as `IWorkerClient`.)
|
||||||
|
|
||||||
|
`tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandPlanningTests.cs` line 77 — update the override signature:
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
public override Task<MergeResultDto?> ApproveReviewAsync(string taskId, string targetBranch) =>
|
||||||
|
/* keep whatever recording/behavior this override had, now returning Task<MergeResultDto?> */
|
||||||
|
Task.FromResult<MergeResultDto?>(null);
|
||||||
|
```
|
||||||
|
|
||||||
|
(Preserve any side effect the existing override performed — e.g. recording the call — just change the signature and return type.)
|
||||||
|
|
||||||
|
- [ ] **Step 6: Build UI + run both UI-touching test projects**
|
||||||
|
|
||||||
|
Run: `dotnet build src/ClaudeDo.App/ClaudeDo.App.csproj -c Release`
|
||||||
|
Run: `dotnet test tests/ClaudeDo.Ui.Tests/ClaudeDo.Ui.Tests.csproj -c Release`
|
||||||
|
Run: `dotnet test tests/ClaudeDo.Worker.Tests/ClaudeDo.Worker.Tests.csproj -c Release --filter FullyQualifiedName~TasksIslandViewModelPlanning`
|
||||||
|
Expected: all green.
|
||||||
|
|
||||||
|
- [ ] **Step 7: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add src/ClaudeDo.Ui/Services/Interfaces/IWorkerClient.cs src/ClaudeDo.Ui/Services/WorkerClient.cs src/ClaudeDo.Ui/ViewModels/Islands/TasksIslandViewModel.cs src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs tests/ClaudeDo.Ui.Tests/StubWorkerClient.cs tests/ClaudeDo.Ui.Tests/ViewModels/DetailsIslandPlanningTests.cs tests/ClaudeDo.Worker.Tests/UiVm/TasksIslandViewModelPlanningTests.cs
|
||||||
|
git commit -m "feat(ui): wire merge-aware approve and preview into the worker client"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 5: Mergeability presenter + DetailsIslandViewModel wiring
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Create: `src/ClaudeDo.Ui/ViewModels/Islands/MergePreviewPresenter.cs`
|
||||||
|
- Modify: `src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs`
|
||||||
|
- Test: `tests/ClaudeDo.Ui.Tests/ViewModels/MergePreviewPresenterTests.cs` (create)
|
||||||
|
|
||||||
|
- [ ] **Step 1: Write the failing presenter tests**
|
||||||
|
|
||||||
|
Create `tests/ClaudeDo.Ui.Tests/ViewModels/MergePreviewPresenterTests.cs`:
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
using ClaudeDo.Ui.Services;
|
||||||
|
using ClaudeDo.Ui.ViewModels.Islands;
|
||||||
|
|
||||||
|
namespace ClaudeDo.Ui.Tests.ViewModels;
|
||||||
|
|
||||||
|
public class MergePreviewPresenterTests
|
||||||
|
{
|
||||||
|
[Fact]
|
||||||
|
public void Clean_Plural()
|
||||||
|
{
|
||||||
|
var (text, clean, conflict) = MergePreviewPresenter.Describe(
|
||||||
|
new MergePreviewDto("clean", System.Array.Empty<string>(), 3));
|
||||||
|
Assert.Equal("Merges cleanly · 3 files", text);
|
||||||
|
Assert.True(clean);
|
||||||
|
Assert.False(conflict);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Clean_Singular()
|
||||||
|
{
|
||||||
|
var (text, _, _) = MergePreviewPresenter.Describe(
|
||||||
|
new MergePreviewDto("clean", System.Array.Empty<string>(), 1));
|
||||||
|
Assert.Equal("Merges cleanly · 1 file", text);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Conflict_ListsUpToThree()
|
||||||
|
{
|
||||||
|
var (text, clean, conflict) = MergePreviewPresenter.Describe(
|
||||||
|
new MergePreviewDto("conflict", new[] { "a.cs", "b.cs" }, 0));
|
||||||
|
Assert.Equal("Conflicts in a.cs, b.cs", text);
|
||||||
|
Assert.False(clean);
|
||||||
|
Assert.True(conflict);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Conflict_TruncatesWithMore()
|
||||||
|
{
|
||||||
|
var (text, _, _) = MergePreviewPresenter.Describe(
|
||||||
|
new MergePreviewDto("conflict", new[] { "a", "b", "c", "d", "e" }, 0));
|
||||||
|
Assert.Equal("Conflicts in a, b, c (+2 more)", text);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Unavailable_IsMuted()
|
||||||
|
{
|
||||||
|
var (text, clean, conflict) = MergePreviewPresenter.Describe(
|
||||||
|
new MergePreviewDto("unavailable", System.Array.Empty<string>(), 0));
|
||||||
|
Assert.Equal("Mergeability unknown", text);
|
||||||
|
Assert.False(clean);
|
||||||
|
Assert.False(conflict);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void Null_IsEmpty()
|
||||||
|
{
|
||||||
|
var (text, clean, conflict) = MergePreviewPresenter.Describe(null);
|
||||||
|
Assert.Equal("", text);
|
||||||
|
Assert.False(clean);
|
||||||
|
Assert.False(conflict);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Run, verify it fails to compile**
|
||||||
|
|
||||||
|
Run: `dotnet test tests/ClaudeDo.Ui.Tests/ClaudeDo.Ui.Tests.csproj -c Release --filter FullyQualifiedName~MergePreviewPresenterTests`
|
||||||
|
Expected: build error — `MergePreviewPresenter` does not exist.
|
||||||
|
|
||||||
|
- [ ] **Step 3: Create the presenter**
|
||||||
|
|
||||||
|
Create `src/ClaudeDo.Ui/ViewModels/Islands/MergePreviewPresenter.cs`:
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
using System.Linq;
|
||||||
|
using ClaudeDo.Ui.Services;
|
||||||
|
|
||||||
|
namespace ClaudeDo.Ui.ViewModels.Islands;
|
||||||
|
|
||||||
|
/// Pure mapping from a merge-preview DTO to display text + color flags.
|
||||||
|
public static class MergePreviewPresenter
|
||||||
|
{
|
||||||
|
public static (string Text, bool IsClean, bool IsConflict) Describe(MergePreviewDto? dto)
|
||||||
|
{
|
||||||
|
if (dto is null) return ("", false, false);
|
||||||
|
|
||||||
|
switch (dto.Status)
|
||||||
|
{
|
||||||
|
case "clean":
|
||||||
|
var unit = dto.ChangedFileCount == 1 ? "file" : "files";
|
||||||
|
return ($"Merges cleanly · {dto.ChangedFileCount} {unit}", true, false);
|
||||||
|
|
||||||
|
case "conflict":
|
||||||
|
var names = string.Join(", ", dto.ConflictFiles.Take(3));
|
||||||
|
var more = dto.ConflictFiles.Count > 3 ? $" (+{dto.ConflictFiles.Count - 3} more)" : "";
|
||||||
|
return ($"Conflicts in {names}{more}", false, true);
|
||||||
|
|
||||||
|
default:
|
||||||
|
return ("Mergeability unknown", false, false);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 4: Run, verify the presenter tests pass**
|
||||||
|
|
||||||
|
Run: `dotnet test tests/ClaudeDo.Ui.Tests/ClaudeDo.Ui.Tests.csproj -c Release --filter FullyQualifiedName~MergePreviewPresenterTests`
|
||||||
|
Expected: PASS (6 tests).
|
||||||
|
|
||||||
|
- [ ] **Step 5: Wire the presenter into DetailsIslandViewModel**
|
||||||
|
|
||||||
|
In `src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs`:
|
||||||
|
|
||||||
|
(a) Add observable properties (near the other merge properties, ~line 334):
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
[ObservableProperty]
|
||||||
|
[NotifyPropertyChangedFor(nameof(ShowMergePreviewMuted))]
|
||||||
|
private string _mergePreviewText = "";
|
||||||
|
|
||||||
|
[ObservableProperty]
|
||||||
|
[NotifyPropertyChangedFor(nameof(ShowMergePreviewMuted))]
|
||||||
|
private bool _mergeIsClean;
|
||||||
|
|
||||||
|
[ObservableProperty]
|
||||||
|
[NotifyPropertyChangedFor(nameof(ShowMergePreviewMuted))]
|
||||||
|
private bool _mergeIsConflict;
|
||||||
|
|
||||||
|
public bool ShowMergePreviewMuted =>
|
||||||
|
!MergeIsClean && !MergeIsConflict && !string.IsNullOrEmpty(MergePreviewText);
|
||||||
|
|
||||||
|
public bool ShowSingleMerge =>
|
||||||
|
WorktreePath != null && Task?.IsPlanningParent != true;
|
||||||
|
```
|
||||||
|
|
||||||
|
(b) Add the refresh method:
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
private async System.Threading.Tasks.Task RefreshMergePreviewAsync()
|
||||||
|
{
|
||||||
|
if (Task is null || WorktreePath is null)
|
||||||
|
{
|
||||||
|
MergePreviewText = ""; MergeIsClean = false; MergeIsConflict = false;
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
// Only probe Active worktrees; terminal states show their label instead.
|
||||||
|
if (WorktreeStateLabel is { } label && label != "Active")
|
||||||
|
{
|
||||||
|
MergePreviewText = label; MergeIsClean = false; MergeIsConflict = false;
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
var dto = await _worker.PreviewMergeAsync(Task.Id, SelectedMergeTarget ?? "");
|
||||||
|
var (text, clean, conflict) = MergePreviewPresenter.Describe(dto);
|
||||||
|
MergePreviewText = text; MergeIsClean = clean; MergeIsConflict = conflict;
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
(c) Recompute when the merge target changes — add (or extend) the generated partial:
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
partial void OnSelectedMergeTargetChanged(string? value)
|
||||||
|
{
|
||||||
|
_ = RefreshMergePreviewAsync();
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
(d) Notify `ShowSingleMerge` when the worktree path changes. In the existing `OnWorktreePathChanged` (line ~1141) add:
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
OnPropertyChanged(nameof(ShowSingleMerge));
|
||||||
|
```
|
||||||
|
|
||||||
|
(e) Load merge targets for standalone worktree tasks. In `BindAsync`, after the `if (entity.PlanningPhase != None) {...} else {...}` block (~line 814), add:
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
if (entity.Worktree != null
|
||||||
|
&& entity.PlanningPhase == ClaudeDo.Data.Models.PlanningPhase.None
|
||||||
|
&& MergeTargetBranches.Count == 0)
|
||||||
|
{
|
||||||
|
var targets = await _worker.GetMergeTargetsAsync(row.Id);
|
||||||
|
if (targets != null)
|
||||||
|
{
|
||||||
|
MergeTargetBranches.Clear();
|
||||||
|
foreach (var b in targets.LocalBranches) MergeTargetBranches.Add(b);
|
||||||
|
SelectedMergeTarget = targets.DefaultBranch; // triggers OnSelectedMergeTargetChanged → preview
|
||||||
|
}
|
||||||
|
}
|
||||||
|
await RefreshMergePreviewAsync();
|
||||||
|
```
|
||||||
|
|
||||||
|
(f) Replace the body of `ApproveReviewAsync` (line ~1362) to surface conflicts:
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
[RelayCommand]
|
||||||
|
private async System.Threading.Tasks.Task ApproveReviewAsync()
|
||||||
|
{
|
||||||
|
if (Task is null || !_worker.IsConnected) return;
|
||||||
|
try
|
||||||
|
{
|
||||||
|
var result = await _worker.ApproveReviewAsync(Task.Id, SelectedMergeTarget ?? "");
|
||||||
|
if (result?.Status == "conflict")
|
||||||
|
{
|
||||||
|
var (text, _, _) = MergePreviewPresenter.Describe(
|
||||||
|
new MergePreviewDto("conflict", result.ConflictFiles, 0));
|
||||||
|
MergePreviewText = text; MergeIsClean = false; MergeIsConflict = true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
catch { /* stale review action; broadcast reconciles */ }
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
(g) Add the single-task `MergeCommand` (place near `OpenDiffAsync`):
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
[RelayCommand]
|
||||||
|
private async System.Threading.Tasks.Task MergeAsync()
|
||||||
|
{
|
||||||
|
if (Task is null || WorktreePath is null || !_worker.IsConnected) return;
|
||||||
|
try
|
||||||
|
{
|
||||||
|
var result = await _worker.MergeTaskAsync(Task.Id, SelectedMergeTarget ?? "", false, "Merge task");
|
||||||
|
if (result.Status == "conflict")
|
||||||
|
{
|
||||||
|
var (text, _, _) = MergePreviewPresenter.Describe(
|
||||||
|
new MergePreviewDto("conflict", result.ConflictFiles, 0));
|
||||||
|
MergePreviewText = text; MergeIsClean = false; MergeIsConflict = true;
|
||||||
|
}
|
||||||
|
else
|
||||||
|
{
|
||||||
|
await RefreshMergePreviewAsync();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
catch { /* broadcast reconciles */ }
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 6: Build UI + run the UI tests**
|
||||||
|
|
||||||
|
Run: `dotnet build src/ClaudeDo.App/ClaudeDo.App.csproj -c Release`
|
||||||
|
Run: `dotnet test tests/ClaudeDo.Ui.Tests/ClaudeDo.Ui.Tests.csproj -c Release`
|
||||||
|
Expected: green. (If `OnSelectedMergeTargetChanged` already exists, merge the new line into it instead of duplicating.)
|
||||||
|
|
||||||
|
- [ ] **Step 7: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add src/ClaudeDo.Ui/ViewModels/Islands/MergePreviewPresenter.cs src/ClaudeDo.Ui/ViewModels/Islands/DetailsIslandViewModel.cs tests/ClaudeDo.Ui.Tests/ViewModels/MergePreviewPresenterTests.cs
|
||||||
|
git commit -m "feat(ui): show mergeability and surface approve conflicts in the work console"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 6: WorkConsole — status line + Merge button
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Modify: `src/ClaudeDo.Ui/Views/Islands/Detail/WorkConsole.axaml`
|
||||||
|
|
||||||
|
No unit test (XAML); verified by build + manual visual check in Task 7.
|
||||||
|
|
||||||
|
- [ ] **Step 1: Add the mergeability status line and the Merge button**
|
||||||
|
|
||||||
|
In the `MERGE & WORKTREE` `StackPanel` (starts line 196), insert the status line **between** the merge-target `StackPanel` (ends line 203) and the `<WrapPanel>` (line 204). Three single-line `TextBlock`s, one visible at a time by color:
|
||||||
|
|
||||||
|
```xml
|
||||||
|
<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>
|
||||||
|
```
|
||||||
|
|
||||||
|
In the `<WrapPanel>` (line 204), add a **Merge** button immediately after the "Open Diff" button (line 206):
|
||||||
|
|
||||||
|
```xml
|
||||||
|
<Button Classes="btn accent" Content="Merge" Margin="0,0,8,8"
|
||||||
|
Command="{Binding MergeCommand}"
|
||||||
|
IsVisible="{Binding ShowSingleMerge}" />
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Build UI, verify green**
|
||||||
|
|
||||||
|
Run: `dotnet build src/ClaudeDo.App/ClaudeDo.App.csproj -c Release`
|
||||||
|
Expected: Build succeeded (XAML compiles; all bound members exist from Task 5).
|
||||||
|
|
||||||
|
- [ ] **Step 3: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add src/ClaudeDo.Ui/Views/Islands/Detail/WorkConsole.axaml
|
||||||
|
git commit -m "feat(ui): add mergeability indicator and Merge button to work console"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Task 7: Full build, full test, manual verification
|
||||||
|
|
||||||
|
**Files:** none (verification only)
|
||||||
|
|
||||||
|
- [ ] **Step 1: Build the whole app + worker**
|
||||||
|
|
||||||
|
Run: `dotnet build src/ClaudeDo.App/ClaudeDo.App.csproj -c Release`
|
||||||
|
Run: `dotnet build src/ClaudeDo.Worker/ClaudeDo.Worker.csproj -c Release`
|
||||||
|
Expected: both succeed.
|
||||||
|
|
||||||
|
- [ ] **Step 2: Run all touched test projects**
|
||||||
|
|
||||||
|
Run: `dotnet test tests/ClaudeDo.Worker.Tests/ClaudeDo.Worker.Tests.csproj -c Release`
|
||||||
|
Run: `dotnet test tests/ClaudeDo.Ui.Tests/ClaudeDo.Ui.Tests.csproj -c Release`
|
||||||
|
Expected: all green.
|
||||||
|
|
||||||
|
- [ ] **Step 3: Manual verification (cannot be automated — no real Claude in tests)**
|
||||||
|
|
||||||
|
Start the Worker, then the App. Pick a list whose `WorkingDir` is a real git repo and use a task that already has an Active worktree (or create one).
|
||||||
|
|
||||||
|
Verify each acceptance criterion:
|
||||||
|
1. **Clean approve:** Open a `WaitingForReview` task whose worktree merges cleanly → the Session tab shows green "Merges cleanly · N files". Click **Approve** → the worktree merges into the target, the task becomes **Done**, and the worktree state becomes **Merged** (check the worktree overview).
|
||||||
|
2. **Conflicting approve:** Open a task whose worktree conflicts with the target → the Session tab shows red "Conflicts in …". Click **Approve** → the task stays **WaitingForReview** (NOT Done), the conflict line remains, and the target branch is unchanged.
|
||||||
|
3. **Done task preview:** Open a previously-Done task that was never merged (worktree still Active) → the merge/conflict status appears without any tree mutation; the **Merge** button merges it on demand.
|
||||||
|
|
||||||
|
Report the result of each check explicitly. If any visual issue appears (colors, layout, missing controls), note it for the user — do not claim the UI works without running it.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Self-review notes
|
||||||
|
|
||||||
|
- **Spec coverage:** Approve-merge (Task 2/3/5), conflict-keeps-review (Task 2 test + Task 5 surfacing), non-destructive preview (Task 1/2 + indicator in Task 5/6), real single-task Merge button (Task 5/6), standalone target-loading gap (Task 5e). All spec sections map to a task.
|
||||||
|
- **Type consistency:** `MergePreview` (Data) → `MergePreviewResult` (Worker service) → `MergePreviewDto` (hub + UI). Status strings `clean`/`conflict`/`unavailable` and merge statuses `merged`/`conflict`/`blocked` are used consistently across worker, client, presenter, and VM.
|
||||||
|
- **No new statuses, no DB migration, no localization keys** (literals match the surrounding controls).
|
||||||
|
- **External MCP unchanged:** `ExternalMcpService.ReviewTask` keeps calling `TaskStateService.ApproveReviewAsync` directly (its documented scope excludes merges); that method's signature is unchanged.
|
||||||
Reference in New Issue
Block a user