Open
Conversation
KaveeshaPiumini
requested review from
ThaminduDilshan,
rajithacharith and
thiva-k
as code owners
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Redis as an optional runtime datastore: new runtime config fields, a Redis provider and closer, NoOp transactioner for Redis runtime, and Redis-backed stores (attribute cache, passkey sessions, flow execution, OAuth auth codes/requests) plus tests, mocks, and CI adjustments; initialization now selects store implementations by runtime type. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App Startup
participant Config as Config
participant RedisProv as Redis Provider
participant DBProv as DB Provider
participant Stores as Store Factory
participant Redis as Redis Server
App->>Config: Read Database.Runtime.Type
alt runtime == "redis"
App->>RedisProv: GetRedisProvider()
RedisProv->>Redis: Dial & PING
Redis-->>RedisProv: PONG
RedisProv-->>App: RedisProvider (client, keyPrefix)
App->>Stores: newRedis*Store(redisProvider)
Stores-->>App: Redis-backed stores
App->>DBProv: GetRuntimeDBTransactioner()
DBProv-->>App: NoOpTransactioner
else runtime != "redis"
App->>DBProv: GetDBProvider()
DBProv-->>App: SQL client & Transactioner
App->>Stores: newSQL*Store(dbProvider)
Stores-->>App: SQL-backed stores
end
App->>Stores: Store methods (Create/Get/Consume/Delete)
Stores->>Redis: SET/GET/DEL/TTL/EXPIRE/EVAL
Redis-->>Stores: Results
Stores-->>App: Data / success/fail
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
backend/internal/authn/passkey/redis_store.go (1)
56-68: Consider propagating context through the interface.The store methods use
context.Background()because thesessionStoreInterfacedoesn't accept acontext.Contextparameter. This diverges from other Redis stores in this PR (auth code, flow, attribute cache) that properly propagate the caller's context.While this implementation is correct given the interface constraint, consider updating
sessionStoreInterfaceto acceptctx context.Contextfor consistency with repository conventions around context propagation and to enable request-scoped timeouts/cancellation for Redis calls.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/authn/passkey/redis_store.go` around lines 56 - 68, The session store currently uses context.Background() because sessionStoreInterface lacks a context parameter; update the interface to accept ctx context.Context and then change redisSessionStore.storeSession to signature storeSession(ctx context.Context, sessionKey string, session *sessionData, expirySeconds int64) and use that ctx when calling s.client.Set (and any other Redis ops) instead of context.Background(); update all implementations and call sites of sessionStoreInterface/storeSession to pass through the caller's context so Redis calls honor request-scoped cancellation/timeouts.backend/internal/system/database/provider/redisprovider.go (1)
92-102:GetRedisProviderreturnsnilwhen runtime type is not Redis.When
runtime.typeis not"redis",initRedisProvider()is a no-op andredisInstanceremainsnil. Callers must check the return value before use, but none of the Redis store constructors in this PR do so—they directly callp.GetRedisClient().The initialization paths in
init.gofiles checkconfig.Database.Runtime.Type == DataSourceTypeRedisbefore callingGetRedisProvider(), which makes this safe in practice. However, a defensive nil check or documentation would prevent future misuse.📝 Suggested documentation or defensive check
// GetRedisProvider returns the singleton Redis provider. +// Returns nil if runtime database type is not "redis" or if initialization failed. +// Callers should check Config.Database.Runtime.Type before calling this function. func GetRedisProvider() RedisProviderInterface { initRedisProvider() return redisInstance }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/system/database/provider/redisprovider.go` around lines 92 - 102, GetRedisProvider and GetRedisProviderCloser can return nil if initRedisProvider never created redisInstance; add a defensive check after calling initRedisProvider(): if redisInstance == nil then panic (or log.Fatalf) with a clear message indicating DataSourceTypeRedis must be configured (include symbols GetRedisProvider, GetRedisProviderCloser, initRedisProvider, and redisInstance in the message) so callers like those invoking GetRedisClient won't get a nil receiver; keep the rest of the function behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/internal/flow/flowexec/redis_store.go`:
- Around line 101-122: The TTL handling in the block that calls
s.client.TTL(ctx, key).Result() must distinguish missing keys (TTL == -2) from
keys with no expiry (TTL == -1); change the logic so that when TTL returns -2
you return an error (matching the SQL path) instead of treating it like ttl=0,
when TTL == -1 keep ttl=0 to preserve no-expiration semantics, and otherwise use
the positive TTL; update the function that builds/saves the Redis value (the
code around s.client.TTL, FromEngineContext, and s.client.Set) to perform this
check and return a clear error when the key is missing before marshalling and
calling s.client.Set.
In `@backend/internal/oauth/oauth2/authz/auth_code_redis_store.go`:
- Around line 76-79: The code currently computes ttl :=
time.Until(authzCode.ExpiryTime) and passes it directly to s.client.Set, which
can be negative for already-expired authzCode; add a guard that computes ttl :=
time.Until(authzCode.ExpiryTime) and if ttl <= 0 return an error (e.g.,
fmt.Errorf("authorization code already expired: %s", authzCode.Code)) instead of
calling s.client.Set; keep using s.authCodeKey(authzCode.Code) and s.client.Set
only when ttl > 0.
In `@backend/internal/system/config/config.go`:
- Around line 77-80: Add user documentation for the new Redis runtime backend
introduced via the config fields Address, DB, KeyPrefix (and existing
database.runtime.username/password reuse for Redis ACL). Update the runtime
configuration guide (e.g., docs/content/guides/runtime-database-redis.mdx or the
runtime guide under docs/content/guides/) to describe database.runtime.type:
"redis", database.runtime.address, database.runtime.db,
database.runtime.key_prefix, and explain that database.runtime.username/password
are used for Redis ACL auth; also list which runtime stores move to Redis (flow
contexts, authorization codes, authorization requests, passkey sessions, and
attribute cache) and include example configuration snippets and any notes about
migrations/compatibility.
In `@backend/internal/system/database/provider/dbprovider.go`:
- Around line 193-196: The runtime Redis check currently returns an error in
initializeClient when DataSourceTypeRedis, but initializeAllClients still
eagerly calls initializeClient for the runtime datasource causing spurious
errors; modify initializeAllClients() to skip calling initializeClient for the
runtime datasource when its Type == DataSourceTypeRedis (i.e., detect the
runtime dataSource.Type and bypass/omit the initializeClient(runtime)
invocation), leaving initializeClient unchanged so lazy SQL runtime access can
occur only when needed.
In `@backend/internal/system/database/provider/redisprovider.go`:
- Around line 76-82: The current client.Ping failure path logs and returns
without setting redisInstance, leaving sync.Once completed and causing
GetRedisProvider() to permanently return nil; change the behavior to fail-fast
by replacing the silent return with a process-level failure: after logging and
closing the client, call logger.Fatal or panic with the error (or os.Exit(1)) so
initialization cannot silently succeed, or alternatively persist the error to a
new package-level variable (e.g., redisInitErr) and modify GetRedisProvider() to
return (RedisProvider, error) so callers can detect and handle the
initialization error; update references to redisInstance, GetRedisProvider, and
the initialization block that calls client.Ping(context.Background()).Err()
accordingly.
---
Nitpick comments:
In `@backend/internal/authn/passkey/redis_store.go`:
- Around line 56-68: The session store currently uses context.Background()
because sessionStoreInterface lacks a context parameter; update the interface to
accept ctx context.Context and then change redisSessionStore.storeSession to
signature storeSession(ctx context.Context, sessionKey string, session
*sessionData, expirySeconds int64) and use that ctx when calling s.client.Set
(and any other Redis ops) instead of context.Background(); update all
implementations and call sites of sessionStoreInterface/storeSession to pass
through the caller's context so Redis calls honor request-scoped
cancellation/timeouts.
In `@backend/internal/system/database/provider/redisprovider.go`:
- Around line 92-102: GetRedisProvider and GetRedisProviderCloser can return nil
if initRedisProvider never created redisInstance; add a defensive check after
calling initRedisProvider(): if redisInstance == nil then panic (or log.Fatalf)
with a clear message indicating DataSourceTypeRedis must be configured (include
symbols GetRedisProvider, GetRedisProviderCloser, initRedisProvider, and
redisInstance in the message) so callers like those invoking GetRedisClient
won't get a nil receiver; keep the rest of the function behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 61a0a574-fc87-4fd5-9df1-d07fa28d31ab
⛔ Files ignored due to path filters (2)
backend/tests/mocks/database/providermock/RedisProviderCloser_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/database/providermock/RedisProviderInterface_mock.gois excluded by!**/*_mock.go
📒 Files selected for processing (13)
backend/cmd/server/repository/resources/conf/default.jsonbackend/internal/attributecache/init.gobackend/internal/attributecache/redis_store.gobackend/internal/authn/passkey/init.gobackend/internal/authn/passkey/redis_store.gobackend/internal/flow/flowexec/init.gobackend/internal/flow/flowexec/redis_store.gobackend/internal/oauth/oauth2/authz/auth_code_redis_store.gobackend/internal/oauth/oauth2/authz/auth_req_redis_store.gobackend/internal/oauth/oauth2/authz/init.gobackend/internal/system/config/config.gobackend/internal/system/database/provider/dbprovider.gobackend/internal/system/database/provider/redisprovider.go
KaveeshaPiumini
force-pushed
the
refactor
branch
from
4cad7b8 to
ae6e88f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
backend/internal/system/database/provider/dbprovider.go (1)
169-173:⚠️ Potential issue | 🟠 MajorEager runtime SQL initialization still occurs when Redis is configured.
initializeAllClients()unconditionally callsinitializeClient()for the runtime datasource (line 170). Whendatabase.runtime.type: redis, this will fail and emit a spurious "Failed to initialize runtime database client" error on every startup.Suggested fix
runtimeDBConfig := config.GetThunderRuntime().Config.Database.Runtime + if runtimeDBConfig.Type != DataSourceTypeRedis { err = d.initializeClient(&d.runtimeClient, runtimeDBConfig, dbNameRuntime) if err != nil { logger.Error("Failed to initialize runtime database client", log.Error(err)) } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/system/database/provider/dbprovider.go` around lines 169 - 173, initializeAllClients() is eagerly calling initializeClient() for the runtime datasource even when runtimeDBConfig.Type is redis, causing spurious "Failed to initialize runtime database client" errors; update the logic around the call that uses runtimeDBConfig (runtimeDBConfig := config.GetThunderRuntime().Config.Database.Runtime) so you only call initializeClient(&d.runtimeClient, runtimeDBConfig, dbNameRuntime) when the runtime DB type is not redis (e.g., check runtimeDBConfig.Type or equivalent enum/const), otherwise skip initialization for the runtime client and optionally log a debug/info message indicating Redis is used.backend/internal/oauth/oauth2/authz/auth_code_redis_store.go (1)
76-79:⚠️ Potential issue | 🟡 MinorAdd guard against already-expired authorization codes.
When
authzCode.ExpiryTimeis in the past,time.Until()returns a negative duration. With go-redis v9, a negative TTL results in a plainSETwith no expiry, causing the code to persist indefinitely.Proposed fix
ttl := time.Until(authzCode.ExpiryTime) + if ttl <= 0 { + return fmt.Errorf("authorization code has already expired") + } if err := s.client.Set(ctx, s.authCodeKey(authzCode.Code), data, ttl).Err(); err != nil {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/oauth/oauth2/authz/auth_code_redis_store.go` around lines 76 - 79, The code currently uses ttl := time.Until(authzCode.ExpiryTime) and then calls s.client.Set(..., ttl) which with a negative ttl will remove expiry in go-redis v9; update the logic in the auth code store (around authzCode.ExpiryTime, time.Until, s.client.Set and s.authCodeKey(authzCode.Code)) to guard against already-expired codes by checking if ttl <= 0 and returning a clear error (e.g., "authorization code already expired") instead of calling s.client.Set, so you never pass a non-positive TTL to Set.backend/internal/system/database/provider/redisprovider.go (1)
76-82:⚠️ Potential issue | 🔴 CriticalSilent failure on Redis connection error leaves provider in unusable state.
When
Pingfails, the function logs an error and returns without settingredisInstance. Due tosync.Once, this is permanent—subsequent calls toGetRedisProvider()returnnil, causing nil pointer panics in all Redis store constructors (e.g.,newRedisAuthorizationCodeStoreatauth_code_redis_store.go:54-60,newRedisAuthorizationRequestStoreatauth_req_redis_store.go:44-51).Either fail-fast with panic/fatal during startup, or return an error from
GetRedisProvider().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/system/database/provider/redisprovider.go` around lines 76 - 82, The Ping failure currently logs and returns leaving redisInstance unset (so GetRedisProvider() later returns nil); change the error path in the Ping block to fail-fast instead of silently returning: after logging the Ping error (from client.Ping(...).Err()) call logger.Fatal or panic with the error (include the error details) and still attempt to close the client (logging any closeErr) so the process terminates rather than leaving redisInstance unset; this touches the Ping error handling in the same function that sets redisInstance and affects GetRedisProvider() and the redisInstance variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/internal/system/database/provider/redisprovider.go`:
- Around line 31-32: The PR adds a new Redis runtime type (DataSourceTypeRedis)
and configuration keys (database.runtime.type with values "redis" and
Redis-specific fields address, db, key_prefix) but lacks documentation; update
the docs to include a configuration reference entry describing database.runtime
(showing how to set type to "redis" and the address/db/key_prefix fields), add a
Redis connection and setup guide (connection requirements, authentication,
network/port, TLS and ACL notes), and provide migration guidance for switching
runtime stores between SQL and Redis (data export/import, compatibility caveats,
and recommended steps) so the new runtime is fully documented for users.
---
Duplicate comments:
In `@backend/internal/oauth/oauth2/authz/auth_code_redis_store.go`:
- Around line 76-79: The code currently uses ttl :=
time.Until(authzCode.ExpiryTime) and then calls s.client.Set(..., ttl) which
with a negative ttl will remove expiry in go-redis v9; update the logic in the
auth code store (around authzCode.ExpiryTime, time.Until, s.client.Set and
s.authCodeKey(authzCode.Code)) to guard against already-expired codes by
checking if ttl <= 0 and returning a clear error (e.g., "authorization code
already expired") instead of calling s.client.Set, so you never pass a
non-positive TTL to Set.
In `@backend/internal/system/database/provider/dbprovider.go`:
- Around line 169-173: initializeAllClients() is eagerly calling
initializeClient() for the runtime datasource even when runtimeDBConfig.Type is
redis, causing spurious "Failed to initialize runtime database client" errors;
update the logic around the call that uses runtimeDBConfig (runtimeDBConfig :=
config.GetThunderRuntime().Config.Database.Runtime) so you only call
initializeClient(&d.runtimeClient, runtimeDBConfig, dbNameRuntime) when the
runtime DB type is not redis (e.g., check runtimeDBConfig.Type or equivalent
enum/const), otherwise skip initialization for the runtime client and optionally
log a debug/info message indicating Redis is used.
In `@backend/internal/system/database/provider/redisprovider.go`:
- Around line 76-82: The Ping failure currently logs and returns leaving
redisInstance unset (so GetRedisProvider() later returns nil); change the error
path in the Ping block to fail-fast instead of silently returning: after logging
the Ping error (from client.Ping(...).Err()) call logger.Fatal or panic with the
error (include the error details) and still attempt to close the client (logging
any closeErr) so the process terminates rather than leaving redisInstance unset;
this touches the Ping error handling in the same function that sets
redisInstance and affects GetRedisProvider() and the redisInstance variable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3fe11a6c-b6f7-4902-95ff-ebae097188aa
⛔ Files ignored due to path filters (2)
backend/tests/mocks/database/providermock/RedisProviderCloser_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/database/providermock/RedisProviderInterface_mock.gois excluded by!**/*_mock.go
📒 Files selected for processing (13)
backend/cmd/server/repository/resources/conf/default.jsonbackend/internal/attributecache/init.gobackend/internal/attributecache/redis_store.gobackend/internal/authn/passkey/init.gobackend/internal/authn/passkey/redis_store.gobackend/internal/flow/flowexec/init.gobackend/internal/flow/flowexec/redis_store.gobackend/internal/oauth/oauth2/authz/auth_code_redis_store.gobackend/internal/oauth/oauth2/authz/auth_req_redis_store.gobackend/internal/oauth/oauth2/authz/init.gobackend/internal/system/config/config.gobackend/internal/system/database/provider/dbprovider.gobackend/internal/system/database/provider/redisprovider.go
✅ Files skipped from review due to trivial changes (3)
- backend/cmd/server/repository/resources/conf/default.json
- backend/internal/system/config/config.go
- backend/internal/flow/flowexec/redis_store.go
🚧 Files skipped from review as they are similar to previous changes (4)
- backend/internal/authn/passkey/init.go
- backend/internal/attributecache/init.go
- backend/internal/flow/flowexec/init.go
- backend/internal/attributecache/redis_store.go
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2218 +/- ##
==========================================
- Coverage 89.51% 89.49% -0.03%
==========================================
Files 911 919 +8
Lines 60145 60578 +433
==========================================
+ Hits 53841 54212 +371
- Misses 4683 4722 +39
- Partials 1621 1644 +23
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
KaveeshaPiumini
force-pushed
the
refactor
branch
from
ae6e88f to
7049abc
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/internal/attributecache/redis_store.go`:
- Around line 42-47: Add documentation under docs/content/guides/ describing how
to configure Redis as the runtime DB used by the new Redis-backed attribute
cache (referenced by newRedisAttributeCacheStore and
config.GetThunderRuntime()), including required config keys
database.runtime.type=redis, database.runtime.address, database.runtime.db,
database.runtime.key_prefix and runtime ACL credentials (username/password) for
auth; include a config reference block showing example YAML entries, a short
setup/connection troubleshooting section, and add a link from the main
deployment/config index page to this new guide.
In `@backend/internal/oauth/oauth2/authz/auth_req_redis_store.go`:
- Around line 35-50: Add documentation describing the new runtime Redis
configuration used by the OAuth stores: document database.runtime.type: redis
and its datasource keys (address, db, key_prefix, username, password) and how
provider.RedisProviderInterface yields client/keyPrefix; explain how
redisAuthorizationRequestStore (and newRedisAuthorizationRequestStore) uses
keyPrefix, deploymentID and validityPeriod (10m TTL) to namespace/stored
authorization requests and codes, note TTL semantics, ACL usage via
username/password, and update or add a guide under docs/content/guides/ (e.g.,
runtime DB configuration) describing configuration examples and the impact on
authorization request/code behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 71f1feac-b32c-4dca-aafe-a84d0a72e3c2
⛔ Files ignored due to path filters (2)
backend/tests/mocks/database/providermock/RedisProviderCloser_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/database/providermock/RedisProviderInterface_mock.gois excluded by!**/*_mock.go
📒 Files selected for processing (13)
backend/cmd/server/repository/resources/conf/default.jsonbackend/internal/attributecache/init.gobackend/internal/attributecache/redis_store.gobackend/internal/authn/passkey/init.gobackend/internal/authn/passkey/redis_store.gobackend/internal/flow/flowexec/init.gobackend/internal/flow/flowexec/redis_store.gobackend/internal/oauth/oauth2/authz/auth_code_redis_store.gobackend/internal/oauth/oauth2/authz/auth_req_redis_store.gobackend/internal/oauth/oauth2/authz/init.gobackend/internal/system/config/config.gobackend/internal/system/database/provider/dbprovider.gobackend/internal/system/database/provider/redisprovider.go
✅ Files skipped from review due to trivial changes (4)
- backend/cmd/server/repository/resources/conf/default.json
- backend/internal/system/config/config.go
- backend/internal/oauth/oauth2/authz/init.go
- backend/internal/flow/flowexec/redis_store.go
🚧 Files skipped from review as they are similar to previous changes (6)
- backend/internal/authn/passkey/init.go
- backend/internal/attributecache/init.go
- backend/internal/flow/flowexec/init.go
- backend/internal/authn/passkey/redis_store.go
- backend/internal/system/database/provider/redisprovider.go
- backend/internal/system/database/provider/dbprovider.go
Contributor
rajithacharith
left a comment
rajithacharith
left a comment
There was a problem hiding this comment.
We need to add some integration tests for this. Check the feasibility of running a redis in test suite
KaveeshaPiumini
force-pushed
the
refactor
branch
from
7049abc to
a42ea4e
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
backend/internal/authn/passkey/redisClient_mock_test.go (1)
1-6: Rename this generated mock file to snake_case and regenerate it.
redisClient_mock_test.goviolates the repository’s Go filename snake_case convention. Please update the mockery output naming (e.g.,redis_client_mock_test.go) and regenerate viamake mockeryrather than manual edits.Based on learnings: "In Go projects, enforce snake_case naming for Go source file names" and "Mocks are auto-generated via
make mockery. Do not generate or modify mock files manually."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/authn/passkey/redisClient_mock_test.go` around lines 1 - 6, The generated mock file package passkey named redisClient_mock_test.go must be renamed to snake_case and fully regenerated instead of manual edits: remove or revert changes to the current redisClient_mock_test.go, configure mockery to emit snake_case filenames (so the output becomes redis_client_mock_test.go) and run the project generator target (make mockery) to regenerate the mock for the Redis client in package passkey; ensure the regenerated file replaces the old file and commit the generated file only (do not hand-edit).backend/internal/attributecache/redis_store.go (1)
96-100: TTL retrieval error is silently ignored.If the
TTLcall fails (line 97), the error is not logged or returned. The function proceeds withresult.TTLSecondsfrom the JSON payload, which could be stale. While this is a minor inconsistency (the caller still gets usable data), consider logging the error at debug level for observability.♻️ Optional: Log TTL retrieval failure
// Reflect the actual remaining TTL from Redis. ttl, err := s.client.TTL(ctx, key).Result() - if err == nil && ttl > 0 { + if err != nil { + // Non-fatal: fall back to TTLSeconds from stored JSON + log.GetLogger().Debug("failed to get TTL from Redis, using stored value", + log.String("key", key), log.Error(err)) + } else if ttl > 0 { result.TTLSeconds = int(ttl.Seconds()) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/attributecache/redis_store.go` around lines 96 - 100, The TTL retrieval error from s.client.TTL in redis_store.go is silently ignored; update the code to log the error at debug/trace level when TTL call returns err (including the key and err) before falling back to the JSON TTL so you retain current behavior; use the store's logger (e.g., s.logger or the package logger) to emit a concise debug message referencing the key and error, while keeping the existing assignment to result.TTLSeconds when ttl > 0.backend/internal/flow/flowexec/redis_store_test.go (1)
128-129: Assert the serialized payload in theseSetexpectations.Using
mock.Anythinghere means the tests only check key and TTL. IfStoreFlowContextorUpdateFlowContextstarts writing the wrong JSON, these cases still pass. Match the argument againstsuite.serializedFlowContext(engineCtx)viamock.MatchedBy.One way to tighten the assertion
import ( + "bytes" "context" "encoding/json" "errors" "fmt" "testing" @@ func (suite *RedisFlowStoreTestSuite) TestStoreFlowContext_Success() { engineCtx, _ := suite.buildEngineContext(redisTestFlowID) expirySeconds := int64(1800) + expected := suite.serializedFlowContext(engineCtx) statusCmd := redis.NewStatusCmd(suite.ctx) - suite.mockClient.On("Set", suite.ctx, suite.flowKey, mock.Anything, + suite.mockClient.On("Set", suite.ctx, suite.flowKey, mock.MatchedBy(func(v interface{}) bool { + payload, ok := v.([]byte) + return ok && bytes.Equal(payload, expected) + }), time.Duration(expirySeconds)*time.Second).Return(statusCmd)Apply the same matcher pattern to the other
Setexpectations in this suite.Also applies to: 141-142, 208-208, 223-223, 264-264
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/flow/flowexec/redis_store_test.go` around lines 128 - 129, The Set expectations currently use mock.Anything for the serialized payload so tests only assert key and TTL; replace mock.Anything with mock.MatchedBy that compares the argument to suite.serializedFlowContext(engineCtx) (i.e., ensure the third argument passed to suite.mockClient.On("Set", suite.ctx, suite.flowKey, <arg>, time.Duration(expirySeconds)*time.Second) matches the exact JSON produced by suite.serializedFlowContext(engineCtx)); apply the same matcher pattern to the other Set expectations mentioned (lines referencing Set at the other locations) so the tests assert the serialized payload as well as key and TTL.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/internal/authn/passkey/redis_store_test.go`:
- Around line 19-37: Add documentation describing the new runtime Redis
configuration and its behavior: document the setting database.runtime.type=redis
and the required runtime datasource keys (address, db, key_prefix) and how Redis
ACL auth maps to the existing username/password fields; list and explain which
runtime stores are affected (flow contexts, auth codes/requests, passkey
sessions, attribute cache) and any behavior differences (key namespacing via
key_prefix, DB selection via db, connection/auth semantics); add this as a guide
under docs/content/guides/ (or extend the existing runtime/database
configuration page) and include cross-links to the runtime/database config page
and any relevant authentication/passkey docs.
In `@backend/internal/flow/flowexec/redis_store_test.go`:
- Around line 64-69: The test initializes the global Thunder runtime via
config.InitializeThunderRuntime and never resets it, causing order-dependent
tests; after successful init and before calling suite.Run with
new(RedisFlowStoreTestSuite), register a deferred cleanup or t.Cleanup that
calls config.ResetThunderRuntime so the singleton is reset when the test
finishes (reference InitializeThunderRuntime, ResetThunderRuntime, suite.Run,
and RedisFlowStoreTestSuite).
- Around line 53-69: The PR adds a Redis runtime backend and new config keys but
does not update documentation; add entries under the runtime DB configuration
guide (docs/content/guides/) and the config reference (docs/content/)
documenting the new settings: database.runtime.type (redis), address, db,
key_prefix, username, password (Redis ACL auth), include expected types,
defaults/example values, and a short example showing how to set these in the
runtime config so users of the RedisFlowStore (RedisFlowStoreTestSuite /
redisTestDeploymentID) can configure it correctly.
In `@backend/internal/oauth/oauth2/authz/auth_code_redis_store.go`:
- Around line 84-86: The current ttl check returns an error that embeds the raw
authorization code (authzCode.Code), which can leak secrets; change the return
to a non-sensitive message such as fmt.Errorf("authorization code already
expired") or include only a non-sensitive identifier (not the full code), i.e.,
remove authzCode.Code from the error in the ttl <= 0 branch so the code value is
never included in logs/telemetry while keeping the ttl check and control flow
intact.
- Around line 60-67: This change introduces runtime Redis configuration and
alters auth-code persistence but lacks documentation; add user-facing docs:
update docs/content/guides/ with a runtime DB setup/migration guide describing
setting database.runtime.type=redis and how to configure Redis datasource fields
(address, db, key_prefix) and ACL auth (runtime username/password), and add a
config reference page under docs/content/ listing all new keys and defaults
(including key_prefix and auth-code persistence behavior that affects
newRedisAuthorizationCodeStore and redisAuthorizationCodeStore behavior); ensure
examples and migration steps are included so users can switch auth-code
persistence to Redis safely.
In `@backend/internal/system/database/provider/redisprovider.go`:
- Around line 104-109: GetRedisClient currently returns a raw *redis.Client that
can be nil after redisProvider.Close() and causes callers to panic; change
GetRedisClient on type redisProvider to return (*redis.Client, error) (or an
explicit ok bool) and inside the method under r.mu.RLock() check r.client (or a
closed flag) and return a descriptive error when nil/closed; then update all
callers (store implementations) to handle the error and fail gracefully instead
of assuming a non-nil client.
- Around line 98-102: The Redis connection pool is never closed on shutdown;
update the shutdown sequence in main.go to call GetRedisProviderCloser().Close()
(using the RedisProviderCloser returned by GetRedisProviderCloser()) alongside
provider.GetDBProviderCloser().Close() and cacheManager.Close(), guarding the
call with a nil-check (or a no-op Close) so it only runs when
initRedisProvider()/GetRedisProviderCloser() actually created redisInstance;
reference the RedisProviderCloser type, GetRedisProviderCloser(), redisInstance
and initRedisProvider() when making the change.
---
Nitpick comments:
In `@backend/internal/attributecache/redis_store.go`:
- Around line 96-100: The TTL retrieval error from s.client.TTL in
redis_store.go is silently ignored; update the code to log the error at
debug/trace level when TTL call returns err (including the key and err) before
falling back to the JSON TTL so you retain current behavior; use the store's
logger (e.g., s.logger or the package logger) to emit a concise debug message
referencing the key and error, while keeping the existing assignment to
result.TTLSeconds when ttl > 0.
In `@backend/internal/authn/passkey/redisClient_mock_test.go`:
- Around line 1-6: The generated mock file package passkey named
redisClient_mock_test.go must be renamed to snake_case and fully regenerated
instead of manual edits: remove or revert changes to the current
redisClient_mock_test.go, configure mockery to emit snake_case filenames (so the
output becomes redis_client_mock_test.go) and run the project generator target
(make mockery) to regenerate the mock for the Redis client in package passkey;
ensure the regenerated file replaces the old file and commit the generated file
only (do not hand-edit).
In `@backend/internal/flow/flowexec/redis_store_test.go`:
- Around line 128-129: The Set expectations currently use mock.Anything for the
serialized payload so tests only assert key and TTL; replace mock.Anything with
mock.MatchedBy that compares the argument to
suite.serializedFlowContext(engineCtx) (i.e., ensure the third argument passed
to suite.mockClient.On("Set", suite.ctx, suite.flowKey, <arg>,
time.Duration(expirySeconds)*time.Second) matches the exact JSON produced by
suite.serializedFlowContext(engineCtx)); apply the same matcher pattern to the
other Set expectations mentioned (lines referencing Set at the other locations)
so the tests assert the serialized payload as well as key and TTL.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9b960bc5-256d-47aa-9381-3335c4399112
⛔ Files ignored due to path filters (4)
backend/tests/mocks/attributecachemock/redisClient_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/database/providermock/RedisProviderCloser_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/database/providermock/RedisProviderInterface_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/flow/flowexecmock/redisClient_mock.gois excluded by!**/*_mock.go
📒 Files selected for processing (27)
.github/actions/run-integration-tests/action.yml.github/workflows/pr-builder.ymlbackend/cmd/server/repository/resources/conf/default.jsonbackend/internal/attributecache/init.gobackend/internal/attributecache/redisClient_mock_test.gobackend/internal/attributecache/redis_store.gobackend/internal/attributecache/redis_store_test.gobackend/internal/authn/passkey/init.gobackend/internal/authn/passkey/redisClient_mock_test.gobackend/internal/authn/passkey/redis_store.gobackend/internal/authn/passkey/redis_store_test.gobackend/internal/flow/flowexec/init.gobackend/internal/flow/flowexec/redisClient_mock_test.gobackend/internal/flow/flowexec/redis_store.gobackend/internal/flow/flowexec/redis_store_test.gobackend/internal/oauth/oauth2/authz/authCodeRedisClient_mock_test.gobackend/internal/oauth/oauth2/authz/authReqRedisClient_mock_test.gobackend/internal/oauth/oauth2/authz/auth_code_redis_store.gobackend/internal/oauth/oauth2/authz/auth_code_redis_store_test.gobackend/internal/oauth/oauth2/authz/auth_req_redis_store.gobackend/internal/oauth/oauth2/authz/auth_req_redis_store_test.gobackend/internal/oauth/oauth2/authz/init.gobackend/internal/system/config/config.gobackend/internal/system/database/provider/dbprovider.gobackend/internal/system/database/provider/redisprovider.gobackend/internal/system/database/provider/redisprovider_test.gotests/integration/resources/scripts/setup-test-config.sh
✅ Files skipped from review due to trivial changes (9)
- backend/cmd/server/repository/resources/conf/default.json
- .github/actions/run-integration-tests/action.yml
- backend/internal/authn/passkey/redis_store.go
- backend/internal/oauth/oauth2/authz/auth_req_redis_store.go
- backend/internal/oauth/oauth2/authz/authReqRedisClient_mock_test.go
- backend/internal/flow/flowexec/redis_store.go
- backend/internal/attributecache/redisClient_mock_test.go
- backend/internal/oauth/oauth2/authz/authCodeRedisClient_mock_test.go
- backend/internal/flow/flowexec/redisClient_mock_test.go
🚧 Files skipped from review as they are similar to previous changes (4)
- backend/internal/attributecache/init.go
- backend/internal/flow/flowexec/init.go
- backend/internal/system/database/provider/dbprovider.go
- backend/internal/authn/passkey/init.go
KaveeshaPiumini
force-pushed
the
refactor
branch
2 times, most recently
from
9d4dd7c to
92a91f3
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
backend/internal/oauth/oauth2/authz/auth_code_redis_store.go (1)
84-86:⚠️ Potential issue | 🟠 MajorDo not include raw authorization code values in error messages.
authzCode.Codeis sensitive and can leak through logs/telemetry if propagated.🔒 Proposed fix
ttl := time.Until(authzCode.ExpiryTime) if ttl <= 0 { - return fmt.Errorf("authorization code already expired: %s", authzCode.Code) + return errors.New("authorization code already expired") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/oauth/oauth2/authz/auth_code_redis_store.go` around lines 84 - 86, The error message in auth_code_redis_store.go currently includes the sensitive authzCode.Code; remove the raw code from fmt.Errorf and return a non-sensitive message (e.g., fmt.Errorf("authorization code already expired")) or, if a non-sensitive identifier exists (like authzCode.ID), include only that identifier; update the return in the code path that references authzCode.Code accordingly and adjust any tests or callers that expect the old error string.backend/cmd/server/repository/resources/conf/default.json (1)
46-49:⚠️ Potential issue | 🟠 Major🔴 Documentation Required: This PR introduces a user-facing change
(e.g., new/modified API endpoint, configuration option, authentication flow, or SDK behavior)
but does not include corresponding documentation updates underdocs/.
Please update the relevant documentation before merging.Specifically, document
database.runtimeRedis configuration (type: redis,address,db,key_prefix, and ACL use viausername/password) and add migration/setup guidance for switching runtime store from SQL to Redis (suggested locations:docs/content/guides/and config reference underdocs/content/).As per coding guidelines: "If ANY of the above are detected and the PR does NOT include corresponding updates under
docs/, flag this as a major issue with the required message."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/cmd/server/repository/resources/conf/default.json` around lines 46 - 49, Add documentation entries describing the new database.runtime Redis configuration and migration steps: document the config fields (database.runtime with type: redis and keys address, db, key_prefix, conn_max_lifetime, plus ACL usage via username and password) in the config reference, and create a guide under docs/content/guides explaining how to switch the runtime store from SQL to Redis (setup steps, example config snippets, migration considerations, and any data compatibility caveats). Ensure the docs reference the exact config keys shown in default.json (database.runtime, address, db, key_prefix, conn_max_lifetime, username, password) and include a small migration/setup checklist for users.
🧹 Nitpick comments (1)
backend/internal/authn/passkey/redis_store_test.go (1)
110-110: Minor: Consider consistent error handling in test marshaling.This line ignores the marshal error, while the
serializedSessionDatahelper at line 188-189 usessuite.Require().NoError(err). For consistency and to catch unexpected test setup failures, consider using the helper method here as well.♻️ Suggested fix
- data, _ := json.Marshal(sd) + data := suite.serializedSessionData(sd)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/authn/passkey/redis_store_test.go` at line 110, Replace the unchecked json.Marshal call for sd with the same error-asserting approach used elsewhere: either call the existing helper serializedSessionData(sd) to get the marshaled bytes, or change data, _ := json.Marshal(sd) to capture the error and assert it with suite.Require().NoError(err); this touches the test that currently uses the variable sd and should mirror the serializedSessionData helper behavior to ensure test setup failures are caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/internal/attributecache/redis_store_test.go`:
- Around line 101-115: TestGetAttributeCache_Success currently marshals a full
AttributeCache struct but CreateAttributeCache only persists cache.Attributes;
update the test to mirror the real persisted shape by marshaling only
suite.testCache.Attributes (or alternatively add a create→get round-trip using
CreateAttributeCache then GetAttributeCache) and then assert the returned
result.Attributes explicitly; reference the test TestGetAttributeCache_Success
and the production functions CreateAttributeCache and GetAttributeCache
(redis_store.go handling of cache.Attributes) when making the change.
- Around line 1-232: The PR adds new runtime Redis configuration and behavior
(config keys like database.runtime.type=redis, address, db, key_prefix and
Redis-backed runtime stores) but lacks documentation updates; please add/modify
docs to cover these new config options and runtime-store behavior: update the
configuration guide and deployment/setup guide to document
database.runtime.type, address, db, key_prefix (include examples, defaults, and
recommended values), add a runtime-store behavior note explaining Redis-backed
stores, TTL semantics, and migration/compatibility considerations, and include a
short example snippet and validation instructions so users can enable Redis
runtime storage safely.
---
Duplicate comments:
In `@backend/cmd/server/repository/resources/conf/default.json`:
- Around line 46-49: Add documentation entries describing the new
database.runtime Redis configuration and migration steps: document the config
fields (database.runtime with type: redis and keys address, db, key_prefix,
conn_max_lifetime, plus ACL usage via username and password) in the config
reference, and create a guide under docs/content/guides explaining how to switch
the runtime store from SQL to Redis (setup steps, example config snippets,
migration considerations, and any data compatibility caveats). Ensure the docs
reference the exact config keys shown in default.json (database.runtime,
address, db, key_prefix, conn_max_lifetime, username, password) and include a
small migration/setup checklist for users.
In `@backend/internal/oauth/oauth2/authz/auth_code_redis_store.go`:
- Around line 84-86: The error message in auth_code_redis_store.go currently
includes the sensitive authzCode.Code; remove the raw code from fmt.Errorf and
return a non-sensitive message (e.g., fmt.Errorf("authorization code already
expired")) or, if a non-sensitive identifier exists (like authzCode.ID), include
only that identifier; update the return in the code path that references
authzCode.Code accordingly and adjust any tests or callers that expect the old
error string.
---
Nitpick comments:
In `@backend/internal/authn/passkey/redis_store_test.go`:
- Line 110: Replace the unchecked json.Marshal call for sd with the same
error-asserting approach used elsewhere: either call the existing helper
serializedSessionData(sd) to get the marshaled bytes, or change data, _ :=
json.Marshal(sd) to capture the error and assert it with
suite.Require().NoError(err); this touches the test that currently uses the
variable sd and should mirror the serializedSessionData helper behavior to
ensure test setup failures are caught.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c68a5d10-29fa-4f86-89ea-05dd00e3dd21
⛔ Files ignored due to path filters (4)
backend/tests/mocks/attributecachemock/redisClient_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/database/providermock/RedisProviderCloser_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/database/providermock/RedisProviderInterface_mock.gois excluded by!**/*_mock.gobackend/tests/mocks/flow/flowexecmock/redisClient_mock.gois excluded by!**/*_mock.go
📒 Files selected for processing (28)
.github/actions/run-integration-tests/action.yml.github/workflows/pr-builder.ymlbackend/cmd/server/repository/resources/conf/default.jsonbackend/internal/attributecache/init.gobackend/internal/attributecache/redisClient_mock_test.gobackend/internal/attributecache/redis_store.gobackend/internal/attributecache/redis_store_test.gobackend/internal/authn/passkey/init.gobackend/internal/authn/passkey/redisClient_mock_test.gobackend/internal/authn/passkey/redis_store.gobackend/internal/authn/passkey/redis_store_test.gobackend/internal/flow/flowexec/init.gobackend/internal/flow/flowexec/redisClient_mock_test.gobackend/internal/flow/flowexec/redis_store.gobackend/internal/flow/flowexec/redis_store_test.gobackend/internal/oauth/oauth2/authz/authCodeRedisClient_mock_test.gobackend/internal/oauth/oauth2/authz/authReqRedisClient_mock_test.gobackend/internal/oauth/oauth2/authz/auth_code_redis_store.gobackend/internal/oauth/oauth2/authz/auth_code_redis_store_test.gobackend/internal/oauth/oauth2/authz/auth_req_redis_store.gobackend/internal/oauth/oauth2/authz/auth_req_redis_store_test.gobackend/internal/oauth/oauth2/authz/init.gobackend/internal/system/config/config.gobackend/internal/system/database/provider/dbprovider.gobackend/internal/system/database/provider/redisprovider.gobackend/internal/system/database/provider/redisprovider_test.gobackend/internal/system/healthcheck/service/healthcheckservice.gotests/integration/resources/scripts/setup-test-config.sh
✅ Files skipped from review due to trivial changes (8)
- backend/internal/system/config/config.go
- tests/integration/resources/scripts/setup-test-config.sh
- backend/internal/authn/passkey/redis_store.go
- backend/internal/oauth/oauth2/authz/auth_req_redis_store.go
- backend/internal/oauth/oauth2/authz/authReqRedisClient_mock_test.go
- backend/internal/oauth/oauth2/authz/auth_code_redis_store_test.go
- backend/internal/flow/flowexec/redis_store.go
- backend/internal/flow/flowexec/redis_store_test.go
🚧 Files skipped from review as they are similar to previous changes (11)
- backend/internal/authn/passkey/init.go
- backend/internal/attributecache/init.go
- backend/internal/system/database/provider/redisprovider_test.go
- .github/actions/run-integration-tests/action.yml
- backend/internal/system/database/provider/dbprovider.go
- backend/internal/attributecache/redis_store.go
- .github/workflows/pr-builder.yml
- backend/internal/attributecache/redisClient_mock_test.go
- backend/internal/authn/passkey/redisClient_mock_test.go
- backend/internal/flow/flowexec/redisClient_mock_test.go
- backend/internal/oauth/oauth2/authz/authCodeRedisClient_mock_test.go
KaveeshaPiumini
force-pushed
the
refactor
branch
4 times, most recently
from
e1ed4c7 to
a78e72f
Compare
KaveeshaPiumini
force-pushed
the
refactor
branch
3 times, most recently
from
5c26f8d to
84fce7c
Compare
KaveeshaPiumini
requested review from
ThumulaPerera,
ayeshajay and
darshanasbg
as code owners
KaveeshaPiumini
force-pushed
the
refactor
branch
from
84fce7c to
db5c7cd
Compare
KaveeshaPiumini
force-pushed
the
refactor
branch
from
db5c7cd to
53580de
Compare
Contributor
Author
Done |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Add Redis as an optional runtime database store. The five runtime stores (flow contexts, authorization codes, authorization requests, passkey sessions, and attribute cache) previously required a SQLite or PostgreSQL backend. Redis is a natural fit for this data — it is entirely TTL-driven and key-value shaped — and this change allows operators to configure it as a drop-in alternative.
Approach
Configuration — Redis is configured inline under
database.runtime, the same way SQLite and PostgreSQL are, by settingtype: "redis". Three Redis-specific fields are added toDataSource:address,db, andkey_prefix. The existingusernameandpasswordfields are reused for Redis ACL authentication.Provider — A new
redisprovider.gointroducesRedisProviderInterfaceand its singleton, following the sameinit/Get/GetCloserpattern asdbprovider.go. The lifecycle manager closes the Redis connection on shutdown viaRedisProviderCloser.SQL provider guard —
dbprovider.goreturns an explicit error whenGetRuntimeDBClientis called whileruntime.typeis"redis", preventing accidental SQL usage.GetRuntimeDBTransactionerreturns a no-op transactioner for Redis (SQL transactions are not applicable).Store implementations — Each of the five runtime stores gets a Redis-backed sibling:
flowexec/redis_store.goauthz/auth_code_redis_store.goauthz/auth_req_redis_store.gopasskey/redis_store.goattributecache/redis_store.goAll stores serialize their data as JSON and rely on Redis native TTL. Key format:
{key_prefix}:runtime:{deploymentID}:{entity_type}:{id}.ConsumeAuthorizationCodeuses a Lua script to atomically transition a code fromACTIVEtoINACTIVEin a single round-trip, preserving the existing TTL viaKEEPTTL.Dispatch — Each package's
Initializefunction checksruntime.typeat startup and instantiates either the SQL or Redis store; no runtime branching occurs in the hot path.Related Issues
Related PRs
Merge after
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit
New Features
Chores
Tests