From f0dc299eaa726f2e86941962c5f38686aea6166b Mon Sep 17 00:00:00 2001 From: Andrew Brookins Date: Fri, 12 Dec 2025 17:44:12 -0800 Subject: [PATCH 1/4] Add proper server-side query support for instances --- redis_sre_agent/api/instances.py | 57 ++++++- redis_sre_agent/core/instances.py | 209 +++++++++++++++++++++-- redis_sre_agent/mcp_server/server.py | 48 ++++-- tests/unit/api/test_instances_api.py | 68 ++++++-- tests/unit/mcp_server/test_mcp_server.py | 79 ++++++++- ui/src/pages/Dashboard.tsx | 2 +- ui/src/pages/Instances.tsx | 91 ++++++---- ui/src/pages/Schedules.tsx | 2 +- ui/src/pages/Triage.tsx | 4 +- ui/src/services/sreAgentApi.ts | 41 ++++- 10 files changed, 515 insertions(+), 86 deletions(-) diff --git a/redis_sre_agent/api/instances.py b/redis_sre_agent/api/instances.py index 6c125af..d9d489a 100644 --- a/redis_sre_agent/api/instances.py +++ b/redis_sre_agent/api/instances.py @@ -5,7 +5,7 @@ from typing import List, Optional from urllib.parse import urlparse -from fastapi import APIRouter, HTTPException +from fastapi import APIRouter, HTTPException, Query from pydantic import BaseModel, Field, SecretStr, field_serializer, field_validator from redis_sre_agent.core import instances as core_instances @@ -93,6 +93,15 @@ class RedisInstanceResponse(BaseModel): redis_cloud_database_name: Optional[str] = None +class InstanceListResponse(BaseModel): + """Response model for paginated instance list with filtering.""" + + instances: List[RedisInstanceResponse] + total: int = Field(..., description="Total number of instances matching the filter") + limit: int = Field(..., description="Maximum number of results returned") + offset: int = Field(..., description="Number of results skipped") + + class CreateInstanceRequest(BaseModel): """Request model for creating a Redis instance.""" @@ -268,12 +277,48 @@ def dump_secret(self, v): connections: Optional[int] = None -@router.get("/instances", response_model=List[RedisInstanceResponse]) -async def list_instances(): - """List all Redis instances with masked credentials.""" +@router.get("/instances", response_model=InstanceListResponse) +async def list_instances( + environment: Optional[str] = Query( + None, description="Filter by environment (development, staging, production)" + ), + usage: Optional[str] = Query( + None, description="Filter by usage type (cache, analytics, session, queue, custom)" + ), + status: Optional[str] = Query( + None, description="Filter by status (healthy, unhealthy, unknown)" + ), + instance_type: Optional[str] = Query( + None, + description="Filter by type (oss_single, oss_cluster, redis_enterprise, redis_cloud)", + ), + user_id: Optional[str] = Query(None, description="Filter by user ID"), + search: Optional[str] = Query(None, description="Search by instance name"), + limit: int = Query(100, ge=1, le=1000, description="Maximum number of results"), + offset: int = Query(0, ge=0, description="Number of results to skip for pagination"), +): + """List Redis instances with server-side filtering and pagination. + + All filters are optional. When no filters are provided, returns all instances. + Results are sorted by updated_at descending (most recently updated first). + """ try: - instances = await core_instances.get_instances() - return [to_response(inst) for inst in instances] + result = await core_instances.query_instances( + environment=environment, + usage=usage, + status=status, + instance_type=instance_type, + user_id=user_id, + search=search, + limit=limit, + offset=offset, + ) + return InstanceListResponse( + instances=[to_response(inst) for inst in result.instances], + total=result.total, + limit=result.limit, + offset=result.offset, + ) except Exception as e: logger.error(f"Failed to list instances: {e}") raise HTTPException(status_code=500, detail="Failed to retrieve instances") diff --git a/redis_sre_agent/core/instances.py b/redis_sre_agent/core/instances.py index baf2cad..8b257c7 100644 --- a/redis_sre_agent/core/instances.py +++ b/redis_sre_agent/core/instances.py @@ -1,20 +1,22 @@ """ Redis instance domain model and storage helpers. -TODO: Use a better persistence structure than serializing a list of JSON - objects into a string. +Instances are stored as Redis Hash documents with a RediSearch index for +efficient querying by environment, usage, status, instance_type, and user_id. """ from __future__ import annotations import json import logging +from dataclasses import dataclass from datetime import datetime, timezone from enum import Enum -from typing import Any, Dict, List, Optional +from typing import Any, Dict, List, Optional, Tuple from pydantic import BaseModel, Field, SecretStr, field_serializer, field_validator from redisvl.query import CountQuery, FilterQuery +from redisvl.query.filter import Tag, Text from .encryption import encrypt_secret, get_secret_value from .keys import RedisKeys @@ -348,6 +350,132 @@ async def get_instances() -> List[RedisInstance]: return [] +@dataclass +class InstanceQueryResult: + """Result of an instance query with pagination info.""" + + instances: List[RedisInstance] + total: int + limit: int + offset: int + + +async def query_instances( + *, + environment: Optional[str] = None, + usage: Optional[str] = None, + status: Optional[str] = None, + instance_type: Optional[str] = None, + user_id: Optional[str] = None, + search: Optional[str] = None, + limit: int = 100, + offset: int = 0, +) -> InstanceQueryResult: + """Query instances with server-side filtering and pagination. + + Args: + environment: Filter by environment (development, staging, production) + usage: Filter by usage type (cache, analytics, session, queue, custom) + status: Filter by status (healthy, unhealthy, unknown) + instance_type: Filter by type (oss_single, oss_cluster, redis_enterprise, redis_cloud) + user_id: Filter by user ID + search: Text search on instance name + limit: Maximum number of results (default 100, max 1000) + offset: Number of results to skip for pagination + + Returns: + InstanceQueryResult with instances list, total count, and pagination info + """ + try: + await _ensure_instances_index_exists() + index = await get_instances_index() + + # Build filter expression from provided parameters + filter_expr = None + + if environment: + env_filter = Tag("environment") == environment.lower() + filter_expr = env_filter if filter_expr is None else (filter_expr & env_filter) + + if usage: + usage_filter = Tag("usage") == usage.lower() + filter_expr = usage_filter if filter_expr is None else (filter_expr & usage_filter) + + if status: + status_filter = Tag("status") == status.lower() + filter_expr = status_filter if filter_expr is None else (filter_expr & status_filter) + + if instance_type: + type_filter = Tag("instance_type") == instance_type.lower() + filter_expr = type_filter if filter_expr is None else (filter_expr & type_filter) + + if user_id: + user_filter = Tag("user_id") == user_id + filter_expr = user_filter if filter_expr is None else (filter_expr & user_filter) + + if search: + # Text search on name field (supports prefix matching) + name_filter = Text("name") % f"*{search}*" + filter_expr = name_filter if filter_expr is None else (filter_expr & name_filter) + + # Get total count with filter + count_expr = filter_expr if filter_expr is not None else "*" + try: + total = await index.query(CountQuery(filter_expression=count_expr)) + total = int(total) if isinstance(total, int) else 0 + except Exception: + total = 0 + + if total == 0: + return InstanceQueryResult(instances=[], total=0, limit=limit, offset=offset) + + # Build query with pagination + # Clamp limit to reasonable bounds + limit = max(1, min(limit, 1000)) + offset = max(0, offset) + + fq = FilterQuery( + return_fields=["data"], + num_results=limit, + ).sort_by("updated_at", asc=False) + + if filter_expr is not None: + fq.set_filter(filter_expr) + + fq.paging(offset, limit) + + results = await index.query(fq) + + # Parse results + instances: List[RedisInstance] = [] + for doc in results or []: + try: + raw = doc.get("data") + if not raw: + continue + if isinstance(raw, bytes): + raw = raw.decode("utf-8") + inst_data = json.loads(raw) + if inst_data.get("connection_url"): + inst_data["connection_url"] = get_secret_value(inst_data["connection_url"]) + if inst_data.get("admin_password"): + inst_data["admin_password"] = get_secret_value(inst_data["admin_password"]) + instances.append(RedisInstance(**inst_data)) + except Exception as e: + logger.error("Failed to load instance from query result: %s. Skipping.", e) + + return InstanceQueryResult( + instances=instances, + total=total, + limit=limit, + offset=offset, + ) + + except Exception as e: + logger.error("Failed to query instances: %s", e) + return InstanceQueryResult(instances=[], total=0, limit=limit, offset=offset) + + # --- Instances search index helpers (non-breaking integration) --- async def _ensure_instances_index_exists() -> None: try: @@ -622,23 +750,82 @@ async def create_instance( # Convenience lookups async def get_instance_by_id(instance_id: str) -> Optional[RedisInstance]: - for inst in await get_instances(): - if inst.id == instance_id: - return inst - return None + """Get a single instance by ID using direct key lookup. + + This is more efficient than get_instances() when you only need one instance, + as it does a direct HGETALL on the instance key instead of searching. + """ + try: + client = get_redis_client() + key = f"{SRE_INSTANCES_INDEX}:{instance_id}" + data = await client.hget(key, "data") + + if not data: + return None + + if isinstance(data, bytes): + data = data.decode("utf-8") + + inst_data = json.loads(data) + if inst_data.get("connection_url"): + inst_data["connection_url"] = get_secret_value(inst_data["connection_url"]) + if inst_data.get("admin_password"): + inst_data["admin_password"] = get_secret_value(inst_data["admin_password"]) + + return RedisInstance(**inst_data) + except Exception as e: + logger.error("Failed to get instance by ID %s: %s", instance_id, e) + return None async def get_instance_by_name(instance_name: str) -> Optional[RedisInstance]: - for inst in await get_instances(): - if inst.name == instance_name: - return inst - return None + """Get a single instance by name using index query. + + Uses RediSearch text search on the name field for efficient lookup. + """ + try: + await _ensure_instances_index_exists() + index = await get_instances_index() + + # Use exact text match on name field + # Note: name is indexed as TEXT, so we search for the exact term + fq = FilterQuery( + return_fields=["data"], + num_results=1, + ) + fq.set_filter(Text("name") % instance_name) + + results = await index.query(fq) + + if not results: + return None + + doc = results[0] + raw = doc.get("data") + if not raw: + return None + + if isinstance(raw, bytes): + raw = raw.decode("utf-8") + + inst_data = json.loads(raw) + if inst_data.get("connection_url"): + inst_data["connection_url"] = get_secret_value(inst_data["connection_url"]) + if inst_data.get("admin_password"): + inst_data["admin_password"] = get_secret_value(inst_data["admin_password"]) + + return RedisInstance(**inst_data) + except Exception as e: + logger.error("Failed to get instance by name %s: %s", instance_name, e) + return None async def get_instance_map() -> Dict[str, RedisInstance]: + """Get all instances as a dict keyed by instance ID.""" return {inst.id: inst for inst in await get_instances()} async def get_instance_name(instance_id: str) -> Optional[str]: + """Get just the name of an instance by ID.""" inst = await get_instance_by_id(instance_id) return inst.name if inst else None diff --git a/redis_sre_agent/mcp_server/server.py b/redis_sre_agent/mcp_server/server.py index c95a40a..46d1e49 100644 --- a/redis_sre_agent/mcp_server/server.py +++ b/redis_sre_agent/mcp_server/server.py @@ -746,28 +746,53 @@ async def redis_sre_get_task_status(task_id: str) -> Dict[str, Any]: @mcp.tool() -async def redis_sre_list_instances() -> Dict[str, Any]: - """List all configured Redis instances. +async def redis_sre_list_instances( + environment: Optional[str] = None, + usage: Optional[str] = None, + status: Optional[str] = None, + instance_type: Optional[str] = None, + search: Optional[str] = None, + limit: int = 100, +) -> Dict[str, Any]: + """List configured Redis instances with optional filtering. - Returns a list of all Redis instances that have been configured - in the SRE agent. Sensitive information like connection URLs and - passwords are masked. + Returns a list of Redis instances that have been configured in the SRE agent. + All filter parameters are optional - when not provided, returns all instances. Use this to find instance IDs before calling other tools like redis_sre_deep_triage() or redis_sre_general_chat(). + Args: + environment: Filter by environment (development, staging, production) + usage: Filter by usage type (cache, analytics, session, queue, custom) + status: Filter by status (healthy, unhealthy, unknown) + instance_type: Filter by type (oss_single, oss_cluster, redis_enterprise, redis_cloud) + search: Search by instance name (partial match supported) + limit: Maximum number of results (default 100) + Returns: - Dictionary with list of instance information + Dictionary with filtered list of instance information and total count """ - from redis_sre_agent.core.instances import get_instances + from redis_sre_agent.core.instances import query_instances - logger.info("MCP list_instances request") + logger.info( + f"MCP list_instances request: env={environment}, usage={usage}, " + f"status={status}, type={instance_type}, search={search}, limit={limit}" + ) try: - instances = await get_instances() + result = await query_instances( + environment=environment, + usage=usage, + status=status, + instance_type=instance_type, + search=search, + limit=limit, + offset=0, + ) instance_list = [] - for inst in instances: + for inst in result.instances: instance_list.append( { "id": inst.id, @@ -783,7 +808,8 @@ async def redis_sre_list_instances() -> Dict[str, Any]: return { "instances": instance_list, - "total": len(instance_list), + "total": result.total, + "limit": result.limit, } except Exception as e: diff --git a/tests/unit/api/test_instances_api.py b/tests/unit/api/test_instances_api.py index 061c794..b61b486 100644 --- a/tests/unit/api/test_instances_api.py +++ b/tests/unit/api/test_instances_api.py @@ -6,7 +6,7 @@ from fastapi.testclient import TestClient from redis_sre_agent.api.app import app -from redis_sre_agent.core.instances import get_instances +from redis_sre_agent.core.instances import get_instances, InstanceQueryResult @pytest.fixture @@ -39,26 +39,74 @@ class TestInstancesAPI: def test_list_instances_success(self, client, sample_instance): """Test successful instance listing.""" - with patch("redis_sre_agent.core.instances.get_instances") as mock_get: - mock_get.return_value = [sample_instance] + mock_result = InstanceQueryResult( + instances=[sample_instance], + total=1, + limit=100, + offset=0, + ) + + with patch("redis_sre_agent.core.instances.query_instances") as mock_query: + mock_query.return_value = mock_result response = client.get("/api/v1/instances") assert response.status_code == 200 data = response.json() - assert len(data) == 1 - assert data[0]["id"] == "test-instance-123" + assert data["total"] == 1 + assert len(data["instances"]) == 1 + assert data["instances"][0]["id"] == "test-instance-123" def test_list_instances_empty(self, client): """Test instance listing when no instances exist.""" - with patch("redis_sre_agent.core.instances.get_instances") as mock_get: - mock_get.return_value = [] + mock_result = InstanceQueryResult( + instances=[], + total=0, + limit=100, + offset=0, + ) + + with patch("redis_sre_agent.core.instances.query_instances") as mock_query: + mock_query.return_value = mock_result response = client.get("/api/v1/instances") assert response.status_code == 200 data = response.json() - assert len(data) == 0 + assert data["total"] == 0 + assert len(data["instances"]) == 0 + + def test_list_instances_with_filters(self, client, sample_instance): + """Test instance listing with query parameters.""" + mock_result = InstanceQueryResult( + instances=[sample_instance], + total=1, + limit=100, + offset=0, + ) + + with patch("redis_sre_agent.core.instances.query_instances") as mock_query: + mock_query.return_value = mock_result + + response = client.get( + "/api/v1/instances", + params={ + "environment": "production", + "usage": "cache", + "status": "healthy", + "limit": 50, + "offset": 10, + }, + ) + + assert response.status_code == 200 + mock_query.assert_called_once() + call_kwargs = mock_query.call_args[1] + assert call_kwargs["environment"] == "production" + assert call_kwargs["usage"] == "cache" + assert call_kwargs["status"] == "healthy" + assert call_kwargs["limit"] == 50 + assert call_kwargs["offset"] == 10 def test_get_instance_success(self, client, sample_instance): """Test successful instance retrieval.""" @@ -237,8 +285,8 @@ class TestInstancesAPIErrorHandling: def test_list_instances_redis_error(self, client): """Test list instances when Redis fails.""" - with patch("redis_sre_agent.core.instances.get_instances") as mock_get: - mock_get.side_effect = Exception("Redis connection failed") + with patch("redis_sre_agent.core.instances.query_instances") as mock_query: + mock_query.side_effect = Exception("Redis connection failed") response = client.get("/api/v1/instances") diff --git a/tests/unit/mcp_server/test_mcp_server.py b/tests/unit/mcp_server/test_mcp_server.py index c628075..00fae78 100644 --- a/tests/unit/mcp_server/test_mcp_server.py +++ b/tests/unit/mcp_server/test_mcp_server.py @@ -326,6 +326,8 @@ class TestListInstancesTool: @pytest.mark.asyncio async def test_list_instances_success(self): """Test successful instance listing.""" + from redis_sre_agent.core.instances import InstanceQueryResult + mock_instance = MagicMock() mock_instance.id = "redis-prod-1" mock_instance.name = "Production Redis" @@ -335,11 +337,18 @@ async def test_list_instances_success(self): mock_instance.instance_type = "redis_cloud" mock_instance.repo_url = "https://github.com/example/repo" + mock_result = InstanceQueryResult( + instances=[mock_instance], + total=1, + limit=100, + offset=0, + ) + with patch( - "redis_sre_agent.core.instances.get_instances", + "redis_sre_agent.core.instances.query_instances", new_callable=AsyncMock, - ) as mock_get: - mock_get.return_value = [mock_instance] + ) as mock_query: + mock_query.return_value = mock_result result = await redis_sre_list_instances() @@ -351,25 +360,77 @@ async def test_list_instances_success(self): @pytest.mark.asyncio async def test_list_instances_empty(self): """Test empty instance list.""" + from redis_sre_agent.core.instances import InstanceQueryResult + + mock_result = InstanceQueryResult( + instances=[], + total=0, + limit=100, + offset=0, + ) + with patch( - "redis_sre_agent.core.instances.get_instances", + "redis_sre_agent.core.instances.query_instances", new_callable=AsyncMock, - ) as mock_get: - mock_get.return_value = [] + ) as mock_query: + mock_query.return_value = mock_result result = await redis_sre_list_instances() assert result["total"] == 0 assert result["instances"] == [] + @pytest.mark.asyncio + async def test_list_instances_with_filters(self): + """Test instance listing with filter parameters.""" + from redis_sre_agent.core.instances import InstanceQueryResult + + mock_instance = MagicMock() + mock_instance.id = "redis-prod-1" + mock_instance.name = "Production Redis" + mock_instance.environment = "production" + mock_instance.usage = "cache" + mock_instance.description = "Main cache" + mock_instance.instance_type = "redis_cloud" + mock_instance.repo_url = None + + mock_result = InstanceQueryResult( + instances=[mock_instance], + total=1, + limit=100, + offset=0, + ) + + with patch( + "redis_sre_agent.core.instances.query_instances", + new_callable=AsyncMock, + ) as mock_query: + mock_query.return_value = mock_result + + result = await redis_sre_list_instances( + environment="production", + usage="cache", + status="healthy", + ) + + # Verify query_instances was called with the correct parameters + mock_query.assert_called_once() + call_kwargs = mock_query.call_args[1] + assert call_kwargs["environment"] == "production" + assert call_kwargs["usage"] == "cache" + assert call_kwargs["status"] == "healthy" + + assert result["total"] == 1 + assert result["instances"][0]["environment"] == "production" + @pytest.mark.asyncio async def test_list_instances_error(self): """Test list instances error handling.""" with patch( - "redis_sre_agent.core.instances.get_instances", + "redis_sre_agent.core.instances.query_instances", new_callable=AsyncMock, - ) as mock_get: - mock_get.side_effect = Exception("Connection failed") + ) as mock_query: + mock_query.side_effect = Exception("Connection failed") result = await redis_sre_list_instances() diff --git a/ui/src/pages/Dashboard.tsx b/ui/src/pages/Dashboard.tsx index 5392fc0..c48f2df 100644 --- a/ui/src/pages/Dashboard.tsx +++ b/ui/src/pages/Dashboard.tsx @@ -100,7 +100,7 @@ const Dashboard = () => { // Instances if (instancesRes.status === "fulfilled") { - setInstances(instancesRes.value); + setInstances(instancesRes.value.instances); } else { console.warn("Instances unavailable:", instancesRes.reason); setInstances([]); diff --git a/ui/src/pages/Instances.tsx b/ui/src/pages/Instances.tsx index 44fff81..74d4012 100644 --- a/ui/src/pages/Instances.tsx +++ b/ui/src/pages/Instances.tsx @@ -1,8 +1,9 @@ -import { useState, useEffect } from "react"; +import { useState, useEffect, useCallback } from "react"; import { Card, CardContent, Button } from "@radar/ui-kit"; import sreAgentApi, { RedisInstance as APIRedisInstance, CreateInstanceRequest, + ListInstancesParams, } from "../services/sreAgentApi"; import { ConfirmDialog } from "../components/Modal"; import { maskRedisUrl } from "../utils/urlMasking"; @@ -811,6 +812,7 @@ const AddInstanceForm = ({ const Instances = () => { const [instances, setInstances] = useState([]); + const [totalCount, setTotalCount] = useState(0); const [isLoading, setIsLoading] = useState(true); const [isRefreshing, setIsRefreshing] = useState(false); const [error, setError] = useState(""); @@ -823,54 +825,74 @@ const Instances = () => { const [deletingInstance, setDeletingInstance] = useState(null); - // Load instances on component mount - useEffect(() => { - loadInstances(); - }, []); + // Convert API instance to UI format + const convertToUIInstance = (instance: APIRedisInstance): RedisInstance => ({ + ...instance, + connectionUrl: instance.connection_url, + repoUrl: instance.repo_url, + lastChecked: instance.last_checked, + createdAt: instance.created_at, + updatedAt: instance.updated_at, + monitoringIdentifier: instance.monitoring_identifier, + loggingIdentifier: instance.logging_identifier, + instanceType: instance.instance_type || "unknown", + adminUrl: instance.admin_url, + adminUsername: instance.admin_username, + adminPassword: instance.admin_password, + // Redis Cloud identifiers to UI camelCase + cloudSubscriptionType: + (instance as any).redis_cloud_subscription_type || "", + cloudSubscriptionId: (instance as any).redis_cloud_subscription_id + ? String((instance as any).redis_cloud_subscription_id) + : "", + cloudDatabaseId: (instance as any).redis_cloud_database_id + ? String((instance as any).redis_cloud_database_id) + : "", + cloudDatabaseName: (instance as any).redis_cloud_database_name || "", + }); + + // Build API params from current filter state + const buildFilterParams = useCallback((): ListInstancesParams => { + const params: ListInstancesParams = { + limit: 100, + }; + if (selectedEnvironment !== "all") { + params.environment = selectedEnvironment; + } + if (selectedUsage !== "all") { + params.usage = selectedUsage; + } + return params; + }, [selectedEnvironment, selectedUsage]); - const loadInstances = async () => { + const loadInstances = useCallback(async () => { try { setIsLoading(true); setError(""); - // Load instances from API - const apiInstances = await sreAgentApi.listInstances(); + // Load instances from API with server-side filtering + const response = await sreAgentApi.listInstances(buildFilterParams()); // Convert API format to UI format - const uiInstances: RedisInstance[] = apiInstances.map((instance) => ({ - ...instance, - connectionUrl: instance.connection_url, - repoUrl: instance.repo_url, - lastChecked: instance.last_checked, - createdAt: instance.created_at, - updatedAt: instance.updated_at, - monitoringIdentifier: instance.monitoring_identifier, - loggingIdentifier: instance.logging_identifier, - instanceType: instance.instance_type || "unknown", - adminUrl: instance.admin_url, - adminUsername: instance.admin_username, - adminPassword: instance.admin_password, - // Redis Cloud identifiers to UI camelCase - cloudSubscriptionType: - (instance as any).redis_cloud_subscription_type || "", - cloudSubscriptionId: (instance as any).redis_cloud_subscription_id - ? String((instance as any).redis_cloud_subscription_id) - : "", - cloudDatabaseId: (instance as any).redis_cloud_database_id - ? String((instance as any).redis_cloud_database_id) - : "", - cloudDatabaseName: (instance as any).redis_cloud_database_name || "", - })); + const uiInstances: RedisInstance[] = + response.instances.map(convertToUIInstance); setInstances(uiInstances); + setTotalCount(response.total); } catch (err) { setError("Failed to load Redis instances. Please try again."); console.error("Error loading instances:", err); setInstances([]); + setTotalCount(0); } finally { setIsLoading(false); } - }; + }, [buildFilterParams]); + + // Load instances when component mounts or filters change + useEffect(() => { + loadInstances(); + }, [loadInstances]); const handleRefresh = async () => { setIsRefreshing(true); @@ -1002,6 +1024,9 @@ const Instances = () => { return new Date(dateString).toLocaleString(); }; + // Filtering is now done server-side, but we keep client-side filtering + // as a safety net in case filters change before the API response arrives. + // In most cases, instances will already be filtered correctly from the server. const filteredInstances = instances.filter((instance) => { const environmentMatch = selectedEnvironment === "all" || diff --git a/ui/src/pages/Schedules.tsx b/ui/src/pages/Schedules.tsx index 73881ab..0036add 100644 --- a/ui/src/pages/Schedules.tsx +++ b/ui/src/pages/Schedules.tsx @@ -78,7 +78,7 @@ const Schedules = () => { if (instancesRes.status === "fulfilled") { // Map API instances to minimal shape used by this page - const mapped = instancesRes.value.map((i: any) => ({ + const mapped = instancesRes.value.instances.map((i: any) => ({ id: i.id, name: i.name, connection_url: i.connection_url, diff --git a/ui/src/pages/Triage.tsx b/ui/src/pages/Triage.tsx index 913ed9f..a7e456f 100644 --- a/ui/src/pages/Triage.tsx +++ b/ui/src/pages/Triage.tsx @@ -120,8 +120,8 @@ const Triage = () => { const loadInstances = async () => { try { - const apiInstances = await sreAgentApi.listInstances(); - setInstances(apiInstances); + const response = await sreAgentApi.listInstances(); + setInstances(response.instances); } catch (err) { console.error("Failed to load instances:", err); // Don't show error to user, just log it diff --git a/ui/src/services/sreAgentApi.ts b/ui/src/services/sreAgentApi.ts index e082c29..b5d1b8f 100644 --- a/ui/src/services/sreAgentApi.ts +++ b/ui/src/services/sreAgentApi.ts @@ -94,6 +94,24 @@ export interface RedisInstance { updated_at: string; } +export interface ListInstancesParams { + environment?: string; + usage?: string; + status?: string; + instance_type?: string; + user_id?: string; + search?: string; + limit?: number; + offset?: number; +} + +export interface InstanceListResponse { + instances: RedisInstance[]; + total: number; + limit: number; + offset: number; +} + export interface CreateInstanceRequest { name: string; connection_url: string; @@ -728,8 +746,27 @@ class SREAgentAPI { } // Instance Management Methods - async listInstances(): Promise { - const response = await fetch(`${this.tasksBaseUrl}/instances`); + async listInstances( + params?: ListInstancesParams, + ): Promise { + const url = new URL(`${this.tasksBaseUrl}/instances`); + + if (params) { + if (params.environment) + url.searchParams.set("environment", params.environment); + if (params.usage) url.searchParams.set("usage", params.usage); + if (params.status) url.searchParams.set("status", params.status); + if (params.instance_type) + url.searchParams.set("instance_type", params.instance_type); + if (params.user_id) url.searchParams.set("user_id", params.user_id); + if (params.search) url.searchParams.set("search", params.search); + if (params.limit !== undefined) + url.searchParams.set("limit", params.limit.toString()); + if (params.offset !== undefined) + url.searchParams.set("offset", params.offset.toString()); + } + + const response = await fetch(url.toString()); if (!response.ok) { throw new Error(`Failed to list instances: ${response.statusText}`); } From 786587ec7cb49129964c3e8c6ff360c04c26164b Mon Sep 17 00:00:00 2001 From: Andrew Brookins Date: Fri, 12 Dec 2025 19:27:46 -0800 Subject: [PATCH 2/4] lint --- redis_sre_agent/core/instances.py | 2 +- tests/unit/api/test_instances_api.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/redis_sre_agent/core/instances.py b/redis_sre_agent/core/instances.py index 8b257c7..cff26da 100644 --- a/redis_sre_agent/core/instances.py +++ b/redis_sre_agent/core/instances.py @@ -12,7 +12,7 @@ from dataclasses import dataclass from datetime import datetime, timezone from enum import Enum -from typing import Any, Dict, List, Optional, Tuple +from typing import Any, Dict, List, Optional from pydantic import BaseModel, Field, SecretStr, field_serializer, field_validator from redisvl.query import CountQuery, FilterQuery diff --git a/tests/unit/api/test_instances_api.py b/tests/unit/api/test_instances_api.py index b61b486..a14c49e 100644 --- a/tests/unit/api/test_instances_api.py +++ b/tests/unit/api/test_instances_api.py @@ -6,7 +6,7 @@ from fastapi.testclient import TestClient from redis_sre_agent.api.app import app -from redis_sre_agent.core.instances import get_instances, InstanceQueryResult +from redis_sre_agent.core.instances import InstanceQueryResult, get_instances @pytest.fixture From 5f39caf44435d9057730dc79de05fb326ac4803d Mon Sep 17 00:00:00 2001 From: Andrew Brookins Date: Mon, 15 Dec 2025 17:22:07 -0800 Subject: [PATCH 3/4] Use a TAG field for instance names --- redis_sre_agent/core/instances.py | 11 ++-- redis_sre_agent/core/redis.py | 2 +- tests/unit/api/test_instances_api.py | 67 ++++++++++++++++++++++++ tests/unit/mcp_server/test_mcp_server.py | 37 +++++++++++++ ui/src/pages/Instances.tsx | 3 -- 5 files changed, 110 insertions(+), 10 deletions(-) diff --git a/redis_sre_agent/core/instances.py b/redis_sre_agent/core/instances.py index cff26da..dec3e85 100644 --- a/redis_sre_agent/core/instances.py +++ b/redis_sre_agent/core/instances.py @@ -16,7 +16,7 @@ from pydantic import BaseModel, Field, SecretStr, field_serializer, field_validator from redisvl.query import CountQuery, FilterQuery -from redisvl.query.filter import Tag, Text +from redisvl.query.filter import Tag from .encryption import encrypt_secret, get_secret_value from .keys import RedisKeys @@ -414,8 +414,8 @@ async def query_instances( filter_expr = user_filter if filter_expr is None else (filter_expr & user_filter) if search: - # Text search on name field (supports prefix matching) - name_filter = Text("name") % f"*{search}*" + # Tag filter on name field with wildcard for partial matching + name_filter = Tag("name") == f"*{search}*" filter_expr = name_filter if filter_expr is None else (filter_expr & name_filter) # Get total count with filter @@ -787,13 +787,12 @@ async def get_instance_by_name(instance_name: str) -> Optional[RedisInstance]: await _ensure_instances_index_exists() index = await get_instances_index() - # Use exact text match on name field - # Note: name is indexed as TEXT, so we search for the exact term + # Use exact tag match on name field fq = FilterQuery( return_fields=["data"], num_results=1, ) - fq.set_filter(Text("name") % instance_name) + fq.set_filter(Tag("name") == instance_name) results = await index.query(fq) diff --git a/redis_sre_agent/core/redis.py b/redis_sre_agent/core/redis.py index 58580de..8ea83d0 100644 --- a/redis_sre_agent/core/redis.py +++ b/redis_sre_agent/core/redis.py @@ -188,7 +188,7 @@ "storage_type": "hash", }, "fields": [ - {"name": "name", "type": "text"}, + {"name": "name", "type": "tag"}, {"name": "environment", "type": "tag"}, {"name": "usage", "type": "tag"}, {"name": "instance_type", "type": "tag"}, diff --git a/tests/unit/api/test_instances_api.py b/tests/unit/api/test_instances_api.py index a14c49e..e87577b 100644 --- a/tests/unit/api/test_instances_api.py +++ b/tests/unit/api/test_instances_api.py @@ -108,6 +108,73 @@ def test_list_instances_with_filters(self, client, sample_instance): assert call_kwargs["limit"] == 50 assert call_kwargs["offset"] == 10 + def test_list_instances_with_search(self, client, sample_instance): + """Test instance listing with search parameter.""" + mock_result = InstanceQueryResult( + instances=[sample_instance], + total=1, + limit=100, + offset=0, + ) + + with patch("redis_sre_agent.core.instances.query_instances") as mock_query: + mock_query.return_value = mock_result + + response = client.get( + "/api/v1/instances", + params={"search": "test-redis"}, + ) + + assert response.status_code == 200 + mock_query.assert_called_once() + call_kwargs = mock_query.call_args[1] + assert call_kwargs["search"] == "test-redis" + + def test_list_instances_with_empty_search(self, client, sample_instance): + """Test instance listing with empty search string.""" + mock_result = InstanceQueryResult( + instances=[sample_instance], + total=1, + limit=100, + offset=0, + ) + + with patch("redis_sre_agent.core.instances.query_instances") as mock_query: + mock_query.return_value = mock_result + + response = client.get( + "/api/v1/instances", + params={"search": ""}, + ) + + assert response.status_code == 200 + mock_query.assert_called_once() + call_kwargs = mock_query.call_args[1] + # Empty string should be passed as empty (falsy) + assert call_kwargs["search"] == "" + + def test_list_instances_with_instance_type_filter(self, client, sample_instance): + """Test instance listing with instance_type filter.""" + mock_result = InstanceQueryResult( + instances=[sample_instance], + total=1, + limit=100, + offset=0, + ) + + with patch("redis_sre_agent.core.instances.query_instances") as mock_query: + mock_query.return_value = mock_result + + response = client.get( + "/api/v1/instances", + params={"instance_type": "redis_cloud"}, + ) + + assert response.status_code == 200 + mock_query.assert_called_once() + call_kwargs = mock_query.call_args[1] + assert call_kwargs["instance_type"] == "redis_cloud" + def test_get_instance_success(self, client, sample_instance): """Test successful instance retrieval.""" with patch("redis_sre_agent.core.instances.get_instances") as mock_get: diff --git a/tests/unit/mcp_server/test_mcp_server.py b/tests/unit/mcp_server/test_mcp_server.py index 00fae78..11c1880 100644 --- a/tests/unit/mcp_server/test_mcp_server.py +++ b/tests/unit/mcp_server/test_mcp_server.py @@ -423,6 +423,43 @@ async def test_list_instances_with_filters(self): assert result["total"] == 1 assert result["instances"][0]["environment"] == "production" + @pytest.mark.asyncio + async def test_list_instances_with_search(self): + """Test instance listing with search parameter.""" + from redis_sre_agent.core.instances import InstanceQueryResult + + mock_instance = MagicMock() + mock_instance.id = "redis-prod-1" + mock_instance.name = "Production Redis" + mock_instance.environment = "production" + mock_instance.usage = "cache" + mock_instance.description = "Main cache" + mock_instance.instance_type = "redis_cloud" + mock_instance.repo_url = None + + mock_result = InstanceQueryResult( + instances=[mock_instance], + total=1, + limit=100, + offset=0, + ) + + with patch( + "redis_sre_agent.core.instances.query_instances", + new_callable=AsyncMock, + ) as mock_query: + mock_query.return_value = mock_result + + result = await redis_sre_list_instances(search="Production") + + # Verify query_instances was called with the search parameter + mock_query.assert_called_once() + call_kwargs = mock_query.call_args[1] + assert call_kwargs["search"] == "Production" + + assert result["total"] == 1 + assert result["instances"][0]["name"] == "Production Redis" + @pytest.mark.asyncio async def test_list_instances_error(self): """Test list instances error handling.""" diff --git a/ui/src/pages/Instances.tsx b/ui/src/pages/Instances.tsx index 74d4012..bd1888a 100644 --- a/ui/src/pages/Instances.tsx +++ b/ui/src/pages/Instances.tsx @@ -812,7 +812,6 @@ const AddInstanceForm = ({ const Instances = () => { const [instances, setInstances] = useState([]); - const [totalCount, setTotalCount] = useState(0); const [isLoading, setIsLoading] = useState(true); const [isRefreshing, setIsRefreshing] = useState(false); const [error, setError] = useState(""); @@ -878,12 +877,10 @@ const Instances = () => { response.instances.map(convertToUIInstance); setInstances(uiInstances); - setTotalCount(response.total); } catch (err) { setError("Failed to load Redis instances. Please try again."); console.error("Error loading instances:", err); setInstances([]); - setTotalCount(0); } finally { setIsLoading(false); } From df9d7c41e44ba4271295b4e15c9a68b711ee3310 Mon Sep 17 00:00:00 2001 From: Andrew Brookins Date: Mon, 15 Dec 2025 17:45:31 -0800 Subject: [PATCH 4/4] Note why we are using FilterExpression --- redis_sre_agent/core/instances.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/redis_sre_agent/core/instances.py b/redis_sre_agent/core/instances.py index dec3e85..d2deaa6 100644 --- a/redis_sre_agent/core/instances.py +++ b/redis_sre_agent/core/instances.py @@ -16,7 +16,7 @@ from pydantic import BaseModel, Field, SecretStr, field_serializer, field_validator from redisvl.query import CountQuery, FilterQuery -from redisvl.query.filter import Tag +from redisvl.query.filter import FilterExpression, Tag from .encryption import encrypt_secret, get_secret_value from .keys import RedisKeys @@ -414,8 +414,8 @@ async def query_instances( filter_expr = user_filter if filter_expr is None else (filter_expr & user_filter) if search: - # Tag filter on name field with wildcard for partial matching - name_filter = Tag("name") == f"*{search}*" + # Use raw FilterExpression for wildcard matching (Tag escapes wildcards) + name_filter = FilterExpression(f"@name:{{*{search}*}}") filter_expr = name_filter if filter_expr is None else (filter_expr & name_filter) # Get total count with filter