Recover stranded message-tool finals by default#99536
Conversation
c71c9e8 to
4298c6e
Compare
|
Codex review: needs real behavior proof before merge. Reviewed July 10, 2026, 3:28 AM ET / 07:28 UTC. Summary PR surface: Source +450, Tests +1082, Docs 0, Other +130. Total +1662 across 22 files. Reproducibility: yes. at source level: Codex exposes the completed final message, OpenClaw intentionally suppresses ordinary finals in Review metrics: 2 noteworthy metrics.
Stored data model 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:
Proof guidance:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Keep the original private final suppressed, perform at most one current-source-bound retry using non-persistent normalized text, and emit only a sanitized diagnostic after verified retry failure; approve this as the default only with explicit maintainer policy ownership. Do we have a high-confidence way to reproduce the issue? Yes at source level: Codex exposes the completed final message, OpenClaw intentionally suppresses ordinary finals in Is this the best way to solve the issue? Yes technically: one bounded message-tool re-prompt is narrower and safer than directly publishing the private final; whether it should be default-on is the remaining product decision. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 27ceb13e219f. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +450, Tests +1082, Docs 0, Other +130. Total +1662 across 22 files. View PR surface stats
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
Review history (1 earlier review cycle)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c71c9e8ce6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
/review Fresh ClawSweeper review requested for current head |
|
🦞🧹 I asked ClawSweeper to review this item again. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6f6f4d160
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0c49910980
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
/review Fresh ClawSweeper review requested for current head |
|
🦞🧹 I asked ClawSweeper to review this item again. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8608df2c36
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
/review Fresh ClawSweeper review requested for current head |
|
🦞🧹 I asked ClawSweeper to review this item again. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b5d94be746
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…final-delivery-guard # Conflicts: # extensions/qa-lab/src/providers/mock-openai/server.test.ts # extensions/qa-lab/src/scenario-catalog.test.ts # qa/scenarios/channels/message-tool-stranded-final-reply.yaml # src/agents/system-prompt.ts # src/auto-reply/reply/agent-runner.ts # src/auto-reply/reply/followup-runner.test.ts # src/auto-reply/reply/followup-runner.ts # src/auto-reply/reply/queue/drain.ts # src/auto-reply/reply/queue/enqueue.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aabe68fb81
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| selectDropIndex: (items) => { | ||
| const unprotectedIndex = items.findIndex((item) => item.protectFromQueueOverflow !== true); | ||
| return unprotectedIndex; |
There was a problem hiding this comment.
Avoid debouncing on rejected overflow items
When the queue is full of overflow-protected retries (for example cap: 1 with the stranded-reply retry queued), this selector returns -1 so the incoming normal message is rejected, but enqueueFollowupRun has already refreshed queue.lastEnqueuedAt before calling applyQueueDropPolicy. scheduleFollowupDrain waits on that timestamp before draining the protected priority item, so a busy chat that keeps sending rejected messages within the debounce window can indefinitely postpone the recovery retry and leave the stranded answer unrecovered; only update the debounce timestamp after shouldEnqueue succeeds or roll it back on this protected-overflow rejection.
Useful? React with 👍 / 👎.
| const isStrandedReply = | ||
| !isRoomEvent && | ||
| shouldWarnAboutPrivateMessageToolFinal({ |
There was a problem hiding this comment.
Skip heartbeat turns in stranded reply recovery
When a heartbeat response-tool run falls back to ordinary final text, heartbeat-runner already treats the getReplyFromConfig payload as the heartbeat notification and sends it with sendDurableMessageBatch (src/infra/heartbeat-runner.ts:2073-2277). This new predicate only excludes room events, so the same heartbeat final in sourceReplyDeliveryMode: "message_tool_only" also enqueues a follow-up retry that asks the model to call message(action=send), producing a duplicate source-chat message after the heartbeat notification; exclude opts.isHeartbeat from stranded-reply recovery here.
Useful? React with 👍 / 👎.
The merge kept this PR's strengthened messageToolOnly guidance ('you MUST
call message(action=send)'); the whole-1 shard's prepare test still
asserted main's older phrasing.
# Conflicts: # src/utils/queue-helpers.ts
…, exclude heartbeats from stranded recovery
|
Maintainer pre-merge verification (landing now): Work done on top of the contributor branch
Proof
Known gaps
|
…nclaw#99536) In messages.visibleReplies "message_tool" sessions, a successful agent turn that produced a substantive private final without calling message(action=send) previously left the user with silence and only an operator log. The gateway now enqueues one protected front-of-queue retry prompting the model to deliver the reply, and falls back to a sanitized visible diagnostic when the retry cannot be enqueued or also strands. Queue overflow protection is unified with the in-flight-aware drop policy (skip in-flight or protected items, reject when nothing is droppable), rejected overflow no longer refreshes the drain debounce, heartbeat turns are excluded from recovery, and recovery retries no longer share the client turn's queued-turn lifecycle. Fixes openclaw#85714 Thanks to Eva (@100yenadmin) for the contribution.
What Problem This Solves
Fixes #85714 for the direct/source-channel failure where an agent turn can finish successfully with a substantive final answer, but the user sees nothing because the channel is in
message_tool_onlyvisible-reply mode and the model skippedmessage(action=send).This supersedes #88992. I kept the useful shape from that PR: substantive-final detection, room-event and send-policy guards, a one-shot retry marker, message-tool retry, collect isolation, deferred drain, QA coverage, and clearer prompt wording. The replacement removes the new
messages.strandedReplyRecoveryopt-in surface so guarded recovery is the default for the specific stranded-final class.Why This Change Was Made
The July 3 Telegram repro is not a broad final-fallback problem; it is a silent-success problem. In
message_tool_only, final assistant text is intentionally private, but a substantive final with no message-tool delivery should not end the user-visible source turn as if everything succeeded.The fix now:
room_event,sendPolicyDenied, successful source delivery, short/silent finals, and explicit silence;stranded-reply-retry;message(action=send);User Impact
For users in Telegram/source-channel chats configured for tool-only visible replies, a substantive reply that previously disappeared should now either:
message(action=send)retry, orI generated a reply but could not deliver it to this chat. Please try again.Intentional silence remains silent. Hidden reasoning, tool output, trace text, raw private finals, send-policy-denied text, room events, and short notes are not auto-published.
Evidence
Local evidence packet:
/Volumes/LEXAR/Codex/reports/openclaw-85714-stranded-final-recovery/2026-07-03/evidence.mdCurrent head:
7b2eefe38678b1c79a3b9189f6a60ccfabfe2f20.89bc1fc82c30868b5d5336e62927a24c1a5dbd8e;7b2eefe386is an empty CI retrigger commit after an unrelated QA Smoke timeout.153fed790a5c0a8dda7a30a055f1d8203937e0e1.Focused local validation:
git diff --check pnpm exec oxlint src/auto-reply/reply/agent-runner.ts src/auto-reply/reply/followup-runner.ts src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts src/auto-reply/reply/followup-runner.test.ts node scripts/run-vitest.mjs src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts src/auto-reply/reply/followup-runner.test.ts node scripts/run-vitest.mjs src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts src/auto-reply/reply/private-message-tool-final.test.ts src/auto-reply/reply/queue.collect.test.ts src/auto-reply/reply/followup-runner.test.ts extensions/qa-lab/src/providers/mock-openai/server.test.ts extensions/qa-lab/src/scenario-catalog.test.tsResult: passed. The focused followup/agent wrapper passed
167tests, and the broader focused wrapper passed387tests after the final privacy/send-policy fixes.Mock QA scenarios:
Results:
recovered=1,stranded turns=1,retry delivery turns=1,WARN logged=true; report.artifacts/qa-e2e/suite-mr52q6gz-8ca15370/qa-suite-report.md.diagnostic=1,rawOutbound=0,retryRequests=1; report.artifacts/qa-e2e/suite-mr52s43q-56f9d6e6/qa-suite-report.md.GitHub Actions:
https://github.com/openclaw/openclaw/actions/runs/28669985255passed.6m51s.0failing checks; the only long-running status was the optional Mantis Telegram Desktop Proof.https://github.com/openclaw/openclaw/actions/runs/28670361228was still running live Telegram capture during the final local polling window; it is not blockingmergeStateStatus=CLEAN.Review and bot gates:
MERGEABLE,CLEAN, cross-repo, and maintainer edits are enabled.16, unresolved0, outdated8, current actionable0.https://github.com/openclaw/openclaw/pull/99536#issuecomment-4876442214; markersha=7b2eefe38678b1c79a3b9189f6a60ccfabfe2f20, verdictneeds-human, confidencehigh, conclusion ready for maintainer review with no concrete contributor-facing blocker.https://github.com/openclaw/clawsweeper/actions/runs/28670627111completed successfully after requesthttps://github.com/openclaw/openclaw/pull/99536#issuecomment-4877744029; no stale CI note remains.Remote false-red disposition:
89bc1fc82cwas isolated to unrelatednative-command-session-targettimeout.native command reply=Agent was aborted.; recovery reply=QA-NATIVE-STOP-RECOVERY-OK; report.artifacts/qa-e2e/suite-mr539y5a-3b54c528/qa-suite-report.md.Codex protocol contract checked in sibling source:
/Volumes/LEXAR/repos/codex/codex-rs/protocol/src/protocol.rs:1265-1268maps turn completion totask_complete/turn_complete./Volumes/LEXAR/repos/codex/codex-rs/protocol/src/protocol.rs:1941-1944definesTurnCompleteEvent.last_agent_message./Volumes/LEXAR/repos/codex/codex-rs/core/src/session/turn.rs:1806-1819and:2042-2044populatelast_agent_message./Volumes/LEXAR/repos/codex/codex-rs/core/src/tasks/mod.rs:745-752emits it on turn completion.