feat(websocket): add after_timestamp filter for bi-directional event loading

[jpshackelford]

Summary

Add an after_timestamp query parameter to the WebSocket events endpoint that filters events during resend_all to only return events with timestamps >= the specified value.

Background

This is a prerequisite for the bi-directional event loading optimization described in OpenHands/OpenHands#12705.

Changes

• Added after_timestamp: datetime | None query parameter to /sockets/events/{conversation_id} WebSocket endpoint
• When resend_all=True and after_timestamp is provided, only events with timestamp >= after_timestamp are resent
• Added comprehensive tests for the new parameter

Use Case

This enables efficient bi-directional loading where:

1. REST API fetches historical events (paginated backwards from newest)
2. WebSocket handles real-time events after a specific timestamp

Timeline:
[oldest] ←←←← REST (backwards) ←←←← [newest] →→→→ WebSocket (forward) →→→→ [new events]

The underlying search_events already supports timestamp__gte filtering, so this change simply exposes that capability through the WebSocket API.

Testing

• Added 3 new tests in TestAfterTimestampFiltering class
• Updated existing test_resend_all_true_resends_events to verify the new parameter passing

Related Issue

Resolves: OpenHands/OpenHands#12705 (agent-server prerequisite)

---

Agent Server images for this PR

• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server

Variants & Base Images

Variant	Architectures	Base Image	Docs / Tags
java	amd64, arm64	eclipse-temurin:17-jdk	Link
python	amd64, arm64	nikolaik/python-nodejs:python3.13-nodejs22	Link
golang	amd64, arm64	golang:1.21-bookworm	Link

Pull (multi-arch manifest)

# Each variant is a multi-arch manifest supporting both amd64 and arm64
docker pull ghcr.io/openhands/agent-server:bce2fa7-python

Run

docker run -it --rm \
  -p 8000:8000 \
  --name agent-server-bce2fa7-python \
  ghcr.io/openhands/agent-server:bce2fa7-python

All tags pushed for this build

ghcr.io/openhands/agent-server:bce2fa7-golang-amd64
ghcr.io/openhands/agent-server:bce2fa7-golang_tag_1.21-bookworm-amd64
ghcr.io/openhands/agent-server:bce2fa7-golang-arm64
ghcr.io/openhands/agent-server:bce2fa7-golang_tag_1.21-bookworm-arm64
ghcr.io/openhands/agent-server:bce2fa7-java-amd64
ghcr.io/openhands/agent-server:bce2fa7-eclipse-temurin_tag_17-jdk-amd64
ghcr.io/openhands/agent-server:bce2fa7-java-arm64
ghcr.io/openhands/agent-server:bce2fa7-eclipse-temurin_tag_17-jdk-arm64
ghcr.io/openhands/agent-server:bce2fa7-python-amd64
ghcr.io/openhands/agent-server:bce2fa7-nikolaik_s_python-nodejs_tag_python3.13-nodejs22-amd64
ghcr.io/openhands/agent-server:bce2fa7-python-arm64
ghcr.io/openhands/agent-server:bce2fa7-nikolaik_s_python-nodejs_tag_python3.13-nodejs22-arm64
ghcr.io/openhands/agent-server:bce2fa7-golang
ghcr.io/openhands/agent-server:bce2fa7-java
ghcr.io/openhands/agent-server:bce2fa7-python

About Multi-Architecture Support

• Each variant tag (e.g., bce2fa7-python) is a multi-arch manifest supporting both amd64 and arm64
• Docker automatically pulls the correct architecture for your platform
• Individual architecture tags (e.g., bce2fa7-python-amd64) are also available if needed

[all-hands-bot]

Overall solid implementation! The feature is well-tested and maintains backward compatibility. Main concern is timezone handling - the parameter should enforce or document timezone-aware datetimes to avoid potential bugs.

[github-actions]

Coverage Report •

File	Stmts	Miss	Cover	Missing
openhands-agent-server/openhands/agent_server
event_router.py	47	8	82%	172–175, 185–186, 208–209
sockets.py	131	22	83%	145, 150–152, 261–262, 267–269, 275–277, 282–283, 285–286, 311–315, 325
TOTAL	20711	9288	55%

[enyst]

@OpenHands Do a /codereview-roasted on this PR.

When you’re done, look at the linked issue and tell WDYT.

Post your feedback directly as a comment on github here. Use smolpaws. Note that it will be rendered as markdown.

[openhands-ai]

I'm on it! enyst can track my progress at all-hands.dev

[smolpaws]

Taste Rating: 🟡 Acceptable — small change, but there’s a correctness landmine hiding in the timestamp handling.

---

Linus-Style Analysis

[CRITICAL ISSUES]

• [openhands-agent-server/openhands/agent_server/event_service.py, Lines 110–141; openhands-agent-server/openhands/agent_server/sockets.py, Lines 81–89] Correctness / Data semantics: after_timestamp is a datetime, but events store timestamp as string and comparisons are lexicographic. That “works” only if all timestamps are normalized to the same ISO format and timezone handling is consistent. Right now events default to datetime.now().isoformat() (naive), while after_timestamp can be tz-aware, which means "2024-01-15T10:30:00+00:00" vs "2024-01-15T10:30:00" lexicographic order is not reliable. That can silently drop or duplicate events at the boundary. Fix: normalize to UTC (or strip tzinfo) before comparison, or parse event.timestamp to datetime and compare actual datetimes. At minimum, enforce/validate timezone format in the WebSocket param and document it.

[IMPROVEMENT OPPORTUNITIES]

• [openhands-agent-server/openhands/agent_server/sockets.py, Lines 47–57] API contract clarity: the new param is only honored when resend_all=True, but the API silently ignores it otherwise. Consider returning a 400 when after_timestamp is set without resend_all, or explicitly document in the OpenAPI/README for the socket endpoint. Silent no-ops are a debugging tax.
• [tests/agent_server/test_event_router_websocket.py, Lines 452–588] Coverage gap: tests only assert that the param is passed through. You’re not asserting the boundary behavior (event.timestamp == after_timestamp). Add one test case to prove inclusive filtering doesn’t cause duplicates or misses — especially given the string-compare behavior.

[STYLE NOTES]

• [tests/agent_server/test_event_router_websocket.py, Lines 459–546] Minor: repeated local imports (datetime, cast) inside tests make the file noisy; pull them to the top-level unless there’s a cycle.

---

VERDICT: ✅ Worth merging once timestamp normalization is addressed. Otherwise you’re adding a correctness bug under load.

KEY INSIGHT: If timestamps aren’t normalized, the filter is gambling on string ordering — that’s not a protocol, it’s a coincidence.

---

WDYT on OpenHands/OpenHands#12705

The bi-directional loading plan is solid and pragmatic — REST for bulk history + WebSocket for tail is the right shape. Two gotchas to keep in mind:

1. Boundary duplication: using >= on a timestamp boundary will duplicate if multiple events share the same timestamp or the REST page ends on that exact timestamp. Either make the websocket filter strictly greater, or include (timestamp, id) ordering and dedupe client-side.
2. Timestamp consistency: the backend currently treats timestamps as strings; if the frontend starts mixing timezones or formats, you’ll get ordering bugs. Standardize on UTC ISO strings (or switch to numeric epoch) end-to-end.

Overall: good direction, just nail the timestamp semantics so you don’t ship a subtle “missing events” bug.

[jpshackelford]

Thanks for the detailed review! I've pushed fixes that address the timezone handling concerns. Let me explain the architecture and our decisions.

Background: How Event Timestamps Work

Event timestamps are defined in openhands-sdk (not openhands-agent-server):

# openhands-sdk/openhands/sdk/event/base.py
timestamp: str = Field(
    default_factory=lambda: datetime.now().isoformat(),
)  # consistent with V1

Key points:

• Stored as string (ISO format), not datetime object
• Uses naive local time (datetime.now() without tzinfo)
• Marked "consistent with V1" - backward compatibility constraint

The agent-server's filtering code must work with this existing format.

The Problem

The original implementation used string comparison for timestamp filtering:

if event.timestamp < timestamp_gte_str:  # string comparison

This breaks when ISO strings have different timezone suffixes:

• "2024-01-15T10:30:00" (naive)
• "2024-01-15T10:30:00+00:00" (tz-aware)

These represent potentially different moments but don't compare correctly as strings.

Our Solution

We had two options to enable proper datetime comparison:

Option	Approach	Tradeoff
A	Normalize filter to naive	One-line change in normalize_datetime_to_server_timezone
B	Add tzinfo when parsing event timestamps	Changes in event_service.py for every comparison

We chose Option A because:

1. Simpler - change is localized to the normalization function
2. Matches the source of truth - events are stored naive, so filters should be naive
3. Both REST and WebSocket already call normalize_datetime_to_server_timezone

Changes Made

1. WebSocket endpoint: Added normalize_datetime_to_server_timezone() (matching REST)

2. normalize_datetime_to_server_timezone(): Now returns naive datetime

   # Converts to server local time AND strips tzinfo
   return dt.astimezone(None).replace(tzinfo=None)

3. event_service.py: Replaced string comparison with datetime comparison

   event_dt = datetime.fromisoformat(event.timestamp)
   if event_dt < timestamp__gte:

API Contract

Client provides	Behavior
Naive datetime	Assumed to be server local time, used as-is
Tz-aware datetime	Converted to server local time, made naive

This handles timezones correctly - a client can pass UTC and it will be properly converted to server time before comparison.

No Breaking Changes

Verified that OpenHands/OpenHands frontend doesn't use after_timestamp yet - it only uses resend_all: true. This is a new feature, so clients will adopt the documented behavior from the start.

[openhands-ai]

Looks like there are a few issues preventing this PR from being merged!

• GitHub Actions are failing:
  • Run tests

If you'd like me to help, just leave a comment, like

