Skip to content

fix(speech): bound TTS response reads#96874

Merged
sallyom merged 1 commit into
openclaw:mainfrom
Alix-007:fix/bound-speech-tts
Jun 28, 2026
Merged

fix(speech): bound TTS response reads#96874
sallyom merged 1 commit into
openclaw:mainfrom
Alix-007:fix/bound-speech-tts

Conversation

@Alix-007

@Alix-007 Alix-007 commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

speech/TTS success-path 之前会对不可信响应体执行无界 await response.text() / await response.json()，音频生成 JSON/base64 或 Seed frame 随音频长度膨胀时可能导致 OOM/hang。

Changes

  • 将 Volcengine Seed Speech success response 从无界 response.text() 改为共享 readResponseWithLimit，16MiB cap，overflow 时 cancel stream。
  • 将 Volcengine legacy TTS success response 从无界 response.text() 改为共享 readResponseWithLimit，16MiB cap，overflow 时 cancel stream。
  • 将 Xiaomi TTS success response 从无界 response.json() 改为共享 readProviderJsonResponse，复用 provider JSON 16MiB cap。
  • 增加 Volcengine Seed/legacy 与 Xiaomi TTS oversized success body 单测。

Real behavior proof

Behavior addressed:

  • Volcengine Seed Speech success body、Volcengine legacy TTS success body、Xiaomi TTS success JSON 都不再无界读取。
  • overflow 时真实 fetch 响应流被 cancel，node:http loopback proxy 看到 socket abort。
  • 小 JSON happy-path 仍正常解析。
  • 负向 control 证明旧式无界 .text() 会完整 buffer 超过 cap 的 body。

Real environment tested:

  • 本地 worktree fix/bound-speech-tts，基于最新 upstream/main rebase 后执行。
  • 真实 node:http loopback proxy + production fetchWithSsrFGuard 路径；目标 URL 使用 http://example.com，代理把响应转成本机无 Content-Length 流式 body。

Exact steps:

  • PNPM_CONFIG_MODULES_DIR=/media/vdc/0668001470/workSpace/claw-campaign/openclaw/node_modules node --import tsx scratchpad/proof-speech-tts-bound.ts 2>&1 | tee scratchpad/proof-speech-tts-bound.out
  • PNPM_CONFIG_MODULES_DIR=/media/vdc/0668001470/workSpace/claw-campaign/openclaw/node_modules node scripts/run-vitest.mjs extensions/volcengine/tts.test.ts extensions/xiaomi/speech-provider.test.ts
  • PNPM_CONFIG_MODULES_DIR=/media/vdc/0668001470/workSpace/claw-campaign/openclaw/node_modules node scripts/run-tsgo.mjs -p tsconfig.extensions.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/extensions.tsbuildinfo
  • PNPM_CONFIG_MODULES_DIR=/media/vdc/0668001470/workSpace/claw-campaign/openclaw/node_modules node scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.extensions.test.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/extensions-test.tsbuildinfo
  • .agents/skills/autoreview/scripts/autoreview --mode branch --base upstream/main

Observed result:

  • node:http proof: [proof] 5 PASS, 0 FAIL / [proof] ALL PASS
  • focused Vitest: 2 shards passed, 32 tests passed.
  • extensions prod tsgo: exit 0, no diagnostics.
  • extensions test tsgo: exit 0, no diagnostics.
  • autoreview: autoreview clean: no accepted/actionable findings reported

What was not tested:

  • No live Volcengine or Xiaomi API call; proof uses real local TCP/fetch behavior with deterministic oversized streaming bodies.

Evidence

[proof] node:http loopback proxy=http://127.0.0.1:38051 target=http://example.com cap=16777216 full=67108864
PASS happy-path small JSON parsed for Seed, legacy Volcengine, and Xiaomi: ok
PASS BytePlus Seed Speech bounded overflow: error="BytePlus Seed Speech TTS response exceeds 16777216 bytes" abort=true bytesSent=17825792/67108864 chunks=17
PASS Volcengine legacy bounded overflow: error="Volcengine TTS response exceeds 16777216 bytes" abort=true bytesSent=17825792/67108864 chunks=17
PASS Xiaomi TTS bounded overflow: error="Xiaomi TTS API: JSON response exceeds 16777216 bytes" abort=true bytesSent=17825792/67108864 chunks=17
PASS legacy unbounded negative control buffered: 67108864 bytes (> 16777216)
[proof] 5 PASS, 0 FAIL
[proof] ALL PASS

Label: security
AI-assisted.

@clawsweeper

clawsweeper Bot commented Jun 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 25, 2026, 8:31 PM ET / 00:31 UTC.

Summary
The PR changes Volcengine and Xiaomi TTS success-response parsing to use bounded response readers and adds oversized response regression tests.

PR surface: Source +18, Tests +81. Total +99 across 4 files.

