fix: index sessions in sorted set for listing #119

epicdigitalmedia · 2026-01-27T02:08:28Z

Summary

Add zadd call in set_working_memory() to index sessions
Add zrem call in delete_working_memory() to remove sessions from index
Add tests for session indexing behavior

Problem

The list_sessions() function reads from a sorted set (sessions:{namespace}) that was never populated. Sessions were saved to Redis but never indexed, causing the list endpoint to always return empty.

Solution

When saving working memory, add the session to the sorted set with current timestamp as score
When deleting working memory, remove the session from the sorted set

Testing

Added unit tests for indexing behavior
Added integration test for list_sessions with indexed sessions
All existing tests pass

jit-ci · 2026-01-27T02:08:40Z

Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset.

In case there are security findings, they will be communicated to you as a comment inside the PR.

Hope you’ll enjoy using Jit.

Questions? Comments? Want to learn more? Get in touch with us.

Copilot

Pull request overview

Fixes session listing by ensuring sessions are indexed in Redis when working memory is saved and removed from the index when working memory is deleted.

Changes:

Add Redis ZADD indexing to set_working_memory() so list_sessions() has data to read.
Add Redis ZREM cleanup to delete_working_memory() to keep the index in sync on explicit deletes.
Add unit/integration tests validating session indexing and listing behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
agent_memory_server/working_memory.py	Populates and maintains the `sessions:{namespace}` sorted-set index used by `list_sessions()`.
tests/test_working_memory.py	Adds tests covering indexing on set, de-indexing on delete, and `list_sessions()` returning indexed sessions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-27T02:15:02Z

agent_memory_server/working_memory.py

+        # Index session in sorted set for listing
+        sessions_key = Keys.sessions_key(namespace=working_memory.namespace)
+        await redis_client.zadd(
+            sessions_key,
+            {working_memory.session_id: time.time()}
+        )


set_working_memory() now always adds the session ID to the sessions:{namespace} sorted set, but nothing removes that entry when the working-memory JSON key expires via TTL. With the current default TTL usage (e.g., MCP tool defaults), this will accumulate stale session IDs indefinitely and list_sessions() may return sessions whose working memory no longer exists. Consider implementing a cleanup strategy (e.g., store expiration timestamp as the score and prune expired members in list_sessions(), or run a background/keyspace-notification cleanup) so the index doesn’t grow without bound and doesn’t return expired sessions.

Copilot · 2026-01-27T02:15:03Z

agent_memory_server/working_memory.py

