fix(client): prevent race conditions when starting multiple sessions concurrently

Author: Fu-Jie

nodejs/src/client.ts

@@ -232,6 +232,8 @@ export class CopilotClient {
      * Parse CLI URL into host and port
      * Supports formats: "host:port", "http://host:port", "https://host:port", or just "port"
      */
+    private startPromise: Promise<void> | null = null;
+
     private parseCliUrl(url: string): { host: string; port: number } {
         // Remove protocol if present
         let cleanUrl = url.replace(/^https?:\/\//, "");
@@ -282,25 +284,34 @@ export class CopilotClient {
             return;
         }
 
-        this.state = "connecting";
+        if (this.startPromise) {
+            return this.startPromise;
+        }
 
-        try {
-            // Only start CLI server process if not connecting to external server
-            if (!this.isExternalServer) {
-                await this.startCLIServer();
-            }
+        this.startPromise = (async () => {
+            this.state = "connecting";
+
+            try {
+                // Only start CLI server process if not connecting to external server
+                if (!this.isExternalServer) {
+                    await this.startCLIServer();
+                }
 
-            // Connect to the server
-            await this.connectToServer();
+                // Connect to the server
+                await this.connectToServer();
 
-            // Verify protocol version compatibility
-            await this.verifyProtocolVersion();
+                // Verify protocol version compatibility
+                await this.verifyProtocolVersion();
 
-            this.state = "connected";
-        } catch (error) {
-            this.state = "error";
-            throw error;
-        }
+                this.state = "connected";
+            } catch (error) {
+                this.state = "error";
+                this.startPromise = null;
+                throw error;
+            }
+        })();
+
+        return this.startPromise;
     }
 
     /**
@@ -403,6 +414,7 @@ export class CopilotClient {
         }
 
         this.state = "disconnected";
+        this.startPromise = null;
         this.actualPort = null;
         this.stderrBuffer = "";
         this.processExitPromise = null;
@@ -475,6 +487,7 @@ export class CopilotClient {
         }
 
         this.state = "disconnected";
+        this.startPromise = null;
         this.actualPort = null;
         this.stderrBuffer = "";
         this.processExitPromise = null;

nodejs/test/e2e/session.test.ts

@@ -130,11 +130,7 @@ describe("Sessions", async () => {
         expect(functionNames).not.toContain("view");
     });
 
-    // TODO: This test shows there's a race condition inside client.ts. If createSession is called
-    // concurrently and autoStart is on, it may start multiple child processes. This needs to be fixed.
-    // Right now it manifests as being unable to delete the temp directories during afterAll even though
-    // we stopped all the clients (one or more child processes were left orphaned).
-    it.skip("should handle multiple concurrent sessions", async () => {
+    it("should handle multiple concurrent sessions", async () => {
         const [s1, s2, s3] = await Promise.all([
             client.createSession({ onPermissionRequest: approveAll }),
             client.createSession({ onPermissionRequest: approveAll }),

python/copilot/client.py

@@ -201,6 +201,7 @@ def __init__(self, options: CopilotClientOptions | None = None):
         self._process: subprocess.Popen | None = None
         self._client: JsonRpcClient | None = None
         self._state: ConnectionState = "disconnected"
+        self._start_lock = asyncio.Lock()
         self._sessions: dict[str, CopilotSession] = {}
         self._sessions_lock = threading.Lock()
         self._models_cache: list[ModelInfo] | None = None
@@ -281,39 +282,40 @@ async def start(self) -> None:
             >>> await client.start()
             >>> # Now ready to create sessions
         """
-        if self._state == "connected":
-            return
+        async with self._start_lock:
+            if self._state == "connected":
+                return
 
-        self._state = "connecting"
+            self._state = "connecting"
 
-        try:
-            # Only start CLI server process if not connecting to external server
-            if not self._is_external_server:
-                await self._start_cli_server()
-
-            # Connect to the server
-            await self._connect_to_server()
-
-            # Verify protocol version compatibility
-            await self._verify_protocol_version()
-
-            self._state = "connected"
-        except ProcessExitedError as e:
-            # Process exited with error - reraise as RuntimeError with stderr
-            self._state = "error"
-            raise RuntimeError(str(e)) from None
-        except Exception as e:
-            self._state = "error"
-            # Check if process exited and capture any remaining stderr
-            if self._process and hasattr(self._process, "poll"):
-                return_code = self._process.poll()
-                if return_code is not None and self._client:
-                    stderr_output = self._client.get_stderr_output()
-                    if stderr_output:
-                        raise RuntimeError(
-                            f"CLI process exited with code {return_code}\nstderr: {stderr_output}"
-                        ) from e
-            raise
+            try:
+                # Only start CLI server process if not connecting to external server
+                if not self._is_external_server:
+                    await self._start_cli_server()
+
+                # Connect to the server
+                await self._connect_to_server()
+
+                # Verify protocol version compatibility
+                await self._verify_protocol_version()
+
+                self._state = "connected"
+            except ProcessExitedError as e:
+                # Process exited with error - reraise as RuntimeError with stderr
+                self._state = "error"
+                raise RuntimeError(str(e)) from None
+            except Exception as e:
+                self._state = "error"
+                # Check if process exited and capture any remaining stderr
+                if self._process and hasattr(self._process, "poll"):
+                    return_code = self._process.poll()
+                    if return_code is not None and self._client:
+                        stderr_output = self._client.get_stderr_output()
+                        if stderr_output:
+                            raise RuntimeError(
+                                f"CLI process exited with code {return_code}\nstderr: {stderr_output}"
+                            ) from e
+                raise
 
     async def stop(self) -> None:
         """

python/e2e/test_session.py

@@ -127,7 +127,7 @@ async def test_should_create_a_session_with_excludedTools(self, ctx: E2ETestCont
     # is called concurrently and autoStart is on, it may start multiple child processes.
     # This needs to be fixed. Right now it manifests as being unable to delete the temp
     # directories during afterAll even though we stopped all the clients.
-    @pytest.mark.skip(reason="Known race condition - see TypeScript test")
+    # @pytest.mark.skip
     async def test_should_handle_multiple_concurrent_sessions(self, ctx: E2ETestContext):
         import asyncio