Skip to content

Commit 70d5d27

Browse files
brookscclaude
brooksc
and
claude
committed
test(coordinator): add missing tests for johannesjo#33, johannesjo#36, johannesjo#39
- johannesjo#33: restart round-trip — hydrateTask rewrites config with new subtaskToken (not coordinator token) when setMCPServerInfo has already run; and waitForIdle resolves after agent output fires post-hydration - johannesjo#36: mcpConfigPath directory scoping — path traversal rejected, wrong-dir rejected, correct host tmpdir accepted, Docker dirname(serverPath) accepted, Docker wrong-dir rejected - johannesjo#39: per-task close isolation — closing task-1 deletes only its config file; task-2 config and task entry remain untouched Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 1b3124c commit 70d5d27

1 file changed

Lines changed: 269 additions & 0 deletions

File tree

‎electron/mcp/coordinator.test.ts‎

Lines changed: 269 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
2+
import os from 'os';
3+
import { join, dirname } from 'path';
24

35
// --- fs / child_process mocks (must come before dynamic import) ---
46
const mockWriteFileSync = vi.fn();
@@ -2402,6 +2404,273 @@ describe('Coordinator removeCoordinatedTask', () => {
24022404
});
24032405
});
24042406

2407+
// ─── #33: Post-restart coordinator flow integration test ─────────────────────
2408+
2409+
describe('Coordinator restart round-trip integration', () => {
2410+
let coordinator: InstanceType<typeof Coordinator>;
2411+
2412+
beforeEach(() => {
2413+
vi.clearAllMocks();
2414+
mockExistsSync.mockReturnValue(false);
2415+
coordinator = new Coordinator();
2416+
coordinator.setWindow(mockWin);
2417+
coordinator.setDefaultProject('proj-1', '/tmp/project');
2418+
coordinator.registerCoordinator('coord-1', 'proj-1');
2419+
});
2420+
2421+
it('hydrateTask rewrites config with new subtaskToken (not coordinator token) when server info is already set', () => {
2422+
coordinator.setMCPServerInfo(
2423+
'coord-1',
2424+
'http://localhost:3002',
2425+
'new-coordinator-secret',
2426+
'new-subtask-secret',
2427+
'/path/server.js',
2428+
);
2429+
2430+
const taskId = 'hydrated-restart-1';
2431+
const configPath = join(os.tmpdir(), `parallel-code-subtask-${taskId}.json`);
2432+
2433+
mockWriteFileSync.mockClear();
2434+
coordinator.hydrateTask({
2435+
id: taskId,
2436+
name: 'hydrated-task',
2437+
projectId: 'proj-1',
2438+
projectRoot: '/tmp/project',
2439+
branchName: 'task/hydrated',
2440+
worktreePath: '/tmp/hydrated',
2441+
agentId: 'agent-restart-1',
2442+
coordinatorTaskId: 'coord-1',
2443+
mcpConfigPath: configPath,
2444+
});
2445+
2446+
const rewrite = mockWriteFileSync.mock.calls.find((c) => c[0] === configPath);
2447+
expect(rewrite).toBeDefined();
2448+
if (!rewrite) throw new Error('expected config rewrite');
2449+
const config = JSON.parse(rewrite[1] as string) as {
2450+
mcpServers: { 'parallel-code': { env: Record<string, string> } };
2451+
};
2452+
const writtenToken = config.mcpServers['parallel-code'].env['PARALLEL_CODE_MCP_TOKEN'];
2453+
expect(writtenToken).toBe('new-subtask-secret');
2454+
expect(writtenToken).not.toBe('new-coordinator-secret');
2455+
});
2456+
2457+
it('waitForIdle resolves after agent output fires post-hydration', async () => {
2458+
coordinator.setMCPServerInfo(
2459+
'coord-1',
2460+
'http://localhost:3002',
2461+
'new-coordinator-secret',
2462+
'new-subtask-secret',
2463+
'/path/server.js',
2464+
);
2465+
2466+
const taskId = 'hydrated-restart-2';
2467+
const agentId = 'agent-restart-2';
2468+
const configPath = join(os.tmpdir(), `parallel-code-subtask-${taskId}.json`);
2469+
2470+
coordinator.hydrateTask({
2471+
id: taskId,
2472+
name: 'hydrated-task',
2473+
projectId: 'proj-1',
2474+
projectRoot: '/tmp/project',
2475+
branchName: 'task/hydrated',
2476+
worktreePath: '/tmp/hydrated',
2477+
agentId,
2478+
coordinatorTaskId: 'coord-1',
2479+
mcpConfigPath: configPath,
2480+
});
2481+
2482+
// Simulate agent respawn: task transitions from exited → running
2483+
const task = coordinator.getTask(taskId);
2484+
expect(task).toBeDefined();
2485+
if (!task) throw new Error('task not found after hydration');
2486+
task.status = 'running';
2487+
2488+
// Get the output callback registered during hydrateTask
2489+
const hydratedCb = mockSubscribeToAgent.mock.calls.find((c) => c[0] === agentId)?.[1] as
2490+
| ((encoded: string) => void)
2491+
| undefined;
2492+
expect(hydratedCb).toBeDefined();
2493+
if (!hydratedCb) throw new Error('hydrateTask did not subscribe to agent');
2494+
2495+
const waitPromise = coordinator.waitForIdle(taskId);
2496+
hydratedCb(encode('Work done ❯ '));
2497+
await expect(waitPromise).resolves.toEqual({ reason: 'idle' });
2498+
});
2499+
});
2500+
2501+
// ─── #36: hydrateTask mcpConfigPath directory scoping ────────────────────────
2502+
2503+
describe('Coordinator hydrateTask — mcpConfigPath directory scoping', () => {
2504+
let coordinator: InstanceType<typeof Coordinator>;
2505+
2506+
beforeEach(() => {
2507+
vi.clearAllMocks();
2508+
mockExistsSync.mockReturnValue(false);
2509+
coordinator = new Coordinator();
2510+
coordinator.setWindow(mockWin);
2511+
coordinator.setDefaultProject('proj-1', '/tmp/project');
2512+
coordinator.registerCoordinator('coord-1', 'proj-1');
2513+
coordinator.setMCPServerInfo(
2514+
'coord-1',
2515+
'http://localhost:3001',
2516+
'tok',
2517+
'subtok',
2518+
'/srv/app/.parallel-code/mcp-server.js',
2519+
);
2520+
});
2521+
2522+
it('path traversal (../../etc/passwd-style) is rejected — mcpConfigPath is undefined, no config write', () => {
2523+
coordinator.hydrateTask({
2524+
id: 'task-traversal',
2525+
name: 'evil',
2526+
projectId: 'proj-1',
2527+
projectRoot: '/tmp/project',
2528+
branchName: 'task/evil',
2529+
worktreePath: '/tmp/evil',
2530+
agentId: 'agent-evil',
2531+
coordinatorTaskId: 'coord-1',
2532+
mcpConfigPath: '../../etc/passwd',
2533+
});
2534+
2535+
expect(coordinator.getTask('task-traversal')?.mcpConfigPath).toBeUndefined();
2536+
const evilWrite = mockWriteFileSync.mock.calls.find((c) =>
2537+
(c[0] as string).includes('etc/passwd'),
2538+
);
2539+
expect(evilWrite).toBeUndefined();
2540+
});
2541+
2542+
it('right filename in wrong dir is rejected — mcpConfigPath is undefined', () => {
2543+
const taskId = 'task-wrong-dir';
2544+
coordinator.hydrateTask({
2545+
id: taskId,
2546+
name: 'wrong-dir',
2547+
projectId: 'proj-1',
2548+
projectRoot: '/tmp/project',
2549+
branchName: 'task/wrong-dir',
2550+
worktreePath: '/tmp/wrong-dir',
2551+
agentId: 'agent-wrong-dir',
2552+
coordinatorTaskId: 'coord-1',
2553+
mcpConfigPath: `/tmp/evil/parallel-code-subtask-${taskId}.json`,
2554+
});
2555+
2556+
expect(coordinator.getTask(taskId)?.mcpConfigPath).toBeUndefined();
2557+
});
2558+
2559+
it('correct host tmpdir path is accepted and config write occurs', () => {
2560+
const taskId = 'task-valid-host';
2561+
const validPath = join(os.tmpdir(), `parallel-code-subtask-${taskId}.json`);
2562+
2563+
mockWriteFileSync.mockClear();
2564+
coordinator.hydrateTask({
2565+
id: taskId,
2566+
name: 'valid-host',
2567+
projectId: 'proj-1',
2568+
projectRoot: '/tmp/project',
2569+
branchName: 'task/valid-host',
2570+
worktreePath: '/tmp/valid-host',
2571+
agentId: 'agent-valid-host',
2572+
coordinatorTaskId: 'coord-1',
2573+
mcpConfigPath: validPath,
2574+
});
2575+
2576+
expect(coordinator.getTask(taskId)?.mcpConfigPath).toBe(validPath);
2577+
const configWrite = mockWriteFileSync.mock.calls.find((c) => c[0] === validPath);
2578+
expect(configWrite).toBeDefined();
2579+
});
2580+
2581+
it('Docker mode: dirname(serverPath)/subtask-{id}.json is accepted and config write occurs', () => {
2582+
const taskId = 'task-valid-docker';
2583+
const serverPath = '/srv/app/.parallel-code/mcp-server.js';
2584+
const dockerPath = join(dirname(serverPath), `subtask-${taskId}.json`);
2585+
2586+
mockWriteFileSync.mockClear();
2587+
coordinator.hydrateTask({
2588+
id: taskId,
2589+
name: 'valid-docker',
2590+
projectId: 'proj-1',
2591+
projectRoot: '/tmp/project',
2592+
branchName: 'task/valid-docker',
2593+
worktreePath: '/tmp/valid-docker',
2594+
agentId: 'agent-valid-docker',
2595+
coordinatorTaskId: 'coord-1',
2596+
mcpConfigPath: dockerPath,
2597+
});
2598+
2599+
expect(coordinator.getTask(taskId)?.mcpConfigPath).toBe(dockerPath);
2600+
const configWrite = mockWriteFileSync.mock.calls.find((c) => c[0] === dockerPath);
2601+
expect(configWrite).toBeDefined();
2602+
});
2603+
2604+
it('Docker mode: path in wrong dir is rejected — mcpConfigPath is undefined', () => {
2605+
const taskId = 'task-evil-docker';
2606+
const wrongPath = `/some/other/dir/subtask-${taskId}.json`;
2607+
2608+
coordinator.hydrateTask({
2609+
id: taskId,
2610+
name: 'evil-docker',
2611+
projectId: 'proj-1',
2612+
projectRoot: '/tmp/project',
2613+
branchName: 'task/evil-docker',
2614+
worktreePath: '/tmp/evil-docker',
2615+
agentId: 'agent-evil-docker',
2616+
coordinatorTaskId: 'coord-1',
2617+
mcpConfigPath: wrongPath,
2618+
});
2619+
2620+
expect(coordinator.getTask(taskId)?.mcpConfigPath).toBeUndefined();
2621+
});
2622+
});
2623+
2624+
// ─── #39: Docker coordinator child-close isolation ────────────────────────────
2625+
2626+
describe('Coordinator closeTask — per-task config isolation (two sub-tasks)', () => {
2627+
let coordinator: InstanceType<typeof Coordinator>;
2628+
2629+
beforeEach(() => {
2630+
vi.clearAllMocks();
2631+
mockExistsSync.mockReturnValue(false);
2632+
coordinator = new Coordinator();
2633+
coordinator.setWindow(mockWin);
2634+
coordinator.setDefaultProject('proj-1', '/tmp/project');
2635+
coordinator.registerCoordinator('coord-1', 'proj-1');
2636+
coordinator.setMCPServerInfo(
2637+
'coord-1',
2638+
'http://localhost:3001',
2639+
'tok',
2640+
'subtask-tok',
2641+
'/path/server.js',
2642+
);
2643+
});
2644+
2645+
it('closing task-1 deletes only its config; task-2 config and task-2 entry are untouched', async () => {
2646+
mockCreateBackendTask
2647+
.mockResolvedValueOnce({ id: 'task-1', branch_name: 'task/a', worktree_path: '/tmp/a' })
2648+
.mockResolvedValueOnce({ id: 'task-2', branch_name: 'task/b', worktree_path: '/tmp/b' });
2649+
2650+
await coordinator.createTask({ name: 'task-one', prompt: 'do', coordinatorTaskId: 'coord-1' });
2651+
await coordinator.createTask({ name: 'task-two', prompt: 'do', coordinatorTaskId: 'coord-1' });
2652+
2653+
const config1 = coordinator.getTask('task-1')?.mcpConfigPath;
2654+
const config2 = coordinator.getTask('task-2')?.mcpConfigPath;
2655+
expect(config1).toBeDefined();
2656+
expect(config2).toBeDefined();
2657+
expect(config1).not.toBe(config2);
2658+
2659+
mockUnlinkSync.mockClear();
2660+
await coordinator.closeTask('task-1');
2661+
2662+
// task-1's config was deleted
2663+
expect(mockUnlinkSync).toHaveBeenCalledWith(config1);
2664+
// task-2's config was NOT deleted
2665+
expect(mockUnlinkSync).not.toHaveBeenCalledWith(config2);
2666+
2667+
// task-2 is still present, task-1 is gone
2668+
const tasks = coordinator.listTasks();
2669+
expect(tasks.some((t) => t.id === 'task-2')).toBe(true);
2670+
expect(tasks.some((t) => t.id === 'task-1')).toBe(false);
2671+
});
2672+
});
2673+
24052674
// ─── preload allowlist regression test ───────────────────────────────────────
24062675

24072676
describe('preload.cjs MCP channel allowlist', () => {

0 commit comments

Comments
 (0)