fix(msteams): strip internal tool-trace banners from outbound text#103692
fix(msteams): strip internal tool-trace banners from outbound text#103692ZOOWH wants to merge 7 commits into
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed July 14, 2026, 8:46 PM ET / July 15, 2026, 00:46 UTC. Summary PR surface: Source +2, Tests +133. Total +135 across 3 files. Reproducibility: yes. at source level: current main lacks the Microsoft Teams sanitizeText hook while the shared delivery pipeline only sanitizes when that hook exists. No real current-main Teams conversation was executed during this review. Review metrics: none identified. 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:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Use this branch’s private adapter hook and delivery-boundary tests as the preferred implementation shape, then add redacted live proof from a real Gateway and Microsoft Teams/Bot Framework conversation showing mixed prose preserved, internal banners removed, trace-only output suppressed, and clean prose unchanged before selecting one canonical PR. Do we have a high-confidence way to reproduce the issue? Yes at source level: current main lacks the Microsoft Teams sanitizeText hook while the shared delivery pipeline only sanitizes when that hook exists. No real current-main Teams conversation was executed during this review. Is this the best way to solve the issue? Yes on code shape: using the canonical sanitizer at the channel outbound adapter is the narrowest established solution and matches merged sibling channels. Merge readiness remains unresolved until real Teams proof and canonical-branch selection are complete. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 525db34f80f4. Label changesLabel justifications:
Evidence reviewedPR surface: Source +2, Tests +133. Total +135 across 3 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 (18 earlier review cycles; latest 8 shown)
|
|
@clawsweeper re-review
|
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review Added delivery-pipeline transport capture proof ( |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
2b2440d to
9576fb0
Compare
|
@clawsweeper re-review Replaced callback-only proof script with delivery-pipeline test:
OVERALL: 22 tests passed (19 channel + 3 delivery-pipeline) |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
9576fb0 to
465a118
Compare
|
@clawsweeper re-review Added transport-boundary proof: deliverOutboundPayloads → local HTTP server capture → sanitized text verified at the transport layer (matching tlon #103450 pattern). Rebated. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Add sanitizeAssistantVisibleText to both msteams outbound adapters (channel.ts and outbound.ts) so internal tool-trace banners are stripped before delivery to Teams users. Co-Authored-By: Claude <[email protected]>
The msteamsOutbound in outbound.ts is used as a runtime delegate (sendText, sendMedia, etc.), not as the delivery-pipeline outbound adapter. The sanitizeText hook on this delegate is never invoked by the delivery pipeline. The canonical hook on msteamsChannelOutbound in channel.ts is the one that actually strips banners.
Delete scripts/proof/msteams-sanitize-proof.mts (callback-only, never drives deliverOutboundPayloads) and add outbound-tool-trace-sanitize.test.ts which exercises the shared delivery pipeline through the send-dependency boundary. Co-Authored-By: Claude <[email protected]>
deliverOutboundPayloads drives the msteams channel with a send dependency that POSTs to a local HTTP server — capturing the exact text that would reach the Bot Framework API. Proves tool-trace banners are stripped and trace-only payloads suppressed before the transport layer.
Co-authored-by: Cursor <[email protected]>
465a118 to
e026313
Compare
Update for re-review
Live Gateway + Bot Framework Teams session capture is still pending (no bot credentials in this environment). Please re-evaluate patch correctness now that the synthetic transport script is gone. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
What Problem This Solves
Microsoft Teams outbound adapter did not strip internal tool-trace banners
from assistant-visible text before delivery. Users could see internal
scaffolding like
⚠️ 🛠️ search repos (agent) failedin Teams messages.Why This Change Was Made
Added
sanitizeAssistantVisibleText(the canonical shared sanitizer) tomsteamsChannelOutboundvia thesanitizeTexthook. This matches the fixalready applied to tlon (#103450) and synology-chat (#102925).
Removed the synthetic
scripts/proof/msteams-sanitize-transport.mtsscriptthat injected a custom HTTP sender through
deps.msteams, bypassing theproduction
sendMessageMSTeams/Bot Framework path.User Impact
Before: internal tool-trace banners visible in Teams messages
After: internal scaffolding stripped, only user-visible prose delivered
Evidence
Branch:
fix/msteams-sanitize-outbound| Base:main| HEAD:e02631348aRebased onto latest
main. Diff is now 3 files (no proof script).Regression tests (delivery-pipeline send boundary)
Cases covered at the
deliverOutboundPayloads→ msteams send dependency boundary:Diff summary
Real behavior proof
Behavior addressed: Teams outbound text must not include internal tool-trace banners; trace-only assistant replies must not be delivered.
Environment tested: Local OpenClaw checkout on Windows (Node 24), msteams extension Vitest lane (
vitest.extension-msteams.config.ts).Steps to reproduce: Run
node scripts/run-vitest.mjs extensions/msteams/src/outbound-tool-trace-sanitize.test.ts extensions/msteams/src/channel.test.tson branchfix/msteams-sanitize-outbound.Before behavior: Mixed prose +
⚠️ 🛠️ \search repos (agent)` failed` would pass unsanitized to the msteams send dependency; trace-only payloads could still invoke send.After behavior:
sanitizeTextstrips banners before send; trace-only payloads suppress delivery; clean prose is preserved.Proof artifact: Vitest output above (22/22 passed). Live Gateway + Bot Framework Teams session proof is not yet attached in this update; will follow once Teams bot credentials and an established conversation ref are available.