feat(coordinator): frontend store wiring, IPC lifecycle, and session restore (PR 3/4)

[brooksc]

Overview

This is PR 3 of 4 in the coordinator series splitting #100 as requested in the round-4 review. It is stacked on PR 2 (#120) (coordinator-2-mcp-backend) and must be merged after that one. The diff shown here includes PRs 1–2's content; the meaningful delta for this PR is the frontend store wiring and IPC lifecycle described below.

PR sequence:

PR	Branch	Status	Contents
1 (#118)	coordinator-1-security	Open	Atomic writes, input validators, static analysis configs
2 (#120)	coordinator-2-mcp-backend	Open	MCP coordinator engine + REST API hardening
3 (this PR)	coordinator-3-store-ipc	Open	Frontend store wiring + IPC handlers
4	coordinator-4-ui	Pending	UI components + coordinator entry points

Nothing coordinator-related is user-visible until PR 4 adds the NewTaskDialog checkbox and Settings toggle (coordinatorModeEnabled defaults to false).

---

What's in this PR (delta over PR 2)

Task model extensions (src/store/types.ts)

New fields on Task/CollapsedTask:

• coordinatorMode, coordinatedBy, controlledBy ('coordinator' | 'human'), propagateSkipPermissions
• mcpConfigPath, mcpStartupStatus ('pending' | 'ready' | 'error'), mcpStartupError
• signalDoneReceived, signalDoneAt, signalDoneConsumed, needsReview, initialPrompt
• stagedNotification: StagedNotification — pending coordinator batch notification
• Global store: coordinatorModeEnabled, coordinatorNotificationDelayMs, coordinatorControlHintDismissed
• MCPStatus type for frontend polling

MCP lifecycle (src/store/tasks.ts)

New store functions wired to IPC:

• initMCPListeners() — subscribes to 7 MCP push events (MCP_TaskCreated, MCP_TaskClosed, MCP_TaskCleanupFailed, MCP_CoordinatorNotificationStaged/Cleared/Orphaned, MCP_TaskStateSync, MCP_TaskHydrated)
• markTaskMcpPending/Ready/Error() — MCP startup state transitions
• retryTaskMcpStartup() — retry after error, with coordinator-dependency guard
• setTaskControl() — toggles controlledBy between 'coordinator' and 'human'
• clearStagedNotification(), setStagedNotificationUserEdited()
• getCoordinatorCloseWarning() — warning text when closing a coordinator with active children
• reorderTaskVisually() — drag-reorder respecting coordinator group boundaries
• collapseTask() guard — no-op for coordinated children (prevents orphaning from coordinator group)
• closeTask() — calls MCP_CoordinatedTaskClosed / MCP_CoordinatorDeregistered on close; errors swallowed so the task is always removed

Session restore (src/App.tsx)

On startup, iterates all persisted tasks with coordinatorMode === true and calls StartMCPServer for each, rewriting .mcp.json config with the new session's port and token. Child tasks start with mcpStartupStatus: 'pending' (terminal spawn deferred) until MCP_HydrateCoordinatedTask returns.

Persistence (src/store/persistence.ts)

saveState()/loadState() now round-trips all coordinator fields: coordinatorMode, coordinatedBy, controlledBy, propagateSkipPermissions, mcpConfigPath, signalDone*, needsReview. Global fields coordinatorModeEnabled, coordinatorNotificationDelayMs, coordinatorControlHintDismissed also persisted. Tasks with coordinatorMode or coordinatedBy load with mcpStartupStatus: 'pending' to defer terminal spawn until the session's MCP server is ready.

Coordinator preamble (src/store/coordinator-preamble.ts)

System preamble prepended to the coordinator agent's initial prompt. Documents all 10 MCP tools and operating rules.

MCP status polling (src/store/mcpStatus.ts)

Polls GetMCPStatus IPC every 3 s; stores result in store.mcpStatus. Returns { running: false, ... } when no coordinator is active — no user-visible effect until a coordinator task exists.

Staged notification store (src/store/taskStatus.ts)

getTaskStatus() / getTaskDotStatus() return 'review' for tasks with needsReview set. Replace-on-arrival: new batch replaces the previous one before it fires; userEdited resets per-notification (not sticky).

Backend fix (electron/mcp/coordinator.ts)

In deregisterCoordinator, child tasks that never received their prompt (assignedPromptDelivered false) have reviewNotificationQueued set — suppresses a spurious "needs review" notification for tasks the user never saw.

IPC preload (electron/preload.cjs)

New channels exposed: set_coordinator_mode_enabled, mcp_hydrate_coordinated_task, mcp_task_hydrated, mcp_coordinated_task_closed, mcp_task_cleanup_failed.

---

Tests

File	New / updated cases
src/store/tasks.test.ts	~35 new — controlledBy state machine, MCP_TaskCreated handler, collapseTask child guard, hasActiveCoordinator, MCP startup transitions (pending/ready/error/retry/child-failure), MCP_TaskCleanupFailed, closeTask IPC ordering
src/store/notifications.test.ts	3 new — staged notification replace-on-arrival, userEdited per-notification reset, clearStagedNotification
src/store/taskStatus.test.ts	4 new — getTaskStatus/getTaskDotStatus return 'review' for needsReview; busy overrides review
src/store/persistence.test.ts	Pruned stale cases; updated for coordinator fields
electron/mcp/docker.integration.test.ts	UUID fix: coordinatorTaskId changed from 'coord-1' to a valid UUID constant

---

Assumptions and important notes

1. PR 2 prerequisite — coordinator-3 removed its own copies of getCoordinatorChildren/isCoordinatedChild from sidebar-order.ts because PR 2 provides them. Without PR 2, the app will not typecheck.
2. MCP polling side effect — mcpStatus.ts polls every 3 s as soon as this lands, but returns empty status until a coordinator task exists.
3. Session restore timing — StartMCPServer calls fire in parallel for all coordinator tasks on startup. Each child stays mcpStartupStatus: 'pending' until hydration completes. The UI for that state lives in PR 4.
4. Docker coordinator restore — Container names are reconstructed from agentId (parallel-code-{agentId.slice(0,12)}). The container is likely dead after restart; wiring exists so the user can manually restart the coordinator agent.
5. Feature is dark — coordinatorModeEnabled defaults to false. No user-visible change until PR 4 adds the Settings checkbox.

🤖 Generated with Claude Code

[johannesjo]

Reviewed the stacked delta from #120 (b43a427) to this PR head (5808195) with multiple passes. I think these need attention before this lands:

1. Blocking: MCP-created sub-tasks can be double-spawned and lose their MCP config. The backend writes the per-sub-task MCP config and spawns the agent with --mcp-config in electron/mcp/coordinator.ts:626-666, then emits MCP_TaskCreated. The renderer creates a normal Agent for the same agentId in src/store/tasks.ts:999-1045 without attachExisting, and TerminalView immediately calls SpawnAgent. In electron/ipc/pty.ts:205-242, a same-id existing PTY is only attached when attachExisting is true; otherwise it is killed and replaced. That replacement uses the renderer args, which omit --mcp-config, so signal_done / sub-task MCP tooling can break as soon as the child renders.

2. Blocking: restore marks MCP tasks pending, but rendering does not gate terminal spawn on it. loadState() sets coordinator/coordinated tasks to mcpStartupStatus: 'pending' because the MCP config has stale session tokens (src/store/persistence.ts:635-645), and App.tsx rewrites/restores those configs afterward (src/App.tsx:338-383). But TaskPanel still creates TaskAITerminal unconditionally and TerminalView starts spawning immediately (src/components/TaskPanel.tsx:215-236, src/components/TerminalView.tsx:625-671). I could not find any render-path consumer of mcpStartupStatus, so restored agents can start before StartMCPServer / hydration completes.

3. Medium: MCP-created child tasks lose baseBranch in the renderer state. The backend records the sub-task baseBranch, but the MCP_TaskCreated payload does not include it (electron/mcp/coordinator.ts:694-708), and the renderer-created Task does not set it (src/store/tasks.ts:999-1020). UI merges and restart hydration later pass task.baseBranch, so these sub-tasks can merge/diff against the wrong inferred base after creation or restore.

4. Medium: concurrent coordinator restore can race remote server startup. App.tsx restores all persisted coordinators in parallel with Promise.allSettled (src/App.tsx:346-383). Each StartMCPServer checks if (!remoteServer) before awaiting startRemoteServerOnFreePort (electron/ipc/register.ts:1389-1421). Two restored coordinators can both observe remoteServer === null, start separate servers, and leave only the later one in remoteServer for status/cleanup.

5. Medium: autosave no longer watches some fields that saveState() persists. saveState() persists agentDef(s), agentIds, selectedAgentId, promptedAgentIds, and initialPrompt, but src/store/autosave.ts:45-76 removed those from the snapshot. Paths such as switchAgent() and selected-agent changes do not call saveState() directly, so those changes can be lost on restart unless another watched field changes afterward.

6. Low/cleanup: MCP status shape is inconsistent. src/store/types.ts:224-232 defines { running, port, coordinatorTaskId, mcpConfigPath }, while GetMCPStatus and the offline fallback still use the older { mcpRunning, remoteRunning, coordinatorRoutesAttached, coordinatorRegistered, serverUrl, mcpConfigPath } shape (electron/ipc/register.ts:1615-1627, src/store/mcpStatus.ts:15-29). Any future UI reading the new fields will see undefined after refresh.

[brooksc]

All six issues addressed — in the order you listed them:

1. Blocking: double-spawn loses MCP config

Added attachExisting: true to the Agent object created in the MCP_TaskCreated handler (src/store/tasks.ts). When this flag is set, electron/ipc/pty.ts reattaches to the existing PTY the backend already spawned (with --mcp-config) rather than killing and replacing it with a renderer-args-only spawn.

2. Blocking: restore spawns terminals before MCP is ready

TerminalView now gates spawn on mcpStartupStatus. When the task has an mcpStartupStatus field (coordinator/coordinated tasks), a createEffect watches for 'ready' before calling startSpawn(), and falls through to the normal immediate-spawn path for non-MCP tasks. The 'error' state is left to the overlay rendered outside onMount. This means restored agents wait for StartMCPServer/hydration to complete before any PTY is touched.

3. Medium: child tasks lose baseBranch

Added baseBranch to the MCP_TaskCreated IPC payload in electron/mcp/coordinator.ts, the MCPTaskCreatedEvent interface, and the Task construction in src/store/tasks.ts. Sub-tasks now carry the correct base from creation through merge/diff operations.

4. Medium: concurrent coordinator restore races remote server

Added a remoteServerStarting: Promise<void> | null guard in electron/ipc/register.ts. The first StartMCPServer call creates the promise; concurrent callers from Promise.allSettled await the same promise instead of each racing through the if (!remoteServer) check. A finally block clears the guard so future calls after the server stops work normally. Added a post-await null assertion so TypeScript narrows correctly.

5. Medium: autosave snapshot missing persisted fields

src/store/autosave.ts now includes agentIds, selectedAgentId, initialPrompt, promptedAgentIds, agentDef, and agentDefs in the per-task snapshot, and adds a top-level agents snapshot that captures agent def objects. This makes switchAgent() and initial-prompt changes trigger the debounced autosave instead of being silently dropped until an unrelated field changes.

6. Low: MCP status shape inconsistency

GetMCPStatus in electron/ipc/register.ts now returns { running, port, coordinatorTaskId, mcpConfigPath } matching the MCPStatus type. MCP_STATUS_OFFLINE in src/store/mcpStatus.ts updated to the same shape. coordinatorTaskId is null for now (the coordinator's internal map is private); can be surfaced later if the UI needs it.

---

Also cherry-picked two commits from integration that belonged in this PR: fix(status-dot): show review before busy, add tooltips and chore(debug): add spawn/kill/attach logging to pty and TerminalView (the TerminalView conflict was resolved by taking the more robust version from integration that only fires on explicit 'ready' and preserves the immediate-spawn path for non-MCP tasks).

[brooksc]

Additional fixes pushed in ff080ee:

• Restored forced auto-trust for coordinator-controlled subtasks with skipPermissions, even when global autoTrustFolders is disabled.
• Kept human-controlled coordinator subtasks out of forced auto-trust.
• Passed agent/task context through PromptInput auto-send question blocking so trust dialogs handled by forced coordinator auto-trust do not stall initial prompt delivery.
• Persisted and restored preambleFileExistedBefore for active and collapsed tasks so restart/merge cleanup preserves pre-existing AGENTS.md/GEMINI.md files correctly.
• Added regression coverage for forced coordinator auto-trust and preamble provenance persistence.

Verification:

• npm test -- src/store/persistence.test.ts src/store/taskStatus.test.ts
• npm run typecheck
• npm run lint
• npm run format:check
• commit/push hooks also ran npm run check successfully.

[johannesjo]

Hey hey! Thank you very much! Looks good to merge, after the conflicts are resolved.

A few optional minor nits:

Reviewed ff080ee9 against your six points – all addressed. A few minor / optional nits I noticed while verifying:

• src/store/tasks.ts:1057 — markAgentSpawned(evt.agentId) runs after the produce block on every MCP_TaskCreated event. The duplicate-event early return inside produce does not guard it. If markAgentSpawned accumulates state, duplicate deliveries would double-fire. Move inside the guard or confirm idempotency.

• src/store/tasks.ts:1024 — mcpStartupStatus: 'ready' as const on backend-spawned subtasks bypasses the gating mechanism added for fix Add evaluation of Electron migration for xterm.js performance #2. Correct by design (backend already spawned with --mcp-config), but a one-line comment would future-proof against someone changing the gating contract later.

• electron/ipc/register.ts:1638 — coordinatorTaskId: null is the documented placeholder. A // TODO: surface from coordinator map if UI needs it would prevent the next reader assuming it's unsupported.

• src/store/autosave.ts:60-77 — preambleFileExistedBefore is persisted by saveState() (persistence.ts:178,234) but absent from the autosave snapshot. Low probability in practice (field is set alongside other tracked fields at task creation), but a late mutation to this field alone would not trip the debounced save. Adding preambleFileExistedBefore: t.preambleFileExistedBefore to the per-task snapshot would close the gap.

None block merge — just drive-by polish.

[brooksc]

Pushed 13e5bcc to address the optional polish nits from the last review comment and rechecked the branch.

What changed:

1. src/store/tasks.ts: duplicate MCP_TaskCreated events now fully no-op after the existing task guard. markAgentSpawned() and rescheduleTaskStatusPolling() only run when the task was actually created, so duplicate deliveries cannot reset agent activity state or double-fire polling.
2. src/store/tasks.test.ts: added a regression test proving duplicate MCP_TaskCreated events create one task and call markAgentSpawned() / rescheduleTaskStatusPolling() once.
3. src/store/tasks.ts: added the requested comment explaining why backend-spawned MCP child tasks start with mcpStartupStatus: 'ready' while restored MCP tasks still go through the pending-to-ready hydration gate.
4. electron/ipc/register.ts: added the TODO on the placeholder coordinatorTaskId: null in GetMCPStatus.
5. src/store/autosave.ts: added preambleFileExistedBefore to the autosave snapshot so a future isolated mutation of that persisted field will trigger the debounced save path.

Verification:

• npm test -- src/store/tasks.test.ts
• npm run typecheck
• npm run lint
• npm run format:check
• commit hook ran npm run check
• pre-push hook ran npm run check

I believe all issues raised in the latest comment are now fully addressed.

[johannesjo]

Great work @brooksc ! Could you have a look at the conflicts /merge master into the branch?

[johannesjo]

Ah I think we already merged it as part of the other PRs? @brooksc can you maybe double check?

[brooksc]

Double-checked this after rebasing coordinator-3-store-ipc onto current main.

The original PR124 stack has effectively already landed through the later coordinator stack / PRs on main. After the rebase, this PR is reduced to one small defensive guard in src/store/tasks.ts: child MCP retry refuses to hydrate while its coordinator is still marked pending.

That guard is valid, but it only protects a narrow manual retry race. The normal restore path already starts coordinators first, waits for coordinator restore promises, and only hydrates children whose coordinator is ready, so this is not worth keeping PR124 open as a separate PR.

Closing this PR as superseded by the later coordinator work on main.

[brooksc]

Closing as superseded by the later coordinator stack already merged to main.