Skip to content

fix(discord): preserve streamed replies after tool warnings#83844

Closed
neeravmakwana wants to merge 1 commit into
openclaw:mainfrom
neeravmakwana:fix-discord-tool-warning-preview
Closed

fix(discord): preserve streamed replies after tool warnings#83844
neeravmakwana wants to merge 1 commit into
openclaw:mainfrom
neeravmakwana:fix-discord-tool-warning-preview

Conversation

@neeravmakwana

@neeravmakwana neeravmakwana commented May 19, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Preserve an already-started non-error Discord final reply path when a later isError tool-warning final is delivered in the same turn.
  • Add regression coverage for Discord partial streaming where delivery survived remains in the finalized preview and the warning is sent separately.

Root cause

Discord partial streaming reused the live draft finalizer for every final payload. When Pi produced a normal assistant final followed by an isError tool-warning final, the warning could enter the draft fallback path and call draftStream.clear(), deleting the finalized preview message that held the assistant answer.

Fixes #83831.

Why this is safe

The change is scoped to Discord message delivery for later error final payloads after a non-error final delivery has already started. Standalone error finals still use the existing fallback/cleanup path, and normal preview finalization behavior is unchanged for the first assistant final.

Security/runtime controls are unchanged: this does not alter prompt text, model behavior, tool execution, warning generation, permissions, channel routing, or Discord auth. The preservation decision is enforced in runtime delivery state.

Real behavior proof

Behavior addressed: Discord partial-stream final answer should remain visible when a later tool-warning final is delivered.

Real environment tested: local Discord message-handler runtime test exercising the same partial-stream delivery lifecycle with mocked Discord transport; live Discord/Pi credentials were not used in this PR.

Exact steps or command run after this patch: node scripts/run-vitest.mjs extensions/discord/src/monitor/message-handler.process.test.ts --testNamePattern "keeps finalized previews when later tool warning finals are delivered"

Evidence after fix: the targeted test passes with 1 passed | 69 skipped, and the test asserts the preview edit contains delivery survived, draftStream.clear() is not called, the draft message id remains preview-1, and the warning payload is delivered separately.

Observed result after fix: the finalized preview remains attached while the tool-warning final is sent through normal Discord delivery.

What was not tested: live Discord guild transport with Pi runtime credentials was not run here.

Related before-fix log: with only the production guard removed, the same targeted test fails at expect(draftStream.clear).not.toHaveBeenCalled() because draftStream.clear() is called once.

Verification

  • git diff --check
  • node scripts/run-vitest.mjs extensions/discord/src/monitor/message-handler.process.test.ts
  • node scripts/run-vitest.mjs extensions/discord/src/monitor/message-handler.process.test.ts src/channels/message/lifecycle.test.ts
  • pnpm check:changed

Out of scope

  • Changing Pi tool-warning generation or suppression policy.
  • Changing Discord streaming modes, default config, auth, or routing.
  • Adding live Discord credentialed E2E coverage in this small fix.

Made with Cursor

@neeravmakwana neeravmakwana mentioned this pull request May 19, 2026
Closed
@openclaw-barnacle openclaw-barnacle Bot added channel: discord Channel integration: discord size: XS triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. labels May 19, 2026
@clawsweeper

clawsweeper Bot commented May 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

Workflow note: Future ClawSweeper reviews update this same comment in place.

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.

Summary
The branch adds a Discord partial-stream guard and regression test so an isError tool-warning final delivered after a non-error final does not clear the already-finalized preview.

Reproducibility: yes. Source inspection on current main shows later error finals still enter the draft-preview finalizer and the shared finalizer clears the draft after fallback delivery; the linked issue and PR comments provide the real Discord symptom.

PR rating
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Summary: Focused bug fix with source-level cause, regression coverage, and sufficient live Discord proof; remaining risk is ordinary maintainer acceptance for a message-delivery change.

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.

PR egg
✨ Hatched: 🥚 common Pearl Lint Imp

       /\  .---.  /\         
      /  \/     \/  \        
     /   ( -   - )   \       
    |       ._.       |      
    |   /|  ===  |\   |      
     \  \|______/|/  /       
      '._  `--'  _.'         
         '-.__.-'            
       _/|_|  |_|\_          
      /__|      |__\         
       .-----------.         
      '-------------'

