feat(coordinator): sliding-window preamble, max concurrent UI, and sub-task preamble cleanup

- Rewrite coordinator preamble with strict sliding-window pattern, native
  background Agent landing flow, and explicit rules for baseBranch, dirty
  worktree, merge-before-close, scope discipline, and test verification
- Add MAX_CONCURRENT placeholder substituted at task creation time
- Add max concurrent tasks numeric input to NewTaskDialog (default 3)
- Update sub-task preamble to require tests/typecheck before signal_done
- Add TODOS.md items R1-R4 (regressions) and johannesjo#12-johannesjo#19 (new findings)
- Rule 6c now passes coordinator worktree path to landing Agent

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: brooksc

TODOS.md

@@ -2,6 +2,32 @@
 
 Items ordered from simplest to hardest.
 
+## Regressions (introduced by sub-agent session)
+
+### R1. `MCP_TaskClosed` neighbor selection still broken
+
+`s.taskOrder.indexOf(taskId)` is called at `tasks.ts:902` **after** `cleanupPanelEntries` has already removed `taskId` from `s.taskOrder` (line 897). It always returns `-1`, so `neighborIdx` is always `0` — the active task always jumps to the first task instead of the adjacent one.
+
+Fix: capture idx before `cleanupPanelEntries`, use the return value, and index into `s.taskOrder` (already filtered) rather than re-filtering.
+
+- File: `src/store/tasks.ts` `MCP_TaskClosed` handler (~line 895)
+
+### R2. `signalDoneReceived` and `needsReview` not persisted
+
+Both fields exist on `Task` in `types.ts` but are absent from `saveState()` and `loadState()` in `persistence.ts`, and from `autosave.ts`. They are lost on app restart — "needs review" badges and done signals disappear on reload.
+
+Fix: add both fields to `PersistedTask` in `types.ts` and include them in the save/load blocks in `persistence.ts` (active and collapsed), plus the autosave snapshot.
+
+- Files: `src/store/types.ts`, `src/store/persistence.ts`, `src/store/autosave.ts`
+
+### R3. `createTask` failure cleanup incomplete — zombie tasks left in memory
+
+On `createTask` failure, the `catch` block at `coordinator.ts:578` only restores the preamble file then re-throws. It does NOT call `this.tasks.delete(task.id)`, `this.tailBuffers.delete(agentId)`, `this.subscribers.delete(agentId)`, or `killAgent(agentId)`. If `spawnAgent` succeeded before the failure, a running PTY agent is leaked and the task remains in `this.tasks` indefinitely.
+
+Fix: the catch block should remove all in-memory state and kill the agent if it was spawned (the full cleanup that was implemented earlier and reverted).
+
+- File: `electron/mcp/coordinator.ts` `createTask()` catch block (~line 578)
+
 ## Medium (known edge cases — no fix yet)
 
 ### 7. Autofire expiry window — coordinator in long tool call during countdown
@@ -65,6 +91,42 @@ drop-ack path should mark affected sub-tasks `needsReview` before acking.
 
 ## UI / Behavior
 
+### R4. `.claude/settings.local.json` preamble injection not stripped before merge
+
+For Claude agents, the preamble is written into `.claude/settings.local.json` (`coordinator.ts:446`). This file is assumed to be gitignored, but if a project tracks it, `git add -A` in the auto-commit will stage the injected preamble content and it will land in the merge. `stripPreambleFromBranch` has no code path for this file.
+
+Fix: either explicitly `git restore .claude/settings.local.json` before auto-commit, or track it the same way as AGENTS.md/GEMINI.md (store original content, strip before commit).
+
+- File: `electron/mcp/coordinator.ts` `mergeTask()` / `stripPreambleFromBranch()`
+
+### 12. `review_and_merge_task` merges before coordinator can review
+
+`reviewAndMergeTask()` calls `getTaskDiff()` then immediately calls `mergeTask()` (`coordinator.ts:774-775`), returning the diff only after the merge is complete. The coordinator preamble instructs agents to "review the result and call `review_and_merge_task`" — but the diff arrives post-merge, so the "review" is cosmetic and cannot abort the merge.
+
+Fix: either update the preamble to use `get_task_diff → merge_task → close_task` explicitly and deprecate `review_and_merge_task`, or split the tool into two calls with a genuine gate between them.
+
+### 13. `gitIsolation` accepted by REST but silently ignored end-to-end
+
+REST validates and forwards `gitIsolation` (`remote/server.ts:271`), but `MCPClient.createTask()` has no `gitIsolation` field, the MCP tool schema omits it, and `Coordinator.createTask()` always creates a worktree unconditionally. A caller that requests a different isolation mode gets a worktree with no error.
+
+Fix: either remove `gitIsolation` from the REST path, or implement and forward only supported modes explicitly.
+
+### 14. `SubTaskStrip` collapsed sub-task click selects without uncollapsing
+
+The strip now includes collapsed sub-tasks, but the click handler only calls `setActiveTask(task.id)` (`SubTaskStrip.tsx:186`). Clicking a collapsed sub-task selects it without uncollapsing it, leaving it hidden.
+
+Fix: if `task.collapsed`, call `uncollapseTask(task.id)` before `setActiveTask`.
+
+- File: `src/components/SubTaskStrip.tsx`
+
+### 15. Preamble stripping deletes intentionally empty tracked instruction files
+
+`stripPreambleFromBranch()` deletes the preamble file when the content before the injected block is empty/whitespace (`coordinator.ts:800`). If a project had an intentionally empty tracked `AGENTS.md`, `GEMINI.md`, or `.agent.md`, the merge will stage its deletion.
+
+Fix: store whether the file existed originally (not just its content) and always restore to original state — delete only if the file was newly created, restore the original bytes (even if empty) if it existed.
+
+- File: `electron/mcp/coordinator.ts` `stripPreambleFromBranch()`
+
 ### 18. Sidebar drag indices wrong when coordinated children are hidden/nested
 
 `taskIndexById` indexes raw `store.taskOrder` (`Sidebar.tsx:134`). Coordinated children are filtered out of the flat rendered list and nested under the coordinator (`sidebar-order.ts:54`). `computeDropIndex()` returns a rendered DOM index (`Sidebar.tsx:253`), then `reorderTask(from, to)` applies that index to raw `taskOrder` (`Sidebar.tsx:296`). If `taskOrder` contains hidden coordinated children, dragging top-level tasks can insert them between a coordinator and its child in raw order.

electron/mcp/sub-task-preamble.ts

@@ -5,7 +5,10 @@ You have one special MCP tool available via the parallel-code server:
 - signal_done — Call this when your assigned work is fully complete and the coordinator should review it.
 
 RULES:
-1. Complete your assigned work fully before calling signal_done.
+1. Complete your assigned work fully before calling signal_done. Before signaling:
+   - Commit all changes (git add -A && git commit) with a meaningful message.
+   - Run the project's tests and type checker and fix any failures you introduced. \
+signal_done means "I verified it passes" — do not call it if tests or typecheck are failing.
 2. Ask questions if requirements are unclear or if you are about to do something risky or destructive — the user can see your terminal and can respond.
 3. When your work is done, call signal_done. Do NOT ask "what would you like to do?" or offer merge/PR options — signal_done is your finish line. Do NOT use finishing-a-development-branch or similar workflow skills.
 

src/components/NewTaskDialog.tsx

@@ -626,7 +626,11 @@ export function NewTaskDialog(props: NewTaskDialogProps) {
                   if (canSubmit()) handleSubmit(e);
                 }
               }}
