Skip to content

Conversation

@zhaohuabing
Copy link
Contributor

@zhaohuabing zhaohuabing commented Aug 28, 2025

mcp: change return value of Server.complete to concrete type

This makes it consistent with other methods that implement RPCs.

Fixes #375

cc @findleyr

Signed-off-by: Huabing (Robin) Zhao <[email protected]>
@jba
Copy link
Contributor

jba commented Aug 28, 2025

Can you add a test? Our tests should have caught this.

@jba
Copy link
Contributor

jba commented Aug 28, 2025

Also the commit message should be something like:

mcp: change return value of Server.complete to concrete type

This makes it consistent with other methods that implement RPCs.

Fixes #375.

jba
jba previously requested changes Aug 28, 2025
Copy link
Contributor

@jba jba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments in Conversation.

@zhaohuabing
Copy link
Contributor Author

zhaohuabing commented Aug 28, 2025

See comments in Conversation.

@jba this is my first PR on MCP Go SDK - could you suggest where is the best place to add this test?

@zhaohuabing zhaohuabing requested a review from jba August 28, 2025 12:31
@zhaohuabing
Copy link
Contributor Author

See comments in Conversation.

@jba this is my first PR on MCP Go SDK - could you suggest where is the best place to add this test?

Guess it should be added to mcp_test.go.

Signed-off-by: Huabing (Robin) Zhao <[email protected]>
@zhaohuabing
Copy link
Contributor Author

Can you add a test? Our tests should have caught this.

Done.

@findleyr
Copy link
Contributor

Thank you for the fix!

@findleyr findleyr dismissed jba’s stale review August 29, 2025 14:04

Comments addressed.

@findleyr findleyr merged commit f6118aa into modelcontextprotocol:main Aug 29, 2025
5 checks passed
@zhaohuabing zhaohuabing deleted the fix-complete branch August 29, 2025 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

reflect: Elem of invalid type mcp.Result

3 participants