Skip to content
Open
39 changes: 25 additions & 14 deletions dotnet/src/Session.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1920,8 +1920,13 @@ public async ValueTask DisposeAsync()

try
{
await InvokeRpcAsync<object>(
"session.destroy", [new SessionDestroyRequest() { SessionId = SessionId }], CancellationToken.None);
var response = await InvokeRpcAsync<SessionDetachResponse>(
"session.detach", [new SessionDetachRequest() { SessionId = SessionId }], CancellationToken.None);
if (!response.Success)
{
throw new InvalidOperationException(
$"Failed to detach session {SessionId}: {response.Error ?? "unknown error"}");
Comment thread
jmoseley marked this conversation as resolved.
Outdated
Comment thread
jmoseley marked this conversation as resolved.
Outdated
}
}
catch (ObjectDisposedException)
{
Expand All @@ -1934,18 +1939,17 @@ await InvokeRpcAsync<object>(
finally
{
RemoveFromClient();
_eventHandlers = ImmutableInterlocked.InterlockedExchange(ref _eventHandlers, ImmutableArray<EventSubscription>.Empty);
_toolHandlers.Clear();
_commandHandlers.Clear();

_permissionHandler = null;
_userInputHandler = null;
_elicitationHandler = null;
_exitPlanModeHandler = null;
_autoModeSwitchHandler = null;
GC.SuppressFinalize(this);
}

_eventHandlers = ImmutableInterlocked.InterlockedExchange(ref _eventHandlers, ImmutableArray<EventSubscription>.Empty);
_toolHandlers.Clear();
_commandHandlers.Clear();

_permissionHandler = null;
_userInputHandler = null;
_elicitationHandler = null;
_exitPlanModeHandler = null;
_autoModeSwitchHandler = null;
}

[LoggerMessage(Level = LogLevel.Error, Message = "Unhandled exception in broadcast event handler")]
Expand Down Expand Up @@ -1991,11 +1995,17 @@ internal record SessionAbortRequest
public string SessionId { get; init; } = string.Empty;
}

internal record SessionDestroyRequest
internal record SessionDetachRequest
{
public string SessionId { get; init; } = string.Empty;
}

internal record SessionDetachResponse
{
public bool Success { get; init; }
public string? Error { get; init; }
}

internal void ThrowIfDisposed()
{
ObjectDisposedException.ThrowIf(Volatile.Read(ref _isDisposed) != 0, this);
Expand Down Expand Up @@ -2028,7 +2038,8 @@ internal void ThrowIfDisposed()
[JsonSerializable(typeof(SendMessageRequest))]
[JsonSerializable(typeof(SendMessageResponse))]
[JsonSerializable(typeof(SessionAbortRequest))]
[JsonSerializable(typeof(SessionDestroyRequest))]
[JsonSerializable(typeof(SessionDetachRequest))]
[JsonSerializable(typeof(SessionDetachResponse))]
[JsonSerializable(typeof(SessionEndHookInput))]
[JsonSerializable(typeof(SessionEndHookOutput))]
[JsonSerializable(typeof(SessionStartHookInput))]
Expand Down
2 changes: 1 addition & 1 deletion dotnet/test/E2E/ClientLifecycleE2ETests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public async Task Should_Receive_Session_Deleted_Lifecycle_Event_When_Deleted()
}
});

// Do NOT DisposeAsync the session before deleting: dispose sends session.destroy
// Do NOT DisposeAsync the session before deleting: dispose sends session.detach
// which closes in-memory state but does not remove the disk file; calling
// delete afterwards still succeeds, but skipping dispose keeps the test minimal.
await Client.DeleteSessionAsync(sessionId);
Expand Down
4 changes: 4 additions & 0 deletions dotnet/test/E2E/ClientOptionsE2ETests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1068,6 +1068,10 @@ function handleMessage(message) {
writeResponse(message.id, { messageId: "fake-message" });
return;
}
if (message.method === "session.detach") {
writeResponse(message.id, { success: true });
return;
}

writeResponse(message.id, {});
}
Expand Down
2 changes: 1 addition & 1 deletion dotnet/test/Harness/E2ETestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ protected static async Task SuspendAndUntrackSessionForResumeAsync(CopilotSessio
{
await session.Rpc.SuspendAsync();

// In-process clients host separate runtimes, while session.destroy removes the
// In-process clients host separate runtimes, while session.detach removes the
// session from the current runtime. Untrack locally to exercise resume without
// either replacing an active wrapper or destroying the session first.
var removeFromClient = typeof(CopilotSession).GetMethod(
Expand Down
2 changes: 1 addition & 1 deletion dotnet/test/Harness/E2ETestContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ private static async Task StopClientForCleanupAsync(CopilotClient client)
$"Graceful in-process client cleanup exceeded {s_gracefulClientStopTimeout}; forcing shutdown.");
await client.ForceStopAsync();

// Disposing the connection completes any session.destroy RPC that
// Disposing the connection completes any session.detach RPC that
// blocked graceful cleanup. Observe that task before continuing.
await gracefulStop.WaitAsync(s_gracefulClientStopTimeout);
}
Expand Down
6 changes: 3 additions & 3 deletions dotnet/test/Unit/ClientSessionLifetimeTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -847,7 +847,7 @@ private async Task HandleRequestAsync(Stream stream, JsonElement request, Cancel
{
["success"] = true
},
"session.destroy" => await DestroySessionAsync(cancellationToken),
"session.detach" => await DetachSessionAsync(cancellationToken),
"runtime.shutdown" => HandleRuntimeShutdown(),
_ => throw new InvalidOperationException($"Unexpected RPC method '{method}'.")
};
Expand Down Expand Up @@ -884,15 +884,15 @@ private async Task HandleRequestAsync(Stream stream, JsonElement request, Cancel
};
}

