fix(synology-chat): strip internal tool-trace banners from outbound text by Pick-cat · Pull Request #102925 · openclaw/openclaw

Pick-cat · 2026-07-09T14:51:05Z

What Problem This Solves

Synology Chat outbound delivery omitted the channel sanitizer hook, so internal assistant tool-trace banners such as “⚠️ 🛠️ search repos (agent) failed” could reach end users. A trace-only reply was also sent as a standalone webhook message.

Why This Change Was Made

The Synology Chat outbound adapter now uses the canonical sanitizeAssistantVisibleText hook before transport, matching the existing channel-owned delivery contract. The adapter type reuses ChannelOutboundAdapter["sanitizeText"] instead of maintaining a narrower local callback shape that could drift from ReplyPayload.

This is the narrowest correct owner boundary: the shared durable-delivery pipeline invokes the explicit channel hook, then drops payloads that become empty before platform I/O. A global default would change channels that intentionally preserve different text, while sanitizing inside the HTTP client would be too late and would duplicate delivery policy.

User Impact

Mixed replies keep visible prose and remove internal tool-trace banners.
Trace-only replies produce no Synology webhook request.
Normal prose is unchanged.

Evidence

Exact rewritten head: a416d86e0d6caafbec575c6f838b6462ae424bd3.
Blacksmith Testbox tbx_01kx56yss8pk3ze695em4bz009 (harbor-barnacle), Linux Node 24.
Real local Gateway + actual openclaw message send --channel synology-chat + real HTTP webhook capture:
- current main posted all three inputs verbatim: mixed prose/banner, trace-only banner, and normal prose;
- rewritten head posted Done. and the unchanged normal prose;
- rewritten head made no HTTP request for the trace-only payload.
pnpm test extensions/synology-chat/src/channel.test.ts extensions/synology-chat/src/client.test.ts — 62/62 passed.
Exact rewritten-head remote check:changed — passed, including TypeScript, extension/core lint, import cycles, package guards, and repository policy checks.
Fresh autoreview — clean, no accepted/actionable findings (correctness confidence 0.98).

The environment has no Synology NAS webhook credentials or hardware, so the final dependency hop into a physical NAS was unavailable. The real Gateway, plugin loader, durable-delivery sanitizer/drop path, CLI client, Synology HTTP transport, recipient encoding, and webhook request were exercised end to end against a local endpoint. No screenshot is useful for this non-visual transport fix; the captured request bodies are the behavioral proof.

clawsweeper · 2026-07-09T15:13:50Z

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

Summary
The PR adds Synology Chat outbound text sanitization through sanitizeAssistantVisibleText and covers tool-trace stripping in the channel tests.

PR surface: Source +2, Tests +26. Total +28 across 2 files.

Reproducibility: yes. Current main source shows Synology Chat lacks the sanitizeText hook, and the shared delivery path only runs assistant-visible cleanup when that hook exists; the PR body also provides terminal output showing the proposed hook strips the banner.

Review metrics: none identified.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #90684
Summary: This PR is a channel-specific candidate fix for the Synology Chat slice of the open non-Discord outbound sanitizer umbrella.

Members:

canonical: Feishu (and non-Discord channels) should apply sanitizeAssistantVisibleText() on outbound text #90684 - The PR body says it is part of the umbrella issue, and the issue tracks missing sanitizeAssistantVisibleText coverage across non-Discord channels.
partial_overlap: fix(feishu): strip internal tool-trace banners from outbound text #98705 - Merged Feishu sibling PR fixed the same sanitizer gap pattern for a different channel.
partial_overlap: fix(line): strip internal tool-trace banners from outbound text #101708 - Open LINE sibling PR targets the same umbrella sanitizer gap for a different channel adapter.
partial_overlap: fix(nextcloud-talk): strip internal tool-trace banners from outbound text #101712 - Open Nextcloud Talk sibling PR targets the same umbrella sanitizer gap for a different channel adapter.

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] No live Synology Chat NAS round trip is attached, so service-level confidence relies on exported hook proof, the shared outbound delivery contract, and existing sibling channel coverage.

Maintainer options:

Decide the mitigation before merge
Land the narrow Synology Chat sanitizer hook with regression coverage, while keeping the umbrella issue open for the remaining channel adapters and any shared-boundary decision.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

No ClawSweeper repair is needed; the remaining action is ordinary maintainer review and exact-head merge gating for this contributor PR.

Security
Cleared: The diff only wires an existing outbound sanitizer into Synology Chat and adds tests; it introduces no new dependency, secret handling, workflow, or supply-chain surface.

Review details

Best possible solution:

Land the narrow Synology Chat sanitizer hook with regression coverage, while keeping the umbrella issue open for the remaining channel adapters and any shared-boundary decision.

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

Yes. Current main source shows Synology Chat lacks the sanitizeText hook, and the shared delivery path only runs assistant-visible cleanup when that hook exists; the PR body also provides terminal output showing the proposed hook strips the banner.

Is this the best way to solve the issue?

