fix: detach aliased agent.tools at the outbound sanitization chokepoint
api_kwargs["tools"] is rebuilt from agent.tools on every attempt (_build_api_kwargs_for_mode: tools_for_api = agent.tools; transports set api_kwargs["tools"] = tools without copying), so the list usually aliases the canonical tool schemas. The ASCII retry path then runs sanitize_outbound_kwargs with _force_ascii_payload set, and its in-place strip rewrote agent.tools for the rest of the session. Move the guard to the chokepoint: when the flag is set and tools IS agent.tools, deepcopy before stripping. The deepcopy the contributor pick added inside _recover_unicode_encode_error only protected the failed request's kwargs, which are discarded before the retry; drop it and have recovery skip an aliased tools list entirely (request-local lists are still stripped for the diagnostic message). The kept test now exercises the chokepoint directly: with the flag set on kwargs whose tools aliases agent.tools, agent.tools must be byte-stable afterwards. Verified red against the previous chokepoint.
This commit is contained in:
@@ -7,6 +7,7 @@ characters that would crash ``json.dumps`` in the OpenAI SDK or be rejected upst
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import copy
|
||||
import hashlib
|
||||
import json
|
||||
import logging
|
||||
@@ -132,6 +133,10 @@ def sanitize_outbound_kwargs(agent: Any, api_kwargs: dict) -> None:
|
||||
"""
|
||||
_sanitize_structure_surrogates(api_kwargs)
|
||||
if agent._force_ascii_payload:
|
||||
# ``tools`` is built from ``agent.tools`` per attempt and usually aliases it; detach
|
||||
# before the in-place strip so the retry never rewrites the canonical tool schemas.
|
||||
if api_kwargs.get("tools") is not None and api_kwargs["tools"] is getattr(agent, "tools", None):
|
||||
api_kwargs["tools"] = copy.deepcopy(api_kwargs["tools"])
|
||||
_sanitize_structure_non_ascii(api_kwargs)
|
||||
|
||||
|
||||
|
||||
@@ -182,10 +182,15 @@ def _recover_unicode_encode_error(
|
||||
_messages_sanitized = isinstance(api_messages, list) and _sanitize_messages_non_ascii(api_messages)
|
||||
_tools_sanitized = False
|
||||
if isinstance(api_kwargs, dict):
|
||||
if api_kwargs.get("tools") is getattr(agent, "tools", None):
|
||||
api_kwargs["tools"] = copy.deepcopy(api_kwargs["tools"])
|
||||
# The retry rebuilds kwargs from ``agent.tools`` and ``sanitize_outbound_kwargs`` detaches
|
||||
# that alias before stripping; here only strip a request-local tools list, never the
|
||||
# canonical schemas.
|
||||
_tools = api_kwargs.pop("tools", None)
|
||||
_tools_sanitized = _sanitize_structure_non_ascii(api_kwargs)
|
||||
_tools_sanitized = _sanitize_tools_non_ascii(api_kwargs.get("tools")) or _tools_sanitized
|
||||
if _tools is not None:
|
||||
if _tools is not getattr(agent, "tools", None):
|
||||
_tools_sanitized = _sanitize_tools_non_ascii(_tools) or _tools_sanitized
|
||||
api_kwargs["tools"] = _tools
|
||||
|
||||
_system_sanitized = False
|
||||
if isinstance(active_system_prompt, str):
|
||||
|
||||
@@ -6,7 +6,7 @@ that can't encode non-ASCII characters in API request payloads.
|
||||
|
||||
import pytest
|
||||
|
||||
from agent.message_sanitization import _strip_non_ascii, _sanitize_messages_non_ascii, _sanitize_structure_non_ascii, _sanitize_tools_non_ascii, _sanitize_messages_surrogates
|
||||
from agent.message_sanitization import _strip_non_ascii, _sanitize_messages_non_ascii, _sanitize_structure_non_ascii, _sanitize_tools_non_ascii, _sanitize_messages_surrogates, sanitize_outbound_kwargs
|
||||
|
||||
|
||||
class TestStripNonAscii:
|
||||
@@ -358,9 +358,17 @@ class TestSanitizeMessagesPersistMarker:
|
||||
assert canonical[0][_DB_PERSISTED_MARKER] is True
|
||||
assert api_messages[0] is not canonical[0]
|
||||
api_messages[0]["content"].encode("ascii")
|
||||
assert api_kwargs["tools"] is not agent.tools
|
||||
api_kwargs["extra_body"]["note"].encode("ascii")
|
||||
api_kwargs["tools"][0]["function"]["description"].encode("ascii")
|
||||
# Recovery leaves the aliased canonical tools alone; the outbound chokepoint detaches
|
||||
# the alias on the retry before stripping, so agent.tools stays byte-stable.
|
||||
assert api_kwargs["tools"] is agent.tools
|
||||
assert agent._force_ascii_payload is True
|
||||
retry_kwargs = {"tools": agent.tools, "extra_body": {"note": "retry ☕"}}
|
||||
sanitize_outbound_kwargs(agent, retry_kwargs)
|
||||
assert retry_kwargs["tools"] is not agent.tools
|
||||
assert repr(tools) == tools_before
|
||||
retry_kwargs["tools"][0]["function"]["description"].encode("ascii")
|
||||
retry_kwargs["extra_body"]["note"].encode("ascii")
|
||||
|
||||
def test_ascii_word_in_error_does_not_strip_utf8_request_copy(self, monkeypatch):
|
||||
from agent.turn_recovery import _recover_unicode_encode_error
|
||||
|
||||
Reference in New Issue
Block a user