diff --git a/agent/agent_runtime_helpers.py b/agent/agent_runtime_helpers.py index b98fe4b44e77..4b01e3299473 100644 --- a/agent/agent_runtime_helpers.py +++ b/agent/agent_runtime_helpers.py @@ -1719,6 +1719,23 @@ def sanitize_api_messages(messages: List[Dict[str, Any]]) -> List[Dict[str, Any] "Pre-call sanitizer: added %d stub tool result(s)", len(missing_results), ) + + # 3. Coerce non-string tool results to JSON strings (#29920). + # MCP tools and memory helpers may return Python dicts/lists as content. + # The OpenAI API rejects these with HTTP 400 "invalid message content type". + for msg in messages: + if msg.get("role") == "tool": + content = msg.get("content") + if content is not None and not isinstance(content, str): + try: + msg["content"] = json.dumps(content, ensure_ascii=False) + except (TypeError, ValueError): + _ra().logger.warning( + "Pre-call sanitizer: failed to JSON-serialize tool result for %s", + msg.get("name", "?"), + ) + msg["content"] = repr(content) + return messages diff --git a/agent/transports/chat_completions.py b/agent/transports/chat_completions.py index fa36301bd81d..ade98d1a1e5b 100644 --- a/agent/transports/chat_completions.py +++ b/agent/transports/chat_completions.py @@ -519,6 +519,32 @@ def _build_kwargs_from_profile(self, profile, model, sanitized, tools, params): if extra_body: api_kwargs["extra_body"] = extra_body + # System-message guard (#29871): ensure persona/identity content reaches API. + # When a provider's hooks silently strip the role="system" message + # (known with some Ollama Cloud variants), re-inject from original input + # so SOUL.md is never lost mid-flight. + _had_system = ( + len(params.get("messages", [])) > 0 + and isinstance(params["messages"][0], dict) + and params["messages"][0].get("role") == "system" + ) + _has_system = ( + len(api_kwargs.get("messages", [])) > 0 + and isinstance(api_kwargs["messages"][0], dict) + and api_kwargs["messages"][0].get("role") == "system" + ) + if _had_system and not _has_system: + logger.debug( + "System-message guard (%s): profile/hooks stripped system role. " + "Re-injecting from input (input_msgs=%d, output_msgs=%d).", + profile.name, + len(params["messages"]), + len(api_kwargs["messages"]), + ) + api_kwargs["messages"] = [ + {"role": "system", "content": params["messages"][0]["content"]} + ] + api_kwargs["messages"] + return api_kwargs def normalize_response(self, response: Any, **kwargs) -> NormalizedResponse: diff --git a/cli.py b/cli.py index 2783ca31bf24..542447ba157f 100644 --- a/cli.py +++ b/cli.py @@ -11216,6 +11216,7 @@ def run_agent(): agent_message = _srn + "\n\n" + agent_message self._pending_skills_reload_note = None try: + self._cli_last_run_old_session_id = getattr(self.agent, "session_id", None) result = self.agent.run_conversation( user_message=agent_message, conversation_history=self.conversation_history[:-1], # Exclude the message we just added @@ -11359,8 +11360,25 @@ def run_agent(): sys.stdout.flush() time.sleep(0.15) - # Update history with full conversation - self.conversation_history = result.get("messages", self.conversation_history) if result else self.conversation_history + # Update history with full conversation. + # If auto-compression rotated the session mid-turn, result["messages"] + # is inflated (compressed baseline + this turn's growth). Use the + # agent's internal _session_messages instead — it holds the actual + # post-loop state the agent used for its final API call(s). Mirrors + # the gateway path fix in PR #29505. + if result: + compressed = getattr(self.agent, "_session_messages", None) + session_rotated = ( + self.agent + and self._cli_last_run_old_session_id is not None + and getattr(self.agent, "session_id", None) != self._cli_last_run_old_session_id + ) + if session_rotated and compressed: + self.conversation_history = list(compressed) + else: + self.conversation_history = result.get("messages", self.conversation_history) + elif self.conversation_history: + pass # Keep existing history on error/null result # If auto-compression fired mid-turn, the agent created a new # continuation session and mutated self.agent.session_id. Sync diff --git a/gateway/platforms/discord.py b/gateway/platforms/discord.py index 0d64b24d7e4b..d646d8c7fff5 100644 --- a/gateway/platforms/discord.py +++ b/gateway/platforms/discord.py @@ -1386,6 +1386,12 @@ async def send( if not self._client: return SendResult(success=False, error="Not connected") + # ── Suppress NO_REPLY delivery (#29932) ────────────────────── + # Agent returns NO_REPLY as a silence token — not actual content. + if (content or "").strip() == "NO_REPLY": + logger.debug("[%s] Agent returned NO_REPLY — suppressing delivery", self.name) + return SendResult(success=True, message_id=None, raw_response={"suppressed_no_reply": True}) + try: # Determine target channel: thread_id in metadata takes precedence. thread_id = None @@ -3789,6 +3795,10 @@ async def _fetch_channel_context( if not content: continue + # Exclude NO_REPLY sentinel from backfill (#29932). + if content.strip() == "NO_REPLY": + continue + name = msg.author.display_name if getattr(msg.author, "bot", False): name = f"{name} [bot]" @@ -4477,6 +4487,21 @@ async def _handle_message(self, message: DiscordMessage) -> None: normalized_content = raw_content mention_prefix = False + # ── NO_REPLY sentinel filter (#29932) ─────────────────────── + # Drop bot-to-bot NO_REPLY messages before they enter the agent loop. + # NO_REPLY is a control/silence token — not a user prompt. When + # DISCORD_ALLOW_BOTS=mentions, other bots' NO_REPLY must be ignored. + _NO_REPLY_SENTINEL = "NO_REPLY" + if ( + getattr(message.author, "bot", False) + and normalized_content.strip() == _NO_REPLY_SENTINEL + ): + logger.debug( + "[%s] Dropping bot NO_REPLY sentinel from %s — not a user prompt.", + self.name, message.author.display_name, + ) + return + snapshot_attachments = [] if hasattr(message, "message_snapshots") and message.message_snapshots: snapshot_text_parts = [] diff --git a/run_agent.py b/run_agent.py index 001d03784ad8..6d6ca85dc947 100644 --- a/run_agent.py +++ b/run_agent.py @@ -3348,7 +3348,19 @@ def _tool_result_content_for_active_model(self, tool_name: str, result: Any) -> not receive those image parts, because a rejected tool result becomes part of the canonical history and can make the next user turn fail before the agent has a chance to recover. + + Non-string results (Python dicts/lists from MCP tools or memory helpers) + are JSON-serialised here to prevent HTTP 400 ``invalid message content type`` + errors on the API side (#29920). """ + # JSON-serialize non-string, non-list results early (#29920). + if not isinstance(result, (str, list)): + try: + return json.dumps(result, ensure_ascii=False) + except (TypeError, ValueError): + logger.warning("Failed to JSON-serialize tool result for %s; using repr.", tool_name) + return repr(result) + if not _is_multimodal_tool_result(result): return result diff --git a/tests/cli/test_cli_compression_history_sync.py b/tests/cli/test_cli_compression_history_sync.py new file mode 100644 index 000000000000..831718d9444d --- /dev/null +++ b/tests/cli/test_cli_compression_history_sync.py @@ -0,0 +1,248 @@ +"""Tests for CLI conversation_history sync after run_conversation with auto-compression. + +Regression for issue #29926: when auto-compression rotates the session mid-run, +result["messages"] contains the inflated post-turn list (compressed baseline + +this turn's tool output), but conversation_history must use the agent's internal +_session_messages instead so the next turn starts from the compressed state. +""" + +import threading +from unittest.mock import MagicMock, patch + +import pytest + +from tests.cli.test_cli_init import _make_cli + + +def test_post_run_sync_uses_session_messages_when_session_rotated(): + """When run_conversation rotates session via auto-compression, conversation_history + must come from agent._session_messages, not result["messages"]. + """ + shell = _make_cli() + old_id = shell.session_id + new_child_id = "20260101_000000_compressed_child" + + # Pre-turn history (inflated) + pre_history = [{"role": "user", "content": f"msg_{i}"} for i in range(50)] + + # After compression, agent._session_messages holds the compressed baseline + compressed_messages = [ + {"role": "system", "content": "[COMPACTED CONTEXT]"}, + {"role": "user", "content": "msg_1"}, + {"role": "assistant", "content": "msg_2"}, + {"role": "user", "content": "msg_49"}, # compressed down to ~3 messages + ] + + # result["messages"] is inflated: compressed + this turn's tool output + inflated_result = list(compressed_messages) + [ + {"role": "assistant", "content": "expanded response with tools"}, + {"role": "user", "content": "new user msg"}, + ] + + shell.conversation_history = pre_history + shell.agent = MagicMock() + shell.agent.session_id = old_id # starts at parent session + + # Simulate the post-run logic from cli.py lines ~11360-11378 + result = {"final_response": "done", "messages": inflated_result} + shell._cli_last_run_old_session_id = old_id + # After run_conversation returns, agent.session_id rotated + shell.agent.session_id = new_child_id + # _session_messages has the compressed state (what agent actually used) + shell.agent._session_messages = compressed_messages + + # Reproduce the fix logic from cli.py + if result: + compressed_attr = getattr(shell.agent, "_session_messages", None) + session_rotated = ( + shell.agent + and shell._cli_last_run_old_session_id is not None + and getattr(shell.agent, "session_id", None) != shell._cli_last_run_old_session_id + ) + if session_rotated and compressed_attr: + shell.conversation_history = list(compressed_attr) + else: + shell.conversation_history = result.get("messages", shell.conversation_history) + else: + pass + + # Must use compressed, NOT inflated + assert len(shell.conversation_history) == 4 + assert shell.conversation_history[0]["role"] == "system" + assert "[COMPACTED CONTEXT]" in shell.conversation_history[0]["content"] + assert len(shell.conversation_history) != len(inflated_result) + + +def test_post_run_sync_uses_result_messages_when_no_rotation(): + """When session does NOT rotate (normal completion, no compression), + conversation_history must come from result["messages"] as before. + """ + shell = _make_cli() + + pre_history = [{"role": "user", "content": f"msg_{i}"} for i in range(10)] + result_messages = list(pre_history) + [ + {"role": "assistant", "content": "response"}, + ] + + shell.conversation_history = pre_history + shell.agent = MagicMock() + shell.agent.session_id = shell.session_id # same session, no rotation + + result = {"final_response": "done", "messages": result_messages} + shell._cli_last_run_old_session_id = shell.session_id + + # Reproduce the fix logic from cli.py + if result: + compressed_attr = getattr(shell.agent, "_session_messages", None) + session_rotated = ( + shell.agent + and shell._cli_last_run_old_session_id is not None + and getattr(shell.agent, "session_id", None) != shell._cli_last_run_old_session_id + ) + if session_rotated and compressed_attr: + shell.conversation_history = list(compressed_attr) + else: + shell.conversation_history = result.get("messages", shell.conversation_history) + else: + pass + + # Must use result["messages"] since no rotation + assert len(shell.conversation_history) == 11 + assert shell.conversation_history[-1]["content"] == "response" + + +def test_post_run_sync_no_session_messages_falls_back_to_result(): + """When session rotated but _session_messages is not set (edge case), + must fall back to result["messages"] rather than crashing. + """ + shell = _make_cli() + old_id = shell.session_id + new_child_id = "20260101_000000_compressed_child" + + inflated_result = [ + {"role": "system", "content": "[COMPACTED]"}, + {"role": "assistant", "content": "expanded response"}, + ] + + shell.conversation_history = [{"role": "user", "content": "pre"}] + shell.agent = MagicMock() + shell.agent.session_id = new_child_id # rotated but no _session_messages attr + shell.agent._session_messages = None # explicitly None (not unset) + shell._cli_last_run_old_session_id = old_id + + result = {"final_response": "done", "messages": inflated_result} + + # Reproduce the fix logic from cli.py + if result: + compressed_attr = getattr(shell.agent, "_session_messages", None) + session_rotated = ( + shell.agent + and shell._cli_last_run_old_session_id is not None + and getattr(shell.agent, "session_id", None) != shell._cli_last_run_old_session_id + ) + if session_rotated and compressed_attr: + shell.conversation_history = list(compressed_attr) + else: + shell.conversation_history = result.get("messages", shell.conversation_history) + else: + pass + + # Must fall back to result when _session_messages missing/None + assert len(shell.conversation_history) == 2 + assert shell.conversation_history[-1]["content"] == "expanded response" + + +def test_post_run_sync_null_result_preserves_history(): + """When result is None/empty, conversation_history must stay unchanged.""" + shell = _make_cli() + original_history = [ + {"role": "user", "content": "msg_1"}, + {"role": "assistant", "content": "msg_2"}, + ] + shell.conversation_history = list(original_history) + shell.agent = MagicMock() + + result = None # error path + shell._cli_last_run_old_session_id = shell.session_id + + # Reproduce the fix logic from cli.py + if result: + compressed_attr = getattr(shell.agent, "_session_messages", None) + session_rotated = ( + shell.agent + and shell._cli_last_run_old_session_id is not None + and getattr(shell.agent, "session_id", None) != shell._cli_last_run_old_session_id + ) + if session_rotated and compressed_attr: + shell.conversation_history = list(compressed_attr) + else: + shell.conversation_history = result.get("messages", shell.conversation_history) + elif shell.conversation_history: + pass # Keep existing history on error/null result + + assert shell.conversation_history == original_history + + +def test_post_run_sync_old_session_id_none_preserves_history(): + """When _old_session_id is None (first run or agent not initialized), + must NOT attempt rotation check and use result["messages"] normally. + """ + shell = _make_cli() + + pre_history = [{"role": "user", "content": f"msg_{i}"} for i in range(10)] + result_messages = list(pre_history) + [ + {"role": "assistant", "content": "response"}, + ] + + shell.conversation_history = pre_history + shell.agent = MagicMock() + shell.agent.session_id = None # no session set yet + + result = {"final_response": "done", "messages": result_messages} + shell._cli_last_run_old_session_id = None # No old session to compare against + + # Reproduce the fix logic from cli.py + if result: + compressed_attr = getattr(shell.agent, "_session_messages", None) + session_rotated = ( + shell.agent + and shell._cli_last_run_old_session_id is not None + and getattr(shell.agent, "session_id", None) != shell._cli_last_run_old_session_id + ) + if session_rotated and compressed_attr: + shell.conversation_history = list(compressed_attr) + else: + shell.conversation_history = result.get("messages", shell.conversation_history) + else: + pass + + # old_session_id is None → session_rotated is False → use result["messages"] + assert len(shell.conversation_history) == 11 + + +def test_post_run_sync_no_agent(): + """When self.agent is None (edge case), must NOT crash.""" + shell = _make_cli() + + pre_history = [{"role": "user", "content": f"msg_{i}"} for i in range(10)] + shell.conversation_history = list(pre_history) + + result = {"final_response": "done", "messages": pre_history} + shell._cli_last_run_old_session_id = shell.session_id + shell.agent = None # agent not set + + # Reproduce the fix logic from cli.py + if result: + compressed_attr = getattr(shell.agent, "_session_messages", None) + session_rotated = ( + shell.agent + and shell._cli_last_run_old_session_id is not None + and getattr(shell.agent, "session_id", None) != shell._cli_last_run_old_session_id + ) + if session_rotated and compressed_attr: + shell.conversation_history = list(compressed_attr) + else: + shell.conversation_history = result.get("messages", shell.conversation_history) + + # Must use result["messages"] (no agent → no rotation possible) + assert len(shell.conversation_history) == 10