From 318b23f991da12b5db88afc09a465527b3c1be98 Mon Sep 17 00:00:00 2001 From: Turgut Kural <58116817+TurgutKural@users.noreply.github.com> Date: Mon, 27 Jul 2026 12:50:09 +0300 Subject: [PATCH 1/3] fix(transport): strip empty/null tool_calls on assistant messages Strict OpenAI-compatible providers (onerouter / Qwen, DeepSeek v4) reject an assistant message carrying tool_calls: [] (or null) with HTTP 400 'Empty tool_calls is not supported in message.' The pre-API sanitizer in agent_runtime_helpers.sanitize_api_messages already drops these on the conversation_loop path, but auxiliary / custom-provider routes that bypass that sanitizer can still reach the wire with an invalid empty array and abort the whole session (non-retryable 400). Normalize at the transport layer too: detect an empty-list / null tool_calls on assistant messages, strip the key on the per-call copy (never mutate the stored history), and keep real tool_calls untouched. Includes unit tests covering empty-list, null, real-call preservation, mixed batches, user-role non-mutation, copy-on-write, and cross-provider parity. Follow-up to #58755. --- agent/transports/chat_completions.py | 46 +++++++ .../test_chat_completions_empty_tool_calls.py | 128 ++++++++++++++++++ 2 files changed, 174 insertions(+) create mode 100644 tests/agent/transports/test_chat_completions_empty_tool_calls.py diff --git a/agent/transports/chat_completions.py b/agent/transports/chat_completions.py index 275a66db3299e..694b074d38a7f 100644 --- a/agent/transports/chat_completions.py +++ b/agent/transports/chat_completions.py @@ -272,6 +272,22 @@ def convert_messages( break tool_calls = msg.get("tool_calls") if isinstance(tool_calls, list): + # Defense-in-depth: a strict OpenAI-compatible provider + # (e.g. onerouter / Qwen, DeepSeek v4) rejects an assistant + # message carrying ``tool_calls: []`` (empty array) with + # HTTP 400 "Empty tool_calls is not supported in message." + # The pre-API sanitizer in agent_runtime_helpers drops these, + # but only on the conversation_loop path — auxiliary / custom + # provider routes can reach the wire without it. Normalize here + # so any empty/invalid tool_calls array is stripped at the + # transport layer on every call. (#58755 follow-up) + if ( + msg.get("role") == "assistant" + and "tool_calls" in msg + and not tool_calls + ): + needs_sanitize = True + break for tc in tool_calls: if isinstance(tc, dict) and ( "call_id" in tc @@ -282,6 +298,15 @@ def convert_messages( break if needs_sanitize: break + elif ( + isinstance(tool_calls, type(None)) + and msg.get("role") == "assistant" + and "tool_calls" in msg + ): + # Explicit ``tool_calls: null`` is equally invalid on strict + # providers — treat it like the empty-array case. + needs_sanitize = True + break if not needs_sanitize: return messages @@ -328,6 +353,19 @@ def mutable_msg() -> dict[str, Any]: tool_calls = msg.get("tool_calls") if isinstance(tool_calls, list): + # Strip empty/invalid tool_calls arrays at the transport + # layer (see detection above). Strict OpenAI-compatible + # providers reject ``tool_calls: []`` with HTTP 400; dropping + # the key keeps the message schema-valid. Matches the + # pre-API sanitizer's behaviour so all routes agree. + if ( + msg.get("role") == "assistant" + and "tool_calls" in msg + and not tool_calls + ): + out_msg = mutable_msg() + out_msg.pop("tool_calls", None) + continue copied_tool_calls: list[Any] | None = None for tc_idx, tc in enumerate(tool_calls): if isinstance(tc, dict): @@ -347,6 +385,14 @@ def mutable_msg() -> dict[str, Any]: copied_tool_calls[tc_idx] = copied_tc if copied_tool_calls is not None: mutable_msg()["tool_calls"] = copied_tool_calls + elif ( + isinstance(tool_calls, type(None)) + and msg.get("role") == "assistant" + and "tool_calls" in msg + ): + # Explicit ``tool_calls: null`` is invalid on strict + # providers — drop the key entirely. + mutable_msg().pop("tool_calls", None) return sanitized def convert_tools(self, tools: list[dict[str, Any]]) -> list[dict[str, Any]]: diff --git a/tests/agent/transports/test_chat_completions_empty_tool_calls.py b/tests/agent/transports/test_chat_completions_empty_tool_calls.py new file mode 100644 index 0000000000000..2c8d59fecfa64 --- /dev/null +++ b/tests/agent/transports/test_chat_completions_empty_tool_calls.py @@ -0,0 +1,128 @@ +"""Tests for empty / null ``tool_calls`` stripping in ChatCompletionsTransport. + +Strict OpenAI-compatible providers (onerouter / Qwen, DeepSeek v4) reject an +assistant message carrying ``tool_calls: []`` (or ``null``) with HTTP 400 +"Empty tool_calls is not supported in message." The pre-API sanitizer in +``agent_runtime_helpers.sanitize_api_messages`` already drops these on the +conversation_loop path, but the transport layer must also normalize them so +auxiliary / custom-provider routes that bypass that sanitizer cannot reach the +wire with an invalid array. See #58755 (follow-up). +""" + +import pytest + +from agent.transports import get_transport + + +@pytest.fixture +def transport(): + import agent.transports.chat_completions # noqa: F401 + return get_transport("chat_completions") + + +class TestEmptyToolCallsStripping: + """Assistant messages with empty/invalid tool_calls must be normalized.""" + + def test_assistant_empty_list_dropped(self, transport): + msgs = [{"role": "assistant", "content": "ok", "tool_calls": []}] + out = transport.convert_messages(msgs) + assert "tool_calls" not in out[0] + assert out[0]["content"] == "ok" + + def test_assistant_null_dropped(self, transport): + msgs = [{"role": "assistant", "content": "ok", "tool_calls": None}] + out = transport.convert_messages(msgs) + assert "tool_calls" not in out[0] + + def test_assistant_real_calls_preserved(self, transport): + real_tc = [{ + "id": "call_abc", + "type": "function", + "function": {"name": "read_file", "arguments": "{}"}, + }] + msgs = [{"role": "assistant", "content": "c", "tool_calls": real_tc}] + out = transport.convert_messages(msgs) + assert out[0]["tool_calls"] == real_tc + + def test_only_empty_assistant_stripped_in_mixed_batch(self, transport): + msgs = [ + {"role": "user", "content": "hi"}, + {"role": "assistant", "content": "thinking", "tool_calls": []}, + { + "role": "assistant", + "content": "acting", + "tool_calls": [{ + "id": "call_x", + "type": "function", + "function": {"name": "f", "arguments": "{}"}, + }], + }, + ] + out = transport.convert_messages(msgs) + assert "tool_calls" not in out[1] + assert out[1]["content"] == "thinking" + assert out[2]["tool_calls"] and out[2]["tool_calls"][0]["id"] == "call_x" + + def test_user_role_empty_tool_calls_untouched(self, transport): + # User messages should not carry tool_calls at all, but if a stray + # empty array is present we must NOT strip it (it's not the invalid + # assistant shape, and mutating unrelated roles risks breaking the + # schema assumptions elsewhere). The transport only normalizes + # assistant messages. + msgs = [{"role": "user", "content": "hi", "tool_calls": []}] + out = transport.convert_messages(msgs) + assert "tool_calls" in out[0] + assert out[0]["tool_calls"] == [] + + def test_empty_array_with_codex_fields_still_dropped(self, transport): + # When an empty tool_calls array also carries codex scaffolding + # markers, the empty array takes precedence and is stripped outright. + msgs = [{ + "role": "assistant", + "content": "ok", + "tool_calls": [{ + "id": "fc_1", + "call_id": "call_1", + "response_item_id": "fc_1", + "type": "function", + "function": {"name": "f", "arguments": "{}"}, + }], + }] + out = transport.convert_messages(msgs, model="gpt-4o") + # Non-empty array: codex fields stripped, call preserved. + assert out[0]["tool_calls"] + tc = out[0]["tool_calls"][0] + assert "call_id" not in tc + assert "response_item_id" not in tc + + def test_clean_list_is_identity(self, transport): + msgs = [ + {"role": "user", "content": "hi"}, + { + "role": "assistant", + "content": "c", + "tool_calls": [{ + "id": "call_1", + "type": "function", + "function": {"name": "f", "arguments": "{}"}, + }], + }, + ] + assert transport.convert_messages(msgs) is msgs + + def test_empty_array_triggers_copy_on_write(self, transport): + msgs = [{"role": "assistant", "content": "ok", "tool_calls": []}] + out = transport.convert_messages(msgs) + # Original list/message must not be mutated in place. + assert msgs[0]["tool_calls"] == [] + assert "tool_calls" not in out[0] + assert out is not msgs + + @pytest.mark.parametrize( + "model", + ["qwen/qwen3.8-max-preview:free", "deepseek/deepseek-v4-flash", "gpt-4o"], + ) + def test_empty_array_stripped_across_providers(self, transport, model): + msgs = [{"role": "assistant", "content": "ok", "tool_calls": []}] + out = transport.convert_messages(msgs, model=model) + assert "tool_calls" not in out[0] From c4c109c72f400814b62c5696aa0b19c53933dbd3 Mon Sep 17 00:00:00 2001 From: Turgut Kural <58116817+TurgutKural@users.noreply.github.com> Date: Thu, 30 Jul 2026 18:55:18 +0300 Subject: [PATCH 2/3] =?UTF-8?q?fix(test):=20rename=20misleading=20test=20?= =?UTF-8?q?=E2=80=94=20non-empty=20array=20codex=20field=20stripping,=20no?= =?UTF-8?q?t=20empty=20array?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer noted the test name suggested an empty-array case but the fixture has one tool call; renamed to match actual behavior. --- .../transports/test_chat_completions_empty_tool_calls.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/agent/transports/test_chat_completions_empty_tool_calls.py b/tests/agent/transports/test_chat_completions_empty_tool_calls.py index 2c8d59fecfa64..4f547d0e6a715 100644 --- a/tests/agent/transports/test_chat_completions_empty_tool_calls.py +++ b/tests/agent/transports/test_chat_completions_empty_tool_calls.py @@ -74,9 +74,10 @@ def test_user_role_empty_tool_calls_untouched(self, transport): assert "tool_calls" in out[0] assert out[0]["tool_calls"] == [] - def test_empty_array_with_codex_fields_still_dropped(self, transport): - # When an empty tool_calls array also carries codex scaffolding - # markers, the empty array takes precedence and is stripped outright. + def test_nonempty_array_codex_fields_stripped(self, transport): + # A non-empty tool_calls array carrying codex scaffolding markers + # (call_id, response_item_id) must have those fields stripped while + # the call itself is preserved. msgs = [{ "role": "assistant", "content": "ok", From a2389e1d52c6be578642d0c4d8e2314fa633f29f Mon Sep 17 00:00:00 2001 From: Turgut Kural <58116817+TurgutKural@users.noreply.github.com> Date: Thu, 13 Aug 2026 06:55:56 +0300 Subject: [PATCH 3/3] fix(transport): scope empty tool_calls comment to transport-layer coverage --- agent/transports/chat_completions.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/agent/transports/chat_completions.py b/agent/transports/chat_completions.py index 694b074d38a7f..88a5d2ea178ce 100644 --- a/agent/transports/chat_completions.py +++ b/agent/transports/chat_completions.py @@ -277,10 +277,13 @@ def convert_messages( # message carrying ``tool_calls: []`` (empty array) with # HTTP 400 "Empty tool_calls is not supported in message." # The pre-API sanitizer in agent_runtime_helpers drops these, - # but only on the conversation_loop path — auxiliary / custom - # provider routes can reach the wire without it. Normalize here - # so any empty/invalid tool_calls array is stripped at the - # transport layer on every call. (#58755 follow-up) + # but only on the conversation_loop path — other routes can + # reach the wire without it. For every request that + # serializes through this transport (conversation loop and + # any caller using it), this is the last boundary, so + # normalize here. Requests built by fully separate payload + # paths (e.g. some auxiliary clients) never pass through + # this layer and are out of scope for it. (#58755 follow-up) if ( msg.get("role") == "assistant" and "tool_calls" in msg