fix(agent): un-brick sessions on non-retryable 400s that carry image parts
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 <noreply@anthropic.com> Co-authored-by: paultaki <paultaki@users.noreply.github.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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",
|
||||
|
||||
154
tests/run_agent/test_69078_image_corrupt_recovery.py
Normal file
154
tests/run_agent/test_69078_image_corrupt_recovery.py
Normal file
@@ -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
|
||||
Reference in New Issue
Block a user