From fb560ad5e251b0aa09fba6a8685aa37eb6ce81db Mon Sep 17 00:00:00 2001 From: Steven Molen Date: Mon, 23 Feb 2026 12:45:33 -0600 Subject: [PATCH] fix(nodejs): add CJS compatibility for VS Code extensions (#528) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace import.meta.resolve with createRequire + path walking in getBundledCliPath(). The new implementation falls back to __filename when import.meta.url is unavailable (shimmed CJS environments like VS Code extensions bundled with esbuild format:"cjs"). Single ESM build output retained — no dual CJS/ESM builds needed. The fallback logic handles both native ESM and shimmed CJS contexts. --- nodejs/src/client.ts | 30 ++++++++++++++---- nodejs/test/cjs-compat.test.ts | 58 ++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 7 deletions(-) create mode 100644 nodejs/test/cjs-compat.test.ts diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index 7df64e50..96549225 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -13,9 +13,10 @@ import { spawn, type ChildProcess } from "node:child_process"; import { existsSync } from "node:fs"; +import { createRequire } from "node:module"; import { Socket } from "node:net"; import { dirname, join } from "node:path"; -import { fileURLToPath } from "node:url"; +import { pathToFileURL } from "node:url"; import { createMessageConnection, MessageConnection, @@ -117,14 +118,29 @@ function getNodeExecPath(): string { /** * Gets the path to the bundled CLI from the @github/copilot package. * Uses index.js directly rather than npm-loader.js (which spawns the native binary). + * + * The @github/copilot package only exposes an ESM-only "./sdk" export, + * which breaks in CJS contexts (e.g., VS Code extensions bundled with esbuild). + * Instead of resolving through the package's exports, we locate the package + * root by walking module resolution paths and checking for its directory. + * See: https://github.com/github/copilot-sdk/issues/528 */ function getBundledCliPath(): string { - // Find the actual location of the @github/copilot package by resolving its sdk export - const sdkUrl = import.meta.resolve("@github/copilot/sdk"); - const sdkPath = fileURLToPath(sdkUrl); - // sdkPath is like .../node_modules/@github/copilot/sdk/index.js - // Go up two levels to get the package root, then append index.js - return join(dirname(dirname(sdkPath)), "index.js"); + // import.meta.url is defined in ESM; in CJS bundles (esbuild format:"cjs") + // it's undefined, so we fall back to __filename via pathToFileURL. + const require = createRequire(import.meta.url ?? pathToFileURL(__filename).href); + // The @github/copilot package has strict ESM-only exports, so require.resolve + // cannot resolve it. Instead, walk the module resolution paths to find it. + const searchPaths = require.resolve.paths("@github/copilot") ?? []; + for (const base of searchPaths) { + const candidate = join(base, "@github", "copilot", "index.js"); + if (existsSync(candidate)) { + return candidate; + } + } + throw new Error( + `Could not find @github/copilot package. Searched ${searchPaths.length} paths. Ensure it is installed.` + ); } export class CopilotClient { diff --git a/nodejs/test/cjs-compat.test.ts b/nodejs/test/cjs-compat.test.ts new file mode 100644 index 00000000..e04fc400 --- /dev/null +++ b/nodejs/test/cjs-compat.test.ts @@ -0,0 +1,58 @@ +/** + * CJS shimmed environment compatibility test + * + * Verifies that getBundledCliPath() works when the ESM build is loaded in a + * shimmed CJS environment (e.g., VS Code extensions bundled with esbuild + * format:"cjs"). In these environments, import.meta.url may be undefined but + * __filename is available via the CJS shim. + * + * See: https://github.com/github/copilot-sdk/issues/528 + */ + +import { describe, expect, it } from "vitest"; +import { existsSync } from "node:fs"; +import { execFileSync } from "node:child_process"; +import { join } from "node:path"; + +const esmEntryPoint = join(import.meta.dirname, "../dist/index.js"); + +describe("CJS shimmed environment compatibility (#528)", () => { + it("ESM dist file should exist", () => { + expect(existsSync(esmEntryPoint)).toBe(true); + }); + + it("getBundledCliPath() should resolve in a CJS shimmed context", () => { + // Simulate what esbuild format:"cjs" does: __filename is defined, + // import.meta.url may be undefined. The SDK's fallback logic + // (import.meta.url ?? pathToFileURL(__filename).href) handles this. + // + // We test by requiring the ESM build via --input-type=module in a + // subprocess that has __filename available, verifying the constructor + // (which calls getBundledCliPath()) doesn't throw. + const script = ` + import { createRequire } from 'node:module'; + const require = createRequire(import.meta.url); + const sdk = await import(${JSON.stringify(esmEntryPoint)}); + if (typeof sdk.CopilotClient !== 'function') { + process.exit(1); + } + try { + const client = new sdk.CopilotClient({ cliUrl: "8080" }); + console.log('CopilotClient constructor: OK'); + } catch (e) { + console.error('constructor failed:', e.message); + process.exit(1); + } + `; + const output = execFileSync( + process.execPath, + ["--input-type=module", "--eval", script], + { + encoding: "utf-8", + timeout: 10000, + cwd: join(import.meta.dirname, ".."), + }, + ); + expect(output).toContain("CopilotClient constructor: OK"); + }); +});