Avoid Windows in-process teardown deadlock

stephentoub · stephentoub · commit 5e60dadac2ee · 2026-07-15T19:26:37.000-04:00
Do not retry removal of the in-process runtime's session home while its Vitest worker still owns a locked session database. Retrying until the hook timeout prevents the worker from exiting and releasing the lock.

Co-authored-by: Copilot App &lt;223556219+Copilot@users.noreply.github.com&gt;

Copilot-Session: 6e902b9a-5527-4a32-a5a3-e0bf5bfef3f7
diff --git a/.github/workflows/nodejs-sdk-tests.yml b/.github/workflows/nodejs-sdk-tests.yml
@@ -10,19 +10,19 @@ on:
   pull_request:
     types: [opened, synchronize, reopened, ready_for_review]
     paths:
-      - "nodejs/**"
-      - "test/**"
-      - ".github/workflows/nodejs-sdk-tests.yml"
-      - "!nodejs/scripts/**"
-      - "!**/*.md"
-      - "!**/LICENSE*"
-      - "!**/.gitignore"
-      - "!**/.editorconfig"
-      - "!**/*.png"
-      - "!**/*.jpg"
-      - "!**/*.jpeg"
-      - "!**/*.gif"
-      - "!**/*.svg"
+      - 'nodejs/**'
+      - 'test/**'
+      - '.github/workflows/nodejs-sdk-tests.yml'
+      - '!nodejs/scripts/**'
+      - '!**/*.md'
+      - '!**/LICENSE*'
+      - '!**/.gitignore'
+      - '!**/.editorconfig'
+      - '!**/*.png'
+      - '!**/*.jpg'
+      - '!**/*.jpeg'
+      - '!**/*.gif'
+      - '!**/*.svg'
   workflow_dispatch:
   merge_group:
 
@@ -38,8 +38,8 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        os: [windows-latest]
-        transport: ["inprocess"]
+        os: [ubuntu-latest, macos-latest, windows-latest]
+        transport: ["default", "inprocess"]
     runs-on: ${{ matrix.os }}
     defaults:
       run:
@@ -80,38 +80,8 @@ jobs:
         if: matrix.transport == 'inprocess'
         run: |
           echo "COPILOT_SDK_DEFAULT_CONNECTION=inprocess" >> "$GITHUB_ENV"
-          echo "COPILOT_SDK_TEST_DIAGNOSTICS=1" >> "$GITHUB_ENV"
 
       - name: Run Node.js SDK tests
         env:
           COPILOT_HMAC_KEY: ${{ secrets.COPILOT_DEVELOPER_CLI_INTEGRATION_HMAC_KEY }}
-          COPILOT_SDK_PRESERVE_TEST_HOME: "1"
-        run: |
-          pwsh.exe -NoProfile -Command '
-            while ($true) {
-              $cpu = [math]::Round((Get-Counter "\Processor(_Total)\% Processor Time").CounterSamples.CookedValue)
-              $memory = [math]::Round((Get-Counter "\Memory\Available MBytes").CounterSamples.CookedValue)
-              $nodeCount = @(Get-Process -Name node -ErrorAction SilentlyContinue).Count
-              Write-Host "[resource-diagnostic] $(Get-Date -Format o) cpu=$cpu availableMb=$memory nodeProcesses=$nodeCount"
-              Start-Sleep -Seconds 5
-            }
-          ' &
-          monitor_pid=$!
-          trap 'kill "$monitor_pid"' EXIT
-          npm test
-
-      - name: Collect runtime diagnostics
-        if: failure()
-        shell: pwsh
-        run: |
-          $paths = Get-ChildItem $env:TEMP -Directory -Filter "copilot-test-home-*"
-          if ($paths) {
-            Compress-Archive -Path $paths.FullName -DestinationPath runtime-diagnostics.zip
-          }
-
-      - name: Upload runtime diagnostics
-        if: failure()
-        uses: actions/upload-artifact@v4
-        with:
-          name: nodejs-runtime-diagnostics
-          path: nodejs/runtime-diagnostics.zip
+        run: npm test
diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts
@@ -121,14 +121,6 @@ async function withTimeout<T>(promise: Promise<T>, timeoutMs: number, message: s
     }
 }
 
