diff --git a/workspaces/lightspeed/.changeset/sixty-chefs-refuse.md b/workspaces/lightspeed/.changeset/sixty-chefs-refuse.md new file mode 100644 index 00000000000..ac660373eb8 --- /dev/null +++ b/workspaces/lightspeed/.changeset/sixty-chefs-refuse.md @@ -0,0 +1,13 @@ +--- +'@red-hat-developer-hub/backstage-plugin-lightspeed': minor +'@red-hat-developer-hub/backstage-plugin-lightspeed-backend': minor +--- + +Added the MCP servers selector/settings feature in Lightspeed with backend +integration for listing servers, per-user token updates, and validation. + +In the settings panel, users can review server status, enable or disable +eligible servers, configure personal tokens, and get inline token validation +feedback. Token validation now runs automatically after typing stops and shows +success (`Connection successful`) or error (`Authorization failed. Try again.`) +before save. diff --git a/workspaces/lightspeed/packages/app-legacy/e2e-tests/fixtures/mcpServerMocks.ts b/workspaces/lightspeed/packages/app-legacy/e2e-tests/fixtures/mcpServerMocks.ts index 29cceff652c..e439a83e990 100644 --- a/workspaces/lightspeed/packages/app-legacy/e2e-tests/fixtures/mcpServerMocks.ts +++ b/workspaces/lightspeed/packages/app-legacy/e2e-tests/fixtures/mcpServerMocks.ts @@ -14,6 +14,9 @@ * limitations under the License. */ +import type { LightspeedMessages } from '../utils/translations'; +import { formatMcpToolCountStatus } from '../utils/translations'; + /** * GET /api/lightspeed/mcp-servers body shape (see McpServersSettings McpServerResponse). * Use {@link mcpServer} for defaults; override fields per scenario. @@ -49,20 +52,33 @@ export function mcpServer( }; } +/** + * Expected MCP header “selected” line — mirrors McpServersSettings `selectedCount` useMemo + * (`enabled && !failed && !tokenRequired`). + */ +export function getExpectedMcpSelectedCountForMock( + mcpList: McpServersListMock, +): { selectedCount: number; totalCount: number } { + const totalCount = mcpList.servers.length; + const selectedCount = mcpList.servers.filter( + server => server.enabled && server.hasToken && server.status !== 'error', + ).length; + return { selectedCount, totalCount }; +} + /** * Expected Status column text for a mock row — mirrors McpServersSettings getDisplayStatus + * getDisplayDetail. */ export function getExpectedMcpStatusDetailForMock( server: McpServerMockEntry, + t: LightspeedMessages, ): string { - // Same branch order as McpServersSettings getDisplayStatus + getDisplayDetail. - if (!server.hasToken) return 'Token required'; - if (!server.enabled) return 'Disabled'; - if (server.status === 'error') return 'Failed'; - if (server.status === 'unknown') return 'Unknown'; - const suffix = server.toolCount === 1 ? 'tool' : 'tools'; - return `${server.toolCount} ${suffix}`; + if (!server.hasToken) return t['mcp.settings.status.tokenRequired']; + if (!server.enabled) return t['mcp.settings.status.disabled']; + if (server.status === 'error') return t['mcp.settings.status.failed']; + if (server.status === 'unknown') return t['mcp.settings.status.unknown']; + return formatMcpToolCountStatus(t, server.toolCount); } /** Named presets for Playwright `mockMcpServers(page, scenario)` and panel assertions. */ @@ -155,3 +171,32 @@ export const mcpServerScenarios = { export const mockedMcpServersResponse: McpServersListMock = mcpServerScenarios.default; + +/** + * Token accepted by e2e route mocks for `POST .../mcp-servers/validate` + * (credential check before PATCH). + */ +export const E2E_MCP_VALID_TOKEN = 'e2e-mcp-valid-token'; + +/** One row with `url` set so the configure modal runs credential + server validation. */ +export const tokenCredentialValidationScenario = { + servers: [ + mcpServer('credential-test-mcp', { + hasToken: false, + toolCount: 0, + status: 'unknown', + url: 'http://127.0.0.1:7777/mcp', + }), + ], +} satisfies McpServersListMock; + +/** Token required but no `url` — UI must not call credential validate (shows URL error). */ +export const tokenCredentialNoUrlScenario = { + servers: [ + mcpServer('no-url-mcp', { + hasToken: false, + toolCount: 0, + status: 'unknown', + }), + ], +} satisfies McpServersListMock; diff --git a/workspaces/lightspeed/packages/app-legacy/e2e-tests/fixtures/responses.ts b/workspaces/lightspeed/packages/app-legacy/e2e-tests/fixtures/responses.ts index 042389641d5..95e3e9afa8f 100644 --- a/workspaces/lightspeed/packages/app-legacy/e2e-tests/fixtures/responses.ts +++ b/workspaces/lightspeed/packages/app-legacy/e2e-tests/fixtures/responses.ts @@ -94,10 +94,14 @@ export const mockedShields = [ ]; export { + E2E_MCP_VALID_TOKEN, + getExpectedMcpSelectedCountForMock, getExpectedMcpStatusDetailForMock, mcpServer, mcpServerScenarios, mockedMcpServersResponse, + tokenCredentialNoUrlScenario, + tokenCredentialValidationScenario, type McpServerMockEntry, type McpServersListMock, } from './mcpServerMocks'; diff --git a/workspaces/lightspeed/packages/app-legacy/e2e-tests/lightspeed.test.ts b/workspaces/lightspeed/packages/app-legacy/e2e-tests/lightspeed.test.ts index 2c37b47b810..5f96a6f125b 100644 --- a/workspaces/lightspeed/packages/app-legacy/e2e-tests/lightspeed.test.ts +++ b/workspaces/lightspeed/packages/app-legacy/e2e-tests/lightspeed.test.ts @@ -23,7 +23,10 @@ import { botResponse, moreConversations, mockedShields, + E2E_MCP_VALID_TOKEN, mcpServerScenarios, + tokenCredentialNoUrlScenario, + tokenCredentialValidationScenario, type McpServersListMock, thinkingContent, assistantResponse, @@ -59,7 +62,9 @@ import { clickMcpServersStatusColumn, clickMcpServersNameColumn, mcpServersTableBodyRows, + type DisplayMode, } from './pages/LightspeedPage'; +import { McpConfigureTokenPage } from './pages/McpConfigureTokenPage'; import { uploadFiles, uploadAndAssertDuplicate, @@ -128,10 +133,10 @@ import { import { LightspeedMessages, evaluateMessage, + formatMcpToolCountStatus, getTranslations, } from './utils/translations'; import { runAccessibilityTests } from './utils/accessibility'; -import { skipUnlessLocales } from './utils/localeSkip'; test.describe('Lightspeed tests', () => { const botQuery = 'Please respond'; @@ -213,12 +218,13 @@ test.describe('Lightspeed tests', () => { await verifyMcpSettingsPanel(sharedPage, translations, mcpList); } - test.beforeEach(async ({}, testInfo) => { - skipUnlessLocales( - testInfo, - ['en'], - 'Chatbot MCP settings uses English-only UI strings.', - ); + /** Opens the chatbot and sets display mode before MCP configure-server token flows. */ + async function openChatbotInDisplayMode(mode: DisplayMode) { + await openChatbot(sharedPage); + await selectDisplayMode(sharedPage, translations, mode); + } + + test.beforeEach(async () => { await sharedPage.goto('/'); }); @@ -271,15 +277,15 @@ test.describe('Lightspeed tests', () => { await openChatbot(sharedPage); await openMcpSettingsPanel(sharedPage, translations); - const rows = mcpServersTableBodyRows(sharedPage); + const rows = mcpServersTableBodyRows(sharedPage, translations); await expect(rows.nth(0)).toContainText('alpha-mcp'); await expect(rows.nth(1)).toContainText('beta-mcp'); - await clickMcpServersNameColumn(sharedPage); + await clickMcpServersNameColumn(sharedPage, translations); await expect(rows.nth(0)).toContainText('beta-mcp'); await expect(rows.nth(1)).toContainText('alpha-mcp'); - await closeMcpSettingsPanel(sharedPage); + await closeMcpSettingsPanel(sharedPage, translations); }); test('Toggle works as expected', async () => { @@ -287,15 +293,156 @@ test.describe('Lightspeed tests', () => { await openChatbot(sharedPage); await openMcpSettingsPanel(sharedPage, translations); - const row = mcpServerRow(sharedPage, serverName); - await clickMcpServersStatusColumn(sharedPage); - await mcpServerToggle(sharedPage, serverName).click(); - await expect(row.getByText('Disabled', { exact: true })).toBeVisible(); + const row = mcpServerRow(sharedPage, serverName, translations); + await clickMcpServersStatusColumn(sharedPage, translations); + await mcpServerToggle(sharedPage, serverName, translations).click(); + await expect( + row.getByText(translations['mcp.settings.status.disabled'], { + exact: true, + }), + ).toBeVisible(); - await mcpServerToggle(sharedPage, serverName).click(); - await expect(row.getByText('14 tools', { exact: true })).toBeVisible(); + await mcpServerToggle(sharedPage, serverName, translations).click(); + await expect( + row.getByText(formatMcpToolCountStatus(translations, 14), { + exact: true, + }), + ).toBeVisible(); - await closeMcpSettingsPanel(sharedPage); + await closeMcpSettingsPanel(sharedPage, translations); + }); + + test.describe('Configure MCP server token', () => { + let mcpToken: McpConfigureTokenPage; + + test.beforeEach(() => { + mcpToken = new McpConfigureTokenPage(sharedPage, translations); + }); + + test('Valid token saves and row shows tools — Overlay', async () => { + await mcpToken.gotoMcpSettings( + tokenCredentialValidationScenario, + 'Overlay', + ); + + const serverName = 'credential-test-mcp'; + await mcpToken.seeRowStatus( + serverName, + translations['mcp.settings.status.tokenRequired'], + ); + + await mcpToken.openEditServer(serverName); + await mcpToken.typeToken(E2E_MCP_VALID_TOKEN); + await mcpToken.save(); + + await mcpToken.seeTokenHidden(); + await mcpToken.seeRowStatus( + serverName, + formatMcpToolCountStatus(translations, 5), + ); + + await mcpToken.closeMcpPanel(); + }); + + test('Invalid token then valid token — Dock to window', async () => { + await mcpToken.gotoMcpSettings( + tokenCredentialValidationScenario, + 'Dock to window', + ); + + const serverName = 'credential-test-mcp'; + await mcpToken.openEditServer(serverName); + + await mcpToken.typeToken('bad-token'); + await mcpToken.save(); + await mcpToken.seeMessage( + translations['mcp.settings.token.invalidCredentials'], + ); + + await mcpToken.typeToken(E2E_MCP_VALID_TOKEN); + await mcpToken.save(); + await mcpToken.seeTokenHidden(); + await mcpToken.seeRowStatus( + serverName, + formatMcpToolCountStatus(translations, 5), + ); + + await mcpToken.closeMcpPanel(); + }); + + test('Cancel discards without saving — Fullscreen', async () => { + await mcpToken.gotoMcpSettings( + tokenCredentialValidationScenario, + 'Fullscreen', + ); + + const serverName = 'credential-test-mcp'; + await mcpToken.openEditServer(serverName); + await mcpToken.typeToken('draft-token'); + await mcpToken.cancel(); + + await mcpToken.seeModalClosed(); + await mcpToken.seeRowStatus( + serverName, + translations['mcp.settings.status.tokenRequired'], + ); + + await mcpToken.closeMcpPanel(); + }); + + test('Server validation failure shows error — Overlay', async () => { + await mcpToken.gotoMcpSettings( + tokenCredentialValidationScenario, + 'Overlay', + { + failServerValidateFor: 'credential-test-mcp', + failServerValidateError: + translations['mcp.settings.token.validationFailed'], + }, + ); + + const serverName = 'credential-test-mcp'; + await mcpToken.openEditServer(serverName); + await mcpToken.typeToken(E2E_MCP_VALID_TOKEN); + await mcpToken.save(); + + await mcpToken.seeMessage( + translations['mcp.settings.token.validationFailed'], + ); + await mcpToken.cancel(); + await mcpToken.closeMcpPanel(); + }); + + test('Missing server URL shows error — Dock to window', async () => { + await mcpToken.gotoMcpSettings( + tokenCredentialNoUrlScenario, + 'Dock to window', + ); + + const serverName = 'no-url-mcp'; + await mcpToken.openEditServer(serverName); + await mcpToken.typeToken(E2E_MCP_VALID_TOKEN); + await mcpToken.save(); + + await mcpToken.seeMessage( + translations['mcp.settings.token.urlUnavailableForValidation'], + ); + await mcpToken.cancel(); + await mcpToken.closeMcpPanel(); + }); + + test('Clear token input empties PAT — Fullscreen', async () => { + await mcpToken.gotoMcpSettings( + tokenCredentialNoUrlScenario, + 'Fullscreen', + ); + + await mcpToken.openEditServer('no-url-mcp'); + await mcpToken.typeThenClearToken('e2e-draft-personal-access-token'); + + await mcpToken.cancel(); + await mcpToken.closeMcpPanel(); + }); }); }); diff --git a/workspaces/lightspeed/packages/app-legacy/e2e-tests/pages/LightspeedPage.ts b/workspaces/lightspeed/packages/app-legacy/e2e-tests/pages/LightspeedPage.ts index 7bc75b3e38b..704fbdfc50c 100644 --- a/workspaces/lightspeed/packages/app-legacy/e2e-tests/pages/LightspeedPage.ts +++ b/workspaces/lightspeed/packages/app-legacy/e2e-tests/pages/LightspeedPage.ts @@ -16,17 +16,19 @@ import { Page, expect, type Locator } from '@playwright/test'; import { + getExpectedMcpSelectedCountForMock, getExpectedMcpStatusDetailForMock, mockedMcpServersResponse, type McpServersListMock, } from '../fixtures/responses'; -import { LightspeedMessages, evaluateMessage } from '../utils/translations'; +import { + LightspeedMessages, + evaluateMessage, + formatMcpSelectedCount, +} from '../utils/translations'; export type DisplayMode = 'Overlay' | 'Dock to window' | 'Fullscreen'; -/** Menu label in LightspeedChatBoxHeader (not yet in i18n). */ -export const MCP_SETTINGS_MENU_ITEM = 'MCP settings'; - // Actions export async function openChatbot(page: Page) { await page.getByRole('button', { name: 'lightspeed-close' }).click(); @@ -82,72 +84,207 @@ export async function verifyDisplayModeMenuOptions( t: LightspeedMessages, ) { await page.getByRole('button', { name: t['aria.settings.label'] }).click(); - await expect(page.getByLabel('Chatbot', { exact: true })) - .toMatchAriaSnapshot(` - - menu: - - menuitem "${t['settings.displayMode.label']}" [disabled] - - menuitem "${t['settings.displayMode.overlay']}" - - menuitem "${t['settings.displayMode.docked']}" - - menuitem "${t['settings.displayMode.fullscreen']}" - - separator - - menu: - - menuitem "${t['settings.pinned.disable']} ${t['settings.pinned.enabled.description']}" - - menuitem "${MCP_SETTINGS_MENU_ITEM}" - `); + const settingsMenu = page + .getByRole('menu') + .filter({ + has: page.getByRole('menuitem', { + name: t['settings.displayMode.label'], + }), + }) + .first(); + + await expect(settingsMenu).toBeVisible(); + await expect( + settingsMenu.getByRole('menuitem', { + name: t['settings.displayMode.label'], + }), + ).toBeDisabled(); + await expect( + settingsMenu.getByRole('menuitem', { + name: t['settings.displayMode.overlay'], + }), + ).toBeVisible(); + await expect( + settingsMenu.getByRole('menuitem', { + name: t['settings.displayMode.docked'], + }), + ).toBeVisible(); + await expect( + settingsMenu.getByRole('menuitem', { + name: t['settings.displayMode.fullscreen'], + }), + ).toBeVisible(); + + await expect( + page.getByRole('menuitem', { + name: `${t['settings.pinned.disable']} ${t['settings.pinned.enabled.description']}`, + }), + ).toBeVisible(); + await expect( + page.getByRole('menuitem', { name: t['settings.mcp.label'] }), + ).toBeVisible(); } -// MCP settings (McpServersSettings — English strings until full i18n) +// MCP settings (McpServersSettings — strings from `mcp.settings.*` translations) export async function openMcpSettingsPanel(page: Page, t: LightspeedMessages) { await page.getByRole('button', { name: t['aria.settings.label'] }).click(); await expect( - page.getByRole('menuitem', { name: MCP_SETTINGS_MENU_ITEM }), + page.getByRole('menuitem', { name: t['settings.mcp.label'] }), ).toBeVisible(); - await page.getByRole('menuitem', { name: MCP_SETTINGS_MENU_ITEM }).click(); + await page.getByRole('menuitem', { name: t['settings.mcp.label'] }).click(); } -export async function closeMcpSettingsPanel(page: Page) { - await page.getByRole('button', { name: 'Close MCP settings' }).click(); +export async function closeMcpSettingsPanel(page: Page, t: LightspeedMessages) { + await page + .getByRole('button', { name: t['mcp.settings.closeAriaLabel'] }) + .click(); } -export function mcpServersTable(page: Page): Locator { - return page.getByLabel('MCP servers table'); +export function mcpServersTable(page: Page, t: LightspeedMessages): Locator { + return page.getByLabel(t['mcp.settings.tableAriaLabel']); } -export function mcpServersTableBodyRows(page: Page): Locator { - return mcpServersTable(page).locator('tbody tr'); +export function mcpServersTableBodyRows( + page: Page, + t: LightspeedMessages, +): Locator { + return mcpServersTable(page, t).locator('tbody tr'); } -export function mcpServerRow(page: Page, serverName: string): Locator { - return mcpServersTableBodyRows(page).filter({ hasText: serverName }); +export function mcpServerRow( + page: Page, + serverName: string, + t: LightspeedMessages, +): Locator { + return mcpServersTableBodyRows(page, t).filter({ hasText: serverName }); } -export function mcpServerToggle(page: Page, serverName: string): Locator { - return mcpServersTable(page) - .getByRole('gridcell', { name: `Toggle ${serverName}` }) +export function mcpServerToggle( + page: Page, + serverName: string, + t: LightspeedMessages, +): Locator { + return mcpServersTable(page, t) + .getByRole('gridcell', { + name: evaluateMessage( + t['mcp.settings.toggleServerAriaLabel'], + serverName, + ), + }) .locator('span'); } -export async function clickMcpServersStatusColumn(page: Page) { - await mcpServersTable(page) - .getByRole('columnheader', { name: 'Status' }) +export function mcpEditServerButton( + page: Page, + serverName: string, + t: LightspeedMessages, +): Locator { + return page.getByRole('button', { + name: evaluateMessage(t['mcp.settings.editServerAriaLabel'], serverName), + }); +} + +export function mcpPersonalAccessTokenInput(page: Page): Locator { + return page.locator('#mcp-pat-input'); +} + +/** Configure-server modal that contains the PAT field (avoids matching other dialogs). */ +export function mcpCredentialConfigureModal(page: Page): Locator { + return page + .getByRole('dialog') + .filter({ has: page.locator('#mcp-pat-input') }); +} + +/** Clear (×) control on the PAT field (`mcp.settings.token.clearAriaLabel`). */ +export function mcpClearTokenInputButton( + page: Page, + t: LightspeedMessages, +): Locator { + return mcpCredentialConfigureModal(page).getByRole('button', { + name: t['mcp.settings.token.clearAriaLabel'], + }); +} + +export function mcpConfigureModalSaveButton( + page: Page, + t: LightspeedMessages, +): Locator { + return mcpCredentialConfigureModal(page).getByRole('button', { + name: t['modal.save'], + }); +} + +export function mcpConfigureModalCancelButton( + page: Page, + t: LightspeedMessages, +): Locator { + return mcpCredentialConfigureModal(page).getByRole('button', { + name: t['modal.cancel'], + exact: true, + }); +} + +/** Validation/helper line under the PAT field after Save (matches i18n `mcp.settings.token.*` copy). */ +export function mcpConfigureModalMessage( + page: Page, + exactText: string, +): Locator { + return mcpCredentialConfigureModal(page).getByText(exactText, { + exact: true, + }); +} + +/** + * Asserts configure-server modal is ready: Close, Clear, Save, Cancel, and PAT field. + */ +export async function expectMcpConfigureModalReady( + page: Page, + t: LightspeedMessages, +) { + await expect( + mcpCredentialConfigureModal(page).getByRole('button', { + name: t['mcp.settings.closeConfigureModalAriaLabel'], + }), + ).toBeVisible(); + await expect(mcpClearTokenInputButton(page, t)).toBeVisible(); + await expect(mcpConfigureModalSaveButton(page, t)).toBeVisible(); + await expect(mcpConfigureModalCancelButton(page, t)).toBeVisible(); + await expect(mcpPersonalAccessTokenInput(page)).toBeVisible(); +} + +export async function clickMcpServersStatusColumn( + page: Page, + t: LightspeedMessages, +) { + await mcpServersTable(page, t) + .getByRole('columnheader', { name: t['mcp.settings.status'] }) .click(); } -export async function clickMcpServersNameColumn(page: Page) { - await mcpServersTable(page).getByRole('button', { name: 'Name' }).click(); +export async function clickMcpServersNameColumn( + page: Page, + t: LightspeedMessages, +) { + await mcpServersTable(page, t) + .getByRole('button', { name: t['mcp.settings.name'] }) + .click(); } -function mcpServersSettingsHeading(page: Page): Locator { - return page.getByRole('heading', { name: 'MCP servers', exact: true }); +function mcpServersSettingsHeading(page: Page, t: LightspeedMessages): Locator { + return page.getByRole('heading', { + name: t['mcp.settings.title'], + exact: true, + }); } /** Assert the MCP servers settings heading is shown or dismissed with the panel. */ export async function expectMcpServersSettingsHeading( page: Page, visible: boolean, + t: LightspeedMessages, ) { - const heading = mcpServersSettingsHeading(page); + const heading = mcpServersSettingsHeading(page, t); const assertion = visible ? expect(heading) : expect(heading).not; await assertion.toBeVisible(); } @@ -162,34 +299,42 @@ export async function verifyMcpSettingsPanel( ) { await openMcpSettingsPanel(page, t); - const table = mcpServersTable(page); + const table = mcpServersTable(page, t); await expect(table).toBeVisible(); - await expectMcpServersSettingsHeading(page, true); - await expect(page.getByText(/^\d+ of \d+ selected/)).toBeVisible(); + await expectMcpServersSettingsHeading(page, true, t); + const { selectedCount, totalCount } = + getExpectedMcpSelectedCountForMock(mcpList); + await expect( + page.getByText(formatMcpSelectedCount(t, selectedCount, totalCount), { + exact: true, + }), + ).toBeVisible(); // Scope to MCP grid: Dock/overlay leaves the catalog visible, which also has "Name" sort buttons. - await expect(table.getByRole('button', { name: 'Name' })).toBeVisible(); await expect( - table.getByRole('columnheader', { name: 'Status' }), + table.getByRole('button', { name: t['mcp.settings.name'] }), + ).toBeVisible(); + await expect( + table.getByRole('columnheader', { name: t['mcp.settings.status'] }), ).toBeVisible(); - await clickMcpServersStatusColumn(page); + await clickMcpServersStatusColumn(page, t); // Close + selected count live in the MCP header, not always inside