{{ message }}
C#: make per-client Environment coherent per transport - #1930
Merged
Conversation
CopilotClientOptions.Environment is a per-client dictionary that only makes sense for child-process transports (each client owns its own OS process). Under the in-process (FFI) transport the native runtime loads into the shared host process, whose single environment block cannot carry per-client values, so the option silently did nothing for host-resident native code. SDK changes: - Add Environment to ChildProcessRuntimeConnection (clear+replace semantics, matching options.Environment for easy migration). - Guard in CopilotClient: throw if options.Environment is set with the in-process transport; throw if options.Telemetry is set with the in-process transport (telemetry lowers to env vars read host-side); throw if both options.Environment and the connection's Environment are set. Test harness changes: - E2ETestContext.CreateClient gains an 'environment' parameter that routes the environment to the connection (child-process) or mirrors it onto the host process (in-process), and rejects options.Environment. - Convert all E2E call sites off options.Environment onto the new parameter, dogfooding ChildProcessRuntimeConnection.Environment. - Remove the useStdio shortcut (misleading false==TCP overload); tests that pin a transport now set options.Connection directly. - Pin the file-telemetry E2E test to stdio (telemetry is unsupported in-process). This is the SDK-side slice; making in-process telemetry actually work is a follow-up runtime change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
… ctor The Dictionary<TKey,TValue>(IEnumerable<KeyValuePair>) constructor does not exist on .NET Framework 4.7.2; use LINQ ToDictionary, which works across all target frameworks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
SteveSandersonMS
marked this pull request as ready for review
July 7, 2026 13:45
Copilot stopped reviewing on behalf of
SteveSandersonMS due to an error
July 7, 2026 13:46
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Refactors .NET E2E harness/client environment handling to support transport-aware environment routing, adds a per-connection environment option for child-process transports, and updates tests to use the new pattern.
Changes:
- Update
E2ETestContext.CreateClientto take an explicitenvironmentparameter and route env vars via in-process host env or child-process connection env. - Add
ChildProcessRuntimeConnection.Environmentand validate incompatible environment/telemetry options for in-process transport inCopilotClient. - Update E2E tests to stop using
CopilotClientOptions.Environmentand to pin stdio/tcp viaRuntimeConnection.
Show a summary per file
Review details
- Files reviewed: 18/18 changed files
- Comments generated: 4
- Review effort level: Low
3 tasks
edburns
pushed a commit
that referenced
this pull request
Aug 4, 2026
edburns
pushed a commit
that referenced
this pull request
Aug 4, 2026
This was referenced Sep 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Problem
CopilotClientOptions.Environmentis a per-client dictionary. That model works for child-process transports (each client = its own OS process with its own environment) but is incoherent for the in-process (FFI) transport, where the native Rust runtime loads into the single shared .NET host process. A process has one environment block, so "per-client env vars" cannot be honored for host-resident native code — the option silently did nothing there.What this PR does (C# / SDK slice)
SDK (
dotnet/src)EnvironmenttoChildProcessRuntimeConnection— per-client env where it's actually coherent (stdio/TCP). Clear+replace semantics, matchingoptions.Environmentfor easy migration.CopilotClient(fail loud instead of fail silent):options.Environmentis set withRuntimeConnection.ForInProcess()options.Telemetryis set with the in-process transport (telemetry lowers to env vars read host-side, so it can't be honored per-client in-process)options.Environmentand the connection'sEnvironmentare setTest harness (
dotnet/test)E2ETestContext.CreateClientgains anenvironmentparameter that routes the environment to the connection (child-process) or mirrors it onto the host process (in-process), and rejectsoptions.Environment.options.Environmentonto the new parameter, dogfoodingChildProcessRuntimeConnection.Environment.useStdioshortcut (misleadingfalse == TCPoverload); the few transport-pinned tests now setoptions.Connectiondirectly.Scope / follow-ups
This is deliberately the SDK-side slice: it makes every previously-silent incoherence a loud error and gives child-process transports the correct API shape. It does not make in-process telemetry (or in-process env) actually work — that requires threading the exporter/env config through the FFI boundary as explicit per-connection config in the runtime, which is a separate change. Deprecating
options.Environmentoutright is planned for v2.Validation
dotnet format --verify-no-changespasses.ConnectionTokenTests/SessionFsSqliteand the stdio-pinned telemetry test).