feat(session): implement Redis storage backend (RC-6)#4233
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 358e8a194e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4233 +/- ##
==========================================
+ Coverage 68.82% 69.22% +0.39%
==========================================
Files 468 471 +3
Lines 47087 47300 +213
==========================================
+ Hits 32407 32742 +335
- Misses 11996 12019 +23
+ Partials 2684 2539 -145 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Implements a Redis-backed Storage implementation for pkg/transport/session to externalize session state (enabling horizontal scaling), including configuration structs and unit tests.
Changes:
- Added
RedisConfig/SentinelConfig/RedisTLSConfig(with default timeout constants) for configuring Redis connectivity. - Implemented
RedisStoragewith Redis TTL-based expiry and standalone/Sentinel support (plus optional TLS). - Added
miniredisunit tests covering CRUD behavior, TTL refresh/expiry, key format, and multiple session types; removed now-unneedednolint:unusedin serialization helpers.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| pkg/transport/session/storage_redis.go | Adds Redis-backed Storage implementation and TLS helper. |
| pkg/transport/session/redis_config.go | Introduces Redis configuration structs and default timeout constants. |
| pkg/transport/session/storage_redis_test.go | Adds miniredis-based unit tests for RedisStorage. |
| pkg/transport/session/serialization.go | Removes obsolete “unused” annotations/comments now that serialization is exercised. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Adds a Redis-backed Storage implementation to pkg/transport/session so session state can be shared across replicas (supporting horizontal scaling), with configuration structs and unit tests.
Changes:
- Introduces
RedisConfig/SentinelConfig/RedisTLSConfigwith defaulted timeouts. - Implements
RedisStorage(standalone + Sentinel, optional TLS) plus constructor(s). - Adds
miniredis-backed unit tests and removes now-unneedednolint:unusedannotations from session serialization helpers.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/transport/session/redis_config.go | Adds Redis configuration structs and default timeout constants. |
| pkg/transport/session/storage_redis.go | Implements the Redis-backed Storage plus config validation and TLS builder. |
| pkg/transport/session/storage_redis_test.go | Adds unit tests for Redis storage behavior using miniredis. |
| pkg/transport/session/serialization.go | Removes “unused” comments/linters now that serialization is used by Redis storage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
This PR adds a Redis-backed Storage implementation to pkg/transport/session, enabling session state to be shared across vMCP replicas for horizontal scaling (RC-6 / Fixes #4205).
Changes:
- Introduces
RedisStoragewith standalone + Sentinel support, TTL-based expiry, and TLS options. - Adds Redis configuration structs and default timeout constants for consistent client behavior.
- Adds unit tests using
miniredisto cover storage operations, TTL refresh behavior, and round-trips across session types.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/transport/session/redis_config.go | Adds Redis/Sentinel/TLS config structs and default timeout constants. |
| pkg/transport/session/storage_redis.go | Implements the Storage interface backed by Redis, including TTL refresh on Load via GETEX. |
| pkg/transport/session/storage_redis_test.go | Adds miniredis-based unit tests for Redis storage behavior and config validation. |
| pkg/transport/session/storage.go | Clarifies Load semantics around backend TTL refresh vs. UpdatedAt. |
| pkg/transport/session/serialization.go | Removes now-unneeded nolint:unused annotations since serialization is used by Redis storage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add RedisStorage as a distributed Storage implementation for the transport/session package, enabling horizontal scaling of the vMCP component by externalising session state to a shared Redis/Sentinel backend. - Add RedisConfig, SentinelConfig, and RedisTLSConfig structs with configurable timeouts (defaults: dial 5s, read 3s, write 3s) - Implement RedisStorage with Store (SET+EX TTL refresh), Load (ErrSessionNotFound on miss), Delete (no-op on missing key), DeleteExpired (no-op; Redis TTL handles expiry), and Close - Support standalone and Sentinel modes; optional TLS with system or custom CA roots - Add unit tests using miniredis covering all methods, TTL refresh, idempotent upsert, key format, and all three session types - Remove nolint:unused annotations from serializeSession/deserializeSession now that they are consumed by RedisStorage Closes #4205
Add RedisStorage as a distributed Storage implementation for the transport/session package, enabling horizontal scaling of the vMCP component by externalising session state to a shared Redis/Sentinel backend. - Add RedisConfig, SentinelConfig, and RedisTLSConfig structs with configurable timeouts (defaults: dial 5s, read 3s, write 3s) - Implement RedisStorage with Store (SET+EX TTL refresh), Load (ErrSessionNotFound on miss), Delete (no-op on missing key), DeleteExpired (no-op; Redis TTL handles expiry), and Close - Support standalone and Sentinel modes; optional TLS with system or custom CA roots - Add unit tests using miniredis covering all methods, TTL refresh, idempotent upsert, key format, and all three session types - Remove nolint:unused annotations from serializeSession/deserializeSession now that they are consumed by RedisStorage Closes stacklok#4205 Co-authored-by: taskbot <taskbot@users.noreply.github.com>
Summary
Add RedisStorage as a distributed Storage implementation for the transport/session package, enabling horizontal scaling of the vMCP component by externalising session state to a shared Redis/Sentinel backend.
Fixes #4205
Type of change
Test plan
task test)task test-e2e)task lint-fix)Changes
Does this introduce a user-facing change?
Special notes for reviewers