From 210cdb0ed35d4f7ef0957182312baaaa9e19bfbc Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 18 Aug 2026 16:09:47 -0700 Subject: [PATCH] fix(agent): legacy hidden redirect placeholders get the neutral wire payload at projection time (#88955) The salvaged writer-side fix stamps api_content on NEW hidden redirect placeholders, but rows persisted before it (content="" + display_kind=hidden, no sidecar) would keep re-triggering repair_empty_non_final_messages on every call forever. Substitute [response interrupted] on the wire copy at the api_content/display_kind projection stage so legacy sessions converge too. Never the interrupt scaffold (#81841). Durable transcript untouched. Regression tests drive run_conversation end-to-end with a spied sanitizer: the projection must leave the sanitizer nothing to heal (its per-turn warning spam is the bug), verified failing via sabotage run against the writer-only fix. Projection-side approach credit: @JoaoMarcos44 (PR #88996). --- agent/conversation_loop.py | 22 +++++- tests/run_agent/test_steer.py | 136 ++++++++++++++++++++++++++++++++++ 2 files changed, 157 insertions(+), 1 deletion(-) diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index 8a7a4f33fb..ab8e60b6d0 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -2136,9 +2136,29 @@ def run_conversation( # from every outgoing copy so strict OpenAI-compatible backends # don't reject the request after a model switch or resumed typed # event row enters the live history. - api_msg.pop("display_kind", None) + _display_kind = api_msg.pop("display_kind", None) api_msg.pop("display_metadata", None) + # Legacy hidden redirect placeholders (#88955): rows persisted + # BEFORE the writer-side api_content stamp in + # _apply_active_turn_redirect are content="" with no sidecar. + # Once display_kind is stripped the pre-call sanitizer + # (repair_empty_non_final_messages) would re-heal such a row on + # every call forever, since the durable transcript is never + # mutated. Give the wire copy the same neutral payload here so + # old sessions converge too. Never the interrupt scaffold — + # replaying scaffold bytes as assistant text is #81841. + if ( + _display_kind == "hidden" + and api_msg.get("role") == "assistant" + and not _api_content + and not (api_msg.get("content") or "").strip() + and not api_msg.get("tool_calls") + ): + from agent.agent_runtime_helpers import _INTERRUPTED_PLACEHOLDER + + api_msg["content"] = _INTERRUPTED_PLACEHOLDER + # Durable row identity stamped by _rows_to_conversation so the # desktop can address a specific persisted message (reactions). # Bookkeeping, never a provider field — only the chat-completions diff --git a/tests/run_agent/test_steer.py b/tests/run_agent/test_steer.py index a02c85ad51..cd5266ef3a 100644 --- a/tests/run_agent/test_steer.py +++ b/tests/run_agent/test_steer.py @@ -711,3 +711,139 @@ class TestSteerCommandRegistry: if __name__ == "__main__": # pragma: no cover pytest.main([__file__, "-v"]) + + +class TestLegacyHiddenPlaceholderWireSubstitution: + """Projection-side half of #88955: rows persisted BEFORE the writer-side + ``api_content`` stamp are ``content=""`` + ``display_kind="hidden"`` with + no sidecar. The send-time projection must give the WIRE copy the neutral + ``[response interrupted]`` payload so legacy sessions converge instead of + re-healing forever — while the durable row stays hidden and empty.""" + + def _loop_agent(self): + from unittest.mock import MagicMock, patch + + from run_agent import AIAgent + + with ( + patch("run_agent.get_tool_definitions", return_value=[]), + patch("run_agent.check_toolset_requirements", return_value={}), + patch("run_agent.OpenAI"), + ): + agent = AIAgent( + api_key="test-key-1234567890", + base_url="https://openrouter.ai/api/v1", + quiet_mode=True, + skip_context_files=True, + skip_memory=True, + ) + agent.client = MagicMock() + agent._cached_system_prompt = "You are helpful." + agent._use_prompt_caching = False + agent.tool_delay = 0 + agent.compression_enabled = False + agent.save_trajectories = False + return agent + + def test_legacy_empty_hidden_assistant_row_gets_neutral_wire_payload(self): + """The projection itself must fill the row — the sanitizer must have + NOTHING left to heal (its per-turn warning spam IS the bug).""" + from unittest.mock import patch + + import agent.agent_runtime_helpers as _arh + + from tests.run_agent.test_run_agent import _mock_response + + agent = self._loop_agent() + agent.client.chat.completions.create.side_effect = [ + _mock_response(content="ok", finish_reason="stop"), + ] + sanitizer_inputs = [] + _real_repair = _arh.repair_empty_non_final_messages + + def _spy_repair(messages, *a, **k): + sanitizer_inputs.append( + [ + (m.get("role"), m.get("content")) + for m in messages + if isinstance(m, dict) + ] + ) + return _real_repair(messages, *a, **k) + # Legacy pre-fix row: no api_content sidecar. + history = [ + {"role": "user", "content": "start"}, + {"role": "assistant", "content": "", "display_kind": "hidden"}, + {"role": "user", "content": "correction", "finish_reason": "stop"}, + {"role": "assistant", "content": "earlier reply", "finish_reason": "stop"}, + ] + + with ( + patch.object(agent, "_flush_messages_to_session_db"), + patch.object(agent, "_persist_session"), + patch.object(agent, "_save_trajectory"), + patch.object(agent, "_cleanup_task_resources"), + patch.object( + _arh, "repair_empty_non_final_messages", side_effect=_spy_repair + ), + ): + agent.run_conversation("next question", conversation_history=history) + + # Precondition: the sanitizer actually ran on this call path. + assert sanitizer_inputs, "sanitizer was never invoked — test is vacuous" + # The projection already filled the legacy row BEFORE sanitization: + # every assistant row the sanitizer saw carried payload, so it healed 0. + for snapshot in sanitizer_inputs: + for role, content in snapshot: + if role == "assistant": + assert (content or "").strip(), ( + "sanitizer still received an empty assistant row — " + "the re-heal loop is back (#88955)" + ) + + wire = agent.client.chat.completions.create.call_args.kwargs["messages"] + wire_assistants = [m for m in wire if m.get("role") == "assistant"] + legacy = wire_assistants[0] + # Substituted on the wire by the projection (not the sanitizer): + assert legacy["content"] == "[response interrupted]" + assert "display_kind" not in legacy + # #81841: never the interrupt scaffold. + assert "[This response was interrupted" not in legacy["content"] + # Durable history untouched. + assert history[1]["content"] == "" + assert history[1]["display_kind"] == "hidden" + assert "api_content" not in history[1] + + def test_hidden_row_with_tool_calls_or_text_is_not_touched(self): + from agent.conversation_loop import _clone_message_for_send # noqa: F401 + from unittest.mock import patch + + from tests.run_agent.test_run_agent import _mock_response + + agent = self._loop_agent() + agent.client.chat.completions.create.side_effect = [ + _mock_response(content="ok", finish_reason="stop"), + ] + history = [ + {"role": "user", "content": "start"}, + { + "role": "assistant", + "content": "visible text", + "display_kind": "hidden", + "finish_reason": "stop", + }, + {"role": "user", "content": "more"}, + {"role": "assistant", "content": "reply", "finish_reason": "stop"}, + ] + + with ( + patch.object(agent, "_flush_messages_to_session_db"), + patch.object(agent, "_persist_session"), + patch.object(agent, "_save_trajectory"), + patch.object(agent, "_cleanup_task_resources"), + ): + agent.run_conversation("next", conversation_history=history) + + wire = agent.client.chat.completions.create.call_args.kwargs["messages"] + wire_assistants = [m for m in wire if m.get("role") == "assistant"] + assert wire_assistants[0]["content"] == "visible text"