fix: refactor handoff image_urls preprocessing flow

Author: zouyonghe

astrbot/core/astr_agent_tool_exec.py

@@ -5,6 +5,8 @@
 import traceback
 import typing as T
 import uuid
+from collections.abc import Sequence
+from collections.abc import Set as AbstractSet
 
 import mcp
 
@@ -66,22 +68,23 @@ def _is_supported_image_ref(cls, image_ref: str) -> bool:
         return ext in cls._ALLOWED_IMAGE_EXTENSIONS
 
     @classmethod
-    async def _prepare_handoff_image_urls(
-        cls,
-        run_context: ContextWrapper[AstrAgentContext],
-        tool_args: dict[str, T.Any],
-    ) -> list[str]:
-        image_urls = tool_args.get("image_urls")
+    def _coerce_image_urls(cls, image_urls: T.Any) -> list[T.Any]:
         if image_urls is None:
-            candidates: list[T.Any] = []
-        elif isinstance(image_urls, str):
-            candidates = [image_urls]
-        else:
-            try:
-                candidates = list(image_urls)
-            except (TypeError, ValueError):
-                candidates = [image_urls]
+            return []
+        if isinstance(image_urls, str):
+            return [image_urls]
+        if isinstance(image_urls, (Sequence, AbstractSet)) and not isinstance(
+            image_urls, (str, bytes, bytearray)
+        ):
+            return list(image_urls)
+        logger.warning(
+            "Unsupported image_urls type in handoff tool args: %s",
+            type(image_urls).__name__,
+        )
+        return []
 
+    @classmethod
+    def _filter_supported_image_urls(cls, candidates: list[T.Any]) -> list[str]:
         normalized = normalize_and_dedupe_strings(candidates)
         sanitized = [item for item in normalized if cls._is_supported_image_ref(item)]
         dropped_count = len(normalized) - len(sanitized)
@@ -90,8 +93,13 @@ async def _prepare_handoff_image_urls(
                 "Dropped %d invalid image_urls entries in handoff tool args.",
                 dropped_count,
             )
+        return sanitized
 
-        # Merge current event image attachments so sub-agent behavior matches main-agent flow.
+    @classmethod
+    async def _iter_event_image_paths(
+        cls, run_context: ContextWrapper[AstrAgentContext]
+    ) -> list[str]:
+        paths: list[str] = []
         event = getattr(run_context.context, "event", None)
         message_obj = getattr(event, "message_obj", None)
         message = getattr(message_obj, "message", None)
@@ -101,22 +109,27 @@ async def _prepare_handoff_image_urls(
                     continue
                 try:
                     path = await component.convert_to_file_path()
-                    if (
-                        path
-                        and cls._is_supported_image_ref(path)
-                        and path not in sanitized
-                    ):
-                        sanitized.append(path)
+                    if path and cls._is_supported_image_ref(path):
+                        paths.append(path)
                 except Exception as e:
                     logger.error(
                         "Failed to convert handoff image component at index %d: %s",
                         idx,
                         e,
                         exc_info=True,
                     )
+        return paths
 
-        tool_args["image_urls"] = sanitized
-        return sanitized
+    @classmethod
+    async def _prepare_handoff_image_urls(
+        cls,
+        run_context: ContextWrapper[AstrAgentContext],
+        image_urls: T.Any,
+    ) -> list[str]:
+        candidates = cls._coerce_image_urls(image_urls)
+        event_paths = await cls._iter_event_image_paths(run_context)
+        candidates.extend(event_paths)
+        return cls._filter_supported_image_urls(candidates)
 
     @classmethod
     async def execute(cls, tool, run_context, **tool_args):
@@ -138,7 +151,7 @@ async def execute(cls, tool, run_context, **tool_args):
                 ):
                     yield r
                 return
-            async for r in cls._execute_handoff(tool, run_context, **tool_args):
+            async for r in cls._execute_handoff(tool, run_context, tool_args):
                 yield r
             return
 
