From e4ed36c698369a3811f7fd3573cfa9d9adea1a7b Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Fri, 31 Jul 2026 21:57:22 -0400 Subject: [PATCH 1/4] Fix background task removal E2E race Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b3ebc2e7-3ffd-4573-becd-bb1954344115 --- .../test/E2E/RpcTasksAndHandlersE2ETests.cs | 21 ++++++++++++------- python/e2e/test_rpc_tasks_and_handlers_e2e.py | 16 +++++++++----- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/dotnet/test/E2E/RpcTasksAndHandlersE2ETests.cs b/dotnet/test/E2E/RpcTasksAndHandlersE2ETests.cs index 989fef7c55..a446080924 100644 --- a/dotnet/test/E2E/RpcTasksAndHandlersE2ETests.cs +++ b/dotnet/test/E2E/RpcTasksAndHandlersE2ETests.cs @@ -152,17 +152,16 @@ await TestHelper.WaitForConditionAsync( async () => { task = await FindAgentTaskAsync(session, started.AgentId); - return task?.LatestResponse?.Contains("TASK_AGENT_DONE", StringComparison.Ordinal) == true - || task?.Result?.Contains("TASK_AGENT_DONE", StringComparison.Ordinal) == true - || task?.Status == GitHub.Copilot.Rpc.TaskStatus.Completed - || task?.Status == GitHub.Copilot.Rpc.TaskStatus.Failed; + return task?.Status == GitHub.Copilot.Rpc.TaskStatus.Completed + || task?.Status == GitHub.Copilot.Rpc.TaskStatus.Failed + || task?.Status == GitHub.Copilot.Rpc.TaskStatus.Cancelled + || task?.Status == GitHub.Copilot.Rpc.TaskStatus.Idle; }, timeout: TimeSpan.FromSeconds(60), timeoutMessage: $"Background agent task '{started.AgentId}' did not produce a final observable state."); Assert.NotNull(task); Assert.Contains("TASK_AGENT_DONE", task.LatestResponse ?? task.Result ?? string.Empty); - await taskCompletionNotification.Task.WaitAsync(TimeSpan.FromSeconds(30)); if (task.Status == GitHub.Copilot.Rpc.TaskStatus.Idle) { @@ -171,10 +170,16 @@ await TestHelper.WaitForConditionAsync( } var remove = await session.Rpc.Tasks.RemoveAsync(started.AgentId); - Assert.True(remove.Removed); - var afterRemove = await session.Rpc.Tasks.ListAsync(); - Assert.DoesNotContain(afterRemove.Tasks.OfType(), t => string.Equals(t.Id, started.AgentId, StringComparison.Ordinal)); + var taskAfterRemove = afterRemove.Tasks.OfType() + .SingleOrDefault(t => string.Equals(t.Id, started.AgentId, StringComparison.Ordinal)); + // Completion delivery also removes finished tasks, so either this call wins or the task is already absent. + Assert.True( + remove.Removed || taskAfterRemove is null, + $"Background agent task '{started.AgentId}' remained tracked after remove returned false."); + Assert.Null(taskAfterRemove); + + await taskCompletionNotification.Task.WaitAsync(TimeSpan.FromSeconds(30)); } [Fact] diff --git a/python/e2e/test_rpc_tasks_and_handlers_e2e.py b/python/e2e/test_rpc_tasks_and_handlers_e2e.py index 6a99cbb75d..fcdc4abd1b 100644 --- a/python/e2e/test_rpc_tasks_and_handlers_e2e.py +++ b/python/e2e/test_rpc_tasks_and_handlers_e2e.py @@ -447,18 +447,24 @@ def on_event(event): ) assert found_task is not None, f"Task {task_id} disappeared before it completed" assert "TASK_AGENT_DONE" in (found_task.latest_response or found_task.result or "") - await asyncio.wait_for(task_completion_notification, timeout=30.0) if found_task.status == TaskInfoStatus.IDLE: cancel = await session.rpc.tasks.cancel(TasksCancelRequest(id=task_id)) assert cancel.cancelled is True - # Remove the task remove = await session.rpc.tasks.remove(TasksRemoveRequest(id=task_id)) - assert remove.removed is True - after_remove = await session.rpc.tasks.list() - assert not any(t.id == task_id for t in (after_remove.tasks or [])) + task_after_remove = next( + (task for task in (after_remove.tasks or []) if task.id == task_id), + None, + ) + # Completion delivery also removes finished tasks, so this call may lose that race. + assert remove.removed or task_after_remove is None, ( + f"Task {task_id} remained tracked after remove returned false" + ) + assert task_after_remove is None + + await asyncio.wait_for(task_completion_notification, timeout=30.0) finally: unsubscribe() await session.disconnect() From a240bb1462eb3445ff1a3ed70962242bb766b1f1 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Fri, 31 Jul 2026 22:28:23 -0400 Subject: [PATCH 2/4] Strengthen task removal race coverage Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b3ebc2e7-3ffd-4573-becd-bb1954344115 --- dotnet/test/E2E/RpcTasksAndHandlersE2ETests.cs | 13 +++++++------ python/e2e/test_rpc_tasks_and_handlers_e2e.py | 4 ++-- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/dotnet/test/E2E/RpcTasksAndHandlersE2ETests.cs b/dotnet/test/E2E/RpcTasksAndHandlersE2ETests.cs index a446080924..236706ae4b 100644 --- a/dotnet/test/E2E/RpcTasksAndHandlersE2ETests.cs +++ b/dotnet/test/E2E/RpcTasksAndHandlersE2ETests.cs @@ -152,10 +152,11 @@ await TestHelper.WaitForConditionAsync( async () => { task = await FindAgentTaskAsync(session, started.AgentId); - return task?.Status == GitHub.Copilot.Rpc.TaskStatus.Completed - || task?.Status == GitHub.Copilot.Rpc.TaskStatus.Failed - || task?.Status == GitHub.Copilot.Rpc.TaskStatus.Cancelled - || task?.Status == GitHub.Copilot.Rpc.TaskStatus.Idle; + return task is null + || task.Status == GitHub.Copilot.Rpc.TaskStatus.Completed + || task.Status == GitHub.Copilot.Rpc.TaskStatus.Failed + || task.Status == GitHub.Copilot.Rpc.TaskStatus.Cancelled + || task.Status == GitHub.Copilot.Rpc.TaskStatus.Idle; }, timeout: TimeSpan.FromSeconds(60), timeoutMessage: $"Background agent task '{started.AgentId}' did not produce a final observable state."); @@ -175,8 +176,8 @@ await TestHelper.WaitForConditionAsync( .SingleOrDefault(t => string.Equals(t.Id, started.AgentId, StringComparison.Ordinal)); // Completion delivery also removes finished tasks, so either this call wins or the task is already absent. Assert.True( - remove.Removed || taskAfterRemove is null, - $"Background agent task '{started.AgentId}' remained tracked after remove returned false."); + remove.Removed || taskCompletionNotification.Task.IsCompleted, + $"Background agent task '{started.AgentId}' was not removed before its completion notification was delivered."); Assert.Null(taskAfterRemove); await taskCompletionNotification.Task.WaitAsync(TimeSpan.FromSeconds(30)); diff --git a/python/e2e/test_rpc_tasks_and_handlers_e2e.py b/python/e2e/test_rpc_tasks_and_handlers_e2e.py index fcdc4abd1b..48cd59fd33 100644 --- a/python/e2e/test_rpc_tasks_and_handlers_e2e.py +++ b/python/e2e/test_rpc_tasks_and_handlers_e2e.py @@ -459,8 +459,8 @@ def on_event(event): None, ) # Completion delivery also removes finished tasks, so this call may lose that race. - assert remove.removed or task_after_remove is None, ( - f"Task {task_id} remained tracked after remove returned false" + assert remove.removed or task_completion_notification.done(), ( + f"Task {task_id} was not removed before its completion notification was delivered" ) assert task_after_remove is None From d6531dfe3fc4e0c103746d165928da20cbed7079 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Fri, 31 Jul 2026 22:43:00 -0400 Subject: [PATCH 3/4] Handle completion cleanup before task polling Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b3ebc2e7-3ffd-4573-becd-bb1954344115 --- .../test/E2E/RpcTasksAndHandlersE2ETests.cs | 25 +++++++++++-------- python/e2e/test_rpc_tasks_and_handlers_e2e.py | 21 ++++++++-------- 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/dotnet/test/E2E/RpcTasksAndHandlersE2ETests.cs b/dotnet/test/E2E/RpcTasksAndHandlersE2ETests.cs index 236706ae4b..640e4f72f5 100644 --- a/dotnet/test/E2E/RpcTasksAndHandlersE2ETests.cs +++ b/dotnet/test/E2E/RpcTasksAndHandlersE2ETests.cs @@ -161,23 +161,26 @@ await TestHelper.WaitForConditionAsync( timeout: TimeSpan.FromSeconds(60), timeoutMessage: $"Background agent task '{started.AgentId}' did not produce a final observable state."); - Assert.NotNull(task); - Assert.Contains("TASK_AGENT_DONE", task.LatestResponse ?? task.Result ?? string.Empty); - - if (task.Status == GitHub.Copilot.Rpc.TaskStatus.Idle) + if (task is not null) { - var cancel = await session.Rpc.Tasks.CancelAsync(started.AgentId); - Assert.True(cancel.Cancelled); + Assert.Contains("TASK_AGENT_DONE", task.LatestResponse ?? task.Result ?? string.Empty); + + if (task.Status == GitHub.Copilot.Rpc.TaskStatus.Idle) + { + var cancel = await session.Rpc.Tasks.CancelAsync(started.AgentId); + Assert.True(cancel.Cancelled); + } + + var remove = await session.Rpc.Tasks.RemoveAsync(started.AgentId); + // Completion delivery also removes finished tasks, so this call may lose that race. + Assert.True( + remove.Removed || taskCompletionNotification.Task.IsCompleted, + $"Background agent task '{started.AgentId}' was not removed before its completion notification was delivered."); } - var remove = await session.Rpc.Tasks.RemoveAsync(started.AgentId); var afterRemove = await session.Rpc.Tasks.ListAsync(); var taskAfterRemove = afterRemove.Tasks.OfType() .SingleOrDefault(t => string.Equals(t.Id, started.AgentId, StringComparison.Ordinal)); - // Completion delivery also removes finished tasks, so either this call wins or the task is already absent. - Assert.True( - remove.Removed || taskCompletionNotification.Task.IsCompleted, - $"Background agent task '{started.AgentId}' was not removed before its completion notification was delivered."); Assert.Null(taskAfterRemove); await taskCompletionNotification.Task.WaitAsync(TimeSpan.FromSeconds(30)); diff --git a/python/e2e/test_rpc_tasks_and_handlers_e2e.py b/python/e2e/test_rpc_tasks_and_handlers_e2e.py index 48cd59fd33..558a1386c4 100644 --- a/python/e2e/test_rpc_tasks_and_handlers_e2e.py +++ b/python/e2e/test_rpc_tasks_and_handlers_e2e.py @@ -445,23 +445,24 @@ def on_event(event): 60.0, f"Task {task_id} did not produce a final observable state", ) - assert found_task is not None, f"Task {task_id} disappeared before it completed" - assert "TASK_AGENT_DONE" in (found_task.latest_response or found_task.result or "") + if found_task is not None: + assert "TASK_AGENT_DONE" in (found_task.latest_response or found_task.result or "") - if found_task.status == TaskInfoStatus.IDLE: - cancel = await session.rpc.tasks.cancel(TasksCancelRequest(id=task_id)) - assert cancel.cancelled is True + if found_task.status == TaskInfoStatus.IDLE: + cancel = await session.rpc.tasks.cancel(TasksCancelRequest(id=task_id)) + assert cancel.cancelled is True + + remove = await session.rpc.tasks.remove(TasksRemoveRequest(id=task_id)) + # Completion delivery also removes finished tasks, so this call may lose that race. + assert remove.removed or task_completion_notification.done(), ( + f"Task {task_id} was not removed before its completion notification was delivered" + ) - remove = await session.rpc.tasks.remove(TasksRemoveRequest(id=task_id)) after_remove = await session.rpc.tasks.list() task_after_remove = next( (task for task in (after_remove.tasks or []) if task.id == task_id), None, ) - # Completion delivery also removes finished tasks, so this call may lose that race. - assert remove.removed or task_completion_notification.done(), ( - f"Task {task_id} was not removed before its completion notification was delivered" - ) assert task_after_remove is None await asyncio.wait_for(task_completion_notification, timeout=30.0) From ea6e145ff38e96a690e0ac504ea3d417b65a6eef Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Fri, 31 Jul 2026 23:06:03 -0400 Subject: [PATCH 4/4] Fix Python E2E lint failure Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: b3ebc2e7-3ffd-4573-becd-bb1954344115 --- python/e2e/test_rpc_tasks_and_handlers_e2e.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/python/e2e/test_rpc_tasks_and_handlers_e2e.py b/python/e2e/test_rpc_tasks_and_handlers_e2e.py index 558a1386c4..f0dd8f7577 100644 --- a/python/e2e/test_rpc_tasks_and_handlers_e2e.py +++ b/python/e2e/test_rpc_tasks_and_handlers_e2e.py @@ -455,7 +455,8 @@ def on_event(event): remove = await session.rpc.tasks.remove(TasksRemoveRequest(id=task_id)) # Completion delivery also removes finished tasks, so this call may lose that race. assert remove.removed or task_completion_notification.done(), ( - f"Task {task_id} was not removed before its completion notification was delivered" + f"Task {task_id} was not removed before its completion " + "notification was delivered" ) after_remove = await session.rpc.tasks.list()