Skip to content

fix(tlon): strip internal tool-trace banners from outbound text#102211

Closed
masatohoshino wants to merge 1 commit into
openclaw:mainfrom
masatohoshino:tlon-outbound-tool-trace-sanitize
Closed

fix(tlon): strip internal tool-trace banners from outbound text#102211
masatohoshino wants to merge 1 commit into
openclaw:mainfrom
masatohoshino:tlon-outbound-tool-trace-sanitize

Conversation

@masatohoshino

@masatohoshino masatohoshino commented Jul 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Related: #90684 (Tlon slice)

What Problem This Solves

Addresses the Tlon slice of #90684: Tlon users can see internal assistant
tool-call/tool-trace markup — XML tool-call tags and banners such as
⚠️ 🛠️ \search repos (agent)` failed` — verbatim in their DMs and group
chats when an agent reply carries leftover internal markup.

The shared delivery pipeline applies the assistant-visible sanitizer only
when a channel adapter defines the outbound sanitizeText hook. The Tlon
adapter does not define it, so Tlon delivers raw text while sibling adapters
(most recently Feishu #98705 and Mattermost #98693) already strip this
markup. I did not find an open PR covering the Tlon adapter during
pre-submit review.

Why This Change Was Made

tlonChannelOutbound was missing the sanitizeText hook, and the shared
pipeline only sanitizes when that hook exists. This wires
sanitizeAssistantVisibleText into the hook; the single adapter object feeds
both the plugin outbound surface and the channel message adapter, so one
wiring point covers both delivery routes.

This follows the per-channel hook pattern established by the merged sibling
fixes and fills an adapter that has no candidate PR. It takes no position on
the per-channel vs. shared-default design question discussed on #90684, and
does not close that umbrella. Inbound sanitization and
sanitizeAssistantVisibleText itself are unchanged.

User Impact

Tlon users no longer receive internal tool-call/tool-trace markup in
outbound messages; ordinary prose and markdown pass through unchanged. No
config changes, no behavior change for any other channel.

Evidence

  • New regression test extensions/tlon/src/outbound-tool-trace-sanitize.test.ts:
    tool-trace banner is stripped before delivery; ordinary assistant prose
    passes through unchanged (over-strip negative control). Assertions run
    against the real plugin adapter export (no mocks of the sanitizer itself).
  • Full tlon extension suite: node scripts/run-vitest.mjs extensions/tlon
    → 17 files / 143 tests passed (includes the new file).
  • oxfmt --check and scripts/run-oxlint.mjs clean on both touched files.

Live behavior proof (production send path, captured at the ship HTTP boundary)

Captured with node --import tsx against the real production pipeline — no
product code mocked: deliverOutboundPayloads (src/infra/outbound/deliver.ts)
→ tlon plugin bootstrap → OpenClaw's real Urbit login request flow
(POST /~/login) → real chat-dm-action poke PUT /~/channel/<id>. The
substitution is at the network boundary: a local HTTP capture server stands in
for the Urbit ship and records the exact poke bytes OpenClaw would send to a
ship. This is boundary-capture proof, not a live hosted ship run (declared
below). Isolated OPENCLAW_STATE_DIR; ship ~zod with its well-known
development code and a loopback URL (no private ships, endpoints, or
credentials involved).

Input payload 1: "Done.\n⚠️ 🛠️ \search repos (agent)` failed"(tool-trace banner). Input payload 2:"The pipeline has 3 open deals."` (ordinary prose).

With this fix (head 370326f) the outgoing memo is banner-free:

{ "app": "chat", "mark": "chat-dm-action",
  "json": { "ship": "~sampel-palnet", "diff": { "delta": { "add": { "memo": {
    "content": [ { "inline": [ "Done." ] } ], "author": "~zod" } } } } } }

banner present in ship payload: false
prose present in ship payload: true

Negative control — same script with extensions/tlon/src/channel.ts reverted
to the base commit (fc05a81), everything else identical: the internal
banner leaves OpenClaw verbatim in the outgoing poke:

"content": [ { "inline": [ "Done.", { "break": null },
                           "⚠️ 🛠️ ",
                           { "inline-code": "search repos (agent)" },
                           " failed" ] } ]

banner present in ship payload: true
prose present in ship payload: true

Both runs completed with OpenClaw's own per-payload delivery receipts from
the exercised send path (not hosted-ship acknowledgements), so sanitization
happens in the send path itself, not via a dropped or failed payload.

  • Not run: a credentialed round trip against a live hosted Tlon/Urbit ship.
    The capture above exercises the full OpenClaw send path up to the ship's
    HTTP interface.

@masatohoshino
fix(tlon): strip internal tool-trace banners from outbound text
370326f 
Part of openclaw#90684

Wire sanitizeAssistantVisibleText into the tlon outbound adapter's
sanitizeText hook, matching the merged sibling channel fixes (Feishu
openclaw#98705, Mattermost openclaw#98693, Slack, Signal, Matrix, IRC). Without the
hook, shared delivery passes internal tool-call/reasoning scaffolding
verbatim to Tlon end users.
@openclaw-barnacle openclaw-barnacle Bot added channel: tlon Channel integration: tlon size: XS labels Jul 8, 2026
@clawsweeper

clawsweeper Bot commented Jul 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 8, 2026, 11:32 AM ET / 15:32 UTC.

Summary
The branch adds Tlon outbound sanitizeText wiring through sanitizeAssistantVisibleText and a focused regression test for stripping tool-trace banners while preserving ordinary prose.

PR surface: Source +2, Tests +21. Total +23 across 2 files.

Reproducibility: yes. at source level: current main's Tlon outbound adapter lacks sanitizeText, and shared delivery only calls assistant-visible cleanup when that hook exists. The PR body also includes before/after boundary-capture proof, but I did not run a live hosted Tlon round trip in this read-only review.

Review metrics: 1 noteworthy metric.

  • Tlon outbound text transform: 1 sanitizer hook added. The hook changes user-visible message text before Tlon delivery, so maintainers should notice the delivery-boundary behavior change before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #90684
Summary: This PR is a candidate fix for the Tlon slice of the broader non-Discord outbound assistant-visible sanitizer gap.

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: 🐚 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:

  • none.

Risk before merge

  • [P1] Merging intentionally changes the outbound text bytes handed to Tlon DM and group delivery, so maintainers should be comfortable applying the shared assistant-visible sanitizer semantics to Tlon formatting.
  • [P1] No credentialed hosted Tlon/Urbit round trip was run; the provided proof captures the production send path at the Urbit HTTP boundary, which is strong for the sanitizer path but not an external hosted acknowledgement.

Maintainer options:

  1. Land With Boundary-Capture Proof (recommended)
    Accept the production-path HTTP boundary proof plus focused regression test as sufficient for this sanitizer hook and proceed through exact-head merge validation.
  2. Ask For Hosted Tlon Round Trip
    Require one more redacted live hosted Tlon or Urbit DM/group-send proof if maintainers want external acknowledgement evidence before merge.
  3. Pause For Shared Default
    Hold this Tlon slice only if maintainers want to replace the accepted per-channel hook pattern with a central outbound sanitizer default before more slices land.

Next step before merge

  • No repair lane is needed; this contributor PR has no actionable code finding and should proceed through normal maintainer review and exact-head validation.

Security
Cleared: The diff uses an existing SDK sanitizer and adds no dependency, workflow, secret, install script, or supply-chain surface; it narrows internal-scaffolding exposure.

Review details

Best possible solution:

Land the narrow Tlon hook after exact-head validation while keeping #90684 open for remaining channel slices or a future shared sanitizer-default decision.

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

Yes at source level: current main's Tlon outbound adapter lacks sanitizeText, and shared delivery only calls assistant-visible cleanup when that hook exists. The PR body also includes before/after boundary-capture proof, but I did not run a live hosted Tlon round trip in this read-only review.

Is this the best way to solve the issue?

Yes for the narrow Tlon slice: the PR uses the existing channel-owned SDK sanitizeText seam and matches merged sibling fixes without adding config or changing other channels. A shared default remains a broader option for #90684, but it is not required for this focused fix.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P1: The PR targets a user-facing channel bug where internal tool-trace markup can reach Tlon users in outbound messages.
  • merge-risk: 🚨 message-delivery: Merging changes outbound Tlon text filtering before DM or group delivery, which can affect visible message contents.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes before/after terminal output from the production delivery path through Tlon bootstrap to captured Urbit HTTP poke payloads; no hosted ship round trip was run, but the changed sanitizer boundary is exercised.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes before/after terminal output from the production delivery path through Tlon bootstrap to captured Urbit HTTP poke payloads; no hosted ship round trip was run, but the changed sanitizer boundary is exercised.
Evidence reviewed

PR surface:

Source +2, Tests +21. Total +23 across 2 files.

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

What I checked:

  • Repository policy: Root and scoped extension policy were read fully; the applicable guidance requires bundled plugin code to stay on public SDK seams and PR review to inspect channel delivery boundaries beyond the diff. (AGENTS.md:1, c69724ef3b50)
  • Current-main Tlon gap: Current main's tlonChannelOutbound defines direct delivery and a text chunk limit but has no sanitizeText hook, so the PR is still necessary. (extensions/tlon/src/channel.ts:64, c69724ef3b50)
  • Latest-release Tlon gap: The latest release tag v2026.6.11 also lacks the Tlon sanitizer hook, so this is not already shipped. (extensions/tlon/src/channel.ts:64, e085fa1a3ffd)
  • Shared delivery hook contract: Shared outbound delivery only calls assistant-visible cleanup when the loaded outbound adapter provides sanitizeText, then writes the returned text into the payload. (src/infra/outbound/deliver.ts:434, c69724ef3b50)
  • SDK sanitizer contract: openclaw/plugin-sdk/text-chunking exports sanitizeAssistantVisibleText, and the shared sanitizer tests cover stripping internal tool trace failure lines while preserving visible text. (src/plugin-sdk/text-chunking.ts:46, c69724ef3b50)
  • PR implementation: The PR head imports the SDK sanitizer, adds sanitizeText to Tlon outbound, and tests stripping the banner while preserving ordinary prose. (extensions/tlon/src/channel.ts:68, 370326fb63ee)

Likely related people:

  • vincentkoc: Current blame for the Tlon outbound adapter, shared sanitizer regexes, and delivery-path surfaces points to recent refactor work that carried this boundary forward. (role: recent area contributor; confidence: medium; commits: 4115e832d833; files: extensions/tlon/src/channel.ts, src/infra/outbound/deliver.ts, src/shared/text/assistant-visible-text.ts)
  • arthyn: Authored the merged Tlon plugin expansion that changed the channel, send path, and Tlon docs that this PR now adjusts. (role: Tlon feature contributor; confidence: medium; commits: f4682742d9d1; files: extensions/tlon/src/channel.ts, extensions/tlon/src/urbit/send.ts, docs/channels/tlon.md)
  • masatohoshino: Authored the merged Slack sanitizer hook fix that established the same outbound adapter pattern, and has relevant prior work in this sanitizer cluster beyond this PR. (role: sibling sanitizer contributor; confidence: high; commits: cd6d0f9b00f0; files: extensions/slack/src/channel.ts)
  • ZengWen-DT: Authored merged Feishu and Mattermost fixes adding the same assistant-visible sanitizer hook for other channel adapters in the same umbrella cluster. (role: recent sibling-fix contributor; confidence: high; commits: 6378d5efcc0a, 1ef45448712f; files: extensions/feishu/src/channel.ts, extensions/mattermost/src/channel.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.
Review history (3 earlier review cycles)
  • reviewed 2026-07-08T14:55:28.746Z sha 370326f :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-08T15:02:44.881Z sha 370326f :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-08T15:19:28.394Z sha 370326f :: needs maintainer review before merge. :: none

@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. 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. 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 Jul 8, 2026
@steipete steipete self-assigned this Jul 10, 2026
@steipete steipete mentioned this pull request Jul 10, 2026
Merged
@steipete

steipete commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Thank you, @masatohoshino — this fix is now landed on main via #103450 as commit 1d205424494de6633857796ff56a3cc09f41552b.

I preserved your contribution with the original commit author name/email in Co-authored-by, and strengthened the regression coverage to exercise the real shared outbound path through the final Urbit poke payload for DMs, groups, and media captions.

The original branch could not be updated after syncing to current main: GitHub's signed createCommitOnBranch path refused the resulting request at its 45 MB payload limit. Allow edits by maintainers was already enabled, so no contributor setting change was needed; a maintainer replacement was the safe path.

Exact replacement-head CI passed, along with the focused before/after boundary proof, all 143 Tlon tests, the changed-surface gate, and a clean structured autoreview. Closing this PR as superseded by the credited landed replacement. The umbrella #90684 remains open for other channels.

@steipete steipete closed this Jul 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@steipete steipete

Labels

channel: tlon Channel integration: tlon 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: 🐚 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 @steipete