From ffac8d3b0af1bb13a3a3f5e23153d0c052021d2a Mon Sep 17 00:00:00 2001 From: flobo3 Date: Sun, 19 Apr 2026 20:28:32 +0300 Subject: [PATCH 1/8] fix: deduplicate SPF/DKIM-rejected emails to stop log spam --- nanobot/channels/email.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/nanobot/channels/email.py b/nanobot/channels/email.py index 681e71ef0..ea30a8e58 100644 --- a/nanobot/channels/email.py +++ b/nanobot/channels/email.py @@ -395,6 +395,10 @@ class EmailChannel(BaseChannel): "(no 'spf=pass' in Authentication-Results header)", sender, ) + if uid: + cycle_uids.add(uid) + if dedupe: + self._processed_uids.add(uid) continue if self.config.verify_dkim and not dkim_pass: logger.warning( @@ -402,6 +406,10 @@ class EmailChannel(BaseChannel): "(no 'dkim=pass' in Authentication-Results header)", sender, ) + if uid: + cycle_uids.add(uid) + if dedupe: + self._processed_uids.add(uid) continue subject = self._decode_header_value(parsed.get("Subject", "")) From ecfbb0ed4fe1ce0edcddaf196114e0c2e39778f1 Mon Sep 17 00:00:00 2001 From: chengyongru Date: Mon, 20 Apr 2026 14:50:44 +0800 Subject: [PATCH 2/8] refactor(email): use _remember_processed_uid in SPF/DKIM reject paths Replaces inline dedup logic with the existing helper to match the style of _is_self_address and other reject branches, and to keep the _processed_uids eviction logic in one place. --- nanobot/channels/email.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/nanobot/channels/email.py b/nanobot/channels/email.py index ea30a8e58..5b5856560 100644 --- a/nanobot/channels/email.py +++ b/nanobot/channels/email.py @@ -395,10 +395,7 @@ class EmailChannel(BaseChannel): "(no 'spf=pass' in Authentication-Results header)", sender, ) - if uid: - cycle_uids.add(uid) - if dedupe: - self._processed_uids.add(uid) + self._remember_processed_uid(uid, dedupe, cycle_uids) continue if self.config.verify_dkim and not dkim_pass: logger.warning( @@ -406,10 +403,7 @@ class EmailChannel(BaseChannel): "(no 'dkim=pass' in Authentication-Results header)", sender, ) - if uid: - cycle_uids.add(uid) - if dedupe: - self._processed_uids.add(uid) + self._remember_processed_uid(uid, dedupe, cycle_uids) continue subject = self._decode_header_value(parsed.get("Subject", "")) From 297b852f6ef05d95d2fe20936b3121433ec29f31 Mon Sep 17 00:00:00 2001 From: jhkim43 Date: Sat, 11 Apr 2026 15:49:12 +0900 Subject: [PATCH 3/8] feat(telegram): change to mid-stream split per review feedback(#2967 PR) --- nanobot/channels/telegram.py | 64 +++++++++++++++++++++++++++++------- 1 file changed, 52 insertions(+), 12 deletions(-) diff --git a/nanobot/channels/telegram.py b/nanobot/channels/telegram.py index f63704aa7..26a77acb0 100644 --- a/nanobot/channels/telegram.py +++ b/nanobot/channels/telegram.py @@ -626,19 +626,59 @@ class TelegramChannel(BaseChannel): logger.warning("Stream initial send failed: {}", e) raise # Let ChannelManager handle retry elif (now - buf.last_edit) >= self.config.stream_edit_interval: - try: - await self._call_with_retry( - self._app.bot.edit_message_text, - chat_id=int_chat_id, message_id=buf.message_id, - text=buf.text, - ) - buf.last_edit = now - except Exception as e: - if self._is_not_modified_error(e): + if len(buf.text) > TELEGRAM_MAX_MESSAGE_LEN: + # Finish current message + current_text = buf.text[:TELEGRAM_MAX_MESSAGE_LEN] + try: + await self._call_with_retry( + self._app.bot.edit_message_text, + chat_id=int_chat_id, + message_id=buf.message_id, + text=current_text, + ) + except Exception as e: + logger.warning("Failed to edit current message before splitting: {}", e) + raise # Let ChannelManager handle retry + + # Prepare remaining content for a new message + remaining = buf.text[TELEGRAM_MAX_MESSAGE_LEN:] + logger.debug(f"[!] Splitting long message: {len(buf.text)} chars → new message with {len(remaining)} chars") + + # Create new buffer for the next chunk + self._stream_bufs[chat_id] = _StreamBuf(stream_id=stream_id) + new_buf = self._stream_bufs[chat_id] + new_buf.text = remaining + new_buf.last_edit = now + + # Immediately start the new message + if remaining.strip(): + try: + sent = await self._call_with_retry( + self._app.bot.send_message, + chat_id=int_chat_id, + text=remaining[:TELEGRAM_MAX_MESSAGE_LEN], + **thread_kwargs + ) + new_buf.message_id = sent.message_id + except Exception as e: + logger.error("Failed to send new message chunk after split: {}", e) + raise # Let ChannelManager handle retry + else: + # Normal edit (message is still under the limit) + try: + await self._call_with_retry( + self._app.bot.edit_message_text, + chat_id=int_chat_id, + message_id=buf.message_id, + text=buf.text, + ) buf.last_edit = now - return - logger.warning("Stream edit failed: {}", e) - raise # Let ChannelManager handle retry + except Exception as e: + if self._is_not_modified_error(e): + buf.last_edit = now + return + logger.warning("Stream edit failed: {}", e) + raise # Let ChannelManager handle retry async def _on_start(self, update: Update, context: ContextTypes.DEFAULT_TYPE) -> None: """Handle /start command.""" From fd8f08cc83da000a928891815dcfeb301bf9abe0 Mon Sep 17 00:00:00 2001 From: himax12 Date: Mon, 20 Apr 2026 11:32:24 +0800 Subject: [PATCH 4/8] fix(telegram): convert markdown to HTML before splitting to avoid message length overflow Cherry-picked from #3316 (himax12). When streaming completes in send_delta(), the code was splitting raw markdown text by 4000, then converting to HTML. The markdown-to-HTML conversion adds 10-33% characters, which could push the result over Telegram's 4096 character limit. The fix converts markdown to HTML first, then splits by 4096 (actual Telegram limit), ensuring the edited message always fits. Fixes #3315 --- nanobot/channels/telegram.py | 29 ++++++++----- tests/channels/test_telegram_channel.py | 54 ++++++++++++++++++++++--- 2 files changed, 69 insertions(+), 14 deletions(-) diff --git a/nanobot/channels/telegram.py b/nanobot/channels/telegram.py index 26a77acb0..d2265e386 100644 --- a/nanobot/channels/telegram.py +++ b/nanobot/channels/telegram.py @@ -561,14 +561,22 @@ class TelegramChannel(BaseChannel): await self._remove_reaction(chat_id, int(reply_to_message_id)) except ValueError: pass - chunks = split_message(buf.text, TELEGRAM_MAX_MESSAGE_LEN) - primary_text = chunks[0] if chunks else buf.text + thread_kwargs = {} + if message_thread_id := meta.get("message_thread_id"): + thread_kwargs["message_thread_id"] = message_thread_id + html = _markdown_to_telegram_html(buf.text) + if len(html) <= 4096: + primary_html = html + extra_html_chunks = [] + else: + html_chunks = split_message(html, 4096) + primary_html = html_chunks[0] + extra_html_chunks = html_chunks[1:] try: - html = _markdown_to_telegram_html(primary_text) await self._call_with_retry( self._app.bot.edit_message_text, chat_id=int_chat_id, message_id=buf.message_id, - text=html, parse_mode="HTML", + text=primary_html, parse_mode="HTML", ) except BadRequest as e: # Only fall back to plain text on actual HTML parse/format errors. @@ -583,7 +591,7 @@ class TelegramChannel(BaseChannel): await self._call_with_retry( self._app.bot.edit_message_text, chat_id=int_chat_id, message_id=buf.message_id, - text=primary_text, + text=primary_html, ) except Exception as e2: if self._is_not_modified_error(e2): @@ -591,10 +599,13 @@ class TelegramChannel(BaseChannel): else: logger.warning("Final stream edit failed: {}", e2) raise # Let ChannelManager handle retry - # If final content exceeds Telegram limit, keep the first chunk in - # the edited stream message and send the rest as follow-up messages. - for extra_chunk in chunks[1:]: - await self._send_text(int_chat_id, extra_chunk) + for extra_html_chunk in extra_html_chunks: + await self._call_with_retry( + self._app.bot.send_message, + chat_id=int_chat_id, text=extra_html_chunk, + parse_mode="HTML", + **thread_kwargs, + ) self._stream_bufs.pop(chat_id, None) return diff --git a/tests/channels/test_telegram_channel.py b/tests/channels/test_telegram_channel.py index 8d9431ba6..6b24bc1ac 100644 --- a/tests/channels/test_telegram_channel.py +++ b/tests/channels/test_telegram_channel.py @@ -467,8 +467,45 @@ async def test_send_delta_stream_end_falls_back_on_bad_request() -> None: @pytest.mark.asyncio async def test_send_delta_stream_end_splits_oversized_reply() -> None: - """Final streamed reply exceeding Telegram limit is split into chunks.""" - from nanobot.channels.telegram import TELEGRAM_MAX_MESSAGE_LEN + """Final streamed reply exceeding Telegram limit is split into chunks. + + The fix converts markdown to HTML first, then splits by 4096 (actual Telegram + limit), ensuring the edited message always fits within Telegram's constraint. + Previously, the code split by 4000 (TELEGRAM_MAX_MESSAGE_LEN) before HTML + conversion, which could still overflow when HTML tags were added. + """ + channel = TelegramChannel( + TelegramConfig(enabled=True, token="123:abc", allow_from=["*"]), + MessageBus(), + ) + channel._app = _FakeApp(lambda: None) + channel._app.bot.edit_message_text = AsyncMock() + channel._app.bot.send_message = AsyncMock(return_value=SimpleNamespace(message_id=99)) + + oversized = "x" * (4000 + 500) + channel._stream_bufs["123"] = _StreamBuf(text=oversized, message_id=7, last_edit=0.0) + + await channel.send_delta("123", "", {"_stream_end": True}) + + channel._app.bot.edit_message_text.assert_called_once() + edit_text = channel._app.bot.edit_message_text.call_args.kwargs.get("text", "") + assert len(edit_text) <= 4096, f"edit_text length {len(edit_text)} exceeds Telegram's 4096 limit" + + channel._app.bot.send_message.assert_called_once() + send_text = channel._app.bot.send_message.call_args.kwargs.get("text", "") + assert len(send_text) <= 4096 + assert "123" not in channel._stream_bufs + + +async def test_send_delta_stream_end_html_expansion_does_not_overflow() -> None: + """Markdown that expands when converted to HTML is still split correctly. + + This is the actual bug from issue #3315: markdown like **bold** expands to + bold, adding ~33% characters. A 3600-char message with heavy markdown + could become 4800+ chars after HTML conversion, exceeding 4096 limit. + The fix converts to HTML first, THEN splits by 4096. + """ + from nanobot.channels.telegram import _markdown_to_telegram_html channel = TelegramChannel( TelegramConfig(enabled=True, token="123:abc", allow_from=["*"]), @@ -478,14 +515,21 @@ async def test_send_delta_stream_end_splits_oversized_reply() -> None: channel._app.bot.edit_message_text = AsyncMock() channel._app.bot.send_message = AsyncMock(return_value=SimpleNamespace(message_id=99)) - oversized = "x" * (TELEGRAM_MAX_MESSAGE_LEN + 500) - channel._stream_bufs["123"] = _StreamBuf(text=oversized, message_id=7, last_edit=0.0) + markdown_text = "**bold** " * 400 # 3600 chars raw, expands ~33% to 4800 HTML + raw_len = len(markdown_text) + html_len = len(_markdown_to_telegram_html(markdown_text)) + assert html_len > 4096, f"Test precondition failed: HTML should exceed 4096 (was {html_len})" + + channel._stream_bufs["123"] = _StreamBuf(text=markdown_text, message_id=7, last_edit=0.0) await channel.send_delta("123", "", {"_stream_end": True}) channel._app.bot.edit_message_text.assert_called_once() edit_text = channel._app.bot.edit_message_text.call_args.kwargs.get("text", "") - assert len(edit_text) <= TELEGRAM_MAX_MESSAGE_LEN + assert len(edit_text) <= 4096, ( + f"HTML text length {len(edit_text)} exceeds Telegram's 4096 limit. " + f"Raw was {raw_len}, HTML was {html_len}." + ) channel._app.bot.send_message.assert_called_once() assert "123" not in channel._stream_bufs From 2eea82f5ee0941a32f7e9d766fa4bc41f02a4a86 Mon Sep 17 00:00:00 2001 From: stutiredboy Date: Mon, 20 Apr 2026 11:33:44 +0800 Subject: [PATCH 5/8] fix(telegram): split oversized stream buffer mid-flight Cherry-picked from #3311 (stutiredboy). Streaming edits called edit_message_text(text=buf.text) without chunking, so once accumulated deltas crossed Telegram's 4096-char limit an ongoing stream would fail with BadRequest. Extracts _flush_stream_overflow helper that edits the first chunk in place, sends any middle chunks, and re-anchors the buffer to a new message for the tail so subsequent deltas keep streaming. Co-Authored-By: stutiredboy --- nanobot/channels/telegram.py | 104 ++++++++++++------------ tests/channels/test_telegram_channel.py | 33 ++++++++ 2 files changed, 86 insertions(+), 51 deletions(-) diff --git a/nanobot/channels/telegram.py b/nanobot/channels/telegram.py index d2265e386..bfb7e70a9 100644 --- a/nanobot/channels/telegram.py +++ b/nanobot/channels/telegram.py @@ -638,58 +638,60 @@ class TelegramChannel(BaseChannel): raise # Let ChannelManager handle retry elif (now - buf.last_edit) >= self.config.stream_edit_interval: if len(buf.text) > TELEGRAM_MAX_MESSAGE_LEN: - # Finish current message - current_text = buf.text[:TELEGRAM_MAX_MESSAGE_LEN] - try: - await self._call_with_retry( - self._app.bot.edit_message_text, - chat_id=int_chat_id, - message_id=buf.message_id, - text=current_text, - ) - except Exception as e: - logger.warning("Failed to edit current message before splitting: {}", e) - raise # Let ChannelManager handle retry - - # Prepare remaining content for a new message - remaining = buf.text[TELEGRAM_MAX_MESSAGE_LEN:] - logger.debug(f"[!] Splitting long message: {len(buf.text)} chars → new message with {len(remaining)} chars") - - # Create new buffer for the next chunk - self._stream_bufs[chat_id] = _StreamBuf(stream_id=stream_id) - new_buf = self._stream_bufs[chat_id] - new_buf.text = remaining - new_buf.last_edit = now - - # Immediately start the new message - if remaining.strip(): - try: - sent = await self._call_with_retry( - self._app.bot.send_message, - chat_id=int_chat_id, - text=remaining[:TELEGRAM_MAX_MESSAGE_LEN], - **thread_kwargs - ) - new_buf.message_id = sent.message_id - except Exception as e: - logger.error("Failed to send new message chunk after split: {}", e) - raise # Let ChannelManager handle retry - else: - # Normal edit (message is still under the limit) - try: - await self._call_with_retry( - self._app.bot.edit_message_text, - chat_id=int_chat_id, - message_id=buf.message_id, - text=buf.text, - ) + await self._flush_stream_overflow(int_chat_id, buf, thread_kwargs) + buf.last_edit = now + return + try: + await self._call_with_retry( + self._app.bot.edit_message_text, + chat_id=int_chat_id, message_id=buf.message_id, + text=buf.text, + ) + buf.last_edit = now + except Exception as e: + if self._is_not_modified_error(e): buf.last_edit = now - except Exception as e: - if self._is_not_modified_error(e): - buf.last_edit = now - return - logger.warning("Stream edit failed: {}", e) - raise # Let ChannelManager handle retry + return + logger.warning("Stream edit failed: {}", e) + raise # Let ChannelManager handle retry + + async def _flush_stream_overflow( + self, + chat_id: int, + buf: "_StreamBuf", + thread_kwargs: dict, + ) -> None: + """Split an oversized stream buffer mid-flight. + + Edits the current stream message with the first chunk, sends any + intermediate chunks as standalone messages, then opens a new message + for the tail so subsequent deltas continue streaming into it. + """ + chunks = split_message(buf.text, TELEGRAM_MAX_MESSAGE_LEN) + if len(chunks) <= 1: + return + try: + await self._call_with_retry( + self._app.bot.edit_message_text, + chat_id=chat_id, message_id=buf.message_id, + text=chunks[0], + ) + except Exception as e: + if not self._is_not_modified_error(e): + logger.warning("Stream overflow edit failed: {}", e) + raise + for chunk in chunks[1:-1]: + await self._call_with_retry( + self._app.bot.send_message, + chat_id=chat_id, text=chunk, **thread_kwargs, + ) + tail = chunks[-1] + sent = await self._call_with_retry( + self._app.bot.send_message, + chat_id=chat_id, text=tail, **thread_kwargs, + ) + buf.message_id = sent.message_id + buf.text = tail async def _on_start(self, update: Update, context: ContextTypes.DEFAULT_TYPE) -> None: """Handle /start command.""" diff --git a/tests/channels/test_telegram_channel.py b/tests/channels/test_telegram_channel.py index 6b24bc1ac..c5cc3cf4e 100644 --- a/tests/channels/test_telegram_channel.py +++ b/tests/channels/test_telegram_channel.py @@ -574,6 +574,39 @@ async def test_send_delta_incremental_edit_treats_not_modified_as_success() -> N assert channel._stream_bufs["123"].last_edit > 0.0 +@pytest.mark.asyncio +async def test_send_delta_incremental_edit_splits_oversized_buffer() -> None: + """Mid-stream overflow: once buf.text exceeds Telegram's limit, split into + chunks, edit the current message with the first chunk, and re-anchor the + buffer to a new message for the tail so further deltas keep streaming.""" + from nanobot.channels.telegram import TELEGRAM_MAX_MESSAGE_LEN + + channel = TelegramChannel( + TelegramConfig(enabled=True, token="123:abc", allow_from=["*"]), + MessageBus(), + ) + channel._app = _FakeApp(lambda: None) + channel._app.bot.edit_message_text = AsyncMock() + channel._app.bot.send_message = AsyncMock(return_value=SimpleNamespace(message_id=99)) + + oversized = "x" * (TELEGRAM_MAX_MESSAGE_LEN + 500) + channel._stream_bufs["123"] = _StreamBuf( + text=oversized, message_id=7, last_edit=0.0, stream_id="s:0" + ) + + await channel.send_delta("123", "y", {"_stream_delta": True, "_stream_id": "s:0"}) + + channel._app.bot.edit_message_text.assert_called_once() + edit_text = channel._app.bot.edit_message_text.call_args.kwargs.get("text", "") + assert len(edit_text) <= TELEGRAM_MAX_MESSAGE_LEN + + channel._app.bot.send_message.assert_called_once() + buf = channel._stream_bufs["123"] + assert buf.message_id == 99 + assert len(buf.text) <= TELEGRAM_MAX_MESSAGE_LEN + assert buf.last_edit > 0.0 + + @pytest.mark.asyncio async def test_send_delta_initial_send_keeps_message_in_thread() -> None: channel = TelegramChannel( From f900c5bb8ee7e603ce97b6c43150e1d968f0c1f0 Mon Sep 17 00:00:00 2001 From: chengyongru Date: Mon, 20 Apr 2026 11:59:28 +0800 Subject: [PATCH 6/8] fix(telegram): address code review issues from cherry-pick merge - Fix critical plain-text fallback that was sending raw HTML tags to users: keep raw markdown available for the fallback path - Extract TELEGRAM_HTML_MAX_LEN (4096) constant to replace hardcoded magic number and document the difference from TELEGRAM_MAX_MESSAGE_LEN - Add fallback to _send_text for extra HTML chunks when HTML parse fails - Add missing @pytest.mark.asyncio decorator on test_send_delta_stream_end_html_expansion_does_not_overflow --- nanobot/channels/telegram.py | 32 +++++++++++++++++-------- tests/channels/test_telegram_channel.py | 1 + 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/nanobot/channels/telegram.py b/nanobot/channels/telegram.py index bfb7e70a9..ca0639bc1 100644 --- a/nanobot/channels/telegram.py +++ b/nanobot/channels/telegram.py @@ -26,6 +26,11 @@ from nanobot.security.network import validate_url_target from nanobot.utils.helpers import split_message TELEGRAM_MAX_MESSAGE_LEN = 4000 # Telegram message character limit +# Telegram's actual API limit is 4096; we split raw markdown at 4000 as a +# safety margin for mid-stream edits (plain text). For _stream_end, we +# convert to HTML first and then split at the true 4096-char boundary so +# the final rendered message never overflows. +TELEGRAM_HTML_MAX_LEN = 4096 TELEGRAM_REPLY_CONTEXT_MAX_LEN = TELEGRAM_MAX_MESSAGE_LEN # Max length for reply context in user message @@ -564,12 +569,13 @@ class TelegramChannel(BaseChannel): thread_kwargs = {} if message_thread_id := meta.get("message_thread_id"): thread_kwargs["message_thread_id"] = message_thread_id - html = _markdown_to_telegram_html(buf.text) - if len(html) <= 4096: + raw_text = buf.text + html = _markdown_to_telegram_html(raw_text) + if len(html) <= TELEGRAM_HTML_MAX_LEN: primary_html = html extra_html_chunks = [] else: - html_chunks = split_message(html, 4096) + html_chunks = split_message(html, TELEGRAM_HTML_MAX_LEN) primary_html = html_chunks[0] extra_html_chunks = html_chunks[1:] try: @@ -587,11 +593,13 @@ class TelegramChannel(BaseChannel): self._stream_bufs.pop(chat_id, None) return logger.debug("Final stream edit failed (HTML), trying plain: {}", e) + # Fall back to raw markdown (not HTML) so users don't see raw tags. + primary_plain = split_message(raw_text, TELEGRAM_MAX_MESSAGE_LEN)[0] if len(raw_text) > TELEGRAM_MAX_MESSAGE_LEN else raw_text try: await self._call_with_retry( self._app.bot.edit_message_text, chat_id=int_chat_id, message_id=buf.message_id, - text=primary_html, + text=primary_plain, ) except Exception as e2: if self._is_not_modified_error(e2): @@ -600,12 +608,16 @@ class TelegramChannel(BaseChannel): logger.warning("Final stream edit failed: {}", e2) raise # Let ChannelManager handle retry for extra_html_chunk in extra_html_chunks: - await self._call_with_retry( - self._app.bot.send_message, - chat_id=int_chat_id, text=extra_html_chunk, - parse_mode="HTML", - **thread_kwargs, - ) + try: + await self._call_with_retry( + self._app.bot.send_message, + chat_id=int_chat_id, text=extra_html_chunk, + parse_mode="HTML", + **thread_kwargs, + ) + except Exception: + # Fall back to _send_text which handles HTML→plain gracefully. + await self._send_text(int_chat_id, extra_html_chunk) self._stream_bufs.pop(chat_id, None) return diff --git a/tests/channels/test_telegram_channel.py b/tests/channels/test_telegram_channel.py index c5cc3cf4e..e02ca5318 100644 --- a/tests/channels/test_telegram_channel.py +++ b/tests/channels/test_telegram_channel.py @@ -497,6 +497,7 @@ async def test_send_delta_stream_end_splits_oversized_reply() -> None: assert "123" not in channel._stream_bufs +@pytest.mark.asyncio async def test_send_delta_stream_end_html_expansion_does_not_overflow() -> None: """Markdown that expands when converted to HTML is still split correctly. From 8e7d8bef6a36e7b2159d8697c12ea2333dc4338b Mon Sep 17 00:00:00 2001 From: hlg Date: Mon, 20 Apr 2026 16:28:26 +0800 Subject: [PATCH 7/8] fix(utils): handle malformed think tags and channel markers in strip_think MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some models / Ollama renderers occasionally emit tokenizer-level template leaks that the existing regexes miss: 1. Malformed opening tags with no closing `>`, running straight into user-facing content — e.g. `[\s\S]*?` and `^\s*[\s\S]*$` patterns both require `>`, so these leak into rendered messages. 2. Harmony-style channel markers like `` / `<|channel|>` at the start of a response. 3. Orphan `` / `` closing tags left behind when only the opener was consumed upstream. Handles each case conservatively: - Malformed `/]`). Explicit ASCII class instead of `\w` because Python's Unicode `\w` matches CJK and would defeat the primary fix. - Orphan closing tags and channel markers are stripped **only at the start or end of the text**. `strip_think` is also applied before persisting history (memory.py), so mid-text stripping would silently rewrite transcripts where the tokens themselves are discussed. Preserves: ``, ``, ``, ``, ``, ``, literal `` `` `` / `` `` `` inside prose or code blocks. Adds 16 new regression tests covering both the leak cases and the preserved-prose cases. --- nanobot/utils/helpers.py | 73 ++++++++++++++++++++++++++----- tests/utils/test_strip_think.py | 77 +++++++++++++++++++++++++++++++-- 2 files changed, 136 insertions(+), 14 deletions(-) diff --git a/nanobot/utils/helpers.py b/nanobot/utils/helpers.py index 6c3849ef8..74c80c110 100644 --- a/nanobot/utils/helpers.py +++ b/nanobot/utils/helpers.py @@ -15,12 +15,48 @@ from loguru import logger def strip_think(text: str) -> str: - """Remove thinking blocks and any unclosed trailing tag.""" + """Remove thinking blocks, unclosed trailing tags, and tokenizer-level + template leaks occasionally emitted by some models (notably Gemma 4's + Ollama renderer). + + Covers: + 1. Well-formed `...` and `...` blocks. + 2. Streaming prefixes where the block is never closed. + 3. *Malformed* opening tags missing the `>` — e.g. `` / `<|channel|>` + **at the start of the text** — conservative to avoid eating + explanatory prose that mentions these tokens. + 5. Orphan closing tags `` / `` **at the very start + or end of the text** only, for the same reason. + + Since this is also applied before persisting to history (memory.py), + the edge-only stripping of (4) and (5) is deliberate: stripping those + tokens mid-text would silently rewrite any message where a user or the + assistant discusses the tokens themselves. + """ + # Well-formed blocks first. text = re.sub(r"[\s\S]*?", "", text) text = re.sub(r"^\s*[\s\S]*$", "", text) - # Gemma 4 and similar models use ... blocks text = re.sub(r"[\s\S]*?", "", text) text = re.sub(r"^\s*[\s\S]*$", "", text) + # Malformed opening tags: `` / `/` — we can't use `\w` here because in Python's default + # Unicode regex mode it matches CJK characters too, which would defeat + # the primary fix for `/])", "", text) + text = re.sub(r"/])", "", text) + # Edge-only orphan closing tags (start or end of text). + text = re.sub(r"^\s*\s*", "", text) + text = re.sub(r"\s*\s*$", "", text) + text = re.sub(r"^\s*\s*", "", text) + text = re.sub(r"\s*\s*$", "", text) + # Edge-only channel markers (harmony / Gemma 4 variant leaks). + text = re.sub(r"^\s*<\|?channel\|?>\s*", "", text) return text.strip() @@ -37,7 +73,9 @@ def detect_image_mime(data: bytes) -> str | None: return None -def build_image_content_blocks(raw: bytes, mime: str, path: str, label: str) -> list[dict[str, Any]]: +def build_image_content_blocks( + raw: bytes, mime: str, path: str, label: str +) -> list[dict[str, Any]]: """Build native image blocks plus a short text label.""" b64 = base64.b64encode(raw).decode() return [ @@ -83,6 +121,7 @@ _TOOL_RESULTS_DIR = ".nanobot/tool-results" _TOOL_RESULT_RETENTION_SECS = 7 * 24 * 60 * 60 _TOOL_RESULT_MAX_BUCKETS = 32 + def safe_filename(name: str) -> str: """Replace unsafe path characters with underscores.""" return _UNSAFE_CHARS.sub("_", name).strip() @@ -258,9 +297,9 @@ def split_message(content: str, max_len: int = 2000) -> list[str]: break cut = content[:max_len] # Try to break at newline first, then space, then hard break - pos = cut.rfind('\n') + pos = cut.rfind("\n") if pos <= 0: - pos = cut.rfind(' ') + pos = cut.rfind(" ") if pos <= 0: pos = max_len chunks.append(content[:pos]) @@ -404,7 +443,7 @@ def build_status_content( max_completion_tokens: int = 8192, ) -> str: """Build a human-readable runtime status snapshot. - + Args: search_usage_text: Optional pre-formatted web search usage string (produced by SearchUsageInfo.format()). When provided @@ -423,7 +462,11 @@ def build_status_content( # Budget mirrors Consolidator formula: ctx_window - max_completion - _SAFETY_BUFFER ctx_budget = max(ctx_total - int(max_completion_tokens) - 1024, 1) ctx_pct = min(int((context_tokens_estimate / ctx_budget) * 100), 999) if ctx_budget > 0 else 0 - ctx_used_str = f"{context_tokens_estimate // 1000}k" if context_tokens_estimate >= 1000 else str(context_tokens_estimate) + ctx_used_str = ( + f"{context_tokens_estimate // 1000}k" + if context_tokens_estimate >= 1000 + else str(context_tokens_estimate) + ) ctx_total_str = f"{ctx_total // 1000}k" if ctx_total > 0 else "n/a" token_line = f"\U0001f4ca Tokens: {last_in} in / {last_out} out" if cached and last_in: @@ -439,12 +482,13 @@ def build_status_content( ] if search_usage_text: lines.append(search_usage_text) - return "\n".join(lines) + return "\n".join(lines) def sync_workspace_templates(workspace: Path, silent: bool = False) -> list[str]: """Sync bundled templates to workspace. Only creates missing files.""" from importlib.resources import files as pkg_files + try: tpl = pkg_files("nanobot") / "templates" except Exception: @@ -470,15 +514,22 @@ def sync_workspace_templates(workspace: Path, silent: bool = False) -> list[str] if added and not silent: from rich.console import Console + for name in added: Console().print(f" [dim]Created {name}[/dim]") # Initialize git for memory version control try: from nanobot.utils.gitstore import GitStore - gs = GitStore(workspace, tracked_files=[ - "SOUL.md", "USER.md", "memory/MEMORY.md", - ]) + + gs = GitStore( + workspace, + tracked_files=[ + "SOUL.md", + "USER.md", + "memory/MEMORY.md", + ], + ) gs.init() except Exception: logger.warning("Failed to initialize git store for {}", workspace) diff --git a/tests/utils/test_strip_think.py b/tests/utils/test_strip_think.py index 5828c6d1f..1eda89eb5 100644 --- a/tests/utils/test_strip_think.py +++ b/tests/utils/test_strip_think.py @@ -1,5 +1,3 @@ -import pytest - from nanobot.utils.helpers import strip_think @@ -48,7 +46,7 @@ class TestStripThinkFalsePositive: assert strip_think(text) == text def test_code_block_think_tag_preserved(self): - text = "Example:\n```\ntext = re.sub(r\"[\\s\\S]*\", \"\", text)\n```\nDone." + text = 'Example:\n```\ntext = re.sub(r"[\\s\\S]*", "", text)\n```\nDone.' assert strip_think(text) == text def test_backtick_thought_tag_preserved(self): @@ -63,3 +61,76 @@ class TestStripThinkFalsePositive: def test_prefix_unclosed_thought_still_stripped(self): assert strip_think("reasoning without closing") == "" + + +class TestStripThinkMalformedLeaks: + """Regression: Gemma 4's Ollama renderer occasionally emits a tag name + with no closing '>', running straight into the user-facing content + (e.g. `' and + let these through.""" + + def test_malformed_think_no_gt_chinese(self): + assert strip_think("` is a valid tag name variant; must not match. + assert strip_think("content") == "content" + + def test_self_closing_preserved(self): + assert strip_think("ok") == "ok" + assert strip_think("ok") == "ok" + + def test_orphan_closing_think_at_end_stripped(self): + # Typical leak: model opens `` without closing; we strip the + # opener from the start, leaving an orphan `` at the end. + assert strip_think("answer") == "answer" + + def test_orphan_closing_think_at_start_stripped(self): + assert strip_think("answer") == "answer" + + def test_channel_marker_at_start_stripped(self): + # Harmony / Gemma 4 channel markers leak at the start of a response. + assert strip_think("喷泉策略：09:00 开启") == ("喷泉策略：09:00 开启") + assert strip_think("<|channel|>answer") == "answer" + + +class TestStripThinkConservativePreserve: + """Regression: the malformed-tag / orphan cleanup must NOT touch + legitimate prose or code that mentions these tokens literally, otherwise + `strip_think` (which runs before history is persisted, memory.py) will + silently rewrite the conversation transcript.""" + + def test_think_dash_variant_preserved(self): + assert strip_think("bar") == "bar" + + def test_think_underscore_variant_preserved(self): + assert strip_think("bar") == "bar" + + def test_think_numeric_variant_preserved(self): + assert strip_think("bar") == "bar" + + def test_think_namespaced_variant_preserved(self): + assert strip_think("bar") == "bar" + + def test_literal_close_think_in_prose_preserved(self): + # Mid-prose references to `` in backticks or plain text must + # not be stripped; edge-only regex protects this. + text = "Use `` to close a thinking block." + assert strip_think(text) == text + + def test_literal_channel_marker_in_prose_preserved(self): + text = "The Harmony spec uses `<|channel|>` and `` markers." + assert strip_think(text) == text + + def test_literal_channel_marker_in_code_block_preserved(self): + text = "Example:\n```\nif line.startswith(''):\n skip()\n```" + assert strip_think(text) == text From 899a9073cec52e3655726802678f7fb9fff3cbe5 Mon Sep 17 00:00:00 2001 From: hlg Date: Mon, 20 Apr 2026 16:32:38 +0800 Subject: [PATCH 8/8] fix(memory): do not fall back to raw entry when strip_think empties it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `append_history` previously used `strip_think(entry) or entry.rstrip()` as a safety net, so if the entire entry was a template-token leak (e.g. `reasoning` or `` alone), the raw leaked text was still persisted to history — later re-introducing the very content `strip_think` was meant to scrub, via consolidation / replay. Persist the cleaned content directly. When cleanup empties a non-empty entry, log at debug and store an empty-content record (cursor continuity preserved). Adds 3 regression tests in test_memory_store.py covering: - Well-formed thinking blocks are stripped before persistence. - Pure-leak entries persist as empty, not as raw text. - Malformed prefix leaks (``) also persist as empty. --- nanobot/agent/memory.py | 20 ++++++++++++++-- tests/agent/test_memory_store.py | 41 +++++++++++++++++++++++++------- 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/nanobot/agent/memory.py b/nanobot/agent/memory.py index fb630ce13..60b542082 100644 --- a/nanobot/agent/memory.py +++ b/nanobot/agent/memory.py @@ -221,10 +221,26 @@ class MemoryStore: # -- history.jsonl — append-only, JSONL format --------------------------- def append_history(self, entry: str) -> int: - """Append *entry* to history.jsonl and return its auto-incrementing cursor.""" + """Append *entry* to history.jsonl and return its auto-incrementing cursor. + + Entries are passed through `strip_think` to drop template-level leaks + (e.g. unclosed `` markers) before being + persisted. If the cleaned content is empty but the raw entry wasn't, + the record is persisted with an empty string rather than falling back + to the raw leak — otherwise `strip_think`'s guarantees would be + undone by history replay / consolidation downstream. + """ cursor = self._next_cursor() ts = datetime.now().strftime("%Y-%m-%d %H:%M") - record = {"cursor": cursor, "timestamp": ts, "content": strip_think(entry.rstrip()) or entry.rstrip()} + raw = entry.rstrip() + content = strip_think(raw) + if raw and not content: + logger.debug( + "history entry {} stripped to empty (likely template leak); " + "persisting empty content to avoid re-polluting context", + cursor, + ) + record = {"cursor": cursor, "timestamp": ts, "content": content} with open(self.history_file, "a", encoding="utf-8") as f: f.write(json.dumps(record, ensure_ascii=False) + "\n") self._cursor_file.write_text(str(cursor), encoding="utf-8") diff --git a/tests/agent/test_memory_store.py b/tests/agent/test_memory_store.py index 7bb23fc69..94adbf376 100644 --- a/tests/agent/test_memory_store.py +++ b/tests/agent/test_memory_store.py @@ -1,8 +1,7 @@ """Tests for the restructured MemoryStore — pure file I/O layer.""" -from datetime import datetime import json -from pathlib import Path +from datetime import datetime import pytest @@ -65,6 +64,34 @@ class TestHistoryWithCursor: cursor = store.append_history("event 3") assert cursor == 3 + def test_append_history_strips_thinking_content(self, store): + """`strip_think` must run before persistence — well-formed thinking + blocks shouldn't land in history.""" + cursor = store.append_history("reasoningfinal answer") + content = store.read_file(store.history_file) + data = json.loads(content) + assert data["cursor"] == cursor + assert data["content"] == "final answer" + + def test_append_history_drops_pure_leak_content(self, store): + """Regression: entries that strip down to empty (pure template-token + leak) must NOT fall back to the raw leak. Persisting the raw text + would re-pollute context via consolidation / replay, undoing the + protection `strip_think` provides.""" + cursor = store.append_history("nothing user-facing") + content = store.read_file(store.history_file) + data = json.loads(content) + assert data["cursor"] == cursor + assert data["content"] == "" + + def test_append_history_drops_malformed_leak_prefix(self, store): + """Channel-marker / malformed opening leaks should not survive.""" + cursor = store.append_history("") + content = store.read_file(store.history_file) + data = json.loads(content) + assert data["cursor"] == cursor + assert data["content"] == "" + def test_read_unprocessed_history(self, store): store.append_history("event 1") store.append_history("event 2") @@ -134,7 +161,8 @@ class TestLegacyHistoryMigration: """JSONL entries with cursor=1 are correctly parsed and returned.""" store.history_file.write_text( '{"cursor": 1, "timestamp": "2026-03-30 14:30", "content": "Old event"}\n', - encoding="utf-8") + encoding="utf-8", + ) entries = store.read_unprocessed_history(since_cursor=0) assert len(entries) == 1 assert entries[0]["cursor"] == 1 @@ -218,8 +246,7 @@ class TestLegacyHistoryMigration: memory_dir.mkdir() legacy_file = memory_dir / "HISTORY.md" legacy_content = ( - "[2026-03-25–2026-04-02] Multi-day summary.\n" - "[2026-03-26/27] Cross-day summary.\n" + "[2026-03-25–2026-04-02] Multi-day summary.\n[2026-03-26/27] Cross-day summary.\n" ) legacy_file.write_text(legacy_content, encoding="utf-8") @@ -277,9 +304,7 @@ class TestLegacyHistoryMigration: memory_dir = tmp_path / "memory" memory_dir.mkdir() legacy_file = memory_dir / "HISTORY.md" - legacy_file.write_bytes( - b"[2026-04-01 10:00] Broken \xff data still needs migration.\n\n" - ) + legacy_file.write_bytes(b"[2026-04-01 10:00] Broken \xff data still needs migration.\n\n") store = MemoryStore(tmp_path)