Rarity: 🥚 common.
Trait: purrs at green checks.
Share on X: post this hatch
Copy: My PR egg hatched a 🥚 common Pearl Lint Imp in ClawSweeper.

What is this egg doing here?
  • Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
  • The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
  • How to hatch it: reach status: 👀 ready for maintainer look or status: 🚀 automerge armed; that usually means sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness.
  • The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
  • Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

Real behavior proof
Sufficient (live_output): The PR discussion includes after-fix live Discord output and Discord API message state showing a real failed tool call no longer removes the final reply.

Mantis proof suggestion
A short live Discord visual run would be useful independent proof for the message-preservation behavior, even though the contributor’s live output is already sufficient. A maintainer can ask Mantis to capture proof by posting a new PR comment that starts with the OpenClaw Mantis account mention, followed by:

visual task: verify in Discord that a failed tool-call warning does not remove the final `delivery survived` reply.

Risk before merge
Why this matters: - This remains message-delivery-sensitive: the guard changes which later Discord error finals bypass draft-preview cleanup, so exact-head CI or the listed Discord process tests should be green before merge.

Maintainer options:

  1. Merge after exact-head Discord checks (recommended)
    Accept the scoped delivery risk once CI or the listed targeted Discord process tests pass for bbdfb165ed5f512ce947195053ebe9ad07f16e3d.
  2. Ask for independent visual proof
    Request a short live Discord recording if reviewers want extra confirmation that the final reply and warning both remain visible.

Next step before merge
Remaining action is maintainer merge judgment on a message-delivery-sensitive Discord fix; there is no narrow repair for ClawSweeper to queue.

Security
Cleared: The diff only changes Discord delivery state handling and a colocated regression test; it does not touch credentials, permissions, dependencies, CI, scripts, or package resolution.

Review details

Best possible solution:

Land the focused Discord guard and regression test after exact-head checks; optional visual Discord proof can further reduce the remaining delivery-risk uncertainty.

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

Yes. Source inspection on current main shows later error finals still enter the draft-preview finalizer and the shared finalizer clears the draft after fallback delivery; the linked issue and PR comments provide the real Discord symptom.

Is this the best way to solve the issue?

Yes. The patch preserves standalone error-final cleanup while bypassing the finalizer only after a non-error final delivery has started, which is a narrow fix for the observed lifecycle bug.

Label justifications:

  • P1: The PR fixes a Discord workflow where a visible final assistant reply can disappear after a later tool-warning final.
  • merge-risk: 🚨 message-delivery: The patch changes Discord final-payload routing and draft cleanup, which could affect whether replies or warnings are lost, duplicated, or preserved.

Acceptance criteria:

  • node scripts/run-vitest.mjs extensions/discord/src/monitor/message-handler.process.test.ts --testNamePattern "keeps finalized previews when later tool warning finals are delivered"
  • node scripts/run-vitest.mjs extensions/discord/src/monitor/message-handler.process.test.ts src/channels/message/lifecycle.test.ts
  • pnpm check:changed

What I checked:

  • Current-main root cause: Current main sends every final payload with a draft stream through the finalizable preview adapter; for payload.isError, buildFinalEdit returns undefined, so the shared finalizer follows its normal fallback path. (extensions/discord/src/monitor/message-handler.process.ts:553, d75e16a1b920)
  • Shared finalizer cleanup behavior: When a final cannot be edited into the preview, deliverFinalizableLivePreview discards pending draft work, delivers normally, and clears the draft after successful fallback delivery, matching the reported disappearance path. (src/channels/message/live.ts:194, d75e16a1b920)
  • PR implementation: The PR records when non-error final delivery has started and skips draft-preview finalization only for later error finals, leaving standalone error finals on the existing fallback path. (extensions/discord/src/monitor/message-handler.process.ts:551, bbdfb165ed5f)
  • Regression coverage: The new test queues a normal final followed by an isError warning final, asserts the preview edit still contains delivery survived, asserts draftStream.clear() is not called, and asserts the warning is delivered separately. (extensions/discord/src/monitor/message-handler.process.test.ts:2064, bbdfb165ed5f)
  • Live behavior proof: The PR comments include a live Discord turn with a real failed exec tool call and Discord API state for the bot message showing the final delivery survived reply remained visible after the warning path. (bbdfb165ed5f)
  • Formatting sanity: The PR diff has no whitespace errors against current main for the touched Discord files. (bbdfb165ed5f)

