fix(agents): treat read_file JSON errors as document data (#7369)

Co-authored-by: haotianqi666 <250384991+haotianqi666@users.noreply.github.com>
This commit is contained in:
haotianqi666 2026-10-24 08:06:38 +08:00 committed by GitHub
parent 8c2f066159
commit 868c1ff807
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 145 additions and 2 deletions

View File

@ -954,6 +954,12 @@ BoxLite shutdown rejects late VM registration and keeps its SDK loop open while
in-flight acquisitions finish. If they cannot drain within five seconds,
shutdown fails with resources still owned and can be retried.
Successful `read_file` calls treat a JSON document's `error` field as file data,
not a failure of the read tool. This keeps receipts accurate when inspecting
saved API error responses and avoids blocking later reads when the optional
`tool_progress` guard is enabled. Actual read failures and output truncation
still retain their error or partial-result status.
#### MCP Server
In the chat UI, enable **Token Usage → Debug** to inspect generic/MCP tool calls.

View File

@ -114,6 +114,12 @@ No match: skip inspection, log ID. Tests: `test_read_mark_tool_call_correlation.
13. **ReadBeforeWriteMiddleware** - *(`read_before_write.enabled`, default on; #3857)* Existing-file `write_file` (including append) and `str_replace` require the newest matching `read_file` content hash. Marks live on ToolMessages: dropping a read loses its mark; writes never refresh it. Sync/async read/check/execution/mark steps share a per-(scope, path) gate. Uninspectable content fails open. It owns sandbox authorization; `SandboxAuthorizationError` becomes an error ToolMessage. Outside ToolProgress/ToolErrorHandling, blocks consume no slot and stamp `deerflow_tool_meta` plus `deerflow_write_block` (`{path, tool}`). `wrap_model_call` elides blocked payloads with deterministic placeholders (`elide_blocked_payloads`, `elide_min_chars`), preserving state, receipts and journal. All model-bound argument rewrites use `tool_call_args`; policy stays in this gate.
14. **ToolProgressMiddleware** - *(optional, if `tool_progress.enabled`)* State-machine-based stagnation guard (RFC #3177). Outer wrapper around ToolErrorHandlingMiddleware so its tool wrapper receives results already stamped with `deerflow_tool_meta`. Recoverable problems stay WARNED with hints; retryable non-recoverable problems escalate to BLOCKED; stop-category failures block immediately. It is independent of LoopDetection's turn-level call-pattern guard. Effective phase changes emit `middleware:tool_progress`, including `reset` when a later agent invocation (for example, goal continuation) clears WARNED/BLOCKED state. The state machine supplies the action and count threshold (`null` for category rules, recovery, and reset); recorder calls happen after releasing the state lock. Events include the actual sync/async hook plus bounded status, error, and recovery fields. Server-owned recorder keys determine subagent attribution. Arguments, content, prompts, and hashes are never persisted; recorder failures are fail-open. See [event semantics](../../../../../docs/RUN_EVENT_STREAM.md).
15. **ToolReceiptMiddleware + ToolErrorHandlingMiddleware** - `ToolReceiptMiddleware` is *(optional, if `verification.receipts_enabled`, default on)*. It is the **outermost `wrap_tool_call` layer** — registered ahead of authorization, audit, write and progress gates — because Guardrail/SandboxAudit/ReadBeforeWrite/ToolProgress can short-circuit a call with their own ToolMessage (and SandboxAudit annotates medium-risk results); an inner receipt layer would silently gap the ledger on those results (ordering constraints in `deerflow.extensions.ordering`). Normal results still carry the `deerflow_tool_meta` status ToolErrorHandlingMiddleware stamps on the inner return path; short-circuit messages self-stamp meta or fall back to `message.status`. It stamps deterministic provenance (tool name, status, args/output hashes, byte count, timestamp) onto direct `ToolMessage` results and every matching `ToolMessage` carried in `Command.update.messages`, including delegated `task`, `present_file`, `view_image`, and `tool_search` results; before model calls it derives a hidden receipt ledger (display ids r1..rN) from message state, and when the 2,000-character budget is exceeded the newest receipts are retained in chronological order with their original ids plus an older-receipts omission marker. Rendering returns both the text and its retained receipt subset; every response that received a ledger carries only that exact server-owned subset, never omitted receipts. Snapshot validation accepts a strictly consecutive positive original-id range (for example `r24`–`r30`) rather than requiring `r1`, so subagent terminal citation verification resolves ids against evidence present in the citing turn even when later summarization drops and renumbers tool messages. Model-generated citation IDs are digit-bounded before integer conversion; oversized IDs are ignored as malformed input rather than raising through task write-back. Citation parsing deduplicates exact `(id, anchor)` pairs, not IDs alone, so repeated identical references stay compact while every distinct anchor claim is verified. Gateway strips delegated receipts/verdicts from external messages. `ToolErrorHandlingMiddleware` receives `AppConfig`, converts tool exceptions into error `ToolMessage`s so the run can continue instead of aborting, stamps every result with `deerflow_tool_meta` (status / error_type / recoverable_by_model / recommended_next_action / source) via `tool_result_meta.normalize_tool_result`, stamps structured metadata for task exception wrappers, and stamps skill-read metadata for downstream durable-context capture. Task tool result text is generated from the same status/result/error inputs as the structured metadata so callers do not hand-write a second protocol string.
JSON error-envelope inference excludes successful native `read_file` results:
their `error` fields belong to the document. Explicit error status, existing
metadata and legacy `Error:` failures remain authoritative; other tools keep
JSON error inference. Real truncation markers still produce partial results.
`tests/test_read_file_json_result_meta.py` exercises native sync/async reads,
receipts and follow-up calls with the progress guard on and off.
16. **ArtifactCaptureMiddleware** - See [tool artifacts](TOOL_ARTIFACTS.md).
Authorization identity is independent of enforcement. Gateway strips client identity overrides: only the server auth source sets `is_internal`, and only authenticated IM `body.context` supplies `channel_user_id` (never `body.config`). `build_principal_from_context` applies role defaults, strict provenance, and copied attributes; RBAC rejects unknown defaults. Delegation and `GuardrailMiddleware` share this identity. Layer 1 precedes deferred assembly across agent paths and its provider is reused for Layer 2; framework skill/memory ordering stays stable. Trusted `DeerFlowClient.stream()` accepts identity overrides. Its graph key always includes effective storage `user_id` and, when enforced, the full Principal; nested attributes are copied so mutation cannot hide stale cache state.

View File

@ -313,8 +313,13 @@ def normalize_tool_message(msg: ToolMessage) -> ToolMessage:
attrs = _classify_error_text(content[len(_ERROR_PREFIX) :])
meta = _make_meta(status="error", source="tool_return", **attrs)
elif (json_error := _extract_json_error_text(content)) is not None:
attrs = _classify_error_text(json_error)
meta = _make_meta(status="error", source="tool_return", **attrs)
if msg.name == "read_file":
# Native reads return document text, not a tool error envelope.
# Explicit failures above still take precedence over file content.
meta = _make_meta(status="success", source="content_analysis")
else:
attrs = _classify_error_text(json_error)
meta = _make_meta(status="error", source="tool_return", **attrs)
elif failed_bash:
# A nonzero shell exit does not raise a Python exception. Keep it
# recoverable: grep/diff and diagnostic probes can legitimately use 1.

View File

@ -0,0 +1,126 @@
"""File contents are not a JSON error envelope from the read_file tool."""
from __future__ import annotations
import json
from unittest.mock import AsyncMock
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.read_before_write_middleware import ReadBeforeWriteMiddleware
from deerflow.agents.middlewares.tool_error_handling_middleware import ToolErrorHandlingMiddleware
from deerflow.agents.middlewares.tool_progress_middleware import ToolProgressMiddleware
from deerflow.agents.middlewares.tool_receipt import TOOL_RECEIPT_KEY
from deerflow.agents.middlewares.tool_receipt_middleware import ToolReceiptMiddleware
from deerflow.agents.middlewares.tool_result_meta import TOOL_META_KEY, normalize_tool_message
from deerflow.agents.thread_state import ThreadState
from deerflow.config.tool_progress_config import ToolProgressConfig
@pytest.mark.parametrize("error", ["Unauthorized", {"code": 401, "message": "Unauthorized"}, ["missing config"], 500, True, "no results found"])
def test_read_file_json_error_field_is_document_data(error):
content = json.dumps({"error": error})
message = ToolMessage(content=content, name="read_file", tool_call_id="read")
result = normalize_tool_message(message)
assert result.content == content
assert result.additional_kwargs[TOOL_META_KEY] == {
"status": "success",
"error_type": None,
"recoverable_by_model": True,
"recommended_next_action": "continue",
"source": "content_analysis",
}
@pytest.mark.parametrize("name,status", [("read_file", "error"), ("web_search", "success"), ("custom_tool", "success")])
def test_actual_tool_json_errors_are_still_classified(name, status):
message = ToolMessage(content='{"error": {"code": 401, "message": "Unauthorized"}}', name=name, tool_call_id="call", status=status)
meta = normalize_tool_message(message).additional_kwargs[TOOL_META_KEY]
assert meta["status"] == "error"
assert meta["error_type"] == "auth"
assert meta["recoverable_by_model"] is False
assert meta["recommended_next_action"] == "stop"
@pytest.mark.parametrize("source", ["exception", "tool_return", "custom"])
def test_read_file_preserves_explicit_failure_metadata(source):
metadata = {"status": "error", "source": source, "error_type": "auth"}
message = ToolMessage(content='{"error":"Unauthorized"}', name="read_file", tool_call_id="read", additional_kwargs={TOOL_META_KEY: metadata})
assert normalize_tool_message(message).additional_kwargs[TOOL_META_KEY] is metadata
class _ScriptedModel(GenericFakeChatModel):
def bind_tools(self, tools, **kwargs):
return self
@pytest.mark.parametrize("mode", ["sync", "async"])
@pytest.mark.parametrize("progress_enabled", [False, True])
@pytest.mark.parametrize("first_kind", ["json_error", "success", "missing", "truncated"])
@pytest.mark.asyncio
async def test_native_read_file_keeps_followup_reads_available(tmp_path, monkeypatch, mode, progress_enabled, first_kind):
from deerflow.sandbox import tools as sandbox_tools
from deerflow.sandbox.local.local_sandbox import LocalSandbox
outputs = tmp_path / "outputs"
outputs.mkdir()
first_content = '{"error": {"code": 401, "message": "Unauthorized"}}\n'
if first_kind == "success":
first_content = '{"result": "OK"}\n'
elif first_kind == "truncated":
first_content = json.dumps({"error": "Unauthorized", "details": "sample line\n" * 10000})
if first_kind != "missing":
(outputs / "response.json").write_text(first_content, encoding="utf-8")
notes = "The saved response belongs to an earlier request.\n"
(outputs / "notes.txt").write_text(notes, encoding="utf-8")
sandbox = LocalSandbox("json-result-test")
reads = []
original_read = sandbox.read_file
def trace_read(path, *args, **kwargs):
reads.append(path)
return original_read(path, *args, **kwargs)
monkeypatch.setattr(sandbox, "read_file", trace_read)
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": f"/mnt/user-data/outputs/{filename}"}, "id": f"read-{index}"}]) for index, filename in enumerate(("response.json", "notes.txt"))]
responses.append(AIMessage(content="Done."))
middleware = [ToolReceiptMiddleware(render_mode="delegation_only"), ReadBeforeWriteMiddleware()]
if progress_enabled:
middleware.append(ToolProgressMiddleware.from_config(ToolProgressConfig(enabled=True)))
middleware.append(ToolErrorHandlingMiddleware())
graph = create_agent(_ScriptedModel(messages=iter(responses)), tools=[sandbox_tools.read_file_tool], middleware=middleware, state_schema=ThreadState)
state = {
"messages": [HumanMessage(content="Read the saved API response and then the notes explaining it.")],
"sandbox": {"sandbox_id": "local:json-result-test"},
"thread_data": {"outputs_path": str(outputs)},
"thread_directories_created": True,
}
context = {"thread_id": "json-result-test", "user_id": "default"}
result = graph.invoke(state, context=context) if mode == "sync" else await graph.ainvoke(state, context=context)
first, second = [message for message in result["messages"] if isinstance(message, ToolMessage)]
assert second.content == notes
assert str(outputs / "notes.txt") in reads
assert second.additional_kwargs[TOOL_RECEIPT_KEY]["status"] == "success"
expected_status = {"missing": "error", "truncated": "partial_success"}.get(first_kind, "success")
assert first.additional_kwargs[TOOL_META_KEY]["status"] == expected_status
assert first.additional_kwargs[TOOL_RECEIPT_KEY]["status"] == expected_status
if first_kind == "missing":
assert first.content.startswith("Error: File not found:")
assert first.additional_kwargs[TOOL_META_KEY]["error_type"] == "not_found"
elif first_kind == "truncated":
assert len(first.content) < len(first_content)
assert "truncated" in first.content.lower()
else:
assert first.content == first_content