Skip to content

Commit 7aaa13d

Browse files
C#: make per-client Environment coherent per transport (#1930)
1 parent f3ebf3e commit 7aaa13d

18 files changed

Lines changed: 163 additions & 66 deletions

dotnet/src/Client.cs

Lines changed: 58 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,8 @@ public CopilotClient(CopilotClientOptions? options = null)
174174
throw new ArgumentException($"Unsupported RuntimeConnection type: {_connection.GetType().Name}", nameof(options));
175175
}
176176

177+
ValidateEnvironmentOptions(_options, _connection);
178+
177179
_logger = _options.Logger ?? NullLogger.Instance;
178180
_onListModels = _options.OnListModels;
179181

@@ -202,6 +204,53 @@ _options.SessionFs is not null ||
202204
}
203205
}
204206

207+
/// <summary>
208+
/// Validates environment-variable options against the resolved transport.
209+
/// Per-client environment is only representable for child-process transports
210+
/// (each client owns its own OS process). The in-process (FFI) transport
211+
/// loads the native runtime into the shared host process, whose single
212+
/// environment block cannot carry per-client values, so environment and
213+
/// telemetry options that lower to environment variables are rejected there.
214+
/// </summary>
215+
private static void ValidateEnvironmentOptions(CopilotClientOptions options, RuntimeConnection connection)
216+
{
217+
if (connection is InProcessRuntimeConnection)
218+
{
219+
if (options.Environment is not null)
220+
{
221+
throw new ArgumentException(
222+
$"{nameof(CopilotClientOptions)}.{nameof(CopilotClientOptions.Environment)} is not supported with " +
223+
$"{nameof(RuntimeConnection)}.{nameof(RuntimeConnection.ForInProcess)}(): the in-process transport " +
224+
"loads the native runtime into the shared host process, whose single environment block cannot carry " +
225+
"per-client values. Set the variables on the host process environment instead.",
226+
nameof(options));
227+
}
228+
229+
if (options.Telemetry is not null)
230+
{
231+
throw new ArgumentException(
232+
$"{nameof(CopilotClientOptions)}.{nameof(CopilotClientOptions.Telemetry)} is not supported with " +
233+
$"{nameof(RuntimeConnection)}.{nameof(RuntimeConnection.ForInProcess)}(): telemetry configuration is " +
234+
"lowered to environment variables read by native runtime code running in the shared host process, so " +
235+
"per-client telemetry cannot be honored in-process. Configure telemetry via the host process " +
236+
"environment, or use a child-process transport.",
237+
nameof(options));
238+
}
239+
240+
return;
241+
}
242+
243+
if (connection is ChildProcessRuntimeConnection { Environment: not null } && options.Environment is not null)
244+
{
245+
throw new ArgumentException(
246+
$"Set environment variables via either {nameof(CopilotClientOptions)}.{nameof(CopilotClientOptions.Environment)} " +
247+
$"or {nameof(ChildProcessRuntimeConnection)}.{nameof(ChildProcessRuntimeConnection.Environment)}, not both. " +
248+
$"Prefer {nameof(ChildProcessRuntimeConnection)}.{nameof(ChildProcessRuntimeConnection.Environment)} for " +
249+
"child-process transports.",
250+
nameof(options));
251+
}
252+
}
253+
205254
/// <summary>
206255
/// Environment variable that overrides the transport used when the caller does not
207256
/// specify <see cref="CopilotClientOptions.Connection"/>. Accepts <c>"inprocess"</c>
@@ -1943,9 +1992,12 @@ private static void ApplyTelemetryEnvironment(IDictionary<string, string?> envir
19431992
var tcpConnection = _connection as TcpRuntimeConnection;
19441993
var useStdio = _connection is StdioRuntimeConnection;
19451994

1946-
// Use explicit path, COPILOT_CLI_PATH env var (from options.Environment or process env), or bundled runtime - no PATH fallback
1947-
var envCliPath = options.Environment is not null && options.Environment.TryGetValue("COPILOT_CLI_PATH", out var envValue) ? envValue
1948-
: System.Environment.GetEnvironmentVariable("COPILOT_CLI_PATH");
1995+
// Use explicit path, COPILOT_CLI_PATH env var (from the connection's
1996+
// Environment, options.Environment, or process env), or bundled runtime - no PATH fallback
1997+
var envCliPath =
1998+
(childProcessConnection.Environment is not null && childProcessConnection.Environment.TryGetValue("COPILOT_CLI_PATH", out var connEnvValue) ? connEnvValue : null)
1999+
?? (options.Environment is not null && options.Environment.TryGetValue("COPILOT_CLI_PATH", out var envValue) ? envValue : null)
2000+
?? System.Environment.GetEnvironmentVariable("COPILOT_CLI_PATH");
19492001
var cliPath = childProcessConnection.Path
19502002
?? envCliPath
19512003
?? GetBundledCliPath(out var searchedPath)
@@ -2012,10 +2064,11 @@ private static void ApplyTelemetryEnvironment(IDictionary<string, string?> envir
20122064
CreateNoWindow = true
20132065
};
20142066

2015-
if (options.Environment != null)
2067+
var childEnvironment = options.Environment ?? childProcessConnection.Environment;
2068+
if (childEnvironment != null)
20162069
{
20172070
startInfo.Environment.Clear();
2018-
foreach (var (key, value) in options.Environment)
2071+
foreach (var (key, value) in childEnvironment)
20192072
{
20202073
startInfo.Environment[key] = value;
20212074
}

dotnet/src/Types.cs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,16 @@ internal ChildProcessRuntimeConnection() { }
173173

174174
/// <summary>Extra command-line arguments to pass to the runtime process.</summary>
175175
public IList<string>? Args { get; set; }
176+
177+
/// <summary>
178+
/// Gets or sets the environment variables passed to the spawned runtime process,
179+
/// replacing the inherited environment.
180+
/// </summary>
181+
/// <remarks>
182+
/// Cannot be combined with <see cref="CopilotClientOptions.Environment"/>; setting both throws
183+
/// an <see cref="ArgumentException"/> when the client is constructed.
184+
/// </remarks>
185+
public IReadOnlyDictionary<string, string>? Environment { get; set; }
176186
}
177187

178188
/// <summary>
@@ -358,7 +368,15 @@ private CopilotClientOptions(CopilotClientOptions? other)
358368
/// </summary>
359369
public CopilotLogLevel? LogLevel { get; set; }
360370

361-
/// <summary>Environment variables to pass to the runtime process.</summary>
371+
/// <summary>
372+
/// Gets or sets environment variables passed to the runtime process.
373+
/// </summary>
374+
/// <remarks>
375+
/// Not supported with the in-process transport (<see cref="RuntimeConnection.ForInProcess"/>),
376+
/// which runs the runtime in the host process; setting this option there throws an
377+
/// <see cref="ArgumentException"/>. For child-process transports, prefer
378+
/// <see cref="ChildProcessRuntimeConnection.Environment"/>; setting both throws.
379+
/// </remarks>
362380
public IReadOnlyDictionary<string, string>? Environment { get; set; }
363381

364382
/// <summary>Logger instance for SDK diagnostic output.</summary>

dotnet/test/ConnectionTokenTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ public class ConnectionTokenAutoGeneratedTests : IAsyncLifetime
113113
public async Task InitializeAsync()
114114
{
115115
_ctx = await E2ETestContext.CreateAsync();
116-
_client = _ctx.CreateClient(useStdio: false);
116+
_client = _ctx.CreateClient(options: new CopilotClientOptions { Connection = RuntimeConnection.ForTcp() });
117117
}
118118

119119
public async Task DisposeAsync()

dotnet/test/E2E/ClientOptionsE2ETests.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,6 @@ public async Task Should_Propagate_Process_Options_To_Spawned_Cli()
7171
{
7272
Connection = RuntimeConnection.ForStdio(path: cliPath, args: ["--capture-file", capturePath]),
7373
BaseDirectory = copilotHomeFromOption,
74-
Environment = clientEnv,
7574
GitHubToken = "process-option-token",
7675
LogLevel = CopilotLogLevel.Debug,
7776
SessionIdleTimeoutSeconds = 17,
@@ -85,7 +84,7 @@ public async Task Should_Propagate_Process_Options_To_Spawned_Cli()
8584
CaptureContent = true,
8685
},
8786
UseLoggedInUser = false,
88-
});
87+
}, environment: clientEnv);
8988

