Skip to content

Commit e7a9879

Browse files
node: add per-connection env + in-process env/telemetry guards
Bring the Node SDK to parity with .NET and Python on the transport env API: - Factor a shared ChildProcessRuntimeConnection base interface for the stdio/tcp transports and give it a per-connection `env`, exposed via forStdio/forTcp. Connection-level env takes precedence over the client-level env, which falls back to process.env. - Reject at construction: `env` or `telemetry` with forInProcess() (the shared host process can't carry per-client values; workingDirectory was already rejected), and `env` set on both the client and the connection for child-process transports. - Unit tests for all guards + precedence; README documents the connection env and in-process restrictions. Also drop the redundant "coherent only for child-process transports" phrasing from the Python connection env docstring (it's attached to the child-process base already). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a96d1a82-c80e-40f1-a3df-a9708429b68b
1 parent 1275871 commit e7a9879

6 files changed

Lines changed: 136 additions & 17 deletions

File tree

nodejs/README.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,11 @@ new CopilotClient(options?: CopilotClientOptions)
8484
**Options:**
8585

8686
- `connection?: RuntimeConnection` - How to connect to the Copilot runtime. Construct via the factory functions on `RuntimeConnection`:
87-
- `RuntimeConnection.forStdio({ path?, args? })` (default) — spawn the runtime and communicate over its stdin/stdout.
88-
- `RuntimeConnection.forTcp({ port?, connectionToken?, path?, args? })` — spawn the runtime as a TCP server.
87+
- `RuntimeConnection.forStdio({ path?, args?, env? })` (default) — spawn the runtime and communicate over its stdin/stdout.
88+
- `RuntimeConnection.forTcp({ port?, connectionToken?, path?, args?, env? })` — spawn the runtime as a TCP server.
8989
- `RuntimeConnection.forUri(url, { connectionToken? })` — connect to an already-running runtime (mutually exclusive with `gitHubToken`/`useLoggedInUser`). There is no top-level `cliUrl` shortcut; use this factory for URL-based connections.
90+
- `RuntimeConnection.forInProcess()` — host the runtime in-process over its native C ABI (FFI). **Experimental.** Because the runtime shares this process, `env`, `telemetry`, and `workingDirectory` are rejected with this transport; set them on the host process instead.
91+
- The child-process transports (`forStdio`/`forTcp`) also accept a per-connection `env`. Set it there or via the top-level `env` option — not both (setting both throws).
9092
- `mode?: "empty" | "copilot-cli"` - Defaulting strategy. Use `"empty"` for multi-user server mode; defaults to `"copilot-cli"`.
9193
- `workingDirectory?: string` - Working directory for the runtime process (default: current process cwd).
9294
- `baseDirectory?: string` - Base directory for Copilot data (session state, config, etc.). Sets `COPILOT_HOME` on the spawned runtime. When not set, the runtime defaults to `~/.copilot`. Ignored when connecting via `RuntimeConnection.forUri`.

nodejs/src/client.ts

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,32 @@ export class CopilotClient {
644644
"constructing the client instead."
645645
);
646646
}
647+
if (conn.kind === "inprocess" && options.env !== undefined) {
648+
throw new Error(
649+
"env is not supported with RuntimeConnection.forInProcess(): the in-process transport loads " +
650+
"the native runtime into the shared host process, whose single environment block cannot " +
651+
"carry per-client values. Set the variables on the host process environment instead."
652+
);
653+
}
654+
if (conn.kind === "inprocess" && options.telemetry !== undefined) {
655+
throw new Error(
656+
"telemetry is not supported with RuntimeConnection.forInProcess(): telemetry configuration " +
657+
"is lowered to environment variables read by native runtime code running in the shared " +
658+
"host process, so per-client telemetry cannot be honored in-process. Configure telemetry " +
659+
"via the host process environment, or use a child-process transport."
660+
);
661+
}
662+
if (
663+
(conn.kind === "stdio" || conn.kind === "tcp") &&
664+
conn.env !== undefined &&
665+
options.env !== undefined
666+
) {
667+
throw new Error(
668+
"Set environment variables via either the client-level env option or the connection's env " +
669+
"(RuntimeConnection.forStdio/forTcp), not both. Prefer the connection-level env for " +
670+
"child-process transports."
671+
);
672+
}
647673
if (conn.kind === "tcp" && conn.connectionToken !== undefined) {
648674
if (typeof conn.connectionToken !== "string" || conn.connectionToken.length === 0) {
649675
throw new Error("connectionToken must be a non-empty string");
@@ -681,7 +707,13 @@ export class CopilotClient {
681707
this.onGitHubTelemetry = options.onGitHubTelemetry;
682708
this.setupClientGlobalHandlers();
683709

684-
const effectiveEnv = options.env ?? process.env;
710+
// Connection-level env (child-process transports only) takes precedence
711+
// over the client-level env, which falls back to the ambient process env.
712+
// The constructor guard above rejects setting both, so at most one of the
713+
// first two is defined. Mirrors .NET/Python precedence.
714+
const connEnv: Record<string, string> | undefined =
715+
conn.kind === "stdio" || conn.kind === "tcp" ? conn.env : undefined;
716+
const effectiveEnv = connEnv ?? options.env ?? process.env;
685717
this.resolvedEnv = effectiveEnv;
686718
this.resolvedCliPath =
687719
conn.kind === "stdio" || conn.kind === "tcp"

nodejs/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ export type {
6363
InProcessRuntimeConnection,
6464
TcpRuntimeConnection,
6565
UriRuntimeConnection,
66+
ChildProcessRuntimeConnection,
6667
CustomAgentConfig,
6768
ElicitationFieldValue,
6869
ElicitationHandler,

nodejs/src/types.ts

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -101,15 +101,29 @@ export type RuntimeConnection =
101101
| UriRuntimeConnection;
102102

103103
/**
104-
* Spawns a runtime child process and communicates over its stdin/stdout.
105-
* This is the default if no {@link CopilotClientOptions.connection} is set.
104+
* Shared shape for the transports that spawn a runtime **child process**
105+
* ({@link StdioRuntimeConnection} and {@link TcpRuntimeConnection}).
106106
*/
107-
export interface StdioRuntimeConnection {
108-
readonly kind: "stdio";
107+
export interface ChildProcessRuntimeConnection {
109108
/** Path to the runtime executable. When omitted, the bundled runtime is used. */
110109
readonly path?: string;
111110
/** Extra command-line arguments to pass to the runtime process. */
112111
readonly args?: readonly string[];
112+
/**
113+
* Environment variables for the spawned runtime child process, replacing the
114+
* inherited environment. Cannot be combined with
115+
* {@link CopilotClientOptions.env}; setting both throws when the client is
116+
* constructed. When omitted, the client-level env (or `process.env`) is used.
117+
*/
118+
readonly env?: Record<string, string>;
119+
}
120+
121+
/**
122+
* Spawns a runtime child process and communicates over its stdin/stdout.
123+
* This is the default if no {@link CopilotClientOptions.connection} is set.
124+
*/
125+
export interface StdioRuntimeConnection extends ChildProcessRuntimeConnection {
126+
readonly kind: "stdio";
113127
}
114128

115129
/**
@@ -135,7 +149,7 @@ export interface InProcessRuntimeConnection {
135149
/**
136150
* Spawns a runtime child process that listens on a TCP socket and connects to it.
137151
*/
138-
export interface TcpRuntimeConnection {
152+
export interface TcpRuntimeConnection extends ChildProcessRuntimeConnection {
139153
readonly kind: "tcp";
140154
/**
141155
* TCP port to listen on. `0` (the default) auto-allocates a free port.
@@ -148,10 +162,6 @@ export interface TcpRuntimeConnection {
148162
* loopback listener is safe by default.
149163
*/
150164
readonly connectionToken?: string;
151-
/** Path to the runtime executable. When omitted, the bundled runtime is used. */
152-
readonly path?: string;
153-
/** Extra command-line arguments to pass to the runtime process. */
154-
readonly args?: readonly string[];
155165
}
156166

157167
/**
@@ -175,8 +185,10 @@ export const RuntimeConnection = {
175185
* Spawn a runtime child process and communicate over its stdin/stdout.
176186
* This is the default if no {@link CopilotClientOptions.connection} is set.
177187
*/
178-
forStdio(opts: { path?: string; args?: readonly string[] } = {}): StdioRuntimeConnection {
179-
return { kind: "stdio", path: opts.path, args: opts.args };
188+
forStdio(
189+
opts: { path?: string; args?: readonly string[]; env?: Record<string, string> } = {}
190+
): StdioRuntimeConnection {
191+
return { kind: "stdio", path: opts.path, args: opts.args, env: opts.env };
180192
},
181193
/**
182194
* Spawn a runtime child process that listens on a TCP socket and connect to it.
@@ -187,6 +199,7 @@ export const RuntimeConnection = {
187199
connectionToken?: string;
188200
path?: string;
189201
args?: readonly string[];
202+
env?: Record<string, string>;
190203
} = {}
191204
): TcpRuntimeConnection {
192205
return {
@@ -195,6 +208,7 @@ export const RuntimeConnection = {
195208
connectionToken: opts.connectionToken,
196209
path: opts.path,
197210
args: opts.args,
211+
env: opts.env,
198212
};
199213
},
200214
/**

nodejs/test/client.test.ts

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2007,6 +2007,77 @@ describe("CopilotClient", () => {
20072007
/gitHubToken and useLoggedInUser cannot be used with RuntimeConnection.forUri/
20082008
);
20092009
});
2010+
2011+
it("should throw error when env is used with forInProcess", () => {
2012+
expect(() => {
2013+
new CopilotClient({
2014+
connection: RuntimeConnection.forInProcess(),
2015+
env: { FOO: "bar" },
2016+
logLevel: "error",
2017+
});
2018+
}).toThrow(/env is not supported with RuntimeConnection.forInProcess/);
2019+
});
2020+
2021+
it("should throw error when telemetry is used with forInProcess", () => {
2022+
expect(() => {
2023+
new CopilotClient({
2024+
connection: RuntimeConnection.forInProcess(),
2025+
telemetry: { otlpEndpoint: "http://localhost:4318" },
2026+
logLevel: "error",
2027+
});
2028+
}).toThrow(/telemetry is not supported with RuntimeConnection.forInProcess/);
2029+
});
2030+
2031+
it("should throw error when workingDirectory is used with forInProcess", () => {
2032+
expect(() => {
2033+
new CopilotClient({
2034+
connection: RuntimeConnection.forInProcess(),
2035+
workingDirectory: "/tmp",
2036+
logLevel: "error",
2037+
});
2038+
}).toThrow(/workingDirectory is not supported with RuntimeConnection.forInProcess/);
2039+
});
2040+
2041+
it("should throw error when env is set on both the client and a stdio connection", () => {
2042+
expect(() => {
2043+
new CopilotClient({
2044+
connection: RuntimeConnection.forStdio({ env: { FOO: "conn" } }),
2045+
env: { FOO: "client" },
2046+
logLevel: "error",
2047+
});
2048+
}).toThrow(
2049+
/Set environment variables via either the client-level env option or the connection/
2050+
);
2051+
});
2052+
2053+
it("should throw error when env is set on both the client and a tcp connection", () => {
2054+
expect(() => {
2055+
new CopilotClient({
2056+
connection: RuntimeConnection.forTcp({ env: { FOO: "conn" } }),
2057+
env: { FOO: "client" },
2058+
logLevel: "error",
2059+
});
2060+
}).toThrow(
2061+
/Set environment variables via either the client-level env option or the connection/
2062+
);
2063+
});
2064+
2065+
it("should use the connection-level env for child-process transports", () => {
2066+
const client = new CopilotClient({
2067+
connection: RuntimeConnection.forStdio({ env: { FOO: "from-conn" } }),
2068+
logLevel: "error",
2069+
});
2070+
expect((client as any).resolvedEnv).toEqual({ FOO: "from-conn" });
2071+
});
2072+
2073+
it("should allow env on the client alone with a child-process transport", () => {
2074+
const client = new CopilotClient({
2075+
connection: RuntimeConnection.forStdio(),
2076+
env: { FOO: "from-client" },
2077+
logLevel: "error",
2078+
});
2079+
expect((client as any).resolvedEnv).toEqual({ FOO: "from-client" });
2080+
});
20102081
});
20112082

20122083
describe("overridesBuiltInTool in tool definitions", () => {

python/copilot/client.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -400,9 +400,8 @@ class ChildProcessRuntimeConnection(RuntimeConnection):
400400
env: dict[str, str] | None = None
401401
"""Per-connection environment variables for the spawned child process.
402402
403-
Coherent only for child-process transports (each client owns its own OS
404-
process). When set, do not also set :attr:`CopilotClientOptions.env` — the
405-
client rejects setting environment in both places. ``None`` inherits the
403+
When set, do not also set :attr:`CopilotClientOptions.env` — the client
404+
rejects setting environment in both places. ``None`` inherits the
406405
client-level env (or the current process env)."""
407406

408407

0 commit comments

Comments
 (0)