fix(agent-server): size bash event pages by returned events, not files read

[Shailendra005]

HUMAN:

Took this one because a cursor that keeps pointing at nothing is worse than an error — the client can't tell the difference between "slow" and "broken".

---

AGENT:

Why

search_bash_events picks up to limit files, then applies order__gt while loading them. The page boundary is therefore measured before the filter runs, so a page can come back short — or completely empty — while next_page_id is still set. A client following the cursor walks a run of under-full pages, and in the all-filtered case never makes progress.

order__gt is how incremental command-output polling works ("give me output after chunk N"), so this is a live path rather than a corner case. Reported in #4388.

Summary

• Merged the filter into the page-collection loop so the boundary counts events that will actually be returned.
• An unreadable file (_load_event_from_file returning None) no longer consumes a page slot either — previously it silently shrank the page the same way.
• next_page_id semantics are unchanged: it points at the first file not consumed by this page.

Issue Number

Closes #4388

How to Test

Two tests in tests/agent_server/test_bash_service.py:

• test_order_filter_does_not_undersize_pages — 15 outputs, order__gt=9, limit=5. Expects a full page of orders 10–14.
• test_order_filter_never_returns_an_empty_page_with_a_cursor — every event filtered out. Expects no items and no cursor.

$ OPENHANDS_SUPPRESS_BANNER=1 uv run pytest tests/agent_server/test_bash_service.py -k order_filter -q
2 passed

Both fail on main with the reported behaviour:

assert [] == [10, 11, 12, 13, 14]
AssertionError: an empty page must not advertise a next page, or clients loop forever
  assert '20200101000005000000_BashOutput_...' is None

The tests give each event an increasing timestamp, since files sort by name and same-timestamp events would otherwise come back in a nondeterministic order.

Full suite and checks:

$ OPENHANDS_SUPPRESS_BANNER=1 uv run pytest tests/agent_server -q
1918 passed, 13 deselected

$ uv run ruff check <changed files>      # All checks passed!
$ uv run ruff format --check <changed>   # unchanged
$ uv run pyright openhands-agent-server/openhands/agent_server/bash_service.py
0 errors, 0 warnings, 0 informations

Note

A full page still carries a next_page_id even when every remaining file would be filtered out — the service cannot know that without reading them. The following request then returns an empty page with no cursor, so the client terminates after one extra round trip rather than looping. Making that last hop disappear would mean reading ahead past the page boundary, which seemed a worse trade than one predictable extra call; happy to revisit if you would rather pay that cost.

[neubig]

Minor consistency note (non-blocking for me): the two new tests use asyncio.run(service.search_bash_events(...)) while the rest of the file uses async def test_... + await under asyncio_mode = "auto" (see test_run_retention_cleanup_loop_purges_old_events just above). Converting them to the same async def style would keep the async-usage consistent across the file. Happy either way — flagging since inconsistency in async handling is the kind of thing that tends to spread.

[Shailendra005]

@neubig good call — converted in the latest commit. Both tests are now async def + await like the rest of the file, so the asyncio.run(...) outlier is gone.

Their pyright hook then flagged a second thing worth keeping: page.items is typed as list[BashEventBase], which has no order, so the assertion now narrows with isinstance(e, BashOutput) before reading it.