From fa7e1438e118ab0eed2112d2533f8ca295a57a67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=A8=8B=E5=BA=8F=E5=91=98=E9=98=BF=E6=B1=9F=28Relakkes?= =?UTF-8?q?=29?= Date: Mon, 25 May 2026 20:58:06 +0800 Subject: [PATCH] fix: avoid empty MCP flash during settings load The MCP settings page now waits for the full initial aggregation pass before rendering list statistics or the empty state. This covers the extra project path lookup for user-private MCP entries, so the page does not briefly show zero servers while the desktop app is still discovering scoped configs. The focused MCP settings test now asserts the initial loading state and keeps existing list, edit, toggle, and reconnect flows waiting for that load boundary. Constraint: MCP settings now loads from multiple project contexts before the first trustworthy list render Rejected: Keep rendering zero-value stats during discovery | this caused a misleading empty flash on startup Confidence: high Scope-risk: narrow Directive: Do not show MCP empty state until initial project path discovery and server fetch have both completed Tested: cd desktop && bun run test -- mcpSettings.test.tsx Tested: bun run check:desktop Tested: git diff --check --- desktop/src/__tests__/mcpSettings.test.tsx | 60 ++++++-- desktop/src/pages/McpSettings.tsx | 158 ++++++++++++--------- 2 files changed, 138 insertions(+), 80 deletions(-) diff --git a/desktop/src/__tests__/mcpSettings.test.tsx b/desktop/src/__tests__/mcpSettings.test.tsx index 3d59a170..0076d5ee 100644 --- a/desktop/src/__tests__/mcpSettings.test.tsx +++ b/desktop/src/__tests__/mcpSettings.test.tsx @@ -31,6 +31,14 @@ vi.mock('../api/mcp', async (importOriginal) => { } }) +async function renderLoadedMcpSettings() { + const result = render() + await waitFor(() => { + expect(screen.queryByRole('status')).not.toBeInTheDocument() + }) + return result +} + describe('McpSettings', () => { beforeEach(() => { vi.mocked(sessionsApi.getRecentProjects).mockResolvedValue({ @@ -77,7 +85,7 @@ describe('McpSettings', () => { selectedServer: null, isLoading: false, error: null, - fetchServers: vi.fn(), + fetchServers: vi.fn().mockResolvedValue(undefined), createServer: vi.fn(), updateServer: vi.fn(), deleteServer: vi.fn(), @@ -89,7 +97,7 @@ describe('McpSettings', () => { }) it('loads MCP servers for the active and recent projects on mount', async () => { - const fetchServers = vi.fn() + const fetchServers = vi.fn().mockResolvedValue(undefined) useMcpStore.setState({ fetchServers }) render() @@ -102,15 +110,41 @@ describe('McpSettings', () => { }) }) - it('renders the empty state and add button', () => { + it('shows a loading state before project MCP paths and servers finish loading', async () => { + let resolveRecentProjects!: (value: Awaited>) => void + const fetchServers = vi.fn().mockResolvedValue(undefined) + vi.mocked(sessionsApi.getRecentProjects).mockImplementation(() => new Promise((resolve) => { + resolveRecentProjects = resolve + })) + useMcpStore.setState({ fetchServers }) + render() + expect(screen.getByRole('status')).toHaveTextContent('Loading...') + expect(screen.queryByText('No MCP servers configured yet')).not.toBeInTheDocument() + expect(screen.queryByText('Total servers')).not.toBeInTheDocument() + + await act(async () => { + resolveRecentProjects({ projects: [] }) + }) + + await waitFor(() => { + expect(fetchServers).toHaveBeenCalledWith(['/workspace/project', '/workspace/config-project'], '/workspace/project') + }) + await waitFor(() => { + expect(screen.queryByRole('status')).not.toBeInTheDocument() + }) + }) + + it('renders the empty state and add button', async () => { + await renderLoadedMcpSettings() + expect(screen.getByText('MCP servers')).toBeInTheDocument() expect(screen.getByText('No MCP servers configured yet')).toBeInTheDocument() expect(screen.getByRole('button', { name: /add server/i })).toBeInTheDocument() }) - it('shows plugin and user MCP servers in grouped sections', () => { + it('shows plugin and user MCP servers in grouped sections', async () => { useMcpStore.setState({ servers: [ { @@ -146,7 +180,7 @@ describe('McpSettings', () => { ], }) - render() + await renderLoadedMcpSettings() expect(screen.getAllByText('Plugin').length).toBeGreaterThan(0) expect(screen.getAllByText('User').length).toBeGreaterThan(0) @@ -154,7 +188,7 @@ describe('McpSettings', () => { expect(screen.getByText('global-user')).toBeInTheDocument() }) - it('keeps same-name project MCP servers distinct by project path', () => { + it('keeps same-name project MCP servers distinct by project path', async () => { useMcpStore.setState({ servers: [ { @@ -192,7 +226,7 @@ describe('McpSettings', () => { ], }) - render() + await renderLoadedMcpSettings() expect(screen.getAllByText('context7')).toHaveLength(2) expect(screen.getByText('/workspace/project-a')).toBeInTheDocument() @@ -226,7 +260,7 @@ describe('McpSettings', () => { refreshServerStatus, }) - render() + await renderLoadedMcpSettings() expect(screen.getByText('Checking')).toBeInTheDocument() @@ -258,7 +292,7 @@ describe('McpSettings', () => { deleteServer, }) - render() + await renderLoadedMcpSettings() await act(async () => { fireEvent.click(screen.getByRole('button', { name: 'Open global-user' })) @@ -302,7 +336,7 @@ describe('McpSettings', () => { toggleServer, }) - render() + await renderLoadedMcpSettings() await act(async () => { fireEvent.click(screen.getByRole('switch')) @@ -332,7 +366,7 @@ describe('McpSettings', () => { useMcpStore.setState({ createServer }) - render() + await renderLoadedMcpSettings() await act(async () => { fireEvent.click(screen.getByRole('button', { name: /add server/i })) @@ -427,7 +461,7 @@ describe('McpSettings', () => { updateServer, }) - render() + await renderLoadedMcpSettings() await act(async () => { fireEvent.click(screen.getByRole('button', { name: 'Open shared-tools' })) @@ -489,7 +523,7 @@ describe('McpSettings', () => { reconnectServer, }) - render() + await renderLoadedMcpSettings() await act(async () => { fireEvent.click(screen.getByRole('button', { name: 'Open plugin:telegram:telegram' })) diff --git a/desktop/src/pages/McpSettings.tsx b/desktop/src/pages/McpSettings.tsx index 345f0c34..04c7670a 100644 --- a/desktop/src/pages/McpSettings.tsx +++ b/desktop/src/pages/McpSettings.tsx @@ -372,6 +372,19 @@ function StatCard({ label, value, icon }: { label: string; value: number; icon: ) } +function LoadingState({ label }: { label: string }) { + return ( +
+
+
{label}
+
+ ) +} + function ServerRow({ server, isBusy, @@ -440,6 +453,7 @@ export function McpSettings() { const [isDeleting, setIsDeleting] = useState(false) const [busyServerKey, setBusyServerKey] = useState(null) const [pendingDeleteServer, setPendingDeleteServer] = useState(null) + const [isInitialLoading, setIsInitialLoading] = useState(true) const projectPathsForFetchRef = useRef(undefined) const refreshInFlightRef = useRef(new Set()) @@ -449,16 +463,18 @@ export function McpSettings() { useEffect(() => { let cancelled = false + setIsInitialLoading(true) - Promise.all([ - sessionsApi.getRecentProjects() - .then(({ projects }) => projects.map((project) => project.realPath)) - .catch(() => []), - mcpApi.projectPaths() - .then(({ projectPaths }) => projectPaths) - .catch(() => []), - ]) - .then(([recentProjectPaths, privateMcpProjectPaths]) => { + const loadServers = async () => { + try { + const [recentProjectPaths, privateMcpProjectPaths] = await Promise.all([ + sessionsApi.getRecentProjects() + .then(({ projects }) => projects.map((project) => project.realPath)) + .catch(() => []), + mcpApi.projectPaths() + .then(({ projectPaths }) => projectPaths) + .catch(() => []), + ]) if (cancelled) return const paths = [ currentWorkDir, @@ -467,8 +483,13 @@ export function McpSettings() { ].filter((path): path is string => !!path) const projectPathsForFetch = Array.from(new Set(paths)) projectPathsForFetchRef.current = projectPathsForFetch.length ? projectPathsForFetch : undefined - void fetchServers(projectPathsForFetchRef.current, currentWorkDir) - }) + await fetchServers(projectPathsForFetchRef.current, currentWorkDir) + } finally { + if (!cancelled) setIsInitialLoading(false) + } + } + + void loadServers() return () => { cancelled = true @@ -489,6 +510,7 @@ export function McpSettings() { connected: servers.filter((server) => server.status === 'connected').length, attention: servers.filter((server) => server.status === 'failed' || server.status === 'needs-auth').length, }), [servers]) + const showListLoading = isInitialLoading || (isLoading && servers.length === 0) const beginCreate = () => { setDraft(createEmptyDraft()) @@ -1061,64 +1083,66 @@ export function McpSettings() {
-
- - - -
- - {isLoading && servers.length === 0 ? ( -
-
-
- ) : error ? ( -
- error -

{error}

- -
- ) : servers.length === 0 ? ( -
- dns -

{t('settings.mcp.empty')}

-

{t('settings.mcp.emptyHint')}

-
+ {showListLoading ? ( + ) : ( -
- {MCP_GROUP_ORDER.map((group) => { - const groupServers = groupedServers[group] - if (!groupServers?.length) return null + <> +
+ + + +
- return ( -
-
-
- {group === 'plugin' ? t('settings.mcp.scope.plugin') : t(`settings.mcp.scope.${group}`)} -
-
{groupServers.length}
-
-
- {groupServers.map((server) => ( - beginEdit(server)} - onToggle={() => void handleToggle(server)} - t={t} - /> - ))} -
-
- ) - })} -
+ {error ? ( +
+ error +

{error}

+ +
+ ) : servers.length === 0 ? ( +
+ dns +

{t('settings.mcp.empty')}

+

{t('settings.mcp.emptyHint')}

+
+ ) : ( +
+ {MCP_GROUP_ORDER.map((group) => { + const groupServers = groupedServers[group] + if (!groupServers?.length) return null + + return ( +
+
+
+ {group === 'plugin' ? t('settings.mcp.scope.plugin') : t(`settings.mcp.scope.${group}`)} +
+
{groupServers.length}
+
+
+ {groupServers.map((server) => ( + beginEdit(server)} + onToggle={() => void handleToggle(server)} + t={t} + /> + ))} +
+
+ ) + })} +
+ )} + )} {deleteModal}