Skip to content

Recover stranded message-tool finals by default#99536

Merged
obviyus merged 16 commits into
openclaw:mainfrom
100yenadmin:fix/85714-stranded-final-delivery-guard
Jul 10, 2026
Merged

Recover stranded message-tool finals by default#99536
obviyus merged 16 commits into
openclaw:mainfrom
100yenadmin:fix/85714-stranded-final-delivery-guard

Conversation

@100yenadmin

@100yenadmin 100yenadmin commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

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_only visible-reply mode and the model skipped message(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.strandedReplyRecovery opt-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:

  • warns only through the existing substantive private-final predicate;
  • skips room_event, sendPolicyDenied, successful source delivery, short/silent finals, and explicit silence;
  • enqueues exactly one retry marked stranded-reply-retry;
  • applies that recovery to both direct source turns and queued followup turns;
  • prepends that retry ahead of already queued follow-ups, protects it from old-item overflow eviction, drains it before overflow summaries, and uses the refreshed follow-up runner;
  • stores retry identity in private queued-run metadata rather than user-controlled summary text;
  • forces that retry out of collect batching and suppresses retry prompt persistence;
  • asks the model to deliver the sanitized visible reply via message(action=send);
  • if enqueueing fails or the retry strands again, delivers only a marked sanitized diagnostic;
  • suppresses that diagnostic only when the retry already delivered to the source chat through the message tool or internal source-reply path;
  • never marks or auto-delivers the original private final text.

User Impact

For users in Telegram/source-channel chats configured for tool-only visible replies, a substantive reply that previously disappeared should now either:

  • recover through one visible message(action=send) retry, or
  • show I 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.md

Current head:

  • PR head: 7b2eefe38678b1c79a3b9189f6a60ccfabfe2f20.
  • Code head: 89bc1fc82c30868b5d5336e62927a24c1a5dbd8e; 7b2eefe386 is an empty CI retrigger commit after an unrelated QA Smoke timeout.
  • Base: 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.ts

Result: passed. The focused followup/agent wrapper passed 167 tests, and the broader focused wrapper passed 387 tests after the final privacy/send-policy fixes.

Mock QA scenarios:

node scripts/run-node.mjs qa suite --provider-mode mock-openai --scenario message-tool-stranded-final-reply --concurrency 1 --fast
node scripts/run-node.mjs qa suite --provider-mode mock-openai --scenario message-tool-stranded-final-retry-failure --concurrency 1 --fast

Results:

  • Recovery scenario passed: recovered=1, stranded turns=1, retry delivery turns=1, WARN logged=true; report .artifacts/qa-e2e/suite-mr52q6gz-8ca15370/qa-suite-report.md.
  • Retry-failure scenario passed: diagnostic=1, rawOutbound=0, retryRequests=1; report .artifacts/qa-e2e/suite-mr52s43q-56f9d6e6/qa-suite-report.md.

GitHub Actions:

  • Current-head CI run https://github.com/openclaw/openclaw/actions/runs/28669985255 passed.
  • QA Smoke CI passed in 6m51s.
  • Current PR rollup at final body refresh had 0 failing checks; the only long-running status was the optional Mantis Telegram Desktop Proof.
  • Optional Mantis Telegram Desktop Proof https://github.com/openclaw/openclaw/actions/runs/28670361228 was still running live Telegram capture during the final local polling window; it is not blocking mergeStateStatus=CLEAN.

Review and bot gates:

  • PR is open, non-draft, MERGEABLE, CLEAN, cross-repo, and maintainer edits are enabled.
  • Current-head review threads: total 16, unresolved 0, outdated 8, current actionable 0.
  • ClawSweeper exact-head durable review comment: https://github.com/openclaw/openclaw/pull/99536#issuecomment-4876442214; marker sha=7b2eefe38678b1c79a3b9189f6a60ccfabfe2f20, verdict needs-human, confidence high, conclusion ready for maintainer review with no concrete contributor-facing blocker.
  • Final post-CI ClawSweeper run https://github.com/openclaw/clawsweeper/actions/runs/28670627111 completed successfully after request https://github.com/openclaw/openclaw/pull/99536#issuecomment-4877744029; no stale CI note remains.

Remote false-red disposition:

  • Previous QA Smoke failure on 89bc1fc82c was isolated to unrelated native-command-session-target timeout.
  • Local isolated rerun passed: native command reply=Agent was aborted.; recovery reply=QA-NATIVE-STOP-RECOVERY-OK; report .artifacts/qa-e2e/suite-mr539y5a-3b54c528/qa-suite-report.md.
  • The retriggered current-head QA Smoke CI passed.

Codex protocol contract checked in sibling source:

  • /Volumes/LEXAR/repos/codex/codex-rs/protocol/src/protocol.rs:1265-1268 maps turn completion to task_complete / turn_complete.
  • /Volumes/LEXAR/repos/codex/codex-rs/protocol/src/protocol.rs:1941-1944 defines TurnCompleteEvent.last_agent_message.
  • /Volumes/LEXAR/repos/codex/codex-rs/core/src/session/turn.rs:1806-1819 and :2042-2044 populate last_agent_message.
  • /Volumes/LEXAR/repos/codex/codex-rs/core/src/tasks/mod.rs:745-752 emits it on turn completion.

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation gateway Gateway runtime agents Agent runtime and tooling extensions: qa-lab size: L labels Jul 3, 2026
@clawsweeper

clawsweeper Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 10, 2026, 3:28 AM ET / 07:28 UTC.

Summary
The PR adds one guarded source-bound retry for substantive undelivered message_tool_only finals across direct and queued turns, with priority queue handling, sanitized failure diagnostics, prompt and docs updates, and QA scenarios.

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 message_tool_only, and current main has no recovery path for that private-final class.

Review metrics: 2 noteworthy metrics.

  • Default behavior surfaces: 1 existing mode changed, 0 new config options. Existing tool-only users gain an automatic retry and failure diagnostic without an upgrade switch.
  • Shared queue metadata: 3 runtime-only follow-up flags added. Retry identity, individual collect drain, and overflow protection extend generic queue semantics.

Stored data model
Persistent data-model change detected: serialized state: extensions/qa-lab/src/scenario-catalog.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #85714
Summary: This PR is a viable candidate fix for the canonical issue; the older open PR is a competing implementation, and no merged proof-positive replacement currently supersedes it.

Members:

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

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦐 gold shrimp
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

  • [P1] Add redacted current-head Telegram proof for successful recovery and retry exhaustion.
  • Obtain explicit maintainer approval for default-on recovery semantics.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The current-head evidence is deterministic mock-openai and qa-channel runtime output plus tests and CI; before merge, add a redacted real Telegram recording or transcript showing exactly-once recovery and sanitized retry exhaustion. Updating the PR body should trigger review; otherwise ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Mantis proof suggestion
Mantis can supply the missing real-channel proof without modifying code or GitHub state. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis telegram desktop proof: verify on the current head that a stranded tool-only final is recovered exactly once, and retry exhaustion shows only the sanitized diagnostic without raw private text.

Risk before merge

  • [P1] Existing message_tool_only lanes change from strict no-call/no-source-reply behavior to an automatic second model turn and a visible diagnostic on failure.
  • [P1] The patch extends shared follow-up queue ordering and overflow semantics, so maintainers must accept its broader delivery-ordering surface despite green CI.
  • [P1] The retry reintroduces sanitized withheld final text to the model; the current guards prevent direct raw publication, but this privacy behavior needs explicit maintainer signoff.

Maintainer options:

  1. Approve the hardened default (recommended)
    Accept the exact behavior after current-head Telegram proof confirms exactly-once recovery and no raw private-final exposure.
  2. Make recovery opt-in
    Preserve strict upgrade semantics and add proof for both disabled-default and enabled-recovery modes.
  3. Pause the behavior change
    Keep the current warning-only contract if maintainers do not want automatic model retry or visible diagnostics in tool-only mode.

Next step before merge

  • [P1] The implementation has no remaining concrete code finding; maintainers must decide the default behavior, and the contributor must add current-head real Telegram proof before merge.

Maintainer decision needed

  • Question: Should existing message_tool_only lanes automatically run one guarded recovery retry and show a sanitized failure diagnostic when a substantive final is withheld?
  • Rationale: The implementation appears correct, but it intentionally changes the public semantics of an opt-in strict delivery mode and adds a visible fallback without a new configuration switch.
  • Likely owner: steipete — They have the strongest long-term history in the affected runtime and merged the warning predicate defining the guarded stranded-final class.
  • Options:
    • Approve guarded default recovery (recommended): Keep the private final suppressed, retry once through message(action=send) to the current source, and show only the sanitized diagnostic on failure.
    • Require explicit opt-in: Preserve current strict behavior for upgrades and expose stranded-final recovery through a reviewed configuration surface.
    • Retain strict silence: Keep warning-only behavior and rely on stronger prompting or operator diagnostics rather than automatic recovery.

Security
Cleared: No dependency, workflow, secret, permission, or supply-chain regression was found; the privacy-sensitive runtime behavior is captured as merge risk.

Review details

Best 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 message_tool_only, and current main has no recovery path for that private-final class.

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 changes

Label changes:

  • add merge-risk: 🚨 compatibility: Existing message_tool_only configurations gain automatic retry and diagnostic behavior without opting in.
  • add merge-risk: 🚨 message-delivery: The patch changes source-delivery evidence, follow-up ordering, overflow admission, and retry routing.
  • add merge-risk: 🚨 security-boundary: The recovery turn handles withheld private final text and must prevent raw, hidden, trace, or send-policy-denied content from becoming visible.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The current-head evidence is deterministic mock-openai and qa-channel runtime output plus tests and CI; before merge, add a redacted real Telegram recording or transcript showing exactly-once recovery and sanitized retry exhaustion. Updating the PR body should trigger review; otherwise ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • add mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The exactly-once recovered reply and sanitized retry-failure diagnostic are directly visible Telegram behavior suited to a short Desktop recording.

Label justifications:

  • P1: A completed substantive source-channel reply can silently disappear from the user’s conversation.
  • merge-risk: 🚨 compatibility: Existing message_tool_only configurations gain automatic retry and diagnostic behavior without opting in.
  • merge-risk: 🚨 message-delivery: The patch changes source-delivery evidence, follow-up ordering, overflow admission, and retry routing.
  • merge-risk: 🚨 security-boundary: The recovery turn handles withheld private final text and must prevent raw, hidden, trace, or send-policy-denied content from becoming visible.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The current-head evidence is deterministic mock-openai and qa-channel runtime output plus tests and CI; before merge, add a redacted real Telegram recording or transcript showing exactly-once recovery and sanitized retry exhaustion. Updating the PR body should trigger review; otherwise ask a maintainer to comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The exactly-once recovered reply and sanitized retry-failure diagnostic are directly visible Telegram behavior suited to a short Desktop recording.
Evidence reviewed

PR surface:

Source +450, Tests +1082, Docs 0, Other +130. Total +1662 across 22 files.

View PR surface stats
Area Files Added Removed Net
Source 10 473 23 +450
Tests 9 1108 26 +1082
Docs 1 4 4 0
Config 0 0 0 0
Generated 0 0 0 0
Other 2 172 42 +130
Total 22 1757 95 +1662

What I checked:

  • Current-main behavior: Current main deliberately excludes hidden message_tool_only finals from durable pending-final delivery, leaving the reported stranded-final class without recovery. (src/auto-reply/reply/agent-runner.ts:2611, 27ceb13e219f)
  • Direct recovery path: The exact head uses normalized visible assistant text, excludes heartbeat and room-event turns, requires no committed source delivery, enqueues one front-position retry, and falls back to a sanitized marked diagnostic if admission fails. (src/auto-reply/reply/agent-runner.ts:2624, aeba21694a40)
  • Retry privacy and lifecycle: The synthetic recovery turn clears transcript persistence and inbound context, owns no client queued-turn lifecycle, disables collect batching, and preserves the original source delivery mode. (src/auto-reply/reply/stranded-reply-recovery.ts:28, aeba21694a40)
  • Codex completion contract: Codex emits task_complete or turn_complete with optional last_agent_message, confirming that a turn can complete with substantive final text even when no OpenClaw message-tool delivery occurred. (../codex/codex-rs/protocol/src/protocol.rs:1329, 1f0566d3f592)
  • Current-head hardening: The July 10 repairs exclude heartbeat duplication, avoid rejected-item debounce starvation, unify protected and in-flight overflow handling, and detach the retry from the parent queued lifecycle; the remaining GitHub threads target fixed code. (src/auto-reply/reply/queue/enqueue.ts:151, 11ac9f4a9730)
  • Live PR state: GitHub reports the current head as mergeable and behind main with green current-head CI; stale base drift requires refresh but does not establish a merged-code regression. (aeba21694a40)

Likely related people:

  • steipete: Has the strongest historical contribution signal across the core agent-runner and follow-up paths and merged the substantive private-final warning foundation this PR extends. (role: long-term area contributor and merger; confidence: high; commits: 75e0053cf969; files: src/auto-reply/reply/agent-runner.ts, src/auto-reply/reply/followup-runner.ts, src/auto-reply/reply/private-message-tool-final.ts)
  • obviyus: Authored the exact-head heartbeat, overflow, debounce, and lifecycle hardening commits after integrating newer main. (role: recent reviewer and repair contributor; confidence: high; commits: b7a707e654cd, 11ac9f4a9730, aeba21694a40; files: src/auto-reply/reply/agent-runner.ts, src/auto-reply/reply/followup-runner.ts, src/auto-reply/reply/queue/enqueue.ts)
  • tzy-17: Recently fixed in-flight overflow victim selection in the shared queue helper that this PR extends with protected priority runs. (role: recent adjacent queue contributor; confidence: medium; commits: 2f7e2aee1584; files: src/auto-reply/reply/queue/enqueue.ts, src/utils/queue-helpers.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 (1 earlier review cycle)
  • reviewed 2026-07-03T16:07:45.344Z sha 7b2eefe :: needs maintainer review before merge. :: none

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/auto-reply/reply/followup-runner.ts
Comment thread src/auto-reply/reply/agent-runner.ts
Comment thread src/auto-reply/reply/agent-runner.ts Outdated
@100yenadmin

Copy link
Copy Markdown
Contributor Author

/review

Fresh ClawSweeper review requested for current head a6f6f4d160bdcea9f675974ed1eadb628e80d7de after addressing the Codex review threads.

@clawsweeper

clawsweeper Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/auto-reply/reply/agent-runner.ts Outdated
Comment thread src/auto-reply/reply/queue/enqueue.ts
Comment thread src/auto-reply/reply/followup-runner.ts Outdated
Comment thread src/auto-reply/reply/agent-runner.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/auto-reply/reply/agent-runner.ts Outdated
Comment thread src/auto-reply/reply/queue/enqueue.ts Outdated
Comment thread src/auto-reply/reply/agent-runner.ts Outdated
Comment thread src/auto-reply/reply/queue/enqueue.ts
@100yenadmin

Copy link
Copy Markdown
Contributor Author

/review

Fresh ClawSweeper review requested for current head 8608df2c3695a0c6fee33609a1f0b3c9fd01db42 after addressing the final Codex review threads and passing current-head CI.

@clawsweeper

clawsweeper Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/utils/queue-helpers.ts Outdated
Comment thread src/auto-reply/reply/agent-runner.ts Outdated
@100yenadmin

Copy link
Copy Markdown
Contributor Author

/review

Fresh ClawSweeper review requested for current head b5d94be7466d18b7b539abf11f66b633ae193c45 after addressing the latest Codex review threads and passing current-head CI.

@clawsweeper

clawsweeper Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/auto-reply/reply/followup-runner.ts
Comment thread src/auto-reply/reply/followup-runner.ts
…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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/auto-reply/reply/queue/enqueue.ts Outdated
Comment on lines +150 to +152
selectDropIndex: (items) => {
const unprotectedIndex = items.findIndex((item) => item.protectFromQueueOverflow !== true);
return unprotectedIndex;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment on lines +2637 to 2639
const isStrandedReply =
!isRoomEvent &&
shouldWarnAboutPrivateMessageToolFinal({

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Eva and others added 4 commits July 10, 2026 08:25
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
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. 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. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. labels Jul 10, 2026
@obviyus

obviyus commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Maintainer pre-merge verification (landing now):

Work done on top of the contributor branch

  • Merged current main, unifying applyQueueDropPolicy with fix(queue): prevent applyQueueDropPolicy from selecting in-flight items as overflow victims #103284's in-flight awareness: one eviction filter (skip in-flight OR overflow-protected), all-or-nothing victim collection, incoming rejected when the queue holds only undroppable work.
  • Debounce stamp (lastEnqueuedAt) now advances only on admitted enqueues, so rejected overflow can no longer starve the protected stranded-reply retry.
  • Heartbeat runs are excluded from stranded recovery on both the direct and followup paths (heartbeat-runner already delivers those finals via sendDurableMessageBatch).
  • Retry prompt/run construction deduplicated into stranded-reply-recovery.ts; the retry no longer inherits the client turn's one-shot queuedLifecycle, which previously freed the gateway cancel identity while the recovery turn was still pending.

Proof

  • Blacksmith Testbox tbx_01kx5c5az0rvmfytt20efkfsjw @ aeba21694a4: full pnpm check + pnpm test src/auto-reply/reply src/utils/queue-helpers.test.ts → 141 files / 2,932 tests passed.
  • Local tsgo:core + tsgo:core:test clean; focused suites (queue-helpers 16, queue dir 22, queue.collect 85, agent-runner misc 73, followup-runner 141) green.
  • Independent structured review (grok-4.5 high, full origin/main...HEAD bundle): clean after the lifecycle fix.

Known gaps

  • No live Telegram/Mantis roundtrip on the final head; recovery behavior is proven via unit + mock QA scenarios. Default-on recovery is accepted as the product decision by this landing.

@obviyus
obviyus merged commit ae63a48 into openclaw:main Jul 10, 2026
140 of 148 checks passed
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 11, 2026
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling docs Improvements or additions to documentation extensions: qa-lab gateway Gateway runtime 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. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. P1 High-priority user-facing bug, regression, or broken workflow. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. size: XL status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Agent's final agent_message stranded when LLM forgets to call configured delivery tool (no fallback delivery from task_complete)

2 participants