private async Task<Dictionary<string, object?>> DestroySessionAsync(CancellationToken cancellationToken)
private async Task<Dictionary<string, object?>> DetachSessionAsync(CancellationToken cancellationToken)
{
if (_delayDestroy)
{
_destroyStarted.TrySetResult();
await _allowDestroy.Task.WaitAsync(cancellationToken);
}

return [];
return new Dictionary<string, object?> { ["success"] = true };
}

private Dictionary<string, object?> HandleRuntimeShutdown()
Expand Down
2 changes: 1 addition & 1 deletion dotnet/test/Unit/GitHubTelemetryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ private async Task HandleRequestAsync(Stream stream, JsonElement request, Cancel
"session.create" => CaptureCreate(request),
"session.resume" => CaptureResume(request),
"session.send" => new Dictionary<string, object?> { ["messageId"] = "message-1" },
"session.destroy" => new Dictionary<string, object?>(),
"session.detach" => new Dictionary<string, object?> { ["success"] = true },
"session.options.update" => new Dictionary<string, object?> { ["success"] = true },
"runtime.shutdown" => new Dictionary<string, object?>(),
_ => throw new InvalidOperationException($"Unexpected RPC method '{method}'."),
Expand Down
14 changes: 14 additions & 0 deletions go/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -924,6 +924,13 @@ func (c *Client) CreateSession(ctx context.Context, config *SessionConfig) (*Ses
"",
hasManagedSettings(config.EnableManagedSettings, config.ManagedSettings),
)
s.onDisconnected = func() {
c.sessionsMux.Lock()
defer c.sessionsMux.Unlock()
if c.sessions[sessionID] == s {
delete(c.sessions, sessionID)
}
}

s.registerTools(config.Tools)
s.registerPermissionHandler(config.OnPermissionRequest)
Expand Down Expand Up @@ -1258,6 +1265,13 @@ func (c *Client) ResumeSessionWithOptions(ctx context.Context, sessionID string,
"",
hasManagedSettings(config.EnableManagedSettings, config.ManagedSettings),
)
session.onDisconnected = func() {
c.sessionsMux.Lock()
defer c.sessionsMux.Unlock()
if c.sessions[sessionID] == session {
delete(c.sessions, sessionID)
}
}

session.registerTools(config.Tools)
session.registerPermissionHandler(config.OnPermissionRequest)
Expand Down
34 changes: 33 additions & 1 deletion go/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1939,6 +1939,36 @@ func TestClient_MCPAuthInterestRegistration(t *testing.T) {
})
}

func TestSessionDisconnectUnregistersBeforeClientStop(t *testing.T) {
client, requests, cleanup := newInMemoryClient(t)
defer cleanup()

session, err := client.CreateSession(t.Context(), &SessionConfig{
OnPermissionRequest: PermissionHandler.ApproveAll,
})
if err != nil {
t.Fatalf("CreateSession failed: %v", err)
}
requests.clear()

if err := session.Disconnect(); err != nil {
t.Fatalf("Disconnect failed: %v", err)
}
if err := client.Stop(); err != nil {
t.Fatalf("Stop failed: %v", err)
}

detachCount := 0
for _, request := range requests.snapshot() {
if request.Method == "session.detach" {
detachCount++
}
}
if detachCount != 1 {
t.Fatalf("expected exactly one session.detach request, got %d", detachCount)
}
}

type recordedRequest struct {
Method string
Params map[string]any
Expand Down Expand Up @@ -2032,8 +2062,10 @@ func serveInMemoryRuntime(t *testing.T, stdinR *io.PipeReader, stdoutW *io.PipeW
result = map[string]any{"id": "interest-1"}
case "session.options.update":
result = map[string]any{"success": true}
case "session.skills.reload", "session.destroy":
case "session.skills.reload":
result = map[string]any{}
case "session.detach":
result = map[string]any{"success": true}
default:
t.Errorf("unexpected JSON-RPC method %s", request.Method)
return
Expand Down
4 changes: 4 additions & 0 deletions go/internal/e2e/client_options_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -874,6 +874,10 @@ function handleMessage(message) {
writeResponse(message.id, { sessionId, workspacePath: null, capabilities: null });
return;
}
if (message.method === "session.detach") {
writeResponse(message.id, { success: true });
return;
}
writeResponse(message.id, {});
}

Expand Down
29 changes: 26 additions & 3 deletions go/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,11 @@ type Session struct {

// eventCh serializes user event handler dispatch. dispatchEvent enqueues;
// a single goroutine (processEvents) dequeues and invokes handlers in FIFO order.
eventCh chan SessionEvent
closeOnce sync.Once // guards eventCh close so Disconnect is safe to call more than once
eventCh chan SessionEvent
closeOnce sync.Once // guards eventCh close so Disconnect is safe to call more than once
disconnectMu sync.Mutex
disconnected bool
onDisconnected func()

// RPC provides typed session-scoped RPC methods.
RPC *rpc.SessionRPC
Expand Down Expand Up @@ -1716,11 +1719,28 @@ func (s *Session) GetEvents(ctx context.Context) ([]SessionEvent, error) {
// log.Printf("Failed to disconnect session: %v", err)
// }
func (s *Session) Disconnect() error {
_, err := s.client.Request(context.Background(), "session.destroy", sessionDestroyRequest{SessionID: s.SessionID})
s.disconnectMu.Lock()
defer s.disconnectMu.Unlock()
if s.disconnected {
return nil
}

result, err := s.client.Request(context.Background(), "session.detach", sessionDetachRequest{SessionID: s.SessionID})
if err != nil {
return fmt.Errorf("failed to disconnect session: %w", err)
}
var response sessionDetachResponse
if err := json.Unmarshal(result, &response); err != nil {
return fmt.Errorf("failed to decode session detach response: %w", err)
}
if !response.Success {
if response.Error == "" {
response.Error = "unknown error"
}
return fmt.Errorf("failed to disconnect session: %s", response.Error)
}

s.disconnected = true
s.closeOnce.Do(func() { close(s.eventCh) })

// Clear handlers
Expand All @@ -1744,6 +1764,9 @@ func (s *Session) Disconnect() error {
s.elicitationHandler = nil
s.elicitationMu.Unlock()

if s.onDisconnected != nil {
s.onDisconnected()
}
return nil
}

Expand Down
9 changes: 7 additions & 2 deletions go/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -2698,11 +2698,16 @@ type sessionGetMessagesResponse struct {
Events []SessionEvent `json:"events"`
}

// sessionDestroyRequest is the request for session.destroy
type sessionDestroyRequest struct {
// sessionDetachRequest is the request for session.detach
type sessionDetachRequest struct {
SessionID string `json:"sessionId"`
}

type sessionDetachResponse struct {
Success bool `json:"success"`
Error string `json:"error,omitempty"`
}

// sessionAbortRequest is the request for session.abort
type sessionAbortRequest struct {
SessionID string `json:"sessionId"`
Expand Down
24 changes: 16 additions & 8 deletions java/src/main/java/com/github/copilot/CopilotClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -381,26 +381,32 @@ public CompletableFuture<Void> stop() {

for (CopilotSession session : new ArrayList<>(sessions.values())) {
Runnable closeTask = () -> {
try {
session.close();
} catch (Exception e) {
LOG.log(Level.WARNING, "Error closing session " + session.getSessionId(), e);
}
session.close();
};
CompletableFuture<Void> future;
try {
future = CompletableFuture.runAsync(closeTask, executor);
} catch (RejectedExecutionException e) {
LOG.log(Level.WARNING, "Executor rejected session close task; closing inline", e);
closeTask.run();
future = CompletableFuture.completedFuture(null);
try {
closeTask.run();
future = CompletableFuture.completedFuture(null);
} catch (RuntimeException closeError) {
future = CompletableFuture.failedFuture(closeError);
}
}
closeFutures.add(future);
}
sessions.clear();

return CompletableFuture.allOf(closeFutures.toArray(new CompletableFuture[0]))
.thenCompose(v -> cleanupConnection(true));
.handle((ignored, closeError) -> closeError)
.thenCompose(closeError -> cleanupConnection(true).thenApply(ignored -> {
if (closeError != null) {
throw new CompletionException(closeError);
}
return null;
}));
}

/**
Expand Down Expand Up @@ -571,6 +577,7 @@ public CompletableFuture<CopilotSession> createSession(SessionConfig config) {
long setupNanos = System.nanoTime();
var s = new CopilotSession(sid, connection.rpc);
s.setExecutor(executor);
s.setOnClosed(() -> sessions.remove(sid, s));
SessionRequestBuilder.configureSession(s, config);
if (extracted.transformCallbacks() != null) {
s.registerTransformCallbacks(extracted.transformCallbacks());
Expand Down Expand Up @@ -743,6 +750,7 @@ public CompletableFuture<CopilotSession> resumeSession(String sessionId, ResumeS
long setupNanos = System.nanoTime();
var session = new CopilotSession(sessionId, connection.rpc);
session.setExecutor(executor);
session.setOnClosed(() -> sessions.remove(sessionId, session));
SessionRequestBuilder.configureSession(session, config);
sessions.put(sessionId, session);
LoggingHelpers.logTiming(LOG, Level.FINE,
Expand Down
Loading
Loading