From 5f94e8bc01132ad3df68b97c9b056c4d0dd4052e Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Tue, 14 Jul 2026 18:43:55 +0200 Subject: [PATCH 1/2] Fix .NET in-process E2E transport Make the in-process E2E matrix use the FFI runtime, restore the native host environment between tests, and align same-client session resume with the other SDKs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c71188a1-1445-46aa-9faf-3b73cf6a6dd9 --- dotnet/src/Client.cs | 27 ++++--- .../E2E/RpcWorkspaceCheckpointsE2ETests.cs | 2 +- dotnet/test/E2E/SessionE2ETests.cs | 17 ++-- dotnet/test/Harness/E2ETestBase.cs | 24 ++++-- dotnet/test/Harness/E2ETestContext.cs | 40 ++++++---- dotnet/test/Harness/E2ETestFixture.cs | 7 +- .../test/Unit/ClientSessionLifetimeTests.cs | 78 +++++++++++++++++-- dotnet/test/Unit/E2ETestFixtureTests.cs | 27 +++++++ 8 files changed, 175 insertions(+), 47 deletions(-) create mode 100644 dotnet/test/Unit/E2ETestFixtureTests.cs diff --git a/dotnet/src/Client.cs b/dotnet/src/Client.cs index 97e4f1094d..b45282aa5b 100644 --- a/dotnet/src/Client.cs +++ b/dotnet/src/Client.cs @@ -773,7 +773,8 @@ private CopilotSession InitializeSession( SessionConfigBase config, Dictionary>>? transformCallbacks, bool hasHooks, - string callerName) + string callerName, + bool replaceExisting = false) { var setupTimestamp = Stopwatch.GetTimestamp(); var session = new CopilotSession( @@ -807,7 +808,15 @@ private CopilotSession InitializeSession( ConfigureSessionFsHandlers(session, config.CreateSessionFsProvider); session.SetCanvasHandler(config.CanvasHandler); session.RegisterBearerTokenProviders(BuildBearerTokenCallbacks(config)); - RegisterSession(session); + if (replaceExisting) + { + _sessions[session.SessionId] = session; + } + else if (!_sessions.TryAdd(session.SessionId, session)) + { + throw new InvalidOperationException($"Session '{session.SessionId}' is already tracked by this client."); + } + session.StartProcessingEvents(); LoggingHelpers.LogTiming(_logger, LogLevel.Debug, null, callerName + " local setup complete. Elapsed={Elapsed}, SessionId={SessionId}, Tools={ToolsCount}, Commands={CommandsCount}, Hooks={HasHooks}", @@ -1285,6 +1294,9 @@ public async Task CreateSessionAsync(SessionConfig config, Cance /// /// This allows you to continue a previous conversation, maintaining all conversation history. /// The session must have been previously created and not deleted. + /// If this client already tracks the session, the returned instance replaces the previous + /// for event and request routing. Existing references to the + /// previous instance remain usable, but no longer receive routed events or requests. /// /// /// @@ -1331,7 +1343,8 @@ public async Task ResumeSessionAsync(string sessionId, ResumeSes config, transformCallbacks, hasHooks, - "CopilotClient.ResumeSessionAsync"); + "CopilotClient.ResumeSessionAsync", + replaceExisting: true); try { var (traceparent, tracestate) = TelemetryHelpers.GetTraceContext(); @@ -2475,14 +2488,6 @@ private static JsonSerializerOptions CreateSerializerOptions() return session; } - private void RegisterSession(CopilotSession session) - { - if (!_sessions.TryAdd(session.SessionId, session)) - { - throw new InvalidOperationException($"Session '{session.SessionId}' is already tracked by this client."); - } - } - private void RemoveSession(string sessionId) { _sessions.TryRemove(sessionId, out _); diff --git a/dotnet/test/E2E/RpcWorkspaceCheckpointsE2ETests.cs b/dotnet/test/E2E/RpcWorkspaceCheckpointsE2ETests.cs index ea4ae15b42..092b28971d 100644 --- a/dotnet/test/E2E/RpcWorkspaceCheckpointsE2ETests.cs +++ b/dotnet/test/E2E/RpcWorkspaceCheckpointsE2ETests.cs @@ -29,7 +29,7 @@ public async Task Should_Return_Null_Or_Empty_Content_For_Unknown_Checkpoint() { await using var session = await CreateSessionAsync(); - var result = await session.Rpc.Workspaces.ReadCheckpointAsync(long.MaxValue); + var result = await session.Rpc.Workspaces.ReadCheckpointAsync(uint.MaxValue); Assert.True(string.IsNullOrEmpty(result.Content)); } diff --git a/dotnet/test/E2E/SessionE2ETests.cs b/dotnet/test/E2E/SessionE2ETests.cs index bcfb46295a..a5adb80ee7 100644 --- a/dotnet/test/E2E/SessionE2ETests.cs +++ b/dotnet/test/E2E/SessionE2ETests.cs @@ -226,17 +226,20 @@ public async Task Should_Create_Session_With_Custom_Tool() } [Fact] - public async Task Should_Reject_Resuming_Active_Session_Using_The_Same_Client() + public async Task Should_Replace_Active_Session_When_Resuming_Using_The_Same_Client() { var session1 = await CreateSessionAsync(); var sessionId = session1.SessionId; - var exception = await Assert.ThrowsAsync(() => - Client.ResumeSessionAsync(sessionId, new ResumeSessionConfig - { - OnPermissionRequest = PermissionHandler.ApproveAll, - })); - Assert.Contains(sessionId, exception.Message); + await using var session2 = await Client.ResumeSessionAsync(sessionId, new ResumeSessionConfig + { + OnPermissionRequest = PermissionHandler.ApproveAll, + }); + + Assert.Equal(sessionId, session2.SessionId); + _ = await session1.GetEventsAsync(); + + await session1.DisposeAsync(); } [Fact] diff --git a/dotnet/test/Harness/E2ETestBase.cs b/dotnet/test/Harness/E2ETestBase.cs index ddd1b894bb..288fd7a8ba 100644 --- a/dotnet/test/Harness/E2ETestBase.cs +++ b/dotnet/test/Harness/E2ETestBase.cs @@ -59,6 +59,7 @@ internal static string GetTestName(ITestOutputHelper output) public async Task InitializeAsync() { + Ctx.PrepareForTest(); await Ctx.CleanupAfterTestAsync(); await Ctx.ConfigureForTestAsync(_snapshotCategory, _testName); } @@ -88,14 +89,23 @@ protected async Task ResumeSessionAsync(string sessionId, Resume config ??= new ResumeSessionConfig(); config.OnPermissionRequest ??= PermissionHandler.ApproveAll; - await Client.StartAsync(); - var port = Client.RuntimePort - ?? throw new InvalidOperationException("The shared E2E client must use TCP transport to support multi-client resume."); - - var client = Ctx.CreateClient(options: new CopilotClientOptions + CopilotClient client; + if (E2ETestContext.UsesInProcessTransport) + { + client = Client; + } + else { - Connection = RuntimeConnection.ForUri($"localhost:{port}", connectionToken: E2ETestFixture.SharedTcpConnectionToken), - }); + await Client.StartAsync(); + var port = Client.RuntimePort + ?? throw new InvalidOperationException("The shared E2E client must use TCP transport to support multi-client resume."); + + client = Ctx.CreateClient(options: new CopilotClientOptions + { + Connection = RuntimeConnection.ForUri($"localhost:{port}", connectionToken: E2ETestFixture.SharedTcpConnectionToken), + }); + } + return await client.ResumeSessionAsync(sessionId, config); } diff --git a/dotnet/test/Harness/E2ETestContext.cs b/dotnet/test/Harness/E2ETestContext.cs index 4c9b892ff2..7bb1c928e3 100644 --- a/dotnet/test/Harness/E2ETestContext.cs +++ b/dotnet/test/Harness/E2ETestContext.cs @@ -16,6 +16,7 @@ public sealed class E2ETestContext : IAsyncDisposable public string HomeDir { get; } public string WorkDir { get; } public string ProxyUrl { get; } + internal static bool UsesInProcessTransport => IsInProcess(null); /// Optional logger injected by tests; applied to all clients created via . public ILogger? Logger { get; set; } @@ -298,22 +299,8 @@ public CopilotClient CreateClient( if (IsInProcess(options.Connection)) { - // In-process hosting: runtime code runs host-side in this process (the - // loaded cdylib) and reads the ambient process environment rather than - // the environment passed to copilot_runtime_host_start, so the per-test - // redirects, cleared tokens/HMAC, and isolated home must be mirrored - // onto this process's real environment. Restored after each test by - // InProcessEnvIsolationAttribute. - foreach (var (name, value) in env) - { - InProcessEnvIsolation.Apply(name, value); - } - - // A per-client WorkingDirectory is rejected in-process; instead point this - // process's cwd at the desired directory so the worker inherits it at spawn - // (restored after the test by InProcessEnvIsolationAttribute). options.WorkingDirectory = null; - InProcessEnvIsolation.SetWorkingDirectory(desiredWorkingDirectory); + ApplyInProcessEnvironment(env, desiredWorkingDirectory); } else if (options.Connection is ChildProcessRuntimeConnection child) { @@ -352,6 +339,29 @@ public CopilotClient CreateClient( return client; } + internal void PrepareForTest() + { + if (UsesInProcessTransport) + { + ApplyInProcessEnvironment(GetEnvironment(), WorkDir); + } + } + + private static void ApplyInProcessEnvironment(IReadOnlyDictionary environment, string workingDirectory) + { + // Runtime code runs host-side in this process and reads its ambient environment, + // so restore the per-test redirects and isolated home after the assembly-level + // isolation attribute reset them at the end of the preceding test. + foreach (var (name, value) in environment) + { + InProcessEnvIsolation.Apply(name, value); + } + + // The worker inherits the host process cwd because the native host has no + // per-client working-directory parameter. + InProcessEnvIsolation.SetWorkingDirectory(workingDirectory); + } + public void UntrackClient(CopilotClient client) { lock (_clientsLock) diff --git a/dotnet/test/Harness/E2ETestFixture.cs b/dotnet/test/Harness/E2ETestFixture.cs index 95bebc1391..e29f5f7f6c 100644 --- a/dotnet/test/Harness/E2ETestFixture.cs +++ b/dotnet/test/Harness/E2ETestFixture.cs @@ -19,10 +19,15 @@ public async Task InitializeAsync() Ctx = await E2ETestContext.CreateAsync(); Client = Ctx.CreateClient(options: new CopilotClientOptions { - Connection = RuntimeConnection.ForTcp(connectionToken: SharedTcpConnectionToken), + Connection = CreateSharedConnection(E2ETestContext.UsesInProcessTransport), }, persistent: true); } + internal static RuntimeConnection CreateSharedConnection(bool useInProcessTransport) => + useInProcessTransport + ? RuntimeConnection.ForInProcess() + : RuntimeConnection.ForTcp(connectionToken: SharedTcpConnectionToken); + public async Task DisposeAsync() { await Ctx.DisposeAsync(); diff --git a/dotnet/test/Unit/ClientSessionLifetimeTests.cs b/dotnet/test/Unit/ClientSessionLifetimeTests.cs index f736e8dee4..77cb406f9c 100644 --- a/dotnet/test/Unit/ClientSessionLifetimeTests.cs +++ b/dotnet/test/Unit/ClientSessionLifetimeTests.cs @@ -183,25 +183,64 @@ public async Task StopAsync_Keeps_Session_Rooted_Until_Destroy_Completes() } [Fact] - public async Task ResumeSessionAsync_Throws_When_Same_Client_Already_Tracks_Session() + public async Task ResumeSessionAsync_Replaces_Session_Tracked_By_Same_Client() { await using var server = await FakeCopilotServer.StartAsync(); await using var client = new CopilotClient(new CopilotClientOptions { Connection = RuntimeConnection.ForUri(server.Url) }); var sessionId = "same-session-id"; - await using var session = await client.CreateSessionAsync(new SessionConfig + var session = await client.CreateSessionAsync(new SessionConfig { SessionId = sessionId, OnPermissionRequest = PermissionHandler.ApproveAll }); AssertSessionCount(client, sessions: 1); - var exception = await Assert.ThrowsAsync(() => client.ResumeSessionAsync(sessionId, new ResumeSessionConfig + var resumedSession = await client.ResumeSessionAsync(sessionId, new ResumeSessionConfig { OnPermissionRequest = PermissionHandler.ApproveAll - })); - Assert.Contains(sessionId, exception.Message); + }); + + Assert.NotSame(session, resumedSession); AssertSessionCount(client, sessions: 1); + Assert.Same(resumedSession, GetTrackedSession(client, sessionId)); + Assert.Equal("message-1", await session.SendAsync("The previous wrapper remains callable.")); + Assert.DoesNotContain(server.Requests, request => request.Method == "session.destroy"); + + await session.DisposeAsync(); + AssertSessionCount(client, sessions: 1); + + await resumedSession.DisposeAsync(); + AssertSessionCount(client, sessions: 0); + Assert.Equal(2, server.Requests.Count(request => request.Method == "session.destroy")); + } + + [Fact] + public async Task Failed_ResumeSessionAsync_Removes_Replacement_Registration() + { + await using var server = await FakeCopilotServer.StartAsync(); + await using var client = new CopilotClient(new CopilotClientOptions { Connection = RuntimeConnection.ForUri(server.Url) }); + + var sessionId = "same-session-id"; + var session = await client.CreateSessionAsync(new SessionConfig + { + SessionId = sessionId, + OnPermissionRequest = PermissionHandler.ApproveAll + }); + server.FailNextResume(); + + await Assert.ThrowsAsync(() => client.ResumeSessionAsync(sessionId, new ResumeSessionConfig + { + OnPermissionRequest = PermissionHandler.ApproveAll + })); + + AssertSessionCount(client, sessions: 0); + Assert.Null(GetTrackedSession(client, sessionId)); + Assert.Equal("message-1", await session.SendAsync("The original session remains active.")); + + await session.DisposeAsync(); + AssertSessionCount(client, sessions: 0); + Assert.Single(server.Requests, request => request.Method == "session.destroy"); } [Fact] @@ -387,6 +426,13 @@ private static void AssertSessionCount(CopilotClient client, int sessions) Assert.Equal(sessions, GetPrivateDictionaryCount(client, "_sessions")); } + private static CopilotSession? GetTrackedSession(CopilotClient client, string sessionId) + { + var method = typeof(CopilotClient).GetMethod("GetSession", BindingFlags.Instance | BindingFlags.NonPublic) + ?? throw new InvalidOperationException("GetSession method was not found."); + return (CopilotSession?)method.Invoke(client, [sessionId]); + } + private static int GetPrivateDictionaryCount(CopilotClient client, string fieldName) { var field = typeof(CopilotClient).GetField(fieldName, BindingFlags.Instance | BindingFlags.NonPublic) @@ -467,6 +513,7 @@ private sealed class FakeCopilotServer : IAsyncDisposable private string? _lastSessionId; private bool _delayDestroy; private bool _failRuntimeShutdown; + private bool _failNextResume; private FakeCopilotServer(TcpListener listener) { @@ -528,6 +575,11 @@ public void FailRuntimeShutdown() _failRuntimeShutdown = true; } + public void FailNextResume() + { + _failNextResume = true; + } + public async ValueTask DisposeAsync() { _allowDestroy.TrySetResult(); @@ -572,6 +624,22 @@ private async Task HandleRequestAsync(Stream stream, JsonElement request, Cancel var id = idElement.Clone(); var method = request.GetProperty("method").GetString(); + if (method == "session.resume" && _failNextResume) + { + _failNextResume = false; + await WriteMessageAsync(stream, new Dictionary + { + ["jsonrpc"] = "2.0", + ["id"] = id, + ["error"] = new Dictionary + { + ["code"] = -32000, + ["message"] = "session resume failed" + } + }, cancellationToken); + return; + } + if (method == "runtime.shutdown" && _failRuntimeShutdown) { RuntimeShutdownCount++; diff --git a/dotnet/test/Unit/E2ETestFixtureTests.cs b/dotnet/test/Unit/E2ETestFixtureTests.cs new file mode 100644 index 0000000000..f7dee3ce0a --- /dev/null +++ b/dotnet/test/Unit/E2ETestFixtureTests.cs @@ -0,0 +1,27 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------------------------------------------*/ + +using Xunit; + +namespace GitHub.Copilot.Test.Unit; + +public class E2ETestFixtureTests +{ + [Fact] + public void Shared_Client_Uses_InProcess_Connection_For_InProcess_Tests() + { + var connection = E2ETestFixture.CreateSharedConnection(useInProcessTransport: true); + + Assert.IsType(connection); + } + + [Fact] + public void Shared_Client_Preserves_Tcp_Connection_For_OutOfProcess_Tests() + { + var connection = Assert.IsType( + E2ETestFixture.CreateSharedConnection(useInProcessTransport: false)); + + Assert.Equal(E2ETestFixture.SharedTcpConnectionToken, connection.ConnectionToken); + } +} From ad67cd3667d60e66e3ed272d64496d38725cd611 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Tue, 14 Jul 2026 19:09:29 +0200 Subject: [PATCH 2/2] Preserve session routing after failed resume Capture the wrapper displaced before session.resume and restore it only if the failed replacement is still registered. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c71188a1-1445-46aa-9faf-3b73cf6a6dd9 --- dotnet/src/Client.cs | 36 +++++++++++++++---- .../test/Unit/ClientSessionLifetimeTests.cs | 6 ++-- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/dotnet/src/Client.cs b/dotnet/src/Client.cs index b45282aa5b..c6d724999b 100644 --- a/dotnet/src/Client.cs +++ b/dotnet/src/Client.cs @@ -774,8 +774,10 @@ private CopilotSession InitializeSession( Dictionary>>? transformCallbacks, bool hasHooks, string callerName, - bool replaceExisting = false) + bool replaceExisting, + out CopilotSession? replacedSession) { + replacedSession = null; var setupTimestamp = Stopwatch.GetTimestamp(); var session = new CopilotSession( sessionId, @@ -810,7 +812,16 @@ private CopilotSession InitializeSession( session.RegisterBearerTokenProviders(BuildBearerTokenCallbacks(config)); if (replaceExisting) { - _sessions[session.SessionId] = session; + CopilotSession? displacedSession = null; + _sessions.AddOrUpdate( + session.SessionId, + session, + (_, current) => + { + displacedSession = current; + return session; + }); + replacedSession = displacedSession; } else if (!_sessions.TryAdd(session.SessionId, session)) { @@ -1128,7 +1139,9 @@ public async Task CreateSessionAsync(SessionConfig config, Cance config, transformCallbacks, hasHooks, - "CopilotClient.CreateSessionAsync"); + "CopilotClient.CreateSessionAsync", + replaceExisting: false, + out _); } try { @@ -1228,7 +1241,9 @@ public async Task CreateSessionAsync(SessionConfig config, Cance config, transformCallbacks, hasHooks, - "CopilotClient.CreateSessionAsync"); + "CopilotClient.CreateSessionAsync", + replaceExisting: false, + out _); } }; @@ -1344,7 +1359,8 @@ public async Task ResumeSessionAsync(string sessionId, ResumeSes transformCallbacks, hasHooks, "CopilotClient.ResumeSessionAsync", - replaceExisting: true); + replaceExisting: true, + out var previousSession); try { var (traceparent, tracestate) = TelemetryHelpers.GetTraceContext(); @@ -1443,7 +1459,15 @@ public async Task ResumeSessionAsync(string sessionId, ResumeSes } catch (Exception ex) { - session.RemoveFromClient(); + if (previousSession is null) + { + session.RemoveFromClient(); + } + else + { + _sessions.TryUpdate(sessionId, previousSession, session); + } + if (ex is not OperationCanceledException) { LoggingHelpers.LogTiming(_logger, LogLevel.Warning, ex, diff --git a/dotnet/test/Unit/ClientSessionLifetimeTests.cs b/dotnet/test/Unit/ClientSessionLifetimeTests.cs index 77cb406f9c..bfd09253b2 100644 --- a/dotnet/test/Unit/ClientSessionLifetimeTests.cs +++ b/dotnet/test/Unit/ClientSessionLifetimeTests.cs @@ -216,7 +216,7 @@ public async Task ResumeSessionAsync_Replaces_Session_Tracked_By_Same_Client() } [Fact] - public async Task Failed_ResumeSessionAsync_Removes_Replacement_Registration() + public async Task Failed_ResumeSessionAsync_Restores_Previous_Registration() { await using var server = await FakeCopilotServer.StartAsync(); await using var client = new CopilotClient(new CopilotClientOptions { Connection = RuntimeConnection.ForUri(server.Url) }); @@ -234,8 +234,8 @@ public async Task Failed_ResumeSessionAsync_Removes_Replacement_Registration() OnPermissionRequest = PermissionHandler.ApproveAll })); - AssertSessionCount(client, sessions: 0); - Assert.Null(GetTrackedSession(client, sessionId)); + AssertSessionCount(client, sessions: 1); + Assert.Same(session, GetTrackedSession(client, sessionId)); Assert.Equal("message-1", await session.SendAsync("The original session remains active.")); await session.DisposeAsync();