fix(instance): connect on deep-link to refresh modules#6230
Conversation
InstancePage now calls checkConnectToInstanceAction before fetching instance data so Redis modules are refreshed via GET /connect. This fixes Vector Search showing unavailable when deep-linking to /0/vector-search on preconfigured Docker databases. Fixes redis#6080
|
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 81976eb9fb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| dispatch(fetchRecommendationsAction(connectionInstanceId)) | ||
| }, | ||
| undefined, | ||
| contextInstanceId !== connectionInstanceId, |
There was a problem hiding this comment.
In the RedisStack build, RedisStackRoutes first connects, calls setConnectedInstanceId, and pushes /:id/browser; those routes are wrapped by ProtectedRoute, which redirects to Pages.home whenever connectedInstance.id is empty. On this first InstancePage mount the app context is still empty, so this reset flag is true and checkConnectToInstanceAction synchronously dispatches resetConnectedInstance before the async connect completes. That clears the id after the route was admitted, causing ProtectedRoute to immediately redirect back home and preventing /0/browser from staying open in the RedisStack path.
Useful? React with 👍 / 👎.
Only reset connectedInstance when switching away from a different DB. Redis Stack sets the connected id before routing into InstancePage; resetting it cleared ProtectedRoute and bounced back to home.
|
Addressed Codex feedback: Redis Stack sets Added a unit test covering the same-id / no-reset case. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
Reviewed by Cursor Bugbot for commit 2ac110e. Configure here.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ac110e7c7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| () => { | ||
| dispatch(fetchConnectedInstanceAction(connectionInstanceId)) | ||
| dispatch(getDatabaseConfigInfoAction(connectionInstanceId)) | ||
| dispatch(fetchConnectedInstanceInfoAction(connectionInstanceId)) | ||
| dispatch(fetchRecommendationsAction(connectionInstanceId)) | ||
| }, |
There was a problem hiding this comment.
When a user opens a bookmarked/direct instance route and GET /databases/:id/connect fails (for example a temporarily unavailable DB returning 424), this success callback never runs and no failure callback is provided. Since the old unconditional fetchConnectedInstanceAction is now only dispatched here, a fresh / :id /vector-search load leaves connectedInstance.loading as undefined, which VectorSearchPageRouter treats as loading forever; Browser children also keep connectedInstance.id empty. Please update the failure path to either redirect/show a terminal state or still fetch/reset the connected instance state.
Useful? React with 👍 / 👎.
Always fetch connected instance/config/info after connect succeeds or fails so connectedInstance.loading settles and Vector Search does not spin forever. Drop unused getRecommendations import in the spec.
|
Addressed Bugbot findings on
|
2 participants
Summary
checkConnectToInstanceActionwhen mountingInstancePage, matching the DB picker / Redis Stack entry path.GET /databases/:id/connectbefore fetching instance data used by Vector Search guards.modules: []) opened via/0/vector-searchincorrectly showing Redis Search as unavailable.Fixes #6080
Test plan
InstancePage.spec.tsx— 9/9 passedmodules: []; deep-link/0/vector-searchtriggers connect and populatessearchmoduleidx:trie), not the RQE unavailable empty state/0/browserstill loads after deep-link / picker entryNote
Medium Risk
Touches core instance mount/connect flow and when global connected state is reset; wrong reset logic could break routing or leave stale DB context, but scope is limited to InstancePage and covered by updated tests.
Overview
InstancePagenow goes throughcheckConnectToInstanceActionon mount (same path as the DB picker / Redis Stack) instead of firingfetchConnectedInstance*, config, and recommendations directly. Instance fetches run in aloadInstanceDatacallback after connect succeeds or fails soconnectedInstance.loadingalways settles—fixing Vector Search spinning when modules were empty untilGET /databases/:id/connectrefreshed them.Connected-instance reset is conditional:
resetConnectedInstanceruns only when the store already has a different connected id (DB switch). If the route id matches the current connected id (e.g. Redis Stack pre-set before navigation), reset is skipped so ProtectedRoute does not bounce to home.Tests seed per-case Redux state, expect
resetConnectedInstanceon switch, and add a case asserting no reset when already on the same instance id.Reviewed by Cursor Bugbot for commit 0ce6401. Bugbot is set up for automated code reviews on this repo. Configure here.