Likely related people:

  • steipete: Peter Steinberger has the largest recent commit footprint in the Discord message-handler and preview-delivery area, including explicit-reply delivery changes near this code path. (role: recent area contributor; confidence: high; commits: a32a3e23314d; files: extensions/discord/src/monitor/message-handler.process.ts, extensions/discord/src/monitor/message-handler.process.test.ts)
  • vincentkoc: Vincent Koc recently changed Discord streaming configuration normalization and touched message-handler.process.ts, which is adjacent to this preview-streaming behavior. (role: adjacent streaming contributor; confidence: medium; commits: 0fdf9e874b7e; files: extensions/discord/src/monitor/message-handler.process.ts, extensions/discord/src/preview-streaming.ts)
  • scoootscooob: The Discord monitor implementation and tests were moved into extensions/discord in the commit that introduced this current file location. (role: introduced current extension placement; confidence: medium; commits: 5682ec37fada; files: extensions/discord/src/monitor/message-handler.process.ts, extensions/discord/src/monitor/message-handler.process.test.ts)

Codex review notes: model gpt-5.5, reasoning high; reviewed against d75e16a1b920.

@neeravmakwana neeravmakwana marked this pull request as ready for review May 19, 2026 01:23
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels May 19, 2026
@neeravmakwana

neeravmakwana commented May 19, 2026

Copy link
Copy Markdown
Contributor Author

Additional local proof from this branch:

$ openclaw --version
OpenClaw 2026.5.19 (bbdfb16)

Local install/update path used:

$ pnpm build && pnpm ui:build
[build-all] ... completed
$ ~/.local/bin/openclaw --version
OpenClaw 2026.5.19 (bbdfb16)

Discord token/live API probe used the repo .env value mapped from discord_bot_token to DISCORD_BOT_TOKEN for the command environment only. Token value was not printed.

$ node ... Discord API /users/@me and /gateway/bot probe
Discord live token probe ok: bot=true user=1481…9801 gateway=true maxConcurrency=1

Focused behavior proof for the reported delivery lifecycle:

$ node scripts/run-vitest.mjs extensions/discord/src/monitor/message-handler.process.test.ts --testNamePattern "keeps finalized previews when later tool warning finals are delivered"

Test Files  1 passed (1)
Tests  1 passed | 69 skipped (70)

This proves the fixed runtime preserves the finalized Discord partial-stream preview while delivering the later tool-warning final separately. I did not run a full live Discord channel roundtrip because this environment only had the bot token; no separate user/driver token or target channel smoke configuration was provided.

@neeravmakwana

neeravmakwana commented May 19, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented May 19, 2026 •
edited
Loading

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.

Re-review progress:

@neeravmakwana

neeravmakwana commented May 19, 2026

Copy link
Copy Markdown
Contributor Author

Real behavior proof from a live Discord turn, using the fixed local build.

Environment:

  • Local checkout/build under test: OpenClaw 2026.5.19 (bbdfb16)
  • Gateway entrypoint: ~/Desktop/github_repos/openclaw_repo_prrefresh/dist/entry.js gateway --force
  • Discord bot: OpenClaw Local Dev 20260311
  • Channel: #general (1481298387058950239)

Live Discord proof:

  • Non-bot user message ID 1506118954228121751 at 2026-05-19T02:19:01.025Z:
Use the terminal/shell tool. Do not answer from memory. Run exactly this command: openclaw definitely-not-a-real-subcommand

After the command result is shown, reply exactly: delivery survived
  • OpenClaw session 95632482-8b6e-4e76-8590-9babebc86e28, run df200f14-58e4-4723-83f1-5a3887351002, recorded a real exec tool call:
{"name":"exec","arguments":{"command":"openclaw definitely-not-a-real-subcommand"}}
  • The real tool result completed with exit code 1:
[openclaw] Reason: Unknown command: openclaw definitely-not-a-real-subcommand. No built-in command or plugin CLI metadata owns "definitely-not-a-real-subcommand".
(Command exited with code 1)
  • Final bot message ID 1506118967717003335 remained visible in Discord as:
