Skip to content

Commit 15935c6

Browse files
brookscbrooksc
committed
chore: mark TODO johannesjo#12 complete (selectMcpJsonDir test added)
1 parent 1b19f06 commit 15935c6

1 file changed

Lines changed: 138 additions & 10 deletions

File tree

‎TODOS.md‎

Lines changed: 138 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,146 @@
11
# TODOs
22

3+
## Project context (read before dispatching tasks)
4+
5+
**Stack:** SolidJS frontend (`src/`), Electron main process (`electron/`), Node-pty terminals, TypeScript strict throughout.
6+
**Test command:** `npm test && npm run typecheck && npm run lint && npm run compile && npm run build:mcp`
7+
**Key conventions:** No `any`, functional SolidJS components, IPC channels defined in `electron/ipc/channels.ts`, MCP tools in `electron/mcp/server.ts`, coordinator logic in `electron/mcp/coordinator.ts`.
8+
**Do not** modify files outside the scope of the assigned task. Run the full test command and fix any failures before calling `signal_done`.
9+
10+
---
11+
12+
## Beta blockers — lifecycle/scoping issues
13+
14+
### 13. Coordinator cross-contamination — tasks not scoped to their coordinator
15+
16+
**Files:** `electron/mcp/server.ts:75-80` (`create_task`), `electron/remote/server.ts:300-325,420-450` (`list_tasks`, `send_prompt`, `merge_task`, `close_task`)
17+
**What's wrong:** `create_task` sends no `projectId`. Tool handlers accept `taskId` without verifying the caller's `coordinatorId`. Multiple concurrent coordinators can see and control each other's sub-tasks.
18+
**Done when:** Every MCP tool call that targets a specific task passes `coordinatorId`, and each handler rejects calls where `task.coordinatorTaskId !== coordinatorId`.
19+
20+
### 14. Coordinator MCP broken when remote access server was already running
21+
22+
**File:** `electron/ipc/register.ts` (`StartMCPServer`), `electron/remote/server.ts` (`startRemoteServer`)
23+
**What's wrong:** `StartMCPServer` skips `startRemoteServer()` if already running. If remote access started first (without a coordinator), coordinator-specific API routes are never registered.
24+
**Done when:** Either route handlers look up coordinator at request time via a mutable closure, or the server is torn down and recreated when a coordinator registers.
25+
26+
### 15. Restart restore racy and incomplete
27+
28+
**Files:** `src/App.tsx:329-349` (`StartMCPServer` calls), `electron/mcp/coordinator.ts:883-914` (`hydrateTask`)
29+
**What's wrong:** `StartMCPServer` is called without `await` before agent respawn. `hydrateTask` does not restore `signalDoneAt` or the consumed-signal flag — after restart, any sub-task that already called `signal_done` will cause `wait_for_signal_done` to block forever.
30+
**Done when:** `StartMCPServer` is awaited before child hydration; `signalDoneAt` and `signalDoneConsumed` are restored in `hydrateTask`.
31+
32+
### 16. Closing a coordinated child leaves stale backend coordinator state
33+
34+
**Files:** `src/store/tasks.ts:349-368` (`closeTask`), `electron/mcp/coordinator.ts` (task removal)
35+
**What's wrong:** Closing a coordinated child from the UI deletes the renderer task and worktree but leaves it in the backend `Coordinator.tasks` map. Subsequent MCP calls reference the deleted task.
36+
**Done when:** A new IPC channel (e.g. `MCP_CoordinatedTaskClosed`) is invoked when a coordinated child is closed from the UI, and the backend removes it from `Coordinator.tasks`.
37+
38+
### 23. Collapsing coordinated children breaks backend agent identity
39+
40+
**Files:** `src/store/tasks.ts` (`collapseTask`, `uncollapseTask`), `electron/mcp/coordinator.ts`
41+
**What's wrong:** `collapseTask` clears `agentIds`; `uncollapseTask` creates a new `agentId`. The backend coordinator registry still holds the old `agentId`, so `send_prompt` and idle detection target a dead PTY.
42+
**Done when:** Either collapse is blocked for tasks with `coordinatedBy` set (simplest), or uncollapse emits an IPC event so the backend updates its agent reference.
43+
44+
### 24. Coordinator-created task agent metadata is hardcoded as Claude
45+
46+
**File:** `src/store/tasks.ts:898`
47+
**What's wrong:** The frontend `Agent` is created with `id: 'claude'` and `name: 'Claude Code'` regardless of `evt.agentCommand`. Coordinators spawning `codex` or `gemini` sub-tasks show wrong display names and broken `resume_args`.
48+
**Done when:** The agent `def` is derived from `evt.agentCommand` — look up by command in the configured agents list, or at minimum set `id` and `name` to match the actual binary.
49+
50+
---
51+
352
## Known edge cases — no fix yet
453

