Skip to content

Commit ff151e8

Browse files
stephentoubCopilot
andcommitted
test: cover pending external tool resume in both warm and cold modes
Runtime 1.0.56 changed disconnect semantics so that ForceStop of the last RPC owner now triggers session cleanup. The previous warm-only test for `Should_Keep_Pending_External_Tool_Handleable_On_*_Resume_When_ContinuePendingWork_Is_False` asserted SessionWasActive == true and that `handlePendingToolCall` fed a result into the assistant reply -- assumptions that only hold for warm resume. On cold resume the runtime intentionally auto-completes orphan tool calls with a synthetic interrupt result, so the SDK call correctly returns success=false. Split that test into two scenarios across Node, Go, .NET, and Python: - Warm (original client stays connected): SessionWasActive=true, handlePendingToolCall returns success=true, and the assistant echoes the supplied result. - Cold (original client ForceStopped before resume): SessionWasActive=false, handlePendingToolCall returns success=false, and a follow-up turn confirms the resumed session is still healthy. In warm mode the resumed client must not re-register the external tool (it would clash with the original owner); in cold mode it re-registers with a throwing handler to assert the runtime does not re-invoke the handler on resume. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 566b2bc commit ff151e8

5 files changed

Lines changed: 445 additions & 208 deletions

File tree

dotnet/test/E2E/PendingWorkResumeE2ETests.cs

Lines changed: 60 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,23 @@ async Task<string> BlockingExternalTool([Description("Value to look up")] string
161161
}
162162

163163
[Fact]
164-
public async Task Should_Keep_Pending_External_Tool_Handleable_On_Warm_Resume_When_ContinuePendingWork_Is_False()
164+
public Task Should_Keep_Pending_External_Tool_Handleable_On_Warm_Resume_When_ContinuePendingWork_Is_False() =>
165+
AssertPendingExternalToolHandleableOnResumeAsync(
166+
disconnectOriginalClient: false,
167+
expectedSessionWasActive: true,
168+
expectedHandleResult: true);
169+
170+
[Fact]
171+
public Task Should_Keep_Pending_External_Tool_Handleable_On_Cold_Resume_When_ContinuePendingWork_Is_False() =>
172+
AssertPendingExternalToolHandleableOnResumeAsync(
173+
disconnectOriginalClient: true,
174+
expectedSessionWasActive: false,
175+
expectedHandleResult: false);
176+
177+
private async Task AssertPendingExternalToolHandleableOnResumeAsync(
178+
bool disconnectOriginalClient,
179+
bool expectedSessionWasActive,
180+
bool expectedHandleResult)
165181
{
166182
var originalToolStarted = new TaskCompletionSource<string>(TaskCreationOptions.RunContinuationsAsynchronously);
167183
var releaseOriginalTool = new TaskCompletionSource<string>(TaskCreationOptions.RunContinuationsAsynchronously);
@@ -191,28 +207,59 @@ await session1.SendAsync(new MessageOptions
191207
var toolEvent = await toolRequested;
192208
Assert.Equal("beta", await originalToolStarted.Task.WaitAsync(PendingWorkTimeout));
193209

194-
await suspendedClient.ForceStopAsync();
210+
if (disconnectOriginalClient)
211+
{
212+
await suspendedClient.ForceStopAsync();
213+
}
195214

196215
await using var resumedClient = Ctx.CreateClient(options: new CopilotClientOptions { Connection = RuntimeConnection.ForUri(cliUrl, connectionToken: SharedToken) });
197-
var session2 = await resumedClient.ResumeSessionAsync(sessionId, new ResumeSessionConfig
216+
217+
// In warm mode the original client still owns the tool registration;
218+
// re-registering it from the resumed client would cause a name-clash. In
219+
// cold mode the original is gone, so we register a fresh throwing handler
220+
// to assert the runtime doesn't re-invoke the tool on resume (orphan
221+
// auto-completion happens internally).
222+
var resumeConfig = new ResumeSessionConfig
198223
{
199224
ContinuePendingWork = false,
200225
OnPermissionRequest = PermissionHandler.ApproveAll,
201-
});
226+
};
227+
if (disconnectOriginalClient)
228+
{
229+
resumeConfig.Tools = [AIFunctionFactory.Create(ResumedExternalTool, "resume_external_tool")];
230+
}
231+
232+
var session2 = await resumedClient.ResumeSessionAsync(sessionId, resumeConfig);
202233

203234
var resumeEvent = await GetSingleResumeEventAsync(session2);
204235
Assert.Equal(false, resumeEvent.Data.ContinuePendingWork);
205-
Assert.Equal(true, resumeEvent.Data.SessionWasActive);
236+
Assert.Equal(expectedSessionWasActive, resumeEvent.Data.SessionWasActive);
206237

238+
// Warm: the runtime still has the pending request and HandlePendingToolCall
239+
// will succeed, feeding the result into the assistant's reply.
240+
// Cold: the runtime auto-completed the orphaned tool call with a synthetic
241+
// interrupt result during resume, so HandlePendingToolCall correctly reports
242+
// success=false. The session should still be healthy for new turns.
207243
var resumedResult = await session2.Rpc.Tools.HandlePendingToolCallAsync(
208244
toolEvent.Data.RequestId,
209245
result: JsonDocument.Parse("\"EXTERNAL_RESUMED_BETA\"").RootElement.Clone());
210-
Assert.True(resumedResult.Success);
211-
212-
// continuePendingWork=false may interrupt agent continuation before this response,
213-
// but the pending call should still accept an explicit completion.
246+
Assert.Equal(expectedHandleResult, resumedResult.Success);
214247
Assert.Equal(1, invocationCount);
215248

249+
if (expectedHandleResult)
250+
{
251+
var answer = await TestHelper.GetFinalAssistantMessageAsync(session2, PendingWorkTimeout);
252+
Assert.Contains("EXTERNAL_RESUMED_BETA", answer?.Data.Content ?? string.Empty);
253+
}
254+
else
255+
{
256+
var followUp = await session2.SendAndWaitAsync(new MessageOptions
257+
{
258+
Prompt = "Reply with exactly: COLD_RESUMED_FOLLOWUP",
259+
});
260+
Assert.Contains("COLD_RESUMED_FOLLOWUP", followUp?.Data.Content ?? string.Empty);
261+
}
262+
216263
await session2.DisposeAsync();
217264
await resumedClient.ForceStopAsync();
218265
}
@@ -228,6 +275,10 @@ async Task<string> BlockingExternalTool([Description("Value to look up")] string
228275
originalToolStarted.TrySetResult(value);
229276
return await releaseOriginalTool.Task;
230277
}
278+
279+
[Description("Looks up a value after resumption")]
280+
string ResumedExternalTool([Description("Value to look up")] string value) =>
281+
throw new InvalidOperationException("Resumed-session handler should not be invoked");
231282
}
232283

