diff --git a/backend/packages/harness/deerflow/agents/middlewares/loop_detection_middleware.py b/backend/packages/harness/deerflow/agents/middlewares/loop_detection_middleware.py index 1ab721ac1b..027fd38255 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/loop_detection_middleware.py +++ b/backend/packages/harness/deerflow/agents/middlewares/loop_detection_middleware.py @@ -101,6 +101,11 @@ _MAX_PENDING_WARNINGS_PER_RUN = 4 # Stands in for ``read_file``'s omitted ``end_line`` in a call key: the read # runs to the last line, which is not the same window as any numbered bound. _OPEN_ENDED_READ = "end" +# Sandbox tools on the generic key path whose ``description`` is only the UI +# explanation of the call. Elsewhere ``description`` can be the operation's +# payload (``update_agent``/``setup_agent`` persist it; MCP tools such as +# ``create_issue`` send it), so it stays in the key. +_UI_NARRATION_TOOLS = frozenset({"bash", "ls", "glob", "grep"}) type _RunScopeKey = tuple[str, str | None] @@ -170,7 +175,7 @@ def _normalized_read_range(args: dict) -> tuple[int, int | None]: def _stable_tool_key(name: str, args: dict, fallback_key: str | None) -> str: - """Derive a stable key from salient args without overfitting to noise.""" + """Derive a stable key from the args that change what a call does.""" if name == "read_file" and fallback_key is None: path = args.get("path") or "" start_line, end_line = _normalized_read_range(args) @@ -184,14 +189,17 @@ def _stable_tool_key(name: str, args: dict, fallback_key: str | None) -> str: return fallback_key return json.dumps(args, sort_keys=True, default=str) - salient_fields = ("path", "url", "query", "command", "pattern", "glob", "cmd") - stable_args = {field: args[field] for field in salient_fields if args.get(field) is not None} - if stable_args: - return json.dumps(stable_args, sort_keys=True, default=str) - if fallback_key is not None: return fallback_key + # Every other argument can change what the call does: keying only a salient + # field (``path``/``url``/``query``...) collapsed paging a URL or a search + # (``start_index``, ``page``, ``cursor``) and rewriting one skill file onto a + # single key, so the fifth *distinct* call hard-stopped the run. A sandbox + # tool's UI narration is dropped, so rewording it cannot make a repeated + # call look new (#1905). + if name in _UI_NARRATION_TOOLS: + args = {field: value for field, value in args.items() if field != "description"} return json.dumps(args, sort_keys=True, default=str) diff --git a/backend/tests/test_loop_detection_middleware.py b/backend/tests/test_loop_detection_middleware.py index c9d4be5ad4..ef15187139 100644 --- a/backend/tests/test_loop_detection_middleware.py +++ b/backend/tests/test_loop_detection_middleware.py @@ -277,6 +277,96 @@ class TestReadFileRangeKey: assert _hash_tool_calls([self._read_call(path="/w/a.py", start_line=1, end_line=40)]) != _hash_tool_calls([self._read_call(path="/w/b.py", start_line=1, end_line=40)]) +class TestGenericToolKey: + """Tools without a dedicated key rule must keep every argument that changes the call. + + Keying only the first salient field (``path``/``url``/``query``/...) collapsed + distinct calls onto one key: paging a URL or a search, or rewriting one + skill file with new content, hard-stopped on its fifth *distinct* call. + Only a sandbox tool's UI narration (``description``) may vary without + making a call new. + """ + + @staticmethod + def _call(tool_name, args): + return {"name": tool_name, "id": f"call_{tool_name}", "args": args} + + @pytest.mark.parametrize( + ("name", "first", "second"), + [ + ("fetch", {"url": "https://example.com/doc", "start_index": 0}, {"url": "https://example.com/doc", "start_index": 5000}), + ("search_issues", {"query": "is:open label:bug", "page": 1}, {"query": "is:open label:bug", "page": 2}), + ("list_uploaded_files", {"query": "report"}, {"query": "report", "cursor": "next-page"}), + ("skill_manage", {"action": "write_file", "name": "etl", "path": "scripts/run.py", "content": "v1"}, {"action": "write_file", "name": "etl", "path": "scripts/run.py", "content": "v2"}), + ("grep", {"path": "/w", "pattern": "foo", "case_sensitive": False}, {"path": "/w", "pattern": "foo", "case_sensitive": True}), + ], + ) + def test_non_salient_args_affect_hash(self, name, first, second): + assert _hash_tool_calls([self._call(name, first)]) != _hash_tool_calls([self._call(name, second)]) + + @pytest.mark.parametrize( + ("name", "args"), + [ + ("bash", {"command": "make test"}), + ("ls", {"path": "/w"}), + ("glob", {"path": "/w", "pattern": "*.py"}), + ("grep", {"path": "/w", "pattern": "foo"}), + ], + ) + def test_sandbox_ui_narration_does_not_affect_hash(self, name, args): + """Regression guard for #1905: rewording the narration must not dodge detection.""" + first = self._call(name, {**args, "description": "run it"}) + second = self._call(name, {**args, "description": "run it once more"}) + + assert _hash_tool_calls([first]) == _hash_tool_calls([second]) + + @pytest.mark.parametrize( + ("name", "args"), + [ + ("update_agent", {}), + ("setup_agent", {"soul": "You are helpful."}), + ("create_issue", {"title": "Crash on start"}), + ], + ) + def test_description_payload_affects_hash(self, name, args): + """``description`` is the operation's payload outside the sandbox narration tools.""" + first = self._call(name, {**args, "description": "Reviews pull requests."}) + second = self._call(name, {**args, "description": "Reviews pull requests and triages issues."}) + + assert _hash_tool_calls([first]) != _hash_tool_calls([second]) + + def test_description_only_agent_updates_do_not_hard_stop(self): + mw = LoopDetectionMiddleware(warn_threshold=3, hard_limit=5) + runtime = _make_runtime() + + for version in range(6): + call = self._call("update_agent", {"description": f"Assistant v{version}"}) + assert mw._apply(_make_state(tool_calls=[call]), runtime) is None, f"update to v{version} was treated as a loop" + assert mw.consume_stop_reason("test-run") is None + + def test_paging_a_url_does_not_hard_stop(self): + mw = LoopDetectionMiddleware(warn_threshold=3, hard_limit=5) + runtime = _make_runtime() + + for start in range(0, 50_000, 5_000): + call = self._call("fetch", {"url": "https://example.com/doc", "max_length": 5_000, "start_index": start}) + assert mw._apply(_make_state(tool_calls=[call]), runtime) is None, f"page at start_index={start} was treated as a loop" + assert mw.consume_stop_reason("test-run") is None + + def test_repeating_one_page_still_hard_stops(self): + mw = LoopDetectionMiddleware(warn_threshold=3, hard_limit=5) + runtime = _make_runtime() + call = [self._call("fetch", {"url": "https://example.com/doc", "start_index": 5_000})] + + for _ in range(4): + assert mw._apply(_make_state(tool_calls=call), runtime) is None + hard_stop = mw._apply(_make_state(tool_calls=call), runtime) + + assert hard_stop is not None + assert hard_stop["messages"][0].tool_calls == [] + assert mw.consume_stop_reason("test-run") == "loop_capped" + + class TestLoopDetection: def test_no_tool_calls_returns_none(self): mw = LoopDetectionMiddleware()