Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions agent/transports/chat_completions.py
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,25 @@ 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 — 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
and not tool_calls
):
needs_sanitize = True
break
for tc in tool_calls:
if isinstance(tc, dict) and (
"call_id" in tc
Expand All @@ -282,6 +301,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
Expand Down Expand Up @@ -328,6 +356,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):
Expand All @@ -347,6 +388,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]]:
Expand Down
129 changes: 129 additions & 0 deletions tests/agent/transports/test_chat_completions_empty_tool_calls.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
"""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_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",
"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]
Loading