Skip to content

Commit 5099957

Browse files
brookscjohannesjo
authored andcommitted
fix(coordinator): address PR124 polish nits
1 parent 35cd01b commit 5099957

8 files changed

Lines changed: 39 additions & 32 deletions

File tree

electron/ipc/register.ts

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1623,14 +1623,12 @@ export function registerAllHandlers(win: BrowserWindow): void {
16231623
// The MCP server process is spawned by the agent CLI via launch args,
16241624
// not by us. We report whether the remote HTTP server that the MCP
16251625
// server connects to is running — if it's up, MCP tools should work.
1626-
const remoteRunning = remoteServer !== null;
16271626
return {
1628-
mcpRunning: remoteRunning,
1629-
remoteRunning,
1630-
coordinatorRoutesAttached: coordinator !== null,
1631-
coordinatorRegistered: coordinator?.hasActiveCoordinator() ?? false,
1632-
serverUrl: remoteServer ? getMCPRemoteServerUrl(remoteServer.port) : null,
1633-
mcpConfigPath: lastMcpConfigPath,
1627+
running: remoteServer !== null,
1628+
port: remoteServer?.port ?? null,
1629+
// TODO: Surface this from the coordinator map if the UI needs it.
1630+
coordinatorTaskId: null,
1631+
mcpConfigPath: lastMcpConfigPath ?? null,
16341632
};
16351633
});
16361634

src/components/SidebarFooter.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ export function SidebarFooter() {
2828

2929
onCleanup(() => stopMCPStatusPolling());
3030

31-
const mcpOk = () => store.mcpStatus.mcpRunning;
31+
const mcpOk = () => store.mcpStatus.running;
3232

3333
return (
3434
<>

src/store/autosave.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ function persistedSnapshot(): string {
6262
coordinatedBy: t.coordinatedBy,
6363
coordinatorMode: t.coordinatorMode,
6464
mcpConfigPath: t.mcpConfigPath,
65+
preambleFileExistedBefore: t.preambleFileExistedBefore,
6566
signalDoneReceived: t.signalDoneReceived,
6667
signalDoneAt: t.signalDoneAt,
6768
signalDoneConsumed: t.signalDoneConsumed,

src/store/core.ts

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -80,14 +80,7 @@ export const [store, setStore] = createStore<AppStore>({
8080
coordinatorModeEnabled: false,
8181
coordinatorNotificationDelayMs: 60_000,
8282
coordinatorControlHintDismissed: false,
83-
mcpStatus: {
84-
mcpRunning: false,
85-
remoteRunning: false,
86-
coordinatorRoutesAttached: false,
87-
coordinatorRegistered: false,
88-
serverUrl: null,
89-
mcpConfigPath: null,
90-
},
83+
mcpStatus: { running: false, port: null, coordinatorTaskId: null, mcpConfigPath: null },
9184
});
9285

9386
type CleanupPanelStore = Pick<

src/store/mcpStatus.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { store, setStore } from './core';
22
import { invoke } from '../lib/ipc';
33
import { IPC } from '../../electron/ipc/channels';
4+
import type { MCPStatus } from './types';
45

56
let pollTimer: ReturnType<typeof setInterval> | null = null;
67
const POLL_INTERVAL_MS = 3_000;
@@ -12,18 +13,16 @@ export function hasAnyCoordinatorTask(): boolean {
1213
return false;
1314
}
1415

15-
const MCP_STATUS_OFFLINE = {
16-
mcpRunning: false,
17-
remoteRunning: false,
18-
coordinatorRoutesAttached: false,
19-
coordinatorRegistered: false,
20-
serverUrl: null,
16+
const MCP_STATUS_OFFLINE: MCPStatus = {
17+
running: false,
18+
port: null,
19+
coordinatorTaskId: null,
2120
mcpConfigPath: null,
22-
} as const;
21+
};
2322

2423
export async function refreshMCPStatus(): Promise<void> {
2524
try {
26-
const result = await invoke<import('./types').MCPStatus>(IPC.GetMCPStatus);
25+
const result = await invoke<MCPStatus>(IPC.GetMCPStatus);
2726
setStore('mcpStatus', result);
2827
} catch {
2928
setStore('mcpStatus', MCP_STATUS_OFFLINE);

src/store/tasks.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ import {
137137
retryTaskMcpStartup,
138138
} from './tasks';
139139
import { getCoordinatorChildren } from './sidebar-order';
140+
import { markAgentSpawned, rescheduleTaskStatusPolling } from './taskStatus';
140141

141142
// ─── Coordinator listener setup ───────────────────────────────────────────────
142143

@@ -147,6 +148,8 @@ const taskStateSyncHandler = ipcHandlers.get('mcp_task_state_sync');
147148
if (!taskStateSyncHandler) throw new Error('mcp_task_state_sync handler not registered');
148149

149150
beforeEach(() => {
151+
vi.clearAllMocks();
152+
mockSetStore.mockImplementation((...args: unknown[]) => applySetStore(...args));
150153
mockTasks = {};
151154
mockAgents = {};
152155
mockTaskOrder = [];
@@ -221,6 +224,15 @@ describe('coordinator controlledBy state machine (item 9: UI disabled-state regr
221224
});
222225

223226
describe('MCP_TaskCreated IPC handler', () => {
227+
it('ignores duplicate task-created events without resetting spawn state', () => {
228+
taskCreatedHandler(baseEvent);
229+
taskCreatedHandler(baseEvent);
230+
231+
expect(mockTaskOrder).toEqual(['sub-task-1']);
232+
expect(markAgentSpawned).toHaveBeenCalledTimes(1);
233+
expect(rescheduleTaskStatusPolling).toHaveBeenCalledTimes(1);
234+
});
235+
224236
it('sets controlledBy to coordinator on the new sub-task', () => {
225237
taskCreatedHandler(baseEvent);
226238
expect(mockTasks['sub-task-1'].controlledBy).toBe('coordinator');

src/store/tasks.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1029,7 +1029,8 @@ export function initMCPListeners(): () => void {
10291029
mcpConfigPath: evt.mcpConfigPath,
10301030
preambleFileExistedBefore: evt.preambleFileExistedBefore,
10311031
skipPermissions: evt.skipPermissions ?? false,
1032-
// MCP server is already running when the task is created mid-session.
1032+
// Backend-spawned children are already attached to a live MCP server;
1033+
// restore-created MCP tasks start pending until hydration marks them ready.
10331034
mcpStartupStatus: 'ready' as const,
10341035
};
10351036

@@ -1055,18 +1056,23 @@ export function initMCPListeners(): () => void {
10551056
signal: null,
10561057
lastOutput: [],
10571058
generation: 0,
1059+
attachExisting: true,
10581060
};
10591061

1062+
let created = false;
10601063
setStore(
10611064
produce((s) => {
10621065
if (s.tasks[evt.taskId]) return; // idempotent — ignore duplicate events
10631066
s.tasks[evt.taskId] = task;
10641067
s.agents[evt.agentId] = agent;
10651068
s.taskOrder.push(evt.taskId);
1069+
created = true;
10661070
}),
10671071
);
1068-
markAgentSpawned(evt.agentId);
1069-
rescheduleTaskStatusPolling();
1072+
if (created) {
1073+
markAgentSpawned(evt.agentId);
1074+
rescheduleTaskStatusPolling();
1075+
}
10701076
}),
10711077
);
10721078

src/store/types.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -226,11 +226,9 @@ export interface PersistedState {
226226
}
227227

228228
export interface MCPStatus {
229-
mcpRunning: boolean;
230-
remoteRunning: boolean;
231-
coordinatorRoutesAttached: boolean;
232-
coordinatorRegistered: boolean;
233-
serverUrl: string | null;
229+
running: boolean;
230+
port: number | null;
231+
coordinatorTaskId: string | null;
234232
mcpConfigPath: string | null;
235233
}
236234

0 commit comments

Comments
 (0)