ClaudeDo/docs/explore-notes/review-merge.md

Review, merge & conflict resolution

Explore-note — verify before trusting. Distilled map of a subsystem, not authoritative. Last verified against 2f3f938 (2026-08-07), which finished the diff-viewer rework: Planning mode renders per file and DiffLinesView is retired. Drift check: git log --oneline 20bce9b..HEAD -- src/ClaudeDo.Worker/Lifecycle src/ClaudeDo.Worker/State src/ClaudeDo.Worker/Planning src/ClaudeDo.Ui/ViewModels/Conflicts src/ClaudeDo.Worker/External Stable structure only (no line numbers). See docs/explore-notes/README.md.

Covers the review→merge path: TaskStateService review transitions, TaskMergeService, PlanningMergeOrchestrator, the post-merge verify gate, and the UI conflict resolver.

Approve = merge the whole unit

ApproveReview (hub) and review_task approve (MCP) are the single review+merge action. There is no separate "Merge all" entry.

• Task with children → drives PlanningMergeOrchestrator: merges the parent worktree if Active, then each Done child in order, then sets the parent Done. A mid-merge conflict pauses for ContinuePlanningMerge / AbortPlanningMerge.
• Childless task → TaskMergeService.ApproveAndMergeAsync. A conflict keeps the task in WaitingForReview.
• No active worktree (sandbox run) → straight to Done.

Review transitions all live in TaskStateService: SubmitForReviewAsync, SubmitForChildrenAsync, ApproveReviewAsync, RejectToQueueAsync, RejectToIdleAsync, ClearReviewFeedbackAsync.

ReviewFeedback (nullable string on TaskEntity) is the reviewer's rejection comment: set by RejectToQueueAsync, consumed and cleared by QueueService on the next re-run, where it becomes the next-turn prompt of the resumed Claude session.

Unified parent model

Every parent — planning or improvement — flows … → WaitingForChildren → WaitingForReview → Done, advanced by the single TaskStateService.TryAdvanceParentAsync. It surfaces any WaitingForChildren parent for review once all children are terminal; failed/cancelled children are annotated on the result, not wedged.

• A planning parent enters WaitingForChildren at FinalizePlanningAsync (or WaitingForReview directly if it has no children).
• An improvement parent enters it from TaskRunner.HandleSuccess when its run spawned children.
• Planning/improvement children go straight to Done — no individual review. Only the parent is reviewed.

A child that hits a roadblock (fails, or reports CLAUDEDO_BLOCKED roadblocks) does not advance the parent — the parent stays in WaitingForChildren until every child is terminal. The UI surfaces blocked children on the parent's Session tab (ChildOutcomes + a "children need attention" band) so the roadblock is visible without forcing a transition.

A blocked planning/improvement child still goes straight to Done per the unified parent model above — it committed nothing, but nothing prevents its (empty) branch from being unit-merged like any other Done child once the parent is approved. The MCP surface has no UI equivalent of ChildOutcomes, so review_task's approve on a parent additionally returns emptyChildren (the Done children whose review range is empty) and every task-returning tool exposes TaskRefDto.roadblockCount → external-mcp.md → ReviewTask/PreviewMerge. An empty branch is still mergeable by design (some tasks — e.g. an audit — legitimately produce no diff); this is a visibility fix, not a merge gate.

Cancel is blocked while a unit merge is draining

ApproveReview on a task with children awaits PlanningMergeOrchestrator.StartAsync / DrainAsync synchronously — the parent sits in WaitingForReview for the whole (potentially minutes-long, one-child-at-a-time) drain. Without a guard, a concurrent CancelReview (UI or update_task_status/cancel_task via MCP) could flip the parent to Cancelled mid-drain while the orchestrator kept merging children's worktrees onto the target branch; FinalizeParentDoneAsync then finds the parent no longer WaitingForReview and gives up, leaving the merged children's diffs stranded with no rollback.

TaskStateService.CancelAsync now rejects with a TransitionResult reason whenever PlanningMergeOrchestrator.HasActiveMerge(taskId) is true for the task being cancelled, before any DB write. Cycle note: TaskStateService can't take a direct constructor dependency on PlanningMergeOrchestrator (which itself depends on ITaskStateService), so it takes a lazily-resolved Func<IActiveMergeState> instead — same cycle-breaking shape as the existing Func<ITaskStateService> handed to PlanningChainCoordinator. IActiveMergeState (Planning/Interfaces/) is PlanningMergeOrchestrator's only public surface TaskStateService needs.

