mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(state): stop a lone surrogate from silently killing session persistence
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.
This commit is contained in:
parent
8fe9706da8
commit
81d4619707
2 changed files with 95 additions and 6 deletions
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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 <20> 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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue