fix: avoid index-shift bug when trimming images in flush_messages

[Avinash2468]

Problem

Worker.flush_messages() trims old images from the LLM message history to keep at most max_trajectory_length images in the prompt. For long-context providers (anthropic, openai, gemini) it walks each message's content list and deletes image entries once the running count exceeds max_images:

for i in range(len(agent.messages) - 1, -1, -1):
    for j in range(len(agent.messages[i]["content"])):
        if "image" in agent.messages[i]["content"][j].get("type", ""):
            img_count += 1
            if img_count > max_images:
                del agent.messages[i]["content"][j]   # shifts every later index

The outer loop iterates in reverse, but the inner loop does not. del at index j shifts every subsequent element down by one while the loop keeps walking indices computed from the original length. For a message whose content holds more than one image, that means:

1. images get skipped, so more than max_trajectory_length images can survive into the prompt, and
2. it can raise IndexError: list index out of range.

There is also a smaller correctness problem independent of the deletion bug: because the inner loop counts front-to-back, the images kept within a single message are the oldest ones, not the newest — the opposite of the intended "keep the latest k images" policy.

Scope — this is a latent bug

I want to be upfront that I found this by reading the code, not from a failing run.

As of this commit the bug is not reachable through the current call paths: every add_message call feeding the two agents flush_messages operates on (worker.py:153, worker.py:160, worker.py:305) passes a single obs["screenshot"], so each message holds exactly one image and no deletion ever shifts an unvisited index.

It is reachable the moment any caller passes multiple images for one message, which LMMAgent.add_message / add_message_to_role explicitly support — they accept a list and append one content entry per image (gui_agents/s3/core/mllm.py:211-225). So this is hardening a landmine rather than fixing a live crash, plus the newest-vs-oldest fix above.

To be clear about what this does not do: it does not fix #168. That report is a configuration mismatch — max_trajectory_length defaults to 8 (worker.py:30, agent_s.py:56) while the provider there caps at 3 images — and flush_messages behaves as designed in that scenario. Happy to look at that separately if it'd be useful.

Fix

Iterate the inner loop in reverse as well. Deleting index j then only affects indices > j, which have already been visited, so nothing is skipped and no out-of-range access occurs. Walking content back-to-front also counts the most-recently-appended images first, matching the intended policy.

One line in each of the two affected files:

• gui_agents/s3/agents/worker.py
• gui_agents/s2_5/agents/worker.py

Tests

Adds tests/test_flush_messages.py, exercising the real Worker.flush_messages on both s3 and s2_5 (constructed via __new__ so no network or model setup is needed). Coverage:

• multiple images within a single message
• images spread across messages
• the newest images are the ones kept
• non-image content entries are preserved

Reproduction, one message with 4 images and max_images=2:

before:  CRASHED: IndexError: list index out of range
after:   kept 2 images (expected 2)

6 of the 8 new cases fail on main; all 8 pass with the fix.

black --check gui_agents passes.

If you'd rather not carry tests for a code path nothing currently exercises, I'm happy to trim them down or drop them and keep just the two-line fix.

[Avinash2468]

Correcting my own PR description: I originally cited #168 as something this change might address. On a closer look that was wrong, so I've edited the description.

#168 is a config mismatch — max_trajectory_length defaults to 8 while the provider in that report caps at 3 images — and flush_messages behaves as designed there. This PR doesn't change that.

The index-shift bug here is real but currently latent: every caller feeding the generator and reflection agents passes a single screenshot per message, so no deletion shifts an unvisited index today. It becomes reachable as soon as anyone uses the multi-image list form that add_message already supports. The newest-vs-oldest selection fix within a message applies either way.

Flagging it here rather than quietly rewriting the description. Happy to trim scope if you'd prefer just the two-line fix without tests.