diff --git a/agent/codex_responses_adapter.py b/agent/codex_responses_adapter.py index e9b6ace9b85..cda1f87be12 100644 --- a/agent/codex_responses_adapter.py +++ b/agent/codex_responses_adapter.py @@ -1166,15 +1166,23 @@ def _normalize_codex_response( if item_type == "message": item_phase = getattr(item, "phase", None) normalized_phase = None + is_commentary_phase = False if isinstance(item_phase, str): normalized_phase = item_phase.strip().lower() if normalized_phase in {"commentary", "analysis"}: saw_commentary_phase = True + is_commentary_phase = True elif normalized_phase in {"final_answer", "final"}: saw_final_answer_phase = True message_text = _extract_responses_message_text(item) if message_text: - content_parts.append(message_text) + # Responses ``commentary``/``analysis`` phase text is scratch/tool + # preamble for the model/provider protocol, not user-visible final + # answer text. Preserve the exact message item for replay/cache + # continuity, but do not expose it as assistant content where the + # gateway/CLI/interim callbacks can leak it to the user. + if not is_commentary_phase: + content_parts.append(message_text) raw_message_item: Dict[str, Any] = { "type": "message", "role": "assistant", @@ -1269,7 +1277,11 @@ def _normalize_codex_response( )) final_text = "\n".join([p for p in content_parts if p]).strip() - if not final_text and hasattr(response, "output_text"): + if ( + not final_text + and hasattr(response, "output_text") + and not (saw_commentary_phase and not saw_final_answer_phase) + ): out_text = getattr(response, "output_text", "") if isinstance(out_text, str): final_text = out_text.strip() diff --git a/agent/codex_runtime.py b/agent/codex_runtime.py index 1e7b9fd7d2b..38ba98f5835 100644 --- a/agent/codex_runtime.py +++ b/agent/codex_runtime.py @@ -500,6 +500,14 @@ def _event_field(event: Any, name: str, default: Any = None) -> Any: return value if value is not None else default +def _item_field(item: Any, name: str, default: Any = None) -> Any: + """Field access for nested Response items (attr-style SDK object or dict).""" + value = getattr(item, name, None) + if value is None and isinstance(item, dict): + value = item.get(name, default) + return value if value is not None else default + + def _raise_stream_error(event: Any) -> None: """Raise a ``_StreamErrorEvent`` from a ``type=error`` SSE frame. @@ -562,6 +570,7 @@ def _consume_codex_event_stream( collected_text_deltas: List[str] = [] has_tool_calls = False first_delta_fired = False + active_message_phase: str | None = None terminal_status: str = "completed" terminal_usage: Any = None terminal_response_id: str = None @@ -595,9 +604,26 @@ def _consume_codex_event_stream( if event_type == "error": _raise_stream_error(event) + # Track the phase of the active streamed message item. Codex/Harmony + # ``commentary``/``analysis`` text is protocol/tool preamble, not the + # final answer. We still collect completed output items for replay, but + # suppress those deltas from live user-visible callbacks. + if event_type == "response.output_item.added": + item = _event_field(event, "item") + item_type = _item_field(item, "type", "") + if item_type == "message": + phase = _item_field(item, "phase", None) + active_message_phase = phase.strip().lower() if isinstance(phase, str) else None + else: + active_message_phase = None + if "function_call" in str(item_type): + has_tool_calls = True + continue + if "output_text.delta" in event_type or event_type == "response.output_text.delta": delta_text = _event_field(event, "delta", "") - if delta_text: + is_commentary_delta = active_message_phase in {"commentary", "analysis"} + if delta_text and not is_commentary_delta: collected_text_deltas.append(delta_text) if not has_tool_calls: if not first_delta_fired: diff --git a/tests/run_agent/test_run_agent_codex_responses.py b/tests/run_agent/test_run_agent_codex_responses.py index a1a46e2f44f..0b9f7daa06f 100644 --- a/tests/run_agent/test_run_agent_codex_responses.py +++ b/tests/run_agent/test_run_agent_codex_responses.py @@ -629,6 +629,71 @@ def test_run_codex_stream_returns_collected_items_when_stream_ends_without_termi assert response.output == [output_item] +def test_consume_codex_stream_suppresses_commentary_phase_deltas(monkeypatch): + from agent.codex_runtime import _consume_codex_event_stream + + commentary_item = SimpleNamespace( + type="message", + phase="commentary", + status="completed", + content=[SimpleNamespace(type="output_text", text="I’ll call the tool now.")], + ) + function_item = SimpleNamespace( + type="function_call", + id="fc_1", + call_id="call_1", + name="terminal", + arguments="{}", + ) + streamed = [] + + response = _consume_codex_event_stream( + _FakeCreateStream([ + SimpleNamespace(type="response.created"), + SimpleNamespace( + type="response.output_item.added", + item=SimpleNamespace(type="message", phase="commentary"), + ), + SimpleNamespace(type="response.output_text.delta", delta="I’ll call the tool now."), + SimpleNamespace(type="response.output_item.done", item=commentary_item), + SimpleNamespace( + type="response.output_item.added", + item=SimpleNamespace(type="function_call"), + ), + SimpleNamespace(type="response.output_item.done", item=function_item), + SimpleNamespace(type="response.completed", response=SimpleNamespace(status="completed")), + ]), + model="gpt-5-codex", + on_text_delta=streamed.append, + ) + + assert streamed == [] + assert response.output == [commentary_item, function_item] + assert response.output_text == "" + + +def test_consume_codex_stream_keeps_final_answer_phase_deltas(monkeypatch): + from agent.codex_runtime import _consume_codex_event_stream + + streamed = [] + response = _consume_codex_event_stream( + _FakeCreateStream([ + SimpleNamespace(type="response.created"), + SimpleNamespace( + type="response.output_item.added", + item=SimpleNamespace(type="message", phase="final_answer"), + ), + SimpleNamespace(type="response.output_text.delta", delta="visible answer"), + SimpleNamespace(type="response.completed", response=SimpleNamespace(status="completed")), + ]), + model="gpt-5-codex", + on_text_delta=streamed.append, + ) + + assert streamed == ["visible answer"] + assert response.output_text == "visible answer" + + def test_run_codex_stream_surfaces_failed_status_in_final_response(monkeypatch): """A ``response.failed`` terminal event is reflected on the returned object.""" agent = _build_agent(monkeypatch) @@ -1156,7 +1221,7 @@ def test_run_conversation_codex_tool_round_trip(monkeypatch): responses = [_codex_tool_call_response(), _codex_message_response("done")] monkeypatch.setattr(agent, "_interruptible_api_call", lambda api_kwargs: responses.pop(0)) - def _fake_execute_tool_calls(assistant_message, messages, effective_task_id): + def _fake_execute_tool_calls(assistant_message, messages, effective_task_id, *_args): for call in assistant_message.tool_calls: messages.append( { @@ -1347,7 +1412,7 @@ def test_run_conversation_codex_replay_payload_keeps_call_id(monkeypatch): monkeypatch.setattr(agent, "_interruptible_api_call", _fake_api_call) - def _fake_execute_tool_calls(assistant_message, messages, effective_task_id): + def _fake_execute_tool_calls(assistant_message, messages, effective_task_id, *_args): for call in assistant_message.tool_calls: messages.append( { @@ -1382,7 +1447,7 @@ def test_run_conversation_codex_continues_after_incomplete_interim_message(monke ] monkeypatch.setattr(agent, "_interruptible_api_call", lambda api_kwargs: responses.pop(0)) - def _fake_execute_tool_calls(assistant_message, messages, effective_task_id): + def _fake_execute_tool_calls(assistant_message, messages, effective_task_id, *_args): for call in assistant_message.tool_calls: messages.append( { @@ -1569,9 +1634,25 @@ def test_normalize_codex_response_marks_commentary_only_message_as_incomplete(mo ) assert finish_reason == "incomplete" - assert "inspect the repository" in (assistant_message.content or "") + assert (assistant_message.content or "") == "" + assert assistant_message.codex_message_items + assert assistant_message.codex_message_items[0]["phase"] == "commentary" + assert "inspect the repository" in assistant_message.codex_message_items[0]["content"][0]["text"] +def test_normalize_codex_response_does_not_fallback_to_output_text_for_commentary_only(monkeypatch): + agent = _build_agent(monkeypatch) + from agent.codex_responses_adapter import _normalize_codex_response + + response = _codex_commentary_message_response("I’ll call the tool now.") + response.output_text = "I’ll call the tool now." + + assistant_message, finish_reason = _normalize_codex_response(response) + + assert finish_reason == "incomplete" + assert (assistant_message.content or "") == "" + assert assistant_message.codex_message_items[0]["phase"] == "commentary" + def test_normalize_codex_response_final_answer_overrides_top_level_incomplete(monkeypatch): from agent.codex_responses_adapter import _normalize_codex_response @@ -1974,7 +2055,7 @@ def test_run_conversation_codex_continues_after_commentary_phase_message(monkeyp ] monkeypatch.setattr(agent, "_interruptible_api_call", lambda api_kwargs: responses.pop(0)) - def _fake_execute_tool_calls(assistant_message, messages, effective_task_id): + def _fake_execute_tool_calls(assistant_message, messages, effective_task_id, *_args): for call in assistant_message.tool_calls: messages.append( { @@ -1990,11 +2071,17 @@ def test_run_conversation_codex_continues_after_commentary_phase_message(monkeyp assert result["completed"] is True assert result["final_response"] == "Architecture summary complete." + commentary_messages = [ + msg for msg in result["messages"] + if msg.get("role") == "assistant" and msg.get("finish_reason") == "incomplete" + ] + assert commentary_messages + assert all((msg.get("content") or "") == "" for msg in commentary_messages) assert any( - msg.get("role") == "assistant" - and msg.get("finish_reason") == "incomplete" - and "inspect the repo structure" in (msg.get("content") or "") - for msg in result["messages"] + "inspect the repo structure" in item["content"][0]["text"] + for msg in commentary_messages + for item in (msg.get("codex_message_items") or []) + if item.get("phase") == "commentary" ) assert any(msg.get("role") == "tool" and msg.get("tool_call_id") == "call_1" for msg in result["messages"]) @@ -2010,7 +2097,7 @@ def test_run_conversation_codex_continues_after_ack_stop_message(monkeypatch): ] monkeypatch.setattr(agent, "_interruptible_api_call", lambda api_kwargs: responses.pop(0)) - def _fake_execute_tool_calls(assistant_message, messages, effective_task_id): + def _fake_execute_tool_calls(assistant_message, messages, effective_task_id, *_args): for call in assistant_message.tool_calls: messages.append( { @@ -2051,7 +2138,7 @@ def test_run_conversation_codex_continues_after_ack_for_directory_listing_prompt ] monkeypatch.setattr(agent, "_interruptible_api_call", lambda api_kwargs: responses.pop(0)) - def _fake_execute_tool_calls(assistant_message, messages, effective_task_id): + def _fake_execute_tool_calls(assistant_message, messages, effective_task_id, *_args): for call in assistant_message.tool_calls: messages.append( {