Skip to content

Commit bbf26db

Browse files
brookscclaude
andcommitted
chore: clean up TODOS.md — all actionable items resolved
Only items 7 and 8 remain (known edge cases with no fix yet). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 66e326d commit bbf26db

1 file changed

Lines changed: 1 addition & 123 deletions

File tree

TODOS.md

Lines changed: 1 addition & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,6 @@
11
# TODOs
22

3-
Items ordered from simplest to hardest.
4-
5-
## Regressions (introduced by sub-agent session)
6-
7-
### R1. `MCP_TaskClosed` neighbor selection still broken
8-
9-
`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.
10-
11-
Fix: capture idx before `cleanupPanelEntries`, use the return value, and index into `s.taskOrder` (already filtered) rather than re-filtering.
12-
13-
- File: `src/store/tasks.ts` `MCP_TaskClosed` handler (~line 895)
14-
15-
### R2. `signalDoneReceived` and `needsReview` not persisted
16-
17-
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.
18-
19-
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.
20-
21-
- Files: `src/store/types.ts`, `src/store/persistence.ts`, `src/store/autosave.ts`
22-
23-
### R3. `createTask` failure cleanup incomplete — zombie tasks left in memory
24-
25-
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.
26-
27-
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).
28-
29-
- File: `electron/mcp/coordinator.ts` `createTask()` catch block (~line 578)
30-
31-
## Medium (known edge cases — no fix yet)
3+
## Known edge cases — no fix yet
324

335
### 7. Autofire expiry window — coordinator in long tool call during countdown
346

@@ -44,97 +16,3 @@ fix yet.
4416
the new port/token. However, if the coordinator Claude process itself restarts
4517
(not the Electron app), the MCP server URL and token change and any running
4618
sub-tasks are unreachable. Edge case, but real.
47-
48-
## Hard
49-
50-
### 9. Backend coordinator task registry is not hydrated after app restart
51-
52-
On app restart, `App.tsx` restarts the MCP server for persisted coordinator
53-
tasks, but the backend `Coordinator.tasks` map is empty — it's only populated
54-
by live `createTask()` calls. Resumed coordinator sessions can have visible
55-
child tasks in the UI while MCP tools (`list_tasks`, `send_prompt`,
56-
`wait_for_signal_done`, `close_task`) operate on an empty registry and fail
57-
silently.
58-
59-
Fix: add a backend hydration path. After `loadState()` and coordinator MCP
60-
startup, replay each restored coordinated child into `Coordinator.tasks`
61-
including task id, agent id, branch, worktree, coordinator id, status, and
62-
base branch. This is non-trivial because the backend task type differs from the
63-
frontend `PersistedTask` shape.
64-
65-
### 10. Backend `controlMap` is not restored after app restart
66-
67-
The frontend persists and restores `controlledBy` correctly, but the backend
68-
gate uses an in-memory `controlMap` in `coordinator.ts`. On restart, no path
69-
replays `controlledBy === 'human'` sub-task state into `IPC.MCP_ControlChanged`.
70-
The UI can show "You have control" while the backend still allows coordinator
71-
`send_prompt` to go through.
72-
73-
Fix: after coordinator MCP startup completes, replay `MCP_ControlChanged` for
74-
any restored coordinated children whose `controlledBy === 'human'`. Depends on
75-
fix #9 above (backend must have the task registered before control state can be
76-
set on it).
77-
78-
### 11. `MCP_CoordinatorOrphanedNotification` channel is overloaded with incompatible payloads
79-
80-
Main→renderer orphan notifications carry `{ subTaskId, notificationId, state, text }`
81-
(`coordinator.ts:195`). Renderer→main uses the same channel name for
82-
`{ coordinatorTaskId, batchId }` (`PromptInput.tsx`). Direction prevents
83-
runtime collision today, but the overload is masking a real behavior gap:
84-
the renderer-to-main path just `ackNotification()`s without surfacing any
85-
review state on the affected sub-tasks — the notification is silently dropped.
86-
87-
Fix: split into two channels (`MCP_CoordinatorNotificationDropAck` for the
88-
renderer-to-main ack path, keep `MCP_CoordinatorOrphanedNotification` for
89-
main-to-renderer sub-task review surfacing). Separately, decide whether the
90-
drop-ack path should mark affected sub-tasks `needsReview` before acking.
91-
92-
## UI / Behavior
93-
94-
### R4. `.claude/settings.local.json` preamble injection not stripped before merge
95-
96-
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.
97-
98-
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).
99-
100-
- File: `electron/mcp/coordinator.ts` `mergeTask()` / `stripPreambleFromBranch()`
101-
102-
### 12. `review_and_merge_task` merges before coordinator can review
103-
104-
`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.
105-
106-
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.
107-
108-
### 13. `gitIsolation` accepted by REST but silently ignored end-to-end
109-
110-
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.
111-
112-
Fix: either remove `gitIsolation` from the REST path, or implement and forward only supported modes explicitly.
113-
114-
### 14. `SubTaskStrip` collapsed sub-task click selects without uncollapsing
115-
116-
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.
117-
118-
Fix: if `task.collapsed`, call `uncollapseTask(task.id)` before `setActiveTask`.
119-
120-
- File: `src/components/SubTaskStrip.tsx`
121-
122-
### 15. Preamble stripping deletes intentionally empty tracked instruction files
123-
124-
`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.
125-
126-
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.
127-
128-
- File: `electron/mcp/coordinator.ts` `stripPreambleFromBranch()`
129-
130-
### 18. Sidebar drag indices wrong when coordinated children are hidden/nested
131-
132-
`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.
133-
134-
Fix: base dragging on task IDs / visible order, then translate to raw `taskOrder` preserving coordinator child blocks.
135-
136-
### 19. Collapsed coordinator children can recurse into odd UI states
137-
138-
`CollapsedTaskEntry` renders children for collapsed coordinators (`Sidebar.tsx:1071`). Clicking a collapsed child uncollapses it directly while its parent coordinator remains collapsed, creating a visible active child nested under a collapsed coordinator or moving it depending on derived order.
139-
140-
Fix: restoring a collapsed child should restore/activate the coordinator, or children should not be independently restorable while their coordinator is collapsed.

0 commit comments

Comments
 (0)