554
### 7. Autofire expiry window — coordinator in long tool call during countdown
655

7-
`bf07118` escalates to an orphaned notification after 10 prompt misses, but if
8-
the coordinator is mid-tool-call when the autofire countdown fires (no PTY
9-
prompt visible), the countdown fires, finds no prompt, misses 10 times, and
10-
escalates unnecessarily. Whether this happens in practice depends on timing; no
11-
fix yet.
56+
**What's wrong:** If the coordinator is mid-tool-call when autofire countdown fires, it finds no PTY prompt, misses 10 times, and escalates unnecessarily.
57+
**No fix yet** — depends on timing; low frequency in practice.
58+
59+
### 8. Post-restart coordinator MCP config stale if coordinator process restarts
60+
61+
**What's wrong:** `mcpConfigPath` is persisted and rewritten on app restart, but if the coordinator Claude process itself restarts, the MCP token changes and running sub-tasks become unreachable.
62+
**No fix yet** — edge case.
63+
64+
### 17. MCP token visible in process list (`ps aux`)
65+
66+
**File:** `electron/mcp/coordinator.ts` (`buildCoordinatorMCPConfig`), `electron/mcp/mcp-server.cjs` entry point
67+
**What's wrong:** `--token <value>` is passed as a CLI argument, visible in `ps aux`.
68+
**Done when:** Token is passed via env var (`PARALLEL_CODE_MCP_TOKEN`) or stdin instead of a CLI flag.
69+
70+
### 18. Docker sub-agent process cleanup is shaky
71+
72+
**File:** `electron/mcp/coordinator.ts:507`
73+
**What's wrong:** Sub-agents spawned via `docker exec` are not marked `dockerMode` with a `containerName`. Killing the PTY stops the host `docker exec` client but may leave the inner agent process running inside the coordinator container.
74+
**Done when:** Docker-exec PTY sessions are marked with the coordinator `containerName` and a `SIGTERM` is sent to the inner process via `docker exec <container> kill <pid>` on cleanup.
75+
76+
### 19. All Docker sub-tasks share coordinator container HOME (`/tmp`)
77+
78+
**File:** `electron/ipc/pty.ts` (docker exec spawn args)
79+
**What's wrong:** All `docker exec` sub-tasks inherit `HOME=/tmp`. Multiple Claude processes can collide on `/tmp/.claude` config files.
80+
**Done when:** Each `docker exec` call passes `-e HOME=/tmp/agent-<taskId>` and pre-creates the dir, OR the limitation is documented and single-sub-task-at-a-time is enforced.
81+
82+
### 20. Claude trust seeding (.claude.json) has a read-modify-write race
83+
84+
**File:** `electron/ipc/pty.ts:642`
85+
**What's wrong:** Two concurrent agent spawns can each read `.claude.json` before the other writes, dropping one spawn's trusted project entry.
86+
**Done when:** Writes use an atomic rename pattern or a per-file advisory lock.
87+
88+
### 21. Stale Docker MCP URL detection is passive
89+
90+
**File:** `electron/ipc/register.ts` (`StartMCPServer`), `electron/mcp/config.ts` (`detectStaleDockerMCPUrl`)
91+
**What's wrong:** `detectStaleDockerMCPUrl()` exists but is not called in the startup path. Users with `127.0.0.1` in `.mcp.json` on macOS get a silent connection failure.
92+
**Done when:** `detectStaleDockerMCPUrl()` is called in `StartMCPServer` after config is written, and a warning is surfaced to the renderer.
93+
94+
### 22. Remote MCP server listen readiness not awaited
95+
96+
**File:** `electron/remote/server.ts` (`startRemoteServer`)
97+
**What's wrong:** `server.listen()` is async but `startRemoteServer` returns immediately. Config files are written before the TCP port is bound, so fast Docker clients can connect before the host is listening.
98+
**Done when:** `startRemoteServer` resolves only inside `server.on('listening', ...)`.
99+
100+
### 25. Remote server started without coordinator routes when remote access precedes coordinator
101+
102+
**File:** `electron/ipc/register.ts` (`StartRemoteServer`, `StartMCPServer`)
103+
**What's wrong:** If remote access was enabled before any coordinator task exists, `coordinator` is `null` at `startRemoteServer` time and coordinator routes (`/api/tasks`, `signal_done`) are never registered. `StartMCPServer` reuses the existing server without adding the missing routes.
104+
**Done when:** Route handlers receive a mutable `getCoordinator()` callback so they look up the coordinator at request time, OR the server is restarted with the coordinator attached when `StartMCPServer` is called.
105+
106+
### 26. MCP status reporting is too shallow
107+
108+
**File:** `electron/ipc/register.ts` (`GetMCPStatus`)
109+
**What's wrong:** Returns only `{ mcpRunning: remoteServer !== null }`. Does not verify coordinator routes are attached, server has finished listening, or coordinator is registered.
110+
**Done when:** Returns separate fields: `remoteRunning`, `coordinatorRoutesAttached`, `coordinatorRegistered`, `serverUrl`, `mcpConfigPath`.
111+
112+
### 27. Coordinator deregistration does not clean child backend resources
113+
114+
**File:** `electron/mcp/coordinator.ts` (`deregisterCoordinator`, ~line 1063)
115+
**What's wrong:** Deletes child tasks from `this.tasks` but leaves `subscribers`, `tailBuffers`, `decoders`, `idleResolvers`, `controlMap`, `blockedByHumanControl` entries alive with callbacks closing over deleted task objects.
116+
**Done when:** Child tasks are either converted to a detached state or all coordinator-owned backend resources are cleaned on deregistration.
117+
118+
### 28. `setTaskControl` is optimistic with no rollback
119+
120+
**File:** `src/store/tasks.ts` (`setTaskControl`, ~line 1063)
121+
**What's wrong:** Updates frontend `controlledBy` before the `MCP_ControlChanged` IPC call completes. If IPC fails, UI shows wrong control state.
122+
**Done when:** IPC is awaited before committing UI state for sub-tasks, or previous value is restored on failure.
123+
124+
### 30. MCP remote server bind address — WAITING FOR REPO OWNER INPUT — DO NOT FIX
125+
126+
> ⚠️ **DO NOT IMPLEMENT A SOLUTION HERE.** This item is parked pending a decision from the repo owner. Do not touch `electron/remote/server.ts` or `electron/ipc/register.ts` bind address logic.
127+
128+
**Background:** The coordinator MCP HTTP server (`StartMCPServer` path in `electron/ipc/register.ts`) was previously binding to `127.0.0.1`, which made it unreachable from Docker containers via `host.docker.internal`. A fix was applied to bind to `0.0.0.0` so Docker coordinator mode works.
129+
130+
**The concern (raised by repo owner):** Binding to `0.0.0.0` exposes mutating REST endpoints (`POST /api/tasks`, `POST /api/tasks/:id/merge`, `DELETE /api/tasks/:id`, `POST /api/tasks/:id/prompt`) to anyone on the LAN. The repo owner previously flagged this for the mobile-remote feature (where the surface was read-mostly) and suggested: _(a)_ bind to `127.0.0.1` by default and only widen when the user explicitly enables remote mobile, or _(b)_ scope the token so coordinator endpoints require an additional capability the mobile-remote token doesn't get.
131+
132+
**The conflict:** Option (a) breaks Docker coordinator — `host.docker.internal` from inside a Docker container resolves to the host's gateway IP (not loopback), so `127.0.0.1:7777` is unreachable. Docker coordinator requires either `0.0.0.0` or a separate server/transport (Unix socket, second port, etc.).
133+
134+
**Options on the table:**
135+
136+
1. Keep `0.0.0.0` for coordinator MCP (current state). Token protects it. Document the risk.
137+
2. Split into two servers: a separate minimal `0.0.0.0` server for coordinator-only routes, mobile remote stays independent.
138+
3. Unix socket bind-mounted into the container — eliminates TCP exposure entirely, larger change.
139+
140+
**Status:** Waiting for repo owner to weigh in on preferred approach before any work begins.
12141

13-
### 8. Post-restart coordinator MCP config becomes stale if coordinator process restarts
142+
### 29. Pending `initialPrompt` is not persisted across app restarts
14143

15-
`22effac` persists `mcpConfigPath` across app restarts and rewrites configs with
16-
the new port/token. However, if the coordinator Claude process itself restarts
17-
(not the Electron app), the MCP server URL and token change and any running
18-
sub-tasks are unreachable. Edge case, but real.
144+
**Files:** `src/store/persistence.ts:106`, `src/store/tasks.ts:900`
145+
**What's wrong:** `saveState()` persists `savedInitialPrompt` but not `initialPrompt`. Sub-tasks created by MCP store the auto-send prompt only as `initialPrompt`. App restart loses it and the sub-task restarts idle.
146+
**Done when:** Pending `initialPrompt` is persisted and cleared only after `clearInitialPrompt()` confirms delivery.

0 commit comments

Comments
 (0)