-              placeholder="What should the agent work on?"
+              placeholder={
+                coordinatorMode()
+                  ? 'Example: Work through the items in /path/to/todos.md. Only work from that file. Use <branch> as the baseBranch for all sub-tasks.'
+                  : 'What should the agent work on?'
+              }
               rows={3}
               style={{
                 background: theme.bgInput,

src/store/coordinator-preamble.ts

@@ -11,36 +11,57 @@ You have MCP tools to coordinate work across isolated git worktree tasks:
 - get_task_status — Detailed status of a task
 - send_prompt — Send follow-up instructions to a task's agent
 - wait_for_signal_done — Wait for ANY sub-task to call signal_done. Returns { taskId, name, remaining }.
-- review_and_merge_task — Atomically get diff + merge + cleanup a completed task in one call.
 - wait_for_idle — Wait until an agent is idle at its prompt (use for send_prompt follow-ups)
 - get_task_diff — Get changed files and diff for a task
 - get_task_output — Get recent terminal output from a task
 - merge_task — Merge a task's branch into the base branch
-- close_task — Close and clean up a task
+- close_task — Close and clean up a task (ONLY after a successful merge_task)
 
 RULES:
-1. When asked to do work in parallel or break work into pieces, ALWAYS use the create_task MCP tool \
-to create separate Parallel Code tasks. Do NOT use your built-in Agent tool for parallelization — \
-the whole point of coordinator mode is isolated worktree tasks.
-2. If the user's request is ambiguous or you are unsure how to split the work into tasks, \
-ASK the user for clarification before creating tasks. It is better to ask a short question \
-than to guess wrong and spin up tasks that do the wrong thing.
-3. Assign each sub-agent one specific, concrete task — never point at a list and ask it to "pick one."
-   BAD:  "Fix the most important items from KNOWN-TODOS."
-   GOOD: "Fix the orphaned notification badge. The spec: when X is received, set Y on the sub-task..."
-   Give sub-agents complete, self-contained context. Include file paths, expected behavior, and constraints — they start with zero memory of this conversation. Always specify baseBranch when calling create_task.
-4. STANDARD WORKFLOW: create_task for each piece of work → then loop using wait_for_signal_done \
-until remaining === 0 → for each returned task, review the result and call review_and_merge_task \
-to land the work. NEVER chain wait_for_signal_done calls without reviewing the returned task first.
-5. THE LOOP PATTERN — YOU MUST FOLLOW THIS EXACTLY:
-   a. Create all tasks upfront with create_task.
-   b. Call wait_for_signal_done() — NO taskId argument — to wait for ANY sub-task to complete.
-   c. IMMEDIATELY review the returned task (check diff, validate output).
-   d. Call review_and_merge_task(taskId) to merge and clean up.
-   e. If remaining > 0, go back to step (b). If remaining === 0, you are done.
-6. Default to running at most 3 sub-agents concurrently. Try to avoid giving parallel sub-agents work that touches the same files — when overlap is unavoidable, run those tasks sequentially.
-7. Before assigning a task, verify it is not already implemented. Read the relevant files rather than assuming work is pending.
-8. Use send_prompt + wait_for_idle to give follow-up instructions to a running task.
+1. You MUST NOT use your built-in Agent tool to spawn new Parallel Code tasks — you MUST use \
+create_task for all new work. The sole EXCEPTION is review/landing: after wait_for_signal_done \
+returns a completed taskId, you MUST immediately dispatch a native background Agent to run \
+get_task_diff → merge_task → close_task for that taskId, then return to wait_for_signal_done \
+without waiting for the Agent to finish. Give the background Agent the taskId and all context it \
+needs to be self-contained.
+2. If the user's request is ambiguous, the specified work queue file does not exist, or you are \
+unsure how to split the work into tasks, STOP and ASK the user before proceeding. Do not improvise \
+a work queue from other files or directories — work only from sources explicitly specified in your \
+prompt.
+3. Assign each sub-agent one specific, concrete task — never point at a list and ask it to "pick one." \
+Give complete, self-contained context: file paths, expected behavior, constraints. Sub-agents start \
+with zero memory of this conversation. Always tell sub-agents to run the project's tests and type \
+checker before calling signal_done — signal_done means "verified passing", not "I think I'm done."
+4. baseBranch for sub-tasks MUST be the project's durable shared branch (e.g. main or the feature \
+branch you were told to use) — NOT your coordinator task branch. Your coordinator task branch is \
+ephemeral; sub-task work that only lives there never reaches the project's shared history. Ask the \
+user which branch to use if not specified in your prompt.
+5. Run at most {{MAX_CONCURRENT}} sub-tasks concurrently. Never exceed this limit. Avoid giving \
+parallel sub-tasks work that touches the same files — run those sequentially.
+6. THE SLIDING-WINDOW PATTERN — YOU MUST FOLLOW THIS EXACTLY:
+   a. Pick up to {{MAX_CONCURRENT}} items from your backlog and create a task for each. Track your \
+backlog yourself — the set of items not yet assigned.
+   b. Call wait_for_signal_done() — no taskId argument — to wait for ANY in-flight task to complete.
+   c. Immediately dispatch a native background Agent to review and land that task (see rule 1). \
+Do NOT do this work yourself. Pass the Agent: the taskId, the absolute path to the work queue \
+file in YOUR worktree, and the baseBranch. The Agent runs: get_task_diff → update and commit \
+the work queue file (remove the completed item) in your worktree → \
+merge_task(taskId, { squash: true }) → close_task(taskId). \
+The Agent must commit the work queue update BEFORE calling merge_task (rule 8).
+   d. Without waiting for that Agent: if backlog is non-empty AND in-flight count < {{MAX_CONCURRENT}}, \
+spawn a replacement task immediately.
+   e. If remaining > 0 OR backlog is non-empty, go back to step (b). Stop only when remaining === 0 \
+AND backlog is empty.
+7. merge_task is REQUIRED before close_task. close_task without a prior successful merge_task \
+permanently discards all sub-task work. Direct git operations (git merge, git cherry-pick) do NOT \
+substitute for merge_task — the backend cleans up worktrees and branches only when merge_task \
+succeeds. If merge_task fails with "uncommitted changes", commit your local edits first (see rule 8) \
+then retry merge_task.
+8. Commit any local edits in your worktree (e.g. task-list updates) BEFORE calling merge_task or \
+any git operation. A dirty working tree will cause merge_task to fail.
+9. Before assigning a task, verify it is not already implemented. Read the relevant files rather \
+than assuming work is pending.
+10. Use send_prompt + wait_for_idle to give follow-up instructions to a running task.
 
 ---
 `;