mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-14 14:12:44 +00:00
fix(agent): never persist empty-response recovery scaffolding
Ephemeral empty-response/prefill recovery scaffolding (the synthetic assistant "(empty)" turn, the user nudge, the terminal "(empty)" sentinel, and the thinking-only prefill placeholder) exists only to drive the next API retry; the in-memory loop pops it before appending the real response. The append-only flush did not mirror that, so a mid-turn persist could commit scaffolding to the SQLite session store (and JSON log), and a resumed session would replay synthetic "(empty)"/nudge turns as genuine context — re-poisoning the empty-retry boundary forever. Filter ephemeral scaffolding at both durable-write sites (_flush_messages_to_session_db + _save_session_log), by flag not position, so buried scaffolding (an answered nudge leaves the synthetic pair mid-list) is skipped too. Covers all three flags including _thinking_prefill. Adapted onto current main's identity-tracking flush. Cherry-picked from #41281 by petrichor-op.
This commit is contained in:
parent
8db6ed7bd9
commit
f2a528fb59
3 changed files with 120 additions and 0 deletions
37
run_agent.py
37
run_agent.py
|
|
@ -213,6 +213,28 @@ from agent.tool_dispatch_helpers import (
|
|||
from utils import atomic_json_write, base_url_host_matches, base_url_hostname, env_float, is_truthy_value, model_forces_max_completion_tokens
|
||||
|
||||
|
||||
# Internal flags that mark a message as ephemeral empty-response/prefill
|
||||
# recovery scaffolding: the synthetic assistant "(empty)" turn and user nudge
|
||||
# injected after an empty response, the terminal "(empty)" sentinel, and the
|
||||
# thinking-only prefill placeholder. These exist only to drive the next API
|
||||
# retry; the in-memory loop pops them before appending the real response.
|
||||
# Persistence must mirror that, otherwise an append-only flush can commit them
|
||||
# to the session store and a resumed session replays synthetic "(empty)"/nudge
|
||||
# turns as if they were genuine context.
|
||||
_EPHEMERAL_SCAFFOLDING_FLAGS = (
|
||||
"_empty_recovery_synthetic",
|
||||
"_empty_terminal_sentinel",
|
||||
"_thinking_prefill",
|
||||
)
|
||||
|
||||
|
||||
def _is_ephemeral_scaffolding(msg: Any) -> bool:
|
||||
"""Return True when ``msg`` is internal recovery scaffolding that must never
|
||||
be persisted to the durable transcript (SQLite session store or JSON log)."""
|
||||
return isinstance(msg, dict) and any(
|
||||
msg.get(flag) for flag in _EPHEMERAL_SCAFFOLDING_FLAGS
|
||||
)
|
||||
|
||||
|
||||
_MAX_TOOL_WORKERS = 8
|
||||
|
||||
|
|
@ -1706,6 +1728,17 @@ class AIAgent:
|
|||
for msg in messages:
|
||||
if not isinstance(msg, dict):
|
||||
continue
|
||||
# Never write ephemeral recovery scaffolding to the session
|
||||
# store. The flush is append-only (it only advances
|
||||
# _last_flushed_db_idx via identity tracking), so a synthetic
|
||||
# message committed by a mid-turn persist cannot be un-written
|
||||
# when the end-of-turn drop removes it from the in-memory list —
|
||||
# the resumed transcript would then replay synthetic
|
||||
# "(empty)"/nudge/thinking-prefill turns as if they were genuine
|
||||
# context. Skip regardless of position: an answered nudge leaves
|
||||
# the synthetic pair buried mid-list, not just at the tail.
|
||||
if _is_ephemeral_scaffolding(msg):
|
||||
continue
|
||||
msg_id = id(msg)
|
||||
if msg_id in flushed_ids:
|
||||
continue
|
||||
|
|
@ -2430,6 +2463,10 @@ class AIAgent:
|
|||
try:
|
||||
cleaned = []
|
||||
for msg in messages:
|
||||
# Mirror the SQLite flush: ephemeral recovery scaffolding is
|
||||
# internal retry state, never durable transcript content.
|
||||
if _is_ephemeral_scaffolding(msg):
|
||||
continue
|
||||
if msg.get("role") == "assistant" and msg.get("content"):
|
||||
msg = dict(msg)
|
||||
msg["content"] = self._clean_session_content(msg["content"])
|
||||
|
|
|
|||
|
|
@ -46,6 +46,7 @@ ACP_REGISTRY_MANIFEST = REPO_ROOT / "acp_registry" / "agent.json"
|
|||
# Auto-extracted from noreply emails + manual overrides
|
||||
AUTHOR_MAP = {
|
||||
"jnibarger01@gmail.com": "jnibarger01", # PR #35130 salvage (ReDoS-bound threat-pattern filler + FTS5 query cap + V4A Move-File approval/traversal targets)
|
||||
"290868363+petrichor-op@users.noreply.github.com": "petrichor-op", # PR #41281 salvage (never persist ephemeral empty-response recovery scaffolding to the SQLite session store / JSON log; filter by flag not position)
|
||||
"syahidfrd@gmail.com": "syahidfrd", # PR #17059 salvage (tag unverified senders in Slack thread context to mitigate indirect prompt injection)
|
||||
"22971845+H2KFORGIVEN@users.noreply.github.com": "H2KFORGIVEN", # PR #22523 salvage (turn-pair preservation: never orphan the last user ask at head_end during compaction)
|
||||
"5823452+sgabel@users.noreply.github.com": "sgabel", # PR #13139 salvage (redact secrets in user-facing approval prompts)
|
||||
|
|
|
|||
|
|
@ -3,6 +3,28 @@
|
|||
from run_agent import AIAgent
|
||||
|
||||
|
||||
class _CapturingSessionDB:
|
||||
"""Minimal SessionDB stand-in that records every appended message."""
|
||||
|
||||
def __init__(self):
|
||||
self.rows = []
|
||||
|
||||
def append_message(self, session_id, role, content=None, **kwargs):
|
||||
self.rows.append({"role": role, "content": content})
|
||||
return len(self.rows)
|
||||
|
||||
|
||||
def _agent_with_capturing_db():
|
||||
agent = AIAgent.__new__(AIAgent)
|
||||
agent._persist_user_message_idx = None
|
||||
agent._persist_user_message_override = None
|
||||
agent._session_db = _CapturingSessionDB()
|
||||
agent._session_db_created = True
|
||||
agent._last_flushed_db_idx = 0
|
||||
agent.session_id = "sess-test"
|
||||
return agent
|
||||
|
||||
|
||||
def _agent_with_stubbed_persistence():
|
||||
agent = AIAgent.__new__(AIAgent)
|
||||
agent._persist_user_message_idx = None
|
||||
|
|
@ -92,3 +114,63 @@ def test_persist_session_strips_marked_terminal_empty_sentinel():
|
|||
assert messages == [{"role": "user", "content": "continue"}]
|
||||
assert agent.flushed_session_db_messages[-1] == messages
|
||||
assert all(not msg.get("_empty_terminal_sentinel") for msg in messages)
|
||||
|
||||
|
||||
def test_flush_never_writes_buried_empty_recovery_scaffolding():
|
||||
"""When an empty-after-tools nudge is followed by a tool-calling response,
|
||||
the synthetic ``(empty)`` + nudge pair stays buried in the live message
|
||||
list (only the trailing copies are ever dropped). The append-only flush
|
||||
must skip it regardless of position, otherwise the synthetic turns land in
|
||||
the session store and pollute every resumed transcript.
|
||||
"""
|
||||
agent = _agent_with_capturing_db()
|
||||
|
||||
messages = [
|
||||
{"role": "user", "content": "run the task"},
|
||||
{
|
||||
"role": "assistant",
|
||||
"content": "",
|
||||
"tool_calls": [{"id": "call_1", "type": "function",
|
||||
"function": {"name": "x", "arguments": "{}"}}],
|
||||
},
|
||||
{"role": "tool", "content": "{}", "tool_call_id": "call_1"},
|
||||
# Synthetic recovery scaffolding, now buried because the model answered
|
||||
# the nudge with another tool call rather than terminating.
|
||||
{"role": "assistant", "content": "(empty)", "_empty_recovery_synthetic": True},
|
||||
{
|
||||
"role": "user",
|
||||
"content": "You just executed tool calls but returned an empty response.",
|
||||
"_empty_recovery_synthetic": True,
|
||||
},
|
||||
{
|
||||
"role": "assistant",
|
||||
"content": "",
|
||||
"tool_calls": [{"id": "call_2", "type": "function",
|
||||
"function": {"name": "x", "arguments": "{}"}}],
|
||||
},
|
||||
{"role": "tool", "content": "{}", "tool_call_id": "call_2"},
|
||||
{"role": "assistant", "content": "All done."},
|
||||
]
|
||||
|
||||
agent._flush_messages_to_session_db(messages, conversation_history=[])
|
||||
|
||||
persisted = agent._session_db.rows
|
||||
assert all(row["content"] != "(empty)" for row in persisted)
|
||||
assert all("empty response" not in (row["content"] or "") for row in persisted)
|
||||
# Only the genuine turns reach the store, in order.
|
||||
assert [r["role"] for r in persisted] == [
|
||||
"user", "assistant", "tool", "assistant", "tool", "assistant",
|
||||
]
|
||||
assert persisted[-1]["content"] == "All done."
|
||||
|
||||
|
||||
def test_flush_skips_thinking_prefill_scaffolding():
|
||||
agent = _agent_with_capturing_db()
|
||||
messages = [
|
||||
{"role": "user", "content": "hi"},
|
||||
{"role": "assistant", "content": "", "_thinking_prefill": True},
|
||||
{"role": "assistant", "content": "Hello!"},
|
||||
]
|
||||
agent._flush_messages_to_session_db(messages, conversation_history=[])
|
||||
|
||||
assert [r["content"] for r in agent._session_db.rows] == ["hi", "Hello!"]
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue