From c1195a1a78d6da80762480180c10a418202d55e6 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Mon, 6 Jul 2026 14:59:21 +0000 Subject: [PATCH 01/33] Add in-process FFI transport for Rust and TypeScript SDKs Ports the C# in-process FFI hosting transport to the Rust and Node SDKs, mirroring the .NET `RuntimeConnection.ForInProcess()` API. Both load the runtime cdylib and speak JSON-RPC over its C ABI (host_start/connection_*) instead of spawning a stdio/TCP child; framing is unchanged (a transport swap over the existing LSP Content-Length codec). Rust: - `Transport::InProcess` variant + `COPILOT_SDK_DEFAULT_CONNECTION` default - `src/ffi.rs`: libloading C-ABI bindings, AsyncRead/AsyncWrite bridge, teardown - build.rs/embeddedcli extract+rename runtime.node -> libcopilot_runtime.* - in-process E2E test + CI matrix cell TypeScript: - `RuntimeConnection.forInProcess()` + `InProcessRuntimeConnection` - `src/ffiRuntimeHost.ts`: koffi loads runtime.node from node_modules directly - `COPILOT_SDK_DEFAULT_CONNECTION` default + in-process E2E test + CI matrix cell Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/nodejs-sdk-tests.yml | 7 +- .github/workflows/rust-sdk-tests.yml | 64 +++ nodejs/package-lock.json | 11 + nodejs/package.json | 1 + nodejs/src/client.ts | 211 +++++++--- nodejs/src/ffiRuntimeHost.ts | 313 +++++++++++++++ nodejs/src/index.ts | 1 + nodejs/src/types.ts | 20 + nodejs/test/e2e/harness/sdkTestContext.ts | 14 +- nodejs/test/e2e/inprocess_ffi.e2e.test.ts | 49 +++ rust/Cargo.lock | 11 + rust/Cargo.toml | 1 + rust/build.rs | 107 +++++ rust/src/embeddedcli.rs | 58 ++- rust/src/ffi.rs | 465 ++++++++++++++++++++++ rust/src/lib.rs | 157 +++++++- rust/tests/e2e.rs | 2 + rust/tests/e2e/inprocess.rs | 27 ++ rust/tests/e2e/support.rs | 30 ++ 19 files changed, 1491 insertions(+), 58 deletions(-) create mode 100644 nodejs/src/ffiRuntimeHost.ts create mode 100644 nodejs/test/e2e/inprocess_ffi.e2e.test.ts create mode 100644 rust/src/ffi.rs create mode 100644 rust/tests/e2e/inprocess.rs diff --git a/.github/workflows/nodejs-sdk-tests.yml b/.github/workflows/nodejs-sdk-tests.yml index 8880cadfa3..a5e8b77be6 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,10 @@ 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/.github/workflows/rust-sdk-tests.yml b/.github/workflows/rust-sdk-tests.yml index f75a5d6a29..bd0a3348d3 100644 --- a/.github/workflows/rust-sdk-tests.yml +++ b/.github/workflows/rust-sdk-tests.yml @@ -129,6 +129,70 @@ jobs: # The dedicated `bundle` job below exercises the embed pipeline. run: cargo test --no-default-features --features test-support -- --test-threads=4 --nocapture + # Exercises the in-process FFI transport (`Transport::InProcess`, the Rust + # analogue of the .NET `RuntimeConnection.ForInProcess()`), mirroring the + # `inprocess` transport cell in dotnet-sdk-tests.yml. Sets + # COPILOT_SDK_DEFAULT_CONNECTION=inprocess so the client hosts the runtime + # cdylib in-process instead of spawning a stdio child, then runs the + # dedicated in-process E2E test. + test-inprocess: + name: "Rust SDK Tests (in-process transport)" + if: github.event.repository.fork == false + env: + CARGO_TERM_COLOR: always + RUST_BACKTRACE: 1 + runs-on: ubuntu-latest + defaults: + run: + shell: bash + working-directory: ./rust + steps: + - uses: actions/checkout@v6.0.2 + + - uses: ./.github/actions/setup-copilot + id: setup-copilot + + - name: Install Rust toolchain + uses: dtolnay/rust-toolchain@stable + with: + toolchain: "1.94.0" + + - uses: Swatinem/rust-cache@v2 + with: + workspaces: "rust" + prefix-key: v1-rust-no-bin + cache-bin: false + + - name: Read pinned @github/copilot CLI version + id: cli-version + working-directory: ./nodejs + run: | + version=$(node -p "require('./package-lock.json').packages['node_modules/@github/copilot'].version") + echo "version=$version" >> "$GITHUB_OUTPUT" + echo "Pinned CLI version: $version" + + - name: Cache bundled CLI tarball + uses: actions/cache@v4 + with: + path: ./rust/.bundled-cli-cache + key: bundled-cli-ubuntu-latest-${{ steps.cli-version.outputs.version }} + + - name: Install test harness dependencies + working-directory: ./test/harness + run: npm ci --ignore-scripts + + - name: Select in-process transport + run: echo "COPILOT_SDK_DEFAULT_CONNECTION=inprocess" >> "$GITHUB_ENV" + + - name: cargo test (in-process transport) + timeout-minutes: 60 + env: + RUST_E2E_CONCURRENCY: 4 + COPILOT_HMAC_KEY: ${{ secrets.COPILOT_DEVELOPER_CLI_INTEGRATION_HMAC_KEY }} + COPILOT_CLI_PATH: ${{ steps.setup-copilot.outputs.cli-path }} + BUNDLED_CLI_CACHE_DIR: ${{ github.workspace }}/rust/.bundled-cli-cache + run: cargo test --no-default-features --features test-support --test e2e inprocess -- --nocapture + # Validates the bundled-CLI build path on all three supported # platforms. While the regular `cargo test` job above also exercises # build.rs (bundling is on by default now), this matrix job is the diff --git a/nodejs/package-lock.json b/nodejs/package-lock.json index 60de5cf8aa..6c3f32eca5 100644 --- a/nodejs/package-lock.json +++ b/nodejs/package-lock.json @@ -10,6 +10,7 @@ "license": "MIT", "dependencies": { "@github/copilot": "^1.0.69-2", + "koffi": "^2.16.2", "vscode-jsonrpc": "^8.2.1", "zod": "^4.3.6" }, @@ -2592,6 +2593,16 @@ "json-buffer": "3.0.1" } }, + "node_modules/koffi": { + "version": "2.16.2", + "resolved": "https://registry.npmjs.org/koffi/-/koffi-2.16.2.tgz", + "integrity": "sha512-owU0MRwv6xkrVqCd+33uw6BaYppkTRXbO/rVdJNI2dvZG0gzyRhYwW25eWtc5pauwK8TGh3AbkFONSezdykfSA==", + "hasInstallScript": true, + "license": "MIT", + "funding": { + "url": "https://liberapay.com/Koromix" + } + }, "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 23aface8b4..5cf9dd41c5 100644 --- a/nodejs/package.json +++ b/nodejs/package.json @@ -57,6 +57,7 @@ "license": "MIT", "dependencies": { "@github/copilot": "^1.0.69-2", + "koffi": "^2.16.2", "vscode-jsonrpc": "^8.2.1", "zod": "^4.3.6" }, diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index 9f430600ca..b2e4855a41 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; @@ -2195,6 +2254,46 @@ export class CopilotClient { }; } + /** + * Builds the environment for the spawned runtime worker (used by both the stdio/TCP + * child process and the in-process FFI host): applies the auth token, connection + * token, `COPILOT_HOME`, keychain setting, and telemetry variables on top of the + * effective env. + */ + 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 */ @@ -2241,30 +2340,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( @@ -2276,26 +2354,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( @@ -2432,12 +2490,69 @@ 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. + */ + private async startInProcessFfi(): Promise { + const entrypoint = this.resolveCliPathForFfi(); + const host = FfiRuntimeHost.create( + entrypoint, + CopilotClient.getNapiPrebuildsFolder(), + this.buildRuntimeEnv(), + 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 */ diff --git a/nodejs/src/ffiRuntimeHost.ts b/nodejs/src/ffiRuntimeHost.ts new file mode 100644 index 0000000000..681cac7ac8 --- /dev/null +++ b/nodejs/src/ffiRuntimeHost.ts @@ -0,0 +1,313 @@ +/*--------------------------------------------------------------------------------------------- + * 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; + 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"]), + 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", "--log-level", "all"] + : [cliEntrypoint, "--embedded-host", "--log-level", "all"]; + 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 the libuv event loop alive while the FFI connection is open. Unlike the + * stdio/TCP transports (whose pipe/socket handles keep the loop alive), the FFI + * transport has no libuv handle of its own, and native→JS callbacks are delivered + * via the event loop. Without a live handle the loop can park with no work and the + * queued callback delivery races into a crash. + */ + private keepAlive: ReturnType | null = null; + + /** 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: (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.keepAlive = setInterval(() => {}, 60_000); + } + + private writeFrame(frame: Buffer): void { + if (this.disposed || !this.connectionId) { + throw new Error("The in-process runtime connection is closed."); + } + if (!this.lib.connectionWrite(this.connectionId, frame, frame.length)) { + throw new Error("Failed to write a frame to the in-process runtime connection."); + } + } + + private feedInbound(bytesPtr: unknown, bytesLen: number | bigint): void { + const length = Number(bytesLen); + if (!bytesPtr || length <= 0) { + return; + } + const bytes = koffi.decode(bytesPtr, koffi.array("uint8", length, "Typed")) as Uint8Array; + this.receiveStream.write(Buffer.from(bytes)); + } + + private unregisterCallback(): void { + if (this.outboundCallback !== undefined) { + koffi.unregister(this.outboundCallback); + this.outboundCallback = undefined; + } + } + + /** Closes the FFI connection, shuts down the native host, and releases resources. */ + dispose(): void { + if (this.disposed) { + return; + } + this.disposed = true; + + if (this.keepAlive) { + clearInterval(this.keepAlive); + this.keepAlive = null; + } + + 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 97182b5f1c..7d2f7b0fe2 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,16 @@ 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}. + */ +export interface InProcessRuntimeConnection { + readonly kind: "inprocess"; +} + /** * Spawns a runtime child process that listens on a TCP socket and connects to it. */ @@ -183,6 +194,15 @@ 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. + */ + forInProcess(): InProcessRuntimeConnection { + return { kind: "inprocess" }; + }, } as const; /** diff --git a/nodejs/test/e2e/harness/sdkTestContext.ts b/nodejs/test/e2e/harness/sdkTestContext.ts index a59f62126d..8b6d381372 100644 --- a/nodejs/test/e2e/harness/sdkTestContext.ts +++ b/nodejs/test/e2e/harness/sdkTestContext.ts @@ -102,11 +102,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 { 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..7c075c6113 --- /dev/null +++ b/nodejs/test/e2e/inprocess_ffi.e2e.test.ts @@ -0,0 +1,49 @@ +/*--------------------------------------------------------------------------------------------- + * 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", () => { + 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 + }); + + it("should resolve the in-process transport from COPILOT_SDK_DEFAULT_CONNECTION", async () => { + // No explicit connection: the default is resolved from the env var. + const client = new CopilotClient({ + env: { ...process.env, COPILOT_SDK_DEFAULT_CONNECTION: "inprocess" }, + }); + onTestFinishedForceStop(client); + + await client.start(); + + const pong = await client.ping("env default"); + expect(pong.message).toBe("pong: env default"); + + expect(await client.stop()).toHaveLength(0); + }); +}); diff --git a/rust/Cargo.lock b/rust/Cargo.lock index fb7b66e198..969bb1aaf8 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -433,6 +433,7 @@ dependencies = [ "futures-util", "getrandom 0.2.17", "http", + "libloading", "parking_lot", "regex", "reqwest", @@ -773,6 +774,16 @@ version = "0.2.186" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "68ab91017fe16c622486840e4c83c9a37afeff978bd239b5293d61ece587de66" +[[package]] +name = "libloading" +version = "0.8.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d7c4b02199fee7c5d21a5ae7d8cfa79a6ef5bb2fc834d6e9058e89c825efdc55" +dependencies = [ + "cfg-if", + "windows-link", +] + [[package]] name = "libredox" version = "0.1.16" diff --git a/rust/Cargo.toml b/rust/Cargo.toml index 66ef69ad21..078bea1f17 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -48,6 +48,7 @@ tokio-stream = { version = "0.1", features = ["sync"] } tokio-util = { version = "0.7", default-features = false } tracing = "0.1" dirs = "5" +libloading = "0.8" parking_lot = "0.12" regex = "1" getrandom = "0.2" diff --git a/rust/build.rs b/rust/build.rs index 66d1de7bc8..3a26070dca 100644 --- a/rust/build.rs +++ b/rust/build.rs @@ -285,6 +285,21 @@ struct Platform { binary_name: &'static str, } +impl Platform { + /// Natural platform shared-library file name for the FFI runtime cdylib — + /// the tarball's `runtime.node` renamed to what the Rust cdylib would be + /// called on this OS. Derived from the asset name's platform token. + fn runtime_library_name(&self) -> &'static str { + if self.asset_name.contains("win32") { + "copilot_runtime.dll" + } else if self.asset_name.contains("darwin") { + "libcopilot_runtime.dylib" + } else { + "libcopilot_runtime.so" + } + } +} + fn target_platform() -> Option { let os = std::env::var("CARGO_CFG_TARGET_OS").ok()?; let arch = std::env::var("CARGO_CFG_TARGET_ARCH").ok()?; @@ -432,9 +447,101 @@ fn extract_to_cache(archive: &[u8], install_dir: &Path, platform: Platform) -> P final_path.display() ); + // Best-effort: extract the in-process FFI runtime library next to the CLI, + // renamed from the tarball's `prebuilds//runtime.node` to the + // natural platform shared-library name (mirrors the .NET `.targets` and the + // bundled-CLI runtime extraction). Absence is not fatal — stdio/TCP + // transports don't need it, and `Transport::InProcess` surfaces a clear + // error at load time if it's missing. + extract_runtime_library_to_cache(archive, install_dir, platform); + final_path } +/// Write the tarball's `runtime.node` (the FFI cdylib) next to the extracted +/// CLI under the natural platform shared-library name. Best-effort: warns and +/// returns on any failure or when the archive doesn't ship it. +fn extract_runtime_library_to_cache(archive: &[u8], install_dir: &Path, platform: Platform) { + let target = install_dir.join(platform.runtime_library_name()); + if std::fs::metadata(&target) + .map(|m| m.len() > 0) + .unwrap_or(false) + { + return; + } + let Some(bytes) = extract_runtime_library_bytes(archive, platform) else { + // The current GitHub-release tarball ships only the CLI binary; the FFI + // cdylib lives in the npm package layout. Absence is expected — stay + // quiet on the normal path (visible only with `-vv`). + println!( + "Archive `{}` has no runtime.node; Transport::InProcess needs COPILOT_CLI_PATH with a sibling runtime library", + platform.asset_name + ); + return; + }; + + let nanos = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_nanos()) + .unwrap_or(0); + let staging_path = install_dir.join(format!( + ".{}.staging-{}-{nanos}", + platform.runtime_library_name(), + std::process::id(), + )); + if let Err(e) = std::fs::write(&staging_path, &bytes) { + let _ = std::fs::remove_file(&staging_path); + println!( + "cargo:warning=Failed to stage runtime library {}: {e}", + staging_path.display() + ); + return; + } + if let Err(e) = std::fs::rename(&staging_path, &target) { + let _ = std::fs::remove_file(&staging_path); + println!( + "cargo:warning=Failed to place runtime library {}: {e}", + target.display() + ); + return; + } + println!( + "cargo:warning=Extracted in-process runtime library to {}", + target.display() + ); +} + +/// Extract the archive's `prebuilds//runtime.node` bytes, matched by +/// the `runtime.node` filename. Returns `None` when the archive doesn't ship it. +fn extract_runtime_library_bytes(archive: &[u8], platform: Platform) -> Option> { + if platform.asset_name.ends_with(".zip") { + let cursor = std::io::Cursor::new(archive); + let mut zip = zip::ZipArchive::new(cursor).ok()?; + for i in 0..zip.len() { + let mut entry = zip.by_index(i).ok()?; + let name = entry.name().to_string(); + if name == "runtime.node" || name.ends_with("/runtime.node") { + let mut bytes = Vec::with_capacity(entry.size() as usize); + std::io::copy(&mut entry, &mut bytes).ok()?; + return Some(bytes); + } + } + } else { + let gz = flate2::read::GzDecoder::new(archive); + let mut tar = tar::Archive::new(gz); + for entry in tar.entries().ok()? { + let mut entry = entry.ok()?; + let name = entry.path().ok()?.to_string_lossy().into_owned(); + if name == "runtime.node" || name.ends_with("/runtime.node") { + let mut bytes = Vec::with_capacity(entry.size() as usize); + entry.read_to_end(&mut bytes).ok()?; + return Some(bytes); + } + } + } + None +} + /// Replace characters outside `[a-zA-Z0-9._-]` with `_` so the version /// string is always safe to use as a path component. Kept in sync with /// `embeddedcli::sanitize_version` and `resolve::sanitize_version` so all diff --git a/rust/src/embeddedcli.rs b/rust/src/embeddedcli.rs index 56f97e0c0e..6815a44289 100644 --- a/rust/src/embeddedcli.rs +++ b/rust/src/embeddedcli.rs @@ -41,7 +41,7 @@ use std::sync::OnceLock; use std::sync::atomic::{AtomicU64, Ordering}; #[cfg(has_bundled_cli)] -use tracing::{info, warn}; +use tracing::{debug, info, warn}; // When the `bundled-cli` cargo feature is enabled and the target platform is // supported, build.rs generates `bundled_cli.rs` exposing the raw archive @@ -157,8 +157,64 @@ fn default_install_dir(version: &str) -> PathBuf { #[cfg(has_bundled_cli)] const MAX_PUBLISH_ATTEMPTS: u32 = 3; +// Natural platform shared-library name for the in-process FFI runtime cdylib — +// the tarball's `prebuilds//runtime.node` renamed to what the Rust +// cdylib would be called on this OS. Extracted next to the CLI so +// `Transport::InProcess` can load it without a separate download. +#[cfg(all(has_bundled_cli, windows))] +const RUNTIME_LIBRARY_NAME: &str = "copilot_runtime.dll"; +#[cfg(all(has_bundled_cli, target_os = "macos"))] +const RUNTIME_LIBRARY_NAME: &str = "libcopilot_runtime.dylib"; +#[cfg(all(has_bundled_cli, not(windows), not(target_os = "macos")))] +const RUNTIME_LIBRARY_NAME: &str = "libcopilot_runtime.so"; + +/// Install the CLI binary and, best-effort, the sibling in-process FFI runtime +/// library. The runtime library is optional and ships only in archive layouts +/// that include `prebuilds//runtime.node` (the npm package layout); +/// the GitHub-release CLI tarball this build downloads currently ships only the +/// CLI binary, so its absence is expected and non-fatal — stdio/TCP transports +/// don't need it and `Transport::InProcess` reports a clear error at load time. #[cfg(has_bundled_cli)] fn install(install_dir: &Path, archive: &[u8]) -> Result { + let final_path = install_cli(install_dir, archive)?; + if let Err(e) = install_runtime_library(install_dir, archive) { + warn!(error = %e, "failed to publish in-process FFI runtime library"); + } + Ok(final_path) +} + +/// Extract the archive's `runtime.node` (the FFI cdylib) and publish it next to +/// the CLI under the natural platform shared-library name. Idempotent — skips +/// when a non-empty library is already present. When the archive doesn't ship a +/// `runtime.node`, this is a no-op (logged at debug), since the current +/// GitHub-release tarball layout omits it. +#[cfg(has_bundled_cli)] +fn install_runtime_library(install_dir: &Path, archive: &[u8]) -> Result<(), EmbeddedCliError> { + let target = install_dir.join(RUNTIME_LIBRARY_NAME); + if fs::metadata(&target).map(|m| m.len() > 0).unwrap_or(false) { + return Ok(()); + } + let bytes = match extract_binary(archive, "runtime.node") { + Ok(bytes) if !bytes.is_empty() => bytes, + _ => { + debug!( + "bundled archive has no runtime.node; Transport::InProcess requires COPILOT_CLI_PATH \ + pointing at a CLI with a sibling runtime library" + ); + return Ok(()); + } + }; + let tmp = write_temp_file(install_dir, &bytes)?; + if let Err(e) = publish(&tmp, &target) { + let _ = fs::remove_file(&tmp); + return Err(e); + } + debug!(path = %target.display(), "in-process FFI runtime library installed"); + Ok(()) +} + +#[cfg(has_bundled_cli)] +fn install_cli(install_dir: &Path, archive: &[u8]) -> Result { let verbose = std::env::var("COPILOT_CLI_INSTALL_VERBOSE").ok().as_deref() == Some("1"); fs::create_dir_all(install_dir) diff --git a/rust/src/ffi.rs b/rust/src/ffi.rs new file mode 100644 index 0000000000..257b6b72d7 --- /dev/null +++ b/rust/src/ffi.rs @@ -0,0 +1,465 @@ +//! In-process FFI transport: hosts the Copilot runtime in-process by loading +//! the runtime cdylib (`runtime.node`) and speaking JSON-RPC over its C ABI, +//! instead of spawning a CLI child process and communicating over stdio/TCP. +//! +//! The runtime's `host_start` export spawns the residual TypeScript worker +//! itself — the packaged single-file CLI (`copilot --embedded-host`) or, for +//! dev, `node dist-cli/index.js --embedded-host`. JSON-RPC frames are pumped +//! across the ABI: writes go to `connection_write`; inbound frames arrive on a +//! native callback that feeds an async reader. The framing is unchanged — the +//! same LSP `Content-Length:` frames the stdio transport uses. + +use std::ffi::c_void; +use std::path::{Path, PathBuf}; +use std::pin::Pin; +use std::sync::Arc; +use std::sync::atomic::{AtomicBool, AtomicPtr, AtomicU32, Ordering}; +use std::task::{Context, Poll}; + +use libloading::Library; +use tokio::io::{AsyncRead, AsyncWrite, ReadBuf}; +use tokio::sync::mpsc; +use tracing::debug; + +use crate::{Error, ErrorKind}; + +type OutboundCallback = unsafe extern "C" fn(*mut c_void, *const u8, usize); +type HostStartFn = unsafe extern "C" fn(*const u8, usize, *const u8, usize) -> u32; +type HostShutdownFn = unsafe extern "C" fn(u32) -> bool; +#[allow(clippy::type_complexity)] +type ConnectionOpenFn = unsafe extern "C" fn( + u32, + OutboundCallback, + *mut c_void, + *const u8, + usize, + *const u8, + usize, + *const u8, + usize, +) -> u32; +type ConnectionWriteFn = unsafe extern "C" fn(u32, *const u8, usize) -> bool; +type ConnectionCloseFn = unsafe extern "C" fn(u32) -> bool; + +/// State handed to the native side as `user_data` so the outbound callback can +/// route inbound frames back to the reader. +struct CallbackState { + tx: mpsc::UnboundedSender>, +} + +extern "C" fn on_outbound(user_data: *mut c_void, bytes: *const u8, len: usize) { + if user_data.is_null() || bytes.is_null() || len == 0 { + return; + } + let state = unsafe { &*(user_data as *const CallbackState) }; + let slice = unsafe { std::slice::from_raw_parts(bytes, len) }; + let _ = state.tx.send(slice.to_vec()); +} + +/// Loaded library, bound exports, and connection lifecycle state, shared +/// between the [`FfiWriter`] and the owning [`Client`]. Kept alive for the +/// connection's lifetime; dropping it unloads the cdylib. +pub(crate) struct FfiShared { + _lib: Library, + host_shutdown: HostShutdownFn, + connection_write: ConnectionWriteFn, + connection_close: ConnectionCloseFn, + server_id: AtomicU32, + connection_id: AtomicU32, + callback_state: AtomicPtr, + closed: AtomicBool, + library_path: PathBuf, +} + +// The raw fn pointers and the boxed callback state are safe to move across +// threads: the native side copies buffers synchronously and the callback only +// forwards to a thread-safe channel sender. +unsafe impl Send for FfiShared {} +unsafe impl Sync for FfiShared {} + +impl FfiShared { + /// Close the connection, shut the host down, and free the callback state. + /// Idempotent; called from [`Client::stop`], drop, and on startup failure. + pub(crate) fn close(&self) { + if self.closed.swap(true, Ordering::SeqCst) { + return; + } + let conn = self.connection_id.swap(0, Ordering::SeqCst); + if conn != 0 { + unsafe { (self.connection_close)(conn) }; + } + let server = self.server_id.swap(0, Ordering::SeqCst); + if server != 0 { + unsafe { (self.host_shutdown)(server) }; + } + // Free the callback state only after the connection is closed and the + // host is shut down, so native can no longer invoke the callback. + let state = self + .callback_state + .swap(std::ptr::null_mut(), Ordering::SeqCst); + if !state.is_null() { + drop(unsafe { Box::from_raw(state) }); + } + debug!(library = %self.library_path.display(), "FFI runtime connection closed"); + } + + fn write_frame(&self, frame: &[u8]) -> bool { + if self.closed.load(Ordering::SeqCst) { + return false; + } + let conn = self.connection_id.load(Ordering::SeqCst); + if conn == 0 { + return false; + } + unsafe { (self.connection_write)(conn, frame.as_ptr(), frame.len()) } + } +} + +impl Drop for FfiShared { + fn drop(&mut self) { + self.close(); + } +} + +/// Read side of the FFI transport, fed by the native outbound callback via an +/// unbounded channel. Implements [`AsyncRead`] for the JSON-RPC read loop. +pub(crate) struct FfiReader { + rx: mpsc::UnboundedReceiver>, + leftover: Vec, + pos: usize, +} + +impl AsyncRead for FfiReader { + fn poll_read( + mut self: Pin<&mut Self>, + cx: &mut Context<'_>, + buf: &mut ReadBuf<'_>, + ) -> Poll> { + if self.pos >= self.leftover.len() { + match self.rx.poll_recv(cx) { + Poll::Ready(Some(chunk)) => { + self.leftover = chunk; + self.pos = 0; + } + Poll::Ready(None) => return Poll::Ready(Ok(())), + Poll::Pending => return Poll::Pending, + } + } + let available = self.leftover.len() - self.pos; + let n = available.min(buf.remaining()); + let start = self.pos; + buf.put_slice(&self.leftover[start..start + n]); + self.pos += n; + Poll::Ready(Ok(())) + } +} + +/// Write side of the FFI transport. Each frame is forwarded synchronously to +/// the native `connection_write` export (native copies before returning). +pub(crate) struct FfiWriter { + shared: Arc, +} + +impl AsyncWrite for FfiWriter { + fn poll_write( + self: Pin<&mut Self>, + _cx: &mut Context<'_>, + buf: &[u8], + ) -> Poll> { + if self.shared.write_frame(buf) { + Poll::Ready(Ok(buf.len())) + } else { + Poll::Ready(Err(std::io::Error::new( + std::io::ErrorKind::BrokenPipe, + "failed to write a frame to the in-process runtime connection", + ))) + } + } + + fn poll_flush(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } + + fn poll_shutdown(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) + } +} + +/// Prepared FFI host: the loaded cdylib plus the spawn arguments needed to +/// start the runtime worker. +pub(crate) struct FfiHost { + lib: Library, + library_path: PathBuf, + entrypoint: PathBuf, + environment: Vec<(String, String)>, + host_start: HostStartFn, + host_shutdown: HostShutdownFn, + connection_open: ConnectionOpenFn, + connection_write: ConnectionWriteFn, + connection_close: ConnectionCloseFn, +} + +// SAFETY: as for `FfiShared` — the bound exports are plain fn pointers and the +// library handle is safe to move to the blocking thread that starts the host. +unsafe impl Send for FfiHost {} + +impl FfiHost { + /// Load the cdylib next to `entrypoint` and bind its exports. + /// + /// `entrypoint` is the packaged single-file CLI binary or, for dev, a + /// `.js` file launched via `node`. The cdylib is resolved relative to the + /// entrypoint directory: first the flat natural shared-library name, then + /// the dev/tarball `prebuilds/-/runtime.node` layout. + pub(crate) fn create( + entrypoint: &Path, + environment: Vec<(String, String)>, + ) -> Result { + let library_path = resolve_library_path(entrypoint)?; + let lib = unsafe { Library::new(&library_path) }.map_err(|e| { + Error::with_message( + ErrorKind::InvalidConfig, + format!( + "failed to load in-process runtime library '{}': {e}", + library_path.display() + ), + ) + })?; + + let host_start = + *bind::(&lib, b"copilot_runtime_host_start\0", &library_path)?; + let host_shutdown = + *bind::(&lib, b"copilot_runtime_host_shutdown\0", &library_path)?; + let connection_open = + *bind::(&lib, b"copilot_runtime_connection_open\0", &library_path)?; + let connection_write = + *bind::(&lib, b"copilot_runtime_connection_write\0", &library_path)?; + let connection_close = + *bind::(&lib, b"copilot_runtime_connection_close\0", &library_path)?; + + Ok(Self { + lib, + library_path, + entrypoint: entrypoint.to_path_buf(), + environment, + host_start, + host_shutdown, + connection_open, + connection_write, + connection_close, + }) + } + + /// Start the runtime worker and open the FFI JSON-RPC connection. + /// + /// `host_start` blocks until the worker connects back and signals + /// readiness (up to ~30s), and must not run on an async executor thread, so + /// the blocking handshake is offloaded to [`tokio::task::spawn_blocking`]. + pub(crate) async fn start(self) -> Result<(FfiReader, FfiWriter, Arc), Error> { + tokio::task::spawn_blocking(move || self.start_blocking()) + .await + .map_err(|e| { + Error::with_message( + ErrorKind::InvalidConfig, + format!("in-process runtime startup task failed: {e}"), + ) + })? + } + + fn start_blocking(self) -> Result<(FfiReader, FfiWriter, Arc), Error> { + let argv = build_argv_json(&self.entrypoint); + let env = build_env_json(&self.environment); + + let (env_ptr, env_len) = match &env { + Some(bytes) => (bytes.as_ptr(), bytes.len()), + None => (std::ptr::null(), 0), + }; + let server_id = unsafe { (self.host_start)(argv.as_ptr(), argv.len(), env_ptr, env_len) }; + if server_id == 0 { + return Err(Error::with_message( + ErrorKind::InvalidConfig, + format!( + "copilot_runtime_host_start failed (library '{}', entrypoint '{}')", + self.library_path.display(), + self.entrypoint.display() + ), + )); + } + + let (tx, rx) = mpsc::unbounded_channel::>(); + let state_ptr = Box::into_raw(Box::new(CallbackState { tx })); + let connection_id = unsafe { + (self.connection_open)( + server_id, + on_outbound, + state_ptr as *mut c_void, + std::ptr::null(), + 0, + std::ptr::null(), + 0, + std::ptr::null(), + 0, + ) + }; + if connection_id == 0 { + drop(unsafe { Box::from_raw(state_ptr) }); + unsafe { (self.host_shutdown)(server_id) }; + return Err(Error::with_message( + ErrorKind::InvalidConfig, + "copilot_runtime_connection_open failed", + )); + } + + let shared = Arc::new(FfiShared { + _lib: self.lib, + host_shutdown: self.host_shutdown, + connection_write: self.connection_write, + connection_close: self.connection_close, + server_id: AtomicU32::new(server_id), + connection_id: AtomicU32::new(connection_id), + callback_state: AtomicPtr::new(state_ptr), + closed: AtomicBool::new(false), + library_path: self.library_path.clone(), + }); + + debug!( + library = %self.library_path.display(), + server_id, connection_id, "FFI runtime host started" + ); + + let reader = FfiReader { + rx, + leftover: Vec::new(), + pos: 0, + }; + let writer = FfiWriter { + shared: Arc::clone(&shared), + }; + Ok((reader, writer, shared)) + } +} + +fn bind<'lib, T>( + lib: &'lib Library, + symbol: &[u8], + library_path: &Path, +) -> Result, Error> { + unsafe { lib.get::(symbol) }.map_err(|e| { + Error::with_message( + ErrorKind::InvalidConfig, + format!( + "in-process runtime library '{}' is missing an expected export ({}): {e}", + library_path.display(), + String::from_utf8_lossy(symbol.strip_suffix(b"\0").unwrap_or(symbol)) + ), + ) + }) +} + +/// The natural platform shared-library file name for the runtime cdylib — the +/// `.node` file renamed to what the Rust cdylib would be called on this OS. +fn natural_library_name() -> &'static str { + if cfg!(windows) { + "copilot_runtime.dll" + } else if cfg!(target_os = "macos") { + "libcopilot_runtime.dylib" + } else { + "libcopilot_runtime.so" + } +} + +/// The napi-rs 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`. +pub(crate) fn prebuilds_folder() -> Option { + let platform = if cfg!(target_os = "windows") { + "win32" + } else if cfg!(target_os = "macos") { + "darwin" + } else if cfg!(target_os = "linux") { + "linux" + } else { + return None; + }; + let arch = if cfg!(target_arch = "x86_64") { + "x64" + } else if cfg!(target_arch = "aarch64") { + "arm64" + } else { + return None; + }; + Some(format!("{platform}-{arch}")) +} + +fn resolve_library_path(entrypoint: &Path) -> Result { + let dir = entrypoint.parent().ok_or_else(|| { + Error::with_message( + ErrorKind::InvalidConfig, + format!( + "could not determine directory for CLI entrypoint '{}'", + entrypoint.display() + ), + ) + })?; + + // Bundled/flat layout: natural shared-library name next to the CLI. + let flat = dir.join(natural_library_name()); + if flat.is_file() { + return Ok(flat); + } + + // Dev/tarball layout: prebuilds/-/runtime.node. + let prebuilds = + prebuilds_folder().map(|folder| dir.join("prebuilds").join(folder).join("runtime.node")); + if let Some(prebuilds_path) = &prebuilds + && prebuilds_path.is_file() + { + return Ok(prebuilds_path.clone()); + } + + let searched = match &prebuilds { + Some(p) => format!("'{}' and '{}'", flat.display(), p.display()), + None => format!("'{}'", flat.display()), + }; + Err(Error::with_message( + ErrorKind::BinaryNotFound { + name: "runtime.node".into(), + hint: Some(format!( + "in-process runtime library not found; looked for {searched}. Ensure the \ + bundled CLI's sibling runtime library is present or set COPILOT_CLI_PATH to a \ + CLI whose directory contains it." + )), + }, + "in-process runtime library not found", + )) +} + +fn build_argv_json(entrypoint: &Path) -> Vec { + // A `.js` entrypoint (dev / dist-cli) is launched via node; the packaged + // single-file CLI binary embeds its own Node and is invoked directly. + let entrypoint_str = entrypoint.to_string_lossy().into_owned(); + let is_js = entrypoint + .extension() + .and_then(|ext| ext.to_str()) + .is_some_and(|ext| ext.eq_ignore_ascii_case("js")); + let argv: Vec = if is_js { + vec![ + "node".to_string(), + entrypoint_str, + "--embedded-host".to_string(), + ] + } else { + vec![entrypoint_str, "--embedded-host".to_string()] + }; + serde_json::to_vec(&argv).expect("argv serializes") +} + +fn build_env_json(environment: &[(String, String)]) -> Option> { + if environment.is_empty() { + return None; + } + let map: serde_json::Map = environment + .iter() + .map(|(k, v)| (k.clone(), serde_json::Value::String(v.clone()))) + .collect(); + Some(serde_json::to_vec(&map).expect("env serializes")) +} diff --git a/rust/src/lib.rs b/rust/src/lib.rs index 0333281f59..392db08bad 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -10,6 +10,8 @@ mod canvas_dispatch; #[cfg(feature = "bundled-cli")] pub(crate) mod embeddedcli; mod errors; +/// In-process FFI transport hosting the runtime cdylib (`Transport::InProcess`). +pub(crate) mod ffi; pub use errors::*; /// Connection-level Copilot request handler — intercept and replace the /// model-layer HTTP and WebSocket traffic the runtime issues for both CAPI and @@ -112,6 +114,13 @@ pub enum Transport { /// Communicate over stdin/stdout pipes (default). #[default] Stdio, + /// Host the runtime in-process over FFI (no child process). + /// + /// Loads the runtime cdylib (`runtime.node`) next to the resolved CLI + /// entrypoint and speaks JSON-RPC over its C ABI. The runtime spawns its + /// own worker; the SDK never launches a CLI child process. This is the + /// Rust analogue of the .NET `RuntimeConnection.ForInProcess()`. + InProcess, /// Spawn the CLI with `--port` and connect via TCP. Tcp { /// Port to listen on (0 for OS-assigned). @@ -850,6 +859,38 @@ fn generate_connection_token() -> String { hex } +/// Environment variable that overrides the transport used when the caller +/// leaves [`ClientOptions::transport`] at its default ([`Transport::Stdio`]). +/// Accepts `"inprocess"` or `"stdio"` (case-insensitive); unset preserves +/// stdio. Any other value is an error. Mirrors the .NET +/// `COPILOT_SDK_DEFAULT_CONNECTION` handling. +const DEFAULT_CONNECTION_ENV_VAR: &str = "COPILOT_SDK_DEFAULT_CONNECTION"; + +/// Resolve a transport override from [`DEFAULT_CONNECTION_ENV_VAR`], checking +/// [`ClientOptions::env`] first, then the process environment. Returns +/// `Ok(None)` when the override is absent or selects stdio. +fn resolve_default_transport(options: &ClientOptions) -> Result> { + let value = options + .env + .iter() + .find(|(key, _)| key.as_os_str() == std::ffi::OsStr::new(DEFAULT_CONNECTION_ENV_VAR)) + .map(|(_, value)| value.to_string_lossy().into_owned()) + .or_else(|| std::env::var(DEFAULT_CONNECTION_ENV_VAR).ok()); + + match value.as_deref() { + None => Ok(None), + Some(v) if v.is_empty() || v.eq_ignore_ascii_case("stdio") => Ok(None), + Some(v) if v.eq_ignore_ascii_case("inprocess") => Ok(Some(Transport::InProcess)), + Some(v) => Err(Error::with_message( + ErrorKind::InvalidConfig, + format!( + "invalid {DEFAULT_CONNECTION_ENV_VAR} value '{v}'. \ + Expected 'inprocess', 'stdio', or unset." + ), + )), + } +} + /// Connection to a GitHub Copilot CLI server (stdio, TCP, or external). /// /// Cheaply cloneable — cloning shares the underlying connection. @@ -870,6 +911,9 @@ impl std::fmt::Debug for Client { struct ClientInner { child: parking_lot::Mutex>, + /// In-process FFI runtime host, set only for [`Transport::InProcess`]. + /// Closing it tears down the FFI connection and unloads the cdylib. + ffi_host: parking_lot::Mutex>>, rpc: JsonRpcClient, cwd: PathBuf, request_rx: parking_lot::Mutex>>, @@ -929,6 +973,16 @@ impl Client { if let Some(cfg) = &options.session_fs { validate_session_fs_config(cfg)?; } + let mut options = options; + // Honor COPILOT_SDK_DEFAULT_CONNECTION when the caller leaves the + // transport at its default (stdio). Mirrors the .NET + // `ResolveDefaultConnection`: `inprocess` selects in-process FFI + // hosting, `stdio`/unset keeps stdio, anything else is an error. + if matches!(options.transport, Transport::Stdio) + && let Some(resolved) = resolve_default_transport(&options)? + { + options.transport = resolved; + } // Auth options only make sense when the SDK spawns the CLI; with an // external server, the server manages its own auth. if matches!(options.transport, Transport::External { .. }) { @@ -970,9 +1024,8 @@ impl Client { // to the server. For Tcp, the SDK auto-generates one when the // caller leaves it unset so the loopback listener is safe by // default. - let mut options = options; let effective_connection_token: Option = match &mut options.transport { - Transport::Stdio => None, + Transport::Stdio | Transport::InProcess => None, Transport::Tcp { connection_token, .. } => Some( @@ -1094,8 +1147,28 @@ impl Client { options.mode, )? } + Transport::InProcess => { + let environment = Self::build_ffi_environment(&options); + info!(entrypoint = %program.display(), "hosting copilot runtime in-process (FFI)"); + let host = crate::ffi::FfiHost::create(&program, environment)?; + let (reader, writer, shared) = host.start().await?; + let client = Self::from_transport( + reader, + writer, + None, + options.working_directory, + options.on_list_models, + session_fs_config.is_some(), + session_fs_sqlite_declared, + options.on_get_trace_context, + options.on_github_telemetry, + effective_connection_token.clone(), + options.mode, + )?; + *client.inner.ffi_host.lock() = Some(shared); + client + } }; - debug!( elapsed_ms = start_time.elapsed().as_millis(), "Client::start transport setup complete" @@ -1294,6 +1367,7 @@ impl Client { let client = Self { inner: Arc::new(ClientInner { child: parking_lot::Mutex::new(child), + ffi_host: parking_lot::Mutex::new(None), rpc, cwd, request_rx: parking_lot::Mutex::new(Some(request_rx)), @@ -1362,6 +1436,66 @@ impl Client { }); } + fn build_ffi_environment(options: &ClientOptions) -> Vec<(String, String)> { + let mut env: std::collections::BTreeMap = std::env::vars_os() + .map(|(k, v)| { + ( + k.to_string_lossy().into_owned(), + v.to_string_lossy().into_owned(), + ) + }) + .collect(); + let mut set = |key: &str, value: String| { + env.insert(key.to_string(), value); + }; + if let Some(token) = &options.github_token { + set("COPILOT_SDK_AUTH_TOKEN", token.clone()); + } + if let Some(telemetry) = &options.telemetry { + set("COPILOT_OTEL_ENABLED", "true".to_string()); + if let Some(endpoint) = &telemetry.otlp_endpoint { + set("OTEL_EXPORTER_OTLP_ENDPOINT", endpoint.clone()); + } + if let Some(protocol) = telemetry.otlp_protocol { + set("OTEL_EXPORTER_OTLP_PROTOCOL", protocol.as_str().to_string()); + } + if let Some(path) = &telemetry.file_path { + set( + "COPILOT_OTEL_FILE_EXPORTER_PATH", + path.to_string_lossy().into_owned(), + ); + } + if let Some(exporter) = telemetry.exporter_type { + set("COPILOT_OTEL_EXPORTER_TYPE", exporter.as_str().to_string()); + } + if let Some(source) = &telemetry.source_name { + set("COPILOT_OTEL_SOURCE_NAME", source.clone()); + } + if let Some(capture) = telemetry.capture_content { + set( + "OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT", + if capture { "true" } else { "false" }.to_string(), + ); + } + } + if let Some(dir) = &options.base_directory { + set("COPILOT_HOME", dir.to_string_lossy().into_owned()); + } + if options.mode == ClientMode::Empty { + set("COPILOT_DISABLE_KEYTAR", "1".to_string()); + } + for (key, value) in &options.env { + env.insert( + key.to_string_lossy().into_owned(), + value.to_string_lossy().into_owned(), + ); + } + for key in &options.env_remove { + env.remove(&*key.to_string_lossy()); + } + env.into_iter().collect() + } + fn build_command(program: &Path, options: &ClientOptions) -> Command { let mut command = Command::new(program); for arg in &options.prefix_args { @@ -2063,7 +2197,8 @@ impl Client { self.inner.router.unregister(&session_id); } - let should_shutdown_runtime = self.inner.child.lock().is_some(); + let should_shutdown_runtime = + self.inner.child.lock().is_some() || self.inner.ffi_host.lock().is_some(); if should_shutdown_runtime { let runtime_shutdown_start = Instant::now(); match tokio::time::timeout(RUNTIME_SHUTDOWN_TIMEOUT, self.rpc().runtime().shutdown()) @@ -2120,6 +2255,13 @@ impl Client { } } + // In-process FFI host: close the connection, shut the host down, and + // unload the cdylib. The runtime.shutdown RPC above already asked the + // worker to clean up; closing here tears down the transport. + if let Some(host) = self.inner.ffi_host.lock().take() { + host.close(); + } + info!(pid = ?pid, errors = errors.len(), "CLI process stopped"); if errors.is_empty() { Ok(()) @@ -2165,6 +2307,9 @@ impl Client { { error!(pid = ?pid, error = %e, "failed to send kill signal"); } + if let Some(host) = self.inner.ffi_host.lock().take() { + host.close(); + } self.inner.rpc.force_close(); // Drop all session channels so any awaiters see a closed channel // instead of waiting for responses that will never arrive. @@ -2222,6 +2367,9 @@ impl Drop for ClientInner { info!(pid = ?pid, "kill signal sent for CLI process on drop"); } } + if let Some(host) = self.ffi_host.lock().take() { + host.close(); + } } } @@ -2798,6 +2946,7 @@ mod tests { Client { inner: Arc::new(ClientInner { child: parking_lot::Mutex::new(None), + ffi_host: parking_lot::Mutex::new(None), rpc: { let (req_tx, _req_rx) = mpsc::unbounded_channel(); let (notif_tx, _notif_rx) = broadcast::channel(16); diff --git a/rust/tests/e2e.rs b/rust/tests/e2e.rs index 62412963b8..c60d095170 100644 --- a/rust/tests/e2e.rs +++ b/rust/tests/e2e.rs @@ -37,6 +37,8 @@ mod github_telemetry; mod hooks; #[path = "e2e/hooks_extended.rs"] mod hooks_extended; +#[path = "e2e/inprocess.rs"] +mod inprocess; #[path = "e2e/mcp_and_agents.rs"] mod mcp_and_agents; #[path = "e2e/mcp_oauth.rs"] diff --git a/rust/tests/e2e/inprocess.rs b/rust/tests/e2e/inprocess.rs new file mode 100644 index 0000000000..bc9d6c855e --- /dev/null +++ b/rust/tests/e2e/inprocess.rs @@ -0,0 +1,27 @@ +use super::support::with_e2e_context; + +/// Mirrors the .NET `Should_Start_And_Connect_Over_InProcess_Ffi`: start a +/// client that hosts the runtime in-process over FFI, perform a simple +/// round-trip, and stop cleanly. Fails hard (does not skip) if the in-process +/// runtime library can't be loaded. +#[tokio::test] +async fn should_start_ping_and_stop_inprocess_client() { + with_e2e_context("client", "should_start_ping_and_stop_stdio_client", |ctx| { + Box::pin(async move { + let client = ctx.start_inprocess_client().await; + + let response = client + .ping(Some("hello from rust in-process")) + .await + .expect("ping over in-process FFI transport"); + assert_eq!(response.message, "pong: hello from rust in-process"); + assert!(!response.timestamp.is_empty()); + + let status = client.get_status().await.expect("get status"); + assert!(status.protocol_version > 0); + + client.stop().await.expect("stop in-process client"); + }) + }) + .await; +} diff --git a/rust/tests/e2e/support.rs b/rust/tests/e2e/support.rs index 7e47d8fbae..a00fc05b5f 100644 --- a/rust/tests/e2e/support.rs +++ b/rust/tests/e2e/support.rs @@ -170,6 +170,24 @@ impl E2eContext { .expect("start E2E client") } + /// Start a client that hosts the runtime in-process over FFI + /// ([`Transport::InProcess`]). Unlike the stdio harness, the CLI + /// entrypoint is passed as the program directly (the FFI host builds the + /// `node --embedded-host` argv itself and loads the sibling + /// runtime cdylib), so a `.js` entrypoint is not split into node + + /// prefix_args here. + pub async fn start_inprocess_client(&self) -> Client { + let options = ClientOptions::new() + .with_cwd(self.work_dir.path()) + .with_env(self.environment()) + .with_use_logged_in_user(false) + .with_program(CliProgram::Path(self.cli_path.clone())) + .with_transport(Transport::InProcess); + Client::start(options) + .await + .expect("start in-process FFI E2E client") + } + /// Start a client wired to a Copilot request handler, appending `extra_env` /// to the spawned runtime's environment (used to flip the WebSocket ExP /// flag for the WS transport tests). @@ -626,6 +644,18 @@ fn client_options_for_cli( .with_cwd(cwd) .with_env(env) .with_use_logged_in_user(false); + // When the in-process FFI transport is the default (matrix cell that sets + // COPILOT_SDK_DEFAULT_CONNECTION=inprocess), pass the CLI entrypoint + // directly: the FFI host builds the `node --embedded-host` + // argv itself and loads the sibling runtime cdylib. Splitting a `.js` + // entrypoint into node + prefix_args (the stdio layout) would point the + // library resolver at node's directory instead. + let inprocess_default = std::env::var("COPILOT_SDK_DEFAULT_CONNECTION") + .map(|value| value.eq_ignore_ascii_case("inprocess")) + .unwrap_or(false); + if inprocess_default { + return options.with_program(CliProgram::Path(cli_path.to_path_buf())); + } if cli_path .extension() .and_then(|extension| extension.to_str()) From 259911fb1ee8c46a1fc600ab354f6b89d2bff3d9 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Mon, 6 Jul 2026 15:46:10 +0000 Subject: [PATCH 02/33] Remove debug --log-level flag from in-process FFI argv Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/src/ffiRuntimeHost.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nodejs/src/ffiRuntimeHost.ts b/nodejs/src/ffiRuntimeHost.ts index 681cac7ac8..44afb96ee0 100644 --- a/nodejs/src/ffiRuntimeHost.ts +++ b/nodejs/src/ffiRuntimeHost.ts @@ -97,8 +97,8 @@ 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", "--log-level", "all"] - : [cliEntrypoint, "--embedded-host", "--log-level", "all"]; + ? ["node", cliEntrypoint, "--embedded-host"] + : [cliEntrypoint, "--embedded-host"]; return Buffer.from(JSON.stringify(argv), "utf8"); } From c4bba5074f64bb925f6c3b619f018e9266febbd9 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 7 Jul 2026 14:23:27 +0000 Subject: [PATCH 03/33] TS: align in-process (FFI) transport with .NET behavior Per-client options that lower to environment variables are not honored by the in-process transport, because the native runtime loads into the shared host process and its worker inherits that process's ambient environment. Match .NET: the in-process path no longer wires `env`, `telemetry`, `gitHubToken`, or `baseDirectory` into the FFI worker (previously it passed a partially-honored env block); the worker now inherits the ambient host environment. This is tracked as a shared cross-SDK gap in #1934 (workaround: set the variables on the host process environment). - client.ts: startInProcessFfi passes no per-client env; buildRuntimeEnv is now child-process (stdio/tcp) only. - types.ts: document + mark @experimental that in-process ignores env-lowered options; point to #1934. - E2E harness: for in-process, mirror the per-test redirects/home/credentials onto the real process env (Node process.env writes reach native getenv), route auth via GH_TOKEN/GITHUB_TOKEN, disable HMAC so host-side auth picks the Bearer token the replay snapshots expect, and restore the mutated entries afterwards. - Narrow the dedicated in-process E2E test to the single explicit ForInProcess smoke test, matching .NET; env-var resolution is covered by the inprocess CI matrix cell running the full suite. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/src/client.ts | 18 +++++++--- nodejs/src/types.ts | 16 +++++++++ nodejs/test/e2e/harness/sdkTestContext.ts | 41 ++++++++++++++++++++++- nodejs/test/e2e/inprocess_ffi.e2e.test.ts | 18 ++-------- 4 files changed, 72 insertions(+), 21 deletions(-) diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index b2e4855a41..238361b21e 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -2255,10 +2255,11 @@ export class CopilotClient { } /** - * Builds the environment for the spawned runtime worker (used by both the stdio/TCP - * child process and the in-process FFI host): applies the auth token, connection - * token, `COPILOT_HOME`, keychain setting, and telemetry variables on top of the - * effective env. + * 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 }; @@ -2501,13 +2502,20 @@ export class CopilotClient { /** * 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(), - this.buildRuntimeEnv(), + undefined, this.options.workingDirectory ); this.ffiHost = host; diff --git a/nodejs/src/types.ts b/nodejs/src/types.ts index 7d2f7b0fe2..de2055857c 100644 --- a/nodejs/src/types.ts +++ b/nodejs/src/types.ts @@ -117,6 +117,16 @@ export interface StdioRuntimeConnection { * 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"; @@ -199,6 +209,12 @@ export const RuntimeConnection = { * 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" }; diff --git a/nodejs/test/e2e/harness/sdkTestContext.ts b/nodejs/test/e2e/harness/sdkTestContext.ts index 8b6d381372..6920f2fabe 100644 --- a/nodejs/test/e2e/harness/sdkTestContext.ts +++ b/nodejs/test/e2e/harness/sdkTestContext.ts @@ -120,9 +120,39 @@ 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 restoreProcessEnv: Array<[string, string | undefined]> = []; + if (isInProcess) { + const inProcessEnv: Record = { + ...(mergedEnv as Record), + GH_TOKEN: authTokenToUse, + GITHUB_TOKEN: authTokenToUse, + COPILOT_HMAC_KEY: "", + CAPI_HMAC_KEY: "", + }; + for (const [key, value] of Object.entries(inProcessEnv)) { + restoreProcessEnv.push([key, process.env[key]]); + process.env[key] = value; + } + } + const copilotClient = new CopilotClient({ workingDirectory: workDir, - env: { ...env, ...userEnv }, + // In-process hosting mirrors the environment onto the real process (above), 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, @@ -159,6 +189,15 @@ export async function createSdkTestContext({ afterAll(async () => { await copilotClient.stop(); await openAiEndpoint.stop(anyTestFailed); + // Restore any process-environment entries the in-process path mirrored, so + // the mutation doesn't leak into other suites sharing this worker process. + for (const [key, previous] of restoreProcessEnv.reverse()) { + if (previous === undefined) { + delete process.env[key]; + } else { + process.env[key] = previous; + } + } await rmDir("remove e2e test copilotHomeDir", copilotHomeDir); await rmDir("remove e2e test homeDir", homeDir); await rmDir("remove e2e test workDir", workDir); diff --git a/nodejs/test/e2e/inprocess_ffi.e2e.test.ts b/nodejs/test/e2e/inprocess_ffi.e2e.test.ts index 7c075c6113..28b0380a96 100644 --- a/nodejs/test/e2e/inprocess_ffi.e2e.test.ts +++ b/nodejs/test/e2e/inprocess_ffi.e2e.test.ts @@ -16,6 +16,9 @@ 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. 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 @@ -31,19 +34,4 @@ describe("In-process FFI transport", () => { expect(await client.stop()).toHaveLength(0); // No errors on stop }); - - it("should resolve the in-process transport from COPILOT_SDK_DEFAULT_CONNECTION", async () => { - // No explicit connection: the default is resolved from the env var. - const client = new CopilotClient({ - env: { ...process.env, COPILOT_SDK_DEFAULT_CONNECTION: "inprocess" }, - }); - onTestFinishedForceStop(client); - - await client.start(); - - const pong = await client.ping("env default"); - expect(pong.message).toBe("pong: env default"); - - expect(await client.stop()).toHaveLength(0); - }); }); From 4d211d95586b6c2d4e6ef6b9fda60529c190d8f1 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 7 Jul 2026 14:40:27 +0000 Subject: [PATCH 04/33] TS: fix in-process E2E env mirroring (per-test proxy + HMAC) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit My previous commit mirrored the per-test environment onto process.env once at context construction. Since a test file contains several createSdkTestContext calls (each with its own CapiProxy on a distinct port) and process.env is a single global, the last-constructed context won — so every in-process worker inherited the wrong proxy URL and requests hit an unconfigured/torn-down proxy (ECONNREFUSED / "ReplayingCapiProxy not yet initialized"). That regressed the in-process cell from 9 to 32 failures. Mirror per-test instead: apply this context's environment in beforeEach (the client auto-starts on first use inside the test body, so the worker spawns under the right values) and restore in afterEach. vitest runs tests within a file serially and separate files in separate forks, so the global process.env is coherent for the running test. Also neutralize the ambient COPILOT_HMAC_KEY/CAPI_HMAC_KEY at module load (the analogue of .NET's InProcessEnvIsolation ModuleInitializer): in-process auth resolves host-side in this process and ranks HMAC above the GitHub token, and the key can be captured as early as client construction — before any beforeEach runs — so clearing it only per-test is too late. Gated to the in-process transport; stdio/tcp resolve auth in their own process where the token outranks HMAC. Verified locally with an ambient COPILOT_HMAC_KEY set: the previously-failing multi-context files (rpc, session, rpc_session_state) now pass in-process, and their embedded stdio/tcp sub-tests are unaffected. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/test/e2e/harness/sdkTestContext.ts | 72 +++++++++++++++-------- 1 file changed, 47 insertions(+), 25 deletions(-) diff --git a/nodejs/test/e2e/harness/sdkTestContext.ts b/nodejs/test/e2e/harness/sdkTestContext.ts index 6920f2fabe..b813abadd2 100644 --- a/nodejs/test/e2e/harness/sdkTestContext.ts +++ b/nodejs/test/e2e/harness/sdkTestContext.ts @@ -16,6 +16,19 @@ import { formatError, retry } from "./sdkTestHelper"; export const isCI = process.env.GITHUB_ACTIONS === "true"; export const DEFAULT_GITHUB_TOKEN = "fake-token-for-e2e-tests"; +// 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"); @@ -133,25 +146,21 @@ export async function createSdkTestContext({ // `--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 restoreProcessEnv: Array<[string, string | undefined]> = []; - if (isInProcess) { - const inProcessEnv: Record = { - ...(mergedEnv as Record), - GH_TOKEN: authTokenToUse, - GITHUB_TOKEN: authTokenToUse, - COPILOT_HMAC_KEY: "", - CAPI_HMAC_KEY: "", - }; - for (const [key, value] of Object.entries(inProcessEnv)) { - restoreProcessEnv.push([key, process.env[key]]); - process.env[key] = value; - } - } + 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, - // In-process hosting mirrors the environment onto the real process (above), so - // the worker inherits it; passing a per-client env here would have no effect. + // 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, @@ -164,6 +173,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 @@ -171,6 +183,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, @@ -182,15 +204,7 @@ export async function createSdkTestContext({ }); afterEach(async () => { - // Empty directories but leave them in place for next test - await rimraf([join(homeDir, "*"), join(workDir, "*")], { glob: true }); - }); - - afterAll(async () => { - await copilotClient.stop(); - await openAiEndpoint.stop(anyTestFailed); - // Restore any process-environment entries the in-process path mirrored, so - // the mutation doesn't leak into other suites sharing this worker process. + // 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]; @@ -198,6 +212,14 @@ export async function createSdkTestContext({ process.env[key] = previous; } } + restoreProcessEnv = []; + // Empty directories but leave them in place for next test + await rimraf([join(homeDir, "*"), join(workDir, "*")], { glob: true }); + }); + + afterAll(async () => { + await copilotClient.stop(); + await openAiEndpoint.stop(anyTestFailed); await rmDir("remove e2e test copilotHomeDir", copilotHomeDir); await rmDir("remove e2e test homeDir", homeDir); await rmDir("remove e2e test workDir", workDir); From 679609883f4e57e6b6f1138afd046f8b8dce5984 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 7 Jul 2026 14:54:58 +0000 Subject: [PATCH 05/33] TS: skip in-process-unsupported E2E tests so the inprocess cell is green Four tests exercise behavior the in-process (FFI) transport cannot support, because the runtime loads into the shared host process. Skip them when COPILOT_SDK_DEFAULT_CONNECTION=inprocess (via a shared `isInProcessTransport` guard / it.skipIf); they remain covered by the default (stdio) cell. This matches .NET, which pins the equivalent tests to stdio. - telemetry file export: telemetry lowers to env vars the in-process worker can't receive per-client (#1934). - copilot_request_cancel_error "fires ctx.signal": a CopilotRequestHandler registers a process-wide LLM inference provider, so a second in-process client can't register while another holds it. - rpc_workspace_checkpoints unknown-checkpoint: readCheckpoint is answered by the native runtime in-process, which decodes the id as u32 and rejects the MAX_SAFE_INTEGER sentinel (runtime gap, #1934). - client.test.ts child-process-kill: no child process exists in-process. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/test/client.test.ts | 39 ++-- .../copilot_request_cancel_error.e2e.test.ts | 46 +++-- nodejs/test/e2e/harness/sdkTestContext.ts | 10 + .../e2e/rpc_workspace_checkpoints.e2e.test.ts | 30 +-- nodejs/test/e2e/telemetry.e2e.test.ts | 172 +++++++++--------- 5 files changed, 169 insertions(+), 128 deletions(-) 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 b813abadd2..f2d474449f 100644 --- a/nodejs/test/e2e/harness/sdkTestContext.ts +++ b/nodejs/test/e2e/harness/sdkTestContext.ts @@ -16,6 +16,16 @@ 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 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/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 afecf46c64c152cfc12b76f0fbb2091ee5f8c348 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 7 Jul 2026 15:14:27 +0000 Subject: [PATCH 06/33] Rust: full in-process (FFI) parity with .NET/TS Bring the Rust in-process transport in line with the .NET and TypeScript SDKs. Product: - Transport::InProcess passes no per-client env to the FFI worker; the worker inherits this host process's ambient environment. Per-client options that lower to env vars (env/env_remove, telemetry, github_token, base_directory) are not honored in-process, matching .NET/TS; documented as experimental, pointing at #1934. Removes the now-unused build_ffi_environment. E2E harness: - InProcessEnvGuard mirrors each test's environment onto the real process environment (which the in-process worker inherits) and restores it on drop, plus disables ambient HMAC so host-side auth picks the token the snapshots expect. Forces serial execution in-process (concurrency 1) so the process-wide env mutation is coherent; stdio/tcp are unchanged. - skip_inprocess guard skips tests for features the in-process transport can't support; applied to the telemetry file-export test (telemetry lowers to env vars) and the unknown-checkpoint test (readCheckpoint decodes the id as u32 natively in-process and rejects the i64::MAX sentinel). CI: - The in-process job now runs the whole E2E suite over the in-process transport (serially) instead of only the smoke test, mirroring the .NET/TS inprocess cell. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/rust-sdk-tests.yml | 13 +-- rust/src/lib.rs | 79 ++++--------------- rust/tests/e2e/rpc_workspace_checkpoints.rs | 6 ++ rust/tests/e2e/support.rs | 87 +++++++++++++++++++++ rust/tests/e2e/telemetry.rs | 5 ++ 5 files changed, 123 insertions(+), 67 deletions(-) diff --git a/.github/workflows/rust-sdk-tests.yml b/.github/workflows/rust-sdk-tests.yml index bd0a3348d3..4154330f5b 100644 --- a/.github/workflows/rust-sdk-tests.yml +++ b/.github/workflows/rust-sdk-tests.yml @@ -133,8 +133,10 @@ jobs: # analogue of the .NET `RuntimeConnection.ForInProcess()`), mirroring the # `inprocess` transport cell in dotnet-sdk-tests.yml. Sets # COPILOT_SDK_DEFAULT_CONNECTION=inprocess so the client hosts the runtime - # cdylib in-process instead of spawning a stdio child, then runs the - # dedicated in-process E2E test. + # cdylib in-process instead of spawning a stdio child, then runs the whole + # E2E suite over the in-process transport. The suite runs serially in-process + # (the harness forces concurrency to 1) because it mirrors each test's + # environment onto the shared process environment the in-process worker inherits. test-inprocess: name: "Rust SDK Tests (in-process transport)" if: github.event.repository.fork == false @@ -184,14 +186,15 @@ jobs: - name: Select in-process transport run: echo "COPILOT_SDK_DEFAULT_CONNECTION=inprocess" >> "$GITHUB_ENV" - - name: cargo test (in-process transport) + - name: cargo test (in-process transport, full E2E suite) timeout-minutes: 60 env: - RUST_E2E_CONCURRENCY: 4 COPILOT_HMAC_KEY: ${{ secrets.COPILOT_DEVELOPER_CLI_INTEGRATION_HMAC_KEY }} COPILOT_CLI_PATH: ${{ steps.setup-copilot.outputs.cli-path }} BUNDLED_CLI_CACHE_DIR: ${{ github.workspace }}/rust/.bundled-cli-cache - run: cargo test --no-default-features --features test-support --test e2e inprocess -- --nocapture + # The harness forces serial execution in-process, so RUST_E2E_CONCURRENCY is + # intentionally not set here. + run: cargo test --no-default-features --features test-support --test e2e -- --nocapture # Validates the bundled-CLI build path on all three supported # platforms. While the regular `cargo test` job above also exercises diff --git a/rust/src/lib.rs b/rust/src/lib.rs index 392db08bad..ca83d350ea 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -120,6 +120,15 @@ pub enum Transport { /// entrypoint and speaks JSON-RPC over its C ABI. The runtime spawns its /// own worker; the SDK never launches a CLI child process. This is the /// Rust analogue of the .NET `RuntimeConnection.ForInProcess()`. + /// + /// **Experimental.** Per-client options that lower to environment variables + /// — [`ClientOptions::env`]/[`ClientOptions::env_remove`], + /// [`ClientOptions::telemetry`], [`ClientOptions::github_token`], and + /// [`ClientOptions::base_directory`] — are **not** honored with this + /// transport: the runtime loads into the shared host process and its worker + /// inherits that process's ambient environment. Configure the runtime via + /// the host process environment instead. See + /// . InProcess, /// Spawn the CLI with `--port` and connect via TCP. Tcp { @@ -1148,9 +1157,15 @@ impl Client { )? } Transport::InProcess => { - let environment = Self::build_ffi_environment(&options); + // Per-client options that lower to environment variables (env, + // telemetry, github_token, base_directory) are not honored over the + // in-process transport: the runtime loads into this shared host + // process and its worker inherits this process's ambient environment, + // which a single env block cannot vary per client. Pass no per-client + // env; configure the runtime via the host process environment instead. + // See https://github.com/github/copilot-sdk/issues/1934. info!(entrypoint = %program.display(), "hosting copilot runtime in-process (FFI)"); - let host = crate::ffi::FfiHost::create(&program, environment)?; + let host = crate::ffi::FfiHost::create(&program, Vec::new())?; let (reader, writer, shared) = host.start().await?; let client = Self::from_transport( reader, @@ -1436,66 +1451,6 @@ impl Client { }); } - fn build_ffi_environment(options: &ClientOptions) -> Vec<(String, String)> { - let mut env: std::collections::BTreeMap = std::env::vars_os() - .map(|(k, v)| { - ( - k.to_string_lossy().into_owned(), - v.to_string_lossy().into_owned(), - ) - }) - .collect(); - let mut set = |key: &str, value: String| { - env.insert(key.to_string(), value); - }; - if let Some(token) = &options.github_token { - set("COPILOT_SDK_AUTH_TOKEN", token.clone()); - } - if let Some(telemetry) = &options.telemetry { - set("COPILOT_OTEL_ENABLED", "true".to_string()); - if let Some(endpoint) = &telemetry.otlp_endpoint { - set("OTEL_EXPORTER_OTLP_ENDPOINT", endpoint.clone()); - } - if let Some(protocol) = telemetry.otlp_protocol { - set("OTEL_EXPORTER_OTLP_PROTOCOL", protocol.as_str().to_string()); - } - if let Some(path) = &telemetry.file_path { - set( - "COPILOT_OTEL_FILE_EXPORTER_PATH", - path.to_string_lossy().into_owned(), - ); - } - if let Some(exporter) = telemetry.exporter_type { - set("COPILOT_OTEL_EXPORTER_TYPE", exporter.as_str().to_string()); - } - if let Some(source) = &telemetry.source_name { - set("COPILOT_OTEL_SOURCE_NAME", source.clone()); - } - if let Some(capture) = telemetry.capture_content { - set( - "OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT", - if capture { "true" } else { "false" }.to_string(), - ); - } - } - if let Some(dir) = &options.base_directory { - set("COPILOT_HOME", dir.to_string_lossy().into_owned()); - } - if options.mode == ClientMode::Empty { - set("COPILOT_DISABLE_KEYTAR", "1".to_string()); - } - for (key, value) in &options.env { - env.insert( - key.to_string_lossy().into_owned(), - value.to_string_lossy().into_owned(), - ); - } - for key in &options.env_remove { - env.remove(&*key.to_string_lossy()); - } - env.into_iter().collect() - } - fn build_command(program: &Path, options: &ClientOptions) -> Command { let mut command = Command::new(program); for arg in &options.prefix_args { diff --git a/rust/tests/e2e/rpc_workspace_checkpoints.rs b/rust/tests/e2e/rpc_workspace_checkpoints.rs index 2c185a535a..0a8bf56152 100644 --- a/rust/tests/e2e/rpc_workspace_checkpoints.rs +++ b/rust/tests/e2e/rpc_workspace_checkpoints.rs @@ -40,6 +40,12 @@ async fn should_list_no_checkpoints_for_fresh_session() { #[tokio::test] async fn should_return_null_or_empty_content_for_unknown_checkpoint() { + // In-process, session.workspaces.readCheckpoint is answered by the native runtime, + // which decodes the checkpoint number as a u32 and rejects the i64::MAX sentinel this + // test uses. Covered by the default (stdio) transport. See issue #1934. + if super::support::skip_inprocess("readCheckpoint decodes the id as u32 in-process") { + return; + } with_e2e_context( "rpc_workspace_checkpoints", "should_return_null_or_empty_content_for_unknown_checkpoint", diff --git a/rust/tests/e2e/support.rs b/rust/tests/e2e/support.rs index a00fc05b5f..f5c844516a 100644 --- a/rust/tests/e2e/support.rs +++ b/rust/tests/e2e/support.rs @@ -36,6 +36,13 @@ where .await .unwrap_or_else(|err| panic!("create E2E context: {err}")); + // In-process hosting: the runtime loads into this test process and its worker + // inherits the ambient environment (per-client env is not honored in-process, see + // https://github.com/github/copilot-sdk/issues/1934), so mirror this context's env + // onto the process for the duration of the test and restore on drop. Safe because + // E2E_CONCURRENCY is 1 in-process, serializing the whole critical section. + let _env_guard = InProcessEnvGuard::activate(&ctx); + let timed_out = tokio::time::timeout(default_test_timeout(), test(&mut ctx)) .await .is_err(); @@ -65,6 +72,10 @@ where .await .unwrap_or_else(|err| panic!("create E2E context: {err}")); + // See `with_e2e_context` for why the in-process transport mirrors env onto the + // process (restored on drop). + let _env_guard = InProcessEnvGuard::activate(&ctx); + let timed_out = tokio::time::timeout(default_test_timeout(), test(&mut ctx)) .await .is_err(); @@ -546,6 +557,12 @@ fn default_test_timeout() -> Duration { } fn e2e_concurrency() -> usize { + // The in-process transport mirrors per-test environment onto the shared process + // environment (see `InProcessEnvGuard`), which is only coherent when one test runs + // at a time. Force serial execution in-process; otherwise honor RUST_E2E_CONCURRENCY. + if is_inprocess_default() { + return 1; + } std::env::var("RUST_E2E_CONCURRENCY") .ok() .and_then(|value| value.parse::().ok()) @@ -553,6 +570,76 @@ fn e2e_concurrency() -> usize { .unwrap_or(4) } +/// True when the E2E suite runs over the in-process (FFI) transport, i.e. the SDK +/// resolves `COPILOT_SDK_DEFAULT_CONNECTION=inprocess` to [`Transport::InProcess`]. +pub fn is_inprocess_default() -> bool { + std::env::var("COPILOT_SDK_DEFAULT_CONNECTION") + .map(|value| value.eq_ignore_ascii_case("inprocess")) + .unwrap_or(false) +} + +/// Skip guard for E2E tests exercising features the in-process (FFI) transport does not +/// support (the runtime loads into the shared host process). Returns `true` — and logs — +/// when running in-process so the caller can `return` early; such tests remain covered +/// by the default (stdio) transport. See . +pub fn skip_inprocess(reason: &str) -> bool { + if is_inprocess_default() { + eprintln!("skipping test over the in-process (FFI) transport: {reason}"); + true + } else { + false + } +} + +/// Mirrors an [`E2eContext`]'s environment onto the real process environment for the +/// in-process transport, whose worker inherits this process's ambient environment +/// rather than a per-client env block. Restores the previous values on drop. Only the +/// in-process transport needs this; for stdio/tcp the environment is handed to the +/// spawned child directly. Auth flows via GH_TOKEN/GITHUB_TOKEN and HMAC is disabled so +/// host-side auth resolution picks the token the replay snapshots expect. +struct InProcessEnvGuard { + saved: Vec<(OsString, Option)>, +} + +impl InProcessEnvGuard { + /// Returns `Some` guard (having applied the env) when in-process, else `None`. + fn activate(ctx: &E2eContext) -> Option { + if !is_inprocess_default() { + return None; + } + let mut pairs: Vec<(OsString, OsString)> = ctx.environment(); + pairs.push(("GH_TOKEN".into(), "fake-token-for-e2e-tests".into())); + pairs.push(("GITHUB_TOKEN".into(), "fake-token-for-e2e-tests".into())); + + let mut saved: Vec<(OsString, Option)> = Vec::new(); + for (key, value) in &pairs { + saved.push((key.clone(), std::env::var_os(key))); + // SAFETY: the E2E suite runs serially in-process (concurrency 1), so no + // other thread races these process-wide env mutations. + unsafe { std::env::set_var(key, value) }; + } + for key in ["COPILOT_HMAC_KEY", "CAPI_HMAC_KEY"] { + let key = OsString::from(key); + saved.push((key.clone(), std::env::var_os(&key))); + // SAFETY: as above. + unsafe { std::env::remove_var(&key) }; + } + Some(Self { saved }) + } +} + +impl Drop for InProcessEnvGuard { + fn drop(&mut self) { + for (key, previous) in self.saved.iter().rev() { + // SAFETY: as in `activate` — serial execution in-process. + match previous { + Some(value) => unsafe { std::env::set_var(key, value) }, + None => unsafe { std::env::remove_var(key) }, + } + } + } +} + pub fn get_system_message(exchange: &serde_json::Value) -> String { exchange .get("request") diff --git a/rust/tests/e2e/telemetry.rs b/rust/tests/e2e/telemetry.rs index 38bf4a404a..f6905427ae 100644 --- a/rust/tests/e2e/telemetry.rs +++ b/rust/tests/e2e/telemetry.rs @@ -13,6 +13,11 @@ use super::support::{assistant_message_content, with_e2e_context}; #[tokio::test] async fn should_export_file_telemetry_for_sdk_interactions() { + // Telemetry lowers to environment variables the in-process worker cannot receive + // per-client; covered by the default (stdio) transport. See issue #1934. + if super::support::skip_inprocess("telemetry configuration is not honored in-process") { + return; + } with_e2e_context( "telemetry", "should_export_file_telemetry_for_sdk_interactions", From e3dcfc0ffe993a61d5efd8caa16850fa5fe16413 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 7 Jul 2026 15:23:40 +0000 Subject: [PATCH 07/33] TS: pump in-process FFI inbound frames promptly (fix macOS RPC hangs) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The in-process FFI host has no libuv handle of its own, so koffi delivers inbound (server→client) frames only when the event loop turns. The keep-alive timer used a 60s interval, which kept the loop from exiting but let it park between ticks. When the SDK issues a bare request and only awaits the response — e.g. the sequential `session.rpc.permissions.*` calls — there is no other loop activity, so on macOS the inbound response frame sat undelivered until the next tick and the round-trip stalled past the test timeout. Streaming turns kept the loop busy and so were unaffected, which is why only a handful of RPC-only tests hung and only on macOS. Shorten the pump interval so inbound frames are serviced within a few milliseconds. The empty callback is cheap and only runs while a connection is open. This is the koffi analogue of the .NET host's thread-safe Channel callback, which wakes its reader immediately regardless of loop state. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/src/ffiRuntimeHost.ts | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/nodejs/src/ffiRuntimeHost.ts b/nodejs/src/ffiRuntimeHost.ts index 44afb96ee0..9f1dbe0072 100644 --- a/nodejs/src/ffiRuntimeHost.ts +++ b/nodejs/src/ffiRuntimeHost.ts @@ -125,13 +125,25 @@ export class FfiRuntimeHost { private disposed = false; private outboundCallback: KoffiRegisteredCallback | undefined; /** - * Keeps the libuv event loop alive while the FFI connection is open. Unlike the - * stdio/TCP transports (whose pipe/socket handles keep the loop alive), the FFI - * transport has no libuv handle of its own, and native→JS callbacks are delivered - * via the event loop. Without a live handle the loop can park with no work and the - * queued callback delivery races into a crash. + * Keeps the libuv event loop alive AND turning while the FFI connection is open. + * Unlike the stdio/TCP transports (whose pipe/socket handles keep the loop alive), + * the FFI transport has no libuv handle of its own, and native→JS callbacks + * (inbound server→client frames) are delivered via the event loop. Without a live + * handle the loop can park with no work, and koffi's cross-thread callback delivery + * is only serviced when the loop next turns. + * + * The interval is deliberately short: when the SDK issues a bare request and then + * only `await`s the response (e.g. `session.rpc.permissions.*`), there is no other + * loop activity, so a coarse interval let the loop sleep and the inbound response + * frame sat undelivered until the next tick — on macOS this stalled such round-trips + * past the test timeout (streaming turns kept the loop busy and so were unaffected). + * A few-millisecond tick keeps inbound delivery prompt; the empty callback is cheap + * and only runs while a connection is open. This is the koffi analogue of the .NET + * host's thread-safe `Channel` callback, which wakes its reader immediately. */ private keepAlive: ReturnType | null = null; + /** Interval (ms) for {@link keepAlive}; short so inbound frames are pumped promptly. */ + private static readonly INBOUND_PUMP_INTERVAL_MS = 4; /** The stream JSON-RPC reads server→client frames from. */ readonly receiveStream: PassThrough; @@ -249,7 +261,7 @@ export class FfiRuntimeHost { throw new Error("copilot_runtime_connection_open failed."); } - this.keepAlive = setInterval(() => {}, 60_000); + this.keepAlive = setInterval(() => {}, FfiRuntimeHost.INBOUND_PUMP_INTERVAL_MS); } private writeFrame(frame: Buffer): void { From 02da6b4a5522855ccf7443c5dd69aa0ee6237216 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 7 Jul 2026 15:25:27 +0000 Subject: [PATCH 08/33] Rust CI: include OS (and transport) in job names The `test` and `bundle` jobs are 3-OS matrices but their names omitted the OS, so all three cells showed identical names. The in-process job named itself "(in-process transport)" without the OS. Align all with the Node/.NET convention "(, )": - test: "Rust SDK Tests (${{ matrix.os }}, default)" - test-inprocess: "Rust SDK Tests (ubuntu-latest, inprocess)" - bundle: "Rust SDK Bundled CLI Build (${{ matrix.os }})" Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/rust-sdk-tests.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/rust-sdk-tests.yml b/.github/workflows/rust-sdk-tests.yml index 4154330f5b..f6334b53c6 100644 --- a/.github/workflows/rust-sdk-tests.yml +++ b/.github/workflows/rust-sdk-tests.yml @@ -29,7 +29,7 @@ permissions: jobs: test: - name: "Rust SDK Tests" + name: "Rust SDK Tests (${{ matrix.os }}, default)" if: github.event.repository.fork == false env: POWERSHELL_UPDATECHECK: Off @@ -138,7 +138,7 @@ jobs: # (the harness forces concurrency to 1) because it mirrors each test's # environment onto the shared process environment the in-process worker inherits. test-inprocess: - name: "Rust SDK Tests (in-process transport)" + name: "Rust SDK Tests (ubuntu-latest, inprocess)" if: github.event.repository.fork == false env: CARGO_TERM_COLOR: always @@ -203,7 +203,7 @@ jobs: # extract / embed pipeline. Catches regressions before they ship to # crates.io and before bundling consumers hit them downstream. bundle: - name: "Rust SDK Bundled CLI Build" + name: "Rust SDK Bundled CLI Build (${{ matrix.os }})" if: github.event.repository.fork == false env: CARGO_TERM_COLOR: always From 0d9995f0c347d453e6dd4745a625e142136a1f97 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 7 Jul 2026 15:42:04 +0000 Subject: [PATCH 09/33] TS: decouple in-process FFI inbound delivery from the native callback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The native outbound callback previously wrote inbound frames straight to the receive stream, which synchronously drives frame parsing and JSON-RPC dispatch — and can re-enter the SDK's write path — all while still on the native call stack. For most flows this worked, but the `session.rpc.permissions.*` round-trips hung on macOS (silent 30s timeouts), consistent with a re-entrant delivery stalling against an in-flight connection_write. Copy the frame bytes eagerly (the native pointer is only valid during the call) but defer delivery to the receive stream 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. Combined with the short keep-alive pump, queued frames are delivered within a few milliseconds. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/src/ffiRuntimeHost.ts | 40 +++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/nodejs/src/ffiRuntimeHost.ts b/nodejs/src/ffiRuntimeHost.ts index 9f1dbe0072..30b231e53f 100644 --- a/nodejs/src/ffiRuntimeHost.ts +++ b/nodejs/src/ffiRuntimeHost.ts @@ -144,6 +144,13 @@ export class FfiRuntimeHost { private keepAlive: ReturnType | null = null; /** Interval (ms) for {@link keepAlive}; short so inbound frames are pumped promptly. */ private static readonly INBOUND_PUMP_INTERVAL_MS = 4; + /** + * 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; @@ -273,13 +280,44 @@ 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). + */ private feedInbound(bytesPtr: unknown, bytesLen: number | bigint): void { const length = Number(bytesLen); if (!bytesPtr || length <= 0) { return; } const bytes = koffi.decode(bytesPtr, koffi.array("uint8", length, "Typed")) as Uint8Array; - this.receiveStream.write(Buffer.from(bytes)); + this.inboundQueue.push(Buffer.from(bytes)); + if (!this.drainScheduled) { + this.drainScheduled = true; + setImmediate(() => this.drainInbound()); + } + } + + /** 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 { From c64a8aa1f7da958d151f75ae2ce904ea780fef4e Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 7 Jul 2026 15:57:29 +0000 Subject: [PATCH 10/33] Rust CI: run in-process E2E on ubuntu + macOS (exclude windows) The in-process job was ubuntu-only, so the transport was never exercised on macOS. Make it a matrix over ubuntu-latest and macos-latest (excluding windows, matching dotnet-sdk-tests.yml, pending in-process SQLite file-locking on shutdown), and pass --test-threads=1 so libtest also runs serially (the harness mirrors each test's environment onto the shared process environment). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/rust-sdk-tests.yml | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/.github/workflows/rust-sdk-tests.yml b/.github/workflows/rust-sdk-tests.yml index f6334b53c6..76e69ca87a 100644 --- a/.github/workflows/rust-sdk-tests.yml +++ b/.github/workflows/rust-sdk-tests.yml @@ -137,13 +137,19 @@ jobs: # E2E suite over the in-process transport. The suite runs serially in-process # (the harness forces concurrency to 1) because it mirrors each test's # environment onto the shared process environment the in-process worker inherits. + # Windows is excluded (matching dotnet-sdk-tests.yml) pending in-process SQLite + # file-locking on shutdown. test-inprocess: - name: "Rust SDK Tests (ubuntu-latest, inprocess)" + name: "Rust SDK Tests (${{ matrix.os }}, inprocess)" if: github.event.repository.fork == false env: CARGO_TERM_COLOR: always RUST_BACKTRACE: 1 - runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, macos-latest] + runs-on: ${{ matrix.os }} defaults: run: shell: bash @@ -177,7 +183,7 @@ jobs: uses: actions/cache@v4 with: path: ./rust/.bundled-cli-cache - key: bundled-cli-ubuntu-latest-${{ steps.cli-version.outputs.version }} + key: bundled-cli-${{ matrix.os }}-${{ steps.cli-version.outputs.version }} - name: Install test harness dependencies working-directory: ./test/harness @@ -192,9 +198,10 @@ jobs: COPILOT_HMAC_KEY: ${{ secrets.COPILOT_DEVELOPER_CLI_INTEGRATION_HMAC_KEY }} COPILOT_CLI_PATH: ${{ steps.setup-copilot.outputs.cli-path }} BUNDLED_CLI_CACHE_DIR: ${{ github.workspace }}/rust/.bundled-cli-cache - # The harness forces serial execution in-process, so RUST_E2E_CONCURRENCY is - # intentionally not set here. - run: cargo test --no-default-features --features test-support --test e2e -- --nocapture + # The harness forces serial execution in-process (both the async semaphore and + # libtest via --test-threads=1) because it mirrors each test's environment onto + # the shared process environment, so RUST_E2E_CONCURRENCY is not set here. + run: cargo test --no-default-features --features test-support --test e2e -- --test-threads=1 --nocapture # Validates the bundled-CLI build path on all three supported # platforms. While the regular `cargo test` job above also exercises From 02e407b941b257ed1fb4bcb05c444cd48ac68317 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 7 Jul 2026 15:57:29 +0000 Subject: [PATCH 11/33] TS: async in-process FFI writes + guard native callbacks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root-cause fix for hung session.rpc.permissions.* round-trips on macOS: the SDK called connection_write synchronously, blocking the single JS thread inside the native call. koffi delivers inbound frames (the response, or a server→client request) on a secondary thread and queues them to run when the event loop gets a turn — but a synchronous write never yields, so when the runtime produced the response during the write, main and worker deadlocked. koffi's own docs warn about exactly this. Issue the write via koffi's async FFI call so the event loop stays free to service inbound callbacks; Node's Writable still serializes writes, preserving frame order. Also guard both Node-API callbacks (the native outbound callback and the async write completion) with try/catch that logs, since a throw across the FFI boundary is otherwise swallowed and only surfaces as a DEP0168 "Uncaught Node-API callback exception" warning. This both hardens teardown (a late inbound frame after dispose can no longer escape) and reveals the underlying error when it occurs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/src/ffiRuntimeHost.ts | 87 +++++++++++++++++++++++++++--------- 1 file changed, 67 insertions(+), 20 deletions(-) diff --git a/nodejs/src/ffiRuntimeHost.ts b/nodejs/src/ffiRuntimeHost.ts index 30b231e53f..35b323eae0 100644 --- a/nodejs/src/ffiRuntimeHost.ts +++ b/nodejs/src/ffiRuntimeHost.ts @@ -166,13 +166,18 @@ export class FfiRuntimeHost { this.lib = loadLibrary(libraryPath); this.receiveStream = new PassThrough(); this.sendStream = new Writable({ + // Write frames via an ASYNCHRONOUS FFI call so the JS event loop is never + // blocked inside native code. The runtime can deliver an inbound frame (the + // response, or a server→client request) on a secondary thread while this + // write is in flight; koffi queues that callback to run when the event loop + // gets a turn. A synchronous connection_write would block the single JS + // thread for the whole native call, so if the runtime delivered the response + // during the write and waited for it to be consumed, main and worker would + // deadlock — koffi warns about exactly this. Observed as hung + // `session.rpc.permissions.*` round-trips on macOS. Node's Writable serializes + // writes (the next write waits for this callback), so frame order is preserved. write: (chunk: Buffer, _encoding, callback) => { - try { - this.writeFrame(chunk); - callback(); - } catch (error) { - callback(error as Error); - } + this.writeFrame(chunk, callback); }, }); } @@ -271,13 +276,41 @@ export class FfiRuntimeHost { this.keepAlive = setInterval(() => {}, FfiRuntimeHost.INBOUND_PUMP_INTERVAL_MS); } - private writeFrame(frame: Buffer): void { + private writeFrame(frame: Buffer, callback: (error?: Error | null) => void): void { if (this.disposed || !this.connectionId) { - throw new Error("The in-process runtime connection is closed."); - } - if (!this.lib.connectionWrite(this.connectionId, frame, frame.length)) { - throw new Error("Failed to write a frame to the in-process runtime connection."); + callback(new Error("The in-process runtime connection is closed.")); + return; } + // Asynchronous FFI call: runs on a libuv worker thread and invokes this callback + // when the native write completes, keeping the JS event loop free to service + // inbound native→JS callbacks in the meantime (see the sendStream comment). + this.lib.connectionWrite.async( + this.connectionId, + frame, + frame.length, + (error: Error | null, result: boolean) => { + // Node-API completion callback: guard so a throw here (e.g. the stream's + // own callback rejecting after teardown) can't escape across the FFI + // boundary as an uncaught Node-API callback exception (DEP0168). + try { + if (error) { + callback(error); + } else if (!result) { + callback( + new Error( + "Failed to write a frame to the in-process runtime connection." + ) + ); + } else { + callback(); + } + } catch (callbackError) { + console.error( + `In-process FFI write completion callback failed: ${callbackError instanceof Error ? (callbackError.stack ?? callbackError.message) : String(callbackError)}` + ); + } + } + ); } /** @@ -295,15 +328,29 @@ export class FfiRuntimeHost { * on macOS). */ private feedInbound(bytesPtr: unknown, bytesLen: number | bigint): void { - 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()); + // 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)}` + ); } } From 1553944098929fd35f2bd5b124881113815bf960 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 7 Jul 2026 16:03:33 +0000 Subject: [PATCH 12/33] TS: defer FFI callback unregister to avoid DEP0168 at teardown The in-process cell is green, but teardown emitted repeated DEP0168 "Uncaught Node-API callback exception" warnings. They come from inside koffi, not the SDK's callbacks (the guards added alongside never logged): koffi delivers the native outbound callback from a secondary thread by queuing it onto the event loop, and at teardown one such delivery can still be queued when dispose() calls koffi.unregister synchronously. Unregistering while a queued call is pending makes koffi invoke a freed callback and raise inside its own native code, which no JS try/catch can intercept. Defer the unregister to a setImmediate: the native connection and host are already closed by then, so no new deliveries originate, and a pending delivery fires in libuv's poll phase with the callback still valid (it no-ops since we're disposed) before the check-phase setImmediate frees the slot. The immediate is unref'd so it never keeps the process alive. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/src/ffiRuntimeHost.ts | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/nodejs/src/ffiRuntimeHost.ts b/nodejs/src/ffiRuntimeHost.ts index 35b323eae0..d9d987edc2 100644 --- a/nodejs/src/ffiRuntimeHost.ts +++ b/nodejs/src/ffiRuntimeHost.ts @@ -368,10 +368,31 @@ export class FfiRuntimeHost { } private unregisterCallback(): void { - if (this.outboundCallback !== undefined) { - koffi.unregister(this.outboundCallback); - this.outboundCallback = undefined; + 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. */ From e72a7d798dfe025881360d568c8ecaca8a814d82 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 7 Jul 2026 16:29:20 +0000 Subject: [PATCH 13/33] TEMP: trace in-process FFI frames to diagnose macOS RPC stalls Adds env-gated (COPILOT_FFI_TRACE=1) stderr tracing of FFI frame writes, write completions, inbound arrivals, deliveries, and an event-loop heartbeat, with JSON-RPC id/method correlation. Enabled on the Node inprocess CI cell to capture exactly where the macOS session.rpc.permissions.* round-trips stall (write vs inbound delivery vs loop parking). To be reverted once the root cause is fixed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/nodejs-sdk-tests.yml | 4 ++- nodejs/src/ffiRuntimeHost.ts | 49 ++++++++++++++++++++++++-- 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/.github/workflows/nodejs-sdk-tests.yml b/.github/workflows/nodejs-sdk-tests.yml index a5e8b77be6..fc556220ba 100644 --- a/.github/workflows/nodejs-sdk-tests.yml +++ b/.github/workflows/nodejs-sdk-tests.yml @@ -78,7 +78,9 @@ jobs: - name: Select inprocess transport if: matrix.transport == 'inprocess' - run: echo "COPILOT_SDK_DEFAULT_CONNECTION=inprocess" >> "$GITHUB_ENV" + run: | + echo "COPILOT_SDK_DEFAULT_CONNECTION=inprocess" >> "$GITHUB_ENV" + echo "COPILOT_FFI_TRACE=1" >> "$GITHUB_ENV" - name: Run Node.js SDK tests env: diff --git a/nodejs/src/ffiRuntimeHost.ts b/nodejs/src/ffiRuntimeHost.ts index d9d987edc2..1612c92bef 100644 --- a/nodejs/src/ffiRuntimeHost.ts +++ b/nodejs/src/ffiRuntimeHost.ts @@ -24,6 +24,38 @@ import { PassThrough, Writable } from "node:stream"; const SYMBOL_PREFIX = "copilot_runtime_"; +/** + * Temporary, env-gated tracing (COPILOT_FFI_TRACE=1) for diagnosing in-process FFI + * transport stalls. Logs frame writes/completions/inbound arrivals with a monotonic + * sequence and the JSON-RPC id/method, plus a periodic heartbeat, so we can tell + * whether a stalled round-trip is stuck on the write, on inbound delivery, or on the + * event loop parking. Writes to stderr to avoid interleaving with JSON-RPC stdout. + */ +const FFI_TRACE = process.env.COPILOT_FFI_TRACE === "1"; +function ffiTrace(message: string): void { + if (FFI_TRACE) { + process.stderr.write(`[ffi ${Date.now() % 100000} pid=${process.pid}] ${message}\n`); + } +} + +/** Extracts a compact `id/method` tag from a JSON-RPC frame for trace correlation. */ +function frameTag(frame: Buffer): string { + if (!FFI_TRACE) { + return ""; + } + try { + const text = frame.toString("utf8"); + const bodyStart = text.indexOf("\r\n\r\n"); + const body = bodyStart >= 0 ? text.slice(bodyStart + 4) : text; + const parsed = JSON.parse(body) as { id?: unknown; method?: unknown }; + const id = parsed.id !== undefined ? `id=${JSON.stringify(parsed.id)}` : ""; + const method = parsed.method !== undefined ? `m=${String(parsed.method)}` : ""; + return [id, method].filter(Boolean).join(" ") || "(no id/method)"; + } catch { + return `(unparsed ${frame.length}B)`; + } +} + type KoffiFunction = ReturnType["func"]>; type KoffiType = ReturnType; type KoffiRegisteredCallback = ReturnType; @@ -273,14 +305,23 @@ export class FfiRuntimeHost { throw new Error("copilot_runtime_connection_open failed."); } - this.keepAlive = setInterval(() => {}, FfiRuntimeHost.INBOUND_PUMP_INTERVAL_MS); + let heartbeat = 0; + this.keepAlive = setInterval(() => { + if (FFI_TRACE && ++heartbeat % 250 === 0) { + ffiTrace(`~tick ${heartbeat} pending=${this.inboundQueue.length}`); + } + }, FfiRuntimeHost.INBOUND_PUMP_INTERVAL_MS); } + private writeSeq = 0; + private writeFrame(frame: Buffer, callback: (error?: Error | null) => void): void { if (this.disposed || !this.connectionId) { callback(new Error("The in-process runtime connection is closed.")); return; } + const seq = ++this.writeSeq; + ffiTrace(`>>W seq=${seq} len=${frame.length} ${frameTag(frame)}`); // Asynchronous FFI call: runs on a libuv worker thread and invokes this callback // when the native write completes, keeping the JS event loop free to service // inbound native→JS callbacks in the meantime (see the sendStream comment). @@ -289,6 +330,7 @@ export class FfiRuntimeHost { frame, frame.length, (error: Error | null, result: boolean) => { + ffiTrace(`>>WDONE seq=${seq} ok=${!error && result} err=${error?.message ?? "-"}`); // Node-API completion callback: guard so a throw here (e.g. the stream's // own callback rejecting after teardown) can't escape across the FFI // boundary as an uncaught Node-API callback exception (DEP0168). @@ -342,7 +384,9 @@ export class FfiRuntimeHost { bytesPtr, koffi.array("uint8", length, "Typed") ) as Uint8Array; - this.inboundQueue.push(Buffer.from(bytes)); + const frame = Buffer.from(bytes); + ffiTrace(`< this.drainInbound()); @@ -363,6 +407,7 @@ export class FfiRuntimeHost { } let frame: Buffer | undefined; while ((frame = this.inboundQueue.shift()) !== undefined) { + ffiTrace(`< Date: Tue, 7 Jul 2026 16:43:56 +0000 Subject: [PATCH 14/33] TS: pump koffi async broker so idle in-process RPC responses arrive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of the macOS/Windows in-process hangs (bare session.rpc.* round-trips timing out at 30s): the runtime invokes our outbound callback from a secondary thread, and koffi only services its foreign-thread callback queue while a koffi ASYNCHRONOUS FFI call is in flight. During streaming the constant connectionWrite.async traffic keeps that queue pumped, but once the SDK goes idle awaiting a single response, a plain keep-alive timer kept the event loop alive WITHOUT pumping koffi's broker, so the response frame sat undelivered. CI tracing confirmed it: the request wrote and completed, the loop kept ticking every 4ms with an empty inbound queue, and the response never reached the inbound callback. Fix: on the same short interval, issue a cheap async FFI call (copilot_runtime_log_dropped_count — a no-arg, side-effect-free u64 getter) to keep koffi's asynchronous callback broker serviced while idle, so pending inbound frames are delivered within a few ms. Only one pump is in flight at a time. This is the koffi analogue of the .NET host's raw function-pointer callback, which needs no event-loop pumping. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/src/ffiRuntimeHost.ts | 50 +++++++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 15 deletions(-) diff --git a/nodejs/src/ffiRuntimeHost.ts b/nodejs/src/ffiRuntimeHost.ts index 1612c92bef..7e36c1006f 100644 --- a/nodejs/src/ffiRuntimeHost.ts +++ b/nodejs/src/ffiRuntimeHost.ts @@ -66,6 +66,7 @@ interface FfiLibrary { connectionOpen: KoffiFunction; connectionWrite: KoffiFunction; connectionClose: KoffiFunction; + logDroppedCount: KoffiFunction; outboundCallbackType: KoffiType; } @@ -119,6 +120,10 @@ 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 pumping while the connection is idle (see the pump in start()). + logDroppedCount: lib.func(`${SYMBOL_PREFIX}log_dropped_count`, "uint64", []), outboundCallbackType, }; loadedLibraryPath = libraryPath; @@ -157,24 +162,24 @@ export class FfiRuntimeHost { private disposed = false; private outboundCallback: KoffiRegisteredCallback | undefined; /** - * Keeps the libuv event loop alive AND turning while the FFI connection is open. - * Unlike the stdio/TCP transports (whose pipe/socket handles keep the loop alive), - * the FFI transport has no libuv handle of its own, and native→JS callbacks - * (inbound server→client frames) are delivered via the event loop. Without a live - * handle the loop can park with no work, and koffi's cross-thread callback delivery - * is only serviced when the loop next turns. + * Keeps koffi's asynchronous callback broker pumping while the connection is open, + * so inbound native→JS frames are delivered promptly even when the SDK is otherwise + * idle (waiting on a bare request/response). * - * The interval is deliberately short: when the SDK issues a bare request and then - * only `await`s the response (e.g. `session.rpc.permissions.*`), there is no other - * loop activity, so a coarse interval let the loop sleep and the inbound response - * frame sat undelivered until the next tick — on macOS this stalled such round-trips - * past the test timeout (streaming turns kept the loop busy and so were unaffected). - * A few-millisecond tick keeps inbound delivery prompt; the empty callback is cheap - * and only runs while a connection is open. This is the koffi analogue of the .NET - * host's thread-safe `Channel` callback, which wakes its reader immediately. + * The runtime invokes our outbound callback from a secondary thread. koffi marshals + * such foreign-thread callbacks to the JS main thread, but only services that queue + * while a koffi asynchronous FFI call is in flight — during streaming, the constant + * `connectionWrite.async` traffic keeps it pumped, but once the SDK goes idle after + * a single request a plain timer keeps the event loop alive WITHOUT pumping koffi's + * broker, so the response frame sits undelivered until the next koffi async call + * (observed as 30s-timeout hangs of bare `session.rpc.*` round-trips, most visibly on + * macOS/Windows). Issuing a cheap async FFI call on a short interval keeps the broker + * serviced. This is the koffi analogue of the .NET host's raw function-pointer + * callback, which needs no event-loop pumping because it runs directly on the runtime + * thread into a thread-safe `Channel`. */ private keepAlive: ReturnType | null = null; - /** Interval (ms) for {@link keepAlive}; short so inbound frames are pumped promptly. */ + /** Interval (ms) between async broker pumps; short so idle round-trips stay prompt. */ private static readonly INBOUND_PUMP_INTERVAL_MS = 4; /** * Inbound frame bytes copied out of the native callback, awaiting delivery to @@ -306,10 +311,25 @@ export class FfiRuntimeHost { } let heartbeat = 0; + let pumpInFlight = false; this.keepAlive = setInterval(() => { if (FFI_TRACE && ++heartbeat % 250 === 0) { ffiTrace(`~tick ${heartbeat} pending=${this.inboundQueue.length}`); } + // Issue a cheap async FFI call to keep koffi's asynchronous callback broker + // servicing pending inbound frames while idle. Skip if one is still in flight + // (a completed call already pumped the broker) or we're tearing down. + if (pumpInFlight || this.disposed || !this.connectionId) { + return; + } + pumpInFlight = true; + try { + this.lib.logDroppedCount.async(() => { + pumpInFlight = false; + }); + } catch { + pumpInFlight = false; + } }, FfiRuntimeHost.INBOUND_PUMP_INTERVAL_MS); } From b950762c6ca65fb35b2f3e0812f446501365fc5b Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 7 Jul 2026 16:59:56 +0000 Subject: [PATCH 15/33] TEMP: standalone koffi foreign-thread callback repro + dedicated CI job Isolates the in-process FFI hang hypothesis with no SDK, runtime cdylib, or JSON-RPC: a tiny C shared library (compiled on the fly with cc/clang) spawns a background thread that invokes a koffi-registered callback after the main event loop goes idle (only a keep-alive timer), mirroring how the runtime's worker-reader thread invokes our outbound callback while the SDK awaits a response. Runs on ubuntu + macOS via a dedicated fast CI job to get a clean cross-platform signal. To be removed once the hang is fixed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/nodejs-sdk-tests.yml | 27 ++++ .../e2e/_koffi_callback_repro.e2e.test.ts | 134 ++++++++++++++++++ 2 files changed, 161 insertions(+) create mode 100644 nodejs/test/e2e/_koffi_callback_repro.e2e.test.ts diff --git a/.github/workflows/nodejs-sdk-tests.yml b/.github/workflows/nodejs-sdk-tests.yml index fc556220ba..55d4a87a6e 100644 --- a/.github/workflows/nodejs-sdk-tests.yml +++ b/.github/workflows/nodejs-sdk-tests.yml @@ -30,6 +30,33 @@ permissions: contents: read jobs: + # TEMP: standalone koffi foreign-thread callback repro (no SDK/runtime). Fast + # signal on whether koffi delivers a background-thread callback to JS while the + # main loop is idle, across all three OSes. Remove once the in-process hang is fixed. + koffi-callback-repro: + name: "koffi callback repro (${{ matrix.os }})" + if: github.event.repository.fork == false + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, macos-latest, windows-latest] + runs-on: ${{ matrix.os }} + defaults: + run: + shell: bash + working-directory: ./nodejs + steps: + - uses: actions/checkout@v6.0.2 + - uses: actions/setup-node@v6 + with: + cache: "npm" + cache-dependency-path: "./nodejs/package-lock.json" + node-version: 22 + - name: Install dependencies + run: npm ci --ignore-scripts + - name: Run koffi callback repro + run: npx vitest run test/e2e/_koffi_callback_repro.e2e.test.ts + test: name: "Node.js SDK Tests (${{ matrix.os }}, ${{ matrix.transport }})" if: github.event.repository.fork == false diff --git a/nodejs/test/e2e/_koffi_callback_repro.e2e.test.ts b/nodejs/test/e2e/_koffi_callback_repro.e2e.test.ts new file mode 100644 index 0000000000..7357c56cc9 --- /dev/null +++ b/nodejs/test/e2e/_koffi_callback_repro.e2e.test.ts @@ -0,0 +1,134 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------------------------------------------*/ + +/** + * Standalone repro for the in-process FFI hang: does koffi deliver a foreign-thread + * callback to JS when the main event loop is otherwise idle (only a keep-alive timer)? + * + * No SDK, no runtime cdylib, no JSON-RPC — just a tiny C shared library whose exported + * function spawns a background thread that, after a delay, invokes a koffi-registered + * callback. Mirrors how the runtime's worker-reader thread invokes our outbound + * callback while the SDK sits idle awaiting a response. + * + * Compiled on the fly with cc/clang (Linux + macOS). Skipped where no C compiler / + * POSIX threads are available (e.g. Windows without a toolchain). + */ + +import { execFileSync } from "node:child_process"; +import { existsSync, mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; +import koffi from "koffi"; + +const C_SOURCE = ` +#include +#include + +typedef void (*ping_cb)(int seq); + +static ping_cb g_cb; +static int g_delay_ms; +static int g_count; + +static void *pinger(void *arg) { + (void)arg; + for (int i = 0; i < g_count; i++) { + usleep(g_delay_ms * 1000); + if (g_cb) g_cb(i); + } + return 0; +} + +// Spawn a detached background thread that invokes cb 'count' times, 'delay_ms' apart. +void start_pinger(ping_cb cb, int delay_ms, int count) { + g_cb = cb; + g_delay_ms = delay_ms; + g_count = count; + pthread_t t; + pthread_create(&t, 0, pinger, 0); + pthread_detach(t); +} +`; + +function findCc(): string | null { + for (const cc of ["cc", "clang", "gcc"]) { + try { + execFileSync(cc, ["--version"], { stdio: "ignore" }); + return cc; + } catch { + // try next + } + } + return null; +} + +function buildLibrary(): string | null { + const cc = findCc(); + if (!cc || process.platform === "win32") { + return null; + } + const dir = mkdtempSync(join(tmpdir(), "koffi-cb-repro-")); + const src = join(dir, "pinger.c"); + const ext = process.platform === "darwin" ? "dylib" : "so"; + const lib = join(dir, `libpinger.${ext}`); + writeFileSync(src, C_SOURCE); + try { + execFileSync(cc, ["-shared", "-fPIC", "-o", lib, src, "-lpthread"], { stdio: "ignore" }); + } catch { + return null; + } + return existsSync(lib) ? lib : null; +} + +describe("koffi foreign-thread callback delivery repro", () => { + const libPath = buildLibrary(); + + it.runIf(libPath)( + "delivers background-thread callbacks while the main loop is idle", + async () => { + const lib = koffi.load(libPath!); + const pingCb = koffi.pointer(koffi.proto("void ping_cb(int seq)")); + const startPinger = lib.func("void start_pinger(ping_cb *cb, int delay_ms, int count)"); + + const received: number[] = []; + const receivedAt: number[] = []; + const total = 5; + + // Keep-alive timer, exactly like FfiRuntimeHost: keeps the loop alive but the + // main thread is otherwise IDLE between callbacks. + const keepAlive = setInterval(() => {}, 4); + + const done = new Promise((resolve) => { + const cb = koffi.register((seq: number) => { + received.push(seq); + receivedAt.push(Date.now()); + if (received.length >= total) { + resolve(); + } + }, pingCb); + + // Background thread pings every 500ms — long enough that the main loop + // goes fully idle (only the keep-alive timer) between pings. + startPinger(cb, 500, total); + }); + + const settled = await Promise.race([ + done.then(() => "done" as const), + new Promise<"timeout">((r) => setTimeout(() => r("timeout"), 15_000)), + ]); + + clearInterval(keepAlive); + + process.stderr.write( + `[repro] received ${received.length}/${total} (${settled}); ` + + `gaps(ms)=${receivedAt.map((t, i) => (i ? t - receivedAt[i - 1] : 0)).join(",")}\n` + ); + + expect(settled).toBe("done"); + expect(received).toEqual([0, 1, 2, 3, 4]); + }, + 20_000 + ); +}); From 166f211ffdf8c30bfbc6dd8fe52319333c1765c8 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 7 Jul 2026 17:09:22 +0000 Subject: [PATCH 16/33] TEMP: revert to sync FFI writes + add burst repro scenario Trace analysis showed inbound frame delivery stops mid-stream on macOS (a streaming turn's events flow then halt), not just on idle bare RPCs. The async (libuv-threadpool) write variant competes with koffi's blocking inbound callback relay, so revert connection_write to a synchronous call matching the .NET host and drop the async broker pump. Also add a repro scenario that blasts a rapid burst of background-thread callbacks including a 23638-byte payload (the size that preceded the stall) to test whether koffi drops frames under burst on macOS. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/src/ffiRuntimeHost.ts | 79 ++++------------- .../e2e/_koffi_callback_repro.e2e.test.ts | 88 +++++++++++++++++++ 2 files changed, 107 insertions(+), 60 deletions(-) diff --git a/nodejs/src/ffiRuntimeHost.ts b/nodejs/src/ffiRuntimeHost.ts index 7e36c1006f..4718464d2d 100644 --- a/nodejs/src/ffiRuntimeHost.ts +++ b/nodejs/src/ffiRuntimeHost.ts @@ -203,18 +203,19 @@ export class FfiRuntimeHost { this.lib = loadLibrary(libraryPath); this.receiveStream = new PassThrough(); this.sendStream = new Writable({ - // Write frames via an ASYNCHRONOUS FFI call so the JS event loop is never - // blocked inside native code. The runtime can deliver an inbound frame (the - // response, or a server→client request) on a secondary thread while this - // write is in flight; koffi queues that callback to run when the event loop - // gets a turn. A synchronous connection_write would block the single JS - // thread for the whole native call, so if the runtime delivered the response - // during the write and waited for it to be consumed, main and worker would - // deadlock — koffi warns about exactly this. Observed as hung - // `session.rpc.permissions.*` round-trips on macOS. Node's Writable serializes - // writes (the next write waits for this callback), so frame order is preserved. + // Write frames with a SYNCHRONOUS FFI call, matching the .NET host. The + // native connection_write copies the bytes and returns; it does not block on + // the worker. Using koffi's async (libuv-threadpool) variant here instead + // competes for threadpool threads with koffi's inbound callback relay (which + // blocks its worker thread per frame via napi_tsfn_blocking), and under load + // that starved inbound delivery on macOS. Keep writes synchronous and cheap. write: (chunk: Buffer, _encoding, callback) => { - this.writeFrame(chunk, callback); + try { + this.writeFrame(chunk); + callback(); + } catch (error) { + callback(error as Error); + } }, }); } @@ -311,68 +312,26 @@ export class FfiRuntimeHost { } let heartbeat = 0; - let pumpInFlight = false; this.keepAlive = setInterval(() => { if (FFI_TRACE && ++heartbeat % 250 === 0) { ffiTrace(`~tick ${heartbeat} pending=${this.inboundQueue.length}`); } - // Issue a cheap async FFI call to keep koffi's asynchronous callback broker - // servicing pending inbound frames while idle. Skip if one is still in flight - // (a completed call already pumped the broker) or we're tearing down. - if (pumpInFlight || this.disposed || !this.connectionId) { - return; - } - pumpInFlight = true; - try { - this.lib.logDroppedCount.async(() => { - pumpInFlight = false; - }); - } catch { - pumpInFlight = false; - } }, FfiRuntimeHost.INBOUND_PUMP_INTERVAL_MS); } private writeSeq = 0; - private writeFrame(frame: Buffer, callback: (error?: Error | null) => void): void { + private writeFrame(frame: Buffer): void { if (this.disposed || !this.connectionId) { - callback(new Error("The in-process runtime connection is closed.")); - return; + throw new Error("The in-process runtime connection is closed."); } const seq = ++this.writeSeq; ffiTrace(`>>W seq=${seq} len=${frame.length} ${frameTag(frame)}`); - // Asynchronous FFI call: runs on a libuv worker thread and invokes this callback - // when the native write completes, keeping the JS event loop free to service - // inbound native→JS callbacks in the meantime (see the sendStream comment). - this.lib.connectionWrite.async( - this.connectionId, - frame, - frame.length, - (error: Error | null, result: boolean) => { - ffiTrace(`>>WDONE seq=${seq} ok=${!error && result} err=${error?.message ?? "-"}`); - // Node-API completion callback: guard so a throw here (e.g. the stream's - // own callback rejecting after teardown) can't escape across the FFI - // boundary as an uncaught Node-API callback exception (DEP0168). - try { - if (error) { - callback(error); - } else if (!result) { - callback( - new Error( - "Failed to write a frame to the in-process runtime connection." - ) - ); - } else { - callback(); - } - } catch (callbackError) { - console.error( - `In-process FFI write completion callback failed: ${callbackError instanceof Error ? (callbackError.stack ?? callbackError.message) : String(callbackError)}` - ); - } - } - ); + const ok = this.lib.connectionWrite(this.connectionId, frame, frame.length); + ffiTrace(`>>WDONE seq=${seq} ok=${ok}`); + if (!ok) { + throw new Error("Failed to write a frame to the in-process runtime connection."); + } } /** diff --git a/nodejs/test/e2e/_koffi_callback_repro.e2e.test.ts b/nodejs/test/e2e/_koffi_callback_repro.e2e.test.ts index 7357c56cc9..4fed8cd114 100644 --- a/nodejs/test/e2e/_koffi_callback_repro.e2e.test.ts +++ b/nodejs/test/e2e/_koffi_callback_repro.e2e.test.ts @@ -50,6 +50,42 @@ void start_pinger(ping_cb cb, int delay_ms, int count) { pthread_create(&t, 0, pinger, 0); pthread_detach(t); } + +// A side-effect-free call used to mimic the SDK's async FFI traffic (writes/pump) +// happening concurrently with background-thread callbacks. +int noop(void) { return 0; } + +typedef void (*blob_cb)(int seq, const unsigned char *data, int len); + +static blob_cb g_blob_cb; +static int g_burst; +static int g_big_at; +static int g_big_len; + +static void *burster(void *arg) { + (void)arg; + // Small settle so the JS main loop is idle/parked before the burst starts. + usleep(200 * 1000); + static unsigned char buf[65536]; + for (int i = 0; i < g_burst; i++) { + int len = (i == g_big_at) ? g_big_len : 64; + for (int j = 0; j < len; j++) buf[j] = (unsigned char)(i + j); + if (g_blob_cb) g_blob_cb(i, buf, len); + } + return 0; +} + +// Blast 'burst' callbacks back-to-back (no delay), one of them 'big_len' bytes, +// mirroring a streaming turn's rapid inbound frames including a large payload. +void start_burster(blob_cb cb, int burst, int big_at, int big_len) { + g_blob_cb = cb; + g_burst = burst; + g_big_at = big_at; + g_big_len = big_len; + pthread_t t; + pthread_create(&t, 0, burster, 0); + pthread_detach(t); +} `; function findCc(): string | null { @@ -131,4 +167,56 @@ describe("koffi foreign-thread callback delivery repro", () => { }, 20_000 ); + + it.runIf(libPath)( + "delivers a rapid burst of callbacks (incl. a large payload) after going idle", + async () => { + const lib = koffi.load(libPath!); + const blobCb = koffi.pointer( + koffi.proto("void blob_cb(int seq, uint8 *data, int len)") + ); + const startBurster = lib.func( + "void start_burster(blob_cb *cb, int burst, int big_at, int big_len)" + ); + + const burst = 40; + const bigAt = 20; + const bigLen = 23638; // same size as the streaming frame that preceded the stall + const received: number[] = []; + const lengths: number[] = []; + + // Keep-alive timer exactly like FfiRuntimeHost. + const keepAlive = setInterval(() => {}, 4); + + const done = new Promise((resolve) => { + const cb = koffi.register((seq: number, dataPtr: unknown, len: number) => { + // Decode like feedInbound does, to exercise the same path. + koffi.decode(dataPtr, koffi.array("uint8", len, "Typed")); + received.push(seq); + lengths.push(len); + if (received.length >= burst) { + resolve(); + } + }, blobCb); + startBurster(cb, burst, bigAt, bigLen); + }); + + const settled = await Promise.race([ + done.then(() => "done" as const), + new Promise<"timeout">((r) => setTimeout(() => r("timeout"), 15_000)), + ]); + + clearInterval(keepAlive); + + process.stderr.write( + `[repro-burst] received ${received.length}/${burst} (${settled}); ` + + `bigLen=${lengths[bigAt] ?? "n/a"}\n` + ); + + expect(settled).toBe("done"); + expect(received.length).toBe(burst); + expect(lengths[bigAt]).toBe(bigLen); + }, + 20_000 + ); }); From 5184218feb20344f85974e44f0405c366617e3f5 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 7 Jul 2026 17:12:53 +0000 Subject: [PATCH 17/33] TEMP: add bidirectional reentrancy scenario to koffi repro Adds the one untested koffi interaction matching real in-process usage: the inbound callback (blocking the reader thread via napi_tsfn_blocking) synchronously re-enters native (like connection_write from inside feedInbound). If this passes on macOS, koffi is fully exonerated and the stall is runtime/worker-side. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../e2e/_koffi_callback_repro.e2e.test.ts | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/nodejs/test/e2e/_koffi_callback_repro.e2e.test.ts b/nodejs/test/e2e/_koffi_callback_repro.e2e.test.ts index 4fed8cd114..2b80ab6788 100644 --- a/nodejs/test/e2e/_koffi_callback_repro.e2e.test.ts +++ b/nodejs/test/e2e/_koffi_callback_repro.e2e.test.ts @@ -86,6 +86,40 @@ void start_burster(blob_cb cb, int burst, int big_at, int big_len) { pthread_create(&t, 0, burster, 0); pthread_detach(t); } + +// Bidirectional reentrancy repro. A background "reader" thread invokes an inbound +// callback for each frame; the JS callback may synchronously call back into +// 'sink' (mimicking the SDK's synchronous connection_write from inside the koffi +// inbound callback). Because koffi relays foreign-thread callbacks with +// napi_tsfn_blocking, the reader thread is blocked in the callback while JS runs — +// and JS re-enters native via 'sink' on the main thread. This is the exact +// native->JS->native reentrancy the in-process transport performs under streaming. +typedef void (*inbound_cb)(int seq); + +static inbound_cb g_inbound; +static int g_frames; +static volatile int g_sink_count; + +// Called by JS from inside the inbound callback (like connection_write). Cheap. +void sink(int seq) { (void)seq; g_sink_count++; } + +static void *reader(void *arg) { + (void)arg; + usleep(150 * 1000); + for (int i = 0; i < g_frames; i++) { + if (g_inbound) g_inbound(i); // blocks until the JS callback returns + } + return 0; +} + +void start_reader(inbound_cb cb, int frames) { + g_inbound = cb; + g_frames = frames; + g_sink_count = 0; + pthread_t t; + pthread_create(&t, 0, reader, 0); + pthread_detach(t); +} `; function findCc(): string | null { @@ -219,4 +253,49 @@ describe("koffi foreign-thread callback delivery repro", () => { }, 20_000 ); + + it.runIf(libPath)( + "delivers frames when the JS callback re-enters native synchronously", + async () => { + const lib = koffi.load(libPath!); + const inboundCb = koffi.pointer(koffi.proto("void inbound_cb(int seq)")); + const startReader = lib.func("void start_reader(inbound_cb *cb, int frames)"); + // Synchronous native call issued from inside the inbound callback, mimicking + // the SDK's synchronous connection_write from inside feedInbound. + const sink = lib.func("void sink(int seq)"); + + const frames = 60; + const received: number[] = []; + + const keepAlive = setInterval(() => {}, 4); + + const done = new Promise((resolve) => { + const cb = koffi.register((seq: number) => { + // Re-enter native synchronously while this callback (running on, and + // blocking, the reader thread relay) is on the stack. + sink(seq); + received.push(seq); + if (received.length >= frames) { + resolve(); + } + }, inboundCb); + startReader(cb, frames); + }); + + const settled = await Promise.race([ + done.then(() => "done" as const), + new Promise<"timeout">((r) => setTimeout(() => r("timeout"), 15_000)), + ]); + + clearInterval(keepAlive); + + process.stderr.write( + `[repro-reentrant] received ${received.length}/${frames} (${settled})\n` + ); + + expect(settled).toBe("done"); + expect(received.length).toBe(frames); + }, + 20_000 + ); }); From 06048dad5808eb3b7d4d5d986fae7648fa3b01cb Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 7 Jul 2026 18:18:35 +0000 Subject: [PATCH 18/33] TEMP: upgrade koffi 2.16.2 -> 3.1.0 to test macOS in-process delivery koffi 3.0 rewrote call preparation/execution ("vastly improved performance") and distributes prebuilt binaries as @koromix/koffi- optionalDependency subpackages (so `npm ci --ignore-scripts` still resolves the native binary). Testing whether the newer callback/threading machinery fixes the macOS/Windows in-process RPC stalls. Our koffi API surface (load/func/pointer/proto/register/ unregister/decode/array) is unchanged across the major bump. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/package-lock.json | 248 ++++++++++++++++++++++++++++++++++++++- nodejs/package.json | 2 +- 2 files changed, 245 insertions(+), 5 deletions(-) diff --git a/nodejs/package-lock.json b/nodejs/package-lock.json index 6c3f32eca5..b979ce0e0d 100644 --- a/nodejs/package-lock.json +++ b/nodejs/package-lock.json @@ -10,7 +10,7 @@ "license": "MIT", "dependencies": { "@github/copilot": "^1.0.69-2", - "koffi": "^2.16.2", + "koffi": "^3.1.0", "vscode-jsonrpc": "^8.2.1", "zod": "^4.3.6" }, @@ -922,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", @@ -2594,13 +2818,29 @@ } }, "node_modules/koffi": { - "version": "2.16.2", - "resolved": "https://registry.npmjs.org/koffi/-/koffi-2.16.2.tgz", - "integrity": "sha512-owU0MRwv6xkrVqCd+33uw6BaYppkTRXbO/rVdJNI2dvZG0gzyRhYwW25eWtc5pauwK8TGh3AbkFONSezdykfSA==", + "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": { diff --git a/nodejs/package.json b/nodejs/package.json index 5cf9dd41c5..482d57d319 100644 --- a/nodejs/package.json +++ b/nodejs/package.json @@ -57,7 +57,7 @@ "license": "MIT", "dependencies": { "@github/copilot": "^1.0.69-2", - "koffi": "^2.16.2", + "koffi": "^3.1.0", "vscode-jsonrpc": "^8.2.1", "zod": "^4.3.6" }, From 661a37eca2597bdfcfa8c168e1c76880db8a0a1b Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 7 Jul 2026 21:03:11 +0000 Subject: [PATCH 19/33] TEMP: replace FFI frame tracing with session-event tracing Remove the COPILOT_FFI_TRACE frame-level instrumentation from FfiRuntimeHost (writes/inbound/heartbeat and the log_dropped_count binding) now that the FFI transport is proven to deliver every frame. Restore the plain 1s keep-alive and synchronous writes. Add COPILOT_EVENT_TRACE session-event tracing at the client's session.event / session.lifecycle notification handlers, logging each dispatched event's type. This pinpoints whether the in-process macOS stall is a missing session.idle (runtime never emits it) versus an emitted-but-not-dispatched event, for the approve-all-short-circuit + shell-tool turn that wedges sendAndWait. Enabled on the Node inprocess CI cell. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/nodejs-sdk-tests.yml | 2 +- nodejs/src/client.ts | 27 ++++++++- nodejs/src/ffiRuntimeHost.ts | 77 +++----------------------- 3 files changed, 35 insertions(+), 71 deletions(-) diff --git a/.github/workflows/nodejs-sdk-tests.yml b/.github/workflows/nodejs-sdk-tests.yml index 55d4a87a6e..e9be30e038 100644 --- a/.github/workflows/nodejs-sdk-tests.yml +++ b/.github/workflows/nodejs-sdk-tests.yml @@ -107,7 +107,7 @@ jobs: if: matrix.transport == 'inprocess' run: | echo "COPILOT_SDK_DEFAULT_CONNECTION=inprocess" >> "$GITHUB_ENV" - echo "COPILOT_FFI_TRACE=1" >> "$GITHUB_ENV" + echo "COPILOT_EVENT_TRACE=1" >> "$GITHUB_ENV" - name: Run Node.js SDK tests env: diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index 238361b21e..155fff6b3f 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -93,6 +93,22 @@ import { defaultJoinSessionPermissionHandler } from "./types.js"; const MIN_PROTOCOL_VERSION = 3; const RUNTIME_SHUTDOWN_TIMEOUT_MS = 10_000; +/** + * Temporary, env-gated event tracing (COPILOT_EVENT_TRACE=1) for diagnosing in-process + * session stalls (e.g. `sendAndWait` never seeing `session.idle`). Logs each inbound + * `session.event` / `session.lifecycle` notification's type as it is dispatched, to + * stderr. Remove once the in-process event-delivery gap is resolved. + */ +const EVENT_TRACE = process.env.COPILOT_EVENT_TRACE === "1"; +function eventTrace(channel: string, sessionId: string, type: unknown, disposition: string): void { + if (EVENT_TRACE) { + process.stderr.write( + `[evt ${Date.now() % 100000} pid=${process.pid}] ${channel} type=${String(type)} ` + + `sid=${sessionId.slice(0, 8)} ${disposition}\n` + ); + } +} + /** * Check if value is a Zod schema (has toJSONSchema method) */ @@ -2743,8 +2759,15 @@ export class CopilotClient { } const session = this.sessions.get((notification as { sessionId: string }).sessionId); + const event = (notification as { event: SessionEvent }).event; + eventTrace( + "session.event", + (notification as { sessionId: string }).sessionId, + (event as { type?: unknown })?.type, + session ? "dispatch" : "no-session" + ); if (session) { - session._dispatchEvent((notification as { event: SessionEvent }).event); + session._dispatchEvent(event); } } @@ -2781,6 +2804,8 @@ export class CopilotClient { metadata, } as SessionLifecycleEvent; + eventTrace("session.lifecycle", raw.sessionId, raw.type, "dispatch"); + // Dispatch to typed handlers for this specific event type const typedHandlers = this.typedLifecycleHandlers.get(event.type); if (typedHandlers) { diff --git a/nodejs/src/ffiRuntimeHost.ts b/nodejs/src/ffiRuntimeHost.ts index 4718464d2d..0ce0dbb800 100644 --- a/nodejs/src/ffiRuntimeHost.ts +++ b/nodejs/src/ffiRuntimeHost.ts @@ -24,38 +24,6 @@ import { PassThrough, Writable } from "node:stream"; const SYMBOL_PREFIX = "copilot_runtime_"; -/** - * Temporary, env-gated tracing (COPILOT_FFI_TRACE=1) for diagnosing in-process FFI - * transport stalls. Logs frame writes/completions/inbound arrivals with a monotonic - * sequence and the JSON-RPC id/method, plus a periodic heartbeat, so we can tell - * whether a stalled round-trip is stuck on the write, on inbound delivery, or on the - * event loop parking. Writes to stderr to avoid interleaving with JSON-RPC stdout. - */ -const FFI_TRACE = process.env.COPILOT_FFI_TRACE === "1"; -function ffiTrace(message: string): void { - if (FFI_TRACE) { - process.stderr.write(`[ffi ${Date.now() % 100000} pid=${process.pid}] ${message}\n`); - } -} - -/** Extracts a compact `id/method` tag from a JSON-RPC frame for trace correlation. */ -function frameTag(frame: Buffer): string { - if (!FFI_TRACE) { - return ""; - } - try { - const text = frame.toString("utf8"); - const bodyStart = text.indexOf("\r\n\r\n"); - const body = bodyStart >= 0 ? text.slice(bodyStart + 4) : text; - const parsed = JSON.parse(body) as { id?: unknown; method?: unknown }; - const id = parsed.id !== undefined ? `id=${JSON.stringify(parsed.id)}` : ""; - const method = parsed.method !== undefined ? `m=${String(parsed.method)}` : ""; - return [id, method].filter(Boolean).join(" ") || "(no id/method)"; - } catch { - return `(unparsed ${frame.length}B)`; - } -} - type KoffiFunction = ReturnType["func"]>; type KoffiType = ReturnType; type KoffiRegisteredCallback = ReturnType; @@ -66,7 +34,6 @@ interface FfiLibrary { connectionOpen: KoffiFunction; connectionWrite: KoffiFunction; connectionClose: KoffiFunction; - logDroppedCount: KoffiFunction; outboundCallbackType: KoffiType; } @@ -120,10 +87,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 pumping while the connection is idle (see the pump in start()). - logDroppedCount: lib.func(`${SYMBOL_PREFIX}log_dropped_count`, "uint64", []), outboundCallbackType, }; loadedLibraryPath = libraryPath; @@ -162,25 +125,14 @@ export class FfiRuntimeHost { private disposed = false; private outboundCallback: KoffiRegisteredCallback | undefined; /** - * Keeps koffi's asynchronous callback broker pumping while the connection is open, - * so inbound native→JS frames are delivered promptly even when the SDK is otherwise - * idle (waiting on a bare request/response). - * - * The runtime invokes our outbound callback from a secondary thread. koffi marshals - * such foreign-thread callbacks to the JS main thread, but only services that queue - * while a koffi asynchronous FFI call is in flight — during streaming, the constant - * `connectionWrite.async` traffic keeps it pumped, but once the SDK goes idle after - * a single request a plain timer keeps the event loop alive WITHOUT pumping koffi's - * broker, so the response frame sits undelivered until the next koffi async call - * (observed as 30s-timeout hangs of bare `session.rpc.*` round-trips, most visibly on - * macOS/Windows). Issuing a cheap async FFI call on a short interval keeps the broker - * serviced. This is the koffi analogue of the .NET host's raw function-pointer - * callback, which needs no event-loop pumping because it runs directly on the runtime - * thread into a thread-safe `Channel`. + * Keeps the libuv event loop alive while the FFI connection is open. Unlike the + * stdio/TCP transports (whose pipe/socket handles keep the loop alive), the FFI + * transport has no libuv handle of its own; without a live handle the loop could + * park with no work while awaiting inbound native→JS frames. */ private keepAlive: ReturnType | null = null; - /** Interval (ms) between async broker pumps; short so idle round-trips stay prompt. */ - private static readonly INBOUND_PUMP_INTERVAL_MS = 4; + /** Interval (ms) for the keep-alive timer. */ + private static readonly KEEPALIVE_INTERVAL_MS = 1000; /** * Inbound frame bytes copied out of the native callback, awaiting delivery to * {@link receiveStream} on a clean event-loop tick. See {@link feedInbound}. @@ -311,24 +263,14 @@ export class FfiRuntimeHost { throw new Error("copilot_runtime_connection_open failed."); } - let heartbeat = 0; - this.keepAlive = setInterval(() => { - if (FFI_TRACE && ++heartbeat % 250 === 0) { - ffiTrace(`~tick ${heartbeat} pending=${this.inboundQueue.length}`); - } - }, FfiRuntimeHost.INBOUND_PUMP_INTERVAL_MS); + this.keepAlive = setInterval(() => {}, FfiRuntimeHost.KEEPALIVE_INTERVAL_MS); } - private writeSeq = 0; - private writeFrame(frame: Buffer): void { if (this.disposed || !this.connectionId) { throw new Error("The in-process runtime connection is closed."); } - const seq = ++this.writeSeq; - ffiTrace(`>>W seq=${seq} len=${frame.length} ${frameTag(frame)}`); const ok = this.lib.connectionWrite(this.connectionId, frame, frame.length); - ffiTrace(`>>WDONE seq=${seq} ok=${ok}`); if (!ok) { throw new Error("Failed to write a frame to the in-process runtime connection."); } @@ -363,9 +305,7 @@ export class FfiRuntimeHost { bytesPtr, koffi.array("uint8", length, "Typed") ) as Uint8Array; - const frame = Buffer.from(bytes); - ffiTrace(`< this.drainInbound()); @@ -386,7 +326,6 @@ export class FfiRuntimeHost { } let frame: Buffer | undefined; while ((frame = this.inboundQueue.shift()) !== undefined) { - ffiTrace(`< Date: Tue, 7 Jul 2026 21:18:59 +0000 Subject: [PATCH 20/33] TEMP: full sessionId in event trace + sendAndWait receive trace Log the full sessionId (not truncated to 8 chars, which was ambiguous across sessions sharing a prefix) and add a trace line inside sendAndWait's event listener, to conclusively determine whether the in-process macOS hang is caused by session.idle events arriving for an unregistered session ("no-session" drop) vs sendAndWait's own listener not receiving them. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/src/client.ts | 2 +- nodejs/src/session.ts | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index 155fff6b3f..5208998d35 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -104,7 +104,7 @@ function eventTrace(channel: string, sessionId: string, type: unknown, dispositi if (EVENT_TRACE) { process.stderr.write( `[evt ${Date.now() % 100000} pid=${process.pid}] ${channel} type=${String(type)} ` + - `sid=${sessionId.slice(0, 8)} ${disposition}\n` + `sid=${sessionId} ${disposition}\n` ); } } diff --git a/nodejs/src/session.ts b/nodejs/src/session.ts index 8d8fc6714f..c64202c070 100644 --- a/nodejs/src/session.ts +++ b/nodejs/src/session.ts @@ -299,6 +299,12 @@ export class CopilotSession { // Register event handler BEFORE calling send to avoid race condition // where session.idle fires before we start listening const unsubscribe = this.on((event) => { + if (process.env.COPILOT_EVENT_TRACE === "1") { + process.stderr.write( + `[evt ${Date.now() % 100000} pid=${process.pid}] sendAndWait-recv ` + + `type=${event.type} sid=${this.sessionId}\n` + ); + } if (event.type === "assistant.message") { lastAssistantMessage = event; } else if (event.type === "session.idle") { From 4dc4f25d12140decab4d396dd25cf08a9b8d10f7 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 7 Jul 2026 21:42:26 +0000 Subject: [PATCH 21/33] TEMP: run only permissions.e2e over in-process to isolate mac failures Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/nodejs-sdk-tests.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/nodejs-sdk-tests.yml b/.github/workflows/nodejs-sdk-tests.yml index e9be30e038..05beb3d0e8 100644 --- a/.github/workflows/nodejs-sdk-tests.yml +++ b/.github/workflows/nodejs-sdk-tests.yml @@ -108,8 +108,11 @@ jobs: run: | echo "COPILOT_SDK_DEFAULT_CONNECTION=inprocess" >> "$GITHUB_ENV" echo "COPILOT_EVENT_TRACE=1" >> "$GITHUB_ENV" + # TEMP experiment: run ONLY permissions in isolation over in-process to test + # whether the mac failures reproduce without any other test file in the fork. + echo "TEST_TARGET=test/e2e/permissions.e2e.test.ts" >> "$GITHUB_ENV" - name: Run Node.js SDK tests env: COPILOT_HMAC_KEY: ${{ secrets.COPILOT_DEVELOPER_CLI_INTEGRATION_HMAC_KEY }} - run: npm test + run: npx vitest run $TEST_TARGET From ebb550c55a98cd350015439a06f13078f7ea28bd Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 7 Jul 2026 21:50:11 +0000 Subject: [PATCH 22/33] TEMP: RPC-level tracing + run single permissions RPC test to isolate mac hang Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/nodejs-sdk-tests.yml | 12 +++++++--- .github/workflows/rust-sdk-tests.yml | 10 +++++--- nodejs/src/client.ts | 33 ++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 6 deletions(-) diff --git a/.github/workflows/nodejs-sdk-tests.yml b/.github/workflows/nodejs-sdk-tests.yml index 05beb3d0e8..dbd7b6e749 100644 --- a/.github/workflows/nodejs-sdk-tests.yml +++ b/.github/workflows/nodejs-sdk-tests.yml @@ -108,11 +108,17 @@ jobs: run: | echo "COPILOT_SDK_DEFAULT_CONNECTION=inprocess" >> "$GITHUB_ENV" echo "COPILOT_EVENT_TRACE=1" >> "$GITHUB_ENV" - # TEMP experiment: run ONLY permissions in isolation over in-process to test - # whether the mac failures reproduce without any other test file in the fork. + # TEMP experiment: run ONE pure-RPC failing test in isolation over in-process + # to test whether a single test reproduces the mac hang with no prior tests. echo "TEST_TARGET=test/e2e/permissions.e2e.test.ts" >> "$GITHUB_ENV" + echo 'TEST_NAME_FILTER=should configure and update permission paths' >> "$GITHUB_ENV" - name: Run Node.js SDK tests env: COPILOT_HMAC_KEY: ${{ secrets.COPILOT_DEVELOPER_CLI_INTEGRATION_HMAC_KEY }} - run: npx vitest run $TEST_TARGET + run: | + if [ -n "$TEST_NAME_FILTER" ]; then + npx vitest run "$TEST_TARGET" -t "$TEST_NAME_FILTER" + else + npx vitest run $TEST_TARGET + fi diff --git a/.github/workflows/rust-sdk-tests.yml b/.github/workflows/rust-sdk-tests.yml index 76e69ca87a..e4f49d9e9d 100644 --- a/.github/workflows/rust-sdk-tests.yml +++ b/.github/workflows/rust-sdk-tests.yml @@ -137,8 +137,8 @@ jobs: # E2E suite over the in-process transport. The suite runs serially in-process # (the harness forces concurrency to 1) because it mirrors each test's # environment onto the shared process environment the in-process worker inherits. - # Windows is excluded (matching dotnet-sdk-tests.yml) pending in-process SQLite - # file-locking on shutdown. + # Runs the whole E2E suite over the in-process transport on all three OSes, + # matching the Node in-process matrix. test-inprocess: name: "Rust SDK Tests (${{ matrix.os }}, inprocess)" if: github.event.repository.fork == false @@ -148,7 +148,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-latest, macos-latest] + os: [ubuntu-latest, macos-latest, windows-latest] runs-on: ${{ matrix.os }} defaults: run: @@ -189,6 +189,10 @@ jobs: working-directory: ./test/harness run: npm ci --ignore-scripts + - name: Warm up PowerShell + if: runner.os == 'Windows' + run: pwsh.exe -Command "Write-Host 'PowerShell ready'" + - name: Select in-process transport run: echo "COPILOT_SDK_DEFAULT_CONNECTION=inprocess" >> "$GITHUB_ENV" diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index 5208998d35..ad903065d1 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -109,6 +109,13 @@ function eventTrace(channel: string, sessionId: string, type: unknown, dispositi } } +let rpcTraceSeq = 0; +function rpcTrace(msg: string): void { + if (EVENT_TRACE) { + process.stderr.write(`[rpc ${Date.now() % 100000} pid=${process.pid}] ${msg}\n`); + } +} + /** * Check if value is a Zod schema (has toJSONSchema method) */ @@ -2673,6 +2680,32 @@ export class CopilotClient { return; } + if (EVENT_TRACE) { + const conn = this.connection; + const original = conn.sendRequest.bind(conn); + (conn as { sendRequest: unknown }).sendRequest = (method: unknown, ...args: unknown[]) => { + const seq = ++rpcTraceSeq; + rpcTrace(`>>REQ seq=${seq} method=${String(method)}`); + let result: Promise; + try { + result = original(method as never, ...(args as never[])); + } catch (err) { + rpcTrace(`!!REQ seq=${seq} method=${String(method)} threw=${String(err)}`); + throw err; + } + return Promise.resolve(result).then( + (value) => { + rpcTrace(`< { + rpcTrace(`< { this.handleSessionEventNotification(notification); }); From a71f9c409fc5d580f1e3fd2aea266a7832b8ddd4 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 7 Jul 2026 21:52:15 +0000 Subject: [PATCH 23/33] Trigger CI Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> From 31acc5565d322a2ce6b846bd440d7731c51c2a61 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 7 Jul 2026 21:56:43 +0000 Subject: [PATCH 24/33] TEMP: full permissions file with event+RPC tracing on mac inproc Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/nodejs-sdk-tests.yml | 7 ++++--- nodejs/src/client.ts | 5 ++++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/.github/workflows/nodejs-sdk-tests.yml b/.github/workflows/nodejs-sdk-tests.yml index dbd7b6e749..ad0537787b 100644 --- a/.github/workflows/nodejs-sdk-tests.yml +++ b/.github/workflows/nodejs-sdk-tests.yml @@ -108,10 +108,11 @@ jobs: run: | echo "COPILOT_SDK_DEFAULT_CONNECTION=inprocess" >> "$GITHUB_ENV" echo "COPILOT_EVENT_TRACE=1" >> "$GITHUB_ENV" - # TEMP experiment: run ONE pure-RPC failing test in isolation over in-process - # to test whether a single test reproduces the mac hang with no prior tests. + # TEMP experiment: run the FULL permissions file with event+RPC tracing over + # in-process to capture the exact RPC that first wedges (mac fails tests 13-16 + # in-file, but each passes in isolation -> accumulated-state/ordering bug). echo "TEST_TARGET=test/e2e/permissions.e2e.test.ts" >> "$GITHUB_ENV" - echo 'TEST_NAME_FILTER=should configure and update permission paths' >> "$GITHUB_ENV" + echo 'TEST_NAME_FILTER=' >> "$GITHUB_ENV" - name: Run Node.js SDK tests env: diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index ad903065d1..5488f940b7 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -2683,7 +2683,10 @@ export class CopilotClient { if (EVENT_TRACE) { const conn = this.connection; const original = conn.sendRequest.bind(conn); - (conn as { sendRequest: unknown }).sendRequest = (method: unknown, ...args: unknown[]) => { + (conn as { sendRequest: unknown }).sendRequest = ( + method: unknown, + ...args: unknown[] + ) => { const seq = ++rpcTraceSeq; rpcTrace(`>>REQ seq=${seq} method=${String(method)}`); let result: Promise; From c203b26f1c5aba9ed138c03568ef06eba63383a9 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 7 Jul 2026 22:03:00 +0000 Subject: [PATCH 25/33] TEMP: raise ulimit -n on mac inproc cell to test fd-exhaustion hypothesis Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/nodejs-sdk-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/nodejs-sdk-tests.yml b/.github/workflows/nodejs-sdk-tests.yml index ad0537787b..ddaa98ac74 100644 --- a/.github/workflows/nodejs-sdk-tests.yml +++ b/.github/workflows/nodejs-sdk-tests.yml @@ -118,6 +118,9 @@ jobs: env: COPILOT_HMAC_KEY: ${{ secrets.COPILOT_DEVELOPER_CLI_INTEGRATION_HMAC_KEY }} run: | + echo "ulimit -n before: $(ulimit -n)" + ulimit -n 8192 || true + echo "ulimit -n after: $(ulimit -n)" if [ -n "$TEST_NAME_FILTER" ]; then npx vitest run "$TEST_TARGET" -t "$TEST_NAME_FILTER" else From 041ff0691bf08b87881980ea9695883288648f0f Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 7 Jul 2026 22:10:34 +0000 Subject: [PATCH 26/33] TEMP-test: abort in-flight turn before disconnect in noresult perm test (match .NET) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/nodejs-sdk-tests.yml | 3 --- nodejs/test/e2e/permissions.e2e.test.ts | 8 ++++++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/.github/workflows/nodejs-sdk-tests.yml b/.github/workflows/nodejs-sdk-tests.yml index ddaa98ac74..ad0537787b 100644 --- a/.github/workflows/nodejs-sdk-tests.yml +++ b/.github/workflows/nodejs-sdk-tests.yml @@ -118,9 +118,6 @@ jobs: env: COPILOT_HMAC_KEY: ${{ secrets.COPILOT_DEVELOPER_CLI_INTEGRATION_HMAC_KEY }} run: | - echo "ulimit -n before: $(ulimit -n)" - ulimit -n 8192 || true - echo "ulimit -n after: $(ulimit -n)" if [ -n "$TEST_NAME_FILTER" ]; then npx vitest run "$TEST_TARGET" -t "$TEST_NAME_FILTER" else diff --git a/nodejs/test/e2e/permissions.e2e.test.ts b/nodejs/test/e2e/permissions.e2e.test.ts index 5ef9206d14..e0ac73ba79 100644 --- a/nodejs/test/e2e/permissions.e2e.test.ts +++ b/nodejs/test/e2e/permissions.e2e.test.ts @@ -427,6 +427,14 @@ describe("Permission callbacks", async () => { await permissionCalled; + // Abort the in-flight turn before tearing down, mirroring the .NET test + // (Should_Deny_Permission_With_NoResult_Kind uses AbortAsync). With no-result + // the SDK never answers the CLI's permission request, so the tool call stays + // in-flight; over the in-process transport the runtime worker is shared across + // sessions, and destroying a session with a dangling permission round-trip + // (via disconnect) wedges the worker for subsequent sessions. Aborting cancels + // the pending turn cleanly first. + await session.abort(); await session.disconnect(); }); From a45209690795acf11e53b1976b245910076c24a2 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 7 Jul 2026 22:15:30 +0000 Subject: [PATCH 27/33] TEST async writes vs disconnect-while-pending (revert test change to expose bug) Make in-process connection_write async so the JS main thread is never blocked in a synchronous write while koffi holds the cdylib reader thread in a blocking inbound callback (bidirectional deadlock). Reverts the noresult test's abort so the failing disconnect-while-pending pattern is exercised against the transport fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/src/ffiRuntimeHost.ts | 60 +++++++++++++++++-------- nodejs/test/e2e/permissions.e2e.test.ts | 8 ---- 2 files changed, 42 insertions(+), 26 deletions(-) diff --git a/nodejs/src/ffiRuntimeHost.ts b/nodejs/src/ffiRuntimeHost.ts index 0ce0dbb800..295a24d5df 100644 --- a/nodejs/src/ffiRuntimeHost.ts +++ b/nodejs/src/ffiRuntimeHost.ts @@ -155,19 +155,27 @@ export class FfiRuntimeHost { this.lib = loadLibrary(libraryPath); this.receiveStream = new PassThrough(); this.sendStream = new Writable({ - // Write frames with a SYNCHRONOUS FFI call, matching the .NET host. The - // native connection_write copies the bytes and returns; it does not block on - // the worker. Using koffi's async (libuv-threadpool) variant here instead - // competes for threadpool threads with koffi's inbound callback relay (which - // blocks its worker thread per frame via napi_tsfn_blocking), and under load - // that starved inbound delivery on macOS. Keep writes synchronous and cheap. + // Write frames with an ASYNCHRONOUS FFI call. This is the crux of Node + // in-process parity with .NET. koffi delivers the runtime's outbound + // (worker->SDK) callback by marshalling it to the JS main thread and BLOCKING + // the cdylib's reader thread (napi_tsfn_blocking) until JS runs it — unlike + // .NET's raw fn-pointer callback, which runs inline on the reader thread with + // a non-blocking Channel.TryWrite and never blocks. If we also ran + // connection_write SYNCHRONOUSLY on the JS main thread, a burst of inbound + // frames could deadlock: the reader thread blocks in the callback for frame B + // (awaiting JS) while the JS main thread blocks in a synchronous + // connection_write triggered by dispatching frame A (awaiting the reader + // thread) — a bidirectional wedge observed as permanently stalled inbound + // delivery on macOS/Windows (e.g. a session.destroy issued with a permission + // round-trip still in flight). Issuing the write asynchronously (libuv + // threadpool) keeps the JS main thread free to service the inbound callback, + // so the reader thread never stays blocked. Node's Writable serializes writes + // (one in flight, next only after callback), so frame order is preserved. write: (chunk: Buffer, _encoding, callback) => { - try { - this.writeFrame(chunk); - callback(); - } catch (error) { - callback(error as Error); - } + this.writeFrame(chunk).then( + () => callback(), + (error) => callback(error as Error) + ); }, }); } @@ -266,14 +274,30 @@ export class FfiRuntimeHost { this.keepAlive = setInterval(() => {}, FfiRuntimeHost.KEEPALIVE_INTERVAL_MS); } - private writeFrame(frame: Buffer): void { + private writeFrame(frame: Buffer): Promise { 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."); + return Promise.reject(new Error("The in-process runtime connection is closed.")); } + return new Promise((resolvePromise, rejectPromise) => { + this.lib.connectionWrite.async( + this.connectionId, + frame, + frame.length, + (error: Error | null, ok: boolean) => { + if (error) { + rejectPromise(error); + } else if (!ok) { + rejectPromise( + new Error( + "Failed to write a frame to the in-process runtime connection." + ) + ); + } else { + resolvePromise(); + } + } + ); + }); } /** diff --git a/nodejs/test/e2e/permissions.e2e.test.ts b/nodejs/test/e2e/permissions.e2e.test.ts index e0ac73ba79..5ef9206d14 100644 --- a/nodejs/test/e2e/permissions.e2e.test.ts +++ b/nodejs/test/e2e/permissions.e2e.test.ts @@ -427,14 +427,6 @@ describe("Permission callbacks", async () => { await permissionCalled; - // Abort the in-flight turn before tearing down, mirroring the .NET test - // (Should_Deny_Permission_With_NoResult_Kind uses AbortAsync). With no-result - // the SDK never answers the CLI's permission request, so the tool call stays - // in-flight; over the in-process transport the runtime worker is shared across - // sessions, and destroying a session with a dangling permission round-trip - // (via disconnect) wedges the worker for subsequent sessions. Aborting cancels - // the pending turn cleanly first. - await session.abort(); await session.disconnect(); }); From 9b2651c70055395eabbbe21515209a067f09c8e3 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 7 Jul 2026 22:31:55 +0000 Subject: [PATCH 28/33] TEMP: callback-liveness tracing (CB-ENTER/EXIT/TICK) + no-result requestId Instrument feedInbound entry/exit and a keepalive heartbeat, plus log the parked requestId when a no-result permission handler skips the reply, to determine at the macOS/Windows in-process wedge whether koffi stops invoking the inbound callback (worker/cdylib side) or invokes it but stalls inside (JS side). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/nodejs-sdk-tests.yml | 9 ++- nodejs/src/ffiRuntimeHost.ts | 88 +++++++++++++------------- nodejs/src/session.ts | 6 ++ 3 files changed, 57 insertions(+), 46 deletions(-) diff --git a/.github/workflows/nodejs-sdk-tests.yml b/.github/workflows/nodejs-sdk-tests.yml index ad0537787b..1210b192cd 100644 --- a/.github/workflows/nodejs-sdk-tests.yml +++ b/.github/workflows/nodejs-sdk-tests.yml @@ -108,9 +108,12 @@ jobs: run: | echo "COPILOT_SDK_DEFAULT_CONNECTION=inprocess" >> "$GITHUB_ENV" echo "COPILOT_EVENT_TRACE=1" >> "$GITHUB_ENV" - # TEMP experiment: run the FULL permissions file with event+RPC tracing over - # in-process to capture the exact RPC that first wedges (mac fails tests 13-16 - # in-file, but each passes in isolation -> accumulated-state/ordering bug). + echo "COPILOT_FFI_TRACE=1" >> "$GITHUB_ENV" + # TEMP experiment: run the FULL permissions file with event+RPC+callback-liveness + # tracing over in-process to determine whether koffi STOPS invoking the inbound + # callback at the wedge (CB-ENTER count stops -> worker/cdylib side) or invokes + # it but stalls inside (CB-ENTER without CB-EXIT -> JS side), while TICKs prove + # the event loop is alive. echo "TEST_TARGET=test/e2e/permissions.e2e.test.ts" >> "$GITHUB_ENV" echo 'TEST_NAME_FILTER=' >> "$GITHUB_ENV" diff --git a/nodejs/src/ffiRuntimeHost.ts b/nodejs/src/ffiRuntimeHost.ts index 295a24d5df..9b124b296d 100644 --- a/nodejs/src/ffiRuntimeHost.ts +++ b/nodejs/src/ffiRuntimeHost.ts @@ -24,6 +24,9 @@ import { PassThrough, Writable } from "node:stream"; const SYMBOL_PREFIX = "copilot_runtime_"; +/** Temporary, env-gated (COPILOT_FFI_TRACE=1) inbound-callback liveness tracing. */ +const FFI_TRACE = process.env.COPILOT_FFI_TRACE === "1"; + type KoffiFunction = ReturnType["func"]>; type KoffiType = ReturnType; type KoffiRegisteredCallback = ReturnType; @@ -155,27 +158,15 @@ export class FfiRuntimeHost { this.lib = loadLibrary(libraryPath); this.receiveStream = new PassThrough(); this.sendStream = new Writable({ - // Write frames with an ASYNCHRONOUS FFI call. This is the crux of Node - // in-process parity with .NET. koffi delivers the runtime's outbound - // (worker->SDK) callback by marshalling it to the JS main thread and BLOCKING - // the cdylib's reader thread (napi_tsfn_blocking) until JS runs it — unlike - // .NET's raw fn-pointer callback, which runs inline on the reader thread with - // a non-blocking Channel.TryWrite and never blocks. If we also ran - // connection_write SYNCHRONOUSLY on the JS main thread, a burst of inbound - // frames could deadlock: the reader thread blocks in the callback for frame B - // (awaiting JS) while the JS main thread blocks in a synchronous - // connection_write triggered by dispatching frame A (awaiting the reader - // thread) — a bidirectional wedge observed as permanently stalled inbound - // delivery on macOS/Windows (e.g. a session.destroy issued with a permission - // round-trip still in flight). Issuing the write asynchronously (libuv - // threadpool) keeps the JS main thread free to service the inbound callback, - // so the reader thread never stays blocked. Node's Writable serializes writes - // (one in flight, next only after callback), so frame order is preserved. + // 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) => { - this.writeFrame(chunk).then( - () => callback(), - (error) => callback(error as Error) - ); + try { + this.writeFrame(chunk); + callback(); + } catch (error) { + callback(error as Error); + } }, }); } @@ -271,33 +262,25 @@ export class FfiRuntimeHost { throw new Error("copilot_runtime_connection_open failed."); } - this.keepAlive = setInterval(() => {}, FfiRuntimeHost.KEEPALIVE_INTERVAL_MS); + let ticks = 0; + this.keepAlive = setInterval(() => { + if (FFI_TRACE) { + ticks++; + process.stderr.write( + `[ffi ${Date.now() % 100000} pid=${process.pid}] TICK ${ticks} inboundSeq=${this.inboundSeq} qlen=${this.inboundQueue.length}\n` + ); + } + }, FfiRuntimeHost.KEEPALIVE_INTERVAL_MS); } - private writeFrame(frame: Buffer): Promise { + private writeFrame(frame: Buffer): void { if (this.disposed || !this.connectionId) { - return Promise.reject(new Error("The in-process runtime connection is closed.")); + 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."); } - return new Promise((resolvePromise, rejectPromise) => { - this.lib.connectionWrite.async( - this.connectionId, - frame, - frame.length, - (error: Error | null, ok: boolean) => { - if (error) { - rejectPromise(error); - } else if (!ok) { - rejectPromise( - new Error( - "Failed to write a frame to the in-process runtime connection." - ) - ); - } else { - resolvePromise(); - } - } - ); - }); } /** @@ -314,12 +297,20 @@ export class FfiRuntimeHost { * still on the stack and deadlock (observed as hung `session.rpc.*` round-trips * on macOS). */ + private inboundSeq = 0; + 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. + const seq = ++this.inboundSeq; + if (FFI_TRACE) { + process.stderr.write( + `[ffi ${Date.now() % 100000} pid=${process.pid}] CB-ENTER seq=${seq} len=${Number(bytesLen)}\n` + ); + } try { const length = Number(bytesLen); if (!bytesPtr || length <= 0) { @@ -338,6 +329,12 @@ export class FfiRuntimeHost { console.error( `In-process FFI inbound callback failed: ${error instanceof Error ? (error.stack ?? error.message) : String(error)}` ); + } finally { + if (FFI_TRACE) { + process.stderr.write( + `[ffi ${Date.now() % 100000} pid=${process.pid}] CB-EXIT seq=${seq}\n` + ); + } } } @@ -350,6 +347,11 @@ export class FfiRuntimeHost { } let frame: Buffer | undefined; while ((frame = this.inboundQueue.shift()) !== undefined) { + if (FFI_TRACE) { + process.stderr.write( + `[ffi ${Date.now() % 100000} pid=${process.pid}] DRAIN len=${frame.length}\n` + ); + } this.receiveStream.write(frame); } } diff --git a/nodejs/src/session.ts b/nodejs/src/session.ts index c64202c070..3ec5b7ee6a 100644 --- a/nodejs/src/session.ts +++ b/nodejs/src/session.ts @@ -663,6 +663,12 @@ export class CopilotSession { sessionId: this.sessionId, }); if (result.kind === "no-result") { + if (process.env.COPILOT_EVENT_TRACE === "1") { + process.stderr.write( + `[evt ${Date.now() % 100000} pid=${process.pid}] no-result-skip ` + + `requestId=${requestId} sid=${this.sessionId}\n` + ); + } return; } if (this.disconnected) { From 2bc7b44386ae91ea5b02d9f8ff25f48d1f520cd1 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 7 Jul 2026 22:40:27 +0000 Subject: [PATCH 29/33] EXPERIMENT: reply approve-once instead of no-result to test orphan-request causation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/test/e2e/permissions.e2e.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/nodejs/test/e2e/permissions.e2e.test.ts b/nodejs/test/e2e/permissions.e2e.test.ts index 5ef9206d14..987df0ecc3 100644 --- a/nodejs/test/e2e/permissions.e2e.test.ts +++ b/nodejs/test/e2e/permissions.e2e.test.ts @@ -419,7 +419,10 @@ describe("Permission callbacks", async () => { const session = await client.createSession({ onPermissionRequest: (_request: PermissionRequest): PermissionRequestResult => { resolvePermissionCalled(); - return { kind: "no-result" }; + // EXPERIMENT: reply instead of no-result to test whether the orphaned + // (unanswered) permission request is what wedges the shared in-process + // worker for the subsequent tests. + return { kind: "approve-once" }; }, }); From cd359531fb7aaeb96c8c15b29a538d588a86c10f Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 7 Jul 2026 22:51:04 +0000 Subject: [PATCH 30/33] FIX: continuously pump koffi async-callback broker in in-process host MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of the macOS/Windows-only in-process stall: koffi services its threadsafe-function broker (which marshals foreign-thread outbound callbacks to the JS main thread) ONLY while a koffi async FFI call is in flight; a plain JS timer does not pump it. During an idle stretch mid-turn (worker awaiting a model HTTP response, no client->server writes), the next outbound frame — the model response and everything after — sits undelivered until the next koffi call, wedging the session for 30s. Linux's libuv services the broker differently, so it only reproduced off-Linux. Keep exactly one cheap async FFI call (log_dropped_count) continuously in flight so the broker always pumps; the in-flight libuv request also keeps the event loop alive, replacing the keep-alive timer. Restores the no-result permission test to its real disconnect-while-pending pattern (no longer needs to reply to pass). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- nodejs/src/ffiRuntimeHost.ts | 76 ++++++++++++++++++------- nodejs/test/e2e/permissions.e2e.test.ts | 5 +- 2 files changed, 58 insertions(+), 23 deletions(-) diff --git a/nodejs/src/ffiRuntimeHost.ts b/nodejs/src/ffiRuntimeHost.ts index 9b124b296d..4ef4d1d297 100644 --- a/nodejs/src/ffiRuntimeHost.ts +++ b/nodejs/src/ffiRuntimeHost.ts @@ -37,6 +37,7 @@ interface FfiLibrary { connectionOpen: KoffiFunction; connectionWrite: KoffiFunction; connectionClose: KoffiFunction; + logDroppedCount: KoffiFunction; outboundCallbackType: KoffiType; } @@ -90,6 +91,10 @@ 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; @@ -128,14 +133,28 @@ export class FfiRuntimeHost { private disposed = false; private outboundCallback: KoffiRegisteredCallback | undefined; /** - * Keeps the libuv event loop alive while the FFI connection is open. Unlike the - * stdio/TCP transports (whose pipe/socket handles keep the loop alive), the FFI - * transport has no libuv handle of its own; without a live handle the loop could - * park with no work while awaiting inbound native→JS frames. + * 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 keepAlive: ReturnType | null = null; - /** Interval (ms) for the keep-alive timer. */ - private static readonly KEEPALIVE_INTERVAL_MS = 1000; + 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}. @@ -262,15 +281,35 @@ export class FfiRuntimeHost { throw new Error("copilot_runtime_connection_open failed."); } - let ticks = 0; - this.keepAlive = setInterval(() => { - if (FFI_TRACE) { - ticks++; - process.stderr.write( - `[ffi ${Date.now() % 100000} pid=${process.pid}] TICK ${ticks} inboundSeq=${this.inboundSeq} qlen=${this.inboundQueue.length}\n` - ); + 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; } - }, FfiRuntimeHost.KEEPALIVE_INTERVAL_MS); + 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 { @@ -391,10 +430,9 @@ export class FfiRuntimeHost { } this.disposed = true; - if (this.keepAlive) { - clearInterval(this.keepAlive); - this.keepAlive = null; - } + // The broker pump observes `disposed` and stops rescheduling on its next + // completion; no timer to clear. + this.pumpActive = false; try { if (this.connectionId) { diff --git a/nodejs/test/e2e/permissions.e2e.test.ts b/nodejs/test/e2e/permissions.e2e.test.ts index 987df0ecc3..5ef9206d14 100644 --- a/nodejs/test/e2e/permissions.e2e.test.ts +++ b/nodejs/test/e2e/permissions.e2e.test.ts @@ -419,10 +419,7 @@ describe("Permission callbacks", async () => { const session = await client.createSession({ onPermissionRequest: (_request: PermissionRequest): PermissionRequestResult => { resolvePermissionCalled(); - // EXPERIMENT: reply instead of no-result to test whether the orphaned - // (unanswered) permission request is what wedges the shared in-process - // worker for the subsequent tests. - return { kind: "approve-once" }; + return { kind: "no-result" }; }, }); From 5f4f75becc64024ae678312e3afc8b6acfe54f6c Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 7 Jul 2026 23:02:00 +0000 Subject: [PATCH 31/33] EXPERIMENT: run mac inprocess cell on macos-latest-xlarge (6-core vs 3-core) Test whether the in-process wedge is CPU-count dependent (tokio in the runtime cdylib uses Builder::new_multi_thread() = one worker per core; macos-latest is 3-core M1). macos-latest-xlarge is the same Apple-Silicon arch with 6 cores, so a pass would isolate core count from OS/arch as the trigger. Only the runner changes vs the last failing run. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/nodejs-sdk-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/nodejs-sdk-tests.yml b/.github/workflows/nodejs-sdk-tests.yml index 1210b192cd..6ad22ce716 100644 --- a/.github/workflows/nodejs-sdk-tests.yml +++ b/.github/workflows/nodejs-sdk-tests.yml @@ -65,7 +65,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-latest, macos-latest, windows-latest] + os: [ubuntu-latest, macos-latest-xlarge, windows-latest] transport: ["default", "inprocess"] runs-on: ${{ matrix.os }} defaults: From 86ed828e74ca1995ed3b45866342e70df5791865 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 7 Jul 2026 23:18:25 +0000 Subject: [PATCH 32/33] EXPERIMENT: pin Linux inprocess to 1 CPU via taskset to test tokio-worker-count theory Reverts mac runner to macos-latest. If Linux under taskset -c 0 (=> tokio default worker_threads=1 via available_parallelism, which honors CPU affinity on Linux) reproduces the in-process wedge, the trigger is tokio worker-thread count, not OS/arch. Linux runs are fast and reliable, making this a cleaner test than swapping mac runner sizes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/nodejs-sdk-tests.yml | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/.github/workflows/nodejs-sdk-tests.yml b/.github/workflows/nodejs-sdk-tests.yml index 6ad22ce716..ccd04c2c81 100644 --- a/.github/workflows/nodejs-sdk-tests.yml +++ b/.github/workflows/nodejs-sdk-tests.yml @@ -65,7 +65,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-latest, macos-latest-xlarge, windows-latest] + os: [ubuntu-latest, macos-latest, windows-latest] transport: ["default", "inprocess"] runs-on: ${{ matrix.os }} defaults: @@ -121,8 +121,18 @@ jobs: env: COPILOT_HMAC_KEY: ${{ secrets.COPILOT_DEVELOPER_CLI_INTEGRATION_HMAC_KEY }} run: | + # TEMP experiment: on Linux inprocess, pin the whole process tree to a single + # CPU via taskset so tokio's default worker count (available_parallelism, which + # respects CPU affinity on Linux) is 1. If Linux then reproduces the mac/win + # in-process wedge, the trigger is tokio worker-thread count, not OS/arch. + RUNNER="" + if [ "$RUNNER_OS" = "Linux" ] && [ "$COPILOT_SDK_DEFAULT_CONNECTION" = "inprocess" ]; then + nproc || true + RUNNER="taskset -c 0" + echo "Pinning to 1 CPU via: $RUNNER" + fi if [ -n "$TEST_NAME_FILTER" ]; then - npx vitest run "$TEST_TARGET" -t "$TEST_NAME_FILTER" + $RUNNER npx vitest run "$TEST_TARGET" -t "$TEST_NAME_FILTER" else - npx vitest run $TEST_TARGET + $RUNNER npx vitest run $TEST_TARGET fi From b5c2943793ea65464e019b25f2642d9dd011f19b Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Tue, 7 Jul 2026 23:21:21 +0000 Subject: [PATCH 33/33] EXPERIMENT: Linux inprocess on 2 CPUs (taskset -c 0,1) to find worker-count threshold Linux on 1 CPU reproduced the in-process wedge (tokio worker_threads=1), confirming the trigger is tokio worker-thread count. Test 2 CPUs to locate the threshold at which it stops wedging. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/nodejs-sdk-tests.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/nodejs-sdk-tests.yml b/.github/workflows/nodejs-sdk-tests.yml index ccd04c2c81..168e3bcf8d 100644 --- a/.github/workflows/nodejs-sdk-tests.yml +++ b/.github/workflows/nodejs-sdk-tests.yml @@ -128,8 +128,8 @@ jobs: RUNNER="" if [ "$RUNNER_OS" = "Linux" ] && [ "$COPILOT_SDK_DEFAULT_CONNECTION" = "inprocess" ]; then nproc || true - RUNNER="taskset -c 0" - echo "Pinning to 1 CPU via: $RUNNER" + RUNNER="taskset -c 0,1" + echo "Pinning to 2 CPUs via: $RUNNER" fi if [ -n "$TEST_NAME_FILTER" ]; then $RUNNER npx vitest run "$TEST_TARGET" -t "$TEST_NAME_FILTER"