diff --git a/agent/chat_completion_helpers.py b/agent/chat_completion_helpers.py index 486aa2b1b9..13dc0dfbd1 100644 --- a/agent/chat_completion_helpers.py +++ b/agent/chat_completion_helpers.py @@ -38,8 +38,8 @@ from agent.model_metadata import is_local_endpoint from agent.message_content import flatten_message_text from agent.message_metadata import append_message, stamp_message_timestamp from agent.message_sanitization import ( - _sanitize_structure_non_ascii, _sanitize_structure_surrogates, _sanitize_surrogates, - _repair_tool_call_arguments, normalize_finish_reason as _normalize_finish_reason, + _sanitize_surrogates, _repair_tool_call_arguments, normalize_finish_reason as _normalize_finish_reason, + sanitize_outbound_kwargs, ) from agent.reasoning_summaries import append_streamed_reasoning_detail, separate_glued_reasoning_blocks from agent.stream_single_writer import claim_stream_writer, stream_writer_is_current @@ -2015,8 +2015,8 @@ def _managed_summary_call(agent, api_request_id: str, request, callback, *, retr def _summary_text(agent, response, **normalize_kwargs) -> str: normalized = agent._get_transport().normalize_response(response, **normalize_kwargs) if normalized.tool_calls: - # The summary request carries tools (cache lineage), but this path never executes a - # call; log so a tool-only response that lands in the empty-summary retry is diagnosable. + # No summary path executes tool calls; log so a tool-only response that falls into the + # empty-summary retry is diagnosable. logger.warning("Iteration summary emitted tool calls; discarding them") return (normalized.content or "").strip() @@ -2046,16 +2046,14 @@ def _anthropic_summary_attempt(agent, api_messages: list, api_request_id: str): def _chat_summary_attempt(agent, api_messages: list, api_request_id: str): - # Same builder as the main loop so the summary keeps the cached prefix (tools, prompt_cache_key, - # xAI alias, Moonshot sanitization). Do not omit tools or force tool_choice="none" here: - # SGLang renders the prompt with tools=None in that mode and the KV prefix diverges. + # Same kwargs builder as the main loop so the summary keeps the cached prefix (tools, + # prompt_cache_key, xAI alias, Moonshot sanitization). Do not omit tools or force + # tool_choice="none" here: SGLang renders the prompt with tools=None in that mode and the KV + # prefix diverges. (cache_control breakpoint decoration is not re-applied on this path.) summary_kwargs = agent._build_api_kwargs(api_messages) - # Same outbound chokepoint as turn_api_request: the summary now carries ``tools``, and on - # cache-planned routes the main loop scrubbed a deep copy, so ``agent.tools`` may still hold - # the lone surrogates / non-ASCII bytes the provider 400s on (#50959 class). - _sanitize_structure_surrogates(summary_kwargs) - if agent._force_ascii_payload: - _sanitize_structure_non_ascii(summary_kwargs) + # The summary now carries ``tools``; on cache-planned routes the main loop scrubbed a deep + # copy, so ``agent.tools`` may still hold bytes the provider 400s on. + sanitize_outbound_kwargs(agent, summary_kwargs) def _attempt(retry_count: int) -> str: summary_client = agent._ensure_primary_openai_client(reason="iteration_limit_summary_retry" if retry_count else "iteration_limit_summary") diff --git a/agent/message_sanitization.py b/agent/message_sanitization.py index b91c08a3eb..4575937a36 100644 --- a/agent/message_sanitization.py +++ b/agent/message_sanitization.py @@ -95,6 +95,19 @@ _sanitize_messages_non_ascii = partial(_sanitize_messages, fix=_strip_non_ascii, _sanitize_tools_non_ascii = _sanitize_structure_non_ascii +def sanitize_outbound_kwargs(agent: Any, api_kwargs: dict) -> None: + """Outbound-request chokepoint for every built kwargs dict (main loop and iteration summary). + + Tool descriptions, extra_body and kwargs strings can carry invalid code points that + providers reject with a non-retryable 400 (#50959); one in-place walk makes the whole + payload json.dumps()-safe. The ASCII strip is opt-in via the recovery flag set after an + ASCII-codec rejection. + """ + _sanitize_structure_surrogates(api_kwargs) + if agent._force_ascii_payload: + _sanitize_structure_non_ascii(api_kwargs) + + def _escape_invalid_chars_in_json_strings(raw: str) -> str: """Escape literal control chars (0x00-0x1F) inside JSON string values as ``\\uXXXX`` (for llama.cpp-style output mixing control chars with other malformations).""" @@ -315,7 +328,7 @@ __all__ = [ "_sanitize_surrogates", "_sanitize_structure_surrogates", "_sanitize_messages_surrogates", "_escape_invalid_chars_in_json_strings", "_repair_tool_call_arguments", "_strip_non_ascii", "_sanitize_messages_non_ascii", "_sanitize_tools_non_ascii", - "_strip_images_from_messages", "_sanitize_structure_non_ascii", + "_strip_images_from_messages", "_sanitize_structure_non_ascii", "sanitize_outbound_kwargs", # call_id policy owners "deterministic_call_id", "coalesce_tool_call_id", "tool_call_id_variants", "tool_result_id_variants", "uniquify_tool_call_ids", diff --git a/agent/turn_api_request.py b/agent/turn_api_request.py index a77aa6c6dc..f218023921 100644 --- a/agent/turn_api_request.py +++ b/agent/turn_api_request.py @@ -12,9 +12,7 @@ from dataclasses import dataclass import logging from typing import Any -from agent.message_sanitization import ( - _sanitize_structure_non_ascii, _sanitize_structure_surrogates -) +from agent.message_sanitization import sanitize_outbound_kwargs from utils import env_var_enabled logger = logging.getLogger("agent.conversation_loop") @@ -120,17 +118,9 @@ def build_api_request( api_kwargs = agent._build_api_kwargs(api_messages) else: api_kwargs = agent._build_api_kwargs(api_messages, tools_for_api=tools_for_api) - # Surrogate chokepoint: tool descriptions, extra_body and kwargs strings can carry - # invalid code points (HTTP 400). One walk makes the payload json.dumps()-safe. - # Outbound-request surrogate chokepoint (#50959): the messages were scrubbed above, but the rest of the - # request body — tool/function descriptions (session_search's ±-heavy text is the recorded repro), - # extra_body, system strings routed via kwargs — can still carry invalid code points that providers - # reject with a non-retryable HTTP 400 ("invalid unicode code point"). One in-place walk here guarantees - # the entire payload json.dumps()-safe regardless of which leaf produced the string. Fast no-op when the - # payload is clean. - _sanitize_structure_surrogates(api_kwargs) - if agent._force_ascii_payload: - _sanitize_structure_non_ascii(api_kwargs) + # Messages were scrubbed above; this walk covers the rest of the payload (tool descriptions, + # extra_body, kwargs strings) — see sanitize_outbound_kwargs for the #50959 rationale. + sanitize_outbound_kwargs(agent, api_kwargs) if agent.api_mode == "codex_responses": api_kwargs = agent._get_transport().preflight_kwargs( api_kwargs, allow_stream=False, is_github_responses=agent._is_copilot_url(), diff --git a/tests/agent/test_surrogate_chokepoints.py b/tests/agent/test_surrogate_chokepoints.py index 24b87822df..7be75859d6 100644 --- a/tests/agent/test_surrogate_chokepoints.py +++ b/tests/agent/test_surrogate_chokepoints.py @@ -183,7 +183,7 @@ def test_conversation_loop_sanitizes_api_kwargs_after_build(): src = inspect.getsource(rq.build_api_request) build_idx = src.index("api_kwargs = agent._build_api_kwargs(api_messages)") - sanitize_idx = src.index("_sanitize_structure_surrogates(api_kwargs)") + sanitize_idx = src.index("sanitize_outbound_kwargs(agent, api_kwargs)") assert build_idx < sanitize_idx loop_src = inspect.getsource(cl._run_api_retry_loop) assert loop_src.index("build_api_request,") < loop_src.index("perform_api_call,")