diff --git a/src/schema/search/input.ts b/src/schema/search/input.ts index db58a694..714c577c 100644 --- a/src/schema/search/input.ts +++ b/src/schema/search/input.ts @@ -1,4 +1,5 @@ import { z } from "zod"; +import { withoutAdvertisedRequired } from "../without-advertised-required.js"; const searchQuerySchema = z.object({ operator: z @@ -74,8 +75,8 @@ const searchHitTypeSchema = z.enum([ export const searchInputSchema = { query: z - .tuple([searchQuerySchema]) - .rest(searchQuerySchema) + .tuple([withoutAdvertisedRequired(searchQuerySchema)]) + .rest(withoutAdvertisedRequired(searchQuerySchema)) // Zod renders a tuple with a rest element as prefixItems + items, which // does not tell a client that one query is required, so state it here. .meta({ diff --git a/src/schema/without-advertised-required.ts b/src/schema/without-advertised-required.ts new file mode 100644 index 00000000..8147d0b8 --- /dev/null +++ b/src/schema/without-advertised-required.ts @@ -0,0 +1,9 @@ +import type { z } from "zod"; + +// Some MCP clients never send tools/call when a required array's items declare +// `required`, and keep asking the user for the item keys instead. +// https://github.com/mondaycom/mcp/issues/499 +// Tool schemas are advertised with io: "input", which renders the input side of +// the pipe; tools/call is still validated against the original schema. +export const withoutAdvertisedRequired = (schema: T) => + schema.partial().pipe(schema); diff --git a/src/server/__tests__/tool-definitions.test.ts b/src/server/__tests__/tool-definitions.test.ts index 1dd89bd3..c82f85f5 100644 --- a/src/server/__tests__/tool-definitions.test.ts +++ b/src/server/__tests__/tool-definitions.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from "vitest"; +import { z } from "zod"; import Ajv2020 from "ajv/dist/2020.js"; import { Client } from "@modelcontextprotocol/sdk/client/index.js"; import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; @@ -124,6 +125,74 @@ describe("tool schemas", () => { ).toEqual([]); }); + // See src/schema/without-advertised-required.ts. Only "items" and + // "prefixItems" themselves are checked here; "required" nested further down + // is left alone, because kintone-add-records advertises it and is not affected. + it("never declare required directly on the items of a required array", async () => { + const advertised = await listTools(); + + expect( + advertised.flatMap((tool) => { + const required: string[] = tool.inputSchema.required ?? []; + return Object.entries(tool.inputSchema.properties ?? {}) + .filter(([name]) => required.includes(name)) + .filter(([, property]) => { + const { items, prefixItems = [] } = property as { + items?: { required?: unknown }; + prefixItems?: Array<{ required?: unknown }>; + }; + return [items, ...prefixItems].some( + (item) => item?.required !== undefined, + ); + }) + .map(([name]) => `${tool.name}: ${name}`); + }), + ).toEqual([]); + }); + + const arrays = [ + ["kintone-update-records", "records", { app: "1", records: [{}] }], + ["kintone-update-statuses", "records", { app: "1", records: [{}] }], + ["kintone-deploy-app", "apps", { apps: [{}] }], + [ + "kintone-add-space-from-template", + "members", + { id: "1", name: "space", members: [{ isAdmin: true }] }, + ], + ["kintone-search", "query", { query: [{}] }], + ] as const; + + // Dropping the item-level "required" must not be done by dropping the array + // itself from "required", which would let a client omit the argument. + it("keep arrays whose items hide required still required", async () => { + const advertised = await listTools(); + + expect( + arrays.map(([toolName, property]) => { + const tool = advertised.find( + (candidate) => candidate.name === toolName, + ); + return [ + toolName, + tool?.inputSchema.required?.includes(property), + (tool?.inputSchema.properties?.[property] as { type?: string })?.type, + ]; + }), + ).toEqual(arrays.map(([toolName]) => [toolName, true, "array"])); + }); + + it("still reject items missing a field they require", () => { + expect( + arrays.map(([toolName, , args]) => { + const tool = tools.find((candidate) => candidate.name === toolName); + return [ + toolName, + z.object(tool?.config.inputSchema ?? {}).safeParse(args).success, + ]; + }), + ).toEqual(arrays.map(([toolName]) => [toolName, false])); + }); + it("compile as JSON Schema 2020-12", async () => { const advertised = await listTools(); diff --git a/src/tools/kintone/app/deploy-app.ts b/src/tools/kintone/app/deploy-app.ts index 70b27140..363456d4 100644 --- a/src/tools/kintone/app/deploy-app.ts +++ b/src/tools/kintone/app/deploy-app.ts @@ -1,23 +1,25 @@ import { z } from "zod"; import { createTool } from "../../factory.js"; +import { withoutAdvertisedRequired } from "../../../schema/without-advertised-required.js"; import type { KintoneToolCallback } from "../../types/tool.js"; +const deployAppSchema = z.object({ + app: z + .string() + .describe( + "Required. The ID of the app to deploy (numeric value as string)", + ), + revision: z.string().optional().describe("The expected revision number"), +}); + const inputSchema = { apps: z - .array( - z.object({ - app: z - .string() - .describe("The ID of the app to deploy (numeric value as string)"), - revision: z - .string() - .optional() - .describe("The expected revision number"), - }), - ) + .array(withoutAdvertisedRequired(deployAppSchema)) .min(1) .max(300) - .describe("List of apps to deploy (minimum 1, maximum 300 apps)"), + .describe( + "List of apps to deploy (minimum 1, maximum 300 apps). Each entry must have app (which app to deploy).", + ), revert: z .boolean() .optional() diff --git a/src/tools/kintone/record/update-records.ts b/src/tools/kintone/record/update-records.ts index 74b63295..0084d3d3 100644 --- a/src/tools/kintone/record/update-records.ts +++ b/src/tools/kintone/record/update-records.ts @@ -1,13 +1,16 @@ import { z } from "zod"; import { createTool } from "../../factory.js"; import { recordSchemaForParameter } from "../../../schema/record/index.js"; +import { withoutAdvertisedRequired } from "../../../schema/without-advertised-required.js"; import type { KintoneToolCallback } from "../../types/tool.js"; const updateRecordSchema = z.object({ // updateKey指定は対象外 - id: z.string().describe("Record ID to update (numeric value as string)"), + id: z + .string() + .describe("Required. Record ID to update (numeric value as string)"), record: recordSchemaForParameter.describe( - "Record data with field codes as keys. Use kintone-get-form-fields tool first to discover available field codes and their types.", + "Required. Record data with field codes as keys. Use kintone-get-form-fields tool first to discover available field codes and their types.", ), revision: z .string() @@ -24,11 +27,11 @@ const inputSchema = { "The ID of the app to update records in (numeric value as string)", ), records: z - .array(updateRecordSchema) + .array(withoutAdvertisedRequired(updateRecordSchema)) .min(1) .max(100) .describe( - "Array of records to update (min 1, max 100). Each record must have an ID to identify which record to update.", + "Array of records to update (min 1, max 100). Each record must have both id (which record to update) and record (the field values to set).", ), }; diff --git a/src/tools/kintone/record/update-statuses.ts b/src/tools/kintone/record/update-statuses.ts index ada41e5a..c78ec978 100644 --- a/src/tools/kintone/record/update-statuses.ts +++ b/src/tools/kintone/record/update-statuses.ts @@ -1,13 +1,14 @@ import { z } from "zod"; import { createTool } from "../../factory.js"; +import { withoutAdvertisedRequired } from "../../../schema/without-advertised-required.js"; import type { KintoneToolCallback } from "../../types/tool.js"; const statusRecordSchema = z.object({ - id: z.string().describe("Record ID (numeric value as string)"), + id: z.string().describe("Required. Record ID (numeric value as string)"), action: z .string() .describe( - "Action name to execute. Must be specified in the user's display language if multiple languages are configured. If multiple actions with the same name exist for the current status, an error will occur.", + "Required. Action name to execute. Must be specified in the user's display language if multiple languages are configured. If multiple actions with the same name exist for the current status, an error will occur.", ), assignee: z .string() @@ -26,11 +27,11 @@ const statusRecordSchema = z.object({ const inputSchema = { app: z.string().describe("The ID of the app (numeric value as string)"), records: z - .array(statusRecordSchema) + .array(withoutAdvertisedRequired(statusRecordSchema)) .min(1) .max(100) .describe( - "Array of records to update status (min 1, max 100). Each record contains id, action, and optionally assignee and revision.", + "Array of records to update status (min 1, max 100). Each record must have both id (which record to update) and action (the action to execute), and may have assignee and revision.", ), }; diff --git a/src/tools/kintone/space/add-space-from-template.ts b/src/tools/kintone/space/add-space-from-template.ts index 60f7547d..5869c4db 100644 --- a/src/tools/kintone/space/add-space-from-template.ts +++ b/src/tools/kintone/space/add-space-from-template.ts @@ -1,7 +1,41 @@ import { z } from "zod"; import { createTool } from "../../factory.js"; +import { withoutAdvertisedRequired } from "../../../schema/without-advertised-required.js"; import type { KintoneToolCallback } from "../../types/tool.js"; +const spaceMemberSchema = z.object({ + entity: z + .object({ + type: z + .enum(["USER", "GROUP", "ORGANIZATION"]) + .describe( + "Entity type: USER (individual user), GROUP (role/group), or ORGANIZATION (department).", + ), + code: z + .string() + .describe( + "Entity code: login name for USER, group code for GROUP, organization code for ORGANIZATION.", + ), + }) + .describe( + "Required. The entity (user, group, or organization) granted membership.", + ), + isAdmin: z + .boolean() + .optional() + .default(false) + .describe( + "Whether this member is a space administrator. At least one member in the array must have isAdmin set to true (default: false).", + ), + includeSubs: z + .boolean() + .optional() + .default(false) + .describe( + "For ORGANIZATION entity, also include affiliated departments. Ignored for USER and GROUP entities (default: false).", + ), +}); + const inputSchema = { id: z .string() @@ -10,43 +44,10 @@ const inputSchema = { ), name: z.string().min(1).describe("The name of the new space."), members: z - .array( - z.object({ - entity: z - .object({ - type: z - .enum(["USER", "GROUP", "ORGANIZATION"]) - .describe( - "Entity type: USER (individual user), GROUP (role/group), or ORGANIZATION (department).", - ), - code: z - .string() - .describe( - "Entity code: login name for USER, group code for GROUP, organization code for ORGANIZATION.", - ), - }) - .describe( - "The entity (user, group, or organization) granted membership.", - ), - isAdmin: z - .boolean() - .optional() - .default(false) - .describe( - "Whether this member is a space administrator. At least one member in the array must have isAdmin set to true (default: false).", - ), - includeSubs: z - .boolean() - .optional() - .default(false) - .describe( - "For ORGANIZATION entity, also include affiliated departments. Ignored for USER and GROUP entities (default: false).", - ), - }), - ) + .array(withoutAdvertisedRequired(spaceMemberSchema)) .min(1) .describe( - "Array of space members. Must contain at least one entry and at least one of them must have isAdmin: true.", + "Array of space members. Each member must have entity (who is granted membership). Must contain at least one entry and at least one of them must have isAdmin: true.", ) .refine((members) => members.some((m) => m.isAdmin), { message: "At least one member must have isAdmin: true",