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

openhands-agent-server/openhands/agent_server/bash_service.py

@@ -149,7 +149,6 @@ async def search_bash_events(
             files = [file for file in files if file.name < timestamp_lt_str]
 
         # Handle pagination
-        page_files = []
         start_index = 0
 
         # Find the starting point if page_id is provided
@@ -159,27 +158,29 @@ async def search_bash_events(
                     start_index = i
                     break
 
-        # Collect items for this page
+        # Collect items for this page. Filtering happens inside this loop so the
+        # boundary is measured in returned events rather than files read: sizing
+        # the page by file count first lets `order__gt` empty a page while
+        # `next_page_id` is still set, walking the client through under-full or
+        # entirely empty pages.
+        page_events = []
         next_page_id = None
         for i in range(start_index, len(files)):
-            if len(page_files) >= limit:
+            if len(page_events) >= limit:
                 # We have collected enough items for this page
                 # Set next_page_id to the current file for next page
                 next_page_id = str(files[i].name)
                 break
-            page_files.append(files[i])
 
-        # Load only the page files (not all files)
-        page_events = []
-        for file_path in page_files:
-            event = self._load_event_from_file(file_path)
-            if event is not None:
-                # Filter by order if specified (only applies to BashOutput events)
-                if order__gt is not None:
-                    event_order = getattr(event, "order", None)
-                    if event_order is not None and event_order <= order__gt:
-                        continue
-                page_events.append(event)
+            event = self._load_event_from_file(files[i])
+            if event is None:
+                continue
+            # Filter by order if specified (only applies to BashOutput events)
+            if order__gt is not None:
+                event_order = getattr(event, "order", None)
+                if event_order is not None and event_order <= order__gt:
+                    continue
+            page_events.append(event)
 
         return BashEventPage(items=page_events, next_page_id=next_page_id)
 

tests/agent_server/test_bash_service.py

@@ -5,10 +5,10 @@
 import logging
 import time
 from collections.abc import AsyncIterator
-from datetime import UTC, datetime
+from datetime import UTC, datetime, timedelta
 from pathlib import Path
 from unittest.mock import AsyncMock, patch
-from uuid import UUID
+from uuid import UUID, uuid4
 
 import httpx
 import pytest
@@ -18,7 +18,7 @@
 from openhands.agent_server import bash_router as bash_router_module
 from openhands.agent_server.bash_service import BashEventService
 from openhands.agent_server.config import Config
-from openhands.agent_server.models import BashCommand
+from openhands.agent_server.models import BashCommand, BashOutput
 from openhands.agent_server.server_details_router import (
     mark_initialization_complete,
     server_details_router,
@@ -195,3 +195,62 @@ async def test_run_retention_cleanup_loop_purges_old_events(tmp_path: Path):
     assert len(service._get_event_files_by_pattern("*")) == 0, (
         "Old event file should have been purged by the retention loop"
     )
+
+
+# ---------------------------------------------------------------------------
+# search_bash_events pagination with order__gt
+# ---------------------------------------------------------------------------
+
+
+async def test_order_filter_does_not_undersize_pages(tmp_path: Path):
+    """The page boundary must count returned events, not files read (#4388).
+
+    Filtering after the page was already sized by file count let `order__gt`
+    hand back a short (or empty) page while `next_page_id` was still set, so a
+    client following the cursor walked a run of near-empty pages.
+    """
+    service = BashEventService(bash_events_dir=tmp_path / "bash_events")
+    command_id = uuid4()
+
+    # 10 events the filter rejects, then 5 it accepts.
+    for order in range(15):
+        service._save_event_to_file(
+            BashOutput(
+                command_id=command_id,
+                order=order,
+                stdout=f"chunk {order}",
+                timestamp=_OLD + timedelta(seconds=order),
+            )
+        )
+
+    page = await service.search_bash_events(order__gt=9, limit=5, kind__eq="BashOutput")
+
+    # `items` is typed as the event base class, so narrow before reading `order`.
+    orders = sorted(e.order for e in page.items if isinstance(e, BashOutput))
+    assert orders == [10, 11, 12, 13, 14]
+    assert page.next_page_id is None
+
+
+async def test_order_filter_never_returns_an_empty_page_with_a_cursor(tmp_path: Path):
+    """An all-filtered range must terminate instead of handing back a cursor."""
+    service = BashEventService(bash_events_dir=tmp_path / "bash_events")
+    command_id = uuid4()
+
+    for order in range(10):
+        service._save_event_to_file(
+            BashOutput(
+                command_id=command_id,
+                order=order,
+                stdout=f"chunk {order}",
+                timestamp=_OLD + timedelta(seconds=order),
+            )
+        )
+
+    page = await service.search_bash_events(
+        order__gt=100, limit=5, kind__eq="BashOutput"
+    )
+
+    assert page.items == []
+    assert page.next_page_id is None, (
+        "an empty page must not advertise a next page, or clients loop forever"
+    )