fix(auto-reply): hide recovered failed tool progress#97135
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 19, 2026, 12:09 AM ET / 04:09 UTC. Summary PR surface: Source +499, Tests +90. Total +589 across 4 files. Reproducibility: yes. for the underlying behavior: the supplied Telegram observation and the shared-dispatch runtime scenario show recovered failed progress followed by a successful final reply. I did not execute the reproduction because this review environment could not start read-only shell inspection. Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Have the auto-reply delivery owner explicitly choose whether regular verbose should suppress recovered failures by default; if accepted, retain this confirmed-delivery buffering with its direct and queued regressions, otherwise keep the existing default and pursue any alternative only through an explicit policy/config design. Do we have a high-confidence way to reproduce the issue? Yes for the underlying behavior: the supplied Telegram observation and the shared-dispatch runtime scenario show recovered failed progress followed by a successful final reply. I did not execute the reproduction because this review environment could not start read-only shell inspection. Is this the best way to solve the issue? Unclear: the confirmed-delivery buffering is the narrowest implementation for the proposed UX, but whether it should become the unconditional regular-verbose default needs maintainer policy approval rather than another mechanical patch. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against c684b1321333. Label changesLabel justifications:
Evidence reviewedPR surface: Source +499, Tests +90. Total +589 across 4 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 (12 earlier review cycles; latest 8 shown)
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
|
@clawsweeper re-review |
|
@clawsweeper re-review |
|
@clawsweeper re-review |
|
Adding one real Telegram direct-chat observation of the user-visible problem this PR is meant to address. This is not after-fix proof for this PR branch; it is current-release evidence that the behavior happens in a real Telegram conversation. Environment:
Observed behavior: Redacted examples of user-visible messages: In both cases, the warning made the successful turn look broken to the user even though the agent either recovered in the same turn or had already reported the completed work successfully. This is not a new code finding from me, just additional live Telegram evidence for the behavior this PR describes: recovered failed helper/tool progress can still appear as user-visible standalone Telegram messages after a successful final response. Why this evidence may help:
I have not attached screenshots here because they include private chat context, but the examples above are redacted from the actual visible Telegram messages. |
|
@clawsweeper re-review Addressed the exact-head same-channel final settlement finding:
Validation for
Sanitized AWS Crabbox remains unavailable because the local installation has no broker credentials, so the documented narrow local Node fallback was used and fork CI is running. The two prior QA Smoke failures were independently reproduced on |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
# Conflicts: # src/auto-reply/reply/dispatch-from-config.test.ts # src/auto-reply/reply/dispatch-from-config.ts # src/auto-reply/reply/followup-runner.ts
What Problem This Solves
Recovered helper/tool failures could leak as visible failed progress in regular verbose auto-reply delivery even when the turn later produced a successful final reply. This made a recovered chat turn look failed. The PR hides those recovered failed progress events in regular verbose mode while preserving full-verbose diagnostics and terminal failure policy.
Evidence
Runtime proof was captured on PR head
076f03256a30d164453efbade434e395e72e6383withOPENCLAW_PROOF_HEAD=$(git rev-parse HEAD) node --import tsx /tmp/openclaw-pr97135-proof.ts. The script used a disposable OpenClaw home/session store, imported the realdispatchReplyFromConfigentrypoint, and ran the actual dispatch path twice.Observed after-fix runtime result: with
verboseLevel: "on", failed command/item/tool-result progress producedcommandOutput: 0,itemEvents: 0,toolResultCallback: 0,tool: 0, and still deliveredfinal: 1. WithverboseLevel: "full", diagnostics remained visible withcommandOutput: 1,itemEvents: 1,toolResultCallback: 1,tool: 1, andfinal: 1.Focused validation also passed:
pnpm test -- --run src/auto-reply/reply/dispatch-from-config.test.ts src/auto-reply/reply/followup-runner.test.ts,pnpm exec oxfmt --check --threads=1 ..., andgit diff --check.Summary
Suppress recovered failed tool progress from regular verbose delivery while keeping full verbose diagnostics visible.
onCommandOutput, failed item events, and failed tool-result progress in regular verbose mode.verbose=fulland for forced tool progress.Why
A transient helper/tool failure can be recovered by a later successful tool call or final assistant response. Showing the failed progress immediately makes the chat look like the turn failed even when the agent recovered and produced a valid answer.
This keeps the actual error classification intact: tool failures still remain in the run state/transcript, and terminal tool-error warnings remain available when the failure is not recovered. The change only gates user-visible progress delivery for ordinary verbose mode.
Real behavior proof
Behavior addressed: in regular verbose delivery, a failed helper command/item/tool-result progress event could be shown to the user immediately even when the agent later recovered and produced a successful final reply. This made a recovered turn look failed.
Real environment tested: local OpenClaw source checkout on macOS, PR head
076f03256a30d164453efbade434e395e72e6383, Nodev24.13.1, using a disposable OpenClaw home/session store under/tmp. This is a direct runtime proof, not a Vitest assertion: the proof script imports the PR-headdispatchReplyFromConfigentrypoint and lets the real dispatch pipeline call the dispatcher and verbose progress callbacks.Exact command run after this patch:
OPENCLAW_PROOF_HEAD=$(git rev-parse HEAD) node --import tsx /tmp/openclaw-pr97135-proof.tsRuntime proof script shape:
OPENCLAW_HOMEand per-scenario session store.dispatchReplyFromConfigtwice through the real runtime dispatch path.verboseLevel: "on", emits failedonCommandOutput, failedonItemEvent, failedonToolResult, then returns a normal final reply.verboseLevel: "full"with the same failed progress and final reply.Observed runtime output:
{ "marker": "PR97135-RUNTIME-PROOF-2026-06-27T14:36:02.903Z", "head": "076f03256a30d164453efbade434e395e72e6383", "proofRoot": "/var/folders/r8/x37z43b52tv63kn_ss3571t40000gp/T/openclaw-pr97135-proof-QY2c1J", "scenarios": [ { "verboseLevel": "on", "dispatchResult": { "queuedFinal": true, "counts": { "tool": 0, "block": 0, "final": 1 } }, "callbackCounts": { "commandOutput": 0, "itemEvents": 0, "toolResultCallback": 0 }, "callbacks": { "commandOutput": [], "itemEvents": [], "toolResultCallback": [] }, "sentCounts": { "tool": 0, "block": 0, "final": 1 }, "sent": [ { "kind": "final", "payload": { "text": "PR97135 on final reply delivered" } } ] }, { "verboseLevel": "full", "dispatchResult": { "queuedFinal": true, "counts": { "tool": 1, "block": 0, "final": 1 } }, "callbackCounts": { "commandOutput": 1, "itemEvents": 1, "toolResultCallback": 1 }, "callbacks": { "commandOutput": [ { "phase": "end", "title": "Exec", "name": "exec", "status": "failed", "exitCode": 1 } ], "itemEvents": [ { "itemId": "proof-item-full", "kind": "tool", "name": "exec", "status": "failed" } ], "toolResultCallback": [ { "text": "PR97135 full recovered helper failure output", "isError": true } ] }, "sentCounts": { "tool": 1, "block": 0, "final": 1 }, "sent": [ { "kind": "tool", "payload": { "text": "PR97135 full recovered helper failure output", "isError": true } }, { "kind": "final", "payload": { "text": "PR97135 full final reply delivered" } } ] } ] }Observed result after fix: regular
verbose=onrecovered failed progress produced no user-visible tool send and no progress callback delivery, while the final reply was still delivered (final: 1). Full verbose preserved the failed command output, item event, and tool-result diagnostic callbacks, and delivered both the failed tool payload and final reply.Additional focused validation:
pnpm test -- --run src/auto-reply/reply/dispatch-from-config.test.ts src/auto-reply/reply/followup-runner.test.tspnpm exec oxfmt --check --threads=1 src/auto-reply/reply/dispatch-from-config.ts src/auto-reply/reply/dispatch-from-config.test.ts src/auto-reply/reply/followup-runner.ts src/auto-reply/reply/followup-runner.test.tsgit diff --checkWhat was not tested: a native Telegram Desktop/Mantis recording. The proof above exercises the shared dispatch layer that feeds channel delivery; channel adapters are not changed by this PR.
Tests
pnpm test -- --run src/auto-reply/reply/dispatch-from-config.test.ts src/auto-reply/reply/followup-runner.test.tspnpm exec oxfmt --check --threads=1 src/auto-reply/reply/dispatch-from-config.ts src/auto-reply/reply/dispatch-from-config.test.ts src/auto-reply/reply/followup-runner.ts src/auto-reply/reply/followup-runner.test.tsgit diff --checkNotes
Full repository
tscwas attempted with the default heap and withNODE_OPTIONS=--max-old-space-size=8192; both runs aborted with Node heap OOM before reporting TypeScript diagnostics.