Resolve the option param with using OpenAPI v3 for tools macro by Hendler · Pull Request #137 · modelcontextprotocol/rust-sdk

Hendler · 2025-04-17T06:02:25Z

Adds doc and test.

#135

Motivation and Context

How Has This Been Tested?

Breaking Changes

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
[] Documentation update

Checklist

I have read the MCP Documentation
My code follows the repository's style guidelines
New and existing tests pass locally
I have added appropriate error handling
I have added or updated documentation as needed

Additional context

Hendler · 2025-04-20T09:11:37Z

+ .map(|text| text.text.as_str())
+ .expect("Expected text content");
+
+ assert_eq!(


I would think this would fail per #135

jokemanfire · 2025-05-12T07:14:46Z

+}
+
+#[test]
+fn test_optional_field_schema_generation_via_macro() {


How about use a more little change test .
impl Server { /// This tool is used to get the weather of a city. #[tool(name = "get-weather", description = "Get the weather of a city.", vis = )] pub async fn get_weather(&self, #[tool(param)] city: String) -> String { drop(city); "rain".to_string() } #[tool(description = "Empty Parameter")] async fn empty_param(&self) {} +#[tool(description = "Optional Parameter")] +async fn optional_param(&self, #[tool(param)] city: Option<String>) -> String { + city.unwrap_or_default() +} }
#[tokio::test] async fn test_tool_macros_with_optional_param() { let _attr = Server::optional_param_tool_attr(); // println!("{_attr:?}"); let attr_type = _attr.input_schema.get("properties").unwrap().get("city").unwrap().get("type").unwrap(); println!("_attr.input_schema: {:?}", attr_type); assert_eq!(attr_type, "string"); }

jokemanfire · 2025-05-12T07:15:36Z

Sorry for the late reply, as the questioner has not updated for a long time.

…ld schema workaround (modelcontextprotocol#135) test(update): added test tool macro with optional

Hendler · 2025-05-13T17:07:16Z

@jokemanfire feel free to approve. Maybe fixing this will fix other issues as well?
cc @4t145

I may have more time to work on this soon.

…ld schema workaround (modelcontextprotocol#135) (modelcontextprotocol#137)

Hendler · 2025-04-17T06:02:25Z

Adds doc and test.

#135

Motivation and Context

How Has This Been Tested?

Breaking Changes

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
[] Documentation update

Checklist

I have read the MCP Documentation
My code follows the repository's style guidelines
New and existing tests pass locally
I have added appropriate error handling
I have added or updated documentation as needed

Additional context

Hendler · 2025-04-20T09:11:37Z

+ .map(|text| text.text.as_str())
+ .expect("Expected text content");
+
+ assert_eq!(


I would think this would fail per #135

jokemanfire · 2025-05-12T07:14:46Z

+}
+
+#[test]
+fn test_optional_field_schema_generation_via_macro() {


How about use a more little change test .
impl Server { /// This tool is used to get the weather of a city. #[tool(name = "get-weather", description = "Get the weather of a city.", vis = )] pub async fn get_weather(&self, #[tool(param)] city: String) -> String { drop(city); "rain".to_string() } #[tool(description = "Empty Parameter")] async fn empty_param(&self) {} +#[tool(description = "Optional Parameter")] +async fn optional_param(&self, #[tool(param)] city: Option<String>) -> String { + city.unwrap_or_default() +} }
#[tokio::test] async fn test_tool_macros_with_optional_param() { let _attr = Server::optional_param_tool_attr(); // println!("{_attr:?}"); let attr_type = _attr.input_schema.get("properties").unwrap().get("city").unwrap().get("type").unwrap(); println!("_attr.input_schema: {:?}", attr_type); assert_eq!(attr_type, "string"); }

jokemanfire · 2025-05-12T07:15:36Z

Sorry for the late reply, as the questioner has not updated for a long time.

…ld schema workaround (modelcontextprotocol#135) test(update): added test tool macro with optional

Hendler · 2025-05-13T17:07:16Z

@jokemanfire feel free to approve. Maybe fixing this will fix other issues as well?
cc @4t145

I may have more time to work on this soon.

…ld schema workaround (modelcontextprotocol#135) (modelcontextprotocol#137)

Hendler · 2025-04-17T06:02:25Z

Adds doc and test.

#135

Motivation and Context

How Has This Been Tested?

Breaking Changes

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
[] Documentation update

Checklist

I have read the MCP Documentation
My code follows the repository's style guidelines
New and existing tests pass locally
I have added appropriate error handling
I have added or updated documentation as needed

Additional context

Hendler · 2025-04-20T09:11:37Z

+ .map(|text| text.text.as_str())
+ .expect("Expected text content");
+
+ assert_eq!(


I would think this would fail per #135

jokemanfire · 2025-05-12T07:14:46Z

+}
+
+#[test]
+fn test_optional_field_schema_generation_via_macro() {


How about use a more little change test .
impl Server { /// This tool is used to get the weather of a city. #[tool(name = "get-weather", description = "Get the weather of a city.", vis = )] pub async fn get_weather(&self, #[tool(param)] city: String) -> String { drop(city); "rain".to_string() } #[tool(description = "Empty Parameter")] async fn empty_param(&self) {} +#[tool(description = "Optional Parameter")] +async fn optional_param(&self, #[tool(param)] city: Option<String>) -> String { + city.unwrap_or_default() +} }
#[tokio::test] async fn test_tool_macros_with_optional_param() { let _attr = Server::optional_param_tool_attr(); // println!("{_attr:?}"); let attr_type = _attr.input_schema.get("properties").unwrap().get("city").unwrap().get("type").unwrap(); println!("_attr.input_schema: {:?}", attr_type); assert_eq!(attr_type, "string"); }

jokemanfire · 2025-05-12T07:15:36Z

Sorry for the late reply, as the questioner has not updated for a long time.

…ld schema workaround (modelcontextprotocol#135) test(update): added test tool macro with optional

Hendler · 2025-05-13T17:07:16Z

@jokemanfire feel free to approve. Maybe fixing this will fix other issues as well?
cc @4t145

I may have more time to work on this soon.

…ld schema workaround (modelcontextprotocol#135) (modelcontextprotocol#137)

Hendler · 2025-04-17T06:02:25Z

Adds doc and test.

#135

Motivation and Context

How Has This Been Tested?

Breaking Changes

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
[] Documentation update

Checklist

I have read the MCP Documentation
My code follows the repository's style guidelines
New and existing tests pass locally
I have added appropriate error handling
I have added or updated documentation as needed

Additional context

Hendler · 2025-04-20T09:11:37Z

+ .map(|text| text.text.as_str())
+ .expect("Expected text content");
+
+ assert_eq!(


I would think this would fail per #135

jokemanfire · 2025-05-12T07:14:46Z

+}
+
+#[test]
+fn test_optional_field_schema_generation_via_macro() {


How about use a more little change test .
impl Server { /// This tool is used to get the weather of a city. #[tool(name = "get-weather", description = "Get the weather of a city.", vis = )] pub async fn get_weather(&self, #[tool(param)] city: String) -> String { drop(city); "rain".to_string() } #[tool(description = "Empty Parameter")] async fn empty_param(&self) {} +#[tool(description = "Optional Parameter")] +async fn optional_param(&self, #[tool(param)] city: Option<String>) -> String { + city.unwrap_or_default() +} }
#[tokio::test] async fn test_tool_macros_with_optional_param() { let _attr = Server::optional_param_tool_attr(); // println!("{_attr:?}"); let attr_type = _attr.input_schema.get("properties").unwrap().get("city").unwrap().get("type").unwrap(); println!("_attr.input_schema: {:?}", attr_type); assert_eq!(attr_type, "string"); }

jokemanfire · 2025-05-12T07:15:36Z

Sorry for the late reply, as the questioner has not updated for a long time.

…ld schema workaround (modelcontextprotocol#135) test(update): added test tool macro with optional

Hendler · 2025-05-13T17:07:16Z

@jokemanfire feel free to approve. Maybe fixing this will fix other issues as well?
cc @4t145

I may have more time to work on this soon.

…ld schema workaround (modelcontextprotocol#135) (modelcontextprotocol#137)

Hendler · 2025-04-17T06:02:25Z

Adds doc and test.

#135

Motivation and Context

How Has This Been Tested?

Breaking Changes

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
[] Documentation update

Checklist

I have read the MCP Documentation
My code follows the repository's style guidelines
New and existing tests pass locally
I have added appropriate error handling
I have added or updated documentation as needed

Additional context

Hendler · 2025-04-20T09:11:37Z

+ .map(|text| text.text.as_str())
+ .expect("Expected text content");
+
+ assert_eq!(


I would think this would fail per #135

jokemanfire · 2025-05-12T07:14:46Z

+}
+
+#[test]
+fn test_optional_field_schema_generation_via_macro() {


How about use a more little change test .
impl Server { /// This tool is used to get the weather of a city. #[tool(name = "get-weather", description = "Get the weather of a city.", vis = )] pub async fn get_weather(&self, #[tool(param)] city: String) -> String { drop(city); "rain".to_string() } #[tool(description = "Empty Parameter")] async fn empty_param(&self) {} +#[tool(description = "Optional Parameter")] +async fn optional_param(&self, #[tool(param)] city: Option<String>) -> String { + city.unwrap_or_default() +} }
#[tokio::test] async fn test_tool_macros_with_optional_param() { let _attr = Server::optional_param_tool_attr(); // println!("{_attr:?}"); let attr_type = _attr.input_schema.get("properties").unwrap().get("city").unwrap().get("type").unwrap(); println!("_attr.input_schema: {:?}", attr_type); assert_eq!(attr_type, "string"); }

jokemanfire · 2025-05-12T07:15:36Z

Sorry for the late reply, as the questioner has not updated for a long time.

…ld schema workaround (modelcontextprotocol#135) test(update): added test tool macro with optional

Hendler · 2025-05-13T17:07:16Z

@jokemanfire feel free to approve. Maybe fixing this will fix other issues as well?
cc @4t145

I may have more time to work on this soon.

…ld schema workaround (modelcontextprotocol#135) (modelcontextprotocol#137)

Hendler · 2025-04-17T06:02:25Z

Adds doc and test.

#135

Motivation and Context

How Has This Been Tested?

Breaking Changes

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
[] Documentation update

Checklist

I have read the MCP Documentation
My code follows the repository's style guidelines
New and existing tests pass locally
I have added appropriate error handling
I have added or updated documentation as needed

Additional context

Hendler · 2025-04-20T09:11:37Z

+ .map(|text| text.text.as_str())
+ .expect("Expected text content");
+
+ assert_eq!(


I would think this would fail per #135

jokemanfire · 2025-05-12T07:14:46Z

+}
+
+#[test]
+fn test_optional_field_schema_generation_via_macro() {


How about use a more little change test .
impl Server { /// This tool is used to get the weather of a city. #[tool(name = "get-weather", description = "Get the weather of a city.", vis = )] pub async fn get_weather(&self, #[tool(param)] city: String) -> String { drop(city); "rain".to_string() } #[tool(description = "Empty Parameter")] async fn empty_param(&self) {} +#[tool(description = "Optional Parameter")] +async fn optional_param(&self, #[tool(param)] city: Option<String>) -> String { + city.unwrap_or_default() +} }
#[tokio::test] async fn test_tool_macros_with_optional_param() { let _attr = Server::optional_param_tool_attr(); // println!("{_attr:?}"); let attr_type = _attr.input_schema.get("properties").unwrap().get("city").unwrap().get("type").unwrap(); println!("_attr.input_schema: {:?}", attr_type); assert_eq!(attr_type, "string"); }

jokemanfire · 2025-05-12T07:15:36Z

Sorry for the late reply, as the questioner has not updated for a long time.

…ld schema workaround (modelcontextprotocol#135) test(update): added test tool macro with optional

Hendler · 2025-05-13T17:07:16Z

@jokemanfire feel free to approve. Maybe fixing this will fix other issues as well?
cc @4t145

I may have more time to work on this soon.

…ld schema workaround (modelcontextprotocol#135) (modelcontextprotocol#137)

Hendler · 2025-04-17T06:02:25Z

Adds doc and test.

#135

Motivation and Context

How Has This Been Tested?

Breaking Changes

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
[] Documentation update

Checklist

I have read the MCP Documentation
My code follows the repository's style guidelines
New and existing tests pass locally
I have added appropriate error handling
I have added or updated documentation as needed

Additional context

Hendler · 2025-04-20T09:11:37Z

+ .map(|text| text.text.as_str())
+ .expect("Expected text content");
+
+ assert_eq!(


I would think this would fail per #135

jokemanfire · 2025-05-12T07:14:46Z

+}
+
+#[test]
+fn test_optional_field_schema_generation_via_macro() {


How about use a more little change test .
impl Server { /// This tool is used to get the weather of a city. #[tool(name = "get-weather", description = "Get the weather of a city.", vis = )] pub async fn get_weather(&self, #[tool(param)] city: String) -> String { drop(city); "rain".to_string() } #[tool(description = "Empty Parameter")] async fn empty_param(&self) {} +#[tool(description = "Optional Parameter")] +async fn optional_param(&self, #[tool(param)] city: Option<String>) -> String { + city.unwrap_or_default() +} }
#[tokio::test] async fn test_tool_macros_with_optional_param() { let _attr = Server::optional_param_tool_attr(); // println!("{_attr:?}"); let attr_type = _attr.input_schema.get("properties").unwrap().get("city").unwrap().get("type").unwrap(); println!("_attr.input_schema: {:?}", attr_type); assert_eq!(attr_type, "string"); }

jokemanfire · 2025-05-12T07:15:36Z

Sorry for the late reply, as the questioner has not updated for a long time.

…ld schema workaround (modelcontextprotocol#135) test(update): added test tool macro with optional

Hendler · 2025-05-13T17:07:16Z

@jokemanfire feel free to approve. Maybe fixing this will fix other issues as well?
cc @4t145

I may have more time to work on this soon.

…ld schema workaround (modelcontextprotocol#135) (modelcontextprotocol#137)

Hendler · 2025-04-17T06:02:25Z

Adds doc and test.

#135

Motivation and Context

How Has This Been Tested?

Breaking Changes

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
[] Documentation update

Checklist

I have read the MCP Documentation
My code follows the repository's style guidelines
New and existing tests pass locally
I have added appropriate error handling
I have added or updated documentation as needed

Additional context

Hendler · 2025-04-20T09:11:37Z

+ .map(|text| text.text.as_str())
+ .expect("Expected text content");
+
+ assert_eq!(


I would think this would fail per #135

jokemanfire · 2025-05-12T07:14:46Z

+}
+
+#[test]
+fn test_optional_field_schema_generation_via_macro() {


How about use a more little change test .
impl Server { /// This tool is used to get the weather of a city. #[tool(name = "get-weather", description = "Get the weather of a city.", vis = )] pub async fn get_weather(&self, #[tool(param)] city: String) -> String { drop(city); "rain".to_string() } #[tool(description = "Empty Parameter")] async fn empty_param(&self) {} +#[tool(description = "Optional Parameter")] +async fn optional_param(&self, #[tool(param)] city: Option<String>) -> String { + city.unwrap_or_default() +} }
#[tokio::test] async fn test_tool_macros_with_optional_param() { let _attr = Server::optional_param_tool_attr(); // println!("{_attr:?}"); let attr_type = _attr.input_schema.get("properties").unwrap().get("city").unwrap().get("type").unwrap(); println!("_attr.input_schema: {:?}", attr_type); assert_eq!(attr_type, "string"); }

jokemanfire · 2025-05-12T07:15:36Z

Sorry for the late reply, as the questioner has not updated for a long time.

…ld schema workaround (modelcontextprotocol#135) test(update): added test tool macro with optional

Hendler · 2025-05-13T17:07:16Z

@jokemanfire feel free to approve. Maybe fixing this will fix other issues as well?
cc @4t145

I may have more time to work on this soon.

…ld schema workaround (modelcontextprotocol#135) (modelcontextprotocol#137)

Hendler mentioned this pull request Apr 17, 2025
Optional Fields in rust-sdk MCP Schemas (type: ["T", "null"]) Cause Client Incompatibility #135
Closed

Hendler force-pushed the user/hendler/135 branch 3 times, most recently from 2113a98 to 3ccbc64Compare April 18, 2025 18:57

Hendler changed the title ~~Document OpenAPI compat issue~~Use OpenAPI v3 for tools macroApr 18, 2025

Hendler force-pushed the user/hendler/135 branch 3 times, most recently from a70f10a to 6527e83Compare April 20, 2025 09:10

Hendler commented Apr 20, 2025
View reviewed changes

jokemanfire reviewed May 12, 2025
View reviewed changes

jokemanfire changed the title ~~Use OpenAPI v3 for tools macro~~Resolve the option param with using OpenAPI v3 for tools macroMay 12, 2025

Hendler force-pushed the user/hendler/135 branch 3 times, most recently from b5919e0 to 9642796Compare May 13, 2025 16:42

test(openapi): add OpenAPI v3 compatibility and test for nullable fie…
1cc56b2
…ld schema workaround (modelcontextprotocol#135) test(update): added test tool macro with optional

Hendler force-pushed the user/hendler/135 branch from 9642796 to 1cc56b2Compare May 13, 2025 17:01

jokemanfire approved these changes May 14, 2025
View reviewed changes

jokemanfire merged commit 5d92061 into modelcontextprotocol:mainMay 14, 2025

4t145 mentioned this pull request May 16, 2025
ListToolsResult contains URL to non-reachable schema #179
Closed

github-actionsBot mentioned this pull request Jul 2, 2025
chore: release v0.2.0 #300
Merged

takumi-earth pushed a commit to earthlings-dev/rmcp that referenced this pull request Jan 27, 2026
feat(openapi): add OpenAPI v3 compatibility and test for nullable fie…
3c200d3
…ld schema workaround (modelcontextprotocol#135) (modelcontextprotocol#137)