From 823368484e6d27d6abb714a7fc3d57132af8a254 Mon Sep 17 00:00:00 2001 From: mika kuns Date: Thu, 1 Oct 2026 11:26:17 +0200 Subject: [PATCH] docs(worker): document unit branch, child absorb and legacy approve routing --- docs/explore-notes/review-merge.md | 23 +++++++++++++++-------- src/ClaudeDo.Worker/CLAUDE.md | 20 +++++++++++++++++++- 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/docs/explore-notes/review-merge.md b/docs/explore-notes/review-merge.md index 2351dfc1..f2c28f70 100644 --- a/docs/explore-notes/review-merge.md +++ b/docs/explore-notes/review-merge.md @@ -1,8 +1,8 @@ # Review, merge & conflict resolution > **Explore-note — verify before trusting.** Distilled map of a subsystem, not authoritative. -> Last verified against `168ab1cd` (2026-08-26), which added the batch verify -> (`skipVerify` + `verify_merges`). +> Last verified against `44d94caa` (2026-10-01), which made the parent's worktree the unit +> branch (children fast-forwarded into it). > 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. @@ -15,11 +15,17 @@ Covers the review→merge path: `TaskStateService` review transitions, `TaskMerg `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`. +- **Legacy unit** (`UnitBranchService.HasUnabsorbedChildrenAsync`: a `Done` child whose worktree + is still `Active`, i.e. planned before the unit branch existed) → drives + `PlanningMergeOrchestrator`: merges the parent worktree if `Active` (planning parents too, for a + mixed unit), then each `Done` child in order, then sets the parent `Done`. A mid-merge conflict + pauses for `ContinuePlanningMerge` / `AbortPlanningMerge`. Children in `WaitingForReview` are + silently skipped there. +- **Everything else** — a childless task, or a parent whose children were absorbed into its unit + branch → `TaskMergeService.ApproveAndMergeAsync` on the task's own worktree. A conflict keeps + the task in `WaitingForReview`. +- **Child of a unit** (parent has an `Active` worktree) → `ApproveAndMergeAsync` absorbs it into + the unit branch (`UnitBranchService.AbsorbChildAsync`, ff-only) instead of merging to the target. - **No active worktree** (sandbox run) → straight to `Done`. Review transitions all live in `TaskStateService`: `SubmitForReviewAsync`, @@ -203,7 +209,8 @@ succeeds, written **only** by `WorktreeRepository.SetMergedAsync` (which atomica It is the only thing that makes `revert_merge` possible without heuristically searching `git log` — see [external-mcp.md](external-mcp.md) → `RevertMerge`. Null for any worktree -merged before the field existed. +merged before the field existed, and for a child absorbed into its unit branch (a fast-forward +has no merge commit) — such a child is reverted with its whole unit, never alone. ## Review gate in the UI diff --git a/src/ClaudeDo.Worker/CLAUDE.md b/src/ClaudeDo.Worker/CLAUDE.md index 7293329e..fa1735bf 100644 --- a/src/ClaudeDo.Worker/CLAUDE.md +++ b/src/ClaudeDo.Worker/CLAUDE.md @@ -96,6 +96,19 @@ nothing to merge; a worktree task's branch just stays Active for the worktrees o `… → WaitingForChildren → WaitingForReview → Done` via the single `TryAdvanceParentAsync`. Planning/improvement **children** go straight to `Done`; only the parent is reviewed. +**Unit branch.** The parent's own worktree is the unit branch (improvement parents have it from +their run, planning parents get it in `QueuePlanAsync` via `UnitBranchService.EnsureUnitWorktreeAsync`). +Children fork from its current head and `UnitBranchService.AbsorbChildAsync` fast-forwards it to +the child (`merge --ff-only` in the parent worktree) — from `TaskRunner.HandleSuccess` right before +`CompleteAsync`, and from `TaskMergeService.ApproveAndMergeAsync` for an interactive child in +review. The child row goes `Merged` with `MergeCommit` null (so `revert_merge` only works on the +whole unit), its worktree and branch are removed, the parent row's `HeadCommit`/`DiffStat` move +along. A child that can't fast-forward (e.g. started in parallel via Run now) is `Failed` — no +merge fallback; in the approve path it stays `WaitingForReview` with a `blocked` result. +`UnitBranchService` never writes task status (it would close a DI cycle through +`PlanningChainCoordinator`). ⚠️ `QueuePicker` counts `worktrees.state='merged'` as a satisfied +dependency, so an absorbed child satisfies a `dependsOn` edge although nothing reached `main` yet. + **Approve = merge the whole unit.** `ApproveReview` / `review_task` approve is the single review+merge action — there is no separate "Merge all". Details, plus the post-merge verify gate and the conflict resolver → [review-merge](../../docs/explore-notes/review-merge.md). @@ -108,6 +121,11 @@ gate and the conflict resolver → [review-merge](../../docs/explore-notes/revie 2. `PlanningChainCoordinator.SetupChainAsync(parent, enqueue: false)` establishes the blocked-by chain (child[i] → child[i-1]) but **leaves children `Idle`** — finalize never auto-queues. Queueing is a deliberate user action: `QueuePlanAsync` (hub `QueuePlanningSubtasksAsync`, the "Queue plan" button) calls `SetupChainAsync(parent, enqueue: true)`. 3. Once queued, the first child is woken automatically; successors unblock as their predecessor reaches a terminal state via `OnChildFinishedAsync`. +`QueuePlanAsync` first gives the parent its worktree (the unit branch, see Unified parent model); +the integration branch in `PlanningAggregator` (`planning/{slug}-integration`, `checkout -b` in the +main checkout) is only built for legacy units — a parent that owns the unit returns +`unitBase..unitBranch` as its combined diff. + A child that hits a roadblock (fails, or reports `CLAUDEDO_BLOCKED`) does **not** advance the parent — it stays in `WaitingForChildren` until every child is terminal, with blocked children surfaced on the parent's Session tab instead. @@ -130,7 +148,7 @@ Full flow, invariants, and model/effort/max-turns resolution (including the low- - **ClaudeProcess** — spawns `claude -p --output-format stream-json --verbose --permission-mode auto` (or whatever app settings specify). Prompt via stdin, NDJSON from stdout. CancellationToken kills the process tree. Resolves `_cfg.ClaudeBin` via `ExecutableResolver` (`ClaudeDo.Data`, shared with `ClaudeCliPreflight` and the Installer's checks) before spawning — a `.cmd`/`.bat` shim (e.g. an npm-installed `claude`) is launched through `cmd.exe /c` since `UseShellExecute = false` can't exec a shim directly; a resolved `.exe` starts exactly as before. Throws if nothing resolves. *(Not yet on `main` — see `Environment Checks` in `ClaudeDo.Installer/CLAUDE.md`.)* - **ClaudeArgsBuilder** — `--model`, `--effort`, `--max-turns`, `--append-system-prompt`, `--agents`, `--json-schema`, `--resume` - **StreamAnalyzer** — parses NDJSON; extracts session_id, token counts, turn counts, result text, structured output. Replaced MessageParser. -- **WorktreeManager** — worktrees on `/-` branches, same leaf as the folder (`NameFor`, slug capped at 40 chars, umlauts transliterated). Typ from `BranchTypeFor` (`feat`→`feature`, `fix/refactor/docs/test/chore` as-is, anything else `chore`). Nummer = ticket id (own `TicketRef` or the parent's), else the task number — in a company repo (`CompanyRepo`, origin matches `company_remote_pattern`) without a ticket no number at all. A taken folder/branch name gets an 8-char task-id suffix, so the self-heal never force-deletes a foreign branch. Base = parent head for children, else `list_config.base_branch`, else the checked-out HEAD. Commits with semantic messages, updates DB with head commit + diff stats +- **WorktreeManager** — worktrees on `/-` branches, same leaf as the folder (`NameFor`, slug capped at 40 chars, umlauts transliterated). Typ from `BranchTypeFor` (`feat`→`feature`, `fix/refactor/docs/test/chore` as-is, anything else `chore`). Nummer = ticket id (own `TicketRef` or the parent's), else the task number — in a company repo (`CompanyRepo`, origin matches `company_remote_pattern`) without a ticket no number at all. A taken folder/branch name gets an 8-char task-id suffix, so the self-heal never force-deletes a foreign branch. Base = `rev-parse ` for a child whose parent has an Active worktree, else `list_config.base_branch`, else the checked-out HEAD. Commits with semantic messages, updates DB with head commit + diff stats - **CommitMessageBuilder** — `{commitType}(slug): title\n\ndescription\n\nClaudeDo-Task: taskId`; `BuildMerge` is the merge-commit variant (`{commitType}(slug): merge title` + trailer). **Every merge caller passes a blank commit message on purpose** — `TaskMergeService` fills in `BuildMerge` from the task's commit type and its list's name, which is the only place that knows both. Don't reintroduce a caller-side literal. - **TaskResetService** — discards a failed task's worktree, resets the row to Idle, preserves run history - **AgentFileService** — manages `~/.claudeDo/agents/*.md`; list/refresh via SignalR