Reproducibility: yes. from source inspection: current main still calls unbounded response.text()/response.json() on the affected TTS success paths. The PR body adds after-fix loopback proof plus a negative unbounded-read control, but I did not execute commands in this read-only review.

Review metrics: 1 noteworthy metric.

  • Bounded TTS success paths: 3 reads capped at 16 MiB. This is the behavior-changing hardening and the compatibility tradeoff maintainers should notice before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

  • [P2] Maintainer should accept or adjust the 16 MiB fail-fast behavior before merge.

Risk before merge

  • [P1] The 16 MiB cap changes behavior for Volcengine or Xiaomi success bodies above the cap: those requests now fail instead of buffering the full response.

Maintainer options:

  1. Land With Cap Accepted (recommended)
    If maintainers accept 16 MiB as the provider success-body budget, the PR can proceed because it uses existing SDK bounded-reader helpers and has real loopback proof.
  2. Adjust The Cap Before Merge
    If long-form Volcengine or Xiaomi TTS responses are expected to exceed 16 MiB, change the cap and tests before merge instead of preserving unbounded reads.

Next step before merge

  • [P2] The remaining action is maintainer acceptance or adjustment of the 16 MiB fail-fast compatibility tradeoff, not an automated code repair.

Security
Cleared: No concrete security or supply-chain concern was found; the diff reduces unbounded response buffering and does not change dependencies, workflows, secrets, or install behavior.

Review details

Best possible solution:

Land the bounded-read hardening after maintainers accept the 16 MiB fail-fast cap for these TTS providers and exact-head checks stay clean.

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

Yes, from source inspection: current main still calls unbounded response.text()/response.json() on the affected TTS success paths. The PR body adds after-fix loopback proof plus a negative unbounded-read control, but I did not execute commands in this read-only review.

Is this the best way to solve the issue?

Yes: the patch uses existing public plugin-sdk bounded-reader helpers, keeps provider behavior in owner modules, and matches sibling generated-media size caps. The remaining question is maintainer acceptance of the 16 MiB cap, not a better implementation layer.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against dd0e4f6e61b8.

Label changes

Label justifications:

  • P2: This is a focused security/availability hardening fix for optional bundled TTS providers with limited blast radius.
  • merge-risk: 🚨 compatibility: Existing long Volcengine or Xiaomi TTS success responses above 16 MiB would now fail instead of being fully buffered.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body provides copied live output from a node:http loopback proxy exercising production fetch behavior, overflow cancellation, happy paths, and an unbounded negative control.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides copied live output from a node:http loopback proxy exercising production fetch behavior, overflow cancellation, happy paths, and an unbounded negative control.
Evidence reviewed

PR surface:

Source +18, Tests +81. Total +99 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 2 22 4 +18
Tests 2 81 0 +81
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 103 4 +99

What I checked:

Likely related people:

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.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. labels Jun 26, 2026
@mushuiyu886 mushuiyu886 mentioned this pull request Jun 26, 2026
Merged
@clawsweeper clawsweeper Bot mentioned this pull request Jun 26, 2026
Closed
@sallyom sallyom self-assigned this Jun 28, 2026
@sallyom

sallyom commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Merge-ready for 2774988.

Local and ClawSweeper review clean, CI green, best narrow fix, and follows the established bounded TTS/media response-read pattern. Maintainer cap accepted: 16 MiB is defensible here because these are generated-audio TTS responses wrapped in JSON/base64, not arbitrary file downloads; the cap matches OpenClaw's existing generated-audio/media budget used by sibling TTS/media providers, and gives roughly 12 MiB decoded audio after base64 overhead, which is enough for normal OpenClaw TTS replies/voice notes. Long-form narration above that should use a deliberate product/config path, not unbounded provider reads.

@sallyom
sallyom merged commit 2f851ec into openclaw:main Jun 28, 2026
146 of 152 checks passed
@github-actions github-actions Bot mentioned this pull request Jun 28, 2026
Open
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 29, 2026
@Alix-007
fix(speech): bound TTS response reads (openclaw#96874)
537ca03
QiuYuang pushed a commit to QiuYuang/openclaw that referenced this pull request Jul 1, 2026
@Alix-007 @QiuYuang
fix(speech): bound TTS response reads (openclaw#96874)
7505e9d
chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 1, 2026
@Alix-007 chenyangjun-xy
fix(speech): bound TTS response reads (openclaw#96874)
f717806
Rorqualx pushed a commit to Rorqualx/cortex that referenced this pull request Jul 2, 2026
@Alix-007 cron
fix(speech): bound TTS response reads (openclaw#96874)
b4e71fe 
(cherry picked from commit 2f851ec)
chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 3, 2026
@Alix-007 @chenyangjun-xy
fix(speech): bound TTS response reads (openclaw#96874)
434a75c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@sallyom sallyom

Labels

extensions: volcengine extensions: xiaomi merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Alix-007 @sallyom