mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-25 17:18:11 +00:00
fix(state): stop double-encoding display_metadata on write
export_session() reads through get_messages(), so before the read fix an already-serialized string went straight back into _insert_message_rows() and got re-dumped — an export/import round trip permanently corrupted the row. Guard the three write paths the same way tool_calls already is: parse a string argument before storing it, and drop metadata that isn't an object rather than persisting something no reader can use. Co-authored-by: xxxigm <xxxigm@users.noreply.github.com> Co-authored-by: aml1973 <aml1973@users.noreply.github.com>
This commit is contained in:
parent
3399bf28a5
commit
19dc35cf57
2 changed files with 68 additions and 6 deletions
|
|
@ -5983,14 +5983,43 @@ class SessionDB:
|
|||
return content
|
||||
return content
|
||||
|
||||
@staticmethod
|
||||
def _encode_display_metadata(display_metadata: Any) -> Optional[str]:
|
||||
"""Serialize ``display_metadata`` for its TEXT column without double-encoding.
|
||||
|
||||
Import/replace paths can hand us an already-serialized JSON string (the
|
||||
same hazard ``tool_calls`` guards against above). ``json.dumps`` on that
|
||||
string would store a quoted JSON string, and the single ``json.loads``
|
||||
on read then yields a ``str`` instead of a dict.
|
||||
"""
|
||||
if not display_metadata:
|
||||
return None
|
||||
if isinstance(display_metadata, str):
|
||||
try:
|
||||
parsed = json.loads(display_metadata)
|
||||
except (json.JSONDecodeError, TypeError):
|
||||
logger.warning("Ignoring non-JSON display metadata on write")
|
||||
return None
|
||||
if not isinstance(parsed, dict):
|
||||
logger.warning("Ignoring non-object display metadata on write")
|
||||
return None
|
||||
return json.dumps(parsed)
|
||||
if isinstance(display_metadata, dict):
|
||||
return json.dumps(display_metadata)
|
||||
logger.warning(
|
||||
"Ignoring unexpected display metadata type on write: %s",
|
||||
type(display_metadata).__name__,
|
||||
)
|
||||
return None
|
||||
|
||||
@staticmethod
|
||||
def _decode_display_metadata(raw: Any) -> Optional[Dict[str, Any]]:
|
||||
"""Decode a ``display_metadata`` column into the dict every reader expects.
|
||||
|
||||
Every message read path must go through this. Returning the raw TEXT
|
||||
instead reaches the desktop as a string, where ``'task_count' in meta``
|
||||
throws and fails the whole resume. Some rows are double-encoded, so
|
||||
unwrap a second layer when we find one.
|
||||
throws and fails the whole resume. Rows written before the encode guard
|
||||
landed are double-encoded, so unwrap a second layer when we find one.
|
||||
"""
|
||||
if raw is None:
|
||||
return None
|
||||
|
|
@ -6052,7 +6081,7 @@ class SessionDB:
|
|||
"""
|
||||
# Display metadata is presentation-only and never changes the model
|
||||
# context role/content replayed to providers.
|
||||
display_metadata_json = json.dumps(display_metadata) if display_metadata else None
|
||||
display_metadata_json = self._encode_display_metadata(display_metadata)
|
||||
# Serialize structured fields to JSON before entering the write txn
|
||||
reasoning_details_json = (
|
||||
json.dumps(reasoning_details)
|
||||
|
|
@ -6191,7 +6220,7 @@ class SessionDB:
|
|||
"UPDATE messages SET display_kind = ?, display_metadata = ? WHERE id = ?",
|
||||
(
|
||||
_scrub_surrogates(display_kind),
|
||||
json.dumps(display_metadata) if display_metadata else None,
|
||||
self._encode_display_metadata(display_metadata),
|
||||
row[0],
|
||||
),
|
||||
)
|
||||
|
|
@ -6285,7 +6314,7 @@ class SessionDB:
|
|||
1,
|
||||
_scrub_surrogates(api_content) if isinstance(api_content, str) else None,
|
||||
_scrub_surrogates(msg.get("display_kind")) if isinstance(msg.get("display_kind"), str) else None,
|
||||
json.dumps(msg["display_metadata"]) if msg.get("display_metadata") else None,
|
||||
self._encode_display_metadata(msg.get("display_metadata")),
|
||||
),
|
||||
)
|
||||
inserted += 1
|
||||
|
|
|
|||
|
|
@ -7469,7 +7469,8 @@ class TestDisplayMetadataReadPaths:
|
|||
assert self._read(db, reader, message_id, anchor_id)["display_metadata"] == self.META
|
||||
|
||||
@pytest.mark.parametrize("reader", READERS)
|
||||
def test_every_reader_unwraps_double_encoded_rows(self, db, reader):
|
||||
def test_every_reader_unwraps_historically_double_encoded_rows(self, db, reader):
|
||||
"""Rows written before the encode guard landed carry a second JSON layer."""
|
||||
message_id, anchor_id = self._seed(db)
|
||||
|
||||
def _corrupt(conn):
|
||||
|
|
@ -7498,3 +7499,35 @@ class TestDisplayMetadataReadPaths:
|
|||
assert message.get("display_metadata") is None
|
||||
assert message["content"] == "event"
|
||||
|
||||
def test_export_import_round_trip_keeps_metadata_decodable(self, db, tmp_path):
|
||||
"""The read leak used to write a permanently double-encoded row here.
|
||||
|
||||
``export_session`` reads through ``get_messages``, so an undecoded
|
||||
string went back through ``_insert_message_rows`` and got re-dumped.
|
||||
"""
|
||||
self._seed(db)
|
||||
blob = db.export_session("s1")
|
||||
assert isinstance(blob["messages"][0]["display_metadata"], dict)
|
||||
|
||||
target = SessionDB(db_path=tmp_path / "imported.db")
|
||||
try:
|
||||
target.import_sessions([json.loads(json.dumps(blob))])
|
||||
assert target.get_messages_as_conversation("s1")[0]["display_metadata"] == self.META
|
||||
assert target.get_messages("s1")[0]["display_metadata"] == self.META
|
||||
finally:
|
||||
target.close()
|
||||
|
||||
def test_write_paths_do_not_double_encode_serialized_metadata(self, db):
|
||||
"""Import/replace can hand us metadata that is already a JSON string."""
|
||||
db.create_session("s1", source="cli")
|
||||
db.replace_messages(
|
||||
"s1",
|
||||
[{
|
||||
"role": "user",
|
||||
"content": "event",
|
||||
"display_kind": "async_delegation_complete",
|
||||
"display_metadata": json.dumps(self.META),
|
||||
}],
|
||||
)
|
||||
assert db.get_messages_as_conversation("s1")[0]["display_metadata"] == self.META
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue