From 8aeb3f6ee3c4bc082a835c17bac8ce129c4b56ff Mon Sep 17 00:00:00 2001 From: Sora-bluesky Date: Wed, 22 Jul 2026 12:44:21 +0900 Subject: [PATCH] fix(agent): un-brick sessions on non-retryable 400s that carry image parts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The permanent-brick class in #69078: xAI returns 'Invalid PNG image' when a re-serialized image part in replayed history becomes undecodable. The existing image-error patterns cover only Anthropic 'exceeds max dimension' wordings and 'model does not support images' strings, so the classifier lands on a generic non-retryable 400 and neither the shrink path nor the strip path fires. Every subsequent turn (even bare text) fails identically because the poison stays in history — the session is permanently wedged until deleted. Two recovery layers, deliberately separate: - Semantic split: new FailoverReason.image_corrupt with _IMAGE_CORRUPT_PATTERNS ('invalid png image' / 'invalid jpeg image'), checked BEFORE _IMAGE_TOO_LARGE_PATTERNS in both _classify_400 and _classify_by_message. Corrupt bytes route to strip-and-retry, never to the shrink path (shrinking corrupt bytes cannot help). - Generic fallback: any non-retryable 400 whose outgoing messages still contain image parts gets one strip-and-retry via the existing _strip_images_from_messages helper, guarded by a new stripped_images_this_turn one-shot flag on TurnRetryState. This un-bricks the session for any current or future provider wording without adding another pattern list to maintain. Item 3 from the report (multimodal-part integrity across FTS persistence + compaction handoff) is a separate investigation and remains follow-up work. Co-Authored-By: Claude Fable 5 Co-authored-by: paultaki --- agent/conversation_loop.py | 55 +++++++ agent/error_classifier.py | 32 ++++ agent/turn_retry_state.py | 1 + tests/agent/test_error_classifier.py | 1 + tests/agent/test_turn_retry_state.py | 1 + .../test_69078_image_corrupt_recovery.py | 154 ++++++++++++++++++ 6 files changed, 244 insertions(+) create mode 100644 tests/run_agent/test_69078_image_corrupt_recovery.py diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index f6f36d1903..d6832ae489 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -4886,6 +4886,61 @@ def run_conversation( "messages with image parts found; surfacing original error." ) + # Image-corrupt recovery: the provider decoded the request but + # rejected the image bytes themselves (e.g. xAI's "Invalid PNG + # image." on a re-serialized image part from replayed + # history). Shrinking corrupt bytes doesn't help, so strip the + # image parts and retry once instead of routing through the + # shrink path above. See issue #69078. + if ( + classified.reason == FailoverReason.image_corrupt + and not _retry.stripped_images_this_turn + ): + _retry.stripped_images_this_turn = True + _imgs_removed = _strip_images_from_messages(messages) + if isinstance(api_messages, list): + _imgs_removed = _strip_images_from_messages(api_messages) or _imgs_removed + if _imgs_removed: + agent._vprint( + f"{agent.log_prefix}⚠️ Provider rejected a corrupted image — " + f"stripped images from history and retrying...", + force=True, + ) + continue + else: + logger.info( + "image-corrupt recovery: no image parts found to " + "strip; surfacing original error." + ) + + # Generic strip-and-retry fallback: any other non-retryable + # 400 whose outgoing request still contains image parts. + # Providers invent new image-rejection wordings faster than + # we can catalogue them (this is exactly what bit us in + # issue #69078 — an xAI corruption message that matched + # none of the patterns above and permanently bricked the + # session). Rather than chase every wording, treat "400 + + # non-retryable + image parts present" as reason enough to + # try once without the images before giving up. One-shot per + # attempt via the same guard as the image-corrupt branch + # above so this never loops. + if ( + status_code == 400 + and not classified.retryable + and not _retry.stripped_images_this_turn + ): + _retry.stripped_images_this_turn = True + _imgs_removed = _strip_images_from_messages(messages) + if isinstance(api_messages, list): + _imgs_removed = _strip_images_from_messages(api_messages) or _imgs_removed + if _imgs_removed: + agent._vprint( + f"{agent.log_prefix}⚠️ Non-retryable 400 with image content in " + f"the request — stripped images from history and retrying...", + force=True, + ) + continue + # Anthropic OAuth subscription rejected the 1M-context beta # header ("long context beta is not yet available for this # subscription"). Disable the beta for the rest of this diff --git a/agent/error_classifier.py b/agent/error_classifier.py index 39e8ed5c52..09c176a448 100644 --- a/agent/error_classifier.py +++ b/agent/error_classifier.py @@ -57,6 +57,7 @@ class FailoverReason(enum.Enum): context_overflow = "context_overflow" # Context too large — compress, not failover payload_too_large = "payload_too_large" # 413 — compress payload image_too_large = "image_too_large" # Native image part exceeds provider's per-image limit — shrink and retry + image_corrupt = "image_corrupt" # Provider says the image bytes are undecodable — shrinking won't help, strip and retry instead # Model / provider policy model_not_found = "model_not_found" # 404 or invalid model — fallback to different model @@ -287,6 +288,20 @@ _IMAGE_TOO_LARGE_PATTERNS = [ # the likely culprit; we still try the shrink path before giving up. ] +# Image-corruption patterns — distinct from _IMAGE_TOO_LARGE_PATTERNS above. +# These fire when the provider can decode the request but not the image +# bytes themselves (e.g. a re-serialized image part in replayed history that +# lost data along the way). Re-encoding/shrinking corrupt bytes does not fix +# corruption, so this list is routed to the strip-and-retry path +# (FailoverReason.image_corrupt), never to the shrink path. +# +# xAI wording: {"code":"invalid-argument","error":"...Invalid PNG image."} +# See: https://github.com/NousResearch/hermes-agent/issues/69078 +_IMAGE_CORRUPT_PATTERNS = [ + "invalid png image", + "invalid jpeg image", +] + # Providers that follow the OpenAI spec strictly require tool message # ``content`` to be a string. Some (Anthropic native, Codex Responses, # Gemini native, first-party OpenAI) extend this to accept a content-parts @@ -1586,6 +1601,16 @@ def _classify_400( retryable=True, ) + # Image-corruption from 400 (xAI's undecodable-image check fires this way). + # Must be checked BEFORE image_too_large: both are image-shaped 400s, but + # corrupt bytes need strip-and-retry, not shrink-and-retry — shrinking + # can't repair a truncated/malformed PNG. + if any(p in error_msg for p in _IMAGE_CORRUPT_PATTERNS): + return result_fn( + FailoverReason.image_corrupt, + retryable=True, + ) + # Image-too-large from 400 (Anthropic's 5 MB per-image check fires this way). # Must be checked BEFORE context_overflow because messages can trip both # patterns ("exceeds" + "image") and image-shrink is a cheaper recovery. @@ -1877,6 +1902,13 @@ def _classify_by_message( retryable=True, ) + # Image-corruption patterns (from message text when no status_code) + if any(p in error_msg for p in _IMAGE_CORRUPT_PATTERNS): + return result_fn( + FailoverReason.image_corrupt, + retryable=True, + ) + # Image-too-large patterns (from message text when no status_code) if any(p in error_msg for p in _IMAGE_TOO_LARGE_PATTERNS): return result_fn( diff --git a/agent/turn_retry_state.py b/agent/turn_retry_state.py index 49790c6528..4111eecced 100644 --- a/agent/turn_retry_state.py +++ b/agent/turn_retry_state.py @@ -61,6 +61,7 @@ class TurnRetryState: native_compaction_reject_retry_attempted: bool = False image_shrink_retry_attempted: bool = False multimodal_tool_content_retry_attempted: bool = False + stripped_images_this_turn: bool = False oauth_1m_beta_retry_attempted: bool = False llama_cpp_grammar_retry_attempted: bool = False diff --git a/tests/agent/test_error_classifier.py b/tests/agent/test_error_classifier.py index 56817ffd69..5aebaa2eb4 100644 --- a/tests/agent/test_error_classifier.py +++ b/tests/agent/test_error_classifier.py @@ -61,6 +61,7 @@ class TestFailoverReason: "overloaded", "server_error", "timeout", "ssl_cert_verification", "context_overflow", "payload_too_large", "image_too_large", + "image_corrupt", "model_not_found", "format_error", "invalid_encrypted_content", "multimodal_tool_content_unsupported", diff --git a/tests/agent/test_turn_retry_state.py b/tests/agent/test_turn_retry_state.py index a182f17795..ab117b619a 100644 --- a/tests/agent/test_turn_retry_state.py +++ b/tests/agent/test_turn_retry_state.py @@ -26,6 +26,7 @@ EXPECTED_FIELDS = { "native_compaction_reject_retry_attempted", "image_shrink_retry_attempted", "multimodal_tool_content_retry_attempted", + "stripped_images_this_turn", "oauth_1m_beta_retry_attempted", "llama_cpp_grammar_retry_attempted", "primary_recovery_attempted", diff --git a/tests/run_agent/test_69078_image_corrupt_recovery.py b/tests/run_agent/test_69078_image_corrupt_recovery.py new file mode 100644 index 0000000000..5c11bb11ee --- /dev/null +++ b/tests/run_agent/test_69078_image_corrupt_recovery.py @@ -0,0 +1,154 @@ +"""Tests for reactive recovery when a provider rejects an image as corrupt. + +Covers issue #69078: xAI returns a 400 with "...Invalid PNG image." when a +re-serialized image part in replayed history becomes undecodable. None of +the existing image-error pattern lists matched that wording, so the turn +aborted as non-retryable and the session was permanently bricked (no +further image-bearing turn could ever succeed). + +Two independent pieces are locked in here: + + 1. agent/error_classifier.py: xAI's "Invalid PNG image." wording + classifies as FailoverReason.image_corrupt — a new reason distinct + from image_too_large, because shrinking corrupt bytes can't fix them. + It must NOT be routed to the shrink path. + 2. The generic strip-and-retry fallback: ANY non-retryable 400 whose + outgoing request still contains image parts gets one strip-and-retry + attempt, even for a wording nobody has catalogued yet. This is the + un-brick-generically half of the fix — it doesn't require enumerating + every provider's phrasing. + +Both recovery paths share a single one-shot-per-attempt guard +(TurnRetryState.stripped_images_this_turn) so at most one strip happens +per attempt regardless of which branch fires. +""" + +from __future__ import annotations + +from agent.error_classifier import FailoverReason, classify_api_error +from agent.message_sanitization import _strip_images_from_messages +from agent.turn_retry_state import TurnRetryState + + +class _FakeApiError(Exception): + """Stand-in for an openai.BadRequestError with status_code + body.""" + + def __init__(self, status_code: int, message: str, body: dict | None = None): + super().__init__(message) + self.status_code = status_code + self.body = body or {"error": {"message": message}} + self.response = None + + +# ─── Classifier: xAI corrupt-image wording ─────────────────────────────────── + + +class TestImageCorruptClassification: + def test_xai_invalid_png_image_classifies_as_image_corrupt(self): + err = _FakeApiError( + status_code=400, + message='{"code":"invalid-argument","error":"Request contains an ' + 'invalid argument: Invalid PNG image."}', + ) + result = classify_api_error(err, provider="xai", model="grok-5") + assert result.reason == FailoverReason.image_corrupt + assert result.retryable is True + + def test_xai_invalid_jpeg_image_classifies_as_image_corrupt(self): + err = _FakeApiError(status_code=400, message="Invalid JPEG image.") + result = classify_api_error(err, provider="xai", model="grok-5") + assert result.reason == FailoverReason.image_corrupt + + def test_does_not_route_to_shrink_path(self): + """image_corrupt must be a distinct reason from image_too_large — + the retry loop only enters the shrink branch on an exact match, so + this alone proves corrupt images skip the (useless) shrink attempt.""" + err = _FakeApiError(status_code=400, message="Invalid PNG image.") + result = classify_api_error(err, provider="xai", model="grok-5") + assert result.reason != FailoverReason.image_too_large + + def test_no_status_code_message_only_path(self): + err = Exception("Invalid PNG image.") + result = classify_api_error(err, provider="xai", model="grok-5") + assert result.reason == FailoverReason.image_corrupt + + def test_unrelated_400_still_falls_through_to_format_error(self): + err = _FakeApiError(status_code=400, message="unsupported parameter: foo") + result = classify_api_error(err, provider="xai", model="grok-5") + assert result.reason == FailoverReason.format_error + assert result.retryable is False + + +# ─── Generic strip-and-retry fallback trigger condition ────────────────────── + + +class TestGenericStripFallbackTriggerCondition: + """Mirrors the condition added at agent/conversation_loop.py: a + non-retryable 400 whose request still contains image parts gets one + strip-and-retry, regardless of the exact wording. + """ + + def test_novel_wording_falls_through_to_format_error_non_retryable(self): + """A wording nobody catalogued yet — the generic fallback's whole + point is that classification does NOT need to recognize it; only + (400, non-retryable, image parts present) matters.""" + err = _FakeApiError(status_code=400, message="unrecognized image format") + result = classify_api_error(err, provider="some-new-provider", model="x") + assert result.reason == FailoverReason.format_error + assert result.retryable is False + # The trigger condition in conversation_loop.py is: + # status_code == 400 and not classified.retryable + # — both hold here, so the generic fallback fires as long as the + # outgoing request has image parts (checked separately, at retry + # time, via _strip_images_from_messages's return value). + + +# ─── TurnRetryState guard shape ─────────────────────────────────────────────── + + +class TestStrippedImagesGuard: + def test_guard_defaults_false(self): + state = TurnRetryState() + assert state.stripped_images_this_turn is False + + def test_guard_is_shared_by_both_branches(self): + """Both the image_corrupt branch and the generic fallback branch key + off the same flag, so setting it once from either blocks the other — + proving at most one strip happens per attempt.""" + state = TurnRetryState() + state.stripped_images_this_turn = True + assert state.stripped_images_this_turn is True + + def test_one_shot_semantics_mirror_the_loop(self): + """Simulates the loop's guard-and-strip sequence directly against + the real strip helper: first pass strips and would retry, a second + pass on the same attempt is blocked by the guard so it never loops + forever on a request that keeps failing after stripping.""" + state = TurnRetryState() + msgs = [{ + "role": "user", + "content": [ + {"type": "text", "text": "look"}, + {"type": "image_url", "image_url": {"url": "data:image/png;base64,corrupt"}}, + ], + }] + + def attempt_strip_and_retry() -> bool: + if state.stripped_images_this_turn: + return False # guard blocks a second attempt + state.stripped_images_this_turn = True + return _strip_images_from_messages(msgs) + + assert attempt_strip_and_retry() is True + assert msgs[0]["content"] == [{"type": "text", "text": "look"}] + # Second call on the same attempt: guard blocks it even though the + # (now text-only) messages have nothing left to strip anyway. + assert attempt_strip_and_retry() is False + + def test_no_image_parts_present_nothing_to_strip(self): + """If the request has no image parts, the generic fallback's own + indicator (the strip helper's return value) is False, so the loop + must not treat this as a recovered attempt and must fall through to + the normal (non-retryable) error path.""" + msgs = [{"role": "user", "content": "just text, no images"}] + assert _strip_images_from_messages(msgs) is False