feat(lightspeed): add MCP servers settings panel#2582
feat(lightspeed): add MCP servers settings panel#2582debsmita1 merged 12 commits intoredhat-developer:mainfrom
Conversation
Missing ChangesetsThe following package(s) are changed by this PR but do not have a changeset:
See CONTRIBUTING.md for more information about how to add changesets. Changed Packages
|
4d9b9ea to
dec2e83
Compare
Review Summary by QodoAdd MCP servers settings panel with management UI
WalkthroughsDescription• Add MCP servers settings panel with table UI for managing servers • Implement network error handling with detailed error messages • Add Bearer token validation and formatting in MCP server validator • Support fullscreen and embedded layout modes for settings panel • Add PatternFly React Table dependency for server list display Diagramflowchart LR
A["MCP Server Validator"] -->|Enhanced Error Handling| B["Network Error Messages"]
A -->|Token Formatting| C["Bearer Token Validation"]
D["LightspeedChat Component"] -->|Settings State| E["MCP Settings Panel"]
E -->|Display| F["MCP Servers Table"]
D -->|Layout Modes| G["Fullscreen/Embedded View"]
H["LightspeedChatBoxHeader"] -->|Settings Click| E
File Changes1. workspaces/lightspeed/plugins/lightspeed-backend/src/service/mcp-server-validator.ts
|
Code Review by Qodo
1.
|
| const getDisplayStatus = (server: McpServer): ServerStatus => { | ||
| if (!server.enabled) return 'disabled'; | ||
| if (!server.hasToken) return 'tokenRequired'; | ||
| if (server.status === 'error') return 'failed'; | ||
| if (server.status === 'connected') return 'ok'; | ||
| return 'unknown'; |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| <Tbody> | ||
| {isLoading && ( | ||
| <Tr> | ||
| <Td colSpan={4}>Loading MCP servers...</Td> | ||
| </Tr> | ||
| )} | ||
| {sortedServers.map(server => { | ||
| const displayStatus = getDisplayStatus(server); | ||
| const displayDetail = getDisplayDetail(server, displayStatus); | ||
| let statusClass = classes.statusWarn; | ||
| if (displayStatus === 'ok') { | ||
| statusClass = classes.statusOk; | ||
| } else if (displayStatus === 'tokenRequired') { | ||
| statusClass = classes.statusToken; | ||
| } else if (displayStatus === 'disabled') { | ||
| statusClass = classes.statusDisabled; | ||
| } | ||
|
|
||
| return ( | ||
| <Tr key={server.id}> | ||
| <Td width={10} className={classes.toggleCell}> | ||
| {(() => { | ||
| const isUnavailable = | ||
| displayStatus === 'failed' || | ||
| displayStatus === 'tokenRequired'; | ||
| const isChecked = isUnavailable ? false : server.enabled; | ||
| const isRowSaving = Boolean(isSaving[server.name]); | ||
|
|
||
| return ( | ||
| <Switch | ||
| id={`mcp-switch-${server.id}`} | ||
| aria-label={`Toggle ${server.name}`} | ||
| isChecked={isChecked} | ||
| isDisabled={isUnavailable || isRowSaving} | ||
| onChange={(_event, checked) => { | ||
| patchServer(server.name, { enabled: checked }); | ||
| }} | ||
| /> | ||
| ); | ||
| })()} | ||
| </Td> | ||
| <Td | ||
| width={35} | ||
| className={`${classes.rowName} ${classes.nameCell}`} | ||
| > | ||
| <Typography component="span" className={classes.nameValue}> | ||
| {server.name} | ||
| </Typography> | ||
| </Td> | ||
| <Td width={40} className={classes.statusColumnCell}> | ||
| <div className={classes.statusCell}> | ||
| {getStatusIcon(displayStatus, statusClass)} | ||
| {displayStatus === 'failed' ? ( | ||
| <Tooltip | ||
| content={ | ||
| server.validationError ?? | ||
| 'Validation failed. Check server URL and token.' | ||
| } | ||
| > | ||
| <Typography | ||
| component="span" | ||
| className={classes.statusValue} | ||
| > | ||
| {displayDetail} | ||
| </Typography> | ||
| </Tooltip> | ||
| ) : ( | ||
| <Typography | ||
| component="span" | ||
| className={classes.statusValue} | ||
| > | ||
| {displayDetail} | ||
| </Typography> | ||
| )} | ||
| </div> | ||
| </Td> | ||
| <Td width={15} isActionCell style={{ textAlign: 'right' }}> | ||
| <Button | ||
| aria-label={`Edit ${server.name}`} | ||
| icon={<ModeEditOutlineOutlinedIcon fontSize="small" />} | ||
| variant="plain" | ||
| className={classes.actionButton} | ||
| onClick={onEditClick} | ||
| /> | ||
| </Td> | ||
| </Tr> | ||
| ); | ||
| })} |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| const trimmedToken = token.trim(); | ||
| const authorizationHeader = /^Bearer\s+/i.test(trimmedToken) | ||
| ? trimmedToken | ||
| : `Bearer ${trimmedToken}`; | ||
| const headers: Record<string, string> = { | ||
| 'Content-Type': 'application/json', | ||
| Authorization: `${token}`, | ||
| Authorization: authorizationHeader, | ||
| Accept: 'application/json, text/event-stream', |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| const validateServer = useCallback( | ||
| async (serverName: string) => { | ||
| const baseUrl = getBaseUrl(); | ||
| const data = await fetchJson<McpServersValidateResponse>( | ||
| `${baseUrl}/mcp-servers/${encodeURIComponent(serverName)}/validate`, | ||
| { | ||
| method: 'POST', | ||
| }, | ||
| ); | ||
|
|
||
| setServers(prev => | ||
| prev.map(server => | ||
| server.name === serverName | ||
| ? { | ||
| ...server, | ||
| status: data.status, | ||
| toolCount: data.toolCount, | ||
| validationError: | ||
| data.status === 'error' | ||
| ? (data.validation?.error ?? 'Validation failed') | ||
| : undefined, | ||
| } | ||
| : server, | ||
| ), | ||
| ); | ||
| }, | ||
| [fetchJson, getBaseUrl], | ||
| ); | ||
|
|
||
| const loadServers = useCallback(async () => { | ||
| setIsLoading(true); | ||
| setError(null); | ||
| try { | ||
| const baseUrl = getBaseUrl(); | ||
| const data = await fetchJson<McpServersListResponse>( | ||
| `${baseUrl}/mcp-servers`, | ||
| ); | ||
| const uiServers = (data.servers ?? []).map(server => toUiServer(server)); | ||
| setServers(uiServers); | ||
|
|
||
| const serversToValidate = uiServers.filter(server => server.hasToken); | ||
| void Promise.allSettled( | ||
| serversToValidate.map(async server => { | ||
| try { | ||
| await validateServer(server.name); | ||
| } catch (validationError) { | ||
| setError( | ||
| prev => | ||
| prev ?? | ||
| (validationError instanceof Error | ||
| ? validationError.message | ||
| : `Failed to validate ${server.name}`), | ||
| ); | ||
| } | ||
| }), | ||
| ); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
aa21675 to
50cbb36
Compare
This comment was marked as resolved.
This comment was marked as resolved.
| jest.clearAllMocks(); | ||
| }); | ||
|
|
||
| it('tries raw token first, then falls back to Bearer on 401/403', async () => { |
There was a problem hiding this comment.
I have added tests related to the Bearer issue on MCP Server Validation with #2671
We are always prepending Bearer, so raw token will never be used to auth with a MCP Server, you can see the comment on my PR here https://github.com/redhat-developer/rhdh-plugins/pull/2671/changes#diff-8573ea31dc3990a1eab86a5f321d47a7b95b4c7574c7092b90ca1c5203cf1178R35
455ea69 to
a213278
Compare
|
Hi @debsmita1 , thanks for the review. Good callout, currently the reason is displayed in the Status cell, but it's kind of far from the toggle button, so I added a tooltip to display the same. The Edit button will be ready in my very next PR(#2657 ) for RHIDP-12079. |
Signed-off-by: Yi Cai <yicai@redhat.com>
Signed-off-by: Yi Cai <yicai@redhat.com>
Signed-off-by: Yi Cai <yicai@redhat.com>
Signed-off-by: Yi Cai <yicai@redhat.com>
Signed-off-by: Yi Cai <yicai@redhat.com>
Signed-off-by: Yi Cai <yicai@redhat.com>
Signed-off-by: Yi Cai <yicai@redhat.com>
Signed-off-by: Yi Cai <yicai@redhat.com>
Signed-off-by: Yi Cai <yicai@redhat.com>
Signed-off-by: Yi Cai <yicai@redhat.com>
Signed-off-by: Yi Cai <yicai@redhat.com>
ef30077 to
98a7a23
Compare
|




Hey, I just made a Pull Request!
For RHIDP-12076
Changeset and documentation updates will be included in the following pr for RHIDP-12079.
✔️ Checklist
rhidp_12076.mp4
How to test
run.yamllightspeed-stack.yamlas following: