feat: add Redis Sentinel and TLS support in TrueForge - #765
Merged
Merged
Conversation
thesujai
requested review from
bhaveshpatel640,
chiragjn,
debajyoti-truefoundry,
heerambavi1998 and
sr07asthana
as code owners
September 16, 2026 12:25
🦋 Changeset detectedLatest commit: 4b04f86 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…ntinel and add TypeScript error handling
…and improve error handling in helpers
thesujai
force-pushed
the
feat/redis-connection-options
branch
from
September 22, 2026 06:15
49bd3b1 to
55f843d
Compare
sr07asthana
reviewed
Sep 22, 2026
sr07asthana
reviewed
Sep 22, 2026
sr07asthana
reviewed
Sep 22, 2026
sr07asthana
reviewed
Sep 22, 2026
sr07asthana
reviewed
Sep 22, 2026
sr07asthana
reviewed
Sep 22, 2026
sr07asthana
reviewed
Sep 22, 2026
- redis.enabled now conflicts with any external Redis signal (url/host/ sentinel), not just externalRedis.enabled, so a configured external Redis can no longer be silently shadowed by the bundled one. - Fail when tls.enabled is set with a redis:// url: node-redis throws a TypeError at client creation on the scheme mismatch, which would otherwise surface as a crash loop. - Fail when auth or a non-zero db accompany a url without Sentinel: the app prefers the url and silently drops them, connecting unauthenticated. - Normalize tls.enabled / sentinel.enabled through toString so quoted "true" values enable the feature instead of silently rendering "false". Signed-off-by: Raman Tehlan <ramantehlan@gmail.com>
… validation and improved error handling
…nto feat/redis-connection-options Co-authored-by: Cursor <cursoragent@cursor.com>
… for controller/migrate
- pass tls.serverName through fromStringOrValueFrom like the other TLS fields so a valueFrom ref renders correctly - bump chart version: 0.2.2 is already pinned by the umbrella chart Signed-off-by: Raman Tehlan <ramantehlan@gmail.com>
ramantehlan
enabled auto-merge (squash)
September 22, 2026 13:16
ramantehlan
approved these changes
Sep 22, 2026
chiragjn
reviewed
Sep 22, 2026
chiragjn
reviewed
Sep 22, 2026
chiragjn
reviewed
Sep 22, 2026
chiragjn
reviewed
Sep 22, 2026
chiragjn
reviewed
Sep 22, 2026
chiragjn
reviewed
Sep 22, 2026
chiragjn
reviewed
Sep 22, 2026
chiragjn
reviewed
Sep 22, 2026
chiragjn
reviewed
Sep 22, 2026
Collapse host/url into standalone URL mode, rename RedisPeerClient to RedisClient, require externalRedis.enabled in Helm, and tidy TLS typing. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit bec846a. Configure here.
Bare addresses like ::1 produced invalid redis:// URLs after host mode was collapsed to a URL. Co-authored-by: Cursor <cursoragent@cursor.com>
chiragjn
reviewed
Sep 22, 2026
chiragjn
reviewed
Sep 22, 2026
chiragjn
reviewed
Sep 22, 2026
chiragjn
reviewed
Sep 22, 2026
chiragjn
approved these changes
Sep 22, 2026
chiragjn
left a comment
chiragjn
left a comment
Member
There was a problem hiding this comment.
Keeping it approved, I will check the helpers.tpl separately
TLS is applied through node-redis socket options (REDIS_TLS_*), not the url scheme, and host mode builds a redis:// url itself. Failing on redis:// + tls.enabled rejected a working config and contradicted the host path, which allows the same combination. Signed-off-by: Raman Tehlan <ramantehlan@gmail.com>
… and add unit tests
…on for improved readability
The app reads REDIS_TLS_REJECT_UNAUTHORIZED (default true) but the chart never emitted it, so a Redis with a self-signed cert and no obtainable CA could only be reached through server.extraEnv. Key presence is tested rather than piping through default, which would turn an explicit false back into true. Also records the tls/sentinel fields that accept valueFrom in the README. Signed-off-by: Raman Tehlan <ramantehlan@gmail.com>
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
4 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.
Summary
feat: add Redis Sentinel and TLS support in TrueForge
Closes AGE-2071
Checklist
pnpm build,pnpm test,pnpm typecheck,pnpm lint:ci, andpnpm format:checkpass locallypackages/trueforge-sdk,python/trueforge_sdk,.github/fern/openapi/openapi.json,docs/openapi.json) — fork PRs omit SDK regen; maintainers regenerate after merge.env.exampleupdated if configuration or behavior changedNote
High Risk
Changes how every peered replica connects to Redis (TLS, Sentinel, transport rules) and how executor peering/subscribe behaves, which can break multi-replica cancel/streaming if misconfigured.
Overview
Adds Redis Sentinel and TLS for distributed mode, replacing the single
REDIS_URLknob with a resolvedREDIS_CONNECTIONtransport (URL,REDIS_HOST+ auth/DB, or Sentinel) that fail-fast when env vars conflict. Controller/migrate can boot without Redis at config load; the server still requires a transport when it connects.Runtime wiring connects standalone or Sentinel (with TLS socket options, connect/ping timeouts, and Sentinel connect rebuild/retry), uses a duplicate subscriber for standalone but the shared client for Sentinel pub/sub, and dedupes Redis clients on shutdown. Request-reply peering keeps heartbeats running through transient subscriber errors so reconnect can re-subscribe.
The Helm chart (0.2.3) expands
externalRedis(enabled, url/host/sentinel/tls/auth) and maps it to the newREDIS_*env vars, with template validation for mutually exclusive modes. Docs,.env.example, and abuildRedisStandaloneUrlunit test accompany the change.Reviewed by Cursor Bugbot for commit 4b04f86. Bugbot is set up for automated code reviews on this repo. Configure here.