Skip to content

Commit ea76f20

Browse files
Use session detach for SDK cleanup
Add real-runtime cold resume coverage across SDKs so disconnect preserves persisted sessions across client/runtime restart. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 4c72ec1 commit ea76f20

39 files changed

Lines changed: 564 additions & 104 deletions

‎dotnet/src/Session.cs‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1962,8 +1962,12 @@ public async ValueTask DisposeAsync()
19621962

19631963
try
19641964
{
1965-
await InvokeRpcAsync<object>(
1966-
"session.destroy", [new SessionDestroyRequest() { SessionId = SessionId }], CancellationToken.None);
1965+
var response = await InvokeRpcAsync<SessionDetachResponse>(
1966+
"session.detach", [new SessionDetachRequest() { SessionId = SessionId }], CancellationToken.None);
1967+
if (!response.Success)
1968+
{
1969+
LogSessionDetachFailed(SessionId, response.Error ?? "unknown error");
1970+
}
19671971
}
19681972
catch (ObjectDisposedException)
19691973
{
@@ -2000,6 +2004,9 @@ await InvokeRpcAsync<object>(
20002004
[LoggerMessage(Level = LogLevel.Debug, Message = "Failed to fetch tool metadata for {toolName}")]
20012005
private partial void LogToolMetadataFetchFailed(Exception exception, string toolName);
20022006

2007+
[LoggerMessage(Level = LogLevel.Warning, Message = "Failed to detach session {sessionId}: {error}")]
2008+
private partial void LogSessionDetachFailed(string sessionId, string error);
2009+
20032010
[LoggerMessage(Level = LogLevel.Error, Message = "Permission handler or response delivery failed. SessionId={SessionId}, RequestId={RequestId}")]
20042011
private partial void LogPermissionHandlerOrDeliveryFailed(Exception exception, string sessionId, string requestId);
20052012

@@ -2037,11 +2044,17 @@ internal record SessionAbortRequest
20372044
public string SessionId { get; init; } = string.Empty;
20382045
}
20392046

2040-
internal record SessionDestroyRequest
2047+
internal record SessionDetachRequest
20412048
{
20422049
public string SessionId { get; init; } = string.Empty;
20432050
}
20442051

2052+
internal record SessionDetachResponse
2053+
{
2054+
public bool Success { get; init; }
2055+
public string? Error { get; init; }
2056+
}
2057+
20452058
internal void ThrowIfDisposed()
20462059
{
20472060
ObjectDisposedException.ThrowIf(Volatile.Read(ref _isDisposed) != 0, this);
@@ -2074,7 +2087,8 @@ internal void ThrowIfDisposed()
20742087
[JsonSerializable(typeof(SendMessageRequest))]
20752088
[JsonSerializable(typeof(SendMessageResponse))]
20762089
[JsonSerializable(typeof(SessionAbortRequest))]
2077-
[JsonSerializable(typeof(SessionDestroyRequest))]
2090+
[JsonSerializable(typeof(SessionDetachRequest))]
2091+
[JsonSerializable(typeof(SessionDetachResponse))]
20782092
[JsonSerializable(typeof(SessionEndHookInput))]
20792093
[JsonSerializable(typeof(SessionEndHookOutput))]
20802094
[JsonSerializable(typeof(SessionStartHookInput))]

‎dotnet/test/E2E/ClientLifecycleE2ETests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ public async Task Should_Receive_Session_Deleted_Lifecycle_Event_When_Deleted()
121121
}
122122
});
123123

124-
// Do NOT DisposeAsync the session before deleting: dispose sends session.destroy
124+
// Do NOT DisposeAsync the session before deleting: dispose sends session.detach
125125
// which closes in-memory state but does not remove the disk file; calling
126126
// delete afterwards still succeeds, but skipping dispose keeps the test minimal.
127127
await Client.DeleteSessionAsync(sessionId);

‎dotnet/test/E2E/SessionE2ETests.cs‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,50 @@ public async Task Resumes_A_Persisted_Session_From_A_New_Client_When_An_Mcp_OAut
295295
Assert.Equal(sessionId, session2.SessionId);
296296
}
297297

