From 09b6d22dfec471d9d7aa814aa0da3758dad9ebec Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 15 Jul 2026 07:59:12 -0700 Subject: [PATCH] fix(gateway): harden hidden-incomplete detection against the sentinel final_response MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the salvaged #51657: the conversation loop returns the retry-exhaustion sentinel as BOTH final_response and error, so the original detector (which required final_response to be falsy) never fired on real exhaustion turns — the sentinel text was delivered verbatim into the channel, exactly the #51628 poisoning vector. Detect the sentinel echo, blank it before empty-response normalization, and never suppress a turn whose final_response is genuine model text. Also: dedupe-guard mock fix in the test fixture (has_platform_message_id must return False, not a truthy MagicMock) and two guard tests (real answer never suppressed; interrupted/failed never classified). --- gateway/run.py | 28 +++++++++++--- .../gateway/test_incomplete_gateway_turns.py | 37 +++++++++++++++++-- 2 files changed, 57 insertions(+), 8 deletions(-) diff --git a/gateway/run.py b/gateway/run.py index 15042bd4c394..51dcd5e2f492 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -2697,17 +2697,27 @@ def _normalize_empty_agent_response( def _is_gateway_hidden_reasoning_incomplete_turn(agent_result: dict) -> bool: - """Detect retry-exhausted turns with hidden reasoning but no visible answer.""" + """Detect retry-exhausted turns with hidden reasoning but no visible answer. + + The conversation loop returns the retry-exhaustion sentinel as BOTH + ``final_response`` and ``error`` ("Codex response remained incomplete + after 3 continuation attempts"), so ``final_response`` being non-empty + does not mean the model produced a visible answer. Treat the turn as + hidden when the error sentinel is present and ``final_response`` is + either empty or merely echoes that sentinel — any genuinely different + final text means the model DID answer and must be delivered. + """ if not isinstance(agent_result, dict): return False if agent_result.get("failed") or agent_result.get("interrupted"): return False - if agent_result.get("final_response"): - return False if not agent_result.get("partial"): return False - error_text = str(agent_result.get("error", "") or "").lower() - return "remained incomplete after" in error_text + error_text = str(agent_result.get("error", "") or "").strip() + if "remained incomplete after" not in error_text.lower(): + return False + final_response = str(agent_result.get("final_response") or "").strip() + return not final_response or final_response == error_text def _should_clear_resume_pending_after_turn(agent_result: dict) -> bool: @@ -11825,6 +11835,14 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew return None response = agent_result.get("final_response") or "" + # Hidden-reasoning-only retry exhaustion: the loop's sentinel text + # ("Codex response remained incomplete after 3 continuation + # attempts") doubles as final_response, so it would be delivered + # verbatim into the channel — where peer agents can ingest it as a + # completed assistant turn (#51628). Blank it here so the normal + # empty-response handling (and the suppression below) applies. + if _is_gateway_hidden_reasoning_incomplete_turn(agent_result): + response = "" try: from gateway.response_filters import is_intentional_silence_agent_result _intentional_silence = is_intentional_silence_agent_result( diff --git a/tests/gateway/test_incomplete_gateway_turns.py b/tests/gateway/test_incomplete_gateway_turns.py index 1ac356711c36..1d777548faa6 100644 --- a/tests/gateway/test_incomplete_gateway_turns.py +++ b/tests/gateway/test_incomplete_gateway_turns.py @@ -50,8 +50,12 @@ class CaptureSlackAdapter(BasePlatformAdapter): def _make_incomplete_result() -> dict: + # Mirror the REAL conversation-loop exhaustion shape: the sentinel text is + # returned as BOTH final_response and error (agent/conversation_loop.py's + # "remained incomplete after 3 continuation attempts" return). + _sentinel = "Codex response remained incomplete after 3 continuation attempts" return { - "final_response": None, + "final_response": _sentinel, "messages": [ {"role": "user", "content": "hello"}, {"role": "assistant", "content": ""}, @@ -62,7 +66,7 @@ def _make_incomplete_result() -> dict: "partial": True, "completed": False, "interrupted": False, - "error": "Codex response remained incomplete after 3 continuation attempts", + "error": _sentinel, "last_prompt_tokens": 0, } @@ -89,6 +93,10 @@ def _make_runner(adapter: CaptureSlackAdapter) -> gateway_run.GatewayRunner: runner.session_store.rewrite_transcript = MagicMock() runner.session_store.append_to_transcript = MagicMock() runner.session_store.update_session = MagicMock() + # The transient-failure persistence path dedupes on platform message_id + # (#47237). A bare MagicMock returns a truthy mock, which would wrongly + # mark the user turn as a duplicate and skip persisting it. + runner.session_store.has_platform_message_id = MagicMock(return_value=False) runner._running_agents = {} runner._pending_messages = {} runner._pending_approvals = {} @@ -116,15 +124,38 @@ def _make_event() -> MessageEvent: def test_incomplete_codex_warning_is_not_surfaced_as_chat_text(): agent_result = _make_incomplete_result() + # Mirror the gateway pipeline: the hidden-turn detector blanks the + # sentinel final_response BEFORE empty-response normalization runs. + response = agent_result.get("final_response") or "" + assert gateway_run._is_gateway_hidden_reasoning_incomplete_turn(agent_result) + response = "" + response = gateway_run._normalize_empty_agent_response( agent_result, - agent_result.get("final_response") or "", + response, history_len=4, ) assert response == "" +def test_real_answer_alongside_incomplete_error_is_never_suppressed(): + """A turn whose final_response is genuine model text (not the sentinel + echo) must be delivered even when the error field carries the + retry-exhaustion sentinel — suppression is only for hidden turns.""" + agent_result = _make_incomplete_result() + agent_result["final_response"] = "Here is the actual answer." + + assert not gateway_run._is_gateway_hidden_reasoning_incomplete_turn(agent_result) + + +def test_interrupted_or_failed_turns_are_not_classified_hidden(): + for key in ("interrupted", "failed"): + agent_result = _make_incomplete_result() + agent_result[key] = True + assert not gateway_run._is_gateway_hidden_reasoning_incomplete_turn(agent_result) + + @pytest.mark.asyncio async def test_incomplete_codex_turn_stays_out_of_slack_transcript(monkeypatch, tmp_path): adapter = CaptureSlackAdapter()