From 7eb9716ad7c91c449501234acbdc36d5df2a127c Mon Sep 17 00:00:00 2001 From: kyssta-exe Date: Wed, 1 Jul 2026 17:21:38 +0530 Subject: [PATCH] fix(agent): apply persist override to the DB row only, never the live list (#48677) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The persist user-message override was applied in place to the live messages list. On the early crash-resilience persist (which runs BEFORE api_messages is built), that stripped observed group-chat context off the live user message and silently dropped it when observe_unmentioned_group_messages was enabled. Fix at the single chokepoint: _flush_messages_to_session_db resolves the override (idx/content/timestamp) locally and applies it ONLY to the row written to the DB — the live dict is never mutated, so EVERY persist caller (early persist, mid tool-loop flush, /resume, /branch) is protected uniformly. This supersedes the earlier shallow-copy approach, which broke the intrinsic _DB_PERSISTED_MARKER idempotency (copies never propagated the marker back to the live dicts → duplicate rows) and closes the sibling class tracked in #56303. Trailing empty-response scaffolding is still dropped from the live list in _persist_session (unchanged behavior). Salvaged from #48817; chokepoint reworked to coexist with the marker-based dedup (#50372). Co-authored-by: kyssta-exe --- run_agent.py | 37 +++++++++++++++++++++++++++---- tests/run_agent/test_run_agent.py | 11 ++++++++- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/run_agent.py b/run_agent.py index 9112061a839..5587fe648f0 100644 --- a/run_agent.py +++ b/run_agent.py @@ -1649,9 +1649,17 @@ class AIAgent: """Save session state to both JSON log and SQLite on any exit path. Ensures conversations are never lost, even on errors or early returns. + + Trailing empty-response scaffolding is dropped from the live list in + place (it is ephemeral junk the real transcript should shed). The + persist user-message *override* is NOT applied here — it is resolved + inside ``_flush_messages_to_session_db`` and written only to the DB row, + never mutating the live message list used by the API call (#48677 is + thus closed for every persist caller, not just this one). """ + # Scaffolding removal mutates the live list (desired — ephemeral + # retry/failure sentinels must not survive into the real transcript). self._drop_trailing_empty_response_scaffolding(messages) - self._apply_persist_user_message_override(messages) self._session_messages = messages self._save_session_log(messages) self._flush_messages_to_session_db(messages, conversation_history) @@ -1742,7 +1750,18 @@ class AIAgent: return if not self._session_db: return - self._apply_persist_user_message_override(messages) + # Persist user-message override (#48677 chokepoint): historically this + # mutated the live `messages` list in place, which — on the early + # crash-resilience persist that runs BEFORE the API call is built — + # stripped observed group-chat context off the live user message and + # silently dropped it. Instead, resolve the override here and apply it + # ONLY to the value written to the DB (see the write loop below); the + # live dict is never mutated, so every caller (early persist, mid-loop + # flush, /resume, /branch) is protected uniformly. Timestamp override is + # metadata and is likewise applied only to the written row. + _ov_idx = getattr(self, "_persist_user_message_idx", None) + _ov_content = getattr(self, "_persist_user_message_override", None) + _ov_timestamp = getattr(self, "_persist_user_message_timestamp", None) try: # Retry row creation if the earlier attempt failed transiently. if not self._session_db_created: @@ -1784,7 +1803,7 @@ class AIAgent: if isinstance(item, dict) } - for msg in messages: + for _msg_idx, msg in enumerate(messages): if not isinstance(msg, dict): continue # Never write ephemeral recovery scaffolding to the session @@ -1808,6 +1827,16 @@ class AIAgent: continue role = msg.get("role", "unknown") content = msg.get("content") + _row_timestamp = msg.get("timestamp") + # Apply the persist override to THIS row's written values only + # (never to the live dict). Match the original guard: text-only + # content is replaced; multimodal (list) content is left intact + # so image/audio blocks aren't clobbered by the text override. + if _ov_idx == _msg_idx and msg.get("role") == "user": + if _ov_content is not None and not isinstance(content, list): + content = _ov_content + if _ov_timestamp is not None: + _row_timestamp = _ov_timestamp # Persist multimodal tool results as their text summary only — # base64 images would bloat the session DB and aren't useful # for cross-session replay. @@ -1843,7 +1872,7 @@ class AIAgent: reasoning_details=msg.get("reasoning_details") if role == "assistant" else None, codex_reasoning_items=msg.get("codex_reasoning_items") if role == "assistant" else None, codex_message_items=msg.get("codex_message_items") if role == "assistant" else None, - timestamp=msg.get("timestamp"), + timestamp=_row_timestamp, ) msg[_DB_PERSISTED_MARKER] = True # The intrinsic markers are now the sole source of truth. Reset the diff --git a/tests/run_agent/test_run_agent.py b/tests/run_agent/test_run_agent.py index dee29d0b5bd..98f42c68e16 100644 --- a/tests/run_agent/test_run_agent.py +++ b/tests/run_agent/test_run_agent.py @@ -7151,7 +7151,16 @@ class TestPersistUserMessageOverride: agent._persist_session(messages, []) - assert messages[0]["content"] == "Hello there" + # The original messages list must NOT be mutated — the persist + # override is applied only to the DB row (resolved inside the flush + # chokepoint), so the live list keeps the original content for the + # API call (#48677). + assert ( + messages[0]["content"] + == "[Voice input — respond concisely and conversationally, " + "2-3 sentences max. No code blocks or markdown.] Hello there" + ) + # But the DB write must get the override. first_db_write = agent._session_db.append_message.call_args_list[0].kwargs assert first_db_write["content"] == "Hello there"