@@ -241,10 +254,14 @@ async def _execute_handoff(
         cls,
         tool: HandoffTool,
         run_context: ContextWrapper[AstrAgentContext],
-        **tool_args,
+        tool_args: dict[str, T.Any],
     ):
         input_ = tool_args.get("input")
-        image_urls = await cls._prepare_handoff_image_urls(run_context, tool_args)
+        image_urls = await cls._prepare_handoff_image_urls(
+            run_context,
+            tool_args.get("image_urls"),
+        )
+        tool_args["image_urls"] = image_urls
 
         # Build handoff toolset from registered tools plus runtime computer tools.
         toolset = cls._build_handoff_toolset(run_context, tool.agent.tools)
@@ -345,10 +362,7 @@ async def _do_handoff_background(
         result_text = ""
         prepared_tool_args = dict(tool_args)
         try:
-            await cls._prepare_handoff_image_urls(run_context, prepared_tool_args)
-            async for r in cls._execute_handoff(
-                tool, run_context, **prepared_tool_args
-            ):
+            async for r in cls._execute_handoff(tool, run_context, prepared_tool_args):
                 if isinstance(r, mcp.types.CallToolResult):
                     for content in r.content:
                         if isinstance(content, mcp.types.TextContent):

tests/unit/test_astr_agent_tool_exec.py

@@ -39,26 +39,41 @@ async def _fake_convert_to_file_path(self):
     monkeypatch.setattr(Image, "convert_to_file_path", _fake_convert_to_file_path)
 
     run_context = _build_run_context([Image(file="file:///tmp/original.png")])
-    tool_args = {
-        "image_urls": (
-            " https://example.com/a.png ",
-            "/tmp/not_an_image.txt",
-            "/tmp/local.webp",
-            123,
-        )
-    }
+    image_urls_input = (
+        " https://example.com/a.png ",
+        "/tmp/not_an_image.txt",
+        "/tmp/local.webp",
+        123,
+    )
 
     image_urls = await FunctionToolExecutor._prepare_handoff_image_urls(
         run_context,
-        tool_args,
+        image_urls_input,
     )
 
     assert image_urls == [
         "https://example.com/a.png",
         "/tmp/local.webp",
         "/tmp/event_image.png",
     ]
-    assert tool_args["image_urls"] == image_urls
+
+
+@pytest.mark.asyncio
+async def test_prepare_handoff_image_urls_skips_failed_event_image_conversion(
+    monkeypatch: pytest.MonkeyPatch,
+):
+    async def _fake_convert_to_file_path(self):
+        raise RuntimeError("boom")
+
+    monkeypatch.setattr(Image, "convert_to_file_path", _fake_convert_to_file_path)
+
+    run_context = _build_run_context([Image(file="file:///tmp/original.png")])
+    image_urls = await FunctionToolExecutor._prepare_handoff_image_urls(
+        run_context,
+        ["https://example.com/a.png"],
+    )
+
+    assert image_urls == ["https://example.com/a.png"]
 
 
 @pytest.mark.asyncio
@@ -67,11 +82,11 @@ async def test_do_handoff_background_reports_prepared_image_urls(
 ):
     captured: dict = {}
 
-    async def _fake_prepare(cls, run_context, tool_args):
-        tool_args["image_urls"] = ["prepared://image.png"]
-        return tool_args["image_urls"]
+    async def _unexpected_prepare(cls, run_context, image_urls):
+        raise AssertionError("background path should not pre-prepare image urls")
 
-    async def _fake_execute_handoff(cls, tool, run_context, **tool_args):
+    async def _fake_execute_handoff(cls, tool, run_context, tool_args):
+        tool_args["image_urls"] = ["prepared://image.png"]
         yield mcp.types.CallToolResult(
             content=[mcp.types.TextContent(type="text", text="ok")]
         )
@@ -82,7 +97,7 @@ async def _fake_wake(cls, run_context, **kwargs):
     monkeypatch.setattr(
         FunctionToolExecutor,
         "_prepare_handoff_image_urls",
-        classmethod(_fake_prepare),
+        classmethod(_unexpected_prepare),
     )
     monkeypatch.setattr(
         FunctionToolExecutor,