Merge branch 'worktree-fix+mcp-config-storage' into dev

src/main/index.ts

@@ -31,6 +31,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,
@@ -759,6 +760,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) =>
@@ -1008,6 +1011,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();

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<string, unknown>;
 
 function isRecord(value: unknown): value is Record<string, unknown> {
@@ -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<boolean> {
@@ -66,6 +84,14 @@ async function pathExists(targetPath: string): Promise<boolean> {
   }
 }
 
+/** Check that both index.js and package.json exist in a directory. */
+async function hasValidServerCopy(dir: string): Promise<boolean> {
+  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<string> {
   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/<appVersion>/index.js
+ *   userData/mcp-server/<appVersion>/package.json
+ *
+ * Returns the resolved index.js path (stable copy or resourcesPath fallback).
+ */
+async function resolvePackagedServerEntry(): Promise<string> {
+  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<McpLaunchSpec> {
   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<McpLaunchSpec> {
   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<McpLaunchSpec> {
 export class TeamMcpConfigBuilder {
   async writeConfigFile(_projectPath?: string): Promise<string> {
     const launchSpec = await resolveMcpLaunchSpec();
-    const configDir = path.join(os.tmpdir(), 'claude-team-mcp');
+    const configDir = getMcpConfigsBasePath();
-    const configPath = path.join(configDir, `agent-teams-mcp-${randomUUID()}.json`);
+    const configPath = path.join(
+      configDir,
+      `${MCP_CONFIG_PREFIX}${process.pid}-${Date.now()}-${randomUUID()}.json`
+    );
     const userServers = await this.readUserMcpServers();
     const generatedServers: Record<string, McpServerConfig> = {
       [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<void> {
+    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<void> {
+    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<void> {
+    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<Record<string, McpServerConfig>> {
     const configPath = path.join(getHomeDir(), USER_MCP_CONFIG_NAME);
     return this.readMcpServersFromFile(configPath, 'user');

src/main/services/team/TeamProvisioningService.ts

@@ -261,6 +261,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;
@@ -2773,6 +2775,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<typeof spawn>;
     try {
@@ -3053,6 +3081,7 @@ export class TeamProvisioningService {
         apiErrorWarningEmitted: false,
         waitingTasksSince: null,
         provisioningComplete: false,
+        mcpConfigPath: null,
         isLaunch: false,
         fsPhase: 'waiting_config',
         leadRelayCapture: null,
@@ -3101,6 +3130,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);
@@ -3487,6 +3517,7 @@ export class TeamProvisioningService {
         apiErrorWarningEmitted: false,
         waitingTasksSince: null,
         provisioningComplete: false,
+        mcpConfigPath: null,
         isLaunch: true,
         fsPhase: 'waiting_members',
         leadRelayCapture: null,
@@ -3557,6 +3588,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);
@@ -6881,6 +6913,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);
   }

src/main/utils/pathDecoder.ts

@@ -416,8 +416,8 @@ export function getAppDataPath(): string {
 
 let appDataBasePathOverride: string | null = null;
 
-export function setAppDataBasePath(p: string): void {
+export function setAppDataBasePath(p: string | null | undefined): void {
-  appDataBasePathOverride = p;
+  appDataBasePathOverride = p ?? null;
 }
 
 function getAppDataBasePath(): string {
@@ -432,3 +432,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');
+}

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();
+  });
 });