Skip to content

fix(sessions): restore oldest-first Redis session lists - #7124

Open
baba9811 wants to merge 1 commit into
google:mainfrom
baba9811:fix/redis-session-list-order
Open

baba9811 wants to merge 1 commit into
google:mainfrom
baba9811:fix/redis-session-list-order

Conversation

@baba9811

Copy link
Copy Markdown
Contributor

Description of Change

Problem: RedisSessionService.list_sessions() returns sessions newest first, contrary to the base service contract. An application selecting response.sessions[-1] to resume its most recently active session instead selects an older session.

Solution: Sort the shared result oldest first using (last_update_time, user_id, id), matching the in-memory, SQLite and database implementations. Remove Redis's recorded ordering divergence from the shared contract tests, and cover both listing scopes and timestamp ties. The separate Redis timestamp-source and unknown-session append divergences remain unchanged.

The activity-time order and user/session tie-breakers were established for the database backend in #6272. Redis's deviation is recorded in the shared test registry.

Reproduction

Environment details: Current source based on 322e3bf, ADK 2.9.0 package metadata, macOS 26.6.2 arm64, Python 3.12.10, Redis 8.4.0, redis-py 8.1.0.

Model information: LiteLLM: No. Model: N/A (no model call).

Steps to reproduce:

  1. Install ADK and redis in a virtual environment.
  2. Start a disposable Redis instance with a Unix socket and set REDIS_SOCKET to its path. For example, redis-server --port 0 --unixsocket /tmp/adk-order-example.sock --save "" --appendonly no, in a separate terminal.
  3. Run the following code. It creates three sessions, makes the oldest-created session active again, and lists them. The unique key prefix and 60-second TTL isolate the example's records.
import asyncio
import os
from uuid import uuid4

from google.adk.events import Event
from google.adk.integrations.redis import RedisSessionService, RedisSessionServiceConfig
import redis.asyncio as redis


async def main():
    async with redis.Redis(unix_socket_path=os.environ["REDIS_SOCKET"], decode_responses=True) as client:
        service = RedisSessionService(
            redis_client=client,
            config=RedisSessionServiceConfig(key_prefix=f"order-repro:{uuid4()}:", ttl_seconds=60),
        )
        scope = dict(app_name="demo", user_id="user")
        for session_id in ("a", "b", "c"):
            await service.create_session(**scope, session_id=session_id)
        session = await service.get_session(**scope, session_id="a")
        await service.append_event(session, Event(author="user", message="continue"))
        response = await service.list_sessions(**scope)
        actual = [session.id for session in response.sessions]
        print("Returned:", actual, "Expected:", ["b", "c", "a"])
        assert actual == ["b", "c", "a"]


asyncio.run(main())

Observed behavior on main:

Returned: ['a', 'c', 'b'] Expected: ['b', 'c', 'a']
AssertionError

Expected behavior, verified after the correction:

Returned: ['b', 'c', 'a'] Expected: ['b', 'c', 'a']

Testing Plan

  • Before the production change, the existing Redis contract test and both new ordering cases failed for the expected reason. The revised tie fixture also rejects a deliberately swapped user/session tie priority.
  • Final Redis and shared session suites on each Python version from 3.10 through 3.14: 289 passed, 2 existing expected failures. Broader session, Redis, FastAPI and local-storage run: 648 passed, 5 skipped, 3 expected failures; production code is unchanged since that run.
  • Built baseline and fixed wheels and installed both into the same clean environment. The real Redis/API-server acceptance check fails before and passes after. Fixed-wheel checks cover user/app-wide ordering, empty/single results, evaluation-session filtering, literal scope matching, state/event persistence, and actual expiry. API routes were exercised through ASGI transport; no cloud/model call or browser was involved.
  • Final changed-file pre-commit and commit hooks pass. Full pre-commit fails on both unchanged main and the fixed tree because pyink removes the same pre-existing blank line in tests/unittests/plugins/test_bigquery_agent_analytics_plugin.py; all other hooks pass. The unrelated formatter change is excluded from this PR.
  • Mypy 2.3.1 comparisons under Python 3.10–3.13 found zero new diagnostics. Baseline and fixed trees each report the same 839 errors across 130 files after normalizing line numbers, so raw mypy exits remain 1. These macOS checks reuse identical test-extra environments; Linux CI's full-extras matrix was not reproduced locally.
  • tox -p 2 completed the full tests/unittests suite on Python 3.10–3.14, with all five environments exiting 0. Each reports 14,839 passed (14,846 on 3.11), 86–87 skipped, 26 xfailed and 2 xpassed. Production source and the conformance registry were unchanged throughout; the final tie-fixture identifiers were verified separately in all five environments as reported above.

Compatibility and Documentation

The method signature, response shape and stored data are unchanged. Redis callers that compensated for its previous returned order may need to remove that compensation.

This restores the documented oldest-first contract and changes Redis's previous returned order. Is normal bug-fix release handling sufficient, or should this also carry a migration note for callers that compensated for the old order?

Companion documentation clarifies the existing Python session-list contract in the session guide change.

Follow the base session service ordering contract and use user and session IDs to break timestamp ties. Remove the recorded ordering divergence and cover both user-scoped and app-wide lists.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants