fix(agent): drop the api_content sidecar when stripping images from history

`api_content` is the byte-stability sidecar from #67274: it holds the exact
bytes previously sent for a message, and every turn substitutes it back into
`content` when building `api_messages`. `drop_stale_api_content` exists so a
content rewrite cannot be replayed from it — its own docstring states the
contract, and names the historical image strip as one of the callers:

    Replaying the pre-rewrite sidecar would resend exactly what the rewrite
    removed, so it must be dropped — the cost is one cache boundary miss,
    never wrong content.

`_strip_images_from_messages` never drops it. The image-rejection recovery in
`conversation_loop` runs it over the persistent history, not just the per-call
copy:

    agent._vision_supported = False
    _imgs_removed = _strip_images_from_messages(messages)      # history
    if isinstance(api_messages, list):
        _strip_images_from_messages(api_messages)

and `api_messages` are copies (`api_msg = msg.copy()`), so the history message
keeps its sidecar. The strip is therefore undone on the very next turn.

Reproduced with the real functions:

    history content after strip : [{'type': 'text', 'text': 'look'}]
    sidecar still present       : True
    NEXT TURN sends             : 'look<IMAGE BYTES SENT LAST TURN>'

This is worse than a one-turn glitch, because the recovery cannot fire again:
it is gated on `getattr(agent, "_vision_supported", True)` and just set that
False. So on every subsequent turn the sidecar re-injects the images, the
text-only endpoint rejects them again, and the branch that would strip them is
disabled — the session stays wedged on a 4xx it already knew how to fix.

Drop the sidecar on each message the strip rewrites, inside the function so
every caller is covered. Messages with no images keep theirs, so only the
rewritten message pays a cache boundary — the tradeoff the invariant
prescribes. The two sibling recovery paths, `_sanitize_messages_surrogates`
and `_sanitize_messages_non_ascii`, are already safe: both walk every string
field on the message and so scrub the sidecar in passing. This one only
touches `content`.

tests/run_agent/test_image_rejection_fallback.py: new
TestStripImagesDropsStaleApiContent — the rewritten message loses its sidecar,
the next turn does not resend the stripped images, the tool-placeholder rewrite
is covered too, and untouched messages keep their sidecar. All four fail on
main. 53 passed across the image-rejection and api_content-sidecar suites; 307
passed across the sanitization/image/sidecar/replay agent tests (8 failures in
test_image_routing.py / test_save_url_image.py are pre-existing and fail
identically on clean main).
This commit is contained in:
Frowtek
2026-07-21 20:09:48 +03:00
committed by Teknium
parent a5c7eed5f3
commit e1762bd30b
2 changed files with 87 additions and 0 deletions

View File

@@ -416,8 +416,16 @@ def _strip_images_from_messages(messages: list) -> bool:
practice this only hits synthetic image-only user messages appended
for attachment delivery; real user turns always include text.
This runs on the persistent history as well as the per-call copy, so any
message it rewrites must also lose its ``api_content`` sidecar: the sidecar
carries the exact bytes previously sent — here, the images this strip
exists to remove — and the next turn substitutes it back into ``content``,
undoing the strip on the wire.
Returns True if any image parts were removed.
"""
from agent.turn_context import drop_stale_api_content
found = False
to_delete = []
for i, msg in enumerate(messages):
@@ -443,6 +451,8 @@ def _strip_images_from_messages(messages: list) -> bool:
# Synthetic image-only user/assistant message with no text;
# safe to drop.
to_delete.append(i)
# Content was rewritten — the pre-strip sidecar is now stale.
drop_stale_api_content(msg)
for i in reversed(to_delete):
del messages[i]
return found

View File

@@ -173,3 +173,80 @@ class TestImageRejectionPhraseIsolation:
assert self._matches(body) is True, f"false negative on: {body}"
class TestStripImagesDropsStaleApiContent:
"""The strip runs on the persistent history, not just the per-call copy.
``api_content`` is the byte-stability sidecar: it holds the exact bytes
previously sent for a message, and the next turn substitutes it back into
``content``. Leaving it in place on a message this function rewrote would
replay the images the strip just removed — and the recovery cannot re-fire,
because it sets ``_vision_supported = False`` and gates itself on that. The
session would then send rejected images on every subsequent turn.
Same contract the other content-rewrite paths follow (stale-confirmation
redaction in ``replay_cleanup``, compression rewrites, merge-into-tail):
"the cost is one cache boundary miss, never wrong content".
"""
@staticmethod
def _wire(msg):
"""What the next turn actually sends for this history message."""
from agent.turn_context import substitute_api_content
api_msg = msg.copy()
substitute_api_content(api_msg)
return api_msg["content"]
def _image_msg(self, sidecar="look<IMAGE BYTES SENT LAST TURN>"):
return {
"role": "user",
"content": [
{"type": "text", "text": "look"},
{"type": "image_url", "image_url": {"url": "data:image/png;base64,AAAA"}},
],
"api_content": sidecar,
}
def test_stripped_message_loses_its_sidecar(self):
msgs = [self._image_msg()]
assert _strip_images_from_messages(msgs) is True
assert "api_content" not in msgs[0]
def test_next_turn_does_not_resend_the_stripped_images(self):
msgs = [self._image_msg()]
_strip_images_from_messages(msgs)
wire = self._wire(msgs[0])
assert "IMAGE BYTES" not in str(wire), (
"the stale sidecar replayed the images the strip removed"
)
assert wire == [{"type": "text", "text": "look"}]
def test_tool_placeholder_message_also_loses_its_sidecar(self):
"""An image-only tool result becomes a placeholder — same rewrite."""
msgs = [
{
"role": "tool",
"tool_call_id": "call_1",
"content": [{"type": "image_url", "image_url": {"url": "x"}}],
"api_content": "<SCREENSHOT BYTES>",
}
]
assert _strip_images_from_messages(msgs) is True
assert "api_content" not in msgs[0]
assert "image content removed" in msgs[0]["content"]
def test_untouched_messages_keep_their_sidecar(self):
"""Only rewritten messages pay the cache boundary — not the whole prefix."""
msgs = [
{
"role": "user",
"content": [{"type": "text", "text": "no images here"}],
"api_content": "no images here<injected ctx>",
},
self._image_msg(),
]
_strip_images_from_messages(msgs)
assert msgs[0]["api_content"] == "no images here<injected ctx>"
assert "api_content" not in msgs[1]