feat(tools): add Model Context Protocol tool #265

pilartomas · 2025-01-02T13:51:14Z

Which issue(s) does this pull-request address?

Closes: #238

Description

This PR introduces a base for Model Context Protocol related tools and add support for MCP Resources. This gives framework users to give the agent the ability to query MCP compliant servers for resources.

Checklist

I have read the contributor guide
I have signed off on my commit
Linting passes: yarn lint or yarn lint:fix
Formatting is applied: yarn format or yarn format:fix
Unit tests pass: yarn test:unit
E2E tests pass: yarn test:e2e
Tests are included
Documentation is changed or added
Commit messages and PR title follow conventional commits

pilartomas · 2025-01-03T09:11:57Z

src/tools/mcp/mcpResource.ts

+    return z.object({
+      uri: z
+        .string()
+        .describe(
+          `URI of the resource to read, ${resources.length > 0 ? `available resources are:\n\n${resources.map(({ uri, name, description }) => JSON.stringify({ uri, name, description })).join("\n\n")}` : "no resources available at the moment"}.`,
+        ),
+    });


Suggestions are welcome

What about using never or literal?

https://github.com/i-am-bee/bee-api/blob/c1e3c6dc86cab913c3a246c4cfafbe2456030355/src/runs/execution/tools/read-file-tool.ts#L51

pilartomas · 2025-01-03T09:14:02Z

src/tools/mcp/mcpResource.ts

+  }
+
+  async inputSchema() {
+    const resources = await this.listPaginatedResources().catch(() => []);


Resource template support can be added later

Signed-off-by: Tomas Pilar <[email protected]>

pilartomas · 2025-01-03T13:34:20Z

src/tools/mcp/mcpResource.ts

+    const resources = await this.listResources().catch((err) => {
+      if (err instanceof McpError && err.code === ErrorCode.MethodNotFound) {
+        return [];
+      }
+      throw err;
+    });


Limit to pagination and ignoring not found error are not ideal solutions, ideas?

If the method for listing resources does not exist, the agent will need to know the uri of the resource exactly which I think will be rarely correct.

I'd just say that the servers not implementing this method are not supported for now or they will require a new specialized implementation.

Ad limit - idea:

wouldn't it be better to have a separate tool for listing resources? Pagination could be implemented as input parameter of the tool. Then the ResourceRead tool would have "you must first list resources" in the prompt

Can't we let the developer hardcode the list of resources when instantiating the tool, as an alternative to listing them?

A separate tool for listing resources doesn't sound right to me, we want to be able to take advantage of constrained decoding and validation through the input schema, and also having more tools degrades performance.

If the method for listing resources does not exist, the agent will need to know the uri of the resource exactly which I think will be rarely correct.
I'd just say that the servers not implementing this method are not supported for now or they will require a new specialized implementation.

Agreed, I'll remove the catch.

wouldn't it be better to have a separate tool for listing resources? Pagination could be implemented as input parameter of the tool. Then the ResourceRead tool would have "you must first list resources" in the prompt

I think that would be too heavy on the agent, also the user would have to make sure to include both tools which is quite error prone (but we could solve it with a joint factory somehow).

Can't we let the developer hardcode the list of resources when instantiating the tool, as an alternative to listing them?

We could but that would just put the problem on the user's back. The user would have to use the client, make a list a and inject the values into the constructor. It would also never update for that instance. I think we should solve it on our side. It would also sort of clash with another MCP concept from his perspective - roots

src/tools/mcp/mcpResource.ts

jezekra1 · 2025-01-03T14:17:17Z

src/tools/mcp/mcpResource.ts

+    const resources = await this.listResources().catch((err) => {
+      if (err instanceof McpError && err.code === ErrorCode.MethodNotFound) {
+        return [];
+      }
+      throw err;
+    });


If the method for listing resources does not exist, the agent will need to know the uri of the resource exactly which I think will be rarely correct.

I'd just say that the servers not implementing this method are not supported for now or they will require a new specialized implementation.

Ad limit - idea:

wouldn't it be better to have a separate tool for listing resources? Pagination could be implemented as input parameter of the tool. Then the ResourceRead tool would have "you must first list resources" in the prompt

src/tools/mcp/base.ts

src/tools/mcp/mcpResource.ts

Signed-off-by: Tomas Pilar <[email protected]>

Tomas2D · 2025-01-06T14:28:28Z

examples/tools/mcp.ts

+  });
+});
+
+await client.close();


Add some explanation please.

I would close to the finally block.

Tomas2D · 2025-01-06T14:31:37Z

src/internals/helpers/paginate.ts

