docs: add worktree merge design spec
Captures decisions for merging a task worktree into a target branch: merge-commit strategy, dual UI entry (Details island + DiffModal), per-merge cleanup checkbox, pre-flight + abort-on-conflict.
This commit is contained in:
Mika Kuns
208
docs/superpowers/specs/2026-04-22-worktree-merge-design.md
Normal file
208
docs/superpowers/specs/2026-04-22-worktree-merge-design.md
Normal file
@@ -0,0 +1,208 @@
|
||||
# Worktree merge into target branch — design
|
||||
|
||||
Date: 2026-04-22
|
||||
Status: Approved (pending user review)
|
||||
|
||||
## Problem
|
||||
|
||||
`WorktreeState.Merged` exists but nothing sets it. `GitService.MergeFfOnlyAsync` exists but is unused. `DetailsIslandViewModel.ApproveMergeAsync` is a stub (`// TODO: call worker merge hub method when available`). Users have no way to merge a task's worktree back into a target branch; the only post-task options today are Discard (via Reset) or leave it Active.
|
||||
|
||||
## Goals
|
||||
|
||||
- Allow merging a task worktree's `claudedo/{id}` branch into a chosen local branch of the list's `WorkingDir`.
|
||||
- Preserve merge history via a real merge commit.
|
||||
- Never leave the target branch in a broken state.
|
||||
- Reuse existing patterns: `TaskResetService`, maintenance sweeper, dialog factory.
|
||||
|
||||
## Non-goals
|
||||
|
||||
- Remote push after merge (user does this manually).
|
||||
- Pull/fetch before merge.
|
||||
- Rebasing the task branch onto a moved target (done via Continue prompt or manually).
|
||||
- Merging across repos or handling submodules.
|
||||
- Automated UI tests (project has none).
|
||||
|
||||
## Decisions
|
||||
|
||||
| Decision | Choice |
|
||||
| --- | --- |
|
||||
| Target branch | Default to current `HEAD` branch of `WorkingDir`; user may override via dropdown. |
|
||||
| Merge strategy | Always `git merge --no-ff -m <msg> claudedo/{id}` — explicit merge commit. |
|
||||
| Post-merge cleanup | Per-merge checkbox in the dialog, default on: remove worktree dir + delete branch. |
|
||||
| Conflicts | Pre-flight guard (worktree/branch state checks); on conflict during merge, `git merge --abort` and return conflicted files to UI. |
|
||||
| UI entry points | Details island agent strip (wires existing stub) **and** DiffModal "Merge" button — both open the same modal. |
|
||||
|
||||
## Architecture
|
||||
|
||||
### New backend service
|
||||
|
||||
`src/ClaudeDo.Worker/Services/TaskMergeService.cs` — mirrors `TaskResetService`.
|
||||
|
||||
```
|
||||
public sealed class TaskMergeService
|
||||
{
|
||||
Task<MergeResult> MergeAsync(
|
||||
string taskId,
|
||||
string targetBranch,
|
||||
bool removeWorktree,
|
||||
string commitMessage,
|
||||
CancellationToken ct);
|
||||
|
||||
Task<MergeTargets> GetTargetsAsync(string taskId, CancellationToken ct);
|
||||
}
|
||||
|
||||
public sealed record MergeResult(string Status, IReadOnlyList<string> ConflictFiles, string? ErrorMessage);
|
||||
// Status ∈ "merged" | "conflict" | "blocked"
|
||||
|
||||
public sealed record MergeTargets(string DefaultBranch, IReadOnlyList<string> LocalBranches);
|
||||
```
|
||||
|
||||
Pre-flight checks (all must pass):
|
||||
1. Task exists, status not `Running`.
|
||||
2. Worktree exists, state == `Active`.
|
||||
3. `list.WorkingDir` is set and is a git repo.
|
||||
4. Target working tree is clean (`HasChangesAsync == false`).
|
||||
5. Target repo is not mid-merge (`IsMidMergeAsync == false`).
|
||||
|
||||
Failures short-circuit to `MergeResult("blocked", [], reason)` before any git write.
|
||||
|
||||
Success path:
|
||||
1. `GitService.MergeNoFfAsync(list.WorkingDir, wt.BranchName, commitMessage, ct)`.
|
||||
2. If `removeWorktree`:
|
||||
- `WorktreeRemoveAsync(list.WorkingDir, wt.Path, force: false, ct)` — reuse existing method.
|
||||
- `BranchDeleteAsync(list.WorkingDir, wt.BranchName, force: false, ct)`.
|
||||
3. `WorktreeRepository.SetStateAsync(taskId, WorktreeState.Merged, ct)`.
|
||||
4. `HubBroadcaster.BroadcastWorktreeUpdated(taskId)`.
|
||||
5. Log info; return `MergeResult("merged", [], null)`.
|
||||
|
||||
Conflict path (merge invoked, git returns non-zero with `CONFLICT` on stderr/stdout):
|
||||
1. Collect conflicted files: `git diff --name-only --diff-filter=U`.
|
||||
2. `GitService.MergeAbortAsync(list.WorkingDir, ct)`.
|
||||
3. Worktree state stays `Active`; no broadcast (nothing changed).
|
||||
4. Return `MergeResult("conflict", files, null)`.
|
||||
|
||||
### GitService additions
|
||||
|
||||
```
|
||||
Task<string> GetCurrentBranchAsync(string repoDir, CancellationToken ct); // git symbolic-ref --short HEAD
|
||||
Task<List<string>> ListLocalBranchesAsync(string repoDir, CancellationToken ct); // git branch --format=%(refname:short)
|
||||
Task MergeNoFfAsync(string repoDir, string sourceBranch, string message, CancellationToken ct); // git merge --no-ff -m <msg> <src>
|
||||
Task MergeAbortAsync(string repoDir, CancellationToken ct); // git merge --abort
|
||||
Task<bool> IsMidMergeAsync(string repoDir, CancellationToken ct); // File.Exists($"{gitDir}/MERGE_HEAD")
|
||||
Task<List<string>> ListConflictedFilesAsync(string repoDir, CancellationToken ct); // git diff --name-only --diff-filter=U
|
||||
```
|
||||
|
||||
`MergeNoFfAsync` must NOT throw on non-zero — it must return the exit code/stderr so the caller can distinguish conflict from other failures. Two ways:
|
||||
- Overload to return `(int ExitCode, string Stderr)`; or
|
||||
- Throw a dedicated `GitMergeConflictException` vs `InvalidOperationException`.
|
||||
|
||||
**Pick:** expose a tuple-returning variant for `MergeNoFfAsync` only — keeps other methods consistent, avoids exception-for-control-flow.
|
||||
|
||||
### Hub surface
|
||||
|
||||
`src/ClaudeDo.Worker/Hub/WorkerHub.cs` gains:
|
||||
|
||||
```
|
||||
Task<MergeResultDto> MergeTask(string taskId, string targetBranch, bool removeWorktree, string commitMessage);
|
||||
Task<MergeTargetsDto> GetMergeTargets(string taskId);
|
||||
```
|
||||
|
||||
DTOs mirror the service records. Unexpected exceptions are re-wrapped as `HubException` (same pattern as `ResetTask`). Expected conditions (blocked, conflict) travel via the result DTO, not exceptions.
|
||||
|
||||
### WorkerClient
|
||||
|
||||
`src/ClaudeDo.Ui/Services/WorkerClient.cs`:
|
||||
|
||||
```
|
||||
Task<MergeResultDto> MergeTaskAsync(string taskId, string targetBranch, bool removeWorktree, string commitMessage);
|
||||
Task<MergeTargetsDto> GetMergeTargetsAsync(string taskId);
|
||||
```
|
||||
|
||||
### UI
|
||||
|
||||
**New modal:** `src/ClaudeDo.Ui/Views/Modals/MergeModalView.axaml` + `MergeModalViewModel.cs`.
|
||||
|
||||
Dialog fields:
|
||||
- **Target branch** combobox (source: `GetMergeTargetsAsync.LocalBranches`; default: `DefaultBranch`).
|
||||
- **Remove worktree after merge** checkbox (default: checked).
|
||||
- **Commit message** text field (default: `Merge task: {Task.Title}`).
|
||||
- OK / Cancel buttons.
|
||||
|
||||
Post-submit UI states (rendered inside the modal, not a second dialog):
|
||||
- `merged` → brief success line, modal closes after 1–2s; parent refreshes.
|
||||
- `conflict` → red inline panel listing files; OK button hidden, only Close remains.
|
||||
- `blocked` → orange inline panel with the reason; OK button hidden, only Close remains.
|
||||
|
||||
**Wiring:**
|
||||
- `DetailsIslandViewModel.ApproveMergeAsync` opens `MergeModalView` (factory injected through `MainWindowViewModel`'s existing dialog pattern).
|
||||
- `DiffModalView` gains a Merge button in its command strip; click opens the same modal with the current task's id.
|
||||
- Both entry points are only visible/enabled when `Task.Worktree?.State == Active` (same predicate as the existing Reset/Continue visibility logic — extend `ShowFailedActions`-style gating with a new flag `CanMerge`).
|
||||
|
||||
`MergeModalViewModel` depends only on `WorkerClient`. It does not touch `GitService` directly — all git access stays worker-side.
|
||||
|
||||
## Data flow
|
||||
|
||||
```
|
||||
User clicks Merge (Details island or DiffModal)
|
||||
→ DetailsIslandViewModel / DiffModalViewModel opens MergeModalView
|
||||
→ MergeModalViewModel.InitializeAsync
|
||||
→ WorkerClient.GetMergeTargetsAsync(taskId)
|
||||
→ Hub.GetMergeTargets
|
||||
→ TaskMergeService.GetTargetsAsync
|
||||
→ GitService.GetCurrentBranchAsync + ListLocalBranchesAsync
|
||||
→ Combobox populated, default selected
|
||||
User edits fields, clicks OK
|
||||
→ WorkerClient.MergeTaskAsync(taskId, branch, remove, msg)
|
||||
→ Hub.MergeTask
|
||||
→ TaskMergeService.MergeAsync
|
||||
→ pre-flight checks
|
||||
→ GitService.MergeNoFfAsync → (success | conflict)
|
||||
→ on success: optional remove + branch delete, SetState(Merged), broadcast
|
||||
→ on conflict: MergeAbortAsync, return conflict DTO
|
||||
→ MergeModalViewModel renders result
|
||||
```
|
||||
|
||||
## Error handling
|
||||
|
||||
| Case | Surfaced as |
|
||||
| --- | --- |
|
||||
| Task running | `MergeResult("blocked", [], "task is running")` |
|
||||
| Worktree not Active | `("blocked", [], "worktree state is {state}")` |
|
||||
| Working dir dirty | `("blocked", [], "target branch has uncommitted changes")` |
|
||||
| Target mid-merge | `("blocked", [], "target branch is mid-merge")` |
|
||||
| `list.WorkingDir` null | `("blocked", [], "list has no working directory")` |
|
||||
| Merge conflict | `("conflict", [files], null)` — target auto-restored |
|
||||
| Unknown git failure | `HubException` with stderr |
|
||||
| Post-merge cleanup fails | Log a warning; merge already succeeded, state already `Merged`. Return `("merged", [], null)` with a note in `ErrorMessage`. |
|
||||
|
||||
## Testing
|
||||
|
||||
`tests/ClaudeDo.Worker.Tests/TaskMergeServiceTests.cs` (real SQLite + real git, matching existing test conventions):
|
||||
|
||||
1. Happy path, ff-able history → one merge commit, state Merged, broadcast fired.
|
||||
2. Happy path, diverged non-conflicting → merge commit created.
|
||||
3. Conflict path → conflicted files returned, target branch HEAD matches pre-merge, `MERGE_HEAD` absent, worktree state still Active.
|
||||
4. Pre-flight: worktree Merged/Discarded → blocked.
|
||||
5. Pre-flight: dirty working tree → blocked.
|
||||
6. Pre-flight: mid-merge target → blocked.
|
||||
7. `removeWorktree=true` → worktree dir gone, branch deleted, state Merged.
|
||||
8. `removeWorktree=false` → worktree + branch survive, state Merged.
|
||||
9. Task Running → blocked.
|
||||
|
||||
`tests/ClaudeDo.Worker.Tests/GitServiceMergeTests.cs` (narrow tests for new GitService methods): `MergeNoFfAsync` success/conflict tuple semantics, `MergeAbortAsync` clears MERGE_HEAD, `IsMidMergeAsync` true/false, `ListLocalBranchesAsync` returns expected set, `GetCurrentBranchAsync` on fresh repo.
|
||||
|
||||
Manual UI checklist captured in the implementation plan, not automated.
|
||||
|
||||
## Implementation order (sketch)
|
||||
|
||||
1. GitService additions + their tests.
|
||||
2. `TaskMergeService` + its tests (hub/UI not yet wired).
|
||||
3. Hub methods + `WorkerClient` methods.
|
||||
4. `MergeModalView` + `MergeModalViewModel`.
|
||||
5. Wire `DetailsIslandViewModel.ApproveMergeAsync`.
|
||||
6. Wire DiffModal Merge button.
|
||||
7. Manual UI walkthrough against the checklist.
|
||||
|
||||
## Open items
|
||||
|
||||
None.
|
||||
Reference in New Issue
Block a user