Skip to content

Commit de46143

Browse files
SteveSandersonMSSteveSandersonMS
committed
Node: use bundled CLI
1 parent 82729d4 commit de46143

File tree

9 files changed

+53
-52
lines changed

9 files changed

+53
-52
lines changed

‎.github/workflows/dotnet-sdk-tests.yml‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ on:
77
- 'test/**'
88
- 'nodejs/package.json'
99
- '.github/workflows/dotnet-sdk-tests.yml'
10-
- '.github/actions/setup-copilot/**'
1110
- '!**/*.md'
1211
- '!**/LICENSE*'
1312
- '!**/.gitignore'
@@ -39,8 +38,6 @@ jobs:
3938
working-directory: ./dotnet
4039
steps:
4140
- uses: actions/checkout@v6.0.2
42-
- uses: ./.github/actions/setup-copilot
43-
id: setup-copilot
4441
- uses: actions/setup-dotnet@v5
4542
with:
4643
dotnet-version: "8.0.x"
@@ -49,7 +46,7 @@ jobs:
4946
cache: "npm"
5047
cache-dependency-path: "./nodejs/package-lock.json"
5148

52-
- name: Install Node.js dependencies (for CLI)
49+
- name: Install Node.js dependencies (for CLI version extraction)
5350
working-directory: ./nodejs
5451
run: npm ci --ignore-scripts
5552

‎.github/workflows/nodejs-sdk-tests.yml‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ on:
99
- 'nodejs/**'
1010
- 'test/**'
1111
- '.github/workflows/nodejs-sdk-tests.yml'
12-
- '.github/actions/setup-copilot/**'
1312
- '!**/*.md'
1413
- '!**/LICENSE*'
1514
- '!**/.gitignore'
@@ -46,8 +45,6 @@ jobs:
4645
cache: "npm"
4746
cache-dependency-path: "./nodejs/package-lock.json"
4847
node-version: 22
49-
- uses: ./.github/actions/setup-copilot
50-
id: setup-copilot
5148
- name: Install dependencies
5249
run: npm ci --ignore-scripts
5350

@@ -72,5 +69,4 @@ jobs:
7269
- name: Run Node.js SDK tests
7370
env:
7471
COPILOT_HMAC_KEY: ${{ secrets.COPILOT_DEVELOPER_CLI_INTEGRATION_HMAC_KEY }}
75-
COPILOT_CLI_PATH: ${{ steps.setup-copilot.outputs.cli-path }}
7672
run: npm test

