From 19dc35cf577a339f1ba6f7106d37704638549ca0 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Fri, 24 Jul 2026 19:53:25 -0500 Subject: [PATCH] fix(state): stop double-encoding display_metadata on write MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Co-authored-by: aml1973 --- hermes_state.py | 39 +++++++++++++++++++++++++++++++++----- tests/test_hermes_state.py | 35 +++++++++++++++++++++++++++++++++- 2 files changed, 68 insertions(+), 6 deletions(-) diff --git a/hermes_state.py b/hermes_state.py index 90f2c8ee5d02..4ba71b295566 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -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 diff --git a/tests/test_hermes_state.py b/tests/test_hermes_state.py index f28ef42b7ba8..2aa5199612ab 100644 --- a/tests/test_hermes_state.py +++ b/tests/test_hermes_state.py @@ -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 +