mirror of
https://github.com/NanmiCoder/cc-haha
synced 2026-07-16 13:03:31 +08:00
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
This commit is contained in:
程序员阿江(Relakkes)
parent
043f0e7a2e
commit
fa7e1438e1
@ -31,6 +31,14 @@ vi.mock('../api/mcp', async (importOriginal) => {
|
||||
}
|
||||
})
|
||||
|
||||
async function renderLoadedMcpSettings() {
|
||||
const result = render(<McpSettings />)
|
||||
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(<McpSettings />)
|
||||
@ -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<ReturnType<typeof sessionsApi.getRecentProjects>>) => void
|
||||
const fetchServers = vi.fn().mockResolvedValue(undefined)
|
||||
vi.mocked(sessionsApi.getRecentProjects).mockImplementation(() => new Promise((resolve) => {
|
||||
resolveRecentProjects = resolve
|
||||
}))
|
||||
useMcpStore.setState({ fetchServers })
|
||||
|
||||
render(<McpSettings />)
|
||||
|
||||
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(<McpSettings />)
|
||||
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(<McpSettings />)
|
||||
await renderLoadedMcpSettings()
|
||||
|
||||
expect(screen.getAllByText('context7')).toHaveLength(2)
|
||||
expect(screen.getByText('/workspace/project-a')).toBeInTheDocument()
|
||||
@ -226,7 +260,7 @@ describe('McpSettings', () => {
|
||||
refreshServerStatus,
|
||||
})
|
||||
|
||||
render(<McpSettings />)
|
||||
await renderLoadedMcpSettings()
|
||||
|
||||
expect(screen.getByText('Checking')).toBeInTheDocument()
|
||||
|
||||
@ -258,7 +292,7 @@ describe('McpSettings', () => {
|
||||
deleteServer,
|
||||
})
|
||||
|
||||
render(<McpSettings />)
|
||||
await renderLoadedMcpSettings()
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Open global-user' }))
|
||||
@ -302,7 +336,7 @@ describe('McpSettings', () => {
|
||||
toggleServer,
|
||||
})
|
||||
|
||||
render(<McpSettings />)
|
||||
await renderLoadedMcpSettings()
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByRole('switch'))
|
||||
@ -332,7 +366,7 @@ describe('McpSettings', () => {
|
||||
|
||||
useMcpStore.setState({ createServer })
|
||||
|
||||
render(<McpSettings />)
|
||||
await renderLoadedMcpSettings()
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByRole('button', { name: /add server/i }))
|
||||
@ -427,7 +461,7 @@ describe('McpSettings', () => {
|
||||
updateServer,
|
||||
})
|
||||
|
||||
render(<McpSettings />)
|
||||
await renderLoadedMcpSettings()
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Open shared-tools' }))
|
||||
@ -489,7 +523,7 @@ describe('McpSettings', () => {
|
||||
reconnectServer,
|
||||
})
|
||||
|
||||
render(<McpSettings />)
|
||||
await renderLoadedMcpSettings()
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Open plugin:telegram:telegram' }))
|
||||
|
||||
@ -372,6 +372,19 @@ function StatCard({ label, value, icon }: { label: string; value: number; icon:
|
||||
)
|
||||
}
|
||||
|
||||
function LoadingState({ label }: { label: string }) {
|
||||
return (
|
||||
<div
|
||||
role="status"
|
||||
aria-live="polite"
|
||||
className="flex min-h-[220px] flex-col items-center justify-center gap-3 rounded-2xl border border-dashed border-[var(--color-border)] bg-[var(--color-surface-container-low)] text-center"
|
||||
>
|
||||
<div className="h-7 w-7 animate-spin rounded-full border-2 border-[var(--color-brand)] border-t-transparent" />
|
||||
<div className="text-sm font-medium text-[var(--color-text-secondary)]">{label}</div>
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
function ServerRow({
|
||||
server,
|
||||
isBusy,
|
||||
@ -440,6 +453,7 @@ export function McpSettings() {
|
||||
const [isDeleting, setIsDeleting] = useState(false)
|
||||
const [busyServerKey, setBusyServerKey] = useState<string | null>(null)
|
||||
const [pendingDeleteServer, setPendingDeleteServer] = useState<McpServerRecord | null>(null)
|
||||
const [isInitialLoading, setIsInitialLoading] = useState(true)
|
||||
const projectPathsForFetchRef = useRef<string[] | undefined>(undefined)
|
||||
const refreshInFlightRef = useRef(new Set<string>())
|
||||
|
||||
@ -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() {
|
||||
</Button>
|
||||
</div>
|
||||
|
||||
<div className="grid gap-4 md:grid-cols-3 mb-8">
|
||||
<StatCard label={t('settings.mcp.stats.total')} value={stats.total} icon="dns" />
|
||||
<StatCard label={t('settings.mcp.stats.connected')} value={stats.connected} icon="check_circle" />
|
||||
<StatCard label={t('settings.mcp.stats.attention')} value={stats.attention} icon="error" />
|
||||
</div>
|
||||
|
||||
{isLoading && servers.length === 0 ? (
|
||||
<div className="flex justify-center py-16">
|
||||
<div className="animate-spin h-6 w-6 rounded-full border-2 border-[var(--color-brand)] border-t-transparent" />
|
||||
</div>
|
||||
) : error ? (
|
||||
<div className="text-center py-16 rounded-2xl border border-dashed border-[var(--color-border)] bg-[var(--color-surface-container-low)]">
|
||||
<span className="material-symbols-outlined text-[40px] text-[var(--color-error)] mb-3 block">error</span>
|
||||
<p className="text-sm text-[var(--color-error)] mb-3">{error}</p>
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => void fetchServers(projectPathsForFetchRef.current, currentWorkDir)}
|
||||
className="text-sm text-[var(--color-text-accent)] hover:underline"
|
||||
>
|
||||
{t('common.retry')}
|
||||
</button>
|
||||
</div>
|
||||
) : servers.length === 0 ? (
|
||||
<div className="text-center py-16 rounded-2xl border border-dashed border-[var(--color-border)] bg-[var(--color-surface-container-low)]">
|
||||
<span className="material-symbols-outlined text-[40px] text-[var(--color-text-tertiary)] mb-3 block">dns</span>
|
||||
<p className="text-sm text-[var(--color-text-secondary)] mb-1">{t('settings.mcp.empty')}</p>
|
||||
<p className="text-xs text-[var(--color-text-tertiary)]">{t('settings.mcp.emptyHint')}</p>
|
||||
</div>
|
||||
{showListLoading ? (
|
||||
<LoadingState label={t('common.loading')} />
|
||||
) : (
|
||||
<div className="flex flex-col gap-6">
|
||||
{MCP_GROUP_ORDER.map((group) => {
|
||||
const groupServers = groupedServers[group]
|
||||
if (!groupServers?.length) return null
|
||||
<>
|
||||
<div className="grid gap-4 md:grid-cols-3 mb-8">
|
||||
<StatCard label={t('settings.mcp.stats.total')} value={stats.total} icon="dns" />
|
||||
<StatCard label={t('settings.mcp.stats.connected')} value={stats.connected} icon="check_circle" />
|
||||
<StatCard label={t('settings.mcp.stats.attention')} value={stats.attention} icon="error" />
|
||||
</div>
|
||||
|
||||
return (
|
||||
<section key={group}>
|
||||
<div className="flex items-center justify-between mb-3">
|
||||
<div className="text-[1.35rem] font-semibold text-[var(--color-text-primary)]">
|
||||
{group === 'plugin' ? t('settings.mcp.scope.plugin') : t(`settings.mcp.scope.${group}`)}
|
||||
</div>
|
||||
<div className="text-sm text-[var(--color-text-tertiary)]">{groupServers.length}</div>
|
||||
</div>
|
||||
<div className="rounded-[28px] border border-[var(--color-border)] bg-[var(--color-surface)] overflow-hidden">
|
||||
{groupServers.map((server) => (
|
||||
<ServerRow
|
||||
key={getServerIdentityKey(server)}
|
||||
server={server}
|
||||
isBusy={busyServerKey === getServerIdentityKey(server)}
|
||||
onOpen={() => beginEdit(server)}
|
||||
onToggle={() => void handleToggle(server)}
|
||||
t={t}
|
||||
/>
|
||||
))}
|
||||
</div>
|
||||
</section>
|
||||
)
|
||||
})}
|
||||
</div>
|
||||
{error ? (
|
||||
<div className="text-center py-16 rounded-2xl border border-dashed border-[var(--color-border)] bg-[var(--color-surface-container-low)]">
|
||||
<span className="material-symbols-outlined text-[40px] text-[var(--color-error)] mb-3 block">error</span>
|
||||
<p className="text-sm text-[var(--color-error)] mb-3">{error}</p>
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => void fetchServers(projectPathsForFetchRef.current, currentWorkDir)}
|
||||
className="text-sm text-[var(--color-text-accent)] hover:underline"
|
||||
>
|
||||
{t('common.retry')}
|
||||
</button>
|
||||
</div>
|
||||
) : servers.length === 0 ? (
|
||||
<div className="text-center py-16 rounded-2xl border border-dashed border-[var(--color-border)] bg-[var(--color-surface-container-low)]">
|
||||
<span className="material-symbols-outlined text-[40px] text-[var(--color-text-tertiary)] mb-3 block">dns</span>
|
||||
<p className="text-sm text-[var(--color-text-secondary)] mb-1">{t('settings.mcp.empty')}</p>
|
||||
<p className="text-xs text-[var(--color-text-tertiary)]">{t('settings.mcp.emptyHint')}</p>
|
||||
</div>
|
||||
) : (
|
||||
<div className="flex flex-col gap-6">
|
||||
{MCP_GROUP_ORDER.map((group) => {
|
||||
const groupServers = groupedServers[group]
|
||||
if (!groupServers?.length) return null
|
||||
|
||||
return (
|
||||
<section key={group}>
|
||||
<div className="flex items-center justify-between mb-3">
|
||||
<div className="text-[1.35rem] font-semibold text-[var(--color-text-primary)]">
|
||||
{group === 'plugin' ? t('settings.mcp.scope.plugin') : t(`settings.mcp.scope.${group}`)}
|
||||
</div>
|
||||
<div className="text-sm text-[var(--color-text-tertiary)]">{groupServers.length}</div>
|
||||
</div>
|
||||
<div className="rounded-[28px] border border-[var(--color-border)] bg-[var(--color-surface)] overflow-hidden">
|
||||
{groupServers.map((server) => (
|
||||
<ServerRow
|
||||
key={getServerIdentityKey(server)}
|
||||
server={server}
|
||||
isBusy={busyServerKey === getServerIdentityKey(server)}
|
||||
onOpen={() => beginEdit(server)}
|
||||
onToggle={() => void handleToggle(server)}
|
||||
t={t}
|
||||
/>
|
||||
))}
|
||||
</div>
|
||||
</section>
|
||||
)
|
||||
})}
|
||||
</div>
|
||||
)}
|
||||
</>
|
||||
)}
|
||||
{deleteModal}
|
||||
</div>
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user