-function logTestDiagnostic(message: string, startMs?: number): void {
-    if (process.env.COPILOT_SDK_TEST_DIAGNOSTICS !== "1") {
-        return;
-    }
-    const elapsed = startMs === undefined ? "" : ` elapsed=${Date.now() - startMs}ms`;
-    process.stderr.write(`[copilot-sdk-diagnostic pid=${process.pid}] ${message}${elapsed}\n`);
-}
-
 async function waitForChildExit(child: ChildProcess, timeoutMs: number): Promise<boolean> {
     if (child.exitCode != null || child.signalCode != null) {
         return true;
@@ -885,26 +877,21 @@ export class CopilotClient {
             return;
         }
 
-        const startMs = Date.now();
-        logTestDiagnostic(`client.start begin transport=${this.connectionConfig.kind}`);
         this.state = "connecting";
 
         try {
             // Only start CLI server process if not connecting to external server
             if (this.connectionConfig.kind === "inprocess") {
                 await this.startInProcessFfi();
-                logTestDiagnostic("client.start FFI host ready", startMs);
             } else if (!this.isExternalServer) {
                 await this.startCLIServer();
             }
 
             // Connect to the server
             await this.connectToServer();
-            logTestDiagnostic("client.start JSON-RPC connected", startMs);
 
             // Verify protocol version compatibility
             await this.verifyProtocolVersion();
-            logTestDiagnostic("client.start protocol verified", startMs);
 
             // If a session filesystem provider was configured, register it
             if (this.sessionFsConfig) {
@@ -924,10 +911,8 @@ export class CopilotClient {
             }
 
             this.state = "connected";
-            logTestDiagnostic("client.start complete", startMs);
         } catch (error) {
             this.state = "error";
-            logTestDiagnostic("client.start failed", startMs);
             throw error;
         }
     }
@@ -957,12 +942,10 @@ export class CopilotClient {
      * ```
      */
     async stop(): Promise<Error[]> {
-        const stopMs = Date.now();
         const errors: Error[] = [];
 
         // Disconnect all active sessions with retry logic
         const activeSessions = [...this.sessions.values()];
-        logTestDiagnostic(`client.stop begin sessions=${activeSessions.length}`);
         // TEMPORARY: over the in-process (FFI) transport the runtime shares this
         // process, so a turn still running when the runtime disposes the session
         // can leave that session's SQLite session.db handle open — it isn't
@@ -976,7 +959,6 @@ export class CopilotClient {
         // may still resume. Remove once the runtime cleans up fully on shutdown.
         if (this.connectionConfig.kind === "inprocess") {
             await Promise.allSettled(activeSessions.map((session) => session.abort()));
-            logTestDiagnostic("client.stop session aborts settled", stopMs);
         }
         for (const session of activeSessions) {
             const sessionId = session.sessionId;
@@ -1007,7 +989,6 @@ export class CopilotClient {
                 );
             }
         }
-        logTestDiagnostic("client.stop session disconnects complete", stopMs);
         for (const session of activeSessions) {
             session._markDisconnected();
         }
@@ -1042,7 +1023,6 @@ export class CopilotClient {
                 );
             }
         }
-        logTestDiagnostic("client.stop runtime shutdown stage complete", stopMs);
 
         // Close connection. Suppress writer failures first: tearing down the
         // transport can reject an in-flight server→client response write with
@@ -1066,7 +1046,6 @@ export class CopilotClient {
             this._rpc = null;
             this._internalRpc = null;
         }
-        logTestDiagnostic("client.stop JSON-RPC disposed", stopMs);
 
         // Clear models cache
         this.modelsCache = null;
@@ -1125,9 +1104,7 @@ export class CopilotClient {
             const host = this.ffiHost;
             this.ffiHost = null;
             try {
-                logTestDiagnostic("client.stop FFI host dispose begin", stopMs);
                 host.dispose();
-                logTestDiagnostic("client.stop FFI host dispose complete", stopMs);
             } catch (error) {
                 errors.push(
                     new Error(
@@ -1145,7 +1122,6 @@ export class CopilotClient {
         this.runtimePort = null;
         this.stderrBuffer = "";
         this.processExitPromise = null;
-        logTestDiagnostic(`client.stop complete errors=${errors.length}`, stopMs);
 
         return errors;
     }
@@ -1440,11 +1416,8 @@ export class CopilotClient {
     }
 
     async createSession(config: SessionConfig): Promise<CopilotSession> {
-        const createMs = Date.now();
-        logTestDiagnostic(`client.createSession begin connected=${this.connection !== null}`);
         if (!this.connection) {
             await this.start();
-            logTestDiagnostic("client.createSession client started", createMs);
         }
 
         config = { ...this.configDefaultsForMode(), ...config };
@@ -1615,7 +1588,6 @@ export class CopilotClient {
                 expAssignments: config.expAssignments,
                 enableManagedSettings: config.enableManagedSettings,
             });
-            logTestDiagnostic("client.createSession RPC complete", createMs);
 
             const {
                 sessionId: returnedSessionId,
@@ -1657,7 +1629,6 @@ export class CopilotClient {
             throw e;
         }
 
-        logTestDiagnostic("client.createSession complete", createMs);
         return session;
     }
 
@@ -2589,8 +2560,6 @@ export class CopilotClient {
 
     /** Starts the in-process FFI runtime with SDK-managed typed options. */
     private async startInProcessFfi(): Promise<void> {
-        const startMs = Date.now();
-        logTestDiagnostic("client.startInProcessFfi begin");
         const entrypoint = this.resolveCliPathForFfi();
         // Load the FFI host lazily so the native `koffi` addon (and its
         // platform-specific `koffi.node`) is only loaded on the in-process path;
@@ -2634,7 +2603,6 @@ export class CopilotClient {
         );
         this.ffiHost = host;
         await host.start();
-        logTestDiagnostic("client.startInProcessFfi complete", startMs);
     }
 
     /**
diff --git a/nodejs/src/ffiRuntimeHost.ts b/nodejs/src/ffiRuntimeHost.ts
@@ -28,14 +28,6 @@ const SYMBOL_PREFIX = "copilot_runtime_";
 // connection is open (see start()); the exact interval is irrelevant.
 const KEEP_ALIVE_INTERVAL_MS = 1 << 30;
 
-function logTestDiagnostic(message: string, startMs?: number): void {
-    if (process.env.COPILOT_SDK_TEST_DIAGNOSTICS !== "1") {
-        return;
-    }
-    const elapsed = startMs === undefined ? "" : ` elapsed=${Date.now() - startMs}ms`;
-    process.stderr.write(`[copilot-sdk-diagnostic pid=${process.pid}] ${message}${elapsed}\n`);
-}
-
 type KoffiFunction = ReturnType<ReturnType<typeof koffi.load>["func"]>;
 type KoffiType = ReturnType<typeof koffi.pointer>;
 type KoffiRegisteredCallback = ReturnType<typeof koffi.register>;
@@ -194,8 +186,6 @@ export class FfiRuntimeHost {
      * waits for readiness, and opens the FFI JSON-RPC connection.
      */
     async start(): Promise<void> {
-        const startMs = Date.now();
-        logTestDiagnostic("ffi.start begin");
         const argvJson = buildArgvJson(this.cliEntrypoint, this.args);
         const envJson = buildEnvJson(this.environment);
 
@@ -226,7 +216,6 @@ export class FfiRuntimeHost {
                 `copilot_runtime_host_start failed (library '${this.libraryPath}', entrypoint '${this.cliEntrypoint}').`
             );
         }
-        logTestDiagnostic("ffi.start hostStart complete", startMs);
 
         this.outboundCallback = koffi.register(
             (_userData: unknown, bytesPtr: unknown, bytesLen: number | bigint) =>
@@ -251,7 +240,6 @@ export class FfiRuntimeHost {
             this.serverId = 0;
             throw new Error("copilot_runtime_connection_open failed.");
         }
-        logTestDiagnostic("ffi.start connectionOpen complete", startMs);
 
         // The in-process transport has no socket/pipe handle to keep the Node event loop
         // alive while the SDK is idle awaiting a server→client frame. koffi delivers the
@@ -323,10 +311,6 @@ export class FfiRuntimeHost {
             return;
         }
         this.disposed = true;
-        const disposeMs = Date.now();
-        logTestDiagnostic(
-            `ffi.dispose begin server=${this.serverId !== 0} connection=${this.connectionId !== 0}`
-        );
 
         if (this.keepAliveTimer !== undefined) {
             clearInterval(this.keepAliveTimer);
@@ -335,28 +319,23 @@ export class FfiRuntimeHost {
 
         try {
             if (this.connectionId) {
-                logTestDiagnostic("ffi.dispose connectionClose begin", disposeMs);
                 this.lib.connectionClose(this.connectionId);
                 this.connectionId = 0;
-                logTestDiagnostic("ffi.dispose connectionClose complete", disposeMs);
             }
         } catch {
             // Ignore teardown failures.
         }
 
         try {
             if (this.serverId) {
-                logTestDiagnostic("ffi.dispose hostShutdown begin", disposeMs);
                 this.lib.hostShutdown(this.serverId);
                 this.serverId = 0;
-                logTestDiagnostic("ffi.dispose hostShutdown complete", disposeMs);
             }
         } catch {
             // Ignore teardown failures.
         }
 
         this.receiveStream.end();
         this.unregisterCallback();
-        logTestDiagnostic("ffi.dispose complete", disposeMs);
     }
 }
diff --git a/nodejs/src/session.ts b/nodejs/src/session.ts
@@ -61,14 +61,6 @@ import type {
     UserInputResponse,
 } from "./types.js";
 
-function logTestDiagnostic(message: string, startMs?: number): void {
-    if (process.env.COPILOT_SDK_TEST_DIAGNOSTICS !== "1") {
-        return;
-    }
-    const elapsed = startMs === undefined ? "" : ` elapsed=${Date.now() - startMs}ms`;
-    process.stderr.write(`[copilot-sdk-diagnostic pid=${process.pid}] ${message}${elapsed}\n`);
-}
-
 /**
  * Convert a raw hook input received over the wire into its public-facing shape.
  * This deserializes the numeric Unix-ms `timestamp` field on BaseHookInput
@@ -300,8 +292,6 @@ export class CopilotSession {
         optionsOrPrompt: MessageOptions | string,
         timeout?: number
     ): Promise<AssistantMessageEvent | undefined> {
-        const sendMs = Date.now();
-        logTestDiagnostic(`session.sendAndWait begin session=${this.sessionId}`);
         const options: MessageOptions =
             typeof optionsOrPrompt === "string" ? { prompt: optionsOrPrompt } : optionsOrPrompt;
         const effectiveTimeout = timeout ?? 60_000;
@@ -332,10 +322,6 @@ export class CopilotSession {
         let timeoutId: ReturnType<typeof setTimeout> | undefined;
         try {
             await this.send(options);
-            logTestDiagnostic(
-                `session.sendAndWait send complete session=${this.sessionId}`,
-                sendMs
-            );
 
             const timeoutPromise = new Promise<never>((_, reject) => {
                 timeoutId = setTimeout(
@@ -349,15 +335,13 @@ export class CopilotSession {
                 );
             });
             await Promise.race([idlePromise, timeoutPromise]);
-            logTestDiagnostic(`session.sendAndWait idle session=${this.sessionId}`, sendMs);
 
             return lastAssistantMessage;
         } finally {
             if (timeoutId !== undefined) {
                 clearTimeout(timeoutId);
             }
             unsubscribe();
-            logTestDiagnostic(`session.sendAndWait complete session=${this.sessionId}`, sendMs);
         }
     }
 
diff --git a/nodejs/test/e2e/harness/sdkTestContext.ts b/nodejs/test/e2e/harness/sdkTestContext.ts
diff --git a/test/harness/capturingHttpProxy.ts b/test/harness/capturingHttpProxy.ts
