mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-11-09 04:20:34 +00:00
fix(agents): keep HarnessGuard from rewriting the result that ends a loop (#8001)
* fix(agents): keep HarnessGuard from rewriting the result that ends a loop The alternating-loop check fired when three of the last four tool calls failed, without checking the current call. After A and B failed in turn, a successful B therefore received the OSCILLATION DETECTED warning: the model was told to stop just as the loop ended, and structured output such as JSON was corrupted by the appended text. Only a failing call now continues the pattern, so successful results reach the model unchanged. Failing alternation still warns. * fix(agents): only warn on an alternating loop when the whole window failed The oscillation check still counted 3 errors out of the last 4 calls, so a window that contained a success could warn on the next failure, e.g. A err, B err, A ok, B err. A success ends the loop, so require all four calls in the window to have failed, and pin the break shapes with tests. --------- Co-authored-by: Totoro <msy626836554@gmail.com>
This commit is contained in:
parent
d8e085630a
commit
a73a94a6a3
@ -123,7 +123,7 @@ No match: skip inspection, log ID. Tests: `test_read_mark_tool_call_correlation.
|
||||
No config gate — appended unconditionally in `_build_runtime_middlewares`, for both lead and subagents.
|
||||
|
||||
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.
|
||||
13b. **HarnessGuardMiddleware** - *(`harness_guard.enabled`, default on)* Error diagnosis and optional write gate. When `write_verify_syntax: true`, checks `.py`/`.json` syntax on `str_replace` for valid files. Classifies tool failures and injects targeted hints. Warns on repeated error signatures and alternating tool loops per `(thread_id, run_id)` via `BoundedDict`.
|
||||
13b. **HarnessGuardMiddleware** - *(`harness_guard.enabled`, default on)* Error diagnosis and optional write gate. When `write_verify_syntax: true`, checks `.py`/`.json` syntax on `str_replace` for valid files. Classifies tool failures and injects targeted hints. Warns on repeated error signatures and alternating tool loops (four consecutive failures) per `(thread_id, run_id)` via `BoundedDict`; successful results are never rewritten.
|
||||
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:
|
||||
|
||||
@ -359,11 +359,13 @@ class HarnessGuardMiddleware(AgentMiddleware):
|
||||
if matching_streak + 1 >= self.config.max_repeated_error_signatures:
|
||||
repetition_warning = f"[HARNESS GUARD: CRITICAL REPETITION] Error '{sig_str}' has occurred {matching_streak + 1} times consecutively. Do NOT repeat this action without addressing the root cause."
|
||||
|
||||
# Check alternating oscillation pattern (A -> B -> A -> B)
|
||||
if self.config.detect_alternating_loops and len(history) >= 3:
|
||||
# Check alternating oscillation pattern (A -> B -> A -> B). Only
|
||||
# failures form the loop: a success ends it, its content must reach
|
||||
# the model unchanged, and a window containing it is not a loop.
|
||||
if self.config.detect_alternating_loops and is_error and len(history) >= 3:
|
||||
h = list(history)
|
||||
r0, r1, r2, r3 = h[-3], h[-2], h[-1], record
|
||||
if r0.tool_name == r2.tool_name and r1.tool_name == r3.tool_name and r0.tool_name != r1.tool_name and sum(1 for r in (r0, r1, r2, r3) if r.is_error) >= 3:
|
||||
if r0.tool_name == r2.tool_name and r1.tool_name == r3.tool_name and r0.tool_name != r1.tool_name and all(r.is_error for r in (r0, r1, r2)):
|
||||
oscillation_warning = f"[HARNESS GUARD: OSCILLATION DETECTED] Alternating failing tool loop detected between '{r0.tool_name}' and '{r1.tool_name}'. Pause and re-evaluate your strategy."
|
||||
|
||||
history.append(record)
|
||||
|
||||
@ -1,5 +1,6 @@
|
||||
"""Unit tests for HarnessGuardMiddleware."""
|
||||
|
||||
import json
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
@ -571,6 +572,53 @@ class TestHarnessGuardMiddleware:
|
||||
assert "OSCILLATION DETECTED" in res_b2.content
|
||||
assert "between 'bash' and 'read_file'" in res_b2.content
|
||||
|
||||
def test_successful_result_that_ends_an_alternating_loop_is_not_rewritten(self):
|
||||
middleware = HarnessGuardMiddleware(HarnessGuardConfig(detect_alternating_loops=True))
|
||||
failures = [
|
||||
("web_fetch", "Error: upstream returned 503"),
|
||||
("bash", "boom\nExit Code: 1"),
|
||||
("web_fetch", "Error: upstream returned 503"),
|
||||
]
|
||||
for index, (name, content) in enumerate(failures):
|
||||
middleware.wrap_tool_call(
|
||||
_make_request(name, {}, tool_call_id=f"call-{index}", thread_id="t-osc-ok", run_id="r-osc-ok"),
|
||||
lambda r, name=name, content=content: ToolMessage(content=content, tool_call_id=r.tool_call["id"], name=name, status="error"),
|
||||
)
|
||||
|
||||
payload = '{"rows": 3, "ok": true}'
|
||||
result = middleware.wrap_tool_call(
|
||||
_make_request("bash", {}, tool_call_id="call-ok", thread_id="t-osc-ok", run_id="r-osc-ok"),
|
||||
lambda r: ToolMessage(content=payload, tool_call_id=r.tool_call["id"], name="bash"),
|
||||
)
|
||||
|
||||
assert result.content == payload
|
||||
assert json.loads(result.content) == {"rows": 3, "ok": True}
|
||||
assert HARNESS_GUARD_KEY not in result.additional_kwargs
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"outcomes",
|
||||
[
|
||||
pytest.param([True, True, False, True], id="success-before-the-failure"),
|
||||
pytest.param([True, False, True, True], id="success-mid-window"),
|
||||
pytest.param([True, True, False, True, True], id="window-slid-past-break"),
|
||||
],
|
||||
)
|
||||
def test_window_containing_a_success_is_not_an_oscillation(self, outcomes):
|
||||
middleware = HarnessGuardMiddleware(HarnessGuardConfig(detect_alternating_loops=True))
|
||||
results = []
|
||||
for index, failed in enumerate(outcomes):
|
||||
name = ("web_fetch", "bash")[index % 2]
|
||||
content = f"Error: attempt {index} failed" if failed else '{"ok": true}'
|
||||
results.append(
|
||||
middleware.wrap_tool_call(
|
||||
_make_request(name, {}, tool_call_id=f"call-{index}", thread_id="t-osc-break", run_id="r-osc-break"),
|
||||
lambda r, name=name, content=content, failed=failed: ToolMessage(content=content, tool_call_id=r.tool_call["id"], name=name, status="error" if failed else "success"),
|
||||
)
|
||||
)
|
||||
|
||||
assert all("OSCILLATION DETECTED" not in str(result.content) for result in results)
|
||||
assert not any((result.additional_kwargs.get(HARNESS_GUARD_KEY) or {}).get("oscillation_warning") for result in results)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_async_awrap_tool_call(self):
|
||||
middleware = HarnessGuardMiddleware(HarnessGuardConfig())
|
||||
@ -618,3 +666,62 @@ class TestHarnessGuardMiddleware:
|
||||
)
|
||||
assert "[HARNESS GUARD]" in res.content
|
||||
assert "pip install torch_geometric" in res.content
|
||||
|
||||
|
||||
def _alternating_loop_graph(final_query_succeeds: bool):
|
||||
"""A real create_agent graph whose tools fail, alternate, then maybe recover."""
|
||||
from _agent_e2e_helpers import FakeToolCallingModel
|
||||
from langchain.agents import create_agent
|
||||
from langchain_core.messages import AIMessage
|
||||
from langchain_core.tools import tool
|
||||
|
||||
from deerflow.agents.middlewares.tool_error_handling_middleware import ToolErrorHandlingMiddleware
|
||||
|
||||
query_results = iter(["Error: database is locked", '{"rows": 3, "ok": true}' if final_query_succeeds else "Error: database is locked"])
|
||||
|
||||
@tool
|
||||
def fetch_page(url: str) -> str:
|
||||
"""Fetch a page."""
|
||||
return "Error: upstream returned 503"
|
||||
|
||||
@tool
|
||||
def query_rows(sql: str) -> str:
|
||||
"""Query rows."""
|
||||
return next(query_results)
|
||||
|
||||
calls = [("fetch_page", {"url": "https://example.com"}), ("query_rows", {"sql": "select 1"})] * 2
|
||||
responses = [AIMessage(content="", tool_calls=[{"name": name, "args": args, "id": f"call-{index}", "type": "tool_call"}]) for index, (name, args) in enumerate(calls)]
|
||||
responses.append(AIMessage(content="done"))
|
||||
return create_agent(
|
||||
model=FakeToolCallingModel(responses=responses),
|
||||
tools=[fetch_page, query_rows],
|
||||
middleware=[HarnessGuardMiddleware(HarnessGuardConfig()), ToolErrorHandlingMiddleware()],
|
||||
)
|
||||
|
||||
|
||||
def _last_tool_message(state):
|
||||
return [message for message in state["messages"] if isinstance(message, ToolMessage)][-1]
|
||||
|
||||
|
||||
class TestAlternatingLoopGraph:
|
||||
@pytest.mark.asyncio
|
||||
async def test_recovery_result_reaches_state_unchanged(self):
|
||||
graph = _alternating_loop_graph(final_query_succeeds=True)
|
||||
state = await graph.ainvoke({"messages": [("user", "count the rows")]}, context={"thread_id": "t-graph", "run_id": "r-graph"})
|
||||
|
||||
recovered = _last_tool_message(state)
|
||||
assert recovered.name == "query_rows"
|
||||
assert recovered.additional_kwargs[TOOL_META_KEY]["status"] == "success"
|
||||
assert recovered.content == '{"rows": 3, "ok": true}'
|
||||
assert json.loads(recovered.content) == {"rows": 3, "ok": True}
|
||||
assert HARNESS_GUARD_KEY not in recovered.additional_kwargs
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_failing_alternation_still_warns(self):
|
||||
graph = _alternating_loop_graph(final_query_succeeds=False)
|
||||
state = await graph.ainvoke({"messages": [("user", "count the rows")]}, context={"thread_id": "t-graph-fail", "run_id": "r-graph-fail"})
|
||||
|
||||
failed = _last_tool_message(state)
|
||||
assert failed.additional_kwargs[TOOL_META_KEY]["status"] == "error"
|
||||
assert "OSCILLATION DETECTED" in failed.content
|
||||
assert failed.additional_kwargs[HARNESS_GUARD_KEY]["oscillation_warning"] is True
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user