Files

T

mika kuns e59f6c2438 fix(worker): surface empty review ranges and blocked children over MCP

preview_merge/preview_merge_set now report isEmpty (ahead==0, or
HandlerBaseCommit==HandlerHeadCommit for a worktree-less list-handler
task) instead of leaving an empty branch indistinguishable from a small
one. preview_merge also stops throwing for worktree-less handler tasks,
falling back to their fixed commit range. review_task's parent approve
returns emptyChildren, naming the Done children whose review range
contributed nothing before the unit merge lands. TaskRefDto/TaskDto now
expose roadblockCount so a CLAUDEDO_BLOCKED child is identifiable over
MCP, since it still reaches Done per the unified parent model.

2026-08-06 11:54:46 +02:00

11 KiB

Raw Blame History

Review, merge & conflict resolution

Explore-note — verify before trusting. Distilled map of a subsystem, not authoritative. Last verified against commit 8247a74 (2026-08-06). Drift check: git log --oneline 8247a74..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.

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.

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; Flatten injects file-header rows for a combined single-pane view. 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 + flat diff right, combined integration-branch toggle.

DiffLinesView renders per-file content with binary/empty placeholders.

11 KiB Raw Blame History Unescape Escape