Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions agent/chat_completion_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -1308,6 +1308,21 @@ def build_assistant_message(agent, assistant_message, finish_reason: str) -> dic
from agent.redact import redact_sensitive_text
_san_content = redact_sensitive_text(_san_content)

# Defence-in-depth: never serialize a TEXTLESS assistant turn with an
# empty content string. Providers with strict validation (Moonshot/Kimi
# via OpenRouter: "the message at position N with role 'assistant' must
# not be empty") reject the replay with HTTP 400, which permanently
# poisons the persisted session — every subsequent turn re-sends the
# offending message. Reachable via the partial-stream-stub path when a
# stream drops before delivering any text (the loop now skips that case,
# but other callers of this builder get the same guarantee). A single
# space satisfies non-empty validation without fabricating content —
# the same trick the reasoning_content pad uses above (#15250, #17400).
# Tool-call turns are exempt: ``content: ""`` alongside ``tool_calls``
# is accepted everywhere and normalizing it would alter cache keys.
if not _san_content and not assistant_tool_calls:
_san_content = " "

msg = {
"role": "assistant",
"content": _san_content,
Expand Down
49 changes: 45 additions & 4 deletions agent/conversation_loop.py
Original file line number Diff line number Diff line change
Expand Up @@ -1073,6 +1073,30 @@ def run_conversation(
# the OpenAI SDK. Sanitizing here prevents the 3-retry cycle.
_sanitize_messages_surrogates(api_messages)

# Pad a textless assistant turn's empty content to a single space.
# Strict providers (Moonshot/Kimi via OpenRouter: "the message at
# position N with role 'assistant' must not be empty") reject the
# replay with HTTP 400 — and the session is poisoned for every
# subsequent turn. This is the DURABLE repair for ALREADY-poisoned
# persisted sessions: the partial-stream-stub rows older builds
# wrote (content:'' finish_reason:'length') are rebuilt to '' on
# every reload — ``_rows_to_conversation`` strips whitespace, so a
# DB-side pad can't survive — and only a SEND-time pad repairs
# them. It must run AFTER the whitespace-normalization pass above
# (which would strip the pad back to '') and after
# _drop_thinking_only_and_merge_users (which can leave a textless
# turn). Tool-call turns are exempt: ``content: ''`` alongside
# ``tool_calls`` is accepted everywhere and normalizing it would
# alter prompt-cache keys.
for am in api_messages:
if (
am.get("role") == "assistant"
and not am.get("tool_calls")
and isinstance(am.get("content"), str)
and not am["content"].strip()
):
am["content"] = " "

# One image-stripped message estimate feeds both figures. Was: a
# str(msg) char walk (re-serialized base64 every call) + a second
# messages walk inside estimate_request_tokens_rough. Tools added
Expand Down Expand Up @@ -2025,10 +2049,27 @@ def _perform_api_call(next_api_kwargs):
)
if assistant_message is not None and not _trunc_has_tool_calls:
length_continue_retries += 1
interim_msg = agent._build_assistant_message(assistant_message, finish_reason)
messages.append(interim_msg)
if assistant_message.content:
truncated_response_parts.append(assistant_message.content)
# An EMPTY partial-stream stub (stream dropped
# mid tool-call before any text was delivered)
# must not be appended as an interim assistant
# message: it would serialize as
# {"role": "assistant", "content": ""}, and
# strict providers (Moonshot/Kimi via OpenRouter)
# reject empty assistant content with HTTP 400
# ("message ... with role 'assistant' must not be
# empty") on the very next replay — permanently
# poisoning the session history. There is no
# partial text to continue from anyway, so only
# the continuation user-message is appended.
_is_empty_partial_stub = (
getattr(response, "id", "") == PARTIAL_STREAM_STUB_ID
and not getattr(assistant_message, "content", None)
)
if not _is_empty_partial_stub:
interim_msg = agent._build_assistant_message(assistant_message, finish_reason)
messages.append(interim_msg)
if assistant_message.content:
truncated_response_parts.append(assistant_message.content)

if length_continue_retries < 4:
_is_partial_stream_stub = (
Expand Down
307 changes: 307 additions & 0 deletions tests/run_agent/test_partial_stream_finish_reason.py
Original file line number Diff line number Diff line change
Expand Up @@ -592,3 +592,310 @@ def _fake_activate(reason=None):
"_try_activate_fallback — it should fall through to continuation."
)
assert result["completed"] is True


class TestEmptyPartialStreamStubNotPersisted:
"""Regression for the session-poisoning bug hit with moonshotai/kimi-k3
via OpenRouter (2026-07-20): a stream dropped mid-``write_file`` tool
call before ANY text was delivered. The partial-stream-stub carries
``content=""`` and ``tool_calls=None``, so the loop's truncation path
took the "no tool calls" branch and appended
``{"role": "assistant", "content": ""}`` to history before the
continuation user-message. Moonshot rejects empty assistant content
("the message at position N with role 'assistant' must not be empty")
with HTTP 400 on the very next replay — and since the message is
persisted, EVERY subsequent turn re-fails: session unrecoverable.

Fix layer 1 (conversation_loop): an empty partial-stream stub must not
be appended as an interim assistant message — only the continuation
user-message is.
"""

def test_empty_stub_only_appends_continuation_user_message(self, loop_agent):
from tests.run_agent.test_run_agent import _mock_response, _mock_assistant_msg

# First API call: empty partial-stream stub — stream died mid
# tool-call args with zero text delivered.
empty_stub = SimpleNamespace(
id=PARTIAL_STREAM_STUB_ID,
model="test/model",
choices=[SimpleNamespace(
index=0,
message=_mock_assistant_msg(content=""),
finish_reason=FINISH_REASON_LENGTH,
)],
usage=None,
_dropped_tool_names=["write_file"],
)
# Second API call: the model answers normally after the nudge.
recovery = _mock_response(content="Done — wrote it in chunks.",
finish_reason="stop")

loop_agent.client.chat.completions.create.side_effect = [
empty_stub, recovery,
]

with (
patch.object(loop_agent, "_persist_session"),
patch.object(loop_agent, "_save_trajectory"),
patch.object(loop_agent, "_cleanup_task_resources"),
):
result = loop_agent.run_conversation("make me a webpage")

assert loop_agent.client.chat.completions.create.call_count == 2

# Inspect the history replayed on the SECOND call: there must be NO
# empty-content assistant message anywhere — that is the exact shape
# Moonshot 400s on.
second_call_kwargs = loop_agent.client.chat.completions.create.call_args_list[1]
msgs = second_call_kwargs.kwargs.get("messages") or second_call_kwargs.args[0].get("messages")
empty_assistants = [
m for m in msgs
if m.get("role") == "assistant" and not m.get("content")
]
assert empty_assistants == [], (
"Empty partial-stream stub must not be persisted as an "
"empty-content assistant message — strict providers (Moonshot/"
"Kimi) reject the replay with HTTP 400 and poison the session."
)

# The continuation nudge is still appended as a user message, and
# it's the chunking variant (dropped tool call), not the length lie.
last_user = next(
(m for m in reversed(msgs) if m.get("role") == "user"), None,
)
assert last_user is not None
assert "too large" in (last_user.get("content") or "")
assert "output length limit" not in (last_user.get("content") or "")

assert result["completed"] is True

def test_non_empty_partial_stub_still_persisted(self, loop_agent):
"""Guard against over-correction: a stub that DID deliver partial
text must still be appended so the continuation stitches correctly
(existing behavior from #32086)."""
from tests.run_agent.test_run_agent import _mock_response, _mock_assistant_msg

partial_stub = SimpleNamespace(
id=PARTIAL_STREAM_STUB_ID,
model="test/model",
choices=[SimpleNamespace(
index=0,
message=_mock_assistant_msg(content="The first half of "),
finish_reason=FINISH_REASON_LENGTH,
)],
usage=None,
)
continuation = _mock_response(
content="the answer is forty-two.", finish_reason="stop",
)

loop_agent.client.chat.completions.create.side_effect = [
partial_stub, continuation,
]

with (
patch.object(loop_agent, "_persist_session"),
patch.object(loop_agent, "_save_trajectory"),
patch.object(loop_agent, "_cleanup_task_resources"),
):
result = loop_agent.run_conversation("ask me something")

second_call_kwargs = loop_agent.client.chat.completions.create.call_args_list[1]
msgs = second_call_kwargs.kwargs.get("messages") or second_call_kwargs.args[0].get("messages")
partial_assistants = [
m for m in msgs
if m.get("role") == "assistant" and "first half" in (m.get("content") or "")
]
assert partial_assistants, (
"A partial-stream stub WITH text must still be persisted so the "
"continuation can stitch the halves."
)
assert "first half of" in result["final_response"]
assert "forty-two" in result["final_response"]


class TestBuildAssistantMessageEmptyContentPad:
"""Regression layer 2 (chat_completion_helpers.build_assistant_message):
never serialize a textless assistant turn with ``content: ""`` — pad to
a single space, the same trick as the reasoning_content pad (#15250).
Tool-call turns are exempt (``content: ""`` + ``tool_calls`` is accepted
everywhere)."""

def _agent_for_builder(self):
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"),
):
a = AIAgent(
api_key="test-key-1234567890",
base_url="https://openrouter.ai/api/v1",
quiet_mode=True,
skip_context_files=True,
skip_memory=True,
)
return a