delivery survived
  • Discord API state for that bot message confirms it was edited into the final reply instead of being deleted/cleared:
{"id":"1506118967717003335","content":"delivery survived","timestamp":"2026-05-19T02:19:04.241000+00:00","edited_timestamp":"2026-05-19T02:19:06.968000+00:00","flags":4}

Manual observation during the live run: Discord first showed transient command/progress/log content, then that content was edited away and replaced with the final delivery survived message. That is the behavior this PR is intended to preserve: a later tool-warning/failure path no longer clears the already-finalized assistant reply.

@clawsweeper re-review

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels May 19, 2026
@clawsweeper clawsweeper Bot mentioned this pull request May 19, 2026
Closed
@xueqingli1

xueqingli1 commented May 19, 2026 •
edited
Loading

Copy link
Copy Markdown

Drive-by note from a Discord + Codex streaming setup that hit a similar symptom, not a formal review.

This fix matches the order we observed: assistant final first, then a recovered tool-warning final. One edge case I think we should sanity-check: is the reverse order impossible in the current Pi/Codex path?

Example sequence:

  1. progress-mode draft preview is active
  2. a recovered/non-terminal tool warning is delivered first:
    sendFinalReply({ text: "⚠️ ... failed", isError: true })
  3. the assistant’s non-error final is delivered later:
    sendFinalReply({ text: "delivery survived" })

If that ordering cannot happen, then I think this PR covers the real-world case we saw. If it can happen through a plugin/runtime/tail-dispatch path, a small regression test for warning-first-then-final might be useful.

One design thought: I think we should distinguish terminal error finals from recovered/non-terminal tool-warning finals. I can see why terminal error finals use the existing preview finalizer cleanup path: if the turn ends in an error while a draft preview is active, the draft should be discarded/cleared and the error delivered normally. But recovered tool warnings are supplemental status, not the owner of the assistant preview lane, so ideally they would bypass preview finalization/cleanup regardless of ordering.

The broader invariant I think we should preserve is: terminal run errors can clean up the live draft, but recovered/non-terminal tool warnings should not clear or cancel the active assistant preview lane.

@clawsweeper clawsweeper Bot mentioned this pull request May 19, 2026
Closed
25 tasks
@thewilloftheshadow

thewilloftheshadow commented May 19, 2026

Copy link
Copy Markdown
Member

@clawsweeper automerge

@clawsweeper clawsweeper Bot added the clawsweeper:automerge Maintainer opted this PR into bounded ClawSweeper-reviewed automerge label May 19, 2026
@clawsweeper

clawsweeper Bot commented May 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🔧
ClawSweeper automerge is enabled.

Draft PRs stay fix-only until GitHub marks them ready for review. Pause with /clawsweeper stop.

Automerge progress:

  • 2026-05-19 14:01:10 UTC review queued bbdfb165ed5f (queued)

@clawsweeper clawsweeper Bot mentioned this pull request May 19, 2026
Merged
@clawsweeper

clawsweeper Bot commented May 19, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper 🐠 reef update

Thanks for the work here. GitHub would not let ClawSweeper push to this branch with the available credentials, so the fix moved to a narrow replacement PR. nothing personal, just permission currents.

Why replacement: ClawSweeper could not update the source PR branch directly; GitHub did not grant sufficient push rights to the bot for that branch.
Replacement PR: #84169
Why close: this run explicitly closes the superseded source PR after the credited replacement PR is open, so review continues in one place.
Closing this source PR because this run explicitly enabled source-PR closeout.
Contributor credit is carried into the replacement PR body and changelog plan.
Co-author credit kept:

fish notes: model gpt-5.5, reasoning high; reviewed against 9dcd993.

@clawsweeper clawsweeper Bot closed this May 19, 2026
@joshavant joshavant mentioned this pull request May 19, 2026
Closed
25 tasks
@clawsweeper clawsweeper Bot mentioned this pull request May 20, 2026
Closed
25 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

channel: discord Channel integration: discord clawsweeper:automerge Maintainer opted this PR into bounded ClawSweeper-reviewed automerge 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: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. size: XS status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bug: Discord streamed reply disappears after tool-call failure warning

3 participants

@neeravmakwana @xueqingli1 @thewilloftheshadow