298+
[Fact]
299+
public async Task Should_Recover_Marker_After_Cold_Resume_With_Explicit_Session_Id()
300+
{
301+
await using var isolatedCtx = await E2ETestContext.CreateAsync();
302+
await isolatedCtx.ConfigureForTestAsync("session", nameof(Should_Recover_Marker_After_Cold_Resume_With_Explicit_Session_Id));
303+
304+
var sessionId = $"e2e-cold-resume-{Guid.NewGuid()}";
305+
306+
var client1 = isolatedCtx.CreateClient();
307+
var session1 = await isolatedCtx.CreateSessionAsync(client1, new SessionConfig
308+
{
309+
SessionId = sessionId,
310+
OnPermissionRequest = PermissionHandler.ApproveAll,
311+
});
312+
Assert.Equal(sessionId, session1.SessionId);
313+
314+
var answer = await session1.SendAndWaitAsync(new MessageOptions
315+
{
316+
Prompt = "Please remember this exact secret marker for later - MARKER-7f3ac21e. Reply with only the single word \"Acknowledged\".",
317+
});
318+
Assert.NotNull(answer);
319+
Assert.Contains("Acknowledged", answer!.Data.Content ?? string.Empty);
320+
321+
await session1.DisposeAsync();
322+
await client1.ForceStopAsync();
323+
324+
var client2 = isolatedCtx.CreateClient();
325+
var session2 = await isolatedCtx.ResumeSessionAsync(client2, sessionId, new ResumeSessionConfig
326+
{
327+
OnPermissionRequest = PermissionHandler.ApproveAll,
328+
});
329+
Assert.Equal(sessionId, session2.SessionId);
330+
331+
var answer2 = await session2.SendAndWaitAsync(new MessageOptions
332+
{
333+
Prompt = "What was the exact secret marker I asked you to remember earlier? Reply with only that marker value and nothing else.",
334+
});
335+
Assert.NotNull(answer2);
336+
Assert.Contains("MARKER-7f3ac21e", answer2!.Data.Content ?? string.Empty);
337+
338+
await session2.DisposeAsync();
339+
await client2.ForceStopAsync();
340+
}
341+
298342
[Fact]
299343
public async Task Should_Throw_Error_When_Resuming_Non_Existent_Session()
300344
{

‎dotnet/test/Harness/E2ETestBase.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ protected static async Task SuspendAndUntrackSessionForResumeAsync(CopilotSessio
113113
{
114114
await session.Rpc.SuspendAsync();
115115

116-
// In-process clients host separate runtimes, while session.destroy removes the
116+
// In-process clients host separate runtimes, while session.detach releases the
117117
// session from the current runtime. Untrack locally to exercise resume without
118118
// either replacing an active wrapper or destroying the session first.
119119
var removeFromClient = typeof(CopilotSession).GetMethod(

‎dotnet/test/Harness/E2ETestContext.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -597,7 +597,7 @@ private static async Task StopClientForCleanupAsync(CopilotClient client)
597597
$"Graceful in-process client cleanup exceeded {s_gracefulClientStopTimeout}; forcing shutdown.");
598598
await client.ForceStopAsync();
599599

600-
// Disposing the connection completes any session.destroy RPC that
600+
// Disposing the connection completes any session.detach RPC that
601601
// blocked graceful cleanup. Observe that task before continuing.
602602
await gracefulStop.WaitAsync(s_gracefulClientStopTimeout);
603603
}

‎dotnet/test/Unit/ClientSessionLifetimeTests.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1671,7 +1671,7 @@ private async Task HandleRequestAsync(Stream stream, JsonElement request, Cancel
16711671
{
16721672
["success"] = true
16731673
},
1674-
"session.destroy" => await DestroySessionAsync(cancellationToken),
1674+
"session.detach" => await DetachSessionAsync(cancellationToken),
16751675
"runtime.shutdown" => HandleRuntimeShutdown(),
16761676
_ => throw new InvalidOperationException($"Unexpected RPC method '{method}'.")
16771677
};
@@ -1708,15 +1708,15 @@ private async Task HandleRequestAsync(Stream stream, JsonElement request, Cancel
17081708
};
17091709
}
17101710

1711-
private async Task<Dictionary<string, object?>> DestroySessionAsync(CancellationToken cancellationToken)
1711+
private async Task<Dictionary<string, object?>> DetachSessionAsync(CancellationToken cancellationToken)
17121712
{
17131713
if (_delayDestroy)
17141714
{
17151715
_destroyStarted.TrySetResult();
17161716
await _allowDestroy.Task.WaitAsync(cancellationToken);
17171717
}
17181718

1719-
return [];
1719+
return new Dictionary<string, object?> { ["success"] = true };
17201720
}
17211721

17221722
private Dictionary<string, object?> HandleRuntimeShutdown()

‎dotnet/test/Unit/GitHubTelemetryTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -492,7 +492,7 @@ private async Task HandleRequestAsync(Stream stream, JsonElement request, Cancel
492492
"session.create" => CaptureCreate(request),
493493
"session.resume" => CaptureResume(request),
494494
"session.send" => new Dictionary<string, object?> { ["messageId"] = "message-1" },
495-
"session.destroy" => new Dictionary<string, object?>(),
495+
"session.detach" => new Dictionary<string, object?> { ["success"] = true },
496496
"session.options.update" => new Dictionary<string, object?> { ["success"] = true },
497497
"runtime.shutdown" => new Dictionary<string, object?>(),
498498
_ => throw new InvalidOperationException($"Unexpected RPC method '{method}'."),

‎go/client_test.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2875,8 +2875,10 @@ func serveInMemoryRuntime(t *testing.T, stdinR *io.PipeReader, stdoutW *io.PipeW
28752875
result = map[string]any{"id": "interest-1"}
28762876
case "session.options.update":
28772877
result = map[string]any{"success": true}
2878-
case "session.skills.reload", "session.destroy":
2878+
case "session.skills.reload":
28792879
result = map[string]any{}
2880+
case "session.detach":
2881+
result = map[string]any{"success": true}
28802882
default:
28812883
t.Errorf("unexpected JSON-RPC method %s", request.Method)
28822884
return

‎go/github_token_provider_test.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,8 @@ func TestGitHubTokenProviderCreateRequestAndCallback(t *testing.T) {
4848
sessionID := sessionIDFromParams(t, params)
4949
return []byte(`{"sessionId":"` + sessionID + `","workspacePath":"/workspace"}`), nil
5050
})
51-
server.SetRequestHandler("session.destroy", func(json.RawMessage) (json.RawMessage, *jsonrpc2.Error) {
52-
return []byte(`{}`), nil
51+
server.SetRequestHandler("session.detach", func(json.RawMessage) (json.RawMessage, *jsonrpc2.Error) {
52+
return []byte(`{"success":true}`), nil
5353
})
5454

5555
var gotArgs GitHubTokenProviderArgs
@@ -201,8 +201,8 @@ func TestGitHubTokenStringRedactsAccessToken(t *testing.T) {
201201
func TestGitHubTokenProviderCleanupOnDisconnectError(t *testing.T) {
202202
rpcClient, server, _ := newRuntimeShutdownRpcPair(t)
203203
t.Cleanup(server.Stop)
204-
server.SetRequestHandler("session.destroy", func(json.RawMessage) (json.RawMessage, *jsonrpc2.Error) {
205-
return nil, &jsonrpc2.Error{Code: -32000, Message: "destroy failed"}
204+
server.SetRequestHandler("session.detach", func(json.RawMessage) (json.RawMessage, *jsonrpc2.Error) {
205+
return nil, &jsonrpc2.Error{Code: -32000, Message: "detach failed"}
206206
})
207207
client := &Client{}
208208
registrationID := client.registerGitHubTokenProvider(func(GitHubTokenProviderArgs) (*GitHubTokenProviderResult, error) {
@@ -213,7 +213,7 @@ func TestGitHubTokenProviderCleanupOnDisconnectError(t *testing.T) {
213213
client.unregisterGitHubTokenProvider(registrationID)
214214
})
215215

216-
if err := session.Disconnect(); err == nil || !strings.Contains(err.Error(), "destroy failed") {
216+
if err := session.Disconnect(); err == nil || !strings.Contains(err.Error(), "detach failed") {
217217
t.Fatalf("Disconnect error = %v", err)
218218
}
219219
if len(client.gitHubTokenProviders) != 0 {

‎go/internal/e2e/client_options_e2e_test.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -869,6 +869,10 @@ function handleMessage(message) {
869869
writeResponse(message.id, { sessionId, workspacePath: null, capabilities: null });
870870
return;
871871
}
872+
if (message.method === "session.detach") {
873+
writeResponse(message.id, { success: true });
874+
return;
875+
}
872876
if (message.method === "session.resume") {
873877
const sessionId = (message.params && message.params.sessionId) || "fake-session";
874878
writeResponse(message.id, { sessionId, workspacePath: null, capabilities: null });

0 commit comments

Comments
 (0)