Skip to content

Strip lone surrogates from chat-surface replies at the gateway delivery boundary - #55310

Open
MaxFreedomPollard wants to merge 1 commit into
NousResearch:mainfrom
MaxFreedomPollard:fix/gateway-delivery-surrogate-sanitize
Open

Strip lone surrogates from chat-surface replies at the gateway delivery boundary#55310
MaxFreedomPollard wants to merge 1 commit into
NousResearch:mainfrom
MaxFreedomPollard:fix/gateway-delivery-surrogate-sanitize

Conversation

@MaxFreedomPollard

@MaxFreedomPollard MaxFreedomPollard commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Rebased onto current main and extended per the sweeper review:

  • Final-response boundary (original scope): _sanitize_gateway_final_response surrogate-sanitizes the delivered copy; the stored history copy was already sanitized in build_assistant_message. The interrupt-sentinel fast path added on main (Bug: Telegram users can receive internal system messages like home-channel setup and interrupt status text #7921) is preserved ahead of the sanitize, as requested.
  • Streaming path (review ask): raw deltas used to reach GatewayStreamConsumer unchanged, so Telegram's strict UTF-16 length/edit paths could raise UnicodeEncodeError mid-stream before the boundary sanitizer ran. Deltas are now sanitized at the consumer's on_delta entry — the single chokepoint every delta source (gateway stream callback and the proxy SSE path) funnels through, which also makes the behavior directly unit-testable.

Regressions: chat-surface sanitize + raw-programmatic-surface passthrough at the boundary, and a streaming test asserting the queued delta content is UTF-16 encodable.

Repro/report: #55309.

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery platform/telegram Telegram bot adapter P2 Medium — degraded but workaround exists sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jun 30, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: LGTM

Small fix for lone surrogates crashing Telegram utf16_len encoding.

Looks Good

  • Reuses existing _sanitize_surrogates from message_sanitization module
  • Only affects chat surfaces (raw text passthrough preserved)
  • Test verifies both chat and raw surfaces
  • Complements existing surrogate handling in build_assistant_message

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tracing the raw final-response path; the non-streaming premise is verified on current main.

Problems

  • With streaming enabled, gateway/run.py:18123-18125 forwards raw deltas into GatewayStreamConsumer.on_delta, which queues them unchanged at gateway/stream_consumer.py:370-379. The consumer later calls the adapter edit path (gateway/stream_consumer.py:1771-1774), while Telegram measures content through strict UTF-16 encoding (plugins/platforms/telegram/adapter.py:3980, gateway/platforms/base.py:145). A lone surrogate can therefore still fail before _sanitize_gateway_final_response() is reached.

Suggested changes

  • Retain the final-response boundary fix, and sanitize stream deltas before they enter GatewayStreamConsumer; add a streaming regression that verifies the Telegram-facing content is UTF-16 encodable.
  • Current gateway/run.py:429-432 adds an interrupt-sentinel fast path inside this hunk; preserve it when resolving the current conflict.

Automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 15, 2026
@MaxFreedomPollard
MaxFreedomPollard force-pushed the fix/gateway-delivery-surrogate-sanitize branch 3 times, most recently from 47f78fd to 512b562 Compare July 25, 2026 05:28
…eam deltas

Two delivery paths could hand Telegram text that is not UTF-16 encodable
(a truncated emoji half from some local-model backends), aborting
delivery with UnicodeEncodeError:

1. Final-response boundary: _sanitize_gateway_final_response now
   surrogate-sanitizes the delivered copy (the stored history copy was
   already sanitized in build_assistant_message). The interrupt-sentinel
   fast path added on main (NousResearch#7921) is preserved ahead of it.
2. Streaming: raw deltas reached GatewayStreamConsumer unchanged, so the
   strict UTF-16 length/edit paths could crash mid-stream before the
   boundary sanitizer ever ran. Sanitize at the consumer's on_delta
   entry point - the single chokepoint every delta source (gateway
   callback, proxy SSE) funnels through.

Regressions: boundary tests (chat surface sanitized, raw programmatic
surface untouched) and a streaming test asserting the queued delta is
UTF-16 encodable.
@MaxFreedomPollard
MaxFreedomPollard force-pushed the fix/gateway-delivery-surrogate-sanitize branch from 512b562 to c068898 Compare July 31, 2026 03:52
@swissly

swissly commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

📌 Overlap note: opened #77395 (fix(agent): close unclosed JSON tool-call args in LIFO order). Touches agent/message_sanitization.py — unrelated concern (repair-correctness vs surrogate stripping), but same file; flagging for coordination.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/telegram Telegram bot adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants