diff --git a/src/main/services/team/TeamProvisioningService.ts b/src/main/services/team/TeamProvisioningService.ts index 38629070..9d263a04 100644 --- a/src/main/services/team/TeamProvisioningService.ts +++ b/src/main/services/team/TeamProvisioningService.ts @@ -3203,6 +3203,10 @@ export class TeamProvisioningService { const tasksDir = path.join(getTasksBasePath(), request.teamName); await fs.promises.rm(teamDir, { recursive: true, force: true }).catch(() => {}); await fs.promises.rm(tasksDir, { recursive: true, force: true }).catch(() => {}); + if (run.mcpConfigPath) { + await this.mcpConfigBuilder.removeConfigFile(run.mcpConfigPath).catch(() => {}); + run.mcpConfigPath = null; + } this.runs.delete(runId); this.provisioningRunByTeam.delete(request.teamName); throw error; @@ -3639,6 +3643,10 @@ export class TeamProvisioningService { stdio: ['pipe', 'pipe', 'pipe'], }); } catch (error) { + if (run.mcpConfigPath) { + await this.mcpConfigBuilder.removeConfigFile(run.mcpConfigPath).catch(() => {}); + run.mcpConfigPath = null; + } this.runs.delete(runId); this.provisioningRunByTeam.delete(request.teamName); await this.restorePrelaunchConfig(request.teamName); diff --git a/test/main/services/team/TeamMcpConfigBuilder.test.ts b/test/main/services/team/TeamMcpConfigBuilder.test.ts index 124aff83..6dcb2620 100644 --- a/test/main/services/team/TeamMcpConfigBuilder.test.ts +++ b/test/main/services/team/TeamMcpConfigBuilder.test.ts @@ -1,9 +1,41 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import * as fs from 'fs'; +import Module from 'module'; import * as os from 'os'; import * as path from 'path'; +const hoisted = vi.hoisted(() => ({ + electronState: { + isPackaged: false, + version: '9.9.9-test', + }, + execFileMock: vi.fn( + ( + _file: string, + _args: readonly string[], + _options: + | { encoding?: string; timeout?: number } + | ((error: Error | null, stdout: string, stderr: string) => void), + callback?: (error: Error | null, stdout: string, stderr: string) => void + ) => { + const cb = typeof _options === 'function' ? _options : callback; + cb?.(null, '/mock/node', ''); + } + ), +})); + let mockHomeDir = ''; +type ModuleLoad = (request: string, parent: NodeModule | undefined, isMain: boolean) => unknown; +const moduleInternal = Module as unknown as { _load: ModuleLoad }; +const originalModuleLoad = moduleInternal._load; + +vi.mock('child_process', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + execFile: hoisted.execFileMock, + }; +}); vi.mock('@main/utils/pathDecoder', async (importOriginal) => { const actual = await importOriginal(); @@ -20,15 +52,72 @@ describe('TeamMcpConfigBuilder', () => { const createdPaths: string[] = []; const createdDirs: string[] = []; let tempAppData: string; + let originalResourcesPath: string | undefined; + + function setPackagedMode(isPackaged: boolean, version = '9.9.9-test'): void { + hoisted.electronState.isPackaged = isPackaged; + hoisted.electronState.version = version; + } + + function setResourcesPath(resourcesPath: string | undefined): void { + Object.defineProperty(process, 'resourcesPath', { + value: resourcesPath, + configurable: true, + writable: true, + }); + } + + function createPackagedServerBundle(baseDir: string, body = '// packaged server'): string { + const dir = path.join(baseDir, 'mcp-server'); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, 'index.js'), body); + fs.writeFileSync(path.join(dir, 'package.json'), JSON.stringify({ name: 'agent-teams-mcp' })); + return dir; + } + + function readGeneratedServer(configPath: string): { command?: string; args?: string[] } | undefined { + const raw = fs.readFileSync(configPath, 'utf8'); + const parsed = JSON.parse(raw) as { + mcpServers?: Record; + }; + return parsed.mcpServers?.['agent-teams']; + } + + function expectNodeEntry(server: { command?: string; args?: string[] } | undefined, entry: string): void { + expect(server?.args).toEqual([entry]); + expect(server?.command).toMatch(/(^node$|[\\/]node(?:\.exe)?$)/); + } beforeEach(() => { + originalResourcesPath = (process as NodeJS.Process & { resourcesPath?: string }).resourcesPath; tempAppData = fs.mkdtempSync(path.join(os.tmpdir(), 'team-mcp-appdata-')); createdDirs.push(tempAppData); + moduleInternal._load = ((request, parent, isMain) => { + if (request === 'electron') { + return { + app: { + get isPackaged() { + return hoisted.electronState.isPackaged; + }, + getVersion: () => hoisted.electronState.version, + getPath: () => '/mock/electron-user-data', + }, + }; + } + return originalModuleLoad(request, parent, isMain); + }) as ModuleLoad; setAppDataBasePath(tempAppData); + setPackagedMode(false); + setResourcesPath(undefined); + hoisted.execFileMock.mockClear(); }); afterEach(() => { setAppDataBasePath(null); + setPackagedMode(false); + setResourcesPath(originalResourcesPath); + moduleInternal._load = originalModuleLoad; + vi.restoreAllMocks(); for (const filePath of createdPaths.splice(0)) { try { fs.rmSync(filePath, { force: true }); @@ -321,4 +410,125 @@ describe('TeamMcpConfigBuilder', () => { // Should not throw when directory doesn't exist await builder.gcStaleConfigs(); }); + + // ── Packaged copy / fallback ── + + it('packaged mode reuses an existing valid stable copy', async () => { + setPackagedMode(true, '1.2.3'); + setResourcesPath(tempAppData); + const stableDir = path.join(tempAppData, 'mcp-server', '1.2.3'); + fs.mkdirSync(stableDir, { recursive: true }); + fs.writeFileSync(path.join(stableDir, 'index.js'), '// stable copy'); + fs.writeFileSync(path.join(stableDir, 'package.json'), JSON.stringify({ name: 'stable' })); + + const builder = new TeamMcpConfigBuilder(); + const configPath = await builder.writeConfigFile(); + createdPaths.push(configPath); + + expectNodeEntry(readGeneratedServer(configPath), path.join(stableDir, 'index.js')); + }); + + it('packaged mode copies the MCP server from resourcesPath into userData', async () => { + setPackagedMode(true, '2.0.0'); + const resourcesDir = fs.mkdtempSync(path.join(os.tmpdir(), 'team-mcp-resources-')); + createdDirs.push(resourcesDir); + createPackagedServerBundle(resourcesDir, '// copied server'); + setResourcesPath(resourcesDir); + + const builder = new TeamMcpConfigBuilder(); + const configPath = await builder.writeConfigFile(); + createdPaths.push(configPath); + + const stableDir = path.join(tempAppData, 'mcp-server', '2.0.0'); + expect(fs.existsSync(path.join(stableDir, 'index.js'))).toBe(true); + expect(fs.existsSync(path.join(stableDir, 'package.json'))).toBe(true); + expectNodeEntry(readGeneratedServer(configPath), path.join(stableDir, 'index.js')); + }); + + it('packaged mode heals a partial stable copy and rebuilds it from resourcesPath', async () => { + setPackagedMode(true, '3.0.0'); + const stableDir = path.join(tempAppData, 'mcp-server', '3.0.0'); + fs.mkdirSync(stableDir, { recursive: true }); + fs.writeFileSync(path.join(stableDir, 'index.js'), '// partial copy only'); + + const resourcesDir = fs.mkdtempSync(path.join(os.tmpdir(), 'team-mcp-resources-')); + createdDirs.push(resourcesDir); + createPackagedServerBundle(resourcesDir, '// healed server'); + setResourcesPath(resourcesDir); + + const builder = new TeamMcpConfigBuilder(); + const configPath = await builder.writeConfigFile(); + createdPaths.push(configPath); + + expect(fs.readFileSync(path.join(stableDir, 'index.js'), 'utf8')).toContain('healed server'); + expect(fs.existsSync(path.join(stableDir, 'package.json'))).toBe(true); + expect(readGeneratedServer(configPath)?.args).toEqual([path.join(stableDir, 'index.js')]); + }); + + it('packaged mode falls back to resourcesPath when stable copy creation fails', async () => { + setPackagedMode(true, '4.0.0'); + const resourcesDir = fs.mkdtempSync(path.join(os.tmpdir(), 'team-mcp-resources-')); + createdDirs.push(resourcesDir); + createPackagedServerBundle(resourcesDir, '// fallback server'); + setResourcesPath(resourcesDir); + + vi.spyOn(fs.promises, 'copyFile').mockRejectedValueOnce(new Error('copy failed')); + + const builder = new TeamMcpConfigBuilder(); + const configPath = await builder.writeConfigFile(); + createdPaths.push(configPath); + + expectNodeEntry(readGeneratedServer(configPath), path.join(resourcesDir, 'mcp-server', 'index.js')); + }); + + it('packaged mode uses the winner stable copy when atomic rename loses the race', async () => { + setPackagedMode(true, '5.0.0'); + const resourcesDir = fs.mkdtempSync(path.join(os.tmpdir(), 'team-mcp-resources-')); + createdDirs.push(resourcesDir); + createPackagedServerBundle(resourcesDir, '// race source'); + setResourcesPath(resourcesDir); + + const stableDir = path.join(tempAppData, 'mcp-server', '5.0.0'); + const originalRename = fs.promises.rename.bind(fs.promises); + vi.spyOn(fs.promises, 'rename').mockImplementation(async (from, to) => { + if (to === stableDir) { + fs.mkdirSync(stableDir, { recursive: true }); + fs.writeFileSync(path.join(stableDir, 'index.js'), '// winner copy'); + fs.writeFileSync(path.join(stableDir, 'package.json'), JSON.stringify({ name: 'winner' })); + const err = new Error('EEXIST') as NodeJS.ErrnoException; + err.code = 'EEXIST'; + throw err; + } + return originalRename(from, to); + }); + + const builder = new TeamMcpConfigBuilder(); + const configPath = await builder.writeConfigFile(); + createdPaths.push(configPath); + + expect(fs.readFileSync(path.join(stableDir, 'index.js'), 'utf8')).toContain('winner copy'); + expectNodeEntry(readGeneratedServer(configPath), path.join(stableDir, 'index.js')); + }); + + it('packaged mode falls back to workspace source when resourcesPath bundle is missing', async () => { + setPackagedMode(true, '6.0.0'); + const resourcesDir = fs.mkdtempSync(path.join(os.tmpdir(), 'team-mcp-resources-')); + createdDirs.push(resourcesDir); + setResourcesPath(resourcesDir); + + const builder = new TeamMcpConfigBuilder(); + const configPath = await builder.writeConfigFile(); + createdPaths.push(configPath); + + expect(readGeneratedServer(configPath)).toEqual({ + command: 'pnpm', + args: [ + '--dir', + path.join(process.cwd(), 'mcp-server'), + 'exec', + 'tsx', + path.join(process.cwd(), 'mcp-server', 'src', 'index.ts'), + ], + }); + }); }); diff --git a/test/main/services/team/TeamProvisioningService.test.ts b/test/main/services/team/TeamProvisioningService.test.ts index e17724bd..caddafe8 100644 --- a/test/main/services/team/TeamProvisioningService.test.ts +++ b/test/main/services/team/TeamProvisioningService.test.ts @@ -1,16 +1,55 @@ import type { ChildProcess } from 'child_process'; import { EventEmitter } from 'events'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const hoisted = vi.hoisted(() => ({ + paths: { + claudeRoot: '', + teamsBase: '', + tasksBase: '', + projectsBase: '', + }, +})); + +let tempClaudeRoot = ''; +let tempTeamsBase = ''; +let tempTasksBase = ''; +let tempProjectsBase = ''; vi.mock('@main/services/team/ClaudeBinaryResolver', () => ({ ClaudeBinaryResolver: { resolve: vi.fn() }, })); +vi.mock('@main/services/team/TeamTaskReader', () => ({ + TeamTaskReader: class { + async getTasks() { + return []; + } + }, +})); + vi.mock('@main/utils/childProcess', () => ({ spawnCli: vi.fn(), + killProcessTree: vi.fn(), })); +vi.mock('@main/utils/pathDecoder', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + getAutoDetectedClaudeBasePath: () => hoisted.paths.claudeRoot, + getClaudeBasePath: () => hoisted.paths.claudeRoot, + getHomeDir: () => hoisted.paths.claudeRoot, + getProjectsBasePath: () => hoisted.paths.projectsBase, + getTasksBasePath: () => hoisted.paths.tasksBase, + getTeamsBasePath: () => hoisted.paths.teamsBase, + }; +}); + import { TeamProvisioningService } from '@main/services/team/TeamProvisioningService'; import { ClaudeBinaryResolver } from '@main/services/team/ClaudeBinaryResolver'; import { spawnCli } from '@main/utils/childProcess'; @@ -30,9 +69,47 @@ function createFakeChild(exitCode: number): ChildProcess { return child; } +function createRunningChild() { + return Object.assign(new EventEmitter(), { + pid: 12345, + stdin: { + writable: true, + write: vi.fn(() => true), + end: vi.fn(), + }, + stdout: new EventEmitter(), + stderr: new EventEmitter(), + kill: vi.fn(), + }); +} + describe('TeamProvisioningService', () => { beforeEach(() => { vi.clearAllMocks(); + tempClaudeRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'claude-team-provisioning-')); + tempTeamsBase = path.join(tempClaudeRoot, 'teams'); + tempTasksBase = path.join(tempClaudeRoot, 'tasks'); + tempProjectsBase = path.join(tempClaudeRoot, 'projects'); + hoisted.paths.claudeRoot = tempClaudeRoot; + hoisted.paths.teamsBase = tempTeamsBase; + hoisted.paths.tasksBase = tempTasksBase; + hoisted.paths.projectsBase = tempProjectsBase; + fs.mkdirSync(tempTeamsBase, { recursive: true }); + fs.mkdirSync(tempTasksBase, { recursive: true }); + fs.mkdirSync(tempProjectsBase, { recursive: true }); + }); + + afterEach(() => { + vi.useRealTimers(); + try { + fs.rmSync(tempClaudeRoot, { recursive: true, force: true }); + } catch { + // ignore temp cleanup failures + } + hoisted.paths.claudeRoot = ''; + hoisted.paths.teamsBase = ''; + hoisted.paths.tasksBase = ''; + hoisted.paths.projectsBase = ''; }); describe('warmup', () => { @@ -53,4 +130,198 @@ describe('TeamProvisioningService', () => { expect(spawnCli).toHaveBeenCalled(); }); }); + + it('removes generated MCP config when createTeam spawn fails synchronously', async () => { + allowConsoleLogs(); + vi.mocked(ClaudeBinaryResolver.resolve).mockResolvedValue('/mock/claude'); + vi.mocked(spawnCli).mockImplementation(() => { + throw new Error('spawn EINVAL'); + }); + + const mcpConfigBuilder = { + writeConfigFile: vi.fn(async () => '/mock/mcp-config-create.json'), + removeConfigFile: vi.fn(async () => {}), + }; + const membersMetaStore = { + writeMembers: vi.fn(async () => {}), + }; + const teamMetaStore = { + writeMeta: vi.fn(async () => {}), + deleteMeta: vi.fn(async () => {}), + }; + + const svc = new TeamProvisioningService( + undefined, + undefined, + membersMetaStore as any, + undefined, + mcpConfigBuilder as any, + teamMetaStore as any + ); + (svc as any).buildProvisioningEnv = vi.fn(async () => ({ + env: { ANTHROPIC_API_KEY: 'test' }, + authSource: 'anthropic_api_key', + })); + (svc as any).pathExists = vi.fn(async () => false); + + await expect( + svc.createTeam( + { + teamName: 'cleanup-team', + cwd: tempClaudeRoot, + members: [{ name: 'alice' }], + }, + () => {} + ) + ).rejects.toThrow('spawn EINVAL'); + + expect(mcpConfigBuilder.writeConfigFile).toHaveBeenCalledWith(tempClaudeRoot); + expect(mcpConfigBuilder.removeConfigFile).toHaveBeenCalledWith('/mock/mcp-config-create.json'); + expect(teamMetaStore.deleteMeta).toHaveBeenCalledWith('cleanup-team'); + }); + + it('removes generated MCP config when launchTeam spawn fails synchronously', async () => { + allowConsoleLogs(); + const teamName = 'launch-cleanup-team'; + const teamDir = path.join(tempTeamsBase, teamName); + fs.mkdirSync(teamDir, { recursive: true }); + fs.writeFileSync( + path.join(teamDir, 'config.json'), + JSON.stringify({ + name: teamName, + projectPath: tempClaudeRoot, + members: [{ name: 'team-lead', agentType: 'team-lead' }, { name: 'alice' }], + }), + 'utf8' + ); + + vi.mocked(ClaudeBinaryResolver.resolve).mockResolvedValue('/mock/claude'); + vi.mocked(spawnCli).mockImplementation(() => { + throw new Error('launch spawn EINVAL'); + }); + + const mcpConfigBuilder = { + writeConfigFile: vi.fn(async () => '/mock/mcp-config-launch.json'), + removeConfigFile: vi.fn(async () => {}), + }; + const restorePrelaunchConfig = vi.fn(async () => {}); + + const svc = new TeamProvisioningService( + undefined, + undefined, + undefined, + undefined, + mcpConfigBuilder as any + ); + (svc as any).buildProvisioningEnv = vi.fn(async () => ({ + env: { ANTHROPIC_API_KEY: 'test' }, + authSource: 'anthropic_api_key', + })); + (svc as any).resolveLaunchExpectedMembers = vi.fn(async () => ({ + members: [{ name: 'alice' }], + source: 'members-meta', + warning: undefined, + })); + (svc as any).normalizeTeamConfigForLaunch = vi.fn(async () => {}); + (svc as any).assertConfigLeadOnlyForLaunch = vi.fn(async () => {}); + (svc as any).updateConfigProjectPath = vi.fn(async () => {}); + (svc as any).restorePrelaunchConfig = restorePrelaunchConfig; + (svc as any).pathExists = vi.fn(async () => false); + + await expect(svc.launchTeam({ teamName, cwd: tempClaudeRoot }, () => {})).rejects.toThrow( + 'launch spawn EINVAL' + ); + + expect(mcpConfigBuilder.writeConfigFile).toHaveBeenCalledWith(tempClaudeRoot); + expect(mcpConfigBuilder.removeConfigFile).toHaveBeenCalledWith('/mock/mcp-config-launch.json'); + expect(restorePrelaunchConfig).toHaveBeenCalledWith(teamName); + }); + + it('regenerates a missing --mcp-config before auth-failure respawn', async () => { + vi.useFakeTimers(); + allowConsoleLogs(); + vi.mocked(ClaudeBinaryResolver.resolve).mockResolvedValue('/mock/claude'); + + const firstChild = createRunningChild(); + const secondChild = createRunningChild(); + vi.mocked(spawnCli) + .mockImplementationOnce(() => firstChild as any) + .mockImplementationOnce(() => secondChild as any); + + const mcpConfigBuilder = { + writeConfigFile: vi + .fn() + .mockResolvedValueOnce('/missing/original-mcp-config.json') + .mockResolvedValueOnce('/regenerated/mcp-config.json'), + removeConfigFile: vi.fn(async () => {}), + }; + const membersMetaStore = { + writeMembers: vi.fn(async () => {}), + }; + const teamMetaStore = { + writeMeta: vi.fn(async () => {}), + deleteMeta: vi.fn(async () => {}), + }; + + const svc = new TeamProvisioningService( + undefined, + undefined, + membersMetaStore as any, + undefined, + mcpConfigBuilder as any, + teamMetaStore as any + ); + (svc as any).buildProvisioningEnv = vi.fn(async () => ({ + env: { ANTHROPIC_API_KEY: 'test' }, + authSource: 'anthropic_api_key', + })); + (svc as any).pathExists = vi.fn(async () => false); + (svc as any).startFilesystemMonitor = vi.fn(); + (svc as any).stopFilesystemMonitor = vi.fn(); + (svc as any).startStallWatchdog = vi.fn(); + (svc as any).stopStallWatchdog = vi.fn(); + (svc as any).attachStdoutHandler = vi.fn(); + (svc as any).attachStderrHandler = vi.fn(); + + const { runId } = await svc.createTeam( + { + teamName: 'retry-team', + cwd: tempClaudeRoot, + members: [{ name: 'alice' }], + }, + () => {} + ); + + const run = (svc as any).runs.get(runId); + expect(run).toBeTruthy(); + + const mcpFlagIdx = run.spawnContext.args.indexOf('--mcp-config'); + expect(mcpFlagIdx).toBeGreaterThanOrEqual(0); + run.spawnContext.args[mcpFlagIdx + 1] = path.join(tempClaudeRoot, 'deleted-mcp-config.json'); + run.mcpConfigPath = run.spawnContext.args[mcpFlagIdx + 1]; + run.authRetryInProgress = true; + + const respawnPromise = (svc as any).respawnAfterAuthFailure(run); + await vi.advanceTimersByTimeAsync(2000); + await respawnPromise; + + expect(mcpConfigBuilder.writeConfigFile).toHaveBeenNthCalledWith(2, tempClaudeRoot); + expect(run.spawnContext.args[mcpFlagIdx + 1]).toBe('/regenerated/mcp-config.json'); + expect(run.mcpConfigPath).toBe('/regenerated/mcp-config.json'); + expect(vi.mocked(spawnCli)).toHaveBeenNthCalledWith( + 2, + '/mock/claude', + run.spawnContext.args, + expect.objectContaining({ + cwd: tempClaudeRoot, + stdio: ['pipe', 'pipe', 'pipe'], + }) + ); + expect(run.child).toBe(secondChild); + + if (run.timeoutHandle) { + clearTimeout(run.timeoutHandle); + run.timeoutHandle = null; + } + }); });