Yes. This is the best narrow slice for Synology Chat: it reuses the existing SDK sanitizer hook already used by sibling channels, adds no config or new API surface, and tests the exported plugin hook directly.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add P1: The PR fixes internal assistant/tool diagnostic text leaking into a real messaging channel, matching the open P1 umbrella channel-sanitizer issue.
add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes terminal output importing the exported Synology Chat plugin hook and showing the after-fix sanitizer strips the banner while preserving normal prose.
add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes terminal output importing the exported Synology Chat plugin hook and showing the after-fix sanitizer strips the banner while preserving normal prose.

Label justifications:

P1: The PR fixes internal assistant/tool diagnostic text leaking into a real messaging channel, matching the open P1 umbrella channel-sanitizer issue.
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 terminal output importing the exported Synology Chat plugin hook and showing the after-fix sanitizer strips the banner while preserving normal prose.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes terminal output importing the exported Synology Chat plugin hook and showing the after-fix sanitizer strips the banner while preserving normal prose.

Evidence reviewed

PR surface:

Source +2, Tests +26. Total +28 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	3	1	+2
Tests	1	26	0	+26
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	29	1	+28

What I checked:

Current main Synology outbound gap: Current main's Synology Chat outbound adapter has deliveryMode, textChunkLimit, sendText, and sendMedia, but no sanitizeText hook, so the shared sanitizer hook cannot run for this channel. (extensions/synology-chat/src/channel.ts:441, 12ceefcb0393)
Shared delivery contract: The shared outbound delivery path only calls a channel sanitizer when the loaded outbound adapter provides sanitizeText, then writes the sanitized text back into the payload before normalization. (src/infra/outbound/deliver.ts:435, 12ceefcb0393)
PR hook implementation: The branch imports the existing SDK sanitizer and adds sanitizeText: ({ text }) => sanitizeAssistantVisibleText(text) to the Synology Chat outbound adapter. (extensions/synology-chat/src/channel.ts:443, 26ba7848e13a)
PR regression coverage: The branch adds tests proving the Synology Chat hook strips the internal tool-trace banner, preserves normal prose, and returns an empty string for trace-only replies. (extensions/synology-chat/src/channel.test.ts:558, 26ba7848e13a)
Sibling accepted pattern: Feishu already exposes the same sanitizeAssistantVisibleText outbound hook pattern with focused tool-trace regression tests, supporting this PR's narrow per-channel approach. (extensions/feishu/src/channel.ts:1430, 12ceefcb0393)
Canonical related issue: The linked umbrella issue remains open for non-Discord channel sanitizer coverage and explicitly lists this class of internal scaffolding leaks as the central problem.

Likely related people:

