Skip to content

Commit 395c319

Browse files
stephentoubCopilot
andcommitted
Reword pending-resume skip messages to match actual runtime contract
The previous wording ('Pending runtime fix') implied a runtime change was being tracked separately. That isn't accurate: runtime 1.0.56 (copilot-agent-runtime PR #9040) intentionally cleans up the session when the last RPC client disconnects, and there is no in-flight fix planned. The OLD tests model same-process ForceStop+resume and rely on the old behavior where the session stayed alive in the runtime process. They need to be redesigned to either keep an owner connected (warm resume) or to model true process-restart resume from persisted session state. No code change beyond the skip-reason text; tests remain skipped while the redesign is figured out. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 5f87c88 commit 395c319

4 files changed

Lines changed: 61 additions & 40 deletions

File tree

dotnet/test/E2E/PendingWorkResumeE2ETests.cs

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,14 @@ public class PendingWorkResumeE2ETests(E2ETestFixture fixture, ITestOutputHelper
1919
private static readonly TimeSpan PendingWorkTimeout = TimeSpan.FromSeconds(60);
2020
private const string SharedToken = "pending-work-resume-shared-token";
2121

22-
// TODO: Re-enable once the runtime restores warm-resume behavior for pending permission
23-
// requests after the original client disconnects. Runtime PR #9040 (commit b8e1220b45)
24-
// now cleans up session state when the last RPC owner disconnects, causing
25-
// HandlePendingPermissionRequest to return success=false on resume. A runtime-side
26-
// fix is being tracked separately.
27-
[Fact(Skip = "Pending runtime fix: cold cleanup contract makes HandlePendingPermissionRequest return success=false after disconnect+resume.")]
22+
// Skipped after the runtime 1.0.56 bump. Runtime PR #9040 (commit b8e1220b45) changed
23+
// SDKServer.handleConnectionClosed to tear down the session when the last RPC client
24+
// disconnects, so the in-memory pending permission request is gone before the resumed
25+
// client can satisfy it and HandlePendingPermissionRequest returns success=false. This
26+
// test models same-process ForceStop+resume; it needs to be redesigned to either keep
27+
// an owner connected (warm resume) or to model a true process restart against the
28+
// persisted session state.
29+
[Fact(Skip = "Runtime 1.0.56 cleans up the session on last-client disconnect (copilot-agent-runtime PR #9040), so the in-memory pending request is gone before resume can satisfy it. Test needs redesign.")]
2830
public async Task Should_Continue_Pending_Permission_Request_After_Resume()
2931
{
3032
var originalPermissionRequest = new TaskCompletionSource<PermissionRequest>(TaskCreationOptions.RunContinuationsAsynchronously);
@@ -93,10 +95,12 @@ static string ResumePermissionTool([Description("Value to transform")] string va
9395
$"ORIGINAL_SHOULD_NOT_RUN_{value}";
9496
}
9597

96-
// TODO: Re-enable once the runtime restores warm-resume behavior for pending external
97-
// tool calls after the original client disconnects. Same root cause as
98-
// Should_Continue_Pending_Permission_Request_After_Resume above.
99-
[Fact(Skip = "Pending runtime fix: cold cleanup contract makes HandlePendingToolCall return success=false after disconnect+resume.")]
98+
// Skipped for the same reason as Should_Continue_Pending_Permission_Request_After_Resume:
99+
// runtime 1.0.56 (copilot-agent-runtime PR #9040) tears down the session when the last
100+
// RPC client disconnects, so the in-memory pending external tool call is gone before
101+
// the resumed client can satisfy it. Needs redesign to keep an owner connected (warm)
102+
// or to model true process-restart resume from persisted state.
103+
[Fact(Skip = "Runtime 1.0.56 cleans up the session on last-client disconnect (copilot-agent-runtime PR #9040), so the in-memory pending tool call is gone before resume can satisfy it. Test needs redesign.")]
100104
public async Task Should_Continue_Pending_External_Tool_Request_After_Resume()
101105
{
102106
var originalToolStarted = new TaskCompletionSource<string>(TaskCreationOptions.RunContinuationsAsynchronously);

go/internal/e2e/pending_work_resume_e2e_test.go

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,14 @@ func TestPendingWorkResumeE2E(t *testing.T) {
2525
ctx := testharness.NewTestContext(t)
2626

2727
t.Run("should continue pending permission request after resume", func(t *testing.T) {
28-
// TODO: Re-enable once the runtime restores warm-resume behavior for pending
29-
// permission requests after the original client disconnects. Runtime PR #9040
30-
// (commit b8e1220b45) now cleans up session state when the last RPC owner
31-
// disconnects, causing HandlePendingPermissionRequest to return Success=false
32-
// on resume. A runtime-side fix is being tracked separately.
33-
t.Skip("Pending runtime fix: cold cleanup contract makes HandlePendingPermissionRequest return Success=false after disconnect+resume.")
28+
// Skipped after the runtime 1.0.56 bump. Runtime PR #9040 (commit b8e1220b45)
29+
// changed SDKServer.handleConnectionClosed to tear down the session when the
30+
// last RPC client disconnects, so the in-memory pending permission request is
31+
// gone before the resumed client can satisfy it and HandlePendingPermissionRequest
32+
// returns Success=false. This test models same-process ForceStop+resume; it
33+
// needs to be redesigned to either keep an owner connected (warm resume) or to
34+
// model a true process restart against the persisted session state.
35+
t.Skip("Runtime 1.0.56 cleans up the session on last-client disconnect (copilot-agent-runtime PR #9040), so the in-memory pending request is gone before resume can satisfy it. Test needs redesign.")
3436

3537
ctx.ConfigureForTest(t)
3638

@@ -145,10 +147,13 @@ func TestPendingWorkResumeE2E(t *testing.T) {
145147
})
146148

147149
t.Run("should continue pending external tool request after resume", func(t *testing.T) {
148-
// TODO: Re-enable once the runtime restores warm-resume behavior for pending
149-
// external tool calls after the original client disconnects. Same root cause as
150-
// "should continue pending permission request after resume" above.
151-
t.Skip("Pending runtime fix: cold cleanup contract makes HandlePendingToolCall return Success=false after disconnect+resume.")
150+
// Skipped for the same reason as "should continue pending permission request
151+
// after resume": runtime 1.0.56 (copilot-agent-runtime PR #9040) tears down
152+
// the session when the last RPC client disconnects, so the in-memory pending
153+
// external tool call is gone before the resumed client can satisfy it. Needs
154+
// redesign to keep an owner connected (warm) or to model true process-restart
155+
// resume from persisted state.
156+
t.Skip("Runtime 1.0.56 cleans up the session on last-client disconnect (copilot-agent-runtime PR #9040), so the in-memory pending tool call is gone before resume can satisfy it. Test needs redesign.")
152157

153158
ctx.ConfigureForTest(t)
154159

nodejs/test/e2e/pending_work_resume.e2e.test.ts

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -166,11 +166,13 @@ describe("Pending work resume", async () => {
166166
return `localhost:${port}`;
167167
}
168168

169-
// TODO: Re-enable once the runtime restores warm-resume behavior for pending
170-
// permission requests after the original client disconnects. Runtime PR #9040
171-
// (commit b8e1220b45) now cleans up session state when the last RPC owner
172-
// disconnects, causing handlePendingPermissionRequest to return success=false
173-
// on resume. A runtime-side fix is being tracked separately.
169+
// Skipped after the runtime 1.0.56 bump. Runtime PR #9040 (commit b8e1220b45)
170+
// changed SDKServer.handleConnectionClosed to tear down the session when the
171+
// last RPC client disconnects, so the in-memory pending permission request is
172+
// gone before the resumed client can satisfy it and handlePendingPermissionRequest
173+
// returns success=false. This test models same-process ForceStop+resume; it needs
174+
// to be redesigned to either keep an owner connected (warm resume) or to model
175+
// a true process restart against the persisted session state.
174176
it.skip(
175177
"should continue pending permission request after resume",
176178
{ timeout: TEST_TIMEOUT_MS },
@@ -244,9 +246,12 @@ describe("Pending work resume", async () => {
244246
}
245247
);
246248

247-
// TODO: Re-enable once the runtime restores warm-resume behavior for pending
248-
// external tool calls after the original client disconnects. Same root cause as
249-
// "should continue pending permission request after resume" above.
249+
// Skipped for the same reason as "should continue pending permission request
250+
// after resume": runtime 1.0.56 (copilot-agent-runtime PR #9040) tears down the
251+
// session when the last RPC client disconnects, so the in-memory pending external
252+
// tool call is gone before the resumed client can satisfy it. Needs redesign to
253+
// keep an owner connected (warm) or to model true process-restart resume from
254+
// persisted state.
250255
it.skip(
251256
"should continue pending external tool request after resume",
252257
{ timeout: TEST_TIMEOUT_MS },

python/e2e/test_pending_work_resume_e2e.py

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -128,15 +128,18 @@ async def _safe_force_stop(client: CopilotClient) -> None:
128128

129129

130130
class TestPendingWorkResume:
131-
# TODO: Re-enable once the runtime restores warm-resume behavior for pending
132-
# permission requests after the original client disconnects. Runtime PR #9040
133-
# (commit b8e1220b45) now cleans up session state when the last RPC owner
134-
# disconnects, causing handle_pending_permission_request to return success=False
135-
# on resume. A runtime-side fix is being tracked separately.
131+
# Skipped after the runtime 1.0.56 bump. Runtime PR #9040 (commit b8e1220b45)
132+
# changed SDKServer.handleConnectionClosed to tear down the session when the last
133+
# RPC client disconnects, so the in-memory pending permission request is gone
134+
# before the resumed client can satisfy it and handle_pending_permission_request
135+
# returns success=False. This test models same-process force_stop+resume; it
136+
# needs to be redesigned to either keep an owner connected (warm resume) or to
137+
# model a true process restart against the persisted session state.
136138
@pytest.mark.skip(
137139
reason=(
138-
"Pending runtime fix: cold cleanup contract makes "
139-
"handle_pending_permission_request return success=False after disconnect+resume."
140+
"Runtime 1.0.56 cleans up the session on last-client disconnect "
141+
"(copilot-agent-runtime PR #9040), so the in-memory pending request "
142+
"is gone before resume can satisfy it. Test needs redesign."
140143
)
141144
)
142145
async def test_should_continue_pending_permission_request_after_resume(
@@ -219,13 +222,17 @@ def resumed_tool_handler(args):
219222
finally:
220223
await _safe_force_stop(server)
221224

222-
# TODO: Re-enable once the runtime restores warm-resume behavior for pending
223-
# external tool calls after the original client disconnects. Same root cause as
224-
# test_should_continue_pending_permission_request_after_resume above.
225+
# Skipped for the same reason as
226+
# test_should_continue_pending_permission_request_after_resume: runtime 1.0.56
227+
# (copilot-agent-runtime PR #9040) tears down the session when the last RPC
228+
# client disconnects, so the in-memory pending external tool call is gone before
229+
# the resumed client can satisfy it. Needs redesign to keep an owner connected
230+
# (warm) or to model true process-restart resume from persisted state.
225231
@pytest.mark.skip(
226232
reason=(
227-
"Pending runtime fix: cold cleanup contract makes "
228-
"handle_pending_tool_call return success=False after disconnect+resume."
233+
"Runtime 1.0.56 cleans up the session on last-client disconnect "
234+
"(copilot-agent-runtime PR #9040), so the in-memory pending tool call "
235+
"is gone before resume can satisfy it. Test needs redesign."
229236
)
230237
)
231238
async def test_should_continue_pending_external_tool_request_after_resume(

0 commit comments

Comments
 (0)