Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 58 additions & 5 deletions dotnet/src/Client.cs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,8 @@ public CopilotClient(CopilotClientOptions? options = null)
throw new ArgumentException($"Unsupported RuntimeConnection type: {_connection.GetType().Name}", nameof(options));
}

ValidateEnvironmentOptions(_options, _connection);

_logger = _options.Logger ?? NullLogger.Instance;
_onListModels = _options.OnListModels;

Expand Down Expand Up @@ -202,6 +204,53 @@ _options.SessionFs is not null ||
}
}

/// <summary>
/// Validates environment-variable options against the resolved transport.
/// Per-client environment is only representable for child-process transports
/// (each client owns its own OS process). The in-process (FFI) transport
/// loads the native runtime into the shared host process, whose single
/// environment block cannot carry per-client values, so environment and
/// telemetry options that lower to environment variables are rejected there.
/// </summary>
private static void ValidateEnvironmentOptions(CopilotClientOptions options, RuntimeConnection connection)
{
if (connection is InProcessRuntimeConnection)
{
if (options.Environment is not null)
{
throw new ArgumentException(
$"{nameof(CopilotClientOptions)}.{nameof(CopilotClientOptions.Environment)} is not supported with " +
$"{nameof(RuntimeConnection)}.{nameof(RuntimeConnection.ForInProcess)}(): the in-process transport " +
"loads the native runtime into the shared host process, whose single environment block cannot carry " +
"per-client values. Set the variables on the host process environment instead.",
nameof(options));
}

if (options.Telemetry is not null)
{
throw new ArgumentException(
$"{nameof(CopilotClientOptions)}.{nameof(CopilotClientOptions.Telemetry)} is not supported with " +
$"{nameof(RuntimeConnection)}.{nameof(RuntimeConnection.ForInProcess)}(): telemetry configuration is " +
"lowered to environment variables read by native runtime code running in the shared host process, so " +
"per-client telemetry cannot be honored in-process. Configure telemetry via the host process " +
"environment, or use a child-process transport.",
nameof(options));
}

return;
}

if (connection is ChildProcessRuntimeConnection { Environment: not null } && options.Environment is not null)
{
throw new ArgumentException(
$"Set environment variables via either {nameof(CopilotClientOptions)}.{nameof(CopilotClientOptions.Environment)} " +
$"or {nameof(ChildProcessRuntimeConnection)}.{nameof(ChildProcessRuntimeConnection.Environment)}, not both. " +
$"Prefer {nameof(ChildProcessRuntimeConnection)}.{nameof(ChildProcessRuntimeConnection.Environment)} for " +
"child-process transports.",
nameof(options));
}
}
Comment thread
SteveSandersonMS marked this conversation as resolved.

/// <summary>
/// Environment variable that overrides the transport used when the caller does not
/// specify <see cref="CopilotClientOptions.Connection"/>. Accepts <c>"inprocess"</c>
Expand Down Expand Up @@ -1943,9 +1992,12 @@ private static void ApplyTelemetryEnvironment(IDictionary<string, string?> envir
var tcpConnection = _connection as TcpRuntimeConnection;
var useStdio = _connection is StdioRuntimeConnection;

// Use explicit path, COPILOT_CLI_PATH env var (from options.Environment or process env), or bundled runtime - no PATH fallback
var envCliPath = options.Environment is not null && options.Environment.TryGetValue("COPILOT_CLI_PATH", out var envValue) ? envValue
: System.Environment.GetEnvironmentVariable("COPILOT_CLI_PATH");
// Use explicit path, COPILOT_CLI_PATH env var (from the connection's
// Environment, options.Environment, or process env), or bundled runtime - no PATH fallback
var envCliPath =
(childProcessConnection.Environment is not null && childProcessConnection.Environment.TryGetValue("COPILOT_CLI_PATH", out var connEnvValue) ? connEnvValue : null)
?? (options.Environment is not null && options.Environment.TryGetValue("COPILOT_CLI_PATH", out var envValue) ? envValue : null)
?? System.Environment.GetEnvironmentVariable("COPILOT_CLI_PATH");
var cliPath = childProcessConnection.Path
?? envCliPath
?? GetBundledCliPath(out var searchedPath)
Expand Down Expand Up @@ -2012,10 +2064,11 @@ private static void ApplyTelemetryEnvironment(IDictionary<string, string?> envir
CreateNoWindow = true
};

if (options.Environment != null)
var childEnvironment = options.Environment ?? childProcessConnection.Environment;
if (childEnvironment != null)
{
startInfo.Environment.Clear();
foreach (var (key, value) in options.Environment)
foreach (var (key, value) in childEnvironment)
{
startInfo.Environment[key] = value;
}
Expand Down
20 changes: 19 additions & 1 deletion dotnet/src/Types.cs
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,16 @@ internal ChildProcessRuntimeConnection() { }

/// <summary>Extra command-line arguments to pass to the runtime process.</summary>
public IList<string>? Args { get; set; }

/// <summary>
/// Gets or sets the environment variables passed to the spawned runtime process,
/// replacing the inherited environment.
/// </summary>
/// <remarks>
/// Cannot be combined with <see cref="CopilotClientOptions.Environment"/>; setting both throws
/// an <see cref="ArgumentException"/> when the client is constructed.
/// </remarks>
public IReadOnlyDictionary<string, string>? Environment { get; set; }
}

/// <summary>
Expand Down Expand Up @@ -358,7 +368,15 @@ private CopilotClientOptions(CopilotClientOptions? other)
/// </summary>
public CopilotLogLevel? LogLevel { get; set; }

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

/// <summary>Logger instance for SDK diagnostic output.</summary>
Expand Down
2 changes: 1 addition & 1 deletion dotnet/test/ConnectionTokenTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public class ConnectionTokenAutoGeneratedTests : IAsyncLifetime
public async Task InitializeAsync()
{
_ctx = await E2ETestContext.CreateAsync();
_client = _ctx.CreateClient(useStdio: false);
_client = _ctx.CreateClient(options: new CopilotClientOptions { Connection = RuntimeConnection.ForTcp() });
}

public async Task DisposeAsync()
Expand Down
3 changes: 1 addition & 2 deletions dotnet/test/E2E/ClientOptionsE2ETests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ public async Task Should_Propagate_Process_Options_To_Spawned_Cli()
{
Connection = RuntimeConnection.ForStdio(path: cliPath, args: ["--capture-file", capturePath]),
BaseDirectory = copilotHomeFromOption,
Environment = clientEnv,
GitHubToken = "process-option-token",
LogLevel = CopilotLogLevel.Debug,
SessionIdleTimeoutSeconds = 17,
Expand All @@ -85,7 +84,7 @@ public async Task Should_Propagate_Process_Options_To_Spawned_Cli()
CaptureContent = true,
},
UseLoggedInUser = false,
});
}, environment: clientEnv);