def test_empty_content_padded_to_space(self):
from agent.chat_completion_helpers import build_assistant_message
from tests.run_agent.test_run_agent import _mock_assistant_msg

agent = self._agent_for_builder()
msg = build_assistant_message(agent, _mock_assistant_msg(content=""), "stop")
assert msg["content"] == " ", (
"Textless assistant turn must be padded to a single space — "
"Moonshot/Kimi reject empty assistant content with HTTP 400."
)

def test_none_content_padded_to_space(self):
from agent.chat_completion_helpers import build_assistant_message
from tests.run_agent.test_run_agent import _mock_assistant_msg

agent = self._agent_for_builder()
msg = build_assistant_message(agent, _mock_assistant_msg(content=None), "stop")
assert msg["content"] == " "

def test_tool_call_turn_content_left_empty(self):
from agent.chat_completion_helpers import build_assistant_message
from tests.run_agent.test_run_agent import _mock_assistant_msg, _mock_tool_call

agent = self._agent_for_builder()
msg = build_assistant_message(
agent,
_mock_assistant_msg(content="", tool_calls=[_mock_tool_call()]),
"tool_calls",
)
assert msg["content"] == "", (
"Tool-call turns are exempt from the pad: content:'' alongside "
"tool_calls is accepted by every provider."
)
assert msg["tool_calls"]

