From d5fc146123c4ce7eb197a080aa7c34c5f9f21dd0 Mon Sep 17 00:00:00 2001 From: ciregenz Date: Sun, 7 Jun 2026 23:04:59 -0700 Subject: [PATCH] [eric] browser: never leave a tool_use unanswered (backfill mid-turn + validate both directions) so a 30s upstream reset stops 400-killing the run --- backend/apps/agents/browser/browser_agent.py | 15 +++++++++++++ .../apps/agents/browser/browser_history.py | 22 ++++++++++++------- backend/tests/test_browser_agent_loop.py | 16 ++++++++++++++ 3 files changed, 45 insertions(+), 8 deletions(-) diff --git a/backend/apps/agents/browser/browser_agent.py b/backend/apps/agents/browser/browser_agent.py index 656a1026..1cce8742 100644 --- a/backend/apps/agents/browser/browser_agent.py +++ b/backend/apps/agents/browser/browser_agent.py @@ -1794,6 +1794,21 @@ async def run_browser_agent( "message": result_msg.model_dump(mode="json"), }) + # Integrity backfill: every tool_use in the assistant turn MUST have a + # matching tool_result or the next API call 400s ("tool_use without + # tool_result"). A break mid-loop (cancel, or a turn that ran past the + # 30s upstream reset) can leave some unanswered, which silently corrupts + # the history AND the resume snapshot. Stub any missing one so the array + # is always well-formed, no matter which path fired. + _answered = {tr.get("tool_use_id") for tr in tool_results} + for tu in tool_uses_sorted: + if tu.id not in _answered: + tool_results.append({ + "type": "tool_result", "tool_use_id": tu.id, + "content": [{"type": "text", "text": + "(not run, the turn ended before this tool executed)"}], + "is_error": True, + }) messages.append({"role": "user", "content": tool_results}) if cancelled: diff --git a/backend/apps/agents/browser/browser_history.py b/backend/apps/agents/browser/browser_history.py index 5f58c497..f9a9284e 100644 --- a/backend/apps/agents/browser/browser_history.py +++ b/backend/apps/agents/browser/browser_history.py @@ -176,15 +176,18 @@ def place_cache_marker(messages: list[dict], depth: int = 8) -> None: def _validate_message_pairing(messages: list[dict]) -> bool: - """Verify every tool_result references a tool_use_id from a prior assistant - message in the same list. Returns False if there's an orphan, which means - the cached history would 400 if sent to the API. + """Verify tool_use and tool_result blocks pair up BOTH ways, or the cached + history 400s if sent to the API. Two failure shapes, both checked: + - an orphan tool_result (references a tool_use_id that was never declared), and + - a dangling tool_use (an assistant tool call with no answering tool_result), + which is the exact '`tool_use` ids found without `tool_result` blocks' 400 a + turn that broke early or ran past the upstream 30s reset leaves behind. - This is the last line of defense against cache corruption; if it ever - returns False on a resume, we drop the cache and start fresh rather than - crash on the next API call. + This is the last line of defense against cache corruption; if it returns False + on a resume, we drop the cache and start fresh rather than crash on the next call. """ declared_tool_use_ids: set[str] = set() + answered_tool_use_ids: set[str] = set() for msg in messages: role = msg.get("role") content = msg.get("content") @@ -199,8 +202,11 @@ def _validate_message_pairing(messages: list[dict]) -> bool: if isinstance(block, dict) and block.get("type") == "tool_result": tr_id = block.get("tool_use_id") if tr_id and tr_id not in declared_tool_use_ids: - return False - return True + return False # orphan tool_result + if tr_id: + answered_tool_use_ids.add(tr_id) + # every declared tool_use must have been answered (no dangling call) + return declared_tool_use_ids.issubset(answered_tool_use_ids) def _is_fresh_user_message(msg: dict) -> bool: diff --git a/backend/tests/test_browser_agent_loop.py b/backend/tests/test_browser_agent_loop.py index 87102e6e..907c08e4 100644 --- a/backend/tests/test_browser_agent_loop.py +++ b/backend/tests/test_browser_agent_loop.py @@ -1411,6 +1411,22 @@ def test_recoverable_tool_error_classifier(): assert not recoverable_tool_error("some unrelated failure") +def test_message_pairing_validator_catches_both_orphan_and_dangling(): + from backend.apps.agents.browser.browser_history import _validate_message_pairing + au = lambda i: {"role": "assistant", "content": [{"type": "tool_use", "id": i, "name": "X", "input": {}}]} + tr = lambda i: {"role": "user", "content": [{"type": "tool_result", "tool_use_id": i, "content": []}]} + # well-formed: every tool_use answered + assert _validate_message_pairing([au("t1"), tr("t1")]) is True + # DANGLING tool_use (the exact 400: a call with no result) -> invalid + assert _validate_message_pairing([au("t1")]) is False + assert _validate_message_pairing([au("t1"), tr("t1"), au("t2")]) is False + # ORPHAN tool_result (result for a never-declared id) -> invalid + assert _validate_message_pairing([tr("ghost")]) is False + # plain text turns are fine + assert _validate_message_pairing([{"role": "user", "content": "hi"}, + {"role": "assistant", "content": "done"}]) is True + + def test_composer_fill_detection_and_send_handoff(): from backend.apps.agents.browser.browser_agent import _is_composer_fill, _send_index_in_state # a composer fill is detected across the three ways the model types