diff --git a/src/main/index.ts b/src/main/index.ts index 6f5e2085..6f795ac9 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -30,6 +30,7 @@ import { ReviewApplierService } from '@main/services/team/ReviewApplierService'; import { TeamBackupService } from '@main/services/team/TeamBackupService'; import { TeamConfigReader } from '@main/services/team/TeamConfigReader'; import { TeamInboxWriter } from '@main/services/team/TeamInboxWriter'; +import { TeamMcpConfigBuilder } from '@main/services/team/TeamMcpConfigBuilder'; import { resolveInteractiveShellEnv } from '@main/utils/shellEnv'; import { CONTEXT_CHANGED, @@ -746,6 +747,8 @@ function initializeServices(): void { ptyTerminalService = new PtyTerminalService(); teamDataService = new TeamDataService(); teamProvisioningService = new TeamProvisioningService(); + // Startup GC: remove stale MCP config files from previous sessions (best-effort) + void new TeamMcpConfigBuilder().gcStaleConfigs(); void teamDataService .initializeTaskCommentNotificationState() .catch((error: unknown) => @@ -980,6 +983,9 @@ function shutdownServices(): void { teamProvisioningService.stopAllTeams(); } + // Best-effort cleanup of MCP config files owned by this process + void new TeamMcpConfigBuilder().gcOwnConfigs(); + // Sync backup all team data (files are stable after SIGKILL). if (teamBackupService) { teamBackupService.runShutdownBackupSync(); diff --git a/src/main/services/team/TeamMcpConfigBuilder.ts b/src/main/services/team/TeamMcpConfigBuilder.ts index 85947e31..6987729d 100644 --- a/src/main/services/team/TeamMcpConfigBuilder.ts +++ b/src/main/services/team/TeamMcpConfigBuilder.ts @@ -1,9 +1,8 @@ -import { getHomeDir } from '@main/utils/pathDecoder'; +import { getHomeDir, getMcpConfigsBasePath, getMcpServerBasePath } from '@main/utils/pathDecoder'; import { createLogger } from '@shared/utils/logger'; import { execFile } from 'child_process'; import { randomUUID } from 'crypto'; import * as fs from 'fs'; -import * as os from 'os'; import * as path from 'path'; import { atomicWriteAsync } from './atomicWrite'; @@ -17,6 +16,15 @@ const MCP_SERVER_NAME = 'agent-teams'; const logger = createLogger('Service:TeamMcpConfigBuilder'); const USER_MCP_CONFIG_NAME = '.claude.json'; +const MCP_CONFIG_PREFIX = 'agent-teams-mcp-'; +/** + * Stale configs older than this are removed on startup (best-effort). + * 7 days is intentionally long: respawnAfterAuthFailure() reuses saved + * --mcp-config paths, so shorter TTLs risk deleting configs still needed + * by long-running or retrying sessions in other app instances. + */ +const MCP_CONFIG_STALE_MAX_AGE_MS = 7 * 24 * 60 * 60 * 1000; + type McpServerConfig = Record; function isRecord(value: unknown): value is Record { @@ -32,10 +40,20 @@ function isPackagedApp(): boolean { } } +function getAppVersion(): string { + try { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const { app } = require('electron') as typeof import('electron'); + return app.getVersion(); + } catch { + return '0.0.0-dev'; + } +} + /** * In a packaged Electron build the mcp-server bundle lives under * `process.resourcesPath/mcp-server/index.js` (copied via extraResources). - * In dev mode we resolve relative to the workspace root (process.cwd()). + * This is the fallback location when the stable copy is unavailable. */ function getPackagedServerEntry(): string { return path.join(process.resourcesPath, 'mcp-server', 'index.js'); @@ -45,16 +63,16 @@ function getWorkspaceRoot(): string { return process.cwd(); } -function getMcpServerDir(): string { +function getWorkspaceMcpServerDir(): string { return path.join(getWorkspaceRoot(), 'mcp-server'); } function getBuiltServerEntry(): string { - return path.join(getMcpServerDir(), 'dist', 'index.js'); + return path.join(getWorkspaceMcpServerDir(), 'dist', 'index.js'); } function getSourceServerEntry(): string { - return path.join(getMcpServerDir(), 'src', 'index.ts'); + return path.join(getWorkspaceMcpServerDir(), 'src', 'index.ts'); } async function pathExists(targetPath: string): Promise { @@ -66,6 +84,14 @@ async function pathExists(targetPath: string): Promise { } } +/** Check that both index.js and package.json exist in a directory. */ +async function hasValidServerCopy(dir: string): Promise { + return ( + (await pathExists(path.join(dir, 'index.js'))) && + (await pathExists(path.join(dir, 'package.json'))) + ); +} + let _resolvedNodePath: string | undefined; /** @@ -99,12 +125,88 @@ async function resolveNodePath(): Promise { return _resolvedNodePath; } +/** + * For packaged builds, copy the MCP server to a stable, writable location + * under userData so the server runs from a non-FUSE path (fixes AppImage). + * + * Uses a versioned subdirectory + atomic rename to avoid partial state: + * userData/mcp-server//index.js + * userData/mcp-server//package.json + * + * Returns the resolved index.js path (stable copy or resourcesPath fallback). + */ +async function resolvePackagedServerEntry(): Promise { + const fallbackEntry = getPackagedServerEntry(); + if (!isPackagedApp()) return fallbackEntry; + + const appVersion = getAppVersion(); + const baseDir = getMcpServerBasePath(); + const finalDir = path.join(baseDir, appVersion); + const finalEntry = path.join(finalDir, 'index.js'); + + // Reuse existing valid copy + if (await hasValidServerCopy(finalDir)) { + return finalEntry; + } + + // Heal invalid finalDir (partial state from previous crash) + try { + if ((await pathExists(finalDir)) && !(await hasValidServerCopy(finalDir))) { + logger.warn(`Removing invalid MCP server copy at ${finalDir}`); + await fs.promises.rm(finalDir, { recursive: true, force: true }); + } + } catch { + /* best-effort heal */ + } + + try { + const sourceDir = path.join(process.resourcesPath, 'mcp-server'); + if (!(await hasValidServerCopy(sourceDir))) { + logger.warn(`Packaged MCP server missing in resourcesPath: ${sourceDir}`); + return fallbackEntry; + } + + // Atomic: copy to temp dir, then rename to final + const tmpDir = path.join(baseDir, `${appVersion}.tmp-${process.pid}-${randomUUID()}`); + await fs.promises.mkdir(tmpDir, { recursive: true }); + await fs.promises.copyFile(path.join(sourceDir, 'index.js'), path.join(tmpDir, 'index.js')); + await fs.promises.copyFile( + path.join(sourceDir, 'package.json'), + path.join(tmpDir, 'package.json') + ); + + try { + await fs.promises.rename(tmpDir, finalDir); + } catch { + // finalDir appeared between our check and rename (another process won the race) + await fs.promises.rm(tmpDir, { recursive: true, force: true }).catch(() => {}); + if (await hasValidServerCopy(finalDir)) { + logger.info(`Using stable MCP server copy at ${finalDir} (concurrent copy resolved)`); + return finalEntry; + } + // Neither our copy nor the winner's copy is valid — fallback + logger.warn(`Concurrent MCP server copy failed, using resourcesPath fallback`); + return fallbackEntry; + } + + logger.info(`MCP server copied to stable path ${finalDir} (v${appVersion})`); + return finalEntry; + } catch (error) { + logger.warn( + `Failed to copy MCP server to stable path, using resourcesPath fallback: ${ + error instanceof Error ? error.message : String(error) + }` + ); + return fallbackEntry; + } +} + async function resolveMcpLaunchSpec(): Promise { const checked: string[] = []; - // 1. Packaged Electron app — use extraResources bundle + // 1. Packaged Electron app — prefer stable copy, fall back to resourcesPath if (isPackagedApp()) { - const packagedEntry = getPackagedServerEntry(); + const packagedEntry = await resolvePackagedServerEntry(); checked.push(packagedEntry); if (await pathExists(packagedEntry)) { return { @@ -121,7 +223,7 @@ async function resolveMcpLaunchSpec(): Promise { if (await pathExists(sourceEntry)) { return { command: 'pnpm', - args: ['--dir', getMcpServerDir(), 'exec', 'tsx', sourceEntry], + args: ['--dir', getWorkspaceMcpServerDir(), 'exec', 'tsx', sourceEntry], }; } @@ -143,8 +245,11 @@ async function resolveMcpLaunchSpec(): Promise { export class TeamMcpConfigBuilder { async writeConfigFile(_projectPath?: string): Promise { const launchSpec = await resolveMcpLaunchSpec(); - const configDir = path.join(os.tmpdir(), 'claude-team-mcp'); - const configPath = path.join(configDir, `agent-teams-mcp-${randomUUID()}.json`); + const configDir = getMcpConfigsBasePath(); + const configPath = path.join( + configDir, + `${MCP_CONFIG_PREFIX}${process.pid}-${Date.now()}-${randomUUID()}.json` + ); const userServers = await this.readUserMcpServers(); const generatedServers: Record = { [MCP_SERVER_NAME]: { @@ -169,6 +274,69 @@ export class TeamMcpConfigBuilder { return configPath; } + /** Delete a single MCP config file (best-effort). */ + async removeConfigFile(configPath: string): Promise { + try { + await fs.promises.unlink(configPath); + } catch (error) { + const err = error as NodeJS.ErrnoException; + if (err.code !== 'ENOENT') { + logger.warn(`Failed to remove MCP config ${configPath}: ${err.message}`); + } + } + } + + /** Remove config files owned by current process (shutdown best-effort). */ + async gcOwnConfigs(): Promise { + const configDir = getMcpConfigsBasePath(); + const ownPrefix = `${MCP_CONFIG_PREFIX}${process.pid}-`; + try { + const entries = await fs.promises.readdir(configDir); + await Promise.all( + entries + .filter((n) => n.startsWith(ownPrefix) && n.endsWith('.json')) + .map((n) => fs.promises.unlink(path.join(configDir, n)).catch(() => {})) + ); + } catch (error) { + const err = error as NodeJS.ErrnoException; + if (err.code !== 'ENOENT') { + logger.warn(`Failed to GC own MCP configs: ${err.message}`); + } + } + } + + /** + * Remove stale config files older than maxAgeMs (startup GC, best-effort). + * Risk is reduced but not eliminated for multi-instance scenarios: + * respawnAfterAuthFailure() has its own recovery to regenerate deleted configs. + */ + async gcStaleConfigs(maxAgeMs = MCP_CONFIG_STALE_MAX_AGE_MS): Promise { + const configDir = getMcpConfigsBasePath(); + try { + const entries = await fs.promises.readdir(configDir); + await Promise.all( + entries + .filter((n) => n.startsWith(MCP_CONFIG_PREFIX) && n.endsWith('.json')) + .map(async (n) => { + const fullPath = path.join(configDir, n); + try { + const stat = await fs.promises.stat(fullPath); + if (Date.now() - stat.mtimeMs > maxAgeMs) { + await fs.promises.unlink(fullPath); + } + } catch { + /* ignore per-file errors */ + } + }) + ); + } catch (error) { + const err = error as NodeJS.ErrnoException; + if (err.code !== 'ENOENT') { + logger.warn(`Failed to GC stale MCP configs: ${err.message}`); + } + } + } + private async readUserMcpServers(): Promise> { const configPath = path.join(getHomeDir(), USER_MCP_CONFIG_NAME); return this.readMcpServersFromFile(configPath, 'user'); diff --git a/src/main/services/team/TeamProvisioningService.ts b/src/main/services/team/TeamProvisioningService.ts index 69790de0..0267bbd5 100644 --- a/src/main/services/team/TeamProvisioningService.ts +++ b/src/main/services/team/TeamProvisioningService.ts @@ -250,6 +250,8 @@ interface ProvisioningRun { fsPhase: 'waiting_config' | 'waiting_members' | 'waiting_tasks' | 'all_files_found'; waitingTasksSince: number | null; provisioningComplete: boolean; + /** Path to the generated MCP config file for later cleanup. */ + mcpConfigPath: string | null; isLaunch: boolean; leadRelayCapture: { leadName: string; @@ -2642,6 +2644,32 @@ export class TeamProvisioningService { return; } + // Verify --mcp-config still exists; regenerate if deleted (e.g. by stale GC) + const mcpFlagIdx = ctx.args.indexOf('--mcp-config'); + if (mcpFlagIdx !== -1 && mcpFlagIdx + 1 < ctx.args.length) { + const existingConfigPath = ctx.args[mcpFlagIdx + 1]; + try { + await fs.promises.access(existingConfigPath, fs.constants.F_OK); + } catch { + logger.warn(`[${run.teamName}] MCP config ${existingConfigPath} missing, regenerating`); + try { + const newConfigPath = await this.mcpConfigBuilder.writeConfigFile(ctx.cwd); + ctx.args[mcpFlagIdx + 1] = newConfigPath; + run.mcpConfigPath = newConfigPath; + logger.info(`[${run.teamName}] Regenerated MCP config at ${newConfigPath}`); + } catch (regenErr) { + run.authRetryInProgress = false; + const progress = updateProgress(run, 'failed', 'Failed to regenerate MCP config', { + error: regenErr instanceof Error ? regenErr.message : String(regenErr), + cliLogsTail: extractCliLogsFromRun(run), + }); + run.onProgress(progress); + this.cleanupRun(run); + return; + } + } + } + // Respawn with saved context — CLI handles its own auth refresh. let child: ReturnType; try { @@ -2922,6 +2950,7 @@ export class TeamProvisioningService { apiErrorWarningEmitted: false, waitingTasksSince: null, provisioningComplete: false, + mcpConfigPath: null, isLaunch: false, fsPhase: 'waiting_config', leadRelayCapture: null, @@ -2968,6 +2997,7 @@ export class TeamProvisioningService { let mcpConfigPath: string; try { mcpConfigPath = await this.mcpConfigBuilder.writeConfigFile(request.cwd); + run.mcpConfigPath = mcpConfigPath; } catch (error) { this.runs.delete(runId); this.provisioningRunByTeam.delete(request.teamName); @@ -3354,6 +3384,7 @@ export class TeamProvisioningService { apiErrorWarningEmitted: false, waitingTasksSince: null, provisioningComplete: false, + mcpConfigPath: null, isLaunch: true, fsPhase: 'waiting_members', leadRelayCapture: null, @@ -3422,6 +3453,7 @@ export class TeamProvisioningService { let mcpConfigPath: string; try { mcpConfigPath = await this.mcpConfigBuilder.writeConfigFile(request.cwd); + run.mcpConfigPath = mcpConfigPath; } catch (error) { this.runs.delete(runId); this.provisioningRunByTeam.delete(request.teamName); @@ -6438,6 +6470,11 @@ export class TeamProvisioningService { this.emitToolApprovalEvent({ dismissed: true, teamName: run.teamName, runId: run.runId }); run.pendingApprovals.clear(); } + // Clean up the generated MCP config file (best-effort, fire-and-forget) + if (run.mcpConfigPath) { + void this.mcpConfigBuilder.removeConfigFile(run.mcpConfigPath); + run.mcpConfigPath = null; + } // Remove from runs Map to free memory (stdoutBuffer, stderrBuffer, claudeLogLines) this.runs.delete(run.runId); } diff --git a/src/main/utils/pathDecoder.ts b/src/main/utils/pathDecoder.ts index 5b04b068..46b8d7b1 100644 --- a/src/main/utils/pathDecoder.ts +++ b/src/main/utils/pathDecoder.ts @@ -392,8 +392,8 @@ export function getAppDataPath(): string { let appDataBasePathOverride: string | null = null; -export function setAppDataBasePath(p: string): void { - appDataBasePathOverride = p; +export function setAppDataBasePath(p: string | null | undefined): void { + appDataBasePathOverride = p ?? null; } function getAppDataBasePath(): string { @@ -408,3 +408,21 @@ function getAppDataBasePath(): string { return path.join(getHomeDir(), '.claude-agent-teams-ui'); } } + +/** + * Directory for per-team MCP config JSON files. + * Stored in app's userData so they persist across sessions and are + * accessible by Claude CLI subprocess on all platforms (including AppImage). + */ +export function getMcpConfigsBasePath(): string { + return path.join(getAppDataBasePath(), 'mcp-configs'); +} + +/** + * Directory for the stable MCP server bundle copy (packaged builds). + * Versioned subdirectories contain the copied index.js + package.json + * so the server runs from a writable, non-FUSE location. + */ +export function getMcpServerBasePath(): string { + return path.join(getAppDataBasePath(), 'mcp-server'); +} diff --git a/test/main/services/team/TeamMcpConfigBuilder.test.ts b/test/main/services/team/TeamMcpConfigBuilder.test.ts index b18e40b9..124aff83 100644 --- a/test/main/services/team/TeamMcpConfigBuilder.test.ts +++ b/test/main/services/team/TeamMcpConfigBuilder.test.ts @@ -1,4 +1,4 @@ -import { afterEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; @@ -13,13 +13,22 @@ vi.mock('@main/utils/pathDecoder', async (importOriginal) => { }; }); +import { setAppDataBasePath } from '@main/utils/pathDecoder'; import { TeamMcpConfigBuilder } from '@main/services/team/TeamMcpConfigBuilder'; describe('TeamMcpConfigBuilder', () => { const createdPaths: string[] = []; const createdDirs: string[] = []; + let tempAppData: string; + + beforeEach(() => { + tempAppData = fs.mkdtempSync(path.join(os.tmpdir(), 'team-mcp-appdata-')); + createdDirs.push(tempAppData); + setAppDataBasePath(tempAppData); + }); afterEach(() => { + setAppDataBasePath(null); for (const filePath of createdPaths.splice(0)) { try { fs.rmSync(filePath, { force: true }); @@ -37,6 +46,30 @@ describe('TeamMcpConfigBuilder', () => { mockHomeDir = ''; }); + // ── Config storage ── + + it('writes config to userData/mcp-configs/, not the system default tmp', async () => { + const builder = new TeamMcpConfigBuilder(); + const configPath = await builder.writeConfigFile(); + createdPaths.push(configPath); + + const expectedDir = path.join(tempAppData, 'mcp-configs'); + expect(configPath.startsWith(expectedDir)).toBe(true); + // Config must NOT be in the old hardcoded location + expect(configPath).not.toContain('claude-team-mcp'); + }); + + it('config filename contains pid, timestamp, and uuid', async () => { + const builder = new TeamMcpConfigBuilder(); + const configPath = await builder.writeConfigFile(); + createdPaths.push(configPath); + + const filename = path.basename(configPath); + expect(filename).toMatch( + new RegExp(`^agent-teams-mcp-${process.pid}-\\d+-[0-9a-f-]+\\.json$`) + ); + }); + it('prefers the source MCP entry when workspace source is available', async () => { const builder = new TeamMcpConfigBuilder(); @@ -210,4 +243,82 @@ describe('TeamMcpConfigBuilder', () => { expect(Object.keys(parsed.mcpServers)).toEqual(['agent-teams']); }); + + // ── Cleanup: removeConfigFile ── + + it('removeConfigFile deletes the file', async () => { + const builder = new TeamMcpConfigBuilder(); + const configPath = await builder.writeConfigFile(); + + expect(fs.existsSync(configPath)).toBe(true); + await builder.removeConfigFile(configPath); + expect(fs.existsSync(configPath)).toBe(false); + }); + + it('removeConfigFile ignores ENOENT', async () => { + const builder = new TeamMcpConfigBuilder(); + const bogusPath = path.join(tempAppData, 'nonexistent.json'); + + // Should not throw + await builder.removeConfigFile(bogusPath); + }); + + // ── Cleanup: gcOwnConfigs ── + + it('gcOwnConfigs removes only files owned by current pid', async () => { + const configDir = path.join(tempAppData, 'mcp-configs'); + fs.mkdirSync(configDir, { recursive: true }); + + const ownFile = path.join(configDir, `agent-teams-mcp-${process.pid}-12345-abc.json`); + const otherFile = path.join(configDir, `agent-teams-mcp-99999-12345-xyz.json`); + fs.writeFileSync(ownFile, '{}'); + fs.writeFileSync(otherFile, '{}'); + + const builder = new TeamMcpConfigBuilder(); + await builder.gcOwnConfigs(); + + expect(fs.existsSync(ownFile)).toBe(false); + expect(fs.existsSync(otherFile)).toBe(true); + }); + + // ── Cleanup: gcStaleConfigs ── + + it('gcStaleConfigs removes files older than TTL', async () => { + const configDir = path.join(tempAppData, 'mcp-configs'); + fs.mkdirSync(configDir, { recursive: true }); + + const oldFile = path.join(configDir, `agent-teams-mcp-999-1-old.json`); + fs.writeFileSync(oldFile, '{}'); + // Set mtime to 2 days ago + const twoDaysAgo = new Date(Date.now() - 2 * 24 * 60 * 60 * 1000); + fs.utimesSync(oldFile, twoDaysAgo, twoDaysAgo); + + const freshFile = path.join(configDir, `agent-teams-mcp-999-2-fresh.json`); + fs.writeFileSync(freshFile, '{}'); + + const builder = new TeamMcpConfigBuilder(); + await builder.gcStaleConfigs(24 * 60 * 60 * 1000); // 24h TTL + + expect(fs.existsSync(oldFile)).toBe(false); + expect(fs.existsSync(freshFile)).toBe(true); + }); + + it('gcStaleConfigs does not remove fresh files', async () => { + const configDir = path.join(tempAppData, 'mcp-configs'); + fs.mkdirSync(configDir, { recursive: true }); + + const freshFile = path.join(configDir, `agent-teams-mcp-1-1234-abc.json`); + fs.writeFileSync(freshFile, '{}'); + + const builder = new TeamMcpConfigBuilder(); + await builder.gcStaleConfigs(24 * 60 * 60 * 1000); + + expect(fs.existsSync(freshFile)).toBe(true); + }); + + it('gcStaleConfigs handles empty or missing directory gracefully', async () => { + const builder = new TeamMcpConfigBuilder(); + // Should not throw when directory doesn't exist + await builder.gcStaleConfigs(); + }); });