Jean-Marc: Authored the original Synology Chat native channel commit that added the central plugin files now being patched. (role: introduced behavior; confidence: high; commits: 03586e3d0057; files: extensions/synology-chat/src/channel.ts, extensions/synology-chat/src/channel.test.ts)
Vincent Koc: Current-main blame and recent history route the Synology Chat files and shared outbound sanitizer contract through a broad current-tree refactor boundary. (role: recent area contributor; confidence: medium; commits: cb7468fc58ac, e069169765c9; files: extensions/synology-chat/src/channel.ts, src/channels/plugins/outbound.types.ts, src/infra/outbound/deliver.ts)
ZengWen-DT: Authored merged sibling sanitizer fixes for Feishu and Mattermost that use the same per-channel hook pattern this PR follows. (role: sibling-fix contributor; confidence: high; commits: 6378d5efcc0a, 1ef45448712f; files: extensions/feishu/src/channel.ts, extensions/mattermost/src/channel.ts)
masatohoshino: Authored merged Slack sanitizer coverage that helped establish the current per-channel sanitizer hook pattern for this umbrella issue. (role: sibling-fix contributor; confidence: high; commits: cd6d0f9b00f0; files: extensions/slack/src/channel.ts, extensions/slack/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.

steipete · 2026-07-10T05:48:52Z

Land-ready maintainer proof for exact head a416d86e0d6caafbec575c6f838b6462ae424bd3:

Rebased onto current main and replaced the local narrow sanitizer signature with the canonical ChannelOutboundAdapter["sanitizeText"] contract.
Blacksmith Testbox tbx_01kx56yss8pk3ze695em4bz009 on Linux Node 24: focused Synology channel/client tests passed 62/62; exact-head check:changed passed.
Real local Gateway E2E through actual openclaw message send and the Synology HTTP client: current main posted the mixed banner, trace-only banner, and prose verbatim. This head posted only Done. and unchanged prose; the trace-only input produced no webhook request.
Fresh autoreview found no accepted/actionable findings (correctness confidence 0.98).
A physical Synology NAS roundtrip remains unavailable because this environment has no NAS webhook credentials/hardware. The complete OpenClaw/Gateway/plugin/durable-delivery/HTTP path was exercised against a real local capture endpoint. No screenshot is useful for this non-visual transport change.

No remaining code or proof blocker found.

steipete · 2026-07-10T05:56:24Z

Merged via squash.

Prepared head SHA: a416d86e0d6caafbec575c6f838b6462ae424bd3
Landed commit: 7eb163d90582895150403fa320635a3ef85c81bd

Align with the pattern steipete landed in openclaw#102925: type sanitizeText as NonNullable<ChannelOutboundAdapter["sanitizeText"]> instead of the inherited optional Pick shape, and drop the now-redundant optional chaining on sanitizeText in the tests.

…ext (openclaw#102925) * fix(synology-chat): strip internal tool-trace banners from outbound text * fix(synology-chat): satisfy eslint curly rule * fix(synology-chat): add sanitizeText to outbound type contract * fix(synology-chat): use canonical outbound sanitizer type --------- Co-authored-by: Pick-cat <[email protected]> Co-authored-by: Peter Steinberger <[email protected]>

sanitizeText inherited the optional modifier from Pick<ChannelOutboundAdapter, "sanitizeText">, so even with the NonNullable intersection TS still inferred it as possibly undefined (TS2722 in the tests). Drop it from the Pick union so it is declared only via NonNullable<...> in the intersection, matching the steipete openclaw#102925 pattern.

Align with the pattern steipete landed in openclaw#102925: type sanitizeText as NonNullable<ChannelOutboundAdapter["sanitizeText"]> instead of the inherited optional Pick shape, and drop the now-redundant optional chaining on sanitizeText in the tests.

sanitizeText inherited the optional modifier from Pick<ChannelOutboundAdapter, "sanitizeText">, so even with the NonNullable intersection TS still inferred it as possibly undefined (TS2722 in the tests). Drop it from the Pick union so it is declared only via NonNullable<...> in the intersection, matching the steipete openclaw#102925 pattern.

Align with the pattern steipete landed in openclaw#102925: type sanitizeText as NonNullable<ChannelOutboundAdapter["sanitizeText"]> instead of the inherited optional Pick shape, and drop the now-redundant optional chaining on sanitizeText in the tests.

sanitizeText inherited the optional modifier from Pick<ChannelOutboundAdapter, "sanitizeText">, so even with the NonNullable intersection TS still inferred it as possibly undefined (TS2722 in the tests). Drop it from the Pick union so it is declared only via NonNullable<...> in the intersection, matching the steipete openclaw#102925 pattern.

Align with the pattern steipete landed in openclaw#102925: type sanitizeText as NonNullable<ChannelOutboundAdapter["sanitizeText"]> instead of the inherited optional Pick shape, and drop the now-redundant optional chaining on sanitizeText in the tests.

sanitizeText inherited the optional modifier from Pick<ChannelOutboundAdapter, "sanitizeText">, so even with the NonNullable intersection TS still inferred it as possibly undefined (TS2722 in the tests). Drop it from the Pick union so it is declared only via NonNullable<...> in the intersection, matching the steipete openclaw#102925 pattern.

openclaw-barnacle Bot added channel: synology-chat size: XS labels Jul 9, 2026

steipete self-assigned this Jul 10, 2026

Pick-cat and others added 4 commits July 10, 2026 06:21

fix(synology-chat): strip internal tool-trace banners from outbound text

14c6af0

fix(synology-chat): satisfy eslint curly rule

aea6409

fix(synology-chat): add sanitizeText to outbound type contract

3e7008a

fix(synology-chat): use canonical outbound sanitizer type

a416d86

steipete force-pushed the fix/synology-chat-sanitize-outbound-90684 branch from 26ba784 to a416d86 Compare July 10, 2026 05:45

steipete merged commit 7eb163d into openclaw:main Jul 10, 2026
96 checks passed

github-actions Bot mentioned this pull request Jul 10, 2026

📡 Upstream Digest — 2026-07-10 07:27 UTC curtismercier/openclaw-mods#1249

Open

ZOOWH mentioned this pull request Jul 10, 2026

fix(msteams): strip internal tool-trace banners from outbound text #103692

Closed

2 tasks

clawsweeper Bot mentioned this pull request Jul 10, 2026

fix(nextcloud-talk): strip internal tool-trace banners from outbound text #101712

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(synology-chat): strip internal tool-trace banners from outbound text#102925

fix(synology-chat): strip internal tool-trace banners from outbound text#102925
steipete merged 4 commits into
openclaw:mainfrom
Pick-cat:fix/synology-chat-sanitize-outbound-90684

Pick-cat commented Jul 9, 2026 •

edited by steipete

Loading

Uh oh!

clawsweeper Bot commented Jul 9, 2026 •

edited

Loading

Uh oh!

steipete commented Jul 10, 2026

Uh oh!

Uh oh!

steipete commented Jul 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

Pick-cat commented Jul 9, 2026 • edited by steipete Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

clawsweeper Bot commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

steipete commented Jul 10, 2026

Uh oh!

Uh oh!

steipete commented Jul 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Pick-cat commented Jul 9, 2026 •

edited by steipete

Loading

clawsweeper Bot commented Jul 9, 2026 •

edited

Loading