Skip to content

python: add create_cloud_session#1397

Closed
tclem wants to merge 2 commits into
tclem/verbose-memefrom
tclem/python-create-cloud-session
Closed

python: add create_cloud_session#1397
tclem wants to merge 2 commits into
tclem/verbose-memefrom
tclem/python-create-cloud-session

Conversation

@tclem
Copy link
Copy Markdown
Member

@tclem tclem commented May 23, 2026

Stacked on #1394 — will be retargeted to main when that merges.

Ports create_cloud_session to the Python SDK, following the Rust and TypeScript reference implementations.

What changed

python/copilot/client.py

  • create_cloud_session(): new method for Mission Control–backed cloud sessions. The runtime owns the session ID so sessionId is omitted from the session.create wire payload; caller must not set session_id or provider (both raise ValueError).
  • create_session() now rejects cloud= being set — callers must use create_cloud_session() instead.
  • Pending-routing infrastructure: _PendingSessionRoutingGuard, _begin_pending_session_routing(), _flush_pending_for_session(), _resolve_session() buffer session.event notifications and park inbound JSON-RPC requests (up to _PENDING_SESSION_BUFFER_LIMIT = 128) while a cloud session create is in flight, then replay them once the session is registered.
  • Both notification handler paths (stdio and TCP) updated to participate in pending routing.
  • All 5 inbound request handlers updated to go through _resolve_session().

python/copilot/session.py

  • CopilotSession.remote_url property exposing the remoteUrl from the session.create response.

python/test_cloud_session.py — 7 new unit tests: wire shape, session_id/provider rejection, cloud= required, early notification buffering, parked inbound request handling.

python/test_client.py — updated test_create_cloud_session_forwards_cloud_options to call create_cloud_session instead of create_session(cloud=...).

python/README.md — added ## Cloud Sessions section.

  Generated via Copilot (Claude Sonnet 4.5) on behalf of @tclem

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>
@github-actions

This comment has been minimized.

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>
@github-actions
Copy link
Copy Markdown
Contributor

Cross-SDK Consistency Review ✅

This PR correctly ports create_cloud_session from the Rust reference implementation (PR #1394) to Python, following the same API pattern and semantics.

Python ↔ Rust alignment ✅

Aspect Rust Python (this PR)
Separate method Client::create_cloud_session create_cloud_session()
create_session rejects cloud= Error::InvalidConfig pointing at create_cloud_session ValueError pointing at create_cloud_session
Caller session_id rejected
Caller provider rejected
cloud param required
Pre-registration buffering PendingSessionRouting guard, 128-item ring _PendingSessionRoutingGuard, _PENDING_SESSION_BUFFER_LIMIT = 128
remote_url exposed on session via Session::remote_url() via CopilotSession.remote_url property

Remaining SDK gaps ⚠️

As noted in the #1394 PR description, the following SDKs still need createCloudSession / CreateCloudSession:

  • Node.js/TypeScript — currently still accepts cloud via createSession({ cloud: ... }) which sends a caller-provided sessionId that the runtime rejects. This is the most urgent follow-up.
  • Go — still uses CreateSession with config.Cloud
  • .NET — still uses CreateSessionAsync with config.Cloud
  • Java — not yet implemented

These are known follow-ups from #1394. This PR is not introducing new inconsistencies — it's reducing the gap.

Generated by SDK Consistency Review Agent for issue #1397 · ● 5.3M ·

@tclem
Copy link
Copy Markdown
Member Author

tclem commented May 23, 2026

Consolidating into #1395 so the sdk-consistency-review workflow sees the full cross-SDK matrix in one PR. Commits from this branch are merged into that PR's branch.

  Generated via Copilot (Claude Opus 4.7) on behalf of @tclem

@tclem tclem closed this May 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant