From f2a528fb597b1a6877dac6b0d0faf81947c5e957 Mon Sep 17 00:00:00 2001 From: petrichor-op <290868363+petrichor-op@users.noreply.github.com> Date: Wed, 1 Jul 2026 00:47:07 -0700 Subject: [PATCH] fix(agent): never persist empty-response recovery scaffolding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- run_agent.py | 37 +++++++++ scripts/release.py | 1 + ...est_empty_response_recovery_persistence.py | 82 +++++++++++++++++++ 3 files changed, 120 insertions(+) diff --git a/run_agent.py b/run_agent.py index c2a0864cd41..18e9f8e0c40 100644 --- a/run_agent.py +++ b/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"]) diff --git a/scripts/release.py b/scripts/release.py index e0627eb6629..1f9ba613d29 100755 --- a/scripts/release.py +++ b/scripts/release.py @@ -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) diff --git a/tests/run_agent/test_empty_response_recovery_persistence.py b/tests/run_agent/test_empty_response_recovery_persistence.py index 27e6c23d2d4..70a0dc328a5 100644 --- a/tests/run_agent/test_empty_response_recovery_persistence.py +++ b/tests/run_agent/test_empty_response_recovery_persistence.py @@ -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!"]