diff --git a/agent/agent_runtime_helpers.py b/agent/agent_runtime_helpers.py index b5706cadfec3c..3d9fcca5fc476 100644 --- a/agent/agent_runtime_helpers.py +++ b/agent/agent_runtime_helpers.py @@ -2779,6 +2779,90 @@ def _strip_tool_suffix(s: str) -> str | None: +def _normalize_provider_image_data_urls_in_messages( + messages: List[Dict[str, Any]], +) -> tuple[List[Dict[str, Any]], int]: + """Normalize provider-hostile inline image data URLs in API-bound messages. + + Stored session history can contain old multimodal tool outputs created + before the vision layer learned how to normalize Apple CgBI PNGs. The + session itself should remain an honest transcript, but the per-call API + payload must be repaired so stale chats do not fail every future replay. + + Returns a copy-on-write message list plus the replacement count. Unchanged + branches keep their original objects; changed nested dict/list structures are + copied so canonical conversation history is not mutated by a shallow API + replay copy. + """ + if not messages: + return messages, 0 + + try: + from tools.vision_tools import normalize_image_data_url_for_provider + except Exception as exc: + logger.debug("Pre-call sanitizer: image data URL normalizer unavailable: %s", exc) + return messages, 0 + + changed = 0 + + def _rewrite_url(url: Any) -> Any: + nonlocal changed + if not isinstance(url, str): + return url + replacement = normalize_image_data_url_for_provider(url) + if replacement and replacement != url: + changed += 1 + return replacement + return url + + def _rewrite_value(value: Any) -> Any: + if isinstance(value, dict): + updated: Optional[dict] = None + for key, nested in value.items(): + if key == "image_url": + if isinstance(nested, dict): + url = nested.get("url") + rewritten_url = _rewrite_url(url) + if rewritten_url is not url: + rewritten_nested = dict(nested) + rewritten_nested["url"] = rewritten_url + else: + rewritten_nested = _rewrite_value(nested) + rewritten = rewritten_nested + elif isinstance(nested, str): + rewritten = _rewrite_url(nested) + else: + rewritten = _rewrite_value(nested) + else: + rewritten = _rewrite_value(nested) + + if rewritten is not nested: + if updated is None: + updated = dict(value) + updated[key] = rewritten + return updated if updated is not None else value + + if isinstance(value, list): + updated_items = None + for idx, item in enumerate(value): + rewritten = _rewrite_value(item) + if rewritten is not item: + if updated_items is None: + updated_items = list(value) + updated_items[idx] = rewritten + return updated_items if updated_items is not None else value + + return value + + normalized_messages = _rewrite_value(messages) + if changed: + logger.info( + "Pre-call sanitizer: normalized %d inline image data URL(s) for provider replay", + changed, + ) + return normalized_messages, changed + + # Placeholder substituted for an empty non-final message that would otherwise # make the provider reject the whole request. Kept identical to the stub- # creation placeholder in chat_completion_helpers so a healed transcript reads @@ -2902,6 +2986,7 @@ def repair_empty_non_final_messages( return messages + def sanitize_api_messages(messages: List[Dict[str, Any]]) -> List[Dict[str, Any]]: """Fix orphaned tool_call / tool_result pairs before every LLM call. @@ -3106,6 +3191,9 @@ def sanitize_api_messages(messages: List[Dict[str, Any]]) -> List[Dict[str, Any] "Pre-call sanitizer: removed %d duplicate tool_call_id reference(s)", removed_dupes, ) + + # 4. Repair stale multimodal history on the API-bound copy. + messages, _ = _normalize_provider_image_data_urls_in_messages(messages) return messages diff --git a/agent/codex_responses_adapter.py b/agent/codex_responses_adapter.py index ee75f4190e6db..f51a49321f0d7 100644 --- a/agent/codex_responses_adapter.py +++ b/agent/codex_responses_adapter.py @@ -76,6 +76,22 @@ def _classify_responses_issuer( # Multimodal content helpers # --------------------------------------------------------------------------- +def _normalize_provider_image_data_url(url: str) -> str: + """Normalize provider-hostile inline image data URLs when possible.""" + if not isinstance(url, str) or not url.startswith("data:image/"): + return url + try: + from tools.vision_tools import normalize_image_data_url_for_provider + except Exception as exc: + logger.debug("Codex preflight image data URL normalizer unavailable: %s", exc) + return url + replacement = normalize_image_data_url_for_provider(url) + if replacement and replacement != url: + logger.info("Codex preflight normalized inline image data URL for provider replay") + return replacement + return url + + def _chat_content_to_responses_parts(content: Any, *, role: str = "user") -> List[Dict[str, Any]]: """Convert chat-style multimodal content to Responses API input parts. @@ -120,6 +136,7 @@ def _chat_content_to_responses_parts(content: Any, *, role: str = "user") -> Lis url = image_ref if not isinstance(url, str) or not url: continue + url = _normalize_provider_image_data_url(url) image_part: Dict[str, Any] = {"type": "input_image", "image_url": url} if isinstance(detail, str) and detail.strip(): image_part["detail"] = detail.strip() @@ -666,6 +683,7 @@ def _preflight_codex_input_items( elif ptype == "input_image": url = part.get("image_url") if isinstance(url, str) and url: + url = _normalize_provider_image_data_url(url) entry: Dict[str, Any] = {"type": "input_image", "image_url": url} detail = part.get("detail") if isinstance(detail, str) and detail.strip(): @@ -797,6 +815,7 @@ def _preflight_codex_input_items( url = image_ref if not isinstance(url, str): url = str(url or "") + url = _normalize_provider_image_data_url(url) image_part: Dict[str, Any] = {"type": "input_image", "image_url": url} if isinstance(detail, str) and detail.strip(): image_part["detail"] = detail.strip() diff --git a/tests/run_agent/test_agent_guardrails.py b/tests/run_agent/test_agent_guardrails.py index 9bf565fd63080..2145b25dd4a99 100644 --- a/tests/run_agent/test_agent_guardrails.py +++ b/tests/run_agent/test_agent_guardrails.py @@ -149,6 +149,43 @@ def test_truly_orphaned_with_whitespace_still_removed(self): assert len(tool_msgs) == 1 assert tool_msgs[0]["tool_call_id"] == "c_valid" + def test_image_data_urls_are_normalized_before_api_replay(self, monkeypatch): + """Stale sessions can replay old native image tool outputs. + + Even if the original tool result was written before current vision + normalization existed, the pre-call sanitizer should repair provider- + hostile data URLs on the API-bound copy instead of resending them. + """ + stale_url = "data:image/png;base64,stale-cgbi-png" + fixed_url = "data:image/png;base64,provider-safe-png" + + monkeypatch.setattr( + "tools.vision_tools.normalize_image_data_url_for_provider", + lambda url: fixed_url if url == stale_url else None, + raising=False, + ) + + msgs = [{ + "role": "tool", + "tool_call_id": "c7", + "content": [ + {"type": "input_text", "text": "screenshot"}, + {"type": "input_image", "image_url": stale_url}, + {"type": "image_url", "image_url": {"url": stale_url}}, + ], + }, { + "role": "assistant", + "tool_calls": [assistant_dict_call("c7")], + }] + + out = AIAgent._sanitize_api_messages(msgs) + + tool_content = out[0]["content"] + assert tool_content[1]["image_url"] == fixed_url + assert tool_content[2]["image_url"]["url"] == fixed_url + assert msgs[0]["content"][1]["image_url"] == stale_url + assert msgs[0]["content"][2]["image_url"]["url"] == stale_url + # --------------------------------------------------------------------------- # Phase 2a — _cap_delegate_task_calls diff --git a/tests/run_agent/test_run_agent_codex_responses.py b/tests/run_agent/test_run_agent_codex_responses.py index b6be6812fb4c0..1f560e429a4ed 100644 --- a/tests/run_agent/test_run_agent_codex_responses.py +++ b/tests/run_agent/test_run_agent_codex_responses.py @@ -1768,6 +1768,51 @@ def test_preflight_codex_api_kwargs_rejects_function_call_output_without_call_id ) +def test_preflight_codex_api_kwargs_normalizes_stale_image_data_urls(monkeypatch): + """Preflight is the last guard before Codex sees replayed image bytes.""" + agent = _build_agent(monkeypatch) + stale_url = "data:image/png;base64,stale-cgbi-png" + fixed_url = "data:image/png;base64,provider-safe-png" + + monkeypatch.setattr( + "tools.vision_tools.normalize_image_data_url_for_provider", + lambda url: fixed_url if url == stale_url else None, + raising=False, + ) + + from agent.codex_responses_adapter import _preflight_codex_api_kwargs + preflight = _preflight_codex_api_kwargs( + { + "model": "gpt-5-codex", + "instructions": "You are Hermes.", + "input": [ + { + "role": "user", + "content": [ + {"type": "input_text", "text": "look"}, + {"type": "input_image", "image_url": stale_url}, + ], + }, + { + "type": "function_call_output", + "call_id": "call_vision", + "output": [ + {"type": "input_text", "text": "screenshot"}, + {"type": "input_image", "image_url": stale_url}, + ], + }, + ], + "tools": [], + "store": False, + } + ) + + user_content = preflight["input"][0]["content"] + output = preflight["input"][1]["output"] + assert user_content[1]["image_url"] == fixed_url + assert output[1]["image_url"] == fixed_url + + def test_preflight_codex_api_kwargs_rejects_unsupported_request_fields(monkeypatch): agent = _build_agent(monkeypatch) kwargs = _codex_request_kwargs() diff --git a/tests/tools/test_vision_tools.py b/tests/tools/test_vision_tools.py index 5715603397818..edf8d9f3611f4 100644 --- a/tests/tools/test_vision_tools.py +++ b/tests/tools/test_vision_tools.py @@ -4,6 +4,7 @@ import json import logging import os +import subprocess from pathlib import Path from typing import Awaitable from unittest.mock import AsyncMock, MagicMock, patch @@ -16,11 +17,13 @@ _determine_mime_type, _image_to_base64_data_url, _resize_image_for_vision, + _revert_apple_cgbi_png, _image_exceeds_dimension, _EMBED_MAX_DIMENSION, _is_image_size_error, _MAX_BASE64_BYTES, _RESIZE_TARGET_BYTES, + normalize_image_data_url_for_provider, vision_analyze_tool, check_vision_requirements, ) @@ -166,6 +169,80 @@ def test_file_not_found_raises(self, tmp_path): with pytest.raises(FileNotFoundError): _image_to_base64_data_url(tmp_path / "nonexistent.png") + def test_apple_cgbi_png_is_normalized_for_provider_data_url(self, tmp_path): + """Apple-optimized iOS PNGs contain a CgBI chunk rejected by vision APIs.""" + pytest.importorskip("PIL.Image") + cgbi_png_b64 = ( + "iVBORw0KGgoAAAAEQ2dCSVAAIAYsuHdmAAAADUlIRFIAAAB4AAAAeAgGAAAAOWQ20gAAAARnQU1BAACxjwv8YQUA" + "AAABc1JHQgCuzhzpAAAAIGNIUk0AAHomAACAhAAA+gAAAIDoAAB1MAAA6mAAADqYAAAXcJy6UTwAAABEZVhJZk1N" + "ACoAAAAIAAGHaQAEAAAAAQAAABoAAAAAAAOgAQADAAAAAQABAACgAgAEAAAAAQAAAHigAwAEAAAAAQAAAHgAAAAA" + "COJV7gAAABxpRE9UAAAAAgAAAAAAAAA8AAAAKAAAADwAAAA8AAADToF6LdYAAAMaSURBVOza3U/TUBzGcf4x3Gi7" + "DQGFCAg6Ar7EG6OIGP3TSEAW51u40qzdK2YgMTJRBNrBVtjKHi9IZcrL1m5jZ91z8b1bcpJ98js5PW1PoF8D8249" + "/BMIzAjMCMwIzAjMCMwITGBGYEZgRmBGYEZgRmACMwIzAjMCMwIzAjMCMwITmBGYEZgRmBGYEZgRmMDiJwdVSIH6" + "a2wtzdFadnKQwK5SQhpu301j5uEqph/UbupeBqGBuKu1+hQVI6PJuteyu/9oFRPhNJQQgR3nl1UsRHQYRxVs71/e" + "TqGCr7kyxidTkIPOJtknqQjPZBD/YmK3WHstuz2zgo0fJTyezUJSVAK7AV56a8AEYBxd3n4Z2PxtOQb2STGEZzJI" + "rx+iWKm9jl3hGMjtWpidX4NPinGLdgu8GDVQrAD64eXlS8D3bWfAPklFePoUt9YadgcWsLlj4enzNVzri/GQJSJw" + "Y7hl4XCFBVZC2oUHlFYBexFXSGApoGJoOIHBm4lzH3FaAWwfqNxvy1khcYUDlgIqBm7EEV3J483HPK4Pxc8gNxvY" + "y7hCAdu4kQ8nJ2QTwOt3xhnkZgI3hivutiwcsBRQ0T8Yx1L0BNf+I00Ay/8hNwvY65MrDLCNuxjV/8GtRq6e5GYA" + "e/VAJRzw6eSej3veJPf6Yw0BNz65nYPbVuBak3vRJCshDQsR3RVwI5Ob66Btue3ASkjD0EgCkfdGXbjVyAvLOqIr" + "eRSOnQH3+mOuJ9e+fuw03LYBy0ENt8aTUDOmI2D98OR+ec+s77c28PBoEnem0u5xX3Qmblu36D5FxdhECp+TRcfI" + "9ZYvARtbZTyZyyKZNbtqcoU4ZPnl1iIbR8BP4xjruVJdW7qXJleYx6SrQM6XuhNXmIuOViN3K65QV5XtRvYirnAv" + "G9qF7FVcIV8X+mUVY5MpxNJXg+xlXGFf+PtlFeNXgOx1XKE/2fmLnCqiCOJ68pusauRmTvKBBWztWXjmcdyO+Oiu" + "2dv1gQVs6RbmX617HrdjvqpsFnK34XbUZ7M28qdEAftld/fS336VMfdyrWtwA/0a/gAAAP//qM/nqQAAA1RJREFU" + "7dn/SxNxHMfx+7dCunbe7WZGYolLrV/7oR8ixAoLgn6JCKIf+qVfgggy6YtEIYUWIUH9Jt7ddJtZpiShTpq3zd2+" + "3O7VD+tsk2l328nuy1t4/iac3mPvz+ezOybcpcArdbBzuDj8DWnNgFoEtgvWy+nAl9kcwl0KOsMyvPR/txLjlT80" + "xMs4czYBOakhW7aHa5Y3gOkZFV0nYoFBZjyDO5SAsqghbzSHa6YhWMiMVyZX+do6bi3y1Mw2It3+X64Zz0wunMGt" + "RX77IY1Idwy8j5EZ1+M6OLn7I/t3kpmg4jZC9uMkM34+UNlD9ueezAR1coOyXDNBOFAFeblmgrosB2WSGb/jqkXY" + "fvLlJ2TGz3uuWgTW0xUkV4rYqQQTuW3AnCAjOhiHnLS352ZK1az+7up6GecvLGF2Pg8NwUNuCzAvKjjdH0dsSbN1" + "07Nl4GdKx+JK0RJypgSspXT09i2gL7oAKdEKcgy8KBOwlYSIgu6eGF6/+w0NsPTqL6cDG2oFl0a+Y3I6bWlJN4H7" + "B+JgOQn9A3FIibztU3rhL7J4XPEcctuWaF6s3qyxFynkjYORszqwoeoYGV3GkY45TM2otoF5UUaIl3eRm5nkyWnv" + "Ibf1kCWI1b344eMN7FQaI2fLVdzLo8tgQxJCvNw0sHmwiw4mICdbRPbIntz2r0lCRMGxTgn3H/xCplx/gMrp9bgm" + "UCvA/5DtH/C8iOyKBx1CRAHLSbhzbw3bBQOZkjm5lTpcp4B3kYccQHb5cu2qZ9FsSMLNW6tIawZS2QpG9uA6CVw/" + "yXnfTrLr3iaxnIRrN1Zw9foPsJzUEMUpYEf3ZJdOsmvfB3PC/iBOAu+dZL8dvBivfXE/DGA/L9cEfBjILlquCdjn" + "k+xJ4I+fMzBQvaEHVQSQyhiIDsZtTVUtcsnCdcwKqP68/6TiZO88hAgB264zLOP23TU8f7WFpy8PbnxiC4/GNtFz" + "ah5CRLb9QRo4l8CTZ5sYn/j/tWqbeLOF4SvLrngD5Tlg8+YfDUlgrcRJTU8SL8rWrtEgt+zDngSmCJgiYAKmCJgi" + "YIqAKQKmCJgiYAKmCJgiYIqAKQKmCJgiYAKmm0DAFAFTBEwRMEXAFAFTBEzAFAFTBEwRMEXAFAFT+/UHK7SQEQAA" + "AABJRU5ErkJggg==" + ) + img = tmp_path / "ios-icon.png" + img.write_bytes(base64.b64decode(cgbi_png_b64)) + + result = _image_to_base64_data_url(img, mime_type="image/png") + normalized = base64.b64decode(result.split(",", 1)[1]) + + assert result.startswith("data:image/png;base64,") + assert b"CgBI" in img.read_bytes() + assert b"CgBI" not in normalized + + replayed = normalize_image_data_url_for_provider( + f"data:image/png;base64,{cgbi_png_b64}" + ) + assert replayed is not None + replayed_bytes = base64.b64decode(replayed.split(",", 1)[1]) + assert b"CgBI" not in replayed_bytes + + def test_cgbi_png_utilities_do_not_inherit_interactive_stdin(self): + """Image helpers must not read prompt_toolkit's interactive input.""" + calls = [] + + def fake_run(args, **kwargs): + calls.append((args, kwargs)) + if args[1:3] == ["-find", "pngcrush"]: + return MagicMock(returncode=0, stdout="/usr/bin/pngcrush\n") + return MagicMock(returncode=1) + + with ( + patch( + "shutil.which", + side_effect=lambda name: "/usr/bin/xcrun" if name == "xcrun" else None, + ), + patch("subprocess.run", side_effect=fake_run), + ): + assert _revert_apple_cgbi_png(b"not-a-real-png") is None + + assert len(calls) == 2 + assert all(kwargs.get("stdin") is subprocess.DEVNULL for _, kwargs in calls) + # --------------------------------------------------------------------------- # _handle_vision_analyze — type signature & behavior diff --git a/tools/vision_tools.py b/tools/vision_tools.py index fb5a3820ec6f7..d2c7fdcc020e0 100644 --- a/tools/vision_tools.py +++ b/tools/vision_tools.py @@ -518,6 +518,166 @@ def _determine_mime_type(image_path: Path) -> str: return mime_types.get(extension, 'image/jpeg') +def _png_contains_chunk(data: bytes, chunk_type: bytes) -> bool: + """Return True when PNG bytes contain ``chunk_type``. + + Apple's iOS-optimized PNGs include a private ``CgBI`` chunk. Pillow can + still open them, but some model providers reject the raw bytes as invalid + image data. Detecting the chunk lets us normalize only the problematic + format instead of recompressing every image. + """ + if not data.startswith(b"\x89PNG\r\n\x1a\n"): + return False + + offset = 8 + while offset + 12 <= len(data): + length = int.from_bytes(data[offset:offset + 4], "big") + chunk = data[offset + 4:offset + 8] + if chunk == chunk_type: + return True + offset += 8 + length + 4 + if length < 0 or offset > len(data): + return False + return False + + +def _normalize_image_bytes_for_data_url( + image_path: Path, + data: bytes, + mime_type: str, +) -> tuple[bytes, str]: + """Normalize image bytes that are valid locally but rejected by providers.""" + if mime_type != "image/png" or not _png_contains_chunk(data, b"CgBI"): + return data, mime_type + + normalized = _revert_apple_cgbi_png(data) + if normalized is not None: + logger.info( + "Normalized Apple CgBI PNG for vision payload: %s (%.1f KB -> %.1f KB)", + image_path, + len(data) / 1024, + len(normalized) / 1024, + ) + return normalized, "image/png" + + logger.warning( + "Failed to normalize Apple CgBI PNG for vision payload: %s", + image_path, + ) + return data, mime_type + + +def normalize_image_data_url_for_provider( + data_url: str, + *, + source_label: str = "inline data URL", +) -> Optional[str]: + """Return a provider-safe replacement for an image data URL when needed. + + This is used for stale session replay as well as new vision tool outputs: + a previous Hermes version may have already stored a raw Apple CgBI PNG in + message history, so normalizing only when the file is first read is not + enough. Return ``None`` when the URL is already safe or cannot be helped. + """ + if not isinstance(data_url, str) or not data_url.startswith("data:image/"): + return None + + header, separator, payload = data_url.partition(",") + if separator != "," or ";base64" not in header.lower(): + return None + + mime = header[len("data:"):].split(";", 1)[0].strip().lower() + if mime != "image/png": + # Today the only provider-hostile format we can repair is Apple's + # PNG-with-CgBI variant. Avoid decoding large JPEG/WebP payloads on + # every stale-history replay when we already know they cannot change. + return None + + try: + raw = base64.b64decode(payload) + except Exception as exc: + logger.debug("Could not decode image data URL for provider normalization: %s", exc) + return None + + normalized, normalized_mime = _normalize_image_bytes_for_data_url( + Path(source_label), + raw, + mime, + ) + if normalized == raw and normalized_mime == mime: + return None + + encoded = base64.b64encode(normalized).decode("ascii") + return f"data:{normalized_mime};base64,{encoded}" + + +def _revert_apple_cgbi_png(data: bytes) -> Optional[bytes]: + """Return a standard PNG for Apple CgBI bytes, or None if unavailable.""" + # Xcode's pngcrush knows how to undo iPhone PNG optimisation correctly. + # Prefer it when present; Pillow may open CgBI files but can fail while + # loading their altered IDAT stream. + try: + import shutil + import subprocess + import tempfile + + pngcrush = shutil.which("pngcrush") + if pngcrush is None: + xcrun = shutil.which("xcrun") + if xcrun: + probe = subprocess.run( + [xcrun, "-find", "pngcrush"], + stdin=subprocess.DEVNULL, + capture_output=True, + text=True, + timeout=5, + check=False, + ) + candidate = probe.stdout.strip() + if probe.returncode == 0 and candidate: + pngcrush = candidate + + if pngcrush: + with tempfile.TemporaryDirectory(prefix="hermes-cgbi-") as td: + src = Path(td) / "input.png" + dst = Path(td) / "output.png" + src.write_bytes(data) + proc = subprocess.run( + [pngcrush, "-q", "-revert-iphone-optimizations", str(src), str(dst)], + stdin=subprocess.DEVNULL, + capture_output=True, + timeout=10, + check=False, + ) + if proc.returncode == 0 and dst.exists(): + reverted = dst.read_bytes() + if reverted.startswith(b"\x89PNG\r\n\x1a\n") and not _png_contains_chunk(reverted, b"CgBI"): + return reverted + except Exception as exc: + logger.debug("pngcrush CgBI normalization failed: %s", exc, exc_info=True) + + try: + from PIL import Image, ImageFile + import io as _io + + previous = ImageFile.LOAD_TRUNCATED_IMAGES + ImageFile.LOAD_TRUNCATED_IMAGES = True + try: + with Image.open(_io.BytesIO(data)) as img: + img.load() + buf = _io.BytesIO() + img.save(buf, format="PNG") + normalized = buf.getvalue() + if normalized.startswith(b"\x89PNG\r\n\x1a\n") and not _png_contains_chunk(normalized, b"CgBI"): + return normalized + finally: + ImageFile.LOAD_TRUNCATED_IMAGES = previous + except Exception as exc: + logger.debug("Pillow CgBI normalization failed: %s", exc, exc_info=True) + + return None + + def _image_to_base64_data_url(image_path: Path, mime_type: Optional[str] = None) -> str: """ Convert an image file to a base64-encoded data URL. @@ -531,16 +691,17 @@ def _image_to_base64_data_url(image_path: Path, mime_type: Optional[str] = None) """ # Read the image as bytes data = image_path.read_bytes() - - # Encode to base64 - encoded = base64.b64encode(data).decode("ascii") - + # Determine MIME type mime = mime_type or _determine_mime_type(image_path) - + data, mime = _normalize_image_bytes_for_data_url(image_path, data, mime) + + # Encode to base64 + encoded = base64.b64encode(data).decode("ascii") + # Create data URL data_url = f"data:{mime};base64,{encoded}" - + return data_url