Skip to content

fix(twitch): strip internal tool-trace banners from outbound text#102992

Closed
masatohoshino wants to merge 2 commits into
openclaw:mainfrom
masatohoshino:twitch-outbound-tool-trace-sanitize
Closed

fix(twitch): strip internal tool-trace banners from outbound text#102992
masatohoshino wants to merge 2 commits into
openclaw:mainfrom
masatohoshino:twitch-outbound-tool-trace-sanitize

Conversation

@masatohoshino

@masatohoshino masatohoshino commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Related: #90684

What Problem This Solves

Fixes an issue where assistant internal tool-trace scaffolding — <tool_call>-style XML blocks and ⚠️ 🛠️ … failed tool banners, among the scaffolding forms sanitizeAssistantVisibleText() strips — leaks verbatim into Twitch chat when a model emits it as plain text (common with models that render tool calls as text, e.g. after a failed exec whose error output echoes the prompt's tool XML). Twitch chat is public: every viewer of the channel sees the internal scaffolding, not just the addressed user.

sanitizeAssistantVisibleText() exists for exactly this, but the Twitch outbound adapter never declares the sanitizeText hook, so core delivery sends the raw text through.

Why This Change Was Made

This is the Twitch sibling of the per-channel fixes tracked under #90684: the same adapter hook pattern is already merged for Google Chat, IRC, Signal, Slack, Matrix, SMS, Mattermost, and Feishu (all linked from that issue). ClickClack, the only other bundled channel still missing the hook without an open PR, will follow separately, keeping the family's one-channel-per-PR shape.

The fix adds the family-standard hook to twitchOutbound:

sanitizeText: ({ text }) => sanitizeAssistantVisibleText(text),

Core delivery wires this adapter hook once and applies it before chunk planning, so the 500-char word-boundary Twitch chunker only ever sees sanitized text. The markdown-stripping chunker itself, delivery mode, and all send paths are unchanged.

User Impact

Twitch viewers no longer see internal tool-call XML or tool-failure banners in public chat when a model leaks scaffolding into its reply text. Ordinary assistant prose is delivered unchanged. No config, schema, or API changes.

Evidence

Boundary capture drives the composed production plugin object (twitchPlugin.outbound.sanitizeText, exactly what core delivery reads) under node --import tsx; no sanitizer or plugin code is faked. A live Twitch chat send is not exercised (infeasible here).

                        input                                            before (no hook)                        after (hook)
tool banner             "Done.\n⚠️ 🛠️ `search repos (agent)` failed"     passes through verbatim                 "Done."
XML tool_call leak      "<tool_call>{...}</tool_call>Stream is live."    passes through verbatim                 "Stream is live."
ordinary prose          "The pipeline has 3 open deals."                 "The pipeline has 3 open deals."        unchanged

Focused regression tests (extensions/twitch/src/outbound-tool-trace-sanitize.test.ts, 3 cases): tool banner stripped, XML <tool_call> block stripped, ordinary prose preserved. All 3 fail without the production change (the adapter exposes no hook).

Local: targeted suite 3/3; full twitch extension suite 16 files / 188 tests green; oxfmt and oxlint clean; tsgo:extensions clean. check:changed lanes run on CI.

@openclaw-barnacle openclaw-barnacle Bot added channel: twitch Channel integration: twitch size: XS labels Jul 9, 2026
@clawsweeper

clawsweeper Bot commented Jul 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 9, 2026, 12:28 PM ET / 16:28 UTC.

Summary
The PR adds sanitizeAssistantVisibleText as twitchOutbound.sanitizeText and adds focused regression coverage for tool-trace banners, XML tool-call blocks, and ordinary prose.

PR surface: Source +4, Tests +27. Total +31 across 2 files.

Reproducibility: yes. source-level reproduction is high confidence: current main's Twitch adapter lacks sanitizeText, while shared delivery only runs the channel sanitizer when that hook exists. I did not run a live Twitch send in this read-only review.

Review metrics: none identified.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #90684
Summary: This PR is a candidate fix for the Twitch slice of the broader non-Discord outbound sanitizer gap tracked by the canonical issue.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🦞 diamond lobster
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:

  • none.

Next step before merge

  • No automated repair is needed: the branch is focused, review findings are empty, and the remaining action is normal maintainer review and merge gating.

Security
Cleared: The diff adds an existing SDK sanitizer hook and tests only; it does not change dependencies, CI, credentials, permissions, or package execution surfaces.

Review details

Best possible solution:

Land the narrow Twitch hook after normal exact-head review and checks, while keeping the umbrella issue open for remaining channel slices or a later shared-default decision.

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

Yes, source-level reproduction is high confidence: current main's Twitch adapter lacks sanitizeText, while shared delivery only runs the channel sanitizer when that hook exists. I did not run a live Twitch send in this read-only review.

Is this the best way to solve the issue?

Yes, this is the best narrow fix for this PR: the existing outbound adapter hook is the owner boundary, and sibling channels already use the same sanitizer pattern. A shared default may be a future umbrella decision, but it is a broader contract change than this Twitch repair needs.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add P1: The PR fixes internal assistant/tool scaffolding leaking into public Twitch chat, which is an urgent user-visible channel privacy and message-quality bug.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix boundary output from loading the production twitchPlugin.outbound.sanitizeText under node --import tsx, which directly exercises the changed adapter hook; no live Twitch send was provided.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes after-fix boundary output from loading the production twitchPlugin.outbound.sanitizeText under node --import tsx, which directly exercises the changed adapter hook; no live Twitch send was provided.

Label justifications:

  • P1: The PR fixes internal assistant/tool scaffolding leaking into public Twitch chat, which is an urgent user-visible channel privacy and message-quality bug.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes after-fix boundary output from loading the production twitchPlugin.outbound.sanitizeText under node --import tsx, which directly exercises the changed adapter hook; no live Twitch send was provided.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix boundary output from loading the production twitchPlugin.outbound.sanitizeText under node --import tsx, which directly exercises the changed adapter hook; no live Twitch send was provided.
Evidence reviewed

PR surface:

Source +4, Tests +27. Total +31 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 4 0 +4
Tests 1 27 0 +27
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 31 0 +31

What I checked:

  • Root and scoped policy read: Root AGENTS.md and extensions/AGENTS.md were read fully; the extensions policy allows production plugin code to use openclaw/plugin-sdk/*, which matches the new sanitizer import path. (extensions/AGENTS.md:12, d18e48c928ee)
  • Current Twitch adapter gap: Current main's Twitch outbound adapter declares direct delivery, message-sending hooks, a 500-character limit, and chunker: chunkTextForTwitch, but it has no sanitizeText hook before the PR. (extensions/twitch/src/outbound.ts:31, d18e48c928ee)
  • Shared delivery uses adapter sanitizers only when present: Shared outbound delivery wires outbound.sanitizeText into the channel handler only when the adapter exposes it, then calls it before payload normalization and delivery. (src/infra/outbound/deliver.ts:435, d18e48c928ee)
  • Sanitizer contract: The plugin SDK exports sanitizeAssistantVisibleText, whose canonical delivery path keeps prose while removing internal scaffolding; existing tests cover internal tool-trace warning lines. (src/plugin-sdk/text-chunking.ts:46, d18e48c928ee)
  • Sibling channel pattern: Slack, Matrix, Mattermost, and Signal already expose sanitizeText: ({ text }) => sanitizeAssistantVisibleText(text) on their outbound adapters, so the Twitch fix follows the current per-channel pattern. (extensions/slack/src/channel.ts:466, d18e48c928ee)
  • PR diff: The branch adds the existing SDK sanitizer import, exposes it as twitchOutbound.sanitizeText, and adds regression tests through the composed twitchPlugin.outbound.sanitizeText surface. (extensions/twitch/src/outbound.ts:44, 3dd20f9be786)

Likely related people:

  • masatohoshino: Prior merged sanitizer hook fixes for Slack, Signal, and Matrix established the exact per-channel pattern this PR applies to Twitch. (role: sibling-fix contributor; confidence: high; commits: cd6d0f9b00f0, c0265460630a, 25490d4c4212; files: extensions/slack/src/channel.ts, extensions/signal/src/channel.ts, extensions/matrix/src/channel.ts)
  • jaydenfyi: Git history identifies this author on the original Twitch plugin commit, which introduced the affected adapter surface. (role: introduced behavior; confidence: medium; commits: f5c90f0e5c7a; files: extensions/twitch)
  • Tak Hoffman: Recent Twitch outbound account-resolution work touched the same outbound adapter area. (role: recent area contributor; confidence: medium; commits: 84db697cd66a; files: extensions/twitch/src/outbound.ts)
  • Peter Steinberger: Recent Twitch account-resolution refactoring touched the plugin/account context used by outbound sending. (role: recent area contributor; confidence: medium; commits: 854f3ad0f8b3; files: extensions/twitch/src/outbound.ts, extensions/twitch/src/config.ts)
  • Wynne668: Authored the recent Feishu sanitizer hook fix in the same umbrella issue family. (role: sibling-fix contributor; confidence: medium; commits: 6378d5efcc0a; files: extensions/feishu/src/channel.ts, extensions/feishu/src/outbound-tool-trace-sanitize.test.ts)
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. P1 High-priority user-facing bug, regression, or broken workflow. labels Jul 9, 2026
@Takhoffman

Takhoffman commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

@clawsweeper automerge

@clawsweeper

clawsweeper Bot commented Jul 9, 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-07-09 20:17:00 UTC review queued 3dd20f9be786 (queued)

@clawsweeper clawsweeper Bot added the clawsweeper:automerge Maintainer opted this PR into bounded ClawSweeper-reviewed automerge label Jul 9, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jul 9, 2026
Merged
@clawsweeper

clawsweeper Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper 🐠 reef update

Thanks for the work here. ClawSweeper could not write to the source branch, so it opened a replacement PR rather than letting the fix drift. attribution still points back here.

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: #103109
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 only because source-PR closing was explicitly enabled for this run.
The original contribution stays credited in the replacement PR context.
Co-author credit kept:

fish notes: reasoning high; reviewed against 5dad685.

@clawsweeper clawsweeper Bot closed this Jul 9, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jul 10, 2026
Open
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: twitch Channel integration: twitch clawsweeper:automerge Maintainer opted this PR into bounded ClawSweeper-reviewed automerge P1 High-priority user-facing bug, regression, or broken workflow. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: XS 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

@masatohoshino @Takhoffman