feat(lightspeed): add MCP servers settings panel - #2708
Conversation
ciiay
changed the title
|
Important This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior. Changed Packages
|
Review Summary by QodoAdd MCP token configuration modal with validation feedback
WalkthroughsDescription• Add MCP servers token configuration modal with debounced validation • Implement inline token validation feedback (success/error states) • Support saved token management with forget/clear functionality • Add backend integration endpoints for server listing and validation • Update documentation for MCP servers settings and token validation behavior Diagramflowchart LR
User["User opens MCP settings"] -->|clicks edit| Modal["Token config modal"]
Modal -->|enters token| Validation["Debounced validation"]
Validation -->|validates| Backend["POST /mcp-servers/validate"]
Backend -->|returns status| Feedback["Show success/error feedback"]
Feedback -->|user saves| Save["PATCH /mcp-servers/:name"]
Save -->|updates| Complete["Modal closes"]
File Changes1. workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/sessions/sessionService.test.ts
|
Code Review by Qodo
1.
|
![rhdh-qodo-merge[bot]](https://avatars.githubusercontent.com/in/1961108?s=60&v=4){kind=small}
| ? e.message | ||
| : `Failed to update ${serverName} settings`, | ||
| ); | ||
| throw e; | ||
| } finally { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
debsmita1
left a comment
debsmita1
left a comment
There was a problem hiding this comment.
When I remove the token and save, then edit it again, the field appears non-empty instead of showing the placeholder. This gives the impression that a token is already present
Screen.Recording.2026-04-07.at.12.11.47.AM.mov
| minWidth: 0, | ||
| overflow: 'hidden', | ||
| }, | ||
| mcpChatPane: { |
| width: '100%', | ||
| minWidth: 0, | ||
| }, | ||
| mcpSettingsPane: { |
|
cc @aprilma419 |
|
Hi @aprilma419 , @maysunfaisal I have a UX behavior question about token handling when an MCP server already has an admin-configured credential. In the token configuration modal, when a user clicks “Forget token” for that server:
Follow-up:
I’m asking so we can align persistence logic, validation timing, and status copy with intended UX. |
|
Q Should "Forget token" remove only the user scoped token? Yes. PATCH with {"token": null} only clears the user's row in the DB table (lightspeed_mcp_user_settings). The admin token from app-config.yaml is never touched, it's read-only from config. The backend automatically falls back to the admin token for LCS calls. Q Should we re-validate and refresh the UI? Yes. After the PATCH returns, call GET /api/lightspeed/mcp-servers to refresh. The response will have updated hasUserToken: false and the current status/toolCount. Q What should the status display? The backend already returns two separate booleans: hasToken (admin-level, from app-config) and hasUserToken (user-level, from DB). The frontend can use these to show different states:
Note: Direct MCP validation (POST /mcp-servers/:name/validate) uses the user token if present, otherwise falls back to the admin token. Q Save with empty token field? Should send {"token": null}... same effect as forget. Clears user token, falls back to admin token from the app-config. If there is no admin token either, server shows as unconfigured. |
|
This pull request adds a new top-level directory under |
Review Summary by Qodo
WalkthroughsDescription• Add MCP servers token configuration modal with debounced validation • Implement token validation feedback (success/error states) • Add backend endpoints for server listing and token validation • Update UI layout to grid-based split pane for settings panel • Add comprehensive documentation for MCP settings feature Diagramflowchart LR
A["User opens MCP Settings"] --> B["View Server List"]
B --> C["Click Edit Button"]
C --> D["Token Config Modal"]
D --> E["Enter Token"]
E --> F["Debounced Validation"]
F --> G{Validation Result}
G -->|Success| H["Show Success State"]
G -->|Error| I["Show Error State"]
H --> J["Save Token"]
I --> J
J --> K["Update Backend"]
K --> L["Close Modal"]
File Changes1. workspaces/lightspeed/plugins/lightspeed-backend/src/service/notebooks/sessions/sessionService.test.ts
|
Code Review by Qodo
1. Unhandled patchServer rejection
|
|
Sorry, I accidently closed the PR using a keyboard shortcut I didnt even know existed. |
|
Hi @aprilma419 , we still need UX decisions for display/wording:
|
|
I like it!! 👍 Tested
|
I assume the question is on the token input dialog. I’d suggest to re-validate the token only when the user hit save to avoid too many attempts on trying out the token during typing. If the validate failed, let the user know and allow them to input again.
From the status view, I don't think it is important to display whether it is admin vs user token to avoid having the status field too cluttered. We can show a line under the input field to indicate the admin token is being used when the field is empty in case the user click on edit on the token. |
There was a problem hiding this comment.
The UI breaks when the chat history sidebar is closed while the MCP settings are open, and I can also observe this behavior in the main branch. It might have slipped through in PR #2582. Please refer to the recording below
Screen.Recording.2026-04-09.at.1.03.20.AM.mov
HusneShabbir
left a comment
HusneShabbir
left a comment
There was a problem hiding this comment.
According to the epic RHIDP-11663, this is the final ticket. However, I still see that the translation for the MCP settings hasn't been covered. What's the planned timeline for this?
Screen.Recording.2026-04-09.at.12.26.49.PM.mov
|
Hi @HusneShabbir , thanks for testing out. I have updated the PR to address your comments. Now it's i18n supported and the UI issue got fixed. Please take a look 🤝 Thank you! |
|
/lgtm Thanks for the quick fix. |
Signed-off-by: Yi Cai <yicai@redhat.com>
0377a68 to
94e76f8
Compare
Signed-off-by: Yi Cai <yicai@redhat.com>
maysunfaisal
left a comment
maysunfaisal
left a comment
There was a problem hiding this comment.
lgtm in general, tried out again
|
@ciiay One suggestion I have is, if you look at the Cursor MCP Settings, they show the MCP URL when you click on edit: Maybe we can do something similar for our MCP Settings? 🤔 But nevertheless, since we can't edit MCP Server details, its okay for now and lets merge this PR. Maybe in the future when users can add their own MCP Servers via the Lightspeed UI, we can show the URL. Lightspeed backend provides the URL |
Signed-off-by: Yi Cai <yicai@redhat.com>
- Query MCP and pinned menu items from page scope for visibility Made-with: Cursor
HusneShabbir
left a comment
HusneShabbir
left a comment
There was a problem hiding this comment.
In overlay mode, the MCP credential configuration dialog box is getting covered by the MCP server-side panel on smaller screens, which doesn't happen in other modes. Could you please take a look?
Overlay Mode:
Screen.Recording.2026-04-10.at.12.27.29.PM.mov
DOC mode:
Screen.Recording.2026-04-10.at.12.27.02.PM.mov
… skip Wire MCP page objects and tests to lightspeed mcp.settings.* (and modal.*) strings via getTranslations. Add helpers for selected count and tool status text. Extend mock validate options with failServerValidateError. Remove skipUnlessLocales for Chatbot MCP settings; keep per-test navigation. Made-with: Cursor
- mcpServerMocks: inline status detail logic; derive selected count without getDisplayStatusForMock - devMode: single POST validate payload; PATCH mutates server in place Made-with: Cursor
This reverts commit ea0645e.
…ors-on-2708 test(lightspeed): locale-aware MCP settings e2e (panel + configure token)
Avoid ambiguous getByRole when modal.cancel matches a substring of other labels in the dialog. Made-with: Cursor
c69071c to
c017ea9
Compare
Signed-off-by: Yi Cai <yicai@redhat.com>
c017ea9 to
826a603
Compare
|
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
|
Hi @debsmita1 , can you take a look at this updated pr? It needs your approval to be merged. Thanks 🤝 |
debsmita1
left a comment
debsmita1
left a comment
There was a problem hiding this comment.
When mcp is disabled
When mcp is enabled
Screen.Recording.2026-04-14.at.11.49.44.PM.mov
/lgtm
5 participants
Hey, I just made a Pull Request!
For RHIDP-12079
✔️ Checklist
Test steps refer to #2582
Screen recording:
rhidp_12079.mp4