fix(dotnet): recover from dropped session idle events - #2016
Conversation
11e23c4 to
57c96dd
Compare
There was a problem hiding this comment.
Pull request overview
Adds a resilient .NET SendAndWaitAsync fallback when ephemeral idle notifications are dropped.
Changes:
- Uses runtime task-drain and activity RPCs to confirm completion.
- Adds FIFO event-dispatch barriers and stale-activity protection.
- Adds unit and E2E regression coverage.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
dotnet/src/Session.cs |
Implements runtime completion fallback and dispatch barriers. |
dotnet/test/Unit/ClientSessionLifetimeTests.cs |
Tests dropped-idle recovery and activity races. |
dotnet/test/E2E/SendAndWaitReliabilityE2ETests.cs |
Adds end-to-end reliability coverage. |
test/snapshots/send_and_wait_reliability/should_complete_when_live_sessionidle_is_dropped.yaml |
Provides the replay fixture. |
eb53c32 to
de29350
Compare
47f3640 to
998f42d
Compare
|
Thanks for the careful work here, particularly around FIFO handler completion and the activity races. Before taking this fix, we need to establish that it matches a real failure mode. The current tests deliberately omit This PR specifically changes A small real-runtime reproduction where:
would be extremely useful. It does not need to reproduce Visual Studio itself—a minimal .NET console application would be ideal. With that evidence, we could evaluate this fallback against the actual failure and determine whether it belongs in .NET, the runtime, or all SDKs. For now, I think we should hold this PR rather than merge a substantial recovery mechanism for a simulated-but-unconfirmed failure mode. |
|
Thanks — agreed. The current evidence proves the fallback's race behavior under a deliberately dropped notification, but it does not establish that a healthy live runtime can actually lose I don't have the real-runtime reproduction you need, so I won't extend or push the recovery mechanism as if #2008 confirmed this failure mode. I'll keep the PR on hold while the resumed-session/subscription path is investigated; if I can produce a minimal console case where the preceding events arrive, |
Summary
SendAndWaitAsyncrecover when the ephemeralsession.idlenotification is droppedReproduction
assistant.turn_endsession.idlestill completes the callTests
$HOME/.dotnet/dotnet test test/GitHub.Copilot.SDK.Test.csproj -f net8.0 --no-restore --filter 'FullyQualifiedName~SendAndWaitAsync_Completes_When_SessionIdle_Notification_Is_Dropped|FullyQualifiedName~SendAndWaitAsync_Preserves_Event_Completion_When_Legacy_Runtime_Lacks_Fallback_Rpcs|FullyQualifiedName~SendAndWaitAsync_DroppedIdle_Fallback_Flushes_Preceding_Event_Handlers|FullyQualifiedName~SendAndWaitAsync_DroppedIdle_Fallback_Rechecks_Activity_After_Final_Barrier'— 4 passed$HOME/.dotnet/dotnet test test/GitHub.Copilot.SDK.Test.csproj -f net8.0 --no-restore— 690 passed, 3 skipped$HOME/.dotnet/dotnet build src/GitHub.Copilot.SDK.csproj --no-restore— 0 warnings, 0 errors$HOME/.dotnet/dotnet format --verify-no-changes --no-restoregit diff --checkFixes #2008