The UI mirrors this as polish: DetailsIslandViewModel.IsMergeDraining (set on PlanningMergeStartedEvent, cleared on PlanningMergeAborted/PlanningCompleted for the bound task) gates CancelReviewCommand's CanExecute — same shape as WorktreesOverviewModalViewModel.IsMerging gating CanMergeAll. The worker-side guard remains the actual correctness fix; the button gate just avoids inviting a click the worker would reject. CancelReviewAsync's catch now raises ErrorReported (→ shell FlashFooterError) instead of swallowing the rejection silently.

Post-merge verify gate

A list can set ListConfigEntity.VerifyCommand (List Settings modal → Verification). Null/blank (the default) = no gate, behavior bit-identical to before the feature existed.

When set, TaskMergeService runs it via VerifyCommandRunner (cmd.exe /c <command>, 10-minute fixed timeout, output tail-captured) in list.WorkingDir right after a successful MergeNoFfAsync / ContinueMergeAsync and worktree cleanup, but before the task is allowed to reach Done.

Outcome	Effect
Exit 0	Unchanged flow — worktree Merged, task Done if it was WaitingForReview.
Non-zero exit or timeout	The git merge is deliberately left in place (no auto-revert — that's a separate, unbuilt feature). The worktree is still marked Merged (it's already gone from disk when removeWorktree was requested), but the task stays out of Done.

On failure MergeResult.Status comes back TaskMergeService.StatusVerifyFailed ("verify_failed") with an output excerpt in ErrorMessage. This flows through MergeResultDto (hub) and ReviewTaskResult (review_task) unchanged, because both already treat any non-blocked/conflict status generically. All three UI merge entry points handle it explicitly (detail-pane Approve, merge modal, worktrees batch) — a generic fallback there showed the raw status string instead of the failure.

Worktree-less approvals are gated too. A task with no active WorktreeEntity — a sandbox run, or a list-handler task that commits straight into list.WorkingDir — skips the merge entirely, but ApproveAndMergeAsync still runs the verify command (same per-repo gate, working dir = list.WorkingDir) before the task may reach Done. Without that, the run that lands the most on the target branch at once would be the one run nothing checks.

Serialization: a process-wide ConcurrentDictionary<string, SemaphoreSlim> keyed by list.WorkingDir serializes MergeAsync / ContinueMergeAsync (git ops + verify) per repo, so a verify run can't be interrupted by a second merge landing in the same working dir mid-build.

MergeCommit and revert

WorktreeEntity.MergeCommit (nullable) is the SHA of the merge commit this worktree's branch produced on the target branch. Stamped by TaskMergeService the moment a merge/continue-merge succeeds, written only by WorktreeRepository.SetMergedAsync (which atomically sets State=Merged and stamps the SHA in one update).

It is the only thing that makes revert_merge possible without heuristically searching git log — see external-mcp.md → RevertMerge. Null for any worktree merged before the field existed.

Review gate in the UI

Approve & Merge is gated behind opening the diff. When there is something to inspect (worktree diff / merged range / children combined diff), the button stays disabled until the diff or combined-diff viewer has been opened once. The gate re-locks per run — any state change resets it. Tasks with nothing to inspect are never gated.

The row-level quick-approve in the task list is an intentional bypass.

Implementation: MergeSectionViewModel owns merge-target selection, the mergeability indicator (MergePreviewPresenter over PreviewMergeAsync), and OpenDiffAsync / ReviewCombinedDiffCommand — both build a DiffViewerViewModel, call ShowDiffViewer, and fire the DiffViewed callback. HasReviewableDiff reports whether anything is inspectable and feeds the gate.

Conflict resolver (in-app Rider-style 3-pane merge editor)

ConflictResolverViewModel + Views/Conflicts/ConflictResolverView. Handles both single-task and planning unit-merge conflicts.

Model

Single-task mode starts the conflict merge, then parses each conflicted file into stable and conflict MergeFileSegments via the worker's GetMergeConflictDocuments. Types live in ConflictModels: MergeFile / MergeFileSegment / MergeConflictBlock.

Exposed per active file: ActiveOursText / ActiveResultText / ActiveTheirsText (reconstructed from MergeFile.OursText/ResultText/TheirsText; Result seeds unresolved conflicts with Ours), plus ActiveFile / SelectFileCommand (multi-file switcher), Current / Next / Previous (focused-conflict nav), a per-file PositionText readout, per-block AcceptOurs/Theirs/Both/Base + MergeFile.Compose, and CanContinue gated on every file resolved + no binary. Each file is written via WriteConflictResolution.

Planning mode via OpenForPlanningAsync(parentId, subtaskId) loads the current subtask's mid-merge conflicts without re-starting the merge and routes continue/abort to ContinuePlanningMerge / AbortPlanningMerge, so a unit-merge conflict re-opens the editor per subtask via the PlanningMergeConflict broadcast.

