# Fix: MCP First-Prompt Readiness + Exit Cleanup ## Background Two reproducible issues: 1. **MCP not ready on first prompt**: MCP config is passed via `context_servers` in the chat request. After config sync, the ACP engine's mcp-proxy child process hasn't finished connecting/initializing before the prompt is sent. 2. **MCP proxy not cleaned up on exit**: `persistentMcpBridge.stop()` is async but was called fire-and-forget in `cleanupAllProcesses()`. Electron could exit before child processes are killed. ## Root Cause ### Issue 1: MCP Timing Race **Before fix (first chat request with context_servers):** ``` POST /computer/chat { agent_config: { context_servers: {...} } } | ensureEngineForRequest() 1. Extract context_servers from request 2. syncMcpConfigToProxyAndReload() <-- Only saves config to DB 3. getAgentMcpConfig() <-- Returns stdio mcp-proxy config (command+args) 4. getOrCreateEngine() -> engine.init() -> spawn ACP binary | acpEngine.chat() 5. createSession({cwd}) <-- ACP newSession: spawns mcp-proxy child mcp-proxy needs: start -> parse config -> connect servers -> listTools -> aggregate 6. promptAsync() <-- Sent immediately! mcp-proxy not ready! ``` ### Issue 2: Exit Cleanup Not Awaited `cleanupAllProcesses()` was a sync `void` function. `agentService.destroy()` and `mcpProxyManager.cleanup()` returned Promises that were never awaited. ## Solution ### Fix 1: Pre-start MCP Servers via PersistentMcpBridge **Strategy**: When `syncMcpConfigToProxyAndReload()` is called, restart `PersistentMcpBridge` and load ALL stdio servers. This blocks until all servers pass the `listTools()` readiness check. Then `getAgentMcpConfig()` returns bridge HTTP URLs instead of stdio configs. ACP connects via HTTP instantly. **After fix:** ``` POST /computer/chat { agent_config: { context_servers: {...} } } | ensureEngineForRequest() 1. Extract context_servers from request 2. syncMcpConfigToProxyAndReload() -> Save config to DB -> Restart PersistentMcpBridge with all servers <-- BLOCKS until ready 3. getAgentMcpConfig() <-- Returns bridge HTTP URLs (servers running!) 4. getOrCreateEngine() -> engine.init() -> spawn ACP binary (with HTTP URLs) | acpEngine.chat() 5. createSession({mcpServers: [HTTP URL]}) <-- ACP connects via HTTP -> instant! 6. promptAsync() <-- MCP ready! ``` ### Fix 2: Async Cleanup with Timeout `cleanupAllProcesses()` is now `async`. `before-quit` uses `e.preventDefault()` + `Promise.race()` with a 5-second hard timeout, then calls `app.exit(0)`. ## Files Changed | File | Changes | |------|---------| | `src/main/services/packages/mcp.ts` | Added `getAllStdioServers()`; `start()` launches all stdio servers; `getAgentMcpConfig()` prefers bridge URL; `syncMcpConfigToProxyAndReload()` restarts bridge; `cleanup()` now async | | `src/main/services/engines/unifiedAgent.ts` | `AgentConfig.mcpServers` type supports URL entries | | `src/main/services/engines/acp/acpEngine.ts` | `toAcpMcpServer()` handles URL; qimingcode config injection handles URL | | `src/main/main.ts` | Async cleanup + before-quit timeout + simplified window-all-closed | | `src/main/services/packages/mcp.test.ts` | Fixed test isolation; updated expectations; added 17 new tests | ## Key Design Decisions 1. **All stdio servers go through bridge** (not just persistent ones). This ensures even temporary servers from `context_servers` are ready before the first prompt. 2. **Bridge URL preference with stdio fallback**: If bridge is running and a server is healthy, use the HTTP URL. If bridge isn't running or a specific server failed, fall back to raw stdio config. 3. **Remote URL servers bypass bridge**: SSE/HTTP servers (`{ url: "..." }`) are passed through unchanged. 4. **`PersistentMcpBridge.start()` is safe to call repeatedly**: It internally calls `stop()` first if already running (bridge.ts line 84-87). 5. **5-second hard timeout on exit**: Prevents the app from hanging if cleanup takes too long. ## Test Coverage (17 new tests) ### `getAllStdioServers` (3 tests) - Returns all stdio servers (persistent + temporary) - Excludes remote URL servers - Returns empty for empty config ### Bridge URL Priority (3 tests) - All stdio servers use bridge URL when bridge is running - Partial fallback: healthy servers get URL, unhealthy get stdio config - Remote servers are unaffected by bridge ### `syncMcpConfigToProxyAndReload` (3 tests) - Restarts PersistentMcpBridge after config sync - Skips bridge restart for empty config - Bridge restart failure doesn't block sync flow ### Cleanup (2 tests) - `cleanup()` properly awaits `persistentMcpBridge.stop()` - Bridge stop failure doesn't throw ### `start()` (2 tests) - Starts bridge with all stdio servers (not just persistent) - Skips bridge when only remote servers configured ### Real-world Scenarios (4 tests, from production logs) - Extracts stdio server from uvx bridge entry (markdownify) - Extracts remote SSE server from bridge entry - Handles npx-based servers with allowTools - Full sync with mixed config (uvx + npx + remote URL + default chrome-devtools) ## Verification 1. **MCP first prompt**: Send a chat request with `context_servers`. Logs should show `PersistentMcpBridge 已使用更新配置重启` before the prompt is sent. First prompt should have access to all MCP tools. 2. **Exit cleanup**: Start app with MCP servers, quit app. `ps aux | grep mcp` should show no orphan processes. Logs should show `Cleanup complete, exiting`. 3. **Edge cases**: - MCP server connection failure -> bridge retries/falls back to stdio config - No MCP config -> no blocking - Rapid repeated requests -> bridge handles re-entry via stop-then-start - Multiple quit events -> `isCleaningUp` guard prevents duplicate cleanup