Fix Codex message-tool-only source reply completion#95942
Conversation
4190fac to
dd61c32
Compare
|
Codex review: found issues before merge. Reviewed June 28, 2026, 2:56 PM ET / 18:56 UTC. Summary PR surface: Source +304, Tests +633, Other 0. Total +937 across 9 files. Reproducibility: yes. Current main lacks the source-reply mode/current message id in the Codex dynamic-tool terminalization decision, and the PR body gives live before/after iMessage logs showing the incomplete-turn fallback disappears after the patch. Review metrics: 1 noteworthy metric.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Land the Codex terminalization fix after maintainers either approve and document the public SDK exports or move the detector behind a narrower approved seam. Do we have a high-confidence way to reproduce the issue? Yes. Current main lacks the source-reply mode/current message id in the Codex dynamic-tool terminalization decision, and the PR body gives live before/after iMessage logs showing the incomplete-turn fallback disappears after the patch. Is this the best way to solve the issue? Mostly yes. The Codex bridge/projector is the right layer because upstream Codex dynamic-tool responses only carry returned content and success; the unresolved part is whether the detector should be public SDK API or a narrower seam. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against fcff01cc8251. Label changesLabel justifications:
Evidence reviewedPR surface: Source +304, Tests +633, Other 0. Total +937 across 9 files. View PR surface stats
Acceptance criteria:
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
dd61c32 to
d72b96b
Compare
d72b96b to
d5335b0
Compare
d5335b0 to
432b941
Compare
432b941 to
f6a06ae
Compare
|
Merged via squash.
|
* fix(codex): recognize message tool source replies * fix(codex): accept numeric source message ids * fix(codex): account for source reply SDK surface * fix(codex): require delivered reply receipts * fix(codex): reject alias-routed source replies * fix(codex): require delivered non-send telemetry * fix(codex): honor normalized source routes --------- Co-authored-by: Omar Shahine <[email protected]>
* fix(codex): recognize message tool source replies * fix(codex): accept numeric source message ids * fix(codex): account for source reply SDK surface * fix(codex): require delivered reply receipts * fix(codex): reject alias-routed source replies * fix(codex): require delivered non-send telemetry * fix(codex): honor normalized source routes --------- Co-authored-by: Omar Shahine <[email protected]>
* fix(codex): recognize message tool source replies * fix(codex): accept numeric source message ids * fix(codex): account for source reply SDK surface * fix(codex): require delivered reply receipts * fix(codex): reject alias-routed source replies * fix(codex): require delivered non-send telemetry * fix(codex): honor normalized source routes --------- Co-authored-by: Omar Shahine <[email protected]>
* fix(codex): recognize message tool source replies * fix(codex): accept numeric source message ids * fix(codex): account for source reply SDK surface * fix(codex): require delivered reply receipts * fix(codex): reject alias-routed source replies * fix(codex): require delivered non-send telemetry * fix(codex): honor normalized source routes --------- Co-authored-by: Omar Shahine <[email protected]>
…urn termination Upstream openclaw#95942 (v2026.7.1-beta.1 base) classifies every delivered implicit-route message(action=send) as the completed source reply and releases the Codex app-server turn, which turned mid-turn progress updates into turn-enders. Adds an optional progress boolean to the message tool send schema; progress sends are excluded from source-reply classification (shared classifier + codex receipt-confirmed path) and do not set didDeliverSourceReplyViaMessageTool. Fork carry documented in bex-fork.md; upstream-PR candidate.
What Problem This Solves
Codex
message_tool_onlyturns over iMessage could send the visible reply through themessagetool, then keep the turn open because OpenClaw did not recognize that tool result as the source reply. That left the run waiting until the no-visible-answer path emitted an incomplete-turn fallback.This was visible in live iMessage proof as a sent reply followed by
incomplete turn detectedand a suppressed fallback error.Why This Change Was Made
The fix teaches the Codex dynamic tool bridge to mark delivered
message_tool_onlysource replies as terminal only when the delivery evidence belongs to the current source route/message:messagetool replies from route matches, native target segments, provider/channel context, and current-message reply receiptsUser Impact
Users who run Codex through a messaging channel configured with
sourceReplyDeliveryMode: "message_tool_only"benefit. For iMessage, this is the setup where Codex replies by calling the message tool instead of returning a final text answer for OpenClaw to send afterward.In that setup, OpenClaw can complete the turn immediately after the message tool returns confirmed source-reply delivery instead of waiting for the incomplete-turn fallback path.
Performance Savings
The measured live proof removed the post-delivery incomplete/fallback wait from the successful source-reply path. In the failing run, the iMessage
messagetool released at17:53:40.784, but OpenClaw did not mark the turn terminal; it loggedincomplete turn detectedat17:53:41.389and then suppressed the fallback error at17:53:41.597. That left about0.6sto incomplete detection and about0.8sto fallback after the visible reply had already been sent.In the fixed run, the iMessage tool output returned at
19:31:07.225, terminal release happened at19:31:07.231, and the message finished processing at19:31:07.449. That is about6msfrom confirmed delivery to terminal recognition and about224msfrom confirmed delivery to processed completion, while also eliminating the spurious fallback/error path.Evidence
Focused local validation on the clean branch after rebasing onto latest
origin/main:node scripts/run-vitest.mjs extensions/codex/src/app-server/dynamic-tools.test.ts extensions/codex/src/app-server/event-projector.test.ts src/agents/embedded-agent-message-tool-source-reply.test.ts -- --reporter=dotsrc/agents/embedded-agent-message-tool-source-reply.test.ts: 19/19 passedextensions/codex/src/app-server/dynamic-tools.test.ts+event-projector.test.ts: 168/168 passedgit diff --checkLive iMessage proof from the equivalent patched build:
861:17:53:23.51617:53:26.65717:53:40.784incomplete turn detectedat17:53:41.38917:53:41.59718.115s873:19:30:48.452message_start19:30:48.47419:30:50.276messagetool call at19:31:01.79019:31:07.225with{ ok: true, messageId: "95F25ADC-FA98-4FBC-8988-EB9A8018143F", repliedTo: "A873644F-204D-4807-8C3E-C14B50EE4A4A" }19:31:07.23119:31:07.37519:31:07.449, duration18.994sincomplete turn detected