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()