mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
fix(agent): apply persist override to the DB row only, never the live list (#48677)
Some checks are pending
CI / Detect affected areas (push) Waiting to run
CI / Python tests (push) Blocked by required conditions
CI / Python lints (push) Blocked by required conditions
CI / TypeScript (push) Blocked by required conditions
CI / Docs Site (push) Blocked by required conditions
CI / Deny unrelated histories (push) Blocked by required conditions
CI / Check contributors (push) Blocked by required conditions
CI / Check uv.lock (push) Blocked by required conditions
CI / Lint Docker scripts (push) Blocked by required conditions
CI / Build&Test Docker image (push) Blocked by required conditions
CI / Supply-chain scan (push) Blocked by required conditions
CI / OSV scan (push) Waiting to run
CI / All required checks pass (push) Blocked by required conditions
CI / CI timing report (push) Blocked by required conditions
Deploy Site / deploy-vercel (push) Waiting to run
Deploy Site / deploy-docs (push) Waiting to run
Some checks are pending
CI / Detect affected areas (push) Waiting to run
CI / Python tests (push) Blocked by required conditions
CI / Python lints (push) Blocked by required conditions
CI / TypeScript (push) Blocked by required conditions
CI / Docs Site (push) Blocked by required conditions
CI / Deny unrelated histories (push) Blocked by required conditions
CI / Check contributors (push) Blocked by required conditions
CI / Check uv.lock (push) Blocked by required conditions
CI / Lint Docker scripts (push) Blocked by required conditions
CI / Build&Test Docker image (push) Blocked by required conditions
CI / Supply-chain scan (push) Blocked by required conditions
CI / OSV scan (push) Waiting to run
CI / All required checks pass (push) Blocked by required conditions
CI / CI timing report (push) Blocked by required conditions
Deploy Site / deploy-vercel (push) Waiting to run
Deploy Site / deploy-docs (push) Waiting to run
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 <kyssta-exe@users.noreply.github.com>
This commit is contained in:
parent
34de127200
commit
7eb9716ad7
2 changed files with 43 additions and 5 deletions
37
run_agent.py
37
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
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue