Python: fix(redis): prefer an injected history client over connection settings

python/packages/redis/README.md

@@ -72,8 +72,9 @@ with `SecretString`; use `rediss://` when your server requires TLS.
 `RedisHistoryProvider` can create its own client from a URL or credential
 provider, or borrow a standalone `redis.asyncio.Redis` client configured with
 `decode_responses=True`. A borrowed client lets the application configure
-connection policy such as bounded timeouts and health checks. It remains
-caller-owned and must not be combined with `redis_url` or `credential_provider`.
+connection policy such as bounded timeouts and health checks. It takes
+precedence over `redis_url`, `credential_provider`, and the individual
+connection settings, which are ignored, and remains caller-owned.
 Retry behavior also remains caller-controlled; transcript appends are not
 idempotent if a client replays a write after an ambiguous connection loss.
 

python/packages/redis/agent_framework_redis/_history_provider.py

@@ -95,16 +95,21 @@ def __init__(
         Args:
             source_id: Unique identifier for this provider instance.
             redis_url: Redis connection URL (e.g., "redis://localhost:6379").
-                Mutually exclusive with credential_provider and redis_client.
+                Ignored when redis_client is supplied. Mutually exclusive with credential_provider.
             credential_provider: Redis credential provider for Azure AD authentication.
-                Requires host parameter. Mutually exclusive with redis_url and redis_client.
-            host: Redis host name. Required when using credential_provider.
-            port: Redis port number. Defaults to 6380 (Azure Redis SSL port).
-            ssl: Enable SSL/TLS connection. Defaults to True.
-            username: Redis username.
+                Requires host parameter. Ignored when redis_client is supplied.
+                Mutually exclusive with redis_url.
+            host: Redis host name. Required when credential_provider is used to build a client.
+            port: Redis port number. Defaults to 6380 (Azure Redis SSL port). Used only when
+                the provider builds a client.
+            ssl: Enable SSL/TLS connection. Defaults to True. Used only when the provider
+                builds a client.
+            username: Redis username. Used only when the provider builds a client.
             redis_client: Borrowed standalone async Redis client configured with
-                ``decode_responses=True``. The caller owns its lifetime. Mutually exclusive
-                with redis_url and credential_provider.
+                ``decode_responses=True``. The caller owns its lifetime and ``aclose`` leaves
+                it open. It takes precedence over redis_url, credential_provider and the
+                individual connection settings, which are ignored rather than treated as a
+                conflict, matching the Redis vector stores.
             key_prefix: Base prefix for Redis keys. Scoped mode appends independently encoded
                 tenant, application, agent, provider source, and session segments.
                 Defaults to 'chat_messages'.
@@ -130,7 +135,7 @@ def __init__(
         Raises:
             TypeError: If redis_client is not a standalone async Redis client.
             ValueError: If no Redis connection source is provided.
-            ValueError: If more than one Redis connection source is provided.
+            ValueError: If both redis_url and credential_provider are provided.
             ValueError: If credential_provider is used without host parameter.
             ValueError: If redis_client does not use decoded string responses.
             ValueError: If max_messages is negative.
@@ -145,19 +150,26 @@ def __init__(
             store_context_from=store_context_from,
         )
 
-        connection_sources = sum(source is not None for source in (redis_client, redis_url, credential_provider))
-        if connection_sources == 0:
-            raise ValueError("Either redis_client, redis_url, or credential_provider must be provided")
-        if connection_sources > 1:
-            raise ValueError("redis_client, 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 redis_client is not None:
+            # A caller-owned client is the connection. redis_url, credential_provider and the
+            # individual connection settings describe how to build one, so supplying them
+            # alongside it is redundant rather than conflicting, and none of them is
+            # validated. Only what the provider needs of the client itself still has to
+            # hold; its lifetime stays with the caller. This is the precedence the Redis
+            # vector stores already apply in _create_client.
             if not isinstance(redis_client, redis.Redis):
                 raise TypeError("Redis history providers require a standalone redis.asyncio.Redis client")
             connection_kwargs = cast(dict[str, Any], redis_client.connection_pool.connection_kwargs)  # pyright: ignore[reportUnknownMemberType]
             if not connection_kwargs.get("decode_responses", False):
                 raise ValueError("Redis history providers require decode_responses=True")
+        else:
+            connection_sources = sum(source is not None for source in (redis_url, credential_provider))
+            if connection_sources == 0:
+                raise ValueError("Either redis_client, redis_url, or credential_provider must be provided")
+            if connection_sources > 1:
+                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")
         if key_format not in ("scoped", "legacy"):
@@ -185,7 +197,9 @@ def __init__(
         self.agent_id = agent_id
         self.key_format = key_format
         self.max_messages = max_messages
-        self.redis_url = redis_url
+        # Recorded as the source the provider actually used: a URL passed alongside an
+        # injected client is ignored, so reporting it would claim a connection never made.
+        self.redis_url = redis_url if redis_client is None else None
         self._owns_client = redis_client is None
 
         if redis_client is not None:

python/packages/redis/tests/test_providers.py

@@ -416,21 +416,70 @@ def test_borrowed_client_is_used_directly(self):
     @pytest.mark.parametrize(
         "connection_kwargs",
         [
-            {"redis_url": "redis://localhost:6379"},
-            {"credential_provider": MagicMock(), "host": "myhost"},
+            {"redis_url": "redis://ignored:6379"},
+            {"credential_provider": MagicMock()},
+            {
+                "credential_provider": MagicMock(),
+                "host": "ignored",
+                "port": 1234,
+                "ssl": False,
+                "username": "ignored",
+            },
+            {
+                "redis_url": "redis://ignored:6379",
+                "credential_provider": MagicMock(),
+            },
         ],
     )
-    def test_borrowed_client_rejects_other_connection_sources(self, connection_kwargs: dict[str, Any]):
+    def test_borrowed_client_takes_precedence_over_other_connection_sources(self, connection_kwargs: dict[str, Any]):
+        # A caller-owned client is the connection. URL, credential provider and the
+        # individual connection settings describe how to build one, so supplying
+        # them alongside it is redundant rather than conflicting, and none of them
+        # is validated. This matches what the Redis vector stores already do.
         borrowed = Redis(decode_responses=True)
-
-        with pytest.raises(ValueError, match="mutually exclusive"):
-            RedisHistoryProvider(
+        with patch("agent_framework_redis._history_provider.redis.from_url") as mock_from_url:
+            provider = RedisHistoryProvider(
                 "mem",
                 redis_client=borrowed,
                 application_id="test-app",
                 **connection_kwargs,
             )
 
+        assert provider._redis_client is borrowed
+        assert provider._owns_client is False
+        assert provider.redis_url is None
+        mock_from_url.assert_not_called()
+
+    def test_borrowed_client_still_passes_compatibility_checks_with_redundant_settings(self):
+        # The settings are ignored; the client itself is not.
+        with pytest.raises(ValueError, match="decode_responses=True"):
+            RedisHistoryProvider(
+                "mem",
+                redis_client=Redis(decode_responses=False),
+                redis_url="redis://ignored:6379",
+                application_id="test-app",
+            )
+
+    def test_borrowed_client_keeps_provider_scoping_and_retention(self):
+        # Connection precedence does not reach provider-level storage settings.
+        borrowed = Redis(decode_responses=True)
+        provider = RedisHistoryProvider(
+            "mem",
+            redis_client=borrowed,
+            redis_url="redis://ignored:6379",
+            application_id="app",
+            tenant_id="tenant",
+            agent_id="agent",
+            max_messages=7,
+            key_prefix="hist",
+        )
+
+        assert provider.max_messages == 7
+        assert provider.key_prefix == "hist"
+        key = provider._redis_key("session")
+        assert "|v2|" in key
+        assert "tenant" in key and "agent" in key
+
     def test_borrowed_client_must_be_async_redis(self):
         with pytest.raises(TypeError, match=r"redis\.asyncio\.Redis"):
             RedisHistoryProvider(