+        await redis_client.zadd(
+            sessions_key,
+            {working_memory.session_id: time.time()}


The sessions index is keyed only by namespace and stores only session_id as the member. However, working memory storage supports user_id-scoped keys (Keys.working_memory_key(..., user_id=...)). If two users reuse the same session_id within a namespace, zadd will collide and the index can’t represent both sessions; it also makes it hard to safely filter listings by user. Consider incorporating user_id into the index key (e.g., sessions:{namespace}:{user_id}) or into the sorted-set member format, and updating list_sessions() accordingly.

Suggested change

await redis_client.zadd(

sessions_key,

{working_memory.session_id: time.time()}

# Include user_id in the member to avoid collisions between users

session_index_member = (

f"{working_memory.user_id}:{working_memory.session_id}"

if working_memory.user_id is not None

else working_memory.session_id

)

await redis_client.zadd(

sessions_key,

{session_index_member: time.time()}

Copilot · 2026-01-27T02:15:03Z

agent_memory_server/working_memory.py

        await redis_client.delete(key)
+        # Remove session from sorted set index
+        sessions_key = Keys.sessions_key(namespace=namespace)
+        await redis_client.zrem(sessions_key, session_id)


delete_working_memory() removes from the sessions index using only session_id (and only namespace for the key). If working memories are stored with user_id (supported by Keys.working_memory_key), deleting one user’s session can incorrectly remove the index entry for another user that happens to share the same session_id. If you change the indexing scheme to be user-scoped (or encode user_id into the member), this zrem should be updated to match.

Suggested change

await redis_client.zrem(sessions_key, session_id)

# Remove both legacy session-only member and user-scoped member (if applicable)

members = [session_id]

if user_id is not None:

members.append(f"{user_id}:{session_id}")

await redis_client.zrem(sessions_key, *members)

abrookins

This is good. It looks like we dropped those calls during a refactor. However, the TTLs issue is a certainly a point against continuing to use a sorted set instead of a search index. A search index will update to reflect garbage-collected keys. We should merge your PR first though and return to that later.

abrookins · 2026-01-29T23:59:48Z

@epicdigitalmedia Can you run uv run pre-commit run --all-files and push that? Then I can merge!

epicdigitalmedia · 2026-01-30T00:29:34Z

@abrookins all set thanks!

The set_working_memory function now calls zadd to index sessions, so the test mock needs to include this async method. Built with Epic Flowstate

abrookins · 2026-01-30T19:48:35Z

Merged, thanks again @epicdigitalmedia. Also, thanks for swooping in with that test fix @tegument!

tegument added 5 commits January 26, 2026 18:57

test: add failing test for session indexing on set_working_memory

8561cd4

feat: index sessions in sorted set on set_working_memory

aa72dfc

test: add failing test for session removal from index on delete

c73828c

feat: remove sessions from index on delete_working_memory

b7b2dbb

test: add integration test for list_sessions with indexed sessions

293b3dc

Copilot AI review requested due to automatic review settings January 27, 2026 02:08

Copilot started reviewing on behalf of epicdigitalmedia January 27, 2026 02:08 View session

Copilot AI reviewed Jan 27, 2026

View reviewed changes

abrookins reviewed Jan 27, 2026

View reviewed changes

abrookins mentioned this pull request Jan 27, 2026

Replace sessions sorted set with Redis Search index #121

Open

style: run pre-commit formatting

cf34abd

fix: add zadd mock to test_set_working_memory_with_strategy

55504ac

The set_working_memory function now calls zadd to index sessions, so the test mock needs to include this async method. Built with Epic Flowstate

abrookins merged commit f9e33ec into redis:main Jan 30, 2026
10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: index sessions in sorted set for listing #119

fix: index sessions in sorted set for listing #119

Uh oh!

epicdigitalmedia commented Jan 27, 2026

Uh oh!

jit-ci bot commented Jan 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 27, 2026

Uh oh!

Copilot AI Jan 27, 2026

Uh oh!

Copilot AI Jan 27, 2026

Uh oh!

abrookins left a comment •

edited

Loading

Uh oh!

abrookins commented Jan 29, 2026

Uh oh!

epicdigitalmedia commented Jan 30, 2026

Uh oh!

Uh oh!

abrookins commented Jan 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-        await redis_client.zadd(
-            sessions_key,
-            {working_memory.session_id: time.time()}
+        # Include user_id in the member to avoid collisions between users
+        session_index_member = (
+            f"{working_memory.user_id}:{working_memory.session_id}"
+            if working_memory.user_id is not None
+            else working_memory.session_id
+        )
+        await redis_client.zadd(
+            sessions_key,
+            {session_index_member: time.time()}

-        await redis_client.zrem(sessions_key, session_id)
+        # Remove both legacy session-only member and user-scoped member (if applicable)
+        members = [session_id]
+        if user_id is not None:
+            members.append(f"{user_id}:{session_id}")
+        await redis_client.zrem(sessions_key, *members)

fix: index sessions in sorted set for listing #119

fix: index sessions in sorted set for listing #119

Uh oh!

Conversation

epicdigitalmedia commented Jan 27, 2026

Summary

Problem

Solution

Testing

Uh oh!

jit-ci bot commented Jan 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

abrookins left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abrookins commented Jan 29, 2026

Uh oh!

epicdigitalmedia commented Jan 30, 2026

Uh oh!

Uh oh!

abrookins commented Jan 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

abrookins left a comment •

edited

Loading