233284
[Fact]

go/internal/e2e/pending_work_resume_e2e_test.go

Lines changed: 162 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,10 @@ const pendingWorkTimeout = 60 * time.Second
1818

1919
// Mirrors dotnet/test/PendingWorkResumeTests.cs (snapshot category "pending_work_resume").
2020
//
21-
// Each subtest spawns a TCP server client, connects a "suspended" client through CLIUrl,
22-
// triggers some pending work (permission request or external tool call), then ForceStops
23-
// the suspended client (preserving session state) and resumes from a fresh client with
24-
// ContinuePendingWork=true.
21+
// Most subtests spawn a TCP server client, connect a "suspended" client through CLIUrl,
22+
// trigger pending work, then ForceStop the suspended client (preserving session state)
23+
// and resume from a fresh client with ContinuePendingWork=true. Warm-join coverage keeps
24+
// the original client connected while a second client resumes the same session.
2525
func TestPendingWorkResumeE2E(t *testing.T) {
2626
ctx := testharness.NewTestContext(t)
2727

@@ -433,121 +433,179 @@ func TestPendingWorkResumeE2E(t *testing.T) {
433433
resumedSession.Disconnect()
434434
})
435435

436-
t.Run("should keep pending external tool handleable on warm resume when continuependingwork is false", func(t *testing.T) {
437-
ctx.ConfigureForTest(t)
438-
439-
_, cliURL := startTcpServer(t, ctx)
440-
441-
type ValueParams struct {
442-
Value string `json:"value" jsonschema:"Value to look up"`
443-
}
444-
toolStarted := make(chan string, 1)
445-
releaseTool := make(chan string, 1)
446-
447-
originalTool := copilot.DefineTool("resume_external_tool", "Looks up a value after resumption",
448-
func(params ValueParams, inv copilot.ToolInvocation) (string, error) {
449-
select {
450-
case toolStarted <- params.Value:
451-
default:
452-
}
453-
return <-releaseTool, nil
436+
for _, scenario := range []struct {
437+
name string
438+
disconnectOriginalClient bool
439+
expectedSessionWasActive bool
440+
expectedHandleResult bool
441+
}{
442+
{name: "warm", disconnectOriginalClient: false, expectedSessionWasActive: true, expectedHandleResult: true},
443+
{name: "cold", disconnectOriginalClient: true, expectedSessionWasActive: false, expectedHandleResult: false},
444+
} {
445+
scenario := scenario
446+
t.Run(fmt.Sprintf("should keep pending external tool handleable on %s resume when continuependingwork is false", scenario.name), func(t *testing.T) {
447+
ctx.ConfigureForTest(t)
448+
449+
_, cliURL := startTcpServer(t, ctx)
450+
451+
type ValueParams struct {
452+
Value string `json:"value" jsonschema:"Value to look up"`
453+
}
454+
toolStarted := make(chan string, 1)
455+
releaseTool := make(chan string, 1)
456+
457+
originalTool := copilot.DefineTool("resume_external_tool", "Looks up a value after resumption",
458+
func(params ValueParams, inv copilot.ToolInvocation) (string, error) {
459+
select {
460+
case toolStarted <- params.Value:
461+
default:
462+
}
463+
return <-releaseTool, nil
464+
})
465+
466+
suspendedClient := ctx.NewClient(func(opts *copilot.ClientOptions) {
467+
opts.Connection = copilot.UriConnection{URL: cliURL, ConnectionToken: sharedTcpToken}
454468
})
469+
if !scenario.disconnectOriginalClient {
470+
defer suspendedClient.ForceStop()
471+
}
472+
session1, err := suspendedClient.CreateSession(t.Context(), &copilot.SessionConfig{
473+
Tools: []copilot.Tool{originalTool},
474+
OnPermissionRequest: copilot.PermissionHandler.ApproveAll,
475+
})
476+
if err != nil {
477+
t.Fatalf("Failed to create session: %v", err)
478+
}
479+
sessionID := session1.SessionID
455480

456-
suspendedClient := ctx.NewClient(func(opts *copilot.ClientOptions) {
457-
opts.Connection = copilot.UriConnection{URL: cliURL, ConnectionToken: sharedTcpToken}
458-
})
459-
session1, err := suspendedClient.CreateSession(t.Context(), &copilot.SessionConfig{
460-
Tools: []copilot.Tool{originalTool},
461-
OnPermissionRequest: copilot.PermissionHandler.ApproveAll,
462-
})
463-
if err != nil {
464-
t.Fatalf("Failed to create session: %v", err)
465-
}
466-
sessionID := session1.SessionID
467-
468-
toolEventCh := waitForExternalToolRequests(session1, []string{"resume_external_tool"})
481+
toolEventCh := waitForExternalToolRequests(session1, []string{"resume_external_tool"})
469482

470-
if _, err := session1.Send(t.Context(), copilot.MessageOptions{
471-
Prompt: "Use resume_external_tool with value 'beta', then reply with the result.",
472-
}); err != nil {
473-
t.Fatalf("Failed to send message: %v", err)
474-
}
483+
if _, err := session1.Send(t.Context(), copilot.MessageOptions{
484+
Prompt: "Use resume_external_tool with value 'beta', then reply with the result.",
485+
}); err != nil {
486+
t.Fatalf("Failed to send message: %v", err)
487+
}
475488

476-
toolEvents, err := waitForExternalToolResults(toolEventCh, pendingWorkTimeout)
477-
if err != nil {
478-
t.Fatalf("waiting for external tool requests: %v", err)
479-
}
480-
toolEvent := toolEvents["resume_external_tool"]
489+
toolEvents, err := waitForExternalToolResults(toolEventCh, pendingWorkTimeout)
490+
if err != nil {
491+
t.Fatalf("waiting for external tool requests: %v", err)
492+
}
493+
toolEvent := toolEvents["resume_external_tool"]
481494

482-
select {
483-
case v := <-toolStarted:
484-
if v != "beta" {
485-
t.Errorf("Expected original tool started with 'beta', got %q", v)
495+
select {
496+
case v := <-toolStarted:
497+
if v != "beta" {
498+
t.Errorf("Expected original tool started with 'beta', got %q", v)
499+
}
500+
case <-time.After(pendingWorkTimeout):
501+
t.Fatal("Timed out waiting for original tool to start")
486502
}
487-
case <-time.After(pendingWorkTimeout):
488-
t.Fatal("Timed out waiting for original tool to start")
489-
}
490503

491-
suspendedClient.ForceStop()
504+
if scenario.disconnectOriginalClient {
505+
suspendedClient.ForceStop()
506+
}
492507

493-
resumedClient := ctx.NewClient(func(opts *copilot.ClientOptions) {
494-
opts.Connection = copilot.UriConnection{URL: cliURL, ConnectionToken: sharedTcpToken}
495-
})
496-
t.Cleanup(func() { resumedClient.ForceStop() })
508+
resumedClient := ctx.NewClient(func(opts *copilot.ClientOptions) {
509+
opts.Connection = copilot.UriConnection{URL: cliURL, ConnectionToken: sharedTcpToken}
510+
})
511+
t.Cleanup(func() { resumedClient.ForceStop() })
512+
513+
// In warm mode the original client still owns the tool registration;
514+
// re-registering it from the resumed client would cause a name-clash. In
515+
// cold mode the original is gone, so we register a fresh throwing handler
516+
// to assert the runtime doesn't re-invoke the tool on resume (orphan
517+
// auto-completion happens internally).
518+
resumeConfig := &copilot.ResumeSessionConfig{
519+
ContinuePendingWork: false,
520+
OnPermissionRequest: copilot.PermissionHandler.ApproveAll,
521+
}
522+
if scenario.disconnectOriginalClient {
523+
resumeConfig.Tools = []copilot.Tool{
524+
copilot.DefineTool("resume_external_tool", "Looks up a value after resumption",
525+
func(_ ValueParams, _ copilot.ToolInvocation) (string, error) {
526+
t.Errorf("Resumed-session handler should not be invoked")
527+
return "", fmt.Errorf("resumed-session handler should not be invoked")
528+
}),
529+
}
530+
}
497531

498-
session2, err := resumedClient.ResumeSession(t.Context(), sessionID, &copilot.ResumeSessionConfig{
499-
ContinuePendingWork: false,
500-
OnPermissionRequest: copilot.PermissionHandler.ApproveAll,
501-
})
502-
if err != nil {
503-
t.Fatalf("Failed to resume session: %v", err)
504-
}
532+
session2, err := resumedClient.ResumeSession(t.Context(), sessionID, resumeConfig)
533+
if err != nil {
534+
t.Fatalf("Failed to resume session: %v", err)
535+
}
505536

506-
// Verify resume event reflects ContinuePendingWork=false and SessionWasActive=true
507-
messages, err := session2.GetEvents(t.Context())
508-
if err != nil {
509-
t.Fatalf("GetEvents failed: %v", err)
510-
}
511-
var resumeEvent *copilot.SessionResumeData
512-
for _, msg := range messages {
513-
if msg.Type() == copilot.SessionEventTypeSessionResume {
514-
if d, ok := msg.Data.(*copilot.SessionResumeData); ok {
515-
resumeEvent = d
516-
break
537+
messages, err := session2.GetEvents(t.Context())
538+
if err != nil {
539+
t.Fatalf("GetEvents failed: %v", err)
540+
}
541+
var resumeEvent *copilot.SessionResumeData
542+
for _, msg := range messages {
543+
if msg.Type() == copilot.SessionEventTypeSessionResume {
544+
if d, ok := msg.Data.(*copilot.SessionResumeData); ok {
545+
resumeEvent = d
546+
break
547+
}
517548
}
518549
}
519-
}
520-
if resumeEvent == nil {
521-
t.Fatal("Expected a session.resume event")
522-
return
523-
}
524-
if resumeEvent.ContinuePendingWork == nil || *resumeEvent.ContinuePendingWork != false {
525-
t.Errorf("Expected ContinuePendingWork=false in resume event, got %v", resumeEvent.ContinuePendingWork)
526-
}
527-
if resumeEvent.SessionWasActive == nil || *resumeEvent.SessionWasActive != true {
528-
t.Errorf("Expected SessionWasActive=true in resume event, got %v", resumeEvent.SessionWasActive)
529-
}
550+
if resumeEvent == nil {
551+
t.Fatal("Expected a session.resume event")
552+
return
553+
}
554+
if resumeEvent.ContinuePendingWork != nil && *resumeEvent.ContinuePendingWork {
555+
t.Errorf("Expected ContinuePendingWork=false in resume event, got %v", resumeEvent.ContinuePendingWork)
556+
}
557+
if resumeEvent.SessionWasActive == nil || *resumeEvent.SessionWasActive != scenario.expectedSessionWasActive {
558+
t.Errorf("Expected SessionWasActive=%t in resume event, got %v", scenario.expectedSessionWasActive, resumeEvent.SessionWasActive)
559+
}
530560

531-
// Even with ContinuePendingWork=false, the pending tool call should still be
532-
// handleable via HandlePendingToolCall.
533-
toolResult, err := session2.RPC.Tools.HandlePendingToolCall(t.Context(), &rpc.HandlePendingToolCallRequest{
534-
RequestID: toolEvent.RequestID,
535-
Result: rpc.ExternalToolStringResult("EXTERNAL_RESUMED_BETA"),
536-
})
537-
if err != nil {
538-
t.Fatalf("Failed to handle pending tool call: %v", err)
539-
}
540-
if !toolResult.Success {
541-
t.Errorf("Expected HandlePendingToolCall to succeed, got %+v", toolResult)
542-
}
561+
// In warm mode the runtime still has the pending request; in cold mode the
562+
// runtime auto-completed the orphan with a synthetic interrupt result during
563+
// resume, so HandlePendingToolCall is expected to report Success=false.
564+
toolResult, err := session2.RPC.Tools.HandlePendingToolCall(t.Context(), &rpc.HandlePendingToolCallRequest{
565+
RequestID: toolEvent.RequestID,
566+
Result: rpc.ExternalToolStringResult("EXTERNAL_RESUMED_BETA"),
567+
})
568+
if err != nil {
569+
t.Fatalf("Failed to handle pending tool call: %v", err)
570+
}
571+
if toolResult.Success != scenario.expectedHandleResult {
572+
t.Errorf("Expected HandlePendingToolCall Success=%t, got %+v", scenario.expectedHandleResult, toolResult)
573+
}
543574

544-
select {
545-
case releaseTool <- "ORIGINAL_SHOULD_NOT_WIN":
546-
default:
547-
}
575+
if scenario.expectedHandleResult {
576+
// Warm path: the result flows through to the LLM and the assistant should
577+
// echo it in its final reply.
578+
ctxFinal, cancel := context.WithTimeout(t.Context(), pendingWorkTimeout)
579+
defer cancel()
580+
answer, err := testharness.GetFinalAssistantMessage(ctxFinal, session2)
581+
if err != nil {
582+
t.Fatalf("Failed to wait for final assistant message: %v", err)
583+
}
584+
if assistant, ok := answer.Data.(*copilot.AssistantMessageData); !ok || !strings.Contains(assistant.Content, "EXTERNAL_RESUMED_BETA") {
585+
t.Errorf("Expected answer to contain 'EXTERNAL_RESUMED_BETA', got %v", answer.Data)
586+
}
587+
} else {
588+
// Cold path: orphan auto-completion does not trigger an LLM turn on its
589+
// own, but the session should remain healthy for new work.
590+
followUp, err := session2.SendAndWait(t.Context(), copilot.MessageOptions{
591+
Prompt: "Reply with exactly: COLD_RESUMED_FOLLOWUP",
592+
})
593+
if err != nil {
594+
t.Fatalf("Failed to send follow-up turn: %v", err)
595+
}
596+
if assistant, ok := followUp.Data.(*copilot.AssistantMessageData); !ok || !strings.Contains(assistant.Content, "COLD_RESUMED_FOLLOWUP") {
597+
t.Errorf("Expected follow-up answer to contain 'COLD_RESUMED_FOLLOWUP', got %v", followUp.Data)
598+
}
599+
}
548600

549-
session2.Disconnect()
550-
})
601+
select {
602+
case releaseTool <- "ORIGINAL_SHOULD_NOT_WIN":
603+
default:
604+
}
605+
606+
session2.Disconnect()
607+
})
608+
}
551609

552610
t.Run("should report continuependingwork true in resume event", func(t *testing.T) {
553611
ctx.ConfigureForTest(t)

0 commit comments

Comments
 (0)