def test_non_empty_content_unchanged(self):
from agent.chat_completion_helpers import build_assistant_message
from tests.run_agent.test_run_agent import _mock_assistant_msg

agent = self._agent_for_builder()
msg = build_assistant_message(agent, _mock_assistant_msg(content="hi"), "stop")
assert msg["content"] == "hi"


class TestSendTimeEmptyAssistantPad:
"""Durable repair for ALREADY-poisoned persisted sessions: a partial
-stream-stub row written by an older build (content:'' ,
finish_reason:'length') is rebuilt to content:'' on every reload —
``_rows_to_conversation`` strips whitespace, so a DB-side pad cannot
survive. The send-time pad in conversation_loop's api_messages loop
must therefore repair the empty textless assistant turn at the
serialization boundary, so a RESUMED poisoned session replays
cleanly against strict providers (Moonshot/Kimi HTTP 400 "message ...
with role 'assistant' must not be empty")."""

def _run_one_turn_with_history(self, loop_agent, history):
from tests.run_agent.test_run_agent import _mock_response
loop_agent.client.chat.completions.create.return_value = _mock_response(
content="ok", finish_reason="stop",
)
with (
patch.object(loop_agent, "_persist_session"),
patch.object(loop_agent, "_save_trajectory"),
patch.object(loop_agent, "_cleanup_task_resources"),
):
loop_agent.run_conversation(
"continue", conversation_history=history,
)
kwargs = loop_agent.client.chat.completions.create.call_args_list[0]
return kwargs.kwargs.get("messages") or kwargs.args[0].get("messages")