9089
await client.StartAsync();
9190

dotnet/test/E2E/CopilotRequestWebSocketE2ETests.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,7 @@ public async Task Services_A_WebSocket_Turn_End_To_End_Via_The_Request_Handler()
5151
{
5252
Connection = RuntimeConnection.ForStdio(),
5353
RequestHandler = handler,
54-
Environment = env,
55-
});
54+
}, environment: env);
5655
await client.StartAsync();
5756

5857
var session = await client.CreateSessionAsync(new SessionConfig

dotnet/test/E2E/ModeHandlersE2ETests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ private CopilotClient CreateAuthenticatedClient()
155155
["COPILOT_DEBUG_GITHUB_API_URL"] = Ctx.ProxyUrl,
156156
};
157157

158-
return Ctx.CreateClient(options: new CopilotClientOptions { Environment = env });
158+
return Ctx.CreateClient(environment: env);
159159
}
160160

161161
private Task ConfigureAuthenticatedUserAsync()

dotnet/test/E2E/PerSessionAuthE2ETests.cs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ private CopilotClient CreateAuthTestClient()
2222
};
2323
// Disable the harness's auto-injected client token so the per-session
2424
// auth tests validate only session-scoped tokens.
25-
return Ctx.CreateClient(options: new CopilotClientOptions { Environment = env }, autoInjectGitHubToken: false);
25+
return Ctx.CreateClient(environment: env, autoInjectGitHubToken: false);
2626
}
2727

2828
private CopilotClient CreateNoAuthTestClient()
@@ -32,9 +32,8 @@ private CopilotClient CreateNoAuthTestClient()
3232

3333
return Ctx.CreateClient(options: new CopilotClientOptions
3434
{
35-
Environment = env,
3635
UseLoggedInUser = false,
37-
}, autoInjectGitHubToken: false);
36+
}, autoInjectGitHubToken: false, environment: env);
3837
}
3938

4039
private static Dictionary<string, string> WithoutAuthEnv(Dictionary<string, string> env)

dotnet/test/E2E/ProviderEndpointE2ETests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ private CopilotClient CreateProviderEndpointClient()
2323
{
2424
["COPILOT_ALLOW_GET_PROVIDER_ENDPOINT"] = "true",
2525
};
26-
return Ctx.CreateClient(options: new CopilotClientOptions { Environment = env });
26+
return Ctx.CreateClient(environment: env);
2727
}
2828

2929
[Fact]

dotnet/test/E2E/RpcExtensionsLoadedE2ETests.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,7 @@ private CopilotClient CreateExtensionsClient()
5858
return Ctx.CreateClient(options: new CopilotClientOptions
5959
{
6060
Connection = RuntimeConnection.ForStdio(args: ["--yolo"]),
61-
Environment = ExtensionsEnabledEnvironment(),
62-
});
61+
}, environment: ExtensionsEnabledEnvironment());
6362
}
6463

6564
/// <summary>

dotnet/test/E2E/RpcMcpAndSkillsE2ETests.cs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -398,10 +398,7 @@ private CopilotClient CreateMcpAppsClient()
398398
environment["COPILOT_MCP_APPS"] = "true";
399399
environment["MCP_APPS"] = "true";
400400

401-
return Ctx.CreateClient(options: new CopilotClientOptions
402-
{
403-
Environment = environment,
404-
});
401+
return Ctx.CreateClient(environment: environment);
405402
}
406403

407404
private static void CreateSkill(string skillsDir, string skillName, string description)

0 commit comments

Comments
 (0)