Except when an MCP session is driving the merge. PlanningMergeOrchestrator.StartAsync takes an externallyDriven bool (default false; ExternalMcpService.review_task's parent-with-children path passes true, since a running Claude session — not the UI — will resolve conflicts via continue_merge/abort_merge). The flag rides along on the PlanningMergeConflict broadcast (4th arg); IslandsShellViewModel.OnPlanningMergeConflict only auto-opens the resolver when it's false — otherwise it shows a persistent, non-auto-dismissing banner (IsExternalMergeBannerVisible) with a manual "Open resolver" button, cleared on PlanningMergeAborted/PlanningCompleted. This exists because two parties (a human and the driving session) could otherwise end up editing the same shared checkout at once. PlanningMergeOrchestrator.GetActiveExternalConflictsAsync (hub: GetActiveExternalPlanningMergeConflicts) re-derives this state by checking GitService.IsMidMergeAsync rather than trusting the in-memory flag alone, so a UI restart mid-merge (or a stale entry left behind if something resolved the repo outside the normal Continue/Abort path) can't show a phantom banner — the Ui calls it on ConnectionRestoredEvent. The childless single-task conflict path (TaskMergeService.ApproveAndMergeAsync) has no equivalent broadcast — it only sends the generic TaskUpdated — so it never auto-opened the resolver and needed no change.

View

Three AvaloniaEdit panes showing the whole file: MAIN/ours (read-only) | editable Result | INCOMING/theirs (read-only). TextMate highlighting by extension (theme StyleInclude in App.axaml).

• A code-behind IBackgroundRenderer tints each conflict block (unresolved/resolved) across panes. Tints live in Tokens.axaml (Merge*TintBrush).
• An IReadOnlySectionProvider + TextAnchor regions keep only conflict spans editable in Result; edits flow back to the block.
• Each unresolved conflict starts EMPTY (a thin marker bar).
• The between-pane gutter controls toggle each side in/out of the result: ›/‹ add MAIN/INCOMING in click order (first pick on top), clicking again removes that side — so a conflict can take main, incoming, both, or neither.
• FilesSummary shows how many files still have conflicts. The three panes share a proportional synced vertical scroll.
• A conflict overview ruler right of the Result pane (ConflictMap) maps every conflict in the file proportionally; click a tick to jump. Useful for long files.

Entry points

Review Approve on conflict, and the Merge button in the Diff window (a conflicting MergeTask hands off via RequestConflictResolution).

Hub methods

• Review/merge: ApproveReview(taskId, targetBranch) -> MergeResultDto, ContinuePlanningMerge / AbortPlanningMerge, PreviewMerge(taskId, targetBranch) -> MergePreviewDto, RejectReviewToQueue, RejectReviewToIdle, CancelReview, MergeTask, GetMergeTargets
• Single-task conflict resolver: StartConflictMerge, GetMergeConflictDocuments, WriteConflictResolution, ContinueConflictMerge, AbortConflictMerge — note the service-level TaskMergeService.ContinueMergeAsync / AbortMergeAsync keep their own names.
• Broadcast events: PlanningMergeStarted, PlanningSubtaskMerged, PlanningMergeConflict, PlanningMergeAborted, PlanningCompleted

Diff stack (UI)

UnifiedDiffParser (static) parses git diff output into DiffFileViewModels, detecting added/deleted/renamed/binary files and per-line numbers. DiffModels.cs holds the shared types (DiffLineViewModel, DiffFileViewModel, DiffLineKind, DiffFileStatus, SubtaskDiffRow, DiffTreeNodeViewModel, DiffTree).

DiffViewerViewModel is one unified read-only viewer with two modes:

• Files — dirty worktree / branch-vs-base / commit-range. Loads via GitService, folder file-tree left + per-file diff pane right, Merge button for a live branch source.
• Planning — per-subtask diffs via GetPlanningAggregateAsync, subtask list left + one editor per file right (PlanningFiles), combined integration-branch toggle.

Both modes render through DiffAlignment (pure — pairs diff lines into side-by-side rows and computes word-diff spans) and DiffTextView (AvaloniaEdit + TextMate highlighting keyed off the file extension, unified/split layout, optional line wrap, synced scrolling). Files mode hosts one DiffTextView for the selected file; Planning mode hosts one per file in PlanningFiles so each gets its own grammar (one editor can only carry one TextMate grammar). The split/wrap toggles persist to ui.config.json via AppSettings and reach the per-file editors in Planning mode too.