fix: update model version in README and improve context manager tests for CopilotClient and CopilotSession

Author: Sumanth007

python/README.md

@@ -26,7 +26,7 @@ async def main():
     # Client automatically starts on enter and cleans up on exit
     async with CopilotClient() as client:
         # Create a session with automatic cleanup
-        async with await client.create_session({"model": "gpt-5"}) as session:
+        async with await client.create_session({"model": "gpt-4"}) as session:
             # Wait for response using session.idle event
             done = asyncio.Event()
 

python/copilot/session.py

@@ -536,14 +536,17 @@ async def destroy(self) -> None:
             >>> # Clean up when done
             >>> await session.destroy()
         """
-        if self._destroyed:
-            return
+        # Ensure that the check and update of _destroyed are atomic so that
+        # only the first caller proceeds to send the destroy RPC.
+        with self._event_handlers_lock:
+            if self._destroyed:
+                return
+            self._destroyed = True
 
         try:
             await self._client.request("session.destroy", {"sessionId": self.session_id})
         finally:
-            # Mark as destroyed and clear handlers even if the request fails
-            self._destroyed = True
+            # Clear handlers even if the request fails
             with self._event_handlers_lock:
                 self._event_handlers.clear()
             with self._tool_handlers_lock:

python/e2e/test_context_managers.py

@@ -1,5 +1,7 @@
 """E2E Context Manager Tests"""
 
+import os
+
 import pytest
 
 from copilot import CopilotClient
@@ -9,10 +11,24 @@
 pytestmark = pytest.mark.asyncio(loop_scope="module")
 
 
+def create_test_client(ctx) -> CopilotClient:
+    """Create a fresh CopilotClient configured with the test harness proxy."""
+    github_token = "fake-token-for-e2e-tests" if os.environ.get("CI") == "true" else None
+    return CopilotClient(
+        {
+            "cli_path": ctx.cli_path,
+            "cwd": ctx.work_dir,
+            "env": ctx.get_env(),
+            "github_token": github_token,
+        }
+    )
+
+
 class TestCopilotClientContextManager:
     async def test_should_auto_start_and_cleanup_with_context_manager(self, ctx):
         """Test that CopilotClient context manager auto-starts and cleans up."""
-        async with ctx.client as client:
+        client = create_test_client(ctx)
+        async with client:
             assert client.get_state() == "connected"
             # Verify we can use the client
             pong = await client.ping("test")
@@ -23,7 +39,8 @@ async def test_should_auto_start_and_cleanup_with_context_manager(self, ctx):
 
     async def test_should_create_session_in_context(self, ctx):
         """Test creating and using a session within client context."""
-        async with ctx.client as client:
+        client = create_test_client(ctx)
+        async with client:
             session = await client.create_session({"model": "fake-test-model"})
             assert session.session_id
 
@@ -37,7 +54,8 @@ async def test_should_create_session_in_context(self, ctx):
 
     async def test_should_cleanup_multiple_sessions(self, ctx):
         """Test that all sessions are cleaned up when client context exits."""
-        async with ctx.client as client:
+        client = create_test_client(ctx)
+        async with client:
             session1 = await client.create_session()
             session2 = await client.create_session()
             session3 = await client.create_session()
@@ -51,8 +69,9 @@ async def test_should_cleanup_multiple_sessions(self, ctx):
 
     async def test_should_propagate_exceptions(self, ctx):
         """Test that exceptions within context are propagated."""
+        client = create_test_client(ctx)
         with pytest.raises(ValueError, match="test error"):
-            async with ctx.client as client:
+            async with client:
                 assert client.get_state() == "connected"
                 raise ValueError("test error")
 
@@ -61,7 +80,8 @@ async def test_should_propagate_exceptions(self, ctx):
 
     async def test_should_handle_cleanup_errors_gracefully(self, ctx):
         """Test that cleanup errors don't prevent context from exiting."""
-        async with ctx.client as client:
+        client = create_test_client(ctx)
+        async with client:
             await client.create_session()
 
             # Kill the process to force cleanup to fail
@@ -75,29 +95,34 @@ async def test_should_handle_cleanup_errors_gracefully(self, ctx):
 class TestCopilotSessionContextManager:
     async def test_should_cleanup_session_with_context_manager(self, ctx):
         """Test that CopilotSession context manager cleans up session."""
-        async with await ctx.client.create_session() as session:
-            assert session.session_id
-            # Send a message to verify session is working
-            await session.send({"prompt": "Hello!"})
+        client = create_test_client(ctx)
+        async with client:
+            async with await client.create_session() as session:
+                assert session.session_id
+                # Send a message to verify session is working
+                await session.send({"prompt": "Hello!"})
 
-        # After exiting context, session should be destroyed
-        with pytest.raises(Exception, match="Session not found"):
-            await session.get_messages()
+            # After exiting context, session should be destroyed
+            with pytest.raises(Exception, match="Session not found"):
+                await session.get_messages()
 
     async def test_should_propagate_exceptions_in_session_context(self, ctx):
         """Test that exceptions within session context are propagated."""
-        with pytest.raises(ValueError, match="test session error"):
-            async with await ctx.client.create_session() as session:
-                assert session.session_id
-                raise ValueError("test session error")
-
-        # Session should still be cleaned up after exception
-        with pytest.raises(Exception, match="Session not found"):
-            await session.get_messages()
+        client = create_test_client(ctx)
+        async with client:
+            with pytest.raises(ValueError, match="test session error"):
+                async with await client.create_session() as session:
+                    assert session.session_id
+                    raise ValueError("test session error")
+
+            # Session should still be cleaned up after exception
+            with pytest.raises(Exception, match="Session not found"):
+                await session.get_messages()
 
     async def test_nested_context_managers(self, ctx):
         """Test using nested context managers for client and session."""
-        async with ctx.client as client:
+        client = create_test_client(ctx)
+        async with client:
             async with await client.create_session() as session:
                 assert session.session_id
                 await session.send({"prompt": "Test message"})
@@ -111,7 +136,8 @@ async def test_nested_context_managers(self, ctx):
 
     async def test_multiple_sequential_session_contexts(self, ctx):
         """Test creating multiple sessions sequentially with context managers."""
-        async with ctx.client as client:
+        client = create_test_client(ctx)
+        async with client:
             # First session
             async with await client.create_session() as session1:
                 session1_id = session1.session_id
@@ -128,4 +154,3 @@ async def test_multiple_sequential_session_contexts(self, ctx):
             # First session should be destroyed
             with pytest.raises(Exception, match="Session not found"):
                 await session1.get_messages()
-