diff --git a/agentmain.py b/agentmain.py index d8cab8cf0..d6f40ed7a 100644 --- a/agentmain.py +++ b/agentmain.py @@ -15,6 +15,41 @@ script_dir = os.path.dirname(os.path.abspath(__file__)) BANNED_TOOLS = (['ask_user', 'start_long_term_update'] if '--no-user-tools' in sys.argv else []) + +_VISION_MIMES = {'image/png', 'image/jpeg', 'image/gif', 'image/webp'} + +def _multimodal_initial_content(raw_query, images, llmclient): + """Build the first user turn's content blocks when the task carries images. + + put_task() has always accepted images and every frontend passes them, but + run() used to drop them on the floor. Only NativeToolClient backends + understand Claude-style image blocks (the native Claude API takes them + as-is; the OAI paths convert them), so other clients keep plain text. + Returns None when there is nothing to add — callers pass the result + straight to agent_runner_loop(initial_user_content=...). + """ + import base64, mimetypes + if not images or not isinstance(llmclient, NativeToolClient): + return None + blocks = [{"type": "text", "text": raw_query}] + for img in images: + path = img if isinstance(img, str) else (img.get("path") if isinstance(img, dict) else None) + if not path: + continue + mime = mimetypes.guess_type(path)[0] or 'image/png' + if mime not in _VISION_MIMES: + # Unsupported format (e.g. SVG) — reference the path as text instead + blocks.append({"type": "text", "text": f"[attached file: {path}]"}) + continue + try: + with open(path, 'rb') as f: + data = base64.b64encode(f.read()).decode('ascii') + except OSError as e: + blocks.append({"type": "text", "text": f"[image read failed: {path}: {e}]"}) + continue + blocks.append({"type": "image", "source": {"type": "base64", "media_type": mime, "data": data}}) + return blocks + def load_tool_schema(suffix=''): global TOOLS_SCHEMA TS = open(os.path.join(script_dir, f'assets/tools_schema{suffix}.json'), 'r', encoding='utf-8').read() @@ -181,7 +216,8 @@ def run(self): self.llmclient.backend.stream = False self.llmclient.backend.read_timeout = max(self.llmclient.backend.read_timeout, 1200) gen = agent_runner_loop(self.llmclient, sys_prompt, raw_query, handler, TOOLS_SCHEMA, - max_turns=180, verbose=self.verbose, yield_info=True) + max_turns=180, verbose=self.verbose, yield_info=True, + initial_user_content=_multimodal_initial_content(raw_query, task.get("images"), self.llmclient)) try: full_resp = ""; last_pos = 0; curr_turn = 0; turn_resps = self.all_outputs[-1]["outputs"] for chunk in gen: diff --git a/frontends/desktop_bridge.py b/frontends/desktop_bridge.py index f44ad287b..3b5e35ea0 100644 --- a/frontends/desktop_bridge.py +++ b/frontends/desktop_bridge.py @@ -1039,45 +1039,6 @@ def submit_prompt(self, sid: str, prompt: Any, images: Optional[list] = None, di emit_session_state(sess, "running") return {"ok": True, "sessionId": sid, "accepted": True, "userMessageId": user_msg["id"], "seq": seq} - @staticmethod - def _patch_chat_for_images(client, image_paths): - """Monkey-patch backend.ask to inject base64 image blocks on the first LLM call.""" - import base64 as b64, mimetypes - try: - from llmcore import NativeToolClient - except ImportError: - return - if not isinstance(client, NativeToolClient): - return - backend = client.backend - original_ask = backend.ask - - _VISION_MIMES = {'image/png', 'image/jpeg', 'image/gif', 'image/webp'} - - def patched_ask(msg): - try: - del backend.ask - except AttributeError: - backend.ask = original_ask - if isinstance(msg, dict) and isinstance(msg.get("content"), list): - for p in image_paths: - try: - mime = mimetypes.guess_type(p)[0] or 'image/png' - if mime not in _VISION_MIMES: - # Unsupported image format (e.g. SVG) — inject as text path reference - msg["content"].append({"type": "text", "text": f"[attached file: {p}]"}) - continue - with open(p, 'rb') as f: - raw = f.read() - data = b64.b64encode(raw).decode() - msg["content"].append({"type": "image", "source": {"type": "base64", "media_type": mime, "data": data}}) - except Exception: - pass - resp = yield from original_ask(msg) - return resp - - backend.ask = patched_ask - def run_agent_turn( self, sess: Session, @@ -1131,8 +1092,6 @@ def turn_state() -> tuple[bool, bool]: ) or None except Exception: sess.running_model = None - if images: - self._patch_chat_for_images(agent.llmclient, images) full = "" done_outputs = None # done时agent给的全量轮文本(turn_resps.copy()) if hasattr(agent, "put_task"): diff --git a/frontends/tests/test_bridge_submit.py b/frontends/tests/test_bridge_submit.py index 630c06b10..52e735c2a 100644 --- a/frontends/tests/test_bridge_submit.py +++ b/frontends/tests/test_bridge_submit.py @@ -223,12 +223,41 @@ def test_run_agent_turn_does_not_reference_bare_sid(self): f"Use 'sess.id' instead — 'sid' is only defined in submit_prompt's scope." ) - def test_patch_chat_for_images_exists(self): - """_patch_chat_for_images must exist in bridge — it's the image injection path.""" + def test_core_forwards_images_to_agent_loop(self): + """agentmain.run() must forward task images to agent_runner_loop. + + Replaces the old desktop_bridge._patch_chat_for_images monkey-patch: + the core now injects image blocks via initial_user_content, so image + uploads cannot silently fail on any frontend (fsapp/tui/bridge alike), + and there is exactly one injection point. + """ + from pathlib import Path + candidates = [ + Path(__file__).parent.parent.parent / "agentmain.py", + Path(__file__).parent.parent.parent.parent / "agentmain.py", + ] + source = next((p.read_text(encoding="utf-8") for p in candidates if p.exists()), "") + assert source, "agentmain.py not found" + assert "_multimodal_initial_content" in source, ( + "_multimodal_initial_content missing from agentmain.py — " + "images passed to put_task() would be silently dropped again" + ) + assert "initial_user_content" in source, ( + "agentmain.run() must pass initial_user_content to agent_runner_loop" + ) + + def test_run_agent_turn_does_not_monkeypatch_images(self): + """run_agent_turn must not re-inject images at the backend level. + + The core (put_task -> initial_user_content) is the single injection + point; a leftover backend.ask patch would duplicate every image. + """ source = self._get_bridge_source() - assert "_patch_chat_for_images" in source, ( - "_patch_chat_for_images method missing from desktop_bridge.py — " - "image uploads will silently fail (agent won't see images)" + body = self._extract_method_body(source, "run_agent_turn") + assert body, "Could not extract run_agent_turn body" + assert "_patch_chat_for_images" not in body, ( + "run_agent_turn still monkey-patches backend.ask for images — " + "the core now injects them, so this would duplicate every image" ) def test_submit_prompt_separates_agent_prompt_from_stored_message(self): @@ -257,5 +286,5 @@ def test_image_paths_passed_to_run_agent_turn(self): body = self._extract_method_body(source, "submit_prompt") assert "image_paths" in body, ( "submit_prompt must extract image_paths from image_metas and pass to " - "run_agent_turn. Without this, _patch_chat_for_images receives None." + "run_agent_turn. Without this, the agent never receives the images." ) diff --git a/frontends/tests/test_multimodal_image_passthrough.py b/frontends/tests/test_multimodal_image_passthrough.py new file mode 100644 index 000000000..19c92a766 --- /dev/null +++ b/frontends/tests/test_multimodal_image_passthrough.py @@ -0,0 +1,272 @@ +"""Regression tests for multimodal image passthrough (issue #813, item A-1). + +put_task() has always accepted images and every frontend passes them (fsapp +downloads Feishu images to disk and forwards the paths; desktop_bridge and +tui_v3 forward paths as well), but agentmain.run() never handed them to +agent_runner_loop — the first user turn stayed plain text and images were +silently dropped. Two downstream drop points are pinned here too: + +- NativeToolClient.chat()'s whitespace filter dropped *every* block lacking + non-blank text, image blocks included. +- _to_responses_input() only understood OpenAI-style image_url blocks, so + Claude-style {"type": "image", "source": {...}} blocks vanished on the + Responses API path. +""" +import ast +import base64 +import sys +from pathlib import Path + +PROJECT_ROOT = Path(__file__).resolve().parents[2] +if str(PROJECT_ROOT) not in sys.path: + sys.path.insert(0, str(PROJECT_ROOT)) + +# test_bridge_sessions.py installs empty module stubs via sys.modules.setdefault; +# under full-suite import order it runs first, so evict them to exercise the +# real implementations here. +for _stubbed in ("llmcore", "agentmain", "agent_loop"): + sys.modules.pop(_stubbed, None) + +import llmcore + + +# --------------------------------------------------------------------------- +# _to_responses_input: Claude-style image blocks -> input_image +# --------------------------------------------------------------------------- + +class TestResponsesInputImageBlocks: + """The Responses API converter must carry image blocks through.""" + + def test_base64_image_block_becomes_input_image(self): + msgs = [{"role": "user", "content": [ + {"type": "text", "text": "what is this?"}, + {"type": "image", "source": {"type": "base64", + "media_type": "image/png", "data": "QUJD"}}, + ]}] + assert llmcore._to_responses_input(msgs) == [{"role": "user", "content": [ + {"type": "input_text", "text": "what is this?"}, + {"type": "input_image", "image_url": "data:image/png;base64,QUJD"}, + ]}] + + def test_url_source_image_block_becomes_input_image(self): + msgs = [{"role": "user", "content": [ + {"type": "image", "source": {"type": "url", "url": "https://example.com/a.png"}}, + ]}] + out = llmcore._to_responses_input(msgs) + assert out[0]["content"] == [ + {"type": "input_image", "image_url": "https://example.com/a.png"}] + + def test_openai_style_image_url_block_still_works(self): + """Pre-existing branch: OpenAI-style image_url must not regress.""" + msgs = [{"role": "user", "content": [ + {"type": "image_url", "image_url": {"url": "https://example.com/b.png"}}, + ]}] + out = llmcore._to_responses_input(msgs) + assert out[0]["content"] == [ + {"type": "input_image", "image_url": "https://example.com/b.png"}] + + def test_assistant_image_blocks_are_ignored(self): + msgs = [{"role": "assistant", "content": [ + {"type": "image", "source": {"type": "base64", "media_type": "image/png", "data": "QUJD"}}, + {"type": "text", "text": "seen"}, + ]}] + out = llmcore._to_responses_input(msgs) + assert out[0]["content"] == [{"type": "output_text", "text": "seen"}] + + def test_malformed_image_blocks_are_ignored(self): + msgs = [{"role": "user", "content": [ + {"type": "text", "text": "hi"}, + {"type": "image"}, # no source at all + {"type": "image", "source": {}}, # empty source + {"type": "image", "source": {"type": "base64"}}, # no data + {"type": "image", "source": {"type": "url"}}, # no url + ]}] + out = llmcore._to_responses_input(msgs) + assert out[0]["content"] == [{"type": "input_text", "text": "hi"}] + + +# --------------------------------------------------------------------------- +# _drop_blank_text_blocks: only blank *text* may be dropped +# --------------------------------------------------------------------------- + +class TestDropBlankTextBlocks: + """The whitespace filter must not eat image/tool_result blocks.""" + + def test_blank_text_blocks_dropped(self): + blocks = [{"type": "text", "text": " \n\t "}, + {"type": "text", "text": "keep me"}] + assert llmcore._drop_blank_text_blocks(blocks) == [{"type": "text", "text": "keep me"}] + + def test_image_blocks_kept(self): + img = {"type": "image", "source": {"type": "base64", + "media_type": "image/png", "data": "QUJD"}} + assert llmcore._drop_blank_text_blocks([img]) == [img] + + def test_tool_result_blocks_kept(self): + tr = {"type": "tool_result", "tool_use_id": "toolu_1", "content": "ok"} + assert llmcore._drop_blank_text_blocks([tr]) == [tr] + + def test_empty_input(self): + assert llmcore._drop_blank_text_blocks([]) == [] + + def test_mixed_blocks(self): + blank = {"type": "text", "text": ""} + img = {"type": "image", "source": {"type": "base64", + "media_type": "image/jpeg", "data": "QUJD"}} + keep = {"type": "text", "text": "hello"} + assert llmcore._drop_blank_text_blocks([blank, img, keep]) == [img, keep] + + +# --------------------------------------------------------------------------- +# NativeToolClient.chat: image blocks must reach backend.ask +# --------------------------------------------------------------------------- + +class _FakeBackend: + """Minimal stand-in for a native session backend.""" + + def __init__(self): + self.name = "fake" + self.system = "" + self.tools = None + self.history = [] + self.asked = [] + + def ask(self, msg): + self.asked.append(msg) + return + yield # pragma: no cover - generator so chat()'s next() hits StopIteration + + +class TestNativeToolClientChatKeepsImages: + def test_image_blocks_reach_backend_ask(self): + client = llmcore.NativeToolClient(_FakeBackend()) + client.log_path = False # skip file logging + img = {"type": "image", "source": {"type": "base64", + "media_type": "image/png", "data": "QUJD"}} + messages = [{"role": "user", "content": [ + {"type": "text", "text": "describe"}, img]}] + assert list(client.chat(messages=messages)) == [] + assert len(client.backend.asked) == 1 + content = client.backend.asked[0]["content"] + assert {"type": "text", "text": "describe"} in content + assert img in content + + def test_blank_text_still_dropped_end_to_end(self): + client = llmcore.NativeToolClient(_FakeBackend()) + client.log_path = False + messages = [{"role": "user", "content": [ + {"type": "text", "text": " "}, + {"type": "text", "text": "real"}]}] + list(client.chat(messages=messages)) + assert client.backend.asked[0]["content"] == [{"type": "text", "text": "real"}] + + +# --------------------------------------------------------------------------- +# agentmain._multimodal_initial_content: build the first turn's blocks +# --------------------------------------------------------------------------- + +class _FakeNativeClient: + """Stands in for llmcore.NativeToolClient inside the extracted helper.""" + + +def _load_helper(): + """Extract _multimodal_initial_content from agentmain.py via ast (no import).""" + src = (PROJECT_ROOT / "agentmain.py").read_text(encoding="utf-8") + tree = ast.parse(src) + nodes = [] + for node in tree.body: + if isinstance(node, ast.FunctionDef) and node.name == "_multimodal_initial_content": + nodes.append(node) + elif isinstance(node, ast.Assign) and any( + isinstance(t, ast.Name) and t.id == "_VISION_MIMES" for t in node.targets): + nodes.append(node) + assert nodes, "_multimodal_initial_content not found in agentmain.py" + ns = {"NativeToolClient": _FakeNativeClient} + exec(compile(ast.Module(body=nodes, type_ignores=[]), "", "exec"), ns) + return ns["_multimodal_initial_content"], ns + + +class TestMultimodalInitialContent: + def setup_method(self): + self.helper, self.ns = _load_helper() + + def test_no_images_returns_none(self): + assert self.helper("q", [], _FakeNativeClient()) is None + assert self.helper("q", None, _FakeNativeClient()) is None + + def test_non_native_client_returns_none(self, tmp_path): + p = tmp_path / "a.png" + p.write_bytes(b"x") + # Only NativeToolClient backends understand image blocks; other clients + # must keep the old plain-text behaviour. + assert self.helper("q", [str(p)], object()) is None + + def test_builds_text_then_image_blocks(self, tmp_path): + p = tmp_path / "a.png" + p.write_bytes(b"\x89PNG\r\n\x1a\n-fake") + blocks = self.helper("describe this", [str(p)], _FakeNativeClient()) + assert blocks[0] == {"type": "text", "text": "describe this"} + img = blocks[1] + assert img["type"] == "image" + assert img["source"]["type"] == "base64" + assert img["source"]["media_type"] == "image/png" + assert base64.b64decode(img["source"]["data"]) == b"\x89PNG\r\n\x1a\n-fake" + + def test_unknown_extension_falls_back_to_png(self, tmp_path): + p = tmp_path / "a.unknownext" + p.write_bytes(b"x") + blocks = self.helper("q", [str(p)], _FakeNativeClient()) + assert blocks[1]["source"]["media_type"] == "image/png" + + def test_non_vision_mime_becomes_text_reference(self, tmp_path): + p = tmp_path / "a.svg" + p.write_text("", encoding="utf-8") + blocks = self.helper("q", [str(p)], _FakeNativeClient()) + assert blocks[1] == {"type": "text", "text": f"[attached file: {p}]"} + + def test_unreadable_path_becomes_error_text(self, tmp_path): + # A missing/unreadable attachment must surface as text, never vanish + # silently — silent drops are exactly what this issue is about. + blocks = self.helper("q", [str(tmp_path / "gone.png")], _FakeNativeClient()) + assert blocks[0] == {"type": "text", "text": "q"} + assert blocks[1]["type"] == "text" + assert "gone.png" in blocks[1]["text"] + + def test_read_failure_becomes_error_text(self, tmp_path): + def boom(*a, **k): + raise OSError("permission denied") + self.ns["open"] = boom + blocks = self.helper("q", [str(tmp_path / "a.png")], _FakeNativeClient()) + assert blocks[1]["type"] == "text" + assert "permission denied" in blocks[1]["text"] + + def test_dict_shaped_entries_supported(self, tmp_path): + p = tmp_path / "a.png" + p.write_bytes(b"x") + blocks = self.helper("q", [{"name": "a.png", "path": str(p)}], _FakeNativeClient()) + assert blocks[1]["type"] == "image" + + +# --------------------------------------------------------------------------- +# agentmain.run() wiring: images must reach agent_runner_loop +# --------------------------------------------------------------------------- + +class TestAgentmainWiring: + def test_agent_runner_loop_receives_initial_user_content(self): + src = (PROJECT_ROOT / "agentmain.py").read_text(encoding="utf-8") + tree = ast.parse(src) + run_fn = next(n for n in ast.walk(tree) + if isinstance(n, ast.FunctionDef) and n.name == "run") + calls = [n for n in ast.walk(run_fn) + if isinstance(n, ast.Call) and isinstance(n.func, ast.Name) + and n.func.id == "agent_runner_loop"] + assert calls, "agent_runner_loop call not found in run()" + kwargs = {kw.arg: kw.value for kw in calls[0].keywords} + assert "initial_user_content" in kwargs, ( + "run() must forward images via initial_user_content") + value = kwargs["initial_user_content"] + assert isinstance(value, ast.Call) and value.func.id == "_multimodal_initial_content" + all_args = list(value.args) + [kw.value for kw in value.keywords] + get_calls = [a for a in all_args + if isinstance(a, ast.Call) and getattr(a.func, "attr", None) == "get"] + assert get_calls, "images must be read from the task dict (task.get('images'))" diff --git a/llmcore.py b/llmcore.py index b9077f5df..e8765347f 100644 --- a/llmcore.py +++ b/llmcore.py @@ -579,6 +579,15 @@ def _to_responses_input(messages): elif ptype == "image_url": url = (part.get("image_url") or {}).get("url", "") if url and role != "assistant": parts.append({"type": "input_image", "image_url": url}) + elif ptype == "image" and role != "assistant": + # Claude-style image blocks (fsapp/tui/desktop attachments) were + # silently dropped here; convert them like _msgs_claude2oai does. + src = part.get("source") or {} + if src.get("type") == "base64" and src.get("data"): + parts.append({"type": "input_image", + "image_url": f"data:{src.get('media_type', 'image/png')};base64,{src['data']}"}) + elif src.get("type") == "url" and src.get("url"): + parts.append({"type": "input_image", "image_url": src["url"]}) if len(parts) == 0: parts = [{"type": text_type, "text": str(content) if not isinstance(content, list) else '[empty]'}] result.append({"role": role, "content": parts}) pending = [] @@ -590,6 +599,15 @@ def _to_responses_input(messages): return result +def _drop_blank_text_blocks(blocks): + """Drop whitespace-only text blocks (they 400 on strict API proxies). + + Only text blocks are candidates: image/tool_result blocks carry no "text" + key and must survive untouched. + """ + return [b for b in blocks if b.get("type") != "text" or b.get("text", "").strip()] + + def _msgs_claude2oai(messages): result = [] for msg in messages: @@ -1208,7 +1226,7 @@ def chat(self, messages, tools=None): if tid not in tr_id_set: tool_result_blocks.append({"type": "tool_result", "tool_use_id": tid, "content": ""}) self._pending_tool_ids = [] # Filter whitespace-only text blocks that cause 400 on strict API proxies - filtered_content = [c for c in combined_content if c.get("text", "").strip()] + filtered_content = _drop_blank_text_blocks(combined_content) final_content = tool_result_blocks + filtered_content if not final_content: final_content = [{"type": "text", "text": "."}] merged = {"role": "user", "content": final_content}