fix returnDirect attribute default value change false to true

[rlagyu0]

The default value of the returnDirect attribute in the @tool annotation has been changed to true. Users may not want the model to request the same thing twice, and by default, they expect that the return value of a method annotated with @tool will be returned directly to them. Setting the default of the returnDirect attribute to false would be unintuitive.

Furthermore, the majority of open-source tool creators focus on the tool performing some action, rather than having the return value interpreted differently through multiple models. Since the results can vary greatly depending on the model, it is preferable for the tool’s return value to provide a clear and direct answer to the user.

same issue : #3481

[rlagyu0]

same pull request : #3787
I think this approach would make the code messy.

[sunyuhan1998]

same pull request : #3787 I think this approach would make the code messy.

I think these two PRs don't seem to address the same issue? #3787 aims to resolve the problem where the returnDirect attribute of @Tool defined on the MCP server side fails to be passed to the client. However, this current PR appears to merely adjust the default value of returnDirect().

[rlagyu0]

same pull request : #3787 I think this approach would make the code messy.

I think these two PRs don't seem to address the same issue? #3787 aims to resolve the problem where the returnDirect attribute of @Tool defined on the MCP server side fails to be passed to the client. However, this current PR appears to merely adjust the default value of returnDirect().

The fundamental issue doesn’t seem likely to be resolved in the 1.1.0 release.
Adjusting the MCP protocol specification or modifying the project will probably take considerable time.
It looks like it will take too long to make a decision and work out the architecture right now—this isn’t a simple problem.
That’s why all I did was list the reasons why the returnDirect attribute needs to be set to true.

[sunyuhan1998]

The fundamental issue doesn’t seem likely to be resolved in the 1.1.0 release. Adjusting the MCP protocol specification or modifying the project will probably take considerable time. It looks like it will take too long to make a decision and work out the architecture right now—this isn’t a simple problem. That’s why all I did was list the reasons why the returnDirect attribute needs to be set to true.

Regarding the issue of passing the returnDirect property in the MCP protocol, we currently resolve it by transmitting returnDirect within the annotation field. This adjustment has already been merged into the MCP Java-SDK. You can review this PR: modelcontextprotocol/java-sdk#313. Actually, #3787 is built upon that PR. For an overview of the entire process, you might want to refer to the following issue: #3481.

[rlagyu0]

The fundamental issue doesn’t seem likely to be resolved in the 1.1.0 release. Adjusting the MCP protocol specification or modifying the project will probably take considerable time. It looks like it will take too long to make a decision and work out the architecture right now—this isn’t a simple problem. That’s why all I did was list the reasons why the returnDirect attribute needs to be set to true.

Regarding the issue of passing the returnDirect property in the MCP protocol, we currently resolve it by transmitting returnDirect within the annotation field. This adjustment has already been merged into the MCP Java-SDK. You can review this PR: modelcontextprotocol/java-sdk#313. Actually, #3787 is built upon that PR. For an overview of the entire process, you might want to refer to the following issue: #3481.

This is a different perspective from what was discussed in that issue.
The point you addressed concerns the need for the returnDirect attribute to be explicitly determined by the server and passed to the client.
What I’m arguing, however, is about the default value.

If the default is set to false, it will feel more intuitive to users and offer many advantages.
By common sense, the majority of library users would likely agree with this.

If you disagree with this view, please list the advantages of having the default set to true to counter my argument.

[sunyuhan1998]

If the default is set to false, it will feel more intuitive to users and offer many advantages. By common sense, the majority of library users would likely agree with this.

If you disagree with this view, please list the advantages of having the default set to true to counter my argument.

I think there must be some misunderstanding here. The reason I got involved in the current PR discussion is because you said:

same pull request : #3787

; so from the very beginning, I was merely repeating:

I think these two PRs don't seem to address the same issue? #3787 aims to resolve the problem where the returnDirect attribute of @tool defined on the MCP server side fails to be passed to the client. However, this current PR appears to merely adjust the default value of returnDirect().

I've never actually agreed with or denied whether the default value of returnDirect should be true or false—in all honesty, I don't really care about that point. All I've been trying to say from the beginning is that the current PR and #3787 address entirely different issues, they are not "same"—that's all.

[rlagyu0]

If the default is set to false, it will feel more intuitive to users and offer many advantages. By common sense, the majority of library users would likely agree with this.

If you disagree with this view, please list the advantages of having the default set to true to counter my argument.

I think there must be some misunderstanding here. The reason I got involved in the current PR discussion is because you said:

same pull request : #3787

; so from the very beginning, I was merely repeating:

I think these two PRs don't seem to address the same issue? #3787 aims to resolve the problem where the returnDirect attribute of @tool defined on the MCP server side fails to be passed to the client. However, this current PR appears to merely adjust the default value of returnDirect().

I've never actually agreed with or denied whether the default value of returnDirect should be true or false—in all honesty, I don't really care about that point. All I've been trying to say from the beginning is that the current PR and #3787 address entirely different issues, they are not "same"—that's all.

I misunderstood. It seems that the fundamental solution is still under discussion.
I used the word “same” because I thought it was just a part of the issue.
I’ll correct that. Thank you.

[sunyuhan1998]

I misunderstood. It seems that the fundamental solution is still under discussion. I used the word “same” because I thought it was just a part of the issue. I’ll correct that. Thank you.

Yes, you're right. Actually, we are currently discussing two different issues. Prior to #3481, there had already been several issues reporting that the returnDirect attribute defined in tools using the @Tool annotation on the MCP server side was not taking effect on the client side—this is exactly why I initiated PRs modelcontextprotocol/java-sdk#313 and #3787. However, before your current PR, there didn't seem to be any issue discussing what the default value of returnDirect should actually be—true or false. Therefore, I suggest you could simultaneously create a dedicated issue outlining your reasoning for preferring the default to be true. This would help facilitate broader input from users and maintainers, making it easier to reach a consensus.

[ilayaperumalg]

@rlagyu0 Setting the default value of returnDirect to false was by design as we wanted to return the tool execution result back to the model to continue the ongoing conversation between the client and model. Since, it is the model which dispatches the tool call requests, it is by default, the tool call result is sent back to the model itself.

Since this is the typical flow of client (with tools) -> model interaction, the returnDirect is set to false by default which can be overridden to true if needed.

[ilayaperumalg]

@rlagyu0 Closing this as not ready for merge for now as we intend to keep the design to set the returnDirect to false by default.