From 81d4619707bedb33c84c48cabf1c70579fd7e91e Mon Sep 17 00:00:00 2001 From: Frowtek Date: Sat, 18 Jul 2026 15:23:37 +0300 Subject: [PATCH] fix(state): stop a lone surrogate from silently killing session persistence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sqlite3 encodes bound str parameters as UTF-8 and raises UnicodeEncodeError on lone surrogates (U+D800..U+DFFF), but SessionDB._encode_content returned str content untouched. One such code point anywhere in a message therefore aborted the entire message write. The path is reachable with ordinary input — the same scraped web/social text that crashed the guardrail hasher in fb0217c65: 1. a tool result carrying a lone surrogate is appended to the canonical `messages` history unsanitized; 2. the proactive sanitizer only cleans the `api_messages` *copy* (conversation_loop.py), so the API call succeeds; 3. because the API never raises, the UnicodeEncodeError recovery sanitizer (guarded by `isinstance(api_error, UnicodeEncodeError)`) never runs and the history keeps the surrogate; 4. the DB flush hits it and run_agent swallows the failure with `logger.warning("Session DB append_message failed")`. Because replace_messages re-sends the whole history every turn, the poisoned row stays and every later save raises too: the session freezes at its last good state while the live conversation grows, and everything after that point is gone on resume. Observed: persisted rows stuck at 2 while history reached 12, with only a warning in the log. Scrub at the DB write boundary with the canonical _sanitize_surrogates (surrogate -> U+FFFD): in _encode_content, which both INSERT sites share, and on the raw-bound reasoning / reasoning_content columns. The JSON branch already defaults to ensure_ascii=True and was safe. Well-formed text — accents, CJK, emoji — round-trips byte-identically, matching _encode_content's stated intent that persistence never fails. Adds regression tests for content, reasoning, the multi-turn freeze, and benign-Unicode passthrough. --- hermes_state.py | 37 ++++++++++++++++++---- tests/test_hermes_state.py | 64 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 6 deletions(-) diff --git a/hermes_state.py b/hermes_state.py index e45bf61e686..b828890cf23 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -26,12 +26,24 @@ import time from pathlib import Path from agent.memory_manager import sanitize_context +from agent.message_sanitization import _sanitize_surrogates from hermes_constants import get_hermes_home from typing import Any, Callable, Dict, List, Optional, Tuple, TypeVar logger = logging.getLogger(__name__) +def _scrub_surrogates(value: Any) -> Any: + """Replace lone surrogates when *value* is text; pass anything else through. + + sqlite3 encodes bound ``str`` parameters as UTF-8 and raises + ``UnicodeEncodeError`` on lone surrogates (U+D800..U+DFFF), so a single + such code point anywhere in a message aborts the whole write. No-op for + well-formed text. + """ + return _sanitize_surrogates(value) if isinstance(value, str) else value + + def workspace_key(row: Dict[str, Any]) -> Optional[str]: """A session's workspace grouping key: its git repo root when known, else its cwd. @@ -4098,13 +4110,26 @@ class SessionDB: sentinel-prefixed JSON string for lists/dicts. Paired with :meth:`_decode_content` on read. """ - if content is None or isinstance(content, (str, bytes, int, float)): + if isinstance(content, str): + # Lone UTF-16 surrogates reach here inside tool results scraped + # from the web/social platforms (the same input that crashed the + # guardrail hasher). The proactive sanitizer upstream only cleans + # the *api_messages* copy, and the recovery sanitizer only runs + # after the API call itself raises — which it no longer does — so + # the canonical history keeps them and this write is where they + # land. Left raw, sqlite3 raises UnicodeEncodeError, the flush is + # abandoned, and the session silently stops persisting for the + # rest of its life. Scrub so persistence never fails. + return _sanitize_surrogates(content) + if content is None or isinstance(content, (bytes, int, float)): return content try: + # json.dumps defaults to ensure_ascii=True, which escapes any + # surrogate as \udXXX — already safe to bind. return cls._CONTENT_JSON_PREFIX + json.dumps(content) except (TypeError, ValueError): # Last-resort fallback: stringify so persistence never fails. - return str(content) + return _sanitize_surrogates(str(content)) @classmethod def _decode_content(cls, content: Any) -> Any: @@ -4209,8 +4234,8 @@ class SessionDB: message_timestamp, token_count, finish_reason, - reasoning, - reasoning_content, + _scrub_surrogates(reasoning), + _scrub_surrogates(reasoning_content), reasoning_details_json, codex_items_json, codex_message_items_json, @@ -4305,8 +4330,8 @@ class SessionDB: message_timestamp, msg.get("token_count"), msg.get("finish_reason"), - msg.get("reasoning") if role == "assistant" else None, - msg.get("reasoning_content") if role == "assistant" else None, + _scrub_surrogates(msg.get("reasoning")) if role == "assistant" else None, + _scrub_surrogates(msg.get("reasoning_content")) if role == "assistant" else None, reasoning_details_json, codex_items_json, codex_message_items_json, diff --git a/tests/test_hermes_state.py b/tests/test_hermes_state.py index 5561c93d4ea..023155f4775 100644 --- a/tests/test_hermes_state.py +++ b/tests/test_hermes_state.py @@ -6214,3 +6214,67 @@ class TestGetMessagesPagination: self._seed(db, n=5) rows = db.get_messages("s1", offset=3) assert [m["content"] for m in rows] == ["msg-3", "msg-4"] + + +# ========================================================================= +# Lone-surrogate persistence +# ========================================================================= + +class TestLoneSurrogatePersistence: + """sqlite3 encodes bound str params as UTF-8 and raises UnicodeEncodeError + on lone surrogates (U+D800..U+DFFF). Tool results scraped from the web can + carry them, so a single such code point aborted the whole message write — + and because run_agent swallows the failure with a warning, the session then + silently stopped persisting for the rest of its life. + """ + + DIRTY = "scraped \ud835 price" + + def test_append_message_survives_lone_surrogate_content(self, db): + db.create_session("s1", source="cli") + db.append_message("s1", "assistant", "hello world") + db.append_message("s1", "tool", self.DIRTY, tool_name="web_search") + + rows = db.get_messages("s1") + assert len(rows) == 2 + # Surrogate replaced with U+FFFD; the surrounding text is intact. + assert rows[1]["content"] == "scraped � price" + + def test_append_message_survives_lone_surrogate_reasoning(self, db): + db.create_session("s1", source="cli") + db.append_message("s1", "assistant", "fine", reasoning=self.DIRTY) + assert len(db.get_messages("s1")) == 1 + + def test_replace_messages_keeps_persisting_after_dirty_row(self, db): + """The regression that mattered: one poisoned row froze the session. + + replace_messages re-sends the full history each turn, so once a dirty + tool result entered it, every later save raised and nothing after it + was ever written. + """ + db.create_session("s1", source="cli") + history = [ + {"role": "user", "content": "turn 1"}, + {"role": "assistant", "content": "answer 1"}, + {"role": "tool", "content": self.DIRTY, "tool_name": "web_search"}, + {"role": "assistant", "content": "answer 2"}, + ] + db.replace_messages("s1", history) + assert len(db.get_messages("s1")) == 4 + + # Later turns still persist rather than freezing at the poisoned row. + history += [ + {"role": "user", "content": "turn 3"}, + {"role": "assistant", "content": "answer 3"}, + ] + db.replace_messages("s1", history) + rows = db.get_messages("s1") + assert len(rows) == 6 + assert rows[-1]["content"] == "answer 3" + + def test_well_formed_unicode_is_unchanged(self, db): + """Accents, CJK and emoji must round-trip byte-identically.""" + db.create_session("s1", source="cli") + benign = "Ünïcödé ok — 日本語 🎉 emoji fine" + db.append_message("s1", "assistant", benign) + assert db.get_messages("s1")[0]["content"] == benign