await client.StartAsync();

Expand Down
3 changes: 1 addition & 2 deletions dotnet/test/E2E/CopilotRequestWebSocketE2ETests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@ public async Task Services_A_WebSocket_Turn_End_To_End_Via_The_Request_Handler()
{
Connection = RuntimeConnection.ForStdio(),
RequestHandler = handler,
Environment = env,
});
}, environment: env);
await client.StartAsync();

var session = await client.CreateSessionAsync(new SessionConfig
Expand Down
2 changes: 1 addition & 1 deletion dotnet/test/E2E/ModeHandlersE2ETests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ private CopilotClient CreateAuthenticatedClient()
["COPILOT_DEBUG_GITHUB_API_URL"] = Ctx.ProxyUrl,
};

return Ctx.CreateClient(options: new CopilotClientOptions { Environment = env });
return Ctx.CreateClient(environment: env);
}

private Task ConfigureAuthenticatedUserAsync()
Expand Down
5 changes: 2 additions & 3 deletions dotnet/test/E2E/PerSessionAuthE2ETests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ private CopilotClient CreateAuthTestClient()
};
// Disable the harness's auto-injected client token so the per-session
// auth tests validate only session-scoped tokens.
return Ctx.CreateClient(options: new CopilotClientOptions { Environment = env }, autoInjectGitHubToken: false);
return Ctx.CreateClient(environment: env, autoInjectGitHubToken: false);
}

private CopilotClient CreateNoAuthTestClient()
Expand All @@ -32,9 +32,8 @@ private CopilotClient CreateNoAuthTestClient()

return Ctx.CreateClient(options: new CopilotClientOptions
{
Environment = env,
UseLoggedInUser = false,
}, autoInjectGitHubToken: false);
}, autoInjectGitHubToken: false, environment: env);
}

private static Dictionary<string, string> WithoutAuthEnv(Dictionary<string, string> env)
Expand Down
2 changes: 1 addition & 1 deletion dotnet/test/E2E/ProviderEndpointE2ETests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ private CopilotClient CreateProviderEndpointClient()
{
["COPILOT_ALLOW_GET_PROVIDER_ENDPOINT"] = "true",
};
return Ctx.CreateClient(options: new CopilotClientOptions { Environment = env });
return Ctx.CreateClient(environment: env);
}

[Fact]
Expand Down
3 changes: 1 addition & 2 deletions dotnet/test/E2E/RpcExtensionsLoadedE2ETests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,7 @@ private CopilotClient CreateExtensionsClient()
return Ctx.CreateClient(options: new CopilotClientOptions
{
Connection = RuntimeConnection.ForStdio(args: ["--yolo"]),
Environment = ExtensionsEnabledEnvironment(),
});
}, environment: ExtensionsEnabledEnvironment());
}

/// <summary>
Expand Down
5 changes: 1 addition & 4 deletions dotnet/test/E2E/RpcMcpAndSkillsE2ETests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -398,10 +398,7 @@ private CopilotClient CreateMcpAppsClient()
environment["COPILOT_MCP_APPS"] = "true";
environment["MCP_APPS"] = "true";

