fix: prevent orphan ACP processes on tab close and clean up MCP subprocesses on shutdown#2662
Conversation
📋 Review SummaryThis PR addresses a critical issue where MCP server subprocesses were not being cleaned up when the ACP (Agent Client Protocol) connection closes, leading to orphan processes persisting after IDE disconnection. The implementation adds proper cleanup calls to both the signal handler and normal exit paths in ACP mode. The changes are minimal, focused, and well-tested. 🔍 General Feedback
🎯 Specific Feedback🟢 Medium
🔵 Low
✅ Highlights
|
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
Context: relationship with PR #2472This PR is a follow-up to #2472. Together they fix the full chain of orphan process issues. Here's how the four bugs map across the two PRs:
User-visible symptoms:
Manually verified on macOS with VSCode 0.13.0: close tab → processes exit → reopen tab → connects successfully. |
Follow-up idea: make
|
yiliang114
left a comment
There was a problem hiding this comment.
LGTM.
This follow-up fixes the orphan-process issue from both sides:
- the VSCode webview now disconnects the ACP agent on panel/view dispose
- the CLI now runs exit cleanup on both signal shutdown and normal ACP disconnect
The root-cause analysis and fix direction look correct to me, and the added ACP shutdown tests cover the critical signal-handling path well.
|
Closes #2553 |
…esses PR #2472 added SIGTERM/SIGINT handlers in runAcpAgent() that destroy stdin/stdout streams, allowing the ACP connection to close. However, it did not call runExitCleanup() to terminate child processes (MCP servers, etc.) spawned by the CLI. This caused orphan subprocesses to persist after the IDE disconnects, especially noticeable in VSCode with multiple tabs. Changes: - acpAgent.ts: Call runExitCleanup() in the shutdown signal handler before process.exit(0), ensuring MCP server subprocesses are killed - gemini.tsx: Call runExitCleanup() + process.exit(0) after runAcpAgent() returns normally, matching the cleanup pattern used by other non-interactive exit paths - Add 4 unit tests covering SIGTERM cleanup, SIGINT cleanup, idempotent shutdown, and cleanup-failure resilience Closes #1884
…cesses The panel onDidDispose and webviewView onDidDispose callbacks were not calling agentManager.disconnect(), so closing a chat tab or sidebar view left the CLI child process running as an orphan. Only the WebViewProvider.dispose() method (called on extension deactivation) had the disconnect call. Add agentManager.disconnect() to both dispose callbacks so the CLI process receives SIGTERM when the user closes the tab.
761093a to
4aff9d7
Compare
Mingholy
left a comment
There was a problem hiding this comment.
Tested with Idea. Works as exepcted.


Summary
Follow-up to #2472. Fixes orphan ACP child processes that persist after the IDE disconnects, caused by two independent bugs — one in the CLI, one in the VSCode extension.
Root Cause
Three missing pieces across CLI and extension:
WebViewProvider's panelonDidDisposeand sidebar viewonDidDisposecallbacks never calledagentManager.disconnect(). Closing a chat tab did not send SIGTERM to the CLI process at all.shutdownHandlerinrunAcpAgent()destroys stdin/stdout but never callsrunExitCleanup(), so MCP server subprocesses spawned by the CLI are never terminated.runAcpAgent()returns normally (connection closed gracefully),main()returns without callingrunExitCleanup()orprocess.exit(), leaving the Node.js event loop alive due to active child processes.Changes
WebViewProvider.ts(VSCode extension)this.agentManager.disconnect()in the panel dispose callback (editor tab close)this.agentManager.disconnect()in thewebviewView.onDidDisposecallback (sidebar view close)acpAgent.ts(CLI)runExitCleanupfrom cleanup utilitiesrunExitCleanup()in the SIGTERM/SIGINT shutdown handler, withdebugLogger.erroron failure, followed byprocess.exit(0)in.finally()to guarantee exitgemini.tsx(CLI)runAcpAgent()returns, callrunExitCleanup()+process.exit(0), matching the cleanup pattern used by other non-interactive exit pathsacpAgent.test.ts(new, CLI)Verification
Manually verified on macOS with VSCode 0.13.0 extension:
kill -TERM <pid>→ process exits cleanly (previously ignored SIGTERM)Closes #1884
Closes #2433
Closes #2553
Closes #2665