From 6f4d7d9066f25fd71ff3a71d06bf51afc3aa7fb9 Mon Sep 17 00:00:00 2001 From: Maurice Date: Thu, 29 Oct 2026 13:45:53 +0100 Subject: [PATCH] fix(mcp): keep restore_book_page free of tuples so ChatGPT accepts the tool list With the Journey addon on, ChatGPT refused every TREK tool: restore_book_page took a map's path and roads as [lat, lng] tuples, which the SDK writes as the draft-07 array form of items, and JSON Schema 2020-12 refuses that form. The restore now takes a map point as two numbers and parses each page as a spread before it is put back, so lat and lng stay checked and a point out of range is an invalid_op instead of a thrown parse. MCP-SCHEMA-005 holds every tool's input schema free of tuple keywords. Closes #2879 --- .../tests/unit/mcp/tool-input-schemas.test.ts | 25 ++++++++++- shared/src/book/book-ops.schema.ts | 17 +++++++- shared/src/book/book-page-ops.spec.ts | 42 +++++++++++++++++++ shared/src/book/book-page-ops.ts | 18 ++++---- shared/src/book/book.schema.ts | 7 +++- 5 files changed, 94 insertions(+), 15 deletions(-) diff --git a/server/tests/unit/mcp/tool-input-schemas.test.ts b/server/tests/unit/mcp/tool-input-schemas.test.ts index 4be57db60..75ac0afe4 100644 --- a/server/tests/unit/mcp/tool-input-schemas.test.ts +++ b/server/tests/unit/mcp/tool-input-schemas.test.ts @@ -8,7 +8,9 @@ * several MCP clients do not resolve), every id field is the positive integer * of @trek/shared's idSchema, the shared primitive the tools derive their ids * from, and the book restore's schema, which carries whole pages, stays under a ceiling, as do - * the page edit and the design revision, which take every page operation. + * the page edit and the design revision, which take every page operation. And an input schema + * reads the same under draft-07, which the SDK writes, and 2020-12, the MCP default: no tuple, + * since ChatGPT checks against 2020-12 and refuses the whole tool list over one tool's tuple. */ import type { ToolOptions } from '../../../src/nest-mcp'; import { getEntry, isMcpController, type ClassRef } from '../../../src/nest-mcp/metadata'; @@ -39,6 +41,22 @@ function methodNames(ctor: ClassRef): string[] { return [...names]; } +const VALUE_KEYWORDS = new Set(['default', 'const', 'enum', 'examples']); + +/** Where a schema spells a tuple: `items` as an array, `additionalItems` or `prefixItems`. */ +function tuplesIn(node: unknown, at: string): string[] { + if (Array.isArray(node)) return node.flatMap((child, i) => tuplesIn(child, `${at}/${i}`)); + if (!node || typeof node !== 'object') return []; + const schema = node as Record; + const here = Array.isArray(schema.items) || 'additionalItems' in schema || 'prefixItems' in schema ? [at] : []; + const below = Object.entries(schema).flatMap(([key, value]) => { + if (VALUE_KEYWORDS.has(key) || !value || typeof value !== 'object') return []; + if (key !== 'properties') return tuplesIn(value, `${at}/${key}`); + return Object.entries(value).flatMap(([name, prop]) => tuplesIn(prop, `${at}/properties/${name}`)); + }); + return [...here, ...below]; +} + async function toolSchemas(): Promise>> { const out = new Map>(); const files = mcpFiles(NEST_ROOT).sort(); @@ -106,4 +124,9 @@ describe('MCP tool input schemas', () => { } expect(sizeOf('restore_book_layout')).toBeLessThan(2_000); }); + + it('MCP-SCHEMA-005: every input schema is valid JSON Schema 2020-12 too, so no tool holds a tuple', () => { + const tuples = [...schemas].flatMap(([key, json]) => tuplesIn(json, '').map((at) => `${key} ${at}`)); + expect(tuples).toEqual([]); + }); }); diff --git a/shared/src/book/book-ops.schema.ts b/shared/src/book/book-ops.schema.ts index f814f43c2..678f0eebf 100644 --- a/shared/src/book/book-ops.schema.ts +++ b/shared/src/book/book-ops.schema.ts @@ -20,6 +20,10 @@ import { bookElementIdSchema, bookGridIdSchema, bookSpreadIdSchema } from './boo import { bookPaintInSchema } from './book-paint.schema'; import { MAX_BOOK_TITLE, + MAX_PATH_POINTS, + MAX_PATH_SEGMENTS, + MAX_ROAD_LEGS, + MAX_ROAD_POINTS, MAX_SPREAD_ELEMENTS, bookBadgeElementSchema, bookCountriesElementSchema, @@ -336,6 +340,12 @@ export type BookPageDigest = z.infer; * kind the tool's schema, which every client loads with tools/list, outgrew * its size. It takes the same values: restoreBookPage parses the page again as * a spread, which sets every default, before it puts it back. + * + * Its schema also has to read the same under JSON Schema draft-07, which the + * SDK writes, and 2020-12, the MCP default ChatGPT validates against, so it + * holds no tuple: draft-07 spells one as an array of `items`, which 2020-12 + * refuses, and ChatGPT then drops every tool of the server, not only this one. + * A map's points come in as two numbers; the spread's parse checks lat and lng. */ const ELEMENT_BASE = { id: true, @@ -368,6 +378,11 @@ const TRAVEL_TYPE = { } as const; const kindIn = (schema: z.ZodObject) => restoredFields(schema.omit(ELEMENT_BASE)); const travelIn = (schema: z.ZodObject) => restoredFields(schema.omit({ ...ELEMENT_BASE, ...TRAVEL_TYPE })); +const restoredPointIn = z.array(z.number()).length(2); +const restoredMapIn = travelIn(bookMapElementSchema).extend({ + path: z.array(z.array(restoredPointIn).max(MAX_PATH_POINTS)).max(MAX_PATH_SEGMENTS).optional(), + roads: z.array(z.array(restoredPointIn).max(MAX_ROAD_POINTS).nullable()).max(MAX_ROAD_LEGS).optional(), +}); // Each union says what it holds: a described union stays one part of the intersection instead of // zod writing the shared fields out into every kind again. const restoredElementIn = z.intersection( @@ -384,7 +399,7 @@ const restoredElementIn = z.intersection( restoredFields(bookMapElementSchema.pick(TRAVEL_TYPE)), z .union([ - travelIn(bookMapElementSchema), + restoredMapIn, travelIn(bookStatsElementSchema), travelIn(bookCountriesElementSchema), travelIn(bookBadgeElementSchema), diff --git a/shared/src/book/book-page-ops.spec.ts b/shared/src/book/book-page-ops.spec.ts index ebb0c6e16..222358ca0 100644 --- a/shared/src/book/book-page-ops.spec.ts +++ b/shared/src/book/book-page-ops.spec.ts @@ -337,6 +337,48 @@ describe('restore', () => { 'invalid_op', ); }); + + it("BOOKPG-031: puts a map's track and roads back point for point, placed or parked", () => { + const base = book(1); + const frame = { x: 20, y: 20, w: 120, h: 80 }; + const path = [ + [ + [64.1466, -21.9426], + [64.2559, -15.2121], + ], + ]; + const roads = [ + [ + [64.1466, -21.9426], + [63.4186, -19.006], + [64.2559, -15.2121], + ], + null, + ]; + const spread = bookSpreadSchema.parse({ + id: 'sp-in1', + elements: [{ id: 'm1', kind: 'map', frame, path, roads }], + parked: [{ id: 'm2', kind: 'map', frame, roads }], + }); + const back = ok(restoreBookPage(base, { pages: [{ spread, index: 2 }], expectedVersion: 1 })); + expect(back.spreads[2]).toEqual(spread); + expect(back.spreads[2]?.elements[0]).toMatchObject({ path, roads }); + }); + + it('BOOKPG-032: refuses a map point that is no [lat, lng], though the tool takes points as two numbers', () => { + const base = book(1); + const frame = { x: 20, y: 20, w: 120, h: 80 }; + const withPath = (path: number[][][]) => ({ + pages: [{ index: 2, spread: { id: 'sp-in1', elements: [{ id: 'm1', kind: 'map', frame, path }] } }], + expectedVersion: 1, + }); + expect(ok(restoreBookPage(base, withPath([[[64.1, -21.9]]]))).spreads[2]?.elements[0]).toMatchObject({ + path: [[[64.1, -21.9]]], + }); + for (const point of [[120, 10], [10, 200], [64.1, -21.9, 3], [64.1]]) { + expect(refused(restoreBookPage(base, withPath([[point]])))).toBe('invalid_op'); + } + }); }); describe('the cover', () => { diff --git a/shared/src/book/book-page-ops.ts b/shared/src/book/book-page-ops.ts index af6ecd675..26a7de952 100644 --- a/shared/src/book/book-page-ops.ts +++ b/shared/src/book/book-page-ops.ts @@ -14,6 +14,7 @@ import { } from './book-ops-core'; import { addBookPagesRequestSchema, + bookPreviousSpreadSchema, deleteBookPagesRequestSchema, moveBookPageRequestSchema, restoreBookPageRequestSchema, @@ -22,13 +23,7 @@ import { } from './book-ops.schema'; import { blankSpread, canEditSpread, innerSpreadBounds } from './book-pages'; import { bookSpreadPhotoIds, checkRestorablePhotos } from './book-photo-refs'; -import { - bookSpreadSchema, - type BookDocument, - type BookElement, - type BookPhotoElement, - type BookSpread, -} from './book.schema'; +import type { BookDocument, BookElement, BookPhotoElement, BookSpread } from './book.schema'; /** * Operations on the pages of a book rather than on one page: add, move, @@ -144,11 +139,12 @@ export function deleteBookPages(doc: BookDocument, args: unknown): BookEditResul export function restoreBookPage(doc: BookDocument, args: unknown, ctx: BookOpsContext = {}): BookEditResult { const parsed = restoreBookPageRequestSchema.safeParse(args); if (!parsed.success) return { ok: false, error: invalidOp(parsed.error, null) }; + // Read again as the spreads they are: the tool takes them in a shape that states each field once + // and leaves the finer checks, a map point's lat and lng among them, to this parse (book-ops.schema.ts). + const spreads = bookPreviousSpreadSchema.array().safeParse(parsed.data.pages); + if (!spreads.success) return { ok: false, error: invalidOp(spreads.error, null) }; return caught(() => { - // Read again as the spread it is: the tool takes it in a shape that states each field once (book-ops.schema.ts). - const pages = parsed.data.pages - .map((p) => ({ index: p.index, spread: bookSpreadSchema.parse(p.spread) })) - .sort((a, b) => a.index - b.index); + const pages = [...spreads.data].sort((a, b) => a.index - b.index); let cur = doc; for (const { spread, index } of pages) { // A letter fill counts as a photo, as a frame does (book-photo-refs.ts). diff --git a/shared/src/book/book.schema.ts b/shared/src/book/book.schema.ts index ffc59493d..978fd43d6 100644 --- a/shared/src/book/book.schema.ts +++ b/shared/src/book/book.schema.ts @@ -75,6 +75,9 @@ export const MAX_MAP_POINT_LABEL = 120; /** A recorded track: segments, and points within one segment. */ export const MAX_PATH_SEGMENTS = 40; export const MAX_PATH_POINTS = 1200; +/** The roads a map draws: legs, and points along one leg. */ +export const MAX_ROAD_LEGS = 400; +export const MAX_ROAD_POINTS = 200; /** * Every shape the book can draw — and, because a picture frame *is* a shape with @@ -578,10 +581,10 @@ export const bookMapElementSchema = z.object({ .array( z .array(z.tuple([z.number().min(-90).max(90), z.number().min(-180).max(180)])) - .max(200) + .max(MAX_ROAD_POINTS) .nullable(), ) - .max(400) + .max(MAX_ROAD_LEGS) .default([]), /** * How much room to leave around what is drawn, as a share of its own extent.