Covers subtask visibility fix, aggregated diff viewer, and single Merge-all action with VS-Code-assisted conflict resolution. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
198 lines
14 KiB
Markdown
198 lines
14 KiB
Markdown
# Planning Merge-All & Subtask Visibility — Design
|
||
|
||
**Date:** 2026-04-24
|
||
**Status:** Approved design, ready for implementation planning
|
||
|
||
## Problem
|
||
|
||
Three concrete issues with the current Planning feature:
|
||
|
||
1. **Queued subtasks are not visible in the Queue List.** When a planning session finalizes, its subtasks transition to `Queued`, but the Queue List's hierarchy rules only show children when their Planning parent is expanded. A collapsed (or already-`Planned`) parent effectively hides the subtasks.
|
||
2. **Completed subtasks vanish from view.** Once a subtask becomes `Done`, the regroup logic moves it to the "Completed" bucket. Users expect subtasks to remain visible under their Planning parent until the Planning task itself is marked Done.
|
||
3. **No aggregated view or bulk merge.** Each subtask must be merged individually through its worktree. There is no way to see a combined diff of all changes produced by a Planning session, and no "merge everything" action.
|
||
|
||
## Goals
|
||
|
||
- Treat Planning subtasks as belonging to their Planning parent for visibility and lifecycle purposes.
|
||
- Provide a single aggregated diff view that shows all changes produced by a Planning session.
|
||
- Provide a single "Merge all" action that sequentially merges all subtasks, with a usable conflict-resolution flow.
|
||
- Auto-complete the Planning task when all merges succeed.
|
||
|
||
## Non-goals
|
||
|
||
- Building a full-featured in-app diff editor. Textual unified diff is acceptable for now; conflict *editing* happens in VS Code.
|
||
- Persisting Merge-all progress across worker restarts. Restart clears in-memory orchestration state; user re-starts Merge-all (already-merged subtasks are skipped because their worktrees are `Merged`).
|
||
- Modifying how individual subtasks are created, executed, or finalized.
|
||
|
||
## Design
|
||
|
||
### 1. Visibility model
|
||
|
||
Planning subtasks are exclusively children of their Planning parent until the Planning task transitions to `Done`. The Planning parent acts as a roll-up in the Queue List.
|
||
|
||
- Tasks with a non-null `ParentTaskId` are excluded from all virtual lists (`virtual:queued`, `virtual:running`, `CompletedItems`, etc.) as separate rows.
|
||
- A Planning/Planned task is included in `virtual:queued` if **any** child is `Queued`, and in `virtual:running` if any child is `Running`.
|
||
- Children are always attached under their parent in the task tree; expansion purely controls visual collapse.
|
||
- When Merge-all completes successfully, the Planning task is set to `Done` and the entire subtree moves to Completed together.
|
||
- Status badge on the Planning row summarizes children (e.g., `3/5 queued`, `2 running`, `1 failed`).
|
||
|
||
### 2. Planning detail panel
|
||
|
||
Extends the existing task detail view. New elements when the selected task is a Planning task:
|
||
|
||
- **Subtasks list.** Grouped by status badge (Queued / Running / Done / Failed). Each row preserves existing per-subtask actions (view logs, open worktree, individual merge).
|
||
- **Merge target dropdown.** Single target branch that applies to all subtasks in Merge-all. Defaults to the branch that was current when the Planning session started.
|
||
- **`[Review combined diff]` button.** Opens the Aggregated Diff Viewer. Enabled as soon as any subtask has produced a diff.
|
||
- **`[Merge all subtasks]` button.** Orchestrates sequential merge + auto-Done. Disabled until every subtask is `Done` and every worktree is `Active` or `Merged` (no `Discarded` / `Kept`). Tooltip explains why when disabled (e.g., "2 subtasks still running", "1 subtask failed — resolve first", "1 worktree was discarded").
|
||
- Existing per-subtask merge action remains available; Merge-all is additive.
|
||
|
||
### 3. Aggregated diff viewer
|
||
|
||
New Avalonia view `PlanningDiffView` + `PlanningDiffViewModel`, opened as a modal or dedicated tab.
|
||
|
||
**Default — grouped by subtask:**
|
||
- Left pane: subtask list in creation order with `title • +added −deleted • N files`.
|
||
- Right pane: selected subtask's diff. Reuse any existing diff-rendering control; if none exists, render unified diff text with basic syntax coloring (monospace, minimal decoration).
|
||
- Summary stats come from `WorktreeEntity.DiffStat`. Raw diff comes from `git diff <base>..<head>` executed in each subtask's worktree via `GitService`. Cached in memory per subtask until the subtask's HEAD moves.
|
||
|
||
**Toggle — "Preview combined diff":**
|
||
- Calls `PlanningAggregator.BuildIntegrationBranchAsync(planningTaskId, targetBranch, ct)`:
|
||
1. Create/reset branch `planning/<slug>-integration` off the current merge target.
|
||
2. Merge each subtask's branch sequentially with `--no-ff`.
|
||
3. On conflict during preview: abort the merge, reset the integration branch, surface a warning identifying which two subtasks conflict. Grouped view remains available.
|
||
4. On success: compute `git diff <merge-target>..planning/<slug>-integration` and render as a single flat unified diff.
|
||
- Toggle flips back to grouped mode.
|
||
|
||
**Integration-branch lifecycle:** scratch artifact, rebuilt on every preview (deleted + recreated). Cleaned up when the Planning task is marked `Done` or when the Planning session is discarded.
|
||
|
||
### 4. Merge-all orchestration
|
||
|
||
**Happy path (`PlanningMergeOrchestrator.StartAsync`):**
|
||
|
||
1. Pre-flight checks — fail fast with a clear message on any:
|
||
- Every subtask is `Done`.
|
||
- Every subtask's worktree is `Active` or `Merged` (no `Discarded` / `Kept`). `Merged` worktrees are allowed so that an interrupted Merge-all can be restarted.
|
||
- Repo working tree is clean.
|
||
- No mid-merge in progress in the target repo.
|
||
2. For each subtask in creation order, skip if its worktree is already `Merged` (idempotent restart). Otherwise call `TaskMergeService.MergeAsync` with `removeWorktree: true` and `leaveConflictsInTree: true`. Each success flips the worktree to `Merged`.
|
||
3. After the last successful merge:
|
||
- Set Planning task `Status = Done`.
|
||
- Call `PlanningAggregator.CleanupIntegrationBranchAsync` if the integration branch exists.
|
||
- Emit `PlanningCompleted` so the UI removes the row from the Queue List.
|
||
|
||
**Conflict path:**
|
||
|
||
1. `MergeAsync` with `leaveConflictsInTree: true` reports a conflict, leaves the repo in a mid-merge state, and returns the conflicted file paths (`git diff --name-only --diff-filter=U`).
|
||
2. Orchestrator halts the loop, stores the in-progress state (remaining subtasks, target branch, current subtask id) in memory, and emits `PlanningMergeConflict(planningTaskId, subtaskId, conflictedFiles)`.
|
||
3. The UI opens the **Conflict Resolution dialog** — see §5.
|
||
4. On `ContinueAsync`: calls `TaskMergeService.ContinueMergeAsync(subtaskId)` which stages the recorded files and runs `git commit --no-edit`. Flips worktree to `Merged`. Loop resumes with remaining subtasks.
|
||
5. On `AbortAsync`: calls `TaskMergeService.AbortMergeAsync(subtaskId)` which runs `git merge --abort`. Planning stays in `Planned`. Already-merged earlier subtasks remain `Merged`. Orchestration state cleared.
|
||
|
||
**Idempotent restart:** if the worker restarts mid Merge-all, in-memory state is lost. A fresh `StartAsync` re-runs pre-flight; already-`Merged` worktrees are skipped by the loop (their status gates them out). User experience: "I clicked Merge all again and it continued from where it left off."
|
||
|
||
### 5. Conflict Resolution dialog
|
||
|
||
Avalonia modal (`ConflictResolutionView` + `ConflictResolutionViewModel`).
|
||
|
||
- **Header:** `Conflicts in subtask: <title> merging into <target-branch>`.
|
||
- **File list:** full absolute paths of conflicted files.
|
||
- **`[Open all in VS Code]`** — for each file, spawn `code <absolute-path>` via `Process.Start`. If `code` is not on PATH, show an inline error row with the file list so the user can copy paths manually. No popup-on-popup.
|
||
- **`[I've resolved — continue]`** — calls `ContinuePlanningMerge(planningTaskId)` hub method, closes dialog. The orchestration loop continues with the remaining subtasks.
|
||
- **`[Abort this merge]`** — calls `AbortPlanningMerge(planningTaskId)` hub method, closes dialog. Planning stays `Planned`.
|
||
|
||
### 6. Data model
|
||
|
||
**No schema changes.**
|
||
- Conflicted files are queried from git on demand (`git diff --name-only --diff-filter=U`) while the merge is in progress.
|
||
- Integration branch name is derived from the Planning task slug: `planning/<slug>-integration`.
|
||
- Planning completion uses existing `TaskStatus.Done`.
|
||
|
||
### 7. Services
|
||
|
||
**New:**
|
||
|
||
- **`PlanningAggregator`** (`src/ClaudeDo.Worker/Planning/PlanningAggregator.cs`)
|
||
- `GetAggregatedDiffAsync(planningTaskId, ct)` — returns per-subtask diff entries.
|
||
- `BuildIntegrationBranchAsync(planningTaskId, targetBranch, ct)` — creates/resets the integration branch, merges subtasks sequentially, returns `(success, combinedDiff)` or `(failure, firstConflictSubtaskId, conflictedFiles)`. Always leaves the integration branch in a consistent state (aborts + resets on failure).
|
||
- `CleanupIntegrationBranchAsync(planningTaskId, ct)` — deletes the integration branch.
|
||
|
||
- **`PlanningMergeOrchestrator`** (singleton, `src/ClaudeDo.Worker/Planning/PlanningMergeOrchestrator.cs`)
|
||
- Owns in-memory state per planning task: `{ remainingSubtasks, targetBranch, currentSubtaskId }`.
|
||
- `StartAsync(planningTaskId, targetBranch)`, `ContinueAsync(planningTaskId)`, `AbortAsync(planningTaskId)`.
|
||
- Emits SignalR events: `PlanningMergeStarted`, `PlanningSubtaskMerged`, `PlanningMergeConflict`, `PlanningMergeAborted`, `PlanningCompleted`.
|
||
|
||
**Modified:**
|
||
|
||
- **`TaskMergeService`**
|
||
- `MergeAsync` gets a `leaveConflictsInTree: bool` parameter (default `false`). When `true`, on conflict records conflicted files on the returned result, does **not** call `git merge --abort`.
|
||
- New `ContinueMergeAsync(taskId, ct)` — stages the recorded conflicted files and runs `git commit --no-edit`, flips worktree to `Merged`.
|
||
- New `AbortMergeAsync(taskId, ct)` — runs `git merge --abort`, restores pre-merge state.
|
||
- Existing callers unaffected by the default.
|
||
|
||
- **`WorkerHub`** — new methods:
|
||
- `GetPlanningAggregate(planningTaskId)`
|
||
- `BuildPlanningIntegrationBranch(planningTaskId, targetBranch)`
|
||
- `MergeAllPlanning(planningTaskId, targetBranch)`
|
||
- `ContinuePlanningMerge(planningTaskId)`
|
||
- `AbortPlanningMerge(planningTaskId)`
|
||
|
||
- **`TasksIslandViewModel.Regroup`**
|
||
- Exclude tasks with `ParentTaskId != null` from virtual lists.
|
||
- Include Planning parents in `virtual:queued` / `virtual:running` based on children's statuses.
|
||
- Keep children attached to parent in the tree at all times until Planning is `Done`.
|
||
|
||
### 8. UI components (new)
|
||
|
||
- `PlanningDiffView` + `PlanningDiffViewModel` — aggregated diff viewer (§3).
|
||
- `ConflictResolutionView` + `ConflictResolutionViewModel` — conflict dialog (§5).
|
||
- Planning Detail section inside the existing task detail pane — subtask list + merge target dropdown + two buttons (§2).
|
||
|
||
## Error handling
|
||
|
||
- **Pre-flight failures** — surface as inline errors in the Planning detail panel. No merge work attempted.
|
||
- **Preview-build conflict** — keep grouped diff available; show a warning banner identifying the conflicting pair of subtasks.
|
||
- **Merge-all conflict** — Conflict Resolution dialog (§5). The failed subtask's worktree stays `Active`; prior successes stay `Merged`.
|
||
- **VS Code not on PATH** — inline error row in the Conflict dialog with copyable file paths.
|
||
- **Worker restart mid-merge** — in-memory state lost; restarting Merge-all is idempotent because merged worktrees are skipped by status gating.
|
||
|
||
## Testing
|
||
|
||
Convention: xUnit integration tests with real SQLite and real git (`tests/ClaudeDo.Worker.Tests`).
|
||
|
||
**`PlanningAggregatorTests`** — real git fixture
|
||
- `GetAggregatedDiffAsync` returns one entry per subtask with correct stats.
|
||
- `BuildIntegrationBranchAsync` with non-conflicting subtasks — success, branch contains all changes.
|
||
- `BuildIntegrationBranchAsync` with conflicting subtasks — failure, branch reset (not mid-merge), correct subtask id and file list reported.
|
||
- Rebuild overwrites a stale integration branch.
|
||
- `CleanupIntegrationBranchAsync` removes the branch.
|
||
|
||
**`PlanningMergeOrchestratorTests`** — real git + real DB
|
||
- Happy path: all subtasks merge → worktrees `Merged`, Planning `Done`, `PlanningCompleted` emitted.
|
||
- Conflict path: first subtask conflicts → repo left in conflict state, `PlanningMergeConflict` emitted with correct file list, worktree stays `Active`, Planning stays `Planned`.
|
||
- `ContinueAsync` after conflict: resolution commits, loop proceeds, final state `Done`.
|
||
- `AbortAsync` after conflict: `merge --abort` restores clean state, earlier merged subtasks remain `Merged`, Planning stays `Planned`.
|
||
- Pre-flight rejection: running subtask, failed subtask, dirty repo — each returns the expected error with no side effects.
|
||
- Idempotent restart: partial merge + fresh `StartAsync` — already-`Merged` worktrees skipped.
|
||
|
||
**`TaskMergeServiceConflictTests`** (extending existing tests)
|
||
- `MergeAsync(leaveConflictsInTree: true)` on conflict: no `merge --abort`, returns conflicted files, worktree state unchanged.
|
||
- `ContinueMergeAsync`: completes in-progress merge, flips worktree to `Merged`.
|
||
- `AbortMergeAsync`: runs `merge --abort`, restores clean state.
|
||
|
||
**`TasksIslandRegroupTests`** — ViewModel unit tests, no DB
|
||
- Queued subtask with a Planning parent is NOT in `virtual:queued` as its own row.
|
||
- Planning parent with any Queued child IS in `virtual:queued`.
|
||
- Done subtask stays nested under Planning parent until Planning is `Done`.
|
||
- After Planning is marked `Done`, parent + children move to Completed together.
|
||
|
||
**Manual smoke test** (documented in PR description):
|
||
- End-to-end planning session in the app: create plan, finalize, let subtasks run.
|
||
- Open aggregated diff, toggle Preview combined.
|
||
- Merge-all happy path.
|
||
- Merge-all conflict path with VS Code dialog open/continue.
|
||
- Merge-all conflict path abort.
|
||
|
||
## Open questions
|
||
|
||
None at this stage. All decisions from the brainstorming session are captured above.
|