Skip to content

Commit f151945

Browse files
committed
update
1 parent fe757da commit f151945

File tree

3 files changed

+119
-2
lines changed

3 files changed

+119
-2
lines changed

PR_DESCRIPTION.md

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
# Fix: prevent orphan `tool` messages when `enable_snapshot_clean` is on
2+
3+
## Summary
4+
This PR fixes a state-leak issue in `ChatAgent` that can produce invalid message history (orphan `tool` messages) across agent reuse scenarios.
5+
6+
## Root Cause
7+
- `ChatAgent` keeps snapshot-clean metadata in `_tool_output_history`.
8+
- `reset()` previously did not clear this cache.
9+
- When an agent instance is reused (e.g., worker pool), stale entries from a previous task can still be processed.
10+
- `_clean_snapshot_in_memory()` could rewrite a cleaned `FUNCTION/tool` record even if the original referenced records no longer existed in current memory.
11+
- This may inject orphan `tool` messages (without preceding `assistant.tool_calls`), which can trigger strict backend validation errors (e.g., Azure/LiteLLM 400).
12+
13+
## Fix Approach
14+
1. Clear snapshot-clean cache on reset:
15+
- `ChatAgent.reset()` now clears `_tool_output_history`.
16+
2. Add safe-guard before rewriting cleaned tool output:
17+
- In `_clean_snapshot_in_memory()`, only rewrite when referenced record UUIDs still exist in storage.
18+
- If no referenced records are found, skip rewrite and mark the entry as cached.
19+
20+
## Why this is safe
21+
- Normal snapshot-clean flow is unchanged when records exist.
22+
- Only stale/cross-task cache entries are blocked from writing new `tool` records.
23+
- This prevents invalid cross-conversation contamination without affecting valid tool-call chains.
24+
25+
## Tests
26+
Added unit tests in `test/agents/test_chat_agent.py`:
27+
- `test_chat_agent_reset_clears_tool_output_history`
28+
- `test_clean_snapshot_in_memory_skips_missing_records`
29+

camel/agents/chat_agent.py

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -635,6 +635,9 @@ def reset(self):
635635
r"""Resets the :obj:`ChatAgent` to its initial state."""
636636
self.terminated = False
637637
self.init_messages()
638+
# Snapshot-clean cache is per-conversation state and must not survive
639+
# agent reuse (e.g. pooled workers across different tasks).
640+
self._tool_output_history.clear()
638641
for terminator in self.response_terminators:
639642
terminator.reset()
640643

@@ -1504,10 +1507,29 @@ def _clean_snapshot_in_memory(
15041507
return
15051508

15061509
existing_records = storage.load()
1510+
existing_record_uuids = {
1511+
str(record.get("uuid"))
1512+
for record in existing_records
1513+
if record.get("uuid") is not None
1514+
}
1515+
matched_uuids = [
1516+
record_uuid
1517+
for record_uuid in entry.record_uuids
1518+
if record_uuid in existing_record_uuids
1519+
]
1520+
1521+
if not matched_uuids:
1522+
# Record no longer exists in current memory (e.g. after reset).
1523+
# Skip rewriting to avoid injecting orphan tool messages.
1524+
entry.cached = True
1525+
entry.record_uuids = []
1526+
entry.record_timestamps = []
1527+
return
1528+
15071529
updated_records = [
15081530
record
15091531
for record in existing_records
1510-
if record["uuid"] not in entry.record_uuids
1532+
if str(record.get("uuid")) not in matched_uuids
15111533
]
15121534
new_record = MemoryRecord(
15131535
message=cleaned_message,

test/agents/test_chat_agent.py

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,16 @@
3030
from pydantic import BaseModel, Field
3131

3232
from camel.agents import ChatAgent
33-
from camel.agents.chat_agent import StreamContentAccumulator, ToolCallingRecord
33+
from camel.agents.chat_agent import (
34+
StreamContentAccumulator,
35+
ToolCallingRecord,
36+
_ToolOutputHistoryEntry,
37+
)
3438
from camel.configs import ChatGPTConfig
3539
from camel.generators import SystemMessageGenerator
3640
from camel.memories import MemoryRecord
3741
from camel.messages import BaseMessage
42+
from camel.models import BaseModelBackend
3843
from camel.models import AnthropicModel, ModelFactory, OpenAIModel
3944
from camel.terminators import ResponseWordsTerminator
4045
from camel.toolkits import (
@@ -90,6 +95,18 @@
9095
)
9196

9297

98+
class DummyModel(BaseModelBackend):
99+
@property
100+
def token_counter(self):
101+
return self._token_counter
102+
103+
def _run(self, messages, response_format=None, tools=None):
104+
raise NotImplementedError
105+
106+
async def _arun(self, messages, response_format=None, tools=None):
107+
raise NotImplementedError
108+
109+
93110
@parametrize
94111
def test_chat_agent(model, step_call_count=3):
95112
model = model
@@ -145,6 +162,55 @@ def test_chat_agent(model, step_call_count=3):
145162
), f"Error in round {i + 1}"
146163

147164

165+
def test_chat_agent_reset_clears_tool_output_history():
166+
model = DummyModel(ModelType.GPT_4O_MINI)
167+
assistant = ChatAgent(
168+
system_message="You are a helpful assistant.",
169+
model=model,
170+
enable_snapshot_clean=True,
171+
)
172+
assistant._tool_output_history = [
173+
_ToolOutputHistoryEntry(
174+
tool_name="tool_a",
175+
tool_call_id="call_a",
176+
result_text="old",
177+
record_uuids=["uuid_a"],
178+
record_timestamps=[1.0],
179+
)
180+
]
181+
182+
assistant.reset()
183+
184+
assert assistant._tool_output_history == []
185+
186+
187+
def test_clean_snapshot_in_memory_skips_missing_records():
188+
model = DummyModel(ModelType.GPT_4O_MINI)
189+
assistant = ChatAgent(
190+
system_message="You are a helpful assistant.",
191+
model=model,
192+
enable_snapshot_clean=True,
193+
)
194+
entry = _ToolOutputHistoryEntry(
195+
tool_name="browser_get_page_snapshot",
196+
tool_call_id="call_missing",
197+
result_text="- button [ref=1]",
198+
record_uuids=["missing_uuid"],
199+
record_timestamps=[123.0],
200+
)
201+
202+
chat_history_block = getattr(assistant.memory, "_chat_history_block", None)
203+
storage = getattr(chat_history_block, "storage", None)
204+
assert storage is not None
205+
records_before = storage.load()
206+
207+
assistant._clean_snapshot_in_memory(entry)
208+
209+
records_after = storage.load()
210+
assert records_after == records_before
211+
assert entry.cached is True
212+
213+
148214
@pytest.mark.model_backend
149215
def test_chat_agent_stored_messages():
150216
system_msg = BaseMessage(

0 commit comments

Comments
 (0)