Skip to content

fix(codex): keep progress replies from ending active turns#105290

Closed
fazakis wants to merge 6 commits into
openclaw:mainfrom
fazakis:fix/codex-explicit-final-message
Closed

fix(codex): keep progress replies from ending active turns#105290
fazakis wants to merge 6 commits into
openclaw:mainfrom
fazakis:fix/codex-explicit-final-message

Conversation

@fazakis

@fazakis fazakis commented Jul 12, 2026

Copy link
Copy Markdown

Related: #100153

Alternative to #100156.

What Problem This Solves

Fixes an issue where users running Codex with message_tool_only source delivery could receive a progress or acknowledgment reply and then see the active turn end before the promised work or final answer was completed.

Why This Change Was Made

This adds explicit delivery intent to current-source message(action=send) calls for Codex message_tool_only runs. Sends with final: false or an omitted final remain visible but non-terminal; only a successfully delivered current-source send with final: true releases the turn.

The prompt and delivery hints tell the model to use commentary for ordinary progress and final: true only after requested work and verification are complete. The internal final field is removed before provider delivery. Other runtimes, delivery modes, channels, and explicit cross-chat sends retain their existing behavior.

This is a structural alternative to #100156. That PR reduces the failure through prompt steering; this change keeps the runtime safe when a model still sends a progress message through the message tool.

User Impact

Codex-backed users can receive a visible progress message without losing the rest of the active turn. Completed final replies still terminate cleanly, preserving duplicate-reply prevention and normal turn release.

Evidence

Before the change, a real Telegram direct-chat turn on macOS with OpenClaw 2026.7.2, Codex app-server, message_tool_only, and GPT-5.6 followed this sequence:

message(action=send) progress reply
tool result: delivered
turn.dynamic_tool_terminal_release
session ended successfully before the promised investigation

Live Telegram verification after the change:

  • message(action=send, final=false) delivered PROGRESS_SENT.
  • The same turn continued through a 10-second wait and a Git status tool call.
  • message(action=send, final=true) delivered FINAL_SENT.
  • turn.dynamic_tool_terminal_release occurred only after FINAL_SENT.
  • The session ended successfully with no abort or timeout.
  • A separate natural five-minute investigation completed 19 tool calls and delivered its full diagnosis with final=true.

Validation after rebasing onto current main and refreshing canonical generated fixtures:

  • Four focused test files: 385/385 passed.
  • Post-format CLI-runner recheck: 69/69 passed.
  • Prompt snapshot consistency passed: all seven generated files current.
  • Full production build passed.
  • Formatting and diff checks passed.

The optional automated Codex reviewer could not authenticate from the shell (HTTP 401), so no automated review verdict is claimed.

AI-assisted: yes. The implementation, tests, and live reproduction were prepared with Codex assistance and reviewed and validated by the contributor.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. mantis: telegram-visible-proof Mantis should capture Telegram visible proof. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jul 12, 2026
@clawsweeper

clawsweeper Bot commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 12, 2026, 1:23 PM ET / 17:23 UTC.

Summary
Adds a Codex-only optional final field to message sends, keeps progress sends non-terminal, and releases message_tool_only turns only after confirmed delivery with final=true.

PR surface: Source +103, Tests +139, Other 0. Total +242 across 25 files.

Reproducibility: yes. Current main directly terminalizes confirmed message_tool_only source delivery, and the detailed live incident shows a progress send taking that path before the requested work.

Review metrics: 3 noteworthy metrics.

  • Completion default: implicit terminal → explicit opt-in. Confirmed current-source delivery without final=true no longer ends the turn, which is the core compatibility choice.
  • Model-facing parameters: 1 added. Codex message_tool_only schemas gain a completion-intent boolean the model must use correctly.
  • Plugin SDK exports: 1 added. The explicit-final delivery hint expands a public plugin-facing module and should match the chosen permanent contract.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #100153
Summary: The canonical issue remains affected on current main; this PR and two other open PRs are competing fixes, while the earlier merged PR introduced the implicated terminal-recognition path.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Move the selected implementation onto current main and rerun exact-head focused tests plus live Telegram proof.

