diff --git a/README.md b/README.md index 2aa33ce5aa..8ffbd9926e 100644 --- a/README.md +++ b/README.md @@ -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. diff --git a/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md b/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md index 11802329c8..10cd14bead 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md +++ b/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md @@ -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. diff --git a/backend/packages/harness/deerflow/agents/middlewares/tool_result_meta.py b/backend/packages/harness/deerflow/agents/middlewares/tool_result_meta.py index 7dc34c1362..1ad0959539 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/tool_result_meta.py +++ b/backend/packages/harness/deerflow/agents/middlewares/tool_result_meta.py @@ -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) :]) diff --git a/backend/tests/test_tool_result_meta_json_depth.py b/backend/tests/test_tool_result_meta_json_depth.py new file mode 100644 index 0000000000..1fa8735a24 --- /dev/null +++ b/backend/tests/test_tool_result_meta_json_depth.py @@ -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."