diff --git a/src/crates/services/services-integrations/src/mcp/config/cursor_format.rs b/src/crates/services/services-integrations/src/mcp/config/cursor_format.rs index 2fc09260c6..613ab9fabd 100644 --- a/src/crates/services/services-integrations/src/mcp/config/cursor_format.rs +++ b/src/crates/services/services-integrations/src/mcp/config/cursor_format.rs @@ -92,6 +92,11 @@ pub fn config_to_cursor_format(config: &MCPServerConfig) -> serde_json::Value { if let Some(oauth) = &config.oauth { cursor_config.insert("oauth".to_string(), serde_json::json!(oauth)); + if let Some(enabled) = config.oauth_enabled { + cursor_config.insert("oauthEnabled".to_string(), serde_json::json!(enabled)); + } + } else if let Some(enabled) = config.oauth_enabled { + cursor_config.insert("oauth".to_string(), serde_json::json!(enabled)); } if let Some(xaa) = &config.xaa { @@ -244,7 +249,12 @@ pub fn parse_cursor_format(config: &serde_json::Value) -> Vec { .get("oauth") .cloned() .and_then(|value| serde_json::from_value(value).ok()), - oauth_enabled: None, + // Boolean shorthand controls discovery; an object carries + // OAuth options. Do not discard an explicit opt-out. + oauth_enabled: obj + .get("oauth") + .and_then(|value| value.as_bool()) + .or_else(|| obj.get("oauthEnabled").and_then(|value| value.as_bool())), xaa: obj .get("xaa") .cloned() diff --git a/src/crates/services/services-integrations/src/mcp/config/json_config.rs b/src/crates/services/services-integrations/src/mcp/config/json_config.rs index 18ca4f54cc..7966097bd8 100644 --- a/src/crates/services/services-integrations/src/mcp/config/json_config.rs +++ b/src/crates/services/services-integrations/src/mcp/config/json_config.rs @@ -235,7 +235,6 @@ pub fn validate_mcp_json_config( ("args", "array"), ("env", "object"), ("headers", "object"), - ("oauth", "object"), ("xaa", "object"), ] { if let Some(value) = obj.get(key) { @@ -252,6 +251,33 @@ pub fn validate_mcp_json_config( } } } + + if let Some(value) = obj.get("oauth") { + if !value.is_object() && !value.is_boolean() { + return Err(MCPJsonConfigValidationError::new(format!( + "Server '{}' 'oauth' field must be a boolean or an object", + server_id + ))); + } + } + if let Some(value) = obj.get("oauthEnabled") { + if !value.is_boolean() { + return Err(MCPJsonConfigValidationError::new(format!( + "Server '{}' 'oauthEnabled' field must be a boolean", + server_id + ))); + } + if obj + .get("oauth") + .and_then(|oauth| oauth.as_bool()) + .is_some_and(|enabled| Some(enabled) != value.as_bool()) + { + return Err(MCPJsonConfigValidationError::new(format!( + "Server '{}' 'oauth' conflicts with 'oauthEnabled'", + server_id + ))); + } + } } Ok(()) diff --git a/src/crates/services/services-integrations/src/mcp/config/service_helpers.rs b/src/crates/services/services-integrations/src/mcp/config/service_helpers.rs index c261445bb7..32e6f656ef 100644 --- a/src/crates/services/services-integrations/src/mcp/config/service_helpers.rs +++ b/src/crates/services/services-integrations/src/mcp/config/service_helpers.rs @@ -18,6 +18,7 @@ fn config_signature(config: &MCPServerConfig) -> String { "headers": headers, "url": config.url, "oauth": config.oauth, + "oauthEnabled": config.remote_oauth_enabled(), "xaa": config.xaa, }) .to_string() diff --git a/src/crates/services/services-integrations/tests/mcp_contracts.rs b/src/crates/services/services-integrations/tests/mcp_contracts.rs index 7d33b39647..d103c67e66 100644 --- a/src/crates/services/services-integrations/tests/mcp_contracts.rs +++ b/src/crates/services/services-integrations/tests/mcp_contracts.rs @@ -1950,6 +1950,81 @@ async fn mcp_oauth_credential_vault_uses_injected_data_dir_and_roundtrips_creden let _ = std::fs::remove_dir_all(data_dir); } +#[test] +fn mcp_cursor_oauth_policy_survives_validation_and_roundtrip() { + for (options, expected) in [ + (serde_json::json!({}), None), + (serde_json::json!({ "oauth": false }), Some(false)), + (serde_json::json!({ "oauth": true }), Some(true)), + (serde_json::json!({ "oauthEnabled": false }), Some(false)), + (serde_json::json!({ "oauth": { "scopes": ["read"] } }), None), + ( + serde_json::json!({ "oauth": { "scopes": ["read"] }, "oauthEnabled": false }), + Some(false), + ), + ] { + let mut server = serde_json::json!({ "url": "https://example.test/mcp" }); + server + .as_object_mut() + .unwrap() + .extend(options.as_object().unwrap().clone()); + let input = serde_json::json!({ "mcpServers": { "test": server } }); + validate_mcp_json_config(&input).unwrap(); + let parsed = parse_cursor_format(&input); + assert_eq!(parsed.len(), 1); + assert_eq!(parsed[0].oauth_enabled, expected); + assert_eq!(parsed[0].remote_oauth_enabled(), expected.unwrap_or(true)); + + let exported = config_to_cursor_format(&parsed[0]); + if options.get("oauth") == Some(&serde_json::json!(false)) { + assert_eq!(exported["oauth"], false); + } + let saved = serde_json::json!({ "mcpServers": { "test": exported } }); + validate_mcp_json_config(&saved).unwrap(); + let reparsed = parse_cursor_format(&saved); + assert_eq!(reparsed[0].oauth_enabled, expected); + assert_eq!( + serde_json::to_value(&reparsed[0].oauth).unwrap(), + serde_json::to_value(&parsed[0].oauth).unwrap() + ); + } +} + +#[test] +fn mcp_cursor_oauth_validation_rejects_invalid_or_conflicting_policy() { + for options in [ + serde_json::json!({ "oauth": "false" }), + serde_json::json!({ "oauth": [] }), + serde_json::json!({ "oauthEnabled": "false" }), + serde_json::json!({ "oauth": false, "oauthEnabled": true }), + serde_json::json!({ "oauth": true, "oauthEnabled": false }), + ] { + let mut server = serde_json::json!({ "url": "https://example.test/mcp" }); + server + .as_object_mut() + .unwrap() + .extend(options.as_object().unwrap().clone()); + assert!( + validate_mcp_json_config(&serde_json::json!({ "mcpServers": { "test": server } })) + .is_err() + ); + } +} + +#[test] +fn mcp_cursor_oauth_policy_is_part_of_config_identity() { + let disabled = parse_cursor_format(&serde_json::json!({ + "mcpServers": { "disabled": { "url": "https://example.test/mcp", "oauth": false } } + })); + let legacy = parse_cursor_format(&serde_json::json!({ + "mcpServers": { "legacy": { "url": "https://example.test/mcp" } } + })); + let merged = merge_mcp_server_config_sources([disabled, legacy]); + assert_eq!(merged.len(), 2); + assert!(!merged[0].remote_oauth_enabled()); + assert!(merged[1].remote_oauth_enabled()); +} + #[test] fn mcp_cursor_format_helpers_preserve_cursor_compatibility_contract() { let remote = MCPServerConfig { diff --git a/src/web-ui/src/infrastructure/config/components/McpToolsConfig.test.tsx b/src/web-ui/src/infrastructure/config/components/McpToolsConfig.test.tsx index ac88a40d6d..5383b05a99 100644 --- a/src/web-ui/src/infrastructure/config/components/McpToolsConfig.test.tsx +++ b/src/web-ui/src/infrastructure/config/components/McpToolsConfig.test.tsx @@ -12,6 +12,7 @@ const loadJsonConfigMock = vi.hoisted(() => vi.fn()); const saveJsonConfigMock = vi.hoisted(() => vi.fn()); const initializeServersMock = vi.hoisted(() => vi.fn()); const startServerMock = vi.hoisted(() => vi.fn()); +const restartServerMock = vi.hoisted(() => vi.fn()); const startRemoteOAuthMock = vi.hoisted(() => vi.fn()); const getRemoteOAuthSessionMock = vi.hoisted(() => vi.fn()); const cancelRemoteOAuthMock = vi.hoisted(() => vi.fn()); @@ -54,6 +55,7 @@ vi.mock('../../api/service-api/MCPAPI', () => ({ saveMCPJsonConfig: saveJsonConfigMock, initializeServers: initializeServersMock, startServer: startServerMock, + restartServer: restartServerMock, startRemoteOAuth: startRemoteOAuthMock, getRemoteOAuthSession: getRemoteOAuthSessionMock, cancelRemoteOAuth: cancelRemoteOAuthMock, @@ -85,6 +87,7 @@ describe('McpToolsConfig remote behavior', () => { saveJsonConfigMock.mockReset().mockResolvedValue({ runtimeApplied: true }); initializeServersMock.mockReset().mockResolvedValue(undefined); startServerMock.mockReset().mockResolvedValue(undefined); + restartServerMock.mockReset().mockResolvedValue(undefined); startRemoteOAuthMock.mockReset().mockResolvedValue({ serverId: 'notion', status: 'awaitingBrowser', @@ -105,6 +108,7 @@ describe('McpToolsConfig remote behavior', () => { afterEach(async () => { await act(async () => root.unmount()); container.remove(); + vi.useRealTimers(); }); it('does not call desktop MCP management APIs during a remote connection', async () => { @@ -195,6 +199,89 @@ describe('McpToolsConfig remote behavior', () => { expect(container.textContent).not.toContain('section.serverList.loadFailed'); }); + it.each(['Failed', 'Reconnecting'])('keeps %s server cards stable while polling and observes recovery', async (status) => { + vi.useFakeTimers(); + peerState.active = false; + const server = { + id: 'auto-start', + name: 'Auto-start server', + status, + serverType: 'local', + transport: 'stdio', + enabled: true, + autoStart: true, + commandAvailable: true, + startSupported: true, + }; + let resolveServers!: (servers: typeof server[]) => void; + getServersMock.mockImplementation(() => new Promise((resolve) => { + resolveServers = resolve; + })); + + await act(async () => root.render()); + expect(container.textContent).toContain('loading'); + await act(async () => resolveServers([server])); + const card = container.querySelector('[data-testid="mcp-server-item"]'); + expect(card).not.toBeNull(); + expect(card?.textContent).toContain(`status.${status.toLowerCase()}`); + + await act(async () => { vi.advanceTimersByTime(1000); }); + expect(getServersMock).toHaveBeenCalledTimes(2); + expect(container.querySelector('[data-testid="mcp-server-item"]')).toBe(card); + expect(container.textContent).not.toContain('loading'); + expect(card?.textContent).toContain(`status.${status.toLowerCase()}`); + + await act(async () => { vi.advanceTimersByTime(5000); }); + expect(getServersMock).toHaveBeenCalledTimes(2); + await act(async () => resolveServers([{ ...server, status: 'Connected' }])); + expect(container.querySelector('[data-testid="mcp-server-item"]')).toBe(card); + expect(card?.textContent).toContain('status.connected'); + expect(container.textContent).not.toContain('loading'); + await act(async () => { vi.advanceTimersByTime(2000); }); + expect(getServersMock).toHaveBeenCalledTimes(2); + }); + + it('retains stale data and its warning until a background refresh succeeds', async () => { + vi.useFakeTimers(); + peerState.active = false; + const server = { + id: 'auto-start', + name: 'Auto-start server', + status: 'Failed', + serverType: 'local', + transport: 'stdio', + enabled: true, + autoStart: true, + commandAvailable: true, + startSupported: true, + }; + let resolveRefresh!: (servers: typeof server[]) => void; + getServersMock + .mockResolvedValueOnce([server]) + .mockRejectedValueOnce(new Error('MCP status temporarily unavailable')) + .mockImplementationOnce(() => new Promise((resolve) => { + resolveRefresh = resolve; + })); + + await act(async () => root.render()); + const card = container.querySelector('[data-testid="mcp-server-item"]'); + expect(card).not.toBeNull(); + await act(async () => { vi.advanceTimersByTime(1000); }); + expect(container.textContent).toContain('external.status.stale'); + + const retry = container.querySelector('[aria-label="actions.refresh"]'); + expect(retry).not.toBeNull(); + await act(async () => retry?.click()); + expect(getServersMock).toHaveBeenCalledTimes(3); + expect(container.textContent).toContain('external.status.stale'); + expect(container.textContent).not.toContain('loading'); + expect(container.querySelector('[data-testid="mcp-server-item"]')).toBe(card); + + await act(async () => resolveRefresh([{ ...server, status: 'Connected' }])); + expect(container.textContent).not.toContain('external.status.stale'); + expect(card?.textContent).toContain('status.connected'); + }); + it('does not replace an unreadable MCP config with example JSON', async () => { loadJsonConfigMock.mockRejectedValueOnce(new Error('config unavailable')); peerState.active = false; @@ -393,6 +480,94 @@ describe('McpToolsConfig remote behavior', () => { expect(getServersMock).toHaveBeenCalledTimes(1); }); + it.each(['start', 'restart'])('respects disabled OAuth when a failed remote server is asked to %s', async (action) => { + peerState.active = false; + getServersMock.mockResolvedValue([{ + id: 'public-remote', + name: 'Public remote MCP', + status: 'Failed', + serverType: 'Remote', + transport: 'streamable-http', + enabled: true, + autoStart: false, + authConfigured: false, + oauthEnabled: false, + startSupported: true, + }]); + const actionMock = action === 'start' ? startServerMock : restartServerMock; + await act(async () => root.render()); + const button = container.querySelector(`[data-testid="mcp-server-${action}"]`); + expect(button).not.toBeNull(); + await act(async () => button?.click()); + + expect(actionMock).toHaveBeenCalledWith('public-remote'); + expect(startRemoteOAuthMock).not.toHaveBeenCalled(); + expect(getRemoteOAuthSessionMock).not.toHaveBeenCalled(); + expect(openExternalMock).not.toHaveBeenCalled(); + expect(document.querySelector('[data-openbitfun-part="authEditor"]')).toBeNull(); + + // A genuine authentication error may offer manual credentials, but must + // still never start OAuth or render its controls when explicitly disabled. + actionMock.mockRejectedValueOnce(new Error('status code: 401 Unauthorized')); + await act(async () => button?.click()); + expect(document.querySelector('[data-openbitfun-part="authEditor"]')).not.toBeNull(); + expect(document.body.textContent).not.toContain('modal.remoteOAuthDescription'); + expect(document.body.textContent).not.toContain('actions.startRemoteOAuth'); + expect(startRemoteOAuthMock).not.toHaveBeenCalled(); + expect(getRemoteOAuthSessionMock).not.toHaveBeenCalled(); + }); + + it.each([false, true])('retains the auth dialog through its exit and supports reopening (OAuth: %s)', async (oauthEnabled) => { + vi.useFakeTimers(); + peerState.active = false; + getServersMock.mockResolvedValue([{ + id: 'notion', + name: 'Notion', + status: 'Failed', + serverType: 'Remote', + transport: 'streamable-http', + url: 'https://mcp.notion.test/mcp', + enabled: true, + autoStart: false, + authConfigured: false, + oauthEnabled, + startSupported: true, + }]); + startServerMock.mockRejectedValue(new Error('status code: 401 Unauthorized')); + await act(async () => root.render()); + const start = container.querySelector('[data-testid="mcp-server-start"]')!; + await act(async () => start.click()); + const surface = document.querySelector('[role="dialog"]')!; + const editor = surface.querySelector('[data-openbitfun-part="authEditor"]'); + const contents = surface.textContent; + expect(editor).not.toBeNull(); + if (oauthEnabled) { + expect(contents).toContain('modal.remoteOAuthRedirectUri'); + expect(contents).toContain('modal.remoteOAuthStatus'); + } + + await act(async () => surface.querySelector('[data-openbitfun-part="close"]')!.click()); + expect(surface.dataset.state).toBe('exiting'); + expect(surface.getAttribute('aria-hidden')).toBe('true'); + expect(surface.querySelector('[data-openbitfun-part="authEditor"]')).toBe(editor); + expect(surface.textContent).toBe(contents); + expect(cancelRemoteOAuthMock).toHaveBeenCalledTimes(oauthEnabled ? 1 : 0); + // Retained status must not leave the server's start action disabled. + expect(start.disabled).toBe(false); + await act(async () => vi.advanceTimersByTime(90)); + await act(async () => start.click()); + expect(surface.dataset.state).toBe('open'); + await act(async () => vi.advanceTimersByTime(180)); + expect(document.querySelector('[role="dialog"]')).toBe(surface); + + await act(async () => surface.querySelector('[data-openbitfun-part="close"]')!.click()); + await act(async () => vi.advanceTimersByTime(179)); + expect(surface.isConnected).toBe(true); + expect(surface.textContent).toBe(contents); + await act(async () => vi.advanceTimersByTime(1)); + expect(document.querySelector('[role="dialog"]')).toBeNull(); + }); + it('starts OAuth directly for an unauthorized remote server without reporting a start failure', async () => { getServersMock.mockResolvedValueOnce([{ id: 'notion', diff --git a/src/web-ui/src/infrastructure/config/components/McpToolsConfig.tsx b/src/web-ui/src/infrastructure/config/components/McpToolsConfig.tsx index fff9b9ee59..051826a66e 100644 --- a/src/web-ui/src/infrastructure/config/components/McpToolsConfig.tsx +++ b/src/web-ui/src/infrastructure/config/components/McpToolsConfig.tsx @@ -204,6 +204,7 @@ const McpToolsConfig: React.FC = () => { Record >({}); const [authDialogServer, setAuthDialogServer] = useState(null); + const [authDialogOpen, setAuthDialogOpen] = useState(false); const [authValue, setAuthValue] = useState(''); const [authSubmitting, setAuthSubmitting] = useState(false); const [oauthSession, setOauthSession] = useState(null); @@ -283,7 +284,6 @@ const McpToolsConfig: React.FC = () => { const requestId = ++serverLoadRequestIdRef.current; try { setMcpLoading(true); - setServerLoadFailed(false); const serverList = await Promise.race([ MCPAPI.getServers(), new Promise((_, reject) => @@ -445,6 +445,8 @@ const McpToolsConfig: React.FC = () => { setJsonLoading(false); setJsonLoadFailed(false); setAuthDialogServer(null); + setAuthDialogOpen(false); + setAuthValue(''); setAuthSubmitting(false); setOauthSession(null); setOauthStarting(false); @@ -876,6 +878,7 @@ const McpToolsConfig: React.FC = () => { const capabilityEpoch = currentCapabilityEpoch(); if (capabilityEpoch === null) return; setAuthDialogServer(server); + setAuthDialogOpen(true); setAuthValue(''); setOauthSession(null); setOauthStarting(false); @@ -901,15 +904,16 @@ const McpToolsConfig: React.FC = () => { function closeAuthDialog() { stopOAuthPolling(); - setAuthDialogServer(null); + // Preserve the server and status copy while Dialog animates out. The next + // open resets them; clearing them here collapses the exiting surface. + setAuthDialogOpen(false); setAuthValue(''); - setOauthSession(null); setOauthStarting(false); setOauthCancelling(false); } const handleCloseAuthDialog = () => { - if (authSubmitting || oauthCancelling) return; + if (!authDialogOpen || authSubmitting || oauthCancelling) return; const capabilityEpoch = currentCapabilityEpoch(); if (capabilityEpoch === null) { closeAuthDialog(); @@ -945,7 +949,7 @@ const McpToolsConfig: React.FC = () => { }; const handleSaveRemoteAuth = async () => { - if (!authDialogServer || authSubmitting) return; + if (!authDialogOpen || !authDialogServer || authSubmitting) return; const capabilityEpoch = currentCapabilityEpoch(); if (capabilityEpoch === null) return; @@ -1065,7 +1069,7 @@ const McpToolsConfig: React.FC = () => { } const handleStartRemoteOAuth = async () => { - if (!authDialogServer || oauthStarting || authSubmitting) return; + if (!authDialogOpen || !authDialogServer || oauthStarting || authSubmitting) return; await startRemoteOAuthFlow(authDialogServer); }; @@ -1254,7 +1258,7 @@ const McpToolsConfig: React.FC = () => { const renderServerControl = (server: MCPServerInfo) => { const pendingAction = serverLifecycleActions[server.id]; - const oauthPending = authDialogServer?.id === server.id + const oauthPending = authDialogOpen && authDialogServer?.id === server.id && oauthSession !== null && !['authorized', 'failed', 'cancelled'].includes(oauthSession.status); const lifecyclePending = pendingAction !== undefined || oauthPending; @@ -1547,7 +1551,8 @@ const McpToolsConfig: React.FC = () => { )} - {desktopConfigAvailable && !showJsonEditor && mcpLoading && ( + {/* Polling updates existing cards in place; only an empty list needs a loading placeholder. */} + {desktopConfigAvailable && !showJsonEditor && mcpLoading && servers.length === 0 && (

{tMcp('loading')}

@@ -1587,7 +1592,7 @@ const McpToolsConfig: React.FC = () => { {!showJsonEditor && } { if (!nextOpen) handleCloseAuthDialog(); }} size="md" > diff --git a/src/web-ui/src/infrastructure/config/components/common/ConfigCollectionItem.scss b/src/web-ui/src/infrastructure/config/components/common/ConfigCollectionItem.scss index 6b1ce7f300..1b24155a7f 100644 --- a/src/web-ui/src/infrastructure/config/components/common/ConfigCollectionItem.scss +++ b/src/web-ui/src/infrastructure/config/components/common/ConfigCollectionItem.scss @@ -108,7 +108,7 @@ opacity: 0; transition: grid-template-rows 180ms var(--openbitfun-motion-easing-standard), - opacity 120ms ease; + opacity 180ms var(--openbitfun-motion-easing-standard); &[data-open='true'] { grid-template-rows: 1fr; @@ -116,9 +116,13 @@ } } -.openbitfun-collection-item__details { +// Keep padding and borders inside the clipped grid item so 0fr reaches zero. +.openbitfun-collection-item__details-clip { min-height: 0; overflow: hidden; +} + +.openbitfun-collection-item__details { padding: var(--openbitfun-space-4); border-top: 1px dashed var(--openbitfun-collection-details-border); background: var(--openbitfun-collection-details-bg); diff --git a/src/web-ui/src/infrastructure/config/components/common/ConfigCollectionItem.test.tsx b/src/web-ui/src/infrastructure/config/components/common/ConfigCollectionItem.test.tsx index 9948ca1836..4d0008cd60 100644 --- a/src/web-ui/src/infrastructure/config/components/common/ConfigCollectionItem.test.tsx +++ b/src/web-ui/src/infrastructure/config/components/common/ConfigCollectionItem.test.tsx @@ -65,8 +65,11 @@ describe('ConfigCollectionItem', () => { expect(details?.hasAttribute('inert')).toBe(true); act(() => { - vi.advanceTimersByTime(180); + vi.advanceTimersByTime(179); }); + expect(details?.isConnected).toBe(true); + expect(details?.textContent).toBe('Configuration location'); + act(() => vi.advanceTimersByTime(1)); expect(container.querySelector('.openbitfun-collection-item__details-collapse')).toBeNull(); }); diff --git a/src/web-ui/src/infrastructure/config/components/common/ConfigCollectionItem.tsx b/src/web-ui/src/infrastructure/config/components/common/ConfigCollectionItem.tsx index a0c775dde4..ea4bca46d3 100644 --- a/src/web-ui/src/infrastructure/config/components/common/ConfigCollectionItem.tsx +++ b/src/web-ui/src/infrastructure/config/components/common/ConfigCollectionItem.tsx @@ -127,7 +127,9 @@ export const ConfigCollectionItem: React.FC = ({ aria-hidden={!isExpanded} {...(!isExpanded ? { inert: '' } : {})} > -
{details}
+
+
{details}
+
) : null}