Skip to content

Commit 1e1b50a

Browse files
committed
fix(johannesjo#43): surface MCP restore failures with error state and retry UI
1 parent 26ce4be commit 1e1b50a

6 files changed

Lines changed: 300 additions & 25 deletions

File tree

src/App.tsx

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,9 @@ import {
4949
setDockerAvailable,
5050
toggleTaskFocusMode,
5151
initMCPListeners,
52+
markTaskMcpPending,
5253
markTaskMcpReady,
54+
markTaskMcpError,
5355
} from './store/store';
5456
import { isGitHubUrl } from './lib/github-url';
5557
import type { PersistedWindowState } from './store/types';
@@ -346,6 +348,7 @@ function App() {
346348
task.dockerMode && task.agentIds[0]
347349
? `parallel-code-${task.agentIds[0].slice(0, 12)}`
348350
: undefined;
351+
markTaskMcpPending(taskId);
349352
mcpRestorePromises.push(
350353
invoke(IPC.StartMCPServer, {
351354
coordinatorTaskId: task.id,
@@ -358,10 +361,11 @@ function App() {
358361
agentArgs: agentDef?.args ?? [],
359362
dockerContainerName,
360363
})
364+
.then(() => markTaskMcpReady(taskId))
361365
.catch((err) => {
362366
console.warn(`[MCP] Failed to restore MCP server for coordinator task ${taskId}:`, err);
363-
})
364-
.finally(() => markTaskMcpReady(taskId)),
367+
markTaskMcpError(taskId, String(err));
368+
}),
365369
);
366370
}
367371
// Wait for all coordinators to register before hydrating their children —
@@ -375,6 +379,7 @@ function App() {
375379
if (!task?.coordinatedBy) continue;
376380
const projectRoot = store.projects.find((p) => p.id === task.projectId)?.path;
377381
if (!projectRoot) continue;
382+
markTaskMcpPending(task.id);
378383
invoke(IPC.MCP_HydrateCoordinatedTask, {
379384
id: task.id,
380385
name: task.name,
@@ -391,10 +396,11 @@ function App() {
391396
mcpConfigPath: task.mcpConfigPath,
392397
preambleFileExistedBefore: task.preambleFileExistedBefore,
393398
})
399+
.then(() => markTaskMcpReady(task.id))
394400
.catch((err) => {
395401
console.warn(`[MCP] Failed to hydrate coordinated task ${taskId}:`, err);
396-
})
397-
.finally(() => markTaskMcpReady(task.id));
402+
markTaskMcpError(task.id, String(err));
403+
});
398404
}
399405

400406
// Restore plan content for tasks that had a plan file before restart

src/components/TerminalView.tsx

Lines changed: 63 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { onMount, onCleanup, createEffect } from 'solid-js';
1+
import { onMount, onCleanup, createEffect, Show } from 'solid-js';
22
import { Terminal, type IMarker } from '@xterm/xterm';
33
import { FitAddon } from '@xterm/addon-fit';
44
import { WebglAddon } from '@xterm/addon-webgl';
@@ -12,7 +12,7 @@ import { matchesGlobalShortcut } from '../lib/shortcuts';
1212
import { isMac } from '../lib/platform';
1313
import { resolvedBindings } from '../store/keybindings';
1414
import { matchesKeyEvent } from '../lib/keybindings';
15-
import { store, setTaskLastInputAt } from '../store/store';
15+
import { store, setTaskLastInputAt, retryTaskMcpStartup } from '../store/store';
1616
import { warn as logWarn } from '../lib/log';
1717
import { registerTerminal, unregisterTerminal, markDirty } from '../lib/terminalFitManager';
1818
import { dataTransferToShellArgs, escapePath } from '../lib/terminalDrop';
@@ -638,16 +638,18 @@ export function TerminalView(props: TerminalViewProps) {
638638
// For coordinator and coordinated sub-tasks, defer spawn until MCP is ready.
639639
// Coordinator tasks wait for StartMCPServer to complete; sub-tasks wait for hydrateTask.
640640
const task = store.tasks[taskId];
641-
if ((task?.coordinatedBy || task?.coordinatorMode) && !task?.mcpReady) {
641+
if (task?.mcpStartupStatus === 'pending') {
642642
let spawned = false;
643643
createEffect(() => {
644644
if (spawned) return;
645-
if (store.tasks[taskId]?.mcpReady) {
645+
const status = store.tasks[taskId]?.mcpStartupStatus;
646+
if (status === 'ready') {
646647
spawned = true;
647648
spawnNow();
648649
}
650+
// 'error' is handled by the overlay rendered outside onMount
649651
});
650-
} else {
652+
} else if (task?.mcpStartupStatus !== 'error') {
651653
spawnNow();
652654
}
653655

@@ -688,16 +690,62 @@ export function TerminalView(props: TerminalViewProps) {
688690
markDirty(props.agentId);
689691
});
690692

693+
const mcpError = () => store.tasks[props.taskId]?.mcpStartupError;
694+
const mcpStatus = () => store.tasks[props.taskId]?.mcpStartupStatus;
695+
691696
return (
692-
<div
693-
ref={containerRef}
694-
style={{
695-
width: '100%',
696-
height: '100%',
697-
overflow: 'hidden',
698-
padding: '4px 0 0 4px',
699-
contain: 'strict',
700-
}}
701-
/>
697+
<div style={{ position: 'relative', width: '100%', height: '100%' }}>
698+
<div
699+
ref={containerRef}
700+
style={{
701+
width: '100%',
702+
height: '100%',
703+
overflow: 'hidden',
704+
padding: '4px 0 0 4px',
705+
contain: 'strict',
706+
}}
707+
/>
708+
<Show when={mcpStatus() === 'error'}>
709+
<div
710+
style={{
711+
position: 'absolute',
712+
inset: '0',
713+
display: 'flex',
714+
'flex-direction': 'column',
715+
'align-items': 'center',
716+
'justify-content': 'center',
717+
gap: '12px',
718+
background: 'rgba(0,0,0,0.85)',
719+
'font-family': 'var(--font-ui)',
720+
'z-index': '10',
721+
}}
722+
>
723+
<span
724+
style={{
725+
color: '#ff6b6b',
726+
'font-size': '13px',
727+
'text-align': 'center',
728+
padding: '0 16px',
729+
}}
730+
>
731+
MCP startup failed: {mcpError() ?? 'unknown error'}
732+
</span>
733+
<button
734+
style={{
735+
padding: '6px 16px',
736+
background: '#3b82f6',
737+
color: '#fff',
738+
border: 'none',
739+
'border-radius': '4px',
740+
'font-size': '13px',
741+
cursor: 'pointer',
742+
}}
743+
onClick={() => retryTaskMcpStartup(props.taskId)}
744+
>
745+
Retry
746+
</button>
747+
</div>
748+
</Show>
749+
</div>
702750
);
703751
}

src/store/store.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,10 @@ export {
5858
initMCPListeners,
5959
getCoordinatorCloseWarning,
6060
setTaskControl,
61+
markTaskMcpPending,
6162
markTaskMcpReady,
63+
markTaskMcpError,
64+
retryTaskMcpStartup,
6265
} from './tasks';
6366
export {
6467
setActiveTask,

src/store/tasks.test.ts

Lines changed: 146 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ let mockTasks: Record<string, MockTask> = {};
2222
let mockAgents: Record<string, unknown> = {};
2323
let mockTaskOrder: string[] = [];
2424
let mockCollapsedTaskOrder: string[] = [];
25+
let mockProjects: { id: string; path: string }[] = [];
2526
const ipcHandlers = new Map<string, (data: unknown) => void>();
2627

2728
function applySetStore(...args: unknown[]): void {
@@ -72,6 +73,7 @@ vi.mock('./core', () => ({
7273
if (prop === 'taskOrder') return mockTaskOrder;
7374
if (prop === 'collapsedTaskOrder') return mockCollapsedTaskOrder;
7475
if (prop === 'availableAgents') return [];
76+
if (prop === 'projects') return mockProjects;
7577
return undefined;
7678
},
7779
}),
@@ -122,7 +124,16 @@ vi.stubGlobal('window', {
122124
},
123125
});
124126

125-
import { initMCPListeners, setTaskControl, collapseTask, sendPrompt } from './tasks';
127+
import {
128+
initMCPListeners,
129+
setTaskControl,
130+
collapseTask,
131+
sendPrompt,
132+
markTaskMcpPending,
133+
markTaskMcpReady,
134+
markTaskMcpError,
135+
retryTaskMcpStartup,
136+
} from './tasks';
126137

127138
// ─── Coordinator listener setup ───────────────────────────────────────────────
128139

@@ -135,6 +146,7 @@ beforeEach(() => {
135146
mockAgents = {};
136147
mockTaskOrder = [];
137148
mockCollapsedTaskOrder = [];
149+
mockProjects = [];
138150
mockInvoke.mockResolvedValue(undefined);
139151
});
140152

@@ -296,6 +308,139 @@ describe('hasActiveCoordinator condition — coordinator task removal', () => {
296308
});
297309
});
298310

