From 63bb6c0aad27e325caae6860313317a7e049ea57 Mon Sep 17 00:00:00 2001 From: Nicole Haugen Date: Thu, 12 Mar 2026 16:47:22 -0500 Subject: [PATCH 1/4] Checkpoint from Copilot CLI for coding agent session --- docs/fix-createRequire-collision.prompt.md | 57 ++++++++++++++++++++++ tsup.config.ts | 7 ++- 2 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 docs/fix-createRequire-collision.prompt.md diff --git a/docs/fix-createRequire-collision.prompt.md b/docs/fix-createRequire-collision.prompt.md new file mode 100644 index 0000000..677e356 --- /dev/null +++ b/docs/fix-createRequire-collision.prompt.md @@ -0,0 +1,57 @@ +--- +description: Fix "require is not defined" in ESM bundle by adding a createRequire banner to tsup.config.ts +--- + +# Fix: Add createRequire banner to tsup.config.ts + +## Problem + +When CJS dependencies (e.g. vscode-jsonrpc) are bundled into an ESM output, calls to `require()` inside those dependencies fail at runtime with: + +``` +ReferenceError: require is not defined +``` + +The user sees this issue when using the 'instructions' command, and this error occurs: + +Error: Failed to generate instructions with Copilot SDK. Ensure the Copilot CLI is installed (copilot --version) and logged in. Dynamic require of "util" is not supported + +## Solution + +Add a `banner.js` entry in the tsup config that injects a shim at the top of the output bundle. This shim uses Node's built-in `module` package to create a real `require` function. + +## Steps + +1. Open `tsup.config.ts`. + +2. Inside the `defineConfig({})` object, add (or update) a `banner` property: + +```ts +banner: { + js: '#!/usr/bin/env node\nimport { createRequire as __bannerCreateRequire } from "module";\nconst require = __bannerCreateRequire(import.meta.url);'; +} +``` + +> **Note:** Use the alias `createRequire as __bannerCreateRequire` to avoid colliding with any source-level `import { createRequire } from "node:module"` (e.g. in `src/cli.ts`). ESM disallows duplicate top-level binding names in the same module scope, and tsup concatenates the banner with the bundled source. + +3. Ensure any CJS-only dependencies that use `require()` internally are listed in `noExternal` so they get bundled (otherwise the banner has no effect — external deps resolve on their own): + +```ts +noExternal: [/vscode-jsonrpc/]; +``` + +4. Rebuild with `npm run build` and verify the top of `dist/index.js` contains: + +```js +#!/usr/bin/env node +import { createRequire as __bannerCreateRequire } from "module"; +const require = __bannerCreateRequire(import.meta.url); +``` + +## Why this works + +- `import { createRequire as __bannerCreateRequire } from "module"` — imports the factory from Node's built-in `module` package under a unique alias. +- `__bannerCreateRequire(import.meta.url)` — creates a CJS-compatible `require()` function anchored to the bundle's file path. +- Bundled CJS code that calls `require("util")`, `require("path")`, etc. now resolves normally. +- The `#!/usr/bin/env node` shebang is included so the bundle is directly executable as a CLI. +- The alias avoids a `SyntaxError: Identifier 'createRequire' has already been declared` collision with source files that import `createRequire` themselves. diff --git a/tsup.config.ts b/tsup.config.ts index 1717d9a..fec1f81 100644 --- a/tsup.config.ts +++ b/tsup.config.ts @@ -46,7 +46,12 @@ export default defineConfig({ sourcemap: true, dts: false, banner: { - js: "#!/usr/bin/env node" + // vscode-jsonrpc is CJS and uses require("util") etc. When bundled into + // ESM, esbuild's __require polyfill throws because `require` is undefined. + // Providing a real require via createRequire fixes this for Node built-ins. + // Use alias (__bannerCreateRequire) to avoid colliding with cli.ts's own + // `import { createRequire } from "node:module"` in the bundled output. + js: '#!/usr/bin/env node\nimport { createRequire as __bannerCreateRequire } from "module";\nconst require = __bannerCreateRequire(import.meta.url);' }, // Keep node_modules as external — they'll be installed via npm external: [/^[^./]/], From 094f3cd57e64d9032d1d5fe9e69f037458c19547 Mon Sep 17 00:00:00 2001 From: Nicole Haugen Date: Thu, 12 Mar 2026 17:59:09 -0500 Subject: [PATCH 2/4] Checkpoint from Copilot CLI for coding agent session --- packages/core/src/services/copilotSdk.ts | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/packages/core/src/services/copilotSdk.ts b/packages/core/src/services/copilotSdk.ts index 8ad8e47..20711fd 100644 --- a/packages/core/src/services/copilotSdk.ts +++ b/packages/core/src/services/copilotSdk.ts @@ -34,7 +34,10 @@ function shouldFallbackToExternalServer(error: unknown): boolean { return ( message.includes("unknown option '--headless'") || message.includes("unknown option '--no-auto-update'") || - message.includes("copilot cli not found") + // SDK's internal CLI resolution couldn't find the binary + message.includes("copilot cli not found") || + // Node's spawn() can't execute .bat/.cmd directly on Windows + message.includes("spawn einval") ); } @@ -215,12 +218,16 @@ export async function createCopilotClient( : cliConfig.cliPath; logCopilotDebug(`creating SDK client with cliPath=${desc} useStdio=false`); - // npx spawns a grandchild process (npx -> node -> copilot) that the SDK - // cannot clean up — killing npx leaves the copilot binary running. - // Use external server mode where we manage the process tree ourselves. + // npx spawns a grandchild (npx → node → copilot) the SDK can't clean up. + // Windows .bat/.cmd shims can't be spawned directly by Node's child_process. + // In both cases, skip the SDK-managed attempt and go straight to external server. const isNpx = /\bnpx(?:\.cmd)?$/iu.test(cliConfig.cliPath); - if (isNpx) { - logCopilotDebug("npx wrapper detected; using external server mode"); + const isBatShim = process.platform === "win32" && /\.(?:bat|cmd)$/iu.test(cliConfig.cliPath); + + if (isNpx || isBatShim) { + logCopilotDebug( + `${isNpx ? "npx wrapper" : ".bat/.cmd shim"} detected; using external server mode directly` + ); const external = await startExternalServer(cliConfig); const client = new sdk.CopilotClient({ cliUrl: external.cliUrl }); try { From d5119e7ee5ce583f4ad60dc760f96b86f2f3c9bd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Mar 2026 23:06:34 +0000 Subject: [PATCH 3/4] fix: Windows .bat/.cmd shim spawn EINVAL - early-exit to external server mode Co-authored-by: nicolehaugen <10600161+nicolehaugen@users.noreply.github.com> --- packages/core/src/services/copilotSdk.ts | 7 ++- src/services/__tests__/copilotSdk.test.ts | 67 +++++++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/packages/core/src/services/copilotSdk.ts b/packages/core/src/services/copilotSdk.ts index 20711fd..37eb39d 100644 --- a/packages/core/src/services/copilotSdk.ts +++ b/packages/core/src/services/copilotSdk.ts @@ -29,7 +29,7 @@ function normalizeError(error: unknown): Error { return error instanceof Error ? error : new Error(String(error)); } -function shouldFallbackToExternalServer(error: unknown): boolean { +export function shouldFallbackToExternalServer(error: unknown): boolean { const message = normalizeError(error).message.toLowerCase(); return ( message.includes("unknown option '--headless'") || @@ -198,6 +198,9 @@ function attachExternalServerCleanup( }) as typeof client.stop; } +/** Matches Windows .bat/.cmd script files that cannot be spawned directly by Node's child_process. */ +export const BAT_SHIM_PATTERN = /\.(?:bat|cmd)$/iu; + export async function loadCopilotSdk(): Promise { if (!cachedSdkModule) { cachedSdkModule = import("@github/copilot-sdk").catch((error) => { @@ -222,7 +225,7 @@ export async function createCopilotClient( // Windows .bat/.cmd shims can't be spawned directly by Node's child_process. // In both cases, skip the SDK-managed attempt and go straight to external server. const isNpx = /\bnpx(?:\.cmd)?$/iu.test(cliConfig.cliPath); - const isBatShim = process.platform === "win32" && /\.(?:bat|cmd)$/iu.test(cliConfig.cliPath); + const isBatShim = process.platform === "win32" && BAT_SHIM_PATTERN.test(cliConfig.cliPath); if (isNpx || isBatShim) { logCopilotDebug( diff --git a/src/services/__tests__/copilotSdk.test.ts b/src/services/__tests__/copilotSdk.test.ts index b83efcf..0a15b62 100644 --- a/src/services/__tests__/copilotSdk.test.ts +++ b/src/services/__tests__/copilotSdk.test.ts @@ -1,5 +1,7 @@ import { attachDefaultPermissionHandler, + shouldFallbackToExternalServer, + BAT_SHIM_PATTERN, type CopilotSdkModule } from "@agentrc/core/services/copilotSdk"; import { describe, expect, it, vi } from "vitest"; @@ -80,3 +82,68 @@ describe("attachDefaultPermissionHandler", () => { expect(passedConfig.config.infiniteSessions).toEqual({ enabled: false }); }); }); + +describe("shouldFallbackToExternalServer", () => { + it("returns true for unknown headless option error", () => { + expect(shouldFallbackToExternalServer(new Error("unknown option '--headless'"))).toBe(true); + }); + + it("returns true for unknown no-auto-update option error", () => { + expect( + shouldFallbackToExternalServer(new Error("unknown option '--no-auto-update'")) + ).toBe(true); + }); + + it("returns true for copilot cli not found error", () => { + expect( + shouldFallbackToExternalServer(new Error("Copilot CLI not found at cmd. Ensure @github/copilot is installed.")) + ).toBe(true); + }); + + it("returns true for spawn EINVAL error (Windows .bat/.cmd direct spawn)", () => { + expect(shouldFallbackToExternalServer(new Error("spawn EINVAL"))).toBe(true); + }); + + it("returns true for lowercased spawn einval variant", () => { + expect(shouldFallbackToExternalServer(new Error("spawn einval"))).toBe(true); + }); + + it("returns false for unrelated errors", () => { + expect(shouldFallbackToExternalServer(new Error("ENOENT: no such file or directory"))).toBe( + false + ); + expect(shouldFallbackToExternalServer(new Error("Network timeout"))).toBe(false); + expect(shouldFallbackToExternalServer(new Error("spawn EACCES"))).toBe(false); + }); + + it("handles non-Error values gracefully", () => { + expect(shouldFallbackToExternalServer("spawn einval")).toBe(true); + expect(shouldFallbackToExternalServer("some unrelated string")).toBe(false); + }); +}); + +describe("isBatShim detection regex (BAT_SHIM_PATTERN)", () => { + it("matches .bat extension", () => { + expect(BAT_SHIM_PATTERN.test("C:\\Users\\user\\copilotCli\\copilot.bat")).toBe(true); + }); + + it("matches .cmd extension", () => { + expect(BAT_SHIM_PATTERN.test("C:\\Users\\user\\copilotCli\\copilot.cmd")).toBe(true); + }); + + it("matches case-insensitively", () => { + expect(BAT_SHIM_PATTERN.test("copilot.BAT")).toBe(true); + expect(BAT_SHIM_PATTERN.test("copilot.CMD")).toBe(true); + }); + + it("does not match non-shim executables", () => { + expect(BAT_SHIM_PATTERN.test("/usr/bin/copilot")).toBe(false); + expect(BAT_SHIM_PATTERN.test("copilot.exe")).toBe(false); + expect(BAT_SHIM_PATTERN.test("node")).toBe(false); + }); + + it("does not match npx paths (handled by isNpx check)", () => { + expect(BAT_SHIM_PATTERN.test("npx")).toBe(false); + expect(BAT_SHIM_PATTERN.test("npx.cmd")).toBe(true); // .cmd matches, but isNpx fires first + }); +}); From 737d01d6a1368c546d63d8289b41b279ecdd1b00 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Mar 2026 23:18:17 +0000 Subject: [PATCH 4/4] fix: address review feedback - remove tsup/docs files, inline isBatShim, revert test scope Co-authored-by: nicolehaugen <10600161+nicolehaugen@users.noreply.github.com> --- docs/fix-createRequire-collision.prompt.md | 57 ------------------ packages/core/src/services/copilotSdk.ts | 7 +-- src/services/__tests__/copilotSdk.test.ts | 67 ---------------------- tsup.config.ts | 7 +-- 4 files changed, 3 insertions(+), 135 deletions(-) delete mode 100644 docs/fix-createRequire-collision.prompt.md diff --git a/docs/fix-createRequire-collision.prompt.md b/docs/fix-createRequire-collision.prompt.md deleted file mode 100644 index 677e356..0000000 --- a/docs/fix-createRequire-collision.prompt.md +++ /dev/null @@ -1,57 +0,0 @@ ---- -description: Fix "require is not defined" in ESM bundle by adding a createRequire banner to tsup.config.ts ---- - -# Fix: Add createRequire banner to tsup.config.ts - -## Problem - -When CJS dependencies (e.g. vscode-jsonrpc) are bundled into an ESM output, calls to `require()` inside those dependencies fail at runtime with: - -``` -ReferenceError: require is not defined -``` - -The user sees this issue when using the 'instructions' command, and this error occurs: - -Error: Failed to generate instructions with Copilot SDK. Ensure the Copilot CLI is installed (copilot --version) and logged in. Dynamic require of "util" is not supported - -## Solution - -Add a `banner.js` entry in the tsup config that injects a shim at the top of the output bundle. This shim uses Node's built-in `module` package to create a real `require` function. - -## Steps - -1. Open `tsup.config.ts`. - -2. Inside the `defineConfig({})` object, add (or update) a `banner` property: - -```ts -banner: { - js: '#!/usr/bin/env node\nimport { createRequire as __bannerCreateRequire } from "module";\nconst require = __bannerCreateRequire(import.meta.url);'; -} -``` - -> **Note:** Use the alias `createRequire as __bannerCreateRequire` to avoid colliding with any source-level `import { createRequire } from "node:module"` (e.g. in `src/cli.ts`). ESM disallows duplicate top-level binding names in the same module scope, and tsup concatenates the banner with the bundled source. - -3. Ensure any CJS-only dependencies that use `require()` internally are listed in `noExternal` so they get bundled (otherwise the banner has no effect — external deps resolve on their own): - -```ts -noExternal: [/vscode-jsonrpc/]; -``` - -4. Rebuild with `npm run build` and verify the top of `dist/index.js` contains: - -```js -#!/usr/bin/env node -import { createRequire as __bannerCreateRequire } from "module"; -const require = __bannerCreateRequire(import.meta.url); -``` - -## Why this works - -- `import { createRequire as __bannerCreateRequire } from "module"` — imports the factory from Node's built-in `module` package under a unique alias. -- `__bannerCreateRequire(import.meta.url)` — creates a CJS-compatible `require()` function anchored to the bundle's file path. -- Bundled CJS code that calls `require("util")`, `require("path")`, etc. now resolves normally. -- The `#!/usr/bin/env node` shebang is included so the bundle is directly executable as a CLI. -- The alias avoids a `SyntaxError: Identifier 'createRequire' has already been declared` collision with source files that import `createRequire` themselves. diff --git a/packages/core/src/services/copilotSdk.ts b/packages/core/src/services/copilotSdk.ts index 37eb39d..20711fd 100644 --- a/packages/core/src/services/copilotSdk.ts +++ b/packages/core/src/services/copilotSdk.ts @@ -29,7 +29,7 @@ function normalizeError(error: unknown): Error { return error instanceof Error ? error : new Error(String(error)); } -export function shouldFallbackToExternalServer(error: unknown): boolean { +function shouldFallbackToExternalServer(error: unknown): boolean { const message = normalizeError(error).message.toLowerCase(); return ( message.includes("unknown option '--headless'") || @@ -198,9 +198,6 @@ function attachExternalServerCleanup( }) as typeof client.stop; } -/** Matches Windows .bat/.cmd script files that cannot be spawned directly by Node's child_process. */ -export const BAT_SHIM_PATTERN = /\.(?:bat|cmd)$/iu; - export async function loadCopilotSdk(): Promise { if (!cachedSdkModule) { cachedSdkModule = import("@github/copilot-sdk").catch((error) => { @@ -225,7 +222,7 @@ export async function createCopilotClient( // Windows .bat/.cmd shims can't be spawned directly by Node's child_process. // In both cases, skip the SDK-managed attempt and go straight to external server. const isNpx = /\bnpx(?:\.cmd)?$/iu.test(cliConfig.cliPath); - const isBatShim = process.platform === "win32" && BAT_SHIM_PATTERN.test(cliConfig.cliPath); + const isBatShim = process.platform === "win32" && /\.(?:bat|cmd)$/iu.test(cliConfig.cliPath); if (isNpx || isBatShim) { logCopilotDebug( diff --git a/src/services/__tests__/copilotSdk.test.ts b/src/services/__tests__/copilotSdk.test.ts index 0a15b62..b83efcf 100644 --- a/src/services/__tests__/copilotSdk.test.ts +++ b/src/services/__tests__/copilotSdk.test.ts @@ -1,7 +1,5 @@ import { attachDefaultPermissionHandler, - shouldFallbackToExternalServer, - BAT_SHIM_PATTERN, type CopilotSdkModule } from "@agentrc/core/services/copilotSdk"; import { describe, expect, it, vi } from "vitest"; @@ -82,68 +80,3 @@ describe("attachDefaultPermissionHandler", () => { expect(passedConfig.config.infiniteSessions).toEqual({ enabled: false }); }); }); - -describe("shouldFallbackToExternalServer", () => { - it("returns true for unknown headless option error", () => { - expect(shouldFallbackToExternalServer(new Error("unknown option '--headless'"))).toBe(true); - }); - - it("returns true for unknown no-auto-update option error", () => { - expect( - shouldFallbackToExternalServer(new Error("unknown option '--no-auto-update'")) - ).toBe(true); - }); - - it("returns true for copilot cli not found error", () => { - expect( - shouldFallbackToExternalServer(new Error("Copilot CLI not found at cmd. Ensure @github/copilot is installed.")) - ).toBe(true); - }); - - it("returns true for spawn EINVAL error (Windows .bat/.cmd direct spawn)", () => { - expect(shouldFallbackToExternalServer(new Error("spawn EINVAL"))).toBe(true); - }); - - it("returns true for lowercased spawn einval variant", () => { - expect(shouldFallbackToExternalServer(new Error("spawn einval"))).toBe(true); - }); - - it("returns false for unrelated errors", () => { - expect(shouldFallbackToExternalServer(new Error("ENOENT: no such file or directory"))).toBe( - false - ); - expect(shouldFallbackToExternalServer(new Error("Network timeout"))).toBe(false); - expect(shouldFallbackToExternalServer(new Error("spawn EACCES"))).toBe(false); - }); - - it("handles non-Error values gracefully", () => { - expect(shouldFallbackToExternalServer("spawn einval")).toBe(true); - expect(shouldFallbackToExternalServer("some unrelated string")).toBe(false); - }); -}); - -describe("isBatShim detection regex (BAT_SHIM_PATTERN)", () => { - it("matches .bat extension", () => { - expect(BAT_SHIM_PATTERN.test("C:\\Users\\user\\copilotCli\\copilot.bat")).toBe(true); - }); - - it("matches .cmd extension", () => { - expect(BAT_SHIM_PATTERN.test("C:\\Users\\user\\copilotCli\\copilot.cmd")).toBe(true); - }); - - it("matches case-insensitively", () => { - expect(BAT_SHIM_PATTERN.test("copilot.BAT")).toBe(true); - expect(BAT_SHIM_PATTERN.test("copilot.CMD")).toBe(true); - }); - - it("does not match non-shim executables", () => { - expect(BAT_SHIM_PATTERN.test("/usr/bin/copilot")).toBe(false); - expect(BAT_SHIM_PATTERN.test("copilot.exe")).toBe(false); - expect(BAT_SHIM_PATTERN.test("node")).toBe(false); - }); - - it("does not match npx paths (handled by isNpx check)", () => { - expect(BAT_SHIM_PATTERN.test("npx")).toBe(false); - expect(BAT_SHIM_PATTERN.test("npx.cmd")).toBe(true); // .cmd matches, but isNpx fires first - }); -}); diff --git a/tsup.config.ts b/tsup.config.ts index fec1f81..1717d9a 100644 --- a/tsup.config.ts +++ b/tsup.config.ts @@ -46,12 +46,7 @@ export default defineConfig({ sourcemap: true, dts: false, banner: { - // vscode-jsonrpc is CJS and uses require("util") etc. When bundled into - // ESM, esbuild's __require polyfill throws because `require` is undefined. - // Providing a real require via createRequire fixes this for Node built-ins. - // Use alias (__bannerCreateRequire) to avoid colliding with cli.ts's own - // `import { createRequire } from "node:module"` in the bundled output. - js: '#!/usr/bin/env node\nimport { createRequire as __bannerCreateRequire } from "module";\nconst require = __bannerCreateRequire(import.meta.url);' + js: "#!/usr/bin/env node" }, // Keep node_modules as external — they'll be installed via npm external: [/^[^./]/],