Skip to content

Commit bf126ce

Browse files
stephentoubCopilot
andcommitted
Restore shutdown notification coverage
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent ae18c0f commit bf126ce

7 files changed

Lines changed: 214 additions & 35 deletions

File tree

dotnet/test/SessionTests.cs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,7 @@ public async Task Should_Receive_Session_Events()
248248
var session = await CreateSessionAsync();
249249
var receivedEvents = new List<SessionEvent>();
250250
var idleReceived = new TaskCompletionSource<bool>();
251+
var shutdownReceived = new TaskCompletionSource<bool>();
251252

252253
session.On(evt =>
253254
{
@@ -256,6 +257,10 @@ public async Task Should_Receive_Session_Events()
256257
{
257258
idleReceived.TrySetResult(true);
258259
}
260+
else if (evt is SessionShutdownEvent)
261+
{
262+
shutdownReceived.TrySetResult(true);
263+
}
259264
});
260265

261266
// Send a message to trigger events
@@ -276,8 +281,10 @@ public async Task Should_Receive_Session_Events()
276281
Assert.NotNull(assistantMessage);
277282
Assert.Contains("300", assistantMessage!.Data.Content);
278283

279-
// Shut down session (sends RPC without clearing handlers), then dispose
284+
// Shut down session and verify shutdown event is received
280285
await session.ShutdownAsync();
286+
await shutdownReceived.Task.WaitAsync(TimeSpan.FromSeconds(5));
287+
Assert.Contains(receivedEvents, evt => evt is SessionShutdownEvent);
281288
await session.DisposeAsync();
282289
}
283290

go/internal/e2e/session_test.go

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -594,13 +594,19 @@ func TestSession(t *testing.T) {
594594
}
595595

596596
var receivedEvents []copilot.SessionEvent
597-
idle := make(chan bool)
597+
idle := make(chan struct{}, 1)
598+
shutdown := make(chan struct{}, 1)
598599

599600
session.On(func(event copilot.SessionEvent) {
600601
receivedEvents = append(receivedEvents, event)
601602
if event.Type == "session.idle" {
602603
select {
603-
case idle <- true:
604+
case idle <- struct{}{}:
605+
default:
606+
}
607+
} else if event.Type == "session.shutdown" {
608+
select {
609+
case shutdown <- struct{}{}:
604610
default:
605611
}
606612
}
@@ -657,10 +663,24 @@ func TestSession(t *testing.T) {
657663
t.Errorf("Expected assistant message to contain '300', got %v", assistantMessage.Data.Content)
658664
}
659665

660-
// Shut down session (sends RPC without clearing handlers), then destroy
666+
// Shut down session and verify shutdown event is received
661667
if err := session.Shutdown(); err != nil {
662668
t.Fatalf("Failed to shut down session: %v", err)
663669
}
670+
select {
671+
case <-shutdown:
672+
case <-time.After(5 * time.Second):
673+
t.Fatal("Timed out waiting for session.shutdown")
674+
}
675+
hasShutdown := false
676+
for _, evt := range receivedEvents {
677+
if evt.Type == "session.shutdown" {
678+
hasShutdown = true
679+
}
680+
}
681+
if !hasShutdown {
682+
t.Error("Expected to receive session.shutdown event")
683+
}
664684
if err := session.Destroy(); err != nil {
665685
t.Fatalf("Failed to destroy session: %v", err)
666686
}

go/session_test.go

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,3 +119,132 @@ func TestSession_On(t *testing.T) {
119119
}
120120
})
121121
}
122+
123+
func TestSession_Shutdown(t *testing.T) {
124+
t.Run("shutdown event dispatches to handlers", func(t *testing.T) {
125+
session := &Session{
126+
handlers: make([]sessionHandler, 0),
127+
}
128+
129+
var received []SessionEvent
130+
session.On(func(event SessionEvent) {
131+
received = append(received, event)
132+
})
133+
134+
session.dispatchEvent(SessionEvent{Type: "session.shutdown"})
135+
136+
if len(received) != 1 {
137+
t.Fatalf("Expected 1 event, got %d", len(received))
138+
}
139+
if received[0].Type != "session.shutdown" {
140+
t.Errorf("Expected session.shutdown event, got %s", received[0].Type)
141+
}
142+
})
143+
144+
t.Run("handlers still active after shutdown flag set", func(t *testing.T) {
145+
session := &Session{
146+
handlers: make([]sessionHandler, 0),
147+
}
148+
149+
var received []SessionEvent
150+
session.On(func(event SessionEvent) {
151+
received = append(received, event)
152+
})
153+
154+
// Simulate what Shutdown() does: set the flag
155+
session.isShutdown.Store(true)
156+
157+
// Handlers should still be active — Shutdown does not clear them
158+
session.dispatchEvent(SessionEvent{Type: "session.shutdown"})
159+
160+
if len(received) != 1 {
161+
t.Fatalf("Expected 1 event after shutdown, got %d", len(received))
162+
}
163+
if received[0].Type != "session.shutdown" {
164+
t.Errorf("Expected session.shutdown, got %s", received[0].Type)
165+
}
166+
})
167+
168+
t.Run("shutdown idempotency via atomic flag", func(t *testing.T) {
169+
session := &Session{
170+
handlers: make([]sessionHandler, 0),
171+
}
172+
173+
// First swap should return false (was not shut down)
174+
if session.isShutdown.Swap(true) {
175+
t.Error("Expected first Swap to return false")
176+
}
177+
178+
// Second swap should return true (already shut down)
179+
if !session.isShutdown.Swap(true) {
180+
t.Error("Expected second Swap to return true")
181+
}
182+
})
183+
184+
t.Run("disconnect clears handlers", func(t *testing.T) {
185+
session := &Session{
186+
handlers: make([]sessionHandler, 0),
187+
toolHandlers: make(map[string]ToolHandler),
188+
}
189+
190+
var count int
191+
session.On(func(event SessionEvent) { count++ })
192+
193+
// Dispatch before disconnect — handler should fire
194+
session.dispatchEvent(SessionEvent{Type: "test"})
195+
if count != 1 {
196+
t.Fatalf("Expected 1 event before disconnect, got %d", count)
197+
}
198+
199+
// Simulate Disconnect's handler-clearing logic
200+
session.handlerMutex.Lock()
201+
session.handlers = nil
202+
session.handlerMutex.Unlock()
203+
204+
session.toolHandlersM.Lock()
205+
session.toolHandlers = nil
206+
session.toolHandlersM.Unlock()
207+
208+
session.permissionMux.Lock()
209+
session.permissionHandler = nil
210+
session.permissionMux.Unlock()
211+
212+
// Dispatch after disconnect — handler should NOT fire
213+
session.dispatchEvent(SessionEvent{Type: "test"})
214+
if count != 1 {
215+
t.Errorf("Expected no additional events after disconnect, got %d total", count)
216+
}
217+
})
218+
219+
t.Run("two-phase shutdown then disconnect preserves notification", func(t *testing.T) {
220+
session := &Session{
221+
handlers: make([]sessionHandler, 0),
222+
}
223+
224+
var events []string
225+
session.On(func(event SessionEvent) {
226+
events = append(events, string(event.Type))
227+
})
228+
229+
// Phase 1: Shutdown sends the RPC (simulated) — handlers still active
230+
session.isShutdown.Store(true)
231+
232+
// Server sends back shutdown notification — handler receives it
233+
session.dispatchEvent(SessionEvent{Type: "session.shutdown"})
234+
235+
// Phase 2: Clear handlers (simulating Disconnect)
236+
session.handlerMutex.Lock()
237+
session.handlers = nil
238+
session.handlerMutex.Unlock()
239+
240+
// Any further events should not reach handlers
241+
session.dispatchEvent(SessionEvent{Type: "should.not.arrive"})
242+
243+
if len(events) != 1 {
244+
t.Fatalf("Expected exactly 1 event, got %d: %v", len(events), events)
245+
}
246+
if events[0] != "session.shutdown" {
247+
t.Errorf("Expected session.shutdown, got %s", events[0])
248+
}
249+
})
250+
}

