Use session detach for SDK cleanup (#2307)

Add real-runtime cold resume coverage across SDKs so disconnect preserves persisted sessions across client/runtime restart.

Co-authored-by: Steve Sanderson <SteveSandersonMS@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: 3 people

dotnet/src/Session.cs

@@ -2109,8 +2109,12 @@ 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)
+            {
+                LogSessionDetachFailed(SessionId, response.Error ?? "unknown error");
+            }
         }
         catch (ObjectDisposedException)
         {
@@ -2147,6 +2151,9 @@ await InvokeRpcAsync<object>(
     [LoggerMessage(Level = LogLevel.Debug, Message = "Failed to fetch tool metadata for {toolName}")]
     private partial void LogToolMetadataFetchFailed(Exception exception, string toolName);
 
+    [LoggerMessage(Level = LogLevel.Warning, Message = "Failed to detach session {sessionId}: {error}")]
+    private partial void LogSessionDetachFailed(string sessionId, string error);
+
     [LoggerMessage(Level = LogLevel.Error, Message = "Permission handler or response delivery failed. SessionId={SessionId}, RequestId={RequestId}")]
     private partial void LogPermissionHandlerOrDeliveryFailed(Exception exception, string sessionId, string requestId);
 
@@ -2184,11 +2191,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);
@@ -2221,7 +2234,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))]

dotnet/test/E2E/ClientLifecycleE2ETests.cs

@@ -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);

dotnet/test/E2E/SessionE2ETests.cs

@@ -295,6 +295,50 @@ public async Task Resumes_A_Persisted_Session_From_A_New_Client_When_An_Mcp_OAut
         Assert.Equal(sessionId, session2.SessionId);
     }
 
+    [Fact]
+    public async Task Should_Recover_Marker_After_Cold_Resume_With_Explicit_Session_Id()
+    {
+        await using var isolatedCtx = await E2ETestContext.CreateAsync();
+        await isolatedCtx.ConfigureForTestAsync("session", nameof(Should_Recover_Marker_After_Cold_Resume_With_Explicit_Session_Id));
+
+        var sessionId = $"e2e-cold-resume-{Guid.NewGuid()}";
+
+        var client1 = isolatedCtx.CreateClient();
+        var session1 = await isolatedCtx.CreateSessionAsync(client1, new SessionConfig
+        {
+            SessionId = sessionId,
+            OnPermissionRequest = PermissionHandler.ApproveAll,
+        });
+        Assert.Equal(sessionId, session1.SessionId);
+
+        var answer = await session1.SendAndWaitAsync(new MessageOptions
+        {
+            Prompt = "Please remember this exact secret marker for later - MARKER-7f3ac21e. Reply with only the single word \"Acknowledged\".",
+        });
+        Assert.NotNull(answer);
+        Assert.Contains("Acknowledged", answer!.Data.Content ?? string.Empty);
+
+        await session1.DisposeAsync();
+        await client1.ForceStopAsync();
+
+        var client2 = isolatedCtx.CreateClient();
+        var session2 = await isolatedCtx.ResumeSessionAsync(client2, sessionId, new ResumeSessionConfig
+        {
+            OnPermissionRequest = PermissionHandler.ApproveAll,
+        });
+        Assert.Equal(sessionId, session2.SessionId);
+
+        var answer2 = await session2.SendAndWaitAsync(new MessageOptions
+        {
+            Prompt = "What was the exact secret marker I asked you to remember earlier? Reply with only that marker value and nothing else.",
+        });
+        Assert.NotNull(answer2);
+        Assert.Contains("MARKER-7f3ac21e", answer2!.Data.Content ?? string.Empty);
+
+        await session2.DisposeAsync();
+        await client2.ForceStopAsync();
+    }
+
     [Fact]
     public async Task Should_Throw_Error_When_Resuming_Non_Existent_Session()
     {

dotnet/test/Harness/E2ETestBase.cs

@@ -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 releases 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(

dotnet/test/Harness/E2ETestContext.cs

@@ -595,7 +595,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);
             }

dotnet/test/Unit/ClientSessionLifetimeTests.cs

@@ -1950,7 +1950,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}'.")
             };
@@ -1987,15 +1987,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()

dotnet/test/Unit/GitHubTelemetryTests.cs

@@ -495,7 +495,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}'."),

go/client_test.go

@@ -2917,8 +2917,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

go/github_token_provider_test.go

@@ -48,8 +48,8 @@ func TestGitHubTokenProviderCreateRequestAndCallback(t *testing.T) {
 		sessionID := sessionIDFromParams(t, params)
 		return []byte(`{"sessionId":"` + sessionID + `","workspacePath":"/workspace"}`), nil
 	})
-	server.SetRequestHandler("session.destroy", func(json.RawMessage) (json.RawMessage, *jsonrpc2.Error) {
-		return []byte(`{}`), nil
+	server.SetRequestHandler("session.detach", func(json.RawMessage) (json.RawMessage, *jsonrpc2.Error) {
+		return []byte(`{"success":true}`), nil
 	})
 
 	var gotArgs GitHubTokenProviderArgs
@@ -201,8 +201,8 @@ func TestGitHubTokenStringRedactsAccessToken(t *testing.T) {
 func TestGitHubTokenProviderCleanupOnDisconnectError(t *testing.T) {
 	rpcClient, server, _ := newRuntimeShutdownRpcPair(t)
 	t.Cleanup(server.Stop)
-	server.SetRequestHandler("session.destroy", func(json.RawMessage) (json.RawMessage, *jsonrpc2.Error) {
-		return nil, &jsonrpc2.Error{Code: -32000, Message: "destroy failed"}
+	server.SetRequestHandler("session.detach", func(json.RawMessage) (json.RawMessage, *jsonrpc2.Error) {
+		return nil, &jsonrpc2.Error{Code: -32000, Message: "detach failed"}
 	})
 	client := &Client{}
 	registrationID := client.registerGitHubTokenProvider(func(GitHubTokenProviderArgs) (*GitHubTokenProviderResult, error) {
@@ -213,7 +213,7 @@ func TestGitHubTokenProviderCleanupOnDisconnectError(t *testing.T) {
 		client.unregisterGitHubTokenProvider(registrationID)
 	})
 
-	if err := session.Disconnect(); err == nil || !strings.Contains(err.Error(), "destroy failed") {
+	if err := session.Disconnect(); err == nil || !strings.Contains(err.Error(), "detach failed") {
 		t.Fatalf("Disconnect error = %v", err)
 	}
 	if len(client.gitHubTokenProviders) != 0 {

go/internal/e2e/client_options_e2e_test.go

@@ -869,6 +869,10 @@ function handleMessage(message) {
     writeResponse(message.id, { sessionId, workspacePath: null, capabilities: null });
     return;
   }
+  if (message.method === "session.detach") {
+    writeResponse(message.id, { success: true });
+    return;
+  }
   if (message.method === "session.resume") {
     const sessionId = (message.params && message.params.sessionId) || "fake-session";
     writeResponse(message.id, { sessionId, workspacePath: null, capabilities: null });