+export interface PaginateWithCursorInput<T, C> {
+  size: number;
+  handler: (data: {
+    cursor: C | undefined;


not sure whether cursor needs to be typed with generic

Tomas2D · 2025-01-06T14:32:49Z

src/internals/helpers/paginate.ts

+  }
+
+  return acc;
+}


Also, I think that those two functions might be merged.

Tomas2D · 2025-01-06T14:33:41Z

src/tools/mcp/mcpResource.test.ts

+  const server = new Server(
+    {
+      name: "test-server",
+      version: "1.0.0",
+    },
+    {
+      capabilities: {
+        resources: {},
+      },
+    },
+  );
+
+  const client = new Client(
+    {
+      name: "test-client",
+      version: "1.0.0",
+    },
+    {
+      capabilities: {},
+    },
+  );


Based on the usage, I think that it would be better to initiate classes in the beforeEach.

Tomas2D · 2025-01-06T14:34:18Z

src/tools/mcp/mcpResource.ts

+}
+
+export class MCPResourceTool extends MCPTool<ReadResourceResult> {
+  name = "Resource";


MCPResource

Tomas2D · 2025-01-06T14:35:25Z

src/tools/mcp/mcpResource.ts

+        return nextCursor
+          ? ({
+              data: resources,
+              done: false,
+              nextCursor,
+            } as const)
+          : ({
+              data: resources,
+              done: true,
+            } as const);


Instead of passing done it would be much easier to pass falsy value for nextCursor.

Tomas2D · 2025-01-06T14:38:22Z

src/tools/mcp/mcpResource.ts

+    return z.object({
+      uri: z
+        .string()
+        .describe(
+          `URI of the resource to read, ${resources.length > 0 ? `available resources are:\n\n${resources.map(({ uri, name, description }) => JSON.stringify({ uri, name, description })).join("\n\n")}` : "no resources available at the moment"}.`,
+        ),
+    });


What about using never or literal?

https://github.com/i-am-bee/bee-api/blob/c1e3c6dc86cab913c3a246c4cfafbe2456030355/src/runs/execution/tools/read-file-tool.ts#L51

Tomas2D · 2025-01-06T14:38:43Z

src/tools/mcp/mcpResource.ts

+    });
+  }
+
+  protected async _run({ uri }: ToolInput<this>, run: GetRunContext<typeof this>) {


Incorrect usage, run is a third parameter

it should be

protected async _run({ uri }: ToolInput<this>, _options: BaseToolRunOptions, run: GetRunContext<typeof this>) {

pilartomas force-pushed the feat-mcp-tool branch 5 times, most recently from 35346f7 to 1cb77e8 Compare January 3, 2025 09:06

pilartomas commented Jan 3, 2025

View reviewed changes

pilartomas force-pushed the feat-mcp-tool branch from 1cb77e8 to 32790dd Compare January 3, 2025 09:13

pilartomas commented Jan 3, 2025

View reviewed changes

pilartomas force-pushed the feat-mcp-tool branch 2 times, most recently from 5d5093f to 9f04e7f Compare January 3, 2025 10:26

feat(tools): add Model Context Protocol tool

5af7015

Signed-off-by: Tomas Pilar <[email protected]>

pilartomas marked this pull request as ready for review January 3, 2025 10:45

pilartomas requested a review from a team as a code owner January 3, 2025 10:45

pilartomas force-pushed the feat-mcp-tool branch from 9f04e7f to 5af7015 Compare January 3, 2025 10:45

fixup!

d4def3e

Signed-off-by: Tomas Pilar <[email protected]>

pilartomas commented Jan 3, 2025

View reviewed changes

jezekra1 reviewed Jan 3, 2025

View reviewed changes

Tomas2D requested changes Jan 3, 2025

View reviewed changes

src/tools/mcp/base.ts Outdated Show resolved Hide resolved

src/tools/mcp/mcpResource.ts Outdated Show resolved Hide resolved

src/tools/mcp/mcpResource.ts Outdated Show resolved Hide resolved

pilartomas added 3 commits January 6, 2025 11:33

fix: pr comment

23c8449

Signed-off-by: Tomas Pilar <[email protected]>

fix: add cache

ea9cac0

Signed-off-by: Tomas Pilar <[email protected]>

fix: inline

676cac6

Signed-off-by: Tomas Pilar <[email protected]>

pilartomas force-pushed the feat-mcp-tool branch from db9fe6b to 676cac6 Compare January 6, 2025 11:18

fix: add test for cursor pagination

3c2b225

Signed-off-by: Tomas Pilar <[email protected]>

pilartomas force-pushed the feat-mcp-tool branch from d54eb22 to 3c2b225 Compare January 6, 2025 11:55

pilartomas added 2 commits January 6, 2025 13:01

fix: make dependency peer and optional

22250f3

Signed-off-by: Tomas Pilar <[email protected]>

fix: prompt

3de4911

Signed-off-by: Tomas Pilar <[email protected]>

pilartomas requested review from Tomas2D and jezekra1 January 6, 2025 14:24

Tomas2D requested changes Jan 6, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(tools): add Model Context Protocol tool #265

feat(tools): add Model Context Protocol tool #265

pilartomas commented Jan 2, 2025 •

edited

Loading

pilartomas Jan 3, 2025

Tomas2D Jan 6, 2025

pilartomas Jan 3, 2025

pilartomas Jan 3, 2025

jezekra1 Jan 3, 2025

JanPokorny Jan 3, 2025

pilartomas Jan 6, 2025

jezekra1 Jan 3, 2025

Tomas2D Jan 6, 2025

Tomas2D Jan 6, 2025

Tomas2D Jan 6, 2025

Tomas2D Jan 6, 2025

Tomas2D Jan 6, 2025

Tomas2D Jan 6, 2025

Tomas2D Jan 6, 2025

Tomas2D Jan 6, 2025

Tomas2D Jan 6, 2025

feat(tools): add Model Context Protocol tool #265

Are you sure you want to change the base?

feat(tools): add Model Context Protocol tool #265

Conversation

pilartomas commented Jan 2, 2025 • edited Loading

Which issue(s) does this pull-request address?

Description

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pilartomas commented Jan 2, 2025 •

edited

Loading