feat(desktop): daily re-auth nudge for MCP servers with a Disable action
An MCP server whose OAuth refresh token died used to get one warning toast
per app session (on the transition into needs-auth) and then nothing: the
server sat parked, silently, until the user happened to open Capabilities →
MCP. A dead token is a standing problem that needs an action, so the health
checker now re-nudges once a day while the server stays broken (persisted
24h snooze per profile+server, same pattern as the update/skew toasts) and
the toast offers the second way out: Disable, which writes enabled: false
through PUT /api/mcp/servers/{name}/enabled. The gateway's config reconcile
(#109906) and the serve backend's reload both follow that edit.
Toasts gain an optional secondaryAction (outline button beside the primary).
This commit is contained in:
@@ -229,19 +229,35 @@ function NotificationItem({ notification }: { notification: AppNotification }) {
|
||||
<p className="m-0 wrap-break-word">{renderMessage(notification.message, accent)}</p>
|
||||
{notification.meta && <p className="m-0 text-xs text-muted-foreground tabular-nums">{notification.meta}</p>}
|
||||
{hasDetail && <NotificationDetail detail={notification.detail || ''} />}
|
||||
{notification.action && (
|
||||
<Button
|
||||
className="mt-1.5"
|
||||
onClick={() => {
|
||||
notification.action?.onClick()
|
||||
dismissNotification(notification.id)
|
||||
}}
|
||||
size="sm"
|
||||
type="button"
|
||||
variant="default"
|
||||
>
|
||||
{notification.action.label}
|
||||
</Button>
|
||||
{(notification.action || notification.secondaryAction) && (
|
||||
<div className="mt-1.5 flex flex-wrap gap-1.5">
|
||||
{notification.action && (
|
||||
<Button
|
||||
onClick={() => {
|
||||
notification.action?.onClick()
|
||||
dismissNotification(notification.id)
|
||||
}}
|
||||
size="sm"
|
||||
type="button"
|
||||
variant="default"
|
||||
>
|
||||
{notification.action.label}
|
||||
</Button>
|
||||
)}
|
||||
{notification.secondaryAction && (
|
||||
<Button
|
||||
onClick={() => {
|
||||
notification.secondaryAction?.onClick()
|
||||
dismissNotification(notification.id)
|
||||
}}
|
||||
size="sm"
|
||||
type="button"
|
||||
variant="outline"
|
||||
>
|
||||
{notification.secondaryAction.label}
|
||||
</Button>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
</AlertDescription>
|
||||
</div>
|
||||
|
||||
@@ -188,7 +188,10 @@ export const ar = defineLocale({
|
||||
errorTitle: 'تعذر الوصول إلى خادم MCP',
|
||||
errorMessage: name => `فشل فحص سلامة ${name} MCP.`,
|
||||
signIn: 'تسجيل الدخول',
|
||||
view: 'عرض'
|
||||
view: 'عرض',
|
||||
disable: 'تعطيل',
|
||||
disabledMessage: name => `تم تعطيل ${name} MCP. يمكنك إعادة تفعيله في أي وقت من الإمكانات → MCP.`,
|
||||
disableFailed: name => `تعذّر تعطيل ${name} MCP.`
|
||||
},
|
||||
errors: {
|
||||
elevenLabsNeedsKey: 'يتطلب ElevenLabs STT المفتاح ELEVENLABS_API_KEY.',
|
||||
|
||||
@@ -218,7 +218,10 @@ export const en: Translations = {
|
||||
errorTitle: 'MCP server unreachable',
|
||||
errorMessage: name => `${name} MCP failed its health check.`,
|
||||
signIn: 'Sign in',
|
||||
view: 'View'
|
||||
view: 'View',
|
||||
disable: 'Disable',
|
||||
disabledMessage: name => `${name} MCP disabled. Re-enable it any time from Capabilities → MCP.`,
|
||||
disableFailed: name => `Could not disable ${name} MCP.`
|
||||
},
|
||||
errors: {
|
||||
elevenLabsNeedsKey: 'ElevenLabs STT needs ELEVENLABS_API_KEY.',
|
||||
|
||||
@@ -183,7 +183,10 @@ export const ja = defineLocale({
|
||||
errorTitle: 'MCP サーバーに接続できません',
|
||||
errorMessage: name => `${name} MCP のヘルスチェックに失敗しました。`,
|
||||
signIn: 'サインイン',
|
||||
view: '表示'
|
||||
view: '表示',
|
||||
disable: '無効化',
|
||||
disabledMessage: name => `${name} MCP を無効にしました。機能 → MCP からいつでも再有効化できます。`,
|
||||
disableFailed: name => `${name} MCP を無効にできませんでした。`
|
||||
},
|
||||
errors: {
|
||||
elevenLabsNeedsKey: 'ElevenLabs STT には ELEVENLABS_API_KEY が必要です。',
|
||||
|
||||
@@ -194,7 +194,10 @@ export const ru = defineLocale({
|
||||
errorTitle: 'Сервер MCP недоступен',
|
||||
errorMessage: name => `MCP ${name} не прошёл проверку работоспособности.`,
|
||||
signIn: 'Войти',
|
||||
view: 'Просмотр'
|
||||
view: 'Просмотр',
|
||||
disable: 'Отключить',
|
||||
disabledMessage: name => `${name} MCP отключён. Включить снова можно в любой момент в разделе Возможности → MCP.`,
|
||||
disableFailed: name => `Не удалось отключить ${name} MCP.`
|
||||
},
|
||||
errors: {
|
||||
elevenLabsNeedsKey: 'Для STT ElevenLabs нужен ELEVENLABS_API_KEY.',
|
||||
|
||||
@@ -258,6 +258,9 @@ export interface Translations {
|
||||
errorMessage: (name: string) => string
|
||||
signIn: string
|
||||
view: string
|
||||
disable: string
|
||||
disabledMessage: (name: string) => string
|
||||
disableFailed: (name: string) => string
|
||||
}
|
||||
errors: {
|
||||
elevenLabsNeedsKey: string
|
||||
|
||||
@@ -178,7 +178,10 @@ export const zhHant = defineLocale({
|
||||
errorTitle: 'MCP 伺服器無法連線',
|
||||
errorMessage: name => `${name} MCP 健康檢查失敗。`,
|
||||
signIn: '登入',
|
||||
view: '檢視'
|
||||
view: '檢視',
|
||||
disable: '停用',
|
||||
disabledMessage: name => `已停用 ${name} MCP。可隨時在「功能 → MCP」重新啟用。`,
|
||||
disableFailed: name => `無法停用 ${name} MCP。`
|
||||
},
|
||||
errors: {
|
||||
elevenLabsNeedsKey: 'ElevenLabs STT 需要 ELEVENLABS_API_KEY。',
|
||||
|
||||
@@ -207,7 +207,10 @@ export const zh = defineLocale({
|
||||
errorTitle: 'MCP 服务器无法连接',
|
||||
errorMessage: name => `${name} MCP 健康检查失败。`,
|
||||
signIn: '登录',
|
||||
view: '查看'
|
||||
view: '查看',
|
||||
disable: '禁用',
|
||||
disabledMessage: name => `已禁用 ${name} MCP。可随时在「能力 → MCP」中重新启用。`,
|
||||
disableFailed: name => `无法禁用 ${name} MCP。`
|
||||
},
|
||||
errors: {
|
||||
elevenLabsNeedsKey: 'ElevenLabs STT 需要 ELEVENLABS_API_KEY。',
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
// @vitest-environment jsdom
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const mocks = vi.hoisted(() => {
|
||||
@@ -31,13 +32,16 @@ const mocks = vi.hoisted(() => {
|
||||
activeProfile: makeAtom('default'),
|
||||
gatewayState: makeAtom<'closed' | 'open'>('closed'),
|
||||
getHermesConfigRecord: vi.fn(),
|
||||
notify: vi.fn()
|
||||
notify: vi.fn(),
|
||||
setMcpServerEnabled: vi.fn().mockResolvedValue({ ok: true }),
|
||||
testMcpServer: vi.fn()
|
||||
}
|
||||
})
|
||||
|
||||
vi.mock('@/hermes', () => ({
|
||||
getHermesConfigRecord: mocks.getHermesConfigRecord,
|
||||
testMcpServer: vi.fn()
|
||||
setMcpServerEnabled: mocks.setMcpServerEnabled,
|
||||
testMcpServer: mocks.testMcpServer
|
||||
}))
|
||||
|
||||
vi.mock('@/i18n', () => ({
|
||||
@@ -45,7 +49,8 @@ vi.mock('@/i18n', () => ({
|
||||
}))
|
||||
|
||||
vi.mock('@/store/notifications', () => ({
|
||||
notify: mocks.notify
|
||||
notify: mocks.notify,
|
||||
notifyError: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('@/store/profile', () => ({
|
||||
@@ -57,7 +62,7 @@ vi.mock('@/store/session', () => ({
|
||||
$gatewayState: mocks.gatewayState
|
||||
}))
|
||||
|
||||
const { shouldNotifyOnTransition, startMcpHealthChecker, stopMcpHealthChecker } = await import('./mcp-health')
|
||||
const { shouldNotify, startMcpHealthChecker, stopMcpHealthChecker } = await import('./mcp-health')
|
||||
|
||||
type Status = 'error' | 'needs-auth' | 'ok'
|
||||
|
||||
@@ -69,11 +74,16 @@ afterEach(() => {
|
||||
mocks.activeProfile.set('default')
|
||||
mocks.getHermesConfigRecord.mockReset()
|
||||
mocks.notify.mockReset()
|
||||
mocks.testMcpServer.mockReset()
|
||||
mocks.setMcpServerEnabled.mockClear()
|
||||
})
|
||||
|
||||
describe('shouldNotifyOnTransition', () => {
|
||||
// The full previous × next decision table: notify only on a TRANSITION into
|
||||
// a bad state. Rechecks of an already-bad server stay quiet; ok never nudges.
|
||||
describe('shouldNotify', () => {
|
||||
const DAY = 24 * 60 * 60 * 1000
|
||||
const now = 1_000_000
|
||||
|
||||
// Transition table with the snooze still active: notify only on a TRANSITION
|
||||
// into a bad state; ok never nudges.
|
||||
it.each<[previous: Status | null, next: Status, notify: boolean]>([
|
||||
[null, 'ok', false],
|
||||
[null, 'needs-auth', true],
|
||||
@@ -87,9 +97,60 @@ describe('shouldNotifyOnTransition', () => {
|
||||
['error', 'needs-auth', true],
|
||||
['needs-auth', 'ok', false],
|
||||
['error', 'ok', false]
|
||||
])('previous=%s next=%s → notify=%s', (previous, next, expected) => {
|
||||
expect(shouldNotifyOnTransition(previous, next)).toBe(expected)
|
||||
])('snoozed: previous=%s next=%s → notify=%s', (previous, next, expected) => {
|
||||
expect(shouldNotify(previous, next, now + DAY, now)).toBe(expected)
|
||||
})
|
||||
|
||||
it('re-nudges a server that stays broken once the daily snooze lapses, never for ok', () => {
|
||||
expect(shouldNotify('needs-auth', 'needs-auth', now - 1, now)).toBe(true)
|
||||
expect(shouldNotify('error', 'error', now, now)).toBe(true)
|
||||
expect(shouldNotify('ok', 'ok', now - DAY, now)).toBe(false)
|
||||
expect(shouldNotify('needs-auth', 'ok', 0, now)).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
it('shows the toast with Sign in + Disable, then stays quiet for a day and re-nudges after it', async () => {
|
||||
const servers = { mcp_servers: { linear: { url: 'https://mcp.linear.app/mcp', auth: 'oauth' } } }
|
||||
mocks.getHermesConfigRecord.mockResolvedValue(servers)
|
||||
mocks.testMcpServer.mockResolvedValue({ ok: false, error: 'OAuth: authorization required', tools: [] })
|
||||
window.localStorage.clear()
|
||||
|
||||
let clock = 1_700_000_000_000
|
||||
const nowSpy = vi.spyOn(Date, 'now').mockImplementation(() => clock)
|
||||
|
||||
try {
|
||||
startMcpHealthChecker()
|
||||
mocks.gatewayState.set('open')
|
||||
await flush()
|
||||
await flush()
|
||||
expect(mocks.notify).toHaveBeenCalledTimes(1)
|
||||
const toast = mocks.notify.mock.calls[0][0]
|
||||
expect(toast.action.label).toBe('notifications.mcp.signIn')
|
||||
expect(toast.secondaryAction.label).toBe('notifications.mcp.disable')
|
||||
|
||||
// Same day, still broken: a reconnect sweep must not re-pop.
|
||||
clock += 60 * 60 * 1000
|
||||
mocks.gatewayState.set('closed')
|
||||
mocks.gatewayState.set('open')
|
||||
await flush()
|
||||
await flush()
|
||||
expect(mocks.notify).toHaveBeenCalledTimes(1)
|
||||
|
||||
// Next day, still broken: one more nudge.
|
||||
clock += 24 * 60 * 60 * 1000
|
||||
mocks.gatewayState.set('closed')
|
||||
mocks.gatewayState.set('open')
|
||||
await flush()
|
||||
await flush()
|
||||
expect(mocks.notify).toHaveBeenCalledTimes(2)
|
||||
|
||||
// Disable from the toast flips enabled:false on the backend.
|
||||
toast.secondaryAction.onClick()
|
||||
await flush()
|
||||
expect(mocks.setMcpServerEnabled).toHaveBeenCalledWith('linear', false)
|
||||
} finally {
|
||||
nowSpy.mockRestore()
|
||||
}
|
||||
})
|
||||
|
||||
it('coalesces reconnects during a sweep into one fresh follow-up sweep', async () => {
|
||||
|
||||
@@ -1,8 +1,10 @@
|
||||
/**
|
||||
* Background MCP health checker. On gateway connect — and every 30 minutes
|
||||
* after — probe the ACTIVE profile's enabled HTTP/SSE MCP servers and nudge
|
||||
* the user when one transitions into needs-auth (expired OAuth token) or
|
||||
* error, with a one-click path to the MCP page's Authenticate button.
|
||||
* the user when one is in needs-auth (expired OAuth token) or error: on the
|
||||
* transition, then at most once a day while it stays broken, with a
|
||||
* one-click path to the MCP page's Authenticate button and a Disable button
|
||||
* for servers the user no longer wants.
|
||||
*
|
||||
* Scope is deliberate: stdio servers are NEVER probed here. Probing a stdio
|
||||
* server SPAWNS a local process, so a background timer would silently launch
|
||||
@@ -12,11 +14,12 @@
|
||||
* the other just learned.
|
||||
*/
|
||||
|
||||
import { getHermesConfigRecord, type McpTestResult, testMcpServer } from '@/hermes'
|
||||
import { getHermesConfigRecord, type McpTestResult, setMcpServerEnabled, testMcpServer } from '@/hermes'
|
||||
import { translateNow } from '@/i18n'
|
||||
import { classifyProbe, freshProbe, probeCache, probeKey } from '@/lib/mcp-probe-cache'
|
||||
import { getServers } from '@/lib/mcp-servers'
|
||||
import { notify } from '@/store/notifications'
|
||||
import { persistString, storedString } from '@/lib/storage'
|
||||
import { notify, notifyError } from '@/store/notifications'
|
||||
import { $activeGatewayProfile, normalizeProfileKey } from '@/store/profile'
|
||||
import { $gatewayState } from '@/store/session'
|
||||
|
||||
@@ -28,21 +31,51 @@ const CHECK_INTERVAL_MS = 30 * 60_000
|
||||
export type McpHealthStatus = 'error' | 'needs-auth' | 'ok'
|
||||
|
||||
/**
|
||||
* The notify decision, as a pure state machine: nudge only on a TRANSITION
|
||||
* into a bad state — never on every recheck of an already-known-bad server,
|
||||
* and never for ok. An unknown previous state (first sweep of the session)
|
||||
* counts as a transition: an expired token discovered at launch is exactly
|
||||
* the case this exists for.
|
||||
* The notify decision, as a pure state machine: nudge on a TRANSITION into a
|
||||
* bad state — never for ok. An unknown previous state (first sweep of the
|
||||
* session) counts as a transition: an expired token discovered at launch is
|
||||
* exactly the case this exists for.
|
||||
*
|
||||
* A server that STAYS broken is nudged again once the daily snooze lapses:
|
||||
* a dead OAuth token is a standing problem the user has to act on (sign in
|
||||
* again, or disable the server), and a one-shot toast they closed on Monday
|
||||
* is forgotten by Wednesday. `snoozedUntil` is the persisted per-server
|
||||
* cooldown (set when a toast is shown); the recheck path re-nudges only past
|
||||
* it, so an unchanged bad state costs at most one toast per day.
|
||||
*/
|
||||
export function shouldNotifyOnTransition(previous: McpHealthStatus | null, next: McpHealthStatus): boolean {
|
||||
return (next === 'error' || next === 'needs-auth') && previous !== next
|
||||
export function shouldNotify(
|
||||
previous: McpHealthStatus | null,
|
||||
next: McpHealthStatus,
|
||||
snoozedUntil: number,
|
||||
now: number
|
||||
): boolean {
|
||||
if (next !== 'error' && next !== 'needs-auth') {
|
||||
return false
|
||||
}
|
||||
|
||||
return previous !== next || now >= snoozedUntil
|
||||
}
|
||||
|
||||
// Last-known status per (profile, server) — the transition memory — and the
|
||||
// per-app-session notification cap. Both keyed by profile so one profile's
|
||||
// broken server can't mute or trigger another's (AGENTS.md scope-in-key).
|
||||
// Same time-based snooze the update/skew toasts use (store/updates.ts): a
|
||||
// shown toast arms a 24h cooldown for that (profile, server), persisted so an
|
||||
// app restart does not re-nudge before the day is up.
|
||||
const SNOOZE_KEY_PREFIX = 'hermes:mcp-health-snooze-until:'
|
||||
const SNOOZE_MS = 24 * 60 * 60 * 1000
|
||||
|
||||
function snoozedUntil(key: string): number {
|
||||
const until = Number(storedString(SNOOZE_KEY_PREFIX + key) || 0)
|
||||
|
||||
return Number.isFinite(until) ? until : 0
|
||||
}
|
||||
|
||||
function snooze(key: string): void {
|
||||
persistString(SNOOZE_KEY_PREFIX + key, String(Date.now() + SNOOZE_MS))
|
||||
}
|
||||
|
||||
// Last-known status per (profile, server) — the transition memory. Keyed by
|
||||
// profile so one profile's broken server can't mute or trigger another's
|
||||
// (AGENTS.md scope-in-key).
|
||||
const lastStatus = new Map<string, McpHealthStatus>()
|
||||
const notifiedThisSession = new Set<string>()
|
||||
|
||||
let started = false
|
||||
let timer: ReturnType<typeof setInterval> | null = null
|
||||
@@ -64,16 +97,33 @@ function openMcpServerPage(name: string): void {
|
||||
window.location.hash = `#/skills?tab=mcp&server=${encodeURIComponent(name)}`
|
||||
}
|
||||
|
||||
// "Disable" from the toast: `enabled: false` in config.yaml (the server stays
|
||||
// listed on the MCP page for a later re-enable). The backend follows the edit
|
||||
// on its own — the gateway's config reconcile and the serve backend's next
|
||||
// reload both drop a disabled server — so no reload RPC is issued here.
|
||||
async function disableServer(profileKey: string, name: string): Promise<void> {
|
||||
try {
|
||||
await setMcpServerEnabled(name, false)
|
||||
lastStatus.delete(`${profileKey}::${name}`)
|
||||
notify({
|
||||
kind: 'success',
|
||||
message: translateNow('notifications.mcp.disabledMessage', name)
|
||||
})
|
||||
} catch (err) {
|
||||
notifyError(err, translateNow('notifications.mcp.disableFailed', name))
|
||||
}
|
||||
}
|
||||
|
||||
function recordResult(profileKey: string, name: string, status: McpHealthStatus): void {
|
||||
const key = `${profileKey}::${name}`
|
||||
const previous = lastStatus.get(key) ?? null
|
||||
lastStatus.set(key, status)
|
||||
|
||||
if (!shouldNotifyOnTransition(previous, status) || notifiedThisSession.has(key)) {
|
||||
if (!shouldNotify(previous, status, snoozedUntil(key), Date.now())) {
|
||||
return
|
||||
}
|
||||
|
||||
notifiedThisSession.add(key)
|
||||
snooze(key)
|
||||
|
||||
const needsAuth = status === 'needs-auth'
|
||||
|
||||
@@ -85,6 +135,10 @@ function recordResult(profileKey: string, name: string, status: McpHealthStatus)
|
||||
id: `mcp-health-${key}`,
|
||||
kind: 'warning',
|
||||
message: translateNow(needsAuth ? 'notifications.mcp.needsAuthMessage' : 'notifications.mcp.errorMessage', name),
|
||||
secondaryAction: {
|
||||
label: translateNow('notifications.mcp.disable'),
|
||||
onClick: () => void disableServer(profileKey, name)
|
||||
},
|
||||
title: translateNow(needsAuth ? 'notifications.mcp.needsAuthTitle' : 'notifications.mcp.errorTitle')
|
||||
})
|
||||
}
|
||||
@@ -203,7 +257,7 @@ export function startMcpHealthChecker(): void {
|
||||
|
||||
// Profile switch: invalidate in-flight sweeps, drop the timer, and re-arm
|
||||
// for the new profile (fresh immediate sweep + fresh interval) if its
|
||||
// gateway is connected. lastStatus/notifiedThisSession are profile-keyed,
|
||||
// gateway is connected. lastStatus and the snooze keys are profile-keyed,
|
||||
// so no wipe is needed — profile A's transition memory stays intact for
|
||||
// when the user switches back.
|
||||
offProfile = $activeGatewayProfile.listen(() => {
|
||||
|
||||
@@ -25,6 +25,8 @@ export interface AppNotification {
|
||||
message: string
|
||||
detail?: string
|
||||
action?: NotificationAction
|
||||
/** Second, quieter button beside `action` (e.g. "Disable" next to "Sign in"). */
|
||||
secondaryAction?: NotificationAction
|
||||
onDismiss?: () => void
|
||||
createdAt: number
|
||||
placement?: NotificationPlacement
|
||||
@@ -40,6 +42,7 @@ export interface NotificationInput {
|
||||
message: string
|
||||
detail?: string
|
||||
action?: NotificationAction
|
||||
secondaryAction?: NotificationAction
|
||||
onDismiss?: () => void
|
||||
durationMs?: number
|
||||
placement?: NotificationPlacement
|
||||
@@ -173,6 +176,7 @@ export function notify(input: NotificationInput): string {
|
||||
message: input.message,
|
||||
detail: input.detail,
|
||||
action: input.action,
|
||||
secondaryAction: input.secondaryAction,
|
||||
onDismiss: input.onDismiss,
|
||||
createdAt: Date.now(),
|
||||
placement: input.placement ?? defaultPlacement(kind, input.action)
|
||||
|
||||
Reference in New Issue
Block a user