5.8 KiB
5.8 KiB
Fix: MCP First-Prompt Readiness + Exit Cleanup
Background
Two reproducible issues:
- MCP not ready on first prompt: MCP config is passed via
context_serversin the chat request. After config sync, the ACP engine's mcp-proxy child process hasn't finished connecting/initializing before the prompt is sent. - MCP proxy not cleaned up on exit:
persistentMcpBridge.stop()is async but was called fire-and-forget incleanupAllProcesses(). 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
- All stdio servers go through bridge (not just persistent ones). This ensures even temporary servers from
context_serversare ready before the first prompt. - 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.
- Remote URL servers bypass bridge: SSE/HTTP servers (
{ url: "..." }) are passed through unchanged. PersistentMcpBridge.start()is safe to call repeatedly: It internally callsstop()first if already running (bridge.ts line 84-87).- 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 awaitspersistentMcpBridge.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
- MCP first prompt: Send a chat request with
context_servers. Logs should showPersistentMcpBridge 已使用更新配置重启before the prompt is sent. First prompt should have access to all MCP tools. - Exit cleanup: Start app with MCP servers, quit app.
ps aux | grep mcpshould show no orphan processes. Logs should showCleanup complete, exiting. - 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 ->
isCleaningUpguard prevents duplicate cleanup