fix(mcp): include OAuth state parameter in authorization URLs (#1049)

Some MCP servers (e.g. Attio) require the `state` parameter in OAuth
authorization requests and reject requests without it:

  {"error":"invalid_request","error_description":"Invalid value provided for: state"}

While OAuth 2.1 makes `state` optional when PKCE is used, the MCP
specification does not forbid servers from requiring it. This caused a
hard failure when authenticating with any MCP server that enforces the
state parameter.

Generate a 128-bit cryptographically random state (via OsRng, base64url
encoded without padding) and inject it into extra_params before building
the authorization URL. This covers both pre-configured OAuth and Dynamic
Client Registration (DCR) code paths.

The callback listener intentionally does not validate the echoed state
because: (1) PKCE already binds the authorization code to the token
exchange, preventing code injection attacks, and (2) not all MCP servers
echo state back — strict validation would break those servers. Other
OAuth flows in the codebase (tool.rs, extensions/manager.rs) that
generate and validate state are unaffected.

src/tools/mcp/auth.rs

@@ -669,7 +669,7 @@ pub async fn authorize_mcp_server(
     }
 
     // Determine client_id and endpoints
-    let (client_id, authorization_url, token_url, use_pkce, scopes, extra_params) =
+    let (client_id, authorization_url, token_url, use_pkce, scopes, mut extra_params) =
         if let Some(oauth) = &server_config.oauth {
             // Pre-configured OAuth
             let (auth_url, tok_url) = discover_oauth_endpoints(server_config).await?;
@@ -711,6 +711,13 @@ pub async fn authorize_mcp_server(
         None
     };
 
+    // Generate OAuth state parameter. While optional in OAuth 2.1 with PKCE,
+    // some MCP servers (e.g. Attio) require it.
+    let mut state_bytes = [0u8; 16];
+    rand::rngs::OsRng.fill_bytes(&mut state_bytes);
+    let state = URL_SAFE_NO_PAD.encode(state_bytes);
+    extra_params.insert("state".to_string(), state);
+
     // Compute canonical resource URI for RFC 8707
     let resource = canonical_resource_uri(&server_config.url);
 
@@ -741,7 +748,10 @@ pub async fn authorize_mcp_server(
 
     println!("  Waiting for authorization...");
 
-    // Wait for callback
+    // Wait for callback. State is sent in the URL for servers that require it
+    // (e.g. Attio), but we don't enforce validation on the callback because MCP
+    // servers use PKCE which already binds the request to the token exchange,
+    // and some servers may not echo state back.
     let code = wait_for_authorization_callback(listener, &server_config.name).await?;
 
     println!("  Exchanging code for token...");
@@ -1711,4 +1721,65 @@ mod tests {
 
         assert!(!url.contains("resource="));
     }
+
+    /// Regression test: MCP OAuth authorization URLs must include a `state`
+    /// parameter. While OAuth 2.1 makes `state` optional when PKCE is used,
+    /// some MCP servers (e.g. Attio) require it and reject requests without it:
+    /// {"error":"invalid_request","error_description":"Invalid value provided
+    /// for: state"}
+    ///
+    /// Including `state` is harmless for servers that don't require it, since
+    /// it is a standard OAuth parameter that compliant servers will echo back
+    /// or ignore.
+    ///
+    /// The state is generated in `authorize_mcp_server` and injected into
+    /// `extra_params` before `build_authorization_url` is called. This test
+    /// verifies that `build_authorization_url` correctly propagates state from
+    /// extra_params into the URL, and that each generated state is unique.
+    #[test]
+    fn test_authorization_url_includes_state_parameter() {
+        // Simulate what authorize_mcp_server does: generate state and
+        // insert it into extra_params.
+        let mut extra_params = HashMap::new();
+        let mut state_bytes = [0u8; 16];
+        rand::rngs::OsRng.fill_bytes(&mut state_bytes);
+        let state = URL_SAFE_NO_PAD.encode(state_bytes);
+        extra_params.insert("state".to_string(), state.clone());
+
+        let pkce = PkceChallenge::generate();
+        let url = build_authorization_url(
+            "https://app.attio.com/oidc/authorize",
+            "test-client",
+            "http://127.0.0.1:9876/callback",
+            &["mcp".to_string(), "offline_access".to_string(), "openid".to_string()],
+            Some(&pkce),
+            &extra_params,
+            Some("https://mcp.attio.com/mcp"),
+        );
+
+        // State must be present in the URL
+        assert!(
+            url.contains(&format!("state={}", state)),
+            "Authorization URL must include the state parameter, got: {}",
+            url,
+        );
+
+        // State must be base64url-encoded (no padding, no +/)
+        assert!(!state.contains('+'), "State must be base64url-safe");
+        assert!(!state.contains('/'), "State must be base64url-safe");
+        assert!(!state.contains('='), "State must not have padding");
+
+        // State must have sufficient entropy (16 bytes -> 22 base64url chars)
+        assert!(
+            state.len() >= 22,
+            "State must have at least 128 bits of entropy, got {} chars",
+            state.len(),
+        );
+
+        // Two generated states must differ
+        let mut state_bytes_2 = [0u8; 16];
+        rand::rngs::OsRng.fill_bytes(&mut state_bytes_2);
+        let state_2 = URL_SAFE_NO_PAD.encode(state_bytes_2);
+        assert_ne!(state, state_2, "State must be unique per request");
+    }
 }