Skip to content

Commit ab8cc5a

Browse files
authored
[nodejs] Ignore cliPath if cliUrl is set (#787)
* Don't pass cliPath to client if cliUrl was provided * Add cliPath option and validate it at runtime Include an optional cliPath in the client options type and update the Omit<> to account for it. Simplify the default assignment for cliPath (use cliUrl to disable, otherwise use provided cliPath or bundled fallback). Add a runtime check that throws a clear error if cliPath is not available before trying to access the file system, ensuring users provide a local CLI path or use cliUrl. * Add test: cliPath undefined when cliUrl set Add a unit test to nodejs/test/client.test.ts that verifies CopilotClient does not resolve or set options.cliPath when instantiated with a cliUrl. This ensures providing a remote CLI URL doesn't trigger resolution of a local CLI path.
1 parent 27f487f commit ab8cc5a

File tree

2 files changed

+21
-2
lines changed

2 files changed

+21
-2
lines changed

nodejs/src/client.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,12 @@ export class CopilotClient {
141141
private sessions: Map<string, CopilotSession> = new Map();
142142
private stderrBuffer: string = ""; // Captures CLI stderr for error messages
143143
private options: Required<
144-
Omit<CopilotClientOptions, "cliUrl" | "githubToken" | "useLoggedInUser" | "onListModels">
144+
Omit<
145+
CopilotClientOptions,
146+
"cliPath" | "cliUrl" | "githubToken" | "useLoggedInUser" | "onListModels"
147+
>
145148
> & {
149+
cliPath?: string;
146150
cliUrl?: string;
147151
githubToken?: string;
148152
useLoggedInUser?: boolean;
@@ -230,7 +234,7 @@ export class CopilotClient {
230234
this.onListModels = options.onListModels;
231235

232236
this.options = {
233-
cliPath: options.cliPath || getBundledCliPath(),
237+
cliPath: options.cliUrl ? undefined : options.cliPath || getBundledCliPath(),
234238
cliArgs: options.cliArgs ?? [],
235239
cwd: options.cwd ?? process.cwd(),
236240
port: options.port || 0,
@@ -1135,6 +1139,12 @@ export class CopilotClient {
11351139
envWithoutNodeDebug.COPILOT_SDK_AUTH_TOKEN = this.options.githubToken;
11361140
}
11371141

1142+
if (!this.options.cliPath) {
1143+
throw new Error(
1144+
"Path to Copilot CLI is required. Please provide it via the cliPath option, or use cliUrl to rely on a remote CLI."
1145+
);
1146+
}
1147+
11381148
// Verify CLI exists before attempting to spawn
11391149
if (!existsSync(this.options.cliPath)) {
11401150
throw new Error(

nodejs/test/client.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,15 @@ describe("CopilotClient", () => {
210210

211211
expect((client as any).isExternalServer).toBe(true);
212212
});
213+
214+
it("should not resolve cliPath when cliUrl is provided", () => {
215+
const client = new CopilotClient({
216+
cliUrl: "localhost:8080",
217+
logLevel: "error",
218+
});
219+
220+
expect(client["options"].cliPath).toBeUndefined();
221+
});
213222
});
214223

215224
describe("Auth options", () => {

0 commit comments

Comments
 (0)