-
Notifications
You must be signed in to change notification settings - Fork 172
feat(elicitation): add user consent through elicitation MCP-185 #551
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
Changes from 2 commits
7f3ed9c
faa2824
7cf9386
80258d4
6c3e17a
3425bb6
896150c
a19e2f0
f883282
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,59 @@ | ||
| import type { PrimitiveSchemaDefinition } from "@modelcontextprotocol/sdk/types.js"; | ||
| import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; | ||
|
|
||
| export class Elicitation { | ||
| private readonly server: McpServer["server"]; | ||
| constructor({ server }: { server: McpServer["server"] }) { | ||
| this.server = server; | ||
| } | ||
|
|
||
| /** | ||
| * Checks if the client supports elicitation capabilities. | ||
| * @returns True if the client supports elicitation, false otherwise. | ||
| */ | ||
| public supportsElicitation(): boolean { | ||
| const clientCapabilities = this.server.getClientCapabilities(); | ||
| return clientCapabilities?.elicitation !== undefined; | ||
nirinchev marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| /** | ||
| * Requests a boolean confirmation from the user. | ||
| * @param message - The message to display to the user. | ||
| * @returns True if the user confirms the action or the client does not support elicitation, false otherwise. | ||
| */ | ||
| public async requestConfirmation(message: string): Promise<boolean> { | ||
| if (!this.supportsElicitation()) { | ||
| return true; | ||
| } | ||
|
|
||
| const result = await this.server.elicitInput({ | ||
| message, | ||
| requestedSchema: Elicitation.CONFIRMATION_SCHEMA, | ||
| }); | ||
| return result.action === "accept" && result.content?.confirmation === "Yes"; | ||
| } | ||
|
|
||
| /** | ||
| * The schema for the confirmation question. | ||
| * TODO: In the future would be good to use Zod 4's toJSONSchema() to generate the schema. | ||
| */ | ||
| public static CONFIRMATION_SCHEMA: MCPElicitationSchema = { | ||
gagik marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| type: "object", | ||
| properties: { | ||
| confirmation: { | ||
| type: "string", | ||
| title: "Would you like to confirm?", | ||
| description: "Would you like to confirm?", | ||
| enum: ["Yes", "No"], | ||
| enumNames: ["Yes, I confirm", "No, I do not confirm"], | ||
| }, | ||
| }, | ||
| required: ["confirmation"], | ||
| }; | ||
| } | ||
|
|
||
| export type MCPElicitationSchema = { | ||
| type: "object"; | ||
| properties: Record<string, PrimitiveSchemaDefinition>; | ||
| required?: string[]; | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; | |
| import { DbOperationArgs, MongoDBToolBase } from "../mongodbTool.js"; | ||
| import type { ToolArgs, OperationType } from "../../tool.js"; | ||
| import { checkIndexUsage } from "../../../helpers/indexCheck.js"; | ||
| import { EJSON } from "bson"; | ||
|
|
||
| export class DeleteManyTool extends MongoDBToolBase { | ||
| public name = "delete-many"; | ||
|
|
@@ -55,4 +56,17 @@ export class DeleteManyTool extends MongoDBToolBase { | |
| ], | ||
| }; | ||
| } | ||
|
|
||
| protected getConfirmationMessage({ database, collection, filter }: ToolArgs<typeof this.argsShape>): string { | ||
|
Collaborator
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. q1: Do we want to get confirmation for deletions even when we have a filter specified? I thought we only want to do it if you try and delete everything in the collection.
Collaborator
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 remember the discussions around this but maybe we still should? I think with deletions it's still good to review the filter and whatnot.
This does seem useful but could this be an expensive operation for extremely large collections? Seems like a potentially problematic side effect.
Collaborator
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 would not expect the MCP server to be used for perf critical applications, so am more inclined to think it's a worthwhile tradeoff, but don't feel strongly either way.
Collaborator
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 do think running other DB functions for the sake of a confirmation message is a bit unexpected but probably not critically terrible yeah. |
||
| const filterDescription = | ||
| filter && Object.keys(filter).length > 0 ? EJSON.stringify(filter) : "All documents (No filter)"; | ||
| return ( | ||
| `You are about to delete documents from the \`${collection}\` collection in the \`${database}\` database:\n\n` + | ||
| "```json\n" + | ||
| `{ "filter": ${filterDescription} }\n` + | ||
| "```\n\n" + | ||
| "This operation will permanently remove all documents matching the filter.\n\n" + | ||
| "**Do you confirm the execution of the action?**" | ||
| ); | ||
| } | ||
| } | ||
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.
If one wants to be paranoid... there's might be a chance some client will claim it supports
elicitationwhile not supporting it. Then this would likely make this tool a confusing no-op.That said this would be client's fault and one could unblock themselves by setting confirmation required tools to nothing.