From 4a4d8cd81de4641fb41b02f19ef083520ab0f766 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Fri, 25 Sep 2026 18:34:51 +0200 Subject: [PATCH 01/10] docs(architecture): revise runtime separation plan --- docs/runtime-separation-plan.md | 52 ++++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 17 deletions(-) diff --git a/docs/runtime-separation-plan.md b/docs/runtime-separation-plan.md index e3267ec5e..0fc2c5c4b 100644 --- a/docs/runtime-separation-plan.md +++ b/docs/runtime-separation-plan.md @@ -1,19 +1,21 @@ # Runtime separation plan -Status: revised after three reviews (two focused, one adversarial). Steps 0 and 1 are done. Everything else is a proposal. +Status: revised after three reviews (two focused, one adversarial) and re-checked against the code on 2026-09-25. Steps 0 and 1 are done. Everything else is a proposal. References name functions rather than line numbers, because line numbers drift. ## Why The desktop renderer is the source of truth for tasks, and the main process co-owns some of their state: -- Agent processes are spawned when `TerminalView` mounts (`src/components/TerminalView.tsx:1137`). Agent terminals pass `preserveSessionOnCleanup` (`src/components/TaskAITerminal.tsx:916`), so unmounting a view does not kill its agent. After a renderer reload, main reattaches the running PTY and replays its scrollback (`electron/ipc/pty.ts:726`). Agents are killed on purpose when a task is collapsed or closed (`src/store/tasks.ts:986`). -- The renderer persists task state as one JSON blob (`src/store/persistence.ts:429`). Main also writes that file: it normalizes every renderer save and does its own read-modify-write for delegation state (`electron/ipc/register.ts:642`, `:1220`; `electron/mcp/delegation.ts:912`). -- The phone and MCP agents reach task, reasoning, mind-map and notes state through the renderer (`callRenderer`, `electron/ipc/register.ts:1739`), with a 120-second timeout. -- Tasks are created three ways. Desktop and phone tasks go through the renderer store (`src/store/tasks.ts:257`). Coordinator tasks are created in main (`electron/mcp/coordinator.ts:1183`). Every path provisions the worktree before the task is durably saved: autosave is debounced by 1–5 s (`src/store/autosave.ts:134`), and coordinator children are saved only after the renderer adopts them. Nothing reconciles worktrees at startup, so a crash in that window leaves an orphaned worktree. -- Question detection and trust-dialog acceptance run in the renderer (`src/store/taskStatus.ts:705`). The coordinator has a separate readiness monitor (`electron/mcp/coordinator.ts:597`). -- Only xterm in the renderer answers terminal queries (`TerminalView.tsx:955`). Codex exits if its cursor-position query goes unanswered for about 2 s (commit `1204de82`, patched by disabling background throttling in `electron/main.ts:196`). Coordinator sub-tasks are spawned in main before any view exists (`coordinator.ts:1356`), so they rely on a view mounting in time. **This is the only user-facing bug in the history that this ownership split has caused.** +- Agent processes are spawned when `TerminalView` mounts (`startSpawn` in `src/components/TerminalView.tsx`). Agent terminals pass `preserveSessionOnCleanup` (`src/components/TaskAITerminal.tsx`), so unmounting a view does not kill its agent. After a renderer reload, main reattaches the running PTY and replays its scrollback (`attachExisting` in `spawnAgent`, `electron/ipc/pty.ts`). Agents are killed on purpose when a task is collapsed or closed (`collapseTask` and `closeTask` in `src/store/tasks.ts`). +- The renderer persists task state as one JSON blob (`saveState` in `src/store/persistence.ts`). Main also writes that file: it normalizes every renderer save and does its own read-modify-write for delegation state (the `IPC.SaveAppState` handler and the delegation `persist` callback in `electron/ipc/register.ts`; `normalizeState` in `electron/mcp/delegation.ts`). +- The phone and MCP agents reach task, reasoning, mind-map and notes state through the renderer (`callRenderer` in `electron/ipc/register.ts`), with a 120-second timeout. It backs 10 operations: reasoning and mind-map reads and updates, opening the canvas, publishing tours, and the phone's projects, task creation and notes. +- Tasks are created three ways. Desktop and phone tasks go through the renderer store (`createTask` in `src/store/tasks.ts`). Coordinator tasks are created in main (`createTaskUnchecked` in `electron/mcp/coordinator.ts`). Every path provisions the worktree before the task is durably saved: autosave is debounced by 1–5 s (`AUTOSAVE_DEBOUNCE_MS` in `src/store/autosave.ts`), and coordinator children are saved only after the renderer adopts them. Nothing reconciles worktrees at startup, so a crash in that window leaves an orphaned worktree. +- Document workspaces create worktrees in the same `.worktrees/` directory as tasks (`prepareAlternateWorktree` in `electron/documents/runs.ts`). Their records live in document-run state, not `state.json`. +- Question detection and trust-dialog acceptance run in the renderer (`looksLikeQuestion` and `tryAutoTrust` in `src/store/taskStatus.ts`). The coordinator has a separate readiness monitor (`createAgentOutputMonitor` and `scheduleInitialPromptDelivery` in `electron/mcp/coordinator.ts`, a 3,300-line file). +- Until step 1, only xterm in the renderer answered terminal queries. Codex exits if its cursor-position query goes unanswered for about 2 s (commit `1204de82`, patched by disabling `backgroundThrottling` in `electron/main.ts`). Coordinator sub-tasks are spawned in main before any view exists, so they relied on a view mounting in time. This is the only user-facing bug we found in the history that the ownership split caused; the search was by commit message, so others may exist. Step 1 fixed it. +- The coordinator creates and launches agents without the `SpawnAgent` IPC handler, so its children skip canvas MCP, the watchers and admission. This correctness gap remains open; step 4 closes it. - `registerAllHandlers` is a single closure of about 2,100 lines that also runs the remote-server, coordinator and delegation lifecycles. -- Precedent: document workspaces already run headless agents in worktrees from main, with their own records and `reconcileInterrupted` (`electron/documents/runs.ts:513`). Use them as the template for runtime-owned tasks. +- Precedent: document workspaces already run headless agents in worktrees from main, with their own records and `reconcileInterrupted` (`electron/documents/runs.ts`). Use them as the template for runtime-owned tasks. ## Target @@ -44,8 +46,15 @@ This is static only and makes the existing renderer/main rule mean what it says. Each of these changes runtime behavior and needs a smoke test in the real app. 1. **Terminal-query responder (done).** Every PTY session keeps a headless xterm mirror in main (`electron/ipc/terminal-query-responder.ts`), fed the process output and resized with the PTY. It answers cursor-position queries (`CSI 6 n`, `CSI ? 6 n`) itself. The renderer's xterm no longer answers them (`src/lib/terminalQueries.ts`), so there is exactly one answer, whether a view is mounted, hidden, reloading or blocked by an automation write. This design was chosen over attach/detach tracking because it also covers hidden, throttled views and stale replies to replayed scrollback. Banners and replayed scrollback are parsed without answering, so the mirror's cursor matches the pane. Device-attribute, color and keyboard queries still come from the renderer, because the answers depend on its theme and settings and their order matters. Verified: a real-PTY program that exits when its cursor query goes unanswered for 2 s runs with no renderer; with the reply disabled, it fails. -2. **Startup worktree reconciliation.** Compare the managed worktrees from `git worktree list` with the tasks in `state.json`, and write an intent record before provisioning. Report orphans rather than deleting them, because they may hold user work. This covers all three creation paths. → verify: a unit test kills the process between provisioning and save, and the orphan is reported on the next start. -3. **Split `registerAllHandlers` and add the runtime boundary.** Wait for #279 (pooled workspaces, which changes task creation) to be decided, and announce a freeze window: 7 open PRs touch `register.ts`, with hunks across the whole file. Before the split: +2. **Startup worktree reconciliation.** Compare the worktrees under each project's `.worktrees/` from `git worktree list` with every known owner: the tasks in `state.json`, document-run records, and intent records. Report orphans rather than deleting them, because they may hold user work. This covers all three creation paths. + - Match by worktree path, not branch. A task's branch can change after creation when the app adopts a branch the agent switched to. + - Write an intent record before provisioning and remove it once the task is durably saved. Main owns this file, separate from the renderer's `state.json`, and writes it with the helpers in `electron/mcp/atomic.ts`. Design it as the first part of the runtime task registry (step 5), not a separate store. + - → verify: a unit test kills the process between provisioning and save, and the orphan is reported on the next start. A second test confirms that document-run worktrees and tasks with an adopted branch are not reported. +3. **Runtime boundary, then split `registerAllHandlers`.** Two independent changes; land 3a first. + + **3a. Notify port.** Replace `BrowserWindow` in `pty.ts`, `git.ts`, `plans.ts`, `steps.ts` and the coordinator (17 references) with a notify port. This barely touches `register.ts`, so it needs no freeze, and step 4 depends on it. → verify: `npm run check`, the existing tests for those modules, and `rg BrowserWindow` shows no matches in them. + + **3b. Split `register.ts`.** #279 (pooled workspaces) was closed without merging on 2026-09-25, so it no longer blocks this. 5 of the 12 open PRs touch `register.ts` (#262, #248, #247, #162, #23). Do the split between merges, after announcing a short freeze; it does not block steps 4 and later. Before the split: - Commit a wiring test against the old code. It records every `ipcMain.handle` channel, throws on duplicates, asserts the `win.on` events and exactly one registration-time `onPtyEvent('exit')`, and triggers `ensureCoordinator` twice so the lazily registered `MCP_*` handlers are seen exactly once. Design rules from review: @@ -61,19 +70,28 @@ Each of these changes runtime behavior and needs a smoke test in the real app. - Keep new files flat in `electron/ipc/` because of the `import.meta.url` path depth. - Review the diff with `git diff --color-moved`. - Runtime boundary: replace `BrowserWindow` in `pty.ts`, `git.ts`, `plans.ts`, `steps.ts` and the coordinator with a notify port first. Then add `electron/runtime/` with a dependency-cruiser rule that uses `reachable: true`, because a direct-import rule misses transitive Electron imports. + After the split, add `electron/runtime/` with a dependency-cruiser rule that uses `reachable: true`, because a direct-import rule misses transitive Electron imports. → verify: the wiring test passes unchanged, `npm run check:static`, and a smoke test of desktop, phone and coordinator flows in the real app. + +4. **Main-side agent launch.** Extract the `SpawnAgent` handler body into `launchAgent()`, used by both IPC and the coordinator. Today the coordinator bypasses canvas MCP, the watchers and admission. Move `buildTaskAgentArgs` into shared code; it takes a renderer `Task` and imports from `src/documents/`, so it first needs a narrower input type. Merge the renderer's question and trust detection with the coordinator's output monitor into one main-side `PromptDeliverer` instead of adding a third. It could read the step 1 terminal mirror's screen instead of the raw output tail; this is untested. + + Decide before starting: + - **Agent lifetime.** Define what collapsing a task means once main can launch agents without a view. Steps 6 and 7 build on this answer. + - **Settings source.** Trust acceptance depends on `autoTrustFolders`, which exists only in the renderer store and its saved state. Choose how main reads it: sent over IPC when it changes, or owned by the runtime. + + → verify: a unit test shows a coordinator child launched through `launchAgent()` gets canvas MCP, the watchers and admission. The existing question and trust tests pass against `PromptDeliverer`. Smoke-test a desktop task and a coordinator child in the real app. + + **Checkpoint.** Stop here and reassess steps 5–7 against a named product goal, such as the phone creating tasks while the renderer is not running. Steps 2–4 close both known correctness gaps. -4. **Main-side agent launch.** Extract the `SpawnAgent` handler body into `launchAgent()`, used by both IPC and the coordinator. Today the coordinator bypasses canvas MCP, the watchers and admission. Move `buildTaskAgentArgs` into shared code. Merge the renderer's question and trust detection with the coordinator's output monitor into one main-side `PromptDeliverer` instead of adding a third. Trust acceptance depends on the `autoTrustFolders` setting, which main must be able to read. -5. **Runtime task registry.** Main writes task records it creates and reconciles them at startup. It extends the existing `normalizeState` overlay rather than adding a second writer with no ordering. -6. **One task-creation path.** `TaskRuntime.createTask(spec)` provisions, registers authority, launches, delivers the prompt, records the task and emits `TaskCreated`. The renderer adopts the task with `attachExisting`, generalizing `MCP_TaskCreated`. The phone stops using `callRenderer` for creation. -7. **Runtime-owned task state.** Split `state.json` into runtime task state and UI layout, with versioning and a downgrade path. Remove the remaining `callRenderer` uses. Define what collapsing a task means once the runtime owns agent lifetime. +5. **Runtime task registry.** Main writes task records it creates and reconciles them at startup. It grows out of the step 2 intent records, and its effect on `state.json` goes through the existing `normalizeState` overlay rather than a second writer with no ordering. → verify: a coordinator child survives a crash before the renderer adopts it and reappears on the next start. +6. **One task-creation path.** `TaskRuntime.createTask(spec)` provisions, registers authority, launches, delivers the prompt, records the task and emits `TaskCreated`. The renderer adopts the task with `attachExisting`, generalizing `MCP_TaskCreated`. The phone stops using `callRenderer` for creation. → verify: one test per origin (desktop, phone, coordinator) goes through `createTask` and ends with the same durable record and `TaskCreated` event. +7. **Runtime-owned task state.** Split `state.json` into runtime task state and UI layout, with versioning and a downgrade path. Remove the remaining `callRenderer` uses. This is the riskiest step and no current requirement needs it. Do it only when a named goal does, as with step 8. → verify: a migration test round-trips old state through upgrade and downgrade, and phone and MCP operations work with no window open. 8. **Separate runtime process.** Only needed if agents must survive app exit or update, the phone must work without Electron, or execution moves to another machine. ## Deferred: typed IPC contracts `invoke` lets every caller choose its result type, but the history shows no bugs from mismatched types. The task store has about 7 explicit `invoke` calls and already uses the shared result types. A contract on the renderer side only moves the casts into one file. -Do this only together with a compile-only `handleContract` on the main side (after step 3), so that both ends are checked. Design notes from review: +Do this only together with a compile-only `handleContract` on the main side (after step 3b), so that both ends are checked. Design notes from review: - Mocked `invoke` takes the last overload's types, so client tests need a `LooseInvoke` type. - `fireAndForget` and `CHANNELS` in `src/store/usage.ts` pass variables of type `IPC`, so a private `invokeUnchecked` is needed. @@ -84,6 +102,6 @@ Do this only together with a compile-only `handleContract` on the main side (aft ## Alternatives considered -- **Split only `register.ts`.** Cheap, but it leaves every ownership problem in place and conflicts with 7 open PRs. It is kept as step 3. +- **Split only `register.ts`.** Cheap, but it leaves every ownership problem in place and conflicts with 5 open PRs. It is kept as step 3b. - **Move straight to a separate runtime process.** That adds reconnect, shutdown and protocol work before ownership is untangled. It is deferred to step 8. - **Move shared contracts out of `electron/`.** `electron/shared/` already acts as the shared package (21 of the 31 `electron/` modules imported by `src/` live there). Moving everything would touch about 180 files for little gain. From 45ceeb6ca7e73d8ed41cda31344df6c430d37cb1 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Fri, 25 Sep 2026 19:04:07 +0200 Subject: [PATCH 02/10] feat(worktrees): report worktrees orphaned by interrupted task creation --- docs/runtime-separation-plan.md | 15 ++-- electron/ipc/git.ts | 9 +- electron/ipc/register.ts | 4 +- electron/ipc/tasks.test.ts | 52 +++++++++++ electron/ipc/tasks.ts | 9 +- electron/ipc/worktree-intents.test.ts | 105 ++++++++++++++++++++++ electron/ipc/worktree-intents.ts | 124 ++++++++++++++++++++++++++ electron/main.ts | 17 +++- 8 files changed, 324 insertions(+), 11 deletions(-) create mode 100644 electron/ipc/tasks.test.ts create mode 100644 electron/ipc/worktree-intents.test.ts create mode 100644 electron/ipc/worktree-intents.ts diff --git a/docs/runtime-separation-plan.md b/docs/runtime-separation-plan.md index 0fc2c5c4b..0a0c33046 100644 --- a/docs/runtime-separation-plan.md +++ b/docs/runtime-separation-plan.md @@ -1,6 +1,6 @@ # Runtime separation plan -Status: revised after three reviews (two focused, one adversarial) and re-checked against the code on 2026-09-25. Steps 0 and 1 are done. Everything else is a proposal. References name functions rather than line numbers, because line numbers drift. +Status: revised after three reviews (two focused, one adversarial) and re-checked against the code on 2026-09-25. Steps 0, 1 and 2 are done. Everything else is a proposal. References name functions rather than line numbers, because line numbers drift. ## Why @@ -9,7 +9,7 @@ The desktop renderer is the source of truth for tasks, and the main process co-o - Agent processes are spawned when `TerminalView` mounts (`startSpawn` in `src/components/TerminalView.tsx`). Agent terminals pass `preserveSessionOnCleanup` (`src/components/TaskAITerminal.tsx`), so unmounting a view does not kill its agent. After a renderer reload, main reattaches the running PTY and replays its scrollback (`attachExisting` in `spawnAgent`, `electron/ipc/pty.ts`). Agents are killed on purpose when a task is collapsed or closed (`collapseTask` and `closeTask` in `src/store/tasks.ts`). - The renderer persists task state as one JSON blob (`saveState` in `src/store/persistence.ts`). Main also writes that file: it normalizes every renderer save and does its own read-modify-write for delegation state (the `IPC.SaveAppState` handler and the delegation `persist` callback in `electron/ipc/register.ts`; `normalizeState` in `electron/mcp/delegation.ts`). - The phone and MCP agents reach task, reasoning, mind-map and notes state through the renderer (`callRenderer` in `electron/ipc/register.ts`), with a 120-second timeout. It backs 10 operations: reasoning and mind-map reads and updates, opening the canvas, publishing tours, and the phone's projects, task creation and notes. -- Tasks are created three ways. Desktop and phone tasks go through the renderer store (`createTask` in `src/store/tasks.ts`). Coordinator tasks are created in main (`createTaskUnchecked` in `electron/mcp/coordinator.ts`). Every path provisions the worktree before the task is durably saved: autosave is debounced by 1–5 s (`AUTOSAVE_DEBOUNCE_MS` in `src/store/autosave.ts`), and coordinator children are saved only after the renderer adopts them. Nothing reconciles worktrees at startup, so a crash in that window leaves an orphaned worktree. +- Tasks are created three ways. Desktop and phone tasks go through the renderer store (`createTask` in `src/store/tasks.ts`). Coordinator tasks are created in main (`createTaskUnchecked` in `electron/mcp/coordinator.ts`). Every path provisions the worktree before the task is durably saved: autosave is debounced by 1–5 s (`AUTOSAVE_DEBOUNCE_MS` in `src/store/autosave.ts`), and coordinator children are saved only after the renderer adopts them. Before step 2, nothing reconciled worktrees at startup, so a crash in that window left an orphaned worktree that nothing reported. - Document workspaces create worktrees in the same `.worktrees/` directory as tasks (`prepareAlternateWorktree` in `electron/documents/runs.ts`). Their records live in document-run state, not `state.json`. - Question detection and trust-dialog acceptance run in the renderer (`looksLikeQuestion` and `tryAutoTrust` in `src/store/taskStatus.ts`). The coordinator has a separate readiness monitor (`createAgentOutputMonitor` and `scheduleInitialPromptDelivery` in `electron/mcp/coordinator.ts`, a 3,300-line file). - Until step 1, only xterm in the renderer answered terminal queries. Codex exits if its cursor-position query goes unanswered for about 2 s (commit `1204de82`, patched by disabling `backgroundThrottling` in `electron/main.ts`). Coordinator sub-tasks are spawned in main before any view exists, so they relied on a view mounting in time. This is the only user-facing bug we found in the history that the ownership split caused; the search was by commit message, so others may exist. Step 1 fixed it. @@ -46,10 +46,13 @@ This is static only and makes the existing renderer/main rule mean what it says. Each of these changes runtime behavior and needs a smoke test in the real app. 1. **Terminal-query responder (done).** Every PTY session keeps a headless xterm mirror in main (`electron/ipc/terminal-query-responder.ts`), fed the process output and resized with the PTY. It answers cursor-position queries (`CSI 6 n`, `CSI ? 6 n`) itself. The renderer's xterm no longer answers them (`src/lib/terminalQueries.ts`), so there is exactly one answer, whether a view is mounted, hidden, reloading or blocked by an automation write. This design was chosen over attach/detach tracking because it also covers hidden, throttled views and stale replies to replayed scrollback. Banners and replayed scrollback are parsed without answering, so the mirror's cursor matches the pane. Device-attribute, color and keyboard queries still come from the renderer, because the answers depend on its theme and settings and their order matters. Verified: a real-PTY program that exits when its cursor query goes unanswered for 2 s runs with no renderer; with the reply disabled, it fails. -2. **Startup worktree reconciliation.** Compare the worktrees under each project's `.worktrees/` from `git worktree list` with every known owner: the tasks in `state.json`, document-run records, and intent records. Report orphans rather than deleting them, because they may hold user work. This covers all three creation paths. - - Match by worktree path, not branch. A task's branch can change after creation when the app adopts a branch the agent switched to. - - Write an intent record before provisioning and remove it once the task is durably saved. Main owns this file, separate from the renderer's `state.json`, and writes it with the helpers in `electron/mcp/atomic.ts`. Design it as the first part of the runtime task registry (step 5), not a separate store. - - → verify: a unit test kills the process between provisioning and save, and the orphan is reported on the next start. A second test confirms that document-run worktrees and tasks with an adopted branch are not reported. +2. **Startup worktree reconciliation (done).** `createTask` in `electron/ipc/tasks.ts`, which desktop, phone and coordinator tasks all go through, writes an intent to a main-owned journal (`worktree-intents.json` in the user data directory, written with `atomicWriteFileSync`) before provisioning. The `SaveAppState` handler drops intents once a saved state claims their worktree path. At startup, before the window exists, `reconcileWorktreeIntents` (`electron/ipc/worktree-intents.ts`) reports every intent whose worktree still exists but no saved task claims. Nothing is deleted, because the worktree may hold user work. An orphan is reported on each start until its worktree is removed. + - Journal rather than a `git worktree list` scan: a dev run and an installed build share repositories but not state, and Arena and document-run worktrees share `.worktrees/` without being tasks. A scan would report all of them. The journal reports only this instance's own interrupted creations. + - Matching is by worktree path, so a later branch adoption does not matter. + - A journal read or write failure is logged and never blocks startup, task creation or saving. + - Orphans are logged under the `worktrees` category only. Showing them in the UI is follow-up work. + - Limits: it cannot find orphans created before the journal existed, or worktrees left behind for other reasons, such as a failed removal. + - Verified: `electron/ipc/worktree-intents.test.ts` simulates a crash between provisioning and save and sees the orphan on the next start. `electron/ipc/tasks.test.ts` checks that the intent is written before `createWorktree` runs; it fails if the order is reversed. The journal is intended as the first part of the step 5 registry. 3. **Runtime boundary, then split `registerAllHandlers`.** Two independent changes; land 3a first. **3a. Notify port.** Replace `BrowserWindow` in `pty.ts`, `git.ts`, `plans.ts`, `steps.ts` and the coordinator (17 references) with a notify port. This barely touches `register.ts`, so it needs no freeze, and step 4 depends on it. → verify: `npm run check`, the existing tests for those modules, and `rg BrowserWindow` shows no matches in them. diff --git a/electron/ipc/git.ts b/electron/ipc/git.ts index 64268a968..b80df437a 100644 --- a/electron/ipc/git.ts +++ b/electron/ipc/git.ts @@ -905,6 +905,11 @@ async function removeDirWithRetries(dirPath: string): Promise { // --- Public functions (used by tasks.ts and register.ts) --- +/** Where `createWorktree` puts the worktree for a branch. */ +export function worktreePathFor(repoRoot: string, branchName: string): string { + return `${repoRoot}/.worktrees/${branchName}`; +} + export async function createWorktree( repoRoot: string, branchName: string, @@ -912,7 +917,7 @@ export async function createWorktree( baseBranch?: string, forceClean = false, ): Promise<{ path: string; branch: string }> { - const worktreePath = `${repoRoot}/.worktrees/${branchName}`; + const worktreePath = worktreePathFor(repoRoot, branchName); if (forceClean) { // Clean up stale worktree/branch from a previous session that wasn't properly removed @@ -1264,7 +1269,7 @@ export async function removeWorktree( // After the user adopts a branch the agent switched the worktree to, the // folder keeps its original branch-derived name — callers that know the real // path must pass it, deriving from branchName is only a fallback. - const worktreePath = explicitWorktreePath ?? `${repoRoot}/.worktrees/${branchName}`; + const worktreePath = explicitWorktreePath ?? worktreePathFor(repoRoot, branchName); if (!fs.existsSync(repoRoot)) return; diff --git a/electron/ipc/register.ts b/electron/ipc/register.ts index c9362bf37..f30acc8b6 100644 --- a/electron/ipc/register.ts +++ b/electron/ipc/register.ts @@ -116,6 +116,7 @@ import { getUncommittedFileDiffs, } from './git.js'; import { createTask, deleteTask } from './tasks.js'; +import { settleWorktreeIntents } from './worktree-intents.js'; import { listAgents } from './agents.js'; import { saveAppState, @@ -1220,7 +1221,8 @@ export function registerAllHandlers(win: BrowserWindow): void { assertString(args.json, 'json'); const json = delegation.normalizeState(args.json); syncTaskNamesFromJson(json); - return saveAppState(json); + saveAppState(json); + settleWorktreeIntents(json); }); ipcMain.handle(IPC.LoadAppState, () => { const json = loadAppState(); diff --git a/electron/ipc/tasks.test.ts b/electron/ipc/tasks.test.ts new file mode 100644 index 000000000..aa484ff0e --- /dev/null +++ b/electron/ipc/tasks.test.ts @@ -0,0 +1,52 @@ +import fs from 'fs'; +import os from 'os'; +import path from 'path'; + +import { afterEach, describe, expect, it, vi } from 'vitest'; + +const mocks = vi.hoisted(() => ({ createWorktree: vi.fn() })); + +vi.mock('./git.js', async (importOriginal) => ({ + ...(await importOriginal()), + createWorktree: mocks.createWorktree, +})); +vi.mock('./pty.js', () => ({ killAgent: vi.fn(), notifyAgentListChanged: vi.fn() })); +vi.mock('./plans.js', () => ({ stopPlanWatcher: vi.fn() })); +vi.mock('./steps.js', () => ({ stopStepsWatcher: vi.fn() })); + +import { createTask } from './tasks.js'; +import { reconcileWorktreeIntents } from './worktree-intents.js'; + +const tempDirs: string[] = []; + +afterEach(() => { + for (const dir of tempDirs.splice(0)) { + fs.rmSync(dir, { recursive: true, force: true }); + } +}); + +describe('createTask', () => { + it('journals the worktree before provisioning it', async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'parallel-code-tasks-')); + tempDirs.push(root); + const journal = path.join(root, 'worktree-intents.json'); + const projectRoot = path.join(root, 'repo'); + reconcileWorktreeIntents(journal, null); + + let journaledAtProvision: unknown[] = []; + mocks.createWorktree.mockImplementation(async (repoRoot: string, branch: string) => { + journaledAtProvision = JSON.parse(fs.readFileSync(journal, 'utf8')).intents; + return { path: `${repoRoot}/.worktrees/${branch}`, branch }; + }); + + const task = await createTask('Fix login', projectRoot, [], 'task'); + + expect(journaledAtProvision).toEqual([ + expect.objectContaining({ + worktreePath: task.worktree_path, + branchName: task.branch_name, + projectRoot, + }), + ]); + }); +}); diff --git a/electron/ipc/tasks.ts b/electron/ipc/tasks.ts index 6b394c2f2..beb6d803c 100644 --- a/electron/ipc/tasks.ts +++ b/electron/ipc/tasks.ts @@ -1,8 +1,9 @@ import { randomUUID } from 'crypto'; -import { createWorktree, removeWorktree } from './git.js'; +import { createWorktree, removeWorktree, worktreePathFor } from './git.js'; import { killAgent, notifyAgentListChanged } from './pty.js'; import { stopPlanWatcher } from './plans.js'; import { stopStepsWatcher } from './steps.js'; +import { recordWorktreeIntent } from './worktree-intents.js'; const MAX_SLUG_LEN = 72; @@ -41,6 +42,12 @@ export async function createTask( const id = randomUUID(); const prefix = sanitizeBranchPrefix(branchPrefix); const branchName = `${prefix}/${slug(name)}-${id.slice(0, 6)}`; + // Before provisioning, so a crash before the task is saved leaves a trace. + recordWorktreeIntent({ + worktreePath: worktreePathFor(projectRoot, branchName), + branchName, + projectRoot, + }); const worktree = await createWorktree( projectRoot, branchName, diff --git a/electron/ipc/worktree-intents.test.ts b/electron/ipc/worktree-intents.test.ts new file mode 100644 index 000000000..7c3cf245b --- /dev/null +++ b/electron/ipc/worktree-intents.test.ts @@ -0,0 +1,105 @@ +import fs from 'fs'; +import os from 'os'; +import path from 'path'; + +import { afterEach, describe, expect, it } from 'vitest'; + +import { + reconcileWorktreeIntents, + recordWorktreeIntent, + settleWorktreeIntents, +} from './worktree-intents.js'; + +const tempDirs: string[] = []; + +function setup() { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'parallel-code-worktree-intents-')); + tempDirs.push(root); + const worktree = path.join(root, 'repo/.worktrees/task/fix-a1b2c3'); + const intent = { + worktreePath: worktree, + branchName: 'task/fix-a1b2c3', + projectRoot: path.join(root, 'repo'), + }; + return { journal: path.join(root, 'user-data/worktree-intents.json'), worktree, intent }; +} + +/** Stands in for `git worktree add` having run. */ +function provision(worktree: string): void { + fs.mkdirSync(worktree, { recursive: true }); +} + +function savedState(...worktreePaths: string[]): string { + const tasks = Object.fromEntries(worktreePaths.map((p, i) => [`t${i}`, { worktreePath: p }])); + return JSON.stringify({ tasks }); +} + +afterEach(() => { + for (const dir of tempDirs.splice(0)) { + fs.rmSync(dir, { recursive: true, force: true }); + } +}); + +describe('worktree intent journal', () => { + it('reports a worktree provisioned before a crash that preceded the save', () => { + const { journal, worktree, intent } = setup(); + reconcileWorktreeIntents(journal, null); + recordWorktreeIntent(intent); + provision(worktree); + + // Crash: no save. Reopening the journal is what the next start does. + const orphans = reconcileWorktreeIntents(journal, savedState()); + + expect(orphans).toEqual([expect.objectContaining(intent)]); + }); + + it('keeps reporting an orphan on later starts until its worktree is removed', () => { + const { journal, worktree, intent } = setup(); + reconcileWorktreeIntents(journal, null); + recordWorktreeIntent(intent); + provision(worktree); + + expect(reconcileWorktreeIntents(journal, savedState())).toHaveLength(1); + expect(reconcileWorktreeIntents(journal, savedState())).toHaveLength(1); + + fs.rmSync(worktree, { recursive: true }); + expect(reconcileWorktreeIntents(journal, savedState())).toEqual([]); + }); + + it('forgets a worktree once a saved state claims it', () => { + const { journal, worktree, intent } = setup(); + reconcileWorktreeIntents(journal, null); + recordWorktreeIntent(intent); + provision(worktree); + + settleWorktreeIntents(savedState(worktree)); + + expect(reconcileWorktreeIntents(journal, savedState())).toEqual([]); + }); + + it('does not report a worktree the saved state claims when the settle was missed', () => { + const { journal, worktree, intent } = setup(); + reconcileWorktreeIntents(journal, null); + recordWorktreeIntent(intent); + provision(worktree); + + expect(reconcileWorktreeIntents(journal, savedState(worktree))).toEqual([]); + }); + + it('drops an intent whose worktree was never created', () => { + const { journal, intent } = setup(); + reconcileWorktreeIntents(journal, null); + recordWorktreeIntent(intent); + + expect(reconcileWorktreeIntents(journal, savedState())).toEqual([]); + expect(JSON.parse(fs.readFileSync(journal, 'utf8')).intents).toEqual([]); + }); + + it('treats an unreadable journal as empty instead of failing startup', () => { + const { journal } = setup(); + fs.mkdirSync(path.dirname(journal), { recursive: true }); + fs.writeFileSync(journal, '{ not json'); + + expect(reconcileWorktreeIntents(journal, null)).toEqual([]); + }); +}); diff --git a/electron/ipc/worktree-intents.ts b/electron/ipc/worktree-intents.ts new file mode 100644 index 000000000..ccf047104 --- /dev/null +++ b/electron/ipc/worktree-intents.ts @@ -0,0 +1,124 @@ +import fs from 'fs'; +import path from 'path'; +import { atomicWriteFileSync } from '../mcp/atomic.js'; +import { errMessage, warn as logWarn } from '../log.js'; + +/** + * Journal of task worktrees that were provisioned but not yet in a saved state. + * + * Every task path writes its worktree before the renderer's debounced autosave + * records the task, so a crash in between leaves a worktree nothing refers to. + * An intent is written before provisioning and dropped once a saved state claims + * its path. Only this instance's own intents are reported: a dev run and an + * installed build share repositories but not state, and Arena and document-run + * worktrees live in the same `.worktrees/` directory without being tasks, so + * scanning `git worktree list` would report worktrees that are not orphans. + */ +export interface WorktreeIntent { + worktreePath: string; + branchName: string; + projectRoot: string; + createdAt: string; +} + +const JOURNAL_VERSION = 1; + +let journalPath: string | null = null; +const pending = new Map(); + +function isIntent(value: unknown): value is WorktreeIntent { + if (typeof value !== 'object' || value === null) return false; + const v = value as Record; + return ( + typeof v.worktreePath === 'string' && + typeof v.branchName === 'string' && + typeof v.projectRoot === 'string' && + typeof v.createdAt === 'string' + ); +} + +// The journal is a safety net: an unreadable or unwritable journal is logged +// and must never block startup, task creation or saving app state. +function readJournal(file: string): WorktreeIntent[] { + try { + const parsed: unknown = JSON.parse(fs.readFileSync(file, 'utf8')); + const intents = (parsed as { intents?: unknown } | null)?.intents; + return Array.isArray(intents) ? intents.filter(isIntent) : []; + } catch (err) { + if ((err as NodeJS.ErrnoException).code !== 'ENOENT') { + logWarn('worktrees', 'unreadable worktree intent journal', { err: errMessage(err) }); + } + return []; + } +} + +function writeJournal(): void { + if (!journalPath) return; + const data = { version: JOURNAL_VERSION, intents: [...pending.values()] }; + try { + fs.mkdirSync(path.dirname(journalPath), { recursive: true }); + atomicWriteFileSync(journalPath, JSON.stringify(data, null, 2) + '\n'); + } catch (err) { + logWarn('worktrees', 'failed to write worktree intent journal', { err: errMessage(err) }); + } +} + +/** Worktree paths of the tasks in a saved app state. Malformed input claims nothing. */ +function claimedWorktreePaths(stateJson: string | null): Set { + const claimed = new Set(); + if (!stateJson) return claimed; + let tasks: unknown; + try { + tasks = (JSON.parse(stateJson) as { tasks?: unknown } | null)?.tasks; + } catch (err) { + logWarn('worktrees', 'unparseable app state', { err: errMessage(err) }); + return claimed; + } + if (typeof tasks !== 'object' || tasks === null) return claimed; + for (const task of Object.values(tasks)) { + const p = (task as { worktreePath?: unknown } | null)?.worktreePath; + if (typeof p === 'string') claimed.add(path.resolve(p)); + } + return claimed; +} + +/** + * Open the journal at startup and return the orphans: intents whose worktree + * exists but that no saved task claims. Orphans stay in the journal, so they + * are reported again on each start until the worktree is removed; they may hold + * user work, so nothing is deleted here. Must run before any task is created. + */ +export function reconcileWorktreeIntents( + file: string, + savedStateJson: string | null, +): WorktreeIntent[] { + journalPath = file; + pending.clear(); + const claimed = claimedWorktreePaths(savedStateJson); + const intents = readJournal(file); + for (const intent of intents) { + const key = path.resolve(intent.worktreePath); + if (claimed.has(key) || !fs.existsSync(key)) continue; + pending.set(key, intent); + } + if (pending.size !== intents.length) writeJournal(); + return [...pending.values()]; +} + +/** Record a worktree about to be provisioned. A no-op until the journal is opened. */ +export function recordWorktreeIntent(intent: Omit): void { + if (!journalPath) return; + const entry = { ...intent, createdAt: new Date().toISOString() }; + pending.set(path.resolve(intent.worktreePath), entry); + writeJournal(); +} + +/** Drop the intents a just-saved app state claims. */ +export function settleWorktreeIntents(savedStateJson: string): void { + if (pending.size === 0) return; + let changed = false; + for (const key of claimedWorktreePaths(savedStateJson)) { + changed = pending.delete(key) || changed; + } + if (changed) writeJournal(); +} diff --git a/electron/main.ts b/electron/main.ts index 7d6f5fbcd..3eb38ac17 100644 --- a/electron/main.ts +++ b/electron/main.ts @@ -7,7 +7,10 @@ import fs from 'fs'; import { fileURLToPath, pathToFileURL } from 'url'; import { execFileSync } from 'child_process'; import { registerAllHandlers } from './ipc/register.js'; -import { registerLogHandler } from './log.js'; +import { registerLogHandler, warn as logWarn } from './log.js'; +import { loadAppState } from './ipc/persistence.js'; +import { reconcileWorktreeIntents } from './ipc/worktree-intents.js'; +import { getUserDataDir } from './user-data-dir.js'; import { installIpcTracing } from './ipc/trace.js'; import { startAgentHookRuntime, stopAgentHookRuntime } from './agent-hooks/runtime.js'; import { killAllAgents } from './ipc/pty.js'; @@ -182,6 +185,17 @@ function setupApplicationMenu(): void { ); } +/** Report task worktrees an interrupted creation left behind. Runs before the + * window exists, so no task can be created before the journal is open. */ +function reportOrphanedWorktrees(): void { + const journal = path.join(getUserDataDir(), 'worktree-intents.json'); + for (const orphan of reconcileWorktreeIntents(journal, loadAppState())) { + logWarn('worktrees', 'orphaned worktree: task creation was interrupted before save', { + ...orphan, + }); + } +} + function createWindow() { mainWindow = new BrowserWindow({ width: 1400, @@ -328,6 +342,7 @@ if (shouldStartApp) { // agent that misses its hooks. Failure falls back to PTY heuristics. await startAgentHookRuntime(() => mainWindow); setupApplicationMenu(); + reportOrphanedWorktrees(); createWindow(); registerParallelCodeProtocol(); // Linux/Windows cold start: the link is a launch argument. From 45772f0c138e7eddb158c99c594ee38d2958f2ce Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Fri, 25 Sep 2026 19:09:59 +0200 Subject: [PATCH 03/10] refactor(runtime): send UI updates through a notify port --- docs/runtime-separation-plan.md | 4 +- electron/ipc/git.ts | 10 +- electron/ipc/notify.ts | 8 + electron/ipc/plans.test.ts | 4 +- electron/ipc/plans.ts | 17 +- electron/ipc/pty.test.ts | 195 +++++++++--------- electron/ipc/pty.ts | 32 ++- electron/ipc/register-canvas.test.ts | 8 +- electron/ipc/register.ts | 18 +- electron/ipc/steps.ts | 15 +- electron/ipc/window-notifier.test.ts | 25 +++ electron/ipc/window-notifier.ts | 9 + electron/mcp/coordinator-delegation.test.ts | 10 +- ...oordinator-real-agents.integration.test.ts | 16 +- .../coordinator-real-pty.integration.test.ts | 8 +- electron/mcp/coordinator-test-harness.ts | 11 +- electron/mcp/coordinator.test.ts | 84 ++++---- electron/mcp/coordinator.ts | 16 +- 18 files changed, 252 insertions(+), 238 deletions(-) create mode 100644 electron/ipc/notify.ts create mode 100644 electron/ipc/window-notifier.test.ts create mode 100644 electron/ipc/window-notifier.ts diff --git a/docs/runtime-separation-plan.md b/docs/runtime-separation-plan.md index 0a0c33046..b344b50c6 100644 --- a/docs/runtime-separation-plan.md +++ b/docs/runtime-separation-plan.md @@ -1,6 +1,6 @@ # Runtime separation plan -Status: revised after three reviews (two focused, one adversarial) and re-checked against the code on 2026-09-25. Steps 0, 1 and 2 are done. Everything else is a proposal. References name functions rather than line numbers, because line numbers drift. +Status: revised after three reviews (two focused, one adversarial) and re-checked against the code on 2026-09-25. Steps 0, 1, 2 and 3a are done. Everything else is a proposal. References name functions rather than line numbers, because line numbers drift. ## Why @@ -55,7 +55,7 @@ Each of these changes runtime behavior and needs a smoke test in the real app. - Verified: `electron/ipc/worktree-intents.test.ts` simulates a crash between provisioning and save and sees the orphan on the next start. `electron/ipc/tasks.test.ts` checks that the intent is written before `createWorktree` runs; it fails if the order is reversed. The journal is intended as the first part of the step 5 registry. 3. **Runtime boundary, then split `registerAllHandlers`.** Two independent changes; land 3a first. - **3a. Notify port.** Replace `BrowserWindow` in `pty.ts`, `git.ts`, `plans.ts`, `steps.ts` and the coordinator (17 references) with a notify port. This barely touches `register.ts`, so it needs no freeze, and step 4 depends on it. → verify: `npm run check`, the existing tests for those modules, and `rg BrowserWindow` shows no matches in them. + **3a. Notify port (done).** `pty.ts`, `git.ts`, `plans.ts`, `steps.ts` and the coordinator take a `Notify` function (`electron/ipc/notify.ts`) instead of a `BrowserWindow`, and none of them imports Electron. `registerAllHandlers` builds one with `windowNotifier` (`electron/ipc/window-notifier.ts`), which drops messages once the window is destroyed; the coordinator's `setWindow` became `setNotify`. The coordinator still refuses to launch before a notifier is set; step 4 decides whether a no-op notifier is enough for headless launches. One behavior moved: the steps watcher still reads the steps file after the window is destroyed, and only the send is dropped. Verified: `npm run check`, `lint:arch`, the unit suite and the real-PTY coordinator test. **3b. Split `register.ts`.** #279 (pooled workspaces) was closed without merging on 2026-09-25, so it no longer blocks this. 5 of the 12 open PRs touch `register.ts` (#262, #248, #247, #162, #23). Do the split between merges, after announcing a short freeze; it does not block steps 4 and later. Before the split: - Commit a wiring test against the old code. It records every `ipcMain.handle` channel, throws on duplicates, asserts the `win.on` events and exactly one registration-time `onPtyEvent('exit')`, and triggers `ensureCoordinator` twice so the lazily registered `MCP_*` handlers are seen exactly once. diff --git a/electron/ipc/git.ts b/electron/ipc/git.ts index b80df437a..58e4286f4 100644 --- a/electron/ipc/git.ts +++ b/electron/ipc/git.ts @@ -2,7 +2,7 @@ import { execFile, execFileSync as _execFileSync, spawn } from 'child_process'; import { promisify } from 'util'; import fs from 'fs'; import path from 'path'; -import type { BrowserWindow } from 'electron'; +import type { Notify } from './notify.js'; import { debug as logDebug } from '../log.js'; import { appendGitInfoExcludeBlock, @@ -2174,7 +2174,7 @@ export async function getFileDiffFromBranch( } export function pushTask( - win: BrowserWindow, + notify: Notify, projectRoot: string, branchName: string, channelId: string, @@ -2185,11 +2185,7 @@ export function pushTask( stdio: ['ignore', 'pipe', 'pipe'], }); - const send = (msg: string) => { - if (!win.isDestroyed()) { - win.webContents.send(`channel:${channelId}`, msg); - } - }; + const send = (msg: string) => notify(`channel:${channelId}`, msg); proc.stdout?.on('data', (chunk: Buffer) => { send(chunk.toString('utf8')); diff --git a/electron/ipc/notify.ts b/electron/ipc/notify.ts new file mode 100644 index 000000000..5f9de5f15 --- /dev/null +++ b/electron/ipc/notify.ts @@ -0,0 +1,8 @@ +/** + * Sends a message to the desktop UI. + * + * Runtime modules (PTY, git, plan and step watchers, the coordinator) take this + * instead of a `BrowserWindow`, so they do not depend on Electron and can run + * without a window. The desktop shell supplies one with `windowNotifier`. + */ +export type Notify = (channel: string, payload: unknown) => void; diff --git a/electron/ipc/plans.test.ts b/electron/ipc/plans.test.ts index cf842e797..82ca2cc2c 100644 --- a/electron/ipc/plans.test.ts +++ b/electron/ipc/plans.test.ts @@ -2,7 +2,6 @@ import fs from 'node:fs'; import { execFileSync } from 'node:child_process'; import os from 'node:os'; import path from 'node:path'; -import type { BrowserWindow } from 'electron'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { IPC } from './channels.js'; import { readPlanForWorktree, startPlanWatcher, stopAllPlanWatchers } from './plans.js'; @@ -45,8 +44,7 @@ function writeFile(relativePath: string, content = '# Theme plan'): void { function watchPlans() { const send = vi.fn(); - const win = { isDestroyed: () => false, webContents: { send } } as unknown as BrowserWindow; - startPlanWatcher(win, 'task-1', worktreePath); + startPlanWatcher(send, 'task-1', worktreePath); return send; } diff --git a/electron/ipc/plans.ts b/electron/ipc/plans.ts index 611339ff1..27952410e 100644 --- a/electron/ipc/plans.ts +++ b/electron/ipc/plans.ts @@ -2,7 +2,7 @@ import fs from 'fs'; import path from 'path'; import { execFile } from 'child_process'; import { promisify } from 'util'; -import type { BrowserWindow } from 'electron'; +import type { Notify } from './notify.js'; import { IPC } from './channels.js'; interface PlanWatcher { @@ -182,7 +182,7 @@ async function readUncommittedPlan( /** One plan publish to the renderer. */ interface PlanPublish { - win: BrowserWindow; + notify: Notify; taskId: string; plan: PlanFile | null; /** The plan was found already on disk rather than seen being written. The @@ -194,12 +194,11 @@ interface PlanPublish { } /** Sends plan content for a task to the renderer. */ -function sendPlanContent({ win, taskId, plan, recovered }: PlanPublish): void { - if (win.isDestroyed()) return; +function sendPlanContent({ notify, taskId, plan, recovered }: PlanPublish): void { if (plan) { - win.webContents.send(IPC.PlanContent, { taskId, ...plan, recovered }); + notify(IPC.PlanContent, { taskId, ...plan, recovered }); } else { - win.webContents.send(IPC.PlanContent, { + notify(IPC.PlanContent, { taskId, content: null, fileName: null, @@ -278,7 +277,7 @@ function startDirPolling( * The plan found on disk at startup is marked `recovered` so the renderer * shows it without opening the canvas. */ -export function startPlanWatcher(win: BrowserWindow, taskId: string, worktreePath: string): void { +export function startPlanWatcher(notify: Notify, taskId: string, worktreePath: string): void { stopPlanWatcher(taskId); const plansDirs = PLAN_DIRS.map((rel) => path.join(worktreePath, rel)); @@ -315,7 +314,7 @@ export function startPlanWatcher(win: BrowserWindow, taskId: string, worktreePat // sessions, and the agent may have created it a step before writing into // it. Only a plan actually inside it is news; anything else is already out. if (fromAttachOnly && !isPlanInDirs(current.worktreePath, dirs, plan)) return; - sendPlanContent({ win, taskId, plan }); + sendPlanContent({ notify, taskId, plan }); }, 200); }; @@ -334,7 +333,7 @@ export function startPlanWatcher(win: BrowserWindow, taskId: string, worktreePat .then((plan) => { // A live event or a replacement watcher takes precedence over this startup read. if (plan && watchers.get(taskId) === entry && !changedSinceStart) - sendPlanContent({ win, taskId, plan, recovered: true }); + sendPlanContent({ notify, taskId, plan, recovered: true }); }) .catch((error: unknown) => console.warn('[plans] Failed to recover existing plan:', error)); } diff --git a/electron/ipc/pty.test.ts b/electron/ipc/pty.test.ts index 53c60c591..5b65428e3 100644 --- a/electron/ipc/pty.test.ts +++ b/electron/ipc/pty.test.ts @@ -1,8 +1,8 @@ import fs from 'fs'; import os from 'os'; import path from 'path'; -import type { BrowserWindow } from 'electron'; -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { Notify } from './notify.js'; +import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest'; const { mockExecFileSync, mockExecFile, mockChildProcessSpawn, mockPtySpawn, mockLogDebug } = vi.hoisted(() => { @@ -108,13 +108,8 @@ import { let tempPaths: string[] = []; let agentCounter = 0; -function createMockWindow(): BrowserWindow { - return { - isDestroyed: vi.fn(() => false), - webContents: { - send: vi.fn(), - }, - } as unknown as BrowserWindow; +function createMockNotify(): Mock { + return vi.fn(); } function nextAgentId(): string { @@ -281,7 +276,7 @@ describe('buildPtySpawnEnv', () => { describe('spawnAgent docker mode', () => { it('uses --network host (not --add-host, which is incompatible with host networking on Linux)', async () => { - await spawnAgent(createMockWindow(), buildSpawnArgs({ cwd: '/workspace/project' })); + await spawnAgent(createMockNotify(), buildSpawnArgs({ cwd: '/workspace/project' })); const { args } = getLastSpawnCall(); expect(args).toContain('--network'); const netIdx = args.indexOf('--network'); @@ -292,7 +287,7 @@ describe('spawnAgent docker mode', () => { it('sets -w to the worktree cwd so the container starts in the right directory', async () => { const cwd = '/workspace/my-project'; - await spawnAgent(createMockWindow(), buildSpawnArgs({ cwd, dockerMountWorktreeParent: false })); + await spawnAgent(createMockNotify(), buildSpawnArgs({ cwd, dockerMountWorktreeParent: false })); const { args } = getLastSpawnCall(); const wIdx = args.indexOf('-w'); expect(wIdx).toBeGreaterThan(0); @@ -301,7 +296,7 @@ describe('spawnAgent docker mode', () => { it('volume-mounts the worktree cwd at the same host path', async () => { const cwd = '/workspace/my-project'; - await spawnAgent(createMockWindow(), buildSpawnArgs({ cwd, dockerMountWorktreeParent: false })); + await spawnAgent(createMockNotify(), buildSpawnArgs({ cwd, dockerMountWorktreeParent: false })); const volumeFlags = getFlagValues(getLastSpawnCall().args, '-v'); expect(volumeFlags).toContain(`${cwd}:${cwd}`); }); @@ -310,7 +305,7 @@ describe('spawnAgent docker mode', () => { vi.stubEnv('HOME', '/Users/tester'); const agentId = nextAgentId(); - await spawnAgent(createMockWindow(), buildSpawnArgs({ agentId })); + await spawnAgent(createMockNotify(), buildSpawnArgs({ agentId })); const { command, args } = getLastSpawnCall(); expect(command).toBe('docker'); @@ -324,7 +319,7 @@ describe('spawnAgent docker mode', () => { const agentId = nextAgentId(); await spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ agentId, env: { @@ -349,7 +344,7 @@ describe('spawnAgent docker mode', () => { // `-e KEY=VALUE` would expose API keys via ps / /proc//cmdline for the // lifetime of the container. Values must reach docker via its own env. await spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ env: { ANTHROPIC_API_KEY: 'sk-ant-super-secret' } }), ); @@ -361,7 +356,7 @@ describe('spawnAgent docker mode', () => { it('redacts docker env values in spawn debug logs', async () => { await spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ env: { API_KEY: 'secret-api-key', @@ -386,7 +381,7 @@ describe('spawnAgent docker mode', () => { it('redacts inline docker env values in spawn debug logs', async () => { await spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ args: ['--env=INLINE_TOKEN=inline-secret', '--env', 'SPLIT_TOKEN=split-secret'], }), @@ -402,7 +397,7 @@ describe('spawnAgent docker mode', () => { it('redacts shell command strings in spawn debug logs', async () => { await spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ command: '/bin/sh', args: ['-c', 'codex exec "prompt containing private context"'], @@ -421,7 +416,7 @@ describe('spawnAgent docker mode', () => { vi.stubEnv('HOME', home); const agentId = nextAgentId(); - await spawnAgent(createMockWindow(), buildSpawnArgs({ agentId })); + await spawnAgent(createMockNotify(), buildSpawnArgs({ agentId })); const containerHome = `${DOCKER_CONTAINER_HOME}/agent-${agentId}`; const volumeFlags = getFlagValues(getLastSpawnCall().args, '-v'); @@ -446,7 +441,7 @@ describe('spawnAgent docker mode', () => { const agentId = nextAgentId(); await spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ agentId, command, shareDockerAgentAuth: true }), ); @@ -462,7 +457,7 @@ describe('spawnAgent docker mode', () => { vi.stubEnv('HOME', home); await spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ command: 'claude', shareDockerAgentAuth: true }), ); @@ -476,7 +471,7 @@ describe('spawnAgent docker mode', () => { const agentId = nextAgentId(); await spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ agentId, command: 'claude', shareDockerAgentAuth: true }), ); @@ -499,7 +494,7 @@ describe('spawnAgent docker mode', () => { vi.stubEnv('HOME', home); await spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ command: 'claude', cwd: '/workspace/project', @@ -539,7 +534,7 @@ describe('spawnAgent docker mode', () => { ); await spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ command: 'claude', cwd: '/workspace/project', @@ -571,7 +566,7 @@ describe('spawnAgent docker mode', () => { vi.stubEnv('HOME', home); await spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ command: 'claude', shareDockerAgentAuth: false }), ); @@ -584,7 +579,7 @@ describe('spawnAgent docker mode', () => { vi.stubEnv('HOME', home); await spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ command: 'unknown-agent', shareDockerAgentAuth: true }), ); @@ -601,7 +596,7 @@ describe('spawnAgent docker mode', () => { await expect( spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ command: 'claude', shareDockerAgentAuth: true }), ), ).resolves.toBeUndefined(); @@ -623,7 +618,7 @@ describe('spawnAgent docker mode', () => { ); await spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ command: 'claude', cwd: '/workspace/project', @@ -649,7 +644,7 @@ describe('spawnAgent docker mode', () => { vi.stubEnv('HOME', home); await spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ command: 'claude', cwd: '/workspace/task-one', @@ -658,7 +653,7 @@ describe('spawnAgent docker mode', () => { ); await spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ command: 'claude', cwd: '/workspace/task-two', @@ -688,7 +683,7 @@ describe('spawnAgent docker mode', () => { vi.stubEnv('HOME', home); await spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ command: 'claude', cwd: '/workspace/project', @@ -706,7 +701,7 @@ describe('spawnAgent docker mode', () => { // First container spawn — seeds trust await spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ command: 'claude', cwd: '/workspace/my-project', @@ -723,7 +718,7 @@ describe('spawnAgent docker mode', () => { // Second container spawn (same auth dir, same worktree path — simulates container B) await spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ command: 'claude', cwd: '/workspace/my-project', @@ -742,7 +737,7 @@ describe('spawnAgent docker mode', () => { describe('dockerMountWorktreeParent', () => { it('mounts parent directory when dockerMountWorktreeParent is true', async () => { await spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ cwd: '/Users/alice/git/my-repo/.worktrees/task/coordinator-abc', dockerMountWorktreeParent: true, @@ -762,7 +757,7 @@ describe('spawnAgent docker mode', () => { it('does not mount parent directory when dockerMountWorktreeParent is false', async () => { await spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ cwd: '/Users/alice/git/my-repo/.worktrees/task/coordinator-abc', dockerMountWorktreeParent: false, @@ -787,7 +782,7 @@ describe('spawnAgent pending setup', () => { if (!allowed) throw new Error('Launch permission revoked'); }); const startup = spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ cwd: makeTempHome([]), dockerMode: false }), admission, ); @@ -801,7 +796,7 @@ describe('spawnAgent pending setup', () => { it.each(['one', 'all'])('does not launch after stopping %s pending agents', async (mode) => { const agentId = nextAgentId(); const startup = spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ agentId, cwd: makeTempHome([]), dockerMode: false }), ); @@ -816,7 +811,7 @@ describe('spawnAgent pending setup', () => { describe('spawnAgent session reattach', () => { it('reuses an existing PTY session and moves live output to the new channel', async () => { - const win = createMockWindow(); + const notify = createMockNotify(); const agentId = 'agent-reattach'; const args = buildSpawnArgs({ agentId, @@ -826,11 +821,11 @@ describe('spawnAgent session reattach', () => { onOutput: { __CHANNEL_ID__: 'channel-1' }, }); - await spawnAgent(win, args); + await spawnAgent(notify, args); const proc = mockPtySpawn.mock.results[0].value as ReturnType; proc.emitData('before reload'); - await spawnAgent(win, { + await spawnAgent(notify, { ...args, cols: 90, rows: 30, @@ -844,18 +839,18 @@ describe('spawnAgent session reattach', () => { expect(mockPtySpawn).toHaveBeenCalledTimes(1); expect(proc.resume).toHaveBeenCalled(); expect(proc.resize).toHaveBeenCalledWith(90, 30); - expect(win.webContents.send).toHaveBeenCalledWith('channel:channel-2', { + expect(notify).toHaveBeenCalledWith('channel:channel-2', { type: 'Data', data: new Uint8Array(Buffer.from('before reload', 'utf8')), }); - expect(win.webContents.send).toHaveBeenLastCalledWith('channel:channel-2', { + expect(notify).toHaveBeenLastCalledWith('channel:channel-2', { type: 'Data', data: new Uint8Array(Buffer.from('after reload', 'utf8')), }); }); it('reattaches before validating the launch command', async () => { - const win = createMockWindow(); + const notify = createMockNotify(); const agentId = 'agent-reattach-missing-command'; const args = buildSpawnArgs({ agentId, @@ -865,10 +860,10 @@ describe('spawnAgent session reattach', () => { onOutput: { __CHANNEL_ID__: 'channel-1' }, }); - await spawnAgent(win, args); + await spawnAgent(notify, args); await expect( - spawnAgent(win, { + spawnAgent(notify, { ...args, command: 'nonexistent-binary-xyz', attachExisting: true, @@ -879,7 +874,7 @@ describe('spawnAgent session reattach', () => { }); it('replaces an existing same-id PTY when attachExisting is explicitly false', async () => { - const win = createMockWindow(); + const notify = createMockNotify(); const agentId = 'agent-replace'; const args = buildSpawnArgs({ agentId, @@ -889,10 +884,10 @@ describe('spawnAgent session reattach', () => { onOutput: { __CHANNEL_ID__: 'channel-1' }, }); - await spawnAgent(win, args); + await spawnAgent(notify, args); const oldProc = mockPtySpawn.mock.results[0].value as ReturnType; - await spawnAgent(win, { + await spawnAgent(notify, { ...args, attachExisting: false, onOutput: { __CHANNEL_ID__: 'channel-2' }, @@ -904,54 +899,54 @@ describe('spawnAgent session reattach', () => { }); describe('spawnAgent output batching', () => { - function dataMessages(win: BrowserWindow): string[] { + function dataMessages(notify: Mock): string[] { return vi - .mocked(win.webContents.send) + .mocked(notify) .mock.calls.map(([, message]) => message as { type: string; data?: Uint8Array }) .filter((payload) => payload.type === 'Data') .map((payload) => Buffer.from(payload.data ?? []).toString()); } async function launch(agentId: string) { - const win = createMockWindow(); + const notify = createMockNotify(); await spawnAgent( - win, + notify, buildSpawnArgs({ agentId, command: 'claude', args: [], dockerMode: false }), ); const proc = mockPtySpawn.mock.results[mockPtySpawn.mock.results.length - 1] .value as ReturnType; - return { win, proc }; + return { notify, proc }; } it('sends output after a quiet spell at once and coalesces a burst', async () => { vi.useFakeTimers(); try { - const { win, proc } = await launch('agent-batch-burst'); + const { notify, proc } = await launch('agent-batch-burst'); proc.emitData('a'); - expect(dataMessages(win)).toEqual(['a']); + expect(dataMessages(notify)).toEqual(['a']); proc.emitData('b'); proc.emitData('c'); - expect(dataMessages(win)).toEqual(['a']); + expect(dataMessages(notify)).toEqual(['a']); await vi.advanceTimersByTimeAsync(8); - expect(dataMessages(win)).toEqual(['a', 'bc']); + expect(dataMessages(notify)).toEqual(['a', 'bc']); await vi.advanceTimersByTimeAsync(20); proc.emitData('d'); - expect(dataMessages(win)).toEqual(['a', 'bc', 'd']); + expect(dataMessages(notify)).toEqual(['a', 'bc', 'd']); } finally { vi.useRealTimers(); } }); it('sends raw bytes to the window and base64 to subscribers', async () => { - const { win, proc } = await launch('agent-batch-subscriber'); + const { notify, proc } = await launch('agent-batch-subscriber'); const sub = vi.fn(); subscribeToAgent('agent-batch-subscriber', sub); proc.emitData('héllo'); - const calls = vi.mocked(win.webContents.send).mock.calls; + const calls = vi.mocked(notify).mock.calls; const sent = calls[calls.length - 1][1] as { data: Uint8Array }; expect(sent.data).toBeInstanceOf(Uint8Array); expect(Buffer.isBuffer(sent.data)).toBe(false); @@ -963,7 +958,7 @@ describe('spawnAgent output batching', () => { describe('spawnAgent terminal queries', () => { async function launch(agentId: string) { await spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ agentId, command: 'codex', args: [], dockerMode: false }), ); return mockPtySpawn.mock.results[mockPtySpawn.mock.results.length - 1].value as ReturnType< @@ -1124,7 +1119,7 @@ describe('buildDockerImage', () => { const dockerfilePath = path.join(dockerDir, 'Dockerfile'); fs.writeFileSync(dockerfilePath, 'FROM node:20\n'); - buildDockerImage(createMockWindow(), 'channel:build-test', { + buildDockerImage(createMockNotify(), 'channel:build-test', { dockerfilePath, imageTag: 'parallel-code-project:test', buildContext: projectRoot, @@ -1142,7 +1137,7 @@ describe('killAgent — Docker container lifecycle', () => { const agentId = nextAgentId(); const containerName = `parallel-code-${agentId.slice(0, 12)}`; - await spawnAgent(createMockWindow(), buildSpawnArgs({ agentId })); + await spawnAgent(createMockNotify(), buildSpawnArgs({ agentId })); killAgent(agentId); const stopCall = mockExecFile.mock.calls.find( @@ -1164,7 +1159,7 @@ describe('killAgent — Docker container lifecycle', () => { it('does not call docker stop for a non-Docker agent', async () => { const agentId = nextAgentId(); - await spawnAgent(createMockWindow(), buildSpawnArgs({ agentId, dockerMode: false })); + await spawnAgent(createMockNotify(), buildSpawnArgs({ agentId, dockerMode: false })); mockExecFile.mockClear(); killAgent(agentId); @@ -1185,7 +1180,7 @@ describe('spawnAgent docker mode — same-path bind mounts', () => { vi.stubEnv('HOME', home); await spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ cwd: '/workspace/project', shareDockerAgentAuth: false, @@ -1217,7 +1212,7 @@ describe('seedClaudeProjectTrust — concurrent spawns', () => { vi.stubEnv('HOME', home); await spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ command: 'claude', cwd: '/workspace/task-a', @@ -1227,7 +1222,7 @@ describe('seedClaudeProjectTrust — concurrent spawns', () => { ); await spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ command: 'claude', cwd: '/workspace/task-b', @@ -1255,7 +1250,7 @@ describe('spawnAgent docker mode — PTY spawn failure', () => { }); const agentId = nextAgentId(); - await expect(spawnAgent(createMockWindow(), buildSpawnArgs({ agentId }))).rejects.toThrow(); + await expect(spawnAgent(createMockNotify(), buildSpawnArgs({ agentId }))).rejects.toThrow(); }); }); @@ -1269,10 +1264,10 @@ describe('spawnAgent docker mode — PTY spawn failure', () => { describe('spawnAgent docker mode — credential redaction in logs', () => { it('redacts inline Codex MCP credentials from the Docker banner and console', async () => { const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); - const win = createMockWindow(); + const notify = createMockNotify(); const secret = 'test-canvas-secret'; await spawnAgent( - win, + notify, buildSpawnArgs({ command: 'codex', args: ['--config', `mcp_servers.parallel-code={env={PARALLEL_CODE_MCP_TOKEN="${secret}"}}`], @@ -1280,7 +1275,7 @@ describe('spawnAgent docker mode — credential redaction in logs', () => { ); expect(getLastSpawnCall().args.join(' ')).toContain(secret); expect(JSON.stringify(warn.mock.calls)).not.toContain(secret); - const messages = vi.mocked(win.webContents.send).mock.calls; + const messages = vi.mocked(notify).mock.calls; const banner = messages .map(([, message]) => { const payload = message as { type: string; data?: Uint8Array }; @@ -1294,7 +1289,7 @@ describe('spawnAgent docker mode — credential redaction in logs', () => { it('does not log the MCP token when it appears in env vars', async () => { await spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ env: { MCP_TOKEN: 'super-secret-value' }, cwd: '/workspace/project', @@ -1313,7 +1308,7 @@ describe('spawnAgent docker mode — credential redaction in logs', () => { // The redactDockerArgs function should redact -e assignments. // Verify by checking the logged spawn command args. await spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ env: { ANTHROPIC_API_KEY: 'sk-ant-abc123' }, cwd: '/workspace/project', @@ -1368,7 +1363,7 @@ describe('spawnAgent docker mode — path edge cases', () => { it('preserves spaces in worktree path in -v and -w args', async () => { const cwd = '/Users/alice bob/my repos/project name/.worktrees/task/coord-abc'; await spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ cwd, dockerMountWorktreeParent: false, @@ -1390,7 +1385,7 @@ describe('spawnAgent docker mode — path edge cases', () => { vi.stubEnv('HOME', home); await spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ command: 'codex', dockerMode: false, // non-docker, non-claude @@ -1412,7 +1407,7 @@ describe('seedClaudeProjectTrust — file permissions', () => { vi.stubEnv('HOME', home); await spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ command: 'claude', cwd: '/workspace/project', @@ -1446,7 +1441,7 @@ describe('buildDockerCredentialMounts — read-only auth dir', () => { // Should not throw await expect( spawnAgent( - createMockWindow(), + createMockNotify(), buildSpawnArgs({ command: 'claude', shareDockerAgentAuth: true }), ), ).resolves.toBeUndefined(); @@ -1474,8 +1469,8 @@ describe('writeToAgent — interrupt keystrokes', () => { dockerMode: false, isShell: true, }); - await spawnAgent(createMockWindow(), agent); - await spawnAgent(createMockWindow(), shell); + await spawnAgent(createMockNotify(), agent); + await spawnAgent(createMockNotify(), shell); writeToAgent(agent.agentId, 'hello'); writeToAgent(agent.agentId, '\x1b[A'); // arrow key: an escape sequence, not an interrupt @@ -1490,9 +1485,9 @@ describe('writeToAgent — interrupt keystrokes', () => { }); /** Everything a window was sent as terminal output, decoded and joined. */ -function channelData(win: BrowserWindow): string { +function channelData(notify: Mock): string { return vi - .mocked(win.webContents.send) + .mocked(notify) .mock.calls.map(([, message]) => { const payload = message as { type: string; data?: Uint8Array }; return payload.type === 'Data' ? Buffer.from(payload.data ?? []).toString() : ''; @@ -1504,7 +1499,7 @@ describe('Codex terminal handoff', () => { const id = '01999999-1234-4321-9876-0123456789ab'; function launch() { const args = buildSpawnArgs({ command: 'codex', args: [], dockerMode: false }); - spawnAgent(createMockWindow(), args); + spawnAgent(createMockNotify(), args); return { agentId: args.agentId, proc: mockPtySpawn.mock.results[mockPtySpawn.mock.results.length - 1].value, @@ -1548,7 +1543,7 @@ describe('Codex terminal handoff', () => { }); it('replays the pre-handoff output into the relaunched terminal', async () => { const args = buildSpawnArgs({ command: 'codex', args: [], dockerMode: false }); - void spawnAgent(createMockWindow(), args); + void spawnAgent(createMockNotify(), args); const proc = mockPtySpawn.mock.results[mockPtySpawn.mock.results.length - 1].value; proc.emitData('an earlier exchange\r\n'); const promise = handoffCodexTerminal(args.agentId); @@ -1556,7 +1551,7 @@ describe('Codex terminal handoff', () => { proc.emitExit({ exitCode: 0, signal: undefined }); await promise; - const next = createMockWindow(); + const next = createMockNotify(); void spawnAgent(next, { ...args, command: 'codex', args: [], dockerMode: false }); expect(channelData(next)).toContain('an earlier exchange'); expect(channelData(next)).toContain('── resumed ──'); @@ -1586,14 +1581,14 @@ describe('Codex terminal handoff', () => { }); describe('Claude terminal handoff', () => { - function launch(win: BrowserWindow, agentId?: string) { + function launch(notify: Mock, agentId?: string) { const args = buildSpawnArgs({ command: 'claude', args: [], dockerMode: false, ...(agentId ? { agentId } : {}), }); - void spawnAgent(win, args); + void spawnAgent(notify, args); return { agentId: args.agentId, proc: mockPtySpawn.mock.results[mockPtySpawn.mock.results.length - 1].value, @@ -1602,8 +1597,8 @@ describe('Claude terminal handoff', () => { it('confirms the exit with a second Ctrl+D and replays the output into the next launch', async () => { vi.useFakeTimers(); try { - const win = createMockWindow(); - const { agentId, proc } = launch(win); + const notify = createMockNotify(); + const { agentId, proc } = launch(notify); proc.emitData('an earlier exchange\r\n'); const promise = handoffClaudeTerminal(agentId); // The first press only arms Claude's "Press Ctrl-D again to exit". @@ -1615,13 +1610,13 @@ describe('Claude terminal handoff', () => { await promise; expect(proc.kill).not.toHaveBeenCalled(); - const next = createMockWindow(); + const next = createMockNotify(); launch(next, agentId); expect(channelData(next)).toBe( 'an earlier exchange\r\n\x1b\\\x1b[0m\r\n\x1b[2m── resumed ──\x1b[0m\r\n', ); // Only the launch that follows the handoff inherits it. - const third = createMockWindow(); + const third = createMockNotify(); launch(third, agentId); expect(channelData(third)).toBe(''); } finally { @@ -1635,7 +1630,7 @@ describe('Claude terminal handoff', () => { const args = buildSpawnArgs({ command: 'claude', args: [], dockerMode: false, cwd }); // Started but not yet registered: reading that gap as "already exited" would // hand the session to Chat while this CLI is still launching into it. - const spawning = spawnAgent(createMockWindow(), args).catch(() => {}); + const spawning = spawnAgent(createMockNotify(), args).catch(() => {}); await expect(handoffClaudeTerminal(args.agentId)).rejects.toThrow('still starting'); await spawning; }); @@ -1643,8 +1638,8 @@ describe('Claude terminal handoff', () => { it('drops the carried output when the agent is killed instead of resumed', async () => { vi.useFakeTimers(); try { - const win = createMockWindow(); - const { agentId, proc } = launch(win); + const notify = createMockNotify(); + const { agentId, proc } = launch(notify); proc.emitData('an earlier exchange\r\n'); const promise = handoffClaudeTerminal(agentId); await vi.advanceTimersByTimeAsync(300); @@ -1652,7 +1647,7 @@ describe('Claude terminal handoff', () => { await promise; killAgent(agentId); - const next = createMockWindow(); + const next = createMockNotify(); launch(next, agentId); expect(channelData(next)).toBe(''); } finally { @@ -1661,16 +1656,16 @@ describe('Claude terminal handoff', () => { }); it('is a no-op once the CLI has already exited', async () => { - const win = createMockWindow(); - const { agentId, proc } = launch(win); + const notify = createMockNotify(); + const { agentId, proc } = launch(notify); proc.emitExit({ exitCode: 0, signal: undefined }); await expect(handoffClaudeTerminal(agentId)).resolves.toBeUndefined(); }); it('refuses a terminal that is not running Claude', async () => { - const win = createMockWindow(); + const notify = createMockNotify(); const args = buildSpawnArgs({ command: 'codex', args: [], dockerMode: false }); - void spawnAgent(win, args); + void spawnAgent(notify, args); await expect(handoffClaudeTerminal(args.agentId)).rejects.toThrow( 'does not support Claude conversation handoff', ); @@ -1679,8 +1674,8 @@ describe('Claude terminal handoff', () => { it('does not force-kill a terminal which refuses to exit, and restores input after timeout', async () => { vi.useFakeTimers(); try { - const win = createMockWindow(); - const { agentId, proc } = launch(win); + const notify = createMockNotify(); + const { agentId, proc } = launch(notify); const promise = handoffClaudeTerminal(agentId); const rejected = expect(promise).rejects.toThrow('has not exited'); await vi.advanceTimersByTimeAsync(5000); diff --git a/electron/ipc/pty.ts b/electron/ipc/pty.ts index cd48a4c86..2a1bace5d 100644 --- a/electron/ipc/pty.ts +++ b/electron/ipc/pty.ts @@ -6,7 +6,7 @@ import crypto from 'crypto'; import fs from 'fs'; import path from 'path'; import { fileURLToPath } from 'url'; -import type { BrowserWindow } from 'electron'; +import type { Notify } from './notify.js'; import { RingBuffer } from '../remote/ring-buffer.js'; import { resolveUserShell } from '../user-shell.js'; import { @@ -198,10 +198,8 @@ function toIpcBytes(buf: Buffer): Uint8Array { return new Uint8Array(buf); } -function sendToChannel(win: BrowserWindow, channelId: string, msg: unknown): void { - if (!win.isDestroyed()) { - win.webContents.send(`channel:${channelId}`, msg); - } +function sendToChannel(notify: Notify, channelId: string, msg: unknown): void { + notify(`channel:${channelId}`, msg); } // --- PTY event bus for spawn/exit/interrupt notifications --- @@ -544,7 +542,7 @@ function buildPtySpawnSpec( * the relaunch is visible rather than looking like one continuous run. Written * to the new session's buffer too, so a renderer reload keeps it. */ -function replayCarriedScrollback(win: BrowserWindow, session: PtySession): void { +function replayCarriedScrollback(notify: Notify, session: PtySession): void { const carried = carriedScrollback.get(session.agentId); if (!carried) return; carriedScrollback.delete(session.agentId); @@ -556,7 +554,7 @@ function replayCarriedScrollback(win: BrowserWindow, session: PtySession): void const replay = Buffer.concat([Buffer.from(carried, 'base64'), divider]); session.scrollback.write(replay); session.queries.feedDisplayOnly(replay.toString('utf8')); - sendToChannel(win, session.channelId, { type: 'Data', data: toIpcBytes(replay) }); + sendToChannel(notify, session.channelId, { type: 'Data', data: toIpcBytes(replay) }); } function cleanupExistingSession(agentId: string, existing: PtySession | undefined): void { @@ -568,7 +566,7 @@ function cleanupExistingSession(agentId: string, existing: PtySession | undefine } function attachPtyOutputHandlers( - win: BrowserWindow, + notify: Notify, session: PtySession, args: SpawnAgentArgs, command: string, @@ -580,7 +578,7 @@ function attachPtyOutputHandlers( const containerName = session.containerName; const send = (msg: unknown) => { - sendToChannel(win, session.channelId, msg); + sendToChannel(notify, session.channelId, msg); }; if (args.dockerMode) { @@ -723,7 +721,7 @@ export function applyAgentHookLaunch( } export async function spawnAgent( - win: BrowserWindow, + notify: Notify, args: SpawnAgentArgs, beforeSpawn?: () => void, ): Promise { @@ -746,7 +744,7 @@ export async function spawnAgent( existing.queries.resize(args.cols, args.rows); } if (existing.scrollback.length > 0) { - sendToChannel(win, channelId, { + sendToChannel(notify, channelId, { type: 'Data', data: toIpcBytes(existing.scrollback.read()), }); @@ -842,8 +840,8 @@ export async function spawnAgent( containerName: spawnSpec.containerName, }; sessions.set(args.agentId, session); - replayCarriedScrollback(win, session); - attachPtyOutputHandlers(win, session, args, command); + replayCarriedScrollback(notify, session); + attachPtyOutputHandlers(notify, session, args, command); emitPtyEvent('spawn', args.agentId); } @@ -1354,7 +1352,7 @@ let activeBuild: Promise<{ ok: boolean; error?: string }> | null = null; * in-flight promise; custom builds are never deduplicated. */ export function buildDockerImage( - win: BrowserWindow, + notify: Notify, onOutputChannel: string, opts?: { dockerfilePath?: string; buildContext?: string; imageTag?: string }, ): Promise<{ ok: boolean; error?: string }> { @@ -1382,11 +1380,7 @@ export function buildDockerImage( const hash = hashDockerfile(resolvedDockerfilePath) ?? 'unknown'; const imageTag = opts?.imageTag ?? DOCKER_DEFAULT_IMAGE; - const send = (text: string) => { - if (!win.isDestroyed()) { - win.webContents.send(onOutputChannel, text); - } - }; + const send = (text: string) => notify(onOutputChannel, text); const proc = cpSpawn( 'docker', diff --git a/electron/ipc/register-canvas.test.ts b/electron/ipc/register-canvas.test.ts index 93ad0f77f..300bdc608 100644 --- a/electron/ipc/register-canvas.test.ts +++ b/electron/ipc/register-canvas.test.ts @@ -164,7 +164,7 @@ it.each(['transport', 'configuration'])( env: { KEEP: 'value' }, }; expect(await handlers.get(IPC.SpawnAgent)?.(undefined, args)).toEqual({ canvasTools: false }); - expect(spawnAgent).toHaveBeenCalledWith(win, args, expect.any(Function)); + expect(spawnAgent).toHaveBeenCalledWith(expect.any(Function), args, expect.any(Function)); if (failure === 'configuration') expect(server.unregisterCanvasAgent).toHaveBeenCalledWith('agent'); }, @@ -195,7 +195,7 @@ it.each([ }; await handlers.get(IPC.SpawnAgent)?.(undefined, args); expect(start).not.toHaveBeenCalled(); - expect(spawnAgent).toHaveBeenCalledWith(win, args, expect.any(Function)); + expect(spawnAgent).toHaveBeenCalledWith(expect.any(Function), args, expect.any(Function)); }, ); @@ -234,7 +234,7 @@ it('starts an ordinary Codex terminal with canvas MCP when the development port }); expect(result).toEqual({ canvasTools: true }); expect(spawnAgent).toHaveBeenCalledWith( - win, + expect.any(Function), expect.objectContaining({ canvasTools: true, args: ['--config', expect.stringContaining('mcp_servers.parallel-code=')], @@ -353,7 +353,7 @@ it('preserves the configured capability when reattaching, without reconfiguring expect(await handlers.get(IPC.SpawnAgent)?.(undefined, args)).toEqual({ canvasTools: true }); expect(start).not.toHaveBeenCalled(); expect(spawnAgent).toHaveBeenCalledWith( - win, + expect.any(Function), { ...args, canvasTools: true }, expect.any(Function), ); diff --git a/electron/ipc/register.ts b/electron/ipc/register.ts index f30acc8b6..0659a7c1a 100644 --- a/electron/ipc/register.ts +++ b/electron/ipc/register.ts @@ -117,6 +117,7 @@ import { } from './git.js'; import { createTask, deleteTask } from './tasks.js'; import { settleWorktreeIntents } from './worktree-intents.js'; +import { windowNotifier } from './window-notifier.js'; import { listAgents } from './agents.js'; import { saveAppState, @@ -463,6 +464,7 @@ function createThrottledForwarder( * git-exclude a generated file must not block coordinator startup. */ export function registerAllHandlers(win: BrowserWindow): void { + const notify = windowNotifier(win); ipcMain.handle(IPC.AgentChat, async (_event, args: Record) => { assertString(args.agentId, 'agentId'); assertString(args.action, 'action'); @@ -561,8 +563,8 @@ export function registerAllHandlers(win: BrowserWindow): void { ); try { ensurePlansDirectory(args.cwd as string); - startPlanWatcher(win, args.taskId, args.cwd as string); - if (args.stepsEnabled) startStepsWatcher(win, args.taskId, args.cwd as string); + startPlanWatcher(notify, args.taskId, args.cwd as string); + if (args.stepsEnabled) startStepsWatcher(notify, args.taskId, args.cwd as string); } catch (err) { console.warn('Failed to start chat plan/steps watchers:', err); } @@ -887,7 +889,7 @@ export function registerAllHandlers(win: BrowserWindow): void { assertPendingSpawn(); releaseAdmission?.assertAllowed(); try { - await spawnAgent(win, canvasTools ? { ...args, canvasTools: true } : args, () => { + await spawnAgent(notify, canvasTools ? { ...args, canvasTools: true } : args, () => { assertPendingSpawn(); releaseAdmission?.assertAllowed(); }); @@ -904,13 +906,13 @@ export function registerAllHandlers(win: BrowserWindow): void { } if (!args.isShell && args.cwd) { try { - startPlanWatcher(win, args.taskId, args.cwd); + startPlanWatcher(notify, args.taskId, args.cwd); } catch (err) { console.warn('Failed to start plan watcher:', err); } if (args.stepsEnabled) { try { - startStepsWatcher(win, args.taskId, args.cwd); + startStepsWatcher(notify, args.taskId, args.cwd); } catch (err) { console.warn('Failed to start steps watcher:', err); } @@ -989,7 +991,7 @@ export function registerAllHandlers(win: BrowserWindow): void { const buildContext = getOptionalBuildContext(args.buildContext); const imageTag = getOptionalImageTag(args.imageTag); return buildDockerImage( - win, + notify, args.onOutputChannel, dockerfilePath || buildContext || imageTag ? { dockerfilePath, buildContext, imageTag } @@ -1170,7 +1172,7 @@ export function registerAllHandlers(win: BrowserWindow): void { const projectRoot = projectRootArg(args); const branchName = branchNameArg(args); assertString(args.onOutput?.__CHANNEL_ID__, 'channelId'); - return pushTask(win, projectRoot, branchName, args.onOutput.__CHANNEL_ID__); + return pushTask(notify, projectRoot, branchName, args.onOutput.__CHANNEL_ID__); }); ipcMain.handle(IPC.RebaseTask, (_e, args) => { const worktreePath = worktreePathArg(args); @@ -2277,7 +2279,7 @@ export function registerAllHandlers(win: BrowserWindow): void { const { Coordinator } = await import('../mcp/coordinator.js'); coordinator = new Coordinator(); coordinator.setOrchestrationEnabled(delegation.isOrchestrationEnabled()); - coordinator.setWindow(win); + coordinator.setNotify(notify); coordinator.setSessionMcpProvider((task) => { if (!delegation.getTask(task.coordinatorTaskId) || !remoteServer) return undefined; if (!delegation.getTask(task.id)) { diff --git a/electron/ipc/steps.ts b/electron/ipc/steps.ts index 96f7e2265..a9e93aa2d 100644 --- a/electron/ipc/steps.ts +++ b/electron/ipc/steps.ts @@ -1,6 +1,6 @@ import fs from 'fs'; import path from 'path'; -import type { BrowserWindow } from 'electron'; +import type { Notify } from './notify.js'; import { IPC } from './channels.js'; import { appendGitInfoExcludeBlock } from './git-exclude.js'; import { @@ -34,12 +34,11 @@ const watchers = new Map(); const processedCount = new Map(); /** Sends parsed steps content for a task to the renderer. */ -function sendStepsContent(win: BrowserWindow, taskId: string, stepsFile: string): void { - if (win.isDestroyed()) return; +function sendStepsContent(notify: Notify, taskId: string, stepsFile: string): void { const steps = readStepsFile(stepsFile); logInfo('steps', 'send', { taskId, len: steps?.length ?? null }); if (steps) applyTimestamps(steps, stepsFile, taskId); - win.webContents.send(IPC.StepsContent, { taskId, steps }); + notify(IPC.StepsContent, { taskId, steps }); } /** @@ -146,7 +145,7 @@ function ensureStepsIgnored(worktreePath: string): void { * An initial read is performed after starting the watcher to handle * the race condition where the agent writes before the watcher is set up. */ -export function startStepsWatcher(win: BrowserWindow, taskId: string, worktreePath: string): void { +export function startStepsWatcher(notify: Notify, taskId: string, worktreePath: string): void { stopStepsWatcher(taskId); ensureStepsIgnored(worktreePath); @@ -169,7 +168,7 @@ export function startStepsWatcher(win: BrowserWindow, taskId: string, worktreePa if (current.timeout) clearTimeout(current.timeout); current.timeout = setTimeout(() => { current.timeout = null; - sendStepsContent(win, taskId, current.stepsFile); + sendStepsContent(notify, taskId, current.stepsFile); }, 200); }; @@ -188,7 +187,7 @@ export function startStepsWatcher(win: BrowserWindow, taskId: string, worktreePa if (!current) return; attachStepsDirWatcher(current, taskId, onChange); if (fs.existsSync(stepsFile)) { - sendStepsContent(win, taskId, stepsFile); + sendStepsContent(notify, taskId, stepsFile); } }); parentWatcher.on('error', (err) => { @@ -204,7 +203,7 @@ export function startStepsWatcher(win: BrowserWindow, taskId: string, worktreePa // Initial read to catch files written before the watcher was set up if (fs.existsSync(stepsFile)) { - sendStepsContent(win, taskId, stepsFile); + sendStepsContent(notify, taskId, stepsFile); } } diff --git a/electron/ipc/window-notifier.test.ts b/electron/ipc/window-notifier.test.ts new file mode 100644 index 000000000..fc539adaf --- /dev/null +++ b/electron/ipc/window-notifier.test.ts @@ -0,0 +1,25 @@ +import type { BrowserWindow } from 'electron'; +import { describe, expect, it, vi } from 'vitest'; + +import { windowNotifier } from './window-notifier.js'; + +function fakeWindow() { + let destroyed = false; + const send = vi.fn(); + const win = { isDestroyed: () => destroyed, webContents: { send } } as unknown as BrowserWindow; + return { win, send, destroy: () => (destroyed = true) }; +} + +describe('windowNotifier', () => { + it('sends to the window while it exists and drops messages afterwards', () => { + const { win, send, destroy } = fakeWindow(); + const notify = windowNotifier(win); + + notify('channel:a', { n: 1 }); + destroy(); + notify('channel:a', { n: 2 }); + + expect(send).toHaveBeenCalledTimes(1); + expect(send).toHaveBeenCalledWith('channel:a', { n: 1 }); + }); +}); diff --git a/electron/ipc/window-notifier.ts b/electron/ipc/window-notifier.ts new file mode 100644 index 000000000..c4494f14a --- /dev/null +++ b/electron/ipc/window-notifier.ts @@ -0,0 +1,9 @@ +import type { BrowserWindow } from 'electron'; +import type { Notify } from './notify.js'; + +/** A `Notify` that sends to the window and drops messages once it is destroyed. */ +export function windowNotifier(win: BrowserWindow): Notify { + return (channel, payload) => { + if (!win.isDestroyed()) win.webContents.send(channel, payload); + }; +} diff --git a/electron/mcp/coordinator-delegation.test.ts b/electron/mcp/coordinator-delegation.test.ts index 089e64d1a..5297abcc8 100644 --- a/electron/mcp/coordinator-delegation.test.ts +++ b/electron/mcp/coordinator-delegation.test.ts @@ -2,7 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { setupCoordinatorHarness, resetCoordinatorMocks, - mockWin, + mockNotify, mockCreateBackendTask, mockSpawnAgent, mockExecFile, @@ -59,7 +59,7 @@ beforeEach(() => { vi.useFakeTimers(); resetCoordinatorMocks(); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.registerCoordinator('parent', 'project', { projectRoot: '/project', branchName: 'parent', @@ -95,7 +95,7 @@ describe('ordinary delegation lifecycle', () => { 'parent', ); expect(mockSpawnAgent).toHaveBeenCalledWith( - mockWin, + mockNotify, expect.objectContaining({ command: 'codex', envFile: '/agent.env' }), expect.any(Function), ); @@ -111,7 +111,7 @@ describe('ordinary delegation lifecycle', () => { it('uses per-assignment launch options without mutating the parent defaults', async () => { await create({ agentEnvFile: '/selected.env', skipPermissions: true }); expect(mockSpawnAgent).toHaveBeenLastCalledWith( - mockWin, + mockNotify, expect.objectContaining({ envFile: '/selected.env', args: expect.arrayContaining(['--dangerously-bypass-approvals-and-sandbox']), @@ -121,7 +121,7 @@ describe('ordinary delegation lifecycle', () => { coordinator.removeCoordinatedTask('task-1'); await create(); expect(mockSpawnAgent).toHaveBeenLastCalledWith( - mockWin, + mockNotify, expect.objectContaining({ envFile: '/agent.env', args: [] }), expect.any(Function), ); diff --git a/electron/mcp/coordinator-real-agents.integration.test.ts b/electron/mcp/coordinator-real-agents.integration.test.ts index 03e8ed816..f7a58ed61 100644 --- a/electron/mcp/coordinator-real-agents.integration.test.ts +++ b/electron/mcp/coordinator-real-agents.integration.test.ts @@ -5,6 +5,7 @@ import { fileURLToPath } from 'node:url'; import { execFileSync } from 'node:child_process'; import { describe, expect, it } from 'vitest'; import { Coordinator } from './coordinator.js'; +import type { Notify } from '../ipc/notify.js'; import { writeToAgent } from '../ipc/pty.js'; const RUN_REAL_AGENT_SMOKE = process.env.RUN_REAL_AGENT_SMOKE === '1'; @@ -28,15 +29,10 @@ interface RendererEvent { payload: unknown; } -function createMockWindow(events: RendererEvent[]): import('electron').BrowserWindow { - return { - isDestroyed: () => false, - webContents: { - send: (channel: string, payload: unknown) => { - events.push({ channel, payload }); - }, - }, - } as unknown as import('electron').BrowserWindow; +function createMockNotify(events: RendererEvent[]): Notify { + return (channel, payload) => { + events.push({ channel, payload }); + }; } function runGit(cwd: string, args: string[]): void { @@ -174,7 +170,7 @@ describeRealAgents('Coordinator real agent startup smoke', () => { } try { - coordinator.setWindow(createMockWindow(rendererEvents)); + coordinator.setNotify(createMockNotify(rendererEvents)); coordinator.setDefaultProject('proj-1', repo); coordinator.registerCoordinator('coord-1', 'proj-1', { branchName: 'main', diff --git a/electron/mcp/coordinator-real-pty.integration.test.ts b/electron/mcp/coordinator-real-pty.integration.test.ts index c8a175ae8..f34e5d562 100644 --- a/electron/mcp/coordinator-real-pty.integration.test.ts +++ b/electron/mcp/coordinator-real-pty.integration.test.ts @@ -14,6 +14,7 @@ import { execFileSync } from 'node:child_process'; import { fileURLToPath } from 'node:url'; import { describe, expect, it } from 'vitest'; import { Coordinator } from './coordinator.js'; +import type { Notify } from '../ipc/notify.js'; const RUN_REAL_PTY = process.env.RUN_COORDINATOR_PTY_TEST === '1'; const describeRealPty = RUN_REAL_PTY ? describe : describe.skip; @@ -25,10 +26,7 @@ interface CaptureRecord { at: number; } -const mockWin = { - isDestroyed: () => false, - webContents: { send: () => undefined }, -} as unknown as import('electron').BrowserWindow; +const ignoreNotifications: Notify = () => undefined; function runGit(cwd: string, args: string[]): void { execFileSync('git', args, { cwd, stdio: 'ignore' }); @@ -81,7 +79,7 @@ async function expectPromptDeliveredOnce(params: { const assignment = `Do the ${params.profile}${params.promptSuffix ?? ''} startup assignment.`; try { - coordinator.setWindow(mockWin); + coordinator.setNotify(ignoreNotifications); coordinator.setDefaultProject('proj-1', repo); coordinator.registerCoordinator('coord-1', 'proj-1', { branchName: 'main', diff --git a/electron/mcp/coordinator-test-harness.ts b/electron/mcp/coordinator-test-harness.ts index 303af69ea..1b6aa6c37 100644 --- a/electron/mcp/coordinator-test-harness.ts +++ b/electron/mcp/coordinator-test-harness.ts @@ -1,4 +1,4 @@ -import type { BrowserWindow } from 'electron'; +import type { Notify } from '../ipc/notify.js'; import { vi } from 'vitest'; import type { AgentHookEventPayload } from '../agent-hooks/status.js'; @@ -279,10 +279,7 @@ export const { mockVerifyCancel, } = mocks; -export const mockWin = { - isDestroyed: () => false, - webContents: { send: mockNotifyRenderer }, -} as unknown as BrowserWindow; +export const mockNotify: Notify = (channel, payload) => mockNotifyRenderer(channel, payload); export function createCoordinatorTask( overrides: Partial = {}, @@ -375,7 +372,7 @@ export async function setupCoordinatorHarness(options: CoordinatorHarnessOptions return { Coordinator, coordinator, - mockWin, + mockNotify, resetCoordinatorMocks, mockNextTask, registerDefaultCoordinator, @@ -405,7 +402,7 @@ export function registerDefaultCoordinator( register = false, }: CoordinatorHarnessOptions = {}, ) { - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject(projectId, projectPath); if (register) coordinator.registerCoordinator(coordinatorId, projectId); return coordinator; diff --git a/electron/mcp/coordinator.test.ts b/electron/mcp/coordinator.test.ts index 2523d48c6..92d98c787 100644 --- a/electron/mcp/coordinator.test.ts +++ b/electron/mcp/coordinator.test.ts @@ -35,7 +35,7 @@ import { mockVerifyCancel, mockFsMkdir, mockOnAgentHookEvent, - mockWin, + mockNotify, getExitHandler, getHookEventHandler, getInterruptHandler, @@ -68,7 +68,7 @@ describe('Coordinator registerCoordinator — idempotency', () => { worktree_path: '/tmp/test', }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); }); @@ -1072,7 +1072,7 @@ describe('Coordinator coordinator notifications', () => { worktree_path: '/tmp/test', }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); }); @@ -1396,7 +1396,7 @@ describe('Coordinator signal_done', () => { worktree_path: '/tmp/test', }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); }); @@ -1503,7 +1503,7 @@ describe('Coordinator land_self', () => { vi.mocked(mockDeleteTask).mockResolvedValue(undefined); mockGit(); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1', { worktreePath: '/tmp/project' }); await coordinator.createTask({ name: 'test', prompt: 'do', coordinatorTaskId: 'coord-1' }); @@ -1935,7 +1935,7 @@ describe('Coordinator sub-agent spawn settings', () => { worktree_path: '/tmp/test', }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); }); @@ -2074,7 +2074,7 @@ describe('Coordinator settings.local.json sub-task injection', () => { beforeEach(() => { vi.clearAllMocks(); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); mockCreateBackendTask.mockResolvedValue({ @@ -2165,7 +2165,7 @@ describe('Coordinator waitForIdle', () => { worktree_path: '/tmp/test', }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); }); @@ -2268,7 +2268,7 @@ describe('Coordinator waitForSignalDone', () => { worktree_path: '/tmp/test', }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); }); @@ -2406,7 +2406,7 @@ describe('Coordinator sendPrompt', () => { worktree_path: '/tmp/test', }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); }); @@ -2814,7 +2814,7 @@ describe('Coordinator mergeTask active ownership guard', () => { lines_removed: 0, }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); }); @@ -2891,7 +2891,7 @@ describe('Coordinator deregisterCoordinator', () => { worktree_path: '/tmp/test', }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); }); @@ -3077,7 +3077,7 @@ describe('Coordinator per-task projectRoot', () => { worktree_path: '/tmp/test', }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project-a'); coordinator.registerCoordinator('coord-1', 'proj-1'); }); @@ -3109,7 +3109,7 @@ describe('Coordinator waiter resolver cleanup on timeout', () => { worktree_path: '/tmp/test', }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); }); @@ -3171,7 +3171,7 @@ describe('Coordinator MCP_TaskCreated spawn settings', () => { worktree_path: '/tmp/test', }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); }); @@ -3265,7 +3265,7 @@ describe('Coordinator sub-task MCP config isolation', () => { vi.clearAllMocks(); mockExistsSync.mockReturnValue(false); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); }); @@ -3345,7 +3345,7 @@ describe('Coordinator MCP config restart rewrite', () => { worktree_path: '/tmp/test', }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); }); @@ -3505,7 +3505,7 @@ describe('Coordinator two-class token — subtask configs use subtaskToken', () worktree_path: '/tmp/test', }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); }); @@ -3582,7 +3582,7 @@ describe('Coordinator hydrateTask — restart hydration', () => { worktree_path: '/tmp/test', }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); }); @@ -3888,7 +3888,7 @@ describe('Coordinator setTaskControl — queued send until activity lease clears worktree_path: '/tmp/test', }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); }); @@ -3979,7 +3979,7 @@ describe('Coordinator waitForSignalDone — notification lifecycle', () => { worktree_path: '/tmp/test', }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); }); @@ -4098,7 +4098,7 @@ describe('Coordinator cleanupTask — failure resilience', () => { worktree_path: '/tmp/test', }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); }); @@ -4347,7 +4347,7 @@ describe('Coordinator cleanupTask — Docker sub-task container stop', () => { worktree_path: '/tmp/test', }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); coordinator.setDockerContainerName('coord-1', 'my-coord-container'); @@ -4409,7 +4409,7 @@ describe('Coordinator setMCPServerInfo — token rotation', () => { worktree_path: '/tmp/test', }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); }); @@ -4490,12 +4490,12 @@ describe('Multiple Docker coordinators — isolation', () => { }); coordA = new Coordinator(); - coordA.setWindow(mockWin); + coordA.setNotify(mockNotify); coordA.setDefaultProject('proj-a', '/tmp/project-a'); coordA.registerCoordinator('coord-a', 'proj-a'); coordB = new Coordinator(); - coordB.setWindow(mockWin); + coordB.setNotify(mockNotify); coordB.setDefaultProject('proj-b', '/tmp/project-b'); coordB.registerCoordinator('coord-b', 'proj-b'); }); @@ -4547,7 +4547,7 @@ describe('Coordinator Docker sub-task — per-container spawn', () => { worktree_path: '/tmp/test', }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); coordinator.setDockerContainerName('coord-1', 'my-coord-container'); @@ -4593,7 +4593,7 @@ describe('Coordinator interrupted bootstrap', () => { worktree_path: '/tmp/test', }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); }); @@ -4638,7 +4638,7 @@ describe('Coordinator very fast prompt — scrollback detection', () => { worktree_path: '/tmp/test', }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); }); @@ -4708,7 +4708,7 @@ describe('Coordinator close with active sub-tasks', () => { vi.clearAllMocks(); mockExistsSync.mockReturnValue(false); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); }); @@ -4799,7 +4799,7 @@ describe('Coordinator restart hydration with Docker container name', () => { vi.clearAllMocks(); mockExistsSync.mockReturnValue(false); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); }); @@ -4856,7 +4856,7 @@ describe('Coordinator removeCoordinatedTask', () => { worktree_path: '/tmp/test', }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); }); @@ -5008,7 +5008,7 @@ describe('Coordinator restart round-trip integration', () => { vi.clearAllMocks(); mockExistsSync.mockReturnValue(false); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); }); @@ -5197,7 +5197,7 @@ describe('Coordinator hydrateTask — mcpConfigPath directory scoping', () => { vi.clearAllMocks(); mockExistsSync.mockReturnValue(false); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); coordinator.setMCPServerInfo( @@ -5345,7 +5345,7 @@ describe('Coordinator closeTask — per-task config isolation (two sub-tasks)', vi.clearAllMocks(); mockExistsSync.mockReturnValue(false); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); coordinator.setMCPServerInfo( @@ -5400,7 +5400,7 @@ describe('Coordinator Docker mode — per-container sub-tasks', () => { worktree_path: '/tmp/project/.worktrees/task/docker-sub', }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); // Register coordinator in Docker mode @@ -5520,7 +5520,7 @@ describe('Coordinator waitForSignalDone — requestId replay after transport fai worktree_path: '/tmp/test', }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); }); @@ -5661,7 +5661,7 @@ describe('Coordinator getTaskDiff — preamble-bearing files', () => { worktree_path: '/tmp/worktree', }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); vi.mocked(getDiffBaseSha).mockResolvedValue('base-sha-abc'); @@ -5894,7 +5894,7 @@ describe('Coordinator deregisterCoordinator — .mcp.json cleanup', () => { worktree_path: '/tmp/test', }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); coordinator.setMcpJsonInfo('coord-1', '/tmp/.mcp.json', false); @@ -6031,7 +6031,7 @@ describe('Coordinator createTask — deregister race', () => { beforeEach(() => { vi.clearAllMocks(); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); }); @@ -6102,7 +6102,7 @@ describe('Coordinator createTask — concurrency enforcement', () => { }; }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1', { maxConcurrentTasks: 2 }); }); @@ -6228,7 +6228,7 @@ describe('Coordinator hook-driven task state', () => { }; }); coordinator = new Coordinator(); - coordinator.setWindow(mockWin); + coordinator.setNotify(mockNotify); coordinator.setDefaultProject('proj-1', '/tmp/project'); coordinator.registerCoordinator('coord-1', 'proj-1'); }); diff --git a/electron/mcp/coordinator.ts b/electron/mcp/coordinator.ts index 70d2252a0..bbd0773e9 100644 --- a/electron/mcp/coordinator.ts +++ b/electron/mcp/coordinator.ts @@ -30,7 +30,7 @@ import { import { truncateDiffForTool } from './diff-format.js'; const execAsync = promisify(execFile); -import type { BrowserWindow } from 'electron'; +import type { Notify } from '../ipc/notify.js'; import { createTask as createBackendTask, deleteTask } from '../ipc/tasks.js'; import { getSkipPermissionsArgs } from '../shared/skip-permissions.js'; import { @@ -255,7 +255,7 @@ export class Coordinator { // so parallel create_task calls can't overshoot the limit. private pendingCreateCounts = new Map(); private recentlyDelivered = new ReplayCache(); - private win: BrowserWindow | null = null; + private notify: Notify | null = null; private projectRoot: string | null = null; private projectId: string | null = null; private defaultCoordinatorTaskId: string | null = null; @@ -745,8 +745,8 @@ export class Coordinator { } } - setWindow(win: BrowserWindow): void { - this.win = win; + setNotify(notify: Notify): void { + this.notify = notify; } setNotificationDelayMs(ms: number): void { @@ -1298,7 +1298,7 @@ export class Coordinator { this.subscribers.set(agentId, outputCb); // Spawn the agent process - if (!this.win) throw new Error('No window set on coordinator'); + if (!this.notify) throw new Error('No notifier set on coordinator'); const agentCommand = opts.agentCommand ?? coordinatorState.spawnDefaults.command; const dockerContainerName = @@ -1360,7 +1360,7 @@ export class Coordinator { await assertLaunch(); await spawnAgent( - this.win, + this.notify, { taskId: task.id, agentId, @@ -3319,8 +3319,6 @@ export class Coordinator { } private notifyRenderer(channel: string, data: unknown): void { - if (this.win && !this.win.isDestroyed()) { - this.win.webContents.send(channel, data); - } + this.notify?.(channel, data); } } From 9e0db0c607124d65c082d7580362d14965400559 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Fri, 25 Sep 2026 19:25:30 +0200 Subject: [PATCH 04/10] docs(architecture): mark agent-launch merge as structural, not a fix --- docs/runtime-separation-plan.md | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/docs/runtime-separation-plan.md b/docs/runtime-separation-plan.md index b344b50c6..5767d4c84 100644 --- a/docs/runtime-separation-plan.md +++ b/docs/runtime-separation-plan.md @@ -1,6 +1,6 @@ # Runtime separation plan -Status: revised after three reviews (two focused, one adversarial) and re-checked against the code on 2026-09-25. Steps 0, 1, 2 and 3a are done. Everything else is a proposal. References name functions rather than line numbers, because line numbers drift. +Status: revised after three reviews (two focused, one adversarial) and re-checked against the code on 2026-09-25. Steps 0, 1, 2 and 3a are done. Steps 1 and 2 fixed the only known correctness problems; everything from 3b onward is structural and waits for a named product goal (see the checkpoint after step 3). References name functions rather than line numbers, because line numbers drift. ## Why @@ -13,7 +13,7 @@ The desktop renderer is the source of truth for tasks, and the main process co-o - Document workspaces create worktrees in the same `.worktrees/` directory as tasks (`prepareAlternateWorktree` in `electron/documents/runs.ts`). Their records live in document-run state, not `state.json`. - Question detection and trust-dialog acceptance run in the renderer (`looksLikeQuestion` and `tryAutoTrust` in `src/store/taskStatus.ts`). The coordinator has a separate readiness monitor (`createAgentOutputMonitor` and `scheduleInitialPromptDelivery` in `electron/mcp/coordinator.ts`, a 3,300-line file). - Until step 1, only xterm in the renderer answered terminal queries. Codex exits if its cursor-position query goes unanswered for about 2 s (commit `1204de82`, patched by disabling `backgroundThrottling` in `electron/main.ts`). Coordinator sub-tasks are spawned in main before any view exists, so they relied on a view mounting in time. This is the only user-facing bug we found in the history that the ownership split caused; the search was by commit message, so others may exist. Step 1 fixed it. -- The coordinator creates and launches agents without the `SpawnAgent` IPC handler, so its children skip canvas MCP, the watchers and admission. This correctness gap remains open; step 4 closes it. +- The coordinator launches children without the `SpawnAgent` IPC handler, but it covers the same concerns its own way: `createTask` counts in-flight children against the concurrency limit and runs its launch guards, and the session MCP provider (`setSessionMcpProvider` in `registerAllHandlers`) registers the canvas agent, ownership and wide bind. Plan and steps watchers start when the renderer adopts the child through the same handler with `attachExisting`, and their first read picks up a plan written before that. An earlier revision called this a correctness gap; it is duplicated logic, not a bug. The only visible difference is that such a plan arrives marked `recovered`, so it does not open the canvas. - `registerAllHandlers` is a single closure of about 2,100 lines that also runs the remote-server, coordinator and delegation lifecycles. - Precedent: document workspaces already run headless agents in worktrees from main, with their own records and `reconcileInterrupted` (`electron/documents/runs.ts`). Use them as the template for runtime-owned tasks. @@ -57,7 +57,7 @@ Each of these changes runtime behavior and needs a smoke test in the real app. **3a. Notify port (done).** `pty.ts`, `git.ts`, `plans.ts`, `steps.ts` and the coordinator take a `Notify` function (`electron/ipc/notify.ts`) instead of a `BrowserWindow`, and none of them imports Electron. `registerAllHandlers` builds one with `windowNotifier` (`electron/ipc/window-notifier.ts`), which drops messages once the window is destroyed; the coordinator's `setWindow` became `setNotify`. The coordinator still refuses to launch before a notifier is set; step 4 decides whether a no-op notifier is enough for headless launches. One behavior moved: the steps watcher still reads the steps file after the window is destroyed, and only the send is dropped. Verified: `npm run check`, `lint:arch`, the unit suite and the real-PTY coordinator test. - **3b. Split `register.ts`.** #279 (pooled workspaces) was closed without merging on 2026-09-25, so it no longer blocks this. 5 of the 12 open PRs touch `register.ts` (#262, #248, #247, #162, #23). Do the split between merges, after announcing a short freeze; it does not block steps 4 and later. Before the split: + **3b. Split `register.ts`.** #279 (pooled workspaces) was closed without merging on 2026-09-25, so it no longer blocks this. 5 of the 12 open PRs touch `register.ts` (#262, #248, #247, #162, #23). Do the split between merges, after announcing a short freeze; it does not block anything before step 4. Before the split: - Commit a wiring test against the old code. It records every `ipcMain.handle` channel, throws on duplicates, asserts the `win.on` events and exactly one registration-time `onPtyEvent('exit')`, and triggers `ensureCoordinator` twice so the lazily registered `MCP_*` handlers are seen exactly once. Design rules from review: @@ -75,15 +75,17 @@ Each of these changes runtime behavior and needs a smoke test in the real app. After the split, add `electron/runtime/` with a dependency-cruiser rule that uses `reachable: true`, because a direct-import rule misses transitive Electron imports. → verify: the wiring test passes unchanged, `npm run check:static`, and a smoke test of desktop, phone and coordinator flows in the real app. -4. **Main-side agent launch.** Extract the `SpawnAgent` handler body into `launchAgent()`, used by both IPC and the coordinator. Today the coordinator bypasses canvas MCP, the watchers and admission. Move `buildTaskAgentArgs` into shared code; it takes a renderer `Task` and imports from `src/documents/`, so it first needs a narrower input type. Merge the renderer's question and trust detection with the coordinator's output monitor into one main-side `PromptDeliverer` instead of adding a third. It could read the step 1 terminal mirror's screen instead of the raw output tail; this is untested. + **Checkpoint (reached after 3a).** Reassess 3b and every later step against a named product goal, such as the phone creating tasks while the renderer is not running. None of them fixes a known bug. - Decide before starting: - - **Agent lifetime.** Define what collapsing a task means once main can launch agents without a view. Steps 6 and 7 build on this answer. - - **Settings source.** Trust acceptance depends on `autoTrustFolders`, which exists only in the renderer store and its saved state. Choose how main reads it: sent over IPC when it changes, or owned by the runtime. +4. **Main-side agent launch.** Structural: it removes duplication between the IPC and coordinator launch paths and is a prerequisite for step 6. Do it together with step 6, and only after 3b, because the `SpawnAgent` handler depends on state held in the `registerAllHandlers` closure (`pendingSpawns`, `canvasOwners`, `wideBindAgents`, `remoteServer`, `delegation`, `ensureMcpTransport`), which needs a home outside it first. + - **4a. `launchAgent()`.** Extract the `SpawnAgent` handler body, used by both IPC and the coordinator. The coordinator path keeps what is specific to it: preamble injection, the per-task MCP config file for Docker, the legacy subtask token and container arguments. Move `buildTaskAgentArgs` into shared code; it takes a renderer `Task` and imports from `src/documents/`, so it first needs a narrower input type. + - **4b. `PromptDeliverer`.** Merge the renderer's question and trust detection (`src/store/taskStatus.ts`) with the coordinator's output monitor into one main-side deliverer instead of adding a third. It could read the step 1 terminal mirror's screen instead of the raw output tail; this is untested. Plan it separately: it is larger and needs real-app smoke tests. - → verify: a unit test shows a coordinator child launched through `launchAgent()` gets canvas MCP, the watchers and admission. The existing question and trust tests pass against `PromptDeliverer`. Smoke-test a desktop task and a coordinator child in the real app. + Decided (2026-09-25): + - **Agent lifetime.** Collapsing a task keeps its current meaning: it stops the task's agents and saves their session IDs so expanding resumes them. Coordinator children cannot be collapsed. + - **Settings source.** Main reads `autoTrustFolders` from each renderer save, as it already parses them. A toggle takes effect after the next autosave (1–5 s); no new IPC channel. - **Checkpoint.** Stop here and reassess steps 5–7 against a named product goal, such as the phone creating tasks while the renderer is not running. Steps 2–4 close both known correctness gaps. + → verify: the existing IPC and coordinator launch tests pass against `launchAgent()`, and the question and trust tests pass against `PromptDeliverer`. Smoke-test a desktop task and a coordinator child in the real app. 5. **Runtime task registry.** Main writes task records it creates and reconciles them at startup. It grows out of the step 2 intent records, and its effect on `state.json` goes through the existing `normalizeState` overlay rather than a second writer with no ordering. → verify: a coordinator child survives a crash before the renderer adopts it and reappears on the next start. 6. **One task-creation path.** `TaskRuntime.createTask(spec)` provisions, registers authority, launches, delivers the prompt, records the task and emits `TaskCreated`. The renderer adopts the task with `attachExisting`, generalizing `MCP_TaskCreated`. The phone stops using `callRenderer` for creation. → verify: one test per origin (desktop, phone, coordinator) goes through `createTask` and ends with the same durable record and `TaskCreated` event. From 7b2f26bd7911a26ed3c50034c690b1f012d0d9cc Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Fri, 25 Sep 2026 19:34:06 +0200 Subject: [PATCH 05/10] test(ipc): pin registerAllHandlers wiring before the split --- electron/ipc/register-wiring.test.ts | 181 +++++++++++++++++++++++++++ 1 file changed, 181 insertions(+) create mode 100644 electron/ipc/register-wiring.test.ts diff --git a/electron/ipc/register-wiring.test.ts b/electron/ipc/register-wiring.test.ts new file mode 100644 index 000000000..cee80200f --- /dev/null +++ b/electron/ipc/register-wiring.test.ts @@ -0,0 +1,181 @@ +import type { BrowserWindow } from 'electron'; +import os from 'node:os'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { IPC } from './channels.js'; +import { registerAllHandlers } from './register.js'; + +/** + * Guards the wiring of `registerAllHandlers` while it is split into modules: + * every channel is handled exactly once, window and PTY listeners are attached + * once, and the coordinator handlers stay lazy and register exactly once. + */ + +type Handler = (event: unknown, args: Record) => unknown; + +const { handlers, duplicates, onPtyEvent } = vi.hoisted(() => ({ + handlers: new Map) => unknown>(), + duplicates: [] as string[], + onPtyEvent: vi.fn((_event: string, _listener: (agentId: string) => void) => () => {}), +})); + +vi.mock('electron', () => ({ + ipcMain: { + handle: ( + channel: string, + handler: (event: unknown, args: Record) => unknown, + ) => { + if (handlers.has(channel)) duplicates.push(channel); + handlers.set(channel, handler); + }, + }, + app: { getPath: () => os.tmpdir(), isPackaged: false }, + dialog: {}, + shell: {}, + clipboard: {}, + BrowserWindow: {}, + Notification: {}, +})); +vi.mock('./pty.js', async (original) => ({ + ...(await original()), + onPtyEvent, +})); +// Only what `ensureCoordinator` calls exists, so StartMCPServer stops right after it. +vi.mock('../mcp/coordinator.js', () => ({ + Coordinator: class { + setOrchestrationEnabled(): void {} + setNotify(): void {} + setSessionMcpProvider(): void {} + }, +})); + +/** Manifest channels `registerAllHandlers` must not handle. */ +const NOT_HANDLED_HERE: readonly string[] = [ + // Registered by `electron/main.ts` through other modules. + IPC.BrowserBounds, + IPC.BrowserCommand, + IPC.LogFromRenderer, + // Main → renderer only. + IPC.AgentHookEvent, + IPC.BrowserState, + IPC.DelegationChanged, + IPC.DocumentAnnotationEvent, + IPC.DocumentChanged, + IPC.DocumentRunEvent, + IPC.MCP_CoordinatorNotificationCleared, + IPC.MCP_CoordinatorNotificationStaged, + IPC.MCP_CoordinatorOrphanedNotification, + IPC.MCP_OpenCanvasRequest, + IPC.MCP_PublishTourRequest, + IPC.MCP_ReadMindMapRequest, + IPC.MCP_ReadReasoningRequest, + IPC.MCP_StaleUrlWarning, + IPC.MCP_TaskCleanupFailed, + IPC.MCP_TaskClosed, + IPC.MCP_TaskCreated, + IPC.MCP_TaskHydrated, + IPC.MCP_TaskStateSync, + IPC.MCP_UpdateMindMapRequest, + IPC.MCP_UpdateReasoningRequest, + IPC.NotificationClicked, + IPC.NotificationFailed, + IPC.PlanContent, + IPC.PrChecksUpdate, + IPC.Remote_CreateTaskRequest, + IPC.Remote_GetNotesRequest, + IPC.Remote_GetProjectsRequest, + IPC.Remote_SetNotesRequest, + IPC.StepsContent, + IPC.SuperProductivityOpenTaskRequested, + IPC.UpdateStatusChanged, + IPC.WindowBlur, + IPC.WindowCloseRequested, + IPC.WindowFocus, + IPC.WindowMoved, + IPC.WindowResized, +]; + +/** Registered only once the coordinator is loaded. */ +const COORDINATOR_CHANNELS: readonly string[] = [ + IPC.MCP_ControlChanged, + IPC.MCP_CoordinatedTaskClosed, + IPC.MCP_CoordinatedTaskPromptDelivered, + IPC.MCP_CoordinatorDeregistered, + IPC.MCP_CoordinatorNotificationAck, + IPC.MCP_CoordinatorNotificationDropAck, + IPC.MCP_CoordinatorRegistered, + IPC.MCP_CoordinatorRestageAfterUserSend, + IPC.MCP_HydrateCoordinatedTask, + IPC.MCP_TaskLandingReviewCleared, +]; + +function register(): ReturnType { + const on = vi.fn(); + const win = { on, isDestroyed: () => false, webContents: { send: vi.fn() } }; + registerAllHandlers(win as unknown as BrowserWindow); + return on; +} + +/** Loads the coordinator through StartMCPServer; the stub makes the rest of it fail. */ +async function startCoordinator(): Promise { + const start: Handler | undefined = handlers.get(IPC.StartMCPServer); + const pending = start?.(undefined, { + coordinatorTaskId: '12345678-1234-4234-8234-123456789abc', + projectId: 'project', + projectRoot: os.tmpdir(), + }); + await Promise.resolve(pending).catch(() => undefined); +} + +const sorted = (values: Iterable) => [...values].sort(); + +afterEach(() => { + handlers.clear(); + duplicates.length = 0; + vi.clearAllMocks(); +}); + +describe('registerAllHandlers wiring', () => { + it('handles every manifest channel exactly once, coordinator channels only when loaded', async () => { + register(); + const eager = new Set(handlers.keys()); + await startCoordinator(); + await startCoordinator(); + + expect(duplicates).toEqual([]); + expect(sorted(COORDINATOR_CHANNELS.filter((c) => eager.has(c)))).toEqual([]); + expect(sorted([...handlers.keys()].filter((c) => !eager.has(c)))).toEqual( + sorted(COORDINATOR_CHANNELS), + ); + expect(sorted(Object.values(IPC).filter((c) => !handlers.has(c)))).toEqual( + sorted(NOT_HANDLED_HERE), + ); + expect( + sorted([...handlers.keys()].filter((c) => !Object.values(IPC).includes(c))), + ).toEqual([]); + }); + + it('attaches window listeners and the PTY exit listener once', () => { + const on = register(); + + expect(sorted(on.mock.calls.map(([event]) => String(event)))).toEqual( + sorted([ + // register.ts + 'blur', + 'close', + 'closed', + 'focus', + 'move', + 'resize', + // pr-checks.ts polls only while the window is visible + 'hide', + 'minimize', + 'restore', + 'show', + // browser.ts + 'closed', + ]), + ); + expect(onPtyEvent.mock.calls.map(([event]) => event)).toEqual(['exit']); + }); +}); From 3a299d8b81eeea7e1e065e62cbef9690d40a89f6 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Fri, 25 Sep 2026 19:37:17 +0200 Subject: [PATCH 06/10] refactor(remote): move the remote transport lifecycle out of register.ts --- electron/ipc/register.ts | 304 +++++++------------------------ electron/ipc/remote-transport.ts | 250 +++++++++++++++++++++++++ 2 files changed, 316 insertions(+), 238 deletions(-) create mode 100644 electron/ipc/remote-transport.ts diff --git a/electron/ipc/register.ts b/electron/ipc/register.ts index 0659a7c1a..4b0862125 100644 --- a/electron/ipc/register.ts +++ b/electron/ipc/register.ts @@ -118,6 +118,7 @@ import { import { createTask, deleteTask } from './tasks.js'; import { settleWorktreeIntents } from './worktree-intents.js'; import { windowNotifier } from './window-notifier.js'; +import { createRemoteTransport } from './remote-transport.js'; import { listAgents } from './agents.js'; import { saveAppState, @@ -211,22 +212,6 @@ export function buildCoordinatorMCPConfig(opts: CoordinatorMCPConfigOpts): { }; } -async function startRemoteServerOnFreePort( - start: number, - end: number, - opts: Omit[0], 'port'>, -): Promise>> { - for (let port = start; port <= end; port++) { - try { - return await startRemoteServer({ ...opts, port }); - } catch (err) { - if ((err as NodeJS.ErrnoException).code === 'EADDRINUSE' && port < end) continue; - throw err; - } - } - throw new Error(`No free port found in range ${start}–${end}`); -} - function isMissingCommandError(err: unknown, command: string): boolean { const e = err as { path?: unknown; syscall?: unknown }; return ( @@ -533,12 +518,14 @@ export function registerAllHandlers(win: BrowserWindow): void { } catch (error) { console.warn('Could not remove chat canvas credentials:', error); } - void stopIdleMcpTransport().catch((error) => - console.warn('[MCP] Could not stop the idle chat transport:', error), - ); + void transport + .stopIfIdle() + .catch((error) => + console.warn('[MCP] Could not stop the idle chat transport:', error), + ); }; try { - const server = await ensureMcpTransport(false); + const server = await transport.ensureForMcp(false); const token = server.registerCanvasAgent(taskId, canvasId, () => active); unregister = () => server.unregisterCanvasAgent(canvasId); const serverPath = path @@ -587,7 +574,16 @@ export function registerAllHandlers(win: BrowserWindow): void { // --- Remote access state --- // Keep development phone access and coordinator ports separate from the installed app. const defaultRemotePort = app.isPackaged ? 7777 : 8777; - let remoteServer: Awaited> | null = null; + const transport = createRemoteTransport({ + defaultPort: defaultRemotePort, + serverOptions: () => remoteServerOptions(), + coordinator: () => coordinator, + needsWideBind: (dockerMode) => needsWideBind(dockerMode), + wideBindInUse: () => + wideBindAgents.size > 0 || + (process.platform !== 'linux' && delegation.requiresWideTransport()), + rememberedDevicesPath: () => path.join(getUserDataDir(), 'paired-phones.json'), + }); const taskNames = new Map(); // Renderer-derived per-task attention (needs input, working, ready, …), pushed // from the renderer via Remote_UpdateTaskStatus so the mobile overview can show @@ -604,16 +600,6 @@ export function registerAllHandlers(win: BrowserWindow): void { let coordinator: CoordinatorType | null = null; let coordinatorHandlersRegistered = false; let lastMcpConfigPath: string | null = null; - // True when the remote server was started by StartMCPServer (not the manual StartRemoteServer). - // Used to stop the server automatically when the last MCP coordinator deregisters. - let remoteServerStartedForMcp = false; - // True when the user has explicitly requested remote access via StartRemoteServer. - // Prevents auto-stop when coordinator deregisters even if MCP started the server first. - let remoteServerRequestedManually = false; - // True when StopRemoteServer was called while a coordinator was active. - // The server will be stopped when the last coordinator deregisters. - let remoteServerPendingStop = false; - // A kill must also cancel startup while optional MCP transport is awaiting I/O. const pendingSpawns = new Map(); /** Which spawn minted the agent's live canvas token. `pendingSpawns` cannot answer this: a @@ -639,7 +625,7 @@ export function registerAllHandlers(win: BrowserWindow): void { }, currentCoordinator: () => coordinator, prepareParent: prepareDelegationParent, - sessions: () => remoteServer?.getSessionAgents() ?? [], + sessions: () => transport.current()?.getSessionAgents() ?? [], changed: (event) => { if (!win.isDestroyed()) win.webContents.send(IPC.DelegationChanged, event); }, @@ -657,7 +643,7 @@ export function registerAllHandlers(win: BrowserWindow): void { async function prepareDelegationParent(task: TaskAuthorityInput): Promise { await ensureCoordinator(); if (!coordinator) throw new Error('Delegation unavailable.'); - const server = await ensureMcpTransport(task.dockerMode === true); + const server = await transport.ensureForMcp(task.dockerMode === true); const hostServerPath = path .join(path.dirname(fileURLToPath(import.meta.url)), '..', 'mcp-server.cjs') .replace('/app.asar/', '/app.asar.unpacked/'); @@ -711,11 +697,11 @@ export function registerAllHandlers(win: BrowserWindow): void { canvasOwners.delete(agentId); // The server's own exit listener runs after this one; drop the agent here so the // idle check below already sees it gone. - remoteServer?.unregisterCanvasAgent(agentId); + transport.current()?.unregisterCanvasAgent(agentId); delegation.expireMessages(); - stopIdleMcpTransport().catch((error) => - console.warn('[MCP] Could not stop the idle transport:', error), - ); + transport + .stopIfIdle() + .catch((error) => console.warn('[MCP] Could not stop the idle transport:', error)); }); // --- PTY commands --- @@ -758,13 +744,18 @@ export function registerAllHandlers(win: BrowserWindow): void { if (!existing) { wideBindAgents.delete(args.agentId); canvasOwners.delete(args.agentId); - remoteServer?.unregisterCanvasAgent(args.agentId); + transport.current()?.unregisterCanvasAgent(args.agentId); removeCanvasConfig(args.agentId); delegation.expireMessages(); } let canvasTools = existing?.canvasTools === true; let releaseCanvas: (() => void) | undefined; - if (!existing && !args.isShell && remoteServer && canConfigureCanvasMcp(args.command, [])) { + if ( + !existing && + !args.isShell && + transport.current() && + canConfigureCanvasMcp(args.command, []) + ) { canvasTools = !!( coordinator?.isRegisteredCoordinator(args.taskId) || coordinator?.getTask(args.taskId) ); @@ -775,7 +766,7 @@ export function registerAllHandlers(win: BrowserWindow): void { coordinator?.getTask(args.taskId) && delegation.getTask(args.taskId) ) { - const server = await ensureMcpTransport(args.dockerMode === true); + const server = await transport.ensureForMcp(args.dockerMode === true); assertPendingSpawn(); canvasOwners.set(args.agentId, pending); const child = coordinator.getTask(args.taskId); @@ -833,7 +824,7 @@ export function registerAllHandlers(win: BrowserWindow): void { canConfigureCanvasMcp(args.command, args.args) ) { try { - const server = await ensureMcpTransport(args.dockerMode === true); + const server = await transport.ensureForMcp(args.dockerMode === true); assertPendingSpawn(); const thisDir = path.dirname(fileURLToPath(import.meta.url)); const serverPath = path @@ -880,7 +871,7 @@ export function registerAllHandlers(win: BrowserWindow): void { assertPendingSpawn(); } catch (cancelled) { // The kill arrived while the transport was starting; nothing will register on it. - await stopIdleMcpTransport(); + await transport.stopIfIdle(); throw cancelled; } console.warn('Canvas MCP unavailable; starting the agent without canvas tools:', error); @@ -918,7 +909,8 @@ export function registerAllHandlers(win: BrowserWindow): void { } } } - const session = remoteServer + const session = transport + .current() ?.getSessionAgents() .find((item) => item.agentId === args.agentId); return { @@ -929,7 +921,7 @@ export function registerAllHandlers(win: BrowserWindow): void { } catch (error) { if (canvasOwners.get(args.agentId) === pending) { canvasOwners.delete(args.agentId); - remoteServer?.unregisterCanvasAgent(args.agentId); + transport.current()?.unregisterCanvasAgent(args.agentId); wideBindAgents.delete(args.agentId); removeCanvasConfig(args.agentId); delegation.expireMessages(); @@ -1798,7 +1790,6 @@ export function registerAllHandlers(win: BrowserWindow): void { getTaskContext: (taskId: string) => taskContext.get(taskId), }; - type RemoteServerHandle = Awaited>; const remoteServerOptions = (): Omit< Parameters[0], 'port' | 'host' @@ -1819,92 +1810,6 @@ export function registerAllHandlers(win: BrowserWindow): void { ...mobileTaskBridge, }; }; - // One in-flight start for manual and MCP callers alike: a spawn during a manual start - // (or the reverse) waits for that listener instead of opening a second, orphaned one. - // Callers loop on it inline so an idle path reaches startTransport without yielding; - // a failed start reports to its own caller, waiters simply see no server. - let mcpTransportStarting: Promise | undefined; - function startTransport( - start: () => Promise, - forMcp: boolean, - ): Promise { - const starting = (async () => { - try { - const server = await start(); - remoteServer = server; - remoteServerStartedForMcp = forMcp; - return server; - } finally { - mcpTransportStarting = undefined; - } - })(); - mcpTransportStarting = starting; - return starting; - } - // Stopping the server while its listener is being re-bound would orphan the new listener - // and hand callers a stopped handle; an exit during that window defers to the rebind. - let transportRebinding: Promise | undefined; - /** A rebind that leaves nothing listening hands back a dead handle; drop it. */ - async function rebindTransport(server: RemoteServerHandle, host: string): Promise { - transportRebinding = server.rebind(host); - try { - await transportRebinding; - } catch (error) { - if (!server.listening && remoteServer === server) { - remoteServer = null; - remoteServerStartedForMcp = false; - remoteServerRequestedManually = false; - remoteServerPendingStop = false; - } - throw error; - } finally { - transportRebinding = undefined; - // The last agent may have exited meanwhile; its deferred idle check runs once now. - await stopIdleMcpTransport(); - } - } - /** An MCP-only server has no reason to run once neither a coordinator nor a canvas agent needs it. */ - async function stopIdleMcpTransport(): Promise { - const server = remoteServer; - if (transportRebinding || !server) return; - if (coordinator?.hasActiveCoordinator() || server.hasCanvasAgents()) return; - if (!remoteServerPendingStop && !(remoteServerStartedForMcp && !remoteServerRequestedManually)) - return; - const forgetDevices = remoteServerPendingStop; - remoteServer = null; - remoteServerStartedForMcp = false; - remoteServerRequestedManually = false; - remoteServerPendingStop = false; - await server.stop(forgetDevices); - } - const warnDockerBind = () => - console.warn( - '[MCP] Docker mode (macOS): MCP server bound to 0.0.0.0 — reachable from local network ' + - 'interfaces. Traffic from containers uses Docker Desktop internal routing; the bearer ' + - 'token still gates every request.', - ); - async function ensureMcpTransport(dockerMode: boolean): Promise { - while (mcpTransportStarting) await mcpTransportStarting.catch(() => undefined); - // Docker mode on macOS requires 0.0.0.0: containers reach the host through - // host.docker.internal, which routes through Docker Desktop's virtual adapter. - const wideOpen = needsWideBind(dockerMode); - if (remoteServer && wideOpen && remoteServer.bindHost === '127.0.0.1') { - warnDockerBind(); - await rebindTransport(remoteServer, '0.0.0.0'); - } - // The idle check after a rebind may have released a transport nobody used any more. - if (remoteServer) return remoteServer; - if (wideOpen) warnDockerBind(); - return startTransport( - () => - startRemoteServerOnFreePort(defaultRemotePort, defaultRemotePort + 23, { - host: wideOpen ? '0.0.0.0' : '127.0.0.1', - ...remoteServerOptions(), - }), - true, - ); - } - const VALID_ATTENTION: ReadonlySet = new Set([ 'idle', 'active', @@ -1951,115 +1856,35 @@ export function registerAllHandlers(win: BrowserWindow): void { } } // Only bother rebroadcasting when a phone could be listening. - if (remoteServer) notifyAgentListChanged(); + if (transport.current()) notifyAgentListChanged(); }, ); ipcMain.handle(IPC.GeneratePairingPin, () => { - if (!remoteServer) throw new Error('Remote server is not running'); - return remoteServer.generatePairingPin(); + const server = transport.current(); + if (!server) throw new Error('Remote server is not running'); + return server.generatePairingPin(); }); // --- Remote access --- - ipcMain.handle(IPC.StartRemoteServer, async (_e, args: { port?: number }) => { - while (mcpTransportStarting) await mcpTransportStarting.catch(() => undefined); - // If server was started for MCP-only (loopback), rebind to 0.0.0.0 so WiFi/Tailscale - // clients can reach it. Skip rebind while a coordinator is active — restarting the - // server would break ongoing MCP connections. - if (remoteServer && remoteServerStartedForMcp && !coordinator?.hasActiveLegacyCoordinator()) { - await rebindTransport(remoteServer, '0.0.0.0'); - } - // The idle check after a rebind may have released a transport nobody used any more. - if (remoteServer) { - // Loopback-only means the server is MCP-only and inaccessible from other devices. - // Return unavailableReason without marking this as a successful manual start. - if (remoteServer.bindHost === '127.0.0.1') { - return { - url: remoteServer.url, - wifiUrl: null, - tailscaleUrl: null, - port: remoteServer.port, - unavailableReason: 'coordinator_active' as const, - }; - } - remoteServer.enableRememberedDevices(path.join(getUserDataDir(), 'paired-phones.json')); - remoteServerRequestedManually = true; - remoteServerPendingStop = false; - return { - url: remoteServer.url, - wifiUrl: remoteServer.wifiUrl, - tailscaleUrl: remoteServer.tailscaleUrl, - port: remoteServer.port, - }; - } - - // Remote access is an explicit user action — bind to all interfaces so WiFi/Tailscale clients - // can reach the SPA. Coordinator MCP-only mode uses 127.0.0.1 by default. - const server = await startTransport( - () => - startRemoteServer({ - port: args.port ?? defaultRemotePort, - host: '0.0.0.0', - ...remoteServerOptions(), - }), - false, - ); - server.enableRememberedDevices(path.join(getUserDataDir(), 'paired-phones.json')); - remoteServerRequestedManually = true; - remoteServerPendingStop = false; - return { - url: server.url, - wifiUrl: server.wifiUrl, - tailscaleUrl: server.tailscaleUrl, - port: server.port, - }; - }); + ipcMain.handle(IPC.StartRemoteServer, (_e, args: { port?: number }) => + transport.startRemoteAccess(args.port), + ); - ipcMain.handle(IPC.StopRemoteServer, async (): Promise<{ stopped: boolean; reason?: string }> => { - if (!remoteServer) return { stopped: true }; - if (coordinator?.hasActiveLegacyCoordinator()) { - // The coordinator MCP transport shares this HTTP server. Stopping it while - // a coordinator is active would break all in-flight MCP tool calls. - // Record the pending stop so the last coordinator deregistration will auto-stop. - remoteServerPendingStop = true; - console.warn( - '[Remote] Stop requested but coordinator MCP is active — will stop on last coordinator exit', - ); - return { stopped: false, reason: 'coordinator_active' }; - } - if (remoteServer.hasCanvasAgents() || coordinator?.hasActiveCoordinator()) { - // Running agents keep their canvas transport; only the phone-facing access ends. - // Loopback is unreachable from other devices, so the shared URL stops working. - if ( - wideBindAgents.size > 0 || - (process.platform !== 'linux' && delegation.requiresWideTransport()) - ) - return { stopped: false, reason: 'docker_active' }; - remoteServer.forgetRememberedDevices(); - // Mark it MCP-only before narrowing so the idle check after the rebind may release it. - remoteServerStartedForMcp = true; - remoteServerRequestedManually = false; - remoteServerPendingStop = false; - await rebindTransport(remoteServer, '127.0.0.1'); - return { stopped: true }; - } - await remoteServer.stop(true); - remoteServer = null; - remoteServerRequestedManually = false; - return { stopped: true }; - }); + ipcMain.handle(IPC.StopRemoteServer, () => transport.stopRemoteAccess()); ipcMain.handle(IPC.GetRemoteStatus, () => { - if (!remoteServer || remoteServer.bindHost === '127.0.0.1') { + const server = transport.current(); + if (!server || server.bindHost === '127.0.0.1') { return { enabled: false, connectedClients: 0 }; } return { enabled: true, - connectedClients: remoteServer.connectedClients(), - url: remoteServer.url, - wifiUrl: remoteServer.wifiUrl, - tailscaleUrl: remoteServer.tailscaleUrl, - port: remoteServer.port, + connectedClients: server.connectedClients(), + url: server.url, + wifiUrl: server.wifiUrl, + tailscaleUrl: server.tailscaleUrl, + port: server.port, }; }); @@ -2130,7 +1955,7 @@ export function registerAllHandlers(win: BrowserWindow): void { // Stop the remote server when the last coordinator exits if: // - MCP started the server and user hasn't separately requested manual access, OR // - the user explicitly requested stop while coordinator was active (pendingStop) - await stopIdleMcpTransport(); + await transport.stopIfIdle(); }, ); @@ -2281,7 +2106,8 @@ export function registerAllHandlers(win: BrowserWindow): void { coordinator.setOrchestrationEnabled(delegation.isOrchestrationEnabled()); coordinator.setNotify(notify); coordinator.setSessionMcpProvider((task) => { - if (!delegation.getTask(task.coordinatorTaskId) || !remoteServer) return undefined; + const server = transport.current(); + if (!delegation.getTask(task.coordinatorTaskId) || !server) return undefined; if (!delegation.getTask(task.id)) { if (task.status !== 'creating') throw new Error('Task authority was not validated before restoring its MCP session.'); @@ -2295,7 +2121,7 @@ export function registerAllHandlers(win: BrowserWindow): void { canvasOwners.set(task.agentId, owner); } if (needsWideBind(!!task.dockerContainerName)) wideBindAgents.add(task.agentId); - const token = remoteServer.registerCanvasAgent(task.id, task.agentId, undefined, { + const token = server.registerCanvasAgent(task.id, task.agentId, undefined, { sessionInstanceId: owner.sessionInstanceId, capabilities: sessionCapabilities, }); @@ -2375,8 +2201,9 @@ export function registerAllHandlers(win: BrowserWindow): void { verifyCommand: args.verifyCommand, }); - await ensureMcpTransport(!!args.dockerContainerName); - if (!remoteServer) throw new Error('MCP transport unavailable.'); + await transport.ensureForMcp(!!args.dockerContainerName); + const server = transport.current(); + if (!server) throw new Error('MCP transport unavailable.'); // Resolve the source MCP server binary path. const thisDir = path.dirname(fileURLToPath(import.meta.url)); @@ -2393,7 +2220,7 @@ export function registerAllHandlers(win: BrowserWindow): void { : undefined; const mcpServerPath = dockerMcpServerPath ?? hostMcpServerPath; - const serverUrl = getMCPRemoteServerUrl(remoteServer.port, args.dockerContainerName); + const serverUrl = getMCPRemoteServerUrl(server.port, args.dockerContainerName); // Build mcpConfig and mergedMcpJson (pure computation — no filesystem or state side effects). // Doing this before any Docker copy or coordinator mutation ensures that if .mcp.json @@ -2401,7 +2228,7 @@ export function registerAllHandlers(win: BrowserWindow): void { const mcpConfig = buildCoordinatorMCPConfig({ mcpServerPath, serverUrl, - token: remoteServer.coordinatorTokenFor(args.coordinatorTaskId), + token: server.coordinatorTokenFor(args.coordinatorTaskId), coordinatorTaskId: args.coordinatorTaskId, skipPermissions: args.skipPermissions, propagateSkipPermissions: args.propagateSkipPermissions, @@ -2446,8 +2273,8 @@ export function registerAllHandlers(win: BrowserWindow): void { coordinator.setMCPServerInfo( args.coordinatorTaskId, serverUrl, - remoteServer.coordinatorTokenFor(args.coordinatorTaskId), - remoteServer.subtaskToken, + server.coordinatorTokenFor(args.coordinatorTaskId), + server.subtaskToken, mcpServerPath, ); coordinator.setCoordinatorSpawnDefaults( @@ -2517,7 +2344,7 @@ export function registerAllHandlers(win: BrowserWindow): void { configPath, mcpLaunchArgs, serverUrl, - port: remoteServer.port, + port: server.port, }; }, ); @@ -2531,9 +2358,10 @@ export function registerAllHandlers(win: BrowserWindow): void { // The MCP server process is spawned by the agent CLI via launch args, // not by us. We report whether the remote HTTP server that the MCP // server connects to is running — if it's up, MCP tools should work. + const server = transport.current(); return { - running: remoteServer !== null, - port: remoteServer?.port ?? null, + running: server !== null, + port: server?.port ?? null, // TODO: Surface this from the coordinator map if the UI needs it. coordinatorTaskId: null, mcpConfigPath: lastMcpConfigPath ?? null, diff --git a/electron/ipc/remote-transport.ts b/electron/ipc/remote-transport.ts new file mode 100644 index 000000000..3d1a80a89 --- /dev/null +++ b/electron/ipc/remote-transport.ts @@ -0,0 +1,250 @@ +import { startRemoteServer } from '../remote/server.js'; +import type { Coordinator } from '../mcp/coordinator.js'; + +export type RemoteServerHandle = Awaited>; +type RemoteServerOptions = Omit[0], 'port' | 'host'>; + +export interface RemoteTransportOptions { + defaultPort: number; + serverOptions: () => RemoteServerOptions; + coordinator: () => Pick< + Coordinator, + 'hasActiveCoordinator' | 'hasActiveLegacyCoordinator' + > | null; + /** Container agents on macOS reach the host only through a wide bind. */ + needsWideBind: (dockerMode: boolean) => boolean; + /** A running agent or delegated task still needs the wide bind. */ + wideBindInUse: () => boolean; + rememberedDevicesPath: () => string; +} + +export interface RemoteAccessInfo { + url: string; + wifiUrl: string | null; + tailscaleUrl: string | null; + port: number; + unavailableReason?: 'coordinator_active'; +} + +export interface RemoteTransport { + /** The running server, or null. Read it on each use; it changes across awaits. */ + current(): RemoteServerHandle | null; + /** The server for MCP clients, started on loopback (or a wide bind for Docker) if needed. */ + ensureForMcp(dockerMode: boolean): Promise; + /** Stops a server started only for MCP once nothing needs it. */ + stopIfIdle(): Promise; + startRemoteAccess(port?: number): Promise; + stopRemoteAccess(): Promise<{ stopped: boolean; reason?: string }>; +} + +async function startRemoteServerOnFreePort( + start: number, + end: number, + opts: Omit[0], 'port'>, +): Promise { + for (let port = start; port <= end; port++) { + try { + return await startRemoteServer({ ...opts, port }); + } catch (err) { + if ((err as NodeJS.ErrnoException).code === 'EADDRINUSE' && port < end) continue; + throw err; + } + } + throw new Error(`No free port found in range ${start}–${end}`); +} + +const warnDockerBind = () => + console.warn( + '[MCP] Docker mode (macOS): MCP server bound to 0.0.0.0 — reachable from local network ' + + 'interfaces. Traffic from containers uses Docker Desktop internal routing; the bearer ' + + 'token still gates every request.', + ); + +/** + * The HTTP/WebSocket server shared by phone access and MCP clients, and the + * rules for who started it and when it may stop. All of its state changes here. + */ +export function createRemoteTransport(opts: RemoteTransportOptions): RemoteTransport { + let server: RemoteServerHandle | null = null; + // True when the server was started by StartMCPServer (not the manual StartRemoteServer). + // Used to stop the server automatically when the last MCP coordinator deregisters. + let startedForMcp = false; + // True when the user has explicitly requested remote access via StartRemoteServer. + // Prevents auto-stop when coordinator deregisters even if MCP started the server first. + let requestedManually = false; + // True when StopRemoteServer was called while a coordinator was active. + // The server will be stopped when the last coordinator deregisters. + let pendingStop = false; + + // One in-flight start for manual and MCP callers alike: a spawn during a manual start + // (or the reverse) waits for that listener instead of opening a second, orphaned one. + // Callers loop on it inline so an idle path reaches startTransport without yielding; + // a failed start reports to its own caller, waiters simply see no server. + let starting: Promise | undefined; + function startTransport( + start: () => Promise, + forMcp: boolean, + ): Promise { + const pending = (async () => { + try { + const started = await start(); + server = started; + startedForMcp = forMcp; + return started; + } finally { + starting = undefined; + } + })(); + starting = pending; + return pending; + } + // Stopping the server while its listener is being re-bound would orphan the new listener + // and hand callers a stopped handle; an exit during that window defers to the rebind. + let rebinding: Promise | undefined; + /** A rebind that leaves nothing listening hands back a dead handle; drop it. */ + async function rebindTransport(target: RemoteServerHandle, host: string): Promise { + rebinding = target.rebind(host); + try { + await rebinding; + } catch (error) { + if (!target.listening && server === target) { + server = null; + startedForMcp = false; + requestedManually = false; + pendingStop = false; + } + throw error; + } finally { + rebinding = undefined; + // The last agent may have exited meanwhile; its deferred idle check runs once now. + await stopIfIdle(); + } + } + /** An MCP-only server has no reason to run once neither a coordinator nor a canvas agent needs it. */ + async function stopIfIdle(): Promise { + const current = server; + if (rebinding || !current) return; + if (opts.coordinator()?.hasActiveCoordinator() || current.hasCanvasAgents()) return; + if (!pendingStop && !(startedForMcp && !requestedManually)) return; + const forgetDevices = pendingStop; + server = null; + startedForMcp = false; + requestedManually = false; + pendingStop = false; + await current.stop(forgetDevices); + } + async function ensureForMcp(dockerMode: boolean): Promise { + while (starting) await starting.catch(() => undefined); + // Docker mode on macOS requires 0.0.0.0: containers reach the host through + // host.docker.internal, which routes through Docker Desktop's virtual adapter. + const wideOpen = opts.needsWideBind(dockerMode); + if (server && wideOpen && server.bindHost === '127.0.0.1') { + warnDockerBind(); + await rebindTransport(server, '0.0.0.0'); + } + // The idle check after a rebind may have released a transport nobody used any more. + if (server) return server; + if (wideOpen) warnDockerBind(); + return startTransport( + () => + startRemoteServerOnFreePort(opts.defaultPort, opts.defaultPort + 23, { + host: wideOpen ? '0.0.0.0' : '127.0.0.1', + ...opts.serverOptions(), + }), + true, + ); + } + + async function startRemoteAccess(port?: number): Promise { + while (starting) await starting.catch(() => undefined); + // If server was started for MCP-only (loopback), rebind to 0.0.0.0 so WiFi/Tailscale + // clients can reach it. Skip rebind while a coordinator is active — restarting the + // server would break ongoing MCP connections. + if (server && startedForMcp && !opts.coordinator()?.hasActiveLegacyCoordinator()) { + await rebindTransport(server, '0.0.0.0'); + } + // The idle check after a rebind may have released a transport nobody used any more. + if (server) { + // Loopback-only means the server is MCP-only and inaccessible from other devices. + // Return unavailableReason without marking this as a successful manual start. + if (server.bindHost === '127.0.0.1') { + return { + url: server.url, + wifiUrl: null, + tailscaleUrl: null, + port: server.port, + unavailableReason: 'coordinator_active' as const, + }; + } + server.enableRememberedDevices(opts.rememberedDevicesPath()); + requestedManually = true; + pendingStop = false; + return { + url: server.url, + wifiUrl: server.wifiUrl, + tailscaleUrl: server.tailscaleUrl, + port: server.port, + }; + } + + // Remote access is an explicit user action — bind to all interfaces so WiFi/Tailscale clients + // can reach the SPA. Coordinator MCP-only mode uses 127.0.0.1 by default. + const started = await startTransport( + () => + startRemoteServer({ + port: port ?? opts.defaultPort, + host: '0.0.0.0', + ...opts.serverOptions(), + }), + false, + ); + started.enableRememberedDevices(opts.rememberedDevicesPath()); + requestedManually = true; + pendingStop = false; + return { + url: started.url, + wifiUrl: started.wifiUrl, + tailscaleUrl: started.tailscaleUrl, + port: started.port, + }; + } + + async function stopRemoteAccess(): Promise<{ stopped: boolean; reason?: string }> { + if (!server) return { stopped: true }; + const coordinator = opts.coordinator(); + if (coordinator?.hasActiveLegacyCoordinator()) { + // The coordinator MCP transport shares this HTTP server. Stopping it while + // a coordinator is active would break all in-flight MCP tool calls. + // Record the pending stop so the last coordinator deregistration will auto-stop. + pendingStop = true; + console.warn( + '[Remote] Stop requested but coordinator MCP is active — will stop on last coordinator exit', + ); + return { stopped: false, reason: 'coordinator_active' }; + } + if (server.hasCanvasAgents() || coordinator?.hasActiveCoordinator()) { + // Running agents keep their canvas transport; only the phone-facing access ends. + // Loopback is unreachable from other devices, so the shared URL stops working. + if (opts.wideBindInUse()) return { stopped: false, reason: 'docker_active' }; + server.forgetRememberedDevices(); + // Mark it MCP-only before narrowing so the idle check after the rebind may release it. + startedForMcp = true; + requestedManually = false; + pendingStop = false; + await rebindTransport(server, '127.0.0.1'); + return { stopped: true }; + } + await server.stop(true); + server = null; + requestedManually = false; + return { stopped: true }; + } + + return { + current: () => server, + ensureForMcp, + stopIfIdle, + startRemoteAccess, + stopRemoteAccess, + }; +} From 6a9903078216767c708b14dc0d2e7d2e4a0e87f5 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Fri, 25 Sep 2026 19:40:22 +0200 Subject: [PATCH 07/10] refactor(mcp): resolve the MCP server path in one place --- electron/ipc/docker-config.test.ts | 7 ++--- electron/ipc/mcp-paths.ts | 21 ++++++++++++++ electron/ipc/register-mcp.test.ts | 2 +- electron/ipc/register.ts | 37 ++++++------------------- electron/mcp/docker.integration.test.ts | 2 +- 5 files changed, 33 insertions(+), 36 deletions(-) create mode 100644 electron/ipc/mcp-paths.ts diff --git a/electron/ipc/docker-config.test.ts b/electron/ipc/docker-config.test.ts index 8095f4857..7354304ec 100644 --- a/electron/ipc/docker-config.test.ts +++ b/electron/ipc/docker-config.test.ts @@ -6,11 +6,8 @@ */ import { describe, expect, it } from 'vitest'; -import { - buildCoordinatorMCPConfig, - getDockerMcpServerDestPath, - selectMcpJsonDir, -} from './register.js'; +import { buildCoordinatorMCPConfig, selectMcpJsonDir } from './register.js'; +import { getDockerMcpServerDestPath } from './mcp-paths.js'; import { getMCPRemoteServerUrl } from '../mcp/config.js'; // ── MCP server URL ───────────────────────────────────────────────────────────── diff --git a/electron/ipc/mcp-paths.ts b/electron/ipc/mcp-paths.ts new file mode 100644 index 000000000..3af68ab87 --- /dev/null +++ b/electron/ipc/mcp-paths.ts @@ -0,0 +1,21 @@ +import path from 'path'; +import { fileURLToPath } from 'url'; + +/** + * The bundled `mcp-server.cjs` on the host. Packaged builds run it from + * `app.asar.unpacked`, because a child process cannot execute from inside the archive. + * Keep this file in `electron/ipc/`: the path is relative to its build output. + */ +export function hostMcpServerPath(): string { + return path + .join(path.dirname(fileURLToPath(import.meta.url)), '..', 'mcp-server.cjs') + .replace('/app.asar/', '/app.asar.unpacked/'); +} + +/** Path where `mcp-server.cjs` is copied inside the Docker-mounted worktree. */ +export function getDockerMcpServerDestPath( + worktreePath: string | undefined, + projectRoot: string, +): string { + return path.join(worktreePath ?? projectRoot, '.parallel-code', 'mcp-server.cjs'); +} diff --git a/electron/ipc/register-mcp.test.ts b/electron/ipc/register-mcp.test.ts index 6d5c30b66..baf313176 100644 --- a/electron/ipc/register-mcp.test.ts +++ b/electron/ipc/register-mcp.test.ts @@ -16,10 +16,10 @@ import path from 'path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { buildCoordinatorMCPConfig, - getDockerMcpServerDestPath, selectMcpJsonDir, validateStartMCPServerArgs, } from './register.js'; +import { getDockerMcpServerDestPath } from './mcp-paths.js'; import { getMCPRemoteServerUrl } from '../mcp/config.js'; import { startRemoteServer } from '../remote/server.js'; diff --git a/electron/ipc/register.ts b/electron/ipc/register.ts index 4b0862125..b48b043a5 100644 --- a/electron/ipc/register.ts +++ b/electron/ipc/register.ts @@ -119,6 +119,7 @@ import { createTask, deleteTask } from './tasks.js'; import { settleWorktreeIntents } from './worktree-intents.js'; import { windowNotifier } from './window-notifier.js'; import { createRemoteTransport } from './remote-transport.js'; +import { getDockerMcpServerDestPath, hostMcpServerPath } from './mcp-paths.js'; import { listAgents } from './agents.js'; import { saveAppState, @@ -165,14 +166,6 @@ export function selectMcpJsonDir(worktreePath: string | undefined, projectRoot: return worktreePath ?? projectRoot; } -/** Path where `mcp-server.cjs` is copied inside the Docker-mounted worktree. */ -export function getDockerMcpServerDestPath( - worktreePath: string | undefined, - projectRoot: string, -): string { - return path.join(worktreePath ?? projectRoot, '.parallel-code', 'mcp-server.cjs'); -} - export interface CoordinatorMCPConfigOpts { mcpServerPath: string; serverUrl: string; @@ -528,9 +521,7 @@ export function registerAllHandlers(win: BrowserWindow): void { const server = await transport.ensureForMcp(false); const token = server.registerCanvasAgent(taskId, canvasId, () => active); unregister = () => server.unregisterCanvasAgent(canvasId); - const serverPath = path - .join(path.dirname(fileURLToPath(import.meta.url)), '..', 'mcp-server.cjs') - .replace('/app.asar/', '/app.asar.unpacked/'); + const serverPath = hostMcpServerPath(); const launchArgs = prepareCanvasMcpArgs({ command, taskId, @@ -644,9 +635,7 @@ export function registerAllHandlers(win: BrowserWindow): void { await ensureCoordinator(); if (!coordinator) throw new Error('Delegation unavailable.'); const server = await transport.ensureForMcp(task.dockerMode === true); - const hostServerPath = path - .join(path.dirname(fileURLToPath(import.meta.url)), '..', 'mcp-server.cjs') - .replace('/app.asar/', '/app.asar.unpacked/'); + const hostServerPath = hostMcpServerPath(); const serverPath = task.dockerMode ? getDockerMcpServerDestPath(task.worktreePath, task.projectRoot) : hostServerPath; @@ -784,9 +773,7 @@ export function registerAllHandlers(win: BrowserWindow): void { }); launchArgs = prepareCanvasMcpArgs({ ...args, - serverPath: path - .join(path.dirname(fileURLToPath(import.meta.url)), '..', 'mcp-server.cjs') - .replace('/app.asar/', '/app.asar.unpacked/'), + serverPath: hostMcpServerPath(), port: server.port, token, sessionCapabilities: capabilities, @@ -826,10 +813,7 @@ export function registerAllHandlers(win: BrowserWindow): void { try { const server = await transport.ensureForMcp(args.dockerMode === true); assertPendingSpawn(); - const thisDir = path.dirname(fileURLToPath(import.meta.url)); - const serverPath = path - .join(thisDir, '..', 'mcp-server.cjs') - .replace('/app.asar/', '/app.asar.unpacked/'); + const serverPath = hostMcpServerPath(); const capabilities = delegation.capabilities(args.taskId); const token = server.registerCanvasAgent( args.taskId, @@ -2205,12 +2189,7 @@ export function registerAllHandlers(win: BrowserWindow): void { const server = transport.current(); if (!server) throw new Error('MCP transport unavailable.'); - // Resolve the source MCP server binary path. - const thisDir = path.dirname(fileURLToPath(import.meta.url)); - let hostMcpServerPath = path.join(thisDir, '..', 'mcp-server.cjs'); - if (hostMcpServerPath.includes('/app.asar/')) { - hostMcpServerPath = hostMcpServerPath.replace('/app.asar/', '/app.asar.unpacked/'); - } + const hostServerPath = hostMcpServerPath(); // In Docker mode the server is copied into the worktree so the container can reach it. // Compute the destination path now (pure, no side effects) so we can build mcpConfig @@ -2218,7 +2197,7 @@ export function registerAllHandlers(win: BrowserWindow): void { const dockerMcpServerPath = args.dockerContainerName ? getDockerMcpServerDestPath(args.worktreePath, args.projectRoot) : undefined; - const mcpServerPath = dockerMcpServerPath ?? hostMcpServerPath; + const mcpServerPath = dockerMcpServerPath ?? hostServerPath; const serverUrl = getMCPRemoteServerUrl(server.port, args.dockerContainerName); @@ -2253,7 +2232,7 @@ export function registerAllHandlers(win: BrowserWindow): void { // Docker filesystem writes, MCP config file writes. if (dockerMcpServerPath) { fs.mkdirSync(path.dirname(dockerMcpServerPath), { recursive: true }); - fs.copyFileSync(hostMcpServerPath, dockerMcpServerPath); // nosemgrep: semgrep.copyfilesync-side-effect -- all pure computation (mcpConfig, mergedMcpJson) is done above; this is correctly ordered + fs.copyFileSync(hostServerPath, dockerMcpServerPath); // nosemgrep: semgrep.copyfilesync-side-effect -- all pure computation (mcpConfig, mergedMcpJson) is done above; this is correctly ordered coordinator.setDockerContainerName(args.coordinatorTaskId, args.dockerContainerName ?? ''); coordinator.setDockerImage(args.coordinatorTaskId, args.dockerImage ?? null); console.warn('[MCP] Docker mode: copied MCP server to', dockerMcpServerPath); diff --git a/electron/mcp/docker.integration.test.ts b/electron/mcp/docker.integration.test.ts index 5d8ce0202..bd28f8dcc 100644 --- a/electron/mcp/docker.integration.test.ts +++ b/electron/mcp/docker.integration.test.ts @@ -12,10 +12,10 @@ import { getMCPRemoteServerUrl, getSubTaskMcpConfigPath } from './config.js'; import type { Coordinator } from './coordinator.js'; import { selectMcpJsonDir, - getDockerMcpServerDestPath, buildCoordinatorMCPConfig, validateStartMCPServerArgs, } from '../ipc/register.js'; +import { getDockerMcpServerDestPath } from '../ipc/mcp-paths.js'; const RUN_DOCKER_MCP_TEST = process.env.RUN_DOCKER_MCP_TEST === '1'; From 2c2c1f13a058a8eeeeefee41f8164fded02156f4 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Fri, 25 Sep 2026 19:49:10 +0200 Subject: [PATCH 08/10] refactor(mcp): move coordinator lifecycle into mcp-runtime registerAllHandlers no longer owns the MCP runtime state: the transport, the lazy coordinator, delegation, the PTY exit cleanup and the canvas and wide-bind ownership maps now live in createMcpRuntime. Handlers read the coordinator through mcp.coordinator(). The copyFileSync and writeFileSync Semgrep rules cover the new file. --- .semgrep/filesystem-safety.yml | 2 + electron/ipc/mcp-runtime.ts | 213 +++++++++++++++++++++++++++++++ electron/ipc/register.ts | 189 ++++----------------------- electron/ipc/remote-transport.ts | 2 +- 4 files changed, 238 insertions(+), 168 deletions(-) create mode 100644 electron/ipc/mcp-runtime.ts diff --git a/.semgrep/filesystem-safety.yml b/.semgrep/filesystem-safety.yml index 521a98c2c..11ff52240 100644 --- a/.semgrep/filesystem-safety.yml +++ b/.semgrep/filesystem-safety.yml @@ -12,6 +12,7 @@ rules: include: - '**/electron/mcp/**' - '**/electron/ipc/register.ts' + - '**/electron/ipc/mcp-runtime.ts' exclude: - '**/electron/mcp/atomic.ts' - '**/*.test.ts' @@ -27,3 +28,4 @@ rules: paths: include: - '**/electron/ipc/register.ts' + - '**/electron/ipc/mcp-runtime.ts' diff --git a/electron/ipc/mcp-runtime.ts b/electron/ipc/mcp-runtime.ts new file mode 100644 index 000000000..d1ede815d --- /dev/null +++ b/electron/ipc/mcp-runtime.ts @@ -0,0 +1,213 @@ +import crypto from 'crypto'; +import fs from 'fs'; +import path from 'path'; +import { IPC } from './channels.js'; +import type { Notify } from './notify.js'; +import { onPtyEvent } from './pty.js'; +import { appendGitInfoExcludeBlock } from './git-exclude.js'; +import { loadAppState, saveAppState } from './persistence.js'; +import { getDockerMcpServerDestPath, hostMcpServerPath } from './mcp-paths.js'; +import { + createRemoteTransport, + type RemoteServerOptions, + type RemoteTransport, +} from './remote-transport.js'; +import { DelegationService } from '../mcp/delegation.js'; +import type { Coordinator } from '../mcp/coordinator.js'; +import { removeCanvasConfig } from '../mcp/canvas-config.js'; +import { getMCPRemoteServerUrl } from '../mcp/config.js'; +import type { TaskAuthorityInput } from '../shared/delegation-types.js'; +import { warn as logWarn, errMessage } from '../log.js'; + +export interface McpRuntimeOptions { + notify: Notify; + defaultPort: number; + serverOptions: () => RemoteServerOptions; + rememberedDevicesPath: () => string; + /** Runs once, when the coordinator first loads; its IPC handlers are registered here. */ + onCoordinatorLoaded: () => void; +} + +export interface McpRuntime { + transport: RemoteTransport; + delegation: DelegationService; + /** A kill must also cancel startup while optional MCP transport is awaiting I/O. */ + pendingSpawns: Map; + /** Which spawn minted the agent's live canvas token. `pendingSpawns` cannot answer this: a + * kill clears the entry to cancel the spawn, and a finished restart clears its own, so an + * absent entry means both "nobody owns this" and "the owner already left". */ + canvasOwners: Map; + /** Container agents on macOS reach the host only through a wide bind; track who needs it. */ + wideBindAgents: Set; + needsWideBind: (dockerMode: boolean) => boolean; + /** The coordinator, or null until `ensureCoordinator` has loaded it. It is never unloaded. */ + coordinator(): Coordinator | null; + /** Lazily loads the shared coordination backend. Safe to call multiple times. */ + ensureCoordinator(): Promise; +} + +function restoreOrchestrationEnabled(): boolean { + try { + const saved = loadAppState(); + return saved ? JSON.parse(saved).mcpOrchestrationEnabled !== false : true; + } catch (error) { + logWarn('mcp', 'Could not restore orchestration setting; keeping orchestration disabled', { + err: errMessage(error), + }); + return false; + } +} + +/** + * The MCP side of the main process: the remote transport, the lazily loaded + * coordinator, delegation, and which agents hold canvas tokens or a wide bind. + */ +export function createMcpRuntime(opts: McpRuntimeOptions): McpRuntime { + const { notify } = opts; + let coordinator: Coordinator | null = null; + const pendingSpawns = new Map(); + const canvasOwners = new Map(); + const wideBindAgents = new Set(); + const needsWideBind = (dockerMode: boolean) => dockerMode && process.platform !== 'linux'; + + const transport = createRemoteTransport({ + defaultPort: opts.defaultPort, + serverOptions: opts.serverOptions, + coordinator: () => coordinator, + needsWideBind, + wideBindInUse: () => + wideBindAgents.size > 0 || + (process.platform !== 'linux' && delegation.requiresWideTransport()), + rememberedDevicesPath: opts.rememberedDevicesPath, + }); + + const delegation: DelegationService = new DelegationService({ + orchestrationEnabled: restoreOrchestrationEnabled(), + coordinator: async () => { + await ensureCoordinator(); + if (!coordinator) throw new Error('Delegation unavailable.'); + return coordinator; + }, + currentCoordinator: () => coordinator, + prepareParent: prepareDelegationParent, + sessions: () => transport.current()?.getSessionAgents() ?? [], + changed: (event) => notify(IPC.DelegationChanged, event), + persist: () => { + const json = loadAppState(); + if (json) saveAppState(delegation.normalizeState(json)); + }, + parentCreated: (taskId) => notify(IPC.MCP_TaskStateSync, { taskId, delegationParent: true }), + }); + + async function prepareDelegationParent(task: TaskAuthorityInput): Promise { + await ensureCoordinator(); + if (!coordinator) throw new Error('Delegation unavailable.'); + const server = await transport.ensureForMcp(task.dockerMode === true); + const hostServerPath = hostMcpServerPath(); + const serverPath = task.dockerMode + ? getDockerMcpServerDestPath(task.worktreePath, task.projectRoot) + : hostServerPath; + if (task.dockerMode) { + fs.mkdirSync(path.dirname(serverPath), { recursive: true }); + fs.copyFileSync(hostServerPath, serverPath); + appendGitInfoExcludeBlock( + task.worktreePath, + '.parallel-code/', + '# Parallel Code MCP runtime\n.parallel-code/\n', + (error) => + logWarn('mcp', 'Could not exclude MCP runtime directory', { err: errMessage(error) }), + ); + } + coordinator.registerCoordinator(task.taskId, task.projectId, { + projectRoot: task.projectRoot, + worktreePath: task.worktreePath, + branchName: task.branchName, + spawnDefaults: { command: task.agentCommand, args: task.agentArgs }, + agentEnvFile: task.agentEnvFile, + automaticNotifications: task.autoSendChildUpdates ?? task.coordinatorMode ?? false, + paused: task.delegationPaused, + skipPermissions: task.propagateSkipPermissions, + maxConcurrentTasks: task.maxConcurrentTasks, + verifyCommand: task.verifyCommand, + }); + coordinator.setDockerContainerName(task.taskId, task.dockerMode ? 'delegation' : null); + coordinator.setDockerImage(task.taskId, task.dockerImage ?? null); + coordinator.setMCPServerInfo( + task.taskId, + getMCPRemoteServerUrl(server.port, task.dockerMode ? 'delegation' : undefined), + server.coordinatorTokenFor(task.taskId), + server.subtaskToken, + serverPath, + ); + } + + // The canvas token is revoked on exit; the config file holding it must go too. + onPtyEvent('exit', (agentId) => { + try { + removeCanvasConfig(agentId); + } catch (error) { + console.warn('Could not remove canvas MCP credentials:', error); + } + wideBindAgents.delete(agentId); + canvasOwners.delete(agentId); + // The server's own exit listener runs after this one; drop the agent here so the + // idle check below already sees it gone. + transport.current()?.unregisterCanvasAgent(agentId); + delegation.expireMessages(); + transport + .stopIfIdle() + .catch((error) => console.warn('[MCP] Could not stop the idle transport:', error)); + }); + + let coordinatorStarting: Promise | undefined; + async function ensureCoordinator(): Promise { + if (coordinator) return; + if (coordinatorStarting) return coordinatorStarting; + coordinatorStarting = (async () => { + const { Coordinator } = await import('../mcp/coordinator.js'); + const loaded = new Coordinator(); + coordinator = loaded; + loaded.setOrchestrationEnabled(delegation.isOrchestrationEnabled()); + loaded.setNotify(notify); + loaded.setSessionMcpProvider((task) => { + const server = transport.current(); + if (!delegation.getTask(task.coordinatorTaskId) || !server) return undefined; + if (!delegation.getTask(task.id)) { + if (task.status !== 'creating') + throw new Error('Task authority was not validated before restoring its MCP session.'); + delegation.registerChild(task); + } + const sessionCapabilities = delegation.capabilities(task.id); + if (!sessionCapabilities) throw new Error('Task authority is unavailable.'); + let owner = canvasOwners.get(task.agentId); + if (!owner) { + owner = { sessionInstanceId: crypto.randomUUID() }; + canvasOwners.set(task.agentId, owner); + } + if (needsWideBind(!!task.dockerContainerName)) wideBindAgents.add(task.agentId); + const token = server.registerCanvasAgent(task.id, task.agentId, undefined, { + sessionInstanceId: owner.sessionInstanceId, + capabilities: sessionCapabilities, + }); + return { token, sessionCapabilities }; + }); + opts.onCoordinatorLoaded(); + })(); + try { + await coordinatorStarting; + } finally { + coordinatorStarting = undefined; + } + } + + return { + transport, + delegation, + pendingSpawns, + canvasOwners, + wideBindAgents, + needsWideBind, + coordinator: () => coordinator, + ensureCoordinator, + }; +} diff --git a/electron/ipc/register.ts b/electron/ipc/register.ts index b48b043a5..3882219cf 100644 --- a/electron/ipc/register.ts +++ b/electron/ipc/register.ts @@ -4,8 +4,6 @@ import fs from 'fs'; import os from 'os'; import { fileURLToPath } from 'url'; import { IPC } from './channels.js'; -import { DelegationService } from '../mcp/delegation.js'; -import type { TaskAuthorityInput } from '../shared/delegation-types.js'; import { startAgentChat, getAgentChat, @@ -42,7 +40,6 @@ import { buildDockerImage, resolveProjectDockerfile, projectImageTag, - onPtyEvent, } from './pty.js'; import { ensurePlansDirectory, @@ -118,7 +115,7 @@ import { import { createTask, deleteTask } from './tasks.js'; import { settleWorktreeIntents } from './worktree-intents.js'; import { windowNotifier } from './window-notifier.js'; -import { createRemoteTransport } from './remote-transport.js'; +import { createMcpRuntime } from './mcp-runtime.js'; import { getDockerMcpServerDestPath, hostMcpServerPath } from './mcp-paths.js'; import { listAgents } from './agents.js'; import { @@ -565,16 +562,14 @@ export function registerAllHandlers(win: BrowserWindow): void { // --- Remote access state --- // Keep development phone access and coordinator ports separate from the installed app. const defaultRemotePort = app.isPackaged ? 7777 : 8777; - const transport = createRemoteTransport({ + const mcp = createMcpRuntime({ + notify, defaultPort: defaultRemotePort, serverOptions: () => remoteServerOptions(), - coordinator: () => coordinator, - needsWideBind: (dockerMode) => needsWideBind(dockerMode), - wideBindInUse: () => - wideBindAgents.size > 0 || - (process.platform !== 'linux' && delegation.requiresWideTransport()), rememberedDevicesPath: () => path.join(getUserDataDir(), 'paired-phones.json'), + onCoordinatorLoaded: () => registerCoordinatorHandlers(), }); + const { transport, delegation, pendingSpawns, canvasOwners, wideBindAgents, needsWideBind } = mcp; const taskNames = new Map(); // Renderer-derived per-task attention (needs input, working, ready, …), pushed // from the renderer via Remote_UpdateTaskStatus so the mobile overview can show @@ -587,112 +582,10 @@ export function registerAllHandlers(win: BrowserWindow): void { >(); // --- MCP coordinator (lazy — only loaded when coordinator mode is enabled) --- - type CoordinatorType = import('../mcp/coordinator.js').Coordinator; - let coordinator: CoordinatorType | null = null; let coordinatorHandlersRegistered = false; let lastMcpConfigPath: string | null = null; - // A kill must also cancel startup while optional MCP transport is awaiting I/O. - const pendingSpawns = new Map(); - /** Which spawn minted the agent's live canvas token. `pendingSpawns` cannot answer this: a - * kill clears the entry to cancel the spawn, and a finished restart clears its own, so an - * absent entry means both "nobody owns this" and "the owner already left". */ - const canvasOwners = new Map(); - let orchestrationEnabled = true; - try { - const saved = loadAppState(); - if (saved) orchestrationEnabled = JSON.parse(saved).mcpOrchestrationEnabled !== false; - } catch (error) { - orchestrationEnabled = false; - logWarn('mcp', 'Could not restore orchestration setting; keeping orchestration disabled', { - err: errMessage(error), - }); - } - const delegation = new DelegationService({ - orchestrationEnabled, - coordinator: async () => { - await ensureCoordinator(); - if (!coordinator) throw new Error('Delegation unavailable.'); - return coordinator; - }, - currentCoordinator: () => coordinator, - prepareParent: prepareDelegationParent, - sessions: () => transport.current()?.getSessionAgents() ?? [], - changed: (event) => { - if (!win.isDestroyed()) win.webContents.send(IPC.DelegationChanged, event); - }, - persist: () => { - const json = loadAppState(); - if (json) saveAppState(delegation.normalizeState(json)); - }, - parentCreated: (taskId) => { - if (!win.isDestroyed()) - win.webContents.send(IPC.MCP_TaskStateSync, { taskId, delegationParent: true }); - }, - }); ipcMain.handle(IPC.DelegationRequest, (_event, request) => delegation.request(request)); - async function prepareDelegationParent(task: TaskAuthorityInput): Promise { - await ensureCoordinator(); - if (!coordinator) throw new Error('Delegation unavailable.'); - const server = await transport.ensureForMcp(task.dockerMode === true); - const hostServerPath = hostMcpServerPath(); - const serverPath = task.dockerMode - ? getDockerMcpServerDestPath(task.worktreePath, task.projectRoot) - : hostServerPath; - if (task.dockerMode) { - fs.mkdirSync(path.dirname(serverPath), { recursive: true }); - fs.copyFileSync(hostServerPath, serverPath); - appendGitInfoExcludeBlock( - task.worktreePath, - '.parallel-code/', - '# Parallel Code MCP runtime\n.parallel-code/\n', - (error) => - logWarn('mcp', 'Could not exclude MCP runtime directory', { err: errMessage(error) }), - ); - } - coordinator.registerCoordinator(task.taskId, task.projectId, { - projectRoot: task.projectRoot, - worktreePath: task.worktreePath, - branchName: task.branchName, - spawnDefaults: { command: task.agentCommand, args: task.agentArgs }, - agentEnvFile: task.agentEnvFile, - automaticNotifications: task.autoSendChildUpdates ?? task.coordinatorMode ?? false, - paused: task.delegationPaused, - skipPermissions: task.propagateSkipPermissions, - maxConcurrentTasks: task.maxConcurrentTasks, - verifyCommand: task.verifyCommand, - }); - coordinator.setDockerContainerName(task.taskId, task.dockerMode ? 'delegation' : null); - coordinator.setDockerImage(task.taskId, task.dockerImage ?? null); - coordinator.setMCPServerInfo( - task.taskId, - getMCPRemoteServerUrl(server.port, task.dockerMode ? 'delegation' : undefined), - server.coordinatorTokenFor(task.taskId), - server.subtaskToken, - serverPath, - ); - } - /** Container agents on macOS reach the host only through a wide bind; track who needs it. */ - const wideBindAgents = new Set(); - const needsWideBind = (dockerMode: boolean) => dockerMode && process.platform !== 'linux'; - // The canvas token is revoked on exit; the config file holding it must go too. - onPtyEvent('exit', (agentId) => { - try { - removeCanvasConfig(agentId); - } catch (error) { - console.warn('Could not remove canvas MCP credentials:', error); - } - wideBindAgents.delete(agentId); - canvasOwners.delete(agentId); - // The server's own exit listener runs after this one; drop the agent here so the - // idle check below already sees it gone. - transport.current()?.unregisterCanvasAgent(agentId); - delegation.expireMessages(); - transport - .stopIfIdle() - .catch((error) => console.warn('[MCP] Could not stop the idle transport:', error)); - }); - // --- PTY commands --- ipcMain.handle(IPC.SpawnAgent, async (_e, args) => { assertString(args.command, 'command'); @@ -713,7 +606,7 @@ export function registerAllHandlers(win: BrowserWindow): void { if (args.cwd) validatePath(args.cwd, 'cwd'); const attaching = args.attachExisting && getAgentMeta(args.agentId); const releaseAdmission = - attaching || args.isShell ? undefined : coordinator?.reserveChildRestart(args.taskId); + attaching || args.isShell ? undefined : mcp.coordinator()?.reserveChildRestart(args.taskId); const pending = { sessionInstanceId: crypto.randomUUID() }; pendingSpawns.set(args.agentId, pending); const assertPendingSpawn = () => { @@ -738,6 +631,7 @@ export function registerAllHandlers(win: BrowserWindow): void { delegation.expireMessages(); } let canvasTools = existing?.canvasTools === true; + const coordinator = mcp.coordinator(); let releaseCanvas: (() => void) | undefined; if ( !existing && @@ -924,7 +818,7 @@ export function registerAllHandlers(win: BrowserWindow): void { } if (args.taskId !== undefined) { assertString(args.taskId, 'taskId'); - if (coordinator?.isAutomationWriteInFlight(args.taskId)) return false; + if (mcp.coordinator()?.isAutomationWriteInFlight(args.taskId)) return false; } return writeToAgent(args.agentId, args.data); }); @@ -1189,7 +1083,7 @@ export function registerAllHandlers(win: BrowserWindow): void { } const delay = state.coordinatorNotificationDelayMs; if (typeof delay === 'number' && Number.isFinite(delay)) { - coordinator?.setNotificationDelayMs(delay); + mcp.coordinator()?.setNotificationDelayMs(delay); } } catch (e) { console.warn('Ignoring malformed saved state:', e); @@ -1787,7 +1681,7 @@ export function registerAllHandlers(win: BrowserWindow): void { exitCode: null, lastLine: '', }), - getCoordinator: () => coordinator, + getCoordinator: () => mcp.coordinator(), callSessionTool: (caller, name, params) => delegation.callTool(caller, name, params), isOrchestrationEnabled: () => delegation.isOrchestrationEnabled(), chats: { list: listTaskChats, find: findAgentChat, onChange: onAgentChatsChanged }, @@ -1889,6 +1783,7 @@ export function registerAllHandlers(win: BrowserWindow): void { if (args.controlledBy !== 'coordinator' && args.controlledBy !== 'human') { throw new Error(`Invalid controlledBy: ${String(args.controlledBy)}`); } + const coordinator = mcp.coordinator(); if (!coordinator) throw new Error('Task coordination is not initialized'); coordinator.setTaskControl(args.taskId, args.controlledBy); @@ -1913,7 +1808,7 @@ export function registerAllHandlers(win: BrowserWindow): void { validateBranchName(args.coordinatorBranch, 'coordinatorBranch'); } validateOptionalVerifyCommand(args.verifyCommand); - coordinator?.registerCoordinator(args.coordinatorTaskId, args.projectId, { + mcp.coordinator()?.registerCoordinator(args.coordinatorTaskId, args.projectId, { branchName: args.coordinatorBranch, worktreePath: args.worktreePath, verifyCommand: args.verifyCommand, @@ -1925,7 +1820,7 @@ export function registerAllHandlers(win: BrowserWindow): void { IPC.MCP_CoordinatorDeregistered, async (_e, args: { coordinatorTaskId: string }) => { assertString(args.coordinatorTaskId, 'coordinatorTaskId'); - coordinator?.deregisterCoordinator(args.coordinatorTaskId); + mcp.coordinator()?.deregisterCoordinator(args.coordinatorTaskId); // Clean up the host-temp MCP config file written by StartMCPServer (non-Docker only). const tempConfigPath = path.join( app.getPath('temp'), @@ -1950,13 +1845,13 @@ export function registerAllHandlers(win: BrowserWindow): void { (_e, args: { coordinatorTaskId: string; batchId: string }) => { assertString(args.coordinatorTaskId, 'coordinatorTaskId'); assertString(args.batchId, 'batchId'); - coordinator?.dropNotification(args.coordinatorTaskId, args.batchId); + mcp.coordinator()?.dropNotification(args.coordinatorTaskId, args.batchId); }, ); ipcMain.handle(IPC.MCP_CoordinatedTaskPromptDelivered, (_e, args: { taskId: string }) => { assertString(args.taskId, 'taskId'); - coordinator?.markPromptDelivered(args.taskId); + mcp.coordinator()?.markPromptDelivered(args.taskId); }); ipcMain.handle( @@ -1964,7 +1859,7 @@ export function registerAllHandlers(win: BrowserWindow): void { (_e, args: { coordinatorTaskId: string; batchId: string }) => { assertString(args.coordinatorTaskId, 'coordinatorTaskId'); assertString(args.batchId, 'batchId'); - coordinator?.ackNotification(args.coordinatorTaskId, args.batchId); + mcp.coordinator()?.ackNotification(args.coordinatorTaskId, args.batchId); }, ); @@ -1972,13 +1867,13 @@ export function registerAllHandlers(win: BrowserWindow): void { IPC.MCP_CoordinatorRestageAfterUserSend, (_e, args: { coordinatorTaskId: string }) => { assertString(args.coordinatorTaskId, 'coordinatorTaskId'); - coordinator?.rescheduleRestageTimer(args.coordinatorTaskId); + mcp.coordinator()?.rescheduleRestageTimer(args.coordinatorTaskId); }, ); ipcMain.handle(IPC.MCP_TaskLandingReviewCleared, (_e, args: { taskId: string }) => { assertString(args.taskId, 'taskId'); - coordinator?.markTaskReviewed(args.taskId); + mcp.coordinator()?.markTaskReviewed(args.taskId); }); ipcMain.handle( @@ -1986,7 +1881,7 @@ export function registerAllHandlers(win: BrowserWindow): void { (_e, args: { taskId: string; coordinatorTaskId: string }) => { assertString(args.taskId, 'taskId'); assertString(args.coordinatorTaskId, 'coordinatorTaskId'); - coordinator?.removeCoordinatedTask(args.taskId); + mcp.coordinator()?.removeCoordinatedTask(args.taskId); }, ); @@ -2030,6 +1925,7 @@ export function registerAllHandlers(win: BrowserWindow): void { validatePath(args.worktreePath, 'worktreePath'); assertString(args.coordinatorTaskId, 'coordinatorTaskId'); validateUUID(args.coordinatorTaskId, 'coordinatorTaskId'); + const coordinator = mcp.coordinator(); if (!coordinator) throw new Error('Task coordination is not initialized'); const authority = delegation.getTask(args.id); if ( @@ -2078,48 +1974,6 @@ export function registerAllHandlers(win: BrowserWindow): void { ); } - // Lazily initialize the shared coordination backend and register its handlers. - // Safe to call multiple times — only initializes once. - let coordinatorStarting: Promise | undefined; - async function ensureCoordinator(): Promise { - if (coordinator) return; - if (coordinatorStarting) return coordinatorStarting; - coordinatorStarting = (async () => { - const { Coordinator } = await import('../mcp/coordinator.js'); - coordinator = new Coordinator(); - coordinator.setOrchestrationEnabled(delegation.isOrchestrationEnabled()); - coordinator.setNotify(notify); - coordinator.setSessionMcpProvider((task) => { - const server = transport.current(); - if (!delegation.getTask(task.coordinatorTaskId) || !server) return undefined; - if (!delegation.getTask(task.id)) { - if (task.status !== 'creating') - throw new Error('Task authority was not validated before restoring its MCP session.'); - delegation.registerChild(task); - } - const sessionCapabilities = delegation.capabilities(task.id); - if (!sessionCapabilities) throw new Error('Task authority is unavailable.'); - let owner = canvasOwners.get(task.agentId); - if (!owner) { - owner = { sessionInstanceId: crypto.randomUUID() }; - canvasOwners.set(task.agentId, owner); - } - if (needsWideBind(!!task.dockerContainerName)) wideBindAgents.add(task.agentId); - const token = server.registerCanvasAgent(task.id, task.agentId, undefined, { - sessionInstanceId: owner.sessionInstanceId, - capabilities: sessionCapabilities, - }); - return { token, sessionCapabilities }; - }); - registerCoordinatorHandlers(); - })(); - try { - await coordinatorStarting; - } finally { - coordinatorStarting = undefined; - } - } - // StartMCPServer registered eagerly (not inside registerCoordinatorHandlers) so the // renderer can call it during app-restore before ensureCoordinator() resolves. ipcMain.handle( @@ -2171,7 +2025,8 @@ export function registerAllHandlers(win: BrowserWindow): void { } } - await ensureCoordinator(); + await mcp.ensureCoordinator(); + const coordinator = mcp.coordinator(); if (!coordinator) return; // Set coordinator's default project + coordinator task ID, and register this coordinator diff --git a/electron/ipc/remote-transport.ts b/electron/ipc/remote-transport.ts index 3d1a80a89..7a22c95a2 100644 --- a/electron/ipc/remote-transport.ts +++ b/electron/ipc/remote-transport.ts @@ -2,7 +2,7 @@ import { startRemoteServer } from '../remote/server.js'; import type { Coordinator } from '../mcp/coordinator.js'; export type RemoteServerHandle = Awaited>; -type RemoteServerOptions = Omit[0], 'port' | 'host'>; +export type RemoteServerOptions = Omit[0], 'port' | 'host'>; export interface RemoteTransportOptions { defaultPort: number; From 9a21766301ad5dd2437e279df8db18119f83ef91 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Fri, 25 Sep 2026 19:50:40 +0200 Subject: [PATCH 09/10] docs(architecture): record step 3b progress --- docs/runtime-separation-plan.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/docs/runtime-separation-plan.md b/docs/runtime-separation-plan.md index 5767d4c84..8b67361f9 100644 --- a/docs/runtime-separation-plan.md +++ b/docs/runtime-separation-plan.md @@ -1,6 +1,6 @@ # Runtime separation plan -Status: revised after three reviews (two focused, one adversarial) and re-checked against the code on 2026-09-25. Steps 0, 1, 2 and 3a are done. Steps 1 and 2 fixed the only known correctness problems; everything from 3b onward is structural and waits for a named product goal (see the checkpoint after step 3). References name functions rather than line numbers, because line numbers drift. +Status: revised after three reviews (two focused, one adversarial) and re-checked against the code on 2026-09-25. Steps 0, 1, 2 and 3a are done, and 3b is partly done (the runtime state has a home; the handler groups have not moved). Steps 1 and 2 fixed the only known correctness problems; everything from 3b onward is structural and waits for a named product goal (see the checkpoint after step 3). References name functions rather than line numbers, because line numbers drift. ## Why @@ -60,6 +60,13 @@ Each of these changes runtime behavior and needs a smoke test in the real app. **3b. Split `register.ts`.** #279 (pooled workspaces) was closed without merging on 2026-09-25, so it no longer blocks this. 5 of the 12 open PRs touch `register.ts` (#262, #248, #247, #162, #23). Do the split between merges, after announcing a short freeze; it does not block anything before step 4. Before the split: - Commit a wiring test against the old code. It records every `ipcMain.handle` channel, throws on duplicates, asserts the `win.on` events and exactly one registration-time `onPtyEvent('exit')`, and triggers `ensureCoordinator` twice so the lazily registered `MCP_*` handlers are seen exactly once. + Progress: + - Done: the wiring test (`electron/ipc/register-wiring.test.ts`). + - Done: `electron/ipc/remote-transport.ts` owns the remote server and the rules for who started it and when it stops. + - Done: `electron/ipc/mcp-paths.ts` holds `hostMcpServerPath()`, replacing five copies. + - Done: `electron/ipc/mcp-runtime.ts` owns the transport, the lazy coordinator, delegation, `prepareDelegationParent`, the PTY exit cleanup, and `pendingSpawns`, `canvasOwners` and `wideBindAgents`. `register.ts` reads the coordinator through `mcp.coordinator()` and the server through `transport.current()`. This is the home step 4 needed. The wiring test passed unchanged after each move. + - Not done: moving the handler groups out of `register.ts`, which is mechanical, one commit per group. The doc and comment updates and the `electron/runtime/` rule below belong to that stage. + Design rules from review: - Mutations of remote-server and coordinator state stay inside `mcp-runtime` methods (`startRemoteAccess`, `stopRemoteAccess`). `remoteServer()` and `coordinator()` are methods, never destructurable fields, so callers cannot capture a stale `null`. - MCP path helpers move into `mcp-runtime.ts` or `mcp-paths.ts`, including one `hostMcpServerPath()` in place of five copies, so no `register-*` module is imported by the runtime. From 38ffce014fc345f0ef8422abcf6be91ab331953d Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Fri, 25 Sep 2026 20:06:43 +0200 Subject: [PATCH 10/10] docs(architecture): point step 4 at the new mcp-runtime home --- docs/runtime-separation-plan.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/runtime-separation-plan.md b/docs/runtime-separation-plan.md index 8b67361f9..5c7820146 100644 --- a/docs/runtime-separation-plan.md +++ b/docs/runtime-separation-plan.md @@ -9,7 +9,7 @@ The desktop renderer is the source of truth for tasks, and the main process co-o - Agent processes are spawned when `TerminalView` mounts (`startSpawn` in `src/components/TerminalView.tsx`). Agent terminals pass `preserveSessionOnCleanup` (`src/components/TaskAITerminal.tsx`), so unmounting a view does not kill its agent. After a renderer reload, main reattaches the running PTY and replays its scrollback (`attachExisting` in `spawnAgent`, `electron/ipc/pty.ts`). Agents are killed on purpose when a task is collapsed or closed (`collapseTask` and `closeTask` in `src/store/tasks.ts`). - The renderer persists task state as one JSON blob (`saveState` in `src/store/persistence.ts`). Main also writes that file: it normalizes every renderer save and does its own read-modify-write for delegation state (the `IPC.SaveAppState` handler and the delegation `persist` callback in `electron/ipc/register.ts`; `normalizeState` in `electron/mcp/delegation.ts`). - The phone and MCP agents reach task, reasoning, mind-map and notes state through the renderer (`callRenderer` in `electron/ipc/register.ts`), with a 120-second timeout. It backs 10 operations: reasoning and mind-map reads and updates, opening the canvas, publishing tours, and the phone's projects, task creation and notes. -- Tasks are created three ways. Desktop and phone tasks go through the renderer store (`createTask` in `src/store/tasks.ts`). Coordinator tasks are created in main (`createTaskUnchecked` in `electron/mcp/coordinator.ts`). Every path provisions the worktree before the task is durably saved: autosave is debounced by 1–5 s (`AUTOSAVE_DEBOUNCE_MS` in `src/store/autosave.ts`), and coordinator children are saved only after the renderer adopts them. Before step 2, nothing reconciled worktrees at startup, so a crash in that window left an orphaned worktree that nothing reported. +- Tasks are created three ways. Desktop and phone tasks go through the renderer store (`createTask` in `src/store/tasks.ts`). Coordinator tasks are created in main (`createTaskUnchecked` in `electron/mcp/coordinator.ts`). Every path provisions the worktree before the task is durably saved. Desktop and phone task creation calls `saveState()` immediately after registering task authority and adding the task to the store, without awaiting the save; coordinator children are saved only after the renderer adopts them. A crash between provisioning and save completion can therefore leave an orphaned worktree. Before step 2, nothing reconciled worktrees at startup, so these orphans went unreported. - Document workspaces create worktrees in the same `.worktrees/` directory as tasks (`prepareAlternateWorktree` in `electron/documents/runs.ts`). Their records live in document-run state, not `state.json`. - Question detection and trust-dialog acceptance run in the renderer (`looksLikeQuestion` and `tryAutoTrust` in `src/store/taskStatus.ts`). The coordinator has a separate readiness monitor (`createAgentOutputMonitor` and `scheduleInitialPromptDelivery` in `electron/mcp/coordinator.ts`, a 3,300-line file). - Until step 1, only xterm in the renderer answered terminal queries. Codex exits if its cursor-position query goes unanswered for about 2 s (commit `1204de82`, patched by disabling `backgroundThrottling` in `electron/main.ts`). Coordinator sub-tasks are spawned in main before any view exists, so they relied on a view mounting in time. This is the only user-facing bug we found in the history that the ownership split caused; the search was by commit message, so others may exist. Step 1 fixed it. @@ -68,7 +68,7 @@ Each of these changes runtime behavior and needs a smoke test in the real app. - Not done: moving the handler groups out of `register.ts`, which is mechanical, one commit per group. The doc and comment updates and the `electron/runtime/` rule below belong to that stage. Design rules from review: - - Mutations of remote-server and coordinator state stay inside `mcp-runtime` methods (`startRemoteAccess`, `stopRemoteAccess`). `remoteServer()` and `coordinator()` are methods, never destructurable fields, so callers cannot capture a stale `null`. + - Mutations of remote-server and coordinator state stay inside `remote-transport` and `mcp-runtime` methods (`startRemoteAccess`, `stopRemoteAccess`, `ensureCoordinator`). `transport.current()` and `mcp.coordinator()` are methods, never destructurable fields, so callers cannot capture a stale `null`. - MCP path helpers move into `mcp-runtime.ts` or `mcp-paths.ts`, including one `hostMcpServerPath()` in place of five copies, so no `register-*` module is imported by the runtime. - Late-bound callbacks are created per `registerAllHandlers` call, and `coordinatorHandlersRegistered` lives in that closure. - Hoisted `function` declarations stay hoisted. @@ -84,7 +84,7 @@ Each of these changes runtime behavior and needs a smoke test in the real app. **Checkpoint (reached after 3a).** Reassess 3b and every later step against a named product goal, such as the phone creating tasks while the renderer is not running. None of them fixes a known bug. -4. **Main-side agent launch.** Structural: it removes duplication between the IPC and coordinator launch paths and is a prerequisite for step 6. Do it together with step 6, and only after 3b, because the `SpawnAgent` handler depends on state held in the `registerAllHandlers` closure (`pendingSpawns`, `canvasOwners`, `wideBindAgents`, `remoteServer`, `delegation`, `ensureMcpTransport`), which needs a home outside it first. +4. **Main-side agent launch.** Structural: it removes duplication between the IPC and coordinator launch paths and is a prerequisite for step 6. Do it together with step 6. The state the `SpawnAgent` handler depends on (`pendingSpawns`, `canvasOwners`, `wideBindAgents`, the transport, `delegation`, the coordinator) now lives in `createMcpRuntime` (`electron/ipc/mcp-runtime.ts`), so `launchAgent()` can take the runtime as input instead of reading the `registerAllHandlers` closure. Moving the other handler groups out of `register.ts` is not a prerequisite. - **4a. `launchAgent()`.** Extract the `SpawnAgent` handler body, used by both IPC and the coordinator. The coordinator path keeps what is specific to it: preamble injection, the per-task MCP config file for Docker, the legacy subtask token and container arguments. Move `buildTaskAgentArgs` into shared code; it takes a renderer `Task` and imports from `src/documents/`, so it first needs a narrower input type. - **4b. `PromptDeliverer`.** Merge the renderer's question and trust detection (`src/store/taskStatus.ts`) with the coordinator's output monitor into one main-side deliverer instead of adding a third. It could read the step 1 terminal mirror's screen instead of the raw output tail; this is untested. Plan it separately: it is larger and needs real-app smoke tests.