Risk before merge

  • [P1] Completed current-source answers that omit final=true no longer release through confirmed delivery, so existing model behavior could leave a turn running or enter incomplete-answer handling.
  • [P1] The model-visible parameter and exported Plugin SDK hint become compatibility surfaces that green CI cannot approve as a permanent product contract.
  • [P1] The branch predates rewritten current main and is conflicting, so the selected implementation must be transplanted and revalidated on an exact current-main descendant.

Maintainer options:

  1. Transplant explicit-final implementation (recommended)
    Move the selected structural implementation onto rewritten current main and rerun exact-head focused and live proof.
  2. Preserve implicit completion
    Select the prompt-only candidate if maintainers reject a new model-visible completion parameter.
  3. Retire duplicate branches
    After selecting the permanent policy, close the non-canonical implementation candidates.

Next step before merge

  • [P2] A maintainer must choose the permanent completion contract and canonical branch; the selected implementation then needs transplantation to current main and exact-head validation.

Maintainer decision needed

  • Question: Should Codex message_tool_only turns permanently require final=true for confirmed current-source delivery to end the turn, or should confirmed delivery remain implicitly terminal with prompt steering only?
  • Rationale: Upstream Codex provides no terminal-delivery primitive, so both choices are OpenClaw-owned and establish incompatible behavior when the model omits the optional field.
  • Likely owner: omarshahine — They introduced the confirmed source-delivery terminal behavior whose permanent semantics now need calibration.
  • Options:
    • Require explicit final (recommended): Adopt the structural contract on one current-main branch, limited to Codex message_tool_only, with full omission and routing coverage.
    • Keep implicit completion: Preserve confirmed-delivery terminal behavior and select the prompt-only candidate, accepting residual early-termination risk when instructions are ignored.
    • Defer completion API: Keep the canonical issue open and pause all implementation branches until maintainers sponsor a permanent contract.

Security
Cleared: No concrete security or supply-chain concern was found; the diff adds no dependency, permission, secret handling, artifact download, or new execution source.

Review details

Best possible solution:

Adopt one Codex-only explicit-final implementation on current main if maintainers accept the omission behavior, with coverage for omitted/false progress, true final delivery, failed delivery, cross-chat sends, and field stripping.

Do we have a high-confidence way to reproduce the issue?

Yes. Current main directly terminalizes confirmed message_tool_only source delivery, and the detailed live incident shows a progress send taking that path before the requested work.

Is this the best way to solve the issue?

Yes, conditionally. Explicit completion intent is the strongest structural fix because it separates delivery from completion, but it is the best path only after maintainers approve the new omission semantics and select one clean canonical branch.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 6787c2f65359.

Label changes

Label justifications:

  • P1: The reproduced bug can terminate a real channel turn after a progress message, suppressing promised work and the final answer.
  • merge-risk: 🚨 compatibility: The PR changes omitted-field completion behavior and adds public/model-facing contract surface.
  • merge-risk: 🚨 message-delivery: The PR changes which confirmed current-source sends release a turn and therefore affects final-answer completion.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (logs): Detailed live Telegram logs show a progress send, continued wait and tool work, an explicit final send, and terminal release only afterward; linked video extraction failed because ffprobe is unavailable, but the runtime evidence is convincing.
  • proof: sufficient: Contributor real behavior proof is sufficient. Detailed live Telegram logs show a progress send, continued wait and tool work, an explicit final send, and terminal release only afterward; linked video extraction failed because ffprobe is unavailable, but the runtime evidence is convincing.
  • mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The progress-then-final sequence is directly visible in Telegram and is suitable for the Telegram Desktop proof lane.
Evidence reviewed

PR surface:

Source +103, Tests +139, Other 0. Total +242 across 25 files.

View PR surface stats
Area Files Added Removed Net
Source 12 117 14 +103
Tests 12 165 26 +139
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 1 1 1 0
Total 25 283 41 +242

What I checked:

Likely related people:

  • omarshahine: Authored the merged source-reply recognition and terminalization behavior that this PR revises. (role: introduced behavior; confidence: high; commits: 9b9a124cc520; files: extensions/codex/src/app-server/dynamic-tools.ts)
  • steipete: Carried major agent/message-tool architecture and the earlier Codex extension-boundary move, making this a likely product-contract routing point. (role: adjacent owner; confidence: medium; commits: d5698038d71c, bb0bfabec85d; files: extensions/codex/src/app-server/dynamic-tools.ts, src/agents/tools/message-tool.ts)
  • Gustavo Madeira Santana: Introduced shared message-tool discovery and dispatch boundaries through which the new model-facing field passes. (role: message-tool architecture contributor; confidence: medium; commits: a14ad01d66f8, a32c7e16d204, 951f3f992b68; files: src/agents/tools/message-tool.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (8 earlier review cycles)
  • reviewed 2026-07-12T10:56:56.902Z sha e26458e :: found issues before merge. :: [P2] Update the cached-prompt contract assertion
  • reviewed 2026-07-12T11:13:53.803Z sha e6c1f36 :: found issues before merge. :: [P2] Scope final to Codex message-tool-only schemas
  • reviewed 2026-07-12T11:56:38.490Z sha d936c3e :: found issues before merge. :: [P2] Scope final to Codex message-tool-only schemas
  • reviewed 2026-07-12T12:04:56.667Z sha d936c3e :: found issues before merge. :: [P2] Scope final to Codex message-tool-only schemas
  • reviewed 2026-07-12T12:39:21.374Z sha 88f9162 :: found issues before merge. :: [P1] Scope final=true instructions to Codex too
  • reviewed 2026-07-12T12:48:43.831Z sha 74c589d :: needs changes before merge. :: [P1] Scope final=true instructions to Codex too
  • reviewed 2026-07-12T12:58:30.700Z sha 74c589d :: needs changes before merge. :: [P1] Scope final=true instructions to Codex too
  • reviewed 2026-07-12T14:05:41.386Z sha 09c6c65 :: needs maintainer review before merge. :: none

@fazakis
fazakis force-pushed the fix/codex-explicit-final-message branch from e26458e to e6c1f36 Compare July 12, 2026 11:03
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. label Jul 12, 2026

Copy link
Copy Markdown
Contributor

A janela de coordenação passou sem resposta prática do autor, então abri #105365 como sucessora limpa da abordagem de final explícito.

Ela preserva o comportamento estrutural desta PR, mas corrige exatamente o achado de revisão: final só existe no schema projetado do Codex quando sourceReplyDeliveryMode=message_tool_only; não vaza para o message tool genérico, não alcança o provider nem altera a idempotência. Inclui snapshots de prompt e testes positivos/negativos; está rebaseada no main atual e a CI está em andamento.

Não estou propondo fechar esta PR por conta própria. A decisão continua simples para maintainers: adotar o contrato explícito em #105365 ou preferir a alternativa prompt-only em #100156.

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jul 12, 2026
@openclaw-barnacle openclaw-barnacle Bot added the scripts Repository scripts label Jul 12, 2026
@fazakis

fazakis commented Jul 12, 2026

Copy link
Copy Markdown
Author

Author status update

All contributor-actionable findings are now addressed on head 7c03d765b9:

  • final schema and guidance are scoped to Codex message_tool_only only.
  • Non-Codex negative coverage is included.
  • Prompt-cache variants are separated correctly.
  • The Plugin SDK export budget is updated for the new public hint.
  • All required CI checks are green.

@obviyus

obviyus commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Closing as superseded: #105365 landed on main as 4dc6e18, carrying this branch's runtime direction with the review gaps repaired — the final control stays scoped to the projected Codex schema instead of the shared message schema, explicit progress/completion evidence propagates through payload suppression, terminal-failure routing, and stranded-reply recovery, and omitted final preserves the legacy terminate-on-delivery contract. Thanks for establishing the structural approach here — it directly shaped the landed fix. If you spot a case the landed contract misses, please open a fresh issue with run telemetry.

@obviyus obviyus closed this Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling extensions: codex mantis: telegram-visible-proof Mantis should capture Telegram visible proof. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P1 High-priority user-facing bug, regression, or broken workflow. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. scripts Repository scripts size: M status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants