From 649d9409c0e555635787c80d49c70cb24baa56a0 Mon Sep 17 00:00:00 2001 From: jariy17 Date: Thu, 9 Jul 2026 20:15:20 +0000 Subject: [PATCH] fix: floor monotonic timestamps to milliseconds before comparison Comparing at microsecond precision missed collisions: two events in the same millisecond but different microseconds passed the tie check yet collide once the service floors both to the same millisecond (AgentCore Memory stores and orders eventTimestamp at ms resolution). Floor the desired timestamp to ms first so same-ms events are detected as a tie and separated by 1ms. --- .../integrations/strands/session_manager.py | 6 ++++ .../test_agentcore_memory_session_manager.py | 36 +++++++++++++++++-- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/bedrock_agentcore/memory/integrations/strands/session_manager.py b/src/bedrock_agentcore/memory/integrations/strands/session_manager.py index 4677a131..f930b548 100644 --- a/src/bedrock_agentcore/memory/integrations/strands/session_manager.py +++ b/src/bedrock_agentcore/memory/integrations/strands/session_manager.py @@ -113,6 +113,12 @@ def _get_monotonic_timestamp(self, desired_timestamp: Optional[datetime] = None) if desired_timestamp is None: desired_timestamp = datetime.now(timezone.utc) + # Floor to milliseconds — the resolution AgentCore Memory actually stores + # and orders eventTimestamp at. Comparing at microsecond precision would + # miss two events that fall in the same millisecond but different + # microseconds: they pass the tie check here yet collide once stored. + desired_timestamp = desired_timestamp.replace(microsecond=(desired_timestamp.microsecond // 1000) * 1000) + with self._timestamp_lock: if self._last_timestamp is not None and desired_timestamp <= self._last_timestamp: # Break the tie at millisecond granularity (the service's resolution). diff --git a/tests/bedrock_agentcore/memory/integrations/strands/test_agentcore_memory_session_manager.py b/tests/bedrock_agentcore/memory/integrations/strands/test_agentcore_memory_session_manager.py index 50a04217..fa0c4787 100644 --- a/tests/bedrock_agentcore/memory/integrations/strands/test_agentcore_memory_session_manager.py +++ b/tests/bedrock_agentcore/memory/integrations/strands/test_agentcore_memory_session_manager.py @@ -3870,11 +3870,19 @@ def test_later_timestamp_is_not_bumped(self, session_manager): assert session_manager._get_monotonic_timestamp(later) == later def test_none_desired_uses_current_time(self, session_manager): - """Passing None falls back to current UTC time (preserved behavior).""" + """Passing None falls back to current UTC time (preserved behavior). + + The result is floored to ms, so it can sit up to 999us below ``before``; + compare against the ms-floored bounds. + """ + + def floor_ms(dt): + return dt.replace(microsecond=(dt.microsecond // 1000) * 1000) + before = datetime.now(timezone.utc) result = session_manager._get_monotonic_timestamp(None) after = datetime.now(timezone.utc) - assert before <= result <= after + assert floor_ms(before) <= result <= floor_ms(after) def test_within_process_burst_stays_ordered_at_ms_resolution(self, session_manager): """A burst of same-instant events gets strictly increasing 1ms-spaced @@ -3887,3 +3895,27 @@ def test_within_process_burst_stays_ordered_at_ms_resolution(self, session_manag assert len(set(stamps)) == len(stamps) # no ties (ambiguous ordering) # Whole burst stays within a few ms of the requested time, not seconds. assert stamps[-1] - base == timedelta(milliseconds=4) + + def test_returned_timestamps_are_floored_to_milliseconds(self, session_manager): + """Timestamps are floored to ms — the sub-millisecond microseconds the + service would discard are dropped before comparison/storage.""" + ts = datetime(2024, 1, 1, 12, 0, 0, 567890, tzinfo=timezone.utc) # 567.890 ms + result = session_manager._get_monotonic_timestamp(ts) + assert result == ts.replace(microsecond=567000) + assert result.microsecond % 1000 == 0 + + def test_same_millisecond_different_microseconds_is_a_tie(self, session_manager): + """Two events in the same ms but different microseconds must be treated + as a collision and separated by 1ms — otherwise they collide once the + service floors both to the same millisecond.""" + first = datetime(2024, 1, 1, 12, 0, 0, 500, tzinfo=timezone.utc) # 0.500 ms + second = datetime(2024, 1, 1, 12, 0, 0, 900, tzinfo=timezone.utc) # 0.900 ms + + r1 = session_manager._get_monotonic_timestamp(first) + r2 = session_manager._get_monotonic_timestamp(second) + + # Both floored to .000; the second is bumped to .001 rather than passing + # through as a false non-tie. + assert r1 == datetime(2024, 1, 1, 12, 0, 0, 0, tzinfo=timezone.utc) + assert r2 == datetime(2024, 1, 1, 12, 0, 0, 1000, tzinfo=timezone.utc) + assert r2 > r1