return Ctx.CreateClient(options: new CopilotClientOptions
{
Environment = environment,
});
return Ctx.CreateClient(environment: environment);
}

private static void CreateSkill(string skillsDir, string skillName, string description)
Expand Down
10 changes: 3 additions & 7 deletions dotnet/test/E2E/RpcServerE2ETests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,8 @@ private CopilotClient CreateAuthenticatedClient(string token)

return Ctx.CreateClient(options: new CopilotClientOptions
{
Environment = env,
GitHubToken = token,
});
}, environment: env);
}

private async Task ConfigureAuthenticatedUserAsync(
Expand Down Expand Up @@ -237,10 +236,7 @@ public async Task Should_Add_Secret_Filter_Values()
{
var environment = Ctx.GetEnvironment();
environment["COPILOT_ENABLE_SECRET_FILTERING"] = "true";
await using var client = Ctx.CreateClient(options: new CopilotClientOptions
{
Environment = environment,
});
await using var client = Ctx.CreateClient(environment: environment);
await client.StartAsync();
var secret = $"rpc-secret-{Guid.NewGuid():N}";

Expand Down Expand Up @@ -381,7 +377,7 @@ public async Task Should_Check_In_Use_Session_From_Another_Runtime_And_Release_L
{
var sessionId = Guid.NewGuid().ToString();
var workingDirectory = CreateUniqueWorkDirectory("server-rpc-in-use");
await using var otherClient = Ctx.CreateClient(useStdio: true);
await using var otherClient = Ctx.CreateClient(options: new CopilotClientOptions { Connection = RuntimeConnection.ForStdio() });
await using var otherSession = await otherClient.CreateSessionAsync(new SessionConfig
{
SessionId = sessionId,
Expand Down
5 changes: 3 additions & 2 deletions dotnet/test/E2E/RpcServerMiscE2ETests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -235,15 +235,16 @@ public async Task Should_Reject_Send_Attachments_From_Non_Extension_Connection()
env["GITHUB_TOKEN"] = "";
}

var options = new CopilotClientOptions { Environment = env };
var options = new CopilotClientOptions();
if (!autoInjectGitHubToken)
{
options.UseLoggedInUser = false;
}

var client = Ctx.CreateClient(
options: options,
autoInjectGitHubToken: autoInjectGitHubToken);
autoInjectGitHubToken: autoInjectGitHubToken,
environment: env);
await client.StartAsync();
return (client, home);
}
Expand Down
2 changes: 1 addition & 1 deletion dotnet/test/E2E/RpcServerPluginsE2ETests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ This skill exists so the plugin reports at least one installed skill.
env["XDG_CONFIG_HOME"] = home;
env["XDG_STATE_HOME"] = home;

var client = Ctx.CreateClient(options: new CopilotClientOptions { Environment = env });
var client = Ctx.CreateClient(environment: env);
await client.StartAsync();
return (client, home);
}
Expand Down
3 changes: 1 addition & 2 deletions dotnet/test/E2E/RpcSessionStateExtrasE2ETests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -313,9 +313,8 @@ private CopilotClient CreateAuthenticatedClient(string token)

return Ctx.CreateClient(options: new CopilotClientOptions
{
Environment = env,
GitHubToken = token,
});
}, environment: env);
}

private async Task ConfigureAuthenticatedUserAsync(string token)
Expand Down
2 changes: 1 addition & 1 deletion dotnet/test/E2E/SessionFsSqliteE2ETests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,9 @@ await TestHelper.WaitForConditionAsync(
private CopilotClient CreateSessionFsClient()
{
return Ctx.CreateClient(
useStdio: true,
options: new CopilotClientOptions
{
Connection = RuntimeConnection.ForStdio(),
SessionFs = SessionFsConfig,
});
}
Expand Down
2 changes: 1 addition & 1 deletion dotnet/test/E2E/SubagentHooksE2ETests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public async Task Should_Invoke_PreToolUse_And_PostToolUse_Hooks_For_Sub_Agent_T
// Create a client with the session-based subagents feature flag
var env = new Dictionary<string, string>(Ctx.GetEnvironment());
env["COPILOT_EXP_COPILOT_CLI_SESSION_BASED_SUBAGENTS"] = "true";
var client = Ctx.CreateClient(options: new CopilotClientOptions { Environment = env });
var client = Ctx.CreateClient(environment: env);

var session = await client.CreateSessionAsync(new SessionConfig
{
Expand Down
1 change: 1 addition & 0 deletions dotnet/test/E2E/TelemetryExportE2ETests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public async Task Should_Export_File_Telemetry_For_Sdk_Interactions()

await using var client = Ctx.CreateClient(options: new CopilotClientOptions
{
Connection = RuntimeConnection.ForStdio(),
Telemetry = new TelemetryConfig
{
FilePath = telemetryPath,
Expand Down
Loading
Loading