@OpenHands please fix the failing actions on PR #1880 at branch `feature/websocket-after-timestamp-filter`

Feel free to include any additional details that might help me get this PR into a better state.

_{^{You can manage your notification settings}}

[all-hands-bot]

[Automatic Post]: It has been a while since there was any activity on this PR. @jpshackelford, are you still working on it? If so, please go ahead, if not then please request review, close it, or request that someone else follow up.

[neubig]

[automated message] @neubig assigned for review according to git blame

[all-hands-bot]

[Automatic Post]: This PR seems to be currently waiting for review. @neubig, could you please take a look when you have a chance?

[all-hands-bot]

[Automatic Post]: It has been a while since there was any activity on this PR. @jpshackelford, are you still working on it? If so, please go ahead, if not then please request review, close it, or request that someone else follow up.

[all-hands-bot]

[Automatic Post]: It has been a while since there was any activity on this PR. @jpshackelford, are you still working on it? If so, please go ahead, if not then please request review, close it, or request that someone else follow up.

[all-hands-bot]

[Automatic Post]: It has been a while since there was any activity on this PR. @jpshackelford, are you still working on it? If so, please go ahead, if not then please request review, close it, or request that someone else follow up.

[github-actions]

Python API breakage checks — ✅ PASSED

Result: ✅ PASSED

Action log

[github-actions]

REST API breakage checks (OpenAPI) — ✅ PASSED

Result: ✅ PASSED

Action log

[jpshackelford]

Summary: API Breakage Checks

✅ API breakage checks (Griffe) - PASSED

• Checks Python SDK's programmatic API for breaking changes
• No breaking changes detected in openhands-sdk, openhands-workspace, or openhands-tools
• This PR doesn't introduce any Python API breaking changes

⚠️ Agent server REST API breakage checks (OpenAPI) - Failed (but non-blocking)

What happened:

Breaking REST API change detected without MINOR version bump (1.12.0 -> 1.12.0).
- the 'file' request property type/format changed from 'string'/'' to 'string'/'binary'

Key findings:

1. This failure is NOT caused by this PR - I verified by running the same check on origin/main and it produces the identical error
2. The file property change (from string/'' to string/'binary') is a pre-existing breaking change in main that hasn't been addressed
3. The check has continue-on-error: true for non-release PRs (!startsWith(github.head_ref, 'rel-')) so it won't block this PR from merging

Why it doesn't block:

• The workflow is configured to report failures via PR comment but not gate merges for feature branches
• Only release branches (rel-*) are blocked by this check

Action needed:

• None for this PR - it's a pre-existing issue in main
• The repo maintainers may want to address this separately by either:
  1. Bumping the MINOR version to acknowledge the breaking change, or
  2. Investigating why the file property type/format changed

[jpshackelford]

@neubig Ready for final review

[neubig]

@jpshackelford , thanks! When you want to re-request my review, could you press the "cycle" button next to my name under reviewers? That'll put it back on my review queue, and I'm better at checking that than my github messages (which are often a bit noisy). I'll click it for myself for now.

[jpshackelford]

Implemented: resend_mode API with resend_all deprecation

Following the discussion with @enyst, I've implemented the cleaner API design:

New API

resend_mode: Literal['all', 'since'] | None
after_timestamp: datetime | None

Usage:

• ?resend_mode=all → sends all events (replaces resend_all=true)
• ?resend_mode=since&after_timestamp=2024-01-15T10:30:00Z → sends events since timestamp
• No params (default) → just subscribe, no historical events

Backward Compatibility

resend_all is deprecated but still functional:

• Hidden from OpenAPI schema (include_in_schema=False)
• Logs a deprecation warning when used
• ?resend_all=true continues to work (treated as resend_mode='all')

Migration Path

Phase	API Surface	Breaking?
Now	resend_mode, after_timestamp, resend_all (hidden)	No
Deprecation period	Warn in logs when resend_all used	No
Future major release	Remove resend_all	Yes
Final	resend_mode, after_timestamp	-

Frontend Migration (when ready)

- { resend_all: true }
+ { resend_mode: 'all' }

Commit: 2d7f74b

[all-hands-bot]

Taste Rating: 🟢 Good taste

Clean implementation solving a real problem (bi-directional event loading). Key strengths:

• Data structure handling: Correctly normalizes timezone-aware datetimes to naive server local time for comparison with stored events
• Simplicity: No excessive nesting, clear branching logic for resend modes
• Backward compatibility: Deprecated resend_all gracefully with warnings, new resend_mode API is cleaner
• Testing: Comprehensive coverage testing real behavior (not mocks) - timezone normalization, all modes, backward compat

No critical or important issues found. The approach is pragmatic and maintainable.

VERDICT: ✅ Worth merging - solid engineering, well-tested, maintains backward compatibility.

[all-hands-bot]

[Automatic Post]: This PR seems to be currently waiting for review. @neubig @enyst, could you please take a look when you have a chance?

[enyst]

Thank you for this! Let's take it in.