mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(mcp): surface 401s as an actionable "needs sign-in" state (#3719)
Closes #3719. MCP HTTP 401s now report a distinct `Unauthorized` state (typed McpUnauthorizedError + atomic ConnectFailure snapshot) with a localized 'Sign in' notice instead of a raw-error dead-end; recovery polling + i18n across 14 locales.
This commit is contained in:
sanil-23
@@ -262,6 +262,30 @@ describe('InstalledServerDetail', () => {
|
||||
expect(screen.getByText('Timed out')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('renders a graceful auth notice (not a raw error) for unauthorized status', () => {
|
||||
render(
|
||||
<InstalledServerDetail
|
||||
server={BASE_SERVER}
|
||||
connStatus={{
|
||||
server_id: 'srv-1',
|
||||
qualified_name: 'acme/test-server',
|
||||
display_name: 'Test Server',
|
||||
status: 'unauthorized',
|
||||
tool_count: 0,
|
||||
// Core sends no raw error for a 401 (avoids leaking the OAuth URL).
|
||||
last_error: undefined,
|
||||
}}
|
||||
onUninstalled={() => {}}
|
||||
/>
|
||||
);
|
||||
// Friendly "sign in needed" badge + actionable notice, no raw HTTP string.
|
||||
expect(screen.getByText('Sign in needed')).toBeInTheDocument();
|
||||
expect(screen.getByText(/needs you to sign in or add an access token/i)).toBeInTheDocument();
|
||||
expect(screen.queryByText(/HTTP 401/i)).not.toBeInTheDocument();
|
||||
// The primary action is relabelled "Sign in" (it opens the auth modal).
|
||||
expect(screen.getByRole('button', { name: 'Sign in' })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
// ----------------------------------------------------------------------
|
||||
// Env reconfiguration (issue #3039)
|
||||
// ----------------------------------------------------------------------
|
||||
|
||||
@@ -245,8 +245,21 @@ const InstalledServerDetail = ({
|
||||
</div>
|
||||
</div>
|
||||
|
||||
{/* Error */}
|
||||
{(error || connStatus?.last_error) && (
|
||||
{/* Auth required — a graceful, actionable notice rather than the raw HTTP
|
||||
401 string. The core reports `unauthorized` (no raw error) for a 401;
|
||||
the Connect button below opens the auth modal, which probes the server
|
||||
and offers browser sign-in or a token field as appropriate (#3719). */}
|
||||
{status === 'unauthorized' && (
|
||||
<div className="rounded-lg border border-amber-200 dark:border-amber-500/30 bg-amber-50 dark:bg-amber-500/10 px-4 py-3 text-sm text-amber-700 dark:text-amber-300">
|
||||
{t('mcp.detail.authRequired')}
|
||||
</div>
|
||||
)}
|
||||
|
||||
{/* Error — a genuine (non-auth) connect failure. Suppressed entirely while
|
||||
`unauthorized`: the amber notice above is the only message shown, so a
|
||||
local action error (e.g. a reconfigure reconnect that re-hits the 401)
|
||||
can't re-expose raw transport/auth text in this state (#3719). */}
|
||||
{status !== 'unauthorized' && (error || connStatus?.last_error) && (
|
||||
<div className="rounded-lg border border-coral-200 dark:border-coral-500/30 bg-coral-50 dark:bg-coral-500/10 px-4 py-3 text-sm text-coral-700 dark:text-coral-300">
|
||||
{error ?? connStatus?.last_error}
|
||||
</div>
|
||||
@@ -270,7 +283,11 @@ const InstalledServerDetail = ({
|
||||
disabled={busy || status === 'connecting'}
|
||||
onClick={handleConnect}
|
||||
className="rounded-lg bg-primary-500 px-3 py-1.5 text-xs font-medium text-white hover:bg-primary-600 disabled:opacity-50 transition-colors">
|
||||
{status === 'connecting' ? t('mcp.detail.connecting') : t('mcp.detail.connect')}
|
||||
{status === 'connecting'
|
||||
? t('mcp.detail.connecting')
|
||||
: status === 'unauthorized'
|
||||
? t('mcp.detail.authenticate')
|
||||
: t('mcp.detail.connect')}
|
||||
</button>
|
||||
) : (
|
||||
<button
|
||||
|
||||
@@ -28,6 +28,7 @@ const STATUS_DOT: Record<ServerStatus, string> = {
|
||||
connected: 'bg-sage-500',
|
||||
connecting: 'bg-amber-400',
|
||||
disconnected: 'bg-stone-300 dark:bg-neutral-600',
|
||||
unauthorized: 'bg-amber-500',
|
||||
error: 'bg-coral-500',
|
||||
disabled: 'bg-stone-200 dark:bg-neutral-700',
|
||||
};
|
||||
@@ -40,6 +41,7 @@ const STATUS_I18N_KEYS: Record<ServerStatus, string> = {
|
||||
connected: 'channels.status.connected',
|
||||
connecting: 'channels.status.connecting',
|
||||
disconnected: 'channels.status.disconnected',
|
||||
unauthorized: 'mcp.status.unauthorized',
|
||||
error: 'channels.status.error',
|
||||
disabled: 'mcp.status.disabled',
|
||||
};
|
||||
|
||||
@@ -55,6 +55,11 @@ const computeHealthCounts = (statuses: ConnStatus[]): HealthCounts => {
|
||||
case 'connecting':
|
||||
connectingCount += 1;
|
||||
break;
|
||||
// An `unauthorized` server is not connected, but it must NOT join the
|
||||
// `errorIds` set — "Retry all" blindly reconnects those, which would just
|
||||
// 401 again. Re-auth is a deliberate per-server action (sign in / token),
|
||||
// so we surface it under the disconnected tally rather than as an error.
|
||||
case 'unauthorized':
|
||||
case 'disconnected':
|
||||
disconnectedCount += 1;
|
||||
break;
|
||||
|
||||
@@ -32,6 +32,7 @@ const STATUS_DOT: Record<ServerStatus, string> = {
|
||||
connected: 'bg-sage-500',
|
||||
connecting: 'bg-amber-400',
|
||||
disconnected: 'bg-stone-300 dark:bg-neutral-600',
|
||||
unauthorized: 'bg-amber-500',
|
||||
error: 'bg-coral-500',
|
||||
disabled: 'bg-stone-200 dark:bg-neutral-700',
|
||||
};
|
||||
@@ -119,8 +120,18 @@ const McpServersTab = () => {
|
||||
|
||||
// Poll status
|
||||
useEffect(() => {
|
||||
const hasConnected = statuses.some(s => s.status === 'connected');
|
||||
if (!hasConnected) {
|
||||
// Poll while anything is in a non-terminal state — not just `connected`.
|
||||
// An `unauthorized`/`error`/`connecting` server can transition (the
|
||||
// background reconnect supervisor, a completed OAuth sign-in, an expiring
|
||||
// token) and the UI must reflect that without a manual refresh (#3719 RC5).
|
||||
const hasActive = statuses.some(
|
||||
s =>
|
||||
s.status === 'connected' ||
|
||||
s.status === 'connecting' ||
|
||||
s.status === 'unauthorized' ||
|
||||
s.status === 'error'
|
||||
);
|
||||
if (!hasActive) {
|
||||
if (pollTimerRef.current) {
|
||||
clearTimeout(pollTimerRef.current);
|
||||
pollTimerRef.current = null;
|
||||
|
||||
@@ -13,6 +13,7 @@ describe('McpStatusBadge', () => {
|
||||
['connected', 'Connected'],
|
||||
['connecting', 'Connecting'],
|
||||
['disconnected', 'Disconnected'],
|
||||
['unauthorized', 'Sign in needed'],
|
||||
['error', 'Error'],
|
||||
])('renders i18n label for status=%s', (status, expectedLabel) => {
|
||||
render(<McpStatusBadge status={status} />);
|
||||
|
||||
@@ -21,6 +21,10 @@ const STATUS_META: Record<ServerStatus, { i18nKey: string; className: string }>
|
||||
className:
|
||||
'bg-stone-100 dark:bg-neutral-800 text-stone-500 dark:text-neutral-400 border-stone-200 dark:border-neutral-700',
|
||||
},
|
||||
unauthorized: {
|
||||
i18nKey: 'mcp.status.unauthorized',
|
||||
className: 'bg-amber-500/10 text-amber-700 border-amber-500/30 dark:text-amber-300',
|
||||
},
|
||||
error: {
|
||||
i18nKey: 'channels.status.error',
|
||||
className: 'bg-coral-500/10 text-coral-700 border-coral-500/30 dark:text-coral-300',
|
||||
|
||||
@@ -45,7 +45,15 @@ export type InstalledServer = {
|
||||
|
||||
export type McpTool = { name: string; description?: string; input_schema: unknown };
|
||||
|
||||
export type ServerStatus = 'disconnected' | 'connecting' | 'connected' | 'error' | 'disabled';
|
||||
export type ServerStatus =
|
||||
| 'disconnected'
|
||||
| 'connecting'
|
||||
| 'connected'
|
||||
// Server reachable but rejected the connect with HTTP 401 — needs sign-in or
|
||||
// an access token. Distinct from `error` so the UI offers a re-auth path.
|
||||
| 'unauthorized'
|
||||
| 'error'
|
||||
| 'disabled';
|
||||
|
||||
export type ConnStatus = {
|
||||
server_id: string;
|
||||
|
||||
@@ -1305,6 +1305,9 @@ const messages: TranslationMap = {
|
||||
'mcp.detail.suggestedEnvBody': 'أعِد تثبيت هذا الخادم بالقيم المقترحة لتطبيقها: {keys}',
|
||||
'mcp.detail.connect': 'اتصال',
|
||||
'mcp.detail.connecting': 'جارٍ الاتصال...',
|
||||
'mcp.detail.authenticate': 'تسجيل الدخول',
|
||||
'mcp.detail.authRequired':
|
||||
'يتطلب هذا الخادم تسجيل الدخول أو إضافة رمز وصول قبل أن يتمكن من الاتصال. انقر على “تسجيل الدخول” للمصادقة.',
|
||||
'mcp.detail.disconnect': 'قطع الاتصال',
|
||||
'mcp.detail.hideAssistant': 'إخفاء المساعد',
|
||||
'mcp.detail.helpConfigure': 'ساعدني في التهيئة',
|
||||
@@ -1322,6 +1325,7 @@ const messages: TranslationMap = {
|
||||
'mcp.detail.enable': 'تفعيل',
|
||||
'mcp.detail.disable': 'تعطيل',
|
||||
'mcp.status.disabled': 'معطّل',
|
||||
'mcp.status.unauthorized': 'يلزم تسجيل الدخول',
|
||||
'mcp.detail.tools': 'الأدوات',
|
||||
'mcp.connectAuth.title': 'الاتصال بـ {name}',
|
||||
'mcp.connectAuth.hint':
|
||||
|
||||
@@ -1330,6 +1330,9 @@ const messages: TranslationMap = {
|
||||
'এই সার্ভারে-ইনস্টল করার জন্য পরামর্শ দেওয়া হল: xqxq0x ব্যবহার করুন',
|
||||
'mcp.detail.connect': 'সংযোগ করুন',
|
||||
'mcp.detail.connecting': 'সংযুক্ত হচ্ছে...',
|
||||
'mcp.detail.authenticate': 'সাইন ইন করুন',
|
||||
'mcp.detail.authRequired':
|
||||
'এই সার্ভারটি সংযোগ করার আগে আপনাকে সাইন ইন করতে বা একটি অ্যাক্সেস টোকেন যোগ করতে হবে। প্রমাণীকরণ করতে “সাইন ইন করুন”-এ ক্লিক করুন।',
|
||||
'mcp.detail.disconnect': 'সংযোগ বিচ্ছিন্ন করুন',
|
||||
'mcp.detail.hideAssistant': 'সহকারী লুকান',
|
||||
'mcp.detail.helpConfigure': 'কনফিগার করতে আমাকে সাহায্য করুন',
|
||||
@@ -1348,6 +1351,7 @@ const messages: TranslationMap = {
|
||||
'mcp.detail.enable': 'সক্রিয় করুন',
|
||||
'mcp.detail.disable': 'নিষ্ক্রিয় করুন',
|
||||
'mcp.status.disabled': 'নিষ্ক্রিয়',
|
||||
'mcp.status.unauthorized': 'সাইন ইন প্রয়োজন',
|
||||
'mcp.detail.tools': 'টুলস',
|
||||
'mcp.connectAuth.title': '{name} সংযুক্ত করুন',
|
||||
'mcp.connectAuth.hint':
|
||||
|
||||
@@ -1368,6 +1368,9 @@ const messages: TranslationMap = {
|
||||
'Installieren Sie diesen Server mit den vorgeschlagenen Werten neu, um sie anzuwenden: {keys}',
|
||||
'mcp.detail.connect': 'Verbinden',
|
||||
'mcp.detail.connecting': 'Verbinden ...',
|
||||
'mcp.detail.authenticate': 'Anmelden',
|
||||
'mcp.detail.authRequired':
|
||||
'Dieser Server erfordert, dass Sie sich anmelden oder ein Zugriffstoken hinzufügen, bevor er eine Verbindung herstellen kann. Klicken Sie auf „Anmelden“, um sich zu authentifizieren.',
|
||||
'mcp.detail.disconnect': 'Trennen',
|
||||
'mcp.detail.hideAssistant': 'Ausblenden Assistent',
|
||||
'mcp.detail.helpConfigure': 'Helfen Sie mir bei der Konfiguration',
|
||||
@@ -1386,6 +1389,7 @@ const messages: TranslationMap = {
|
||||
'mcp.detail.enable': 'Aktivieren',
|
||||
'mcp.detail.disable': 'Deaktivieren',
|
||||
'mcp.status.disabled': 'Deaktiviert',
|
||||
'mcp.status.unauthorized': 'Anmeldung erforderlich',
|
||||
'mcp.detail.tools': 'Werkzeuge',
|
||||
'mcp.connectAuth.title': '{name} verbinden',
|
||||
'mcp.connectAuth.hint':
|
||||
|
||||
@@ -1685,6 +1685,9 @@ const en: TranslationMap = {
|
||||
'Re-install this server with the suggested values to apply them: {keys}',
|
||||
'mcp.detail.connect': 'Connect',
|
||||
'mcp.detail.connecting': 'Connecting...',
|
||||
'mcp.detail.authenticate': 'Sign in',
|
||||
'mcp.detail.authRequired':
|
||||
'This server needs you to sign in or add an access token before it can connect. Click “Sign in” to authenticate.',
|
||||
'mcp.detail.disconnect': 'Disconnect',
|
||||
'mcp.detail.hideAssistant': 'Hide assistant',
|
||||
'mcp.detail.helpConfigure': 'Help me configure',
|
||||
@@ -1726,6 +1729,7 @@ const en: TranslationMap = {
|
||||
'mcp.detail.enable': 'Enable',
|
||||
'mcp.detail.disable': 'Disable',
|
||||
'mcp.status.disabled': 'Disabled',
|
||||
'mcp.status.unauthorized': 'Sign in needed',
|
||||
'mcp.detail.tools': 'Tools',
|
||||
'onboarding.skipForNow': 'Skip for Now',
|
||||
'onboarding.localAI.continueWithCloud': 'Continue with Cloud',
|
||||
|
||||
@@ -1364,6 +1364,9 @@ const messages: TranslationMap = {
|
||||
'Reinstale este servidor con los valores sugeridos para aplicarlos: {keys}',
|
||||
'mcp.detail.connect': 'Conectar',
|
||||
'mcp.detail.connecting': 'Conectando...',
|
||||
'mcp.detail.authenticate': 'Iniciar sesión',
|
||||
'mcp.detail.authRequired':
|
||||
'Este servidor necesita que inicies sesión o agregues un token de acceso antes de poder conectarse. Haz clic en “Iniciar sesión” para autenticarte.',
|
||||
'mcp.detail.disconnect': 'Desconectar',
|
||||
'mcp.detail.hideAssistant': 'Ocultar asistente',
|
||||
'mcp.detail.helpConfigure': 'Ayúdame a configurar',
|
||||
@@ -1382,6 +1385,7 @@ const messages: TranslationMap = {
|
||||
'mcp.detail.enable': 'Habilitar',
|
||||
'mcp.detail.disable': 'Deshabilitar',
|
||||
'mcp.status.disabled': 'Deshabilitado',
|
||||
'mcp.status.unauthorized': 'Inicio de sesión necesario',
|
||||
'mcp.detail.tools': 'Herramientas',
|
||||
'mcp.connectAuth.title': 'Conectar {name}',
|
||||
'mcp.connectAuth.hint':
|
||||
|
||||
@@ -1370,6 +1370,9 @@ const messages: TranslationMap = {
|
||||
'Réinstallez ce serveur avec les valeurs suggérées pour les appliquer: {keys}',
|
||||
'mcp.detail.connect': 'Connecter',
|
||||
'mcp.detail.connecting': 'Connexion...',
|
||||
'mcp.detail.authenticate': 'Se connecter',
|
||||
'mcp.detail.authRequired':
|
||||
"Ce serveur nécessite que vous vous connectiez ou ajoutiez un jeton d'accès avant de pouvoir se connecter. Cliquez sur « Se connecter » pour vous authentifier.",
|
||||
'mcp.detail.disconnect': 'Déconnecter',
|
||||
'mcp.detail.hideAssistant': "Cacher l'assistant",
|
||||
'mcp.detail.helpConfigure': 'Aidez-moi à configurer',
|
||||
@@ -1388,6 +1391,7 @@ const messages: TranslationMap = {
|
||||
'mcp.detail.enable': 'Activer',
|
||||
'mcp.detail.disable': 'Désactiver',
|
||||
'mcp.status.disabled': 'Désactivé',
|
||||
'mcp.status.unauthorized': 'Connexion requise',
|
||||
'mcp.detail.tools': 'Outils',
|
||||
'mcp.connectAuth.title': 'Connecter {name}',
|
||||
'mcp.connectAuth.hint':
|
||||
|
||||
@@ -1328,6 +1328,9 @@ const messages: TranslationMap = {
|
||||
'इस सर्वर को फिर से स्थापित करने के लिए सुझाए गए मूल्यों के साथ उन्हें लागू करने के लिए: {keys}',
|
||||
'mcp.detail.connect': 'कनेक्ट करें',
|
||||
'mcp.detail.connecting': 'कनेक्ट हो रहा है...',
|
||||
'mcp.detail.authenticate': 'साइन इन करें',
|
||||
'mcp.detail.authRequired':
|
||||
'कनेक्ट होने से पहले इस सर्वर के लिए आपको साइन इन करना होगा या एक एक्सेस टोकन जोड़ना होगा। प्रमाणित करने के लिए “साइन इन करें” पर क्लिक करें।',
|
||||
'mcp.detail.disconnect': 'डिस्कनेक्ट करें',
|
||||
'mcp.detail.hideAssistant': 'सहायक छिपाएँ',
|
||||
'mcp.detail.helpConfigure': 'कॉन्फ़िगर करने में मेरी सहायता करें',
|
||||
@@ -1346,6 +1349,7 @@ const messages: TranslationMap = {
|
||||
'mcp.detail.enable': 'सक्षम करें',
|
||||
'mcp.detail.disable': 'अक्षम करें',
|
||||
'mcp.status.disabled': 'अक्षम',
|
||||
'mcp.status.unauthorized': 'साइन इन आवश्यक',
|
||||
'mcp.detail.tools': 'उपकरण',
|
||||
'mcp.connectAuth.title': '{name} कनेक्ट करें',
|
||||
'mcp.connectAuth.hint':
|
||||
|
||||
@@ -1338,6 +1338,9 @@ const messages: TranslationMap = {
|
||||
'Instal ulang server ini dengan nilai yang disarankan untuk diterapkan: {keys}',
|
||||
'mcp.detail.connect': 'Sambungkan',
|
||||
'mcp.detail.connecting': 'Sambungan...',
|
||||
'mcp.detail.authenticate': 'Masuk',
|
||||
'mcp.detail.authRequired':
|
||||
'Server ini mengharuskan Anda masuk atau menambahkan token akses sebelum dapat tersambung. Klik “Masuk” untuk mengautentikasi.',
|
||||
'mcp.detail.disconnect': 'Putuskan sambungan',
|
||||
'mcp.detail.hideAssistant': 'Sembunyikan asisten',
|
||||
'mcp.detail.helpConfigure': 'Bantu saya mengonfigurasi',
|
||||
@@ -1356,6 +1359,7 @@ const messages: TranslationMap = {
|
||||
'mcp.detail.enable': 'Aktifkan',
|
||||
'mcp.detail.disable': 'Nonaktifkan',
|
||||
'mcp.status.disabled': 'Dinonaktifkan',
|
||||
'mcp.status.unauthorized': 'Perlu masuk',
|
||||
'mcp.detail.tools': 'Alat',
|
||||
'mcp.connectAuth.title': 'Hubungkan {name}',
|
||||
'mcp.connectAuth.hint':
|
||||
|
||||
@@ -1360,6 +1360,9 @@ const messages: TranslationMap = {
|
||||
'Reinstalla questo server con i valori suggeriti per applicarli: {keys}',
|
||||
'mcp.detail.connect': 'Connetti',
|
||||
'mcp.detail.connecting': 'Connessione in corso...',
|
||||
'mcp.detail.authenticate': 'Accedi',
|
||||
'mcp.detail.authRequired':
|
||||
'Questo server richiede di accedere o di aggiungere un token di accesso prima di potersi connettere. Fai clic su “Accedi” per autenticarti.',
|
||||
'mcp.detail.disconnect': 'Disconnetti',
|
||||
'mcp.detail.hideAssistant': 'Nascondi assistente',
|
||||
'mcp.detail.helpConfigure': 'Aiutami a configurare',
|
||||
@@ -1378,6 +1381,7 @@ const messages: TranslationMap = {
|
||||
'mcp.detail.enable': 'Abilita',
|
||||
'mcp.detail.disable': 'Disabilita',
|
||||
'mcp.status.disabled': 'Disabilitato',
|
||||
'mcp.status.unauthorized': 'Accesso necessario',
|
||||
'mcp.detail.tools': 'Strumenti',
|
||||
'mcp.connectAuth.title': 'Connetti {name}',
|
||||
'mcp.connectAuth.hint':
|
||||
|
||||
@@ -1325,6 +1325,9 @@ const messages: TranslationMap = {
|
||||
'제안 값을 적용하려면 이 서버를 해당 값으로 다시 설치하세요: {keys}',
|
||||
'mcp.detail.connect': '연결',
|
||||
'mcp.detail.connecting': '연결 중...',
|
||||
'mcp.detail.authenticate': '로그인',
|
||||
'mcp.detail.authRequired':
|
||||
'이 서버에 연결하려면 먼저 로그인하거나 액세스 토큰을 추가해야 합니다. 인증하려면 “로그인”을 클릭하세요.',
|
||||
'mcp.detail.disconnect': '연결 끊기',
|
||||
'mcp.detail.hideAssistant': '보조 숨기기',
|
||||
'mcp.detail.helpConfigure': '구성 도와주세요',
|
||||
@@ -1342,6 +1345,7 @@ const messages: TranslationMap = {
|
||||
'mcp.detail.enable': '활성화',
|
||||
'mcp.detail.disable': '비활성화',
|
||||
'mcp.status.disabled': '비활성화됨',
|
||||
'mcp.status.unauthorized': '로그인 필요',
|
||||
'mcp.detail.tools': '도구',
|
||||
'mcp.connectAuth.title': '{name} 연결',
|
||||
'mcp.connectAuth.hint':
|
||||
|
||||
@@ -1350,6 +1350,9 @@ const messages: TranslationMap = {
|
||||
'Zainstaluj ten serwer ponownie z sugerowanymi wartościami, aby je zastosować: {keys}',
|
||||
'mcp.detail.connect': 'Połącz',
|
||||
'mcp.detail.connecting': 'Łączenie...',
|
||||
'mcp.detail.authenticate': 'Zaloguj się',
|
||||
'mcp.detail.authRequired':
|
||||
'Ten serwer wymaga zalogowania się lub dodania tokena dostępu, zanim będzie mógł się połączyć. Kliknij „Zaloguj się”, aby uwierzytelnić.',
|
||||
'mcp.detail.disconnect': 'Rozłącz',
|
||||
'mcp.detail.hideAssistant': 'Ukryj asystenta',
|
||||
'mcp.detail.helpConfigure': 'Pomóż skonfigurować',
|
||||
@@ -1368,6 +1371,7 @@ const messages: TranslationMap = {
|
||||
'mcp.detail.enable': 'Włącz',
|
||||
'mcp.detail.disable': 'Wyłącz',
|
||||
'mcp.status.disabled': 'Wyłączony',
|
||||
'mcp.status.unauthorized': 'Wymagane logowanie',
|
||||
'mcp.detail.tools': 'Narzędzia',
|
||||
'mcp.connectAuth.title': 'Połącz {name}',
|
||||
'mcp.connectAuth.hint':
|
||||
|
||||
@@ -1366,6 +1366,9 @@ const messages: TranslationMap = {
|
||||
'Reinstale este servidor com os valores sugeridos para aplicá-los: {keys}',
|
||||
'mcp.detail.connect': 'Conectar',
|
||||
'mcp.detail.connecting': 'Conectando...',
|
||||
'mcp.detail.authenticate': 'Entrar',
|
||||
'mcp.detail.authRequired':
|
||||
'Este servidor exige que você entre ou adicione um token de acesso antes de poder se conectar. Clique em “Entrar” para autenticar.',
|
||||
'mcp.detail.disconnect': 'Desconectar',
|
||||
'mcp.detail.hideAssistant': 'Ocultar assistente',
|
||||
'mcp.detail.helpConfigure': 'Ajude-me a configurar',
|
||||
@@ -1383,6 +1386,7 @@ const messages: TranslationMap = {
|
||||
'mcp.detail.enable': 'Ativar',
|
||||
'mcp.detail.disable': 'Desativar',
|
||||
'mcp.status.disabled': 'Desativado',
|
||||
'mcp.status.unauthorized': 'É necessário entrar',
|
||||
'mcp.detail.tools': 'Ferramentas',
|
||||
'mcp.connectAuth.title': 'Conectar {name}',
|
||||
'mcp.connectAuth.hint':
|
||||
|
||||
@@ -1347,6 +1347,9 @@ const messages: TranslationMap = {
|
||||
'Чтобы применить их, переустановите этот сервер с предложенными значениями: {keys}.',
|
||||
'mcp.detail.connect': 'Подключитесь',
|
||||
'mcp.detail.connecting': 'Подключение...',
|
||||
'mcp.detail.authenticate': 'Войти',
|
||||
'mcp.detail.authRequired':
|
||||
'Этот сервер требует, чтобы вы вошли в систему или добавили токен доступа, прежде чем он сможет подключиться. Нажмите «Войти», чтобы пройти аутентификацию.',
|
||||
'mcp.detail.disconnect': 'Отключите',
|
||||
'mcp.detail.hideAssistant': 'Скрыть помощника',
|
||||
'mcp.detail.helpConfigure': 'Помогите мне настроить',
|
||||
@@ -1365,6 +1368,7 @@ const messages: TranslationMap = {
|
||||
'mcp.detail.enable': 'Включить',
|
||||
'mcp.detail.disable': 'Отключить',
|
||||
'mcp.status.disabled': 'Отключён',
|
||||
'mcp.status.unauthorized': 'Требуется вход',
|
||||
'mcp.detail.tools': 'Инструменты',
|
||||
'mcp.connectAuth.title': 'Подключить {name}',
|
||||
'mcp.connectAuth.hint':
|
||||
|
||||
@@ -1267,6 +1267,8 @@ const messages: TranslationMap = {
|
||||
'mcp.detail.suggestedEnvBody': '使用建议值重新安装此服务器以应用它们：{keys}',
|
||||
'mcp.detail.connect': '连接',
|
||||
'mcp.detail.connecting': '正在连接...',
|
||||
'mcp.detail.authenticate': '登录',
|
||||
'mcp.detail.authRequired': '此服务器需要您先登录或添加访问令牌才能连接。点击“登录”进行身份验证。',
|
||||
'mcp.detail.disconnect': '断开连接',
|
||||
'mcp.detail.hideAssistant': '隐藏助手',
|
||||
'mcp.detail.helpConfigure': '帮我配置一下',
|
||||
@@ -1283,6 +1285,7 @@ const messages: TranslationMap = {
|
||||
'mcp.detail.enable': '启用',
|
||||
'mcp.detail.disable': '禁用',
|
||||
'mcp.status.disabled': '已禁用',
|
||||
'mcp.status.unauthorized': '需要登录',
|
||||
'mcp.detail.tools': '工具',
|
||||
'mcp.connectAuth.title': '连接 {name}',
|
||||
'mcp.connectAuth.hint':
|
||||
|
||||
@@ -144,6 +144,36 @@ pub struct McpAuthorizationContext {
|
||||
pub authorization_server_metadata: Vec<AuthorizationServerMetadata>,
|
||||
}
|
||||
|
||||
/// Typed error for an HTTP 401 from a remote MCP server. Carried as the root
|
||||
/// of the `anyhow` chain so the connect path can recognise an auth failure via
|
||||
/// `downcast_ref` — instead of fragile string matching — and classify the
|
||||
/// server as "needs authentication" rather than a generic transport error
|
||||
/// (issue #3719). The `Display` form is the diagnostic string used in logs;
|
||||
/// the user-facing copy is derived separately in `mcp_registry::connections`.
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct McpUnauthorizedError {
|
||||
/// Redacted endpoint (scheme + authority only) the 401 came from.
|
||||
pub endpoint: String,
|
||||
/// `resource_metadata` URL advertised by the `WWW-Authenticate` challenge,
|
||||
/// when present — the entry point to OAuth discovery.
|
||||
pub resource_metadata: Option<String>,
|
||||
}
|
||||
|
||||
impl std::fmt::Display for McpUnauthorizedError {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
match &self.resource_metadata {
|
||||
Some(rm) => write!(
|
||||
f,
|
||||
"MCP unauthorized for `{}` (HTTP 401; resource metadata: {rm})",
|
||||
self.endpoint
|
||||
),
|
||||
None => write!(f, "MCP unauthorized for `{}` (HTTP 401)", self.endpoint),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl std::error::Error for McpUnauthorizedError {}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
|
||||
pub struct McpSseEvent {
|
||||
pub event: Option<String>,
|
||||
@@ -732,21 +762,16 @@ impl McpHttpClient {
|
||||
let text = response.text().await?;
|
||||
|
||||
if status == reqwest::StatusCode::UNAUTHORIZED {
|
||||
let auth_suffix = if let Some(challenge) = parse_www_authenticate_challenge(&headers) {
|
||||
match challenge.resource_metadata.as_deref() {
|
||||
Some(resource_metadata) => {
|
||||
format!("; resource metadata: {resource_metadata}")
|
||||
}
|
||||
None => String::new(),
|
||||
}
|
||||
} else {
|
||||
String::new()
|
||||
};
|
||||
anyhow::bail!(
|
||||
"MCP unauthorized for `{}` (HTTP 401{})",
|
||||
redact_endpoint(&self.endpoint),
|
||||
auth_suffix
|
||||
);
|
||||
let resource_metadata = parse_www_authenticate_challenge(&headers)
|
||||
.and_then(|challenge| challenge.resource_metadata);
|
||||
// Return a TYPED error (not a string `bail!`) so callers can
|
||||
// `downcast_ref::<McpUnauthorizedError>()` and surface an
|
||||
// actionable "needs authentication" state (#3719) rather than a
|
||||
// generic failure. `anyhow` preserves the root type through `?`.
|
||||
return Err(anyhow::Error::new(McpUnauthorizedError {
|
||||
endpoint: redact_endpoint(&self.endpoint),
|
||||
resource_metadata,
|
||||
}));
|
||||
}
|
||||
if !status.is_success() {
|
||||
anyhow::bail!("MCP HTTP {} — {}", status.as_u16(), text);
|
||||
|
||||
@@ -45,7 +45,7 @@ mod stdio;
|
||||
pub use client::{
|
||||
redact_endpoint, AuthorizationServerMetadata, McpAuthChallenge, McpAuthorizationContext,
|
||||
McpHttpClient, McpInitializeResult, McpRemoteTool, McpServerToolResult, McpSseEvent,
|
||||
ProtectedResourceMetadata,
|
||||
McpUnauthorizedError, ProtectedResourceMetadata,
|
||||
};
|
||||
pub(crate) use registry::apply_safety_filter;
|
||||
pub use registry::{McpRegistrySource, McpServerDefinition, McpServerRegistry, McpTransportClient};
|
||||
|
||||
@@ -205,20 +205,59 @@ fn connections() -> &'static RwLock<HashMap<String, Arc<Connection>>> {
|
||||
|
||||
// ── Per-server last connect error ────────────────────────────────────────────
|
||||
|
||||
static LAST_ERRORS: OnceLock<RwLock<HashMap<String, String>>> = OnceLock::new();
|
||||
/// The most recent connect failure for one server: the raw diagnostic message
|
||||
/// (for logs/debugging) plus whether it was specifically an HTTP 401 (auth
|
||||
/// required). Both live in ONE record under ONE lock so a status read can never
|
||||
/// observe a torn snapshot — e.g. the message updated but the auth flag stale,
|
||||
/// which a two-map design would allow if `all_status` interleaved between the
|
||||
/// two writes (#3719).
|
||||
#[derive(Clone)]
|
||||
struct ConnectFailure {
|
||||
message: String,
|
||||
/// `true` when the failure was an MCP HTTP 401 → drives
|
||||
/// `ServerStatus::Unauthorized` so the UI offers a re-auth path instead of
|
||||
/// a raw error blob.
|
||||
unauthorized: bool,
|
||||
}
|
||||
|
||||
fn last_errors() -> &'static RwLock<HashMap<String, String>> {
|
||||
static LAST_ERRORS: OnceLock<RwLock<HashMap<String, ConnectFailure>>> = OnceLock::new();
|
||||
|
||||
fn last_errors() -> &'static RwLock<HashMap<String, ConnectFailure>> {
|
||||
LAST_ERRORS.get_or_init(|| RwLock::new(HashMap::new()))
|
||||
}
|
||||
|
||||
/// Whether a connect failure's root cause was an MCP HTTP 401 (auth required).
|
||||
/// Uses a typed `downcast` on the `anyhow` chain — not string matching — so the
|
||||
/// classification can't drift from the message wording.
|
||||
fn is_unauthorized_error(err: &anyhow::Error) -> bool {
|
||||
// Walk the whole source chain (not just the outermost error) so the
|
||||
// classification survives any `?`/`.context()` wrapping a caller may add.
|
||||
err.chain()
|
||||
.any(|cause| cause.is::<crate::openhuman::mcp_client::McpUnauthorizedError>())
|
||||
}
|
||||
|
||||
/// Read the most recent connect-failure message for `server_id`. `None` when
|
||||
/// the server has never failed, or when the most recent connect succeeded.
|
||||
pub async fn last_error_for(server_id: &str) -> Option<String> {
|
||||
last_errors().read().await.get(server_id).cloned()
|
||||
last_errors()
|
||||
.read()
|
||||
.await
|
||||
.get(server_id)
|
||||
.map(|f| f.message.clone())
|
||||
}
|
||||
|
||||
/// Drop any recorded error for `server_id`. Called on successful connect,
|
||||
/// explicit disconnect, uninstall, and enable→disable transitions.
|
||||
/// Whether `server_id`'s most recent connect failed due to HTTP 401.
|
||||
pub async fn needs_auth(server_id: &str) -> bool {
|
||||
last_errors()
|
||||
.read()
|
||||
.await
|
||||
.get(server_id)
|
||||
.is_some_and(|f| f.unauthorized)
|
||||
}
|
||||
|
||||
/// Drop any recorded error (generic or auth-required) for `server_id`. Called on
|
||||
/// successful connect, explicit disconnect, uninstall, and enable→disable
|
||||
/// transitions.
|
||||
pub async fn clear_last_error(server_id: &str) {
|
||||
last_errors().write().await.remove(server_id);
|
||||
}
|
||||
@@ -249,12 +288,19 @@ pub async fn connect(config: &Config, server: &InstalledServer) -> anyhow::Resul
|
||||
);
|
||||
}
|
||||
Err(err) => {
|
||||
last_errors()
|
||||
.write()
|
||||
.await
|
||||
.insert(server.server_id.clone(), err.to_string());
|
||||
// Record the raw diagnostic AND the 401 classification together in a
|
||||
// single record under one lock, so a concurrent `all_status` can't
|
||||
// observe a torn snapshot (message set but auth flag stale).
|
||||
let unauthorized = is_unauthorized_error(err);
|
||||
last_errors().write().await.insert(
|
||||
server.server_id.clone(),
|
||||
ConnectFailure {
|
||||
message: err.to_string(),
|
||||
unauthorized,
|
||||
},
|
||||
);
|
||||
tracing::debug!(
|
||||
"[mcp-registry] last_error recorded server_id={} err={err}",
|
||||
"[mcp-registry] last_error recorded server_id={} unauthorized={unauthorized} err={err}",
|
||||
server.server_id
|
||||
);
|
||||
}
|
||||
@@ -466,11 +512,48 @@ pub async fn call_tool(
|
||||
|
||||
/// Return status summaries for all installed servers.
|
||||
///
|
||||
/// Priority order: `Disabled` > `Connected` > `Error` > `Disconnected`.
|
||||
/// Priority order: `Disabled` > `Connected` > `Unauthorized` > `Error` >
|
||||
/// `Disconnected`.
|
||||
/// - `!s.enabled` → `Disabled` (suppresses tool count and last_error).
|
||||
/// - connected (id in live registry) → `Connected` + tool count.
|
||||
/// - recorded connect failure in `LAST_ERRORS` → `Error` + last_error message.
|
||||
/// - connect failed with HTTP 401 (`AUTH_REQUIRED`) → `Unauthorized`. The raw
|
||||
/// error string is intentionally NOT surfaced (it leaks an internal OAuth
|
||||
/// metadata URL, #3719) — the UI renders a localized "needs sign-in" message
|
||||
/// and the re-auth affordance keyed off the status alone.
|
||||
/// - other recorded connect failure in `LAST_ERRORS` → `Error` + message.
|
||||
/// - otherwise → `Disconnected`.
|
||||
/// Pure status decision for one installed server, factored out of
|
||||
/// [`all_status`] so the priority order is unit-testable without a live
|
||||
/// connection registry or store. Inputs:
|
||||
/// - `enabled` — the install's enabled flag.
|
||||
/// - `connected_tool_count` — `Some(n)` when the server is in the live map
|
||||
/// (with its advertised tool count), `None` otherwise.
|
||||
/// - `auth_required` — most recent connect failed with HTTP 401.
|
||||
/// - `generic_error` — most recent (non-401) connect error message, if any.
|
||||
///
|
||||
/// Priority: `Disabled` > `Connected` > `Unauthorized` > `Error` >
|
||||
/// `Disconnected`. The raw error is surfaced ONLY for the generic `Error` case;
|
||||
/// `Unauthorized` deliberately carries no message (the UI localizes it and
|
||||
/// avoids leaking the OAuth metadata URL, #3719).
|
||||
fn classify_server_status(
|
||||
enabled: bool,
|
||||
connected_tool_count: Option<u32>,
|
||||
auth_required: bool,
|
||||
generic_error: Option<String>,
|
||||
) -> (ServerStatus, u32, Option<String>) {
|
||||
if !enabled {
|
||||
(ServerStatus::Disabled, 0, None)
|
||||
} else if let Some(n) = connected_tool_count {
|
||||
(ServerStatus::Connected, n, None)
|
||||
} else if auth_required {
|
||||
(ServerStatus::Unauthorized, 0, None)
|
||||
} else if let Some(err) = generic_error {
|
||||
(ServerStatus::Error, 0, Some(err))
|
||||
} else {
|
||||
(ServerStatus::Disconnected, 0, None)
|
||||
}
|
||||
}
|
||||
|
||||
pub async fn all_status(config: &Config) -> Vec<ConnStatus> {
|
||||
let installed = store::list_servers(config).unwrap_or_default();
|
||||
let connected_ids: Vec<String> = {
|
||||
@@ -478,27 +561,39 @@ pub async fn all_status(config: &Config) -> Vec<ConnStatus> {
|
||||
map.keys().cloned().collect()
|
||||
};
|
||||
|
||||
let errors_snapshot = last_errors().read().await.clone();
|
||||
// One snapshot of the unified failure map — message + auth flag are read
|
||||
// together, so a server's status can't be classified from a torn pair.
|
||||
let failures_snapshot = last_errors().read().await.clone();
|
||||
|
||||
let mut out = Vec::with_capacity(installed.len());
|
||||
for s in installed {
|
||||
let is_connected = connected_ids.iter().any(|id| id == &s.server_id);
|
||||
|
||||
let (status, tool_count, last_error) = if !s.enabled {
|
||||
(ServerStatus::Disabled, 0u32, None)
|
||||
} else if is_connected {
|
||||
// Resolve the live tool count up front (the only async input), then let
|
||||
// the pure classifier pick the status — keeps the priority logic
|
||||
// testable without a live registry / DB.
|
||||
let connected_tool_count = if is_connected {
|
||||
let map = connections().read().await;
|
||||
let tool_count = match map.get(&s.server_id) {
|
||||
Some(match map.get(&s.server_id) {
|
||||
Some(c) => c.tools_snapshot().await.len() as u32,
|
||||
None => 0,
|
||||
};
|
||||
(ServerStatus::Connected, tool_count, None)
|
||||
} else if let Some(err) = errors_snapshot.get(&s.server_id).cloned() {
|
||||
(ServerStatus::Error, 0u32, Some(err))
|
||||
})
|
||||
} else {
|
||||
(ServerStatus::Disconnected, 0u32, None)
|
||||
None
|
||||
};
|
||||
|
||||
let failure = failures_snapshot.get(&s.server_id);
|
||||
let (status, tool_count, last_error) = classify_server_status(
|
||||
s.enabled,
|
||||
connected_tool_count,
|
||||
failure.is_some_and(|f| f.unauthorized),
|
||||
// Only a generic (non-401) failure carries a surfaced message; the
|
||||
// 401 message is withheld (it leaks the OAuth metadata URL).
|
||||
failure
|
||||
.filter(|f| !f.unauthorized)
|
||||
.map(|f| f.message.clone()),
|
||||
);
|
||||
|
||||
out.push(ConnStatus {
|
||||
server_id: s.server_id,
|
||||
qualified_name: s.qualified_name,
|
||||
@@ -593,8 +688,62 @@ fn into_registry_tool(remote: McpRemoteTool) -> McpTool {
|
||||
mod tests {
|
||||
// Live-connection tests require a real MCP subprocess and live in
|
||||
// tests/json_rpc_e2e.rs. Keep this slot for sync helper tests.
|
||||
use super::{build_http_auth, credential_safe_dial_url};
|
||||
use super::{
|
||||
build_http_auth, classify_server_status, credential_safe_dial_url, is_unauthorized_error,
|
||||
};
|
||||
use crate::openhuman::config::McpAuthConfig;
|
||||
use crate::openhuman::mcp_client::McpUnauthorizedError;
|
||||
use crate::openhuman::mcp_registry::types::ServerStatus;
|
||||
|
||||
#[test]
|
||||
fn classify_server_status_priority_order() {
|
||||
// Disabled wins over everything (even a live connection / 401 / error).
|
||||
assert_eq!(
|
||||
classify_server_status(false, Some(3), true, Some("boom".into())),
|
||||
(ServerStatus::Disabled, 0, None)
|
||||
);
|
||||
// Connected → tool count surfaced, no error.
|
||||
assert_eq!(
|
||||
classify_server_status(true, Some(5), true, Some("boom".into())),
|
||||
(ServerStatus::Connected, 5, None)
|
||||
);
|
||||
// Not connected + 401 → Unauthorized, and NO raw error is leaked even
|
||||
// when a generic error is also recorded.
|
||||
assert_eq!(
|
||||
classify_server_status(true, None, true, Some("MCP unauthorized … HTTP 401".into())),
|
||||
(ServerStatus::Unauthorized, 0, None)
|
||||
);
|
||||
// Not connected + generic error (no 401) → Error + message.
|
||||
assert_eq!(
|
||||
classify_server_status(true, None, false, Some("timed out".into())),
|
||||
(ServerStatus::Error, 0, Some("timed out".into()))
|
||||
);
|
||||
// Not connected, no error → Disconnected.
|
||||
assert_eq!(
|
||||
classify_server_status(true, None, false, None),
|
||||
(ServerStatus::Disconnected, 0, None)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn is_unauthorized_error_classifies_typed_401_only() {
|
||||
// A typed 401 from the transport → auth required (regardless of the
|
||||
// message wording, since we downcast rather than string-match).
|
||||
let unauth = anyhow::Error::new(McpUnauthorizedError {
|
||||
endpoint: "https://example.com".into(),
|
||||
resource_metadata: Some("https://example.com/.well-known/x".into()),
|
||||
});
|
||||
assert!(is_unauthorized_error(&unauth));
|
||||
|
||||
// A 401 survives `?`-style context wrapping (anyhow keeps the root
|
||||
// downcastable), matching how the error reaches `connect`.
|
||||
let wrapped = unauth.context("connecting to MCP server");
|
||||
assert!(is_unauthorized_error(&wrapped));
|
||||
|
||||
// Any other transport failure → NOT auth required (stays generic Error).
|
||||
let other = anyhow::anyhow!("MCP HTTP 500 — upstream exploded");
|
||||
assert!(!is_unauthorized_error(&other));
|
||||
}
|
||||
|
||||
fn kv(pairs: &[(&str, &str)]) -> Vec<(String, String)> {
|
||||
pairs
|
||||
|
||||
@@ -179,6 +179,11 @@ pub enum ServerStatus {
|
||||
Disconnected,
|
||||
Connecting,
|
||||
Connected,
|
||||
/// Connect failed specifically because the server returned HTTP 401 — the
|
||||
/// server is reachable but needs authentication (OAuth sign-in or an API
|
||||
/// token). Distinct from `Error` so the UI can offer a re-auth path instead
|
||||
/// of a raw failure (#3719).
|
||||
Unauthorized,
|
||||
Error,
|
||||
Disabled,
|
||||
}
|
||||
@@ -189,6 +194,7 @@ impl ServerStatus {
|
||||
Self::Disconnected => "disconnected",
|
||||
Self::Connecting => "connecting",
|
||||
Self::Connected => "connected",
|
||||
Self::Unauthorized => "unauthorized",
|
||||
Self::Error => "error",
|
||||
Self::Disabled => "disabled",
|
||||
}
|
||||
@@ -322,6 +328,7 @@ mod tests {
|
||||
assert_eq!(ServerStatus::Connected.as_str(), "connected");
|
||||
assert_eq!(ServerStatus::Disconnected.as_str(), "disconnected");
|
||||
assert_eq!(ServerStatus::Connecting.as_str(), "connecting");
|
||||
assert_eq!(ServerStatus::Unauthorized.as_str(), "unauthorized");
|
||||
assert_eq!(ServerStatus::Error.as_str(), "error");
|
||||
assert_eq!(ServerStatus::Disabled.as_str(), "disabled");
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user