fix(codex): surface live tool-progress + interim commentary on codex_app_server runtime - #66142
Merged
Conversation
Adds ``make_codex_app_server_event_bridge(agent)`` plus four small mapping helpers (``_codex_item_to_tool_name`` / ``_codex_item_to_args`` / ``_codex_item_to_preview`` / ``_codex_item_completion_payload``) that translate codex JSON-RPC ``item/*`` notifications into the exact shape Hermes' gateway UI callbacks expect — tool names match ``CodexEventProjector`` so the progress bubbles and the projected ``tool_calls`` entries use the same identifiers. No behaviour change yet: the next commit wires the bridge into ``run_codex_app_server_turn`` (#33200).
…time (#33200) Pass ``on_event=make_codex_app_server_event_bridge(agent)`` when spawning the per-session ``CodexAppServerSession``. The session has always had a raw event hook but ``run_codex_app_server_turn`` never supplied one, so Discord / Telegram / TUI users saw nothing while codex was working — only the final answer landed. Now each ``item/started`` for a tool-shaped item fires ``tool_progress_callback("tool.started", ...)``, ``item/completed`` fires the matching ``"tool.completed"`` with duration + result, ``item/agentMessage/delta`` flows through ``_fire_stream_delta`` and each completed ``agentMessage`` surfaces through ``_emit_interim_assistant_message`` so the gateway's ``already_streamed`` dedupe keeps interim commentary in the channel without duplicating text the stream already showed.
42 tests across five suites: * ``TestCodexItemToToolName`` / ``TestCodexItemToArgs`` / ``TestCodexItemToPreview`` / ``TestCodexItemCompletionPayload`` — pin the per-type mapping so the synthetic tool name + args the UI sees match what ``CodexEventProjector`` writes into messages. * ``TestStreamDeltaDispatch`` / ``TestToolProgressDispatch`` / ``TestAgentMessageInterimDispatch`` — drive each Codex notification shape through the bridge and assert the right agent callback fires with the right arguments (including the duration / is_error / result kwargs the gateway renders). * ``TestBridgeRobustness`` — defensive paths: non-dict notifications, missing params, raising callbacks (must not tear down the codex turn loop), and agents without callbacks registered (cron / gateway-less contexts). * ``TestBridgeWiredInRuntime`` — integration guard that ``run_codex_app_server_turn`` actually constructs the session with ``on_event=<bridge>``, preventing a future refactor from silently regressing live progress visibility again.
…on show_commentary Follow-ups on top of @xxxigm's salvaged bridge (#33294): - Remove the now-dead narrow item/started-only mapper from #38835 (_codex_note_to_tool_progress) — the full bridge supersedes it and keeps the same tool-name contract; its tests are repointed at the bridge helpers. - Preserve main's request_routing/approval-bypass wiring on the CodexAppServerSession constructor (landed after the PR was filed). - Gate agentMessage interim delivery on display.show_commentary so the app-server runtime honors the same toggle as the codex_responses commentary channel (tool progress is unaffected). - Add json import (bridge helpers use json.dumps) and modernize the wiring test's stub agent for main's usage-accounting attributes.
alt-glitch
added
type/bug
tonydwb
reviewed
Jul 17, 2026
tonydwb
left a comment
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Comment
PR #66142 fix(codex): surface live tool-progress + interim commentary on codex_app_server runtime
Assessment
- Scope: ~944 additions, 108 deletions. Significant fix for tool-progress display and interim commentary on the codex runtime.
- Correctness: Fix surfaces live tool-progress and interim commentary from codex_app_server. The change modifies codex-related tooling.
- Note: Large diff for a runtime/display fix. Recommend verifying the tool-progress surfacing works correctly in all codex execution paths.
Reviewed by Hermes Agent
This was referenced Jul 17, 2026
Closed
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Discord/Telegram/TUI now show live tool-progress bubbles AND interim assistant commentary when the provider runs on
openai_runtime: codex_app_server— previously the channel was silent until the final answer landed (issue #33200).Salvages #33294 by @xxxigm with all 3 commits cherry-picked and authorship preserved, reconciled onto current main (which had since grown a narrower
item/started-only bridge via #38835), plus a follow-up commit from us.Changes
agent/codex_runtime.py: full event bridge (make_codex_app_server_event_bridge) — 4 pure mapping helpers translate codexitem/*payloads into Hermes callback shapes:item/started(tool-shaped) →tool_progress_callback("tool.started", name, preview, args)item/completed(tool-shaped) →"tool.completed"with duration (codexdurationMspreferred, wall-clock fallback),is_error, result textitem/agentMessage/delta→_fire_stream_delta;item/reasoning/delta→_fire_reasoning_deltaagentMessage→_emit_interim_assistant_message(gateway'salready_streameddedupe applies)_codex_note_to_tool_progressmapper is removed — the bridge supersedes it with the same tool-name contract; main'srequest_routing/approval-bypass session wiring is preservedshow_commentarygate: agentMessage interim delivery honors thedisplay.show_commentarytoggle (same contract as the codex_responses commentary channel merged in feat(agent): stream Codex commentary as visible interim messages (+ show_commentary toggle) #66115); tool progress is unaffectedValidation
test_codex_app_server_event_bridge.py+test_codex_app_server_integration.py+test_run_agent_codex_responses.py[exit 1]+is_error=TrueAIAgent+ configshow_commentary: false, temp HERMES_HOME)Credit
@xxxigm submitted the complete bridge on May 27 — before the narrower #38835 fix landed — and it includes the
tool.completed+ commentary halves that #38835 never covered. Commits preserved via cherry-pick.Closes #33294. Fixes #33200.
Infographic