Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 28 additions & 1 deletion agent/display.py
Original file line number Diff line number Diff line change
Expand Up @@ -1264,6 +1264,25 @@ def _trim_error(msg: str) -> str:
return msg


def _first_nested_error(value):
"""Return the first meaningful nested error marker in decoded JSON."""
if isinstance(value, dict):
for key in ("error", "failed"):
marker = value.get(key)
if marker not in (None, False, "", 0, [], {}):
return marker
for child in value.values():
marker = _first_nested_error(child)
if marker is not None:
return marker
elif isinstance(value, list):
for child in value:
marker = _first_nested_error(child)
if marker is not None:
return marker
return None


def _detect_tool_failure(tool_name: str, result: str | None) -> tuple[bool, str]:
"""Inspect a tool result string for signs of failure.

Expand Down Expand Up @@ -1302,7 +1321,15 @@ def _detect_tool_failure(tool_name: str, result: str | None) -> tuple[bool, str]
if err and (data.get("success") is False or "error" in data):
return True, f" [{_trim_error(str(err))}]"

# Generic heuristic for non-terminal tools
# Decoded JSON can contain per-item ``error: null`` fields on success
# (notably web_extract). Inspect values rather than flagging the mere key.
if isinstance(data, (dict, list)):
nested_error = _first_nested_error(data)
if nested_error is not None:
return True, f" [{_trim_error(str(nested_error))}]"
return False, ""

# Generic heuristic for non-JSON, non-terminal tool results.
# Multimodal tool results (dicts with _multimodal=True) are not strings —
# treat them as successes since failures would be JSON-encoded strings.
if not isinstance(result, str):
Expand Down
24 changes: 24 additions & 0 deletions tests/agent/test_display_tool_failure.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,30 @@ def test_dict_without_error_or_success_uses_generic_heuristic(self):
is_failure, _ = _detect_tool_failure("web_search", result)
assert is_failure is False

def test_nested_null_error_is_not_failure(self):
result = json.dumps({
"results": [{
"url": "https://example.com",
"title": "Example Domain",
"content": "ok",
"error": None,
}]
})
assert _detect_tool_failure("web_extract", result) == (False, "")

def test_nested_real_error_is_failure_with_message(self):
result = json.dumps({
"results": [{
"url": "https://broken.example",
"content": "",
"error": "reader timed out",
}]
})
assert _detect_tool_failure("web_extract", result) == (
True,
" [reader timed out]",
)


class TestGetCuteToolMessageFailureSuffix:
"""End-to-end: failure suffix is appended by get_cute_tool_message."""
Expand Down
34 changes: 34 additions & 0 deletions tests/tools/test_web_providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,40 @@ def test_fully_backward_compatible_with_web_backend_only(self, monkeypatch):
assert web_tools._get_search_backend() == "tavily"
assert web_tools._get_extract_backend() == "tavily"

def test_custom_extract_backend_loads_plugins_before_availability_probe(
self, monkeypatch
):
"""Fresh standalone dispatch must discover user plugins before resolving.

Without this ordering, an extract-only user plugin configured via
``web.extract_backend`` appears unavailable in a fresh process and the
selector falls back to a search-only shared backend such as ``ddgs``.
"""
from tools import web_tools

state = {"plugins_loaded": False}
monkeypatch.setattr(
web_tools,
"_load_web_config",
lambda: {
"backend": "ddgs",
"extract_backend": "custom-extract",
},
)
monkeypatch.setattr(
web_tools,
"_ensure_web_plugins_loaded",
lambda: state.__setitem__("plugins_loaded", True),
)
monkeypatch.setattr(
web_tools,
"_is_backend_available",
lambda backend: state["plugins_loaded"] and backend == "custom-extract",
)
monkeypatch.setattr(web_tools, "_get_backend", lambda: "ddgs")

assert web_tools._get_extract_backend() == "custom-extract"


# ---------------------------------------------------------------------------
# Config key presence in DEFAULT_CONFIG
Expand Down
5 changes: 5 additions & 0 deletions tools/web_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,11 @@ def _get_capability_backend(capability: str) -> str:
"""
cfg = _load_web_config()
specific = (cfg.get(f"{capability}_backend") or "").lower().strip()
if specific and specific not in _LEGACY_WEB_BACKENDS:
# User plugins are registered lazily. A fresh standalone tool process
# can reach backend selection before the normal agent startup discovery;
# probe only after giving the configured plugin a chance to register.
_ensure_web_plugins_loaded()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please apply this same idempotent discovery guard to check_web_api_key(). That function remains a pre-dispatch check_fn for both web tools and currently probes custom-provider availability against a cold registry, so an extract-only custom provider can still leave the tools unavailable before this corrected selector runs.

if specific and _is_backend_available(specific):
return specific
return _get_backend()
Expand Down