TS, Python, Go, .NET, Java SDKs: add createCloudSession#1395
Draft
tclem wants to merge 18 commits into
Draft
Conversation
- CreateSession now rejects config.Cloud with an error pointing to CreateCloudSession - CreateCloudSession validates config (requires Cloud, rejects caller-supplied SessionID and Provider), omits sessionId from the session.create wire payload so the runtime assigns one - Pending-routing support: a refcounted beginPendingSessionRouting guard buffers session.event notifications (bounded drop-oldest, 128 per id) and parks inbound request handlers (userInput.request, exitPlanMode.request, autoModeSwitch.request, hooks.invoke, systemMessage.transform) until the runtime-assigned session id is registered; waiters are rejected with a clear error if pending mode ends without registration - TOCTOU race fixed: after acquiring pending.mu, both handleSessionEvent and waitForSession re-check c.sessions so a notification/request that races with flushPendingForSession is dispatched directly rather than buffered and abandoned - Waiter buffer is also bounded at 128; oldest waiter is rejected on overflow - README and inline godoc updated Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Port create_cloud_session to the Python SDK, following the Rust (PR #1394) and TypeScript (PR #1395) reference implementations. Key additions: - CopilotClient.create_cloud_session(): creates a Mission Control-backed cloud session. The runtime owns the session ID (omitting sessionId from the wire payload); the caller must not set session_id or provider. - Pending-routing infrastructure: session.event notifications and inbound JSON-RPC requests that arrive before the session is fully registered are buffered (up to _PENDING_SESSION_BUFFER_LIMIT = 128) and replayed once the session is ready. _PendingSessionRoutingGuard + _begin_pending_session_routing + _flush_pending_for_session + _resolve_session implement this. - CopilotSession.remote_url property: exposes the remoteUrl returned in the session.create response for cloud sessions. - create_session() now rejects cloud= being set; callers must use create_cloud_session() instead. - test_cloud_session.py: 7 new unit tests covering wire shape, validation, early notification buffering, and parked inbound request handling. - Updated test_client.py: test_create_session_forwards_cloud_options updated to test create_cloud_session instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add createCloudSession(SessionConfig) to CopilotClient that sends a session.create request without a caller-supplied sessionId, letting the runtime assign one (cloud-managed routing) - Add PendingRoutingState: thread-safe buffer for events and parked request futures that arrive before cloud session registration; bounded at 128 entries per id, drop-oldest; flush on registerAndFlush() - Add SessionRequestBuilder.buildCloudCreateRequest that omits sessionId and provider from the wire payload - Update RpcHandlerDispatcher to accept PendingRoutingState and route all inbound server requests (tool.call, permission, user-input, exit-plan-mode, auto-mode-switch, hooks.invoke, system-message-transform) through resolveSessionForRequest so they park on the pending state when the session is still in flight - Guard createSession against being called with cloud config; guard createCloudSession against caller-supplied sessionId or provider - Preserve existing non-cloud rekey-on-mismatch logic unchanged - 10 new tests in CloudSessionTest covering all 7 contract scenarios; fix RpcHandlerDispatcherTest constructor call for new signature Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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>
Two symmetric fixes ported from Rust SDK (commits 491b442, e0ff254) and TS SDK (commit c167bc3, PR #1395): 1. Pending request buffer overflow: when _resolve_session would push the waiter list past _PENDING_SESSION_BUFFER_LIMIT (128), evict the oldest future and call set_exception(ValueError('pending session buffer overflow')). The JSON-RPC dispatch layer (_dispatch_request) catches the raised exception and sends a proper -32603 error response so the runtime doesn't hang on the request id. 2. Guard drop without registration: _PendingSessionRoutingGuard.dispose() already called set_exception on parked waiters, but used a generic message. Changed to the cross-SDK canonical message 'pending session routing ended before session was registered' so the two failure paths are distinguishable in logs and error messages. Notifications retain warn-and-drop-oldest behaviour (no response needed — they carry no id). Add two unit tests: - TestPendingRequestBufferOverflow: 129 concurrent waiters; oldest raises with the overflow message; remaining 128 resolve after registration. - TestPendingRequestGuardDropWithoutRegistration: session.create fails; parked request raises with the routing-ended message. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Carries the Rust SDK PR #1394 follow-up review fixes into the Go port: 1. Cap the per-session parked-waiter list at 128. When exceeded, reject the oldest waiter with errPendingSessionBufferOverflow ('pending session buffer overflow'). The handler returns a *jsonrpc2.Error with code -32603 via the new pendingRoutingRPCError helper, so the runtime receives a proper error response instead of hanging on the request id until its own timeout. Mirrors Rust commit 491b442 and TS commit c167bc3. 2. When the last pending-routing guard drops without RegisterSession (e.g. session.create failed mid-RPC), signal all parked waiters with errPendingSessionRoutingEnded ('pending session routing ended before session was registered'). Distinct phrasing from the overflow path so debugging can tell the two cases apart. Mirrors Rust commit e0ff254 and TS commit c167bc3. Adds pendingRoutingRPCError helper that routes sentinel errors to -32603 while unknown-session errors keep -32602. Adds two tests: - TestPendingRouting_OverflowEmitsError: 129 parked waiters, oldest gets -32603 overflow error, remaining 128 resolve normally after registration. - TestPendingRouting_GuardDropDistinctMessage: parks a request, drops the guard without registration, verifies exact routing-ended message and -32603 code. Updates TestPendingRouting_RejectsWaitersOnDispose to assert the new exact message and code instead of the old 'dropped' substring check. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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>
Carries forward the Rust SDK PR #1394 review feedback into the Java port: 1. Cap the per-session inbound-request parked-waiter list at 128 (BUFFER_LIMIT). When exceeded, evict the oldest waiter via completeExceptionally("pending session buffer overflow"). The RpcHandlerDispatcher thread blocked in waiter.get() wakes up, catches ExecutionException, and resolveSessionForRequest calls rpc.sendErrorResponse(-32603, ...) so the runtime isn't left waiting on a request id that will never get a reply. Mirrors Rust commit 491b442 and TS commit c167bc3 on PR #1395. 2. decrementGuard now completes all stale waiters internally with a distinct message ("pending session routing ended before session was registered") instead of returning them for callers to complete with ad-hoc strings. A single canonical message lets debugging tell the overflow-eviction path from the create-failed path. Mirrors Rust commit e0ff254 and TS commit c167bc3. 3. Fix isDone() fast path in resolveSessionForRequest: the existing catch-all "fall through" swallowed ExecutionException from an overflow-evicted waiter, sending a generic -32602 "Unknown session" error instead of -32603. Now explicitly catches ExecutionException in the isDone() branch and sends the same -32603 error as the blocking path. Adds two new unit tests in CloudSessionTest: - parkedRequestWaiterBuffer_overflow_evictsOldestWithOverflowMessage: parks 129 waiters, verifies oldest completes with "pending session buffer overflow", remaining 128 resolve normally after registration. - parkedRequestWaiter_guardDropMessage_isDistinctFromOverflowMessage: parks a request via the full handler path, drops the guard without registration, verifies the JSON-RPC error response contains "routing ended before session was registered" but not "buffer overflow". Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Ports the cloud-session contract from the Rust SDK (PR #1394) to the TypeScript SDK. - createSession({ cloud }) now throws — the runtime rejects this shape, and the existing path silently sent cloud + sessionId which RemoteSessionManager.cloud() rejects with a generic error. - New createCloudSession(config) omits sessionId from the wire payload, lets the runtime assign the Mission Control session id, and registers the resulting session under that id. - Pending-routing buffer (bounded, drop-oldest) holds session.event notifications and inbound JSON-RPC request handlers (userInput / exitPlanMode / autoModeSwitch / hooks / systemMessage) that arrive while session.create is in flight, replaying them after the runtime-assigned id is registered. - Rejection guards for caller-provided sessionId, caller-provided provider, and missing cloud config. - Post-response setup is wrapped in try/catch so a setupSessionFs failure rolls back the partial registration and disposes the pending-routing guard. Inbound sessionFs.* requests still use the generated synchronous handler shim and are not pending-buffered. This is unlikely in practice (runtime does not initiate sessionFs before the create response) and is documented as a known limitation; the fix is a codegen update to make registerClientSessionApiHandlers accept an async session resolver. Tests cover wire-shape parity with Rust, rejection guards, early notification buffering, and early inbound-request parking. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Carries forward the Rust SDK PR #1394 follow-up review feedback into the TS port: 1. Cap the per-session inbound-request parked-waiter list at 128. When exceeded, reject the oldest waiter with 'pending session buffer overflow'. vscode-jsonrpc translates the rejection 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. 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. Mirrors Rust commit e0ff254. Adds two tests: - overflow path resolves to overflow error for the oldest waiter, remaining 128 resolve normally after registration - guard-drop path rejects parked waiters with the distinct message
tclem
force-pushed
the
tclem/tclem-ts-create-cloud-session
branch
from
c167bc3 to
f4752b6
Compare
tclem
changed the title
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Generated by SDK Consistency Review Agent for issue #1395 · ● 22.4M
Python (session.remote_url) and .NET (Session.RemoteUrl) already exposed the runtime-assigned remote URL returned from session.create; this brings the TypeScript, Go, and Java SDKs to parity per the sdk-consistency-review feedback on PR #1395. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Apply Spotless formatting after adding the remoteUrl record component. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
SDK Consistency Review ✅This PR adds
The two documented known limitations ( No consistency issues found. 🎉
|
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 the Rust SDK's
Client::create_cloud_sessioncontract (PR #1394) to the remaining five SDKs: TypeScript, Python, Go, .NET, and Java.What each SDK gains
A new entry point —
createCloudSession/create_cloud_session/CreateCloudSession/CreateCloudSessionAsync— that creates a cloud-managed session whose ID is assigned by the runtime. Caller-suppliedsessionIdorprovideris rejected up front.session.createis sent over the wire without asessionIdfield. The non-cloudcreateSessionpath now rejectscloudconfigs so the two paths are clearly separated. Java's existing rekey-on-mismatch behavior oncreateSessionis preserved for the non-cloud path (potentially load-bearing for older CLIs); the new contract applies tocreateCloudSessiononly.The pending-routing buffer
Because the runtime may emit
session.eventnotifications and inbound JSON-RPC requests addressed to the new session ID before thesession.createresponse returns, each SDK now buffers/parks those messages while a cloud create is in flight and flushes them into the registered session afterwards. Per-session cap is 128 in every SDK. Cross-SDK invariants:"pending session buffer overflow"; the SDK's JSON-RPC layer converts the rejection into a-32603error response so the runtime isn't left hanging on the request id.session.createfailed mid-RPC), all parked request waiters are rejected with the distinct message"pending session routing ended before session was registered"— separate phrasing so the two failure modes can be told apart in logs.Doc rule
Adds a "cross-SDK features ship in a single PR" rule to
.github/copilot-instructions.md. Thesdk-consistency-reviewworkflow auto-reviews any PR touchingnodejs/,python/,go/, ordotnet/, so splitting per-language guarantees a "missing in N languages" flag on each.Tests
Per SDK, mirrors of the six Rust integration tests plus the two follow-up overflow/guard-drop tests (8 each, roughly; exact count varies by language harness shape). All unit suites pass locally.
Known limitations
sessionFs.*handlers innodejs/src/generated/rpc.tsuse a synchronousgetHandlers(sessionId)callback, so earlysessionFs.*requests will throw immediately rather than being parked. Documented inline; proper fix requires a codegen update.selectonctx.Done()while parked inwaitForSession(pre-existing behavior in the branch).Related runtime issues
sessionId/providerwith cloud.session.createresponse. If accepted, the entire pending-routing mechanism in this PR can be deleted across all SDKs.