improvement(mcp): make the OAuth experience visible and verbally consistent

apps/sim/app/workspace/[workspaceId]/settings/components/mcp/components/mcp-server-form-modal/mcp-server-form-modal.tsx

@@ -116,7 +116,7 @@ function getTestButtonLabel(
  if (testResult?.success) return 'Connection success'
  if (testResult?.authRequired) return 'Requires OAuth'
  if (testResult && !testResult.success) return 'No connection: retry'
- return 'Test Connection'
+ return 'Test connection'
 }
 
 interface FormattedInputProps {
@@ -609,8 +609,8 @@ export function McpServerFormModal({
  const isSubmitDisabled =
  isSubmitting || !isFormValid || isDomainBlocked || (mode === 'edit' && !hasChanges)
 
- const title = mode === 'add' ? 'Add New MCP Server' : 'Edit MCP Server'
- const submitLabel = mode === 'add' ? 'Add MCP' : 'Save'
+ const title = mode === 'add' ? 'Add MCP server' : 'Edit MCP server'
+ const submitLabel = mode === 'add' ? 'Add server' : 'Save'
 
  const handleToggleJsonMode = () => {
  if (testResult) clearTestResult()
@@ -622,7 +622,7 @@ export function McpServerFormModal({
  const secondaryAction: ChipModalFooterAction | undefined =
  mode === 'add'
  ? {
- label: formMode === 'form' ? 'Edit JSON' : 'Edit Form',
+ label: formMode === 'form' ? 'Edit JSON' : 'Edit form',
  onClick: handleToggleJsonMode,
  }
  : formMode === 'form'

apps/sim/app/workspace/[workspaceId]/settings/components/mcp/mcp.tsx

@@ -99,9 +99,10 @@ function ServerListItem({
  )
  // A live discovery failure whose stored status hasn't caught up yet would otherwise read as
  // "0 tools"; surface it directly so a failed row reads as failed, not empty.
+ // Shown even when cached tools exist: a present discoveryError means the LATEST
+ // discovery failed, and silently showing the stale tool count would hide that.
  const showDiscoveryError =
  Boolean(discoveryError) &&
- tools.length === 0 &&
  server.connectionStatus !== 'error' &&
  server.connectionStatus !== 'disconnected'
  const hasConnectionIssue =
@@ -114,38 +115,37 @@ function ServerListItem({
  <div className='flex min-w-0 flex-col justify-center gap-[1px]'>
  <div className='flex items-center gap-1.5'>
  <span className='max-w-[200px] truncate text-[var(--text-body)] text-sm'>
- {server.name || 'Unnamed Server'}
+ {server.name || 'Unnamed server'}
  </span>
  <span className='text-[var(--text-muted)] text-caption'>({transportLabel})</span>
  </div>
  <p
  className={cn(
- 'truncate text-sm',
- hasConnectionIssue ? 'text-[var(--text-error)]' : 'text-[var(--text-muted)]'
+ 'truncate text-caption',
+ hasConnectionIssue && !isConnecting
+ ? 'text-[var(--text-error)]'
+ : 'text-[var(--text-muted)]'
  )}
  >
- {isRefreshing
- ? 'Refreshing...'
- : isLoadingTools && tools.length === 0
- ? 'Loading...'
- : showDiscoveryError
- ? discoveryError
- : toolsLabel}
+ {isConnecting
+ ? 'Waiting for authorization...'
+ : isRefreshing
+ ? 'Refreshing...'
+ : isLoadingTools && tools.length === 0
+ ? 'Loading...'
+ : showDiscoveryError
+ ? discoveryError
+ : toolsLabel}
  </p>
  </div>
  <div className='flex flex-shrink-0 items-center gap-1'>
+ {canManage && server.authType === 'oauth' && server.connectionStatus !== 'connected' && (
+ <Chip onClick={onAuthorize}>{isConnecting ? 'Reopen authorization' : 'Authorize'}</Chip>
+ )}
  <RowActionsMenu
  label='Server actions'
  actions={[
  { label: 'Details', onSelect: onViewDetails },
- ...(canManage && server.authType === 'oauth' && server.connectionStatus !== 'connected'
- ? [
- {
- label: isConnecting ? 'Reopen authorization' : 'Authorize',
- onSelect: onAuthorize,
- },
- ]
- : []),
  ...(canManage
  ? [
  {
@@ -193,11 +193,7 @@ export function MCP() {
  isLoading: serversLoading,
  error: serversError,
  } = useMcpServers(workspaceId)
- const {
- data: mcpToolsData = [],
- error: toolsError,
- toolsStateByServer,
- } = useMcpToolsQuery(workspaceId)
+ const { data: mcpToolsData = [], toolsStateByServer } = useMcpToolsQuery(workspaceId)
  const { data: storedTools = [], refetch: refetchStoredTools } = useStoredMcpTools(workspaceId)
  const forceRefreshToolsMutation = useForceRefreshMcpTools()
  const forceRefreshTools = forceRefreshToolsMutation.mutate
@@ -399,15 +395,10 @@ export function MCP() {
  return issues
  }
 
- // Only a failure to load the server LIST replaces the list. A tool-discovery failure
- // (`toolsError`) must not blank the page — the servers still render, each row surfacing its
- // own discovery state via `toolsStateByServer`, with a non-blocking notice above the list.
+ // Only a failure to load the server LIST replaces the list. A tool-discovery failure must
+ // not blank the page — the servers still render, each row surfacing its own discovery
+ // state via `toolsStateByServer`.
  const listError = serversError
- // Any per-server discovery failure — even a partial one where other servers succeeded (which
- // suppresses the aggregate `toolsError`) — so the notice below still surfaces it.
- const hasDiscoveryError =
- Boolean(toolsError) ||
- Array.from(toolsStateByServer.values()).some((state) => state.error != null)
  const hasServers = servers && servers.length > 0
  const showNoResults = searchTerm.trim() && filteredServers.length === 0 && servers.length > 0
 
@@ -418,15 +409,13 @@ export function MCP() {
  const refreshAction = getRefreshActionState({
  mutationStatus: isCurrentRefresh ? refreshServerMutation.status : 'idle',
  connectionStatus: isCurrentRefresh ? refreshServerMutation.data?.status : undefined,
- authType: server.authType,
- error: isCurrentRefresh ? refreshServerMutation.data?.error : undefined,
  workflowsUpdated: isCurrentRefresh ? refreshServerMutation.data?.workflowsUpdated : undefined,
  })
 
  return (
  <SettingsPanel
  back={{ text: 'MCP tools', icon: ArrowLeft, onSelect: handleBackToList }}
- title={server.name || 'Unnamed Server'}
+ title={server.name || 'Unnamed server'}
  actions={
  canEdit
  ? [
@@ -447,8 +436,8 @@ export function MCP() {
  <SettingsSection label='Server'>
  <div className='flex flex-col gap-4.5'>
  <div className='flex flex-col gap-2'>
- <span className='text-[var(--text-muted)] text-caption'>Server Name</span>
- <p className='text-[var(--text-body)] text-sm'>{server.name || 'Unnamed Server'}</p>
+ <span className='text-[var(--text-muted)] text-caption'>Server name</span>
+ <p className='text-[var(--text-body)] text-sm'>{server.name || 'Unnamed server'}</p>
  </div>
 
  <div className='flex flex-col gap-2'>
@@ -487,9 +476,7 @@ export function MCP() {
  await startOauthForServer(server.id)
  }}
  >
- {connectingOauthServers.has(server.id)
- ? 'Reopen authorization window'
- : 'Connect with OAuth'}
+ {connectingOauthServers.has(server.id) ? 'Reopen authorization' : 'Authorize'}
  </Chip>
  </div>
  </div>
@@ -651,7 +638,7 @@ export function MCP() {
  search={{
  value: searchTerm,
  onChange: setSearchTerm,
- placeholder: 'Search MCPs...',
+ placeholder: 'Search servers...',
  }}
  actions={
  canEdit
@@ -669,21 +656,18 @@ export function MCP() {
  >
  {listError ? (
  <div className='flex h-full flex-col items-center justify-center gap-2'>
- <p className='text-[var(--text-error)] text-xs leading-tight'>
+ <p className='text-[var(--text-error)] text-small leading-tight'>
  {getErrorMessage(listError, 'Failed to load MCP servers')}
  </p>
  </div>
- ) : serversLoading ? null : !hasServers ? (
+ ) : serversLoading ? (
+ <SettingsEmptyState>Loading...</SettingsEmptyState>
+ ) : !hasServers ? (
  <SettingsEmptyState>
  {canEdit ? 'Click "Add server" above to get started' : 'No MCP servers configured'}
  </SettingsEmptyState>
  ) : (
  <div className='flex flex-col gap-2'>
- {hasDiscoveryError && (
- <p className='text-[var(--text-error)] text-xs leading-tight'>
- {getErrorMessage(toolsError, 'Some tools could not be discovered')}
- </p>
- )}
  {filteredServers.map((server) => {
  if (!server?.id) return null
  const tools = toolsByServer[server.id] || []
@@ -749,8 +733,8 @@ export function MCP() {
  onOpenChange={(open) => {
  if (!open) setServerToDeleteId(null)
  }}
- srTitle='Delete MCP Server'
- title='Delete MCP Server'
+ srTitle='Delete MCP server'
+ title='Delete MCP server'
  text={[
  'Are you sure you want to delete ',
  {

apps/sim/app/workspace/[workspaceId]/settings/components/mcp/refresh-action-state.test.ts

@@ -31,28 +31,11 @@ describe('getRefreshActionState', () => {
  })
  })
 
- it('shows OAuth authorization required when an OAuth refresh finishes disconnected', () => {
- expect(
- getRefreshActionState({
- mutationStatus: 'success',
- connectionStatus: 'disconnected',
- authType: 'oauth',
- workflowsUpdated: 0,
- })
- ).toEqual({
- text: 'OAuth authorization required',
- textTone: 'error',
- disabled: false,
- })
- })
-
  it('keeps Failed when a disconnected OAuth refresh has a concrete error', () => {
  expect(
  getRefreshActionState({
  mutationStatus: 'success',
  connectionStatus: 'disconnected',
- authType: 'oauth',
- error: 'The MCP server took too long to respond and timed out',
  workflowsUpdated: 0,
  })
  ).toEqual({

apps/sim/app/workspace/[workspaceId]/settings/components/mcp/refresh-action-state.ts

@@ -1,12 +1,10 @@
 import type { MutationStatus } from '@tanstack/react-query'
-import type { McpServer, RefreshMcpServerResult } from '@/lib/api/contracts/mcp'
+import type { RefreshMcpServerResult } from '@/lib/api/contracts/mcp'
 import type { SettingsAction } from '@/app/workspace/[workspaceId]/settings/components/settings-header/settings-header'
 
 interface RefreshActionStateInput {
  mutationStatus: MutationStatus
  connectionStatus?: RefreshMcpServerResult['status']
- authType?: McpServer['authType']
- error?: RefreshMcpServerResult['error']
  workflowsUpdated?: number
 }
 
@@ -15,23 +13,12 @@ type RefreshActionState = Pick<SettingsAction, 'text' | 'textTone' | 'disabled'>
 export function getRefreshActionState({
  mutationStatus,
  connectionStatus,
- authType,
- error,
  workflowsUpdated,
 }: RefreshActionStateInput): RefreshActionState {
  if (mutationStatus === 'pending') {
  return { text: 'Refreshing...', textTone: undefined, disabled: true }
  }
 
- if (
- mutationStatus === 'success' &&
- connectionStatus === 'disconnected' &&
- authType === 'oauth' &&
- !error?.trim()
- ) {
- return { text: 'OAuth authorization required', textTone: 'error', disabled: false }
- }
-
  if (
  mutationStatus === 'error' ||
  (mutationStatus === 'success' && connectionStatus !== 'connected')

apps/sim/app/workspace/[workspaceId]/settings/components/mcp/server-tools-label.test.ts

@@ -19,7 +19,7 @@ describe('getServerToolsLabel', () => {
  })
 
  it('keeps the generic disconnected state for non-OAuth servers', () => {
- expect(getServerToolsLabel([], 'disconnected', null, 'headers')).toBe('Not Connected')
+ expect(getServerToolsLabel([], 'disconnected', null, 'headers')).toBe('Not connected')
  })
 
  it('shows the persisted error for disconnected connections', () => {

apps/sim/app/workspace/[workspaceId]/settings/components/mcp/server-tools-label.ts

@@ -16,7 +16,7 @@ export function getServerToolsLabel(
 
  if (connectionStatus === 'disconnected') {
  return (
- lastError?.trim() || (authType === 'oauth' ? 'OAuth authorization required' : 'Not Connected')
+ lastError?.trim() || (authType === 'oauth' ? 'OAuth authorization required' : 'Not connected')
  )
  }
 

apps/sim/hooks/mcp/use-mcp-oauth-popup.test.tsx

@@ -243,4 +243,62 @@ describe('useMcpOauthPopup', () => {
  expect(order).toEqual(['open', 'start'])
  hook.unmount()
  })
+
+ it('clears the label when the popup closes but still honors a late completion', async () => {
+ vi.useFakeTimers()
+ try {
+ const popup = {
+ close: vi.fn(),
+ focus: vi.fn(),
+ location: { replace: vi.fn() },
+ closed: false,
+ }
+ ;(window.open as ReturnType<typeof vi.fn>).mockReturnValue(popup as unknown as Window)
+ let channelHandler: ((e: { data: unknown }) => void) | null = null
+ class CapturingChannel {
+ constructor(public name: string) {}
+ postMessage(): void {}
+ close(): void {}
+ }
+ Object.defineProperty(CapturingChannel.prototype, 'onmessage', {
+ set(h) {
+ channelHandler = h
+ },
+ get() {
+ return channelHandler
+ },
+ configurable: true,
+ })
+ ;(globalThis as unknown as { BroadcastChannel: unknown }).BroadcastChannel = CapturingChannel
+ mockStartOauth.mockResolvedValue({
+ status: 'redirect',
+ authorizationUrl: 'https://as.example/a?state=st',
+ state: 'st',
+ })
+ const hook = renderHookWithClient(() => useMcpOauthPopup({ workspaceId: 'w1' }))
+ await act(async () => {
+ await hook.result().startOauthForServer('s1')
+ })
+ expect(hook.result().connectingServers.has('s1')).toBe(true)
+
+ // User closes the popup — the label share clears within a poll tick...
+ popup.closed = true
+ await act(async () => {
+ vi.advanceTimersByTime(1500)
+ })
+ expect(hook.result().connectingServers.has('s1')).toBe(false)
+
+ // ...but the flow stays registered: a late BroadcastChannel completion is still honored.
+ const invalidateSpy = vi.spyOn(hook.queryClient, 'invalidateQueries')
+ await act(async () => {
+ channelHandler?.({ data: { type: 'mcp-oauth', ok: true, serverId: 's1', state: 'st' } })
+ })
+ expect(invalidateSpy).toHaveBeenCalledWith({ queryKey: ['mcp', 'servers', 'w1'] })
+ // And the count did not go negative / re-clear twice.
+ expect(hook.result().connectingServers.has('s1')).toBe(false)
+ hook.unmount()
+ } finally {
+ vi.useRealTimers()
+ }
+ })
 })