Skip to content

Commit f883282

Browse files
committed
chore: check for default message, use MCP schema type, tweak formatting of delete docs
1 parent a19e2f0 commit f883282

File tree

3 files changed

+17
-29
lines changed

3 files changed

+17
-29
lines changed

src/elicitation.ts

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { PrimitiveSchemaDefinition } from "@modelcontextprotocol/sdk/types.js";
1+
import type { ElicitRequest } from "@modelcontextprotocol/sdk/types.js";
22
import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
33

44
export class Elicitation {
@@ -37,7 +37,7 @@ export class Elicitation {
3737
* The schema for the confirmation question.
3838
* TODO: In the future would be good to use Zod 4's toJSONSchema() to generate the schema.
3939
*/
40-
public static CONFIRMATION_SCHEMA: MCPElicitationSchema = {
40+
public static CONFIRMATION_SCHEMA: ElicitRequest["params"]["requestedSchema"] = {
4141
type: "object",
4242
properties: {
4343
confirmation: {
@@ -51,9 +51,3 @@ export class Elicitation {
5151
required: ["confirmation"],
5252
};
5353
}
54-
55-
export type MCPElicitationSchema = {
56-
type: "object";
57-
properties: Record<string, PrimitiveSchemaDefinition>;
58-
required?: string[];
59-
};

src/tools/mongodb/delete/deleteMany.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,12 @@ export class DeleteManyTool extends MongoDBToolBase {
5959

6060
protected getConfirmationMessage({ database, collection, filter }: ToolArgs<typeof this.argsShape>): string {
6161
const filterDescription =
62-
filter && Object.keys(filter).length > 0 ? EJSON.stringify(filter) : "All documents (No filter)";
62+
filter && Object.keys(filter).length > 0
63+
? "```json\n" + `{ "filter": ${EJSON.stringify(filter)} }\n` + "```\n\n"
64+
: "- **All documents** (No filter)\n\n";
6365
return (
6466
`You are about to delete documents from the \`${collection}\` collection in the \`${database}\` database:\n\n` +
65-
"```json\n" +
66-
`{ "filter": ${filterDescription} }\n` +
67-
"```\n\n" +
67+
filterDescription +
6868
"This operation will permanently remove all documents matching the filter.\n\n" +
6969
"**Do you confirm the execution of the action?**"
7070
);

tests/integration/elicitation.test.ts

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ describe("Elicitation Integration Tests", () => {
2525
{ elicitInput: mockElicitInput }
2626
);
2727

28-
describe("tools requiring confirmation", () => {
28+
describe("tools requiring confirmation by default", () => {
2929
it("should request confirmation for drop-database tool and proceed when confirmed", async () => {
3030
mockElicitInput.confirmYes();
3131

@@ -37,19 +37,7 @@ describe("Elicitation Integration Tests", () => {
3737
expect(mockElicitInput.mock).toHaveBeenCalledTimes(1);
3838
expect(mockElicitInput.mock).toHaveBeenCalledWith({
3939
message: expect.stringContaining("You are about to drop the `test-db` database"),
40-
requestedSchema: {
41-
type: "object",
42-
properties: {
43-
confirmation: {
44-
type: "string",
45-
title: "Would you like to confirm?",
46-
description: "Would you like to confirm?",
47-
enum: ["Yes", "No"],
48-
enumNames: ["Yes, I confirm", "No, I do not confirm"],
49-
},
50-
},
51-
required: ["confirmation"],
52-
},
40+
requestedSchema: Elicitation.CONFIRMATION_SCHEMA,
5341
});
5442

5543
// Should attempt to execute (will fail due to no connection, but confirms flow worked)
@@ -154,7 +142,7 @@ describe("Elicitation Integration Tests", () => {
154142
});
155143
});
156144

157-
describe("tools not requiring confirmation", () => {
145+
describe("tools not requiring confirmation by default", () => {
158146
it("should not request confirmation for read operations", async () => {
159147
const result = await integration.mcpClient().callTool({
160148
name: "list-databases",
@@ -189,7 +177,7 @@ describe("Elicitation Integration Tests", () => {
189177
{ getClientCapabilities: () => ({}) }
190178
);
191179

192-
it("should proceed without confirmation for destructive tools when client lacks elicitation support", async () => {
180+
it("should proceed without confirmation for default confirmation-required tools when client lacks elicitation support", async () => {
193181
const result = await integration.mcpClient().callTool({
194182
name: "drop-database",
195183
arguments: { database: "test-db" },
@@ -217,7 +205,7 @@ describe("Elicitation Integration Tests", () => {
217205
{ elicitInput: mockElicitInput }
218206
);
219207

220-
it("should respect custom confirmationRequiredTools configuration", async () => {
208+
it("should confirm with a generic message with custom configurations for other tools", async () => {
221209
mockElicitInput.confirmYes();
222210

223211
await integration.mcpClient().callTool({
@@ -226,6 +214,12 @@ describe("Elicitation Integration Tests", () => {
226214
});
227215

228216
expect(mockElicitInput.mock).toHaveBeenCalledTimes(1);
217+
expect(mockElicitInput.mock).toHaveBeenCalledWith({
218+
message: expect.stringMatching(
219+
/You are about to execute the `list-databases` tool which requires additional confirmation. Would you like to proceed\?/
220+
),
221+
requestedSchema: expect.objectContaining(Elicitation.CONFIRMATION_SCHEMA),
222+
});
229223
});
230224

231225
it("should not request confirmation when tool is removed from default confirmationRequiredTools", async () => {

0 commit comments

Comments
 (0)