fix(tts): defer text settlement for final-mode TTS to eliminate churn (#83511)#83988
fix(tts): defer text settlement for final-mode TTS to eliminate churn (#83511)#83988Jerry-Xin wants to merge 16 commits into
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed July 20, 2026, 10:01 AM ET / 14:01 UTC. Summary PR surface: Source +1432, Tests +631, Docs +121. Total +2184 across 20 files. Reproducibility: no. high-confidence current-head reproduction is provided. The source path and older live Telegram screenshots support the original churn report, but the current head’s tagged and deferred-text recovery branches have not been demonstrated in a real Telegram run. Review metrics: 1 noteworthy metric.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review findings
Review detailsBest possible solution: Approve or reject the narrow generic capability contract first; if approved, land only after current-head Telegram evidence demonstrates always mode, tagged directive handling, media preservation, and text fallback when the captioned voice is not delivered. Do we have a high-confidence way to reproduce the issue? No high-confidence current-head reproduction is provided. The source path and older live Telegram screenshots support the original churn report, but the current head’s tagged and deferred-text recovery branches have not been demonstrated in a real Telegram run. Is this the best way to solve the issue? Unclear. The generic capability is a plausible core/plugin boundary, but accepting it as a permanent public channel contract requires maintainer direction before this becomes the best solution. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 154a49a78b05. Label changesLabel justifications:
Evidence reviewedPR surface: Source +1432, Tests +631, Docs +121. Total +2184 across 20 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 (52 earlier review cycles; latest 8 shown)
|
aff8a12 to
372b702
Compare
|
Rebased on latest upstream/main (5e0850f, clean rebase, no conflicts). |
372b702 to
813a6f3
Compare
|
Addressed the P1 finding in 813a6f3 — non-deferred TTS supplements are now kept audio-only. Changes:
This ensures text is not delivered twice on non-caption channels where Also rebased on latest upstream/main (03d774d, clean rebase, no conflicts). |
813a6f3 to
2fde42a
Compare
|
Rebased on latest upstream/main (b7ba7c3, clean rebase, no conflicts). |
2fde42a to
fd5e971
Compare
|
Rebased on latest upstream/main (48acdd3, clean rebase, no conflicts). |
fd5e971 to
58cc198
Compare
|
Rebased on latest upstream/main (94d8391, clean rebase, no conflicts). |
ee9ca02 to
1f5751d
Compare
|
Rebased on latest upstream/main (79197b3, clean rebase, no conflicts). |
|
ClawSweeper PR egg: 🎁 locked until real behavior proof passes. Details
|
1f5751d to
388d342
Compare
|
Rebased on latest upstream/main (a002c41, clean rebase, no conflicts). |
|
🦞🧹 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:
|
|
@clawsweeper re-review |
|
🦞🧹 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:
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: fix(tts): defer text settlement for final-mode TTS to eliminate churn (#83511) This is item 1/1 in the current shard. Shard 0/1. This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
…openclaw#83511) Add captionedFinalText channel capability so caption-capable channels (Telegram) receive voice notes with bundled text captions instead of separate text-before-audio delivery. ACP dispatch: defer visible text settlement until TTS synthesis completes; deliver text as voice-note caption on success, fall back to plain text on failure. Non-ACP dispatch: suppress live block text delivery on captioned channels; accumulate text internally and attach as caption to final TTS voice note. Preserve live block streaming for tagged auto-TTS mode while suppressing partial previews to prevent raw [[tts:text]] directive leaks. Gate partial reply forwarding with canCaptionFinalTts (covers both always and tagged modes) instead of willUseCaptionedFinalTts to prevent directive content from leaking through Telegram draft preview lane. Add captionedFinalText documentation to docs/tools/tts.md covering opt-in criteria, tagged mode behavior, text-only fallback, and plugin API. Closes openclaw#83511
Point plugin authors to capabilities.tts.voice.captionedFinalText instead of resolveChannelTtsVoiceDelivery, which is a core runtime resolver, not a plugin implementation hook. Add a complete capabilities snippet matching the Telegram reference implementation.
Narrow block suppression to text-only payloads on both ACP and non-ACP dispatch paths. Blocks containing media, presentation, interactive, or channelData are now delivered with text stripped instead of being silently dropped. Add captionedFinalText capability guidance to the channel plugin SDK docs. Add regression tests for media-block delivery and text-only suppression on both dispatch paths.
Extend captioned-final text stripping to the onToolResult delivery path on both ACP and non-ACP dispatch paths. Tool results carrying media now drop their text to prevent duplicating the final voice note caption. Text-only tool results are preserved — they are progress/status, not caption content. Uses media-only check (via resolveSendableOutboundReplyParts) instead of the broader block-path check to preserve text on interactive/approval tool results that have no media. Enhance captionedFinalText JSDoc to warn that incorrect opt-in silently loses reply text. Add regression tests for tool-result media text stripping and text-only tool result preservation on both dispatch paths.
- Remove unused result binding in captioned final TTS dispatch-acp test - Type mock.calls accesses for dispatcher and routeReply payloads - Allow resolveStatusTtsSnapshot mock to return null per production signature - Enable verbose progress in the text-only tool-result test so the result is actually delivered (text-only progress is suppressed by default), matching production behavior
Compute final TTS media delivery from the actual route/dispatcher outcome
instead of the pre-send payload shape. Previously deliveredMedia was true
whenever the captioned final voice payload carried a media URL, even when
routeReply returned { ok: false } or sendFinalReply returned false. Because
block text is suppressed for captioned final TTS, the accumulated-text
fallback gate was then short-circuited and the reply content was dropped on
a failed final voice send.
deliveredMedia is now gated on confirmed delivery (routed delivery for the
route path, queuedFinal for the dispatcher path), so a failed or suppressed
captioned voice send still lets the accumulated block text fallback run.
Add regression tests covering both the dispatcher and route delivery paths.
…al media A suppressed route-reply means the hook cancelled the send and nothing reached the user, but the suppressed branch still set deliveredFinalTtsMedia. finalizeAcpTurnOutput gates the text fallback on that flag, so a suppressed final-media route silently dropped the routed reply text. Leave the flag false on suppression so the text fallback can still fire.
…tate A hook-suppressed routed final reply (route-reply returns ok+suppressed) is handled but never reaches the user. Both the ACP and non-ACP captioned final-mode TTS fallback gates keyed off handled signals (deliveredFinalReply / queuedFinal and the raw deliver() return), so a suppressed captioned voice send skipped the text fallback and silently dropped the reply. Separate handled from delivered-to-user: add deliveredFinalReplyToUser to the ACP coordinator (set only on genuine routed/direct delivery) and a deliveredFinal result on the non-ACP sendFinalPayload. The captioned-final fallback gates now read the delivered-to-user signal while the no-reply/abort guards keep reading the handled flag. Adds suppressed-route regressions on both paths.
…t inline wait The non-ACP direct dispatcher branch treats the enqueue-only return of sendFinalReply as proof the reply reached the user. sendFinalReply only queues; the visible send (e.g. Telegram sendVoice) settles asynchronously and can fail later. When a captioned final TTS suppressed the accumulated block text and the voice then failed async, the block-text fallback was gated off (it keys on delivered-to-user state), so the user received nothing. The reply operation must not block on dispatcher.waitForIdle() — a queued follow-up would deadlock completion against dispatcher idle (removed upstream in So drive the recovery off the same settle lifecycle: sendFinalPayload keeps its optimistic queued gating and surfaces the pre-send final outcome counts for the direct-dispatcher path. When a captioned final (or the synthesized-voice sibling path) is admitted through the raw dispatcher, register a post-settlement task (registerReplyDispatcherSettledTask) that re-sends the accumulated block text as a text-only final if the voice failed/cancelled. It runs after waitForIdle inside settleReplyDispatcher, out of the reply hot path, so completion never blocks. Rewrite the captioned-final media-failure regression to model async failure (sendFinalReply returns true, getFailedCounts reports the failure after waitForIdle) and settle the dispatcher, and add a sibling test for the synthesis fallback path.
In captioned-final TTS mode, text-only blocks are suppressed and accumulated so the final captioned voice carries them. If the final reply turned out to be ordinary media (an image or file rather than a synthesized voice), the delivery accounting treated that media as delivered TTS media and the final as delivered to the user, which closed the accumulated block-text fallback gate. The user then received only the image and the assistant text was dropped. maybeApplyTts returns the reply unchanged when it already has media, so no voice is synthesized in that case. Distinguish genuinely synthesized captioned media (no media in, media out) from ordinary final media by capturing the original reply's media state before TTS, and only count synthesized media as delivered TTS media at both delivery sites. Fixing the media flag alone is not enough because the fallback gate also blocks on delivered-to-user, which an ordinary media-only final still sets. Add a deliveredFinalVisibleText signal and, in captioned-final mode only, gate the fallback on delivered visible text or delivered TTS media instead of generic delivered-to-user. Non-captioned and directive paths keep their existing delivered-to-user semantics. Narrow the ACP coordinator's final-media check to genuine TTS supplements as a defensive invariant; ACP only emits text finals plus the synthesized voice today, but this prevents a future ordinary media-only final from regressing the same gate. Add a regression where a captioned final is ordinary media-only after a suppressed text block, asserting both the media and the accumulated text reach the user, plus an ACP delivery test asserting non-supplement final media does not satisfy the TTS-media gate.
The ACP dispatch delivery coordinator now resolves the reply delivery account via resolveReplyDeliveryAccountId, which reads plugin.config.defaultAccountId. The dispatch-acp test's mock plugin did not provide a config object, so the resolver threw. Add a config stub matching the dispatch-acp-delivery test mock.
The captioned-final text docs described block-text suppression and caption bundling unconditionally, but caption deferral of ordinary block text only engages when the resolved auto mode is not tagged. In tagged mode plain block replies with visible text stay visible; only the live partial-reply preview is held back, and a directive-only final reply attaches its resolved directive text as the voice-note caption. Scope the wording in both the SDK channel plugin docs and the TTS tool docs so plugin authors do not assume plain tagged replies are hidden.
…lback When a captioned final-mode voice note is deferred, block text is suppressed on the assumption the voice note will carry it. The deferred text fallback only cleared the delivered flag when the final send failed, but a before-deliver / reply_payload_sending hook returning null cancels the send after enqueue() already reported acceptance. That outcome is tracked as cancelled, not failed, so the queued voice was counted as delivered and the plain-text fallback never fired, silently dropping the reply. Fold cancelled block/final counts into the ACP direct-visible-text settlement accounting alongside failed counts, so a cancelled captioned voice final clears finalMediaDelivered and the deferred text fallback delivers. Add readDispatcherCancelledCounts mirroring the failed reader, and a regression test covering an asynchronously cancelled captioned voice on Telegram.
…utcome Non-ACP captioned-final recovery compared aggregate dispatcher final cancelled/failed counts before vs after the whole dispatcher drained. Because one reply can enqueue multiple finals (a captioned voice plus sibling reasoning/commentary/status finals), a sibling final's failure or cancellation inflated the aggregate delta and re-sent text the voice note had already delivered as its caption, reintroducing the duplicate churn this change eliminates. Move the non-ACP path to per-payload outcome attribution, matching the ACP path: add a core-internal per-payload delivery-failed observer that is symmetric to the existing per-payload cancelled hook, and drive registerCaptionedFinalTextRecovery off this captioned voice payload's own cancelled/failed outcome (identified by a capture token) instead of the aggregate counters. Recovery now re-sends the deferred text if and only if this voice's own send was cancelled or failed. Add a mixed-outcome regression: a captioned voice that succeeds while a sibling final fails must not re-send the caption text.
Remove the 'notifies appended delivery-failed observers when deliver throws' test and its appendReplyDispatcherBeforeDeliverFailed import. That helper has no source definition or production consumer, so the test referenced a symbol that never existed, breaking type-check and the suite. The retained cancellation-observer coverage (appendReplyDispatcherBeforeDeliverCancelled) still exercises the real implementation used in dispatch-from-config.transcript.ts.
…zed ACP delivery test, safe mock destructure - Remove retired coalesceIdleMs/maxChunkChars keys from ACP stream test configs (they were retired in 783a5d2 and are asserted dead in dead-config-keys.test.ts; the runtime ignores them), fixing TS2353. - Replace unsafe [[x]] tuple destructures of mock.calls with the existing expectDefined(...) accessor to fix TS2488. - Extract shared setup/fixtures from dispatch-acp-delivery.test.ts into dispatch-acp-delivery.test-utils.ts to satisfy the max-lines lint rule without weakening any test; keep vi.mock in the test file for hoisting. - Reformat the touched test-utils file.
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
Summary
When
messages.tts.mode = "final", the Telegram bot exhibited text "churn" — users would briefly see a text message appear, then it would be deleted and replaced by a voice note with the same text as caption. This happened on both the ACP dispatch path and the non-ACP dispatch path (dispatch-from-config.ts).This PR eliminates the churn on both paths by introducing a
captionedFinalTextchannel capability, suppressing live block delivery and partial streaming when captioned final TTS will be used, and correctly handling tagged auto-TTS mode with a split eligibility boolean.Root cause
Tagged auto-TTS mode
When
messages.tts.auto: "tagged", TTS only fires for replies containing[[tts:...]]directives. Three additional issues were discovered and fixed:Split boolean:
willUseCaptionedFinalTtswas doing two jobs — gating live text suppression AND caption eligibility. Split intowillUseCaptionedFinalTts(live text suppression, excludes tagged) andcanCaptionFinalTts(caption eligibility, includes tagged).Block TTS gate: The accumulated block TTS fallback was gated on
replies.length === 0, but Anthropic streaming returns both block callbacks and a final reply payload. Changed to gate ondeliveredFinalTtsMediaandqueuedFinalinstead.Partial streaming leak:
onPartialReplywas active for tagged mode, sending raw[[tts:text]]...[[/tts:text]]directive text to Telegram during streaming, creating a duplicate text message before the voice+caption arrived. Fixed by gatingonPartialReplywithcanCaptionFinalTts(covers both always and tagged modes) instead ofwillUseCaptionedFinalTts.Media block preservation
When captioned final TTS is active, block suppression is narrowed to text-only payloads. Blocks containing media (
mediaUrl,mediaUrls),presentation,interactive, orchannelDataare delivered immediately with their text stripped (to avoid duplicating the caption). This applies to both ACP and non-ACP dispatch paths.Changes
src/auto-reply/reply/dispatch-from-config.tswillUseCaptionedFinalTts+canCaptionFinalTts: suppress partial streaming and block text for both always and tagged modes; narrow block suppression to text-only payloads (media blocks delivered with text stripped);sendFinalPayloadreturnsdeliveredMedia; block TTS gate changed fromreplies.length === 0to!deliveredFinalTtsMedia && !queuedFinalsrc/auto-reply/reply/dispatch-from-config.test.tssrc/auto-reply/reply/dispatch-acp.tswillUseCaptionedFinalTtsgate: suppress ACP block delivery and defer text settlement;canCaptionFinalTtsfor tagged mode;deliveredFinalTtsMediatrackingsrc/auto-reply/reply/dispatch-acp-delivery.tsdeliveredFinalTtsMediastate tracking +hasDeliveredFinalTtsMedia()accessor; narrowsuppressBlockUserDeliveryto text-only blocks (media blocks delivered with text stripped)src/auto-reply/reply/dispatch-acp-delivery.test.tssuppressBlockUserDelivery, text-only block suppressionsrc/auto-reply/reply/dispatch-acp.test.tssrc/auto-reply/reply/acp-projector.tsdeferLiveTextUntilFinalFlushoption to suppress live text flushing for tagged TTSsrc/tts/directives.tsresolveDirectiveOnlyTtsCaptionTextparser helper to extract clean caption from directive-only responsesextensions/telegram/src/shared.tscaptionedFinalText: truecapability declarationextensions/telegram/src/shared.test.tssrc/channels/plugins/types.core.tscaptionedFinalTextcapability contractsrc/channels/plugins/tts-capabilities.test.tsdocs/tools/tts.mdcaptionedFinalTextcapability: opt-in criteria viacapabilities.tts.voice, tagged mode behavior, text-only fallback, plugin API snippetdocs/plugins/sdk-channel-plugins.mdcaptionedFinalTextcapability in channel plugin SDK guide: opt-in declaration, media block preservation, fallback behavior, tagged modedocs/.generated/plugin-sdk-api-baseline.sha256Tests
Real behavior proof
Behavior or issue addressed: Telegram TTS final-mode text-before-voice churn on both ACP and non-ACP dispatch paths, plus tagged mode partial preview leaking raw
[[tts:text]]directives, plus media block suppression during captioned final TTS.Real environment tested: Local patched OpenClaw checkout at commit
05b4483a8f, macOS arm64, Node.js v24.10.0. Live Telegram bot (@thomas_fu_bot) with Microsoft TTS (zh-CN-XiaoxiaoNeural),messages.tts.mode: "final".Failing proof before fix (live Telegram):
User sent "hello" to the Telegram bot. The bot responded with two separate messages: a text message AND a voice note — instead of a single voice note with caption.
Exact steps or command run after this patch:
npm run build && npm pack && npm install -g openclaw-2026.6.2.tgzopenclaw gateway install && openclaw gateway restartmessages.tts.auto: "always"andmessages.tts.auto: "tagged"— sent plain messages and directive-bearing messages to@thomas_fu_boton TelegramEvidence after fix: Always mode (
messages.tts.auto: "always") at commit05b4483a8f:Always-mode replies produce a single voice note with caption text. No text churn, no separate text bubble.
Evidence — Media block preservation with captioned final TTS at commit
05b4483a8f:When the bot generates an image during a TTS-enabled conversation (
auto: "always"), the image is delivered immediately while text is deferred to the voice note caption. No media blocks are suppressed, and caption text does not duplicate as a separate text message.Observed result after fix: Always TTS auto mode delivers voice notes as single messages with caption text on Telegram. Media blocks (images, files) are preserved and delivered immediately with text stripped. No text flash, no duplicate messages, no media loss.
What was not tested: Other TTS providers (only Microsoft TTS tested live). Group chat TTS (tested in DM only). Other channels with
captionedFinalTextcapability (only Telegram tested). Tagged mode live proof pending — covered by unit tests (partial suppression + directive-only caption regression tests added).Settle-lifecycle reconciliation (rebased onto current main)
Current
mainmade the delivered-transcript mirror fire-and-forget so reply completion no longer blocks ondispatcher.waitForIdle(). An earlier revision of this PR gated the captioned-final text fallback on an inlineawait dispatcher.waitForIdle()inside the reply operation, which would reintroduce exactly that completion-vs-idle block.The fallback rides the same dispatcher settle lifecycle instead of an inline wait, and it is attributed per captioned voice payload rather than by aggregate dispatcher counters:
appendReplyDispatcherBeforeDeliverFailed) is a core-internal observer symmetric to the existing per-payload cancelled hook — it is not part of the plugin-facing dispatcher surface.registerReplyDispatcherSettledTask(the same seam the transcript mirror uses) runs afterwaitForIdle()insidesettleReplyDispatcher, out of the reply hot path. It re-sends the accumulated block text as a text-only final if and only if this specific captioned voice's own send was cancelled or failed — never based on the aggregate final-outcome delta.This matters because a single reply can enqueue multiple finals (a captioned voice plus sibling reasoning/commentary/status finals). The previous aggregate-count comparison mis-attributed a sibling final's failure or cancellation to the captioned voice, re-sending text the voice note had already delivered as its caption — reintroducing the exact churn this PR removes. Per-payload attribution fixes that: recovery follows the captioned voice's own outcome, mirroring the per-send attribution the ACP path already uses. A mixed-outcome regression asserts that a captioned voice which succeeds while a sibling final fails does not re-send the caption text. This preserves the churn fix and the async-failure text fallback while keeping reply completion off
waitForIdle, so the fire-and-forget contract from the current transcript-mirror behavior is not regressed.Cancelled final delivery (not just failed)
A
beforeDeliver/reply_payload_sendinghook returningnullcancels a send afterenqueue()has already reported acceptance. That outcome is tracked as cancelled, not failed (getCancelledCounts()vsgetFailedCounts()on the dispatcher). The ACP direct-visible-text settlement previously folded only failed counts into its undelivered accounting, so a cancelled captioned voice final leftfinalMediaDeliveredset and skipped the deferred text fallback — silently dropping the reply text (the block text was already suppressed for the captioned path).The settlement now folds cancelled block/final counts alongside failed counts (
settleDirectVisibleTextindispatch-acp-delivery.ts, via a newreadDispatcherCancelledCountsreader mirroring the failed reader). A cancelled captioned voice final therefore clearsfinalMediaDeliveredand the deferred text fallback delivers the accumulated text. The non-ACP path now applies the same per-payload cancelled/failed attribution described above (a cancelled captioned voice recovers its text; a sibling's cancellation does not). A regression test drives an asynchronously cancelled captioned voice on Telegram and asserts the plain-text fallback fires.Closes #83511