mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-06-15 07:14:08 +00:00
fix(session): never persist tool results without a declared tool call
This commit is contained in:
parent
2ebf7e2eef
commit
eb25df9b49
@ -1577,6 +1577,13 @@ class AgentLoop:
|
|||||||
"""Save new-turn messages into session, truncating large tool results."""
|
"""Save new-turn messages into session, truncating large tool results."""
|
||||||
from datetime import datetime
|
from datetime import datetime
|
||||||
|
|
||||||
|
declared_tool_call_ids = {
|
||||||
|
str(tc["id"])
|
||||||
|
for m in session.messages
|
||||||
|
if m.get("role") == "assistant"
|
||||||
|
for tc in m.get("tool_calls") or []
|
||||||
|
if isinstance(tc, dict) and tc.get("id")
|
||||||
|
}
|
||||||
last_assistant_idx: int | None = None
|
last_assistant_idx: int | None = None
|
||||||
for m in messages[skip:]:
|
for m in messages[skip:]:
|
||||||
entry = dict(m)
|
entry = dict(m)
|
||||||
@ -1584,6 +1591,17 @@ class AgentLoop:
|
|||||||
if role == "assistant" and not content and not entry.get("tool_calls"):
|
if role == "assistant" and not content and not entry.get("tool_calls"):
|
||||||
continue # skip empty assistant messages — they poison session context
|
continue # skip empty assistant messages — they poison session context
|
||||||
if role == "tool":
|
if role == "tool":
|
||||||
|
tool_call_id = entry.get("tool_call_id")
|
||||||
|
if tool_call_id and str(tool_call_id) not in declared_tool_call_ids:
|
||||||
|
# A tool result without a declared call violates the
|
||||||
|
# OpenAI/Anthropic pairing contract and would poison
|
||||||
|
# every future request built from this session (#4006).
|
||||||
|
logger.warning(
|
||||||
|
"Dropping orphaned tool result {} from session {} during persistence",
|
||||||
|
tool_call_id,
|
||||||
|
session.key,
|
||||||
|
)
|
||||||
|
continue
|
||||||
if isinstance(content, str) and len(content) > self.max_tool_result_chars:
|
if isinstance(content, str) and len(content) > self.max_tool_result_chars:
|
||||||
entry["content"] = truncate_text_fn(content, self.max_tool_result_chars)
|
entry["content"] = truncate_text_fn(content, self.max_tool_result_chars)
|
||||||
elif isinstance(content, list):
|
elif isinstance(content, list):
|
||||||
@ -1613,6 +1631,11 @@ class AgentLoop:
|
|||||||
session.messages.append(entry)
|
session.messages.append(entry)
|
||||||
if role == "assistant":
|
if role == "assistant":
|
||||||
last_assistant_idx = len(session.messages) - 1
|
last_assistant_idx = len(session.messages) - 1
|
||||||
|
declared_tool_call_ids.update(
|
||||||
|
str(tc["id"])
|
||||||
|
for tc in entry.get("tool_calls") or []
|
||||||
|
if isinstance(tc, dict) and tc.get("id")
|
||||||
|
)
|
||||||
if turn_latency_ms is not None and last_assistant_idx is not None:
|
if turn_latency_ms is not None and last_assistant_idx is not None:
|
||||||
session.messages[last_assistant_idx]["latency_ms"] = int(turn_latency_ms)
|
session.messages[last_assistant_idx]["latency_ms"] = int(turn_latency_ms)
|
||||||
session.updated_at = datetime.now()
|
session.updated_at = datetime.now()
|
||||||
|
|||||||
@ -286,11 +286,22 @@ def test_save_turn_keeps_tool_results_under_16k() -> None:
|
|||||||
|
|
||||||
loop._save_turn(
|
loop._save_turn(
|
||||||
session,
|
session,
|
||||||
[{"role": "tool", "tool_call_id": "call_1", "name": "read_file", "content": content}],
|
[
|
||||||
|
{
|
||||||
|
"role": "assistant",
|
||||||
|
"content": "",
|
||||||
|
"tool_calls": [{
|
||||||
|
"id": "call_1",
|
||||||
|
"type": "function",
|
||||||
|
"function": {"name": "read_file", "arguments": "{}"},
|
||||||
|
}],
|
||||||
|
},
|
||||||
|
{"role": "tool", "tool_call_id": "call_1", "name": "read_file", "content": content},
|
||||||
|
],
|
||||||
skip=0,
|
skip=0,
|
||||||
)
|
)
|
||||||
|
|
||||||
assert session.messages[0]["content"] == content
|
assert session.messages[1]["content"] == content
|
||||||
|
|
||||||
|
|
||||||
def test_save_turn_stamps_latency_on_last_assistant() -> None:
|
def test_save_turn_stamps_latency_on_last_assistant() -> None:
|
||||||
@ -1374,3 +1385,46 @@ def test_save_turn_keeps_placeholder_for_empty_tool_result_blocks() -> None:
|
|||||||
assert session.messages[1]["content"] == [
|
assert session.messages[1]["content"] == [
|
||||||
{"type": "text", "text": "[tool result omitted during persistence]"}
|
{"type": "text", "text": "[tool result omitted during persistence]"}
|
||||||
]
|
]
|
||||||
|
|
||||||
|
|
||||||
|
def test_save_turn_drops_orphaned_tool_results() -> None:
|
||||||
|
# Defense in depth for #4006: whatever upstream bug produces a tool
|
||||||
|
# result whose call was never declared, it must not reach history.
|
||||||
|
loop = _mk_loop()
|
||||||
|
session = Session(key="test:orphan-guard")
|
||||||
|
session.add_message("user", "hi")
|
||||||
|
|
||||||
|
loop._save_turn(
|
||||||
|
session,
|
||||||
|
[
|
||||||
|
{"role": "tool", "tool_call_id": "call_ghost", "name": "exec", "content": "boo"},
|
||||||
|
{"role": "assistant", "content": "done"},
|
||||||
|
],
|
||||||
|
skip=0,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert [m["role"] for m in session.messages] == ["user", "assistant"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_save_turn_keeps_tool_results_declared_in_prior_history() -> None:
|
||||||
|
# Declarations may live in already-persisted history (e.g. a restored
|
||||||
|
# runtime checkpoint), not only in the new-turn slice.
|
||||||
|
loop = _mk_loop()
|
||||||
|
session = Session(key="test:prior-declared")
|
||||||
|
session.add_message(
|
||||||
|
"assistant",
|
||||||
|
"working",
|
||||||
|
tool_calls=[{
|
||||||
|
"id": "call_prior",
|
||||||
|
"type": "function",
|
||||||
|
"function": {"name": "exec", "arguments": "{}"},
|
||||||
|
}],
|
||||||
|
)
|
||||||
|
|
||||||
|
loop._save_turn(
|
||||||
|
session,
|
||||||
|
[{"role": "tool", "tool_call_id": "call_prior", "name": "exec", "content": "ok"}],
|
||||||
|
skip=0,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert [m["role"] for m in session.messages] == ["assistant", "tool"]
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user