mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-11-13 07:40:59 +00:00
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
This commit is contained in:
@@ -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<string, unknown>;
|
||||
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<Map<string, Record<string, unknown>>> {
|
||||
const out = new Map<string, Record<string, unknown>>();
|
||||
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([]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<typeof bookPageDigestSchema>;
|
||||
* 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),
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -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).
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user