fix: prevent duplicate block reply delivery for text_end channels#61530
Conversation
Greptile SummaryThis PR fixes a duplicate message delivery bug on channels using The fix correctly adds a short-circuit guard in
Confidence Score: 4/5Safe to merge — the fix is logically sound, preserves fallback delivery when text_end didn't fire, and all existing tests pass. The guard condition correctly keys on both blockReplyBreak and lastBlockReplyText being non-null, cleanly avoiding the false-positive stripping pipeline mismatch described in the PR. The only concern is that the first new test does not exercise the new guard (the outer if-condition is false before reaching it), leaving the guard covered only by the second new test. This is a test quality issue, not a correctness issue. src/agents/pi-embedded-subscribe.handlers.messages.test.ts — the first new test (line 516) does not reach the new guard and has a description that implies it does. Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/agents/pi-embedded-subscribe.handlers.messages.test.ts
Line: 516-577
Comment:
**First new test doesn't exercise the new guard**
The test description implies it verifies the new guard (`blockReplyBreak === "text_end" && lastBlockReplyText != null`), but the guard is never reached during this test.
Because `lastBlockReplyText` is `"Hello world"` and the computed `text` (from `stripBlockTags("Hello world", ...)` with the identity mock) is also `"Hello world"`, the outer `if` condition's third OR-clause (`text !== ctx.state.lastBlockReplyText`) evaluates to `false`. Since `blockReplyBreak` is `"text_end"` (not `"message_end"`) and `hasBufferedBlockReply` is `false`, all three OR-clauses are false — so the entire block at lines 593–633 is skipped before the new guard at line 612 is ever evaluated.
The test passes because of the pre-existing outer equality check, not the new guard code. The **second** new test (line 579) correctly exercises the guard by setting `lastBlockReplyText: "Hello world."` (with a trailing period) so that `text !== lastBlockReplyText` is `true` and the guard fires.
To cover the guard in this test as well, set `lastBlockReplyText` to a value that differs from the computed `text` (e.g., `"Hello world (stripped differently)"`) so the outer condition is entered and the guard has a chance to short-circuit. Alternatively, update the description to clarify it validates the pre-existing equality-check behavior rather than the new guard.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix: prevent duplicate block reply deliv..." | Re-trigger Greptile |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ede045d21f
ℹ️ 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".
* feat(memory-wiki): add import gateway methods * feat(memory-wiki): add shared memory search bridge * feat(memory-wiki): add prompt supplement integration * feat(memory-wiki): surface imported source provenance * feat(memory-wiki): lint imported provenance gaps * feat(memory-wiki): allow per-call search corpus overrides * feat(memory-core): bridge wiki corpus into memory tools * feat(memory-wiki): compile related backlinks blocks * docs(memory-wiki): prefer shared corpus recall guidance * docs(memory-wiki): document shared recall and backlinks * feat(memory-wiki): generate dashboard report pages * test: isolate exec approval suite from bundled plugins * fix(sandbox): harden EXDEV rename fallback * Gateway: keep outbound session metadata in owner store * revert(memory-wiki): back out llm wiki stack * fix: align models status provider auth reporting * fix(ci): narrow control ui locale refresh push runs * style: format remaining local edits * fix: prevent duplicate block reply delivery for text_end channels (openclaw#61530) * fix(gateway): bound silent local pairing scopes * fix: resolve repo check drift * fix: clean rebase leftovers * test: isolate agent runtime seams * docs: refine unreleased changelog * feat(video): add xai and alibaba providers * Revert "fix(gateway): bound silent local pairing scopes" This reverts commit 7f1b159. * fix(build): correct node require typing * docs(security): clarify localhost shared-auth trust model * refactor: move browser runtime seams behind plugin metadata * test: speed up provider policy and auth suites * Memory: move dreaming trail to dreams.md (openclaw#61537) * Memory: move dreaming trail to dreams.md * docs(changelog): add dreams.md entry --------- Co-authored-by: Vincent Koc <vincentkoc@ieee.org> * fix(ci): stabilize ui i18n and gateway watch checks * docs(providers): add generation setup pages * docs(providers): link generation guides * feat: add qa channel foundation * refactor: hide qa channels with exposure metadata * feat: add qa lab extension * chore: polish qa lab follow-ups * feat(qa): recreate qa lab docker stack * fix(qa): restore embedded control ui gateway startup * fix(qa): stabilize docker gateway bootstrap * feat(qa): add repo-backed qa suite runner * fix(qa): stabilize hermetic suite runtime * fix(matrix): split partial and quiet preview streaming (openclaw#61450) Merged via squash. Prepared head SHA: 6a0d7d1 Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras * docs(providers): unify qwen docs * fix(matrix): honor canonical private-network opt-in * fix(matrix): restore cli metadata registrar * test(matrix): isolate migration snapshot seam * fix: prevent duplicate gateway watchers * feat(memory-core): add REM preview and safe promotion replay (openclaw#61540) * memory: add REM preview and safe promotion replay thanks @mbelinky * changelog: note REM preview and promotion replay --------- Co-authored-by: Vignesh <mailvgnsh@gmail.com> * test: fix abort cascade and workspace edit inputs * refactor: harden plugin metadata and browser sdk seams * fix(memory-core): preserve dated DREAMS trail * docs(memory): point dreaming trail docs to dreams.md * fix(memory): standardize DREAMS trail path * fix(google): restore gemini cli provider contract * test(contracts): drop removed claude cli auth export * test(config): align markdown tables with active registry * style(tests): normalize registry mock wrapping * fix: normalize video provider durations * fix: harden video provider transports * fix: honor discord allowlisted channels for native commands * fix: bootstrap pnpm for git updates * docs: add tahoe release-to-dev smoke lane * test: isolate openclaw plugin context coverage * test: stabilize subagent persistence registry coverage * test: isolate gateway tool coverage * fix: surface normalized video durations * fix(google): restore forward-compat provider hooks * test(config): fix markdown table mock typing * test: drop redundant openai extra params coverage * refactor: dedupe discord native command auth * docs: add discord native command changelog note * fix(video): queue fal provider jobs * feat(agents): track video generation tasks * fix(discord): short-circuit bound thread self-loop drops * refactor: harden plugin metadata and bundled channel entry seams * test: fold xai extra params coverage into hot lane * fix: ignore unsupported image generation overrides * docs: document channel persisted auth metadata * test(live): prefer google models over big-pickle * Lobster: run workflows in process (openclaw#61523) * Lobster: run workflows in process * docs: note in-process lobster runtime * docs: add lobster changelog attribution * Lobster: add managed TaskFlow mode (openclaw#61555) * test: split inline provider model coverage * docs: update Lobster in-process mode and REM preview tooling * test: speed up nodes camera coverage * fix: defer plugin sync after git switch * test: optimize macos release-to-dev smoke lane * fix(openai): avoid em dashes in gpt-5 overlay (openclaw#61560) * feat(agents): detach video generation completion * feat(video): add runway provider * docs(video): document runway support * fix: clarify dirty dev update error * fix: ignore unsupported video generation overrides * refactor: add metadata-first channel configured-state probes * fix(video): guard active async generation tasks * docs(providers): surface new video provider pages * feat(qa): add live suite runner and harness * feat(qa): improve qa lab debugger ui * fix: restore pnpm check type safety * test: trim slow agent web and lifecycle coverage * fix: restore green checks * fix(qa): stop embedded control ui reload loop * test: reset guest git root before dev update * test: speed up openai tool id preservation replay coverage * fix: restore qa lab config typing * matrix: align bundled channel metadata * docs: note Matrix persisted auth detection * docs: add changelog note for qa lab config fix * refactor(video): share async task status helpers * memory-core: checkpoint mode-first dreaming refactor * Dreaming: simplify sweep flow and add diary surface * docs: rewrite video generation docs for readability * docs(faq): add gpt-5.4 fast mode entry * feat(memory): add Bedrock embedding provider for memory search (openclaw#61547) * feat(memory): add Bedrock embedding provider for memory search Add Amazon Bedrock as a native embedding provider for memory search. Supports Titan Embed Text v1/v2 and Cohere Embed models via AWS SDK. - New embeddings-bedrock.ts: BedrockRuntimeClient + InvokeModel - Auth via AWS default credential chain (same as Bedrock inference) - Auto-selected in 'auto' mode when AWS credentials are detected - Titan V2: configurable dimensions (256/512/1024), normalization - Cohere: native batch support with search_query/search_document types - 16 new tests covering all model types, auth detection, edge cases Closes openclaw#26289 * fix(memory): harden bedrock embedding selection --------- Co-authored-by: Vincent Koc <vincentkoc@ieee.org> * docs(openai): clarify gpt-5.4 fast mode * test: speed up models config env provider coverage * test: speed up sanitize session history policy smoke * build: refresh lockfile for control ui deps * refactor: narrow bundled channel entry surfaces * test: speed up sanitize session history coverage * fix: skip old-process config writes after git switch * fix(update): bootstrap pnpm for dev preflight * fix(memory-qmd): restore qmd compatibility defaults * test: speed up image tool auth-heavy coverage * test: seed channel setup contract registry in helper tests * Dreaming: update multiphase stats and UI polish * test: add irc runtime api smoke coverage * feat(bedrock-mantle): add IAM credential auth via @aws/bedrock-token-… (openclaw#61563) * feat(bedrock-mantle): add IAM credential auth via @aws/bedrock-token-generator Mantle previously required a manually-created API key (AWS_BEARER_TOKEN_BEDROCK). This adds automatic bearer token generation from IAM credentials using the official @aws/bedrock-token-generator package. Auth priority: 1. Explicit AWS_BEARER_TOKEN_BEDROCK env var (manual API key from Console) 2. IAM credentials via getTokenProvider() → Bearer token (instance roles, SSO profiles, access keys, EKS IRSA, ECS task roles) Token is cached in memory (1hr TTL, generated with 2hr validity) and in process.env.AWS_BEARER_TOKEN_BEDROCK for downstream sync reads. Falls back gracefully when package is not installed or credentials are unavailable — Mantle provider simply not registered. Closes openclaw#45152 * fix(bedrock-mantle): harden IAM auth --------- Co-authored-by: Vincent Koc <vincentkoc@ieee.org> * refactor(update): extract package manager bootstrap logic * feat: add comfy workflow media support * fix: stabilize line and feishu ci shards * feat: add music generation tooling * chore: remove stray finder metadata * docs: document music generation async flow * fix(memory-qmd): streamline compatibility coverage * test: speed up dispatch-from-config thread fallback coverage * docs: improve music generation docs * docs: reorder changelog highlights * fix: skip stale post-switch update follow-ups * test: harden macos release-to-dev smoke verification * fix: route comfy music through shared tool * refactor: remove comfy music tool shim * Gateway: bound websocket shutdown close (openclaw#61565) Merged via squash. Prepared head SHA: 9040dd5 Co-authored-by: mbelinky <132747814+mbelinky@users.noreply.github.com> Reviewed-by: @mbelinky * Docs: clarify Matrix quiet push rules * memory: chunk daily dreaming ingestion (openclaw#61583) Merged via squash. Prepared head SHA: 88816a0 Co-authored-by: mbelinky <17249097+mbelinky@users.noreply.github.com> Co-authored-by: mbelinky <132747814+mbelinky@users.noreply.github.com> Reviewed-by: @mbelinky * fix: stop old cli after package-to-git switch * fix(gateway): accept music generation internal events * docs: update unreleased provider notes * fix(agents): keep large read tool results visible * feat: add vydra media provider * fix(agents): ignore unsupported music generation hints * fix(agents): preserve latest read output during compaction * docs: update changelog for read visibility fixes * test: fix current-main prep blockers (openclaw#61582) Merged via squash. Prepared head SHA: 49f7b12 Reviewed-by: @mbelinky * test: use explicit node entrypoint in macos update smoke * fix: exit after package-to-git handoff * fix: prune staged feishu sdk types from npm pack * fix(qa): harden new scenario suite * fix(agents): prefer overflow compaction for fresh reads * perf(auto-reply): lazy-load TTS helpers on demand * test(plugin-sdk): tighten ACP command dispatch guards * docs(web): clarify control ui language picker * test(auto-reply): split ACP and reply-dispatch regressions * memory: trim generic daily chunk headings (openclaw#61597) * memory: trim generic daily chunk headings * docs: tag dreaming heading cleanup changelog * docs: attribute dreaming heading cleanup changelog * fix(cli): narrow post-update root * fix(ui): localize control ui strings * Lobster: harden embedded runtime integration (openclaw#61566) Merged via squash. Prepared head SHA: a6f4830 Co-authored-by: mbelinky <132747814+mbelinky@users.noreply.github.com> Reviewed-by: @mbelinky * fix(matrix): reuse raw default account key during onboarding promotion * fix: unblock comfy live plugin loading * fix(agents): extend subagent announce timeout * fix(agents): carry async media wake attachments structurally * fix(tasks): hide internal completion wake rows * test(auto-reply): isolate reply abort dispatch seams * test: fix reply dispatch mock contract * fix(ui): localize more control ui strings * fix: deliver async media generation results directly * perf(test): trim send-policy and abort hot paths * perf(agents): isolate subagent announce origin helper * fix(discord): raise default media cap * Matrix: recover from pinned dispatcher runtime failures (openclaw#61595) Merged via squash. Prepared head SHA: f9a2d9b Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras * fix: harden async media completion delivery * fix: gate async media direct delivery behind config * docs: add changelog note for async media delivery flag * perf(test): trim announce and sessions tool imports * fix: resolve global bundled plugin facade fallback (openclaw#61297) (thanks @openperf) * fix(gateway): resolve globally-installed bundled plugins in facade-runtime * fix: resolve global bundled plugin facade fallback (openclaw#61297) (thanks @openperf) --------- Co-authored-by: Ayaan Zaidi <hi@obviy.us> * chore: prepare 2026.4.6-beta.1 release * style: trim facade fallback comment noise * test: stabilize browser and provider ci shards * fix: restore latest-main ci gates * (chore): delete dream-diary-preview file * perf(test): trim runReplyAgent misc mock imports * fix(ci): harden control ui locale refresh rebases * Matrix: clear undici test override after transport test * chore(ui): refresh zh-CN control ui locale * chore(ui): refresh pt-BR control ui locale * chore(ui): refresh zh-TW control ui locale * chore(ui): refresh de control ui locale * fix: support corepack cmd shim on windows * test: add windows dev-update smoke lanes * chore(ui): refresh es control ui locale * chore(ui): refresh ja-JP control ui locale * chore(ui): refresh ko control ui locale * chore(ui): refresh fr control ui locale * test: capture windows npm debug tails in smoke logs * chore(ui): refresh tr control ui locale * chore(ui): refresh uk control ui locale * chore(ui): refresh id control ui locale * chore(ui): refresh pl control ui locale * fix: restore plugin boundary and ui locale ci gates * fix(ci): stabilize control ui locale checks * chore: release 2026.4.5 * perf(test): split subagent command coverage * fix(ci): patch main regression surfaces * fix: install bun in npm release preflight * test: fix subagent command result assertions * perf(test): split allowlist and models command coverage * fix(openai): allow qa image generation mock routing * feat(qa): execute ten new repo-backed scenarios * fix(matrix): harden startup auth bootstrap (openclaw#61383) Merged via squash. Prepared head SHA: d8011a9 Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras * Docs: clarify Matrix autoJoin invite scope * fix(discord): narrow binding runtime imports * fix: stabilize contract loader seams * test: tighten allowlist fixture typing * fix(qa): support image understanding inputs * feat(qa): add attachment understanding scenario * docs(matrix): clarify historyLimit default * feat(memory-wiki): restore llm wiki stack * chore: update appcast for 2026.4.5 * perf(test): split reply command coverage * perf(reply): lazy load compact runtime * refactor(reply): extract subagent text helper * style(reply): normalize subagent import order * fix: restore protocol and extension ci * chore: bump version to 2026.4.6 * fix(config): normalize channel streaming config shape (openclaw#61381) * feat(config): add canonical streaming config helpers * refactor(runtime): prefer canonical streaming accessors * feat(config): normalize preview channel streaming shape * test(config): lock streaming normalization followups * fix(config): polish streaming migration edges * chore(config): refresh streaming baseline hash * docs(memory): add promote-explain and rem-harness CLI reference * build: refresh pnpm lockfile * fix: stop emitting post-background exec updates (openclaw#61627) (thanks @openperf) * fix(exec ): stop emitting tool updates after session is backgrounded When an exec session is backgrounded (background: true), the owning agent run resolves its tool-call promise and may finish. The stdout handler's emitUpdate() closure, however, kept invoking opts.onUpdate(), delivering tool_execution_update events to a listener whose active run had already ended. This surfaced as an unhandled rejection and crashed the gateway process. Guard emitUpdate() with a session.backgrounded || session.exited check so that post-background output is still captured via appendOutput() but no longer forwarded to the (now-stale) agent-loop callback. Fixes openclaw#61592 * style: trim exec backgrounding comments * fix: stop emitting post-background exec updates (openclaw#61627) (thanks @openperf) * fix: place exec changelog entry at end of fixes (openclaw#61627) (thanks @openperf) --------- Co-authored-by: Ayaan Zaidi <hi@obviy.us> * test(memory-core): align dreaming expectations * test(memory-wiki): share plugin test helpers * test(memory-core): share workspace test helper * test(memory-core): reuse narrative workspace helper * test(plugin-sdk): share temp dir test helper * test(plugin-sdk): reuse temp dir helpers in facade tests * test(memory-core): reuse workspace helper in dreaming tests * perf(agents): add continuation-skip context injection (openclaw#61268) * test(agents): cover continuation bootstrap reuse * perf(agents): add continuation-skip context injection * docs(changelog): note context injection reuse * perf(agents): bound continuation bootstrap scan * fix(agents): require full bootstrap proof for continuation skip * fix(agents): decide continuation skip under lock * fix(commands): re-export subagent chat message type * fix(agents): clean continuation rebase leftovers * test(memory-core): reuse workspace helper in temp dir tests * docs: add contextInjection config key to reference * feat(gateway): preserve session history on /new command Backend changes for session sidebar feature: - session.ts: create compound key entry for old session when /new triggered - agent.ts: pass preserveHistory=true to session reset - session-reset-service.ts: add file reuse optimization for preserved sessions - types.ts: add previousSessionKey field to SessionEntry - session-utils.ts: add compound key support in session key resolution This enables the UI to show previous sessions in sidebar while preserving complete chat history for archived sessions. --------- Co-authored-by: Vincent Koc <vincentkoc@ieee.org> Co-authored-by: Peter Steinberger <steipete@gmail.com> Co-authored-by: Gustavo Madeira Santana <gumadeiras@gmail.com> Co-authored-by: Tyler Yust <64381258+tyler6204@users.noreply.github.com> Co-authored-by: Dave Morin <dave@morin.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Co-authored-by: Mariano <132747814+mbelinky@users.noreply.github.com> Co-authored-by: Vignesh <mailvgnsh@gmail.com> Co-authored-by: Vignesh Natarajan <vignesh.natarajan92@gmail.com> Co-authored-by: wirjo <daniel@wirjo.com> Co-authored-by: Mariano <mbelinky@gmail.com> Co-authored-by: mbelinky <17249097+mbelinky@users.noreply.github.com> Co-authored-by: Chunyue Wang <80630709+openperf@users.noreply.github.com> Co-authored-by: Ayaan Zaidi <hi@obviy.us> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: OpenClaw Agent <caoha@openclaw.ai>
Problem
Messages are being sent twice on channels using
blockReplyBreak: "text_end"(e.g. BlueBubbles). Three recent commits introduced this regression:handleMessageEndthat sends a block reply whenevertext !== lastBlockReplyTextshouldDeferTextEndBlockReplyguardRoot Cause
With
blockReplyBreak: "text_end", content is delivered once attext_endthrough the block chunker/coalescer pipeline (emitBlockChunk→stripBlockTagswith runningblockState+stripDowngradedToolCallText), which setslastBlockReplyText.Then at
message_end,handleMessageEndcomputes a freshtextviastripBlockTags(rawVisibleText, { thinking: false, final: false })— a different stripping pipeline with reset state. Because the two pipelines can produce slightly different results, the safety checktext !== lastBlockReplyTextfalse-positives and the message gets sent again.Fix
Added a guard in the
handleMessageEndsafety-send path: whenblockReplyBreak === "text_end"andlastBlockReplyTextis already set (meaningtext_endsuccessfully delivered content), skip the safety send entirely. This is the correct behavior because:text_endalready delivered, there's nothing new to send atmessage_endtext_enddidn't deliver (e.g. commentary was suppressed, or the provider skippedtext_end),lastBlockReplyTextis stillundefinedandmessage_endcorrectly delivers the contentThis preserves the commentary/final-answer phase separation that the original commits intended to fix.
Testing
pi-embedded-subscribe.handlers.messages.test.ts:lastBlockReplyTextmatches (text_end already delivered)pi-embedded-subscribetest files passtext_enddedup tests continue to passmessage_enddelivery for message_end channels unaffected