From f359df7bdad858e23638b7060def4c8617dbf827 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 9 Jul 2026 13:10:53 +0100 Subject: [PATCH 01/18] Add in-process (FFI) transport for the Node.js SDK Host the Copilot runtime in-process by loading the native runtime library and speaking JSON-RPC over its C ABI via koffi, instead of spawning a runtime child process. Adds: - RuntimeConnection.forInProcess() + InProcessRuntimeConnection (experimental) - FfiRuntimeHost: koffi-based C ABI host with deferred, non-reentrant inbound frame delivery and an async broker pump so foreign-thread outbound callbacks are delivered while the loop is idle - client.ts wiring for start/stop/forceStop over the FFI host - E2E harness support (isInProcessTransport, per-test proxy/env mirroring), a dedicated inprocess_ffi E2E test, and an "inprocess" CI matrix cell - skipIf(isInProcessTransport) for tests that rely on per-client env or a child process (telemetry, request-cancel, checkpoint u32, process-kill), documented against https://github.com/github/copilot-sdk/issues/1934 - Graceful stop() (not forceStop) for inproc resume clients so the runtime closes session-store.db and Windows temp-dir cleanup can unlink it - Bump @github/copilot to ^1.0.70-0; add koffi ^3.1.0 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/nodejs-sdk-tests.yml | 8 +- nodejs/package-lock.json | 251 ++++++++++ nodejs/package.json | 1 + nodejs/src/client.ts | 222 +++++++-- nodejs/src/ffiRuntimeHost.ts | 436 ++++++++++++++++++ nodejs/src/index.ts | 1 + nodejs/src/types.ts | 36 ++ nodejs/test/client.test.ts | 39 +- .../copilot_request_cancel_error.e2e.test.ts | 46 +- nodejs/test/e2e/harness/sdkTestContext.ts | 87 +++- nodejs/test/e2e/inprocess_ffi.e2e.test.ts | 37 ++ .../e2e/rpc_workspace_checkpoints.e2e.test.ts | 30 +- nodejs/test/e2e/session.e2e.test.ts | 4 +- .../test/e2e/streaming_fidelity.e2e.test.ts | 4 +- nodejs/test/e2e/telemetry.e2e.test.ts | 172 +++---- 15 files changed, 1187 insertions(+), 187 deletions(-) create mode 100644 nodejs/src/ffiRuntimeHost.ts create mode 100644 nodejs/test/e2e/inprocess_ffi.e2e.test.ts diff --git a/.github/workflows/nodejs-sdk-tests.yml b/.github/workflows/nodejs-sdk-tests.yml index 8880cadfa3..647345e0ea 100644 --- a/.github/workflows/nodejs-sdk-tests.yml +++ b/.github/workflows/nodejs-sdk-tests.yml @@ -31,7 +31,7 @@ permissions: jobs: test: - name: "Node.js SDK Tests" + name: "Node.js SDK Tests (${{ matrix.os }}, ${{ matrix.transport }})" if: github.event.repository.fork == false env: POWERSHELL_UPDATECHECK: Off @@ -39,6 +39,7 @@ jobs: fail-fast: false matrix: os: [ubuntu-latest, macos-latest, windows-latest] + transport: ["default", "inprocess"] runs-on: ${{ matrix.os }} defaults: run: @@ -75,6 +76,11 @@ jobs: if: runner.os == 'Windows' run: pwsh.exe -Command "Write-Host 'PowerShell ready'" + - name: Select inprocess transport + if: matrix.transport == 'inprocess' + run: | + echo "COPILOT_SDK_DEFAULT_CONNECTION=inprocess" >> "$GITHUB_ENV" + - name: Run Node.js SDK tests env: COPILOT_HMAC_KEY: ${{ secrets.COPILOT_DEVELOPER_CLI_INTEGRATION_HMAC_KEY }} diff --git a/nodejs/package-lock.json b/nodejs/package-lock.json index 6ac1928309..351b6e8c74 100644 --- a/nodejs/package-lock.json +++ b/nodejs/package-lock.json @@ -10,6 +10,7 @@ "license": "MIT", "dependencies": { "@github/copilot": "^1.0.70-0", + "koffi": "^3.1.0", "vscode-jsonrpc": "^8.2.1", "zod": "^4.3.6" }, @@ -921,6 +922,230 @@ "dev": true, "license": "MIT" }, + "node_modules/@koromix/koffi-darwin-arm64": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/@koromix/koffi-darwin-arm64/-/koffi-darwin-arm64-3.1.0.tgz", + "integrity": "sha512-VEt5r3fXTfbejr83PnuOP0H7s9Zmazcs+lofu96DOcRkistlMsn59wYyWiKpyAjs9PCgm0Ykh62ChZ3CGMmIOg==", + "cpu": [ + "arm64" + ], + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ], + "funding": { + "url": "https://liberapay.com/Koromix" + } + }, + "node_modules/@koromix/koffi-darwin-x64": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/@koromix/koffi-darwin-x64/-/koffi-darwin-x64-3.1.0.tgz", + "integrity": "sha512-n/tVRB9xIzdXT5H3zZt8ueThgWTSDL+yU7PWnU8wbZPBSawP/otx3swQyd6nMOqj1bmHgSHopiKSBXRS9pllmg==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ], + "funding": { + "url": "https://liberapay.com/Koromix" + } + }, + "node_modules/@koromix/koffi-freebsd-arm64": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/@koromix/koffi-freebsd-arm64/-/koffi-freebsd-arm64-3.1.0.tgz", + "integrity": "sha512-vazoPYIhOAlXZksVIqDRMIID4VeUZKx8F3dR90hOobT2ATyOkqNS5dv5UCV7Q7DSq22lQTrdbvENBAhROzCp0w==", + "cpu": [ + "arm64" + ], + "license": "MIT", + "optional": true, + "os": [ + "freebsd" + ], + "funding": { + "url": "https://liberapay.com/Koromix" + } + }, + "node_modules/@koromix/koffi-freebsd-ia32": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/@koromix/koffi-freebsd-ia32/-/koffi-freebsd-ia32-3.1.0.tgz", + "integrity": "sha512-Vm7Uc97ru6RTSVmae2zCZZQeaizqVZ8WoU4+gG4H03Qe+WOj7kbKt/MxT7VBzdbPYIU5ZJeG/ZED1YlZyab6eQ==", + "cpu": [ + "ia32" + ], + "license": "MIT", + "optional": true, + "os": [ + "freebsd" + ], + "funding": { + "url": "https://liberapay.com/Koromix" + } + }, + "node_modules/@koromix/koffi-freebsd-x64": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/@koromix/koffi-freebsd-x64/-/koffi-freebsd-x64-3.1.0.tgz", + "integrity": "sha512-N+VuVWjoiYPy1Go5mRadZ3B6RM5Qz+eCLhj2LXrMlefbUJ+O4gg7teCUGvPGfBEHDgmSN4yYUrfQmdJC10vOYw==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "freebsd" + ], + "funding": { + "url": "https://liberapay.com/Koromix" + } + }, + "node_modules/@koromix/koffi-linux-arm64": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/@koromix/koffi-linux-arm64/-/koffi-linux-arm64-3.1.0.tgz", + "integrity": "sha512-Wx5iOkeALe2ympLdiYwRpIg5qUkyQIv8N2foZ9rRker0uE7ZtXew2RRkbEgMir4b0yDYR1zyXd6B62GUzLtZ/g==", + "cpu": [ + "arm64" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "funding": { + "url": "https://liberapay.com/Koromix" + } + }, + "node_modules/@koromix/koffi-linux-ia32": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/@koromix/koffi-linux-ia32/-/koffi-linux-ia32-3.1.0.tgz", + "integrity": "sha512-1DjYm1QehXU0dgn0uE+FGYOb3Of7GiTMqLS+ZI2gbl1b+h76sz4LRBvDVrQyAmSMVVU8/7696S21YgE/iBhBVg==", + "cpu": [ + "ia32" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "funding": { + "url": "https://liberapay.com/Koromix" + } + }, + "node_modules/@koromix/koffi-linux-loong64": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/@koromix/koffi-linux-loong64/-/koffi-linux-loong64-3.1.0.tgz", + "integrity": "sha512-NOa0LdyltdESz3oeTqUH6MErHVoJOHoeXIsEp6xIMTUh4eKXEtlDQeoK6EYqo0DnBt83Xud95qLvi4Aw12pG4Q==", + "cpu": [ + "loong64" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "funding": { + "url": "https://liberapay.com/Koromix" + } + }, + "node_modules/@koromix/koffi-linux-riscv64": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/@koromix/koffi-linux-riscv64/-/koffi-linux-riscv64-3.1.0.tgz", + "integrity": "sha512-Ye6kiXZCGxGtAIXSly6XuOP5tJZNYOZ2eVg33k1MilKrzimAy9Mpw4d6e9+Sfsc1jesgeNYs1sb5iaI8HS3ncA==", + "cpu": [ + "riscv64" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "funding": { + "url": "https://liberapay.com/Koromix" + } + }, + "node_modules/@koromix/koffi-linux-x64": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/@koromix/koffi-linux-x64/-/koffi-linux-x64-3.1.0.tgz", + "integrity": "sha512-3yQTOkQrMna4VX+yeyfYImBjLlGrItMpsWyfaW1uSiz/A6GRydqdwYH7DWnp4Z+RSGYZpsewkf7byMc8pOOQKA==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "funding": { + "url": "https://liberapay.com/Koromix" + } + }, + "node_modules/@koromix/koffi-openbsd-ia32": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/@koromix/koffi-openbsd-ia32/-/koffi-openbsd-ia32-3.1.0.tgz", + "integrity": "sha512-/cDoFHb9yx4+yoT3GUpnKnfi3W2drG+/Ewo0TTZaQHb4PsxnYYyT6V8+t4cL5XXbQcTTcOsZxpmBRrn0NBa3dA==", + "cpu": [ + "ia32" + ], + "license": "MIT", + "optional": true, + "os": [ + "openbsd" + ], + "funding": { + "url": "https://liberapay.com/Koromix" + } + }, + "node_modules/@koromix/koffi-openbsd-x64": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/@koromix/koffi-openbsd-x64/-/koffi-openbsd-x64-3.1.0.tgz", + "integrity": "sha512-CoQdqgnKvWgTXXZlUst8cBRQEov7QsxlTN2WAsu9wez01Xe6gEcH/zYePANualzzCbnaELfe5P0rA80QkoDuPA==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "openbsd" + ], + "funding": { + "url": "https://liberapay.com/Koromix" + } + }, + "node_modules/@koromix/koffi-win32-ia32": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/@koromix/koffi-win32-ia32/-/koffi-win32-ia32-3.1.0.tgz", + "integrity": "sha512-WjrA+DEkpy0xEHu48+NSOboHhTnzkIfsFuq3d/WrSs+T9WflWRng3jC7mdJxmR4eHb6i6BqjW3k/U0mNUTjFPA==", + "cpu": [ + "ia32" + ], + "license": "MIT", + "optional": true, + "os": [ + "win32" + ], + "funding": { + "url": "https://liberapay.com/Koromix" + } + }, + "node_modules/@koromix/koffi-win32-x64": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/@koromix/koffi-win32-x64/-/koffi-win32-x64-3.1.0.tgz", + "integrity": "sha512-tnK5+IkzQBauQAQSzuyjso8OOIQRlaTZS39xIWpfqVYDLVDIuLDQk/WwHcOrR5yxlDrZq9ygiebBTOfcJFia7w==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "win32" + ], + "funding": { + "url": "https://liberapay.com/Koromix" + } + }, "node_modules/@napi-rs/wasm-runtime": { "version": "1.1.4", "resolved": "https://registry.npmjs.org/@napi-rs/wasm-runtime/-/wasm-runtime-1.1.4.tgz", @@ -2592,6 +2817,32 @@ "json-buffer": "3.0.1" } }, + "node_modules/koffi": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/koffi/-/koffi-3.1.0.tgz", + "integrity": "sha512-0mCvdjTJBXioiaKNz0vajAEdWtfM5qyhVXSq+wQrrU3odzNvl/J7Cqna79QpNo9mfoKpQgGsyFFDRtDACCwGrQ==", + "hasInstallScript": true, + "license": "MIT", + "funding": { + "url": "https://liberapay.com/Koromix" + }, + "optionalDependencies": { + "@koromix/koffi-darwin-arm64": "3.1.0", + "@koromix/koffi-darwin-x64": "3.1.0", + "@koromix/koffi-freebsd-arm64": "3.1.0", + "@koromix/koffi-freebsd-ia32": "3.1.0", + "@koromix/koffi-freebsd-x64": "3.1.0", + "@koromix/koffi-linux-arm64": "3.1.0", + "@koromix/koffi-linux-ia32": "3.1.0", + "@koromix/koffi-linux-loong64": "3.1.0", + "@koromix/koffi-linux-riscv64": "3.1.0", + "@koromix/koffi-linux-x64": "3.1.0", + "@koromix/koffi-openbsd-ia32": "3.1.0", + "@koromix/koffi-openbsd-x64": "3.1.0", + "@koromix/koffi-win32-ia32": "3.1.0", + "@koromix/koffi-win32-x64": "3.1.0" + } + }, "node_modules/levn": { "version": "0.4.1", "resolved": "https://registry.npmjs.org/levn/-/levn-0.4.1.tgz", diff --git a/nodejs/package.json b/nodejs/package.json index f9df3320cb..d622c2bc07 100644 --- a/nodejs/package.json +++ b/nodejs/package.json @@ -57,6 +57,7 @@ "license": "MIT", "dependencies": { "@github/copilot": "^1.0.70-0", + "koffi": "^3.1.0", "vscode-jsonrpc": "^8.2.1", "zod": "^4.3.6" }, diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index 8f4d3ff2db..b0e2fcbb9a 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -40,6 +40,7 @@ import type { } from "./generated/rpc.js"; import { getSdkProtocolVersion } from "./sdkProtocolVersion.js"; import { CopilotSession } from "./session.js"; +import { FfiRuntimeHost } from "./ffiRuntimeHost.js"; import { createSessionFsAdapter, type SessionFsProvider } from "./sessionFsProvider.js"; import { createCopilotRequestAdapter } from "./copilotRequestHandler.js"; import type { CopilotRequestHandler } from "./copilotRequestHandler.js"; @@ -58,6 +59,7 @@ import type { BearerTokenProvider, GetStatusResponse, InternalRuntimeConnection, + RuntimeConnection, LargeToolOutputConfig, MCPServerConfig, ModelInfo, @@ -473,6 +475,7 @@ class TeardownResilientStreamMessageWriter extends StreamMessageWriter { export class CopilotClient { private cliStartTimeout: ReturnType | null = null; private cliProcess: ChildProcess | null = null; + private ffiHost: FfiRuntimeHost | null = null; private connection: MessageConnection | null = null; private messageWriter: TeardownResilientStreamMessageWriter | null = null; private socket: Socket | null = null; @@ -563,6 +566,33 @@ export class CopilotClient { } } + /** + * Environment variable that overrides the transport when the caller does not set + * {@link CopilotClientOptions.connection}. Accepts `"inprocess"` or `"stdio"` + * (case-insensitive); unset preserves the default stdio transport. Any other value + * is an error. + */ + private static readonly DEFAULT_CONNECTION_ENV_VAR = "COPILOT_SDK_DEFAULT_CONNECTION"; + + /** + * Resolves the default {@link RuntimeConnection} for the no-connection case, + * honoring {@link CopilotClient.DEFAULT_CONNECTION_ENV_VAR}. + */ + private static resolveDefaultConnection(options: CopilotClientOptions): RuntimeConnection { + const env = options.env ?? process.env; + const value = env[CopilotClient.DEFAULT_CONNECTION_ENV_VAR]; + if (!value || value.toLowerCase() === "stdio") { + return { kind: "stdio" }; + } + if (value.toLowerCase() === "inprocess") { + return { kind: "inprocess" }; + } + throw new Error( + `Invalid ${CopilotClient.DEFAULT_CONNECTION_ENV_VAR} value '${value}'. ` + + `Expected 'inprocess', 'stdio', or unset.` + ); + } + /** * Creates a new CopilotClient instance. * @@ -594,8 +624,10 @@ export class CopilotClient { // Resolve the connection mode. `_internalConnection` is set by // `joinSession()` to opt into the parent-process stdio path; consumers // should always go through the public `connection` field. - const conn: InternalRuntimeConnection = options._internalConnection ?? - options.connection ?? { kind: "stdio" }; + const conn: InternalRuntimeConnection = + options._internalConnection ?? + options.connection ?? + CopilotClient.resolveDefaultConnection(options); if ( conn.kind === "uri" && @@ -810,7 +842,9 @@ export class CopilotClient { try { // Only start CLI server process if not connecting to external server - if (!this.isExternalServer) { + if (this.connectionConfig.kind === "inprocess") { + await this.startInProcessFfi(); + } else if (!this.isExternalServer) { await this.startCLIServer(); } @@ -910,7 +944,7 @@ export class CopilotClient { // Ask SDK-owned runtimes to flush and clean up before we tear down // their transport/process. External runtimes may be shared, so only // close our connection to them. - if (this.connection && this.cliProcess && !this.isExternalServer) { + if (this.connection && (this.cliProcess || this.ffiHost) && !this.isExternalServer) { const runtimeShutdownStart = Date.now(); const shutdownPromise = this.rpc.runtime.shutdown(); void shutdownPromise.catch(() => undefined); @@ -1011,6 +1045,21 @@ export class CopilotClient { ); } } + // Tear down the in-process FFI host (closes the native connection and + // shuts down the native runtime host) for SDK-owned in-process runtimes. + if (this.ffiHost) { + const host = this.ffiHost; + this.ffiHost = null; + try { + host.dispose(); + } catch (error) { + errors.push( + new Error( + `Failed to dispose in-process runtime host: ${error instanceof Error ? error.message : String(error)}` + ) + ); + } + } if (this.cliStartTimeout) { clearTimeout(this.cliStartTimeout); this.cliStartTimeout = null; @@ -1113,6 +1162,16 @@ export class CopilotClient { this.cliProcess = null; } + // Tear down the in-process FFI host (if any). + if (this.ffiHost) { + try { + this.ffiHost.dispose(); + } catch { + // Ignore errors during force stop + } + this.ffiHost = null; + } + if (this.cliStartTimeout) { clearTimeout(this.cliStartTimeout); this.cliStartTimeout = null; @@ -2197,6 +2256,47 @@ export class CopilotClient { }; } + /** + * Builds the environment for the spawned runtime child process (stdio/TCP): applies + * the auth token, connection token, `COPILOT_HOME`, keychain setting, and telemetry + * variables on top of the effective env. Not used by the in-process (FFI) transport, + * whose worker inherits the host process's ambient environment + * (see {@link CopilotClient.startInProcessFfi}). + */ + private buildRuntimeEnv(): Record { + const env: Record = { ...this.resolvedEnv }; + delete env.NODE_DEBUG; + + if (this.options.gitHubToken) { + env.COPILOT_SDK_AUTH_TOKEN = this.options.gitHubToken; + } + if (this.effectiveConnectionToken) { + env.COPILOT_CONNECTION_TOKEN = this.effectiveConnectionToken; + } + if (this.options.baseDirectory) { + env.COPILOT_HOME = this.options.baseDirectory; + } + // In empty mode, disable the system keychain. Keytar reads from a + // process-wide store that's shared across sessions, which is unsafe + // for multi-tenant hosts. The runtime falls back to file-based + // credential storage scoped to COPILOT_HOME. + if (this.options.mode === "empty") { + env.COPILOT_DISABLE_KEYTAR = "1"; + } + if (this.options.telemetry) { + const t = this.options.telemetry; + env.COPILOT_OTEL_ENABLED = "true"; + if (t.otlpEndpoint !== undefined) env.OTEL_EXPORTER_OTLP_ENDPOINT = t.otlpEndpoint; + if (t.otlpProtocol !== undefined) env.OTEL_EXPORTER_OTLP_PROTOCOL = t.otlpProtocol; + if (t.filePath !== undefined) env.COPILOT_OTEL_FILE_EXPORTER_PATH = t.filePath; + if (t.exporterType !== undefined) env.COPILOT_OTEL_EXPORTER_TYPE = t.exporterType; + if (t.sourceName !== undefined) env.COPILOT_OTEL_SOURCE_NAME = t.sourceName; + if (t.captureContent !== undefined) + env.OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT = String(t.captureContent); + } + return env; + } + /** * Start the CLI server process */ @@ -2243,30 +2343,9 @@ export class CopilotClient { args.push("--remote"); } - // Suppress debug/trace output that might pollute stdout - const envWithoutNodeDebug = { ...this.resolvedEnv }; - delete envWithoutNodeDebug.NODE_DEBUG; - - // Set auth token in environment if provided - if (this.options.gitHubToken) { - envWithoutNodeDebug.COPILOT_SDK_AUTH_TOKEN = this.options.gitHubToken; - } - - if (this.effectiveConnectionToken) { - envWithoutNodeDebug.COPILOT_CONNECTION_TOKEN = this.effectiveConnectionToken; - } - - if (this.options.baseDirectory) { - envWithoutNodeDebug.COPILOT_HOME = this.options.baseDirectory; - } - - // In empty mode, disable the system keychain. Keytar reads from a - // process-wide store that's shared across sessions, which is unsafe - // for multi-tenant hosts. The runtime falls back to file-based - // credential storage scoped to COPILOT_HOME. - if (this.options.mode === "empty") { - envWithoutNodeDebug.COPILOT_DISABLE_KEYTAR = "1"; - } + // Suppress debug/trace output that might pollute stdout, and apply the + // shared runtime env (auth token, connection token, COPILOT_HOME, telemetry). + const envWithoutNodeDebug = this.buildRuntimeEnv(); if (!this.resolvedCliPath) { throw new Error( @@ -2278,26 +2357,6 @@ export class CopilotClient { ); } - // Set OpenTelemetry environment variables if telemetry is configured - if (this.options.telemetry) { - const t = this.options.telemetry; - envWithoutNodeDebug.COPILOT_OTEL_ENABLED = "true"; - if (t.otlpEndpoint !== undefined) - envWithoutNodeDebug.OTEL_EXPORTER_OTLP_ENDPOINT = t.otlpEndpoint; - if (t.otlpProtocol !== undefined) - envWithoutNodeDebug.OTEL_EXPORTER_OTLP_PROTOCOL = t.otlpProtocol; - if (t.filePath !== undefined) - envWithoutNodeDebug.COPILOT_OTEL_FILE_EXPORTER_PATH = t.filePath; - if (t.exporterType !== undefined) - envWithoutNodeDebug.COPILOT_OTEL_EXPORTER_TYPE = t.exporterType; - if (t.sourceName !== undefined) - envWithoutNodeDebug.COPILOT_OTEL_SOURCE_NAME = t.sourceName; - if (t.captureContent !== undefined) - envWithoutNodeDebug.OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT = String( - t.captureContent - ); - } - // Verify CLI exists before attempting to spawn if (!existsSync(this.resolvedCliPath)) { throw new Error( @@ -2434,12 +2493,76 @@ export class CopilotClient { return this.connectToParentProcessViaStdio(); case "stdio": return this.connectToChildProcessViaStdio(); + case "inprocess": + return this.connectViaFfi(); case "tcp": case "uri": return this.connectViaTcp(); } } + /** + * Start the in-process FFI runtime host: resolve the CLI entrypoint and native + * runtime library, then let the native host spawn the CLI worker. + * + * The worker inherits this host process's ambient environment; per-client options + * that lower to environment variables (`env`, `telemetry`, `gitHubToken`, + * `baseDirectory`) are intentionally not applied here, because the native runtime + * loads into the shared host process and a single env block cannot carry per-client + * values. Configure the in-process runtime via the host process environment instead. + * See https://github.com/github/copilot-sdk/issues/1934. + */ + private async startInProcessFfi(): Promise { + const entrypoint = this.resolveCliPathForFfi(); + const host = FfiRuntimeHost.create( + entrypoint, + CopilotClient.getNapiPrebuildsFolder(), + undefined, + this.options.workingDirectory + ); + this.ffiHost = host; + await host.start(); + } + + /** + * Connect to the in-process FFI runtime host over its receive/send streams, + * reusing the same `vscode-jsonrpc` framing as the stdio transport. + */ + private async connectViaFfi(): Promise { + if (!this.ffiHost) { + throw new Error("In-process FFI runtime host not started"); + } + this.messageWriter = new TeardownResilientStreamMessageWriter(this.ffiHost.sendStream); + this.connection = createMessageConnection( + new StreamMessageReader(this.ffiHost.receiveStream), + this.messageWriter + ); + + this.attachConnectionHandlers(); + this.connection.listen(); + } + + /** + * Resolves the CLI entrypoint used for in-process FFI hosting: `COPILOT_CLI_PATH` + * when set, otherwise the bundled platform-package entrypoint. + */ + private resolveCliPathForFfi(): string { + return this.resolvedEnv.COPILOT_CLI_PATH ?? getBundledCliPath(); + } + + /** + * Returns the napi prebuilds folder name for the current host — the + * `-` convention (e.g. `win32-x64`, `darwin-arm64`, + * `linux-x64`) under which the runtime ships `prebuilds//runtime.node`. + */ + private static getNapiPrebuildsFolder(): string { + const arch = process.arch; + if (arch !== "x64" && arch !== "arm64") { + throw new Error(`Unsupported architecture '${arch}' for in-process FFI hosting.`); + } + return `${process.platform}-${arch}`; + } + /** * Connect to child via stdio pipes */ @@ -2622,8 +2745,9 @@ export class CopilotClient { } const session = this.sessions.get((notification as { sessionId: string }).sessionId); + const event = (notification as { event: SessionEvent }).event; if (session) { - session._dispatchEvent((notification as { event: SessionEvent }).event); + session._dispatchEvent(event); } } diff --git a/nodejs/src/ffiRuntimeHost.ts b/nodejs/src/ffiRuntimeHost.ts new file mode 100644 index 0000000000..f90baa39e0 --- /dev/null +++ b/nodejs/src/ffiRuntimeHost.ts @@ -0,0 +1,436 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------------------------------------------*/ + +/** + * Hosts the Copilot runtime in-process by loading the native `runtime.node` cdylib + * and speaking JSON-RPC over its C ABI (FFI) instead of spawning a CLI child process + * and communicating over stdio/TCP. + * + * The native `host_start` export spawns the CLI worker itself + * (`node --embedded-host` for a `.js` entrypoint, or ` + * --embedded-host` for a packaged binary), so the SDK never launches the worker + * directly. LSP `Content-Length:`-framed JSON-RPC bytes are pumped across the ABI: + * writes go to `connection_write`; inbound frames arrive on a native callback that + * feeds {@link FfiRuntimeHost.receiveStream}. The existing `vscode-jsonrpc` + * `StreamMessageReader`/`StreamMessageWriter` handle framing unchanged — this is a + * transport swap, not a new protocol. + */ + +import { existsSync } from "node:fs"; +import koffi from "koffi"; +import { dirname, join, resolve } from "node:path"; +import { PassThrough, Writable } from "node:stream"; + +const SYMBOL_PREFIX = "copilot_runtime_"; + +type KoffiFunction = ReturnType["func"]>; +type KoffiType = ReturnType; +type KoffiRegisteredCallback = ReturnType; + +interface FfiLibrary { + hostStart: KoffiFunction; + hostShutdown: KoffiFunction; + connectionOpen: KoffiFunction; + connectionWrite: KoffiFunction; + connectionClose: KoffiFunction; + logDroppedCount: KoffiFunction; + outboundCallbackType: KoffiType; +} + +let loadedLibraryPath: string | undefined; +let loadedLibrary: FfiLibrary | undefined; + +/** + * Loads the cdylib once per process and binds the C ABI exports. Loading a + * different library path in the same process is unsupported. + */ +function loadLibrary(libraryPath: string): FfiLibrary { + if (loadedLibrary) { + if (loadedLibraryPath !== libraryPath) { + throw new Error( + `An in-process FFI runtime library is already loaded from '${loadedLibraryPath}'; ` + + `loading a different library from '${libraryPath}' in the same process is not supported.` + ); + } + return loadedLibrary; + } + + const lib = koffi.load(libraryPath); + const outboundCallbackType = koffi.pointer( + koffi.proto( + `void ${SYMBOL_PREFIX}outbound(void *userData, uint8 *bytesPtr, size_t bytesLen)` + ) + ); + + loadedLibrary = { + hostStart: lib.func(`${SYMBOL_PREFIX}host_start`, "uint32", [ + "uint8*", + "size_t", + "uint8*", + "size_t", + ]), + hostShutdown: lib.func(`${SYMBOL_PREFIX}host_shutdown`, "bool", ["uint32"]), + connectionOpen: lib.func(`${SYMBOL_PREFIX}connection_open`, "uint32", [ + "uint32", + outboundCallbackType, + "void*", + "uint8*", + "size_t", + "uint8*", + "size_t", + "uint8*", + "size_t", + ]), + connectionWrite: lib.func(`${SYMBOL_PREFIX}connection_write`, "bool", [ + "uint32", + "uint8*", + "size_t", + ]), + connectionClose: lib.func(`${SYMBOL_PREFIX}connection_close`, "bool", ["uint32"]), + // A no-argument, side-effect-free diagnostic getter (returns a dropped-log + // counter). Used purely as a cheap async FFI call to keep koffi's asynchronous + // callback broker serviced while the connection is idle (see the pump in start()). + logDroppedCount: lib.func(`${SYMBOL_PREFIX}log_dropped_count`, "uint64", []), + outboundCallbackType, + }; + loadedLibraryPath = libraryPath; + return loadedLibrary; +} + +function buildArgvJson(cliEntrypoint: string): Buffer { + // A `.js` entrypoint is launched via node; the packaged single-file CLI binary + // embeds its own Node and is invoked directly. + const argv = cliEntrypoint.toLowerCase().endsWith(".js") + ? ["node", cliEntrypoint, "--embedded-host"] + : [cliEntrypoint, "--embedded-host"]; + return Buffer.from(JSON.stringify(argv), "utf8"); +} + +function buildEnvJson(environment?: Record): Buffer | null { + if (!environment) { + return null; + } + const obj: Record = {}; + for (const [key, value] of Object.entries(environment)) { + if (value !== undefined) { + obj[key] = value; + } + } + if (Object.keys(obj).length === 0) { + return null; + } + return Buffer.from(JSON.stringify(obj), "utf8"); +} + +export class FfiRuntimeHost { + private readonly lib: FfiLibrary; + private serverId = 0; + private connectionId = 0; + private disposed = false; + private outboundCallback: KoffiRegisteredCallback | undefined; + /** + * Keeps koffi's asynchronous callback broker serviced while the FFI connection is + * open, so inbound native→JS frames are delivered promptly even when the SDK is + * otherwise idle (e.g. awaiting a model response with no client→server writes in + * flight). + * + * The runtime invokes our outbound callback from a worker thread. koffi marshals + * such foreign-thread callbacks to the JS main thread via a threadsafe-function + * "broker" that is only serviced WHILE a koffi asynchronous FFI call is in flight — + * a plain JS timer (setInterval) does NOT pump it. During active request/response + * traffic the constant `connection_write` calls keep frames flowing, but once the + * SDK goes idle mid-turn (e.g. the worker is awaiting a model HTTP response, so + * there are no client→server writes) the broker parks and the next outbound frame + * (the model response, then everything after it) sits undelivered until the next + * koffi call — observed as a permanent 30s stall of otherwise-healthy sessions on + * macOS/Windows (libuv services the broker differently on Linux, which is why it + * only reproduces off-Linux). Keeping exactly one cheap async FFI call + * (`log_dropped_count`) continuously in flight keeps the broker pumping so + * foreign-thread frames are always delivered. This is the koffi analogue of the + * .NET host's raw function-pointer callback, which runs directly on the runtime + * thread and needs no event-loop pumping. + */ + private pumpActive = false; + /** + * Inbound frame bytes copied out of the native callback, awaiting delivery to + * {@link receiveStream} on a clean event-loop tick. See {@link feedInbound}. + */ + private readonly inboundQueue: Buffer[] = []; + /** Whether a drain of {@link inboundQueue} is already scheduled. */ + private drainScheduled = false; + + /** The stream JSON-RPC reads server→client frames from. */ + readonly receiveStream: PassThrough; + /** The stream JSON-RPC writes client→server frames to. */ + readonly sendStream: Writable; + + private constructor( + private readonly libraryPath: string, + private readonly cliEntrypoint: string, + private readonly environment?: Record, + private readonly workingDirectory?: string + ) { + this.lib = loadLibrary(libraryPath); + this.receiveStream = new PassThrough(); + this.sendStream = new Writable({ + // Write frames with a synchronous FFI call (restored while diagnosing the + // macOS/Windows inbound-callback stall; async writes were a no-op for it). + write: (chunk: Buffer, _encoding, callback) => { + try { + this.writeFrame(chunk); + callback(); + } catch (error) { + callback(error as Error); + } + }, + }); + } + + /** + * Resolves the cdylib next to the given CLI entrypoint and prepares the FFI host. + * The cdylib is resolved as `prebuilds//runtime.node` relative to + * the entrypoint directory (the napi-rs `-` layout, e.g. + * `linux-x64`). Throws if it cannot be found. + */ + static create( + cliEntrypoint: string, + prebuildsFolder: string, + environment?: Record, + workingDirectory?: string + ): FfiRuntimeHost { + const fullEntrypoint = resolve(cliEntrypoint); + const distDir = dirname(fullEntrypoint); + const libraryPath = join(distDir, "prebuilds", prebuildsFolder, "runtime.node"); + if (!existsSync(libraryPath)) { + throw new Error(`FFI runtime library not found. Looked for '${libraryPath}'.`); + } + return new FfiRuntimeHost(libraryPath, fullEntrypoint, environment, workingDirectory); + } + + /** + * Starts the in-process runtime: spawns the CLI worker via the native host, + * waits for readiness, and opens the FFI JSON-RPC connection. + */ + async start(): Promise { + const argvJson = buildArgvJson(this.cliEntrypoint); + const envJson = buildEnvJson(this.environment); + + // The native host spawns the CLI worker itself and has no cwd parameter, so the + // worker inherits this process's cwd. Mirror the stdio child's `cwd: workingDirectory` + // by switching cwd for the duration of the blocking host_start, then restoring it. + const previousCwd = process.cwd(); + const shouldSwitchCwd = !!this.workingDirectory && this.workingDirectory !== previousCwd; + if (shouldSwitchCwd) { + process.chdir(this.workingDirectory!); + } + + // host_start blocks until the worker connects back and signals readiness + // (up to ~30s); run it as an async FFI call so the Node event loop isn't blocked. + try { + this.serverId = await new Promise((resolvePromise, rejectPromise) => { + this.lib.hostStart.async( + argvJson, + argvJson.length, + envJson, + envJson ? envJson.length : 0, + (error: Error | null, result: number) => { + if (error) { + rejectPromise(error); + } else { + resolvePromise(result); + } + } + ); + }); + } finally { + if (shouldSwitchCwd) { + process.chdir(previousCwd); + } + } + if (!this.serverId) { + throw new Error( + `copilot_runtime_host_start failed (library '${this.libraryPath}', entrypoint '${this.cliEntrypoint}').` + ); + } + + this.outboundCallback = koffi.register( + (_userData: unknown, bytesPtr: unknown, bytesLen: number | bigint) => + this.feedInbound(bytesPtr, bytesLen), + this.lib.outboundCallbackType + ); + + this.connectionId = this.lib.connectionOpen( + this.serverId, + this.outboundCallback, + null, + null, + 0, + null, + 0, + null, + 0 + ); + if (!this.connectionId) { + this.unregisterCallback(); + this.lib.hostShutdown(this.serverId); + this.serverId = 0; + throw new Error("copilot_runtime_connection_open failed."); + } + + this.startBrokerPump(); + } + + /** + * Keeps exactly one cheap async FFI call in flight at all times so koffi's + * threadsafe-function broker stays serviced and foreign-thread outbound callbacks + * are delivered without waiting for the next client→server write. Each call + * reschedules the next on completion; the in-flight libuv request also keeps the + * event loop alive (replacing the old keep-alive timer). Stops when disposed. + */ + private startBrokerPump(): void { + if (this.pumpActive) { + return; + } + this.pumpActive = true; + const pump = (): void => { + if (this.disposed || !this.connectionId) { + this.pumpActive = false; + return; + } + this.lib.logDroppedCount.async((_error: Error | null, _result: unknown) => { + if (this.disposed || !this.connectionId) { + this.pumpActive = false; + return; + } + pump(); + }); + }; + pump(); + } + + private writeFrame(frame: Buffer): void { + if (this.disposed || !this.connectionId) { + throw new Error("The in-process runtime connection is closed."); + } + const ok = this.lib.connectionWrite(this.connectionId, frame, frame.length); + if (!ok) { + throw new Error("Failed to write a frame to the in-process runtime connection."); + } + } + + /** + * Native outbound callback: copies the inbound frame bytes and hands them to the + * event loop for delivery, WITHOUT driving the JSON-RPC reader synchronously here. + * + * The native pointer is only valid for the duration of this call, so the bytes are + * decoded/copied eagerly; but writing them to {@link receiveStream} (which + * synchronously drives frame parsing and JSON-RPC dispatch, and may re-enter the + * SDK's write path) is deferred to a `setImmediate` drain on a clean stack. This + * keeps the callback minimal and non-reentrant — the koffi analogue of the .NET + * host's thread-safe `Channel` callback, which enqueues and returns immediately. + * Delivering synchronously here could re-enter a native `connection_write` call + * still on the stack and deadlock (observed as hung `session.rpc.*` round-trips + * on macOS). + */ + private feedInbound(bytesPtr: unknown, bytesLen: number | bigint): void { + // This runs as a native→JS (Node-API) callback, possibly from a secondary + // thread. An exception thrown here cannot propagate across the FFI boundary and + // is swallowed by the runtime (surfacing only as a DEP0168 "Uncaught Node-API + // callback exception" warning), so catch and log it here instead of letting it + // escape. + try { + const length = Number(bytesLen); + if (!bytesPtr || length <= 0) { + return; + } + const bytes = koffi.decode( + bytesPtr, + koffi.array("uint8", length, "Typed") + ) as Uint8Array; + this.inboundQueue.push(Buffer.from(bytes)); + if (!this.drainScheduled) { + this.drainScheduled = true; + setImmediate(() => this.drainInbound()); + } + } catch (error) { + console.error( + `In-process FFI inbound callback failed: ${error instanceof Error ? (error.stack ?? error.message) : String(error)}` + ); + } + } + + /** Delivers queued inbound frames to {@link receiveStream} on a clean event-loop tick. */ + private drainInbound(): void { + this.drainScheduled = false; + if (this.disposed) { + this.inboundQueue.length = 0; + return; + } + let frame: Buffer | undefined; + while ((frame = this.inboundQueue.shift()) !== undefined) { + this.receiveStream.write(frame); + } + } + + private unregisterCallback(): void { + if (this.outboundCallback === undefined) { + return; + } + const callback = this.outboundCallback; + this.outboundCallback = undefined; + // Defer the unregister to a later tick instead of unregistering synchronously. + // koffi delivers outbound callbacks from a secondary thread by queuing them onto + // the JS event loop; at teardown one such delivery can still be queued after we + // stop the native side. Unregistering while koffi still has a queued call makes + // koffi invoke a torn-down callback and raise inside its own native code — an + // uncaught Node-API callback exception (DEP0168) that no JS try/catch can catch. + // The native connection/host are already closed by the time this runs (see + // dispose), so no new deliveries originate; a queued delivery fires in libuv's + // poll phase and setImmediate (check phase) runs right after it in the same loop + // iteration, so the pending delivery (a no-op, since we are disposed) drains + // before we free the slot. + const immediate = setImmediate(() => { + try { + koffi.unregister(callback); + } catch { + // Ignore teardown failures. + } + }); + // Don't let this housekeeping timer keep the process alive. + immediate.unref?.(); + } + + /** Closes the FFI connection, shuts down the native host, and releases resources. */ + dispose(): void { + if (this.disposed) { + return; + } + this.disposed = true; + + // The broker pump observes `disposed` and stops rescheduling on its next + // completion; no timer to clear. + this.pumpActive = false; + + try { + if (this.connectionId) { + this.lib.connectionClose(this.connectionId); + this.connectionId = 0; + } + } catch { + // Ignore teardown failures. + } + + try { + if (this.serverId) { + this.lib.hostShutdown(this.serverId); + this.serverId = 0; + } + } catch { + // Ignore teardown failures. + } + + this.receiveStream.end(); + this.unregisterCallback(); + } +} diff --git a/nodejs/src/index.ts b/nodejs/src/index.ts index e05b33c158..cf65cb653d 100644 --- a/nodejs/src/index.ts +++ b/nodejs/src/index.ts @@ -59,6 +59,7 @@ export type { CopilotClientMode, CopilotClientOptions, StdioRuntimeConnection, + InProcessRuntimeConnection, TcpRuntimeConnection, UriRuntimeConnection, CustomAgentConfig, diff --git a/nodejs/src/types.ts b/nodejs/src/types.ts index 65fc00cfe1..2992880921 100644 --- a/nodejs/src/types.ts +++ b/nodejs/src/types.ts @@ -96,6 +96,7 @@ export interface TelemetryConfig { */ export type RuntimeConnection = | StdioRuntimeConnection + | InProcessRuntimeConnection | TcpRuntimeConnection | UriRuntimeConnection; @@ -111,6 +112,26 @@ export interface StdioRuntimeConnection { readonly args?: readonly string[]; } +/** + * Hosts the runtime in-process by loading the native runtime library and speaking + * JSON-RPC over its C ABI (FFI), instead of spawning a runtime child process. The + * native host spawns the CLI worker itself. Construct via + * {@link RuntimeConnection.forInProcess}. + * + * @experimental The in-process (FFI) transport is experimental and its behavior may + * change. Per-client options that are lowered to environment variables — including + * {@link CopilotClientOptions.env}, {@link CopilotClientOptions.telemetry}, + * {@link CopilotClientOptions.gitHubToken}, and + * {@link CopilotClientOptions.baseDirectory} — are **not** honored with this + * transport, because the native runtime loads into the shared host process and its + * worker inherits that process's ambient environment. To configure the in-process + * runtime, set the corresponding environment variables on the host process before + * constructing the client. See https://github.com/github/copilot-sdk/issues/1934. + */ +export interface InProcessRuntimeConnection { + readonly kind: "inprocess"; +} + /** * Spawns a runtime child process that listens on a TCP socket and connects to it. */ @@ -183,6 +204,21 @@ export const RuntimeConnection = { forUri(url: string, opts: { connectionToken?: string } = {}): UriRuntimeConnection { return { kind: "uri", url, connectionToken: opts.connectionToken }; }, + /** + * Host the runtime in-process over the native runtime library's C ABI (FFI). + * The native host spawns the CLI worker itself; the SDK does not launch a + * runtime child process. Honors `COPILOT_CLI_PATH` for the CLI entrypoint, + * otherwise resolves the bundled platform package. + * + * @experimental Per-client options lowered to environment variables (`env`, + * `telemetry`, `gitHubToken`, `baseDirectory`) are **not** honored in-process; + * the worker inherits the host process's ambient environment. Set the + * corresponding environment variables on the host process instead. See + * https://github.com/github/copilot-sdk/issues/1934. + */ + forInProcess(): InProcessRuntimeConnection { + return { kind: "inprocess" }; + }, } as const; /** diff --git a/nodejs/test/client.test.ts b/nodejs/test/client.test.ts index 96c32a5951..8605f8d82c 100644 --- a/nodejs/test/client.test.ts +++ b/nodejs/test/client.test.ts @@ -2293,22 +2293,29 @@ describe("CopilotClient", () => { }); describe("unexpected disconnection", () => { - it("transitions to disconnected when child process is killed", async () => { - const client = new CopilotClient(); - await client.start(); - onTestFinished(() => client.forceStop()); - - expect((client as any).state).toBe("connected"); - - // Kill the child process to simulate unexpected termination - const proc = (client as any).cliProcess as import("node:child_process").ChildProcess; - proc.kill(); - - // Wait for the connection.onClose handler to fire - await vi.waitFor(() => { - expect((client as any).state).toBe("disconnected"); - }); - }); + // No child process exists over the in-process (FFI) transport, so this + // child-process-kill scenario does not apply there. Covered by the default + // (stdio) cell. + it.skipIf((process.env.COPILOT_SDK_DEFAULT_CONNECTION ?? "").toLowerCase() === "inprocess")( + "transitions to disconnected when child process is killed", + async () => { + const client = new CopilotClient(); + await client.start(); + onTestFinished(() => client.forceStop()); + + expect((client as any).state).toBe("connected"); + + // Kill the child process to simulate unexpected termination + const proc = (client as any) + .cliProcess as import("node:child_process").ChildProcess; + proc.kill(); + + // Wait for the connection.onClose handler to fire + await vi.waitFor(() => { + expect((client as any).state).toBe("disconnected"); + }); + } + ); }); describe("onGetTraceContext", () => { diff --git a/nodejs/test/e2e/copilot_request_cancel_error.e2e.test.ts b/nodejs/test/e2e/copilot_request_cancel_error.e2e.test.ts index 5a9cad5a59..6b3ff5c437 100644 --- a/nodejs/test/e2e/copilot_request_cancel_error.e2e.test.ts +++ b/nodejs/test/e2e/copilot_request_cancel_error.e2e.test.ts @@ -4,7 +4,7 @@ import { describe, expect, it } from "vitest"; import { approveAll, CopilotRequestHandler, type CopilotRequestContext } from "../../src/index.js"; -import { createSdkTestContext } from "./harness/sdkTestContext.js"; +import { createSdkTestContext, isInProcessTransport } from "./harness/sdkTestContext.js"; /** * Cancellation and error coverage for {@link CopilotRequestHandler}. These two @@ -162,23 +162,31 @@ describe("CopilotRequestHandler observes runtime cancellation", async () => { copilotClientOptions: { requestHandler: handler }, }); - it("fires ctx.signal when the consumer aborts an in-flight inference request", async () => { - await client.start(); - const session = await client.createSession({ onPermissionRequest: approveAll }); - try { - await session.send("Say OK."); - await waitFor(() => handler.inferenceEntered, 60_000); - await session.abort(); - await waitFor(() => handler.sawAbort, 30_000); - } finally { - await session.disconnect(); - } + // A CopilotRequestHandler registers a process-wide LLM inference provider. Over the + // in-process transport every client shares the one host process, so a second client + // cannot register while another still holds the provider. Covered by the default + // (stdio) cell, where each client owns its own process. + it.skipIf(isInProcessTransport)( + "fires ctx.signal when the consumer aborts an in-flight inference request", + async () => { + await client.start(); + const session = await client.createSession({ onPermissionRequest: approveAll }); + try { + await session.send("Say OK."); + await waitFor(() => handler.inferenceEntered, 60_000); + await session.abort(); + await waitFor(() => handler.sawAbort, 30_000); + } finally { + await session.disconnect(); + } - expect(handler.inferenceEntered, "expected the inference callback to be entered").toBe( - true - ); - expect(handler.sawAbort, "expected the callback to observe runtime cancellation").toBe( - true - ); - }, 90_000); + expect(handler.inferenceEntered, "expected the inference callback to be entered").toBe( + true + ); + expect(handler.sawAbort, "expected the callback to observe runtime cancellation").toBe( + true + ); + }, + 90_000 + ); }); diff --git a/nodejs/test/e2e/harness/sdkTestContext.ts b/nodejs/test/e2e/harness/sdkTestContext.ts index a59f62126d..f2d474449f 100644 --- a/nodejs/test/e2e/harness/sdkTestContext.ts +++ b/nodejs/test/e2e/harness/sdkTestContext.ts @@ -16,6 +16,29 @@ import { formatError, retry } from "./sdkTestHelper"; export const isCI = process.env.GITHUB_ACTIONS === "true"; export const DEFAULT_GITHUB_TOKEN = "fake-token-for-e2e-tests"; +/** + * True when the E2E suite is running over the in-process (FFI) transport + * (COPILOT_SDK_DEFAULT_CONNECTION=inprocess). Use with `it.skipIf` / `describe.skipIf` + * to skip tests for features that are not supported over the in-process transport (the + * runtime loads into the shared host process), so the in-process CI cell stays green. + * Such features are covered by the default (stdio) cell. + */ +export const isInProcessTransport = + (process.env.COPILOT_SDK_DEFAULT_CONNECTION ?? "").toLowerCase() === "inprocess"; + +// The in-process (FFI) transport resolves auth host-side, in this test process, and +// ranks HMAC above the GitHub token — so an ambient COPILOT_HMAC_KEY (CI sets one as a +// job-level credential) would be picked over the SDK/Bearer token the replay snapshots +// expect, yielding 401s. Host-side auth can capture the key as early as client +// construction (before any per-test beforeEach runs), so neutralize it at module load — +// the analogue of .NET's InProcessEnvIsolation `[ModuleInitializer]`. Only applied for +// the in-process transport; stdio/tcp children resolve auth in their own process where +// the token already outranks HMAC. See https://github.com/github/copilot-sdk/issues/1934. +if ((process.env.COPILOT_SDK_DEFAULT_CONNECTION ?? "").toLowerCase() === "inprocess") { + delete process.env.COPILOT_HMAC_KEY; + delete process.env.CAPI_HMAC_KEY; +} + const __filename = fileURLToPath(import.meta.url); const __dirname = dirname(__filename); const SNAPSHOTS_DIR = resolve(__dirname, "../../../../test/snapshots"); @@ -102,11 +125,17 @@ export async function createSdkTestContext({ } else { connection = userConn; } + } else if (useStdio === false) { + connection = RuntimeConnection.forTcp({ path: cliPath }); + } else if ( + useStdio === undefined && + (process.env.COPILOT_SDK_DEFAULT_CONNECTION ?? "").toLowerCase() === "inprocess" + ) { + // The in-process FFI transport resolves the CLI entrypoint itself + // (COPILOT_CLI_PATH or the bundled platform package), so no path is passed. + connection = RuntimeConnection.forInProcess(); } else { - connection = - useStdio === false - ? RuntimeConnection.forTcp({ path: cliPath }) - : RuntimeConnection.forStdio({ path: cliPath }); + connection = RuntimeConnection.forStdio({ path: cliPath }); } const { @@ -114,9 +143,35 @@ export async function createSdkTestContext({ env: userEnv, ...remainingClientOptions } = copilotClientOptions ?? {}; + + const mergedEnv = { ...env, ...userEnv }; + + // The in-process (FFI) transport loads the runtime into this test host process, + // and its worker inherits this process's ambient environment rather than a + // per-client env block (see https://github.com/github/copilot-sdk/issues/1934). + // So the per-test redirects, isolated home, and credentials must be mirrored onto + // the real process environment. Node's `process.env` writes reach native `getenv`, + // so host-side runtime reads (auth resolution, GitHub API redirect) observe them. + // Auth flows via GH_TOKEN/GITHUB_TOKEN here (the FFI argv omits the stdio + // `--auth-token-env COPILOT_SDK_AUTH_TOKEN` wiring), and HMAC is disabled so + // host-side auth resolution picks the SDK/Bearer token the replay snapshots expect. + const isInProcess = connection.kind === "inprocess"; + const inProcessEnv: Record = isInProcess + ? { + ...(mergedEnv as Record), + GH_TOKEN: authTokenToUse, + GITHUB_TOKEN: authTokenToUse, + COPILOT_HMAC_KEY: "", + CAPI_HMAC_KEY: "", + } + : {}; + const copilotClient = new CopilotClient({ workingDirectory: workDir, - env: { ...env, ...userEnv }, + // In-process hosting mirrors the environment onto the real process (per test, in + // beforeEach below), so the worker inherits it; passing a per-client env here + // would have no effect. + env: isInProcess ? undefined : mergedEnv, logLevel: logLevel || "error", connection, gitHubToken: authTokenToUse, @@ -128,6 +183,9 @@ export async function createSdkTestContext({ // Track if any test fails to avoid writing corrupted snapshots let anyTestFailed = false; + // Holds the process.env entries the current test overwrote, so afterEach restores them. + let restoreProcessEnv: Array<[string, string | undefined]> = []; + // Wire up to Vitest lifecycle beforeEach(async (testContext) => { // Must be inside beforeEach - vitest requires test context @@ -135,6 +193,16 @@ export async function createSdkTestContext({ anyTestFailed = true; }); + // Mirror this context's environment onto the real process for in-process + // hosting, right before the test runs (see the comment above the client). The + // client auto-starts on first use inside the test body, so the worker spawns + // under these values. + restoreProcessEnv = []; + for (const [key, value] of Object.entries(inProcessEnv)) { + restoreProcessEnv.push([key, process.env[key]]); + process.env[key] = value; + } + await openAiEndpoint.updateConfig({ filePath: getTrafficCapturePath(testContext), workDir, @@ -146,6 +214,15 @@ export async function createSdkTestContext({ }); afterEach(async () => { + // Undo this test's process.env mirror so it can't leak into the next test/suite. + for (const [key, previous] of restoreProcessEnv.reverse()) { + if (previous === undefined) { + delete process.env[key]; + } else { + process.env[key] = previous; + } + } + restoreProcessEnv = []; // Empty directories but leave them in place for next test await rimraf([join(homeDir, "*"), join(workDir, "*")], { glob: true }); }); diff --git a/nodejs/test/e2e/inprocess_ffi.e2e.test.ts b/nodejs/test/e2e/inprocess_ffi.e2e.test.ts new file mode 100644 index 0000000000..28b0380a96 --- /dev/null +++ b/nodejs/test/e2e/inprocess_ffi.e2e.test.ts @@ -0,0 +1,37 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------------------------------------------*/ + +import { describe, expect, it, onTestFinished } from "vitest"; +import { CopilotClient, RuntimeConnection } from "../../src/index.js"; + +function onTestFinishedForceStop(client: CopilotClient) { + onTestFinished(async () => { + try { + await client.forceStop(); + } catch { + // Ignore cleanup errors - process may already be stopped + } + }); +} + +describe("In-process FFI transport", () => { + // Mirrors the .NET `Should_Start_And_Connect_Over_InProcess_Ffi`. Resolution of the + // in-process transport from COPILOT_SDK_DEFAULT_CONNECTION is exercised by the full + // E2E suite running under the `inprocess` CI matrix cell, not a dedicated test. + it("should start and connect over in-process FFI", async () => { + // In-process FFI hosting resolves the CLI entrypoint (COPILOT_CLI_PATH or the + // bundled platform package) and its sibling native runtime library itself. If + // neither is available, start() throws and the test fails hard. + const client = new CopilotClient({ connection: RuntimeConnection.forInProcess() }); + onTestFinishedForceStop(client); + + await client.start(); + + const pong = await client.ping("ffi message"); + expect(pong.message).toBe("pong: ffi message"); + expect(Date.parse(pong.timestamp)).not.toBeNaN(); + + expect(await client.stop()).toHaveLength(0); // No errors on stop + }); +}); diff --git a/nodejs/test/e2e/rpc_workspace_checkpoints.e2e.test.ts b/nodejs/test/e2e/rpc_workspace_checkpoints.e2e.test.ts index d7f478e1f1..a09fbb1f71 100644 --- a/nodejs/test/e2e/rpc_workspace_checkpoints.e2e.test.ts +++ b/nodejs/test/e2e/rpc_workspace_checkpoints.e2e.test.ts @@ -5,7 +5,7 @@ import { existsSync, readFileSync } from "node:fs"; import { describe, expect, it } from "vitest"; import { approveAll } from "../../src/index.js"; -import { createSdkTestContext } from "./harness/sdkTestContext.js"; +import { createSdkTestContext, isInProcessTransport } from "./harness/sdkTestContext.js"; describe("Session workspace checkpoint RPC", async () => { const { copilotClient: client } = await createSdkTestContext(); @@ -20,17 +20,25 @@ describe("Session workspace checkpoint RPC", async () => { } }); - it("should return null or empty content for unknown checkpoint", async () => { - const session = await client.createSession({ onPermissionRequest: approveAll }); - try { - const result = await session.rpc.workspaces.readCheckpoint({ - number: Number.MAX_SAFE_INTEGER, - }); - expect(result.content ?? "").toBe(""); - } finally { - await session.disconnect(); + // Over the in-process transport session.workspaces.readCheckpoint is answered by the + // native runtime, which decodes the checkpoint number as a u32 and rejects the + // MAX_SAFE_INTEGER sentinel this test uses. Covered by the default (stdio) cell; the + // in-process u32 limitation is a runtime gap tracked in + // https://github.com/github/copilot-sdk/issues/1934. + it.skipIf(isInProcessTransport)( + "should return null or empty content for unknown checkpoint", + async () => { + const session = await client.createSession({ onPermissionRequest: approveAll }); + try { + const result = await session.rpc.workspaces.readCheckpoint({ + number: Number.MAX_SAFE_INTEGER, + }); + expect(result.content ?? "").toBe(""); + } finally { + await session.disconnect(); + } } - }); + ); it("should return typed workspace diff result", async () => { const session = await client.createSession({ onPermissionRequest: approveAll }); diff --git a/nodejs/test/e2e/session.e2e.test.ts b/nodejs/test/e2e/session.e2e.test.ts index c29f0790d0..d1435cf824 100644 --- a/nodejs/test/e2e/session.e2e.test.ts +++ b/nodejs/test/e2e/session.e2e.test.ts @@ -388,7 +388,7 @@ describe("Sessions", async () => { gitHubToken: isCI ? "fake-token-for-e2e-tests" : undefined, }); - onTestFinished(() => newClient.forceStop()); + onTestFinished(() => newClient.stop()); const session2 = await newClient.resumeSession(sessionId, { onPermissionRequest: approveAll, }); @@ -487,7 +487,7 @@ describe("Sessions", async () => { ? DEFAULT_GITHUB_TOKEN : (process.env.GITHUB_TOKEN ?? DEFAULT_GITHUB_TOKEN), }); - onTestFinished(() => newClient.forceStop()); + onTestFinished(() => newClient.stop()); const session2 = await newClient.resumeSession(sessionId, { onPermissionRequest: approveAll, diff --git a/nodejs/test/e2e/streaming_fidelity.e2e.test.ts b/nodejs/test/e2e/streaming_fidelity.e2e.test.ts index 52f893469a..ec4f5cc6bd 100644 --- a/nodejs/test/e2e/streaming_fidelity.e2e.test.ts +++ b/nodejs/test/e2e/streaming_fidelity.e2e.test.ts @@ -85,7 +85,7 @@ describe("Streaming Fidelity", async () => { env, gitHubToken: isCI ? "fake-token-for-e2e-tests" : process.env.GITHUB_TOKEN, }); - onTestFinished(() => newClient.forceStop()); + onTestFinished(() => newClient.stop()); const session2 = await newClient.resumeSession(session.sessionId, { onPermissionRequest: approveAll, streaming: true, @@ -124,7 +124,7 @@ describe("Streaming Fidelity", async () => { env, gitHubToken: isCI ? "fake-token-for-e2e-tests" : process.env.GITHUB_TOKEN, }); - onTestFinished(() => newClient.forceStop()); + onTestFinished(() => newClient.stop()); const session2 = await newClient.resumeSession(session.sessionId, { onPermissionRequest: approveAll, streaming: false, diff --git a/nodejs/test/e2e/telemetry.e2e.test.ts b/nodejs/test/e2e/telemetry.e2e.test.ts index 9df6b7f88e..4fe3612f79 100644 --- a/nodejs/test/e2e/telemetry.e2e.test.ts +++ b/nodejs/test/e2e/telemetry.e2e.test.ts @@ -7,7 +7,7 @@ import { join } from "path"; import { describe, expect, it } from "vitest"; import { z } from "zod"; import { approveAll, defineTool } from "../../src/index.js"; -import { createSdkTestContext } from "./harness/sdkTestContext.js"; +import { createSdkTestContext, isInProcessTransport } from "./harness/sdkTestContext.js"; import { getFinalAssistantMessage } from "./harness/sdkTestHelper.js"; interface TelemetryEntry { @@ -67,86 +67,94 @@ describe("Telemetry export", async () => { }, }); - it("should export file telemetry for sdk interactions", { timeout: 90_000 }, async () => { - const session = await client.createSession({ - onPermissionRequest: approveAll, - tools: [ - defineTool(toolName, { - description: "Echoes a marker string for telemetry validation.", - parameters: z.object({ value: z.string() }), - handler: ({ value }) => value, - }), - ], - }); - - await session.send({ prompt }); - const assistantMessage = await getFinalAssistantMessage(session); - expect(assistantMessage).toBeDefined(); - expect(assistantMessage.data.content ?? "").toContain("TELEMETRY_E2E_DONE"); - - await session.disconnect(); - await client.stop(); - - // Telemetry exporter writes to telemetryFileName resolved relative to the CLI cwd (workDir). - const telemetryPath = join(workDir, telemetryFileName); - const entries = await readTelemetryEntries(telemetryPath); - const spans = entries.filter((entry) => entry.type === "span"); - - expect(spans.length).toBeGreaterThan(0); - for (const span of spans) { - expect(span.instrumentationScope?.name).toBe(sourceName); - } - - // All spans for one SDK turn must share the same trace id and must not be in error state. - const traceIds = Array.from( - new Set(spans.map((span) => span.traceId).filter((id): id is string => Boolean(id))) - ); - expect(traceIds).toHaveLength(1); - for (const span of spans) { - expect(span.status?.code).not.toBe(2); - } - - const invokeAgentSpan = spans.find( - (span) => getStringAttribute(span, "gen_ai.operation.name") === "invoke_agent" - ); - expect(invokeAgentSpan).toBeDefined(); - expect(getStringAttribute(invokeAgentSpan!, "gen_ai.conversation.id")).toBe( - session.sessionId - ); - expect(isRootSpan(invokeAgentSpan!)).toBe(true); - const invokeAgentSpanId = invokeAgentSpan!.spanId; - expect(invokeAgentSpanId).toBeTruthy(); - - const chatSpans = spans.filter( - (span) => getStringAttribute(span, "gen_ai.operation.name") === "chat" - ); - expect(chatSpans.length).toBeGreaterThan(0); - for (const chat of chatSpans) { - expect(chat.parentSpanId).toBe(invokeAgentSpanId); - } - expect( - chatSpans.some((span) => - (getStringAttribute(span, "gen_ai.input.messages") ?? "").includes(prompt) - ) - ).toBe(true); - expect( - chatSpans.some((span) => - (getStringAttribute(span, "gen_ai.output.messages") ?? "").includes( - "TELEMETRY_E2E_DONE" + // Telemetry is configured via environment variables the runtime reads, which the + // in-process transport cannot carry per-client (the runtime runs in the shared host + // process); see https://github.com/github/copilot-sdk/issues/1934. Covered by the + // default (stdio) cell. + it.skipIf(isInProcessTransport)( + "should export file telemetry for sdk interactions", + { timeout: 90_000 }, + async () => { + const session = await client.createSession({ + onPermissionRequest: approveAll, + tools: [ + defineTool(toolName, { + description: "Echoes a marker string for telemetry validation.", + parameters: z.object({ value: z.string() }), + handler: ({ value }) => value, + }), + ], + }); + + await session.send({ prompt }); + const assistantMessage = await getFinalAssistantMessage(session); + expect(assistantMessage).toBeDefined(); + expect(assistantMessage.data.content ?? "").toContain("TELEMETRY_E2E_DONE"); + + await session.disconnect(); + await client.stop(); + + // Telemetry exporter writes to telemetryFileName resolved relative to the CLI cwd (workDir). + const telemetryPath = join(workDir, telemetryFileName); + const entries = await readTelemetryEntries(telemetryPath); + const spans = entries.filter((entry) => entry.type === "span"); + + expect(spans.length).toBeGreaterThan(0); + for (const span of spans) { + expect(span.instrumentationScope?.name).toBe(sourceName); + } + + // All spans for one SDK turn must share the same trace id and must not be in error state. + const traceIds = Array.from( + new Set(spans.map((span) => span.traceId).filter((id): id is string => Boolean(id))) + ); + expect(traceIds).toHaveLength(1); + for (const span of spans) { + expect(span.status?.code).not.toBe(2); + } + + const invokeAgentSpan = spans.find( + (span) => getStringAttribute(span, "gen_ai.operation.name") === "invoke_agent" + ); + expect(invokeAgentSpan).toBeDefined(); + expect(getStringAttribute(invokeAgentSpan!, "gen_ai.conversation.id")).toBe( + session.sessionId + ); + expect(isRootSpan(invokeAgentSpan!)).toBe(true); + const invokeAgentSpanId = invokeAgentSpan!.spanId; + expect(invokeAgentSpanId).toBeTruthy(); + + const chatSpans = spans.filter( + (span) => getStringAttribute(span, "gen_ai.operation.name") === "chat" + ); + expect(chatSpans.length).toBeGreaterThan(0); + for (const chat of chatSpans) { + expect(chat.parentSpanId).toBe(invokeAgentSpanId); + } + expect( + chatSpans.some((span) => + (getStringAttribute(span, "gen_ai.input.messages") ?? "").includes(prompt) ) - ) - ).toBe(true); - - const toolSpan = spans.find( - (span) => getStringAttribute(span, "gen_ai.operation.name") === "execute_tool" - ); - expect(toolSpan).toBeDefined(); - expect(toolSpan!.parentSpanId).toBe(invokeAgentSpanId); - expect(getStringAttribute(toolSpan!, "gen_ai.tool.name")).toBe(toolName); - expect(getStringAttribute(toolSpan!, "gen_ai.tool.call.id")).toBeTruthy(); - expect(getStringAttribute(toolSpan!, "gen_ai.tool.call.arguments")).toBe( - `{"value":"${marker}"}` - ); - expect(getStringAttribute(toolSpan!, "gen_ai.tool.call.result")).toBe(marker); - }); + ).toBe(true); + expect( + chatSpans.some((span) => + (getStringAttribute(span, "gen_ai.output.messages") ?? "").includes( + "TELEMETRY_E2E_DONE" + ) + ) + ).toBe(true); + + const toolSpan = spans.find( + (span) => getStringAttribute(span, "gen_ai.operation.name") === "execute_tool" + ); + expect(toolSpan).toBeDefined(); + expect(toolSpan!.parentSpanId).toBe(invokeAgentSpanId); + expect(getStringAttribute(toolSpan!, "gen_ai.tool.name")).toBe(toolName); + expect(getStringAttribute(toolSpan!, "gen_ai.tool.call.id")).toBeTruthy(); + expect(getStringAttribute(toolSpan!, "gen_ai.tool.call.arguments")).toBe( + `{"value":"${marker}"}` + ); + expect(getStringAttribute(toolSpan!, "gen_ai.tool.call.result")).toBe(marker); + } + ); }); From 7982b26fdaf7d1479766bef6c85b134add94ac30 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 9 Jul 2026 13:34:15 +0100 Subject: [PATCH 02/18] Address review: run checkpoint test on all transports; drop .NET reference - rpc_workspace_checkpoints: replace the MAX_SAFE_INTEGER "unknown checkpoint" sentinel with a 32-bit-safe value (4294967294) so the test runs on the in-process transport too, and remove the skipIf(isInProcessTransport). The runtime narrows the checkpoint number to u32 only at the napi wire arg and immediately widens it back to u64, so any real (small) checkpoint number works; only the pathological sentinel exceeded u32. - inprocess_ffi: describe the Node smoke test on its own terms instead of referencing the .NET test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/test/e2e/inprocess_ffi.e2e.test.ts | 7 +++-- .../e2e/rpc_workspace_checkpoints.e2e.test.ts | 30 +++++++------------ 2 files changed, 15 insertions(+), 22 deletions(-) diff --git a/nodejs/test/e2e/inprocess_ffi.e2e.test.ts b/nodejs/test/e2e/inprocess_ffi.e2e.test.ts index 28b0380a96..639c276d5e 100644 --- a/nodejs/test/e2e/inprocess_ffi.e2e.test.ts +++ b/nodejs/test/e2e/inprocess_ffi.e2e.test.ts @@ -16,9 +16,10 @@ function onTestFinishedForceStop(client: CopilotClient) { } describe("In-process FFI transport", () => { - // Mirrors the .NET `Should_Start_And_Connect_Over_InProcess_Ffi`. Resolution of the - // in-process transport from COPILOT_SDK_DEFAULT_CONNECTION is exercised by the full - // E2E suite running under the `inprocess` CI matrix cell, not a dedicated test. + // Smoke test that the in-process FFI transport starts and completes a round-trip. + // Resolution of the in-process transport from COPILOT_SDK_DEFAULT_CONNECTION is + // exercised by the full E2E suite running under the `inprocess` CI matrix cell, + // not a dedicated test. it("should start and connect over in-process FFI", async () => { // In-process FFI hosting resolves the CLI entrypoint (COPILOT_CLI_PATH or the // bundled platform package) and its sibling native runtime library itself. If diff --git a/nodejs/test/e2e/rpc_workspace_checkpoints.e2e.test.ts b/nodejs/test/e2e/rpc_workspace_checkpoints.e2e.test.ts index a09fbb1f71..78a820f67a 100644 --- a/nodejs/test/e2e/rpc_workspace_checkpoints.e2e.test.ts +++ b/nodejs/test/e2e/rpc_workspace_checkpoints.e2e.test.ts @@ -5,7 +5,7 @@ import { existsSync, readFileSync } from "node:fs"; import { describe, expect, it } from "vitest"; import { approveAll } from "../../src/index.js"; -import { createSdkTestContext, isInProcessTransport } from "./harness/sdkTestContext.js"; +import { createSdkTestContext } from "./harness/sdkTestContext.js"; describe("Session workspace checkpoint RPC", async () => { const { copilotClient: client } = await createSdkTestContext(); @@ -20,25 +20,17 @@ describe("Session workspace checkpoint RPC", async () => { } }); - // Over the in-process transport session.workspaces.readCheckpoint is answered by the - // native runtime, which decodes the checkpoint number as a u32 and rejects the - // MAX_SAFE_INTEGER sentinel this test uses. Covered by the default (stdio) cell; the - // in-process u32 limitation is a runtime gap tracked in - // https://github.com/github/copilot-sdk/issues/1934. - it.skipIf(isInProcessTransport)( - "should return null or empty content for unknown checkpoint", - async () => { - const session = await client.createSession({ onPermissionRequest: approveAll }); - try { - const result = await session.rpc.workspaces.readCheckpoint({ - number: Number.MAX_SAFE_INTEGER, - }); - expect(result.content ?? "").toBe(""); - } finally { - await session.disconnect(); - } + it("should return null or empty content for unknown checkpoint", async () => { + const session = await client.createSession({ onPermissionRequest: approveAll }); + try { + // A high but 32-bit-safe checkpoint number that will never exist in a fresh + // session, so the read reports the checkpoint as missing. + const result = await session.rpc.workspaces.readCheckpoint({ number: 4294967294 }); + expect(result.content ?? "").toBe(""); + } finally { + await session.disconnect(); } - ); + }); it("should return typed workspace diff result", async () => { const session = await client.createSession({ onPermissionRequest: approveAll }); From 51a204f671c0d1853dfdf5dc118dca707c0db3d4 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 9 Jul 2026 13:51:25 +0100 Subject: [PATCH 03/18] Revalidate FFI host workarounds against fixed runtime; drop the ones no longer needed With the runtime worker-starvation/threading fault fixed (runtime 1.0.70-0), the in-process FFI host reduces to standard koffi. Revalidated on Windows (inprocess permissions/streaming/session/checkpoint suites, plus a bare-node event-loop probe) and removed the compensating workarounds: - Broker pump: the continuous log_dropped_count async FFI call is gone. koffi delivers the foreign-thread outbound callback on the event loop via a threadsafe function; idle-turn model responses arrive without pumping. - Deferred inbound delivery: feedInbound writes straight to receiveStream (the callback already runs on a loop tick, not nested on the runtime's stack, so there is no reentrancy to defer around). - Deferred callback unregister: koffi.unregister runs synchronously in dispose; no DEP0168 at teardown. Kept, because it is inherent to in-process hosting rather than a workaround: a referenced keep-alive timer. The FFI transport has no socket/pipe handle, and koffi's callback TSFN does not reference the loop, so a bare-node app would otherwise exit mid-turn (probe exited with code 13 without it). Also sharpen the copilot_request_cancel_error inproc-skip comment: confirmed the runtime enforces a single process-wide LLM inference provider (a second requestHandler client's llmInference.setProvider is rejected), so that skip is legitimate and stays. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/src/ffiRuntimeHost.ts | 153 ++++-------------- .../copilot_request_cancel_error.e2e.test.ts | 10 +- 2 files changed, 35 insertions(+), 128 deletions(-) diff --git a/nodejs/src/ffiRuntimeHost.ts b/nodejs/src/ffiRuntimeHost.ts index f90baa39e0..e6b0652618 100644 --- a/nodejs/src/ffiRuntimeHost.ts +++ b/nodejs/src/ffiRuntimeHost.ts @@ -24,6 +24,10 @@ import { PassThrough, Writable } from "node:stream"; const SYMBOL_PREFIX = "copilot_runtime_"; +// A long, referenced no-op timer keeps the Node event loop alive while the in-process +// connection is open (see start()); the exact interval is irrelevant. +const KEEP_ALIVE_INTERVAL_MS = 1 << 30; + type KoffiFunction = ReturnType["func"]>; type KoffiType = ReturnType; type KoffiRegisteredCallback = ReturnType; @@ -34,7 +38,6 @@ interface FfiLibrary { connectionOpen: KoffiFunction; connectionWrite: KoffiFunction; connectionClose: KoffiFunction; - logDroppedCount: KoffiFunction; outboundCallbackType: KoffiType; } @@ -88,10 +91,6 @@ function loadLibrary(libraryPath: string): FfiLibrary { "size_t", ]), connectionClose: lib.func(`${SYMBOL_PREFIX}connection_close`, "bool", ["uint32"]), - // A no-argument, side-effect-free diagnostic getter (returns a dropped-log - // counter). Used purely as a cheap async FFI call to keep koffi's asynchronous - // callback broker serviced while the connection is idle (see the pump in start()). - logDroppedCount: lib.func(`${SYMBOL_PREFIX}log_dropped_count`, "uint64", []), outboundCallbackType, }; loadedLibraryPath = libraryPath; @@ -129,36 +128,7 @@ export class FfiRuntimeHost { private connectionId = 0; private disposed = false; private outboundCallback: KoffiRegisteredCallback | undefined; - /** - * Keeps koffi's asynchronous callback broker serviced while the FFI connection is - * open, so inbound native→JS frames are delivered promptly even when the SDK is - * otherwise idle (e.g. awaiting a model response with no client→server writes in - * flight). - * - * The runtime invokes our outbound callback from a worker thread. koffi marshals - * such foreign-thread callbacks to the JS main thread via a threadsafe-function - * "broker" that is only serviced WHILE a koffi asynchronous FFI call is in flight — - * a plain JS timer (setInterval) does NOT pump it. During active request/response - * traffic the constant `connection_write` calls keep frames flowing, but once the - * SDK goes idle mid-turn (e.g. the worker is awaiting a model HTTP response, so - * there are no client→server writes) the broker parks and the next outbound frame - * (the model response, then everything after it) sits undelivered until the next - * koffi call — observed as a permanent 30s stall of otherwise-healthy sessions on - * macOS/Windows (libuv services the broker differently on Linux, which is why it - * only reproduces off-Linux). Keeping exactly one cheap async FFI call - * (`log_dropped_count`) continuously in flight keeps the broker pumping so - * foreign-thread frames are always delivered. This is the koffi analogue of the - * .NET host's raw function-pointer callback, which runs directly on the runtime - * thread and needs no event-loop pumping. - */ - private pumpActive = false; - /** - * Inbound frame bytes copied out of the native callback, awaiting delivery to - * {@link receiveStream} on a clean event-loop tick. See {@link feedInbound}. - */ - private readonly inboundQueue: Buffer[] = []; - /** Whether a drain of {@link inboundQueue} is already scheduled. */ - private drainScheduled = false; + private keepAliveTimer: ReturnType | undefined; /** The stream JSON-RPC reads server→client frames from. */ readonly receiveStream: PassThrough; @@ -174,8 +144,8 @@ export class FfiRuntimeHost { this.lib = loadLibrary(libraryPath); this.receiveStream = new PassThrough(); this.sendStream = new Writable({ - // Write frames with a synchronous FFI call (restored while diagnosing the - // macOS/Windows inbound-callback stall; async writes were a no-op for it). + // connection_write enqueues the frame into the runtime's inbound channel and + // returns immediately, so a synchronous FFI call is sufficient here. write: (chunk: Buffer, _encoding, callback) => { try { this.writeFrame(chunk); @@ -278,35 +248,11 @@ export class FfiRuntimeHost { throw new Error("copilot_runtime_connection_open failed."); } - this.startBrokerPump(); - } - - /** - * Keeps exactly one cheap async FFI call in flight at all times so koffi's - * threadsafe-function broker stays serviced and foreign-thread outbound callbacks - * are delivered without waiting for the next client→server write. Each call - * reschedules the next on completion; the in-flight libuv request also keeps the - * event loop alive (replacing the old keep-alive timer). Stops when disposed. - */ - private startBrokerPump(): void { - if (this.pumpActive) { - return; - } - this.pumpActive = true; - const pump = (): void => { - if (this.disposed || !this.connectionId) { - this.pumpActive = false; - return; - } - this.lib.logDroppedCount.async((_error: Error | null, _result: unknown) => { - if (this.disposed || !this.connectionId) { - this.pumpActive = false; - return; - } - pump(); - }); - }; - pump(); + // 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 + // outbound callback on the loop but does not reference it, so hold one referenced + // timer for the lifetime of the connection. + this.keepAliveTimer = setInterval(() => {}, KEEP_ALIVE_INTERVAL_MS); } private writeFrame(frame: Buffer): void { @@ -320,25 +266,15 @@ export class FfiRuntimeHost { } /** - * Native outbound callback: copies the inbound frame bytes and hands them to the - * event loop for delivery, WITHOUT driving the JSON-RPC reader synchronously here. - * - * The native pointer is only valid for the duration of this call, so the bytes are - * decoded/copied eagerly; but writing them to {@link receiveStream} (which - * synchronously drives frame parsing and JSON-RPC dispatch, and may re-enter the - * SDK's write path) is deferred to a `setImmediate` drain on a clean stack. This - * keeps the callback minimal and non-reentrant — the koffi analogue of the .NET - * host's thread-safe `Channel` callback, which enqueues and returns immediately. - * Delivering synchronously here could re-enter a native `connection_write` call - * still on the stack and deadlock (observed as hung `session.rpc.*` round-trips - * on macOS). + * Native outbound (server→client) callback. koffi delivers it on the JS event loop + * via a threadsafe function, so the frame is decoded and written straight to + * {@link receiveStream}. The native pointer is only valid for this call, so the + * bytes are copied out before returning. */ private feedInbound(bytesPtr: unknown, bytesLen: number | bigint): void { - // This runs as a native→JS (Node-API) callback, possibly from a secondary - // thread. An exception thrown here cannot propagate across the FFI boundary and - // is swallowed by the runtime (surfacing only as a DEP0168 "Uncaught Node-API - // callback exception" warning), so catch and log it here instead of letting it - // escape. + // An exception thrown across the native→JS (Node-API) boundary cannot propagate + // and would surface only as a DEP0168 "uncaught Node-API callback exception" + // warning, so catch and log it here instead of letting it escape. try { const length = Number(bytesLen); if (!bytesPtr || length <= 0) { @@ -348,11 +284,7 @@ export class FfiRuntimeHost { bytesPtr, koffi.array("uint8", length, "Typed") ) as Uint8Array; - this.inboundQueue.push(Buffer.from(bytes)); - if (!this.drainScheduled) { - this.drainScheduled = true; - setImmediate(() => this.drainInbound()); - } + this.receiveStream.write(Buffer.from(bytes)); } catch (error) { console.error( `In-process FFI inbound callback failed: ${error instanceof Error ? (error.stack ?? error.message) : String(error)}` @@ -360,45 +292,17 @@ export class FfiRuntimeHost { } } - /** Delivers queued inbound frames to {@link receiveStream} on a clean event-loop tick. */ - private drainInbound(): void { - this.drainScheduled = false; - if (this.disposed) { - this.inboundQueue.length = 0; - return; - } - let frame: Buffer | undefined; - while ((frame = this.inboundQueue.shift()) !== undefined) { - this.receiveStream.write(frame); - } - } - private unregisterCallback(): void { if (this.outboundCallback === undefined) { return; } const callback = this.outboundCallback; this.outboundCallback = undefined; - // Defer the unregister to a later tick instead of unregistering synchronously. - // koffi delivers outbound callbacks from a secondary thread by queuing them onto - // the JS event loop; at teardown one such delivery can still be queued after we - // stop the native side. Unregistering while koffi still has a queued call makes - // koffi invoke a torn-down callback and raise inside its own native code — an - // uncaught Node-API callback exception (DEP0168) that no JS try/catch can catch. - // The native connection/host are already closed by the time this runs (see - // dispose), so no new deliveries originate; a queued delivery fires in libuv's - // poll phase and setImmediate (check phase) runs right after it in the same loop - // iteration, so the pending delivery (a no-op, since we are disposed) drains - // before we free the slot. - const immediate = setImmediate(() => { - try { - koffi.unregister(callback); - } catch { - // Ignore teardown failures. - } - }); - // Don't let this housekeeping timer keep the process alive. - immediate.unref?.(); + try { + koffi.unregister(callback); + } catch { + // Ignore teardown failures. + } } /** Closes the FFI connection, shuts down the native host, and releases resources. */ @@ -408,9 +312,10 @@ export class FfiRuntimeHost { } this.disposed = true; - // The broker pump observes `disposed` and stops rescheduling on its next - // completion; no timer to clear. - this.pumpActive = false; + if (this.keepAliveTimer !== undefined) { + clearInterval(this.keepAliveTimer); + this.keepAliveTimer = undefined; + } try { if (this.connectionId) { diff --git a/nodejs/test/e2e/copilot_request_cancel_error.e2e.test.ts b/nodejs/test/e2e/copilot_request_cancel_error.e2e.test.ts index 6b3ff5c437..69bacd4f6e 100644 --- a/nodejs/test/e2e/copilot_request_cancel_error.e2e.test.ts +++ b/nodejs/test/e2e/copilot_request_cancel_error.e2e.test.ts @@ -162,10 +162,12 @@ describe("CopilotRequestHandler observes runtime cancellation", async () => { copilotClientOptions: { requestHandler: handler }, }); - // A CopilotRequestHandler registers a process-wide LLM inference provider. Over the - // in-process transport every client shares the one host process, so a second client - // cannot register while another still holds the provider. Covered by the default - // (stdio) cell, where each client owns its own process. + // The runtime enforces a single, process-wide LLM inference provider: a second + // client.start() with a requestHandler rejects llmInference.setProvider with + // "Another client is already the LLM inference provider." The sibling error test + // above already registers a provider and holds it for this file's lifetime, and + // inproc runs share one runtime host, so this scenario can only run on the default + // (stdio) cell, where each client owns its own runtime process. it.skipIf(isInProcessTransport)( "fires ctx.signal when the consumer aborts an in-flight inference request", async () => { From 0296b1b611a5e3dc167a7acf40fdbf695eccb7df Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 9 Jul 2026 14:39:54 +0100 Subject: [PATCH 04/18] Fix e2e auth so tests run locally without an HMAC key The shared e2e harness blanked GH_TOKEN/GITHUB_TOKEN to "" and relied on an ambient COPILOT_HMAC_KEY (a CI-only secret) to supply the credential. Locally there is no HMAC key, so tests that create their own client without passing gitHubToken (the stdio/tcp "works without onPermissionRequest" cases) had no credential and failed with a 401. Use the proxy-recognized token instead so these authenticate against the replay proxy in both CI and local runs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/test/e2e/harness/sdkTestContext.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/nodejs/test/e2e/harness/sdkTestContext.ts b/nodejs/test/e2e/harness/sdkTestContext.ts index f2d474449f..8872682311 100644 --- a/nodejs/test/e2e/harness/sdkTestContext.ts +++ b/nodejs/test/e2e/harness/sdkTestContext.ts @@ -92,8 +92,14 @@ export async function createSdkTestContext({ COPILOT_HOME: copilotHomeDir, COPILOT_SDK_AUTH_TOKEN: "", GH_CONFIG_DIR: homeDir, - GH_TOKEN: "", - GITHUB_TOKEN: "", + // Use the proxy-recognized token rather than blanking these. Tests that spin up + // their own client without passing `gitHubToken` (e.g. the stdio/tcp + // "works without onPermissionRequest" cases) rely on GH_TOKEN/GITHUB_TOKEN to + // authenticate against the replay proxy. Blanking them only worked on CI, where an + // ambient COPILOT_HMAC_KEY secret supplies the credential instead; locally there is + // no HMAC key, so the child CLI had nothing to authenticate with and got a 401. + GH_TOKEN: authTokenToUse, + GITHUB_TOKEN: authTokenToUse, // TODO: I'm not convinced the SDK should default to using whatever config you happen to have in your homedir. // The SDK config should be independent of the regular CLI app. Likewise it shouldn't mix sessions from the From f642bd6cdff7b4e82a7fda2767cd0d29362aa554 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 9 Jul 2026 14:57:12 +0100 Subject: [PATCH 05/18] Use graceful stop in session.e2e teardown to avoid Windows worker lock The three "works without onPermissionRequest" tests force-stopped their clients in onTestFinished. Force-stopping kills the runtime worker abruptly, so on Windows it can still hold the session.db file lock (EBUSY) after the test, stalling the suite's afterAll teardown. Shut the clients down gracefully instead, matching the rest of the file. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/test/e2e/session.e2e.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/nodejs/test/e2e/session.e2e.test.ts b/nodejs/test/e2e/session.e2e.test.ts index d1435cf824..e26ad57811 100644 --- a/nodejs/test/e2e/session.e2e.test.ts +++ b/nodejs/test/e2e/session.e2e.test.ts @@ -39,7 +39,7 @@ describe("Sessions", async () => { }); onTestFinished(async () => { try { - await standaloneClient.forceStop(); + await standaloneClient.stop(); } catch { // ignore } @@ -64,7 +64,7 @@ describe("Sessions", async () => { }); onTestFinished(async () => { try { - await tcpClient.forceStop(); + await tcpClient.stop(); } catch { // ignore } @@ -84,7 +84,7 @@ describe("Sessions", async () => { }); onTestFinished(async () => { try { - await resumeClient.forceStop(); + await resumeClient.stop(); } catch { // ignore } From 4f8f8cb22b4d4e83c405d54b589ebe798cab2154 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 9 Jul 2026 16:37:26 +0100 Subject: [PATCH 06/18] Abort in-flight session turns before shutdown to release session.db handle A turn still running when the runtime disposes the session can leave that session's SQLite session.db handle open. Over the in-process transport the runtime shares the test process, so the handle is not reclaimed and the file stays locked on Windows, causing EBUSY/ENOTEMPTY when removing the session-state directory during teardown. Abort each session before disconnect so the turn cancels and releases the handle. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/src/client.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index b0e2fcbb9a..fac6e0ee2b 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -907,6 +907,15 @@ export class CopilotClient { // Disconnect all active sessions with retry logic const activeSessions = [...this.sessions.values()]; + // Abort any in-flight turn before teardown. A turn still running when + // the runtime disposes the session can leave that session's SQLite + // session.db handle open in the runtime; over the in-process transport + // the runtime shares this process, so the handle is not reclaimed by + // terminating a child and the file stays locked (Windows), preventing + // removal of the session-state directory. Aborting lets the turn cancel + // and release the handle. Best-effort and idempotent: a session with no + // active turn is a no-op. + await Promise.allSettled(activeSessions.map((session) => session.abort())); for (const session of activeSessions) { const sessionId = session.sessionId; let lastError: Error | null = null; From 5ccfc9fac2f696760fc7b2446cab812c42ef1e4e Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 9 Jul 2026 19:57:17 +0100 Subject: [PATCH 07/18] fix: scope teardown abort to owned runtimes; gate inproc-only e2e tests Only abort in-flight session turns on teardown for SDK-owned runtimes, so a connecting client against an external server leaves its pending work intact for resume. Gate the two disconnect-driven capability tests (multi-client, ui_elicitation) with it.skipIf(isInProcessTransport) since the in-process transport shares the host process. For 'should delete session by id', abort the session before disconnect/delete so the delete-triggered reload leaves no unauthenticated background promise to reject host-side after the harness env teardown. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/src/client.ts | 8 +- nodejs/test/e2e/client_api.e2e.test.ts | 1 + nodejs/test/e2e/multi-client.e2e.test.ts | 128 +++++++++++---------- nodejs/test/e2e/ui_elicitation.e2e.test.ts | 4 +- 4 files changed, 75 insertions(+), 66 deletions(-) diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index fac6e0ee2b..8d76e7f2f6 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -914,8 +914,12 @@ export class CopilotClient { // terminating a child and the file stays locked (Windows), preventing // removal of the session-state directory. Aborting lets the turn cancel // and release the handle. Best-effort and idempotent: a session with no - // active turn is a no-op. - await Promise.allSettled(activeSessions.map((session) => session.abort())); + // active turn is a no-op. Skip for external servers: we don't own that + // runtime, and aborting would cancel pending work other clients may + // still resume. + if (!this.isExternalServer) { + await Promise.allSettled(activeSessions.map((session) => session.abort())); + } for (const session of activeSessions) { const sessionId = session.sessionId; let lastError: Error | null = null; diff --git a/nodejs/test/e2e/client_api.e2e.test.ts b/nodejs/test/e2e/client_api.e2e.test.ts index 4adaad6ec3..46c23cee69 100644 --- a/nodejs/test/e2e/client_api.e2e.test.ts +++ b/nodejs/test/e2e/client_api.e2e.test.ts @@ -44,6 +44,7 @@ describe("Client session management", async () => { await waitFor(async () => (await client.listSessions()).some((s) => s.sessionId === sessionId) ); + await session.abort(); await session.disconnect(); await client.deleteSession(sessionId); diff --git a/nodejs/test/e2e/multi-client.e2e.test.ts b/nodejs/test/e2e/multi-client.e2e.test.ts index a63b1b0ebf..a44ceec3c3 100644 --- a/nodejs/test/e2e/multi-client.e2e.test.ts +++ b/nodejs/test/e2e/multi-client.e2e.test.ts @@ -6,7 +6,7 @@ import { describe, expect, it, afterAll } from "vitest"; import { z } from "zod"; import { CopilotClient, defineTool, approveAll, RuntimeConnection } from "../../src/index.js"; import type { SessionEvent } from "../../src/index.js"; -import { createSdkTestContext } from "./harness/sdkTestContext"; +import { createSdkTestContext, isInProcessTransport } from "./harness/sdkTestContext"; describe("Multi-client broadcast", async () => { // Use TCP mode so a second client can connect to the same CLI process @@ -304,71 +304,75 @@ describe("Multi-client broadcast", async () => { } ); - it("disconnecting client removes its tools", { timeout: 90_000 }, async () => { - const toolA = defineTool("stable_tool", { - description: "A tool that persists across disconnects", - parameters: z.object({ input: z.string() }), - handler: ({ input }) => `STABLE_${input}`, - }); + it.skipIf(isInProcessTransport)( + "disconnecting client removes its tools", + { timeout: 90_000 }, + async () => { + const toolA = defineTool("stable_tool", { + description: "A tool that persists across disconnects", + parameters: z.object({ input: z.string() }), + handler: ({ input }) => `STABLE_${input}`, + }); - const toolB = defineTool("ephemeral_tool", { - description: "A tool that will disappear when its client disconnects", - parameters: z.object({ input: z.string() }), - handler: ({ input }) => `EPHEMERAL_${input}`, - }); + const toolB = defineTool("ephemeral_tool", { + description: "A tool that will disappear when its client disconnects", + parameters: z.object({ input: z.string() }), + handler: ({ input }) => `EPHEMERAL_${input}`, + }); - // Client 1 creates a session with stable_tool - const session1 = await client1.createSession({ - onPermissionRequest: approveAll, - tools: [toolA], - }); + // Client 1 creates a session with stable_tool + const session1 = await client1.createSession({ + onPermissionRequest: approveAll, + tools: [toolA], + }); - // Client 2 resumes with ephemeral_tool - await client2.resumeSession(session1.sessionId, { - onPermissionRequest: approveAll, - tools: [toolB], - }); + // Client 2 resumes with ephemeral_tool + await client2.resumeSession(session1.sessionId, { + onPermissionRequest: approveAll, + tools: [toolB], + }); - // Verify both tools work before disconnect (sequential to avoid nondeterministic tool_call ordering) - const stableResponse = await session1.sendAndWait({ - prompt: "Use the stable_tool with input 'test1' and tell me the result.", - }); - expect(stableResponse?.data.content).toContain("STABLE_test1"); + // Verify both tools work before disconnect (sequential to avoid nondeterministic tool_call ordering) + const stableResponse = await session1.sendAndWait({ + prompt: "Use the stable_tool with input 'test1' and tell me the result.", + }); + expect(stableResponse?.data.content).toContain("STABLE_test1"); - const ephemeralResponse = await session1.sendAndWait({ - prompt: "Use the ephemeral_tool with input 'test2' and tell me the result.", - }); - expect(ephemeralResponse?.data.content).toContain("EPHEMERAL_test2"); - - // Disconnect client 2 without destroying the shared session. - // Suppress "Connection is disposed" rejections that occur when the server - // broadcasts events (e.g. tool_changed_notice) to the now-dead connection. - const suppressDisposed = (reason: unknown) => { - if (reason instanceof Error && reason.message.includes("Connection is disposed")) { - return; - } - throw reason; - }; - process.on("unhandledRejection", suppressDisposed); - await client2.forceStop(); - - // Give the server time to process the connection close and remove tools - await new Promise((resolve) => setTimeout(resolve, 500)); - process.removeListener("unhandledRejection", suppressDisposed); - - // Recreate client2 for cleanup in afterAll (but don't rejoin the session) - client2 = new CopilotClient({ - connection: RuntimeConnection.forUri(`localhost:${runtimePort}`, { - connectionToken: tcpConnectionToken, - }), - }); + const ephemeralResponse = await session1.sendAndWait({ + prompt: "Use the ephemeral_tool with input 'test2' and tell me the result.", + }); + expect(ephemeralResponse?.data.content).toContain("EPHEMERAL_test2"); + + // Disconnect client 2 without destroying the shared session. + // Suppress "Connection is disposed" rejections that occur when the server + // broadcasts events (e.g. tool_changed_notice) to the now-dead connection. + const suppressDisposed = (reason: unknown) => { + if (reason instanceof Error && reason.message.includes("Connection is disposed")) { + return; + } + throw reason; + }; + process.on("unhandledRejection", suppressDisposed); + await client2.forceStop(); + + // Give the server time to process the connection close and remove tools + await new Promise((resolve) => setTimeout(resolve, 500)); + process.removeListener("unhandledRejection", suppressDisposed); + + // Recreate client2 for cleanup in afterAll (but don't rejoin the session) + client2 = new CopilotClient({ + connection: RuntimeConnection.forUri(`localhost:${runtimePort}`, { + connectionToken: tcpConnectionToken, + }), + }); - // Now only stable_tool should be available - const afterResponse = await session1.sendAndWait({ - prompt: "Use the stable_tool with input 'still_here'. Also try using ephemeral_tool if it is available.", - }); - expect(afterResponse?.data.content).toContain("STABLE_still_here"); - // ephemeral_tool should NOT have produced a result - expect(afterResponse?.data.content).not.toContain("EPHEMERAL_"); - }); + // Now only stable_tool should be available + const afterResponse = await session1.sendAndWait({ + prompt: "Use the stable_tool with input 'still_here'. Also try using ephemeral_tool if it is available.", + }); + expect(afterResponse?.data.content).toContain("STABLE_still_here"); + // ephemeral_tool should NOT have produced a result + expect(afterResponse?.data.content).not.toContain("EPHEMERAL_"); + } + ); }); diff --git a/nodejs/test/e2e/ui_elicitation.e2e.test.ts b/nodejs/test/e2e/ui_elicitation.e2e.test.ts index 3bc9335a21..2e85dd5af2 100644 --- a/nodejs/test/e2e/ui_elicitation.e2e.test.ts +++ b/nodejs/test/e2e/ui_elicitation.e2e.test.ts @@ -5,7 +5,7 @@ import { afterAll, describe, expect, it } from "vitest"; import { CopilotClient, approveAll, RuntimeConnection } from "../../src/index.js"; import type { SessionEvent } from "../../src/index.js"; -import { createSdkTestContext } from "./harness/sdkTestContext.js"; +import { createSdkTestContext, isInProcessTransport } from "./harness/sdkTestContext.js"; describe("UI Elicitation", async () => { const { copilotClient: client } = await createSdkTestContext(); @@ -116,7 +116,7 @@ describe("UI Elicitation Multi-Client Capabilities", async () => { } ); - it( + it.skipIf(isInProcessTransport)( "capabilities.changed fires when elicitation provider disconnects", { timeout: 60_000 }, async () => { From d2c1f6cc6f7a88c8d7ddf28ba2728d99d31b320c Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 9 Jul 2026 19:57:32 +0100 Subject: [PATCH 08/18] test: use graceful stop() instead of forceStop() in Node test teardowns Switch client unit tests and e2e teardowns from forceStop() to graceful stop() so in-process (FFI) workers can release their native/SQLite handles and exit cleanly. In client.test.ts this goes through a new stopClient() helper. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/test/client.test.ts | 188 +++++++++--------- nodejs/test/e2e/client.e2e.test.ts | 4 +- nodejs/test/e2e/client_options.e2e.test.ts | 14 +- nodejs/test/e2e/connection_token.test.ts | 4 +- nodejs/test/e2e/inprocess_ffi.e2e.test.ts | 2 +- .../test/e2e/pending_work_resume.e2e.test.ts | 16 +- nodejs/test/e2e/rpc.e2e.test.ts | 2 +- .../test/e2e/rpc_mcp_and_skills.e2e.test.ts | 2 +- nodejs/test/e2e/rpc_mcp_config.e2e.test.ts | 2 +- nodejs/test/e2e/rpc_server.e2e.test.ts | 2 +- nodejs/test/e2e/rpc_server_misc.e2e.test.ts | 4 +- .../test/e2e/rpc_server_plugins.e2e.test.ts | 2 +- .../e2e/rpc_server_remote_control.e2e.test.ts | 2 +- .../e2e/rpc_session_state_extras.e2e.test.ts | 2 +- nodejs/test/e2e/session_fs.e2e.test.ts | 4 +- nodejs/test/e2e/suspend.e2e.test.ts | 2 +- 16 files changed, 130 insertions(+), 122 deletions(-) diff --git a/nodejs/test/client.test.ts b/nodejs/test/client.test.ts index 8605f8d82c..99c1ae5c69 100644 --- a/nodejs/test/client.test.ts +++ b/nodejs/test/client.test.ts @@ -15,10 +15,18 @@ import { defaultJoinSessionPermissionHandler } from "../src/types.js"; // This file is for unit tests. Where relevant, prefer to add e2e tests in e2e/*.test.ts instead +// Tear down a client created during a test. Over the in-process (FFI) transport the +// runtime shares this test process, so clients must shut down gracefully with stop() +// to release their native/SQLite handles and let the vitest worker exit; forceStop() +// would leave them open and the worker would fail to terminate. +async function stopClient(client: CopilotClient): Promise { + await client.stop(); +} + describe("CopilotClient", () => { it("disposes the stdio connection when child stdin emits an error", async () => { const client = new CopilotClient(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const stdin = new PassThrough(); const stdout = new PassThrough(); @@ -128,7 +136,7 @@ describe("CopilotClient", () => { it("registers interest in MCP OAuth required events after create when an auth handler is configured", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi .spyOn((client as any).connection!, "sendRequest") @@ -156,7 +164,7 @@ describe("CopilotClient", () => { it("does not register MCP OAuth interest without an auth handler", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi .spyOn((client as any).connection!, "sendRequest") @@ -183,7 +191,7 @@ describe("CopilotClient", () => { it("registers MCP OAuth interest after cloud create only when an auth handler is configured", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); let cloudCreateCount = 0; const spy = vi @@ -224,7 +232,7 @@ describe("CopilotClient", () => { it("registers MCP OAuth interest after resuming only when an auth handler is configured", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi .spyOn((client as any).connection!, "sendRequest") @@ -278,7 +286,7 @@ describe("CopilotClient", () => { it("forwards canvas declarations and request flags in session.create", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const canvas = createCanvas({ id: "counter", @@ -323,7 +331,7 @@ describe("CopilotClient", () => { it("forwards canvas declarations in session.resume", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll }); const canvas = createCanvas({ @@ -361,7 +369,7 @@ describe("CopilotClient", () => { it("forwards reasoningSummary in session.create and session.resume", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi .spyOn((client as any).connection!, "sendRequest") @@ -393,7 +401,7 @@ describe("CopilotClient", () => { it("forwards contextTier in session.create and session.resume", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi .spyOn((client as any).connection!, "sendRequest") @@ -425,7 +433,7 @@ describe("CopilotClient", () => { it("forwards new session options in session.create and session.resume", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi .spyOn((client as any).connection!, "sendRequest") @@ -465,7 +473,7 @@ describe("CopilotClient", () => { it("opts into GitHub telemetry forwarding when onGitHubTelemetry is provided", async () => { const client = new CopilotClient({ onGitHubTelemetry: () => {} }); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi .spyOn((client as any).connection!, "sendRequest") @@ -490,7 +498,7 @@ describe("CopilotClient", () => { it("opts into GitHub telemetry forwarding on the connect handshake when a handler is provided", async () => { const client = new CopilotClient({ onGitHubTelemetry: () => {} }); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const sendRequest = vi.fn(async (method: string) => { if (method === "connect") return { ok: true, protocolVersion: 3, version: "test" }; @@ -507,7 +515,7 @@ describe("CopilotClient", () => { it("does not opt into GitHub telemetry forwarding on the connect handshake without a handler", async () => { const client = new CopilotClient(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const sendRequest = vi.fn(async (method: string) => { if (method === "connect") return { ok: true, protocolVersion: 3, version: "test" }; @@ -525,7 +533,7 @@ describe("CopilotClient", () => { it("does not opt into GitHub telemetry forwarding without a handler", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi .spyOn((client as any).connection!, "sendRequest") @@ -604,7 +612,7 @@ describe("CopilotClient", () => { it("registers no gitHubTelemetry handler when onGitHubTelemetry is omitted", () => { const client = new CopilotClient(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const handlers = (client as any).clientGlobalHandlers; expect(handlers.gitHubTelemetry).toBeUndefined(); @@ -613,7 +621,7 @@ describe("CopilotClient", () => { it("forwards gitHubTelemetry events to the onGitHubTelemetry handler", () => { const received: GitHubTelemetryNotification[] = []; const client = new CopilotClient({ onGitHubTelemetry: (n) => received.push(n) }); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const handlers = (client as any).clientGlobalHandlers; expect(handlers.gitHubTelemetry).toBeDefined(); @@ -630,7 +638,7 @@ describe("CopilotClient", () => { it("forwards expAssignments in session.create and session.resume", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi .spyOn((client as any).connection!, "sendRequest") @@ -667,7 +675,7 @@ describe("CopilotClient", () => { it("omits expAssignments from session.create and session.resume when unset", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi .spyOn((client as any).connection!, "sendRequest") @@ -693,7 +701,7 @@ describe("CopilotClient", () => { it("forwards capi options in session.create and session.resume", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi .spyOn((client as any).connection!, "sendRequest") @@ -725,7 +733,7 @@ describe("CopilotClient", () => { it("forwards pluginDirectories and largeOutput in session.create and session.resume", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi .spyOn((client as any).connection!, "sendRequest") @@ -966,7 +974,7 @@ describe("CopilotClient", () => { it("forwards clientName in session.create request", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi.spyOn((client as any).connection!, "sendRequest"); await client.createSession({ clientName: "my-app", onPermissionRequest: approveAll }); @@ -980,7 +988,7 @@ describe("CopilotClient", () => { it("forwards cloud options in session.create request", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi .spyOn((client as any).connection!, "sendRequest") @@ -1005,7 +1013,7 @@ describe("CopilotClient", () => { it("forwards clientName in session.resume request", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll }); // Mock sendRequest to capture the call without hitting the runtime @@ -1030,7 +1038,7 @@ describe("CopilotClient", () => { it("forwards enableSessionTelemetry in session.create request", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi.spyOn((client as any).connection!, "sendRequest"); await client.createSession({ @@ -1047,7 +1055,7 @@ describe("CopilotClient", () => { it("forwards enableSessionTelemetry in session.resume request", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll }); const spy = vi @@ -1071,7 +1079,7 @@ describe("CopilotClient", () => { it("forwards enableOnDemandInstructionDiscovery in session.create request", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi.spyOn((client as any).connection!, "sendRequest"); await client.createSession({ @@ -1088,7 +1096,7 @@ describe("CopilotClient", () => { it("forwards enableOnDemandInstructionDiscovery in session.resume request", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll }); const spy = vi @@ -1115,7 +1123,7 @@ describe("CopilotClient", () => { it("defaults includeSubAgentStreamingEvents to true in session.create when not specified", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi.spyOn((client as any).connection!, "sendRequest"); await client.createSession({ onPermissionRequest: approveAll }); @@ -1127,7 +1135,7 @@ describe("CopilotClient", () => { it("forwards explicit false for includeSubAgentStreamingEvents in session.create", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi.spyOn((client as any).connection!, "sendRequest"); await client.createSession({ @@ -1142,7 +1150,7 @@ describe("CopilotClient", () => { it("defaults includeSubAgentStreamingEvents to true in session.resume when not specified", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll }); const spy = vi @@ -1161,7 +1169,7 @@ describe("CopilotClient", () => { it("forwards explicit false for includeSubAgentStreamingEvents in session.resume", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll }); const spy = vi @@ -1183,7 +1191,7 @@ describe("CopilotClient", () => { it("defaults mcpOAuthTokenStorage to 'in-memory' in session.create when mode is empty", async () => { const client = new CopilotClient({ mode: "empty", baseDirectory: "/tmp/copilot-test" }); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi .spyOn((client as any).connection!, "sendRequest") @@ -1201,7 +1209,7 @@ describe("CopilotClient", () => { it("does not send mcpOAuthTokenStorage in session.create when mode is copilot-cli", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi.spyOn((client as any).connection!, "sendRequest"); await client.createSession({ onPermissionRequest: approveAll }); @@ -1213,7 +1221,7 @@ describe("CopilotClient", () => { it("forwards explicit 'persistent' for mcpOAuthTokenStorage in session.create", async () => { const client = new CopilotClient({ mode: "empty", baseDirectory: "/tmp/copilot-test" }); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi .spyOn((client as any).connection!, "sendRequest") @@ -1235,7 +1243,7 @@ describe("CopilotClient", () => { it("defaults mcpOAuthTokenStorage to 'in-memory' in session.resume when mode is empty", async () => { const client = new CopilotClient({ mode: "empty", baseDirectory: "/tmp/copilot-test" }); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi .spyOn((client as any).connection!, "sendRequest") @@ -1255,7 +1263,7 @@ describe("CopilotClient", () => { it("forwards explicit 'persistent' for mcpOAuthTokenStorage in session.resume", async () => { const client = new CopilotClient({ mode: "empty", baseDirectory: "/tmp/copilot-test" }); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi .spyOn((client as any).connection!, "sendRequest") @@ -1279,7 +1287,7 @@ describe("CopilotClient", () => { it("defaults memory to { enabled: false } in session.create when mode is empty", async () => { const client = new CopilotClient({ mode: "empty", baseDirectory: "/tmp/copilot-test" }); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi .spyOn((client as any).connection!, "sendRequest") @@ -1297,7 +1305,7 @@ describe("CopilotClient", () => { it("does not send memory in session.create when mode is copilot-cli", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi.spyOn((client as any).connection!, "sendRequest"); await client.createSession({ onPermissionRequest: approveAll }); @@ -1309,7 +1317,7 @@ describe("CopilotClient", () => { it("forwards explicit memory config in session.create even in empty mode", async () => { const client = new CopilotClient({ mode: "empty", baseDirectory: "/tmp/copilot-test" }); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi .spyOn((client as any).connection!, "sendRequest") @@ -1331,7 +1339,7 @@ describe("CopilotClient", () => { it("defaults memory to { enabled: false } in session.resume when mode is empty", async () => { const client = new CopilotClient({ mode: "empty", baseDirectory: "/tmp/copilot-test" }); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi .spyOn((client as any).connection!, "sendRequest") @@ -1351,7 +1359,7 @@ describe("CopilotClient", () => { it("does not send memory in session.resume when mode is copilot-cli", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll }); const spy = vi @@ -1370,7 +1378,7 @@ describe("CopilotClient", () => { it("forwards continuePendingWork in session.resume request", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll }); const spy = vi @@ -1392,7 +1400,7 @@ describe("CopilotClient", () => { it("omits continuePendingWork from session.resume payload when not specified", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll }); const spy = vi @@ -1411,7 +1419,7 @@ describe("CopilotClient", () => { it("forwards memory configuration in session.create request", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi .spyOn((client as any).connection!, "sendRequest") @@ -1434,7 +1442,7 @@ describe("CopilotClient", () => { it("forwards memory configuration in session.resume request", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll }); const spy = vi @@ -1456,7 +1464,7 @@ describe("CopilotClient", () => { it("omits memory from session.create payload when not specified", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi .spyOn((client as any).connection!, "sendRequest") @@ -1477,7 +1485,7 @@ describe("CopilotClient", () => { it("forwards provider headers in session.create request", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi .spyOn((client as any).connection!, "sendRequest") @@ -1518,7 +1526,7 @@ describe("CopilotClient", () => { it("forwards provider headers in session.resume request", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll }); const spy = vi @@ -1559,7 +1567,7 @@ describe("CopilotClient", () => { it("forwards defaultAgent in session.create request", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi.spyOn((client as any).connection!, "sendRequest"); await client.createSession({ @@ -1578,7 +1586,7 @@ describe("CopilotClient", () => { it("forwards defaultAgent in session.resume request", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll }); const spy = vi.spyOn((client as any).connection!, "sendRequest"); @@ -1598,7 +1606,7 @@ describe("CopilotClient", () => { it("forwards instructionDirectories in session.create request", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const instructionDirectories = ["C:\\extra-instructions", "C:\\more-instructions"]; const spy = vi.spyOn((client as any).connection!, "sendRequest"); @@ -1616,7 +1624,7 @@ describe("CopilotClient", () => { it("forwards instructionDirectories in session.resume request", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll }); const instructionDirectories = ["C:\\resume-instructions"]; @@ -1644,7 +1652,7 @@ describe("CopilotClient", () => { it("does not request permissions on session.resume when using the default joinSession handler", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll }); const spy = vi @@ -1671,7 +1679,7 @@ describe("CopilotClient", () => { it("requests permissions on session.resume when using an explicit handler", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll }); const spy = vi @@ -1698,7 +1706,7 @@ describe("CopilotClient", () => { it("forwards mode callback request flags in session.resume request", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll }); const spy = vi @@ -1728,7 +1736,7 @@ describe("CopilotClient", () => { it("sends session.model.switchTo RPC with correct params", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll }); @@ -1754,7 +1762,7 @@ describe("CopilotClient", () => { it("sends reasoning options with session.model.switchTo when provided", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll }); @@ -2002,7 +2010,7 @@ describe("CopilotClient", () => { it("sends overridesBuiltInTool in tool definition on session.create", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi.spyOn((client as any).connection!, "sendRequest"); await client.createSession({ @@ -2026,7 +2034,7 @@ describe("CopilotClient", () => { it("sends overridesBuiltInTool in tool definition on session.resume", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll }); // Mock sendRequest to capture the call without hitting the runtime @@ -2060,7 +2068,7 @@ describe("CopilotClient", () => { it("sends defer in tool definition on session.create", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi.spyOn((client as any).connection!, "sendRequest"); await client.createSession({ @@ -2084,7 +2092,7 @@ describe("CopilotClient", () => { it("sends defer in tool definition on session.resume", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll }); const spy = vi @@ -2117,7 +2125,7 @@ describe("CopilotClient", () => { it("forwards agent in session.create request", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi.spyOn((client as any).connection!, "sendRequest"); await client.createSession({ @@ -2139,7 +2147,7 @@ describe("CopilotClient", () => { it("forwards custom agent model in session.create request", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi.spyOn((client as any).connection!, "sendRequest"); await client.createSession({ @@ -2162,7 +2170,7 @@ describe("CopilotClient", () => { it("forwards agent in session.resume request", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll }); const spy = vi @@ -2220,7 +2228,7 @@ describe("CopilotClient", () => { const handler = vi.fn().mockReturnValue(customModels); const client = new CopilotClient({ onListModels: handler }); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const models = await client.listModels(); expect(handler).toHaveBeenCalledTimes(1); @@ -2243,7 +2251,7 @@ describe("CopilotClient", () => { const handler = vi.fn().mockReturnValue(customModels); const client = new CopilotClient({ onListModels: handler }); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); await client.listModels(); await client.listModels(); @@ -2265,7 +2273,7 @@ describe("CopilotClient", () => { const handler = vi.fn().mockResolvedValue(customModels); const client = new CopilotClient({ onListModels: handler }); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const models = await client.listModels(); expect(models).toEqual(customModels); @@ -2301,7 +2309,7 @@ describe("CopilotClient", () => { async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); expect((client as any).state).toBe("connected"); @@ -2327,7 +2335,7 @@ describe("CopilotClient", () => { const provider = vi.fn().mockReturnValue(traceContext); const client = new CopilotClient({ onGetTraceContext: provider }); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi.spyOn((client as any).connection!, "sendRequest"); await client.createSession({ onPermissionRequest: approveAll }); @@ -2349,7 +2357,7 @@ describe("CopilotClient", () => { const provider = vi.fn().mockReturnValue(traceContext); const client = new CopilotClient({ onGetTraceContext: provider }); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll }); const spy = vi @@ -2375,7 +2383,7 @@ describe("CopilotClient", () => { const provider = vi.fn().mockReturnValue(traceContext); const client = new CopilotClient({ onGetTraceContext: provider }); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll }); const spy = vi @@ -2397,7 +2405,7 @@ describe("CopilotClient", () => { it("forwards requestHeaders in session.send request", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll }); const spy = vi @@ -2424,7 +2432,7 @@ describe("CopilotClient", () => { it("does not include trace context when no callback is provided", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi.spyOn((client as any).connection!, "sendRequest"); await client.createSession({ onPermissionRequest: approveAll }); @@ -2439,7 +2447,7 @@ describe("CopilotClient", () => { it("forwards commands in session.create RPC", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const spy = vi.spyOn((client as any).connection!, "sendRequest"); await client.createSession({ @@ -2460,7 +2468,7 @@ describe("CopilotClient", () => { it("forwards commands in session.resume RPC", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll }); const spy = vi @@ -2482,7 +2490,7 @@ describe("CopilotClient", () => { it("routes command.execute event to the correct handler", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const handler = vi.fn(); const session = await client.createSession({ @@ -2536,7 +2544,7 @@ describe("CopilotClient", () => { it("sends error when command handler throws", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll, @@ -2584,7 +2592,7 @@ describe("CopilotClient", () => { it("sends error for unknown command", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll, @@ -2630,7 +2638,7 @@ describe("CopilotClient", () => { it("reads capabilities from session.create response", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); // Intercept session.create to inject capabilities const origSendRequest = (client as any).connection!.sendRequest.bind( @@ -2656,7 +2664,7 @@ describe("CopilotClient", () => { it("defaults capabilities when not injected", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll }); // CLI returns actual capabilities (elicitation false in headless mode) @@ -2666,7 +2674,7 @@ describe("CopilotClient", () => { it("elicitation throws when capability is missing", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll }); @@ -2685,7 +2693,7 @@ describe("CopilotClient", () => { it("sends requestElicitation flag when onElicitationRequest is provided", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const rpcSpy = vi.spyOn((client as any).connection!, "sendRequest"); @@ -2711,7 +2719,7 @@ describe("CopilotClient", () => { it("does not send requestElicitation when no handler provided", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const rpcSpy = vi.spyOn((client as any).connection!, "sendRequest"); @@ -2733,7 +2741,7 @@ describe("CopilotClient", () => { it("sends mode callback request flags based on handler presence", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const rpcSpy = vi.spyOn((client as any).connection!, "sendRequest"); @@ -2768,7 +2776,7 @@ describe("CopilotClient", () => { it("dispatches mode callback requests to registered handlers", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll, @@ -2816,7 +2824,7 @@ describe("CopilotClient", () => { it("sends cancel when elicitation handler throws", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const session = await client.createSession({ onPermissionRequest: approveAll, @@ -2876,7 +2884,7 @@ describe("CopilotClient", () => { it("dispatches postToolUseFailure to onPostToolUseFailure handler", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const received: { input: any; invocation: any }[] = []; const session = await client.createSession({ @@ -2917,7 +2925,7 @@ describe("CopilotClient", () => { it("does not fall back to onPostToolUse for postToolUseFailure events", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const postUseCalls: string[] = []; const session = await client.createSession({ @@ -2946,7 +2954,7 @@ describe("CopilotClient", () => { it("dispatches postToolUse and postToolUseFailure to their respective handlers", async () => { const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const postCalls: string[] = []; const failureCalls: string[] = []; @@ -2996,7 +3004,7 @@ describe("CopilotClient", () => { // The SDK maps that to public `{..., timestamp: Date, workingDirectory}`. const client = new CopilotClient(); await client.start(); - onTestFinished(() => client.forceStop()); + onTestFinished(() => stopClient(client)); const received: { input: any; invocation: any }[] = []; const session = await client.createSession({ diff --git a/nodejs/test/e2e/client.e2e.test.ts b/nodejs/test/e2e/client.e2e.test.ts index 33b7a0636b..d38a5c7f48 100644 --- a/nodejs/test/e2e/client.e2e.test.ts +++ b/nodejs/test/e2e/client.e2e.test.ts @@ -5,7 +5,7 @@ import { CopilotClient, approveAll, RuntimeConnection } from "../../src/index.js function onTestFinishedForceStop(client: CopilotClient) { onTestFinished(async () => { try { - await client.forceStop(); + await client.stop(); } catch { // Ignore cleanup errors - process may already be stopped } @@ -111,7 +111,7 @@ describe("Client", () => { onTestFinishedForceStop(client); await client.createSession({ onPermissionRequest: approveAll }); - await client.forceStop(); + await client.stop(); }); it("should get status with version and protocol info", async () => { diff --git a/nodejs/test/e2e/client_options.e2e.test.ts b/nodejs/test/e2e/client_options.e2e.test.ts index 2910b6fb9c..e207f275ab 100644 --- a/nodejs/test/e2e/client_options.e2e.test.ts +++ b/nodejs/test/e2e/client_options.e2e.test.ts @@ -182,7 +182,7 @@ describe("Client options", async () => { }); onTestFinished(async () => { try { - await client.forceStop(); + await client.stop(); } catch { // Ignore cleanup errors } @@ -206,7 +206,7 @@ describe("Client options", async () => { }); onTestFinished(async () => { try { - await client.forceStop(); + await client.stop(); } catch { // Ignore cleanup errors } @@ -237,7 +237,7 @@ describe("Client options", async () => { }); onTestFinished(async () => { try { - await client.forceStop(); + await client.stop(); } catch { // Ignore cleanup errors } @@ -291,7 +291,7 @@ describe("Client options", async () => { }); onTestFinished(async () => { try { - await client.forceStop(); + await client.stop(); } catch { // Ignore cleanup errors } @@ -374,7 +374,7 @@ describe("Client options", async () => { }); onTestFinished(async () => { try { - await client.forceStop(); + await client.stop(); } catch { // Ignore cleanup errors } @@ -523,7 +523,7 @@ describe("Client options", async () => { }); onTestFinished(async () => { try { - await client.forceStop(); + await client.stop(); } catch { // Ignore cleanup errors } @@ -590,7 +590,7 @@ describe("Client options", async () => { }); onTestFinished(async () => { try { - await client.forceStop(); + await client.stop(); } catch { // Ignore cleanup errors } diff --git a/nodejs/test/e2e/connection_token.test.ts b/nodejs/test/e2e/connection_token.test.ts index 079eae51ae..03ac8d25fd 100644 --- a/nodejs/test/e2e/connection_token.test.ts +++ b/nodejs/test/e2e/connection_token.test.ts @@ -24,8 +24,8 @@ describe("Connection token", async () => { }); afterAll(async () => { - await wrongClient.forceStop(); - await noTokenClient.forceStop(); + await wrongClient.stop(); + await noTokenClient.stop(); }); it("connects with the matching token", async () => { diff --git a/nodejs/test/e2e/inprocess_ffi.e2e.test.ts b/nodejs/test/e2e/inprocess_ffi.e2e.test.ts index 639c276d5e..0bf4f2d790 100644 --- a/nodejs/test/e2e/inprocess_ffi.e2e.test.ts +++ b/nodejs/test/e2e/inprocess_ffi.e2e.test.ts @@ -8,7 +8,7 @@ import { CopilotClient, RuntimeConnection } from "../../src/index.js"; function onTestFinishedForceStop(client: CopilotClient) { onTestFinished(async () => { try { - await client.forceStop(); + await client.stop(); } catch { // Ignore cleanup errors - process may already be stopped } diff --git a/nodejs/test/e2e/pending_work_resume.e2e.test.ts b/nodejs/test/e2e/pending_work_resume.e2e.test.ts index 85abc3a900..d788ccf446 100644 --- a/nodejs/test/e2e/pending_work_resume.e2e.test.ts +++ b/nodejs/test/e2e/pending_work_resume.e2e.test.ts @@ -150,7 +150,7 @@ describe("Pending work resume", async () => { }); onTestFinished(async () => { try { - await server.forceStop(); + await server.stop(); } catch { // Ignore cleanup errors } @@ -164,7 +164,7 @@ describe("Pending work resume", async () => { }); onTestFinished(async () => { try { - await client.forceStop(); + await client.stop(); } catch { // Ignore cleanup errors } @@ -222,7 +222,7 @@ describe("Pending work resume", async () => { await permissionRequestedP; expect(initialRequest.kind).toBe("custom-tool"); - await suspendedClient.forceStop(); + await suspendedClient.stop(); const resumedTcpClient = createConnectingClient(cliUrl); const session2 = await resumedTcpClient.resumeSession(sessionId, { @@ -300,7 +300,7 @@ describe("Pending work resume", async () => { ) ).toBe("beta"); - await suspendedClient.forceStop(); + await suspendedClient.stop(); const resumedClient = createConnectingClient(cliUrl); const session2 = await resumedClient.resumeSession(sessionId, { @@ -379,7 +379,7 @@ describe("Pending work resume", async () => { expect(await originalToolAStarted.promise).toBe("alpha"); expect(await originalToolBStarted.promise).toBe("beta"); - await suspendedClient.forceStop(); + await suspendedClient.stop(); const resumedClient = createConnectingClient(cliUrl); const session2 = await resumedClient.resumeSession(sessionId, { @@ -434,7 +434,7 @@ describe("Pending work resume", async () => { expect(firstAnswer?.data.content ?? "").toContain("NO_PENDING_TURN_ONE"); await firstSession.disconnect(); - await firstClient.forceStop(); + await firstClient.stop(); } const resumedClient = createConnectingClient(cliUrl); @@ -516,7 +516,7 @@ describe("Pending work resume", async () => { ).toBe("beta"); if (scenario.disconnectOriginalClient) { - await suspendedClient.forceStop(); + await suspendedClient.stop(); } const resumedClient = createConnectingClient(cliUrl); @@ -615,7 +615,7 @@ describe("Pending work resume", async () => { ); await firstSession.disconnect(); - await firstClient.forceStop(); + await firstClient.stop(); } const resumedClient = createConnectingClient(cliUrl); diff --git a/nodejs/test/e2e/rpc.e2e.test.ts b/nodejs/test/e2e/rpc.e2e.test.ts index 0442ab9267..3243d60b80 100644 --- a/nodejs/test/e2e/rpc.e2e.test.ts +++ b/nodejs/test/e2e/rpc.e2e.test.ts @@ -5,7 +5,7 @@ import { createSdkTestContext } from "./harness/sdkTestContext.js"; function onTestFinishedForceStop(client: CopilotClient) { onTestFinished(async () => { try { - await client.forceStop(); + await client.stop(); } catch { // Ignore cleanup errors - process may already be stopped } diff --git a/nodejs/test/e2e/rpc_mcp_and_skills.e2e.test.ts b/nodejs/test/e2e/rpc_mcp_and_skills.e2e.test.ts index cdd64017c1..4025dc444c 100644 --- a/nodejs/test/e2e/rpc_mcp_and_skills.e2e.test.ts +++ b/nodejs/test/e2e/rpc_mcp_and_skills.e2e.test.ts @@ -74,7 +74,7 @@ describe("Session MCP and skills RPC", async () => { }); onTestFinished(async () => { try { - await mcpAppsClient.forceStop(); + await mcpAppsClient.stop(); } catch { // Ignore cleanup errors } diff --git a/nodejs/test/e2e/rpc_mcp_config.e2e.test.ts b/nodejs/test/e2e/rpc_mcp_config.e2e.test.ts index 581567cb3e..95694a8c63 100644 --- a/nodejs/test/e2e/rpc_mcp_config.e2e.test.ts +++ b/nodejs/test/e2e/rpc_mcp_config.e2e.test.ts @@ -9,7 +9,7 @@ function startEphemeralClient(): CopilotClient { const client = new CopilotClient(); onTestFinished(async () => { try { - await client.forceStop(); + await client.stop(); } catch { // Ignore cleanup errors } diff --git a/nodejs/test/e2e/rpc_server.e2e.test.ts b/nodejs/test/e2e/rpc_server.e2e.test.ts index 8913146b12..5075ae68d9 100644 --- a/nodejs/test/e2e/rpc_server.e2e.test.ts +++ b/nodejs/test/e2e/rpc_server.e2e.test.ts @@ -39,7 +39,7 @@ describe("Server-scoped RPC", async () => { }); onTestFinished(async () => { try { - await extraClient.forceStop(); + await extraClient.stop(); } catch { // Ignore cleanup errors } diff --git a/nodejs/test/e2e/rpc_server_misc.e2e.test.ts b/nodejs/test/e2e/rpc_server_misc.e2e.test.ts index c38fccca17..4f12e507a5 100644 --- a/nodejs/test/e2e/rpc_server_misc.e2e.test.ts +++ b/nodejs/test/e2e/rpc_server_misc.e2e.test.ts @@ -65,7 +65,7 @@ describe("Miscellaneous server-scoped RPC", async () => { async function disposeIsolated(isolatedClient: CopilotClient, home: string): Promise { try { - await isolatedClient.forceStop(); + await isolatedClient.stop(); } catch { // Best-effort cleanup. } @@ -74,7 +74,7 @@ describe("Miscellaneous server-scoped RPC", async () => { async function forceStop(target: CopilotClient): Promise { try { - await target.forceStop(); + await target.stop(); } catch { // Runtime may already be gone. } diff --git a/nodejs/test/e2e/rpc_server_plugins.e2e.test.ts b/nodejs/test/e2e/rpc_server_plugins.e2e.test.ts index 4bc3c63c96..20575a9114 100644 --- a/nodejs/test/e2e/rpc_server_plugins.e2e.test.ts +++ b/nodejs/test/e2e/rpc_server_plugins.e2e.test.ts @@ -59,7 +59,7 @@ describe("Server-scoped plugin RPC", async () => { fixtureDir?: string ): Promise { try { - await client.forceStop(); + await client.stop(); } catch { // Best-effort cleanup. } diff --git a/nodejs/test/e2e/rpc_server_remote_control.e2e.test.ts b/nodejs/test/e2e/rpc_server_remote_control.e2e.test.ts index 2e6c6cf05b..3094d32577 100644 --- a/nodejs/test/e2e/rpc_server_remote_control.e2e.test.ts +++ b/nodejs/test/e2e/rpc_server_remote_control.e2e.test.ts @@ -23,7 +23,7 @@ describe("Server-scoped remote-control RPC", async () => { async function forceStop(client: CopilotClient): Promise { try { - await client.forceStop(); + await client.stop(); } catch { // Runtime may already be gone. } diff --git a/nodejs/test/e2e/rpc_session_state_extras.e2e.test.ts b/nodejs/test/e2e/rpc_session_state_extras.e2e.test.ts index 6e84a6f669..54f40fe12c 100644 --- a/nodejs/test/e2e/rpc_session_state_extras.e2e.test.ts +++ b/nodejs/test/e2e/rpc_session_state_extras.e2e.test.ts @@ -84,7 +84,7 @@ describe("Session-scoped state extras RPC", async () => { } finally { await disconnect(session); try { - await authClient.forceStop(); + await authClient.stop(); } catch { // Best-effort cleanup. } diff --git a/nodejs/test/e2e/session_fs.e2e.test.ts b/nodejs/test/e2e/session_fs.e2e.test.ts index cba98996ef..11de9582e1 100644 --- a/nodejs/test/e2e/session_fs.e2e.test.ts +++ b/nodejs/test/e2e/session_fs.e2e.test.ts @@ -108,7 +108,7 @@ describe("Session Fs", async () => { connection: RuntimeConnection.forTcp({ connectionToken: tcpConnectionToken }), env, }); - onTestFinished(() => client.forceStop()); + onTestFinished(() => client.stop()); await client.createSession({ onPermissionRequest: approveAll, createSessionFsProvider }); const { runtimePort: port } = client as unknown as { runtimePort: number }; @@ -123,7 +123,7 @@ describe("Session Fs", async () => { }), sessionFs: sessionFsConfig, }); - onTestFinished(() => client2.forceStop()); + onTestFinished(() => client2.stop()); await expect(client2.start()).rejects.toThrow(); }); diff --git a/nodejs/test/e2e/suspend.e2e.test.ts b/nodejs/test/e2e/suspend.e2e.test.ts index 79e8987260..27f6d0fe13 100644 --- a/nodejs/test/e2e/suspend.e2e.test.ts +++ b/nodejs/test/e2e/suspend.e2e.test.ts @@ -50,7 +50,7 @@ async function waitWithTimeout( function onTestFinishedForceStop(client: CopilotClient): void { onTestFinished(async () => { try { - await client.forceStop(); + await client.stop(); } catch { // Ignore cleanup errors } From 496818eb5415d35fa9f532b31bdfffc661e9e016 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 9 Jul 2026 21:14:00 +0100 Subject: [PATCH 09/18] fix: scope teardown session-abort to the in-process transport only The abort-before-teardown only exists to release the runtime's SQLite session.db handle over the in-process (FFI) transport, where the runtime shares this process. stdio/tcp runtimes run in a child process we kill on shutdown, which already frees the handle, so don't abort there. Gate on connectionConfig.kind === 'inprocess' and note it's temporary until the runtime cleans up fully on shutdown. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/src/client.ts | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index 8d76e7f2f6..0efcad6dcb 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -907,17 +907,18 @@ export class CopilotClient { // Disconnect all active sessions with retry logic const activeSessions = [...this.sessions.values()]; - // Abort any in-flight turn before teardown. A turn still running when - // the runtime disposes the session can leave that session's SQLite - // session.db handle open in the runtime; over the in-process transport - // the runtime shares this process, so the handle is not reclaimed by - // terminating a child and the file stays locked (Windows), preventing - // removal of the session-state directory. Aborting lets the turn cancel - // and release the handle. Best-effort and idempotent: a session with no - // active turn is a no-op. Skip for external servers: we don't own that - // runtime, and aborting would cancel pending work other clients may - // still resume. - if (!this.isExternalServer) { + // 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 + // reclaimed by terminating a child process, so the file stays locked + // (Windows) and the session-state directory can't be removed. Abort any + // in-flight turn first so it cancels and releases the handle. Best-effort + // and idempotent: a session with no active turn is a no-op. Scoped to + // in-process only: stdio/tcp runtimes run in a child process that we kill + // on shutdown (which frees the handle), and for external servers we don't + // own the runtime and aborting would cancel pending work other clients + // 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())); } for (const session of activeSessions) { From b2ca0d5d4755479351ae762068ec52793eddfa4a Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 9 Jul 2026 21:19:46 +0100 Subject: [PATCH 10/18] test: restore forceStop() where crash/abrupt-disconnect semantics matter Keep graceful stop() for normal teardown (needed so in-process workers release handles and exit), but revert to forceStop() where the test intent requires it: - client.e2e 'should forceStop without cleanup' body - connection_token unauthenticated clients whose start() rejects - client.test.ts stdin-error test using a mock connection - pending_work_resume, which forceStop()s to simulate a crash leaving pending work for resume (restored to base) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/test/client.test.ts | 2 +- nodejs/test/e2e/client.e2e.test.ts | 2 +- nodejs/test/e2e/connection_token.test.ts | 4 ++-- nodejs/test/e2e/pending_work_resume.e2e.test.ts | 16 ++++++++-------- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/nodejs/test/client.test.ts b/nodejs/test/client.test.ts index 99c1ae5c69..56e715c083 100644 --- a/nodejs/test/client.test.ts +++ b/nodejs/test/client.test.ts @@ -26,7 +26,7 @@ async function stopClient(client: CopilotClient): Promise { describe("CopilotClient", () => { it("disposes the stdio connection when child stdin emits an error", async () => { const client = new CopilotClient(); - onTestFinished(() => stopClient(client)); + onTestFinished(() => client.forceStop()); const stdin = new PassThrough(); const stdout = new PassThrough(); diff --git a/nodejs/test/e2e/client.e2e.test.ts b/nodejs/test/e2e/client.e2e.test.ts index d38a5c7f48..adec1fc282 100644 --- a/nodejs/test/e2e/client.e2e.test.ts +++ b/nodejs/test/e2e/client.e2e.test.ts @@ -111,7 +111,7 @@ describe("Client", () => { onTestFinishedForceStop(client); await client.createSession({ onPermissionRequest: approveAll }); - await client.stop(); + await client.forceStop(); }); it("should get status with version and protocol info", async () => { diff --git a/nodejs/test/e2e/connection_token.test.ts b/nodejs/test/e2e/connection_token.test.ts index 03ac8d25fd..079eae51ae 100644 --- a/nodejs/test/e2e/connection_token.test.ts +++ b/nodejs/test/e2e/connection_token.test.ts @@ -24,8 +24,8 @@ describe("Connection token", async () => { }); afterAll(async () => { - await wrongClient.stop(); - await noTokenClient.stop(); + await wrongClient.forceStop(); + await noTokenClient.forceStop(); }); it("connects with the matching token", async () => { diff --git a/nodejs/test/e2e/pending_work_resume.e2e.test.ts b/nodejs/test/e2e/pending_work_resume.e2e.test.ts index d788ccf446..85abc3a900 100644 --- a/nodejs/test/e2e/pending_work_resume.e2e.test.ts +++ b/nodejs/test/e2e/pending_work_resume.e2e.test.ts @@ -150,7 +150,7 @@ describe("Pending work resume", async () => { }); onTestFinished(async () => { try { - await server.stop(); + await server.forceStop(); } catch { // Ignore cleanup errors } @@ -164,7 +164,7 @@ describe("Pending work resume", async () => { }); onTestFinished(async () => { try { - await client.stop(); + await client.forceStop(); } catch { // Ignore cleanup errors } @@ -222,7 +222,7 @@ describe("Pending work resume", async () => { await permissionRequestedP; expect(initialRequest.kind).toBe("custom-tool"); - await suspendedClient.stop(); + await suspendedClient.forceStop(); const resumedTcpClient = createConnectingClient(cliUrl); const session2 = await resumedTcpClient.resumeSession(sessionId, { @@ -300,7 +300,7 @@ describe("Pending work resume", async () => { ) ).toBe("beta"); - await suspendedClient.stop(); + await suspendedClient.forceStop(); const resumedClient = createConnectingClient(cliUrl); const session2 = await resumedClient.resumeSession(sessionId, { @@ -379,7 +379,7 @@ describe("Pending work resume", async () => { expect(await originalToolAStarted.promise).toBe("alpha"); expect(await originalToolBStarted.promise).toBe("beta"); - await suspendedClient.stop(); + await suspendedClient.forceStop(); const resumedClient = createConnectingClient(cliUrl); const session2 = await resumedClient.resumeSession(sessionId, { @@ -434,7 +434,7 @@ describe("Pending work resume", async () => { expect(firstAnswer?.data.content ?? "").toContain("NO_PENDING_TURN_ONE"); await firstSession.disconnect(); - await firstClient.stop(); + await firstClient.forceStop(); } const resumedClient = createConnectingClient(cliUrl); @@ -516,7 +516,7 @@ describe("Pending work resume", async () => { ).toBe("beta"); if (scenario.disconnectOriginalClient) { - await suspendedClient.stop(); + await suspendedClient.forceStop(); } const resumedClient = createConnectingClient(cliUrl); @@ -615,7 +615,7 @@ describe("Pending work resume", async () => { ); await firstSession.disconnect(); - await firstClient.stop(); + await firstClient.forceStop(); } const resumedClient = createConnectingClient(cliUrl); From 67d112097624ddbf500971408a1a7aeec9b974fa Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 9 Jul 2026 21:46:55 +0100 Subject: [PATCH 11/18] test: skip 'should forceStop without cleanup' on inproc Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/test/e2e/client.e2e.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nodejs/test/e2e/client.e2e.test.ts b/nodejs/test/e2e/client.e2e.test.ts index adec1fc282..c786e3d247 100644 --- a/nodejs/test/e2e/client.e2e.test.ts +++ b/nodejs/test/e2e/client.e2e.test.ts @@ -1,6 +1,7 @@ import { ChildProcess } from "child_process"; import { describe, expect, it, onTestFinished } from "vitest"; import { CopilotClient, approveAll, RuntimeConnection } from "../../src/index.js"; +import { isInProcessTransport } from "./harness/sdkTestContext.js"; function onTestFinishedForceStop(client: CopilotClient) { onTestFinished(async () => { @@ -106,7 +107,7 @@ describe("Client", () => { 60_000 ); - it("should forceStop without cleanup", async () => { + it.skipIf(isInProcessTransport)("should forceStop without cleanup", async () => { const client = new CopilotClient({}); onTestFinishedForceStop(client); From 5867a22592f5f62863e5aa9efe23f552023be01d Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 9 Jul 2026 21:55:18 +0100 Subject: [PATCH 12/18] Explanation --- nodejs/test/e2e/client.e2e.test.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/nodejs/test/e2e/client.e2e.test.ts b/nodejs/test/e2e/client.e2e.test.ts index c786e3d247..1d6ecfb277 100644 --- a/nodejs/test/e2e/client.e2e.test.ts +++ b/nodejs/test/e2e/client.e2e.test.ts @@ -1,6 +1,6 @@ import { ChildProcess } from "child_process"; import { describe, expect, it, onTestFinished } from "vitest"; -import { CopilotClient, approveAll, RuntimeConnection } from "../../src/index.js"; +import { approveAll, CopilotClient, RuntimeConnection } from "../../src/index.js"; import { isInProcessTransport } from "./harness/sdkTestContext.js"; function onTestFinishedForceStop(client: CopilotClient) { @@ -107,6 +107,11 @@ describe("Client", () => { 60_000 ); + // Skipping on in-proc: + // - It breaks the macOS E2E run (failure: EPIPE) + // - It's not clear that anyone should use forceStop in the in-proc case - there's no child process + // to terminate, so we can't be sure to leave a clean state + // - If you want to get to a clean state within your process, that's what "stop" (not "forceStop") is for it.skipIf(isInProcessTransport)("should forceStop without cleanup", async () => { const client = new CopilotClient({}); onTestFinishedForceStop(client); From b2a021ba24e0070d268c4f9731f138cd9aaa19a3 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 9 Jul 2026 22:11:27 +0100 Subject: [PATCH 13/18] Cleanup --- nodejs/test/client.test.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/nodejs/test/client.test.ts b/nodejs/test/client.test.ts index 56e715c083..afca975a0e 100644 --- a/nodejs/test/client.test.ts +++ b/nodejs/test/client.test.ts @@ -1,7 +1,7 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import { EventEmitter } from "node:events"; -import { describe, expect, it, onTestFinished, vi } from "vitest"; import { PassThrough } from "stream"; +import { describe, expect, it, onTestFinished, vi } from "vitest"; import { approveAll, CopilotClient, @@ -15,10 +15,6 @@ import { defaultJoinSessionPermissionHandler } from "../src/types.js"; // This file is for unit tests. Where relevant, prefer to add e2e tests in e2e/*.test.ts instead -// Tear down a client created during a test. Over the in-process (FFI) transport the -// runtime shares this test process, so clients must shut down gracefully with stop() -// to release their native/SQLite handles and let the vitest worker exit; forceStop() -// would leave them open and the worker would fail to terminate. async function stopClient(client: CopilotClient): Promise { await client.stop(); } From ce6b96f2fe38e352741d3c02829d03dfaa30f6c2 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 9 Jul 2026 22:13:27 +0100 Subject: [PATCH 14/18] Cleanup --- nodejs/test/e2e/client.e2e.test.ts | 22 +++++++++++----------- nodejs/test/e2e/inprocess_ffi.e2e.test.ts | 14 +------------- nodejs/test/e2e/rpc.e2e.test.ts | 8 ++++---- nodejs/test/e2e/suspend.e2e.test.ts | 8 ++++---- 4 files changed, 20 insertions(+), 32 deletions(-) diff --git a/nodejs/test/e2e/client.e2e.test.ts b/nodejs/test/e2e/client.e2e.test.ts index 1d6ecfb277..89489f78e6 100644 --- a/nodejs/test/e2e/client.e2e.test.ts +++ b/nodejs/test/e2e/client.e2e.test.ts @@ -3,7 +3,7 @@ import { describe, expect, it, onTestFinished } from "vitest"; import { approveAll, CopilotClient, RuntimeConnection } from "../../src/index.js"; import { isInProcessTransport } from "./harness/sdkTestContext.js"; -function onTestFinishedForceStop(client: CopilotClient) { +function onTestFinishedStop(client: CopilotClient) { onTestFinished(async () => { try { await client.stop(); @@ -19,7 +19,7 @@ describe("Client", () => { { transport: "tcp", connection: () => RuntimeConnection.forTcp() }, ])("allows createSession without onPermissionRequest ($transport)", async ({ connection }) => { const client = new CopilotClient({ connection: connection() }); - onTestFinishedForceStop(client); + onTestFinishedStop(client); await using session = await client.createSession({}); expect(session.sessionId).toMatch(/^[a-f0-9-]+$/); @@ -31,7 +31,7 @@ describe("Client", () => { const client = new CopilotClient({ connection: RuntimeConnection.forTcp({ connectionToken }), }); - onTestFinishedForceStop(client); + onTestFinishedStop(client); await using originalSession = await client.createSession({}); @@ -43,7 +43,7 @@ describe("Client", () => { const resumeClient = new CopilotClient({ connection: RuntimeConnection.forUri(`localhost:${port}`, { connectionToken }), }); - onTestFinishedForceStop(resumeClient); + onTestFinishedStop(resumeClient); await using resumedSession = await resumeClient.resumeSession( originalSession.sessionId, @@ -54,7 +54,7 @@ describe("Client", () => { it("should start and connect to server using stdio", async () => { const client = new CopilotClient(); - onTestFinishedForceStop(client); + onTestFinishedStop(client); await client.start(); @@ -67,7 +67,7 @@ describe("Client", () => { it("should start and connect to server using tcp", async () => { const client = new CopilotClient({ connection: RuntimeConnection.forTcp() }); - onTestFinishedForceStop(client); + onTestFinishedStop(client); await client.start(); @@ -114,7 +114,7 @@ describe("Client", () => { // - If you want to get to a clean state within your process, that's what "stop" (not "forceStop") is for it.skipIf(isInProcessTransport)("should forceStop without cleanup", async () => { const client = new CopilotClient({}); - onTestFinishedForceStop(client); + onTestFinishedStop(client); await client.createSession({ onPermissionRequest: approveAll }); await client.forceStop(); @@ -122,7 +122,7 @@ describe("Client", () => { it("should get status with version and protocol info", async () => { const client = new CopilotClient(); - onTestFinishedForceStop(client); + onTestFinishedStop(client); await client.start(); @@ -138,7 +138,7 @@ describe("Client", () => { it("should get auth status", async () => { const client = new CopilotClient(); - onTestFinishedForceStop(client); + onTestFinishedStop(client); await client.start(); @@ -154,7 +154,7 @@ describe("Client", () => { it("should list models when authenticated", async () => { const client = new CopilotClient(); - onTestFinishedForceStop(client); + onTestFinishedStop(client); await client.start(); @@ -183,7 +183,7 @@ describe("Client", () => { const client = new CopilotClient({ connection: RuntimeConnection.forStdio({ args: ["--nonexistent-flag-for-testing"] }), }); - onTestFinishedForceStop(client); + onTestFinishedStop(client); let initialError: Error | undefined; try { diff --git a/nodejs/test/e2e/inprocess_ffi.e2e.test.ts b/nodejs/test/e2e/inprocess_ffi.e2e.test.ts index 0bf4f2d790..af879ea77b 100644 --- a/nodejs/test/e2e/inprocess_ffi.e2e.test.ts +++ b/nodejs/test/e2e/inprocess_ffi.e2e.test.ts @@ -2,19 +2,9 @@ * Copyright (c) Microsoft Corporation. All rights reserved. *--------------------------------------------------------------------------------------------*/ -import { describe, expect, it, onTestFinished } from "vitest"; +import { describe, expect, it } from "vitest"; import { CopilotClient, RuntimeConnection } from "../../src/index.js"; -function onTestFinishedForceStop(client: CopilotClient) { - onTestFinished(async () => { - try { - await client.stop(); - } catch { - // Ignore cleanup errors - process may already be stopped - } - }); -} - describe("In-process FFI transport", () => { // Smoke test that the in-process FFI transport starts and completes a round-trip. // Resolution of the in-process transport from COPILOT_SDK_DEFAULT_CONNECTION is @@ -25,8 +15,6 @@ describe("In-process FFI transport", () => { // bundled platform package) and its sibling native runtime library itself. If // neither is available, start() throws and the test fails hard. const client = new CopilotClient({ connection: RuntimeConnection.forInProcess() }); - onTestFinishedForceStop(client); - await client.start(); const pong = await client.ping("ffi message"); diff --git a/nodejs/test/e2e/rpc.e2e.test.ts b/nodejs/test/e2e/rpc.e2e.test.ts index 3243d60b80..f90547da9b 100644 --- a/nodejs/test/e2e/rpc.e2e.test.ts +++ b/nodejs/test/e2e/rpc.e2e.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it, onTestFinished } from "vitest"; import { CopilotClient, approveAll } from "../../src/index.js"; import { createSdkTestContext } from "./harness/sdkTestContext.js"; -function onTestFinishedForceStop(client: CopilotClient) { +function onTestFinishedStop(client: CopilotClient) { onTestFinished(async () => { try { await client.stop(); @@ -15,7 +15,7 @@ function onTestFinishedForceStop(client: CopilotClient) { describe("RPC", () => { it("should call rpc.ping with typed params and result", async () => { const client = new CopilotClient(); - onTestFinishedForceStop(client); + onTestFinishedStop(client); await client.start(); @@ -28,7 +28,7 @@ describe("RPC", () => { it("should call rpc.models.list with typed result", async () => { const client = new CopilotClient(); - onTestFinishedForceStop(client); + onTestFinishedStop(client); await client.start(); @@ -48,7 +48,7 @@ describe("RPC", () => { // account.getQuota is defined in schema but not yet implemented in CLI it.skip("should call rpc.account.getQuota when authenticated", async () => { const client = new CopilotClient(); - onTestFinishedForceStop(client); + onTestFinishedStop(client); await client.start(); diff --git a/nodejs/test/e2e/suspend.e2e.test.ts b/nodejs/test/e2e/suspend.e2e.test.ts index 27f6d0fe13..2c8639ad38 100644 --- a/nodejs/test/e2e/suspend.e2e.test.ts +++ b/nodejs/test/e2e/suspend.e2e.test.ts @@ -4,8 +4,8 @@ import { describe, expect, it, onTestFinished } from "vitest"; import { z } from "zod"; -import { approveAll, CopilotClient, defineTool, RuntimeConnection } from "../../src/index.js"; import type { PermissionRequest, PermissionRequestResult, SessionEvent } from "../../src/index.js"; +import { approveAll, CopilotClient, defineTool, RuntimeConnection } from "../../src/index.js"; import { createSdkTestContext, DEFAULT_GITHUB_TOKEN } from "./harness/sdkTestContext.js"; const SUSPEND_TIMEOUT_MS = 60_000; @@ -47,7 +47,7 @@ async function waitWithTimeout( } } -function onTestFinishedForceStop(client: CopilotClient): void { +function onTestFinishedStop(client: CopilotClient): void { onTestFinished(async () => { try { await client.stop(); @@ -71,7 +71,7 @@ describe("Suspend RPC", async () => { connectionToken: SHARED_TOKEN, }), }); - onTestFinishedForceStop(server); + onTestFinishedStop(server); return server; } @@ -79,7 +79,7 @@ describe("Suspend RPC", async () => { const connectedClient = new CopilotClient({ connection: RuntimeConnection.forUri(cliUrl, { connectionToken: SHARED_TOKEN }), }); - onTestFinishedForceStop(connectedClient); + onTestFinishedStop(connectedClient); return connectedClient; } From 97fe96c8a03395674fcdb2dd032f57f3beb7be7c Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 9 Jul 2026 22:17:40 +0100 Subject: [PATCH 15/18] Simplify comment --- nodejs/src/types.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/nodejs/src/types.ts b/nodejs/src/types.ts index 2992880921..768d0856df 100644 --- a/nodejs/src/types.ts +++ b/nodejs/src/types.ts @@ -206,9 +206,6 @@ export const RuntimeConnection = { }, /** * Host the runtime in-process over the native runtime library's C ABI (FFI). - * The native host spawns the CLI worker itself; the SDK does not launch a - * runtime child process. Honors `COPILOT_CLI_PATH` for the CLI entrypoint, - * otherwise resolves the bundled platform package. * * @experimental Per-client options lowered to environment variables (`env`, * `telemetry`, `gitHubToken`, `baseDirectory`) are **not** honored in-process; From bb8b064173e3459891cbdb036704799a0bb134d4 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 9 Jul 2026 22:37:22 +0100 Subject: [PATCH 16/18] Lazy-load koffi --- nodejs/src/client.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index 0efcad6dcb..b3d9edc332 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -40,7 +40,7 @@ import type { } from "./generated/rpc.js"; import { getSdkProtocolVersion } from "./sdkProtocolVersion.js"; import { CopilotSession } from "./session.js"; -import { FfiRuntimeHost } from "./ffiRuntimeHost.js"; +import type { FfiRuntimeHost } from "./ffiRuntimeHost.js"; import { createSessionFsAdapter, type SessionFsProvider } from "./sessionFsProvider.js"; import { createCopilotRequestAdapter } from "./copilotRequestHandler.js"; import type { CopilotRequestHandler } from "./copilotRequestHandler.js"; @@ -2528,6 +2528,12 @@ export class CopilotClient { */ private async startInProcessFfi(): Promise { 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; + // out-of-process (stdio/tcp) consumers never touch the native dependency. + // The transpiled output is per-file (not bundled), so this resolves the + // sibling module at runtime in both the ESM and CJS builds. + const { FfiRuntimeHost } = await import("./ffiRuntimeHost.js"); const host = FfiRuntimeHost.create( entrypoint, CopilotClient.getNapiPrebuildsFolder(), From e8565675f22a6c0606838e292df37983d8b7fdc4 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 9 Jul 2026 23:05:31 +0100 Subject: [PATCH 17/18] Address review: process.env-only default connection, reject cwd inproc, guard late FFI frames - resolveDefaultConnection() reads COPILOT_SDK_DEFAULT_CONNECTION from process.env only (not the deprecated options.env), so the host-process transport switch can't be silently disabled by a per-client env block. - Reject a workingDirectory with RuntimeConnection.forInProcess() instead of mutating the shared process-global cwd across the ~30s host_start; removed the chdir dance from FfiRuntimeHost. The e2e harness now changes cwd itself around in-process worker startup to preserve the working-directory behavior. - Guard feedInbound() against a native outbound callback delivered after dispose() ended receiveStream (ERR_STREAM_WRITE_AFTER_END): drop late frames. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/src/client.ts | 22 +++++---- nodejs/src/ffiRuntimeHost.ts | 59 +++++++++++------------ nodejs/test/e2e/harness/sdkTestContext.ts | 24 ++++++++- 3 files changed, 62 insertions(+), 43 deletions(-) diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index b3d9edc332..93ed850b37 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -578,9 +578,8 @@ export class CopilotClient { * Resolves the default {@link RuntimeConnection} for the no-connection case, * honoring {@link CopilotClient.DEFAULT_CONNECTION_ENV_VAR}. */ - private static resolveDefaultConnection(options: CopilotClientOptions): RuntimeConnection { - const env = options.env ?? process.env; - const value = env[CopilotClient.DEFAULT_CONNECTION_ENV_VAR]; + private static resolveDefaultConnection(): RuntimeConnection { + const value = process.env[CopilotClient.DEFAULT_CONNECTION_ENV_VAR]; if (!value || value.toLowerCase() === "stdio") { return { kind: "stdio" }; } @@ -627,7 +626,7 @@ export class CopilotClient { const conn: InternalRuntimeConnection = options._internalConnection ?? options.connection ?? - CopilotClient.resolveDefaultConnection(options); + CopilotClient.resolveDefaultConnection(); if ( conn.kind === "uri" && @@ -637,6 +636,14 @@ export class CopilotClient { "gitHubToken and useLoggedInUser cannot be used with RuntimeConnection.forUri (external server manages its own auth)" ); } + if (conn.kind === "inprocess" && options.workingDirectory !== undefined) { + throw new Error( + "workingDirectory is not supported with RuntimeConnection.forInProcess(): the in-process " + + "transport hosts the runtime in this process, so honoring it would require mutating the " + + "shared process-global cwd. Change the host process's working directory before " + + "constructing the client instead." + ); + } if (conn.kind === "tcp" && conn.connectionToken !== undefined) { if (typeof conn.connectionToken !== "string" || conn.connectionToken.length === 0) { throw new Error("connectionToken must be a non-empty string"); @@ -2534,12 +2541,7 @@ export class CopilotClient { // The transpiled output is per-file (not bundled), so this resolves the // sibling module at runtime in both the ESM and CJS builds. const { FfiRuntimeHost } = await import("./ffiRuntimeHost.js"); - const host = FfiRuntimeHost.create( - entrypoint, - CopilotClient.getNapiPrebuildsFolder(), - undefined, - this.options.workingDirectory - ); + const host = FfiRuntimeHost.create(entrypoint, CopilotClient.getNapiPrebuildsFolder()); this.ffiHost = host; await host.start(); } diff --git a/nodejs/src/ffiRuntimeHost.ts b/nodejs/src/ffiRuntimeHost.ts index e6b0652618..548d4f3b32 100644 --- a/nodejs/src/ffiRuntimeHost.ts +++ b/nodejs/src/ffiRuntimeHost.ts @@ -138,8 +138,7 @@ export class FfiRuntimeHost { private constructor( private readonly libraryPath: string, private readonly cliEntrypoint: string, - private readonly environment?: Record, - private readonly workingDirectory?: string + private readonly environment?: Record ) { this.lib = loadLibrary(libraryPath); this.receiveStream = new PassThrough(); @@ -166,8 +165,7 @@ export class FfiRuntimeHost { static create( cliEntrypoint: string, prebuildsFolder: string, - environment?: Record, - workingDirectory?: string + environment?: Record ): FfiRuntimeHost { const fullEntrypoint = resolve(cliEntrypoint); const distDir = dirname(fullEntrypoint); @@ -175,7 +173,7 @@ export class FfiRuntimeHost { if (!existsSync(libraryPath)) { throw new Error(`FFI runtime library not found. Looked for '${libraryPath}'.`); } - return new FfiRuntimeHost(libraryPath, fullEntrypoint, environment, workingDirectory); + return new FfiRuntimeHost(libraryPath, fullEntrypoint, environment); } /** @@ -187,37 +185,27 @@ export class FfiRuntimeHost { const envJson = buildEnvJson(this.environment); // The native host spawns the CLI worker itself and has no cwd parameter, so the - // worker inherits this process's cwd. Mirror the stdio child's `cwd: workingDirectory` - // by switching cwd for the duration of the blocking host_start, then restoring it. - const previousCwd = process.cwd(); - const shouldSwitchCwd = !!this.workingDirectory && this.workingDirectory !== previousCwd; - if (shouldSwitchCwd) { - process.chdir(this.workingDirectory!); - } + // worker inherits this process's cwd. A custom working directory is intentionally + // unsupported for the in-process transport (rejected by the client constructor) + // rather than mutating the shared process-global cwd here. // host_start blocks until the worker connects back and signals readiness // (up to ~30s); run it as an async FFI call so the Node event loop isn't blocked. - try { - this.serverId = await new Promise((resolvePromise, rejectPromise) => { - this.lib.hostStart.async( - argvJson, - argvJson.length, - envJson, - envJson ? envJson.length : 0, - (error: Error | null, result: number) => { - if (error) { - rejectPromise(error); - } else { - resolvePromise(result); - } + this.serverId = await new Promise((resolvePromise, rejectPromise) => { + this.lib.hostStart.async( + argvJson, + argvJson.length, + envJson, + envJson ? envJson.length : 0, + (error: Error | null, result: number) => { + if (error) { + rejectPromise(error); + } else { + resolvePromise(result); } - ); - }); - } finally { - if (shouldSwitchCwd) { - process.chdir(previousCwd); - } - } + } + ); + }); if (!this.serverId) { throw new Error( `copilot_runtime_host_start failed (library '${this.libraryPath}', entrypoint '${this.cliEntrypoint}').` @@ -276,6 +264,13 @@ export class FfiRuntimeHost { // and would surface only as a DEP0168 "uncaught Node-API callback exception" // warning, so catch and log it here instead of letting it escape. try { + // A native outbound callback can still be delivered on the event loop after + // dispose() has ended receiveStream; writing then would throw + // ERR_STREAM_WRITE_AFTER_END. Drop late frames instead — the connection is + // gone and nothing is reading them. + if (this.disposed || this.receiveStream.writableEnded) { + return; + } const length = Number(bytesLen); if (!bytesPtr || length <= 0) { return; diff --git a/nodejs/test/e2e/harness/sdkTestContext.ts b/nodejs/test/e2e/harness/sdkTestContext.ts index 8872682311..ba44dd0942 100644 --- a/nodejs/test/e2e/harness/sdkTestContext.ts +++ b/nodejs/test/e2e/harness/sdkTestContext.ts @@ -173,7 +173,12 @@ export async function createSdkTestContext({ : {}; const copilotClient = new CopilotClient({ - workingDirectory: workDir, + // The in-process transport rejects a per-client workingDirectory (it would have to + // mutate the shared host process cwd). Instead the harness changes this process's + // cwd to workDir around the in-process worker's startup (see beforeEach below), so + // the worker still spawns with workDir as its cwd. Out-of-process clients get it + // as a normal per-client option. + workingDirectory: isInProcess ? undefined : workDir, // In-process hosting mirrors the environment onto the real process (per test, in // beforeEach below), so the worker inherits it; passing a per-client env here // would have no effect. @@ -192,6 +197,9 @@ export async function createSdkTestContext({ // Holds the process.env entries the current test overwrote, so afterEach restores them. let restoreProcessEnv: Array<[string, string | undefined]> = []; + // Holds the process cwd before an in-process test changed it, so afterEach restores it. + let restoreCwd: string | undefined; + // Wire up to Vitest lifecycle beforeEach(async (testContext) => { // Must be inside beforeEach - vitest requires test context @@ -209,6 +217,15 @@ export async function createSdkTestContext({ process.env[key] = value; } + // The in-process worker inherits this process's cwd at spawn (the client auto-starts + // on first use inside the test body). Point cwd at workDir here so the worker spawns + // with the same working directory the out-of-process transport passes explicitly; + // afterEach restores it. + if (isInProcess) { + restoreCwd = process.cwd(); + process.chdir(workDir); + } + await openAiEndpoint.updateConfig({ filePath: getTrafficCapturePath(testContext), workDir, @@ -229,6 +246,11 @@ export async function createSdkTestContext({ } } restoreProcessEnv = []; + // Restore the cwd an in-process test changed for worker startup. + if (restoreCwd !== undefined) { + process.chdir(restoreCwd); + restoreCwd = undefined; + } // Empty directories but leave them in place for next test await rimraf([join(homeDir, "*"), join(workDir, "*")], { glob: true }); }); From a67adf8bced2e02c0c04354ddf41c827e8ac0f90 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 9 Jul 2026 23:25:26 +0100 Subject: [PATCH 18/18] Use --no-auto-update --- dotnet/src/FfiRuntimeHost.cs | 3 +++ nodejs/src/ffiRuntimeHost.ts | 8 +++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/dotnet/src/FfiRuntimeHost.cs b/dotnet/src/FfiRuntimeHost.cs index 210819de67..ee838ad276 100644 --- a/dotnet/src/FfiRuntimeHost.cs +++ b/dotnet/src/FfiRuntimeHost.cs @@ -167,6 +167,9 @@ private static byte[] BuildArgvJson(string cliEntrypoint) } writer.WriteStringValue(cliEntrypoint); writer.WriteStringValue("--embedded-host"); + // Pin the worker to the bundled pkg matching the loaded cdylib, instead of + // drifting to a newer version under the user's ~/.copilot/pkg (ABI skew). + writer.WriteStringValue("--no-auto-update"); writer.WriteEndArray(); } return stream.ToArray(); diff --git a/nodejs/src/ffiRuntimeHost.ts b/nodejs/src/ffiRuntimeHost.ts index 548d4f3b32..05c8c7c72c 100644 --- a/nodejs/src/ffiRuntimeHost.ts +++ b/nodejs/src/ffiRuntimeHost.ts @@ -99,10 +99,12 @@ function loadLibrary(libraryPath: string): FfiLibrary { function buildArgvJson(cliEntrypoint: string): Buffer { // A `.js` entrypoint is launched via node; the packaged single-file CLI binary - // embeds its own Node and is invoked directly. + // embeds its own Node and is invoked directly. `--no-auto-update` pins the worker + // to the bundled pkg matching the loaded cdylib, instead of drifting to a newer + // version installed under the user's `~/.copilot/pkg` (which would cause ABI skew). const argv = cliEntrypoint.toLowerCase().endsWith(".js") - ? ["node", cliEntrypoint, "--embedded-host"] - : [cliEntrypoint, "--embedded-host"]; + ? ["node", cliEntrypoint, "--embedded-host", "--no-auto-update"] + : [cliEntrypoint, "--embedded-host", "--no-auto-update"]; return Buffer.from(JSON.stringify(argv), "utf8"); }