Python: fix(redis): honour a max_messages retention limit of zero (#7470)

* Python: fix(redis): honour a max_messages retention limit of zero

RedisHistoryProvider documents None as the sentinel for unlimited storage,
so max_messages=0 must retain nothing. It retained everything: trimming to
-max_messages emits LTRIM key 0 -1, which is Redis's "keep the whole list",
and the count > max_messages guard is true for any non-empty list, so the
trim ran on every save and did nothing.

Negative values were worse than a no-op. max_messages=-5 emitted
LTRIM key 5 -1, deleting the five oldest messages on every save while the
list still grew without bound.

Handle a limit of zero by deleting the key, which is what clear() in this
class already does, and reject negative values in __init__ alongside the
three ValueErrors it already raises for invalid configuration. None and
positive limits are unchanged.

* Python: never write the payload when Redis retention is disabled

Addresses the automated review on #7470. With max_messages=0 the previous
change still RPUSHed every message and deleted the key afterwards, so the
payload reached Redis - and any AOF or replica stream - before being removed,
and was briefly visible to other readers. Short-circuit instead: drop any
existing history and return before serializing, so nothing is written at all.

Also documents the new ValueError in the Raises: section, and asserts in the
test that the pipeline is never used.

* Python: leave stored history alone when Redis retention is disabled

max_messages=0 deleted the session key. _redis_key omits source_id, so
two providers with the default prefix share {key_prefix}:{session_id},
and the after-run pass persists in reverse provider order - a
zero-retention provider listed first would drop a co-located provider's
just-written history on every turn.

Return before serializing instead: no payload reaches Redis, an AOF or a
replica, and stored history is left as it is. Removing stored history is
what clear() is for.

---------

Co-authored-by: Chinmay V <203952148+chinmayv095@users.noreply.github.com>

python/packages/redis/agent_framework_redis/_history_provider.py

@@ -62,7 +62,9 @@ class RedisHistoryProvider(HistoryProvider):
             key_prefix: Prefix for Redis keys. Defaults to 'chat_messages'.
             max_messages: Maximum number of messages to retain per session.
                 When exceeded, oldest messages are automatically trimmed.
-                None means unlimited storage.
+                None means unlimited storage; 0 retains nothing, and no message
+                payload is written to Redis at all. Stored history is left as it
+                is - use ``clear`` to remove it.
             load_messages: Whether to load messages before invocation.
             store_outputs: Whether to store response messages.
             store_inputs: Whether to store input messages.
@@ -73,6 +75,7 @@ class RedisHistoryProvider(HistoryProvider):
             ValueError: If neither redis_url nor credential_provider is provided.
             ValueError: If both redis_url and credential_provider are provided.
             ValueError: If credential_provider is used without host parameter.
+            ValueError: If max_messages is negative.
         """
         super().__init__(
             source_id,
@@ -89,6 +92,8 @@ class RedisHistoryProvider(HistoryProvider):
             raise ValueError("redis_url and credential_provider are mutually exclusive")
         if credential_provider is not None and host is None:
             raise ValueError("host is required when using credential_provider")
+        if max_messages is not None and max_messages < 0:
+            raise ValueError("max_messages must be None (unlimited) or a non-negative integer")
 
         self.key_prefix = key_prefix
         self.max_messages = max_messages
@@ -156,6 +161,14 @@ class RedisHistoryProvider(HistoryProvider):
         if not messages:
             return
 
+        if self.max_messages == 0:
+            # Retention is disabled. Trimming cannot express this - LTRIM key 0 -1 keeps
+            # the whole list - so return before serializing: no payload reaches Redis, an
+            # AOF or a replica. Stored history is deliberately left alone. _redis_key omits
+            # source_id, so the list can belong to a co-located provider, and removing
+            # stored history is what clear() is for.
+            return
+
         key = self._redis_key(session_id)
         serialized_messages = [self._serialize_json(msg) for msg in messages]
 

python/packages/redis/tests/test_providers.py

@@ -397,6 +397,10 @@ class TestRedisHistoryProviderInit:
         with pytest.raises(ValueError, match="host is required"):
             RedisHistoryProvider("mem", credential_provider=mock_cred)
 
+    def test_negative_max_messages_raises(self):
+        with pytest.raises(ValueError, match="max_messages"):
+            RedisHistoryProvider("mem", redis_url="redis://localhost:6379", max_messages=-5)
+
     def test_credential_provider_with_host(self):
         mock_cred = MagicMock()
         with patch("agent_framework_redis._history_provider.redis.Redis") as mock_redis_cls:
@@ -495,6 +499,41 @@ class TestRedisHistoryProviderSaveMessages:
 
         mock_redis_client.ltrim.assert_not_called()
 
+    async def test_max_messages_zero_retains_nothing(self, mock_redis_client: MagicMock):
+        """Only None means unlimited, so a retention count of 0 must retain nothing.
+
+        ``LTRIM key 0 -1`` is Redis's "keep the whole list", so trimming to
+        ``-max_messages`` cannot express a limit of zero.
+        """
+        mock_redis_client.llen = AsyncMock(return_value=15)
+
+        with patch("agent_framework_redis._history_provider.redis.from_url") as mock_from_url:
+            mock_from_url.return_value = mock_redis_client
+            provider = RedisHistoryProvider("mem", redis_url="redis://localhost:6379", max_messages=0)
+
+        await provider.save_messages("s1", [Message(role="user", contents=["msg"])])
+
+        # No payload reaches Redis at all, so nothing is exposed to readers, AOF or replicas.
+        mock_redis_client.pipeline.assert_not_called()
+        mock_redis_client.ltrim.assert_not_called()
+
+    async def test_max_messages_zero_leaves_stored_history_alone(self, mock_redis_client: MagicMock):
+        """Disabling retention must not delete history this provider does not own.
+
+        ``_redis_key`` omits ``source_id``, so two providers with the default prefix
+        share ``{key_prefix}:{session_id}``. Persisting runs in reverse provider order,
+        so a zero-retention provider that deleted the key would drop a co-located
+        provider's just-written history on every turn. Removing stored history is
+        ``clear()``'s job, not a retention setting's.
+        """
+        with patch("agent_framework_redis._history_provider.redis.from_url") as mock_from_url:
+            mock_from_url.return_value = mock_redis_client
+            provider = RedisHistoryProvider("mem", redis_url="redis://localhost:6379", max_messages=0)
+
+        await provider.save_messages("s1", [Message(role="user", contents=["msg"])])
+
+        mock_redis_client.delete.assert_not_called()
+
 
 class TestRedisHistoryProviderClear:
     async def test_clear_calls_delete(self, mock_redis_client: MagicMock):