dotnet: add CreateCloudSessionAsync#1399
Closed
tclem wants to merge 2 commits into
Closed
Conversation
Port CreateCloudSessionAsync from the Rust (PR #1394) and TypeScript (PR #1395) SDKs. - CreateCloudSessionAsync sends session.create without a caller-supplied sessionId, so the cloud server assigns one. The server-assigned sessionId is set on the returned session object alongside a new RemoteUrl property. - CreateSessionAsync now rejects configs that include a Cloud block; callers must use CreateCloudSessionAsync for cloud sessions. - A pending-routing buffer handles the window between sending session.create and receiving the response: incoming session.event notifications are buffered per-sessionId (bounded at 128 with drop-oldest semantics) and replayed once the session is registered; inbound RPC requests (userInput.request, permission.request, etc.) park on a TaskCompletionSource and are unblocked atomically when the session is registered. - Added RemoteUrl property to CopilotSession. - 9 unit tests cover the core behaviours: serialization, guard/buffer mechanics, early notification replay, and inbound request parking. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Carries forward the Rust SDK PR #1394 follow-up review feedback into the .NET port: 1. Cap the per-session inbound-request parked-waiter list at 128. When exceeded, reject the oldest waiter with 'pending session buffer overflow'. The custom JsonRpc dispatcher translates the thrown exception into a JSON-RPC error response (-32603) back to the runtime so the request id isn't left hanging — silently dropping it would leave the runtime waiting on the response until its own timeout. Mirrors Rust commit 491b442 and TS commit c167bc3. 2. Use a distinct message ('pending session routing ended before session was registered') when the pending guard drops without registration. Lets debugging tell the overflow path from the create-failed path. Also fixes the pre-existing bug where the waiter-fault loop ran inside _pendingLock despite the comment saying otherwise — moved it outside the lock so TCS continuations can't deadlock against the lock. Mirrors Rust commit e0ff254. Adds two tests: - overflow path: 129 early inbound requests → oldest gets error with 'pending session buffer overflow', remaining 128 resolve after registration - guard-drop path: session.create fails with parked request → error response with 'pending session routing ended before session was registered' Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Cross-SDK Consistency Review 🔍This PR correctly adds Current state across SDKs
What Python and Go are missingBoth Python and Go will need follow-up PRs to reach parity:
No blocking issues with this PR itself — the .NET implementation is consistent with the established pattern. Just flagging the remaining gap so follow-up work can be tracked.
|
Member
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports
CreateCloudSessionAsyncto the .NET SDK, following the Rust (#1394) and TypeScript (#1395) implementations as the reference contract.What changes
CreateCloudSessionAsync(CloudSessionConfig config)— new method onCopilotClient:session.createwithout a caller-suppliedsessionId; the cloud server assigns one.session.SessionIdandsession.RemoteUrl(newCopilotSessionproperty) from the server response.CreateSessionAsyncnow throwsArgumentExceptionif aCloudblock is present, preventing accidental misconfiguration.Pending-routing buffer — handles the window between sending
session.createand receiving the server's response:session.eventnotifications for the not-yet-registered session ID are buffered per-session (bounded at 128, drop-oldest) and replayed atomically when the session is registered viaFlushPendingForSession.userInput.request,permission.request, hooks, etc.) park on aTaskCompletionSourceinResolveSessionAsyncand are unblocked when the session is registered._pendingRoutingCounttracks how many cloud-create calls are in-flight; when all guards are released without registration, parked waiters are faulted.BeginPendingSessionRouting/EndPendingSessionRouting/PendingSessionRoutingGuardmanage the lifetime. All five existingRpcHandlerinbound request handlers now callResolveSessionAsyncinstead ofGetSession.Tests — 9 new unit tests in
CloudSessionTests.cs:sessionIdis omitted from wire JSON;CloudSessionOptionsround-trips correctly.CreateSessionAsyncrejects cloud config;CreateCloudSessionAsyncrequires cloud config and rejects caller-supplied IDs and provider overrides.session.createomitssessionIdand includescloudblock;session.SessionIdandRemoteUrlare populated from the response.session.eventnotifications are replayed after registration.userInput.requestarriving before registration is buffered and answered after the session is registered.