From 6d368f320b07b31499e0eba9a23f50b3baf93f23 Mon Sep 17 00:00:00 2001 From: "@daniel-lxs" <57051444+daniel-lxs@users.noreply.github.com> Date: Tue, 3 Nov 2026 05:41:58 +0000 Subject: [PATCH 1/2] feat(web): test custom MCP connections from tool management --- apps/docs/integrations/custom-mcp-servers.mdx | 14 ++ .../settings/CustomMcpServers.client.test.tsx | 141 ++++++++++++++++++ .../components/settings/CustomMcpServers.tsx | 27 +++- 3 files changed, 179 insertions(+), 3 deletions(-) diff --git a/apps/docs/integrations/custom-mcp-servers.mdx b/apps/docs/integrations/custom-mcp-servers.mdx index d99f0e69c4..b4a57d9e63 100644 --- a/apps/docs/integrations/custom-mcp-servers.mdx +++ b/apps/docs/integrations/custom-mcp-servers.mdx @@ -48,6 +48,20 @@ real credentials per request. This means: and a task cannot run an **Always ask** tool until its session owner allows that exact call. +## Test a remote connection + +After saving a remote server, open **Manage tools** on **Integrations** (or in +Personal Settings for a private server), then select **Test connection**. Each +test makes a fresh request using the saved URL and authentication and reports +whether Roomote can retrieve the server's tool catalog. A server with no tools +can still pass the connection test. + +If a test fails, review the displayed error, correct the server's settings or +reconnect OAuth if needed, and test again. Testing does not execute any tool, +change tool approval choices, or enable the server. It checks the remote +connection from Roomote, not execution inside a task sandbox. Local stdio +servers must be checked in a sandbox instead. + ## OAuth For servers that support the MCP authorization spec, pick **OAuth** and use diff --git a/apps/web/src/components/settings/CustomMcpServers.client.test.tsx b/apps/web/src/components/settings/CustomMcpServers.client.test.tsx index f8019f950f..3d828b2ad6 100644 --- a/apps/web/src/components/settings/CustomMcpServers.client.test.tsx +++ b/apps/web/src/components/settings/CustomMcpServers.client.test.tsx @@ -451,6 +451,147 @@ describe('useCustomMcpServers', () => { ).toBeNull(); }); + it('tests a fresh cached connection, coalesces clicks, and accepts a valid empty catalog', async () => { + state.servers = [buildServer()]; + toolsRequestMock.mockResolvedValueOnce({ + tools: [{ name: 'cached_tool', description: null, enabled: true }], + }); + const { queryClient } = renderHarness(); + queryClient.setQueryDefaults(['customMcpServers', 'listTools'], { + staleTime: Infinity, + }); + fireEvent.click( + await screen.findByRole('button', { + name: 'Manage internal-tools tools', + }), + ); + await screen.findByRole('group', { name: 'Approval mode for cached_tool' }); + let finish!: (result: { tools: typeof state.tools }) => void; + toolsRequestMock.mockImplementationOnce( + () => + new Promise((resolve) => { + finish = resolve; + }), + ); + const test = screen.getByRole('button', { + name: 'Test internal-tools connection', + }); + fireEvent.click(test); + await waitFor(() => expect(toolsRequestMock).toHaveBeenCalledTimes(2)); + expect(test).toBeDisabled(); + expect(test).toHaveAttribute('aria-busy', 'true'); + expect( + screen.getByRole('group', { name: 'Approval mode for cached_tool' }), + ).toBeInTheDocument(); + fireEvent.click(test); + expect(toolsRequestMock).toHaveBeenCalledTimes(2); + finish({ tools: [] }); + await waitFor(() => expect(test).toBeEnabled()); + expect(toastSuccessMock).toHaveBeenCalledWith( + 'Connection test passed for internal-tools.', + ); + expect( + screen.queryByRole('group', { name: 'Approval mode for cached_tool' }), + ).toBeNull(); + expect(screen.queryByRole('alert')).toBeNull(); + }); + + it('reports repeated test failures and tests again after the cause is removed', async () => { + state.servers = [buildServer()]; + toolsRequestMock.mockResolvedValueOnce({ tools: [] }); + renderHarness(); + fireEvent.click( + await screen.findByRole('button', { + name: 'Manage internal-tools tools', + }), + ); + const test = await screen.findByRole('button', { + name: 'Test internal-tools connection', + }); + await waitFor(() => expect(test).toBeEnabled()); + for (const message of ['Unavailable', 'Still unavailable']) { + toolsRequestMock.mockRejectedValueOnce(new Error(message)); + fireEvent.click(test); + await waitFor(() => + expect(screen.getByRole('alert')).toHaveTextContent(message), + ); + expect(toastErrorMock).toHaveBeenLastCalledWith( + 'Connection test failed for internal-tools.', + ); + expect(test).toBeEnabled(); + } + toolsRequestMock.mockResolvedValueOnce({ + tools: [{ name: 'recovered_tool', description: null, enabled: true }], + }); + fireEvent.click(test); + expect( + await screen.findByRole('group', { + name: 'Approval mode for recovered_tool', + }), + ).toBeInTheDocument(); + expect(screen.queryByRole('alert')).toBeNull(); + expect(toastSuccessMock).toHaveBeenCalledWith( + 'Connection test passed for internal-tools.', + ); + expect(toolsRequestMock).toHaveBeenCalledTimes(4); + expect(setEnabledMock).not.toHaveBeenCalled(); + expect(approvals.setMode).not.toHaveBeenCalled(); + }); + + it('keeps a late connection-test response and its feedback identified with the tested server', async () => { + const first = buildServer(); + const second = buildServer({ + id: '00000000-0000-4000-8000-000000000302', + name: 'second-tools', + }); + state.servers = [first, second]; + toolsRequestMock.mockResolvedValueOnce({ tools: [] }); + renderHarness(); + fireEvent.click( + await screen.findByRole('button', { + name: 'Manage internal-tools tools', + }), + ); + const test = await screen.findByRole('button', { + name: 'Test internal-tools connection', + }); + await waitFor(() => expect(test).toBeEnabled()); + let finish!: (result: { tools: typeof state.tools }) => void; + toolsRequestMock.mockImplementationOnce( + () => + new Promise((resolve) => { + finish = resolve; + }), + ); + fireEvent.click(test); + await waitFor(() => expect(test).toBeDisabled()); + fireEvent.click(screen.getByRole('button', { name: 'Close' })); + toolsRequestMock.mockResolvedValueOnce({ + tools: [{ name: 'second_tool', description: null, enabled: true }], + }); + fireEvent.click( + await screen.findByRole('button', { name: 'Manage second-tools tools' }), + ); + await screen.findByRole('group', { name: 'Approval mode for second_tool' }); + finish({ + tools: [{ name: 'first_tool', description: null, enabled: true }], + }); + await waitFor(() => + expect(toastSuccessMock).toHaveBeenCalledWith( + 'Connection test passed for internal-tools.', + ), + ); + expect( + screen.getByRole('group', { name: 'Approval mode for second_tool' }), + ).toBeInTheDocument(); + expect( + screen.queryByRole('group', { name: 'Approval mode for first_tool' }), + ).toBeNull(); + expect( + screen.getByRole('button', { name: 'Test second-tools connection' }), + ).toBeEnabled(); + }); + it('keeps a pending retry and its late response scoped to the selected server', async () => { const first = buildServer(); const second = buildServer({ diff --git a/apps/web/src/components/settings/CustomMcpServers.tsx b/apps/web/src/components/settings/CustomMcpServers.tsx index 63a3a9f6fa..f3e85d66b3 100644 --- a/apps/web/src/components/settings/CustomMcpServers.tsx +++ b/apps/web/src/components/settings/CustomMcpServers.tsx @@ -785,13 +785,34 @@ function CustomToolManagementDialog({ setRetryError((current) => (current === error ? null : current)); } }; + const testConnection = async () => { + if (!server || toolsQuery.isFetching) return; + const result = await toolsQuery.refetch({ cancelRefetch: false }); + if (result.isError) { + toast.error(`Connection test failed for ${server.name}.`); + } else if (result.isSuccess) { + toast.success(`Connection test passed for ${server.name}.`); + } + }; return ( - - Manage tools for {server?.name ?? 'integration'} - +
+ + Manage tools for {server?.name ?? 'integration'} + + +
Choose how the model can use tools from this integration. From 16df38eebd20e795784579e96d98f474ccb33b4c Mon Sep 17 00:00:00 2001 From: "@daniel-lxs" <57051444+daniel-lxs@users.noreply.github.com> Date: Tue, 3 Nov 2026 05:50:45 +0000 Subject: [PATCH 2/2] fix(web): bind connection test results to the tested server --- .../settings/CustomMcpServers.client.test.tsx | 135 +++++++++++------- .../components/settings/CustomMcpServers.tsx | 14 +- 2 files changed, 93 insertions(+), 56 deletions(-) diff --git a/apps/web/src/components/settings/CustomMcpServers.client.test.tsx b/apps/web/src/components/settings/CustomMcpServers.client.test.tsx index 3d828b2ad6..65beec1fc0 100644 --- a/apps/web/src/components/settings/CustomMcpServers.client.test.tsx +++ b/apps/web/src/components/settings/CustomMcpServers.client.test.tsx @@ -538,59 +538,90 @@ describe('useCustomMcpServers', () => { expect(approvals.setMode).not.toHaveBeenCalled(); }); - it('keeps a late connection-test response and its feedback identified with the tested server', async () => { - const first = buildServer(); - const second = buildServer({ - id: '00000000-0000-4000-8000-000000000302', - name: 'second-tools', - }); - state.servers = [first, second]; - toolsRequestMock.mockResolvedValueOnce({ tools: [] }); - renderHarness(); - fireEvent.click( - await screen.findByRole('button', { - name: 'Manage internal-tools tools', - }), - ); - const test = await screen.findByRole('button', { - name: 'Test internal-tools connection', - }); - await waitFor(() => expect(test).toBeEnabled()); - let finish!: (result: { tools: typeof state.tools }) => void; - toolsRequestMock.mockImplementationOnce( - () => - new Promise((resolve) => { - finish = resolve; + it.each([true, false])( + 'keeps opposite late connection-test outcomes scoped to the tested server (first fails: %s)', + async (firstFails) => { + const first = buildServer(); + const second = buildServer({ + id: '00000000-0000-4000-8000-000000000302', + name: 'second-tools', + }); + state.servers = [first, second]; + toolsRequestMock.mockResolvedValueOnce({ tools: [] }); + renderHarness(); + fireEvent.click( + await screen.findByRole('button', { + name: 'Manage internal-tools tools', }), - ); - fireEvent.click(test); - await waitFor(() => expect(test).toBeDisabled()); - fireEvent.click(screen.getByRole('button', { name: 'Close' })); - toolsRequestMock.mockResolvedValueOnce({ - tools: [{ name: 'second_tool', description: null, enabled: true }], - }); - fireEvent.click( - await screen.findByRole('button', { name: 'Manage second-tools tools' }), - ); - await screen.findByRole('group', { name: 'Approval mode for second_tool' }); - finish({ - tools: [{ name: 'first_tool', description: null, enabled: true }], - }); - await waitFor(() => - expect(toastSuccessMock).toHaveBeenCalledWith( - 'Connection test passed for internal-tools.', - ), - ); - expect( - screen.getByRole('group', { name: 'Approval mode for second_tool' }), - ).toBeInTheDocument(); - expect( - screen.queryByRole('group', { name: 'Approval mode for first_tool' }), - ).toBeNull(); - expect( - screen.getByRole('button', { name: 'Test second-tools connection' }), - ).toBeEnabled(); - }); + ); + const test = await screen.findByRole('button', { + name: 'Test internal-tools connection', + }); + await waitFor(() => expect(test).toBeEnabled()); + let finish!: (result: { tools: typeof state.tools }) => void; + let fail!: (error: Error) => void; + toolsRequestMock.mockImplementationOnce( + () => + new Promise((resolve, reject) => { + finish = resolve; + fail = reject; + }), + ); + fireEvent.click(test); + await waitFor(() => expect(test).toBeDisabled()); + fireEvent.click(screen.getByRole('button', { name: 'Close' })); + if (firstFails) { + toolsRequestMock.mockResolvedValueOnce({ + tools: [{ name: 'second_tool', description: null, enabled: true }], + }); + } else { + toolsRequestMock.mockRejectedValueOnce(new Error('Second unavailable')); + } + fireEvent.click( + await screen.findByRole('button', { + name: 'Manage second-tools tools', + }), + ); + if (firstFails) { + await screen.findByRole('group', { + name: 'Approval mode for second_tool', + }); + fail(new Error('First unavailable')); + } else { + expect(await screen.findByRole('alert')).toHaveTextContent( + 'Second unavailable', + ); + finish({ + tools: [{ name: 'first_tool', description: null, enabled: true }], + }); + } + await waitFor(() => + expect( + firstFails ? toastErrorMock : toastSuccessMock, + ).toHaveBeenCalledWith( + `Connection test ${firstFails ? 'failed' : 'passed'} for internal-tools.`, + ), + ); + expect( + firstFails ? toastSuccessMock : toastErrorMock, + ).not.toHaveBeenCalled(); + if (firstFails) { + expect( + screen.getByRole('group', { name: 'Approval mode for second_tool' }), + ).toBeInTheDocument(); + } else { + expect(screen.getByRole('alert')).toHaveTextContent( + 'Second unavailable', + ); + } + expect( + screen.queryByRole('group', { name: 'Approval mode for first_tool' }), + ).toBeNull(); + expect( + screen.getByRole('button', { name: 'Test second-tools connection' }), + ).toBeEnabled(); + }, + ); it('keeps a pending retry and its late response scoped to the selected server', async () => { const first = buildServer(); diff --git a/apps/web/src/components/settings/CustomMcpServers.tsx b/apps/web/src/components/settings/CustomMcpServers.tsx index f3e85d66b3..551684b0b1 100644 --- a/apps/web/src/components/settings/CustomMcpServers.tsx +++ b/apps/web/src/components/settings/CustomMcpServers.tsx @@ -755,6 +755,7 @@ function CustomToolManagementDialog({ onOpenChange: (open: boolean) => void; }) { const trpc = useTRPC(); + const queryClient = useQueryClient(); const { isAdmin } = useAuthorizedUser(); const [retryError, setRetryError] = useState<{ serverId: string; @@ -787,11 +788,16 @@ function CustomToolManagementDialog({ }; const testConnection = async () => { if (!server || toolsQuery.isFetching) return; - const result = await toolsQuery.refetch({ cancelRefetch: false }); - if (result.isError) { - toast.error(`Connection test failed for ${server.name}.`); - } else if (result.isSuccess) { + try { + await queryClient.fetchQuery( + trpc.customMcpServers.listTools.queryOptions( + { id: server.id }, + { staleTime: 0, retry: false }, + ), + ); toast.success(`Connection test passed for ${server.name}.`); + } catch { + toast.error(`Connection test failed for ${server.name}.`); } }; return (