Skip to content

Commit 63adfee

Browse files
committed
Fix orphaned OnDisposed callback when resuming sessions
When ResumeSessionAsync replaces an existing session, clear the old session's OnDisposed callback to prevent it from firing SessionDestroyed if disposed later. Added test to verify old session disposal doesn't fire spurious events.
1 parent d19847d commit 63adfee

File tree

2 files changed

+32
-0
lines changed

2 files changed

+32
-0
lines changed

dotnet/src/Client.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,13 @@ public async Task<CopilotSession> ResumeSessionAsync(string sessionId, ResumeSes
439439
session.RegisterPermissionHandler(config.OnPermissionRequest);
440440
}
441441

442+
// Clear OnDisposed on the old session to prevent it from firing SessionDestroyed
443+
// if it gets disposed after being replaced
444+
if (_sessions.TryGetValue(response.SessionId, out var oldSession))
445+
{
446+
oldSession.OnDisposed = null;
447+
}
448+
442449
// Replace any existing session entry to ensure new config (like permission handler) is used
443450
_sessions[response.SessionId] = session;
444451

dotnet/test/SessionTests.cs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,31 @@ public async Task Should_Fire_SessionCreated_When_Session_Is_Resumed()
212212
Assert.Same(session2, resumedSession);
213213
}
214214

215+
[Fact]
216+
public async Task Should_Not_Fire_SessionDestroyed_When_Old_Session_Is_Disposed_After_Resume()
217+
{
218+
var session1 = await Client.CreateSessionAsync();
219+
var sessionId = session1.SessionId;
220+
221+
var destroyedIds = new List<string>();
222+
Client.SessionDestroyed += id => destroyedIds.Add(id);
223+
224+
// Resume creates a new session object for the same session ID
225+
var session2 = await Client.ResumeSessionAsync(sessionId);
226+
227+
// Disposing the old session object should NOT fire SessionDestroyed
228+
// because session2 is now the active session for this ID
229+
await session1.DisposeAsync();
230+
231+
Assert.Empty(destroyedIds);
232+
233+
// Disposing the new session should fire SessionDestroyed
234+
await session2.DisposeAsync();
235+
236+
Assert.Single(destroyedIds);
237+
Assert.Equal(sessionId, destroyedIds[0]);
238+
}
239+
215240
[Fact]
216241
public async Task Should_Abort_A_Session()
217242
{

0 commit comments

Comments
 (0)