Skip to content

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

Open
brooksc wants to merge 2 commits into
johannesjo:mainfrom
brooksc:coordinator-3-store-ipc
Open

feat(coordinator): frontend store wiring, IPC lifecycle, and session restore (PR 3/4)#124
brooksc wants to merge 2 commits into
johannesjo:mainfrom
brooksc:coordinator-3-store-ipc

Conversation

@brooksc
Copy link
Copy Markdown
Contributor

@brooksc brooksc commented May 17, 2026

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 effectmcpStatus.ts polls every 3 s as soon as this lands, but returns empty status until a coordinator task exists.
  3. Session restore timingStartMCPServer 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 darkcoordinatorModeEnabled defaults to false. No user-visible change until PR 4 adds the Settings checkbox.

🤖 Generated with Claude Code

brooksc and others added 2 commits May 16, 2026 23:09
…task model

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

Wires the coordinator MCP engine (PR 2) into the frontend store and
Electron IPC layer. Feature is dark by default — coordinatorModeEnabled
defaults to false; nothing is user-visible until PR 4 adds the
Settings checkbox and NewTaskDialog entry point.

Key additions:
- Task model: coordinatorMode, coordinatedBy, controlledBy, stagedNotification,
  mcpStartupStatus, signalDone*, needsReview, and 3 global coordinator fields
- initMCPListeners(): subscribes to 7 MCP push events (TaskCreated, TaskClosed,
  CleanupFailed, NotificationStaged/Cleared/Orphaned, TaskStateSync, TaskHydrated)
- MCP startup state machine: markTaskMcpPending/Ready/Error, retryTaskMcpStartup
- setTaskControl(): coordinator ↔ human hand-off; collapseTask guard for children
- closeTask(): calls MCP_CoordinatedTaskClosed / MCP_CoordinatorDeregistered on close
- Session restore: App.tsx calls StartMCPServer for each persisted coordinator task
  on startup, rewriting .mcp.json with the new session port/token; children start
  pending until MCP_HydrateCoordinatedTask returns
- Persistence: all coordinator fields round-tripped through saveState/loadState;
  tasks with coordinatorMode/coordinatedBy load with mcpStartupStatus pending
- coordinator-preamble.ts: system preamble injected into coordinator agent prompts
- mcpStatus.ts: polls GetMCPStatus every 3 s; inert when no coordinator is active
- Deregister fix: suppresses spurious needsReview notification for children that
  never received their prompt (assignedPromptDelivered false)
- Tests: ~40 new cases across tasks, notifications, taskStatus, persistence

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@johannesjo
Copy link
Copy Markdown
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants