From 74a196b1d2fa1a4a1de6ee211aeb3d66ba77c66d Mon Sep 17 00:00:00 2001 From: shabaraba Date: Wed, 7 Oct 2026 11:33:41 +0900 Subject: [PATCH 01/10] fix: let clients call tools whose required array items declare required Some MCP clients cannot build the arguments for an array that is required at the top level when its items schema declares required of its own. They never send tools/call and keep asking the user for the item keys instead. Declare those item fields optional in the input schema of the four affected tools and move the guarantee that they are required to a shared runtime check, which reports every missing field before the kintone API is called. --- src/server/__tests__/tool-definitions.test.ts | 59 ++++++++++++++++++ src/tools/__tests__/validation.test.ts | 61 +++++++++++++++++++ .../kintone/app/__tests__/deploy-app.test.ts | 14 +++++ src/tools/kintone/app/deploy-app.ts | 18 +++++- .../record/__tests__/update-records.test.ts | 53 ++++++++++------ .../record/__tests__/update-statuses.test.ts | 39 +++++++++--- src/tools/kintone/record/update-records.ts | 21 +++++-- src/tools/kintone/record/update-statuses.ts | 16 +++-- .../__tests__/add-space-from-template.test.ts | 21 +++++++ .../kintone/space/add-space-from-template.ts | 11 +++- src/tools/validation.ts | 60 ++++++++++++++++++ 11 files changed, 330 insertions(+), 43 deletions(-) create mode 100644 src/tools/__tests__/validation.test.ts create mode 100644 src/tools/validation.ts diff --git a/src/server/__tests__/tool-definitions.test.ts b/src/server/__tests__/tool-definitions.test.ts index 1dd89bd3..0ca6eeb1 100644 --- a/src/server/__tests__/tool-definitions.test.ts +++ b/src/server/__tests__/tool-definitions.test.ts @@ -124,6 +124,65 @@ describe("tool schemas", () => { ).toEqual([]); }); + // Some MCP clients cannot build the arguments for an array that is required + // at the top level when its "items" schema declares "required" of its own: + // they never send tools/call and keep asking the user for the item keys. The + // affected tools leave those item fields optional and check them in the + // callback instead (src/tools/validation.ts). + // https://github.com/mondaycom/mcp/issues/499 + // + // Only the "items" schema itself is checked. "required" further down (inside + // properties, anyOf or additionalProperties) 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(); + + // "kintone-search: query" is a tuple with a rest element, so it also + // advertises prefixItems. Whether the same clients stumble on that shape + // has not been confirmed, so it is left alone rather than reshaped blind. + const known = ["kintone-search: query"]; + + expect( + advertised.flatMap((tool) => { + const required: string[] = tool.inputSchema.required ?? []; + return Object.entries(tool.inputSchema.properties ?? {}) + .filter( + ([name, property]) => + required.includes(name) && + (property as { items?: { required?: unknown } }).items + ?.required !== undefined, + ) + .map(([name]) => `${tool.name}: ${name}`); + }), + ).toEqual(known); + }); + + // 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 the callback validates required", async () => { + const advertised = await listTools(); + + const arrays = [ + ["kintone-update-records", "records"], + ["kintone-update-statuses", "records"], + ["kintone-deploy-app", "apps"], + ["kintone-add-space-from-template", "members"], + ]; + + 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("compile as JSON Schema 2020-12", async () => { const advertised = await listTools(); diff --git a/src/tools/__tests__/validation.test.ts b/src/tools/__tests__/validation.test.ts new file mode 100644 index 00000000..4b3e258f --- /dev/null +++ b/src/tools/__tests__/validation.test.ts @@ -0,0 +1,61 @@ +import { describe, expect, expectTypeOf, it } from "vitest"; +import { + MissingRequiredFieldsError, + requireItemFields, +} from "../validation.js"; + +describe("requireItemFields", () => { + it("returns the items unchanged when every key is present", () => { + const items = [ + { id: "1", record: {} }, + { id: "2", record: {} }, + ]; + + expect(requireItemFields(items, "records", ["id", "record"])).toBe(items); + }); + + it("reports every missing field across every item at once", () => { + const items = [{ id: "1", record: {} }, { record: {} }, { id: "3" }]; + + try { + requireItemFields(items, "records", ["id", "record"]); + expect.unreachable("should have thrown"); + } catch (error) { + expect(error).toBeInstanceOf(MissingRequiredFieldsError); + expect((error as MissingRequiredFieldsError).paths).toEqual([ + "records[1].id", + "records[2].record", + ]); + expect((error as Error).message).toContain( + "Missing required field(s): records[1].id, records[2].record", + ); + } + }); + + it("treats an explicit undefined as missing but keeps other falsy values", () => { + const items = [{ app: "", revision: undefined }]; + + expect(requireItemFields(items, "apps", ["app"])).toBe(items); + expect(() => requireItemFields(items, "apps", ["revision"])).toThrow( + "apps[0].revision", + ); + }); + + it("accepts an empty array", () => { + expect(requireItemFields([], "records", ["id"])).toEqual([]); + }); + + // The return goes through a type assertion, so tsc is what checks that the + // narrowing matches the keys that were asserted. + it("narrows only the asserted keys to required", () => { + const items: Array<{ id?: string; record?: object; revision?: string }> = [ + { id: "1", record: {} }, + ]; + + const narrowed = requireItemFields(items, "records", ["id", "record"]); + + expectTypeOf(narrowed[0].id).toEqualTypeOf(); + expectTypeOf(narrowed[0].record).toEqualTypeOf(); + expectTypeOf(narrowed[0].revision).toEqualTypeOf(); + }); +}); diff --git a/src/tools/kintone/app/__tests__/deploy-app.test.ts b/src/tools/kintone/app/__tests__/deploy-app.test.ts index e9fd022f..21ac8b8c 100644 --- a/src/tools/kintone/app/__tests__/deploy-app.test.ts +++ b/src/tools/kintone/app/__tests__/deploy-app.test.ts @@ -123,6 +123,20 @@ describe("deploy-app tool", () => { }); describe("callback function", () => { + it("should reject apps missing app without calling the API", async () => { + const mockClient = createMockClient(); + mockClient.app.deployApp = mockDeployApp; + + await expect( + deployApp.callback( + { apps: [{ revision: "3" }] }, + { client: mockClient }, + ), + ).rejects.toThrow("apps[0].app"); + + expect(mockDeployApp).not.toHaveBeenCalled(); + }); + it("should call API and return formatted response", async () => { const expectedResult = { message: diff --git a/src/tools/kintone/app/deploy-app.ts b/src/tools/kintone/app/deploy-app.ts index 70b27140..0fe51400 100644 --- a/src/tools/kintone/app/deploy-app.ts +++ b/src/tools/kintone/app/deploy-app.ts @@ -1,14 +1,21 @@ import { z } from "zod"; import { createTool } from "../../factory.js"; +import { requireItemFields } from "../../validation.js"; import type { KintoneToolCallback } from "../../types/tool.js"; const inputSchema = { apps: z .array( z.object({ + // `app` is required, but the schema declares it optional. See + // `requireItemFields` (src/tools/validation.ts) for why, and for where + // the guarantee lives instead. app: z .string() - .describe("The ID of the app to deploy (numeric value as string)"), + .optional() + .describe( + "Required. The ID of the app to deploy (numeric value as string)", + ), revision: z .string() .optional() @@ -17,7 +24,9 @@ const inputSchema = { ) .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() @@ -42,7 +51,10 @@ const callback: KintoneToolCallback = async ( { apps, revert }, { client }, ) => { - await client.app.deployApp({ apps, revert }); + await client.app.deployApp({ + apps: requireItemFields(apps, "apps", ["app"]), + revert, + }); const result = { message: `Deployment initiated for ${apps.length} app(s). Use kintone-get-app-deploy-status tool to check progress.`, diff --git a/src/tools/kintone/record/__tests__/update-records.test.ts b/src/tools/kintone/record/__tests__/update-records.test.ts index dc515c89..37eca957 100644 --- a/src/tools/kintone/record/__tests__/update-records.test.ts +++ b/src/tools/kintone/record/__tests__/update-records.test.ts @@ -251,17 +251,6 @@ describe("update-records tool", () => { }, description: "too many records (over 100)", }, - { - input: { - app: "123", - records: [ - { - record: { title: { value: "test" } }, - }, - ], - }, - description: "missing required id field", - }, { input: { app: "123", @@ -274,19 +263,28 @@ describe("update-records tool", () => { }, description: "id as number instead of string", }, + ])("rejects $description", ({ input }) => { + expect(() => inputSchema.parse(input)).toThrow(); + }); + }); + + // `id` and `record` are required, but the schema declares them optional + // (src/tools/validation.ts). The callback rejects a missing one instead. + describe("input schema validation with item fields left to the callback", () => { + it.each([ { input: { app: "123", - records: [ - { - id: "1", - }, - ], + records: [{ record: { title: { value: "test" } } }], }, - description: "missing required record field", + description: "record without id", }, - ])("rejects $description", ({ input }) => { - expect(() => inputSchema.parse(input)).toThrow(); + { + input: { app: "123", records: [{ id: "1" }] }, + description: "record without record", + }, + ])("accepts $description", ({ input }) => { + expect(() => inputSchema.parse(input)).not.toThrow(); }); }); @@ -605,6 +603,23 @@ describe("update-records tool", () => { }); }); + it("should reject records missing id or record without calling the API", async () => { + const mockClient = createMockClient(); + mockClient.record.updateRecords = mockUpdateRecords; + + await expect( + updateRecords.callback( + { + app: "123", + records: [{ record: { title: { value: "test" } } }, { id: "2" }], + }, + mockToolCallbackOptions(mockClient), + ), + ).rejects.toThrow("records[0].id, records[1].record"); + + expect(mockUpdateRecords).not.toHaveBeenCalled(); + }); + it("should handle API errors properly", async () => { const mockError = new Error("API Error: Record not found"); mockUpdateRecords.mockRejectedValueOnce(mockError); diff --git a/src/tools/kintone/record/__tests__/update-statuses.test.ts b/src/tools/kintone/record/__tests__/update-statuses.test.ts index 8a361c14..7411a21b 100644 --- a/src/tools/kintone/record/__tests__/update-statuses.test.ts +++ b/src/tools/kintone/record/__tests__/update-statuses.test.ts @@ -143,14 +143,6 @@ describe("update-statuses tool", () => { }, description: "more than 100 records", }, - { - input: { app: "123", records: [{ action: "申請する" }] }, - description: "record missing id field", - }, - { - input: { app: "123", records: [{ id: "1" }] }, - description: "record missing action field", - }, { input: { app: "123", @@ -170,6 +162,23 @@ describe("update-statuses tool", () => { }); }); + // `id` and `action` are required, but the schema declares them optional + // (src/tools/validation.ts). The callback rejects a missing one instead. + describe("input schema validation with item fields left to the callback", () => { + it.each([ + { + input: { app: "123", records: [{ action: "申請する" }] }, + description: "record without id", + }, + { + input: { app: "123", records: [{ id: "1" }] }, + description: "record without action", + }, + ])("accepts $description", ({ input }) => { + expect(() => inputSchema.parse(input)).not.toThrow(); + }); + }); + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const outputSchema = z.object(updateStatuses.config.outputSchema!); @@ -261,6 +270,20 @@ describe("update-statuses tool", () => { }); describe("callback function", () => { + it("should reject records missing id or action without calling the API", async () => { + const mockClient = createMockClient(); + mockClient.record.updateRecordsStatus = mockUpdateRecordsStatus; + + await expect( + updateStatuses.callback( + { app: "123", records: [{ assignee: "user1" }] }, + mockToolCallbackOptions(mockClient), + ), + ).rejects.toThrow("records[0].id, records[0].action"); + + expect(mockUpdateRecordsStatus).not.toHaveBeenCalled(); + }); + it("should call API and return formatted response for single record", async () => { const mockData = { records: [ diff --git a/src/tools/kintone/record/update-records.ts b/src/tools/kintone/record/update-records.ts index 74b63295..88eb94f9 100644 --- a/src/tools/kintone/record/update-records.ts +++ b/src/tools/kintone/record/update-records.ts @@ -1,14 +1,23 @@ import { z } from "zod"; import { createTool } from "../../factory.js"; import { recordSchemaForParameter } from "../../../schema/record/index.js"; +import { requireItemFields } from "../../validation.js"; import type { KintoneToolCallback } from "../../types/tool.js"; +// `id` and `record` are required, but the schema declares them optional. See +// `requireItemFields` (src/tools/validation.ts) for why, and for where the +// guarantee lives instead. const updateRecordSchema = z.object({ // updateKey指定は対象外 - id: z.string().describe("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.", - ), + id: z + .string() + .optional() + .describe("Required. Record ID to update (numeric value as string)"), + record: recordSchemaForParameter + .optional() + .describe( + "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() .optional() @@ -28,7 +37,7 @@ const inputSchema = { .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).", ), }; @@ -58,7 +67,7 @@ const callback: KintoneToolCallback = async ( ) => { const response = await client.record.updateRecords({ app, - records, + records: requireItemFields(records, "records", ["id", "record"]), upsert: false, // upsertモードは対象外 }); diff --git a/src/tools/kintone/record/update-statuses.ts b/src/tools/kintone/record/update-statuses.ts index ada41e5a..a90d38f3 100644 --- a/src/tools/kintone/record/update-statuses.ts +++ b/src/tools/kintone/record/update-statuses.ts @@ -1,13 +1,21 @@ import { z } from "zod"; import { createTool } from "../../factory.js"; +import { requireItemFields } from "../../validation.js"; import type { KintoneToolCallback } from "../../types/tool.js"; +// `id` and `action` are required, but the schema declares them optional. See +// `requireItemFields` (src/tools/validation.ts) for why, and for where the +// guarantee lives instead. const statusRecordSchema = z.object({ - id: z.string().describe("Record ID (numeric value as string)"), + id: z + .string() + .optional() + .describe("Required. Record ID (numeric value as string)"), action: z .string() + .optional() .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() @@ -30,7 +38,7 @@ const inputSchema = { .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.", ), }; @@ -64,7 +72,7 @@ const callback: KintoneToolCallback = async ( ) => { const response = await client.record.updateRecordsStatus({ app, - records, + records: requireItemFields(records, "records", ["id", "action"]), }); const result = { diff --git a/src/tools/kintone/space/__tests__/add-space-from-template.test.ts b/src/tools/kintone/space/__tests__/add-space-from-template.test.ts index d895b7b0..5ca71348 100644 --- a/src/tools/kintone/space/__tests__/add-space-from-template.test.ts +++ b/src/tools/kintone/space/__tests__/add-space-from-template.test.ts @@ -107,6 +107,27 @@ describe("add-space-from-template tool", () => { }); describe("callback function", () => { + it("should reject members missing entity without calling the API", async () => { + const mockClient = createMockClient(); + mockClient.space.addSpaceFromTemplate = mockAddSpaceFromTemplate; + + await expect( + addSpaceFromTemplate.callback( + { + id: "1", + name: "My Space", + members: [{ isAdmin: true, includeSubs: false }], + isPrivate: false, + isGuest: false, + fixedMember: false, + }, + { client: mockClient }, + ), + ).rejects.toThrow("members[0].entity"); + + expect(mockAddSpaceFromTemplate).not.toHaveBeenCalled(); + }); + it("should call addSpaceFromTemplate with the parsed input and return structured content", async () => { mockAddSpaceFromTemplate.mockResolvedValueOnce({ id: "200" }); diff --git a/src/tools/kintone/space/add-space-from-template.ts b/src/tools/kintone/space/add-space-from-template.ts index 60f7547d..9719ccef 100644 --- a/src/tools/kintone/space/add-space-from-template.ts +++ b/src/tools/kintone/space/add-space-from-template.ts @@ -1,5 +1,6 @@ import { z } from "zod"; import { createTool } from "../../factory.js"; +import { requireItemFields } from "../../validation.js"; import type { KintoneToolCallback } from "../../types/tool.js"; const inputSchema = { @@ -12,6 +13,9 @@ const inputSchema = { members: z .array( z.object({ + // `entity` is required, but the schema declares it optional. See + // `requireItemFields` (src/tools/validation.ts) for why, and for where + // the guarantee lives instead. entity: z .object({ type: z @@ -25,8 +29,9 @@ const inputSchema = { "Entity code: login name for USER, group code for GROUP, organization code for ORGANIZATION.", ), }) + .optional() .describe( - "The entity (user, group, or organization) granted membership.", + "Required. The entity (user, group, or organization) granted membership.", ), isAdmin: z .boolean() @@ -46,7 +51,7 @@ const inputSchema = { ) .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", @@ -101,7 +106,7 @@ const callback: KintoneToolCallback = async ( const response = await client.space.addSpaceFromTemplate({ id, name, - members, + members: requireItemFields(members, "members", ["entity"]), isPrivate, isGuest, fixedMember, diff --git a/src/tools/validation.ts b/src/tools/validation.ts new file mode 100644 index 00000000..110d9f29 --- /dev/null +++ b/src/tools/validation.ts @@ -0,0 +1,60 @@ +// Some MCP clients cannot build arguments for an array that is required at the +// top level when its `items` schema declares `required` of its own. They never +// send `tools/call` and keep asking the user for the item keys instead. The same +// symptom is reported in https://github.com/mondaycom/mcp/issues/499. +// +// The affected tools therefore declare those item fields optional in their input +// schema, and `requireItemFields` holds the guarantee that they are required: a +// missing field is rejected before the kintone API is called, and the message +// names the item and the key that is missing. + +/** Error thrown when an array item is missing a field its tool requires. */ +export class MissingRequiredFieldsError extends Error { + /** Paths of the missing fields, e.g. `records[0].id`. */ + readonly paths: readonly string[]; + + constructor(paths: readonly string[]) { + super( + `Missing required field(s): ${paths.join(", ")}. ` + + "These fields are declared optional in the input schema for client compatibility, " + + "but they are required by this tool.", + ); + this.name = "MissingRequiredFieldsError"; + this.paths = paths; + } +} + +export type WithRequired = T & { + [P in K]-?: Exclude; +}; + +/** + * Assert that every item of `items` carries all of `keys`. + * + * Every missing field is reported at once, so the caller does not have to fix + * them one round trip at a time. + * + * @param items The array to check + * @param arrayName Argument name of the array, used in the error message (e.g. `records`) + * @param keys The keys to treat as required + * @returns The same array, with `keys` narrowed to required + */ +export const requireItemFields = ( + items: readonly T[], + arrayName: string, + keys: readonly K[], +): Array> => { + const missing = items.flatMap((item, index) => + keys + .filter((key) => item[key] === undefined) + .map((key) => `${arrayName}[${index}].${key}`), + ); + + if (missing.length > 0) { + throw new MissingRequiredFieldsError(missing); + } + + // The check above rules out `undefined` for every key, which the type checker + // cannot follow back to the element type. A type test pins the result. + return items as Array>; +}; From f8f95383ff6cdeb43e39a7b0216591e3bba8a76e Mon Sep 17 00:00:00 2001 From: shabaraba Date: Wed, 7 Oct 2026 14:15:09 +0900 Subject: [PATCH 02/10] refactor: refer to the single source instead of restating it in tests --- src/server/__tests__/tool-definitions.test.ts | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/server/__tests__/tool-definitions.test.ts b/src/server/__tests__/tool-definitions.test.ts index 0ca6eeb1..c097eb06 100644 --- a/src/server/__tests__/tool-definitions.test.ts +++ b/src/server/__tests__/tool-definitions.test.ts @@ -124,15 +124,9 @@ describe("tool schemas", () => { ).toEqual([]); }); - // Some MCP clients cannot build the arguments for an array that is required - // at the top level when its "items" schema declares "required" of its own: - // they never send tools/call and keep asking the user for the item keys. The - // affected tools leave those item fields optional and check them in the - // callback instead (src/tools/validation.ts). - // https://github.com/mondaycom/mcp/issues/499 - // - // Only the "items" schema itself is checked. "required" further down (inside - // properties, anyOf or additionalProperties) is left alone, because + // The affected tools leave those item fields optional and check them in the + // callback instead (src/tools/validation.ts). Only the "items" schema itself + // is 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(); From db9edd2e6b1d0e81e4e336d7944a65b707f8d95b Mon Sep 17 00:00:00 2001 From: shabaraba Date: Wed, 7 Oct 2026 14:15:54 +0900 Subject: [PATCH 03/10] refactor: keep line comments within two lines --- src/server/__tests__/tool-definitions.test.ts | 5 ++--- src/tools/kintone/app/deploy-app.ts | 5 ++--- src/tools/kintone/record/update-records.ts | 5 ++--- src/tools/kintone/record/update-statuses.ts | 5 ++--- .../kintone/space/add-space-from-template.ts | 5 ++--- src/tools/validation.ts | 15 ++++++--------- 6 files changed, 16 insertions(+), 24 deletions(-) diff --git a/src/server/__tests__/tool-definitions.test.ts b/src/server/__tests__/tool-definitions.test.ts index c097eb06..f6d4e110 100644 --- a/src/server/__tests__/tool-definitions.test.ts +++ b/src/server/__tests__/tool-definitions.test.ts @@ -131,9 +131,8 @@ describe("tool schemas", () => { it("never declare required directly on the items of a required array", async () => { const advertised = await listTools(); - // "kintone-search: query" is a tuple with a rest element, so it also - // advertises prefixItems. Whether the same clients stumble on that shape - // has not been confirmed, so it is left alone rather than reshaped blind. + // "kintone-search: query" is a tuple with a rest element, so it advertises + // prefixItems too. Whether the same clients stumble on that shape is unconfirmed. const known = ["kintone-search: query"]; expect( diff --git a/src/tools/kintone/app/deploy-app.ts b/src/tools/kintone/app/deploy-app.ts index 0fe51400..ccf469a6 100644 --- a/src/tools/kintone/app/deploy-app.ts +++ b/src/tools/kintone/app/deploy-app.ts @@ -7,9 +7,8 @@ const inputSchema = { apps: z .array( z.object({ - // `app` is required, but the schema declares it optional. See - // `requireItemFields` (src/tools/validation.ts) for why, and for where - // the guarantee lives instead. + // `app` is required; the schema declares it optional and + // `requireItemFields` (src/tools/validation.ts) holds the guarantee. app: z .string() .optional() diff --git a/src/tools/kintone/record/update-records.ts b/src/tools/kintone/record/update-records.ts index 88eb94f9..41e924d3 100644 --- a/src/tools/kintone/record/update-records.ts +++ b/src/tools/kintone/record/update-records.ts @@ -4,9 +4,8 @@ import { recordSchemaForParameter } from "../../../schema/record/index.js"; import { requireItemFields } from "../../validation.js"; import type { KintoneToolCallback } from "../../types/tool.js"; -// `id` and `record` are required, but the schema declares them optional. See -// `requireItemFields` (src/tools/validation.ts) for why, and for where the -// guarantee lives instead. +// `id` and `record` are required; the schema declares them optional and +// `requireItemFields` (src/tools/validation.ts) holds the guarantee. const updateRecordSchema = z.object({ // updateKey指定は対象外 id: z diff --git a/src/tools/kintone/record/update-statuses.ts b/src/tools/kintone/record/update-statuses.ts index a90d38f3..461445f9 100644 --- a/src/tools/kintone/record/update-statuses.ts +++ b/src/tools/kintone/record/update-statuses.ts @@ -3,9 +3,8 @@ import { createTool } from "../../factory.js"; import { requireItemFields } from "../../validation.js"; import type { KintoneToolCallback } from "../../types/tool.js"; -// `id` and `action` are required, but the schema declares them optional. See -// `requireItemFields` (src/tools/validation.ts) for why, and for where the -// guarantee lives instead. +// `id` and `action` are required; the schema declares them optional and +// `requireItemFields` (src/tools/validation.ts) holds the guarantee. const statusRecordSchema = z.object({ id: z .string() diff --git a/src/tools/kintone/space/add-space-from-template.ts b/src/tools/kintone/space/add-space-from-template.ts index 9719ccef..7fc005e6 100644 --- a/src/tools/kintone/space/add-space-from-template.ts +++ b/src/tools/kintone/space/add-space-from-template.ts @@ -13,9 +13,8 @@ const inputSchema = { members: z .array( z.object({ - // `entity` is required, but the schema declares it optional. See - // `requireItemFields` (src/tools/validation.ts) for why, and for where - // the guarantee lives instead. + // `entity` is required; the schema declares it optional and + // `requireItemFields` (src/tools/validation.ts) holds the guarantee. entity: z .object({ type: z diff --git a/src/tools/validation.ts b/src/tools/validation.ts index 110d9f29..c1a171b1 100644 --- a/src/tools/validation.ts +++ b/src/tools/validation.ts @@ -1,12 +1,9 @@ // Some MCP clients cannot build arguments for an array that is required at the -// top level when its `items` schema declares `required` of its own. They never -// send `tools/call` and keep asking the user for the item keys instead. The same -// symptom is reported in https://github.com/mondaycom/mcp/issues/499. -// -// The affected tools therefore declare those item fields optional in their input -// schema, and `requireItemFields` holds the guarantee that they are required: a -// missing field is rejected before the kintone API is called, and the message -// names the item and the key that is missing. +// top level when its `items` schema declares `required` of its own: they never +// send `tools/call` and keep asking the user for the item keys instead. The +// affected tools therefore declare those item fields optional in their input +// schema, and the guarantee that they are required lives here. +// https://github.com/mondaycom/mcp/issues/499 /** Error thrown when an array item is missing a field its tool requires. */ export class MissingRequiredFieldsError extends Error { @@ -55,6 +52,6 @@ export const requireItemFields = ( } // The check above rules out `undefined` for every key, which the type checker - // cannot follow back to the element type. A type test pins the result. + // cannot follow back to the element type. return items as Array>; }; From 1d8545b8067cf34627a3128f923c282ed88b9fb8 Mon Sep 17 00:00:00 2001 From: shabaraba Date: Wed, 7 Oct 2026 14:16:06 +0900 Subject: [PATCH 04/10] refactor: drop comments that restate the test name --- src/tools/__tests__/validation.test.ts | 3 +-- src/tools/kintone/record/__tests__/update-records.test.ts | 3 +-- src/tools/kintone/record/__tests__/update-statuses.test.ts | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/tools/__tests__/validation.test.ts b/src/tools/__tests__/validation.test.ts index 4b3e258f..f1162d18 100644 --- a/src/tools/__tests__/validation.test.ts +++ b/src/tools/__tests__/validation.test.ts @@ -45,8 +45,7 @@ describe("requireItemFields", () => { expect(requireItemFields([], "records", ["id"])).toEqual([]); }); - // The return goes through a type assertion, so tsc is what checks that the - // narrowing matches the keys that were asserted. + // The return goes through a type assertion, so tsc is what checks this. it("narrows only the asserted keys to required", () => { const items: Array<{ id?: string; record?: object; revision?: string }> = [ { id: "1", record: {} }, diff --git a/src/tools/kintone/record/__tests__/update-records.test.ts b/src/tools/kintone/record/__tests__/update-records.test.ts index 37eca957..34e16a63 100644 --- a/src/tools/kintone/record/__tests__/update-records.test.ts +++ b/src/tools/kintone/record/__tests__/update-records.test.ts @@ -268,8 +268,7 @@ describe("update-records tool", () => { }); }); - // `id` and `record` are required, but the schema declares them optional - // (src/tools/validation.ts). The callback rejects a missing one instead. + // The callback is what rejects a missing one (src/tools/validation.ts). describe("input schema validation with item fields left to the callback", () => { it.each([ { diff --git a/src/tools/kintone/record/__tests__/update-statuses.test.ts b/src/tools/kintone/record/__tests__/update-statuses.test.ts index 7411a21b..5378f2d5 100644 --- a/src/tools/kintone/record/__tests__/update-statuses.test.ts +++ b/src/tools/kintone/record/__tests__/update-statuses.test.ts @@ -162,8 +162,7 @@ describe("update-statuses tool", () => { }); }); - // `id` and `action` are required, but the schema declares them optional - // (src/tools/validation.ts). The callback rejects a missing one instead. + // The callback is what rejects a missing one (src/tools/validation.ts). describe("input schema validation with item fields left to the callback", () => { it.each([ { From 38a0bc442579cd7c73c1a01428c76a56fab297c3 Mon Sep 17 00:00:00 2001 From: shabaraba Date: Wed, 7 Oct 2026 14:16:14 +0900 Subject: [PATCH 05/10] refactor: drop argument names from the jsdoc summary --- src/tools/validation.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/validation.ts b/src/tools/validation.ts index c1a171b1..6e1bff30 100644 --- a/src/tools/validation.ts +++ b/src/tools/validation.ts @@ -26,7 +26,7 @@ export type WithRequired = T & { }; /** - * Assert that every item of `items` carries all of `keys`. + * Assert that every item of an array carries the fields its tool requires. * * Every missing field is reported at once, so the caller does not have to fix * them one round trip at a time. From 26c6e3939012068476768b42b5f744ebae222f40 Mon Sep 17 00:00:00 2001 From: shabaraba Date: Wed, 7 Oct 2026 14:16:51 +0900 Subject: [PATCH 06/10] refactor: apply comment-density audit fixes across the tool schemas --- src/server/__tests__/tool-definitions.test.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/server/__tests__/tool-definitions.test.ts b/src/server/__tests__/tool-definitions.test.ts index f6d4e110..a85c5605 100644 --- a/src/server/__tests__/tool-definitions.test.ts +++ b/src/server/__tests__/tool-definitions.test.ts @@ -124,15 +124,14 @@ describe("tool schemas", () => { ).toEqual([]); }); - // The affected tools leave those item fields optional and check them in the - // callback instead (src/tools/validation.ts). Only the "items" schema itself - // is checked here: "required" nested further down is left alone, because - // kintone-add-records advertises it and is not affected. + // The item fields are checked in the callback (src/tools/validation.ts). + // Only "items" itself is 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(); - // "kintone-search: query" is a tuple with a rest element, so it advertises - // prefixItems too. Whether the same clients stumble on that shape is unconfirmed. + // "kintone-search: query" is a tuple with a rest element, so it also + // advertises prefixItems. Whether that shape trips clients is unconfirmed. const known = ["kintone-search: query"]; expect( From 8cae34699c83b115b4d0462718c5b57cb436d894 Mon Sep 17 00:00:00 2001 From: shabaraba Date: Wed, 7 Oct 2026 14:57:54 +0900 Subject: [PATCH 07/10] refactor: stop exporting the internal WithRequired type --- src/tools/validation.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/validation.ts b/src/tools/validation.ts index 6e1bff30..813096f4 100644 --- a/src/tools/validation.ts +++ b/src/tools/validation.ts @@ -21,7 +21,7 @@ export class MissingRequiredFieldsError extends Error { } } -export type WithRequired = T & { +type WithRequired = T & { [P in K]-?: Exclude; }; From 7f75e6157ecdf908bd5bb609fb346b8eba093d9d Mon Sep 17 00:00:00 2001 From: shabaraba Date: Tue, 13 Oct 2026 17:11:10 +0900 Subject: [PATCH 08/10] refactor: hide item-level required with a zod pipe instead of the callback --- src/schema/without-advertised-required.ts | 9 +++ src/server/__tests__/tool-definitions.test.ts | 39 +++++++--- src/tools/__tests__/validation.test.ts | 60 --------------- .../kintone/app/__tests__/deploy-app.test.ts | 14 ---- src/tools/kintone/app/deploy-app.ts | 34 ++++----- .../record/__tests__/update-records.test.ts | 52 +++++-------- .../record/__tests__/update-statuses.test.ts | 38 ++-------- src/tools/kintone/record/update-records.ts | 17 ++--- src/tools/kintone/record/update-statuses.ts | 14 +--- .../__tests__/add-space-from-template.test.ts | 21 ------ .../kintone/space/add-space-from-template.ts | 73 +++++++++---------- src/tools/validation.ts | 57 --------------- 12 files changed, 125 insertions(+), 303 deletions(-) create mode 100644 src/schema/without-advertised-required.ts delete mode 100644 src/tools/__tests__/validation.test.ts delete mode 100644 src/tools/validation.ts 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 a85c5605..47a4481f 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,9 +125,9 @@ describe("tool schemas", () => { ).toEqual([]); }); - // The item fields are checked in the callback (src/tools/validation.ts). - // Only "items" itself is checked here; "required" nested further down is - // left alone, because kintone-add-records advertises it and is not affected. + // See src/schema/without-advertised-required.ts. Only "items" itself is + // 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(); @@ -149,18 +150,22 @@ describe("tool schemas", () => { ).toEqual(known); }); + 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 }] }, + ], + ] 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 the callback validates required", async () => { + it("keep arrays whose items hide required still required", async () => { const advertised = await listTools(); - const arrays = [ - ["kintone-update-records", "records"], - ["kintone-update-statuses", "records"], - ["kintone-deploy-app", "apps"], - ["kintone-add-space-from-template", "members"], - ]; - expect( arrays.map(([toolName, property]) => { const tool = advertised.find( @@ -175,6 +180,18 @@ describe("tool schemas", () => { ).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/__tests__/validation.test.ts b/src/tools/__tests__/validation.test.ts deleted file mode 100644 index f1162d18..00000000 --- a/src/tools/__tests__/validation.test.ts +++ /dev/null @@ -1,60 +0,0 @@ -import { describe, expect, expectTypeOf, it } from "vitest"; -import { - MissingRequiredFieldsError, - requireItemFields, -} from "../validation.js"; - -describe("requireItemFields", () => { - it("returns the items unchanged when every key is present", () => { - const items = [ - { id: "1", record: {} }, - { id: "2", record: {} }, - ]; - - expect(requireItemFields(items, "records", ["id", "record"])).toBe(items); - }); - - it("reports every missing field across every item at once", () => { - const items = [{ id: "1", record: {} }, { record: {} }, { id: "3" }]; - - try { - requireItemFields(items, "records", ["id", "record"]); - expect.unreachable("should have thrown"); - } catch (error) { - expect(error).toBeInstanceOf(MissingRequiredFieldsError); - expect((error as MissingRequiredFieldsError).paths).toEqual([ - "records[1].id", - "records[2].record", - ]); - expect((error as Error).message).toContain( - "Missing required field(s): records[1].id, records[2].record", - ); - } - }); - - it("treats an explicit undefined as missing but keeps other falsy values", () => { - const items = [{ app: "", revision: undefined }]; - - expect(requireItemFields(items, "apps", ["app"])).toBe(items); - expect(() => requireItemFields(items, "apps", ["revision"])).toThrow( - "apps[0].revision", - ); - }); - - it("accepts an empty array", () => { - expect(requireItemFields([], "records", ["id"])).toEqual([]); - }); - - // The return goes through a type assertion, so tsc is what checks this. - it("narrows only the asserted keys to required", () => { - const items: Array<{ id?: string; record?: object; revision?: string }> = [ - { id: "1", record: {} }, - ]; - - const narrowed = requireItemFields(items, "records", ["id", "record"]); - - expectTypeOf(narrowed[0].id).toEqualTypeOf(); - expectTypeOf(narrowed[0].record).toEqualTypeOf(); - expectTypeOf(narrowed[0].revision).toEqualTypeOf(); - }); -}); diff --git a/src/tools/kintone/app/__tests__/deploy-app.test.ts b/src/tools/kintone/app/__tests__/deploy-app.test.ts index 21ac8b8c..e9fd022f 100644 --- a/src/tools/kintone/app/__tests__/deploy-app.test.ts +++ b/src/tools/kintone/app/__tests__/deploy-app.test.ts @@ -123,20 +123,6 @@ describe("deploy-app tool", () => { }); describe("callback function", () => { - it("should reject apps missing app without calling the API", async () => { - const mockClient = createMockClient(); - mockClient.app.deployApp = mockDeployApp; - - await expect( - deployApp.callback( - { apps: [{ revision: "3" }] }, - { client: mockClient }, - ), - ).rejects.toThrow("apps[0].app"); - - expect(mockDeployApp).not.toHaveBeenCalled(); - }); - it("should call API and return formatted response", async () => { const expectedResult = { message: diff --git a/src/tools/kintone/app/deploy-app.ts b/src/tools/kintone/app/deploy-app.ts index ccf469a6..86d2d838 100644 --- a/src/tools/kintone/app/deploy-app.ts +++ b/src/tools/kintone/app/deploy-app.ts @@ -1,25 +1,24 @@ import { z } from "zod"; import { createTool } from "../../factory.js"; -import { requireItemFields } from "../../validation.js"; +import { withoutAdvertisedRequired } from "../../../schema/without-advertised-required.js"; import type { KintoneToolCallback } from "../../types/tool.js"; const inputSchema = { apps: z .array( - z.object({ - // `app` is required; the schema declares it optional and - // `requireItemFields` (src/tools/validation.ts) holds the guarantee. - app: z - .string() - .optional() - .describe( - "Required. The ID of the app to deploy (numeric value as string)", - ), - revision: z - .string() - .optional() - .describe("The expected revision number"), - }), + withoutAdvertisedRequired( + 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"), + }), + ), ) .min(1) .max(300) @@ -50,10 +49,7 @@ const callback: KintoneToolCallback = async ( { apps, revert }, { client }, ) => { - await client.app.deployApp({ - apps: requireItemFields(apps, "apps", ["app"]), - revert, - }); + await client.app.deployApp({ apps, revert }); const result = { message: `Deployment initiated for ${apps.length} app(s). Use kintone-get-app-deploy-status tool to check progress.`, diff --git a/src/tools/kintone/record/__tests__/update-records.test.ts b/src/tools/kintone/record/__tests__/update-records.test.ts index 34e16a63..dc515c89 100644 --- a/src/tools/kintone/record/__tests__/update-records.test.ts +++ b/src/tools/kintone/record/__tests__/update-records.test.ts @@ -256,34 +256,37 @@ describe("update-records tool", () => { app: "123", records: [ { - id: 123, record: { title: { value: "test" } }, }, ], }, - description: "id as number instead of string", + description: "missing required id field", }, - ])("rejects $description", ({ input }) => { - expect(() => inputSchema.parse(input)).toThrow(); - }); - }); - - // The callback is what rejects a missing one (src/tools/validation.ts). - describe("input schema validation with item fields left to the callback", () => { - it.each([ { input: { app: "123", - records: [{ record: { title: { value: "test" } } }], + records: [ + { + id: 123, + record: { title: { value: "test" } }, + }, + ], }, - description: "record without id", + description: "id as number instead of string", }, { - input: { app: "123", records: [{ id: "1" }] }, - description: "record without record", + input: { + app: "123", + records: [ + { + id: "1", + }, + ], + }, + description: "missing required record field", }, - ])("accepts $description", ({ input }) => { - expect(() => inputSchema.parse(input)).not.toThrow(); + ])("rejects $description", ({ input }) => { + expect(() => inputSchema.parse(input)).toThrow(); }); }); @@ -602,23 +605,6 @@ describe("update-records tool", () => { }); }); - it("should reject records missing id or record without calling the API", async () => { - const mockClient = createMockClient(); - mockClient.record.updateRecords = mockUpdateRecords; - - await expect( - updateRecords.callback( - { - app: "123", - records: [{ record: { title: { value: "test" } } }, { id: "2" }], - }, - mockToolCallbackOptions(mockClient), - ), - ).rejects.toThrow("records[0].id, records[1].record"); - - expect(mockUpdateRecords).not.toHaveBeenCalled(); - }); - it("should handle API errors properly", async () => { const mockError = new Error("API Error: Record not found"); mockUpdateRecords.mockRejectedValueOnce(mockError); diff --git a/src/tools/kintone/record/__tests__/update-statuses.test.ts b/src/tools/kintone/record/__tests__/update-statuses.test.ts index 5378f2d5..8a361c14 100644 --- a/src/tools/kintone/record/__tests__/update-statuses.test.ts +++ b/src/tools/kintone/record/__tests__/update-statuses.test.ts @@ -143,6 +143,14 @@ describe("update-statuses tool", () => { }, description: "more than 100 records", }, + { + input: { app: "123", records: [{ action: "申請する" }] }, + description: "record missing id field", + }, + { + input: { app: "123", records: [{ id: "1" }] }, + description: "record missing action field", + }, { input: { app: "123", @@ -162,22 +170,6 @@ describe("update-statuses tool", () => { }); }); - // The callback is what rejects a missing one (src/tools/validation.ts). - describe("input schema validation with item fields left to the callback", () => { - it.each([ - { - input: { app: "123", records: [{ action: "申請する" }] }, - description: "record without id", - }, - { - input: { app: "123", records: [{ id: "1" }] }, - description: "record without action", - }, - ])("accepts $description", ({ input }) => { - expect(() => inputSchema.parse(input)).not.toThrow(); - }); - }); - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const outputSchema = z.object(updateStatuses.config.outputSchema!); @@ -269,20 +261,6 @@ describe("update-statuses tool", () => { }); describe("callback function", () => { - it("should reject records missing id or action without calling the API", async () => { - const mockClient = createMockClient(); - mockClient.record.updateRecordsStatus = mockUpdateRecordsStatus; - - await expect( - updateStatuses.callback( - { app: "123", records: [{ assignee: "user1" }] }, - mockToolCallbackOptions(mockClient), - ), - ).rejects.toThrow("records[0].id, records[0].action"); - - expect(mockUpdateRecordsStatus).not.toHaveBeenCalled(); - }); - it("should call API and return formatted response for single record", async () => { const mockData = { records: [ diff --git a/src/tools/kintone/record/update-records.ts b/src/tools/kintone/record/update-records.ts index 41e924d3..0084d3d3 100644 --- a/src/tools/kintone/record/update-records.ts +++ b/src/tools/kintone/record/update-records.ts @@ -1,22 +1,17 @@ import { z } from "zod"; import { createTool } from "../../factory.js"; import { recordSchemaForParameter } from "../../../schema/record/index.js"; -import { requireItemFields } from "../../validation.js"; +import { withoutAdvertisedRequired } from "../../../schema/without-advertised-required.js"; import type { KintoneToolCallback } from "../../types/tool.js"; -// `id` and `record` are required; the schema declares them optional and -// `requireItemFields` (src/tools/validation.ts) holds the guarantee. const updateRecordSchema = z.object({ // updateKey指定は対象外 id: z .string() - .optional() .describe("Required. Record ID to update (numeric value as string)"), - record: recordSchemaForParameter - .optional() - .describe( - "Required. Record data with field codes as keys. Use kintone-get-form-fields tool first to discover available field codes and their types.", - ), + record: recordSchemaForParameter.describe( + "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() .optional() @@ -32,7 +27,7 @@ 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( @@ -66,7 +61,7 @@ const callback: KintoneToolCallback = async ( ) => { const response = await client.record.updateRecords({ app, - records: requireItemFields(records, "records", ["id", "record"]), + records, upsert: false, // upsertモードは対象外 }); diff --git a/src/tools/kintone/record/update-statuses.ts b/src/tools/kintone/record/update-statuses.ts index 461445f9..c78ec978 100644 --- a/src/tools/kintone/record/update-statuses.ts +++ b/src/tools/kintone/record/update-statuses.ts @@ -1,18 +1,12 @@ import { z } from "zod"; import { createTool } from "../../factory.js"; -import { requireItemFields } from "../../validation.js"; +import { withoutAdvertisedRequired } from "../../../schema/without-advertised-required.js"; import type { KintoneToolCallback } from "../../types/tool.js"; -// `id` and `action` are required; the schema declares them optional and -// `requireItemFields` (src/tools/validation.ts) holds the guarantee. const statusRecordSchema = z.object({ - id: z - .string() - .optional() - .describe("Required. Record ID (numeric value as string)"), + id: z.string().describe("Required. Record ID (numeric value as string)"), action: z .string() - .optional() .describe( "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.", ), @@ -33,7 +27,7 @@ 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( @@ -71,7 +65,7 @@ const callback: KintoneToolCallback = async ( ) => { const response = await client.record.updateRecordsStatus({ app, - records: requireItemFields(records, "records", ["id", "action"]), + records, }); const result = { diff --git a/src/tools/kintone/space/__tests__/add-space-from-template.test.ts b/src/tools/kintone/space/__tests__/add-space-from-template.test.ts index 5ca71348..d895b7b0 100644 --- a/src/tools/kintone/space/__tests__/add-space-from-template.test.ts +++ b/src/tools/kintone/space/__tests__/add-space-from-template.test.ts @@ -107,27 +107,6 @@ describe("add-space-from-template tool", () => { }); describe("callback function", () => { - it("should reject members missing entity without calling the API", async () => { - const mockClient = createMockClient(); - mockClient.space.addSpaceFromTemplate = mockAddSpaceFromTemplate; - - await expect( - addSpaceFromTemplate.callback( - { - id: "1", - name: "My Space", - members: [{ isAdmin: true, includeSubs: false }], - isPrivate: false, - isGuest: false, - fixedMember: false, - }, - { client: mockClient }, - ), - ).rejects.toThrow("members[0].entity"); - - expect(mockAddSpaceFromTemplate).not.toHaveBeenCalled(); - }); - it("should call addSpaceFromTemplate with the parsed input and return structured content", async () => { mockAddSpaceFromTemplate.mockResolvedValueOnce({ id: "200" }); diff --git a/src/tools/kintone/space/add-space-from-template.ts b/src/tools/kintone/space/add-space-from-template.ts index 7fc005e6..d27712fd 100644 --- a/src/tools/kintone/space/add-space-from-template.ts +++ b/src/tools/kintone/space/add-space-from-template.ts @@ -1,6 +1,6 @@ import { z } from "zod"; import { createTool } from "../../factory.js"; -import { requireItemFields } from "../../validation.js"; +import { withoutAdvertisedRequired } from "../../../schema/without-advertised-required.js"; import type { KintoneToolCallback } from "../../types/tool.js"; const inputSchema = { @@ -12,41 +12,40 @@ const inputSchema = { name: z.string().min(1).describe("The name of the new space."), members: z .array( - z.object({ - // `entity` is required; the schema declares it optional and - // `requireItemFields` (src/tools/validation.ts) holds the guarantee. - 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.", - ), - }) - .optional() - .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).", - ), - }), + withoutAdvertisedRequired( + 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).", + ), + }), + ), ) .min(1) .describe( @@ -105,7 +104,7 @@ const callback: KintoneToolCallback = async ( const response = await client.space.addSpaceFromTemplate({ id, name, - members: requireItemFields(members, "members", ["entity"]), + members, isPrivate, isGuest, fixedMember, diff --git a/src/tools/validation.ts b/src/tools/validation.ts deleted file mode 100644 index 813096f4..00000000 --- a/src/tools/validation.ts +++ /dev/null @@ -1,57 +0,0 @@ -// Some MCP clients cannot build arguments for an array that is required at the -// top level when its `items` schema declares `required` of its own: they never -// send `tools/call` and keep asking the user for the item keys instead. The -// affected tools therefore declare those item fields optional in their input -// schema, and the guarantee that they are required lives here. -// https://github.com/mondaycom/mcp/issues/499 - -/** Error thrown when an array item is missing a field its tool requires. */ -export class MissingRequiredFieldsError extends Error { - /** Paths of the missing fields, e.g. `records[0].id`. */ - readonly paths: readonly string[]; - - constructor(paths: readonly string[]) { - super( - `Missing required field(s): ${paths.join(", ")}. ` + - "These fields are declared optional in the input schema for client compatibility, " + - "but they are required by this tool.", - ); - this.name = "MissingRequiredFieldsError"; - this.paths = paths; - } -} - -type WithRequired = T & { - [P in K]-?: Exclude; -}; - -/** - * Assert that every item of an array carries the fields its tool requires. - * - * Every missing field is reported at once, so the caller does not have to fix - * them one round trip at a time. - * - * @param items The array to check - * @param arrayName Argument name of the array, used in the error message (e.g. `records`) - * @param keys The keys to treat as required - * @returns The same array, with `keys` narrowed to required - */ -export const requireItemFields = ( - items: readonly T[], - arrayName: string, - keys: readonly K[], -): Array> => { - const missing = items.flatMap((item, index) => - keys - .filter((key) => item[key] === undefined) - .map((key) => `${arrayName}[${index}].${key}`), - ); - - if (missing.length > 0) { - throw new MissingRequiredFieldsError(missing); - } - - // The check above rules out `undefined` for every key, which the type checker - // cannot follow back to the element type. - return items as Array>; -}; From 4a14dcba421674dd6f2a703a30f9013fafa1da59 Mon Sep 17 00:00:00 2001 From: shabaraba Date: Tue, 13 Oct 2026 17:11:42 +0900 Subject: [PATCH 09/10] fix: let clients call kintone-search, whose query items declare required --- src/schema/search/input.ts | 5 ++-- src/server/__tests__/tool-definitions.test.ts | 29 ++++++++++--------- 2 files changed, 18 insertions(+), 16 deletions(-) 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/server/__tests__/tool-definitions.test.ts b/src/server/__tests__/tool-definitions.test.ts index 47a4481f..c82f85f5 100644 --- a/src/server/__tests__/tool-definitions.test.ts +++ b/src/server/__tests__/tool-definitions.test.ts @@ -125,29 +125,29 @@ describe("tool schemas", () => { ).toEqual([]); }); - // See src/schema/without-advertised-required.ts. Only "items" itself is - // checked here; "required" nested further down is left alone, because - // kintone-add-records advertises it and is not affected. + // 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(); - // "kintone-search: query" is a tuple with a rest element, so it also - // advertises prefixItems. Whether that shape trips clients is unconfirmed. - const known = ["kintone-search: query"]; - expect( advertised.flatMap((tool) => { const required: string[] = tool.inputSchema.required ?? []; return Object.entries(tool.inputSchema.properties ?? {}) - .filter( - ([name, property]) => - required.includes(name) && - (property as { items?: { required?: unknown } }).items - ?.required !== undefined, - ) + .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(known); + ).toEqual([]); }); const arrays = [ @@ -159,6 +159,7 @@ describe("tool schemas", () => { "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 From 71acfa23a53458a68cae850af049074359d2030d Mon Sep 17 00:00:00 2001 From: shabaraba Date: Wed, 14 Oct 2026 15:23:30 +0900 Subject: [PATCH 10/10] refactor: extract array item schemas in deploy-app and add-space-from-template --- src/tools/kintone/app/deploy-app.ts | 25 +++---- .../kintone/space/add-space-from-template.ts | 70 +++++++++---------- 2 files changed, 44 insertions(+), 51 deletions(-) diff --git a/src/tools/kintone/app/deploy-app.ts b/src/tools/kintone/app/deploy-app.ts index 86d2d838..363456d4 100644 --- a/src/tools/kintone/app/deploy-app.ts +++ b/src/tools/kintone/app/deploy-app.ts @@ -3,23 +3,18 @@ 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( - withoutAdvertisedRequired( - 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"), - }), - ), - ) + .array(withoutAdvertisedRequired(deployAppSchema)) .min(1) .max(300) .describe( diff --git a/src/tools/kintone/space/add-space-from-template.ts b/src/tools/kintone/space/add-space-from-template.ts index d27712fd..5869c4db 100644 --- a/src/tools/kintone/space/add-space-from-template.ts +++ b/src/tools/kintone/space/add-space-from-template.ts @@ -3,6 +3,39 @@ 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() @@ -11,42 +44,7 @@ const inputSchema = { ), name: z.string().min(1).describe("The name of the new space."), members: z - .array( - withoutAdvertisedRequired( - 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).", - ), - }), - ), - ) + .array(withoutAdvertisedRequired(spaceMemberSchema)) .min(1) .describe( "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.",