-
Notifications
You must be signed in to change notification settings - Fork 293
feat: rename server property to mcp in MCP-related classes for improved developer experience #427
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| --- | ||
| "@cloudflare/agents": major | ||
| --- | ||
|
|
||
| Rename `server` property to `mcp` in MCP-related classes for better developer experience | ||
|
|
||
| ## Changes | ||
|
|
||
| - **BREAKING**: Renamed `server` property to `mcp` in `McpAgent` class and related examples | ||
| - **BREAKING**: Renamed `mcp` property to `mcpClientManager` in `Agent` class to avoid naming conflicts | ||
| - Added backward compatibility support for `server` property in `McpAgent` with deprecation warning | ||
| - Updated all MCP examples to use the new `mcp` property naming convention | ||
| - Improved property naming consistency across the MCP implementation | ||
|
|
||
| ## Migration | ||
|
|
||
| If you're using the `server` property in your `McpAgent` implementations, update your code: | ||
|
|
||
| ```ts | ||
| // Before | ||
| export class MyMcpAgent extends McpAgent { | ||
| server = new McpServer({...}); | ||
| } | ||
|
|
||
| // After | ||
| export class MyMcpAgent extends McpAgent { | ||
| mcp = new McpServer({...}); | ||
| } | ||
| ``` | ||
|
|
||
| The `server` property is still supported for backward compatibility but will be removed in a future version. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,7 +17,7 @@ type Env = { | |
| }; | ||
|
|
||
| export class McpServerAgent extends McpAgent<Env, { counter: number }, {}> { | ||
| server = new McpServer({ | ||
| mcp = new McpServer({ | ||
| name: "Elicitation Demo Server", | ||
| version: "1.0.0" | ||
| }); | ||
|
|
@@ -83,7 +83,7 @@ export class McpServerAgent extends McpAgent<Env, { counter: number }, {}> { | |
|
|
||
| async init() { | ||
| // Counter tool with user confirmation via elicitation | ||
| this.server.tool( | ||
| this.mcp.tool( | ||
| "increment-counter", | ||
| "Increment the counter with user confirmation", | ||
| { | ||
|
|
@@ -151,7 +151,7 @@ export class McpServerAgent extends McpAgent<Env, { counter: number }, {}> { | |
| ); | ||
|
|
||
| // User creation tool with form-based elicitation | ||
| this.server.tool( | ||
| this.mcp.tool( | ||
| "create-user", | ||
| "Create a new user with form input", | ||
| { | ||
|
|
@@ -225,7 +225,7 @@ export class McpServerAgent extends McpAgent<Env, { counter: number }, {}> { | |
| ); | ||
|
|
||
| // Counter resource | ||
| this.server.resource("counter", "mcp://resource/counter", (uri: URL) => { | ||
| this.mcp.resource("counter", "mcp://resource/counter", (uri: URL) => { | ||
| return { | ||
| contents: [ | ||
| { | ||
|
|
@@ -388,7 +388,7 @@ export class MyAgent extends Agent<Env, never> { | |
| __clientSession: this.name | ||
| }; | ||
|
|
||
| const result = await this.mcp.callTool({ | ||
| const result = await this.mcpClientManager.callTool({ | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here's the breaking change. |
||
| serverId, | ||
| name: toolName, | ||
| arguments: enhancedArgs | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,7 @@ type Env = { | |
| type State = { counter: number }; | ||
|
|
||
| export class MyMCP extends McpAgent<Env, State, {}> { | ||
| server = new McpServer({ | ||
| mcp = new McpServer({ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think that would allow me to do this: export class MyMCP extends McpAgent {
mcp = new McpServer({}); // <-- this is the desired API
async init() {
this.mcp.server.setRequestHandler(
SetLevelRequestSchema,
async (request, extra) => {
// ... handler logic
},
);
}
}There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Something like this? export class MyMCP extends McpAgent {
get mcp(): McpServer {
return this.server;
}
server = new McpServer({});
async init() {
this.mcp.server.setRequestHandler(
SetLevelRequestSchema,
async (request, extra) => {
// ... handler logic
},
);
}
}
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could do that myself without having to change anything in
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, actually I can't do that because |
||
| name: "Demo", | ||
| version: "1.0.0" | ||
| }); | ||
|
|
@@ -19,13 +19,13 @@ export class MyMCP extends McpAgent<Env, State, {}> { | |
| }; | ||
|
|
||
| async init() { | ||
| this.server.resource("counter", "mcp://resource/counter", (uri) => { | ||
| this.mcp.resource("counter", "mcp://resource/counter", (uri) => { | ||
| return { | ||
| contents: [{ text: String(this.state.counter), uri: uri.href }] | ||
| }; | ||
| }); | ||
|
|
||
| this.server.tool( | ||
| this.mcp.tool( | ||
| "add", | ||
| "Add to the counter, stored in the MCP", | ||
| { a: z.number() }, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -210,8 +210,24 @@ export abstract class McpAgent< | |
| */ | ||
| private _agent: Agent<Env, State>; | ||
|
|
||
| get mcp() { | ||
| return this._agent.mcp; | ||
| get mcpClientManager() { | ||
| return this._agent.mcpClientManager; | ||
| } | ||
|
Comment on lines
+213
to
+215
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This bit is technically breaking for anyone who's using the client manager, but I don't think anyone is and it's not documented.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Very happy with this change. I think this is super confusing at the moment. We should keep the |
||
|
|
||
| /** | ||
| * Getter that returns the MCP server implementation. | ||
| * Supports both 'server' and 'mcp' properties for better developer experience. | ||
| * If 'mcp' property is set, use that; otherwise fall back to 'server' property. | ||
| */ | ||
| get mcpServer(): MaybePromise<McpServer | Server> { | ||
| const server = this.mcp ?? this.server; | ||
| if (!server) { | ||
| throw new Error( | ||
| 'Neither the `mcp` nor `server` property is set. Please set "mcp".' | ||
| ); | ||
| } | ||
|
|
||
| return server; | ||
|
Comment on lines
+222
to
+230
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this adds to the confusion right now. IMO mcpServer should be the new name for |
||
| } | ||
|
|
||
| protected constructor(ctx: DurableObjectState, env: Env) { | ||
|
|
@@ -333,7 +349,7 @@ export abstract class McpAgent< | |
| )) as TransportType; | ||
| await this._init(this.props); | ||
|
|
||
| const server = await this.server; | ||
| const server = await this.mcpServer; | ||
|
|
||
| // Connect to the MCP server | ||
| if (this._transportType === "sse") { | ||
|
|
@@ -350,8 +366,17 @@ export abstract class McpAgent< | |
|
|
||
| /** | ||
| * McpAgent API | ||
| * @deprecated Use the `mcp` property instead for better developer experience. | ||
| */ | ||
| server?: MaybePromise<McpServer | Server>; | ||
| /** | ||
| * This is only set as optional for backward compatibility in case you're | ||
| * using the legacy `server` property. It's recommended to use `mcp`. | ||
| * | ||
| * In the future, we'll make this a required abstract property to implement | ||
| * and remove the `server` property. | ||
| */ | ||
| abstract server: MaybePromise<McpServer | Server>; | ||
| mcp?: MaybePromise<McpServer | Server>; | ||
| props!: Props; | ||
| initRun = false; | ||
|
|
||
|
|
@@ -430,7 +455,7 @@ export abstract class McpAgent< | |
| // This is not the path that the user requested, but the path that the worker | ||
| // generated. We'll use this path to determine which transport to use. | ||
| const path = url.pathname; | ||
| const server = await this.server; | ||
| const server = await this.mcpServer; | ||
|
|
||
| switch (path) { | ||
| case "/sse": { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to major because there was an example of using the previous existing
mcpproperty so people may actually be using that.