From e7734f8f1f50bf28f5cfd80487a2ff5aa82e6cf3 Mon Sep 17 00:00:00 2001 From: Timothy Clem Date: Fri, 22 May 2026 19:25:01 -0700 Subject: [PATCH 01/14] Go SDK: add Client.CreateCloudSession - 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> --- go/README.md | 24 +++ go/client.go | 397 ++++++++++++++++++++++++++++++++++++--- go/cloud_session_test.go | 329 ++++++++++++++++++++++++++++++++ 3 files changed, 724 insertions(+), 26 deletions(-) create mode 100644 go/cloud_session_test.go diff --git a/go/README.md b/go/README.md index da77033f8..62f3b9198 100644 --- a/go/README.md +++ b/go/README.md @@ -105,6 +105,7 @@ That's it! When your application calls `copilot.NewClient` without a `Connection - `Stop() error` - Stop the CLI server - `ForceStop()` - Forcefully stop without graceful cleanup - `CreateSession(config *SessionConfig) (*Session, error)` - Create a new session +- `CreateCloudSession(ctx context.Context, config *SessionConfig) (*Session, error)` - Create a Mission Control–backed cloud session - `ResumeSession(sessionID string, config *ResumeSessionConfig) (*Session, error)` - Resume an existing session - `ResumeSessionWithOptions(sessionID string, config *ResumeSessionConfig) (*Session, error)` - Resume with additional configuration - `ListSessions(filter *SessionListFilter) ([]SessionMetadata, error)` - List sessions (with optional filter) @@ -170,6 +171,8 @@ Event types: `SessionLifecycleCreated`, `SessionLifecycleDeleted`, `SessionLifec - `Commands` ([]CommandDefinition): Slash-commands registered for this session. See [Commands](#commands) section. - `OnElicitationRequest` (ElicitationHandler): Handler for elicitation requests from the server. See [Elicitation Requests](#elicitation-requests-serverclient) section. +- `Cloud` (\*CloudSessionOptions): Cloud session configuration. When set, `CreateSession` rejects the config; use `CreateCloudSession` instead. Do not set `SessionID` or `Provider` when using cloud sessions. + **ResumeSessionConfig:** - `OnPermissionRequest` (PermissionHandlerFunc): Optional handler called before each tool execution to approve or deny it. See [Permission Handling](#permission-handling) section. @@ -487,6 +490,27 @@ When enabled, sessions emit compaction events: - `session.compaction_start` - Background compaction started - `session.compaction_complete` - Compaction finished (includes token counts) +## Cloud Sessions + +`CreateCloudSession` creates a Mission Control–backed cloud session. The runtime assigns the session ID; do not set `SessionID` or `Provider` on the config (the SDK rejects both). `CreateSession` also rejects any config that has `Cloud` set. + +Any `session.event` notifications or inbound JSON-RPC requests that arrive between sending `session.create` and receiving its response are buffered (bounded, drop-oldest, limit 128 per id) and replayed once the runtime-assigned session ID is registered. + +```go +session, err := client.CreateCloudSession(context.Background(), &copilot.SessionConfig{ + OnPermissionRequest: copilot.PermissionHandler.ApproveAll, + Cloud: &copilot.CloudSessionOptions{ + Repository: &copilot.CloudSessionRepository{ + Owner: "github", Name: "copilot-sdk", + }, + }, +}) +if err != nil { + log.Fatal(err) +} +fmt.Println("cloud session id:", session.SessionID) +``` + ## Custom Providers The SDK supports custom OpenAI-compatible API providers (BYOK - Bring Your Own Key), including local providers like Ollama. When using a custom provider, you must specify the `Model` explicitly. diff --git a/go/client.go b/go/client.go index 9491eb199..0f5db2419 100644 --- a/go/client.go +++ b/go/client.go @@ -87,6 +87,27 @@ func validateSessionFsConfig(config *SessionFsConfig) error { // log.Fatal(err) // } // defer client.Stop() +// +// pendingResult carries the outcome of a parked inbound-request session lookup. +type pendingResult struct { + session *Session + err error +} + +// pendingRouting buffers session.event notifications and parks inbound request +// handlers that arrive before a cloud session.create response is received. +// A refcount tracks how many cloud creates are in flight; when it reaches zero +// the buffers are cleared and parked handlers are rejected. +type pendingRouting struct { + mu sync.Mutex + count int + events map[string][]sessionEventRequest + waiters map[string][]chan pendingResult +} + +// pendingSessionBufferLimit caps buffered notifications per in-flight session id. +const pendingSessionBufferLimit = 128 + type Client struct { options ClientOptions process *exec.Cmd @@ -121,6 +142,10 @@ type Client struct { effectiveConnectionToken string onListModels func(ctx context.Context) ([]ModelInfo, error) + // pending buffers traffic that arrives between session.create being sent and + // the response for cloud sessions. + pending pendingRouting + // RPC provides typed server-scoped RPC methods. // This field is nil until the client is connected via Start(). RPC *rpc.ServerRpc @@ -162,6 +187,10 @@ func NewClient(options *ClientOptions) *Client { actualHost: "localhost", isExternalServer: false, useStdio: true, + pending: pendingRouting{ + events: make(map[string][]sessionEventRequest), + waiters: make(map[string][]chan pendingResult), + }, } if options != nil { @@ -593,6 +622,10 @@ func (c *Client) CreateSession(ctx context.Context, config *SessionConfig) (*Ses config = &SessionConfig{} } + if config.Cloud != nil { + return nil, fmt.Errorf("CreateSession does not support cloud sessions; use CreateCloudSession instead") + } + if err := c.ensureConnected(ctx); err != nil { return nil, err } @@ -754,6 +787,307 @@ func (c *Client) CreateSession(ctx context.Context, config *SessionConfig) (*Ses return session, nil } +// CreateCloudSession creates a Mission Control–backed cloud session. +// +// The runtime owns the session ID for cloud sessions: do not set SessionID or +// Provider on the config (the SDK rejects both). Build the config with Cloud +// set to a [CloudSessionOptions] value; [Client.CreateSession] rejects any +// config that has Cloud set. +// +// Any session.event notifications or inbound JSON-RPC requests that arrive +// between sending session.create and receiving its response are buffered +// (bounded, drop-oldest, limit 128 per id) and replayed once the +// runtime-assigned session ID is registered. +// +// Known limitation: inbound sessionFs.* requests from the generated +// client-session API handlers are not pending-buffered. In practice the +// runtime does not initiate sessionFs.* calls before the session.create +// response, so this is theoretical. +// +// Example: +// +// session, err := client.CreateCloudSession(context.Background(), &copilot.SessionConfig{ +// OnPermissionRequest: copilot.PermissionHandler.ApproveAll, +// Cloud: &copilot.CloudSessionOptions{ +// Repository: &copilot.CloudSessionRepository{ +// Owner: "github", Name: "copilot-sdk", +// }, +// }, +// }) +func (c *Client) CreateCloudSession(ctx context.Context, config *SessionConfig) (*Session, error) { + if config == nil { + config = &SessionConfig{} + } + + if config.Cloud == nil { + return nil, fmt.Errorf("CreateCloudSession requires config.Cloud to be set") + } + if config.SessionID != "" { + return nil, fmt.Errorf("CreateCloudSession does not accept a caller-provided SessionID; the runtime assigns one") + } + if config.Provider != nil { + return nil, fmt.Errorf("CreateCloudSession does not accept config.Provider; cloud sessions use the runtime's provider") + } + + if err := c.ensureConnected(ctx); err != nil { + return nil, err + } + + req := createSessionRequest{} + req.Model = config.Model + req.ClientName = config.ClientName + req.ReasoningEffort = config.ReasoningEffort + req.ConfigDir = config.ConfigDir + if config.EnableConfigDiscovery { + req.EnableConfigDiscovery = Bool(true) + } + req.Tools = config.Tools + wireSystemMessage, transformCallbacks := extractTransformCallbacks(config.SystemMessage) + req.SystemMessage = wireSystemMessage + req.AvailableTools = config.AvailableTools + req.ExcludedTools = config.ExcludedTools + req.EnableSessionTelemetry = config.EnableSessionTelemetry + req.ModelCapabilities = config.ModelCapabilities + req.WorkingDirectory = config.WorkingDirectory + req.MCPServers = config.MCPServers + req.EnvValueMode = "direct" + req.CustomAgents = config.CustomAgents + req.DefaultAgent = config.DefaultAgent + req.Agent = config.Agent + req.SkillDirectories = config.SkillDirectories + req.InstructionDirectories = config.InstructionDirectories + req.DisabledSkills = config.DisabledSkills + req.InfiniteSessions = config.InfiniteSessions + req.GitHubToken = config.GitHubToken + req.RemoteSession = config.RemoteSession + req.Cloud = config.Cloud + // SessionID intentionally omitted: the runtime assigns the id for cloud sessions. + + if len(config.Commands) > 0 { + cmds := make([]wireCommand, 0, len(config.Commands)) + for _, cmd := range config.Commands { + cmds = append(cmds, wireCommand{Name: cmd.Name, Description: cmd.Description}) + } + req.Commands = cmds + } + if config.OnElicitationRequest != nil { + req.RequestElicitation = Bool(true) + } + if config.OnExitPlanModeRequest != nil { + req.RequestExitPlanMode = Bool(true) + } + if config.OnAutoModeSwitchRequest != nil { + req.RequestAutoModeSwitch = Bool(true) + } + if config.Streaming != nil { + req.Streaming = config.Streaming + } + if config.IncludeSubAgentStreamingEvents != nil { + req.IncludeSubAgentStreamingEvents = config.IncludeSubAgentStreamingEvents + } else { + req.IncludeSubAgentStreamingEvents = Bool(true) + } + if config.OnUserInputRequest != nil { + req.RequestUserInput = Bool(true) + } + if config.Hooks != nil && (config.Hooks.OnPreToolUse != nil || + config.Hooks.OnPreMcpToolCall != nil || + config.Hooks.OnPostToolUse != nil || + config.Hooks.OnUserPromptSubmitted != nil || + config.Hooks.OnSessionStart != nil || + config.Hooks.OnSessionEnd != nil || + config.Hooks.OnErrorOccurred != nil) { + req.Hooks = Bool(true) + } + if config.OnPermissionRequest != nil { + req.RequestPermission = Bool(true) + } + + traceparent, tracestate := getTraceContext(ctx) + req.Traceparent = traceparent + req.Tracestate = tracestate + + dispose := c.beginPendingSessionRouting() + + result, err := c.client.Request("session.create", req) + if err != nil { + dispose() + return nil, fmt.Errorf("failed to create cloud session: %w", err) + } + + var response createSessionResponse + if err := json.Unmarshal(result, &response); err != nil { + dispose() + return nil, fmt.Errorf("failed to unmarshal cloud session response: %w", err) + } + + if response.SessionID == "" { + fmt.Println("warning: cloud session.create response missing sessionId; runtime session may leak") + dispose() + return nil, fmt.Errorf("cloud session.create response did not include a sessionId; cannot register session") + } + + sessionID := response.SessionID + session := newSession(sessionID, c.client, response.WorkspacePath) + + session.registerTools(config.Tools) + session.registerPermissionHandler(config.OnPermissionRequest) + if config.OnUserInputRequest != nil { + session.registerUserInputHandler(config.OnUserInputRequest) + } + if config.Hooks != nil { + session.registerHooks(config.Hooks) + } + if transformCallbacks != nil { + session.registerTransformCallbacks(transformCallbacks) + } + if config.OnEvent != nil { + session.On(config.OnEvent) + } + if len(config.Commands) > 0 { + session.registerCommands(config.Commands) + } + if config.OnElicitationRequest != nil { + session.registerElicitationHandler(config.OnElicitationRequest) + } + if config.OnExitPlanModeRequest != nil { + session.registerExitPlanModeHandler(config.OnExitPlanModeRequest) + } + if config.OnAutoModeSwitchRequest != nil { + session.registerAutoModeSwitchHandler(config.OnAutoModeSwitchRequest) + } + session.setCapabilities(response.Capabilities) + + c.sessionsMux.Lock() + c.sessions[sessionID] = session + c.sessionsMux.Unlock() + + if c.options.SessionFs != nil { + if config.CreateSessionFsProvider == nil { + c.sessionsMux.Lock() + delete(c.sessions, sessionID) + c.sessionsMux.Unlock() + dispose() + return nil, fmt.Errorf("CreateSessionFsProvider is required in session config when SessionFs is enabled in client options") + } + provider := config.CreateSessionFsProvider(session) + if c.options.SessionFs.Capabilities != nil && c.options.SessionFs.Capabilities.Sqlite { + if _, ok := provider.(SessionFsSqliteProvider); !ok { + c.sessionsMux.Lock() + delete(c.sessions, sessionID) + c.sessionsMux.Unlock() + dispose() + return nil, fmt.Errorf("SessionFs capabilities declare SQLite support but the provider does not implement SessionFsSqliteProvider") + } + } + session.clientSessionApis.SessionFs = newSessionFsAdapter(provider) + } + + // Drain buffered events and unblock parked request handlers before + // releasing the guard, so they see a fully-wired session. + c.flushPendingForSession(sessionID, session) + dispose() + + return session, nil +} + +// beginPendingSessionRouting increments the pending-routing refcount and +// returns a disposer. While any disposer is undisposed, session.event +// notifications and inbound JSON-RPC requests addressed to unknown session ids +// are buffered/parked rather than dropped. When the last disposer fires, any +// remaining buffers are cleared and parked handlers receive an error so they +// don't block forever. +func (c *Client) beginPendingSessionRouting() func() { + c.pending.mu.Lock() + c.pending.count++ + c.pending.mu.Unlock() + + var once sync.Once + return func() { + once.Do(func() { + c.pending.mu.Lock() + c.pending.count-- + if c.pending.count > 0 { + c.pending.mu.Unlock() + return + } + // Last guard: swap out the maps so we can signal waiters without + // holding the lock (buffered channels make sends non-blocking, but + // releasing the lock is cleaner). + c.pending.events = make(map[string][]sessionEventRequest) + waiters := c.pending.waiters + c.pending.waiters = make(map[string][]chan pendingResult) + c.pending.mu.Unlock() + + for _, chs := range waiters { + for _, ch := range chs { + ch <- pendingResult{err: fmt.Errorf("request dropped: cloud session.create completed without registering this session id")} + } + } + }) + } +} + +// flushPendingForSession drains buffered events and resolves parked request +// handlers for sessionID into the freshly-registered session. Called from +// CreateCloudSession after the session is in c.sessions and before the pending +// guard is released. +func (c *Client) flushPendingForSession(sessionID string, session *Session) { + c.pending.mu.Lock() + events := c.pending.events[sessionID] + delete(c.pending.events, sessionID) + waiters := c.pending.waiters[sessionID] + delete(c.pending.waiters, sessionID) + c.pending.mu.Unlock() + + for _, req := range events { + session.dispatchEvent(req.Event) + } + for _, ch := range waiters { + ch <- pendingResult{session: session} + } +} + +// waitForSession looks up the session by id. If the session is not yet +// registered but pending routing is active, the call parks until the session +// is registered (or pending routing ends without registration). +func (c *Client) waitForSession(sessionID string) (*Session, error) { + c.sessionsMux.Lock() + session, ok := c.sessions[sessionID] + c.sessionsMux.Unlock() + if ok { + return session, nil + } + + c.pending.mu.Lock() + if c.pending.count == 0 { + c.pending.mu.Unlock() + return nil, fmt.Errorf("unknown session %s", sessionID) + } + // Re-check under pending.mu: the session may have been registered and + // flushed between the first lookup and acquiring this lock. + c.sessionsMux.Lock() + session, ok = c.sessions[sessionID] + c.sessionsMux.Unlock() + if ok { + c.pending.mu.Unlock() + return session, nil + } + ch := make(chan pendingResult, 1) + waiters := c.pending.waiters[sessionID] + if len(waiters) >= pendingSessionBufferLimit { + // Reject the oldest waiter to keep the queue bounded. + oldest := waiters[0] + waiters = waiters[1:] + oldest <- pendingResult{err: fmt.Errorf("request dropped: pending session waiter buffer full for %s", sessionID)} + } + c.pending.waiters[sessionID] = append(waiters, ch) + c.pending.mu.Unlock() + + result := <-ch + return result.session, result.err +} + // ResumeSession resumes an existing conversation session by its ID. // // This is a convenience method that calls [Client.ResumeSessionWithOptions]. @@ -1761,14 +2095,35 @@ func (c *Client) handleSessionEvent(req sessionEventRequest) { if req.SessionID == "" { return } - // Dispatch to session c.sessionsMux.Lock() session, ok := c.sessions[req.SessionID] c.sessionsMux.Unlock() if ok { session.dispatchEvent(req.Event) + return + } + + // Buffer if a cloud session.create is in flight for this id. + c.pending.mu.Lock() + if c.pending.count > 0 { + // Re-check under pending.mu: the session may have been registered and + // flushed between the first lookup and acquiring this lock. + c.sessionsMux.Lock() + session, ok = c.sessions[req.SessionID] + c.sessionsMux.Unlock() + if ok { + c.pending.mu.Unlock() + session.dispatchEvent(req.Event) + return + } + buf := c.pending.events[req.SessionID] + if len(buf) >= pendingSessionBufferLimit { + buf = buf[1:] // drop oldest + } + c.pending.events[req.SessionID] = append(buf, req) } + c.pending.mu.Unlock() } // handleUserInputRequest handles a user input request from the CLI server. @@ -1777,11 +2132,9 @@ func (c *Client) handleUserInputRequest(req userInputRequest) (*userInputRespons return nil, &jsonrpc2.Error{Code: -32602, Message: "invalid user input request payload"} } - c.sessionsMux.Lock() - session, ok := c.sessions[req.SessionID] - c.sessionsMux.Unlock() - if !ok { - return nil, &jsonrpc2.Error{Code: -32602, Message: fmt.Sprintf("unknown session %s", req.SessionID)} + session, err := c.waitForSession(req.SessionID) + if err != nil { + return nil, &jsonrpc2.Error{Code: -32602, Message: err.Error()} } response, err := session.handleUserInputRequest(UserInputRequest{ @@ -1806,11 +2159,9 @@ func (c *Client) handleExitPlanModeRequest(req exitPlanModeRequest) (*ExitPlanMo recommendedAction = "autopilot" } - c.sessionsMux.Lock() - session, ok := c.sessions[req.SessionID] - c.sessionsMux.Unlock() - if !ok { - return nil, &jsonrpc2.Error{Code: -32602, Message: fmt.Sprintf("unknown session %s", req.SessionID)} + session, err := c.waitForSession(req.SessionID) + if err != nil { + return nil, &jsonrpc2.Error{Code: -32602, Message: err.Error()} } response, err := session.handleExitPlanModeRequest(ExitPlanModeRequest{ @@ -1832,11 +2183,9 @@ func (c *Client) handleAutoModeSwitchRequest(req autoModeSwitchRequest) (*autoMo return nil, &jsonrpc2.Error{Code: -32602, Message: "invalid auto mode switch request payload"} } - c.sessionsMux.Lock() - session, ok := c.sessions[req.SessionID] - c.sessionsMux.Unlock() - if !ok { - return nil, &jsonrpc2.Error{Code: -32602, Message: fmt.Sprintf("unknown session %s", req.SessionID)} + session, err := c.waitForSession(req.SessionID) + if err != nil { + return nil, &jsonrpc2.Error{Code: -32602, Message: err.Error()} } response, err := session.handleAutoModeSwitchRequest(AutoModeSwitchRequest{ @@ -1856,11 +2205,9 @@ func (c *Client) handleHooksInvoke(req hooksInvokeRequest) (map[string]any, *jso return nil, &jsonrpc2.Error{Code: -32602, Message: "invalid hooks invoke payload"} } - c.sessionsMux.Lock() - session, ok := c.sessions[req.SessionID] - c.sessionsMux.Unlock() - if !ok { - return nil, &jsonrpc2.Error{Code: -32602, Message: fmt.Sprintf("unknown session %s", req.SessionID)} + session, err := c.waitForSession(req.SessionID) + if err != nil { + return nil, &jsonrpc2.Error{Code: -32602, Message: err.Error()} } output, err := session.handleHooksInvoke(req.Type, req.Input) @@ -1881,11 +2228,9 @@ func (c *Client) handleSystemMessageTransform(req systemMessageTransformRequest) return systemMessageTransformResponse{}, &jsonrpc2.Error{Code: -32602, Message: "invalid system message transform payload"} } - c.sessionsMux.Lock() - session, ok := c.sessions[req.SessionID] - c.sessionsMux.Unlock() - if !ok { - return systemMessageTransformResponse{}, &jsonrpc2.Error{Code: -32602, Message: fmt.Sprintf("unknown session %s", req.SessionID)} + session, err := c.waitForSession(req.SessionID) + if err != nil { + return systemMessageTransformResponse{}, &jsonrpc2.Error{Code: -32602, Message: err.Error()} } resp, err := session.handleSystemMessageTransform(req.Sections) diff --git a/go/cloud_session_test.go b/go/cloud_session_test.go new file mode 100644 index 000000000..ce827dbb0 --- /dev/null +++ b/go/cloud_session_test.go @@ -0,0 +1,329 @@ +package copilot + +import ( + "encoding/json" + "strings" + "sync" + "testing" + "time" + + "github.com/github/copilot-sdk/go/internal/jsonrpc2" +) + +// newCloudTestClient returns a Client with pending routing initialized and a +// pre-populated sessions map, suitable for unit-testing cloud session logic +// without a real network connection. +func newCloudTestClient() *Client { + return &Client{ + sessions: make(map[string]*Session), + pending: pendingRouting{ + events: make(map[string][]sessionEventRequest), + waiters: make(map[string][]chan pendingResult), + }, + } +} + +// TestCreateSession_RejectsCloudConfig verifies that CreateSession returns a +// clear error when config.Cloud is set. +func TestCreateSession_RejectsCloudConfig(t *testing.T) { + client := NewClient(&ClientOptions{Connection: StdioConnection{Path: "/__nonexistent__"}}) + _, err := client.CreateSession(t.Context(), &SessionConfig{ + Cloud: &CloudSessionOptions{}, + }) + if err == nil { + t.Fatal("expected error when cloud config is set") + } + if !strings.Contains(err.Error(), "CreateCloudSession") { + t.Errorf("error should mention CreateCloudSession, got: %v", err) + } +} + +// TestCreateCloudSession_RejectsCallerSessionID verifies the SDK rejects a +// caller-supplied SessionID. +func TestCreateCloudSession_RejectsCallerSessionID(t *testing.T) { + client := NewClient(&ClientOptions{Connection: StdioConnection{Path: "/__nonexistent__"}}) + _, err := client.CreateCloudSession(t.Context(), &SessionConfig{ + Cloud: &CloudSessionOptions{}, + SessionID: "caller-supplied-id", + }) + if err == nil { + t.Fatal("expected error when SessionID is set") + } + if !strings.Contains(err.Error(), "SessionID") { + t.Errorf("error should mention SessionID, got: %v", err) + } +} + +// TestCreateCloudSession_RejectsCallerProvider verifies the SDK rejects a +// caller-supplied Provider. +func TestCreateCloudSession_RejectsCallerProvider(t *testing.T) { + client := NewClient(&ClientOptions{Connection: StdioConnection{Path: "/__nonexistent__"}}) + _, err := client.CreateCloudSession(t.Context(), &SessionConfig{ + Cloud: &CloudSessionOptions{}, + Provider: &ProviderConfig{ModelID: "gpt-4"}, + }) + if err == nil { + t.Fatal("expected error when Provider is set") + } + if !strings.Contains(err.Error(), "Provider") { + t.Errorf("error should mention Provider, got: %v", err) + } +} + +// TestCreateCloudSession_RequiresCloud verifies the SDK rejects configs without +// Cloud set. +func TestCreateCloudSession_RequiresCloud(t *testing.T) { + client := NewClient(&ClientOptions{Connection: StdioConnection{Path: "/__nonexistent__"}}) + _, err := client.CreateCloudSession(t.Context(), &SessionConfig{}) + if err == nil { + t.Fatal("expected error when Cloud is nil") + } + if !strings.Contains(err.Error(), "Cloud") { + t.Errorf("error should mention Cloud, got: %v", err) + } +} + +// TestCreateCloudSession_WirePayload verifies that the session.create wire +// payload includes the cloud field and omits sessionId when built by the cloud +// path. +func TestCreateCloudSession_WirePayload(t *testing.T) { + req := createSessionRequest{ + Cloud: &CloudSessionOptions{ + Repository: &CloudSessionRepository{Owner: "github", Name: "copilot-sdk"}, + }, + // SessionID intentionally left empty + } + + data, err := json.Marshal(req) + if err != nil { + t.Fatalf("marshal error: %v", err) + } + + var m map[string]any + if err := json.Unmarshal(data, &m); err != nil { + t.Fatalf("unmarshal error: %v", err) + } + + if _, ok := m["sessionId"]; ok { + t.Error("sessionId must be omitted from the cloud session.create wire payload") + } + + cloud, ok := m["cloud"] + if !ok { + t.Fatal("cloud field must be present in the wire payload") + } + cloudMap, ok := cloud.(map[string]any) + if !ok { + t.Fatalf("cloud field should be a map, got %T", cloud) + } + repo, ok := cloudMap["repository"].(map[string]any) + if !ok { + t.Fatal("cloud.repository should be a map") + } + if repo["owner"] != "github" || repo["name"] != "copilot-sdk" { + t.Errorf("unexpected cloud.repository: %v", repo) + } +} + +// TestPendingRouting_BuffersEarlyNotifications verifies that session.event +// notifications arriving before the session is registered are buffered and +// replayed when flushPendingForSession is called. +func TestPendingRouting_BuffersEarlyNotifications(t *testing.T) { + client := newCloudTestClient() + dispose := client.beginPendingSessionRouting() + defer dispose() + + const pendingID = "runtime-assigned-id" + + // Simulate two session.event notifications arriving before the session is + // registered. + client.handleSessionEvent(sessionEventRequest{ + SessionID: pendingID, + Event: SessionEvent{Data: &SessionIdleData{}}, + }) + client.handleSessionEvent(sessionEventRequest{ + SessionID: pendingID, + Event: SessionEvent{Data: &SessionIdleData{}}, + }) + + // Verify they are buffered. + client.pending.mu.Lock() + bufLen := len(client.pending.events[pendingID]) + client.pending.mu.Unlock() + if bufLen != 2 { + t.Fatalf("expected 2 buffered events, got %d", bufLen) + } + + // Now register the session and flush. + session, cleanup := newTestSession() + defer cleanup() + session.SessionID = pendingID + + var received []SessionEvent + var mu sync.Mutex + var wg sync.WaitGroup + wg.Add(2) + session.On(func(event SessionEvent) { + mu.Lock() + received = append(received, event) + mu.Unlock() + wg.Done() + }) + + client.sessionsMux.Lock() + client.sessions[pendingID] = session + client.sessionsMux.Unlock() + + client.flushPendingForSession(pendingID, session) + + // Wait for the event handler goroutine to process. + done := make(chan struct{}) + go func() { + wg.Wait() + close(done) + }() + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatal("timed out waiting for buffered events to be dispatched") + } + + mu.Lock() + got := len(received) + mu.Unlock() + if got != 2 { + t.Errorf("expected 2 events replayed, got %d", got) + } + + // Buffer should be cleared after flush. + client.pending.mu.Lock() + remaining := len(client.pending.events[pendingID]) + client.pending.mu.Unlock() + if remaining != 0 { + t.Errorf("buffer should be empty after flush, got %d", remaining) + } +} + +// TestPendingRouting_ParksInboundRequests verifies that inbound request handlers +// (e.g. userInput.request) park until the session is registered when pending +// routing is active. +func TestPendingRouting_ParksInboundRequests(t *testing.T) { + client := newCloudTestClient() + dispose := client.beginPendingSessionRouting() + + const pendingID = "runtime-assigned-id-2" + + // Launch a goroutine that simulates an inbound userInput.request arriving + // before the session is registered. + type result struct { + resp *userInputResponse + err *jsonrpcError + } + resultCh := make(chan result, 1) + go func() { + resp, rpcErr := client.handleUserInputRequest(userInputRequest{ + SessionID: pendingID, + Question: "Proceed?", + }) + resultCh <- result{resp, rpcErr} + }() + + // Give the goroutine time to park. + time.Sleep(20 * time.Millisecond) + + // Register the session. + session, cleanup := newTestSession() + defer cleanup() + session.SessionID = pendingID + session.registerUserInputHandler(func(req UserInputRequest, _ UserInputInvocation) (UserInputResponse, error) { + return UserInputResponse{Answer: "yes"}, nil + }) + + client.sessionsMux.Lock() + client.sessions[pendingID] = session + client.sessionsMux.Unlock() + + client.flushPendingForSession(pendingID, session) + dispose() + + select { + case r := <-resultCh: + if r.err != nil { + t.Fatalf("expected success, got rpc error: %v", r.err) + } + if r.resp == nil || r.resp.Answer != "yes" { + t.Errorf("expected answer 'yes', got %+v", r.resp) + } + case <-time.After(2 * time.Second): + t.Fatal("timed out waiting for parked request to be resolved") + } +} + +// TestPendingRouting_DropOldestWhenBufferFull verifies drop-oldest behaviour +// when the notification buffer is full. +func TestPendingRouting_DropOldestWhenBufferFull(t *testing.T) { + client := newCloudTestClient() + dispose := client.beginPendingSessionRouting() + defer dispose() + + const pendingID = "overflow-session" + + // Fill buffer beyond the limit. + for i := range pendingSessionBufferLimit + 5 { + client.handleSessionEvent(sessionEventRequest{ + SessionID: pendingID, + Event: SessionEvent{ + // Embed the index so we can verify drop-oldest. + Data: &SessionIdleData{}, + }, + }) + _ = i + } + + client.pending.mu.Lock() + bufLen := len(client.pending.events[pendingID]) + client.pending.mu.Unlock() + + if bufLen != pendingSessionBufferLimit { + t.Errorf("expected buffer capped at %d, got %d", pendingSessionBufferLimit, bufLen) + } +} + +// TestPendingRouting_RejectsWaitersOnDispose verifies that waiters are +// rejected with an error when pending mode ends without registration. +func TestPendingRouting_RejectsWaitersOnDispose(t *testing.T) { + client := newCloudTestClient() + dispose := client.beginPendingSessionRouting() + + const pendingID = "never-registered" + + resultCh := make(chan *jsonrpcError, 1) + go func() { + _, rpcErr := client.handleUserInputRequest(userInputRequest{ + SessionID: pendingID, + Question: "Proceed?", + }) + resultCh <- rpcErr + }() + + // Give the goroutine time to park. + time.Sleep(20 * time.Millisecond) + + // Dispose without registering the session. + dispose() + + select { + case rpcErr := <-resultCh: + if rpcErr == nil { + t.Fatal("expected an rpc error after dispose without registration") + } + if !strings.Contains(rpcErr.Message, "dropped") { + t.Errorf("expected 'dropped' in error message, got: %s", rpcErr.Message) + } + case <-time.After(2 * time.Second): + t.Fatal("timed out waiting for rejected waiter") + } +} + +// jsonrpcError is a local alias for jsonrpc2.Error used in test assertions. +type jsonrpcError = jsonrpc2.Error From 6d5510a85b6c37a31a70f659ada95c350b0f44ac Mon Sep 17 00:00:00 2001 From: Timothy Clem Date: Fri, 22 May 2026 19:28:13 -0700 Subject: [PATCH 02/14] python: add create_cloud_session 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> --- python/README.md | 34 +++ python/copilot/client.py | 497 +++++++++++++++++++++++++++++++++-- python/copilot/session.py | 11 + python/test_client.py | 10 +- python/test_cloud_session.py | 279 ++++++++++++++++++++ 5 files changed, 801 insertions(+), 30 deletions(-) create mode 100644 python/test_cloud_session.py diff --git a/python/README.md b/python/README.md index 3a504f966..44e409bfe 100644 --- a/python/README.md +++ b/python/README.md @@ -414,6 +414,40 @@ When `streaming=True`: Note: `assistant.message` and `assistant.reasoning` (final events) are always sent regardless of streaming setting. +## Cloud Sessions + +Use `create_cloud_session()` to create a Mission Control–backed cloud session. The +runtime owns the session ID for cloud sessions, so omit `session_id` and `provider` +(the SDK raises `ValueError` if either is set). + +```python +from copilot import CopilotClient, RuntimeConnection +from copilot.client import CloudSessionOptions, CloudSessionRepository + +client = CopilotClient(connection=RuntimeConnection.for_stdio(path="/path/to/cli")) +await client.start() + +session = await client.create_cloud_session( + cloud=CloudSessionOptions( + repository=CloudSessionRepository( + owner="my-org", + name="my-repo", + branch="main", + ) + ), + on_event=lambda event: print(event), +) +print(session.remote_url) # URL of the remote cloud session +``` + +`create_cloud_session()` accepts the same keyword arguments as `create_session()` +(tools, streaming, model, hooks, etc.) with two restrictions: +- `session_id` must not be set (the runtime assigns the ID). +- `provider` must not be set (cloud sessions always use the Mission Control provider). + +Early `session.event` notifications and inbound RPC requests that arrive before the +session is fully registered are buffered and replayed once the session is ready. + ## Infinite Sessions By default, sessions use **infinite sessions** which automatically manage context window limits through background compaction and persist state to a workspace directory. diff --git a/python/copilot/client.py b/python/copilot/client.py index a52b8711f..02a04cc18 100644 --- a/python/copilot/client.py +++ b/python/copilot/client.py @@ -996,6 +996,55 @@ def _extract_transform_callbacks( return wire_payload, callbacks +_PENDING_SESSION_BUFFER_LIMIT = 128 +"""Upper bound on buffered notifications/requests per pending session id. + +Holds traffic that arrives between ``session.create`` being sent and the +SDK learning the runtime-assigned session id from the response (cloud path). +Drop-oldest behaviour is acceptable: cloud handshakes are short and 128 +entries is well above realistic init/replay bursts. +""" + + +class _PendingSessionRoutingGuard: + """RAII guard that keeps pending-routing mode active for a cloud session.create. + + While alive, notifications and inbound requests addressed to session ids + that are not yet registered are buffered instead of dropped, so events + the runtime emits between ``session.create`` and its response are not + lost. Dispose exactly once — either after successful registration (to + replay the buffered messages) or on any error path (to reject parked + request futures so callers don't hang). + """ + + __slots__ = ("_client", "_disposed") + + def __init__(self, client: CopilotClient) -> None: + self._client = client + self._disposed = False + + def dispose(self) -> None: + if self._disposed: + return + self._disposed = True + waiters_to_reject: list[asyncio.Future] = [] + with self._client._sessions_lock: + self._client._pending_routing_count -= 1 + if self._client._pending_routing_count == 0: + self._client._pending_session_events.clear() + for session_waiters in self._client._pending_session_waiters.values(): + waiters_to_reject.extend(session_waiters) + self._client._pending_session_waiters.clear() + for future in waiters_to_reject: + if not future.done(): + future.set_exception( + ValueError( + "Cloud session.create completed without registering this session id; " + "inbound request dropped" + ) + ) + + class CopilotClient: """ Main client for interacting with the Copilot CLI. @@ -1181,6 +1230,10 @@ def __init__( self._state: _ConnectionState = "disconnected" self._sessions: dict[str, CopilotSession] = {} self._sessions_lock = threading.Lock() + # Pending-routing state for create_cloud_session: guarded by _sessions_lock. + self._pending_routing_count: int = 0 + self._pending_session_events: dict[str, list[SessionEvent]] = {} + self._pending_session_waiters: dict[str, list[asyncio.Future[CopilotSession]]] = {} self._models_cache: list[ModelInfo] | None = None self._models_cache_lock = asyncio.Lock() self._lifecycle_handlers: list[SessionLifecycleHandler] = [] @@ -1629,6 +1682,11 @@ async def create_session( ... streaming=True, ... ) """ + if cloud is not None: + raise ValueError( + "CopilotClient.create_session does not support cloud sessions; " + "use create_cloud_session instead." + ) if on_permission_request is not None and not callable(on_permission_request): raise ValueError("on_permission_request must be callable when provided.") if not self._client: @@ -1880,6 +1938,392 @@ async def create_session( ) return session + async def create_cloud_session( + self, + *, + cloud: CloudSessionOptions | None = None, + session_id: str | None = None, + provider: ProviderConfig | None = None, + on_permission_request: _PermissionHandlerFn | None = None, + model: str | None = None, + client_name: str | None = None, + reasoning_effort: ReasoningEffort | None = None, + tools: list[Tool] | None = None, + system_message: SystemMessageConfig | None = None, + available_tools: list[str] | None = None, + excluded_tools: list[str] | None = None, + on_user_input_request: UserInputHandler | None = None, + hooks: SessionHooks | None = None, + working_directory: str | None = None, + enable_session_telemetry: bool | None = None, + model_capabilities: ModelCapabilitiesOverride | None = None, + streaming: bool | None = None, + include_sub_agent_streaming_events: bool | None = None, + mcp_servers: dict[str, MCPServerConfig] | None = None, + custom_agents: list[CustomAgentConfig] | None = None, + default_agent: DefaultAgentConfig | dict[str, Any] | None = None, + agent: str | None = None, + config_dir: str | None = None, + enable_config_discovery: bool | None = None, + skill_directories: list[str] | None = None, + instruction_directories: list[str] | None = None, + disabled_skills: list[str] | None = None, + infinite_sessions: InfiniteSessionConfig | None = None, + on_event: Callable[[SessionEvent], None] | None = None, + commands: list[CommandDefinition] | None = None, + on_elicitation_request: ElicitationHandler | None = None, + on_exit_plan_mode_request: ExitPlanModeHandler | None = None, + on_auto_mode_switch_request: AutoModeSwitchHandler | None = None, + create_session_fs_handler: CreateSessionFsHandler | None = None, + github_token: str | None = None, + remote_session: RemoteSessionMode | None = None, + ) -> CopilotSession: + """ + Create a Mission Control–backed cloud session. + + The runtime owns the session ID for cloud sessions: do **not** set + ``session_id`` or ``provider`` on the call (the SDK rejects both with + :class:`ValueError`). The SDK omits ``sessionId`` from the + ``session.create`` wire payload and registers the resulting session + under the id that the runtime returns. + + Any ``session.event`` notifications or inbound JSON-RPC requests + (``userInput.request``, ``exitPlanMode.request``, etc.) that arrive + between sending ``session.create`` and receiving its response are + buffered (bounded, drop-oldest, up to + ``_PENDING_SESSION_BUFFER_LIMIT`` per id) and replayed once the + returned session id is registered, so early events are not lost. + + **Known limitation:** inbound ``sessionFs.*`` requests (the generated + client-session API handlers) are not pending-buffered. In practice the + runtime does not initiate ``sessionFs.*`` calls before the + ``session.create`` response, so this is theoretical. + + Args: + cloud: Required. Cloud session options (repository, branch, etc.). + session_id: Must be ``None``; the runtime assigns the id. + provider: Must be ``None``; cloud sessions use the runtime's provider. + on_permission_request: Handler for permission requests. + model: Model to use. + client_name: Client name for identification. + reasoning_effort: Reasoning effort level. + tools: Custom tools to register. + system_message: System message configuration. + available_tools: Allowlist of tools. + excluded_tools: Tools to disable. + on_user_input_request: Handler for user input requests. + hooks: Lifecycle hooks. + working_directory: Working directory. + enable_session_telemetry: Enable/disable session telemetry. + model_capabilities: Model capabilities override. + streaming: Enable streaming responses. + include_sub_agent_streaming_events: Include sub-agent streaming events. + mcp_servers: MCP server configurations. + custom_agents: Custom agent configurations. + default_agent: Default agent configuration. + agent: Agent to use. + config_dir: Configuration directory override. + enable_config_discovery: Auto-discover MCP/skill config from cwd. + skill_directories: Directories to search for skills. + instruction_directories: Additional instruction file directories. + disabled_skills: Skills to disable. + infinite_sessions: Infinite session configuration. + on_event: Callback for session events. + commands: Commands to register. + on_elicitation_request: Handler for elicitation requests. + on_exit_plan_mode_request: Handler for exit-plan-mode requests. + on_auto_mode_switch_request: Handler for auto-mode-switch requests. + create_session_fs_handler: Session filesystem handler factory. + github_token: Per-session GitHub token. + remote_session: Remote session mode. + + Returns: + A :class:`CopilotSession` for the cloud session, with its + ``session_id`` set to the runtime-assigned id. + + Raises: + ValueError: If ``cloud`` is ``None``, ``session_id`` is set, or + ``provider`` is set. + + Example: + >>> session = await client.create_cloud_session( + ... cloud=CloudSessionOptions( + ... repository=CloudSessionRepository( + ... owner="github", name="copilot-sdk", branch="main" + ... ) + ... ), + ... ) + >>> print(session.session_id) # runtime-assigned id + """ + if cloud is None: + raise ValueError( + "create_cloud_session requires cloud to be set; " + "use CloudSessionOptions to configure the repository." + ) + if session_id is not None: + raise ValueError( + "create_cloud_session does not accept session_id; the runtime assigns one." + ) + if provider is not None: + raise ValueError( + "create_cloud_session does not accept provider; " + "cloud sessions use the runtime's provider." + ) + + if not self._client: + await self.start() + + tool_defs = [] + if tools: + for tool in tools: + definition: dict[str, Any] = { + "name": tool.name, + "description": tool.description, + } + if tool.parameters: + definition["parameters"] = tool.parameters + if tool.overrides_built_in_tool: + definition["overridesBuiltInTool"] = True + if tool.skip_permission: + definition["skipPermission"] = True + tool_defs.append(definition) + + payload: dict[str, Any] = {} + if model: + payload["model"] = model + if client_name: + payload["clientName"] = client_name + if reasoning_effort: + payload["reasoningEffort"] = reasoning_effort + if tool_defs: + payload["tools"] = tool_defs + + wire_system_message, transform_callbacks = _extract_transform_callbacks(system_message) + if wire_system_message: + payload["systemMessage"] = wire_system_message + + if available_tools is not None: + payload["availableTools"] = available_tools + if excluded_tools is not None: + payload["excludedTools"] = excluded_tools + + payload["requestPermission"] = bool(on_permission_request) + if on_user_input_request: + payload["requestUserInput"] = True + payload["requestElicitation"] = bool(on_elicitation_request) + payload["requestExitPlanMode"] = bool(on_exit_plan_mode_request) + payload["requestAutoModeSwitch"] = bool(on_auto_mode_switch_request) + + if commands: + payload["commands"] = [ + {"name": cmd.name, "description": cmd.description} for cmd in commands + ] + if hooks and any(hooks.values()): + payload["hooks"] = True + if github_token is not None: + payload["gitHubToken"] = github_token + if remote_session is not None: + payload["remoteSession"] = remote_session.value + + # sessionId intentionally omitted: the runtime assigns the id for cloud sessions. + payload["cloud"] = _cloud_session_options_to_dict(cloud) + + if working_directory: + payload["workingDirectory"] = working_directory + if streaming is not None: + payload["streaming"] = streaming + payload["includeSubAgentStreamingEvents"] = ( + include_sub_agent_streaming_events + if include_sub_agent_streaming_events is not None + else True + ) + if enable_session_telemetry is not None: + payload["enableSessionTelemetry"] = enable_session_telemetry + if model_capabilities: + payload["modelCapabilities"] = _capabilities_to_dict(model_capabilities) + if mcp_servers: + payload["mcpServers"] = _mcp_servers_to_wire(mcp_servers) + payload["envValueMode"] = "direct" + if custom_agents: + payload["customAgents"] = [ + self._convert_custom_agent_to_wire_format(a) for a in custom_agents + ] + if default_agent: + payload["defaultAgent"] = self._convert_default_agent_to_wire_format(default_agent) + if agent: + payload["agent"] = agent + if config_dir: + payload["configDir"] = config_dir + if enable_config_discovery is not None: + payload["enableConfigDiscovery"] = enable_config_discovery + if skill_directories: + payload["skillDirectories"] = skill_directories + if instruction_directories is not None: + payload["instructionDirectories"] = instruction_directories + if disabled_skills: + payload["disabledSkills"] = disabled_skills + if infinite_sessions: + wire_config: dict[str, Any] = {} + if "enabled" in infinite_sessions: + wire_config["enabled"] = infinite_sessions["enabled"] + if "background_compaction_threshold" in infinite_sessions: + wire_config["backgroundCompactionThreshold"] = infinite_sessions[ + "background_compaction_threshold" + ] + if "buffer_exhaustion_threshold" in infinite_sessions: + wire_config["bufferExhaustionThreshold"] = infinite_sessions[ + "buffer_exhaustion_threshold" + ] + payload["infiniteSessions"] = wire_config + + if not self._client: + raise RuntimeError("Client not connected") + + trace_ctx = get_trace_context() + payload.update(trace_ctx) + + total_start = time.perf_counter() + guard = self._begin_pending_session_routing() + + try: + rpc_start = time.perf_counter() + response = await self._client.request("session.create", payload) + log_timing( + logger, + logging.DEBUG, + "CopilotClient.create_cloud_session session creation request completed", + rpc_start, + ) + except BaseException: + guard.dispose() + raise + + returned_session_id = response.get("sessionId") + if not isinstance(returned_session_id, str) or not returned_session_id: + logger.warning( + "Cloud session.create response missing sessionId; runtime session may leak" + ) + guard.dispose() + raise ValueError( + "Cloud session.create response did not include a sessionId; " + "cannot register session." + ) + + session = CopilotSession(returned_session_id, self._client, workspace_path=None) + session._register_tools(tools) + session._register_commands(commands) + session._register_permission_handler(on_permission_request) + if on_user_input_request: + session._register_user_input_handler(on_user_input_request) + if on_elicitation_request: + session._register_elicitation_handler(on_elicitation_request) + if on_exit_plan_mode_request: + session._register_exit_plan_mode_handler(on_exit_plan_mode_request) + if on_auto_mode_switch_request: + session._register_auto_mode_switch_handler(on_auto_mode_switch_request) + if hooks: + session._register_hooks(hooks) + if transform_callbacks: + session._register_transform_callbacks(transform_callbacks) + if on_event: + session.on(on_event) + + try: + if self._session_fs_config: + if create_session_fs_handler is None: + raise ValueError( + "create_session_fs_handler is required in session config when " + "session_fs is enabled in client options." + ) + fs_provider: SessionFsProvider = create_session_fs_handler(session) + caps = self._session_fs_config.get("capabilities") + if caps and caps.get("sqlite"): + from .session_fs_provider import SessionFsSqliteProvider + + if not isinstance(fs_provider, SessionFsSqliteProvider): + raise ValueError( + "SessionFs capabilities declare SQLite support but the provider " + "does not implement SessionFsSqliteProvider" + ) + session._client_session_apis.session_fs = create_session_fs_adapter(fs_provider) + + with self._sessions_lock: + self._sessions[returned_session_id] = session + + session._workspace_path = response.get("workspacePath") + session._remote_url = response.get("remoteUrl") + capabilities = response.get("capabilities") + session._set_capabilities(capabilities) + self._flush_pending_for_session(returned_session_id, session) + except BaseException: + with self._sessions_lock: + self._sessions.pop(returned_session_id, None) + guard.dispose() + raise + + guard.dispose() + log_timing( + logger, + logging.DEBUG, + "CopilotClient.create_cloud_session complete", + total_start, + session_id=returned_session_id, + ) + return session + + def _begin_pending_session_routing(self) -> _PendingSessionRoutingGuard: + """Enter pending-routing mode; return a guard that exits it on dispose(). + + While at least one guard is alive, ``session.event`` notifications and + inbound JSON-RPC requests addressed to session ids that are not yet + registered are buffered (bounded, drop-oldest) and replayed on + registration. When the last guard is disposed, any still-pending + messages are dropped and parked request futures are rejected so callers + don't hang. + """ + with self._sessions_lock: + self._pending_routing_count += 1 + return _PendingSessionRoutingGuard(self) + + def _flush_pending_for_session(self, session_id: str, session: CopilotSession) -> None: + """Drain buffered events and resolve parked request futures for ``session_id``. + + Called from :meth:`create_cloud_session` after the session has been + registered in ``_sessions`` and before the pending-routing guard is + released. + """ + events_to_dispatch: list[SessionEvent] = [] + waiters_to_resolve: list[asyncio.Future[CopilotSession]] = [] + with self._sessions_lock: + events_to_dispatch = self._pending_session_events.pop(session_id, []) + waiters_to_resolve = self._pending_session_waiters.pop(session_id, []) + for event in events_to_dispatch: + session._dispatch_event(event) + for future in waiters_to_resolve: + if not future.done(): + future.set_result(session) + + async def _resolve_session(self, session_id: str) -> CopilotSession: + """Look up the session for an inbound request. + + If the session is not yet registered but a cloud ``session.create`` is + in flight (pending-routing mode is active), park the caller on a + :class:`asyncio.Future` until the session is registered or pending mode + ends. Otherwise raise :class:`ValueError` immediately. + """ + future: asyncio.Future[CopilotSession] | None = None + with self._sessions_lock: + session = self._sessions.get(session_id) + if session is None and self._pending_routing_count > 0: + loop = asyncio.get_running_loop() + future = loop.create_future() + self._pending_session_waiters.setdefault(session_id, []).append(future) + if session is not None: + return session + if future is not None: + return await future + raise ValueError(f"unknown session {session_id}") + async def resume_session( self, session_id: str, @@ -2969,6 +3413,16 @@ def handle_notification(method: str, params: dict): event = session_event_from_dict(event_dict) with self._sessions_lock: session = self._sessions.get(session_id) + if session is None and self._pending_routing_count > 0: + buf = self._pending_session_events.setdefault(session_id, []) + if len(buf) >= _PENDING_SESSION_BUFFER_LIMIT: + buf.pop(0) + logger.warning( + "pending session event buffer full for %s; dropping oldest", + session_id, + ) + buf.append(event) + return if session: session._dispatch_event(event) elif method == "session.lifecycle": @@ -3087,7 +3541,18 @@ def handle_notification(method: str, params: dict): event_dict = params["event"] # Convert dict to SessionEvent object event = session_event_from_dict(event_dict) - session = self._sessions.get(session_id) + with self._sessions_lock: + session = self._sessions.get(session_id) + if session is None and self._pending_routing_count > 0: + buf = self._pending_session_events.setdefault(session_id, []) + if len(buf) >= _PENDING_SESSION_BUFFER_LIMIT: + buf.pop(0) + logger.warning( + "pending session event buffer full for %s; dropping oldest", + session_id, + ) + buf.append(event) + return if session: session._dispatch_event(event) elif method == "session.lifecycle": @@ -3153,11 +3618,7 @@ async def _handle_user_input_request(self, params: dict) -> dict: if not session_id or not question: raise ValueError("invalid user input request payload") - with self._sessions_lock: - session = self._sessions.get(session_id) - if not session: - raise ValueError(f"unknown session {session_id}") - + session = await self._resolve_session(session_id) result = await session._handle_user_input_request(params) return {"answer": result["answer"], "wasFreeform": result["wasFreeform"]} @@ -3173,11 +3634,7 @@ async def _handle_exit_plan_mode_request(self, params: dict) -> dict: if not isinstance(actions, list) or not isinstance(recommended_action, str): raise ValueError("invalid exit plan mode request payload") - with self._sessions_lock: - session = self._sessions.get(session_id) - if not session: - raise ValueError(f"unknown session {session_id}") - + session = await self._resolve_session(session_id) return dict(await session._handle_exit_plan_mode_request(params)) async def _handle_auto_mode_switch_request(self, params: dict) -> dict: @@ -3186,11 +3643,7 @@ async def _handle_auto_mode_switch_request(self, params: dict) -> dict: if not session_id: raise ValueError("invalid auto mode switch request payload") - with self._sessions_lock: - session = self._sessions.get(session_id) - if not session: - raise ValueError(f"unknown session {session_id}") - + session = await self._resolve_session(session_id) response = await session._handle_auto_mode_switch_request(params) return {"response": response} @@ -3214,11 +3667,7 @@ async def _handle_hooks_invoke(self, params: dict) -> dict: if not session_id or not hook_type: raise ValueError("invalid hooks invoke payload") - with self._sessions_lock: - session = self._sessions.get(session_id) - if not session: - raise ValueError(f"unknown session {session_id}") - + session = await self._resolve_session(session_id) output = await session._handle_hooks_invoke(hook_type, input_data) return {"output": output} @@ -3230,9 +3679,5 @@ async def _handle_system_message_transform(self, params: dict) -> dict: if not session_id or not sections: raise ValueError("invalid systemMessage.transform payload") - with self._sessions_lock: - session = self._sessions.get(session_id) - if not session: - raise ValueError(f"unknown session {session_id}") - + session = await self._resolve_session(session_id) return await session._handle_system_message_transform(sections) diff --git a/python/copilot/session.py b/python/copilot/session.py index c775ef58e..7bee0530a 100644 --- a/python/copilot/session.py +++ b/python/copilot/session.py @@ -1001,6 +1001,7 @@ def __init__( self.session_id = session_id self._client = client self._workspace_path = os.fsdecode(workspace_path) if workspace_path is not None else None + self._remote_url: str | None = None self._event_handlers: set[Callable[[SessionEvent], None]] = set() self._event_handlers_lock = threading.Lock() self._tool_handlers: dict[str, ToolHandler] = {} @@ -1069,6 +1070,16 @@ def workspace_path(self) -> pathlib.Path | None: # attribute to do the conversion, or just do the conversion lazily via a getter. return pathlib.Path(self._workspace_path) if self._workspace_path else None + @property + def remote_url(self) -> str | None: + """Remote URL for the Mission Control–backed cloud session. + + Set from the ``remoteUrl`` field in the ``session.create`` response for + cloud sessions created via :meth:`CopilotClient.create_cloud_session`. + ``None`` for regular local sessions. + """ + return self._remote_url + async def send( self, prompt: str, diff --git a/python/test_client.py b/python/test_client.py index 14320b3a2..9869d9967 100644 --- a/python/test_client.py +++ b/python/test_client.py @@ -63,7 +63,7 @@ async def test_resume_session_allows_none_permission_handler(self): class TestCreateSessionConfig: @pytest.mark.asyncio - async def test_create_session_forwards_cloud_options(self): + async def test_create_cloud_session_forwards_cloud_options(self): client = CopilotClient(connection=RuntimeConnection.for_stdio(path=CLI_PATH)) await client.start() try: @@ -72,12 +72,11 @@ async def test_create_session_forwards_cloud_options(self): async def mock_request(method, params): captured[method] = params if method == "session.create": - return {"sessionId": params["sessionId"], "workspacePath": None} + return {"sessionId": "cloud-session-id", "workspacePath": None} return {} client._client.request = mock_request - await client.create_session( - on_permission_request=PermissionHandler.approve_all, + await client.create_cloud_session( cloud=CloudSessionOptions( repository=CloudSessionRepository( owner="github", @@ -87,6 +86,9 @@ async def mock_request(method, params): ), ) + assert "sessionId" not in captured["session.create"], ( + "cloud sessions must not send a sessionId in the wire payload" + ) assert captured["session.create"]["cloud"] == { "repository": { "owner": "github", diff --git a/python/test_cloud_session.py b/python/test_cloud_session.py new file mode 100644 index 000000000..f03164a05 --- /dev/null +++ b/python/test_cloud_session.py @@ -0,0 +1,279 @@ +""" +Tests for CopilotClient.create_cloud_session. + +Ports the spirit of the Rust integration tests in rust/tests/session_test.rs. +""" + +from __future__ import annotations + +import asyncio +from datetime import datetime +from uuid import uuid4 + +import pytest + +from copilot import CopilotClient, RuntimeConnection +from copilot.client import CloudSessionOptions, CloudSessionRepository +from copilot.session import ProviderConfig, UserInputRequest, UserInputResponse +from e2e.testharness import CLI_PATH + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _cloud_config() -> dict: + return dict( + cloud=CloudSessionOptions( + repository=CloudSessionRepository(owner="github", name="copilot-sdk", branch="main") + ) + ) + + +def _make_event_dict(event_type: str = "session.buffered_test", data: dict | None = None) -> dict: + """Build a minimal valid session-event dict for injection in tests.""" + return { + "id": str(uuid4()), + "timestamp": datetime.now().isoformat(), + "parentId": None, + "type": event_type, + "data": data or {}, + } + + +# --------------------------------------------------------------------------- +# Test 1: create_session rejects cloud config +# --------------------------------------------------------------------------- + + +class TestCreateSessionRejectsCloud: + @pytest.mark.asyncio + async def test_create_session_rejects_cloud_config(self): + """create_session must raise ValueError mentioning create_cloud_session.""" + client = CopilotClient(connection=RuntimeConnection.for_stdio(path=CLI_PATH)) + await client.start() + try: + with pytest.raises(ValueError, match="create_cloud_session"): + await client.create_session(**_cloud_config()) + finally: + await client.force_stop() + + +# --------------------------------------------------------------------------- +# Test 2: wire shape — sessionId omitted, cloud set, returned id used +# --------------------------------------------------------------------------- + + +class TestCreateCloudSessionWireShape: + @pytest.mark.asyncio + async def test_sends_cloud_without_session_id(self): + """session.create must carry cloud but omit sessionId; the response id is used.""" + client = CopilotClient(connection=RuntimeConnection.for_stdio(path=CLI_PATH)) + await client.start() + try: + captured: dict = {} + + async def mock_request(method, params): + captured[method] = params + if method == "session.create": + return { + "sessionId": "remote-cloud-session", + "remoteUrl": "https://copilot.example.test/agents/remote-cloud-session", + "capabilities": {"ui": {"elicitation": True}}, + } + return {} + + client._client.request = mock_request + session = await client.create_cloud_session(**_cloud_config()) + + wire = captured["session.create"] + assert "sessionId" not in wire, "sessionId must be omitted from cloud create" + assert wire["cloud"]["repository"]["owner"] == "github" + assert wire["cloud"]["repository"]["name"] == "copilot-sdk" + assert wire["cloud"]["repository"]["branch"] == "main" + assert "provider" not in wire + + assert session.session_id == "remote-cloud-session" + assert session.remote_url == "https://copilot.example.test/agents/remote-cloud-session" + assert session.capabilities.get("ui", {}).get("elicitation") is True + finally: + await client.force_stop() + + +# --------------------------------------------------------------------------- +# Test 3: rejects caller-provided session_id +# --------------------------------------------------------------------------- + + +class TestCreateCloudSessionRejectsSessionId: + @pytest.mark.asyncio + async def test_rejects_caller_session_id(self): + """Passing session_id must raise ValueError naming session_id.""" + client = CopilotClient(connection=RuntimeConnection.for_stdio(path=CLI_PATH)) + with pytest.raises(ValueError, match="session_id"): + await client.create_cloud_session(**_cloud_config(), session_id="caller-id") + + +# --------------------------------------------------------------------------- +# Test 4: rejects caller-provided provider +# --------------------------------------------------------------------------- + + +class TestCreateCloudSessionRejectsProvider: + @pytest.mark.asyncio + async def test_rejects_caller_provider(self): + """Passing provider must raise ValueError naming provider.""" + client = CopilotClient(connection=RuntimeConnection.for_stdio(path=CLI_PATH)) + with pytest.raises(ValueError, match="provider"): + await client.create_cloud_session( + **_cloud_config(), + provider=ProviderConfig(type="openai", base_url="https://api.example.test/v1"), + ) + + +# --------------------------------------------------------------------------- +# Test 5: requires cloud +# --------------------------------------------------------------------------- + + +class TestCreateCloudSessionRequiresCloud: + @pytest.mark.asyncio + async def test_requires_cloud(self): + """Omitting cloud (or passing None) must raise ValueError mentioning cloud.""" + client = CopilotClient(connection=RuntimeConnection.for_stdio(path=CLI_PATH)) + with pytest.raises(ValueError, match="cloud"): + await client.create_cloud_session() + + +# --------------------------------------------------------------------------- +# Test 6: buffers early session.event notifications +# --------------------------------------------------------------------------- + + +class TestCreateCloudSessionBuffersEarlyNotifications: + @pytest.mark.asyncio + async def test_early_notifications_dispatched_after_registration(self): + """session.event notifications arriving before registration are buffered and replayed.""" + client = CopilotClient(connection=RuntimeConnection.for_stdio(path=CLI_PATH)) + await client.start() + try: + create_response_gate: asyncio.Future[dict] = asyncio.get_event_loop().create_future() + + async def mock_request(method, params): + if method == "session.create": + return await create_response_gate + return {} + + client._client.request = mock_request + + session_id = "remote-cloud-session" + received_events: list = [] + + create_task = asyncio.ensure_future( + client.create_cloud_session( + **_cloud_config(), + on_event=lambda e: received_events.append(e), + ) + ) + + # Yield control so create_cloud_session enters pending-routing mode. + await asyncio.sleep(0) + await asyncio.sleep(0) + + # Inject a session.event notification while the create is in flight. + notification_handler = client._client.notification_handler + assert notification_handler is not None, "notification handler not registered" + notification_handler( + "session.event", + { + "sessionId": session_id, + "event": _make_event_dict(), + }, + ) + + # Verify it is buffered (not yet dispatched — session not registered yet). + await asyncio.sleep(0) + assert not received_events, "event dispatched before session was registered" + + # Allow session.create to respond; this registers the session. + create_response_gate.set_result({"sessionId": session_id}) + await asyncio.wait_for(create_task, timeout=5.0) + + # Give the event loop a tick to flush the buffered event. + await asyncio.sleep(0) + + assert len(received_events) == 1, ( + f"expected 1 buffered event to be replayed, got {len(received_events)}" + ) + # Our synthetic event uses an unknown type; just confirm it was dispatched. + assert received_events[0].raw_type == "session.buffered_test" + finally: + await client.force_stop() + + +# --------------------------------------------------------------------------- +# Test 7: parks inbound requests until registration +# --------------------------------------------------------------------------- + + +class TestCreateCloudSessionParksInboundRequests: + @pytest.mark.asyncio + async def test_parked_user_input_resolves_after_registration(self): + """userInput.request that arrives before registration is parked, then resolved.""" + answered: list[str] = [] + + async def color_picker(request: UserInputRequest, context: dict) -> UserInputResponse: + answered.append(request["question"]) + return UserInputResponse(answer="blue", wasFreeform=True) + + client = CopilotClient(connection=RuntimeConnection.for_stdio(path=CLI_PATH)) + await client.start() + try: + create_response_gate: asyncio.Future[dict] = asyncio.get_event_loop().create_future() + + async def mock_request(method, params): + if method == "session.create": + return await create_response_gate + return {} + + client._client.request = mock_request + + session_id = "remote-cloud-session" + create_task = asyncio.ensure_future( + client.create_cloud_session(**_cloud_config(), on_user_input_request=color_picker) + ) + + # Yield so pending-routing mode is entered. + await asyncio.sleep(0) + await asyncio.sleep(0) + + # Dispatch a userInput.request while the create is in flight. + user_input_handler = client._client.request_handlers.get("userInput.request") + assert user_input_handler is not None, "userInput.request handler not registered" + + input_task = asyncio.ensure_future( + user_input_handler( + { + "sessionId": session_id, + "question": "Pick a color", + "choices": ["red", "blue"], + "allowFreeform": True, + } + ) + ) + + # Yield to let the handler park on the pending future. + await asyncio.sleep(0) + assert not input_task.done(), "handler should be parked waiting for session" + + # Now let the create response arrive; this registers the session. + create_response_gate.set_result({"sessionId": session_id}) + await asyncio.wait_for(create_task, timeout=5.0) + + # The parked userInput handler should now complete. + result = await asyncio.wait_for(input_task, timeout=5.0) + assert result["answer"] == "blue" + assert result["wasFreeform"] is True + assert answered == ["Pick a color"] + finally: + await client.force_stop() From 1a469905536822cf4ea16b818d11b5b86a3685d1 Mon Sep 17 00:00:00 2001 From: Timothy Clem Date: Fri, 22 May 2026 19:35:11 -0700 Subject: [PATCH 03/14] Java SDK: add CopilotClient.createCloudSession - 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> --- .../com/github/copilot/sdk/CopilotClient.java | 173 ++++++- .../copilot/sdk/PendingRoutingState.java | 183 ++++++++ .../copilot/sdk/RpcHandlerDispatcher.java | 155 ++++++- .../copilot/sdk/SessionRequestBuilder.java | 76 +++ .../github/copilot/sdk/CloudSessionTest.java | 433 ++++++++++++++++++ .../copilot/sdk/RpcHandlerDispatcherTest.java | 2 +- 6 files changed, 1002 insertions(+), 20 deletions(-) create mode 100644 java/src/main/java/com/github/copilot/sdk/PendingRoutingState.java create mode 100644 java/src/test/java/com/github/copilot/sdk/CloudSessionTest.java diff --git a/java/src/main/java/com/github/copilot/sdk/CopilotClient.java b/java/src/main/java/com/github/copilot/sdk/CopilotClient.java index 4d0770319..15391d638 100644 --- a/java/src/main/java/com/github/copilot/sdk/CopilotClient.java +++ b/java/src/main/java/com/github/copilot/sdk/CopilotClient.java @@ -81,6 +81,7 @@ public final class CopilotClient implements AutoCloseable { private final CliServerManager serverManager; private final LifecycleEventManager lifecycleManager = new LifecycleEventManager(); private final Map sessions = new ConcurrentHashMap<>(); + private final PendingRoutingState pendingRoutingState = new PendingRoutingState(); private volatile CompletableFuture connectionFuture; private volatile boolean disposed = false; private final String optionsHost; @@ -210,7 +211,7 @@ private Connection startCoreBody() { // Register handlers for server-to-client calls RpcHandlerDispatcher dispatcher = new RpcHandlerDispatcher(sessions, lifecycleManager::dispatch, - options.getExecutor()); + options.getExecutor(), pendingRoutingState); dispatcher.registerHandlers(rpc); // Verify protocol version @@ -426,6 +427,10 @@ public CompletableFuture createSession(SessionConfig config) { + "For example, to allow all permissions, use: " + "new SessionConfig().setOnPermissionRequest(PermissionHandler.APPROVE_ALL)")); } + if (config.getCloud() != null) { + return CompletableFuture.failedFuture(new IllegalArgumentException( + "CopilotClient.createSession does not support cloud sessions; use createCloudSession instead.")); + } return ensureConnected().thenCompose(connection -> { long totalNanos = System.nanoTime(); // Pre-generate session ID so the session can be registered before the RPC call, @@ -487,6 +492,172 @@ public CompletableFuture createSession(SessionConfig config) { }); } + /** + * Creates a Mission Control–backed cloud session. + * + *

+ * The runtime owns the session ID for cloud sessions. Do not + * set {@link SessionConfig#setSessionId(String) sessionId} or + * {@link SessionConfig#setProvider(com.github.copilot.sdk.json.ProviderConfig) + * provider} on the config; the SDK rejects both with + * {@link IllegalArgumentException}. The config must have + * {@link SessionConfig#setCloud(com.github.copilot.sdk.json.CloudSessionOptions) + * cloud} set; + * {@link SessionConfig#setOnPermissionRequest(com.github.copilot.sdk.json.PermissionHandler) + * onPermissionRequest} is required. + * + *

+ * The SDK omits {@code sessionId} from the {@code session.create} wire request + * and registers the returned session under the id the runtime assigns. Any + * {@code session.event} notifications or inbound JSON-RPC requests that arrive + * between sending {@code session.create} and receiving its response are + * buffered (bounded, drop-oldest, up to + * {@value PendingRoutingState#BUFFER_LIMIT} per session id) and replayed once + * the session is registered. + * + *

+ * Example: + * + * 

{@code
+     * var session = client.createCloudSession(new SessionConfig()
+     * 		.setCloud(new CloudSessionOptions()
+     * 				.setRepository(new CloudSessionRepository().setOwner("github").setName("copilot-sdk")))
+     * 		.setOnPermissionRequest(PermissionHandler.APPROVE_ALL)).get();
+     * }
+ * + * @param config + * configuration for the cloud session; must have {@code cloud} set + * and an {@code onPermissionRequest} handler; must not have + * {@code sessionId} or {@code provider} set + * @return a future that resolves with the created {@link CopilotSession} + * @throws IllegalArgumentException + * if validation fails (see above) + * @see SessionConfig#setCloud(com.github.copilot.sdk.json.CloudSessionOptions) + * @see com.github.copilot.sdk.json.PermissionHandler#APPROVE_ALL + * @since 1.6.0 + */ + public CompletableFuture createCloudSession(SessionConfig config) { + if (config == null || config.getOnPermissionRequest() == null) { + return CompletableFuture.failedFuture( + new IllegalArgumentException("An onPermissionRequest handler is required when creating a session. " + + "For example, to allow all permissions, use: " + + "new SessionConfig().setOnPermissionRequest(PermissionHandler.APPROVE_ALL)")); + } + if (config.getCloud() == null) { + return CompletableFuture.failedFuture( + new IllegalArgumentException("CopilotClient.createCloudSession requires config.cloud to be set.")); + } + if (config.getSessionId() != null && !config.getSessionId().isEmpty()) { + return CompletableFuture.failedFuture(new IllegalArgumentException( + "CopilotClient.createCloudSession does not support a caller-provided sessionId; " + + "the runtime assigns one.")); + } + if (config.getProvider() != null) { + return CompletableFuture.failedFuture( + new IllegalArgumentException("CopilotClient.createCloudSession does not support config.provider; " + + "cloud sessions use the runtime's provider.")); + } + + return ensureConnected().thenCompose(connection -> { + long totalNanos = System.nanoTime(); + + // Enter pending-routing mode before sending session.create so that any + // session.event notifications or inbound RPC requests that arrive during + // the in-flight RPC are buffered rather than dropped. + pendingRoutingState.incrementGuard(); + + var request = SessionRequestBuilder.buildCloudCreateRequest(config); + + // Extract transform callbacks from the system message config. + var extracted = SessionRequestBuilder.extractTransformCallbacks(config.getSystemMessage()); + if (extracted.wireSystemMessage() != config.getSystemMessage()) { + request.setSystemMessage(extracted.wireSystemMessage()); + } + + long rpcNanos = System.nanoTime(); + return connection.rpc.invoke("session.create", request, CreateSessionResponse.class).thenApply(response -> { + LoggingHelpers.logTiming(LOG, Level.FINE, + "CopilotClient.createCloudSession session.create completed. Elapsed={Elapsed}", rpcNanos); + + String returnedId = response.sessionId(); + if (returnedId == null || returnedId.isEmpty()) { + // No id: release the guard and surface the error. Any runtime session + // created on the other side may leak — we have nothing to destroy. + var staleWaiters = pendingRoutingState.decrementGuard(); + completeWaitersExceptionally(staleWaiters, + "Cloud session.create completed without registering this sessionId; request dropped"); + LOG.warning("Cloud session.create response missing sessionId; runtime session may leak"); + throw new RuntimeException( + "Cloud session.create response did not include a sessionId; cannot register session."); + } + + var session = new CopilotSession(returnedId, connection.rpc); + if (options.getExecutor() != null) { + session.setExecutor(options.getExecutor()); + } + SessionRequestBuilder.configureSession(session, config); + if (extracted.transformCallbacks() != null) { + session.registerTransformCallbacks(extracted.transformCallbacks()); + } + + try { + // Atomically register the session in the sessions map and drain any + // buffered events/parked waiters. The sessions.put happens inside the + // PendingRoutingState lock so concurrent tryBufferNotification / + // tryParkRequest calls see the session as registered. + var flush = pendingRoutingState.registerAndFlush(returnedId, session, sessions); + + session.setWorkspacePath(response.workspacePath()); + session.setCapabilities(response.capabilities()); + + // Replay buffered session.event notifications + for (var event : flush.events()) { + session.dispatchEvent(event); + } + // Complete parked request waiters + for (var waiter : flush.waiters()) { + waiter.complete(session); + } + } catch (Exception e) { + // Roll back: remove session from map, release guard with exceptional completion + sessions.remove(returnedId); + var staleWaiters = pendingRoutingState.decrementGuard(); + completeWaitersExceptionally(staleWaiters, + "Cloud session post-registration setup failed; request dropped"); + LoggingHelpers.logTiming(LOG, Level.WARNING, e, + "CopilotClient.createCloudSession post-registration setup failed. Elapsed={Elapsed}", + totalNanos); + throw e instanceof RuntimeException re ? re : new RuntimeException(e); + } + + var staleWaiters = pendingRoutingState.decrementGuard(); + completeWaitersExceptionally(staleWaiters, + "Cloud session.create completed without registering this sessionId; request dropped"); + + LoggingHelpers.logTiming(LOG, Level.FINE, + "CopilotClient.createCloudSession complete. Elapsed={Elapsed}, SessionId=" + returnedId, + totalNanos); + return session; + }).exceptionally(ex -> { + var staleWaiters = pendingRoutingState.decrementGuard(); + completeWaitersExceptionally(staleWaiters, "Cloud session.create failed; request dropped"); + LoggingHelpers.logTiming(LOG, Level.WARNING, ex, + "CopilotClient.createCloudSession failed. Elapsed={Elapsed}", totalNanos); + throw ex instanceof RuntimeException re ? re : new RuntimeException(ex); + }); + }); + } + + private static void completeWaitersExceptionally(java.util.List> waiters, + String message) { + if (!waiters.isEmpty()) { + var ex = new RuntimeException(message); + for (var waiter : waiters) { + waiter.completeExceptionally(ex); + } + } + } + /** * Resumes an existing Copilot session. *

diff --git a/java/src/main/java/com/github/copilot/sdk/PendingRoutingState.java b/java/src/main/java/com/github/copilot/sdk/PendingRoutingState.java new file mode 100644 index 000000000..a0b2e0852 --- /dev/null +++ b/java/src/main/java/com/github/copilot/sdk/PendingRoutingState.java @@ -0,0 +1,183 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------------------------------------------*/ + +package com.github.copilot.sdk; + +import java.util.ArrayDeque; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.CompletableFuture; +import java.util.logging.Logger; + +import com.github.copilot.sdk.generated.SessionEvent; + +/** + * Thread-safe state for pending-routing mode used by + * {@link CopilotClient#createCloudSession}. + * + *

+ * While one or more cloud {@code session.create} calls are in flight (guard + * count {@code > 0}), notifications and inbound RPC requests addressed to + * session ids that are not yet registered are buffered here rather than + * dropped. Once {@link CopilotClient#createCloudSession} receives the + * runtime-assigned session id, it calls {@link #registerAndFlush} to atomically + * insert the session into the sessions map, drain any buffered events into it, + * and complete any parked request waiters. + * + *

+ * All mutation methods synchronize on {@code this}. The sessions-map put inside + * {@link #registerAndFlush} is performed while holding the lock so that the + * {@link #tryBufferNotification} / {@link #tryParkRequest} check-then-act is + * free of TOCTOU races. + */ +final class PendingRoutingState { + + static final int BUFFER_LIMIT = 128; + + private static final Logger LOG = Logger.getLogger(PendingRoutingState.class.getName()); + + private int guardCount = 0; + /** Buffered session.event notifications keyed by session id. */ + private final Map> pendingEvents = new HashMap<>(); + /** + * Parked CompletableFutures for inbound RPC requests waiting for a session to + * be registered. + */ + private final Map>> pendingWaiters = new HashMap<>(); + + /** Increment the guard count. Must be matched by {@link #decrementGuard}. */ + synchronized void incrementGuard() { + guardCount++; + } + + /** + * Decrement the guard count. If the count reaches zero, clears all buffered + * events and completes all parked waiters exceptionally. + * + * @return list of waiters that should be completed exceptionally (caller must + * complete them outside this lock to avoid re-entrancy) + */ + synchronized List> decrementGuard() { + guardCount = Math.max(0, guardCount - 1); + if (guardCount == 0) { + pendingEvents.clear(); + var stale = new ArrayList>(); + for (var list : pendingWaiters.values()) { + stale.addAll(list); + } + pendingWaiters.clear(); + return stale; + } + return Collections.emptyList(); + } + + /** + * Attempt to buffer a {@code session.event} notification for a pending session. + * + *

+ * The {@code sessions} map is checked inside this synchronized method so that + * the "session not found → buffer" decision is atomic with + * {@link #registerAndFlush}'s "put in map → flush buffer" operation. + * + * @param sessionId + * the session id from the notification + * @param event + * the parsed event to buffer + * @param sessions + * the live sessions map (checked under lock) + * @return {@code true} if the event was buffered; {@code false} if the session + * is already registered (caller should dispatch directly) or pending + * routing is inactive (caller should drop) + */ + synchronized boolean tryBufferNotification(String sessionId, SessionEvent event, + Map sessions) { + if (sessions.containsKey(sessionId)) { + return false; // session found; caller dispatches directly + } + if (guardCount == 0) { + return false; // no pending routing; drop + } + var queue = pendingEvents.computeIfAbsent(sessionId, k -> new ArrayDeque<>()); + if (queue.size() >= BUFFER_LIMIT) { + queue.pollFirst(); + LOG.warning("Pending session notification buffer full for session " + sessionId + "; dropping oldest"); + } + queue.addLast(event); + return true; + } + + /** + * Attempt to park an inbound RPC request until the session is registered. + * + *

+ * Like {@link #tryBufferNotification}, the {@code sessions} map is checked + * under the lock to avoid TOCTOU races with {@link #registerAndFlush}. + * + * @param sessionId + * the session id from the request params + * @param sessions + * the live sessions map (checked under lock) + * @return a future that will be resolved with the {@link CopilotSession} when + * registered (or completed exceptionally when the guard is dropped), or + * {@code null} if the session is already registered (callers should use + * it directly) or if pending routing is inactive (caller should send + * error) + */ + synchronized CompletableFuture tryParkRequest(String sessionId, + Map sessions) { + CopilotSession existing = sessions.get(sessionId); + if (existing != null) { + return CompletableFuture.completedFuture(existing); + } + if (guardCount == 0) { + return null; // no pending; caller sends error + } + var future = new CompletableFuture(); + pendingWaiters.computeIfAbsent(sessionId, k -> new ArrayList<>()).add(future); + return future; + } + + /** + * Result of {@link #registerAndFlush}: buffered events to dispatch and parked + * waiters to complete. + */ + record FlushResult(List events, List> waiters) { + } + + /** + * Atomically register a session in the sessions map and drain any buffered + * events and parked waiters for that session. + * + *

+ * The {@code sessions.put} is performed inside the lock so that concurrent + * {@link #tryBufferNotification} / {@link #tryParkRequest} callers that haven't + * yet acquired the lock will see the session as registered. + * + * @param sessionId + * the session id to register + * @param session + * the session object + * @param sessions + * the live sessions map; the put is performed under lock + * @return buffered events and parked waiters to dispatch/complete outside the + * lock + */ + synchronized FlushResult registerAndFlush(String sessionId, CopilotSession session, + Map sessions) { + sessions.put(sessionId, session); + + var queue = pendingEvents.remove(sessionId); + var events = queue != null ? new ArrayList<>(queue) : Collections.emptyList(); + + var waiters = pendingWaiters.remove(sessionId); + var futures = waiters != null + ? new ArrayList<>(waiters) + : Collections.>emptyList(); + + return new FlushResult(events, futures); + } +} diff --git a/java/src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java b/java/src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java index 1d76d8b88..6202860a9 100644 --- a/java/src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java +++ b/java/src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java @@ -9,8 +9,11 @@ import java.util.Collections; import java.util.Map; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; import java.util.concurrent.Executor; import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.logging.Level; import java.util.logging.Logger; @@ -49,6 +52,10 @@ final class RpcHandlerDispatcher { private final Map sessions; private final LifecycleEventDispatcher lifecycleDispatcher; private final Executor executor; + private final PendingRoutingState pendingState; + + /** Timeout in seconds to wait for a pending cloud session to register. */ + private static final int PENDING_SESSION_TIMEOUT_SECONDS = 60; /** * Creates a dispatcher with session registry and lifecycle dispatcher. @@ -59,12 +66,16 @@ final class RpcHandlerDispatcher { * callback for dispatching lifecycle events * @param executor * the executor for async dispatch, or {@code null} for default + * @param pendingState + * the pending-routing state for cloud sessions, or {@code null} to + * disable pending routing */ RpcHandlerDispatcher(Map sessions, LifecycleEventDispatcher lifecycleDispatcher, - Executor executor) { + Executor executor, PendingRoutingState pendingState) { this.sessions = sessions; this.lifecycleDispatcher = lifecycleDispatcher; this.executor = executor; + this.pendingState = pendingState; } /** @@ -96,13 +107,28 @@ private void handleSessionEvent(JsonNode params) { JsonNode eventNode = params.get("event"); LOG.fine("Received session.event: " + eventNode); + if (eventNode == null) { + return; + } + SessionEvent event = MAPPER.treeToValue(eventNode, SessionEvent.class); + if (event == null) { + return; + } + + // Fast path: session already registered — dispatch directly. CopilotSession session = sessions.get(sessionId); - if (session != null && eventNode != null) { - SessionEvent event = MAPPER.treeToValue(eventNode, SessionEvent.class); - if (event != null) { - session.dispatchEvent(event); - } + if (session != null) { + session.dispatchEvent(event); + return; + } + + // Slow path: session not found. Attempt to buffer for a pending cloud + // session.create. The tryBufferNotification check is inside the pending + // state's lock so it's atomic with registerAndFlush's sessions.put. + if (pendingState != null && pendingState.tryBufferNotification(sessionId, event, sessions)) { + return; // buffered; will be replayed when session is registered } + // session not registered and no pending routing active; silently drop } catch (Exception e) { LOG.log(Level.SEVERE, "Error handling session event", e); } @@ -141,9 +167,8 @@ private void handleToolCall(JsonRpcClient rpc, String requestId, JsonNode params String toolName = params.get("toolName").asText(); JsonNode arguments = params.get("arguments"); - CopilotSession session = sessions.get(sessionId); + CopilotSession session = resolveSessionForRequest(sessionId, requestIdLong, rpc); if (session == null) { - rpc.sendErrorResponse(requestIdLong, -32602, "Unknown session " + sessionId); return; } @@ -203,7 +228,20 @@ private void handlePermissionRequest(JsonRpcClient rpc, String requestId, JsonNo String sessionId = params.get("sessionId").asText(); JsonNode permissionRequest = params.get("permissionRequest"); + // Try to resolve the session; for a pending cloud session, park until + // registration. If not found and no pending routing, fall back to the + // protocol-correct DENIED_COULD_NOT_REQUEST_FROM_USER response. CopilotSession session = sessions.get(sessionId); + if (session == null && pendingState != null) { + CompletableFuture waiter = pendingState.tryParkRequest(sessionId, sessions); + if (waiter != null) { + try { + session = waiter.get(PENDING_SESSION_TIMEOUT_SECONDS, TimeUnit.SECONDS); + } catch (Exception e) { + session = null; + } + } + } if (session == null) { var result = new PermissionRequestResult() .setKind(PermissionRequestResultKind.DENIED_COULD_NOT_REQUEST_FROM_USER); @@ -254,11 +292,10 @@ private void handleUserInputRequest(JsonRpcClient rpc, String requestId, JsonNod JsonNode choicesNode = params.get("choices"); JsonNode allowFreeformNode = params.get("allowFreeform"); - CopilotSession session = sessions.get(sessionId); + CopilotSession session = resolveSessionForRequest(sessionId, requestIdLong, rpc); LOG.fine("Found session: " + (session != null)); if (session == null) { LOG.fine("Session not found, sending error"); - rpc.sendErrorResponse(requestIdLong, -32602, "Unknown session " + sessionId); return; } @@ -309,9 +346,8 @@ private void handleExitPlanModeRequest(JsonRpcClient rpc, String requestId, Json try { String sessionId = params.get("sessionId").asText(); - CopilotSession session = sessions.get(sessionId); + CopilotSession session = resolveSessionForRequest(sessionId, requestIdLong, rpc); if (session == null) { - rpc.sendErrorResponse(requestIdLong, -32602, "Unknown session " + sessionId); return; } @@ -363,9 +399,8 @@ private void handleAutoModeSwitchRequest(JsonRpcClient rpc, String requestId, Js try { String sessionId = params.get("sessionId").asText(); - CopilotSession session = sessions.get(sessionId); + CopilotSession session = resolveSessionForRequest(sessionId, requestIdLong, rpc); if (session == null) { - rpc.sendErrorResponse(requestIdLong, -32602, "Unknown session " + sessionId); return; } @@ -409,9 +444,8 @@ private void handleHooksInvoke(JsonRpcClient rpc, String requestId, JsonNode par String hookType = params.get("hookType").asText(); JsonNode input = params.get("input"); - CopilotSession session = sessions.get(sessionId); + CopilotSession session = resolveSessionForRequest(sessionId, requestIdLong, rpc); if (session == null) { - rpc.sendErrorResponse(requestIdLong, -32602, "Unknown session " + sessionId); return; } @@ -454,9 +488,8 @@ private void handleSystemMessageTransform(JsonRpcClient rpc, String requestId, J String sessionId = params.has("sessionId") ? params.get("sessionId").asText() : null; JsonNode sections = params.get("sections"); - CopilotSession session = sessionId != null ? sessions.get(sessionId) : null; + CopilotSession session = resolveSessionForRequest(sessionId, requestIdLong, rpc); if (session == null) { - rpc.sendErrorResponse(requestIdLong, -32602, "Unknown session " + sessionId); return; } @@ -499,6 +532,92 @@ private static long parseRequestId(String requestId, String methodName) { } } + /** + * Resolve a session for an inbound RPC request, optionally parking until a + * pending cloud session is registered. + * + *

+ * If the session is already registered, returns it immediately. If pending + * routing is active (a cloud {@code session.create} is in flight), parks this + * call until the session is registered or the guard expires. On failure, sends + * an error response and returns {@code null}. + * + * @param sessionId + * the session id extracted from the request params + * @param requestId + * the JSON-RPC request id (for error responses) + * @param rpc + * the JSON-RPC client + * @return the resolved session, or {@code null} if an error was sent + */ + private CopilotSession resolveSessionForRequest(String sessionId, long requestId, JsonRpcClient rpc) { + if (sessionId == null) { + try { + rpc.sendErrorResponse(requestId, -32602, "Missing sessionId in request"); + } catch (IOException e) { + LOG.log(Level.SEVERE, "Failed to send missing-sessionId error", e); + } + return null; + } + + // Fast path: session already registered + CopilotSession session = sessions.get(sessionId); + if (session != null) { + return session; + } + + if (pendingState != null) { + CompletableFuture waiter = pendingState.tryParkRequest(sessionId, sessions); + if (waiter != null) { + if (waiter.isDone()) { + // Resolved synchronously (session was already registered under the lock) + try { + return waiter.get(); + } catch (Exception e) { + // fall through to send error + } + } else { + try { + return waiter.get(PENDING_SESSION_TIMEOUT_SECONDS, TimeUnit.SECONDS); + } catch (TimeoutException e) { + try { + rpc.sendErrorResponse(requestId, -32603, "Session " + sessionId + " not registered within " + + PENDING_SESSION_TIMEOUT_SECONDS + "s"); + } catch (IOException ioe) { + LOG.log(Level.SEVERE, "Failed to send timeout error", ioe); + } + return null; + } catch (ExecutionException e) { + Throwable cause = e.getCause(); + try { + rpc.sendErrorResponse(requestId, -32603, "Session " + sessionId + " not registered: " + + (cause != null ? cause.getMessage() : e.getMessage())); + } catch (IOException ioe) { + LOG.log(Level.SEVERE, "Failed to send registration-failed error", ioe); + } + return null; + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + try { + rpc.sendErrorResponse(requestId, -32603, "Interrupted waiting for session " + sessionId); + } catch (IOException ioe) { + LOG.log(Level.SEVERE, "Failed to send interrupted error", ioe); + } + return null; + } + } + } + } + + // No pending routing active or pending routing returned null; send error + try { + rpc.sendErrorResponse(requestId, -32602, "Unknown session " + sessionId); + } catch (IOException e) { + LOG.log(Level.SEVERE, "Failed to send unknown-session error", e); + } + return null; + } + private void runAsync(Runnable task) { try { if (executor != null) { diff --git a/java/src/main/java/com/github/copilot/sdk/SessionRequestBuilder.java b/java/src/main/java/com/github/copilot/sdk/SessionRequestBuilder.java index 0cdc4f942..fd0e4b97a 100644 --- a/java/src/main/java/com/github/copilot/sdk/SessionRequestBuilder.java +++ b/java/src/main/java/com/github/copilot/sdk/SessionRequestBuilder.java @@ -173,6 +173,82 @@ static CreateSessionRequest buildCreateRequest(SessionConfig config) { return buildCreateRequest(config, sessionId); } + /** + * Builds a CreateSessionRequest for cloud session creation. Unlike + * {@link #buildCreateRequest(SessionConfig, String)}, this method omits + * {@code sessionId} from the request — the runtime assigns the session id for + * cloud sessions. + * + * @param config + * the session configuration (may be null) + * @return the built request object with {@code sessionId} left null + */ + static CreateSessionRequest buildCloudCreateRequest(SessionConfig config) { + var request = new CreateSessionRequest(); + // Always request permission callbacks to enable deny-by-default behavior + request.setRequestPermission(true); + // Always send envValueMode=direct for MCP servers + request.setEnvValueMode("direct"); + // sessionId intentionally omitted: the runtime assigns the id for cloud + // sessions + if (config == null) { + return request; + } + + request.setModel(config.getModel()); + request.setClientName(config.getClientName()); + request.setReasoningEffort(config.getReasoningEffort()); + request.setTools(config.getTools()); + request.setSystemMessage(config.getSystemMessage()); + request.setAvailableTools(config.getAvailableTools()); + request.setExcludedTools(config.getExcludedTools()); + // provider intentionally omitted: cloud sessions use the runtime's provider + config.getEnableSessionTelemetry().ifPresent(request::setEnableSessionTelemetry); + if (config.getOnUserInputRequest() != null) { + request.setRequestUserInput(true); + } + if (config.getHooks() != null && config.getHooks().hasHooks()) { + request.setHooks(true); + } + request.setWorkingDirectory(config.getWorkingDirectory()); + if (config.isStreaming()) { + request.setStreaming(true); + } + config.getIncludeSubAgentStreamingEvents().ifPresent(request::setIncludeSubAgentStreamingEvents); + request.setMcpServers(config.getMcpServers()); + request.setCustomAgents(config.getCustomAgents()); + request.setDefaultAgent(config.getDefaultAgent()); + request.setAgent(config.getAgent()); + request.setInfiniteSessions(config.getInfiniteSessions()); + request.setSkillDirectories(config.getSkillDirectories()); + request.setInstructionDirectories(config.getInstructionDirectories()); + request.setDisabledSkills(config.getDisabledSkills()); + request.setConfigDir(config.getConfigDir()); + config.getEnableConfigDiscovery().ifPresent(request::setEnableConfigDiscovery); + request.setModelCapabilities(config.getModelCapabilities()); + + if (config.getCommands() != null && !config.getCommands().isEmpty()) { + var wireCommands = config.getCommands().stream() + .map(c -> new CommandWireDefinition(c.getName(), c.getDescription())) + .collect(java.util.stream.Collectors.toList()); + request.setCommands(wireCommands); + } + if (config.getOnElicitationRequest() != null) { + request.setRequestElicitation(true); + } + if (config.getOnExitPlanMode() != null) { + request.setRequestExitPlanMode(true); + } + if (config.getOnAutoModeSwitch() != null) { + request.setRequestAutoModeSwitch(true); + } + request.setGitHubToken(config.getGitHubToken()); + request.setRemoteSession(config.getRemoteSession()); + request.setCloud(config.getCloud()); + + return request; + } + /** * Builds a ResumeSessionRequest from the given session ID and configuration. * diff --git a/java/src/test/java/com/github/copilot/sdk/CloudSessionTest.java b/java/src/test/java/com/github/copilot/sdk/CloudSessionTest.java new file mode 100644 index 000000000..19eb9ed2d --- /dev/null +++ b/java/src/test/java/com/github/copilot/sdk/CloudSessionTest.java @@ -0,0 +1,433 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------------------------------------------*/ + +package com.github.copilot.sdk; + +import static org.junit.jupiter.api.Assertions.*; + +import java.io.InputStream; +import java.lang.reflect.Field; +import java.net.ServerSocket; +import java.net.Socket; +import java.util.Map; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.function.BiConsumer; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; +import com.github.copilot.sdk.json.CloudSessionOptions; +import com.github.copilot.sdk.json.CloudSessionRepository; +import com.github.copilot.sdk.json.CreateSessionRequest; +import com.github.copilot.sdk.json.PermissionHandler; +import com.github.copilot.sdk.json.ProviderConfig; +import com.github.copilot.sdk.json.SessionConfig; +import com.github.copilot.sdk.json.SessionLifecycleEvent; +import com.github.copilot.sdk.json.ToolDefinition; +import com.github.copilot.sdk.json.ToolResultObject; + +/** + * Tests for {@link CopilotClient#createCloudSession} and + * {@link CopilotClient#createSession} cloud-config rejection. + * + *

+ * Covers: + *

    + *
  1. {@code createSession} rejects cloud config
    2. + *
  2. Wire payload omits {@code sessionId} and includes {@code cloud}
    3. + *
  3. {@code createCloudSession} rejects caller-provided {@code sessionId}
    4. + *
  4. {@code createCloudSession} rejects caller-provided {@code provider}
    5. + *
  5. {@code createCloudSession} requires {@code cloud} to be set
    6. + *
  6. Early {@code session.event} notifications are buffered and replayed
    7. + *
  7. Inbound RPC requests are parked until the session is registered
    8. + *
+ */ +class CloudSessionTest { + + private static final ObjectMapper MAPPER = JsonRpcClient.getObjectMapper(); + private static final int SOCKET_TIMEOUT_MS = 5000; + + // Socket-pair fields used by routing-related tests + private Socket clientSideSocket; + private Socket serverSideSocket; + private JsonRpcClient rpc; + private Map sessions; + private CopyOnWriteArrayList lifecycleEvents; + private PendingRoutingState pendingState; + private RpcHandlerDispatcher dispatcher; + private InputStream responseStream; + private Map> handlers; + + @BeforeEach + void setup() throws Exception { + try (ServerSocket ss = new ServerSocket(0)) { + clientSideSocket = new Socket("localhost", ss.getLocalPort()); + serverSideSocket = ss.accept(); + } + serverSideSocket.setSoTimeout(SOCKET_TIMEOUT_MS); + + rpc = JsonRpcClient.fromSocket(clientSideSocket); + responseStream = serverSideSocket.getInputStream(); + + sessions = new ConcurrentHashMap<>(); + lifecycleEvents = new CopyOnWriteArrayList<>(); + pendingState = new PendingRoutingState(); + + dispatcher = new RpcHandlerDispatcher(sessions, lifecycleEvents::add, null, pendingState); + dispatcher.registerHandlers(rpc); + + // Extract registered handlers via reflection (same pattern as + // RpcHandlerDispatcherTest) + Field f = JsonRpcClient.class.getDeclaredField("notificationHandlers"); + f.setAccessible(true); + @SuppressWarnings("unchecked") + Map> h = (Map>) f.get(rpc); + handlers = h; + } + + @AfterEach + void teardown() throws Exception { + if (rpc != null) { + rpc.close(); + } + if (serverSideSocket != null) { + serverSideSocket.close(); + } + if (clientSideSocket != null) { + clientSideSocket.close(); + } + } + + private void invokeHandler(String method, String requestId, JsonNode params) { + handlers.get(method).accept(requestId, params); + } + + private JsonNode readResponse() throws Exception { + StringBuilder header = new StringBuilder(); + while (!header.toString().endsWith("\r\n\r\n")) { + int b = responseStream.read(); + if (b == -1) { + throw new java.io.IOException("Unexpected end of stream"); + } + header.append((char) b); + } + String headerStr = header.toString().trim(); + int idx = headerStr.indexOf(':'); + int contentLength = Integer.parseInt(headerStr.substring(idx + 1).trim()); + byte[] body = responseStream.readNBytes(contentLength); + return MAPPER.readTree(body); + } + + // ========================================================================= + // Test 1: createSession rejects cloud config + // ========================================================================= + + @Test + void createSession_rejectsCloudConfig() { + var client = new CopilotClient(); + var config = new SessionConfig().setCloud(new CloudSessionOptions()) + .setOnPermissionRequest(PermissionHandler.APPROVE_ALL); + + var future = client.createSession(config); + + var ex = assertThrows(ExecutionException.class, future::get, "createSession should fail with cloud config set"); + assertInstanceOf(IllegalArgumentException.class, ex.getCause(), "Cause should be IllegalArgumentException"); + assertTrue(ex.getCause().getMessage().contains("cloud"), "Error message should mention 'cloud'"); + + try { + client.forceStop().get(5, TimeUnit.SECONDS); + } catch (Exception ignored) { + } + } + + // ========================================================================= + // Test 2: wire payload omits sessionId and includes cloud + // ========================================================================= + + @Test + void buildCloudCreateRequest_omitsSessionIdAndIncludesCloud() throws Exception { + var cloud = new CloudSessionOptions() + .setRepository(new CloudSessionRepository().setOwner("github").setName("copilot-sdk")); + var config = new SessionConfig().setCloud(cloud).setModel("gpt-5") + .setOnPermissionRequest(PermissionHandler.APPROVE_ALL); + + CreateSessionRequest request = SessionRequestBuilder.buildCloudCreateRequest(config); + + // Java-level assertions + assertNull(request.getSessionId(), "sessionId must be null on the cloud create request"); + assertNotNull(request.getCloud(), "cloud must be set on the cloud create request"); + assertEquals("gpt-5", request.getModel()); + + // Serialize to JSON and verify wire shape + String json = MAPPER.writeValueAsString(request); + JsonNode tree = MAPPER.readTree(json); + + assertFalse(tree.has("sessionId"), "sessionId must be absent from serialized JSON (NON_NULL omits it)"); + assertTrue(tree.has("cloud"), "cloud must be present in serialized JSON"); + assertTrue(tree.get("cloud").has("repository"), "cloud.repository must be present"); + assertEquals("github", tree.get("cloud").get("repository").get("owner").asText()); + } + + @Test + void buildCloudCreateRequest_sessionIdOmittedEvenWhenModelIsNull() throws Exception { + // Minimal config: only cloud set, no model + var config = new SessionConfig().setCloud(new CloudSessionOptions()); + + CreateSessionRequest request = SessionRequestBuilder.buildCloudCreateRequest(config); + + assertNull(request.getSessionId()); + String json = MAPPER.writeValueAsString(request); + JsonNode tree = MAPPER.readTree(json); + assertFalse(tree.has("sessionId"), "sessionId must never appear in cloud create wire payload"); + assertTrue(tree.has("cloud")); + } + + // ========================================================================= + // Test 3: createCloudSession rejects caller-provided sessionId + // ========================================================================= + + @Test + void createCloudSession_rejectsCallerSessionId() { + var client = new CopilotClient(); + var config = new SessionConfig().setCloud(new CloudSessionOptions()).setSessionId("my-caller-session") + .setOnPermissionRequest(PermissionHandler.APPROVE_ALL); + + var future = client.createCloudSession(config); + + var ex = assertThrows(ExecutionException.class, future::get, + "createCloudSession should fail when sessionId is set"); + assertInstanceOf(IllegalArgumentException.class, ex.getCause()); + assertTrue(ex.getCause().getMessage().contains("sessionId"), "Error message should mention 'sessionId'"); + + try { + client.forceStop().get(5, TimeUnit.SECONDS); + } catch (Exception ignored) { + } + } + + // ========================================================================= + // Test 4: createCloudSession rejects caller-provided provider + // ========================================================================= + + @Test + void createCloudSession_rejectsCallerProvider() { + var client = new CopilotClient(); + var config = new SessionConfig().setCloud(new CloudSessionOptions()) + .setProvider(new ProviderConfig().setType("openai")) + .setOnPermissionRequest(PermissionHandler.APPROVE_ALL); + + var future = client.createCloudSession(config); + + var ex = assertThrows(ExecutionException.class, future::get, + "createCloudSession should fail when provider is set"); + assertInstanceOf(IllegalArgumentException.class, ex.getCause()); + assertTrue(ex.getCause().getMessage().contains("provider"), "Error message should mention 'provider'"); + + try { + client.forceStop().get(5, TimeUnit.SECONDS); + } catch (Exception ignored) { + } + } + + // ========================================================================= + // Test 5: createCloudSession requires cloud to be set + // ========================================================================= + + @Test + void createCloudSession_requiresCloud() { + var client = new CopilotClient(); + var config = new SessionConfig().setModel("gpt-5").setOnPermissionRequest(PermissionHandler.APPROVE_ALL); + // cloud is NOT set + + var future = client.createCloudSession(config); + + var ex = assertThrows(ExecutionException.class, future::get, + "createCloudSession should fail when cloud is not set"); + assertInstanceOf(IllegalArgumentException.class, ex.getCause()); + assertTrue(ex.getCause().getMessage().contains("cloud"), "Error message should mention 'cloud'"); + + try { + client.forceStop().get(5, TimeUnit.SECONDS); + } catch (Exception ignored) { + } + } + + // ========================================================================= + // Test 6: early session.event notifications are buffered and replayed + // ========================================================================= + + @Test + void bufferEarlySessionEventNotifications() throws Exception { + // Enter pending routing mode (simulates createCloudSession in-flight) + pendingState.incrementGuard(); + + String pendingSessionId = "cloud-session-abc"; + + // Dispatch a session.event while no session is registered yet. + ObjectNode params = MAPPER.createObjectNode(); + params.put("sessionId", pendingSessionId); + ObjectNode event = params.putObject("event"); + event.put("type", "sessionStart"); + ObjectNode data = event.putObject("data"); + data.put("sessionId", pendingSessionId); + + invokeHandler("session.event", null, params); + + // Give the (synchronous) handler a moment — no session registered yet, so the + // event should be buffered, not dispatched. + Thread.sleep(50); + + // Create the session object and register it via registerAndFlush, which + // atomically inserts the session into the map and drains the buffer. + var session = new CopilotSession(pendingSessionId, rpc); + var dispatched = new CopyOnWriteArrayList<>(); + session.on(dispatched::add); + + var flush = pendingState.registerAndFlush(pendingSessionId, session, sessions); + + // Replay buffered events into the session (simulates what createCloudSession + // does) + for (var buffered : flush.events()) { + session.dispatchEvent(buffered); + } + + // Complete parked waiters (none in this test) + for (var waiter : flush.waiters()) { + waiter.complete(session); + } + + // Release the guard + var staleWaiters = pendingState.decrementGuard(); + assertTrue(staleWaiters.isEmpty(), "No stale waiters expected"); + + // The buffered session.event should now have been replayed to the session + Thread.sleep(50); + assertEquals(1, dispatched.size(), "Buffered notification should have been replayed to the session"); + } + + @Test + void bufferRespectsSizeLimit() throws Exception { + pendingState.incrementGuard(); + String sid = "cloud-overflow-test"; + + // Send more than BUFFER_LIMIT events + int overLimit = PendingRoutingState.BUFFER_LIMIT + 10; + for (int i = 0; i < overLimit; i++) { + ObjectNode params = MAPPER.createObjectNode(); + params.put("sessionId", sid); + ObjectNode event = params.putObject("event"); + event.put("type", "assistantMessage"); + event.putObject("data").put("content", "msg-" + i); + invokeHandler("session.event", null, params); + } + + var session = new CopilotSession(sid, rpc); + var flush = pendingState.registerAndFlush(sid, session, sessions); + + // Should have been capped at BUFFER_LIMIT; oldest entries were dropped + assertEquals(PendingRoutingState.BUFFER_LIMIT, flush.events().size(), + "Buffer should be capped at BUFFER_LIMIT"); + + pendingState.decrementGuard(); + } + + // ========================================================================= + // Test 7: inbound RPC requests are parked until the session is registered + // ========================================================================= + + @Test + void parksInboundRequestsUntilRegistration() throws Exception { + String pendingSessionId = "cloud-session-xyz"; + + // Register a tool on the (not-yet-created) session by pre-creating it without + // registering in the sessions map yet. We'll use the pending state directly. + pendingState.incrementGuard(); + + // In a background thread, send a tool.call request for the pending session. + // The handler should park until the session is registered. + var toolCallFuture = CompletableFuture.runAsync(() -> { + ObjectNode params = MAPPER.createObjectNode(); + params.put("sessionId", pendingSessionId); + params.put("toolCallId", "tc-1"); + params.put("toolName", "say_hello"); + params.set("arguments", MAPPER.createObjectNode()); + invokeHandler("tool.call", "42", params); + }); + + // Brief pause to allow the handler thread to start and park + Thread.sleep(100); + + // Create and register the session with the requested tool + var session = new CopilotSession(pendingSessionId, rpc); + session.registerTools(java.util.List.of( + ToolDefinition.create("say_hello", "Greets the user", Map.of("type", "object", "properties", Map.of()), + inv -> CompletableFuture.completedFuture(ToolResultObject.success("hello!"))))); + + var flush = pendingState.registerAndFlush(pendingSessionId, session, sessions); + + // No buffered notifications in this test + assertTrue(flush.events().isEmpty(), "No buffered events expected"); + + // Complete any parked request waiters + for (var waiter : flush.waiters()) { + waiter.complete(session); + } + + pendingState.decrementGuard(); + + // Wait for the handler to finish (it was parked on the waiter) + toolCallFuture.get(5, TimeUnit.SECONDS); + + // The tool.call handler should have executed and sent a response back on the + // wire + JsonNode response = readResponse(); + assertNotNull(response, "Should have received a tool response"); + assertEquals(42, response.get("id").asInt(), "Response id should match request id"); + assertNotNull(response.get("result"), "Tool call should produce a result"); + } + + @Test + void parkedRequestFailsExceptionallyWhenGuardDroppedWithoutRegistration() throws Exception { + String pendingSessionId = "cloud-session-dropped"; + + pendingState.incrementGuard(); + + // Park a request in the background + var toolCallFuture = CompletableFuture.runAsync(() -> { + ObjectNode params = MAPPER.createObjectNode(); + params.put("sessionId", pendingSessionId); + params.put("toolCallId", "tc-2"); + params.put("toolName", "any_tool"); + params.set("arguments", MAPPER.createObjectNode()); + invokeHandler("tool.call", "99", params); + }); + + Thread.sleep(100); + + // Drop the guard WITHOUT registering the session → waiters should be completed + // exceptionally + var staleWaiters = pendingState.decrementGuard(); + // Complete them exceptionally to simulate what CopilotClient does on failure + for (var w : staleWaiters) { + w.completeExceptionally(new RuntimeException("Cloud session.create failed; request dropped")); + } + + // The handler should eventually receive the exceptional completion and send an + // error response + toolCallFuture.get(5, TimeUnit.SECONDS); + + JsonNode response = readResponse(); + assertNotNull(response, "Should have received an error response"); + assertEquals(99, response.get("id").asInt()); + assertNotNull(response.get("error"), "Response should be an error (not a result)"); + } +} diff --git a/java/src/test/java/com/github/copilot/sdk/RpcHandlerDispatcherTest.java b/java/src/test/java/com/github/copilot/sdk/RpcHandlerDispatcherTest.java index 7453a7b26..5c5bcbfd6 100644 --- a/java/src/test/java/com/github/copilot/sdk/RpcHandlerDispatcherTest.java +++ b/java/src/test/java/com/github/copilot/sdk/RpcHandlerDispatcherTest.java @@ -66,7 +66,7 @@ void setup() throws Exception { sessions = new ConcurrentHashMap<>(); lifecycleEvents = new CopyOnWriteArrayList<>(); - dispatcher = new RpcHandlerDispatcher(sessions, lifecycleEvents::add, null); + dispatcher = new RpcHandlerDispatcher(sessions, lifecycleEvents::add, null, null); dispatcher.registerHandlers(rpc); // Extract the registered handlers via reflection so we can invoke them directly From b27f91b470db776df3e2fe1a28173326ac03a71c Mon Sep 17 00:00:00 2001 From: Timothy Clem Date: Fri, 22 May 2026 19:42:02 -0700 Subject: [PATCH 04/14] dotnet: add CreateCloudSessionAsync 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> --- dotnet/README.md | 42 ++ dotnet/src/Client.cs | 397 +++++++++++++++++- dotnet/src/Session.cs | 9 + dotnet/test/Unit/CloudSessionTests.cs | 577 ++++++++++++++++++++++++++ 4 files changed, 1018 insertions(+), 7 deletions(-) create mode 100644 dotnet/test/Unit/CloudSessionTests.cs diff --git a/dotnet/README.md b/dotnet/README.md index a9527f447..b7bea362a 100644 --- a/dotnet/README.md +++ b/dotnet/README.md @@ -133,6 +133,47 @@ Resume an existing session. Returns the session with `WorkspacePath` populated i - `OnPermissionRequest` - Optional handler called before each tool execution to approve or deny it. See [Permission Handling](#permission-handling) section. +##### `CreateCloudSessionAsync(CloudSessionConfig config): Task` + +Create a session that is hosted by and routed to a remote Copilot cloud server rather than the local CLI. The returned `CopilotSession` behaves identically to a locally-created session: send messages, receive events, and use tools through the same API. + +A `Cloud` configuration is required; passing `Cloud = null` throws `ArgumentException`. Conversely, passing a `Cloud` config to the regular `CreateSessionAsync` also throws, preventing accidental misconfiguration. + +The session ID is assigned by the cloud server. After `CreateCloudSessionAsync` returns, `session.SessionId` contains the server-assigned ID and `session.RemoteUrl` contains the URL of the cloud server that owns the session. Any RPC requests (user-input, permission, hooks, etc.) that arrive from the server before the session is fully registered are buffered and replayed automatically — your handlers will never miss an early request. + +**CloudSessionConfig:** + +- `Cloud` *(required)* — Cloud routing options: + - `Repository` *(required)* — The repository to route the session to: + - `Nwo` *(required)* — Full `owner/repo` name (e.g. `"github/my-repo"`) + - `Ref` *(optional)* — Branch or tag ref (e.g. `"refs/heads/main"`) + - `SdkOptions` *(optional)* — Pass-through options forwarded to the cloud provider +- `Model`, `ReasoningEffort`, `Tools`, `SystemMessage`, `AvailableTools`, `ExcludedTools`, `Provider`, `Streaming`, `InfiniteSessions`, `OnPermissionRequest`, `OnUserInputRequest`, `Hooks` — Same as `SessionConfig` + +**Returned session properties:** + +- `SessionId` — Server-assigned session ID +- `RemoteUrl` — URL of the cloud server that owns this session + +```csharp +await using var session = await client.CreateCloudSessionAsync(new CloudSessionConfig +{ + Cloud = new CloudSessionOptions + { + Repository = new CloudSessionRepository { Nwo = "github/my-repo" } + }, + OnUserInputRequest = async (request, _) => + { + Console.Write($"{request.Question} "); + return new UserInputResponse { Answer = Console.ReadLine()! }; + } +}); + +Console.WriteLine($"Cloud session: {session.SessionId} at {session.RemoteUrl}"); +var reply = await session.SendAsync(new MessageOptions { Content = "Hello from the cloud!" }); +Console.WriteLine(reply); +``` + ##### `PingAsync(string? message = null): Task` Ping the server to check connectivity. @@ -193,6 +234,7 @@ Represents a single conversation session. - `SessionId` - The unique identifier for this session - `WorkspacePath` - Path to the session workspace directory when infinite sessions are enabled. Contains `checkpoints/`, `plan.md`, and `files/` subdirectories. Null if infinite sessions are disabled. +- `RemoteUrl` - URL of the cloud server that owns this session. Non-null only for sessions created via `CreateCloudSessionAsync`. #### Methods diff --git a/dotnet/src/Client.cs b/dotnet/src/Client.cs index a5cc62354..c693d57b1 100644 --- a/dotnet/src/Client.cs +++ b/dotnet/src/Client.cs @@ -83,6 +83,16 @@ public sealed partial class CopilotClient : IDisposable, IAsyncDisposable private readonly object _lifecycleHandlersLock = new(); private ServerRpc? _serverRpc; + // Pending-routing state for cloud session.create in-flight windows. + // While _pendingRoutingCount > 0, notifications and inbound requests + // addressed to not-yet-registered session ids are buffered and replayed + // once the runtime-assigned id is registered. + private int _pendingRoutingCount; + private readonly Dictionary> _pendingSessionEvents = new(); + private readonly Dictionary>> _pendingSessionWaiters = new(); + private readonly object _pendingLock = new(); + private const int PendingSessionBufferLimit = 128; + private sealed record LifecycleSubscription(Type EventType, Action Handler); /// @@ -520,6 +530,12 @@ public async Task CreateSessionAsync(SessionConfig config, Cance { ArgumentNullException.ThrowIfNull(config); + if (config.Cloud != null) + { + throw new InvalidOperationException( + "CopilotClient.CreateSessionAsync does not support cloud sessions; use CreateCloudSessionAsync instead."); + } + var connection = await EnsureConnectedAsync(cancellationToken); var totalTimestamp = Stopwatch.GetTimestamp(); @@ -651,6 +667,341 @@ public async Task CreateSessionAsync(SessionConfig config, Cance return session; } + /// + /// Creates a Mission Control–backed cloud session. + /// + /// + /// + /// The runtime owns the session ID for cloud sessions: do not set + /// or on the + /// config (the SDK rejects both). The SDK omits sessionId from the + /// session.create wire payload and registers the resulting session under the id + /// the runtime returns. + /// + /// + /// Any session.event notifications or inbound JSON-RPC requests that arrive between + /// sending session.create and receiving its response are buffered (bounded, + /// drop-oldest, limit 128 per id) and replayed once the returned id is registered, + /// so early events are not lost. + /// + /// + /// Known limitation: inbound sessionFs.* requests are not pending-buffered. + /// In practice the runtime does not initiate sessionFs.* before the + /// session.create response, so this is theoretical. + /// + /// + /// Configuration for the cloud session. is required. + /// A that can be used to cancel the operation. + /// A task that resolves to the created . + /// Thrown when is null. + /// Thrown when is null, or when + /// or is set on the config. + /// + /// + /// var session = await client.CreateCloudSessionAsync(new SessionConfig + /// { + /// OnPermissionRequest = PermissionHandler.ApproveAll, + /// Cloud = new CloudSessionOptions + /// { + /// Repository = new CloudSessionRepository { Owner = "github", Name = "copilot-sdk", Branch = "main" } + /// } + /// }); + /// Console.WriteLine($"Cloud session id: {session.SessionId}"); + /// + /// + public async Task CreateCloudSessionAsync(SessionConfig config, CancellationToken cancellationToken = default) + { + ArgumentNullException.ThrowIfNull(config); + + if (config.Cloud == null) + { + throw new InvalidOperationException( + "CopilotClient.CreateCloudSessionAsync requires config.Cloud to be set."); + } + if (!string.IsNullOrEmpty(config.SessionId)) + { + throw new InvalidOperationException( + "CopilotClient.CreateCloudSessionAsync does not support a caller-provided SessionId; the runtime assigns one."); + } + if (config.Provider != null) + { + throw new InvalidOperationException( + "CopilotClient.CreateCloudSessionAsync does not support config.Provider; cloud sessions use the runtime's provider."); + } + + var connection = await EnsureConnectedAsync(cancellationToken); + var totalTimestamp = Stopwatch.GetTimestamp(); + + var hasHooks = config.Hooks != null && ( + config.Hooks.OnPreToolUse != null || + config.Hooks.OnPreMcpToolCall != null || + config.Hooks.OnPostToolUse != null || + config.Hooks.OnUserPromptSubmitted != null || + config.Hooks.OnSessionStart != null || + config.Hooks.OnSessionEnd != null || + config.Hooks.OnErrorOccurred != null); + + var (wireSystemMessage, transformCallbacks) = ExtractTransformCallbacks(config.SystemMessage); + + // Begin pending-routing mode so notifications/requests that arrive + // before the runtime returns the session id are buffered. + var guard = BeginPendingSessionRouting(); + + CreateSessionResponse response; + try + { + var (traceparent, tracestate) = TelemetryHelpers.GetTraceContext(); + + // sessionId is intentionally omitted (null) on the cloud path: + // the runtime assigns the Mission Control session id. + var request = new CreateSessionRequest( + config.Model, + null /* sessionId omitted */, + config.ClientName, + config.ReasoningEffort, + config.Tools?.Select(ToolDefinition.FromAIFunction).ToList(), + wireSystemMessage, + config.AvailableTools, + config.ExcludedTools, + null /* Provider not allowed on cloud path */, + config.EnableSessionTelemetry, + config.OnPermissionRequest != null ? true : null, + config.OnUserInputRequest != null ? true : null, + config.OnExitPlanModeRequest != null ? true : null, + config.OnAutoModeSwitchRequest != null ? true : null, + hasHooks ? true : null, + config.WorkingDirectory, + config.Streaming is true ? true : null, + config.IncludeSubAgentStreamingEvents, + config.McpServers, + "direct", + config.CustomAgents, + config.DefaultAgent, + config.Agent, + config.ConfigDir, + config.EnableConfigDiscovery, + config.SkillDirectories, + config.DisabledSkills, + config.InfiniteSessions, + Commands: config.Commands?.Select(c => new CommandWireDefinition(c.Name, c.Description)).ToList(), + RequestElicitation: config.OnElicitationRequest != null, + Traceparent: traceparent, + Tracestate: tracestate, + ModelCapabilities: config.ModelCapabilities, + GitHubToken: config.GitHubToken, + RemoteSession: config.RemoteSession, + Cloud: config.Cloud, + InstructionDirectories: config.InstructionDirectories); + + response = await InvokeRpcAsync( + connection.Rpc, "session.create", [request], cancellationToken); + } + catch (Exception ex) + { + guard.Dispose(); + if (ex is not OperationCanceledException) + { + LoggingHelpers.LogTiming(_logger, LogLevel.Warning, ex, + "CopilotClient.CreateCloudSessionAsync failed during session.create RPC. Elapsed={Elapsed}", + totalTimestamp); + } + throw; + } + + if (string.IsNullOrEmpty(response.SessionId)) + { + // No id to issue session.destroy against; release the guard and surface the error. + // Any runtime session created on the other side may leak. + _logger.LogWarning("Cloud session.create response missing sessionId; runtime session may leak."); + guard.Dispose(); + throw new InvalidOperationException( + "Cloud session.create response did not include a sessionId; cannot register session."); + } + + var sessionId = response.SessionId; + var setupTimestamp = Stopwatch.GetTimestamp(); + var session = new CopilotSession(sessionId, connection.Rpc, _logger, this); + session.RegisterTools(config.Tools ?? []); + session.RegisterPermissionHandler(config.OnPermissionRequest); + session.RegisterCommands(config.Commands); + session.RegisterElicitationHandler(config.OnElicitationRequest); + session.RegisterExitPlanModeHandler(config.OnExitPlanModeRequest); + session.RegisterAutoModeSwitchHandler(config.OnAutoModeSwitchRequest); + if (config.OnUserInputRequest != null) + { + session.RegisterUserInputHandler(config.OnUserInputRequest); + } + if (config.Hooks != null) + { + session.RegisterHooks(config.Hooks); + } + if (transformCallbacks != null) + { + session.RegisterTransformCallbacks(transformCallbacks); + } + if (config.OnEvent != null) + { + session.On(config.OnEvent); + } + LoggingHelpers.LogTiming(_logger, LogLevel.Debug, null, + "CopilotClient.CreateCloudSessionAsync local setup complete. Elapsed={Elapsed}, SessionId={SessionId}", + setupTimestamp, + sessionId); + + try + { + RegisterSession(session); + ConfigureSessionFsHandlers(session, config.CreateSessionFsProvider); + session.StartProcessingEvents(); + session.WorkspacePath = response.WorkspacePath; + session.SetCapabilities(response.Capabilities); + session.RemoteUrl = response.RemoteUrl; + + // Flush buffered notifications and unblock parked request waiters + // now that the session is registered. Must happen before the guard + // is released so nothing races into a still-pending buffer. + FlushPendingForSession(sessionId, session); + } + catch (Exception ex) + { + session.RemoveFromClient(); + guard.Dispose(); + // Destroy the cloud session on the server — we already have the sessionId from the + // session.create response, so we can send session.destroy even though setup failed. + try { await session.DisposeAsync().ConfigureAwait(false); } catch { /* best effort */ } + LoggingHelpers.LogTiming(_logger, LogLevel.Warning, ex, + "CopilotClient.CreateCloudSessionAsync failed during post-response setup. Elapsed={Elapsed}, SessionId={SessionId}", + totalTimestamp, + sessionId); + throw; + } + + guard.Dispose(); + + LoggingHelpers.LogTiming(_logger, LogLevel.Debug, null, + "CopilotClient.CreateCloudSessionAsync complete. Elapsed={Elapsed}, SessionId={SessionId}", + totalTimestamp, + sessionId); + return session; + } + + /// + /// Enter pending-routing mode. While the returned guard is undisposed, + /// notifications and inbound requests addressed to session ids that are + /// not yet registered are buffered (up to + /// per id, drop-oldest) and replayed on registration. When the last guard is + /// disposed without registration, buffered events are dropped and parked request + /// waiters are faulted with an . + /// + private PendingSessionRoutingGuard BeginPendingSessionRouting() + { + lock (_pendingLock) + { + _pendingRoutingCount++; + } + return new PendingSessionRoutingGuard(this); + } + + private void EndPendingSessionRouting() + { + lock (_pendingLock) + { + _pendingRoutingCount--; + if (_pendingRoutingCount == 0) + { + _pendingSessionEvents.Clear(); + var waiters = new List>(); + foreach (var list in _pendingSessionWaiters.Values) + waiters.AddRange(list); + _pendingSessionWaiters.Clear(); + + // Fault pending waiters outside the lock so TCS continuations don't run under it. + foreach (var tcs in waiters) + { + tcs.TrySetException(new InvalidOperationException( + "Cloud session.create completed without registering this sessionId; request dropped.")); + } + } + } + } + + private void FlushPendingForSession(string sessionId, CopilotSession session) + { + List events; + List> waiters; + + lock (_pendingLock) + { + _pendingSessionEvents.TryGetValue(sessionId, out var rawEvents); + _pendingSessionEvents.Remove(sessionId); + events = rawEvents ?? []; + + _pendingSessionWaiters.TryGetValue(sessionId, out var rawWaiters); + _pendingSessionWaiters.Remove(sessionId); + waiters = rawWaiters ?? []; + } + + foreach (var evt in events) + { + session.DispatchEvent(evt); + } + + // Resolve waiters outside the lock so TCS continuations don't run under it. + foreach (var tcs in waiters) + { + tcs.TrySetResult(session); + } + } + + private Task ResolveSessionAsync(string sessionId) + { + var session = GetSession(sessionId); + if (session != null) + { + return Task.FromResult(session); + } + + lock (_pendingLock) + { + // Re-check inside lock to avoid the race where the session was registered + // between the unlock-free GetSession call and acquiring _pendingLock. + session = GetSession(sessionId); + if (session != null) + { + return Task.FromResult(session); + } + + if (_pendingRoutingCount > 0) + { + var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + if (!_pendingSessionWaiters.TryGetValue(sessionId, out var list)) + { + list = []; + _pendingSessionWaiters[sessionId] = list; + } + list.Add(tcs); + return tcs.Task; + } + } + + return Task.FromException(new ArgumentException($"Unknown session {sessionId}")); + } + + private sealed class PendingSessionRoutingGuard : IDisposable + { + private readonly CopilotClient _client; + private bool _disposed; + + internal PendingSessionRoutingGuard(CopilotClient client) => _client = client; + + public void Dispose() + { + if (_disposed) return; + _disposed = true; + _client.EndPendingSessionRouting(); + } + } + /// /// Resumes an existing Copilot session with the specified configuration. /// @@ -1710,14 +2061,45 @@ private class RpcHandler(CopilotClient client) { public void OnSessionEvent(string sessionId, JsonElement? @event) { + if (@event == null) return; + var session = client.GetSession(sessionId); - if (session != null && @event != null) + if (session != null) { var evt = SessionEvent.FromJson(@event.Value.GetRawText()); if (evt != null) { session.DispatchEvent(evt); } + return; + } + + // Session not yet registered — buffer if pending routing is active. + lock (client._pendingLock) + { + if (client._pendingRoutingCount == 0) + { + return; + } + + var parsed = SessionEvent.FromJson(@event.Value.GetRawText()); + if (parsed == null) return; + + if (!client._pendingSessionEvents.TryGetValue(sessionId, out var buf)) + { + buf = []; + client._pendingSessionEvents[sessionId] = buf; + } + + if (buf.Count >= PendingSessionBufferLimit) + { + buf.RemoveAt(0); + client._logger.LogWarning( + "Pending session event buffer full for session {SessionId}; dropping oldest event.", + sessionId); + } + + buf.Add(parsed); } } @@ -1747,7 +2129,7 @@ public void OnSessionLifecycle(string type, string sessionId, JsonElement? metad public async ValueTask OnUserInputRequest(string sessionId, string question, IList? choices = null, bool? allowFreeform = null) { - var session = client.GetSession(sessionId) ?? throw new ArgumentException($"Unknown session {sessionId}"); + var session = await client.ResolveSessionAsync(sessionId); var request = new UserInputRequest { Question = question, @@ -1766,7 +2148,7 @@ public async ValueTask OnExitPlanModeRequest( IList? actions = null, string? recommendedAction = null) { - var session = client.GetSession(sessionId) ?? throw new ArgumentException($"Unknown session {sessionId}"); + var session = await client.ResolveSessionAsync(sessionId); var request = new ExitPlanModeRequest { Summary = summary, @@ -1783,7 +2165,7 @@ public async ValueTask OnAutoModeSwitchRequest( string? errorCode = null, double? retryAfterSeconds = null) { - var session = client.GetSession(sessionId) ?? throw new ArgumentException($"Unknown session {sessionId}"); + var session = await client.ResolveSessionAsync(sessionId); var response = await session.HandleAutoModeSwitchRequestAsync(new AutoModeSwitchRequest { ErrorCode = errorCode, @@ -1794,14 +2176,14 @@ public async ValueTask OnAutoModeSwitchRequest( public async ValueTask OnHooksInvoke(string sessionId, string hookType, JsonElement input) { - var session = client.GetSession(sessionId) ?? throw new ArgumentException($"Unknown session {sessionId}"); + var session = await client.ResolveSessionAsync(sessionId); var output = await session.HandleHooksInvokeAsync(hookType, input); return new HooksInvokeResponse(output); } public async ValueTask OnSystemMessageTransform(string sessionId, JsonElement sections) { - var session = client.GetSession(sessionId) ?? throw new ArgumentException($"Unknown session {sessionId}"); + var session = await client.ResolveSessionAsync(sessionId); return await session.HandleSystemMessageTransformAsync(sections); } } @@ -1888,7 +2270,8 @@ public static ToolDefinition FromAIFunction(AIFunctionDeclaration function) internal record CreateSessionResponse( string SessionId, string? WorkspacePath, - SessionCapabilities? Capabilities = null); + SessionCapabilities? Capabilities = null, + string? RemoteUrl = null); internal record ResumeSessionRequest( string SessionId, diff --git a/dotnet/src/Session.cs b/dotnet/src/Session.cs index 6ad8e14d9..646cc256f 100644 --- a/dotnet/src/Session.cs +++ b/dotnet/src/Session.cs @@ -107,6 +107,15 @@ private sealed record EventSubscription(Type EventType, Action Han /// public string? WorkspacePath { get; internal set; } + /// + /// Gets the remote URL of this cloud session, if available. + /// + /// + /// The Mission Control remote URL returned by the runtime when creating a cloud session, + /// or null for local sessions. + /// + public string? RemoteUrl { get; internal set; } + /// /// Gets the capabilities reported by the host for this session. /// diff --git a/dotnet/test/Unit/CloudSessionTests.cs b/dotnet/test/Unit/CloudSessionTests.cs new file mode 100644 index 000000000..f99303026 --- /dev/null +++ b/dotnet/test/Unit/CloudSessionTests.cs @@ -0,0 +1,577 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------------------------------------------*/ + +#if NET8_0_OR_GREATER +using System.Net; +using System.Net.Sockets; +using System.Text; +using System.Text.Json; +using Xunit; + +namespace GitHub.Copilot.Test.Unit; + +/// +/// Unit tests for and the rejection guard +/// on for cloud configs. +/// +public sealed class CloudSessionTests +{ + // ------------------------------------------------------------------------- + // 1. CreateSessionAsync rejects cloud config + // ------------------------------------------------------------------------- + + [Fact] + public async Task CreateSessionAsync_Rejects_CloudConfig() + { + await using var server = await FakeCloudServer.StartAsync(); + await using var client = new CopilotClient(new CopilotClientOptions { Connection = RuntimeConnection.ForUri(server.Url) }); + + var ex = await Assert.ThrowsAsync(() => + client.CreateSessionAsync(new SessionConfig + { + OnPermissionRequest = PermissionHandler.ApproveAll, + Cloud = new CloudSessionOptions { Repository = new CloudSessionRepository { Owner = "github", Name = "copilot-sdk" } } + })); + + Assert.Contains("CreateCloudSessionAsync", ex.Message); + } + + // ------------------------------------------------------------------------- + // 2. CreateCloudSessionAsync sends session.create with cloud and without sessionId + // (wire-shape correctness: assert sessionId absent from serialized JSON) + // ------------------------------------------------------------------------- + + [Fact] + public async Task CreateCloudSessionAsync_Sends_Create_With_Cloud_And_Without_SessionId() + { + await using var server = await FakeCloudServer.StartAsync(cloudSessionId: "remote-cloud-session"); + await using var client = new CopilotClient(new CopilotClientOptions { Connection = RuntimeConnection.ForUri(server.Url) }); + + await using var session = await client.CreateCloudSessionAsync(new SessionConfig + { + OnPermissionRequest = PermissionHandler.ApproveAll, + Cloud = new CloudSessionOptions + { + Repository = new CloudSessionRepository { Owner = "github", Name = "copilot-sdk", Branch = "main" } + } + }); + + // Verify the session id is the runtime-assigned one. + Assert.Equal("remote-cloud-session", session.SessionId); + + // Verify the wire payload captured by the server had no sessionId and had cloud. + var payload = server.LastCreatePayload; + Assert.NotNull(payload); + Assert.False(payload!.Value.TryGetProperty("sessionId", out _), + "session.create payload must not contain 'sessionId' on the cloud path."); + Assert.True(payload.Value.TryGetProperty("cloud", out var cloud)); + Assert.Equal("github", cloud.GetProperty("repository").GetProperty("owner").GetString()); + Assert.Equal("copilot-sdk", cloud.GetProperty("repository").GetProperty("name").GetString()); + Assert.Equal("main", cloud.GetProperty("repository").GetProperty("branch").GetString()); + } + + // Supplementary serialization-layer assertion: JsonSerializer.Serialize on a + // CreateSessionRequest with SessionId=null must not emit the key. + [Fact] + public void CreateSessionRequest_WithNullSessionId_DoesNotEmitSessionIdKey() + { + // Retrieve the private serializer options the SDK uses (same approach as SerializationTests). + var prop = typeof(CopilotClient) + .GetProperty("SerializerOptionsForMessageFormatter", + System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static); + var options = (JsonSerializerOptions?)prop?.GetValue(null); + Assert.NotNull(options); + + var requestType = typeof(CopilotClient) + .GetNestedType("CreateSessionRequest", System.Reflection.BindingFlags.NonPublic); + Assert.NotNull(requestType); + + // Build a request with SessionId = null (the cloud path). + var instance = System.Runtime.CompilerServices.RuntimeHelpers.GetUninitializedObject(requestType!); + var cloudField = requestType!.GetField("k__BackingField", + System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic); + cloudField?.SetValue(instance, new CloudSessionOptions + { + Repository = new CloudSessionRepository { Owner = "o", Name = "r" } + }); + + var json = JsonSerializer.Serialize(instance, requestType, options!); + + Assert.DoesNotContain("\"sessionId\"", json); + Assert.Contains("\"cloud\"", json); + } + + // ------------------------------------------------------------------------- + // 3. Rejects caller-provided SessionId + // ------------------------------------------------------------------------- + + [Fact] + public async Task CreateCloudSessionAsync_Rejects_CallerSessionId() + { + await using var server = await FakeCloudServer.StartAsync(); + await using var client = new CopilotClient(new CopilotClientOptions { Connection = RuntimeConnection.ForUri(server.Url) }); + + var ex = await Assert.ThrowsAsync(() => + client.CreateCloudSessionAsync(new SessionConfig + { + SessionId = "caller-id", + OnPermissionRequest = PermissionHandler.ApproveAll, + Cloud = new CloudSessionOptions { Repository = new CloudSessionRepository { Owner = "github", Name = "copilot-sdk" } } + })); + + Assert.Contains("SessionId", ex.Message); + } + + // ------------------------------------------------------------------------- + // 4. Rejects caller-provided Provider + // ------------------------------------------------------------------------- + + [Fact] + public async Task CreateCloudSessionAsync_Rejects_CallerProvider() + { + await using var server = await FakeCloudServer.StartAsync(); + await using var client = new CopilotClient(new CopilotClientOptions { Connection = RuntimeConnection.ForUri(server.Url) }); + + var ex = await Assert.ThrowsAsync(() => + client.CreateCloudSessionAsync(new SessionConfig + { + Provider = new ProviderConfig { BaseUrl = "https://api.example.com/v1" }, + OnPermissionRequest = PermissionHandler.ApproveAll, + Cloud = new CloudSessionOptions { Repository = new CloudSessionRepository { Owner = "github", Name = "copilot-sdk" } } + })); + + Assert.Contains("Provider", ex.Message); + } + + // ------------------------------------------------------------------------- + // 5. Requires Cloud option + // ------------------------------------------------------------------------- + + [Fact] + public async Task CreateCloudSessionAsync_Requires_Cloud() + { + await using var server = await FakeCloudServer.StartAsync(); + await using var client = new CopilotClient(new CopilotClientOptions { Connection = RuntimeConnection.ForUri(server.Url) }); + + var ex = await Assert.ThrowsAsync(() => + client.CreateCloudSessionAsync(new SessionConfig + { + OnPermissionRequest = PermissionHandler.ApproveAll + // Cloud deliberately absent + })); + + Assert.Contains("Cloud", ex.Message); + } + + // ------------------------------------------------------------------------- + // 6. Buffers early session.event notifications until session id is registered + // ------------------------------------------------------------------------- + + [Fact] + public async Task CreateCloudSessionAsync_Buffers_Early_Notifications_Until_Registration() + { + const string cloudId = "remote-cloud-session"; + + // Server is configured to send a session.event notification before + // responding to session.create. + await using var server = await FakeCloudServer.StartAsync( + cloudSessionId: cloudId, + earlyNotification: new Dictionary + { + ["method"] = "session.event", + ["params"] = new Dictionary + { + ["sessionId"] = cloudId, + ["event"] = new Dictionary + { + ["type"] = "capabilities.changed", + ["data"] = new Dictionary + { + ["ui"] = new Dictionary { ["elicitation"] = true } + } + } + } + }); + + var receivedEvents = new List(); + + await using var client = new CopilotClient(new CopilotClientOptions { Connection = RuntimeConnection.ForUri(server.Url) }); + await using var session = await client.CreateCloudSessionAsync(new SessionConfig + { + OnPermissionRequest = PermissionHandler.ApproveAll, + Cloud = new CloudSessionOptions + { + Repository = new CloudSessionRepository { Owner = "github", Name = "copilot-sdk" } + }, + OnEvent = evt => + { + if (evt is CapabilitiesChangedEvent capEvt) + { + receivedEvents.Add(capEvt); + } + } + }); + + // Allow the event channel to drain. + var deadline = DateTime.UtcNow.AddSeconds(5); + while (receivedEvents.Count == 0 && DateTime.UtcNow < deadline) + { + await Task.Delay(20); + } + + Assert.Single(receivedEvents); + Assert.True(receivedEvents[0].Data?.Ui?.Elicitation == true); + } + + // ------------------------------------------------------------------------- + // 7. Parks inbound requests until session id is registered + // ------------------------------------------------------------------------- + + [Fact] + public async Task CreateCloudSessionAsync_Parks_Inbound_Requests_Until_Registration() + { + const string cloudId = "remote-cloud-session"; + + // Server sends a userInput.request before responding to session.create. + await using var server = await FakeCloudServer.StartAsync( + cloudSessionId: cloudId, + earlyInboundRequest: new Dictionary + { + ["jsonrpc"] = "2.0", + ["id"] = 301, + ["method"] = "userInput.request", + ["params"] = new Dictionary + { + ["sessionId"] = cloudId, + ["question"] = "Pick a color", + ["choices"] = new object?[] { "red", "blue" }, + ["allowFreeform"] = true + } + }); + + await using var client = new CopilotClient(new CopilotClientOptions { Connection = RuntimeConnection.ForUri(server.Url) }); + await using var session = await client.CreateCloudSessionAsync(new SessionConfig + { + OnPermissionRequest = PermissionHandler.ApproveAll, + Cloud = new CloudSessionOptions + { + Repository = new CloudSessionRepository { Owner = "github", Name = "copilot-sdk" } + }, + OnUserInputRequest = (_, _) => Task.FromResult(new UserInputResponse { Answer = "blue", WasFreeform = true }) + }); + + // Wait for the server to receive the userInput response. + var response = await server.WaitForUserInputResponse(TimeSpan.FromSeconds(5)); + Assert.NotNull(response); + Assert.Equal("blue", response!["answer"]?.ToString()); + } + + // ========================================================================= + // Fake server infrastructure + // ========================================================================= + + private sealed class FakeCloudServer : IAsyncDisposable + { + private readonly TcpListener _listener; + private readonly CancellationTokenSource _cts = new(); + private readonly SemaphoreSlim _writeLock = new(1, 1); + private readonly Task _serverTask; + private readonly string _cloudSessionId; + private readonly Dictionary? _earlyNotification; + private readonly Dictionary? _earlyInboundRequest; + private readonly TaskCompletionSource?> _userInputResponseTcs = + new(TaskCreationOptions.RunContinuationsAsynchronously); + + public JsonElement? LastCreatePayload { get; private set; } + + private FakeCloudServer( + TcpListener listener, + string cloudSessionId, + Dictionary? earlyNotification, + Dictionary? earlyInboundRequest) + { + _listener = listener; + _cloudSessionId = cloudSessionId; + _earlyNotification = earlyNotification; + _earlyInboundRequest = earlyInboundRequest; + _serverTask = RunAsync(); + } + + public string Url + { + get + { + var endpoint = (IPEndPoint)_listener.LocalEndpoint; + return $"http://127.0.0.1:{endpoint.Port}"; + } + } + + public static Task StartAsync( + string cloudSessionId = "cloud-session-id", + Dictionary? earlyNotification = null, + Dictionary? earlyInboundRequest = null) + { + var listener = new TcpListener(IPAddress.Loopback, 0); + listener.Start(); + return Task.FromResult(new FakeCloudServer(listener, cloudSessionId, earlyNotification, earlyInboundRequest)); + } + + public Task?> WaitForUserInputResponse(TimeSpan timeout) + => _userInputResponseTcs.Task.WaitAsync(timeout); + + public async ValueTask DisposeAsync() + { + _cts.Cancel(); + _listener.Stop(); + + try { await _serverTask; } + catch (Exception ex) when (ex is OperationCanceledException or ObjectDisposedException or IOException or SocketException) { } + + _cts.Dispose(); + _writeLock.Dispose(); + } + + private async Task RunAsync() + { + using var tcpClient = await _listener.AcceptTcpClientAsync(_cts.Token); + using var stream = tcpClient.GetStream(); + + while (!_cts.Token.IsCancellationRequested) + { + using var request = await ReadMessageAsync(stream, _cts.Token); + if (request is null) return; + await HandleRequestAsync(stream, request.RootElement, _cts.Token); + } + } + + private async Task HandleRequestAsync(Stream stream, JsonElement request, CancellationToken cancellationToken) + { + // Identify the message type: + // - Response from SDK: has "id" and "result"/"error" but no "method" + // - Request from SDK: has "id" and "method" + // - Notification from SDK: has "method" but no "id" + + bool hasId = request.TryGetProperty("id", out var idElement); + bool hasMethod = request.TryGetProperty("method", out var methodEl); + bool hasResult = request.TryGetProperty("result", out var resultEl); + + if (hasId && !hasMethod) + { + // This is a response from the SDK (e.g. userInput reply). + if (hasResult) + { + var dict = new Dictionary(); + foreach (var prop in resultEl.EnumerateObject()) + { + dict[prop.Name] = prop.Value.ValueKind switch + { + JsonValueKind.String => prop.Value.GetString(), + JsonValueKind.True => true, + JsonValueKind.False => false, + _ => prop.Value.GetRawText() + }; + } + _userInputResponseTcs.TrySetResult(dict); + } + return; + } + + if (!hasId) + { + // Notification — nothing to respond to. + return; + } + + var id = idElement.Clone(); + var method = methodEl.GetString(); + + if (method == "connect") + { + await WriteMessageAsync(stream, new Dictionary + { + ["jsonrpc"] = "2.0", + ["id"] = id, + ["result"] = new Dictionary + { + ["ok"] = true, + ["protocolVersion"] = 3, + ["version"] = "test" + } + }, cancellationToken); + return; + } + + if (method == "session.create") + { + // Capture the params for assertions. + if (request.TryGetProperty("params", out var paramsEl)) + { + LastCreatePayload = paramsEl.Clone(); + } + + // Optionally send an early notification before responding. + if (_earlyNotification != null) + { + await WriteMessageAsync(stream, _earlyNotification, cancellationToken); + } + + // Optionally send an early inbound request before responding. + if (_earlyInboundRequest != null) + { + await WriteMessageAsync(stream, _earlyInboundRequest, cancellationToken); + // Give the SDK a moment to park the request before we unblock create. + await Task.Delay(50, cancellationToken); + } + + await WriteMessageAsync(stream, new Dictionary + { + ["jsonrpc"] = "2.0", + ["id"] = id, + ["result"] = new Dictionary + { + ["sessionId"] = _cloudSessionId, + ["workspacePath"] = null, + ["capabilities"] = null + } + }, cancellationToken); + return; + } + + if (method == "session.destroy") + { + await WriteMessageAsync(stream, new Dictionary + { + ["jsonrpc"] = "2.0", + ["id"] = id, + ["result"] = new Dictionary() + }, cancellationToken); + return; + } + + // Default: return an empty success result. + await WriteMessageAsync(stream, new Dictionary + { + ["jsonrpc"] = "2.0", + ["id"] = id, + ["result"] = new Dictionary() + }, cancellationToken); + } + + private async Task WriteMessageAsync(Stream stream, object payload, CancellationToken cancellationToken) + { + using var bodyStream = new MemoryStream(); + using (var writer = new Utf8JsonWriter(bodyStream)) + { + WriteJsonValue(writer, payload); + } + + var body = bodyStream.ToArray(); + var header = Encoding.ASCII.GetBytes($"Content-Length: {body.Length}\r\n\r\n"); + + await _writeLock.WaitAsync(cancellationToken); + try + { + await stream.WriteAsync(header, cancellationToken); + await stream.WriteAsync(body, cancellationToken); + await stream.FlushAsync(cancellationToken); + } + finally + { + _writeLock.Release(); + } + } + + private static void WriteJsonValue(Utf8JsonWriter writer, object? value) + { + switch (value) + { + case null: + writer.WriteNullValue(); + break; + case string s: + writer.WriteStringValue(s); + break; + case bool b: + writer.WriteBooleanValue(b); + break; + case int i: + writer.WriteNumberValue(i); + break; + case long l: + writer.WriteNumberValue(l); + break; + case JsonElement je: + je.WriteTo(writer); + break; + case Dictionary dict: + writer.WriteStartObject(); + foreach (var (k, v) in dict) + { + writer.WritePropertyName(k); + WriteJsonValue(writer, v); + } + writer.WriteEndObject(); + break; + case object?[] arr: + writer.WriteStartArray(); + foreach (var item in arr) + { + WriteJsonValue(writer, item); + } + writer.WriteEndArray(); + break; + default: + throw new InvalidOperationException($"Unexpected JSON value type '{value.GetType().Name}'."); + } + } + + private static async Task ReadMessageAsync(Stream stream, CancellationToken cancellationToken) + { + var headerBytes = new List(); + while (true) + { + var value = await ReadByteAsync(stream, cancellationToken); + if (value < 0) return null; + + headerBytes.Add((byte)value); + var count = headerBytes.Count; + if (count >= 4 && + headerBytes[count - 4] == '\r' && + headerBytes[count - 3] == '\n' && + headerBytes[count - 2] == '\r' && + headerBytes[count - 1] == '\n') + { + break; + } + } + + var header = Encoding.ASCII.GetString([.. headerBytes]); + var contentLength = header + .Split(["\r\n"], StringSplitOptions.RemoveEmptyEntries) + .Select(line => line.Split(':', 2)) + .Where(parts => parts.Length == 2 && parts[0].Equals("Content-Length", StringComparison.OrdinalIgnoreCase)) + .Select(parts => int.Parse(parts[1].Trim(), System.Globalization.CultureInfo.InvariantCulture)) + .Single(); + + var body = new byte[contentLength]; + var offset = 0; + while (offset < body.Length) + { + var read = await stream.ReadAsync(body.AsMemory(offset, body.Length - offset), cancellationToken); + if (read == 0) return null; + offset += read; + } + + return JsonDocument.Parse(body); + } + + private static async Task ReadByteAsync(Stream stream, CancellationToken cancellationToken) + { + var buffer = new byte[1]; + var read = await stream.ReadAsync(buffer, cancellationToken); + return read == 0 ? -1 : buffer[0]; + } + } +} +#endif From adff7fb51116e9f1d1becebfa8bcfbfd6600be56 Mon Sep 17 00:00:00 2001 From: Timothy Clem Date: Fri, 22 May 2026 19:51:54 -0700 Subject: [PATCH 05/14] python: emit JSON-RPC error responses for dropped pending requests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two symmetric fixes ported from Rust SDK (commits 491b4427, e0ff254f) and TS SDK (commit c167bc3e, 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> --- python/copilot/client.py | 20 ++++-- python/test_cloud_session.py | 136 ++++++++++++++++++++++++++++++++++- 2 files changed, 150 insertions(+), 6 deletions(-) diff --git a/python/copilot/client.py b/python/copilot/client.py index 02a04cc18..354ff4d65 100644 --- a/python/copilot/client.py +++ b/python/copilot/client.py @@ -1037,11 +1037,11 @@ def dispose(self) -> None: self._client._pending_session_waiters.clear() for future in waiters_to_reject: if not future.done(): + # Distinct phrasing from the overflow-eviction path so debugging + # can tell the two cases apart. Matches Rust SDK (commit e0ff254f) + # and TS SDK (commit c167bc3e). future.set_exception( - ValueError( - "Cloud session.create completed without registering this session id; " - "inbound request dropped" - ) + ValueError("pending session routing ended before session was registered") ) @@ -2312,12 +2312,22 @@ async def _resolve_session(self, session_id: str) -> CopilotSession: ends. Otherwise raise :class:`ValueError` immediately. """ future: asyncio.Future[CopilotSession] | None = None + evicted: asyncio.Future[CopilotSession] | None = None with self._sessions_lock: session = self._sessions.get(session_id) if session is None and self._pending_routing_count > 0: loop = asyncio.get_running_loop() future = loop.create_future() - self._pending_session_waiters.setdefault(session_id, []).append(future) + waiters = self._pending_session_waiters.setdefault(session_id, []) + # Cap parked waiters at the same limit as notifications. When exceeded, + # reject the oldest so the runtime gets a JSON-RPC error response + # (code -32603) rather than hanging on the request id until timeout. + # Matches Rust SDK fix (commit 491b4427) and TS SDK (commit c167bc3e). + if len(waiters) >= _PENDING_SESSION_BUFFER_LIMIT: + evicted = waiters.pop(0) + waiters.append(future) + if evicted is not None and not evicted.done(): + evicted.set_exception(ValueError("pending session buffer overflow")) if session is not None: return session if future is not None: diff --git a/python/test_cloud_session.py b/python/test_cloud_session.py index f03164a05..a0b0ff87f 100644 --- a/python/test_cloud_session.py +++ b/python/test_cloud_session.py @@ -13,7 +13,11 @@ import pytest from copilot import CopilotClient, RuntimeConnection -from copilot.client import CloudSessionOptions, CloudSessionRepository +from copilot.client import ( + _PENDING_SESSION_BUFFER_LIMIT, + CloudSessionOptions, + CloudSessionRepository, +) from copilot.session import ProviderConfig, UserInputRequest, UserInputResponse from e2e.testharness import CLI_PATH @@ -277,3 +281,133 @@ async def mock_request(method, params): assert answered == ["Pick a color"] finally: await client.force_stop() + + +# --------------------------------------------------------------------------- +# Test 8: pending request buffer overflow emits an error (not silent drop) +# --------------------------------------------------------------------------- + + +class TestPendingRequestBufferOverflow: + @pytest.mark.asyncio + async def test_oldest_waiter_rejected_on_overflow(self): + """When the parked-request buffer is full, the oldest waiter is rejected. + + The rejection causes the JSON-RPC dispatch layer to send a JSON-RPC error + response (code -32603) rather than silently hanging the runtime on that + request id. The remaining _PENDING_SESSION_BUFFER_LIMIT waiters resolve + normally once the session is registered. + """ + session_id = "overflow-session" + client = CopilotClient(connection=RuntimeConnection.for_stdio(path=CLI_PATH)) + await client.start() + try: + # Enter pending-routing mode manually so _resolve_session parks futures. + guard = client._begin_pending_session_routing() + + total = _PENDING_SESSION_BUFFER_LIMIT + 1 # 129 concurrent waiters + tasks = [ + asyncio.ensure_future(client._resolve_session(session_id)) + for _ in range(total) + ] + + # Yield so all _resolve_session calls park on futures. + await asyncio.sleep(0) + await asyncio.sleep(0) + + # The oldest (tasks[0]) should now be rejected with the overflow message. + assert tasks[0].done(), "oldest waiter should have been rejected synchronously" + with pytest.raises(ValueError, match="pending session buffer overflow"): + tasks[0].result() + + # The remaining 128 are still parked. + assert all(not t.done() for t in tasks[1:]), "remaining waiters should still be parked" + + # Register the session so the remaining waiters resolve. + from copilot.session import CopilotSession + + session = CopilotSession(session_id, client._client, workspace_path=None) + with client._sessions_lock: + client._sessions[session_id] = session + client._flush_pending_for_session(session_id, session) + guard.dispose() + + # Let the event loop settle. + await asyncio.sleep(0) + + resolved_sessions = await asyncio.gather(*tasks[1:], return_exceptions=True) + assert all(s is session for s in resolved_sessions), ( + "all remaining parked waiters should resolve to the registered session" + ) + finally: + await client.force_stop() + + +# --------------------------------------------------------------------------- +# Test 9: guard drop without registration rejects parked requests +# --------------------------------------------------------------------------- + + +class TestPendingRequestGuardDropWithoutRegistration: + @pytest.mark.asyncio + async def test_parked_request_rejected_when_create_fails(self): + """When session.create fails, parked request waiters get a distinct error. + + The error message "pending session routing ended before session was registered" + must differ from the overflow message so the two failure modes are + distinguishable in logs and the runtime gets a proper JSON-RPC error + response rather than hanging. + """ + client = CopilotClient(connection=RuntimeConnection.for_stdio(path=CLI_PATH)) + await client.start() + try: + create_response_gate: asyncio.Future[dict] = asyncio.get_event_loop().create_future() + + async def mock_request(method, params): + if method == "session.create": + return await create_response_gate + return {} + + client._client.request = mock_request + + session_id = "failing-cloud-session" + create_task = asyncio.ensure_future( + client.create_cloud_session(**_cloud_config()) + ) + + # Yield so create_cloud_session enters pending-routing mode. + await asyncio.sleep(0) + await asyncio.sleep(0) + + # Park an inbound request while the create is in flight. + user_input_handler = client._client.request_handlers.get("userInput.request") + assert user_input_handler is not None, "userInput.request handler not registered" + + input_task = asyncio.ensure_future( + user_input_handler( + { + "sessionId": session_id, + "question": "Pick a color", + "choices": ["red", "blue"], + "allowFreeform": True, + } + ) + ) + + await asyncio.sleep(0) + assert not input_task.done(), "handler should be parked waiting for session" + + # Make session.create fail; this causes create_cloud_session to call + # guard.dispose() without registering any session id. + create_response_gate.set_exception(RuntimeError("simulated session.create failure")) + with pytest.raises(RuntimeError, match="simulated session.create failure"): + await asyncio.wait_for(create_task, timeout=5.0) + + # The parked waiter should now be rejected with the routing-ended message. + await asyncio.sleep(0) + assert input_task.done(), "parked waiter should be rejected after guard drop" + expected_msg = "pending session routing ended before session was registered" + with pytest.raises(ValueError, match=expected_msg): + await input_task + finally: + await client.force_stop() From 4c61c9558b517eafeab6f5d9077347176883f374 Mon Sep 17 00:00:00 2001 From: Timothy Clem Date: Fri, 22 May 2026 19:52:03 -0700 Subject: [PATCH 06/14] Go SDK: emit JSON-RPC error on pending-buffer overflow + guard drop 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 491b4427 and TS commit c167bc3e. 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 e0ff254f and TS commit c167bc3e. 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> --- go/client.go | 37 ++++++++--- go/cloud_session_test.go | 129 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 156 insertions(+), 10 deletions(-) diff --git a/go/client.go b/go/client.go index 0f5db2419..6ba816b59 100644 --- a/go/client.go +++ b/go/client.go @@ -88,6 +88,13 @@ func validateSessionFsConfig(config *SessionFsConfig) error { // } // defer client.Stop() // +// Sentinel errors for the two pending-routing rejection paths. Using distinct +// values lets callers (and debugging) tell overflow eviction from guard-drop. +var ( + errPendingSessionBufferOverflow = errors.New("pending session buffer overflow") + errPendingSessionRoutingEnded = errors.New("pending session routing ended before session was registered") +) + // pendingResult carries the outcome of a parked inbound-request session lookup. type pendingResult struct { session *Session @@ -1021,7 +1028,7 @@ func (c *Client) beginPendingSessionRouting() func() { for _, chs := range waiters { for _, ch := range chs { - ch <- pendingResult{err: fmt.Errorf("request dropped: cloud session.create completed without registering this session id")} + ch <- pendingResult{err: errPendingSessionRoutingEnded} } } }) @@ -1076,10 +1083,12 @@ func (c *Client) waitForSession(sessionID string) (*Session, error) { ch := make(chan pendingResult, 1) waiters := c.pending.waiters[sessionID] if len(waiters) >= pendingSessionBufferLimit { - // Reject the oldest waiter to keep the queue bounded. + // Reject the oldest waiter to keep the queue bounded. Send a JSON-RPC + // error response via the handler return so the runtime doesn't hang on + // the request id waiting for a reply that would never come. oldest := waiters[0] waiters = waiters[1:] - oldest <- pendingResult{err: fmt.Errorf("request dropped: pending session waiter buffer full for %s", sessionID)} + oldest <- pendingResult{err: errPendingSessionBufferOverflow} } c.pending.waiters[sessionID] = append(waiters, ch) c.pending.mu.Unlock() @@ -2126,6 +2135,18 @@ func (c *Client) handleSessionEvent(req sessionEventRequest) { c.pending.mu.Unlock() } +// pendingRoutingRPCError maps an error from waitForSession to the appropriate +// JSON-RPC error. Overflow and guard-drop rejections use -32603 (internal +// error) so the runtime gets a proper error response instead of hanging on the +// request id. All other waitForSession errors (e.g. unknown session) keep the +// existing -32602 (invalid params) code. +func pendingRoutingRPCError(err error) *jsonrpc2.Error { + if errors.Is(err, errPendingSessionBufferOverflow) || errors.Is(err, errPendingSessionRoutingEnded) { + return &jsonrpc2.Error{Code: -32603, Message: err.Error()} + } + return &jsonrpc2.Error{Code: -32602, Message: err.Error()} +} + // handleUserInputRequest handles a user input request from the CLI server. func (c *Client) handleUserInputRequest(req userInputRequest) (*userInputResponse, *jsonrpc2.Error) { if req.SessionID == "" || req.Question == "" { @@ -2134,7 +2155,7 @@ func (c *Client) handleUserInputRequest(req userInputRequest) (*userInputRespons session, err := c.waitForSession(req.SessionID) if err != nil { - return nil, &jsonrpc2.Error{Code: -32602, Message: err.Error()} + return nil, pendingRoutingRPCError(err) } response, err := session.handleUserInputRequest(UserInputRequest{ @@ -2161,7 +2182,7 @@ func (c *Client) handleExitPlanModeRequest(req exitPlanModeRequest) (*ExitPlanMo session, err := c.waitForSession(req.SessionID) if err != nil { - return nil, &jsonrpc2.Error{Code: -32602, Message: err.Error()} + return nil, pendingRoutingRPCError(err) } response, err := session.handleExitPlanModeRequest(ExitPlanModeRequest{ @@ -2185,7 +2206,7 @@ func (c *Client) handleAutoModeSwitchRequest(req autoModeSwitchRequest) (*autoMo session, err := c.waitForSession(req.SessionID) if err != nil { - return nil, &jsonrpc2.Error{Code: -32602, Message: err.Error()} + return nil, pendingRoutingRPCError(err) } response, err := session.handleAutoModeSwitchRequest(AutoModeSwitchRequest{ @@ -2207,7 +2228,7 @@ func (c *Client) handleHooksInvoke(req hooksInvokeRequest) (map[string]any, *jso session, err := c.waitForSession(req.SessionID) if err != nil { - return nil, &jsonrpc2.Error{Code: -32602, Message: err.Error()} + return nil, pendingRoutingRPCError(err) } output, err := session.handleHooksInvoke(req.Type, req.Input) @@ -2230,7 +2251,7 @@ func (c *Client) handleSystemMessageTransform(req systemMessageTransformRequest) session, err := c.waitForSession(req.SessionID) if err != nil { - return systemMessageTransformResponse{}, &jsonrpc2.Error{Code: -32602, Message: err.Error()} + return systemMessageTransformResponse{}, pendingRoutingRPCError(err) } resp, err := session.handleSystemMessageTransform(req.Sections) diff --git a/go/cloud_session_test.go b/go/cloud_session_test.go index ce827dbb0..63c66ab78 100644 --- a/go/cloud_session_test.go +++ b/go/cloud_session_test.go @@ -317,8 +317,133 @@ func TestPendingRouting_RejectsWaitersOnDispose(t *testing.T) { if rpcErr == nil { t.Fatal("expected an rpc error after dispose without registration") } - if !strings.Contains(rpcErr.Message, "dropped") { - t.Errorf("expected 'dropped' in error message, got: %s", rpcErr.Message) + if !strings.Contains(rpcErr.Message, "routing ended before session was registered") { + t.Errorf("expected routing-ended message, got: %s", rpcErr.Message) + } + if rpcErr.Code != -32603 { + t.Errorf("expected code -32603, got: %d", rpcErr.Code) + } + case <-time.After(2 * time.Second): + t.Fatal("timed out waiting for rejected waiter") + } +} + +// TestPendingRouting_OverflowEmitsError verifies that when the parked-waiter +// buffer reaches its cap, the oldest waiter receives the overflow error response +// and the remaining 128 waiters resolve normally after registration. +func TestPendingRouting_OverflowEmitsError(t *testing.T) { + client := newCloudTestClient() + dispose := client.beginPendingSessionRouting() + + const pendingID = "overflow-request-session" + const total = pendingSessionBufferLimit + 1 // 129 + + type result struct { + resp *userInputResponse + err *jsonrpcError + } + + // Register a user-input handler so the session resolves successfully. + session, cleanup := newTestSession() + defer cleanup() + session.SessionID = pendingID + session.registerUserInputHandler(func(req UserInputRequest, _ UserInputInvocation) (UserInputResponse, error) { + return UserInputResponse{Answer: "yes"}, nil + }) + + results := make([]chan result, total) + for i := range total { + results[i] = make(chan result, 1) + go func(ch chan result) { + resp, rpcErr := client.handleUserInputRequest(userInputRequest{ + SessionID: pendingID, + Question: "Proceed?", + }) + ch <- result{resp, rpcErr} + }(results[i]) + } + + // Give goroutines time to park. + time.Sleep(50 * time.Millisecond) + + // Register the session and flush — this resolves the 128 remaining waiters. + client.sessionsMux.Lock() + client.sessions[pendingID] = session + client.sessionsMux.Unlock() + client.flushPendingForSession(pendingID, session) + dispose() + + // Collect all results with a timeout. + var gotOverflow int + var gotSuccess int + deadline := time.After(2 * time.Second) + for _, ch := range results { + select { + case r := <-ch: + if r.err != nil { + if !strings.Contains(r.err.Message, "pending session buffer overflow") { + t.Errorf("unexpected error message: %s", r.err.Message) + } + if r.err.Code != -32603 { + t.Errorf("expected code -32603 for overflow, got: %d", r.err.Code) + } + gotOverflow++ + } else { + gotSuccess++ + } + case <-deadline: + t.Fatalf("timed out: overflow=%d success=%d", gotOverflow, gotSuccess) + } + } + + if gotOverflow != 1 { + t.Errorf("expected exactly 1 overflow rejection, got %d", gotOverflow) + } + if gotSuccess != pendingSessionBufferLimit { + t.Errorf("expected %d successful resolutions, got %d", pendingSessionBufferLimit, gotSuccess) + } +} + +// TestPendingRouting_GuardDropDistinctMessage verifies that when the last +// pending-routing guard drops without registration, parked waiters receive the +// distinct routing-ended error (not the overflow message) so the two paths are +// distinguishable in logs and debugging. +func TestPendingRouting_GuardDropDistinctMessage(t *testing.T) { + client := newCloudTestClient() + dispose := client.beginPendingSessionRouting() + + const pendingID = "guard-drop-session" + + resultCh := make(chan *jsonrpcError, 1) + go func() { + _, rpcErr := client.handleUserInputRequest(userInputRequest{ + SessionID: pendingID, + Question: "Proceed?", + }) + resultCh <- rpcErr + }() + + // Give the goroutine time to park. + time.Sleep(20 * time.Millisecond) + + // Drop the guard without registering — simulates session.create failing. + dispose() + + select { + case rpcErr := <-resultCh: + if rpcErr == nil { + t.Fatal("expected an rpc error after guard drop without registration") + } + const want = "pending session routing ended before session was registered" + if rpcErr.Message != want { + t.Errorf("expected exact message %q, got %q", want, rpcErr.Message) + } + if rpcErr.Code != -32603 { + t.Errorf("expected code -32603, got: %d", rpcErr.Code) + } + // Must NOT contain the overflow message. + if strings.Contains(rpcErr.Message, "buffer overflow") { + t.Errorf("guard-drop path must not use overflow message, got: %s", rpcErr.Message) } case <-time.After(2 * time.Second): t.Fatal("timed out waiting for rejected waiter") From 8113affc2af4de9f1becc6873427b4dece2cfd07 Mon Sep 17 00:00:00 2001 From: Timothy Clem Date: Fri, 22 May 2026 19:58:28 -0700 Subject: [PATCH 07/14] dotnet: emit JSON-RPC error on pending-buffer overflow + guard drop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 491b4427 and TS commit c167bc3e. 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 e0ff254f. 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> --- dotnet/src/Client.cs | 32 ++++- dotnet/test/Unit/CloudSessionTests.cs | 195 +++++++++++++++++++++++++- 2 files changed, 217 insertions(+), 10 deletions(-) diff --git a/dotnet/src/Client.cs b/dotnet/src/Client.cs index c693d57b1..6d90a668e 100644 --- a/dotnet/src/Client.cs +++ b/dotnet/src/Client.cs @@ -904,23 +904,29 @@ private PendingSessionRoutingGuard BeginPendingSessionRouting() private void EndPendingSessionRouting() { + List>? waiters = null; lock (_pendingLock) { _pendingRoutingCount--; if (_pendingRoutingCount == 0) { _pendingSessionEvents.Clear(); - var waiters = new List>(); + waiters = new List>(); foreach (var list in _pendingSessionWaiters.Values) waiters.AddRange(list); _pendingSessionWaiters.Clear(); + } + } - // Fault pending waiters outside the lock so TCS continuations don't run under it. - foreach (var tcs in waiters) - { - tcs.TrySetException(new InvalidOperationException( - "Cloud session.create completed without registering this sessionId; request dropped.")); - } + // Fault pending waiters outside the lock so TCS continuations don't run under it. + // Distinct phrasing from the overflow-eviction path so the runtime / debugging can tell + // the two cases apart. Matches the Rust SDK message in PR #1394 (commit e0ff254f). + if (waiters != null) + { + foreach (var tcs in waiters) + { + tcs.TrySetException(new InvalidOperationException( + "pending session routing ended before session was registered")); } } } @@ -979,6 +985,18 @@ private Task ResolveSessionAsync(string sessionId) list = []; _pendingSessionWaiters[sessionId] = list; } + + // Cap parked waiters per session id. When exceeded, reject the oldest with a + // distinct message so the runtime isn't left waiting on a hung request id. + // RunContinuationsAsynchronously ensures the continuation won't run under this lock. + // Matches the Rust SDK fix in PR #1394 (commit 491b4427). + if (list.Count >= PendingSessionBufferLimit) + { + var oldest = list[0]; + list.RemoveAt(0); + oldest.TrySetException(new InvalidOperationException("pending session buffer overflow")); + } + list.Add(tcs); return tcs.Task; } diff --git a/dotnet/test/Unit/CloudSessionTests.cs b/dotnet/test/Unit/CloudSessionTests.cs index f99303026..97dfe9f02 100644 --- a/dotnet/test/Unit/CloudSessionTests.cs +++ b/dotnet/test/Unit/CloudSessionTests.cs @@ -267,6 +267,93 @@ public async Task CreateCloudSessionAsync_Parks_Inbound_Requests_Until_Registrat Assert.Equal("blue", response!["answer"]?.ToString()); } + // ------------------------------------------------------------------------- + // 8. Pending-waiter overflow: oldest is rejected, remaining 128 succeed + // ------------------------------------------------------------------------- + + [Fact] + public async Task PendingRequestWaiterOverflow_RejectsOldestWithOverflowMessage() + { + const string cloudId = "overflow-session"; + const int requestCount = 129; // one beyond the 128-waiter cap + + await using var server = await FakeCloudServer.StartAsync( + cloudSessionId: cloudId, + earlyInboundRequestCount: requestCount); + + await using var client = new CopilotClient(new CopilotClientOptions + { Connection = RuntimeConnection.ForUri(server.Url) }); + + await using var _ = await client.CreateCloudSessionAsync(new SessionConfig + { + OnPermissionRequest = PermissionHandler.ApproveAll, + Cloud = new CloudSessionOptions + { + Repository = new CloudSessionRepository { Owner = "github", Name = "copilot-sdk" } + }, + OnUserInputRequest = (_, _) => Task.FromResult(new UserInputResponse { Answer = "yes", WasFreeform = false }) + }); + + var responses = await server.WaitForAllInboundResponses(requestCount, TimeSpan.FromSeconds(10)); + + // Exactly one overflow eviction, 128 successful completions. + Assert.Equal(1, responses.Count(r => r.IsError)); + Assert.Equal(128, responses.Count(r => !r.IsError)); + + var err = responses.Single(r => r.IsError); + Assert.Contains("pending session buffer overflow", err.ErrorMessage ?? ""); + } + + // ------------------------------------------------------------------------- + // 9. Guard-drop path: parked requests are rejected with distinct message + // ------------------------------------------------------------------------- + + [Fact] + public async Task PendingSessionGuardDrop_RejectsParkedRequestWithDistinctMessage() + { + const string cloudId = "guard-drop-session"; + const int inboundRequestId = 500; + + await using var server = await FakeCloudServer.StartAsync( + cloudSessionId: cloudId, + failSessionCreate: true, + earlyInboundRequest: new Dictionary + { + ["jsonrpc"] = "2.0", + ["id"] = inboundRequestId, + ["method"] = "userInput.request", + ["params"] = new Dictionary + { + ["sessionId"] = cloudId, + ["question"] = "Color?", + ["choices"] = new object?[] { "red", "blue" }, + ["allowFreeform"] = false + } + }); + + await using var client = new CopilotClient(new CopilotClientOptions + { Connection = RuntimeConnection.ForUri(server.Url) }); + + await Assert.ThrowsAnyAsync(() => + client.CreateCloudSessionAsync(new SessionConfig + { + OnPermissionRequest = PermissionHandler.ApproveAll, + Cloud = new CloudSessionOptions + { + Repository = new CloudSessionRepository { Owner = "github", Name = "copilot-sdk" } + } + })); + + // The parked request must have been rejected with the guard-drop message (not the overflow message). + var responses = await server.WaitForAllInboundResponses(1, TimeSpan.FromSeconds(5)); + + Assert.Single(responses); + Assert.True(responses[0].IsError); + Assert.Contains( + "pending session routing ended before session was registered", + responses[0].ErrorMessage ?? ""); + } + // ========================================================================= // Fake server infrastructure // ========================================================================= @@ -280,21 +367,35 @@ private sealed class FakeCloudServer : IAsyncDisposable private readonly string _cloudSessionId; private readonly Dictionary? _earlyNotification; private readonly Dictionary? _earlyInboundRequest; + private readonly int _earlyInboundRequestCount; + private readonly bool _failSessionCreate; private readonly TaskCompletionSource?> _userInputResponseTcs = new(TaskCreationOptions.RunContinuationsAsynchronously); + // Response tracking for overflow / guard-drop tests. + private readonly object _inboundResponsesLock = new(); + private readonly List _collectedInboundResponses = []; + private int _waitForInboundResponseCount; + private TaskCompletionSource>? _allInboundResponsesTcs; + public JsonElement? LastCreatePayload { get; private set; } + public record InboundResponse(int RequestId, bool IsError, string? ErrorMessage); + private FakeCloudServer( TcpListener listener, string cloudSessionId, Dictionary? earlyNotification, - Dictionary? earlyInboundRequest) + Dictionary? earlyInboundRequest, + int earlyInboundRequestCount, + bool failSessionCreate) { _listener = listener; _cloudSessionId = cloudSessionId; _earlyNotification = earlyNotification; _earlyInboundRequest = earlyInboundRequest; + _earlyInboundRequestCount = earlyInboundRequestCount; + _failSessionCreate = failSessionCreate; _serverTask = RunAsync(); } @@ -310,16 +411,55 @@ public string Url public static Task StartAsync( string cloudSessionId = "cloud-session-id", Dictionary? earlyNotification = null, - Dictionary? earlyInboundRequest = null) + Dictionary? earlyInboundRequest = null, + int earlyInboundRequestCount = 0, + bool failSessionCreate = false) { var listener = new TcpListener(IPAddress.Loopback, 0); listener.Start(); - return Task.FromResult(new FakeCloudServer(listener, cloudSessionId, earlyNotification, earlyInboundRequest)); + return Task.FromResult(new FakeCloudServer( + listener, cloudSessionId, earlyNotification, earlyInboundRequest, + earlyInboundRequestCount, failSessionCreate)); } public Task?> WaitForUserInputResponse(TimeSpan timeout) => _userInputResponseTcs.Task.WaitAsync(timeout); + /// + /// Waits until the server has collected responses (error or success) + /// from the client for inbound requests. Used by overflow and guard-drop tests. + /// + public Task> WaitForAllInboundResponses(int count, TimeSpan timeout) + { + var tcs = new TaskCompletionSource>( + TaskCreationOptions.RunContinuationsAsynchronously); + lock (_inboundResponsesLock) + { + _waitForInboundResponseCount = count; + _allInboundResponsesTcs = tcs; + if (_collectedInboundResponses.Count >= count) + tcs.TrySetResult(new List(_collectedInboundResponses)); + } + return tcs.Task.WaitAsync(timeout); + } + + private void RecordInboundResponse(InboundResponse response) + { + TaskCompletionSource>? tcs = null; + IReadOnlyList? snapshot = null; + lock (_inboundResponsesLock) + { + _collectedInboundResponses.Add(response); + if (_allInboundResponsesTcs != null && + _collectedInboundResponses.Count >= _waitForInboundResponseCount) + { + tcs = _allInboundResponsesTcs; + snapshot = new List(_collectedInboundResponses); + } + } + tcs?.TrySetResult(snapshot!); + } + public async ValueTask DisposeAsync() { _cts.Cancel(); @@ -373,6 +513,16 @@ private async Task HandleRequestAsync(Stream stream, JsonElement request, Cancel }; } _userInputResponseTcs.TrySetResult(dict); + + if (idElement.ValueKind == JsonValueKind.Number && idElement.TryGetInt32(out var successId)) + RecordInboundResponse(new InboundResponse(successId, IsError: false, null)); + } + else if (request.TryGetProperty("error", out var errorEl)) + { + var requestId = idElement.ValueKind == JsonValueKind.Number && idElement.TryGetInt32(out var errId) + ? errId : -1; + var msg = errorEl.TryGetProperty("message", out var msgEl) ? msgEl.GetString() : null; + RecordInboundResponse(new InboundResponse(requestId, IsError: true, msg)); } return; } @@ -424,6 +574,45 @@ private async Task HandleRequestAsync(Stream stream, JsonElement request, Cancel await Task.Delay(50, cancellationToken); } + // For overflow tests: send N inbound requests to exercise the buffer cap. + if (_earlyInboundRequestCount > 0) + { + for (var i = 1; i <= _earlyInboundRequestCount; i++) + { + await WriteMessageAsync(stream, new Dictionary + { + ["jsonrpc"] = "2.0", + ["id"] = i, + ["method"] = "userInput.request", + ["params"] = new Dictionary + { + ["sessionId"] = _cloudSessionId, + ["question"] = $"Question {i}", + ["choices"] = new object?[] { "yes", "no" }, + ["allowFreeform"] = false + } + }, cancellationToken); + } + + // Give the client time to park/overflow all requests before responding. + await Task.Delay(100, cancellationToken); + } + + if (_failSessionCreate) + { + await WriteMessageAsync(stream, new Dictionary + { + ["jsonrpc"] = "2.0", + ["id"] = id, + ["error"] = new Dictionary + { + ["code"] = -32603, + ["message"] = "session.create failed (test-induced failure)" + } + }, cancellationToken); + return; + } + await WriteMessageAsync(stream, new Dictionary { ["jsonrpc"] = "2.0", From 5b3abb52b8a961e406c2c37e2ab1274cb817d473 Mon Sep 17 00:00:00 2001 From: Timothy Clem Date: Fri, 22 May 2026 20:39:53 -0700 Subject: [PATCH 08/14] Java SDK: emit JSON-RPC error on pending-buffer overflow + guard drop 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 491b4427 and TS commit c167bc3e 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 e0ff254f and TS commit c167bc3e. 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> --- .../com/github/copilot/sdk/CopilotClient.java | 27 +---- .../copilot/sdk/PendingRoutingState.java | 49 ++++++--- .../copilot/sdk/RpcHandlerDispatcher.java | 15 ++- .../github/copilot/sdk/CloudSessionTest.java | 102 ++++++++++++++++-- 4 files changed, 146 insertions(+), 47 deletions(-) diff --git a/java/src/main/java/com/github/copilot/sdk/CopilotClient.java b/java/src/main/java/com/github/copilot/sdk/CopilotClient.java index 15391d638..c7ccc8a03 100644 --- a/java/src/main/java/com/github/copilot/sdk/CopilotClient.java +++ b/java/src/main/java/com/github/copilot/sdk/CopilotClient.java @@ -583,9 +583,7 @@ public CompletableFuture createCloudSession(SessionConfig config if (returnedId == null || returnedId.isEmpty()) { // No id: release the guard and surface the error. Any runtime session // created on the other side may leak — we have nothing to destroy. - var staleWaiters = pendingRoutingState.decrementGuard(); - completeWaitersExceptionally(staleWaiters, - "Cloud session.create completed without registering this sessionId; request dropped"); + pendingRoutingState.decrementGuard(); LOG.warning("Cloud session.create response missing sessionId; runtime session may leak"); throw new RuntimeException( "Cloud session.create response did not include a sessionId; cannot register session."); @@ -619,28 +617,23 @@ public CompletableFuture createCloudSession(SessionConfig config waiter.complete(session); } } catch (Exception e) { - // Roll back: remove session from map, release guard with exceptional completion + // Roll back: remove session from map, release guard. sessions.remove(returnedId); - var staleWaiters = pendingRoutingState.decrementGuard(); - completeWaitersExceptionally(staleWaiters, - "Cloud session post-registration setup failed; request dropped"); + pendingRoutingState.decrementGuard(); LoggingHelpers.logTiming(LOG, Level.WARNING, e, "CopilotClient.createCloudSession post-registration setup failed. Elapsed={Elapsed}", totalNanos); throw e instanceof RuntimeException re ? re : new RuntimeException(e); } - var staleWaiters = pendingRoutingState.decrementGuard(); - completeWaitersExceptionally(staleWaiters, - "Cloud session.create completed without registering this sessionId; request dropped"); + pendingRoutingState.decrementGuard(); LoggingHelpers.logTiming(LOG, Level.FINE, "CopilotClient.createCloudSession complete. Elapsed={Elapsed}, SessionId=" + returnedId, totalNanos); return session; }).exceptionally(ex -> { - var staleWaiters = pendingRoutingState.decrementGuard(); - completeWaitersExceptionally(staleWaiters, "Cloud session.create failed; request dropped"); + pendingRoutingState.decrementGuard(); LoggingHelpers.logTiming(LOG, Level.WARNING, ex, "CopilotClient.createCloudSession failed. Elapsed={Elapsed}", totalNanos); throw ex instanceof RuntimeException re ? re : new RuntimeException(ex); @@ -648,16 +641,6 @@ public CompletableFuture createCloudSession(SessionConfig config }); } - private static void completeWaitersExceptionally(java.util.List> waiters, - String message) { - if (!waiters.isEmpty()) { - var ex = new RuntimeException(message); - for (var waiter : waiters) { - waiter.completeExceptionally(ex); - } - } - } - /** * Resumes an existing Copilot session. *

diff --git a/java/src/main/java/com/github/copilot/sdk/PendingRoutingState.java b/java/src/main/java/com/github/copilot/sdk/PendingRoutingState.java index a0b2e0852..7c0678e20 100644 --- a/java/src/main/java/com/github/copilot/sdk/PendingRoutingState.java +++ b/java/src/main/java/com/github/copilot/sdk/PendingRoutingState.java @@ -56,23 +56,32 @@ synchronized void incrementGuard() { /** * Decrement the guard count. If the count reaches zero, clears all buffered - * events and completes all parked waiters exceptionally. - * - * @return list of waiters that should be completed exceptionally (caller must - * complete them outside this lock to avoid re-entrancy) + * events and completes all parked request waiters exceptionally with a + * canonical message that is distinct from the overflow-eviction path. */ - synchronized List> decrementGuard() { + synchronized void decrementGuard() { guardCount = Math.max(0, guardCount - 1); - if (guardCount == 0) { - pendingEvents.clear(); - var stale = new ArrayList>(); - for (var list : pendingWaiters.values()) { - stale.addAll(list); + if (guardCount != 0) { + return; + } + pendingEvents.clear(); + var stale = new ArrayList>(); + for (var list : pendingWaiters.values()) { + stale.addAll(list); + } + pendingWaiters.clear(); + if (!stale.isEmpty()) { + // Use a distinct phrasing from the overflow-eviction path so that + // debugging can tell the two failure modes apart. Matches the Rust + // SDK message (PR #1394 commit e0ff254f) and the TS SDK (commit + // c167bc3e). + LOG.warning("Pending session routing ended before session was registered; " + "completing " + stale.size() + + " parked request waiter(s) exceptionally"); + var ex = new RuntimeException("pending session routing ended before session was registered"); + for (var waiter : stale) { + waiter.completeExceptionally(ex); } - pendingWaiters.clear(); - return stale; } - return Collections.emptyList(); } /** @@ -137,7 +146,19 @@ synchronized CompletableFuture tryParkRequest(String sessionId, return null; // no pending; caller sends error } var future = new CompletableFuture(); - pendingWaiters.computeIfAbsent(sessionId, k -> new ArrayList<>()).add(future); + var list = pendingWaiters.computeIfAbsent(sessionId, k -> new ArrayList<>()); + if (list.size() >= BUFFER_LIMIT) { + // Cap parked waiters per session. When exceeded, evict the oldest + // and complete it with a distinct overflow message so the runtime + // gets an error response rather than hanging on a reply that will + // never arrive. Matches Rust PR #1394 (commit 491b4427) and TS + // (commit c167bc3e). + var oldest = list.remove(0); + LOG.warning("Pending session request waiter buffer full for session " + sessionId + " (limit=" + + BUFFER_LIMIT + "); evicting oldest request"); + oldest.completeExceptionally(new RuntimeException("pending session buffer overflow")); + } + list.add(future); return future; } diff --git a/java/src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java b/java/src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java index 6202860a9..1b164aa28 100644 --- a/java/src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java +++ b/java/src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java @@ -570,9 +570,22 @@ private CopilotSession resolveSessionForRequest(String sessionId, long requestId CompletableFuture waiter = pendingState.tryParkRequest(sessionId, sessions); if (waiter != null) { if (waiter.isDone()) { - // Resolved synchronously (session was already registered under the lock) + // Session was already registered under tryParkRequest's lock — + // complete synchronously. If the waiter was completed exceptionally + // (e.g. overflow eviction just beat us to isDone()), send the + // same -32603 error as the blocking path so the runtime isn't + // left waiting for a reply that will never arrive. try { return waiter.get(); + } catch (ExecutionException e) { + Throwable cause = e.getCause(); + try { + rpc.sendErrorResponse(requestId, -32603, "Session " + sessionId + " not registered: " + + (cause != null ? cause.getMessage() : e.getMessage())); + } catch (IOException ioe) { + LOG.log(Level.SEVERE, "Failed to send synchronous registration-failed error", ioe); + } + return null; } catch (Exception e) { // fall through to send error } diff --git a/java/src/test/java/com/github/copilot/sdk/CloudSessionTest.java b/java/src/test/java/com/github/copilot/sdk/CloudSessionTest.java index 19eb9ed2d..2177655a5 100644 --- a/java/src/test/java/com/github/copilot/sdk/CloudSessionTest.java +++ b/java/src/test/java/com/github/copilot/sdk/CloudSessionTest.java @@ -306,8 +306,7 @@ void bufferEarlySessionEventNotifications() throws Exception { } // Release the guard - var staleWaiters = pendingState.decrementGuard(); - assertTrue(staleWaiters.isEmpty(), "No stale waiters expected"); + pendingState.decrementGuard(); // The buffered session.event should now have been replayed to the session Thread.sleep(50); @@ -413,15 +412,11 @@ void parkedRequestFailsExceptionallyWhenGuardDroppedWithoutRegistration() throws Thread.sleep(100); - // Drop the guard WITHOUT registering the session → waiters should be completed - // exceptionally - var staleWaiters = pendingState.decrementGuard(); - // Complete them exceptionally to simulate what CopilotClient does on failure - for (var w : staleWaiters) { - w.completeExceptionally(new RuntimeException("Cloud session.create failed; request dropped")); - } + // Drop the guard WITHOUT registering the session. decrementGuard now + // completes parked waiters internally with the canonical message. + pendingState.decrementGuard(); - // The handler should eventually receive the exceptional completion and send an + // The handler should receive the exceptional completion and send an // error response toolCallFuture.get(5, TimeUnit.SECONDS); @@ -429,5 +424,92 @@ void parkedRequestFailsExceptionallyWhenGuardDroppedWithoutRegistration() throws assertNotNull(response, "Should have received an error response"); assertEquals(99, response.get("id").asInt()); assertNotNull(response.get("error"), "Response should be an error (not a result)"); + String errorMessage = response.get("error").get("message").asText(); + assertTrue(errorMessage.contains("routing ended before session was registered"), + "Error message should contain the canonical guard-drop phrase; got: " + errorMessage); + } + + // ========================================================================= + // Test 8: overflow path — oldest parked waiter gets the overflow message + // ========================================================================= + + @Test + void parkedRequestWaiterBuffer_overflow_evictsOldestWithOverflowMessage() throws Exception { + pendingState.incrementGuard(); + String sid = "cloud-overflow-requests"; + + // Park BUFFER_LIMIT + 1 waiters via tryParkRequest. The 129th call must + // evict the very first waiter and complete it with the overflow message. + var waiters = new java.util.ArrayList>(); + for (int i = 0; i < PendingRoutingState.BUFFER_LIMIT + 1; i++) { + waiters.add(pendingState.tryParkRequest(sid, sessions)); + } + + // The first waiter (oldest) must have been evicted with the overflow message. + CompletableFuture oldest = waiters.get(0); + assertTrue(oldest.isCompletedExceptionally(), "Oldest waiter should be completed exceptionally on overflow"); + ExecutionException ex = assertThrows(ExecutionException.class, oldest::get); + assertEquals("pending session buffer overflow", ex.getCause().getMessage()); + + // The remaining BUFFER_LIMIT waiters should still be pending. + for (int i = 1; i <= PendingRoutingState.BUFFER_LIMIT; i++) { + assertFalse(waiters.get(i).isDone(), "Waiter " + i + " should still be pending after overflow eviction"); + } + + // Registering the session resolves the remaining 128 waiters normally. + var session = new CopilotSession(sid, rpc); + var flush = pendingState.registerAndFlush(sid, session, sessions); + assertEquals(PendingRoutingState.BUFFER_LIMIT, flush.waiters().size(), + "registerAndFlush should return all non-evicted waiters"); + for (var waiter : flush.waiters()) { + waiter.complete(session); + } + for (int i = 1; i <= PendingRoutingState.BUFFER_LIMIT; i++) { + assertFalse(waiters.get(i).isCompletedExceptionally(), + "Waiter " + i + " should complete normally, not exceptionally"); + assertEquals(session, waiters.get(i).get(1, TimeUnit.SECONDS)); + } + + pendingState.decrementGuard(); + } + + // ========================================================================= + // Test 9: guard-drop message is distinct from overflow message + // ========================================================================= + + @Test + void parkedRequestWaiter_guardDropMessage_isDistinctFromOverflowMessage() throws Exception { + String pendingSessionId = "cloud-session-distinct-msg"; + + pendingState.incrementGuard(); + + // Park a request in the background via the full handler path so the + // response travels over the socket — this mirrors the real runtime flow. + var toolCallFuture = CompletableFuture.runAsync(() -> { + ObjectNode params = MAPPER.createObjectNode(); + params.put("sessionId", pendingSessionId); + params.put("toolCallId", "tc-distinct"); + params.put("toolName", "noop"); + params.set("arguments", MAPPER.createObjectNode()); + invokeHandler("tool.call", "77", params); + }); + + Thread.sleep(100); + + // Drop the guard without registration. decrementGuard completes waiters + // internally with the canonical guard-drop message. + pendingState.decrementGuard(); + + toolCallFuture.get(5, TimeUnit.SECONDS); + + JsonNode response = readResponse(); + assertEquals(77, response.get("id").asInt()); + assertNotNull(response.get("error"), "Should be an error response"); + String msg = response.get("error").get("message").asText(); + + // Must contain the guard-drop phrase — NOT the overflow phrase. + assertTrue(msg.contains("routing ended before session was registered"), + "Guard-drop error must use the routing-ended phrase; got: " + msg); + assertFalse(msg.contains("buffer overflow"), "Guard-drop error must NOT use the overflow phrase; got: " + msg); } } From b62855b37652e6c1e271fb219fbaf89f39dc820b Mon Sep 17 00:00:00 2001 From: Timothy Clem Date: Fri, 22 May 2026 19:06:13 -0700 Subject: [PATCH 09/14] TS SDK: add createCloudSession MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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> --- nodejs/README.md | 18 ++ nodejs/src/client.ts | 337 ++++++++++++++++++++++++++++++++++--- nodejs/test/client.test.ts | 175 ++++++++++++++++++- 3 files changed, 499 insertions(+), 31 deletions(-) diff --git a/nodejs/README.md b/nodejs/README.md index 1cb6e7836..daa723392 100644 --- a/nodejs/README.md +++ b/nodejs/README.md @@ -127,6 +127,24 @@ Create a new conversation session. Resume an existing session. Returns the session with `workspacePath` populated if infinite sessions were enabled. +##### `createCloudSession(config: SessionConfig): Promise` + +Create a Mission Control–backed cloud session. The runtime owns the session ID for cloud sessions: do **not** set `sessionId` or `provider` on the config (the SDK rejects both). The SDK omits `sessionId` from the `session.create` request and registers the resulting session under the id the runtime returns. + +Any `session.event` notifications or inbound JSON-RPC requests that arrive between sending `session.create` and receiving its response are buffered (bounded, drop-oldest) and replayed once the returned id is registered, so early events aren't lost. + +`config.cloud` is required. Passing `cloud` to `createSession` instead throws — use this method. + +```typescript +const session = await client.createCloudSession({ + onPermissionRequest: approveAll, + cloud: { + repository: { owner: "github", name: "copilot-sdk", branch: "main" }, + }, +}); +console.log("cloud session id:", session.sessionId); +``` + ##### `ping(message?: string): Promise<{ message: string; timestamp: string }>` Ping the server to check connectivity. diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index 21563d598..4410e20bf 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -249,6 +249,30 @@ export class CopilotClient { private actualHost: string = "localhost"; private state: "disconnected" | "connecting" | "connected" | "error" = "disconnected"; private sessions: Map = new Map(); + /** + * Refcount of in-flight cloud `session.create` calls. While >0, the + * notification/request handlers buffer messages addressed to session + * ids that are not yet registered, so events the runtime emits between + * `session.create` and its response are not lost. + */ + private pendingRoutingCount: number = 0; + /** Buffered `session.event` payloads keyed by runtime-assigned sessionId. */ + private pendingSessionEvents: Map = new Map(); + /** + * Waiters parked by request handlers when the addressed session id is + * not yet registered but pending routing is active. Resolved when the + * session is registered; rejected when pending mode ends without + * registration so the JSON-RPC request surfaces a clear error. + */ + private pendingSessionWaiters: Map< + string, + Array<{ resolve: (s: CopilotSession) => void; reject: (e: Error) => void }> + > = new Map(); + /** + * Upper bound on buffered notifications per pending session id. Cloud + * handshakes are short; drop-oldest above this is acceptable. + */ + private static readonly PENDING_SESSION_BUFFER_LIMIT = 128; private stderrBuffer: string = ""; // Captures CLI stderr for error messages /** Resolved connection mode chosen in the constructor. */ private connectionConfig: InternalRuntimeConnection; @@ -788,6 +812,11 @@ export class CopilotClient { * ``` */ async createSession(config: SessionConfig): Promise { + if (config.cloud) { + throw new Error( + "CopilotClient.createSession does not support cloud sessions; use createCloudSession instead." + ); + } if (!this.connection) { await this.start(); } @@ -899,6 +928,243 @@ export class CopilotClient { return session; } + /** + * Creates a Mission Control–backed cloud session. + * + * The runtime owns the session ID for cloud sessions: do **not** set + * `sessionId` or `provider` on the config (the SDK rejects both). The + * SDK omits `sessionId` from the `session.create` request and registers + * the resulting session under the id the runtime returns. + * + * Any `session.event` notifications or inbound JSON-RPC requests that + * arrive between sending `session.create` and receiving its response are + * buffered (bounded, drop-oldest) and replayed once the returned id is + * registered, so early events aren't lost. + * + * **Known limitation:** inbound `sessionFs.*` requests (the generated + * client-session API handlers) are not pending-buffered today. In practice + * the runtime does not initiate `sessionFs.*` calls before the + * `session.create` response, so this is theoretical; if needed, the + * generated `registerClientSessionApiHandlers` shim can be updated to + * support async session resolution. + * + * @example + * ```typescript + * const session = await client.createCloudSession({ + * onPermissionRequest: approveAll, + * cloud: { repository: { owner: "github", name: "copilot-sdk", branch: "main" } }, + * }); + * console.log(session.sessionId); + * ``` + */ + async createCloudSession(config: SessionConfig): Promise { + if (!config.cloud) { + throw new Error("CopilotClient.createCloudSession requires config.cloud to be set."); + } + if (config.sessionId !== undefined) { + throw new Error( + "CopilotClient.createCloudSession does not support a caller-provided sessionId; the runtime assigns one." + ); + } + if (config.provider !== undefined) { + throw new Error( + "CopilotClient.createCloudSession does not support config.provider; cloud sessions use the runtime's provider." + ); + } + if (!this.connection) { + await this.start(); + } + + const { wirePayload: wireSystemMessage, transformCallbacks } = extractTransformCallbacks( + config.systemMessage + ); + + const guard = this.beginPendingSessionRouting(); + + let response: { + sessionId?: unknown; + workspacePath?: string; + capabilities?: { ui?: { elicitation?: boolean } }; + }; + try { + response = (await this.connection!.sendRequest("session.create", { + ...(await getTraceContext(this.onGetTraceContext)), + model: config.model, + // sessionId intentionally omitted: the runtime assigns the id for cloud sessions. + clientName: config.clientName, + reasoningEffort: config.reasoningEffort, + tools: config.tools?.map((tool) => ({ + name: tool.name, + description: tool.description, + parameters: toJsonSchema(tool.parameters), + overridesBuiltInTool: tool.overridesBuiltInTool, + skipPermission: tool.skipPermission, + })), + commands: config.commands?.map((cmd) => ({ + name: cmd.name, + description: cmd.description, + })), + systemMessage: wireSystemMessage, + availableTools: config.availableTools, + excludedTools: config.excludedTools, + enableSessionTelemetry: config.enableSessionTelemetry, + modelCapabilities: config.modelCapabilities, + requestPermission: !!config.onPermissionRequest, + requestUserInput: !!config.onUserInputRequest, + requestElicitation: !!config.onElicitationRequest, + requestExitPlanMode: !!config.onExitPlanModeRequest, + requestAutoModeSwitch: !!config.onAutoModeSwitchRequest, + hooks: !!(config.hooks && Object.values(config.hooks).some(Boolean)), + workingDirectory: config.workingDirectory, + streaming: config.streaming, + includeSubAgentStreamingEvents: config.includeSubAgentStreamingEvents ?? true, + mcpServers: toWireMcpServers(config.mcpServers), + envValueMode: "direct", + customAgents: toWireCustomAgents(config.customAgents), + defaultAgent: config.defaultAgent, + agent: config.agent, + configDir: config.configDir, + enableConfigDiscovery: config.enableConfigDiscovery, + skillDirectories: config.skillDirectories, + instructionDirectories: config.instructionDirectories, + disabledSkills: config.disabledSkills, + infiniteSessions: config.infiniteSessions, + gitHubToken: config.gitHubToken, + remoteSession: config.remoteSession, + cloud: config.cloud, + })) as { + sessionId?: unknown; + workspacePath?: string; + capabilities?: { ui?: { elicitation?: boolean } }; + }; + } catch (e) { + guard.dispose(); + throw e; + } + + if (!response || typeof response !== "object" || typeof response.sessionId !== "string") { + // No id to issue session.destroy against; release the guard and surface the error. + // Any runtime session created on the other side may leak. + + console.warn( + "Cloud session.create response missing sessionId; runtime session may leak." + ); + guard.dispose(); + throw new Error( + "Cloud session.create response did not include a sessionId; cannot register session." + ); + } + + const sessionId = response.sessionId; + const session = new CopilotSession( + sessionId, + this.connection!, + undefined, + this.onGetTraceContext + ); + session.registerTools(config.tools); + session.registerCommands(config.commands); + session.registerPermissionHandler(config.onPermissionRequest); + if (config.onUserInputRequest) { + session.registerUserInputHandler(config.onUserInputRequest); + } + if (config.onElicitationRequest) { + session.registerElicitationHandler(config.onElicitationRequest); + } + if (config.onExitPlanModeRequest) { + session.registerExitPlanModeHandler(config.onExitPlanModeRequest); + } + if (config.onAutoModeSwitchRequest) { + session.registerAutoModeSwitchHandler(config.onAutoModeSwitchRequest); + } + if (config.hooks) { + session.registerHooks(config.hooks); + } + if (transformCallbacks) { + session.registerTransformCallbacks(transformCallbacks); + } + if (config.onEvent) { + session.on(config.onEvent); + } + try { + this.sessions.set(sessionId, session); + this.setupSessionFs(session, config); + session["_workspacePath"] = response.workspacePath; + session.setCapabilities(response.capabilities); + + // Drain anything that arrived during the in-flight session.create + // into the freshly-registered session before releasing the guard. + this.flushPendingForSession(sessionId, session); + } catch (e) { + // Roll back partial registration so a failed post-response setup + // (e.g. setupSessionFs throwing because sessionFs is misconfigured) + // doesn't leave a half-wired session in this.sessions. + this.sessions.delete(sessionId); + guard.dispose(); + throw e; + } + guard.dispose(); + + return session; + } + + /** + * Enter pending-routing mode. While the returned guard is undisposed, + * notifications and inbound requests addressed to session ids that are + * not yet registered are buffered (up to + * {@link CopilotClient.PENDING_SESSION_BUFFER_LIMIT} per id, drop-oldest) + * and replayed on registration. When the last guard is disposed, any + * still-buffered messages are dropped and parked request waiters are + * rejected so the calling JSON-RPC requests don't hang forever. + */ + private beginPendingSessionRouting(): { dispose: () => void } { + this.pendingRoutingCount++; + let disposed = false; + return { + dispose: () => { + if (disposed) return; + disposed = true; + this.pendingRoutingCount--; + if (this.pendingRoutingCount === 0) { + this.pendingSessionEvents.clear(); + for (const waiters of this.pendingSessionWaiters.values()) { + for (const w of waiters) { + w.reject( + new Error( + "Cloud session.create completed without registering this sessionId; request dropped" + ) + ); + } + } + this.pendingSessionWaiters.clear(); + } + }, + }; + } + + /** + * Drain buffered events and pending request waiters for `sessionId` into + * the freshly-registered session. Called from {@link createCloudSession} + * after the session is in `this.sessions` and before the pending guard + * is released. + */ + private flushPendingForSession(sessionId: string, session: CopilotSession): void { + const events = this.pendingSessionEvents.get(sessionId); + if (events) { + this.pendingSessionEvents.delete(sessionId); + for (const event of events) { + session._dispatchEvent(event); + } + } + const waiters = this.pendingSessionWaiters.get(sessionId); + if (waiters) { + this.pendingSessionWaiters.delete(sessionId); + for (const w of waiters) { + w.resolve(session); + } + } + } + /** * Resumes an existing conversation session by its ID. * @@ -1908,9 +2174,27 @@ export class CopilotClient { return; } - const session = this.sessions.get((notification as { sessionId: string }).sessionId); + const sessionId = (notification as { sessionId: string }).sessionId; + const event = (notification as { event: SessionEvent }).event; + const session = this.sessions.get(sessionId); if (session) { - session._dispatchEvent((notification as { event: SessionEvent }).event); + session._dispatchEvent(event); + return; + } + if (this.pendingRoutingCount > 0) { + let buf = this.pendingSessionEvents.get(sessionId); + if (!buf) { + buf = []; + this.pendingSessionEvents.set(sessionId, buf); + } + if (buf.length >= CopilotClient.PENDING_SESSION_BUFFER_LIMIT) { + buf.shift(); + + console.warn( + `pending session notification buffer full for ${sessionId}; dropping oldest` + ); + } + buf.push(event); } } @@ -1969,6 +2253,30 @@ export class CopilotClient { } } + /** + * Look up the session for an inbound request. If the session is not yet + * registered but a cloud `session.create` is in flight (pending routing + * mode is active), park the request until the session is registered or + * pending mode ends. Otherwise throw immediately. + */ + private resolveSession(sessionId: string): Promise { + const session = this.sessions.get(sessionId); + if (session) { + return Promise.resolve(session); + } + if (this.pendingRoutingCount > 0) { + return new Promise((resolve, reject) => { + let waiters = this.pendingSessionWaiters.get(sessionId); + if (!waiters) { + waiters = []; + this.pendingSessionWaiters.set(sessionId, waiters); + } + waiters.push({ resolve, reject }); + }); + } + return Promise.reject(new Error(`Session not found: ${sessionId}`)); + } + private async handleUserInputRequest(params: { sessionId: string; question: string; @@ -1983,10 +2291,7 @@ export class CopilotClient { throw new Error("Invalid user input request payload"); } - const session = this.sessions.get(params.sessionId); - if (!session) { - throw new Error(`Session not found: ${params.sessionId}`); - } + const session = await this.resolveSession(params.sessionId); const result = await session._handleUserInputRequest({ question: params.question, @@ -2009,10 +2314,7 @@ export class CopilotClient { throw new Error("Invalid exit plan mode request payload"); } - const session = this.sessions.get(params.sessionId); - if (!session) { - throw new Error(`Session not found: ${params.sessionId}`); - } + const session = await this.resolveSession(params.sessionId); return await session._handleExitPlanModeRequest({ summary: params.summary, @@ -2029,10 +2331,7 @@ export class CopilotClient { throw new Error("Invalid auto mode switch request payload"); } - const session = this.sessions.get(params.sessionId); - if (!session) { - throw new Error(`Session not found: ${params.sessionId}`); - } + const session = await this.resolveSession(params.sessionId); const response = await session._handleAutoModeSwitchRequest({ errorCode: params.errorCode, @@ -2054,10 +2353,7 @@ export class CopilotClient { throw new Error("Invalid hooks invoke payload"); } - const session = this.sessions.get(params.sessionId); - if (!session) { - throw new Error(`Session not found: ${params.sessionId}`); - } + const session = await this.resolveSession(params.sessionId); const output = await session._handleHooksInvoke(params.hookType, params.input); return { output }; @@ -2076,10 +2372,7 @@ export class CopilotClient { throw new Error("Invalid systemMessage.transform payload"); } - const session = this.sessions.get(params.sessionId); - if (!session) { - throw new Error(`Session not found: ${params.sessionId}`); - } + const session = await this.resolveSession(params.sessionId); return await session._handleSystemMessageTransform(params.sections); } diff --git a/nodejs/test/client.test.ts b/nodejs/test/client.test.ts index b9a34c214..bb42755b9 100644 --- a/nodejs/test/client.test.ts +++ b/nodejs/test/client.test.ts @@ -31,7 +31,20 @@ describe("CopilotClient", () => { ); }); - it("forwards cloud options in session.create request", async () => { + it("createSession rejects cloud config", async () => { + const client = new CopilotClient(); + await client.start(); + onTestFinished(() => client.forceStop()); + + await expect( + client.createSession({ + onPermissionRequest: approveAll, + cloud: { repository: { owner: "github", name: "copilot-sdk" } }, + }) + ).rejects.toThrow(/createCloudSession/); + }); + + it("createCloudSession sends session.create with cloud and without sessionId", async () => { const client = new CopilotClient(); await client.start(); onTestFinished(() => client.forceStop()); @@ -39,21 +52,165 @@ describe("CopilotClient", () => { const spy = vi .spyOn((client as any).connection!, "sendRequest") .mockResolvedValue({ sessionId: "cloud-session" }); - await client.createSession({ + const session = await client.createCloudSession({ onPermissionRequest: approveAll, cloud: { repository: { owner: "github", name: "copilot-sdk", branch: "main" }, }, }); - expect(spy).toHaveBeenCalledWith( - "session.create", - expect.objectContaining({ - cloud: { - repository: { owner: "github", name: "copilot-sdk", branch: "main" }, - }, - }) + expect(session.sessionId).toBe("cloud-session"); + const call = spy.mock.calls.find(([m]) => m === "session.create"); + expect(call).toBeDefined(); + const payload = call![1] as Record; + expect(payload.cloud).toEqual({ + repository: { owner: "github", name: "copilot-sdk", branch: "main" }, + }); + // sessionId must be omitted: the runtime assigns it on the cloud path. + expect("sessionId" in payload).toBe(false); + }); + + it("createCloudSession rejects caller-provided sessionId", async () => { + const client = new CopilotClient(); + await client.start(); + onTestFinished(() => client.forceStop()); + + await expect( + client.createCloudSession({ + onPermissionRequest: approveAll, + sessionId: "caller-id", + cloud: { repository: { owner: "github", name: "copilot-sdk" } }, + } as never) + ).rejects.toThrow(/sessionId/); + }); + + it("createCloudSession rejects caller-provided provider", async () => { + const client = new CopilotClient(); + await client.start(); + onTestFinished(() => client.forceStop()); + + await expect( + client.createCloudSession({ + onPermissionRequest: approveAll, + provider: { baseUrl: "https://example.com", apiKey: "k" } as never, + cloud: { repository: { owner: "github", name: "copilot-sdk" } }, + } as never) + ).rejects.toThrow(/provider/); + }); + + it("createCloudSession requires cloud option", async () => { + const client = new CopilotClient(); + await client.start(); + onTestFinished(() => client.forceStop()); + + await expect( + client.createCloudSession({ onPermissionRequest: approveAll } as never) + ).rejects.toThrow(/cloud/); + }); + + it("createCloudSession buffers early session.event notifications until registration", async () => { + const client = new CopilotClient(); + await client.start(); + onTestFinished(() => client.forceStop()); + + // Capture the assigned event handler so we can fire one before the + // sendRequest promise resolves. + let dispatchEvent: + | ((event: { type: string; data: Record }) => void) + | undefined; + let resolveCreate: ((value: unknown) => void) | undefined; + vi.spyOn((client as any).connection!, "sendRequest").mockImplementation( + (method: unknown) => { + if (method !== "session.create") { + return Promise.resolve({}); + } + return new Promise((resolve) => { + resolveCreate = resolve; + }); + } ); + + const events: Array<{ type: string }> = []; + const sessionPromise = client.createCloudSession({ + onPermissionRequest: approveAll, + cloud: { repository: { owner: "github", name: "copilot-sdk" } }, + onEvent: (e) => { + events.push(e as { type: string }); + dispatchEvent = undefined; + }, + }); + + // Yield so createCloudSession reaches the in-flight sendRequest. + await new Promise((r) => setImmediate(r)); + + // Simulate the runtime pushing an early session.event addressed to + // the not-yet-registered cloud session id. + ( + client as unknown as { + handleSessionEventNotification: (n: unknown) => void; + } + ).handleSessionEventNotification({ + sessionId: "cloud-session", + event: { type: "user.message", data: { text: "hi" } }, + }); + + // Events should be buffered, not yet delivered. + expect(events).toEqual([]); + + // Now resolve the create response. + resolveCreate!({ sessionId: "cloud-session" }); + await sessionPromise; + + expect(events).toEqual([{ type: "user.message", data: { text: "hi" } }]); + // suppress unused-variable lint + void dispatchEvent; + }); + + it("createCloudSession parks inbound requests until registration", async () => { + const client = new CopilotClient(); + await client.start(); + onTestFinished(() => client.forceStop()); + + let resolveCreate: ((value: unknown) => void) | undefined; + vi.spyOn((client as any).connection!, "sendRequest").mockImplementation( + (method: unknown) => { + if (method !== "session.create") { + return Promise.resolve({}); + } + return new Promise((resolve) => { + resolveCreate = resolve; + }); + } + ); + + const sessionPromise = client.createCloudSession({ + onPermissionRequest: approveAll, + cloud: { repository: { owner: "github", name: "copilot-sdk" } }, + onUserInputRequest: async () => ({ answer: "yes", wasFreeform: false }), + }); + + // Yield so createCloudSession reaches the in-flight sendRequest. + await new Promise((r) => setImmediate(r)); + + // Simulate an inbound userInput.request for the not-yet-registered + // session id arriving during the in-flight session.create. + const parked = ( + client as unknown as { + handleUserInputRequest: (p: { + sessionId: string; + question: string; + }) => Promise<{ answer: string; wasFreeform: boolean }>; + } + ).handleUserInputRequest({ + sessionId: "cloud-session", + question: "ok?", + }); + + // Resolve the create response so the session gets registered. + resolveCreate!({ sessionId: "cloud-session" }); + await sessionPromise; + + await expect(parked).resolves.toEqual({ answer: "yes", wasFreeform: false }); }); it("forwards clientName in session.resume request", async () => { From c0a40ff63d644b8467fe499d9bbe050e1ef97bb7 Mon Sep 17 00:00:00 2001 From: Timothy Clem Date: Fri, 22 May 2026 19:39:15 -0700 Subject: [PATCH 10/14] TS: emit JSON-RPC error on pending-buffer overflow + guard drop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 491b4427. 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 e0ff254f. 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 --- nodejs/src/client.ts | 17 ++++++- nodejs/test/client.test.ts | 90 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 1 deletion(-) diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index 4410e20bf..0e94dcaf3 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -1129,9 +1129,12 @@ export class CopilotClient { this.pendingSessionEvents.clear(); for (const waiters of this.pendingSessionWaiters.values()) { for (const w of waiters) { + // Distinct phrasing from the overflow-eviction path so the + // runtime / future debugging can tell the two cases apart. + // Matches the Rust SDK message in PR #1394 (commit e0ff254f). w.reject( new Error( - "Cloud session.create completed without registering this sessionId; request dropped" + "pending session routing ended before session was registered" ) ); } @@ -2271,6 +2274,18 @@ export class CopilotClient { waiters = []; this.pendingSessionWaiters.set(sessionId, waiters); } + // Cap parked waiters per session id. When exceeded, reject the + // oldest with a distinct message; vscode-jsonrpc surfaces the + // rejection as a JSON-RPC error response to the runtime (rather + // than silently dropping, which would hang the runtime on the + // request id). Matches the Rust SDK fix in PR #1394 (commit + // 491b4427). + if (waiters.length >= CopilotClient.PENDING_SESSION_BUFFER_LIMIT) { + const oldest = waiters.shift(); + if (oldest) { + oldest.reject(new Error("pending session buffer overflow")); + } + } waiters.push({ resolve, reject }); }); } diff --git a/nodejs/test/client.test.ts b/nodejs/test/client.test.ts index bb42755b9..e2cfe598a 100644 --- a/nodejs/test/client.test.ts +++ b/nodejs/test/client.test.ts @@ -213,6 +213,96 @@ describe("CopilotClient", () => { await expect(parked).resolves.toEqual({ answer: "yes", wasFreeform: false }); }); + it("rejects parked requests with overflow error when buffer cap exceeded", async () => { + const client = new CopilotClient(); + await client.start(); + onTestFinished(() => client.forceStop()); + + let resolveCreate: ((value: unknown) => void) | undefined; + vi.spyOn((client as any).connection!, "sendRequest").mockImplementation( + (method: unknown) => { + if (method !== "session.create") return Promise.resolve({}); + return new Promise((resolve) => { + resolveCreate = resolve; + }); + } + ); + + const sessionPromise = client.createCloudSession({ + onPermissionRequest: approveAll, + cloud: { repository: { owner: "github", name: "copilot-sdk" } }, + onUserInputRequest: async () => ({ answer: "ok", wasFreeform: false }), + }); + await new Promise((r) => setImmediate(r)); + + const handler = ( + client as unknown as { + handleUserInputRequest: (p: { + sessionId: string; + question: string; + }) => Promise<{ answer: string; wasFreeform: boolean }>; + } + ).handleUserInputRequest.bind(client); + + // Park 128 + 1 requests; the oldest must reject with overflow message. + // vscode-jsonrpc translates the rejection into a JSON-RPC error response + // back to the runtime so the request id isn't left hanging. + const parked: Promise[] = []; + for (let i = 0; i < 129; i++) { + parked.push(handler({ sessionId: "cloud-session", question: `q${i}` })); + } + + await expect(parked[0]).rejects.toThrow(/pending session buffer overflow/); + + resolveCreate!({ sessionId: "cloud-session" }); + await sessionPromise; + + // The remaining 128 parked requests resolve after registration. + for (let i = 1; i < 129; i++) { + await expect(parked[i]).resolves.toEqual({ answer: "ok", wasFreeform: false }); + } + }); + + it("rejects parked requests when pending routing ends without registration", async () => { + const client = new CopilotClient(); + await client.start(); + onTestFinished(() => client.forceStop()); + + let rejectCreate: ((err: Error) => void) | undefined; + vi.spyOn((client as any).connection!, "sendRequest").mockImplementation( + (method: unknown) => { + if (method !== "session.create") return Promise.resolve({}); + return new Promise((_resolve, reject) => { + rejectCreate = reject; + }); + } + ); + + const sessionPromise = client.createCloudSession({ + onPermissionRequest: approveAll, + cloud: { repository: { owner: "github", name: "copilot-sdk" } }, + }); + await new Promise((r) => setImmediate(r)); + + const parked = ( + client as unknown as { + handleUserInputRequest: (p: { + sessionId: string; + question: string; + }) => Promise<{ answer: string; wasFreeform: boolean }>; + } + ).handleUserInputRequest({ sessionId: "cloud-session", question: "ok?" }); + + // session.create fails before registration; guard drop must reject all + // parked waiters with a distinct message (separate from overflow). + rejectCreate!(new Error("create failed")); + await expect(sessionPromise).rejects.toThrow(); + + await expect(parked).rejects.toThrow( + /pending session routing ended before session was registered/ + ); + }); + it("forwards clientName in session.resume request", async () => { const client = new CopilotClient(); await client.start(); From f4752b61861c25cde67e58e11ea48c0b8adb3cdb Mon Sep 17 00:00:00 2001 From: Timothy Clem Date: Fri, 22 May 2026 20:41:39 -0700 Subject: [PATCH 11/14] Docs: document single-PR rule for cross-SDK features --- .github/copilot-instructions.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index b058535ba..a18f7d6f1 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -39,6 +39,8 @@ ## Project-specific conventions & patterns ✅ +- **Cross-SDK features ship in a single PR.** When adding or changing a feature that lives in multiple SDKs (client/session API, wire protocol, etc.), put all the language implementations in **one** pull request — not one PR per language. The `sdk-consistency-review` workflow (`.github/workflows/sdk-consistency-review.md`) auto-reviews every PR touching `nodejs/`, `python/`, `go/`, or `dotnet/` and will flag per-language PRs as "missing in N other languages" even when follow-ups are planned. Exception: an early, isolated reference implementation (e.g. the Rust spike at PR #1394) can ship alone; the remaining languages then follow up as **one** consolidated PR. + - Tools: each SDK has helper APIs to expose functions as tools; prefer the language's `DefineTool`/`@define_tool`/`CopilotTool.DefineTool` patterns (see language READMEs). - Infinite sessions are enabled by default and persist workspace state to `~/.copilot/session-state/{sessionId}`; compaction events are emitted (`session.compaction_start`, `session.compaction_complete`). See language READMEs for usage. - Streaming: when `streaming`/`Streaming=true` you receive delta events (`assistant.message_delta`, `assistant.reasoning_delta`) and final events (`assistant.message`, `assistant.reasoning`) — tests expect this behavior. From 4f14f9801030f6037a32eba5af59a286d1d299d4 Mon Sep 17 00:00:00 2001 From: Timothy Clem Date: Fri, 22 May 2026 20:57:17 -0700 Subject: [PATCH 12/14] Python: ruff-format test_cloud_session.py Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- python/test_cloud_session.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/python/test_cloud_session.py b/python/test_cloud_session.py index a0b0ff87f..fae22d65f 100644 --- a/python/test_cloud_session.py +++ b/python/test_cloud_session.py @@ -307,8 +307,7 @@ async def test_oldest_waiter_rejected_on_overflow(self): total = _PENDING_SESSION_BUFFER_LIMIT + 1 # 129 concurrent waiters tasks = [ - asyncio.ensure_future(client._resolve_session(session_id)) - for _ in range(total) + asyncio.ensure_future(client._resolve_session(session_id)) for _ in range(total) ] # Yield so all _resolve_session calls park on futures. @@ -371,9 +370,7 @@ async def mock_request(method, params): client._client.request = mock_request session_id = "failing-cloud-session" - create_task = asyncio.ensure_future( - client.create_cloud_session(**_cloud_config()) - ) + create_task = asyncio.ensure_future(client.create_cloud_session(**_cloud_config())) # Yield so create_cloud_session enters pending-routing mode. await asyncio.sleep(0) From ed0fa34125c43fea8e79a34d6c65b4b15cd53503 Mon Sep 17 00:00:00 2001 From: Timothy Clem Date: Fri, 22 May 2026 21:20:10 -0700 Subject: [PATCH 13/14] Expose remoteUrl on Session for TS, Go, and Java 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> --- go/client.go | 1 + go/session.go | 9 +++++++ go/types.go | 1 + .../com/github/copilot/sdk/CopilotClient.java | 1 + .../github/copilot/sdk/CopilotSession.java | 26 +++++++++++++++++++ .../sdk/json/CreateSessionResponse.java | 4 +++ nodejs/src/client.ts | 3 +++ nodejs/src/session.ts | 14 ++++++++++ 8 files changed, 59 insertions(+) diff --git a/go/client.go b/go/client.go index 6ba816b59..a34269cdb 100644 --- a/go/client.go +++ b/go/client.go @@ -936,6 +936,7 @@ func (c *Client) CreateCloudSession(ctx context.Context, config *SessionConfig) sessionID := response.SessionID session := newSession(sessionID, c.client, response.WorkspacePath) + session.remoteURL = response.RemoteURL session.registerTools(config.Tools) session.registerPermissionHandler(config.OnPermissionRequest) diff --git a/go/session.go b/go/session.go index f38b4be17..06d0447ab 100644 --- a/go/session.go +++ b/go/session.go @@ -52,6 +52,7 @@ type Session struct { // SessionID is the unique identifier for this session. SessionID string workspacePath string + remoteURL string client *jsonrpc2.Client clientSessionApis *rpc.ClientSessionApiHandlers handlers []sessionHandler @@ -94,6 +95,14 @@ func (s *Session) WorkspacePath() string { return s.workspacePath } +// RemoteURL returns the remote URL for a Mission Control–backed cloud session. +// Populated from the remoteUrl field in the session.create response for cloud +// sessions created via CreateCloudSession. Returns empty string for regular +// local sessions. +func (s *Session) RemoteURL() string { + return s.remoteURL +} + // newSession creates a new session wrapper with the given session ID and client. func newSession(sessionID string, client *jsonrpc2.Client, workspacePath string) *Session { s := &Session{ diff --git a/go/types.go b/go/types.go index be86a326c..a2e7523a7 100644 --- a/go/types.go +++ b/go/types.go @@ -1413,6 +1413,7 @@ type wireCommand struct { type createSessionResponse struct { SessionID string `json:"sessionId"` WorkspacePath string `json:"workspacePath"` + RemoteURL string `json:"remoteUrl,omitempty"` Capabilities *SessionCapabilities `json:"capabilities,omitempty"` } diff --git a/java/src/main/java/com/github/copilot/sdk/CopilotClient.java b/java/src/main/java/com/github/copilot/sdk/CopilotClient.java index c7ccc8a03..4bc8a0d09 100644 --- a/java/src/main/java/com/github/copilot/sdk/CopilotClient.java +++ b/java/src/main/java/com/github/copilot/sdk/CopilotClient.java @@ -606,6 +606,7 @@ public CompletableFuture createCloudSession(SessionConfig config var flush = pendingRoutingState.registerAndFlush(returnedId, session, sessions); session.setWorkspacePath(response.workspacePath()); + session.setRemoteUrl(response.remoteUrl()); session.setCapabilities(response.capabilities()); // Replay buffered session.event notifications diff --git a/java/src/main/java/com/github/copilot/sdk/CopilotSession.java b/java/src/main/java/com/github/copilot/sdk/CopilotSession.java index 5fb8733a2..e134a3b82 100644 --- a/java/src/main/java/com/github/copilot/sdk/CopilotSession.java +++ b/java/src/main/java/com/github/copilot/sdk/CopilotSession.java @@ -154,6 +154,7 @@ public final class CopilotSession implements AutoCloseable { */ private volatile String sessionId; private volatile String workspacePath; + private volatile String remoteUrl; private volatile SessionCapabilities capabilities = new SessionCapabilities(); private final SessionUiApi ui; private final JsonRpcClient rpc; @@ -272,6 +273,31 @@ void setWorkspacePath(String workspacePath) { this.workspacePath = workspacePath; } + /** + * Gets the remote URL for a Mission Control–backed cloud session. + *

+ * Populated from the {@code remoteUrl} field in the {@code session.create} + * response for cloud sessions created via + * {@link CopilotClient#createCloudSession}. Returns {@code null} for regular + * local sessions. + * + * @return the remote URL, or {@code null} for local sessions + */ + public String getRemoteUrl() { + return remoteUrl; + } + + /** + * Sets the remote URL. Package-private; called by CopilotClient after + * session.create RPC response for cloud sessions. + * + * @param remoteUrl + * the remote URL + */ + void setRemoteUrl(String remoteUrl) { + this.remoteUrl = remoteUrl; + } + /** * Gets the capabilities reported by the host for this session. *

diff --git a/java/src/main/java/com/github/copilot/sdk/json/CreateSessionResponse.java b/java/src/main/java/com/github/copilot/sdk/json/CreateSessionResponse.java index b47af050b..27d55d897 100644 --- a/java/src/main/java/com/github/copilot/sdk/json/CreateSessionResponse.java +++ b/java/src/main/java/com/github/copilot/sdk/json/CreateSessionResponse.java @@ -11,6 +11,9 @@ * @param workspacePath * the workspace path, or {@code null} if infinite sessions are * disabled + * @param remoteUrl + * the remote URL for a cloud session, or {@code null} for local + * sessions * @param capabilities * the capabilities reported by the host, or {@code null} * @since 1.0.0 @@ -18,5 +21,6 @@ @JsonInclude(JsonInclude.Include.NON_NULL) public record CreateSessionResponse(@JsonProperty("sessionId") String sessionId, @JsonProperty("workspacePath") String workspacePath, + @JsonProperty("remoteUrl") String remoteUrl, @JsonProperty("capabilities") SessionCapabilities capabilities) { } diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index 0e94dcaf3..66f365084 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -984,6 +984,7 @@ export class CopilotClient { let response: { sessionId?: unknown; workspacePath?: string; + remoteUrl?: string; capabilities?: { ui?: { elicitation?: boolean } }; }; try { @@ -1035,6 +1036,7 @@ export class CopilotClient { })) as { sessionId?: unknown; workspacePath?: string; + remoteUrl?: string; capabilities?: { ui?: { elicitation?: boolean } }; }; } catch (e) { @@ -1090,6 +1092,7 @@ export class CopilotClient { this.sessions.set(sessionId, session); this.setupSessionFs(session, config); session["_workspacePath"] = response.workspacePath; + session["_remoteUrl"] = response.remoteUrl; session.setCapabilities(response.capabilities); // Drain anything that arrived during the in-flight session.create diff --git a/nodejs/src/session.ts b/nodejs/src/session.ts index 6f2a002b1..6484c8593 100644 --- a/nodejs/src/session.ts +++ b/nodejs/src/session.ts @@ -152,6 +152,20 @@ export class CopilotSession { return this._workspacePath; } + /** + * Remote URL for the Mission Control–backed cloud session. + * + * Populated from the `remoteUrl` field in the `session.create` response + * for cloud sessions created via {@link CopilotClient.createCloudSession}. + * Undefined for regular local sessions. + */ + get remoteUrl(): string | undefined { + return this._remoteUrl; + } + + /** @internal Populated by CopilotClient after session.create returns. */ + _remoteUrl?: string; + /** * Host capabilities reported when the session was created or resumed. * Use this to check feature support before calling capability-gated APIs. From 39b012f5e1bbe3db7b30599e0137155b74a76b48 Mon Sep 17 00:00:00 2001 From: Timothy Clem Date: Fri, 22 May 2026 21:33:51 -0700 Subject: [PATCH 14/14] Format Java create session response Apply Spotless formatting after adding the remoteUrl record component. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../com/github/copilot/sdk/json/CreateSessionResponse.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/java/src/main/java/com/github/copilot/sdk/json/CreateSessionResponse.java b/java/src/main/java/com/github/copilot/sdk/json/CreateSessionResponse.java index 27d55d897..bf4d47dc9 100644 --- a/java/src/main/java/com/github/copilot/sdk/json/CreateSessionResponse.java +++ b/java/src/main/java/com/github/copilot/sdk/json/CreateSessionResponse.java @@ -20,7 +20,6 @@ */ @JsonInclude(JsonInclude.Include.NON_NULL) public record CreateSessionResponse(@JsonProperty("sessionId") String sessionId, - @JsonProperty("workspacePath") String workspacePath, - @JsonProperty("remoteUrl") String remoteUrl, + @JsonProperty("workspacePath") String workspacePath, @JsonProperty("remoteUrl") String remoteUrl, @JsonProperty("capabilities") SessionCapabilities capabilities) { }