mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-30 15:10:41 +00:00
fix(agents): tolerate deeply nested JSON tool results (#7679)
* fix(agents): tolerate deeply nested JSON tool results * test(agents): verify the deep JSON parser premise Signed-off-by: Willem Jiang <willem.jiang@gmail.com> --------- Signed-off-by: Willem Jiang <willem.jiang@gmail.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
parent
3f1438d4a0
commit
2488ba7c27
@ -2985,6 +2985,8 @@ Each task gets its own execution environment with a full filesystem view — ski
|
||||
|
||||
The read-before-write gate ties each read mark to that `read_file` call's result, including custom tools returning multi-message `Command` updates. An unrelated result cannot authorize a write after a failed read or hide a successful read.
|
||||
|
||||
Deeply nested JSON returned by tools, including `read_file`, keeps its original content when result analysis reaches the JSON parser's depth limit. Explicit tool failures remain errors with an unknown category when depth prevents analysis.
|
||||
|
||||
On Linux and macOS, local sandbox `write_file` replacements and `str_replace` stage complete text before replacing the destination, so write, flush or publication errors retain the previous file. Existing write access is checked; ownership, permission bits, ACLs and extended attributes are transferred before writing staging content. Failure to transfer metadata aborts the save. Successful revisions get a fresh modification time, including empty saves; source-descriptor cleanup errors are warnings and do not change the save result. Files with set-id bits, macOS file flags or content-bound Linux privilege attributes are refused unchanged. Replacement requires parent-directory write access and publishes a new inode; other hard links retain the previous version. Symlinks follow the existing path-validation rules. Append writes, other host platforms (including Windows), shell writes and remote providers keep their existing behavior. This is not a concurrent-writer or power-loss recovery guarantee.
|
||||
|
||||
Loop warnings let agents save collected results before their final answer. If the save is blocked by the read-before-write gate, the agent may read the target file and retry the save once. A looping or stopped tool must be replaced by an available alternative; if no permitted save path exists or the retry fails, the final answer includes the collected results and reports that the file was not saved. When `present_files` is available, the warning also asks the agent to present the files it already created in `/mnt/user-data/outputs` before answering, so finished outputs are delivered. Recovery calls still count toward the loop limits, and a hard stop prevents further tool execution.
|
||||
|
||||
@ -1,5 +1,13 @@
|
||||
### Middleware Chain
|
||||
|
||||
Tool-result JSON analysis is best-effort: parser depth failures must preserve the
|
||||
original content and must not abort the graph. Explicit error results fall back
|
||||
to `unknown` / recoverable / `try_alternative`, without classifying incidental
|
||||
JSON fields as auth failures. Producer metadata remains authoritative. Keep
|
||||
depth regressions in `tests/test_tool_result_meta_json_depth.py`, including real
|
||||
LocalSandbox reads through both synchronous and asynchronous agent graphs.
|
||||
Deep graph cases must establish a real parser depth failure or report a skip.
|
||||
|
||||
Async gate waiters own separate cross-loop futures. Cancel only their signal;
|
||||
notify outside the state lock. `_await_off_thread` drains started I/O across
|
||||
repeated cancellation.
|
||||
|
||||
@ -156,7 +156,7 @@ def _extract_json_error_text(content: str) -> str | None:
|
||||
"""
|
||||
try:
|
||||
data = json.loads(content)
|
||||
except (json.JSONDecodeError, ValueError):
|
||||
except (json.JSONDecodeError, ValueError, RecursionError):
|
||||
return None
|
||||
error = data.get("error") if isinstance(data, dict) else None
|
||||
if not error:
|
||||
@ -305,9 +305,14 @@ def normalize_tool_message(msg: ToolMessage) -> ToolMessage:
|
||||
# the tool. Classify raw text only when the content is not valid JSON.
|
||||
try:
|
||||
is_json_dict = isinstance(json.loads(content), dict)
|
||||
except RecursionError:
|
||||
# Depth-limited analysis cannot establish an error category.
|
||||
# Do not infer auth/stop from incidental fields in the raw JSON.
|
||||
attrs = {**_UNKNOWN_ERROR}
|
||||
except (json.JSONDecodeError, ValueError):
|
||||
is_json_dict = False
|
||||
attrs = {**_UNKNOWN_ERROR} if is_json_dict else _classify_error_text(content)
|
||||
attrs = _classify_error_text(content)
|
||||
else:
|
||||
attrs = {**_UNKNOWN_ERROR} if is_json_dict else _classify_error_text(content)
|
||||
meta = _make_meta(status="error", source="tool_return", **attrs)
|
||||
elif content.startswith(_ERROR_PREFIX):
|
||||
attrs = _classify_error_text(content[len(_ERROR_PREFIX) :])
|
||||
|
||||
120
backend/tests/test_tool_result_meta_json_depth.py
Normal file
120
backend/tests/test_tool_result_meta_json_depth.py
Normal file
@ -0,0 +1,120 @@
|
||||
"""JSON depth limits must not replace successful native tool results."""
|
||||
|
||||
import json
|
||||
from unittest.mock import AsyncMock, Mock
|
||||
|
||||
import pytest
|
||||
from langchain.agents import create_agent
|
||||
from langchain_core.language_models.fake_chat_models import GenericFakeChatModel
|
||||
from langchain_core.messages import AIMessage, HumanMessage, ToolMessage
|
||||
|
||||
from deerflow.agents.middlewares import tool_result_meta as meta
|
||||
from deerflow.agents.middlewares.tool_error_handling_middleware import ToolErrorHandlingMiddleware
|
||||
from deerflow.agents.middlewares.tool_receipt import TOOL_RECEIPT_KEY
|
||||
from deerflow.agents.middlewares.tool_receipt_middleware import ToolReceiptMiddleware
|
||||
from deerflow.agents.thread_state import ThreadState
|
||||
|
||||
|
||||
@pytest.mark.parametrize("status", ["success", "error"])
|
||||
def test_json_depth_limit_preserves_result(monkeypatch, status):
|
||||
content = '{"records": [[[0]]], "user_id": 401}'
|
||||
loads = Mock(side_effect=RecursionError("parser depth limit"))
|
||||
monkeypatch.setattr(meta.json, "loads", loads)
|
||||
message = ToolMessage(content=content, name="custom_tool", tool_call_id="depth", status=status)
|
||||
|
||||
result = meta.normalize_tool_message(message)
|
||||
|
||||
assert result is message
|
||||
assert result.content == content
|
||||
assert result.tool_call_id == "depth"
|
||||
assert result.status == status
|
||||
assert result.additional_kwargs[meta.TOOL_META_KEY] == {
|
||||
"status": status,
|
||||
"error_type": "unknown" if status == "error" else None,
|
||||
"recoverable_by_model": True,
|
||||
"recommended_next_action": "try_alternative" if status == "error" else "continue",
|
||||
"source": "tool_return" if status == "error" else "content_analysis",
|
||||
}
|
||||
assert loads.call_count == (2 if status == "error" else 1)
|
||||
|
||||
|
||||
def test_second_json_probe_depth_limit_is_unknown(monkeypatch):
|
||||
loads = Mock(side_effect=[{}, RecursionError("parser depth limit")])
|
||||
monkeypatch.setattr(meta.json, "loads", loads)
|
||||
message = ToolMessage(content='{"user_id": 401}', tool_call_id="depth", status="error")
|
||||
|
||||
result = meta.normalize_tool_message(message)
|
||||
|
||||
assert loads.call_count == 2
|
||||
assert result.status == "error"
|
||||
assert result.additional_kwargs[meta.TOOL_META_KEY]["error_type"] == "unknown"
|
||||
assert result.additional_kwargs[meta.TOOL_META_KEY]["recommended_next_action"] == "try_alternative"
|
||||
|
||||
|
||||
def test_producer_metadata_bypasses_depth_analysis(monkeypatch):
|
||||
loads = Mock(side_effect=RecursionError("parser depth limit"))
|
||||
monkeypatch.setattr(meta.json, "loads", loads)
|
||||
stamp = {"status": "error", "error_type": "auth", "source": "tool_return"}
|
||||
message = ToolMessage(content="[0]", tool_call_id="depth", additional_kwargs={meta.TOOL_META_KEY: stamp})
|
||||
|
||||
assert meta.normalize_tool_message(message).additional_kwargs[meta.TOOL_META_KEY] is stamp
|
||||
loads.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.parametrize("name,expected", [("custom_tool", "error"), ("read_file", "success")])
|
||||
def test_shallow_error_envelope_classification(name, expected):
|
||||
message = ToolMessage(content='{"error": {"code": 401}}', name=name, tool_call_id="shallow")
|
||||
result = meta.normalize_tool_message(message)
|
||||
assert result.additional_kwargs[meta.TOOL_META_KEY]["status"] == expected
|
||||
assert result.additional_kwargs[meta.TOOL_META_KEY]["error_type"] == ("auth" if expected == "error" else None)
|
||||
|
||||
|
||||
class _ScriptedModel(GenericFakeChatModel):
|
||||
def bind_tools(self, tools, **kwargs):
|
||||
return self
|
||||
|
||||
|
||||
@pytest.mark.parametrize("mode", ["sync", "async"])
|
||||
@pytest.mark.parametrize("depth", [10, 10000])
|
||||
@pytest.mark.asyncio
|
||||
async def test_native_read_file_graph_preserves_deep_json(tmp_path, monkeypatch, mode, depth):
|
||||
from deerflow.sandbox import tools as sandbox_tools
|
||||
from deerflow.sandbox.local.local_sandbox import LocalSandbox
|
||||
|
||||
# String construction avoids recursive Python fixtures. Count the deep case
|
||||
# as regression coverage only when the real parser rejects this fixture.
|
||||
content = "[" * depth + "0" + "]" * depth
|
||||
if depth > 10:
|
||||
try:
|
||||
json.loads(content)
|
||||
except RecursionError:
|
||||
pass
|
||||
else:
|
||||
pytest.skip(f"json.loads accepts {depth} nested arrays; parser depth limit is not exercised")
|
||||
path = tmp_path / "nested.json"
|
||||
path.write_text(content, encoding="utf-8")
|
||||
sandbox = LocalSandbox("json-depth-test")
|
||||
monkeypatch.setattr(sandbox_tools, "ensure_sandbox_initialized", lambda *_args, **_kwargs: sandbox)
|
||||
monkeypatch.setattr(sandbox_tools, "ensure_sandbox_initialized_async", AsyncMock(return_value=sandbox))
|
||||
responses = [AIMessage(content="", tool_calls=[{"name": "read_file", "args": {"path": str(path)}, "id": "read-depth"}]), AIMessage(content="Done.")]
|
||||
graph = create_agent(
|
||||
_ScriptedModel(messages=iter(responses)),
|
||||
tools=[sandbox_tools.read_file_tool],
|
||||
middleware=[ToolReceiptMiddleware(render_mode="delegation_only"), ToolErrorHandlingMiddleware()],
|
||||
state_schema=ThreadState,
|
||||
)
|
||||
state = {
|
||||
"messages": [HumanMessage(content="Read the JSON file.")],
|
||||
"sandbox": {"sandbox_id": "json-depth-test"},
|
||||
"thread_data": {"outputs_path": str(tmp_path)},
|
||||
"thread_directories_created": True,
|
||||
}
|
||||
context = {"thread_id": "json-depth-test", "user_id": "default"}
|
||||
result = graph.invoke(state, context=context) if mode == "sync" else await graph.ainvoke(state, context=context)
|
||||
message = next(message for message in result["messages"] if isinstance(message, ToolMessage))
|
||||
assert message.content == content
|
||||
assert message.tool_call_id == "read-depth"
|
||||
assert message.status == "success"
|
||||
assert message.additional_kwargs[meta.TOOL_META_KEY]["status"] == "success"
|
||||
assert message.additional_kwargs[TOOL_RECEIPT_KEY]["status"] == "success"
|
||||
assert result["messages"][-1].content == "Done."
|
||||
Loading…
x
Reference in New Issue
Block a user