nodejs/package-lock.json

Lines changed: 28 additions & 28 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

nodejs/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
"author": "GitHub",
4545
"license": "MIT",
4646
"dependencies": {
47-
"@github/copilot": "^1.0.3-0",
47+
"@github/copilot": "^1.0.3",
4848
"vscode-jsonrpc": "^8.2.1",
4949
"zod": "^4.3.6"
5050
},

nodejs/test/e2e/session.test.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,9 +299,16 @@ describe("Sessions", async () => {
299299
it("should receive session events", async () => {
300300
const session = await client.createSession({ onPermissionRequest: approveAll });
301301
const receivedEvents: Array<{ type: string }> = [];
302+
let resolveShutdown: () => void;
303+
const shutdownReceived = new Promise<void>((resolve) => {
304+
resolveShutdown = resolve;
305+
});
302306

303307
session.on((event) => {
304308
receivedEvents.push(event);
309+
if (event.type === "session.shutdown") {
310+
resolveShutdown();
311+
}
305312
});
306313

307314
// Send a message and wait for completion
@@ -316,8 +323,15 @@ describe("Sessions", async () => {
316323
// Verify the assistant response contains the expected answer
317324
expect(assistantMessage?.data.content).toContain("300");
318325

319-
// Shut down session (sends RPC without clearing handlers), then destroy
326+
// Shut down session and verify shutdown event is received
320327
await session.shutdown();
328+
await Promise.race([
329+
shutdownReceived,
330+
new Promise((_, reject) =>
331+
setTimeout(() => reject(new Error("Timed out waiting for session.shutdown")), 5000)
332+
),
333+
]);
334+
expect(receivedEvents.some((e) => e.type === "session.shutdown")).toBe(true);
321335
await session.destroy();
322336
});
323337

python/e2e/test_session.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -455,11 +455,14 @@ async def test_should_receive_session_events(self, ctx: E2ETestContext):
455455
)
456456
received_events = []
457457
idle_event = asyncio.Event()
458+
shutdown_event = asyncio.Event()
458459

459460
def on_event(event):
460461
received_events.append(event)
461462
if event.type.value == "session.idle":
462463
idle_event.set()
464+
elif event.type.value == "session.shutdown":
465+
shutdown_event.set()
463466

464467
session.on(on_event)
465468

@@ -483,8 +486,14 @@ def on_event(event):
483486
assistant_message = await get_final_assistant_message(session)
484487
assert "300" in assistant_message.data.content
485488

486-
# Shut down session (sends RPC without clearing handlers), then destroy
489+
# Shut down session and verify shutdown event is received
487490
await session.shutdown()
491+
try:
492+
await asyncio.wait_for(shutdown_event.wait(), timeout=5)
493+
except TimeoutError:
494+
pytest.fail("Timed out waiting for session.shutdown")
495+
event_types = [e.type.value for e in received_events]
496+
assert "session.shutdown" in event_types
488497
await session.destroy()
489498

490499
async def test_should_create_session_with_custom_config_dir(self, ctx: E2ETestContext):

0 commit comments

Comments
 (0)