def test_poisoned_resumed_history_padded_on_send(self, loop_agent):
# Byte-shape of a persisted poisoned session:
# user -> assistant('' , finish_reason='length', NO tool_calls) -> user.
poisoned = [
{"role": "user", "content": "make me a webpage"},
{"role": "assistant", "content": "", "finish_reason": "length"},
{"role": "user", "content": "please proceed"},
]
sent = self._run_one_turn_with_history(loop_agent, poisoned)
empties = [
m for m in sent
if m.get("role") == "assistant"
and not m.get("tool_calls")
and m.get("content") == ""
]
assert empties == [], (
"A resumed session carrying a persisted empty partial-stream "
"stub must be repaired at the send boundary — strict providers "
"reject the replay with HTTP 400 otherwise."
)
stub = next(
(m for m in sent if m.get("role") == "assistant"
and not m.get("tool_calls")),
None,
)
assert stub is not None and stub["content"] == " "

def test_tool_call_turn_not_padded_on_send(self, loop_agent):
history = [
{"role": "user", "content": "search something"},
{
"role": "assistant",
"content": "",
"tool_calls": [{
"id": "call_1", "type": "function",
"function": {"name": "web_search", "arguments": "{}"},
}],
},
{"role": "tool", "tool_call_id": "call_1", "content": "result"},
{"role": "user", "content": "and now?"},
]
sent = self._run_one_turn_with_history(loop_agent, history)
tc_turn = next(
(m for m in sent if m.get("role") == "assistant" and m.get("tool_calls")),
None,
)
assert tc_turn is not None
assert tc_turn["content"] == "", (
"Tool-call turns are exempt from the pad: content:'' alongside "
"tool_calls is accepted by every provider and normalizing it "
"would alter prompt-cache keys."
)


class TestSendTimePadMultimodalSafety:
"""Regression: the send-time pad must skip multimodal (list) assistant
content instead of crashing — a forked session whose new user turn
attaches an image hit AttributeError: 'list' object has no attribute
'strip' inside the pad loop."""

def test_multimodal_assistant_content_not_touched(self, loop_agent):
from tests.run_agent.test_run_agent import _mock_response
multimodal = [
{"role": "user", "content": "look at this"},
{"role": "assistant", "content": [
{"type": "text", "text": "I see an image"},
]},
{"role": "user", "content": [
{"type": "text", "text": "animate it"},
{"type": "image_url", "image_url": {"url": "data:image/png;base64,iVBORw0KGgo="}},
]},
]
loop_agent.client.chat.completions.create.return_value = _mock_response(
content="ok", finish_reason="stop",
)
with (
patch.object(loop_agent, "_persist_session"),
patch.object(loop_agent, "_save_trajectory"),
patch.object(loop_agent, "_cleanup_task_resources"),
):
result = loop_agent.run_conversation(
"animate it", conversation_history=multimodal,
)
assert result["completed"] is True
kwargs = loop_agent.client.chat.completions.create.call_args_list[0]
sent = kwargs.kwargs.get("messages") or kwargs.args[0].get("messages")
mm = next(m for m in sent if isinstance(m.get("content"), list) and m.get("role") == "assistant")
assert mm["content"] == [{"type": "text", "text": "I see an image"}], (
"Multimodal assistant content must pass through untouched."
)
Loading