Skip to content

Comments

Implement initial version of MCP client#3

Merged
AzeemMuzammil merged 15 commits intoballerina-platform:mainfrom
AzeemMuzammil:fb-initial-implementation
Jun 6, 2025
Merged

Implement initial version of MCP client#3
AzeemMuzammil merged 15 commits intoballerina-platform:mainfrom
AzeemMuzammil:fb-initial-implementation

Conversation

@AzeemMuzammil
Copy link
Contributor

Purpose

$title.

  • Implement init, listTools, and callTool

Examples

Checklist

  • Linked to an issue
  • Updated the specification
  • Updated the changelog
  • Added tests
  • Checked native-image compatibility

@AzeemMuzammil AzeemMuzammil force-pushed the fb-initial-implementation branch from d5146b9 to d381be7 Compare June 4, 2025 08:09
@AzeemMuzammil AzeemMuzammil merged commit 441f099 into ballerina-platform:main Jun 6, 2025
5 checks passed
Comment on lines 99 to 110
json jsonData;
do {
jsonData = check eventData.fromJsonString();
} on fail error e {
return error JsonParsingError(string `Failed to parse SSE event data as JSON: ${e.message()}`);
}

do {
return check jsonData.cloneWithType();
} on fail error e {
return error TypeConversionError(string `Failed to convert JSON data to JsonRpcMessage: ${e.message()}`);
}
Copy link
Member

Choose a reason for hiding this comment

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

Unless you want the different errors, can directly do eventData.fromJsonStringWithType()


# Additional initialization options.
#
# + enforceStrictCapabilities - Whether to restrict emitted requests to only those that the remote
Copy link
Member

Choose a reason for hiding this comment

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

Shall we move this to field-level. This is the old way of documenting record fields.

# + params - The parameters for the progress notification
public type ProgressNotification record {|
*Notification;
"notifications/progress" method;
Copy link
Member

Choose a reason for hiding this comment

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

Shall we introduce consts/an enum for these?

#
# + serverResponse - The response from the server, which may be a single JsonRpcMessage, a stream, or a transport error.
# + return - Extracted ServerResult, ServerResponseError, or StreamError.
isolated function processServerResponse(JsonRpcMessage|stream<JsonRpcMessage, StreamError?>|StreamableHttpTransportError? serverResponse)
Copy link
Member

Choose a reason for hiding this comment

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

Line too long?

# Processes a server response and extracts the result.
#
# + serverResponse - The response from the server, which may be a single JsonRpcMessage, a stream, or a transport error.
# + return - Extracted ServerResult, ServerResponseError, or StreamError.
Copy link
Member

Choose a reason for hiding this comment

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

If you prefix these with type, the references will be validated too.

type `ServerResult`

StreamableHttpClientTransport? currentTransport = self.transport;
if currentTransport is () {
return error UninitializedTransportError(
"Closure failed: client transport is not initialized. Call initialize() first."
Copy link
Member

Choose a reason for hiding this comment

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

Is "Call initialize() first." really useful here?

Comment on lines +76 to +78
} else if contentType.includes(CONTENT_TYPE_JSON) {
return self.processJsonResponse(response);
} else {
Copy link
Member

Choose a reason for hiding this comment

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

else if can be an if
else is not needed, can just move the logic out

# + resources - Present if the server offers any resources to read.
# + tools - Present if the server offers any tools to call.
public type ServerCapabilities record {
record {|record {}...;|} experimental?;
Copy link
Member

Choose a reason for hiding this comment

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

record open by anydata or json?

can be map<map> or map<map>

Copy link
Member

Choose a reason for hiding this comment

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

Applicable elsewhere too.

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.

4 participants