mirror of
https://github.com/openai/codex.git
synced 2026-10-26 18:38:47 +00:00
Start Python SDK turn subscriptions at their attachment point (#44400)
## What changed Make turn event delivery depend on when each consumer attaches: - Handles returned by `thread.turn(...)` receive events from when the request is sent, including events received before the response arrives. - Other handles and explicit low-level registrations receive only events from when they join. Remove replay of completed items, token usage, and terminal events; use `thread.read(include_turns=True)` for earlier history. - Retain unread events for active subscribers and pending starts, release unused turn state, and close the returned subscription when an async start is cancelled. Preserve the existing low-level subscription when repeated `turn_start(...)` calls return the same turn. ## Testing Update coverage for late joins, request-time buffering, completion or transport failure before a start returns, async cancellation cleanup, and repeated low-level starts. Adjust external-message integration tests to allow handles to collect different items while receiving the same final response. GitOrigin-RevId: 3b7f1268df1193dee88385006ae9510101086c24
This commit is contained in:
@@ -82,7 +82,10 @@ def test_external_message_joins_active_turn_with_tool_authority(tmp_path) -> Non
|
||||
harness.responses.wait_for_requests(1)
|
||||
joined = thread.turn(ExternalMessage(tool_name="notifications", content=content))
|
||||
result = consumers.submit(joined.run).result(timeout=15)
|
||||
assert original_result.result(timeout=15) == result
|
||||
first = original_result.result(timeout=15)
|
||||
assert first.final_response == result.final_response
|
||||
assert first.items[0].root.type == "userMessage"
|
||||
assert all(item.root.type != "userMessage" for item in result.items)
|
||||
assert codex._client._router._turn_states == {}
|
||||
requests = harness.responses.requests()
|
||||
|
||||
@@ -151,7 +154,7 @@ def test_async_external_message_allows_both_handles_to_consume(tmp_path) -> None
|
||||
first, second = await asyncio.wait_for(
|
||||
asyncio.gather(original_result, joined.run()), timeout=15
|
||||
)
|
||||
assert first == second
|
||||
assert (first.final_response, first.usage) == (second.final_response, second.usage)
|
||||
assert first.final_response == "Update processed"
|
||||
assert first.usage is not None
|
||||
assert codex._client._sync._router._turn_states == {}
|
||||
|
||||
@@ -617,9 +617,10 @@ def test_client_reader_routes_interleaved_turn_notifications_by_turn_id() -> Non
|
||||
)
|
||||
|
||||
|
||||
def test_turn_notification_router_buffers_events_before_registration() -> None:
|
||||
"""Early turn events should be replayed once their TurnHandle registers."""
|
||||
def test_turn_notification_router_starts_at_explicit_registration() -> None:
|
||||
"""Explicit registration receives events from when the caller attaches."""
|
||||
client = CodexClient()
|
||||
client.register_turn_notifications("turn-1")
|
||||
client._router.route_notification(
|
||||
client._coerce_notification(
|
||||
"item/agentMessage/delta",
|
||||
@@ -632,7 +633,6 @@ def test_turn_notification_router_buffers_events_before_registration() -> None:
|
||||
)
|
||||
)
|
||||
|
||||
client.register_turn_notifications("turn-1")
|
||||
event = client.next_turn_notification("turn-1")
|
||||
|
||||
assert isinstance(event.payload, AgentMessageDeltaNotification)
|
||||
|
||||
@@ -191,10 +191,10 @@ def test_turn_inputs_and_options_reach_the_client(api_type, method, content) ->
|
||||
}
|
||||
),
|
||||
)
|
||||
subscription = SimpleNamespace(next=Mock(return_value=completed), close=Mock())
|
||||
client = SimpleNamespace(
|
||||
turn_start=rpc(return_value=SimpleNamespace(turn=SimpleNamespace(id="turn-1"))),
|
||||
_subscribe_turn_notifications=Mock(
|
||||
return_value=SimpleNamespace(next=Mock(return_value=completed), close=Mock())
|
||||
_start_turn=rpc(
|
||||
return_value=(SimpleNamespace(turn=SimpleNamespace(id="turn-1")), subscription)
|
||||
),
|
||||
)
|
||||
codex = api_type.__new__(api_type)
|
||||
@@ -229,7 +229,7 @@ def test_turn_inputs_and_options_reach_the_client(api_type, method, content) ->
|
||||
"toolOutput": {"name": "notifications", "namespace": "slack", "output": content},
|
||||
}
|
||||
)
|
||||
assert _params_dict(client.turn_start.call_args.kwargs["params"]) == {
|
||||
assert _params_dict(client._start_turn.call_args.kwargs["params"]) == {
|
||||
"threadId": "thread-1",
|
||||
"input": expected_input,
|
||||
"serviceTier": "priority",
|
||||
@@ -249,7 +249,7 @@ def test_external_messages_cannot_be_mixed_with_user_input_or_sent_as_user_steer
|
||||
async_api = api_type is AsyncCodex
|
||||
rpc = AsyncMock if async_api else Mock
|
||||
client = SimpleNamespace(
|
||||
turn_start=rpc(), turn_steer=rpc(), _subscribe_turn_notifications=Mock()
|
||||
_start_turn=rpc(), turn_steer=rpc(), _subscribe_turn_notifications=Mock()
|
||||
)
|
||||
codex = api_type.__new__(api_type)
|
||||
codex._client = client
|
||||
@@ -277,7 +277,7 @@ def test_external_messages_cannot_be_mixed_with_user_input_or_sent_as_user_steer
|
||||
result = thread.turn(ExternalMessage(tool_name=" ", content="Untrusted update"))
|
||||
if async_api:
|
||||
await result
|
||||
client.turn_start.assert_not_called()
|
||||
client._start_turn.assert_not_called()
|
||||
client.turn_steer.assert_not_called()
|
||||
|
||||
asyncio.run(scenario())
|
||||
|
||||
@@ -8,7 +8,7 @@ import pytest
|
||||
|
||||
from openai_codex import AsyncCodex
|
||||
from openai_codex._run import _collect_turn_result
|
||||
from openai_codex.api import AsyncTurnHandle, TurnHandle
|
||||
from openai_codex.api import AsyncThread, AsyncTurnHandle, Thread, TurnHandle
|
||||
from openai_codex.async_client import AsyncCodexClient
|
||||
from openai_codex.client import CodexClient
|
||||
from openai_codex.errors import TransportClosedError
|
||||
@@ -55,23 +55,24 @@ def turn_events(client, *, status="completed"):
|
||||
]
|
||||
|
||||
|
||||
def test_late_join_replays_items_already_consumed_by_original_handle():
|
||||
@pytest.mark.parametrize("consumed", [False, True])
|
||||
def test_late_join_starts_with_future_events(consumed):
|
||||
client = CodexClient()
|
||||
original = TurnHandle(client, "thread-1", "turn-1")
|
||||
events = turn_events(client)
|
||||
client._router.route_notification(events[0])
|
||||
stream = original.stream()
|
||||
first = next(stream)
|
||||
client._router.route_notification(events[1])
|
||||
usage = next(stream)
|
||||
previous = [next(stream)] if consumed else []
|
||||
joined = TurnHandle(client, "thread-1", "turn-1")
|
||||
client._router.route_notification(events[1])
|
||||
for event in events[2:]:
|
||||
client._router.route_notification(event)
|
||||
|
||||
first_result = _collect_turn_result(chain([first, usage], stream), turn_id="turn-1")
|
||||
assert joined.run() == first_result
|
||||
first_result = _collect_turn_result(chain(previous, stream), turn_id="turn-1")
|
||||
joined_result = joined.run()
|
||||
assert first_result.final_response == "done"
|
||||
assert first_result.usage.last.total_tokens == 5
|
||||
assert joined_result.final_response is None
|
||||
assert joined_result.usage == first_result.usage
|
||||
assert client._router._turn_states == {}
|
||||
|
||||
|
||||
@@ -92,7 +93,6 @@ def test_consumed_deltas_are_released_while_turn_is_active():
|
||||
client._router.route_notification(event)
|
||||
assert subscription.next() == event
|
||||
assert state.events == {}
|
||||
assert state.completed_items == {}
|
||||
assert not state.completed
|
||||
subscription.close()
|
||||
|
||||
@@ -113,32 +113,69 @@ def test_slow_subscriber_keeps_unread_deltas_until_it_consumes_them():
|
||||
slow.close()
|
||||
|
||||
|
||||
def test_completion_before_turn_start_response_is_replayed(monkeypatch):
|
||||
client = CodexClient()
|
||||
@pytest.mark.parametrize("async_api", [False, True])
|
||||
@pytest.mark.parametrize("low_level", [False, True])
|
||||
@pytest.mark.parametrize("completed", [False, True])
|
||||
def test_events_or_failure_before_turn_start_returns(monkeypatch, async_api, low_level, completed):
|
||||
codex = AsyncCodex()
|
||||
codex._initialized = True
|
||||
client = codex._client._sync if async_api else CodexClient()
|
||||
|
||||
def request_raw(method, params):
|
||||
assert method == "turn/start"
|
||||
for event in turn_events(client):
|
||||
for event in turn_events(client) if completed else []:
|
||||
client._router.route_notification(event)
|
||||
client._router.fail_all(TransportClosedError("transport failed"))
|
||||
return {"turn": {"id": "turn-1", "status": "inProgress", "items": []}}
|
||||
|
||||
monkeypatch.setattr(client, "_request_raw", request_raw)
|
||||
started = client.turn_start("thread-1", "hello")
|
||||
handle = TurnHandle(client, "thread-1", started.turn.id)
|
||||
assert handle.run().final_response == "done"
|
||||
public = codex._client if async_api else client
|
||||
thread = AsyncThread(codex, "thread-1") if async_api else Thread(client, "thread-1")
|
||||
|
||||
async def value(call):
|
||||
return await call if async_api else call
|
||||
|
||||
async def scenario():
|
||||
if low_level:
|
||||
started = await value(public.turn_start("thread-1", "hello"))
|
||||
assert (
|
||||
await value(public.wait_for_turn_completed(started.turn.id))
|
||||
).turn.id == "turn-1"
|
||||
else:
|
||||
handle = await value(thread.turn("hello"))
|
||||
assert (await value(handle.run())).final_response == "done"
|
||||
|
||||
if completed:
|
||||
asyncio.run(scenario())
|
||||
else:
|
||||
with pytest.raises(TransportClosedError, match="transport failed"):
|
||||
asyncio.run(scenario())
|
||||
assert client._router._turn_states == {}
|
||||
|
||||
|
||||
def test_pending_join_preserves_result_after_original_handle_finishes():
|
||||
def test_pending_join_starts_at_request_while_original_handle_finishes():
|
||||
client = CodexClient()
|
||||
original = TurnHandle(client, "thread-1", "turn-1")
|
||||
with client._router.pending_turn("thread-1"):
|
||||
for event in turn_events(client):
|
||||
client._router.route_notification(event)
|
||||
events = turn_events(client)
|
||||
unknown = [client._coerce_notification(name, {"turnId": "turn-1"}) for name in ("old", "live")]
|
||||
client._router.route_notification(unknown[0])
|
||||
assert original._subscription.next() is unknown[0]
|
||||
|
||||
with client._router.pending_turn("thread-1") as cursors:
|
||||
client._router.route_notification(unknown[1])
|
||||
assert original._subscription.next() is unknown[1]
|
||||
client._router.route_notification(events[0])
|
||||
client._router.route_notification(events[1])
|
||||
manual = TurnHandle(client, "thread-1", "turn-1")
|
||||
client._router.route_notification(events[2])
|
||||
result = original.run()
|
||||
client._router.prepare_turn("turn-1", "thread-1")
|
||||
joined = TurnHandle(client, "thread-1", "turn-1")
|
||||
assert joined.run() == result
|
||||
assert manual.run().usage is None
|
||||
subscription = client._router.prepare_turn("turn-1", "thread-1", cursors, for_handle=True)
|
||||
joined_handle = TurnHandle(client, "thread-1", "turn-1", _subscription=subscription)
|
||||
assert joined_handle._subscription.next() is unknown[1]
|
||||
joined = joined_handle.run()
|
||||
assert result.final_response == joined.final_response == "done"
|
||||
assert joined.usage.last.total_tokens == 5
|
||||
assert client._router._turn_states == {}
|
||||
|
||||
|
||||
@@ -220,7 +257,7 @@ def test_cancelled_turn_start_releases_result_after_response(monkeypatch):
|
||||
entered = threading.Event()
|
||||
respond = threading.Event()
|
||||
released = threading.Event()
|
||||
subscribe = client._sync._subscribe_turn_notifications
|
||||
subscribe = client._sync._router.prepare_turn
|
||||
|
||||
def request_raw(method, params):
|
||||
entered.set()
|
||||
@@ -229,8 +266,8 @@ def test_cancelled_turn_start_releases_result_after_response(monkeypatch):
|
||||
client._sync._router.route_notification(event)
|
||||
return {"turn": {"id": "turn-1", "items": [], "status": "completed"}}
|
||||
|
||||
def observe_release(turn_id):
|
||||
subscription = subscribe(turn_id)
|
||||
def observe_release(*args, **kwargs):
|
||||
subscription = subscribe(*args, **kwargs)
|
||||
close = subscription.close
|
||||
|
||||
def close_and_signal():
|
||||
@@ -241,7 +278,7 @@ def test_cancelled_turn_start_releases_result_after_response(monkeypatch):
|
||||
return subscription
|
||||
|
||||
monkeypatch.setattr(client._sync, "_request_raw", request_raw)
|
||||
monkeypatch.setattr(client._sync, "_subscribe_turn_notifications", observe_release)
|
||||
monkeypatch.setattr(client._sync._router, "prepare_turn", observe_release)
|
||||
task = asyncio.create_task(client.turn_start("thread-1", "hello"))
|
||||
try:
|
||||
assert await asyncio.to_thread(entered.wait, 2)
|
||||
@@ -270,7 +307,7 @@ def test_cancelled_queued_start_does_not_send_a_request(monkeypatch):
|
||||
occupied = executor.submit(occupy_worker)
|
||||
assert worker_started.wait(timeout=5)
|
||||
client = AsyncCodexClient()
|
||||
monkeypatch.setattr(client._sync, "turn_start", lambda *args: request_sent.set())
|
||||
monkeypatch.setattr(client._sync, "_start_turn", lambda *args, **kwargs: request_sent.set())
|
||||
|
||||
async def scenario():
|
||||
task = asyncio.create_task(client.turn_start("thread-1", "hello"))
|
||||
@@ -298,6 +335,9 @@ def test_low_level_start_keeps_implicit_registration_and_explicit_unregister(mon
|
||||
|
||||
monkeypatch.setattr(client, "_request_raw", request_raw)
|
||||
started = client.turn_start("thread-1", "hello")
|
||||
registered = client._router._turn_notifications[started.turn.id]
|
||||
assert client.turn_start("thread-1", "again").turn.id == started.turn.id
|
||||
assert client._router._turn_notifications[started.turn.id] is registered
|
||||
assert client.next_turn_notification(started.turn.id) == turn_events(client)[0]
|
||||
client.unregister_turn_notifications(started.turn.id)
|
||||
with pytest.raises(RuntimeError, match="not registered"):
|
||||
|
||||
Reference in New Issue
Block a user