Skip to content

fix: Fixed the issue where the returnDirect attribute was not effective on the MCP server side. #3787

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 7 commits into from
Closed

fix: Fixed the issue where the returnDirect attribute was not effective on the MCP server side. #3787

wants to merge 7 commits into from

Conversation

@sunyuhan1998
Copy link
Contributor

@sunyuhan1998 sunyuhan1998 commented Jul 11, 2025

Background

Before this PR, when using the @Tool annotation to define tools on the MCP server side and specifying the returnDirect attribute, the attribute would not be properly propagated. As a result, the client was unable to retrieve this information. For a detailed discussion and root cause analysis of this issue, please refer to: #3481 .

Thanks to the merge of PR #3781 , we now have the foundation to fix this problem.

Changes in This PR

  1. Preserve the returnDirect Attribute
    Adjusted the logic in the McpToolUtils class that constructs tool declarations, ensuring that the returnDirect attribute from the @Tool annotation is correctly retained and passed through.

  2. Implemented getToolMetadata Method
    Added implementations of the getToolMetadata method in both AsyncMcpToolCallback and SyncMcpToolCallback classes. This enables clients to retrieve tool metadata, including the returnDirect flag.

  3. Added Unit Tests
    Included corresponding unit tests to verify the correctness and stability of the above changes.

Fixes: #3481

@ilayaperumalg ilayaperumalg added this to the 1.1.x milestone Jul 11, 2025
@ilayaperumalg
Copy link
Member

ilayaperumalg commented Jul 11, 2025

@sunyuhan1998 Thanks for the quick follow up on this!

@ilayaperumalg ilayaperumalg self-assigned this Jul 18, 2025
@sunyuhan1998 sunyuhan1998 mentioned this pull request Aug 27, 2025
Closed
@sunyuhan1998 sunyuhan1998 mentioned this pull request Sep 16, 2025
Open
rlagyu0 pushed a commit to rlagyu0/spring-ai that referenced this pull request Sep 17, 2025
@ai-afk @rlagyu0
fix: spring-projects#3787 fix returnDirect attribute default value ch…
81ce3bc 
…ange false to true

Signed-off-by: rlagyu0 <[email protected]>
@rlagyu0 rlagyu0 mentioned this pull request Sep 17, 2025
Closed
@sunyuhan1998
Merge branch 'main' into fix-issue-3481
aadea40 
# Conflicts:
#	mcp/common/src/main/java/org/springframework/ai/mcp/AsyncMcpToolCallback.java
#	mcp/common/src/main/java/org/springframework/ai/mcp/McpToolUtils.java
#	mcp/common/src/main/java/org/springframework/ai/mcp/SyncMcpToolCallback.java
#	mcp/common/src/test/java/org/springframework/ai/mcp/SyncMcpToolCallbackTests.java
@sunyuhan1998
merge: Merged the main branch code and resolved the conflicts.
651a670 
Signed-off-by: Sun Yuhan <[email protected]>
@sunyuhan1998
fix: Fix code format
f2c6567 
Signed-off-by: Sun Yuhan <[email protected]>
@sunyuhan1998
fix: Fix incorrect merge
1b87887 
Signed-off-by: Sun Yuhan <[email protected]>
@sunyuhan1998
fix: Fix code format
fee5705 
Signed-off-by: Sun Yuhan <[email protected]>
@markpollack markpollack removed this from the 1.1.0.M1 milestone Sep 25, 2025
@KallivdH
Copy link

KallivdH commented Oct 10, 2025 •
edited
Loading

Will this fix be added to the 1.1.0.M3 pre-release @markpollack ?

@markpollack markpollack added this to the 1.1.0.RC1 milestone Oct 22, 2025
@ilayaperumalg
Copy link
Member

ilayaperumalg commented Oct 23, 2025

Hi @sunyuhan1998 Sorry about the delay in getting to this. Having the returnDirect as MCP schema ToolAnnotations attribute in the MCP JavaSDK is rather invasive as the MCP spec and its implementation shouldn't know anything about Spring AI's behaviour. We need to find a better approach to handle returnDirect instead of the current MCP Schema ToolAnnotations. What do you think?

@sunyuhan1998
Copy link
Contributor Author

sunyuhan1998 commented Oct 24, 2025

Hi @sunyuhan1998 Sorry about the delay in getting to this. Having the returnDirect as MCP schema ToolAnnotations attribute in the MCP JavaSDK is rather invasive as the MCP spec and its implementation shouldn't know anything about Spring AI's behaviour. We need to find a better approach to handle returnDirect instead of the current MCP Schema ToolAnnotations. What do you think?

You're right. At the time this PR was conceived and submitted, the mcp-annotations project hadn't yet introduced the @McpTool annotation. Currently, @McpTool is strictly implemented according to the MCP specification and does not include any logic related to returnDirect. I suspect that using the @Tool annotation on the MCP server side early on was perhaps a workaround or a shortcut? I'm curious about our future plans for balancing the capabilities of @McpTool and @Tool on the MCP server side. Do we intend to gradually phase out the use of @Tool for defining tools in the MCP server going forward?

@ilayaperumalg
Copy link
Member

ilayaperumalg commented Oct 24, 2025 •
edited
Loading

@sunyuhan1998 We intend to use @McpTool for the tools specific to MCP servers while the Spring AI @Tool for non MCP tools. But, currently, for backwards compatibility and the ease of use, we support both the options.

Do we intend to gradually phase out the use of @tool for defining tools in the MCP server going forward?

I agree. This could be something in Spring AI 2.0 cc @tzolov

@sunyuhan1998
Copy link
Contributor Author

sunyuhan1998 commented Oct 24, 2025

@sunyuhan1998 We intend to use @McpTool for the tools specific to MCP servers while the Spring AI @Tool for non MCP tools. But, currently, for backwards compatibility and the ease of use, we support both the options.

Do we intend to gradually phase out the use of @tool for defining tools in the MCP server going forward?

I agree. This could be something in Spring AI 2.0 cc @tzolov

Thank you for your explanation. I understand now. If that's the case, does it mean the issue we've been discussing in this PR no longer needs to be followed up on? Should I close this PR?

@ilayaperumalg
Copy link
Member

ilayaperumalg commented Oct 24, 2025

@sunyuhan1998 Yes, please. Let's close this PR.

@ilayaperumalg ilayaperumalg added the invalid This doesn't seem right label Oct 24, 2025
@KallivdH
Copy link

KallivdH commented Oct 24, 2025 •
edited
Loading

Thank you for following up!

Hi @sunyuhan1998 Sorry about the delay in getting to this. Having the returnDirect as MCP schema ToolAnnotations attribute in the MCP JavaSDK is rather invasive as the MCP spec and its implementation shouldn't know anything about Spring AI's behaviour. We need to find a better approach to handle returnDirect instead of the current MCP Schema ToolAnnotations. What do you think?

@sunyuhan1998 The audience annotation look like a right fit for the returnDirect functionality. For example, if audience user is specified.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@rlagyu0 rlagyu0 rlagyu0 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@ilayaperumalg ilayaperumalg

Labels

invalid This doesn't seem right MCP tool/function calling

Projects

None yet

Milestone

1.1.0.RC1

Development

Successfully merging this pull request may close these issues.

@Tool returnDirect Attribute Ignored

5 participants

@sunyuhan1998 @ilayaperumalg @KallivdH @rlagyu0 @markpollack