From 8bbec3a2b7db4dd8e02de1d13148322cecebaf50 Mon Sep 17 00:00:00 2001 From: Jason Robert Date: Mon, 9 Mar 2026 11:58:32 -0400 Subject: [PATCH 1/2] fix(copilot): pin SDK <0.1.32 and revert session serialization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The permission-denied errors in for-each groups are caused by a regression in github-copilot-sdk 0.1.32, not the create_session race condition we diagnosed in #27. Evidence: SDK 0.1.30 (Mar 7) had zero permission denials; SDK 0.1.32 (Mar 8–9) fails consistently. - Pin github-copilot-sdk to <0.1.32 until the regression is fixed - Revert the _session_create_lock serialization from #30 since it addressed the wrong root cause Co-Authored-By: Claude Opus 4.6 --- pyproject.toml | 2 +- src/conductor/providers/copilot.py | 11 ++---- tests/test_providers/test_copilot.py | 52 ---------------------------- uv.lock | 2 +- 4 files changed, 4 insertions(+), 63 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 2c07de1..93c1e6c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -37,7 +37,7 @@ dependencies = [ "ruamel.yaml>=0.18.0", "jinja2>=3.1.0", "simpleeval>=1.0.0", - "github-copilot-sdk>=0.1.0", + "github-copilot-sdk>=0.1.0,<0.1.32", # 0.1.32 introduced permission-denied regression, see #27 "anthropic>=0.77.0,<1.0.0", "mcp>=1.0.0", "fastapi>=0.115.0", diff --git a/src/conductor/providers/copilot.py b/src/conductor/providers/copilot.py index 8af1f7b..e56469d 100644 --- a/src/conductor/providers/copilot.py +++ b/src/conductor/providers/copilot.py @@ -174,7 +174,6 @@ def __init__( self._mcp_servers = mcp_servers or {} self._started = False self._start_lock = asyncio.Lock() - self._session_create_lock = asyncio.Lock() self._idle_recovery_config = idle_recovery_config or IdleRecoveryConfig() self._temperature = temperature self._session_ids: dict[str, str] = {} @@ -471,15 +470,9 @@ async def _execute_sdk_call( ) session = None - # Fall back to creating a new session. - # Serialize session creation to prevent a race condition in the - # Copilot SDK where overlapping create_session calls (e.g. from - # for-each groups with max_concurrent > 1) can cause the CLI to - # send permission.request for a session not yet registered in - # _sessions, raising "Permission denied". See #27 / #29. + # Fall back to creating a new session if session is None: - async with self._session_create_lock: - session = await self._client.create_session(session_config) + session = await self._client.create_session(session_config) # Track session ID for checkpoint persistence sid = getattr(session, "session_id", None) diff --git a/tests/test_providers/test_copilot.py b/tests/test_providers/test_copilot.py index 7182463..bc0ef0b 100644 --- a/tests/test_providers/test_copilot.py +++ b/tests/test_providers/test_copilot.py @@ -1,6 +1,5 @@ """Unit tests for the CopilotProvider implementation.""" -import asyncio import contextlib from typing import Any @@ -557,54 +556,3 @@ def test_log_parse_recovery_truncates_long_error( max_attempts=5, error=long_error, ) - - -class TestSessionCreateLock: - """Tests for session creation serialization (issue #29). - - Verifies that concurrent create_session calls are serialized via - _session_create_lock to prevent the permission-denied race condition - described in issue #27. - """ - - def test_session_create_lock_exists(self) -> None: - """Test that CopilotProvider has a _session_create_lock attribute.""" - provider = CopilotProvider(mock_handler=stub_handler) - assert hasattr(provider, "_session_create_lock") - assert isinstance(provider._session_create_lock, asyncio.Lock) - - @pytest.mark.asyncio - async def test_session_creation_is_serialized(self) -> None: - """Test that concurrent create_session calls do not overlap. - - Simulates concurrent SDK calls by patching _execute_sdk_call to - acquire the lock and verify no two create_session calls run at - the same time. - """ - provider = CopilotProvider(mock_handler=stub_handler) - - # Track whether sessions are created concurrently - active_creates = 0 - max_concurrent_creates = 0 - creation_order: list[str] = [] - - original_lock = provider._session_create_lock - - async def mock_create_session(name: str) -> None: - nonlocal active_creates, max_concurrent_creates - async with original_lock: - active_creates += 1 - max_concurrent_creates = max(max_concurrent_creates, active_creates) - creation_order.append(name) - # Simulate session creation taking some time - await asyncio.sleep(0.05) - active_creates -= 1 - - # Launch multiple concurrent mock session creations - tasks = [asyncio.create_task(mock_create_session(f"agent_{i}")) for i in range(5)] - await asyncio.gather(*tasks) - - # With the lock, at most one create_session should run at a time - assert max_concurrent_creates == 1 - # All sessions should have been created - assert len(creation_order) == 5 diff --git a/uv.lock b/uv.lock index 2d4b0fb..4fb40e6 100644 --- a/uv.lock +++ b/uv.lock @@ -180,7 +180,7 @@ dev = [ requires-dist = [ { name = "anthropic", specifier = ">=0.77.0,<1.0.0" }, { name = "fastapi", specifier = ">=0.115.0" }, - { name = "github-copilot-sdk", specifier = ">=0.1.0" }, + { name = "github-copilot-sdk", specifier = ">=0.1.0,<0.1.32" }, { name = "jinja2", specifier = ">=3.1.0" }, { name = "mcp", specifier = ">=1.0.0" }, { name = "pydantic", specifier = ">=2.0.0" }, From 4462c396e6472ec86796840e1b32d7086a154509 Mon Sep 17 00:00:00 2001 From: Jason Robert Date: Mon, 9 Mar 2026 13:00:33 -0400 Subject: [PATCH 2/2] fix(deps): tighten SDK pin to <0.1.31 SDK 0.1.31 also has the permission-denied regression (confirmed in news repo run 22863258830). The last known good version is 0.1.30. Co-Authored-By: Claude Opus 4.6 --- pyproject.toml | 2 +- uv.lock | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 93c1e6c..46e8f4d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -37,7 +37,7 @@ dependencies = [ "ruamel.yaml>=0.18.0", "jinja2>=3.1.0", "simpleeval>=1.0.0", - "github-copilot-sdk>=0.1.0,<0.1.32", # 0.1.32 introduced permission-denied regression, see #27 + "github-copilot-sdk>=0.1.0,<0.1.31", # 0.1.31+ has permission-denied regression, see #27 "anthropic>=0.77.0,<1.0.0", "mcp>=1.0.0", "fastapi>=0.115.0", diff --git a/uv.lock b/uv.lock index 4fb40e6..15d25a4 100644 --- a/uv.lock +++ b/uv.lock @@ -180,7 +180,7 @@ dev = [ requires-dist = [ { name = "anthropic", specifier = ">=0.77.0,<1.0.0" }, { name = "fastapi", specifier = ">=0.115.0" }, - { name = "github-copilot-sdk", specifier = ">=0.1.0,<0.1.32" }, + { name = "github-copilot-sdk", specifier = ">=0.1.0,<0.1.31" }, { name = "jinja2", specifier = ">=3.1.0" }, { name = "mcp", specifier = ">=1.0.0" }, { name = "pydantic", specifier = ">=2.0.0" },