Skip to content

Commit bfb3a7e

Browse files
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>
1 parent 2cf5855 commit bfb3a7e

7 files changed

Lines changed: 70 additions & 147 deletions

File tree

dotnet/src/Client.cs

Lines changed: 14 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -774,11 +774,8 @@ private CopilotSession InitializeSession(
774774
SessionConfigBase config,
775775
Dictionary<string, Func<string, Task<string>>>? transformCallbacks,
776776
bool hasHooks,
777-
string callerName,
778-
bool replaceExisting,
779-
out CopilotSession? replacedSession)
777+
string callerName)
780778
{
781-
replacedSession = null;
782779
var setupTimestamp = Stopwatch.GetTimestamp();
783780
var session = new CopilotSession(
784781
sessionId,
@@ -811,24 +808,7 @@ private CopilotSession InitializeSession(
811808
ConfigureSessionFsHandlers(session, config.CreateSessionFsProvider);
812809
session.SetCanvasHandler(config.CanvasHandler);
813810
session.RegisterBearerTokenProviders(BuildBearerTokenCallbacks(config));
814-
if (replaceExisting)
815-
{
816-
CopilotSession? displacedSession = null;
817-
_sessions.AddOrUpdate(
818-
session.SessionId,
819-
session,
820-
(_, current) =>
821-
{
822-
displacedSession = current;
823-
return session;
824-
});
825-
replacedSession = displacedSession;
826-
}
827-
else if (!_sessions.TryAdd(session.SessionId, session))
828-
{
829-
throw new InvalidOperationException($"Session '{session.SessionId}' is already tracked by this client.");
830-
}
831-
811+
RegisterSession(session);
832812
session.StartProcessingEvents();
833813
LoggingHelpers.LogTiming(_logger, LogLevel.Debug, null,
834814
callerName + " local setup complete. Elapsed={Elapsed}, SessionId={SessionId}, Tools={ToolsCount}, Commands={CommandsCount}, Hooks={HasHooks}",
@@ -1141,9 +1121,7 @@ public async Task<CopilotSession> CreateSessionAsync(SessionConfig config, Cance
11411121
config,
11421122
transformCallbacks,
11431123
hasHooks,
1144-
"CopilotClient.CreateSessionAsync",
1145-
replaceExisting: false,
1146-
out _);
1124+
"CopilotClient.CreateSessionAsync");
11471125
}
11481126
try
11491127
{
@@ -1243,9 +1221,7 @@ public async Task<CopilotSession> CreateSessionAsync(SessionConfig config, Cance
12431221
config,
12441222
transformCallbacks,
12451223
hasHooks,
1246-
"CopilotClient.CreateSessionAsync",
1247-
replaceExisting: false,
1248-
out _);
1224+
"CopilotClient.CreateSessionAsync");
12491225
}
12501226
};
12511227

@@ -1311,9 +1287,6 @@ public async Task<CopilotSession> CreateSessionAsync(SessionConfig config, Cance
13111287
/// <remarks>
13121288
/// This allows you to continue a previous conversation, maintaining all conversation history.
13131289
/// The session must have been previously created and not deleted.
1314-
/// If this client already tracks the session, the returned instance replaces the previous
1315-
/// <see cref="CopilotSession"/> for event and request routing. Existing references to the
1316-
/// previous instance remain usable, but no longer receive routed events or requests.
13171290
/// </remarks>
13181291
/// <example>
13191292
/// <code>
@@ -1361,9 +1334,7 @@ public async Task<CopilotSession> ResumeSessionAsync(string sessionId, ResumeSes
13611334
config,
13621335
transformCallbacks,
13631336
hasHooks,
1364-
"CopilotClient.ResumeSessionAsync",
1365-
replaceExisting: true,
1366-
out var previousSession);
1337+
"CopilotClient.ResumeSessionAsync");
13671338
try
13681339
{
13691340
var (traceparent, tracestate) = TelemetryHelpers.GetTraceContext();
@@ -1462,15 +1433,7 @@ public async Task<CopilotSession> ResumeSessionAsync(string sessionId, ResumeSes
14621433
}
14631434
catch (Exception ex)
14641435
{
1465-
if (previousSession is null)
1466-
{
1467-
session.RemoveFromClient();
1468-
}
1469-
else
1470-
{
1471-
_sessions.TryUpdate(sessionId, previousSession, session);
1472-
}
1473-
1436+
session.RemoveFromClient();
14741437
if (ex is not OperationCanceledException)
14751438
{
14761439
LoggingHelpers.LogTiming(_logger, LogLevel.Warning, ex,
@@ -2515,6 +2478,14 @@ private static JsonSerializerOptions CreateSerializerOptions()
25152478
return session;
25162479
}
25172480

2481+
private void RegisterSession(CopilotSession session)
2482+
{
2483+
if (!_sessions.TryAdd(session.SessionId, session))
2484+
{
2485+
throw new InvalidOperationException($"Session '{session.SessionId}' is already tracked by this client.");
2486+
}
2487+
}
2488+
25182489
private void RemoveSession(string sessionId)
25192490
{
25202491
_sessions.TryRemove(sessionId, out _);

dotnet/test/E2E/CommandsE2ETests.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,8 +202,9 @@ public async Task Session_With_Commands_Creates_Successfully()
202202
[Fact]
203203
public async Task Session_With_Commands_Resumes_Successfully()
204204
{
205-
var session1 = await CreateSessionAsync();
205+
await using var session1 = await CreateSessionAsync();
206206
var sessionId = session1.SessionId;
207+
await SuspendAndUntrackSessionForResumeAsync(session1);
207208

208209
var session2 = await ResumeSessionAsync(sessionId, new ResumeSessionConfig
209210
{

dotnet/test/E2E/SessionConfigE2ETests.cs

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -173,9 +173,11 @@ public async Task Should_Apply_All_ReasoningEffort_Values_On_Session_Create(stri
173173
[Trait(E2ETestTraits.Backend, E2ETestTraits.SelfConfiguredBackend)]
174174
public async Task Should_Apply_ReasoningEffort_On_Session_Resume()
175175
{
176-
var originalSession = await CreateSessionAsync();
176+
await using var originalSession = await CreateSessionAsync();
177+
var sessionId = originalSession.SessionId;
178+
await SuspendAndUntrackSessionForResumeAsync(originalSession);
177179
const string reasoningModelId = "custom-reasoning-model";
178-
var resumedSession = await ResumeSessionAsync(originalSession.SessionId, new ResumeSessionConfig
180+
var resumedSession = await ResumeSessionAsync(sessionId, new ResumeSessionConfig
179181
{
180182
Model = reasoningModelId,
181183
Provider = CreateProxyProvider("resume-reasoning"),
@@ -187,7 +189,6 @@ public async Task Should_Apply_ReasoningEffort_On_Session_Resume()
187189
Assert.Equal("high", resumeEvent.Data.ReasoningEffort);
188190

189191
await resumedSession.DisposeAsync();
190-
await originalSession.DisposeAsync();
191192
}
192193

193194
[Fact]
@@ -233,8 +234,9 @@ public async Task Should_Forward_Custom_Provider_Headers_On_Create()
233234
[Trait(E2ETestTraits.Backend, E2ETestTraits.SelfConfiguredBackend)]
234235
public async Task Should_Forward_Custom_Provider_Headers_On_Resume()
235236
{
236-
var session1 = await CreateSessionAsync();
237+
await using var session1 = await CreateSessionAsync();
237238
var sessionId = session1.SessionId;
239+
await SuspendAndUntrackSessionForResumeAsync(session1);
238240

239241
var session2 = await ResumeSessionAsync(sessionId, new ResumeSessionConfig
240242
{
@@ -339,8 +341,9 @@ public async Task Should_Apply_WorkingDirectory_On_Session_Resume()
339341
Directory.CreateDirectory(subDir);
340342
await File.WriteAllTextAsync(Path.Join(subDir, "resume-marker.txt"), "I am in the resume working directory");
341343

342-
var session1 = await CreateSessionAsync();
344+
await using var session1 = await CreateSessionAsync();
343345
var sessionId = session1.SessionId;
346+
await SuspendAndUntrackSessionForResumeAsync(session1);
344347

345348
var session2 = await ResumeSessionAsync(sessionId, new ResumeSessionConfig
346349
{
@@ -360,8 +363,9 @@ public async Task Should_Apply_WorkingDirectory_On_Session_Resume()
360363
[Fact]
361364
public async Task Should_Apply_SystemMessage_On_Session_Resume()
362365
{
363-
var session1 = await CreateSessionAsync();
366+
await using var session1 = await CreateSessionAsync();
364367
var sessionId = session1.SessionId;
368+
await SuspendAndUntrackSessionForResumeAsync(session1);
365369

366370
var resumeInstruction = "End the response with RESUME_SYSTEM_MESSAGE_SENTINEL.";
367371
var session2 = await ResumeSessionAsync(sessionId, new ResumeSessionConfig
@@ -422,11 +426,13 @@ await File.WriteAllTextAsync(
422426
Path.Join(instructionFilesDir, "extra.instructions.md"),
423427
$"Always include {sentinel}.");
424428

425-
var session1 = await CreateSessionAsync(new SessionConfig
429+
await using var session1 = await CreateSessionAsync(new SessionConfig
426430
{
427431
WorkingDirectory = projectDir,
428432
});
429-
var session2 = await ResumeSessionAsync(session1.SessionId, new ResumeSessionConfig
433+
var sessionId = session1.SessionId;
434+
await SuspendAndUntrackSessionForResumeAsync(session1);
435+
var session2 = await ResumeSessionAsync(sessionId, new ResumeSessionConfig
430436
{
431437
WorkingDirectory = projectDir,
432438
InstructionDirectories = [instructionDir],
@@ -438,14 +444,14 @@ await File.WriteAllTextAsync(
438444
Assert.Contains(sentinel, GetSystemMessage(exchange));
439445

440446
await session2.DisposeAsync();
441-
await session1.DisposeAsync();
442447
}
443448

444449
[Fact]
445450
public async Task Should_Apply_AvailableTools_On_Session_Resume()
446451
{
447-
var session1 = await CreateSessionAsync();
452+
await using var session1 = await CreateSessionAsync();
448453
var sessionId = session1.SessionId;
454+
await SuspendAndUntrackSessionForResumeAsync(session1);
449455

450456
var session2 = await ResumeSessionAsync(sessionId, new ResumeSessionConfig
451457
{
@@ -493,8 +499,10 @@ public async Task Should_Apply_Session_Limits_On_Create()
493499
[Fact]
494500
public async Task Should_Apply_Session_Limits_On_Resume()
495501
{
496-
var session1 = await CreateSessionAsync();
497-
var session2 = await ResumeSessionAsync(session1.SessionId, new ResumeSessionConfig
502+
await using var session1 = await CreateSessionAsync();
503+
var sessionId = session1.SessionId;
504+
await SuspendAndUntrackSessionForResumeAsync(session1);
505+
var session2 = await ResumeSessionAsync(sessionId, new ResumeSessionConfig
498506
{
499507
SessionLimits = new SessionLimitsConfig
500508
{
@@ -513,7 +521,6 @@ public async Task Should_Apply_Session_Limits_On_Resume()
513521
finally
514522
{
515523
await session2.DisposeAsync();
516-
await session1.DisposeAsync();
517524
}
518525
}
519526

@@ -558,8 +565,10 @@ public async Task Should_Apply_Excluded_Built_In_Agents_On_Resume()
558565
{
559566
const string excludedAgent = "explore";
560567

561-
var session1 = await CreateSessionAsync();
562-
var session2 = await ResumeSessionAsync(session1.SessionId, new ResumeSessionConfig
568+
await using var session1 = await CreateSessionAsync();
569+
var sessionId = session1.SessionId;
570+
await SuspendAndUntrackSessionForResumeAsync(session1);
571+
var session2 = await ResumeSessionAsync(sessionId, new ResumeSessionConfig
563572
{
564573
ExcludedBuiltInAgents = [excludedAgent],
565574
});
@@ -575,7 +584,6 @@ public async Task Should_Apply_Excluded_Built_In_Agents_On_Resume()
575584
finally
576585
{
577586
await session2.DisposeAsync();
578-
await session1.DisposeAsync();
579587
}
580588
}
581589

dotnet/test/E2E/SessionE2ETests.cs

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -226,20 +226,17 @@ public async Task Should_Create_Session_With_Custom_Tool()
226226
}
227227

228228
[Fact]
229-
public async Task Should_Replace_Active_Session_When_Resuming_Using_The_Same_Client()
229+
public async Task Should_Reject_Resuming_Active_Session_Using_The_Same_Client()
230230
{
231-
var session1 = await CreateSessionAsync();
231+
await using var session1 = await CreateSessionAsync();
232232
var sessionId = session1.SessionId;
233233

234-
await using var session2 = await Ctx.ResumeSessionAsync(Client, sessionId, new ResumeSessionConfig
235-
{
236-
OnPermissionRequest = PermissionHandler.ApproveAll,
237-
});
238-
239-
Assert.Equal(sessionId, session2.SessionId);
240-
_ = await session1.GetEventsAsync();
241-
242-
await session1.DisposeAsync();
234+
var exception = await Assert.ThrowsAsync<InvalidOperationException>(() =>
235+
Ctx.ResumeSessionAsync(Client, sessionId, new ResumeSessionConfig
236+
{
237+
OnPermissionRequest = PermissionHandler.ApproveAll,
238+
}));
239+
Assert.Contains(sessionId, exception.Message);
243240
}
244241

245242
[Fact]
@@ -986,8 +983,9 @@ public async Task Should_Create_Session_With_Azure_Provider()
986983
[Trait(E2ETestTraits.Backend, E2ETestTraits.SelfConfiguredBackend)]
987984
public async Task Should_Resume_Session_With_Custom_Provider()
988985
{
989-
var session = await CreateSessionAsync();
986+
await using var session = await CreateSessionAsync();
990987
var sessionId = session.SessionId;
988+
await SuspendAndUntrackSessionForResumeAsync(session);
991989

992990
var session2 = await ResumeSessionAsync(sessionId, new ResumeSessionConfig
993991
{
@@ -1009,7 +1007,5 @@ public async Task Should_Resume_Session_With_Custom_Provider()
10091007
{
10101008
// disconnect may fail since the provider is fake
10111009
}
1012-
1013-
await session.DisposeAsync();
10141010
}
10151011
}

dotnet/test/E2E/SkillsE2ETests.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,13 +208,14 @@ public async Task Should_Apply_Skill_On_Session_Resume_With_SkillDirectories()
208208
var skillsDir = CreateSkillDir();
209209

210210
// Create a session without skills first
211-
var session1 = await CreateSessionAsync();
211+
await using var session1 = await CreateSessionAsync();
212212
var sessionId = session1.SessionId;
213213

214214
// First message without skill - marker should not appear
215215
var message1 = await session1.SendAndWaitAsync(new MessageOptions { Prompt = "Say hi." });
216216
Assert.NotNull(message1);
217217
Assert.DoesNotContain(SkillMarker, message1!.Data.Content);
218+
await SuspendAndUntrackSessionForResumeAsync(session1);
218219

219220
// Resume with skillDirectories - skill should now be active
220221
var session2 = await ResumeSessionAsync(sessionId, new ResumeSessionConfig

dotnet/test/Harness/E2ETestBase.cs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,20 @@ protected async Task<CopilotSession> ResumeSessionAsync(string sessionId, Resume
109109
return await Ctx.ResumeSessionAsync(client, sessionId, config);
110110
}
111111

112+
protected static async Task SuspendAndUntrackSessionForResumeAsync(CopilotSession session)
113+
{
114+
await session.Rpc.SuspendAsync();
115+
116+
// In-process clients host separate runtimes, while session.destroy removes the
117+
// session from the current runtime. Untrack locally to exercise resume without
118+
// either replacing an active wrapper or destroying the session first.
119+
var removeFromClient = typeof(CopilotSession).GetMethod(
120+
"RemoveFromClient",
121+
BindingFlags.Instance | BindingFlags.NonPublic)
122+
?? throw new InvalidOperationException("CopilotSession.RemoveFromClient was not found.");
123+
removeFromClient.Invoke(session, null);
124+
}
125+
112126
protected static string GetSystemMessage(ParsedHttpExchange exchange)
113127
{
114128
return exchange.Request.Messages.FirstOrDefault(m => m.Role == "system")?.StringContent ?? string.Empty;

0 commit comments

Comments
 (0)