fix(websocket): avoid fresh retry for tool-output deltas#1042
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d45ca221cb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Addressed the automated review feedback in @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad09257de2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if not _websocket_input_items_are_self_contained_fresh_replay(input_items): | ||
| return False |
There was a problem hiding this comment.
Apply guard to injected-anchor fresh retries
This self-contained check only runs for client-supplied previous_response_id payloads. The Codex-session full-resend path still injects previous_response_id from session_anchor and then marks original_full_resend_payload retry-safe whenever the fresh text body exists; when that stored prefix is a tool-output delta, a later previous_response_not_found will still replay those function_call_output items without their matching calls and hit the same upstream rejection. Please apply the same guard before marking the injected-anchor fresh body safe.
Useful? React with 👍 / 👎.
Problem
Codex Desktop can send a
response.createrequest withprevious_response_idand aninputdelta that contains onlyfunction_call_outputitems. That payload is valid only while the upstream response history remains anchored byprevious_response_id.The proxy's fresh retry preparation could previously treat a multi-item client resend as safe to replay without that upstream anchor. For tool-output-only deltas, that creates a fresh request containing
function_call_outputitems without the matching priorfunction_callitems, which can be rejected upstream with errors like:No tool call found for function call output with call_id ...Changes
function_call_output,custom_tool_call_output, orapply_patch_call_outputmust have a matching earlier call item with the samecall_idin the replayed input.previous_response_id; this only disables unsafe fresh replay construction.Impact
This prevents the proxy from manufacturing an invalid fresh retry for Codex tool-output deltas. Normal text requests and full self-contained replay payloads continue to behave as before.
Tests
uv run pytest -q tests/unit/test_proxy_utils.py::test_prepare_websocket_response_create_request_does_not_fresh_retry_tool_output_delta tests/unit/test_proxy_utils.py::test_prepare_websocket_response_create_request_captures_client_full_resend_anchor_replay tests/unit/test_proxy_utils.py::test_websocket_client_previous_response_full_resend_retry_rejects_tool_output_delta tests/unit/test_proxy_utils.py::test_websocket_client_previous_response_full_resend_retry_rejects_output_before_call tests/unit/test_proxy_utils.py::test_websocket_client_previous_response_full_resend_retry_allows_self_contained_tool_history