fix(workbench): do not block TimeSeries under ACL module detection failure#6232
fix(workbench): do not block TimeSeries under ACL module detection failure#6232trippyogi wants to merge 4 commits into
Conversation
…er ACL Skip the module gate when the command succeeded or modules are unknown, and fall back to HELLO/INFO module detection when MODULE LIST and COMMAND INFO are unavailable. Fixes redis#5357
Prefer HELLO for module discovery when MODULE LIST fails, and only suppress ModuleNotLoaded for successful command replies.
![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: 6eed2d8154
ℹ️ 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".
| const isSuccessfulResult = Array.isArray(result) | ||
| ? result.length > 0 && | ||
| result.every( | ||
| (item) => item?.status === CommandExecutionStatus.Success, | ||
| ) | ||
| : result?.status === CommandExecutionStatus.Success |
There was a problem hiding this comment.
When Workbench renders grouped results, QueryCardCliGroupResult calls CommonErrorResponse(item.id, item.command, item.response), so this new success check sees the Redis reply itself rather than the item.status. For a successful module command whose reply is an array, such as TS.RANGE, result.every(item => item?.status === 'success') is false because the array entries are data points, so an ACL connection with an empty/unknown modules list still replaces the successful grouped reply with ModuleNotLoaded. Pass the grouped command status or treat the wrapper result consistently before running checkUnsupportedModuleCommand.
Useful? React with 👍 / 👎.
Grouped results were handing CommonErrorResponse the raw Redis reply, so successful TS.RANGE arrays looked like failures under ACL. Pass the CommandExecutionResult wrapper so item.status is visible.
1 participant
Summary
Fixes #5357
Test plan
Note
Medium Risk
Changes module discovery fallbacks and Workbench error heuristics; behavior shifts for restricted ACL connections but successful commands are preserved and failed module commands should still show guidance.
Overview
Fixes Workbench showing Time series unavailable for ACL users who can run
TS.RANGEbut lackMODULE LIST/COMMAND INFO, so successful TimeSeries replies are no longer replaced by ModuleNotLoaded when execution status is success.Backend: After
COMMAND INFOprobing fails, module discovery now falls back toHELLO(preferred) and then INFO, with normalization that ignores collapsed INFOmodule:lines. TimeSeries probe commands now includets.rangeandts.revrange.UI:
CommonErrorResponseonly runs module-missing checks when the result is not successful; group-mode rendering passes{ status, response }so array replies (e.g.TS.RANGE) are not mistaken for errors.Reviewed by Cursor Bugbot for commit ef08c14. Bugbot is set up for automated code reviews on this repo. Configure here.