311+
// ─── MCP startup failure handling (TODO #43) ─────────────────────────────────
312+
313+
describe('MCP startup status transitions', () => {
314+
beforeEach(() => {
315+
vi.clearAllMocks();
316+
mockSetStore.mockImplementation((...args: unknown[]) => applySetStore(...args));
317+
mockInvoke.mockResolvedValue(undefined);
318+
mockProjects = [{ id: 'proj-1', path: '/repo' }];
319+
});
320+
321+
it('markTaskMcpPending sets status to pending', () => {
322+
mockTasks['task-1'] = { agentIds: [], shellAgentIds: [] };
323+
markTaskMcpPending('task-1');
324+
expect(mockTasks['task-1'].mcpStartupStatus).toBe('pending');
325+
});
326+
327+
it('markTaskMcpReady sets status to ready', () => {
328+
mockTasks['task-1'] = { agentIds: [], shellAgentIds: [], mcpStartupStatus: 'pending' };
329+
markTaskMcpReady('task-1');
330+
expect(mockTasks['task-1'].mcpStartupStatus).toBe('ready');
331+
});
332+
333+
it('markTaskMcpError sets status to error with control chars stripped', () => {
334+
mockTasks['task-1'] = { agentIds: [], shellAgentIds: [], mcpStartupStatus: 'pending' };
335+
// \x1b (ESC, 0x1b) is a control char and gets stripped; printable chars like '[31m' remain
336+
markTaskMcpError('task-1', 'Connection refused\x1b[31m injected\x1b[0m');
337+
expect(mockTasks['task-1'].mcpStartupStatus).toBe('error');
338+
expect(mockTasks['task-1'].mcpStartupError).toBe('Connection refused[31m injected[0m');
339+
});
340+
341+
it('failed StartMCPServer marks coordinator task with error instead of staying pending', async () => {
342+
mockTasks['coord-1'] = {
343+
agentIds: ['agent-coord'],
344+
shellAgentIds: [],
345+
coordinatorMode: true,
346+
projectId: 'proj-1',
347+
gitIsolation: 'worktree',
348+
worktreePath: '/repo/.worktrees/coord',
349+
};
350+
mockAgents['agent-coord'] = { def: { command: 'claude', args: [] } };
351+
mockInvoke.mockRejectedValueOnce(new Error('port in use'));
352+
353+
markTaskMcpPending('coord-1');
354+
await retryTaskMcpStartup('coord-1');
355+
356+
expect(mockTasks['coord-1'].mcpStartupStatus).toBe('error');
357+
expect(String(mockTasks['coord-1'].mcpStartupError)).toContain('port in use');
358+
});
359+
360+
it('successful StartMCPServer marks coordinator task as ready', async () => {
361+
mockTasks['coord-1'] = {
362+
agentIds: ['agent-coord'],
363+
shellAgentIds: [],
364+
coordinatorMode: true,
365+
projectId: 'proj-1',
366+
gitIsolation: 'worktree',
367+
worktreePath: '/repo/.worktrees/coord',
368+
};
369+
mockAgents['agent-coord'] = { def: { command: 'claude', args: [] } };
370+
mockInvoke.mockResolvedValueOnce(undefined);
371+
372+
markTaskMcpPending('coord-1');
373+
await retryTaskMcpStartup('coord-1');
374+
375+
expect(mockTasks['coord-1'].mcpStartupStatus).toBe('ready');
376+
});
377+
378+
it('child hydration failure marks only that child as error, leaving sibling spawnable', async () => {
379+
mockTasks['coord-1'] = {
380+
agentIds: [],
381+
shellAgentIds: [],
382+
coordinatorMode: true,
383+
projectId: 'proj-1',
384+
mcpStartupStatus: 'ready',
385+
};
386+
mockTasks['child-a'] = {
387+
agentIds: [],
388+
shellAgentIds: [],
389+
coordinatedBy: 'coord-1',
390+
projectId: 'proj-1',
391+
gitIsolation: 'worktree',
392+
worktreePath: '/repo/.worktrees/child-a',
393+
branchName: 'task/child-a',
394+
};
395+
mockTasks['child-b'] = {
396+
agentIds: [],
397+
shellAgentIds: [],
398+
coordinatedBy: 'coord-1',
399+
projectId: 'proj-1',
400+
gitIsolation: 'worktree',
401+
worktreePath: '/repo/.worktrees/child-b',
402+
branchName: 'task/child-b',
403+
};
404+
405+
// child-a fails, child-b succeeds
406+
mockInvoke.mockRejectedValueOnce(new Error('hydrate failed')).mockResolvedValueOnce(undefined);
407+
408+
markTaskMcpPending('child-a');
409+
await retryTaskMcpStartup('child-a');
410+
markTaskMcpPending('child-b');
411+
await retryTaskMcpStartup('child-b');
412+
413+
expect(mockTasks['child-a'].mcpStartupStatus).toBe('error');
414+
expect(mockTasks['child-b'].mcpStartupStatus).toBe('ready');
415+
});
416+
417+
it('retry of child when coordinator is in error surfaces dependency message', async () => {
418+
mockTasks['coord-1'] = {
419+
agentIds: [],
420+
shellAgentIds: [],
421+
coordinatorMode: true,
422+
projectId: 'proj-1',
423+
mcpStartupStatus: 'error',
424+
};
425+
mockTasks['child-1'] = {
426+
agentIds: [],
427+
shellAgentIds: [],
428+
coordinatedBy: 'coord-1',
429+
projectId: 'proj-1',
430+
gitIsolation: 'worktree',
431+
worktreePath: '/repo/.worktrees/child-1',
432+
branchName: 'task/child-1',
433+
mcpStartupStatus: 'error',
434+
};
435+
436+
await retryTaskMcpStartup('child-1');
437+
438+
expect(mockTasks['child-1'].mcpStartupStatus).toBe('error');
439+
expect(String(mockTasks['child-1'].mcpStartupError)).toContain('coordinator');
440+
expect(mockInvoke).not.toHaveBeenCalledWith(IPC.MCP_HydrateCoordinatedTask, expect.anything());
441+
});
442+
});
443+
299444
// ─── sendPrompt tests ─────────────────────────────────────────────────────────
300445

301446
function writePayloads(): string[] {

0 commit comments

Comments
 (0)