‎nodejs/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@
6262
"vitest": "^4.0.18"
6363
},
6464
"engines": {
65-
"node": ">=18.0.0"
65+
"node": ">=24.0.0"
6666
},
6767
"files": [
6868
"dist/**/*",

‎nodejs/src/client.ts‎

Lines changed: 40 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,10 @@
1212
*/
1313

1414
import { spawn, type ChildProcess } from "node:child_process";
15+
import { existsSync } from "node:fs";
1516
import { Socket } from "node:net";
17+
import { dirname, join } from "node:path";
18+
import { fileURLToPath } from "node:url";
1619
import {
1720
createMessageConnection,
1821
MessageConnection,
@@ -100,6 +103,20 @@ function toJsonSchema(parameters: Tool["parameters"]): Record<string, unknown> |
100103
* await client.stop();
101104
* ```
102105
*/
106+
107+
/**
108+
* Gets the path to the bundled CLI from the @github/copilot package.
109+
* Uses index.js directly rather than npm-loader.js (which spawns the native binary).
110+
*/
111+
function getBundledCliPath(): string {
112+
// Find the actual location of the @github/copilot package by resolving its sdk export
113+
const sdkUrl = import.meta.resolve("@github/copilot/sdk");
114+
const sdkPath = fileURLToPath(sdkUrl);
115+
// sdkPath is like .../node_modules/@github/copilot/sdk/index.js
116+
// Go up two levels to get the package root, then append index.js
117+
return join(dirname(dirname(sdkPath)), "index.js");
118+
}
119+
103120
export class CopilotClient {
104121
private cliProcess: ChildProcess | null = null;
105122
private connection: MessageConnection | null = null;
@@ -168,7 +185,7 @@ export class CopilotClient {
168185
}
169186

170187
this.options = {
171-
cliPath: options.cliPath || "copilot",
188+
cliPath: options.cliPath || getBundledCliPath(),
172189
cliArgs: options.cliArgs ?? [],
173190
cwd: options.cwd ?? process.cwd(),
174191
port: options.port || 0,
@@ -991,35 +1008,34 @@ export class CopilotClient {
9911008
envWithoutNodeDebug.COPILOT_SDK_AUTH_TOKEN = this.options.githubToken;
9921009
}
9931010

994-
// If cliPath is a .js file, spawn it with node
995-
// Note that we can't rely on the shebang as Windows doesn't support it
996-
const isJsFile = this.options.cliPath.endsWith(".js");
997-
const isAbsolutePath =
998-
this.options.cliPath.startsWith("/") || /^[a-zA-Z]:/.test(this.options.cliPath);
1011+
// Verify CLI exists before attempting to spawn
1012+
if (!existsSync(this.options.cliPath)) {
1013+
throw new Error(
1014+
`Copilot CLI not found at ${this.options.cliPath}. Ensure @github/copilot is installed.`
1015+
);
1016+
}
9991017

1000-
let command: string;
1001-
let spawnArgs: string[];
1018+
const stdioConfig: ["pipe", "pipe", "pipe"] | ["ignore", "pipe", "pipe"] = this.options
1019+
.useStdio
1020+
? ["pipe", "pipe", "pipe"]
1021+
: ["ignore", "pipe", "pipe"];
10021022

1023+
// For .js files, spawn node explicitly; for executables, spawn directly
1024+
const isJsFile = this.options.cliPath.endsWith(".js");
10031025
if (isJsFile) {
1004-
command = "node";
1005-
spawnArgs = [this.options.cliPath, ...args];
1006-
} else if (process.platform === "win32" && !isAbsolutePath) {
1007-
// On Windows, spawn doesn't search PATHEXT, so use cmd /c to resolve the executable.
1008-
command = "cmd";
1009-
spawnArgs = ["/c", `${this.options.cliPath}`, ...args];
1026+
this.cliProcess = spawn(process.execPath, [this.options.cliPath, ...args], {
1027+
stdio: stdioConfig,
1028+
cwd: this.options.cwd,
1029+
env: envWithoutNodeDebug,
1030+
});
10101031
} else {
1011-
command = this.options.cliPath;
1012-
spawnArgs = args;
1032+
this.cliProcess = spawn(this.options.cliPath, args, {
1033+
stdio: stdioConfig,
1034+
cwd: this.options.cwd,
1035+
env: envWithoutNodeDebug,
1036+
});
10131037
}
10141038

1015-
this.cliProcess = spawn(command, spawnArgs, {
1016-
stdio: this.options.useStdio
1017-
? ["pipe", "pipe", "pipe"]
1018-
: ["ignore", "pipe", "pipe"],
1019-
cwd: this.options.cwd,
1020-
env: envWithoutNodeDebug,
1021-
});
1022-
10231039
let stdout = "";
10241040
let resolved = false;
10251041

‎nodejs/src/types.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ export type SessionEvent = GeneratedSessionEvent;
1515
*/
1616
export interface CopilotClientOptions {
1717
/**
18-
* Path to the Copilot CLI executable
19-
* @default "copilot" (searches PATH)
18+
* Path to the CLI executable or JavaScript entry point.
19+
* If not specified, uses the bundled CLI from the @github/copilot package.
2020
*/
2121
cliPath?: string;
2222

‎nodejs/test/client.test.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
11
/* eslint-disable @typescript-eslint/no-explicit-any */
22
import { describe, expect, it, onTestFinished } from "vitest";
33
import { CopilotClient } from "../src/index.js";
4-
import { CLI_PATH } from "./e2e/harness/sdkTestContext.js";
54

65
// This file is for unit tests. Where relevant, prefer to add e2e tests in e2e/*.test.ts instead
76

87
describe("CopilotClient", () => {
98
it("returns a standardized failure result when a tool is not registered", async () => {
10-
const client = new CopilotClient({ cliPath: CLI_PATH });
9+
const client = new CopilotClient();
1110
await client.start();
1211
onTestFinished(() => client.forceStop());
1312

‎nodejs/test/e2e/client.test.ts‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { ChildProcess } from "child_process";
22
import { describe, expect, it, onTestFinished } from "vitest";
33
import { CopilotClient } from "../../src/index.js";
4-
import { CLI_PATH } from "./harness/sdkTestContext.js";
54

65
function onTestFinishedForceStop(client: CopilotClient) {
76
onTestFinished(async () => {
@@ -15,7 +14,7 @@ function onTestFinishedForceStop(client: CopilotClient) {
1514

1615
describe("Client", () => {
1716
it("should start and connect to server using stdio", async () => {
18-
const client = new CopilotClient({ cliPath: CLI_PATH, useStdio: true });
17+
const client = new CopilotClient({ useStdio: true });
1918
onTestFinishedForceStop(client);
2019

2120
await client.start();
@@ -30,7 +29,7 @@ describe("Client", () => {
3029
});
3130

3231
it("should start and connect to server using tcp", async () => {
33-
const client = new CopilotClient({ cliPath: CLI_PATH, useStdio: false });
32+
const client = new CopilotClient({ useStdio: false });
3433
onTestFinishedForceStop(client);
3534

3635
await client.start();
@@ -50,7 +49,7 @@ describe("Client", () => {
5049
// saying "Cannot call write after a stream was destroyed"
5150
// because the JSON-RPC logic is still trying to write to stdin after
5251
// the process has exited.
53-
const client = new CopilotClient({ cliPath: CLI_PATH, useStdio: false });
52+
const client = new CopilotClient({ useStdio: false });
5453

5554
await client.createSession();
5655

@@ -67,7 +66,7 @@ describe("Client", () => {
6766
});
6867

6968
it("should forceStop without cleanup", async () => {
70-
const client = new CopilotClient({ cliPath: CLI_PATH });
69+
const client = new CopilotClient({});
7170
onTestFinishedForceStop(client);
7271

7372
await client.createSession();
@@ -76,7 +75,7 @@ describe("Client", () => {
7675
});
7776

7877
it("should get status with version and protocol info", async () => {
79-
const client = new CopilotClient({ cliPath: CLI_PATH, useStdio: true });
78+
const client = new CopilotClient({ useStdio: true });
8079
onTestFinishedForceStop(client);
8180

8281
await client.start();
@@ -92,7 +91,7 @@ describe("Client", () => {
9291
});
9392

9493
it("should get auth status", async () => {
95-
const client = new CopilotClient({ cliPath: CLI_PATH, useStdio: true });
94+
const client = new CopilotClient({ useStdio: true });
9695
onTestFinishedForceStop(client);
9796

9897
await client.start();
@@ -108,7 +107,7 @@ describe("Client", () => {
108107
});
109108

110109
it("should list models when authenticated", async () => {
111-
const client = new CopilotClient({ cliPath: CLI_PATH, useStdio: true });
110+
const client = new CopilotClient({ useStdio: true });
112111
onTestFinishedForceStop(client);
113112

114113
await client.start();

‎nodejs/test/e2e/harness/sdkTestContext.ts‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,6 @@ const __filename = fileURLToPath(import.meta.url);
1717
const __dirname = dirname(__filename);
1818
const SNAPSHOTS_DIR = resolve(__dirname, "../../../../test/snapshots");
1919

20-
export const CLI_PATH =
21-
process.env.COPILOT_CLI_PATH ||
22-
resolve(__dirname, "../../../node_modules/@github/copilot/index.js");
23-
2420
export async function createSdkTestContext({
2521
logLevel,
2622
}: { logLevel?: "error" | "none" | "warning" | "info" | "debug" | "all" } = {}) {
@@ -41,7 +37,6 @@ export async function createSdkTestContext({
4137
};
4238

4339
const copilotClient = new CopilotClient({
44-
cliPath: CLI_PATH,
4540
cwd: workDir,
4641
env,
4742
logLevel: logLevel || "error",

‎nodejs/test/e2e/session.test.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { describe, expect, it, onTestFinished } from "vitest";
22
import { ParsedHttpExchange } from "../../../test/harness/replayingCapiProxy.js";
33
import { CopilotClient } from "../../src/index.js";
4-
import { CLI_PATH, createSdkTestContext } from "./harness/sdkTestContext.js";
4+
import { createSdkTestContext } from "./harness/sdkTestContext.js";
55
import { getFinalAssistantMessage, getNextEventOfType } from "./harness/sdkTestHelper.js";
66

77
describe("Sessions", async () => {
@@ -157,7 +157,6 @@ describe("Sessions", async () => {
157157

158158
// Resume using a new client
159159
const newClient = new CopilotClient({
160-
cliPath: CLI_PATH,
161160
env,
162161
githubToken: process.env.CI === "true" ? "fake-token-for-e2e-tests" : undefined,
163162
});

0 commit comments

Comments
 (0)