From 2cf58555103948e52b578444e3759194e10eec06 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Fri, 17 Jul 2026 08:42:03 +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 while preserving routing after failed resumes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c71188a1-1445-46aa-9faf-3b73cf6a6dd9 --- dotnet/src/Client.cs | 57 ++++++++++---- .../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 | 76 ++++++++++++++++++- dotnet/test/Unit/E2ETestFixtureTests.cs | 27 +++++++ 8 files changed, 201 insertions(+), 49 deletions(-) create mode 100644 dotnet/test/Unit/E2ETestFixtureTests.cs diff --git a/dotnet/src/Client.cs b/dotnet/src/Client.cs index c8d83dfee2..3d78c2052e 100644 --- a/dotnet/src/Client.cs +++ b/dotnet/src/Client.cs @@ -774,8 +774,11 @@ private CopilotSession InitializeSession( SessionConfigBase config, Dictionary>>? transformCallbacks, bool hasHooks, - string callerName) + string callerName, + bool replaceExisting, + out CopilotSession? replacedSession) { + replacedSession = null; var setupTimestamp = Stopwatch.GetTimestamp(); var session = new CopilotSession( sessionId, @@ -808,7 +811,24 @@ private CopilotSession InitializeSession( ConfigureSessionFsHandlers(session, config.CreateSessionFsProvider); session.SetCanvasHandler(config.CanvasHandler); session.RegisterBearerTokenProviders(BuildBearerTokenCallbacks(config)); - RegisterSession(session); + if (replaceExisting) + { + CopilotSession? displacedSession = null; + _sessions.AddOrUpdate( + session.SessionId, + session, + (_, current) => + { + displacedSession = current; + return session; + }); + replacedSession = displacedSession; + } + 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}", @@ -1121,7 +1141,9 @@ public async Task CreateSessionAsync(SessionConfig config, Cance config, transformCallbacks, hasHooks, - "CopilotClient.CreateSessionAsync"); + "CopilotClient.CreateSessionAsync", + replaceExisting: false, + out _); } try { @@ -1221,7 +1243,9 @@ public async Task CreateSessionAsync(SessionConfig config, Cance config, transformCallbacks, hasHooks, - "CopilotClient.CreateSessionAsync"); + "CopilotClient.CreateSessionAsync", + replaceExisting: false, + out _); } }; @@ -1287,6 +1311,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. /// /// /// @@ -1334,7 +1361,9 @@ public async Task ResumeSessionAsync(string sessionId, ResumeSes config, transformCallbacks, hasHooks, - "CopilotClient.ResumeSessionAsync"); + "CopilotClient.ResumeSessionAsync", + replaceExisting: true, + out var previousSession); try { var (traceparent, tracestate) = TelemetryHelpers.GetTraceContext(); @@ -1433,7 +1462,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, @@ -2478,14 +2515,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 e47b6b21b4..eeba268170 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(() => - Ctx.ResumeSessionAsync(Client, sessionId, new ResumeSessionConfig - { - OnPermissionRequest = PermissionHandler.ApproveAll, - })); - Assert.Contains(sessionId, exception.Message); + await using var session2 = await Ctx.ResumeSessionAsync(Client, 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 a3006389bb..94260928ee 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 Ctx.ResumeSessionAsync(client, sessionId, config); } diff --git a/dotnet/test/Harness/E2ETestContext.cs b/dotnet/test/Harness/E2ETestContext.cs index 809ab55a3e..af34de6ec7 100644 --- a/dotnet/test/Harness/E2ETestContext.cs +++ b/dotnet/test/Harness/E2ETestContext.cs @@ -17,6 +17,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; } @@ -322,22 +323,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) { @@ -395,6 +382,29 @@ public Task ResumeSessionAsync( return client.ResumeSessionAsync(sessionId, config); } + 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 e1143db17c..aec0a42e5d 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.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_Restores_Previous_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 })); - Assert.Contains(sessionId, exception.Message); + 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(); + AssertSessionCount(client, sessions: 0); + Assert.Single(server.Requests, request => request.Method == "session.destroy"); } [Fact] @@ -438,6 +477,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) @@ -518,6 +564,7 @@ private sealed class FakeCopilotServer : IAsyncDisposable private string? _lastSessionId; private bool _delayDestroy; private bool _failRuntimeShutdown; + private bool _failNextResume; private FakeCopilotServer(TcpListener listener) { @@ -579,6 +626,11 @@ public void FailRuntimeShutdown() _failRuntimeShutdown = true; } + public void FailNextResume() + { + _failNextResume = true; + } + public async ValueTask DisposeAsync() { _allowDestroy.TrySetResult(); @@ -623,6 +675,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 bfb3a7e0b455e4889d559b2d95d2e1c8bdaa849d Mon Sep 17 00:00:00 2001 From: Steve Sanderson <1101362+SteveSandersonMS@users.noreply.github.com> Date: Thu, 30 Jul 2026 13:52:07 +0000 Subject: [PATCH 2/2] Keep same-client session resume rejected Preserve the existing .NET session ownership model while allowing resume E2E coverage to run through the in-process transport. Suspend and locally untrack the original test wrapper before resuming so the runtime session remains available without introducing replacement semantics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- dotnet/src/Client.cs | 57 ++++---------- dotnet/test/E2E/CommandsE2ETests.cs | 3 +- dotnet/test/E2E/SessionConfigE2ETests.cs | 40 ++++++---- dotnet/test/E2E/SessionE2ETests.cs | 24 +++--- dotnet/test/E2E/SkillsE2ETests.cs | 3 +- dotnet/test/Harness/E2ETestBase.cs | 14 ++++ .../test/Unit/ClientSessionLifetimeTests.cs | 76 +------------------ 7 files changed, 70 insertions(+), 147 deletions(-) diff --git a/dotnet/src/Client.cs b/dotnet/src/Client.cs index 3d78c2052e..c8d83dfee2 100644 --- a/dotnet/src/Client.cs +++ b/dotnet/src/Client.cs @@ -774,11 +774,8 @@ private CopilotSession InitializeSession( SessionConfigBase config, Dictionary>>? transformCallbacks, bool hasHooks, - string callerName, - bool replaceExisting, - out CopilotSession? replacedSession) + string callerName) { - replacedSession = null; var setupTimestamp = Stopwatch.GetTimestamp(); var session = new CopilotSession( sessionId, @@ -811,24 +808,7 @@ private CopilotSession InitializeSession( ConfigureSessionFsHandlers(session, config.CreateSessionFsProvider); session.SetCanvasHandler(config.CanvasHandler); session.RegisterBearerTokenProviders(BuildBearerTokenCallbacks(config)); - if (replaceExisting) - { - CopilotSession? displacedSession = null; - _sessions.AddOrUpdate( - session.SessionId, - session, - (_, current) => - { - displacedSession = current; - return session; - }); - replacedSession = displacedSession; - } - else if (!_sessions.TryAdd(session.SessionId, session)) - { - throw new InvalidOperationException($"Session '{session.SessionId}' is already tracked by this client."); - } - + RegisterSession(session); session.StartProcessingEvents(); LoggingHelpers.LogTiming(_logger, LogLevel.Debug, null, callerName + " local setup complete. Elapsed={Elapsed}, SessionId={SessionId}, Tools={ToolsCount}, Commands={CommandsCount}, Hooks={HasHooks}", @@ -1141,9 +1121,7 @@ public async Task CreateSessionAsync(SessionConfig config, Cance config, transformCallbacks, hasHooks, - "CopilotClient.CreateSessionAsync", - replaceExisting: false, - out _); + "CopilotClient.CreateSessionAsync"); } try { @@ -1243,9 +1221,7 @@ public async Task CreateSessionAsync(SessionConfig config, Cance config, transformCallbacks, hasHooks, - "CopilotClient.CreateSessionAsync", - replaceExisting: false, - out _); + "CopilotClient.CreateSessionAsync"); } }; @@ -1311,9 +1287,6 @@ 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. /// /// /// @@ -1361,9 +1334,7 @@ public async Task ResumeSessionAsync(string sessionId, ResumeSes config, transformCallbacks, hasHooks, - "CopilotClient.ResumeSessionAsync", - replaceExisting: true, - out var previousSession); + "CopilotClient.ResumeSessionAsync"); try { var (traceparent, tracestate) = TelemetryHelpers.GetTraceContext(); @@ -1462,15 +1433,7 @@ public async Task ResumeSessionAsync(string sessionId, ResumeSes } catch (Exception ex) { - if (previousSession is null) - { - session.RemoveFromClient(); - } - else - { - _sessions.TryUpdate(sessionId, previousSession, session); - } - + session.RemoveFromClient(); if (ex is not OperationCanceledException) { LoggingHelpers.LogTiming(_logger, LogLevel.Warning, ex, @@ -2515,6 +2478,14 @@ 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/CommandsE2ETests.cs b/dotnet/test/E2E/CommandsE2ETests.cs index 20db2d7cb4..ce8ddfffa0 100644 --- a/dotnet/test/E2E/CommandsE2ETests.cs +++ b/dotnet/test/E2E/CommandsE2ETests.cs @@ -202,8 +202,9 @@ public async Task Session_With_Commands_Creates_Successfully() [Fact] public async Task Session_With_Commands_Resumes_Successfully() { - var session1 = await CreateSessionAsync(); + await using var session1 = await CreateSessionAsync(); var sessionId = session1.SessionId; + await SuspendAndUntrackSessionForResumeAsync(session1); var session2 = await ResumeSessionAsync(sessionId, new ResumeSessionConfig { diff --git a/dotnet/test/E2E/SessionConfigE2ETests.cs b/dotnet/test/E2E/SessionConfigE2ETests.cs index ad313b116d..1bc4c52eb9 100644 --- a/dotnet/test/E2E/SessionConfigE2ETests.cs +++ b/dotnet/test/E2E/SessionConfigE2ETests.cs @@ -173,9 +173,11 @@ public async Task Should_Apply_All_ReasoningEffort_Values_On_Session_Create(stri [Trait(E2ETestTraits.Backend, E2ETestTraits.SelfConfiguredBackend)] public async Task Should_Apply_ReasoningEffort_On_Session_Resume() { - var originalSession = await CreateSessionAsync(); + await using var originalSession = await CreateSessionAsync(); + var sessionId = originalSession.SessionId; + await SuspendAndUntrackSessionForResumeAsync(originalSession); const string reasoningModelId = "custom-reasoning-model"; - var resumedSession = await ResumeSessionAsync(originalSession.SessionId, new ResumeSessionConfig + var resumedSession = await ResumeSessionAsync(sessionId, new ResumeSessionConfig { Model = reasoningModelId, Provider = CreateProxyProvider("resume-reasoning"), @@ -187,7 +189,6 @@ public async Task Should_Apply_ReasoningEffort_On_Session_Resume() Assert.Equal("high", resumeEvent.Data.ReasoningEffort); await resumedSession.DisposeAsync(); - await originalSession.DisposeAsync(); } [Fact] @@ -233,8 +234,9 @@ public async Task Should_Forward_Custom_Provider_Headers_On_Create() [Trait(E2ETestTraits.Backend, E2ETestTraits.SelfConfiguredBackend)] public async Task Should_Forward_Custom_Provider_Headers_On_Resume() { - var session1 = await CreateSessionAsync(); + await using var session1 = await CreateSessionAsync(); var sessionId = session1.SessionId; + await SuspendAndUntrackSessionForResumeAsync(session1); var session2 = await ResumeSessionAsync(sessionId, new ResumeSessionConfig { @@ -339,8 +341,9 @@ public async Task Should_Apply_WorkingDirectory_On_Session_Resume() Directory.CreateDirectory(subDir); await File.WriteAllTextAsync(Path.Join(subDir, "resume-marker.txt"), "I am in the resume working directory"); - var session1 = await CreateSessionAsync(); + await using var session1 = await CreateSessionAsync(); var sessionId = session1.SessionId; + await SuspendAndUntrackSessionForResumeAsync(session1); var session2 = await ResumeSessionAsync(sessionId, new ResumeSessionConfig { @@ -360,8 +363,9 @@ public async Task Should_Apply_WorkingDirectory_On_Session_Resume() [Fact] public async Task Should_Apply_SystemMessage_On_Session_Resume() { - var session1 = await CreateSessionAsync(); + await using var session1 = await CreateSessionAsync(); var sessionId = session1.SessionId; + await SuspendAndUntrackSessionForResumeAsync(session1); var resumeInstruction = "End the response with RESUME_SYSTEM_MESSAGE_SENTINEL."; var session2 = await ResumeSessionAsync(sessionId, new ResumeSessionConfig @@ -422,11 +426,13 @@ await File.WriteAllTextAsync( Path.Join(instructionFilesDir, "extra.instructions.md"), $"Always include {sentinel}."); - var session1 = await CreateSessionAsync(new SessionConfig + await using var session1 = await CreateSessionAsync(new SessionConfig { WorkingDirectory = projectDir, }); - var session2 = await ResumeSessionAsync(session1.SessionId, new ResumeSessionConfig + var sessionId = session1.SessionId; + await SuspendAndUntrackSessionForResumeAsync(session1); + var session2 = await ResumeSessionAsync(sessionId, new ResumeSessionConfig { WorkingDirectory = projectDir, InstructionDirectories = [instructionDir], @@ -438,14 +444,14 @@ await File.WriteAllTextAsync( Assert.Contains(sentinel, GetSystemMessage(exchange)); await session2.DisposeAsync(); - await session1.DisposeAsync(); } [Fact] public async Task Should_Apply_AvailableTools_On_Session_Resume() { - var session1 = await CreateSessionAsync(); + await using var session1 = await CreateSessionAsync(); var sessionId = session1.SessionId; + await SuspendAndUntrackSessionForResumeAsync(session1); var session2 = await ResumeSessionAsync(sessionId, new ResumeSessionConfig { @@ -493,8 +499,10 @@ public async Task Should_Apply_Session_Limits_On_Create() [Fact] public async Task Should_Apply_Session_Limits_On_Resume() { - var session1 = await CreateSessionAsync(); - var session2 = await ResumeSessionAsync(session1.SessionId, new ResumeSessionConfig + await using var session1 = await CreateSessionAsync(); + var sessionId = session1.SessionId; + await SuspendAndUntrackSessionForResumeAsync(session1); + var session2 = await ResumeSessionAsync(sessionId, new ResumeSessionConfig { SessionLimits = new SessionLimitsConfig { @@ -513,7 +521,6 @@ public async Task Should_Apply_Session_Limits_On_Resume() finally { await session2.DisposeAsync(); - await session1.DisposeAsync(); } } @@ -558,8 +565,10 @@ public async Task Should_Apply_Excluded_Built_In_Agents_On_Resume() { const string excludedAgent = "explore"; - var session1 = await CreateSessionAsync(); - var session2 = await ResumeSessionAsync(session1.SessionId, new ResumeSessionConfig + await using var session1 = await CreateSessionAsync(); + var sessionId = session1.SessionId; + await SuspendAndUntrackSessionForResumeAsync(session1); + var session2 = await ResumeSessionAsync(sessionId, new ResumeSessionConfig { ExcludedBuiltInAgents = [excludedAgent], }); @@ -575,7 +584,6 @@ public async Task Should_Apply_Excluded_Built_In_Agents_On_Resume() finally { await session2.DisposeAsync(); - await session1.DisposeAsync(); } } diff --git a/dotnet/test/E2E/SessionE2ETests.cs b/dotnet/test/E2E/SessionE2ETests.cs index eeba268170..bc9ee703b4 100644 --- a/dotnet/test/E2E/SessionE2ETests.cs +++ b/dotnet/test/E2E/SessionE2ETests.cs @@ -226,20 +226,17 @@ public async Task Should_Create_Session_With_Custom_Tool() } [Fact] - public async Task Should_Replace_Active_Session_When_Resuming_Using_The_Same_Client() + public async Task Should_Reject_Resuming_Active_Session_Using_The_Same_Client() { - var session1 = await CreateSessionAsync(); + await using var session1 = await CreateSessionAsync(); var sessionId = session1.SessionId; - await using var session2 = await Ctx.ResumeSessionAsync(Client, sessionId, new ResumeSessionConfig - { - OnPermissionRequest = PermissionHandler.ApproveAll, - }); - - Assert.Equal(sessionId, session2.SessionId); - _ = await session1.GetEventsAsync(); - - await session1.DisposeAsync(); + var exception = await Assert.ThrowsAsync(() => + Ctx.ResumeSessionAsync(Client, sessionId, new ResumeSessionConfig + { + OnPermissionRequest = PermissionHandler.ApproveAll, + })); + Assert.Contains(sessionId, exception.Message); } [Fact] @@ -986,8 +983,9 @@ public async Task Should_Create_Session_With_Azure_Provider() [Trait(E2ETestTraits.Backend, E2ETestTraits.SelfConfiguredBackend)] public async Task Should_Resume_Session_With_Custom_Provider() { - var session = await CreateSessionAsync(); + await using var session = await CreateSessionAsync(); var sessionId = session.SessionId; + await SuspendAndUntrackSessionForResumeAsync(session); var session2 = await ResumeSessionAsync(sessionId, new ResumeSessionConfig { @@ -1009,7 +1007,5 @@ public async Task Should_Resume_Session_With_Custom_Provider() { // disconnect may fail since the provider is fake } - - await session.DisposeAsync(); } } diff --git a/dotnet/test/E2E/SkillsE2ETests.cs b/dotnet/test/E2E/SkillsE2ETests.cs index 76f84106f6..3b005fc018 100644 --- a/dotnet/test/E2E/SkillsE2ETests.cs +++ b/dotnet/test/E2E/SkillsE2ETests.cs @@ -208,13 +208,14 @@ public async Task Should_Apply_Skill_On_Session_Resume_With_SkillDirectories() var skillsDir = CreateSkillDir(); // Create a session without skills first - var session1 = await CreateSessionAsync(); + await using var session1 = await CreateSessionAsync(); var sessionId = session1.SessionId; // First message without skill - marker should not appear var message1 = await session1.SendAndWaitAsync(new MessageOptions { Prompt = "Say hi." }); Assert.NotNull(message1); Assert.DoesNotContain(SkillMarker, message1!.Data.Content); + await SuspendAndUntrackSessionForResumeAsync(session1); // Resume with skillDirectories - skill should now be active var session2 = await ResumeSessionAsync(sessionId, new ResumeSessionConfig diff --git a/dotnet/test/Harness/E2ETestBase.cs b/dotnet/test/Harness/E2ETestBase.cs index 94260928ee..3eb0f0e97a 100644 --- a/dotnet/test/Harness/E2ETestBase.cs +++ b/dotnet/test/Harness/E2ETestBase.cs @@ -109,6 +109,20 @@ protected async Task ResumeSessionAsync(string sessionId, Resume return await Ctx.ResumeSessionAsync(client, sessionId, config); } + protected static async Task SuspendAndUntrackSessionForResumeAsync(CopilotSession session) + { + await session.Rpc.SuspendAsync(); + + // In-process clients host separate runtimes, while session.destroy removes the + // session from the current runtime. Untrack locally to exercise resume without + // either replacing an active wrapper or destroying the session first. + var removeFromClient = typeof(CopilotSession).GetMethod( + "RemoveFromClient", + BindingFlags.Instance | BindingFlags.NonPublic) + ?? throw new InvalidOperationException("CopilotSession.RemoveFromClient was not found."); + removeFromClient.Invoke(session, null); + } + protected static string GetSystemMessage(ParsedHttpExchange exchange) { return exchange.Request.Messages.FirstOrDefault(m => m.Role == "system")?.StringContent ?? string.Empty; diff --git a/dotnet/test/Unit/ClientSessionLifetimeTests.cs b/dotnet/test/Unit/ClientSessionLifetimeTests.cs index aec0a42e5d..e1143db17c 100644 --- a/dotnet/test/Unit/ClientSessionLifetimeTests.cs +++ b/dotnet/test/Unit/ClientSessionLifetimeTests.cs @@ -183,64 +183,25 @@ public async Task StopAsync_Keeps_Session_Rooted_Until_Destroy_Completes() } [Fact] - public async Task ResumeSessionAsync_Replaces_Session_Tracked_By_Same_Client() + public async Task ResumeSessionAsync_Throws_When_Same_Client_Already_Tracks_Session() { 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 + await using var session = await client.CreateSessionAsync(new SessionConfig { SessionId = sessionId, OnPermissionRequest = PermissionHandler.ApproveAll }); AssertSessionCount(client, sessions: 1); - var resumedSession = await client.ResumeSessionAsync(sessionId, new ResumeSessionConfig - { - OnPermissionRequest = PermissionHandler.ApproveAll - }); - - 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_Restores_Previous_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 + var exception = await Assert.ThrowsAsync(() => client.ResumeSessionAsync(sessionId, new ResumeSessionConfig { OnPermissionRequest = PermissionHandler.ApproveAll })); - + Assert.Contains(sessionId, exception.Message); 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(); - AssertSessionCount(client, sessions: 0); - Assert.Single(server.Requests, request => request.Method == "session.destroy"); } [Fact] @@ -477,13 +438,6 @@ 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) @@ -564,7 +518,6 @@ private sealed class FakeCopilotServer : IAsyncDisposable private string? _lastSessionId; private bool _delayDestroy; private bool _failRuntimeShutdown; - private bool _failNextResume; private FakeCopilotServer(TcpListener listener) { @@ -626,11 +579,6 @@ public void FailRuntimeShutdown() _failRuntimeShutdown = true; } - public void FailNextResume() - { - _failNextResume = true; - } - public async ValueTask DisposeAsync() { _allowDestroy.TrySetResult(); @@ -675,22 +623,6 @@ 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++;