fix: address feedback

Author: jamadeo

crates/rmcp/src/transport/auth.rs

@@ -669,9 +669,8 @@ impl OAuthClientConfig {
 ///
 /// ```rust,ignore
 /// let request = AuthorizationRequest::new("http://localhost:8080/callback")
-///     // pass no scopes to let the SDK auto-select from server metadata
-///     .with_scopes(["mcp", "profile"])
-///     // used when the server supports CIMD and no pre-registered client is set
+///     // Omit `with_scopes` to let the SDK auto-select scopes from server metadata.
+///     // Used when the server supports CIMD and no pre-registered client is set.
 ///     .with_client_metadata_url("https://example.com/client-metadata.json")
 ///     // used for dynamic client registration as a last resort
 ///     .with_client_name("My MCP Client");
@@ -3071,8 +3070,14 @@ impl AuthorizationSession {
     /// retry without losing the original configuration and stores.
     pub async fn new(
         mut auth_manager: AuthorizationManager,
-        request: AuthorizationRequest,
+        mut request: AuthorizationRequest,
     ) -> Result<Self, (AuthorizationManager, AuthError)> {
+        if request.scopes.is_empty() {
+            request.scopes = auth_manager.select_scopes(None, &[]);
+        } else {
+            auth_manager.add_offline_access_if_supported(&mut request.scopes);
+        }
+
         let redirect_uri = request.redirect_uri.clone();
         let scopes = request.scopes.clone();
         let scope_refs: Vec<&str> = scopes.iter().map(|s| s.as_str()).collect();
@@ -3403,7 +3408,7 @@ impl OAuthState {
     /// without losing the original configuration and stores.
     pub async fn start_authorization(
         &mut self,
-        mut request: AuthorizationRequest,
+        request: AuthorizationRequest,
     ) -> Result<(), AuthError> {
         let placeholder = self.placeholder().await?;
         let old = std::mem::replace(self, placeholder);
@@ -3422,11 +3427,6 @@ impl OAuthState {
             }
         };
         manager.metadata = Some(metadata);
-        if request.scopes.is_empty() {
-            request.scopes = manager.select_scopes(None, &[]);
-        } else {
-            manager.add_offline_access_if_supported(&mut request.scopes);
-        }
         debug!("start session");
         match AuthorizationSession::new(manager, request).await {
             Ok(session) => {
@@ -3642,10 +3642,10 @@ mod tests {
 
     use super::{
         AuthError, AuthorizationCallback, AuthorizationManager, AuthorizationMetadata,
-        AuthorizationRequest, CredentialStore, InMemoryCredentialStore, InMemoryStateStore,
-        OAuthClientConfig, OAuthHttpClient, OAuthHttpClientError, OAuthHttpClientFuture,
-        OAuthHttpRedirectPolicy, OAuthHttpRequest, ScopeUpgradeConfig, StateStore,
-        StoredAuthorizationState, is_https_url,
+        AuthorizationRequest, AuthorizationSession, CredentialStore, InMemoryCredentialStore,
+        InMemoryStateStore, OAuthClientConfig, OAuthHttpClient, OAuthHttpClientError,
+        OAuthHttpClientFuture, OAuthHttpRedirectPolicy, OAuthHttpRequest, ScopeUpgradeConfig,
+        StateStore, StoredAuthorizationState, is_https_url,
     };
     use crate::transport::auth::VendorExtraTokenFields;
 
@@ -4149,22 +4149,25 @@ mod tests {
     }
 
     #[tokio::test]
-    async fn preregistered_client_selects_default_scopes_when_none_provided() {
+    async fn authorization_session_selects_default_scopes_when_none_provided() {
         let client = RecordingOAuthHttpClient::with_responses(preregistered_discovery_responses());
-        let mut state = super::OAuthState::new_with_oauth_http_client(
+        let mut manager = AuthorizationManager::new_with_oauth_http_client(
             "https://mcp.example.com/mcp",
-            Arc::new(client.clone()),
+            Arc::new(client),
         )
         .await
         .unwrap();
+        manager.metadata = Some(manager.discover_metadata().await.unwrap());
 
         let request = AuthorizationRequest::new("http://localhost:8080/callback")
             .with_preregistered_client("preregistered-client");
-        state.start_authorization(request).await.unwrap();
+        let session = match AuthorizationSession::new(manager, request).await {
+            Ok(session) => session,
+            Err((_, error)) => panic!("authorization session creation failed: {error}"),
+        };
 
-        // empty request scopes fall back to the discovered scopes_supported
-        let auth_url = state.get_authorization_url().await.unwrap();
-        let query = auth_url_query(&auth_url);
+        // Empty request scopes fall back to the discovered scopes_supported.
+        let query = auth_url_query(&session.auth_url);
         assert_eq!(query.get("scope").unwrap(), "read write offline_access");
     }