From 68eb05986328c2e2f1992253b605fa0889ff9e40 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sun, 13 Sep 2026 09:43:02 -0700 Subject: [PATCH] feat(desktop): daily re-auth nudge for MCP servers with a Disable action MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- apps/desktop/src/components/notifications.tsx | 42 ++++++--- apps/desktop/src/i18n/ar.ts | 5 +- apps/desktop/src/i18n/en.ts | 5 +- apps/desktop/src/i18n/ja.ts | 5 +- apps/desktop/src/i18n/ru.ts | 5 +- apps/desktop/src/i18n/types.ts | 3 + apps/desktop/src/i18n/zh-hant.ts | 5 +- apps/desktop/src/i18n/zh.ts | 5 +- apps/desktop/src/store/mcp-health.test.ts | 79 ++++++++++++++-- apps/desktop/src/store/mcp-health.ts | 90 +++++++++++++++---- apps/desktop/src/store/notifications.ts | 4 + 11 files changed, 202 insertions(+), 46 deletions(-) diff --git a/apps/desktop/src/components/notifications.tsx b/apps/desktop/src/components/notifications.tsx index e8a8dda8d6..267ad31f55 100644 --- a/apps/desktop/src/components/notifications.tsx +++ b/apps/desktop/src/components/notifications.tsx @@ -229,19 +229,35 @@ function NotificationItem({ notification }: { notification: AppNotification }) {

{renderMessage(notification.message, accent)}

{notification.meta &&

{notification.meta}

} {hasDetail && } - {notification.action && ( - + {(notification.action || notification.secondaryAction) && ( +
+ {notification.action && ( + + )} + {notification.secondaryAction && ( + + )} +
)} diff --git a/apps/desktop/src/i18n/ar.ts b/apps/desktop/src/i18n/ar.ts index 6d70110018..7b3711e9eb 100644 --- a/apps/desktop/src/i18n/ar.ts +++ b/apps/desktop/src/i18n/ar.ts @@ -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.', diff --git a/apps/desktop/src/i18n/en.ts b/apps/desktop/src/i18n/en.ts index acbde2d114..d58951e06b 100644 --- a/apps/desktop/src/i18n/en.ts +++ b/apps/desktop/src/i18n/en.ts @@ -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.', diff --git a/apps/desktop/src/i18n/ja.ts b/apps/desktop/src/i18n/ja.ts index cdd512551c..db22eaa6bb 100644 --- a/apps/desktop/src/i18n/ja.ts +++ b/apps/desktop/src/i18n/ja.ts @@ -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 が必要です。', diff --git a/apps/desktop/src/i18n/ru.ts b/apps/desktop/src/i18n/ru.ts index e8ffa262a3..9c4dd27d2d 100644 --- a/apps/desktop/src/i18n/ru.ts +++ b/apps/desktop/src/i18n/ru.ts @@ -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.', diff --git a/apps/desktop/src/i18n/types.ts b/apps/desktop/src/i18n/types.ts index 0c16cb4b06..3a434bcc15 100644 --- a/apps/desktop/src/i18n/types.ts +++ b/apps/desktop/src/i18n/types.ts @@ -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 diff --git a/apps/desktop/src/i18n/zh-hant.ts b/apps/desktop/src/i18n/zh-hant.ts index 4759253d15..6bf6adfe22 100644 --- a/apps/desktop/src/i18n/zh-hant.ts +++ b/apps/desktop/src/i18n/zh-hant.ts @@ -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。', diff --git a/apps/desktop/src/i18n/zh.ts b/apps/desktop/src/i18n/zh.ts index 7e78217d1b..cb3294104b 100644 --- a/apps/desktop/src/i18n/zh.ts +++ b/apps/desktop/src/i18n/zh.ts @@ -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。', diff --git a/apps/desktop/src/store/mcp-health.test.ts b/apps/desktop/src/store/mcp-health.test.ts index 86f8e94e89..3e41277e27 100644 --- a/apps/desktop/src/store/mcp-health.test.ts +++ b/apps/desktop/src/store/mcp-health.test.ts @@ -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 () => { diff --git a/apps/desktop/src/store/mcp-health.ts b/apps/desktop/src/store/mcp-health.ts index c609be13ff..abb0346f35 100644 --- a/apps/desktop/src/store/mcp-health.ts +++ b/apps/desktop/src/store/mcp-health.ts @@ -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() -const notifiedThisSession = new Set() let started = false let timer: ReturnType | 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 { + 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(() => { diff --git a/apps/desktop/src/store/notifications.ts b/apps/desktop/src/store/notifications.ts index 55ce13f047..6fa272d5ea 100644 --- a/apps/desktop/src/store/notifications.ts +++ b/apps/desktop/src/store/notifications.ts @@ -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)