docs(specs): per-list task handler with read/dedupe/enhance/run/merge
This commit is contained in:
Mika Kuns
@@ -0,0 +1,191 @@
|
|||||||
|
# "Let Claude handle it" — per-list handler with read / dedupe / enhance / run / merge
|
||||||
|
|
||||||
|
Date: 2026-07-27
|
||||||
|
Supersedes parts of: `2026-07-24-merge-helper-design.md` (global scope, run-then-merge-only prompt)
|
||||||
|
|
||||||
|
## 1. Problem
|
||||||
|
|
||||||
|
The merge helper shipped in v2.3.0 with two entry points — a per-list context-menu item and a
|
||||||
|
global footer Broom button — and a prompt that only runs and merges the selected tasks.
|
||||||
|
|
||||||
|
Two things are wrong with that:
|
||||||
|
|
||||||
|
- **The global scope is unwanted.** A run spanning several lists spans several repos, which
|
||||||
|
makes `cwd`, the `--add-dir` set and the merge order ambiguous for no benefit. The user
|
||||||
|
works one list (= one repo) at a time.
|
||||||
|
- **The helper starts too late.** It takes the task list as given: it never reads the tasks as
|
||||||
|
a set, so duplicates run twice and produce conflicting worktrees, and vague tasks go into an
|
||||||
|
autonomous run under-specified and come back wrong.
|
||||||
|
|
||||||
|
## 2. Goal
|
||||||
|
|
||||||
|
One entry point, on a user list. It opens an interactive ConPTY session that takes the selected
|
||||||
|
tasks through five phases: read them all, dedupe them, sharpen them for autonomous execution,
|
||||||
|
run them, then review and merge each worktree.
|
||||||
|
|
||||||
|
Non-goals: no change to the autonomous queue path, no change to the ConPTY tile plumbing,
|
||||||
|
no rename of the `MergeHelper*` identifiers (the user-facing label stays "Let Claude handle it").
|
||||||
|
|
||||||
|
## 3. Scope becomes list-only
|
||||||
|
|
||||||
|
`listId` becomes non-nullable across the whole chain:
|
||||||
|
|
||||||
|
| Layer | Change |
|
||||||
|
|---|---|
|
||||||
|
| `ListsIslandViewModel` | `MergeHelperRequest(string ListId, …)`; `LetClaudeHandleAllAsync` deleted |
|
||||||
|
| `IslandsShellViewModel:241` | unchanged (already forwards `req.ListId`) |
|
||||||
|
| `MissionControlViewModel:327` | `OpenMergeHelperConPtySessionAsync(string listId, …)` |
|
||||||
|
| `IWorkerClient:90` / `WorkerClient:525` | `GetMergeHelperLaunchSpecAsync(taskIds, string listId, ct)` |
|
||||||
|
| `WorkerHub:682` | `GetMergeHelperLaunchSpec(string[] taskIds, string listId)` |
|
||||||
|
| `IInteractiveLaunchSpecService:36` | `BuildForMergeHelperAsync(taskIds, string listId, ct)` |
|
||||||
|
|
||||||
|
Deleted UI surface:
|
||||||
|
|
||||||
|
- Broom button `ListsIslandView.axaml:205-210` and `LetClaudeHandleAllCommand`.
|
||||||
|
- `IsGlobal` on `MergeHelperSelectionModalViewModel` and the LIST column
|
||||||
|
(`MergeHelperSelectionModal.axaml:71`) — with a single list the column is constant.
|
||||||
|
- Localization keys `lists.letClaudeAllTip`, `modals.mergeHelper.scopeAll`,
|
||||||
|
`modals.mergeHelper.columnList` (en + de, parity test enforces both).
|
||||||
|
|
||||||
|
`Configure(string listId, string listName)` loses its nullable overload; `ScopeLabel` always
|
||||||
|
renders `modals.mergeHelper.scopeList`.
|
||||||
|
|
||||||
|
### 3.1 Single repo in the launch spec
|
||||||
|
|
||||||
|
`BuildForMergeHelperAsync` currently collects a distinct `repoDirs` set across the selected
|
||||||
|
tasks and picks `cwd` per scope. With a list scope every task shares the list's `WorkingDir`,
|
||||||
|
so this collapses to:
|
||||||
|
|
||||||
|
- Load the list; throw `InvalidOperationException` if it has no existing `WorkingDir`.
|
||||||
|
- `cwd` = that directory; `--add-dir` = the session dir + that one directory.
|
||||||
|
- The per-task brief line drops the now-constant `list:` and `repo:` fields.
|
||||||
|
|
||||||
|
The context-menu item is already hidden when `WorkingDir` is empty, so the throw is a guard,
|
||||||
|
not a normal path.
|
||||||
|
|
||||||
|
### 3.2 Entry point visibility
|
||||||
|
|
||||||
|
The item stays in the list row's context menu, next to "List settings", "Worktrees overview",
|
||||||
|
"Open in Explorer" and "Open in Terminal". That is the established place for list-scoped
|
||||||
|
actions; a second always-visible button in the row would break the pattern.
|
||||||
|
|
||||||
|
## 4. Worker: `Cancelled` becomes externally settable
|
||||||
|
|
||||||
|
`ExternalMcpService.UpdateTaskStatus` accepts only `Idle` and `Queued` and throws
|
||||||
|
`"Status '{target}' is not settable externally. Use run_task_now or cancel_task."` for the
|
||||||
|
rest — but neither escape hatch reaches an **Idle** task: `cancel_task` only cancels a
|
||||||
|
*running* task, and `review_task(decision="cancel")` requires WaitingForReview/Running/Queued.
|
||||||
|
The dedupe phase needs exactly that: retire an Idle duplicate without destroying it.
|
||||||
|
|
||||||
|
Add to the switch:
|
||||||
|
|
||||||
|
```csharp
|
||||||
|
case TaskStatus.Cancelled:
|
||||||
|
var cancelResult = await _state.CancelAsync(taskId, DateTime.UtcNow, cancellationToken);
|
||||||
|
if (!cancelResult.Ok)
|
||||||
|
throw new InvalidOperationException(cancelResult.Reason ?? "Cannot cancel task.");
|
||||||
|
break;
|
||||||
|
```
|
||||||
|
|
||||||
|
`TaskStateService.CancelAsync` (`State/TaskStateService.cs:244`) already owns the transition
|
||||||
|
and its worktree/parent side effects. The existing error message in `UpdateTaskStatus` already
|
||||||
|
lists `Cancelled` as valid, so this also removes a lie. A cancelled task stays visible and can
|
||||||
|
be reset to Idle — nothing is lost, unlike `delete_task`.
|
||||||
|
|
||||||
|
## 5. The five-phase prompt
|
||||||
|
|
||||||
|
`PromptFiles.MergeHelperDefault` is rewritten. The session stays interactive and the helper is
|
||||||
|
told to ask whenever unsure — that is the point of a watched ConPTY session.
|
||||||
|
|
||||||
|
### Phase 0 — Read
|
||||||
|
|
||||||
|
`batch_get_tasks` over every id in the brief before touching anything: title, description,
|
||||||
|
status, parent/child links. The helper must hold the whole set in mind before acting on any
|
||||||
|
single task.
|
||||||
|
|
||||||
|
### Phase 1 — Dedupe
|
||||||
|
|
||||||
|
Compare the tasks pairwise for overlap. Emit a table of candidate pairs with the reason each
|
||||||
|
pair looks like a duplicate, then **ask per pair**:
|
||||||
|
|
||||||
|
- merge → fold the loser's unique content into the survivor via `update_task`, then
|
||||||
|
`update_task_status(loserId, "Cancelled")`;
|
||||||
|
- keep both → note why and move on.
|
||||||
|
|
||||||
|
Nothing is cancelled without an explicit answer.
|
||||||
|
|
||||||
|
### Phase 2 — Enhance
|
||||||
|
|
||||||
|
For each surviving task, sharpen title and description for autonomous execution:
|
||||||
|
|
||||||
|
- concrete acceptance criteria,
|
||||||
|
- the files/areas actually involved — grounded in the repo via Read/Grep/Glob, not guessed,
|
||||||
|
- explicit out-of-scope.
|
||||||
|
|
||||||
|
Write back with `update_task` (title / description / commitType are the settable fields; it
|
||||||
|
refuses while Running, which cannot happen this early). Rules: do not change intent, do not
|
||||||
|
invent requirements. A task too vague to sharpen safely gets a question, not a guess.
|
||||||
|
|
||||||
|
### Phase 3 — Run
|
||||||
|
|
||||||
|
`run_task_now` cannot be used for a batch: `OverrideSlotService.StartInSlot`
|
||||||
|
(`Queue/OverrideSlotService.cs:65-70`) holds a single slot and throws `"override slot busy"`
|
||||||
|
on the second concurrent call. The queue picker is the only parallel path.
|
||||||
|
|
||||||
|
So: read `get_app_settings`, tell the user how many parallel slots are configured
|
||||||
|
(`MaxParallelExecutions`, default 1 — `AppSettingsEntity.cs:14`), then
|
||||||
|
`update_task_status(id, "Queued")` for every surviving task, then poll `get_task` until each
|
||||||
|
has left Queued/Running — `WaitingForReview` on success, `Failed` on error. Announcing the
|
||||||
|
slot count up front stops the user wondering why "run them all" executes one at a time.
|
||||||
|
|
||||||
|
A task already `Running` or `WaitingForChildren` when the session starts is not re-queued, only
|
||||||
|
polled. A task already `WaitingForReview` skips straight to Phase 4.
|
||||||
|
|
||||||
|
### Phase 4 — Review and merge
|
||||||
|
|
||||||
|
Sequential, in list order. A task that came back `Failed` needs human judgement — ask whether
|
||||||
|
to `reset_failed_task` and re-queue it, or skip it. Otherwise, unchanged from the shipped
|
||||||
|
prompt: `get_task_diff` (stat first,
|
||||||
|
full diff when non-trivial), sanity-check against the task's intent, ask before merging
|
||||||
|
anything that looks wrong, then `review_task(taskId, decision="approve",
|
||||||
|
leaveConflictsInTree=true)` and the conflict loop (`continue_merge` / `abort_merge`, parent id
|
||||||
|
for unit merges, hand-resolution only where MCP cannot reach, always
|
||||||
|
`git commit -- <paths>` and never `git add -A` because the checkout is shared).
|
||||||
|
|
||||||
|
New in this phase: the prompt states that because Phase 3 branches all fork from the same
|
||||||
|
base, **conflicts are the normal case, not an exception** — resolve them rather than bailing
|
||||||
|
out of the run.
|
||||||
|
|
||||||
|
### Phase 5 — Summary
|
||||||
|
|
||||||
|
One line per task: title — dedupe action — enhanced? — final status — merge commit —
|
||||||
|
conflicts resolved. Then anything skipped and why, then follow-ups.
|
||||||
|
|
||||||
|
### Brief template
|
||||||
|
|
||||||
|
`MergeHelperInitialDefault` names the list and repo once in the header and drops the per-task
|
||||||
|
`list:`/`repo:` fields, leaving `- [{status}] {title} (id: {id})`. Descriptions stay out of the
|
||||||
|
brief; Phase 0 fetches them.
|
||||||
|
|
||||||
|
## 6. Testing
|
||||||
|
|
||||||
|
| Test | Change |
|
||||||
|
|---|---|
|
||||||
|
| `MergeHelperSelectionModalViewModelTests` | drop `IsGlobal`, list-scoped `Configure` |
|
||||||
|
| `InteractiveLaunchSpecServiceTests` | three `null`-listId cases → non-null; drop the two-repo global-scope test; add "list without WorkingDir throws" |
|
||||||
|
| `MissionControlViewModelTests` | four `OpenMergeHelperConPtySessionAsync(null, …)` calls |
|
||||||
|
| `StubWorkerClient`, `TasksIslandViewModelPlanningTests` fake | signature |
|
||||||
|
| `PromptFilesTests` | assert the five phase markers in the default prompt |
|
||||||
|
| `Localization.Tests` | parity after removing three keys |
|
||||||
|
| new: `ExternalMcpService` / worker test | `UpdateTaskStatus(id, "Cancelled")` cancels an Idle task; unknown status still throws |
|
||||||
|
|
||||||
|
No test spawns the real `claude` CLI — the prompt content is asserted as text, the session
|
||||||
|
itself is a manual smoke step.
|
||||||
|
|
||||||
|
## 7. Verification left to the user
|
||||||
|
|
||||||
|
- The list context menu shows "Let Claude handle it" only for lists with a working dir, and
|
||||||
|
the Broom button is gone from the footer row.
|
||||||
|
- The selection dialog has no LIST column and reads "List: <name>".
|
||||||
|
- A real ConPTY run: dedupe questions appear, enhancements land in the task descriptions,
|
||||||
|
queued tasks execute, merges complete or hand off to conflict resolution.
|
||||||
Reference in New Issue
Block a user