From 985e830ea2cb8766672325402fd0d63805da7e3a Mon Sep 17 00:00:00 2001 From: VascoSch92 Date: Thu, 27 Aug 2026 15:41:34 +0200 Subject: [PATCH 1/2] test(agent-server): cover conversation reads not serializing behind an unrelated start MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #4570 replaced the process-wide `_lifecycle_lock` with per-conversation lifecycle locks, but the tests it added exercise `_conversation_lifecycle` directly. Nothing drives the public API, so the guarantee callers actually depend on — a conversation-scoped read completing while another conversation is mid-start — is unguarded. Add that end-to-end regression test: wedge one conversation inside `_start_event_service` and assert `get_event_service` for a different, already-live conversation still resolves. The test times out against 611629e69 (the commit before #4570) and passes on main. --- .../agent_server/test_conversation_service.py | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/tests/agent_server/test_conversation_service.py b/tests/agent_server/test_conversation_service.py index e867187cf8..2e206b2250 100644 --- a/tests/agent_server/test_conversation_service.py +++ b/tests/agent_server/test_conversation_service.py @@ -614,6 +614,58 @@ async def enter_lifecycle(conversation_id: UUID, entered: asyncio.Event): assert same_id_entered.is_set() +@pytest.mark.asyncio +async def test_conversation_read_completes_while_another_conversation_starts( + tmp_path, +): + """A conversation-scoped read must not queue behind an unrelated start. + + Lifecycle work once ran under a single process-wide lock, so any request + that resolved an ``EventService`` waited for a start, fork, delete or + eviction happening elsewhere in the process — for an unrelated + conversation. Drive the public API rather than the lock helper so the + guarantee is checked where callers actually hit it. + """ + conversations_dir = tmp_path / "conversations" + workspace_dir = tmp_path / "workspace" + workspace_dir.mkdir() + + def request() -> StartConversationRequest: + return StartConversationRequest( + agent=_sample_agent(), + workspace=LocalWorkspace(working_dir=str(workspace_dir)), + confirmation_policy=NeverConfirm(), + ) + + async with ConversationService(conversations_dir=conversations_dir) as service: + live, _ = await service.start_conversation(request()) + + start_entered = asyncio.Event() + release_start = asyncio.Event() + start_event_service = service._start_event_service + + async def blocking_start(stored: StoredConversation, **kwargs) -> EventService: + # Wedge the *other* conversation inside its own lifecycle section. + if stored.id != live.id: + start_entered.set() + await release_start.wait() + return await start_event_service(stored, **kwargs) + + with patch.object(service, "_start_event_service", side_effect=blocking_start): + starting = asyncio.create_task(service.start_conversation(request())) + await asyncio.wait_for(start_entered.wait(), timeout=5) + try: + assert ( + await asyncio.wait_for( + service.get_event_service(live.id), timeout=5 + ) + is not None + ) + finally: + release_start.set() + await starting + + @pytest.mark.asyncio async def test_prepare_for_sandbox_pause_blocks_new_hydration( persisted_conversation, From 9523b2e127e2847b9bb8ba985a9c263229c6b426 Mon Sep 17 00:00:00 2001 From: VascoSch92 Date: Thu, 27 Aug 2026 15:49:25 +0200 Subject: [PATCH 2/2] test(agent-server): bound the trailing await in the lifecycle regression test Addresses review feedback: after release_start.set() the wedged start should unblock immediately, but an unrelated hang would otherwise run the test forever. Bound it like the other two waits in the same test. --- tests/agent_server/test_conversation_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/agent_server/test_conversation_service.py b/tests/agent_server/test_conversation_service.py index 2e206b2250..95f027d714 100644 --- a/tests/agent_server/test_conversation_service.py +++ b/tests/agent_server/test_conversation_service.py @@ -663,7 +663,7 @@ async def blocking_start(stored: StoredConversation, **kwargs) -> EventService: ) finally: release_start.set() - await starting + await asyncio.wait_for(starting, timeout=5) @pytest.mark.asyncio