Conversation
The database index had to be typed by hand, both in the connection form and again in the browser header, with no way to tell how many databases the server actually has. Both places now use a select instead: - the connection form's 'Select Logical Database' field is a select rather than a numeric input - the browser header replaces its inline number editor with the same select, so the index can be changed without reopening the connection form The options come from the database count reported by the connection test, so they match the server. Until a connection has been tested the default of 16 databases is used as a fallback.
|
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…nto the connected instance Two problems reported by the automated review of the logical database picker: - `createDatabaseModel` resolves the credentials onto the model it returns and leaves the caller object untouched, so `getConnectionInfo` built its client from an object without the resolved credentials (Azure Entra ID, access keys) and the info read failed silently. - `DbIndex` took its database count from `connectedInstanceInfoSelector`, which describes the instance that is actually connected. The picker inherited the count of a previously connected server instead of falling back to 16, and testing a new server overwrote the connected instance info. The test result now lives in its own `testedInstanceInfo` slice field, is cleared whenever a connection form opens, and `DbIndex` reads only that.
xiajingg
force-pushed
the
feat/db-index-dropdown
branch
from
September 29, 2026 03:18
10d32b1 to
dbe4829
Compare
…d range Follow-up on the review of the previous fix: - The picker kept the selected index when the option list shrank after a connection test, so the form could submit a database the server does not have. The value is now clamped to the available options. - A cluster reports no database count, so the picker fell back to the 16 offered before testing. Redis Cluster only implements db0, so it now offers exactly one option. - The header used the reported count as a hard option list. That count can be a keyspace-derived lower bound, which hid the index the instance is connected to; the connected index is always kept in the list. Also updates InstanceHeader.spec.tsx, which still targeted the removed inline number editor.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Reviewed by Cursor Bugbot for commit 1927f6f. Configure here.
| if (selected >= databasesCount) { | ||
| formik.setFieldValue('db', databasesCount - 1) | ||
| } | ||
| }, [databasesCount]) |
There was a problem hiding this comment.
Picker silently overwrites the selected index
Medium Severity
The new clamp rewrites db whenever it is at or above databasesCount. That count is a keyspace lower bound when CONFIG GET databases is denied, so a successful test (or the pre-test fallback of 16) can silently replace a chosen or cloned index with a smaller one. The header already keeps the current index in range; the form does not.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 1927f6f. Configure here.
This branch has not been deployed
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
2 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.
What
The logical database index had to be typed by hand — once in the connection
form, and again in the browser header — with no indication of how many
databases the server actually has.
Both places now use a select:
of a numeric input
so the index can be changed without reopening the connection form
The options come from the database count reported by the connection test
(
databasesin the connection info), so the list matches the server. Until aconnection has been tested, the default of 16 databases is used as a fallback.
Why
Typing the index makes it easy to pick a database that does not exist, and the
UI gives no hint of the valid range.
Changes
api/.../database.service.ts—testConnection()now returns theconnection info instead of
void, so the UI learns the real database count.A private
getConnectionInfo()does the work.api/.../database.controller.ts— the two connection-test endpoints returnthe info.
ui/.../form/DbIndex.tsx—NumericInput→RiSelect.ui/.../instance-header/InstanceHeader.tsx— inline number editor →RiSelect.ui/.../instancesService.ts,ui/.../slices/instances/instances.ts—plumb the connection info through.
Tests
DbIndex.spec.tsx(new, 5 cases): the control is a select rather than aninput, the option count follows the reported database count, and the
fallback is 16 when no connection has been tested.
slices/tests/instances— 176 tests pass.npm run i18n:check,npm run lint:ui,npm run type-check— clean.Screenshots
Connection form with the picker open (db0…db15, no db16):
The same control in the browser header (top left):
Note
Medium Risk
Connection-test API response shape changes and each successful test may open an extra Redis client; UI/state changes affect db index selection on connect and in the header but not auth or persistence logic.
Overview
Connection test endpoints now return Redis instance info (including logical
databasescount) instead of an empty success, via a follow-upgetConnectionInforead on a short-lived client aftercreateDatabaseModelsucceeds; failures to read info are swallowed and sentinel “master required” still yieldsnull.The UI replaces free-form numeric db index fields with
RiSelectpickers in the manual connection form (DbIndex) and browser instance header, building options from the tested/connected instance’s database count (default 16 before test, 1 for cluster). Redux addstestedInstanceInfo(cleared on new tests and when opening the connection form) so unsaved connection tests don’t mix with the connected instance’sinstanceInfo.Reviewed by Cursor Bugbot for commit 1927f6f. Bugbot is set up for automated code reviews on this repo. Configure here.