From 3542e5526f3167d66ab009ae9d92f8dd4eacafc5 Mon Sep 17 00:00:00 2001 From: Chenglun Hu Date: Mon, 25 May 2026 09:25:44 +0800 Subject: [PATCH 1/2] fix(agent): coerce tool-result content to string for OpenAI wire format Plugin tool handlers returning Dict[str, Any] were persisted into the chat history with a raw dict as the role:"tool" message content field. The OpenAI Chat Completions spec requires tool message content to be a string; strict upstreams reject the dict with HTTP 400 (Z.ai 1210, Manifest fallback_exhausted), while permissive providers silently coerce and mask the bug. make_tool_result_message now normalizes content via a single defensive helper: strings and multimodal content-part envelopes pass through unchanged; any other value is JSON-encoded with the json.dumps(default=str) idiom already used in this module. Fixes #31435 --- agent/tool_dispatch_helpers.py | 43 ++++++++++++--- tests/test_tool_result_content_coercion.py | 64 ++++++++++++++++++++++ 2 files changed, 100 insertions(+), 7 deletions(-) create mode 100644 tests/test_tool_result_content_coercion.py diff --git a/agent/tool_dispatch_helpers.py b/agent/tool_dispatch_helpers.py index a0f3bfc2683b2..760b7d04f9097 100644 --- a/agent/tool_dispatch_helpers.py +++ b/agent/tool_dispatch_helpers.py @@ -317,23 +317,51 @@ def _trajectory_normalize_msg(msg: Dict[str, Any]) -> Dict[str, Any]: return msg +def _coerce_tool_result_content(content: Any) -> Any: + """Normalize tool-result ``content`` to a wire-valid shape. + + The OpenAI Chat Completions spec requires ``role: "tool"`` message + ``content`` to be a string (or, for multimodal-capable providers, a + content-part list). Plugin tool handlers that return a ``Dict[str, Any]`` + otherwise get persisted as a raw dict, which strict upstreams reject with + HTTP 400 (e.g. Z.ai 1210, Manifest fallback_exhausted). Strings and + multimodal results pass through unchanged; any other non-string value is + JSON-encoded with the same idiom already used elsewhere in this module.""" + if isinstance(content, str): + return content + if _is_multimodal_tool_result(content): + return content + try: + return json.dumps(content, default=str) + except Exception: + return str(content) + + def make_tool_result_message(name: str, content: Any, tool_call_id: str) -> dict: """Build a tool-result message dict with both the OpenAI-format ``name`` field (required by the wire format and provider adapters) and the internal ``tool_name`` field (written to the session DB messages table). - Content from high-risk tools (``web_extract``, ``web_search``, ``browser_*``, - ``mcp_*``) gets wrapped in semantic delimiters telling the model the content - is untrusted data, not instructions. This is the architectural defense - against indirect prompt injection from poisoned web pages, GitHub issues, - and MCP responses — it changes how the model interprets the content rather - than relying on regex pattern matching catching every payload. + ``content`` is first normalized to a wire-valid shape (string, or a + multimodal content-part list) so plugin tools returning a dict cannot + poison the next request with a non-string ``content`` field (#31435). + + The normalized content from high-risk tools (``web_extract``, ``web_search``, + ``browser_*``, ``mcp_*``) is then wrapped in semantic delimiters telling the + model the content is untrusted data, not instructions. This is the + architectural defense against indirect prompt injection from poisoned web + pages, GitHub issues, and MCP responses — it changes how the model + interprets the content rather than relying on regex pattern matching + catching every payload. Wrapping only happens for plain string content. Multimodal results (content lists with image_url parts) pass through unwrapped so the list structure stays valid for vision-capable adapters. """ - wrapped = _maybe_wrap_untrusted(name, content) + # Coerce non-string content (dict/None/etc.) to a wire-valid shape first, + # then apply untrusted-content wrapping on the resulting string so both the + # #31435 stringify fix and the promptware defense compose correctly. + wrapped = _maybe_wrap_untrusted(name, _coerce_tool_result_content(content)) return { "role": "tool", "name": name, @@ -413,5 +441,6 @@ def _maybe_wrap_untrusted(name: str, content: Any) -> Any: "_extract_file_mutation_targets", "_extract_error_preview", "_trajectory_normalize_msg", + "_coerce_tool_result_content", "make_tool_result_message", ] diff --git a/tests/test_tool_result_content_coercion.py b/tests/test_tool_result_content_coercion.py new file mode 100644 index 0000000000000..8a9a60972b65e --- /dev/null +++ b/tests/test_tool_result_content_coercion.py @@ -0,0 +1,64 @@ +"""Regression tests for tool-result ``content`` coercion (issue #31435). + +OpenAI Chat Completions requires ``role: "tool"`` message ``content`` to be a +string. Plugin tool handlers that return ``Dict[str, Any]`` previously got +persisted as a raw dict, which strict upstreams reject with HTTP 400 +(Z.ai 1210, Manifest fallback_exhausted). ``make_tool_result_message`` now +normalizes via ``_coerce_tool_result_content``: strings and multimodal +results pass through; any other value is JSON-encoded. +""" +import json + +from agent.tool_dispatch_helpers import ( + _coerce_tool_result_content, + make_tool_result_message, +) + + +def test_dict_content_is_json_stringified(): + """The reported failure mode: a plugin returning a dict must not reach + the wire as a dict.""" + content = {"definitions": [{"name": "wf"}], "count": 1} + msg = make_tool_result_message("list_workflows", content, "call_1") + assert isinstance(msg["content"], str) + assert json.loads(msg["content"]) == content + + +def test_string_content_passes_through_unchanged(): + msg = make_tool_result_message("terminal", "$ ls\nfile.txt", "call_2") + assert msg["content"] == "$ ls\nfile.txt" + + +def test_multimodal_content_is_preserved(): + """Multimodal envelopes must NOT be flattened — providers that support + multipart tool messages consume the list directly.""" + multimodal = { + "_multimodal": True, + "content": [{"type": "text", "text": "ok"}], + "text_summary": "ok", + } + msg = make_tool_result_message("computer_use", multimodal, "call_3") + assert msg["content"] is multimodal + + +def test_non_serializable_falls_back_to_str(): + class Weird: + def __repr__(self): + return "" + + out = _coerce_tool_result_content(Weird()) + assert isinstance(out, str) + + +def test_list_content_is_json_stringified(): + """A bare list (non-multimodal) is still not a valid string content.""" + out = _coerce_tool_result_content([1, 2, 3]) + assert out == "[1, 2, 3]" + + +def test_make_tool_result_message_preserves_other_fields(): + msg = make_tool_result_message("toolx", {"ok": True}, "call_4") + assert msg["role"] == "tool" + assert msg["name"] == "toolx" + assert msg["tool_name"] == "toolx" + assert msg["tool_call_id"] == "call_4" From cb837cfe98e717f099ff0e45ab4da3a6b2cf8033 Mon Sep 17 00:00:00 2001 From: Chenglun Hu Date: Mon, 25 May 2026 15:32:41 +0800 Subject: [PATCH 2/2] fix(agent): map None tool-result content to empty string Per review feedback on #31770: a handler returning None (silent success) hit the json.dumps path and produced the literal string "null", which strict providers reject the same way as a dict. Map None -> "" before the other checks, with a regression test. --- agent/tool_dispatch_helpers.py | 19 ++++++++++++++++++- tests/test_tool_result_content_coercion.py | 20 +++++++++++++++++++- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/agent/tool_dispatch_helpers.py b/agent/tool_dispatch_helpers.py index 760b7d04f9097..acd618ac18c3f 100644 --- a/agent/tool_dispatch_helpers.py +++ b/agent/tool_dispatch_helpers.py @@ -326,11 +326,28 @@ def _coerce_tool_result_content(content: Any) -> Any: otherwise get persisted as a raw dict, which strict upstreams reject with HTTP 400 (e.g. Z.ai 1210, Manifest fallback_exhausted). Strings and multimodal results pass through unchanged; any other non-string value is - JSON-encoded with the same idiom already used elsewhere in this module.""" + JSON-encoded with the same idiom already used elsewhere in this module. + + ``None`` (a handler returning no output / silent success) maps to an empty + string rather than the literal ``"null"`` — strict providers reject a null + content field the same way they reject a dict, and ``"null"`` would be a + misleading tool result.""" + if content is None: + return "" if isinstance(content, str): return content if _is_multimodal_tool_result(content): return content + # An OpenAI-style content-part list (e.g. ``[{"type": "text", ...}, + # {"type": "image_url", ...}]``) is already a wire-valid shape for + # multimodal-capable providers — pass it through unchanged rather than + # JSON-encoding it into a string. A list that is NOT a content-part array + # (e.g. a plugin returning ``[1, 2, 3]``) is not wire-valid and still gets + # stringified for #31435. + if isinstance(content, list) and content and all( + isinstance(p, dict) and "type" in p for p in content + ): + return content try: return json.dumps(content, default=str) except Exception: diff --git a/tests/test_tool_result_content_coercion.py b/tests/test_tool_result_content_coercion.py index 8a9a60972b65e..d51a75f9c149b 100644 --- a/tests/test_tool_result_content_coercion.py +++ b/tests/test_tool_result_content_coercion.py @@ -29,6 +29,14 @@ def test_string_content_passes_through_unchanged(): assert msg["content"] == "$ ls\nfile.txt" +def test_none_content_becomes_empty_string(): + """A handler returning None (silent success) must become "" — not the + literal "null" — so strict providers don't reject a null content field.""" + assert _coerce_tool_result_content(None) == "" + msg = make_tool_result_message("noop_tool", None, "call_none") + assert msg["content"] == "" + + def test_multimodal_content_is_preserved(): """Multimodal envelopes must NOT be flattened — providers that support multipart tool messages consume the list directly.""" @@ -51,11 +59,21 @@ def __repr__(self): def test_list_content_is_json_stringified(): - """A bare list (non-multimodal) is still not a valid string content.""" + """A non-content-part list (e.g. ``[1, 2, 3]``) is not wire-valid and + gets JSON-stringified.""" out = _coerce_tool_result_content([1, 2, 3]) assert out == "[1, 2, 3]" +def test_content_part_list_passes_through(): + """An OpenAI-style content-part list (multimodal content array) is already + wire-valid for multimodal providers and passes through unchanged so the + list structure stays intact for vision adapters.""" + parts = [{"type": "text", "text": "page contents"}] + out = _coerce_tool_result_content(parts) + assert out is parts + + def test_make_tool_result_message_preserves_other_fields(): msg = make_tool_result_message("toolx", {"ok": True}, "call_4") assert msg["role"] == "tool"