diff --git a/agent/codex_runtime.py b/agent/codex_runtime.py index 584dc1b0bd2f..9c8362b8a39b 100644 --- a/agent/codex_runtime.py +++ b/agent/codex_runtime.py @@ -16,6 +16,7 @@ compatibility. from __future__ import annotations +import json import logging import os import time @@ -25,61 +26,6 @@ from typing import Any, Callable, Dict, List logger = logging.getLogger(__name__) -def _codex_note_to_tool_progress(note: dict) -> tuple[str, str, dict] | None: - """Map a Codex app-server ``item/started`` notification to a Hermes - tool-progress event ``(tool_name, preview, args)``. - - The Codex app-server runtime processes ``item/started`` notifications for - command execution, file changes, and MCP/dynamic tool calls, but never - surfaced them as Hermes tool-progress events — so gateways (Telegram, etc.) - showed no verbose "running X" breadcrumbs on this route while every other - provider did (#38835). Returns None for items that aren't tool-shaped. - """ - if not isinstance(note, dict) or note.get("method") != "item/started": - return None - params = note.get("params") or {} - item = params.get("item") or {} - if not isinstance(item, dict): - return None - - item_type = item.get("type") or "" - if item_type == "commandExecution": - command = item.get("command") or "" - return "exec_command", command, {"command": command, "cwd": item.get("cwd") or ""} - - if item_type == "fileChange": - changes = item.get("changes") or [] - preview = "file changes" - if isinstance(changes, list) and changes: - paths = [ - str(change.get("path")) - for change in changes - if isinstance(change, dict) and change.get("path") - ] - if paths: - preview = ", ".join(paths[:3]) - if len(paths) > 3: - preview += f", +{len(paths) - 3} more" - return "apply_patch", preview, {"changes": changes} - - if item_type == "mcpToolCall": - server = item.get("server") or "mcp" - tool = item.get("tool") or "unknown" - args = item.get("arguments") or {} - if not isinstance(args, dict): - args = {"arguments": args} - return f"mcp.{server}.{tool}", tool, args - - if item_type == "dynamicToolCall": - tool = item.get("tool") or "unknown" - args = item.get("arguments") or {} - if not isinstance(args, dict): - args = {"arguments": args} - return tool, tool, args - - return None - - def _coerce_usage_int(value: Any) -> int: if isinstance(value, bool): return 0 @@ -558,6 +504,11 @@ def make_codex_app_server_event_bridge(agent) -> Callable[[dict], None]: text = item.get("text") or "" if not isinstance(text, str) or not text.strip(): return + # display.show_commentary=false — mid-turn narration stays off the + # visible interim path on this runtime too (same contract as the + # codex_responses commentary channel). + if not getattr(agent, "show_commentary", True): + return emit = getattr(agent, "_emit_interim_assistant_message", None) if emit is None: return diff --git a/tests/agent/test_codex_app_server_event_bridge.py b/tests/agent/test_codex_app_server_event_bridge.py index 980a5b70b19e..e02748cfa697 100644 --- a/tests/agent/test_codex_app_server_event_bridge.py +++ b/tests/agent/test_codex_app_server_event_bridge.py @@ -426,6 +426,23 @@ class TestAgentMessageInterimDispatch: })) agent.tool_progress_callback.assert_not_called() + def test_show_commentary_off_suppresses_interim(self): + """display.show_commentary=false silences agentMessage interim + delivery on the app-server runtime (same contract as the + codex_responses commentary channel).""" + agent = _make_stub_agent() + agent.show_commentary = False + bridge = make_codex_app_server_event_bridge(agent) + bridge(_item_completed({ + "type": "agentMessage", "id": "am-5", "text": "I'll check config.", + })) + agent._emit_interim_assistant_message.assert_not_called() + # Tool progress is unaffected by the commentary toggle. + bridge(_item_started({ + "type": "commandExecution", "id": "cmd-1", "command": "ls", + })) + agent.tool_progress_callback.assert_called_once() + class TestBridgeRobustness: def test_non_dict_notification_is_ignored(self): @@ -544,6 +561,16 @@ class TestBridgeWiredInRuntime: valid_tool_names=set(), _sync_external_memory_for_turn=lambda **_: None, _spawn_background_review=lambda **_: None, + # Usage accounting attrs read by _record_codex_app_server_usage. + session_api_calls=0, + session_prompt_tokens=0, + session_completion_tokens=0, + session_reasoning_tokens=0, + session_cached_tokens=0, + session_total_tokens=0, + context_compressor=None, + event_callback=None, + _session_db=None, ) codex_runtime.run_codex_app_server_turn( diff --git a/tests/run_agent/test_codex_app_server_integration.py b/tests/run_agent/test_codex_app_server_integration.py index c88d45c483af..199ff5d56cb1 100644 --- a/tests/run_agent/test_codex_app_server_integration.py +++ b/tests/run_agent/test_codex_app_server_integration.py @@ -12,6 +12,7 @@ Verifies that: from __future__ import annotations +from types import SimpleNamespace from unittest.mock import MagicMock, patch import pytest @@ -694,48 +695,64 @@ class TestSessionRetirementOnRunAgent: class TestCodexToolProgressBridge: - """#38835: Codex app-server item/started notifications must surface as - Hermes tool-progress so gateways show verbose breadcrumbs on this route.""" + """#38835 / #33200: Codex app-server item notifications must surface as + Hermes tool-progress so gateways show verbose breadcrumbs on this route. + The original item/started-only mapper was superseded by the full event + bridge (make_codex_app_server_event_bridge); these tests pin the same + mapping contract against the bridge helpers.""" def test_mapper_command_execution(self): - from agent.codex_runtime import _codex_note_to_tool_progress - note = {"method": "item/started", "params": {"item": { - "type": "commandExecution", "command": "ls -la", "cwd": "/tmp"}}} - name, preview, args = _codex_note_to_tool_progress(note) - assert name == "exec_command" - assert preview == "ls -la" - assert args == {"command": "ls -la", "cwd": "/tmp"} + from agent.codex_runtime import ( + _codex_item_to_args, + _codex_item_to_preview, + _codex_item_to_tool_name, + ) + item = {"type": "commandExecution", "command": "ls -la", "cwd": "/tmp"} + assert _codex_item_to_tool_name(item) == "exec_command" + assert _codex_item_to_preview(item) == "ls -la" + assert _codex_item_to_args(item) == {"command": "ls -la", "cwd": "/tmp"} def test_mapper_file_change(self): - from agent.codex_runtime import _codex_note_to_tool_progress - note = {"method": "item/started", "params": {"item": { + from agent.codex_runtime import ( + _codex_item_to_preview, + _codex_item_to_tool_name, + ) + item = { "type": "fileChange", - "changes": [{"path": "a.py"}, {"path": "b.py"}]}}} - name, preview, args = _codex_note_to_tool_progress(note) - assert name == "apply_patch" - assert preview == "a.py, b.py" + "changes": [{"path": "a.py"}, {"path": "b.py"}], + } + assert _codex_item_to_tool_name(item) == "apply_patch" + assert _codex_item_to_preview(item) == "a.py, b.py" def test_mapper_mcp_and_dynamic_tool_calls(self): - from agent.codex_runtime import _codex_note_to_tool_progress - mcp = {"method": "item/started", "params": {"item": { - "type": "mcpToolCall", "server": "fs", "tool": "read", "arguments": {"p": 1}}}} - name, preview, args = _codex_note_to_tool_progress(mcp) - assert name == "mcp.fs.read" - assert preview == "read" - assert args == {"p": 1} + from agent.codex_runtime import ( + _codex_item_to_args, + _codex_item_to_tool_name, + ) + mcp = {"type": "mcpToolCall", "server": "fs", "tool": "read", "arguments": {"p": 1}} + assert _codex_item_to_tool_name(mcp) == "mcp.fs.read" + assert _codex_item_to_args(mcp) == {"p": 1} - dyn = {"method": "item/started", "params": {"item": { - "type": "dynamicToolCall", "tool": "web_search", "arguments": {"q": "x"}}}} - assert _codex_note_to_tool_progress(dyn)[0] == "web_search" + dyn = {"type": "dynamicToolCall", "tool": "web_search", "arguments": {"q": "x"}} + assert _codex_item_to_tool_name(dyn) == "web_search" - def test_mapper_ignores_non_tool_items_and_other_methods(self): - from agent.codex_runtime import _codex_note_to_tool_progress - # agentMessage / reasoning items are not tool-shaped - assert _codex_note_to_tool_progress({"method": "item/started", "params": { - "item": {"type": "agentMessage", "text": "hi"}}}) is None - # non-item/started methods - assert _codex_note_to_tool_progress({"method": "item/completed", "params": {}}) is None - assert _codex_note_to_tool_progress({}) is None + def test_bridge_ignores_non_tool_items_and_other_methods(self): + from agent.codex_runtime import make_codex_app_server_event_bridge + events = [] + agent = SimpleNamespace( + tool_progress_callback=lambda *a, **kw: events.append(a), + _fire_stream_delta=None, + _fire_reasoning_delta=None, + _emit_interim_assistant_message=None, + ) + on_event = make_codex_app_server_event_bridge(agent) + # agentMessage started items are not tool-shaped + on_event({"method": "item/started", "params": { + "item": {"type": "agentMessage", "text": "hi"}}}) + # malformed / empty notes + on_event({"method": "item/completed", "params": {}}) + on_event({}) + assert events == [] def test_session_wired_with_on_event_that_fires_tool_progress(self, monkeypatch): """The session is constructed with an on_event hook that, when fed an