diff --git a/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md b/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md index c55621e2d6..0d92922b22 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md +++ b/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md @@ -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: diff --git a/backend/packages/harness/deerflow/agents/middlewares/harness_guard_middleware.py b/backend/packages/harness/deerflow/agents/middlewares/harness_guard_middleware.py index 3ebe8b46ea..b5c3bda788 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/harness_guard_middleware.py +++ b/backend/packages/harness/deerflow/agents/middlewares/harness_guard_middleware.py @@ -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) diff --git a/backend/tests/test_harness_guard_middleware.py b/backend/tests/test_harness_guard_middleware.py index 0048670714..53542e9641 100644 --- a/backend/tests/test_harness_guard_middleware.py +++ b/backend/tests/test_harness_guard_middleware.py @@ -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