diff --git a/src/tools/actor_input_schema.ts b/src/tools/actor_input_schema.ts index e19cc8cb..6c24ab94 100644 --- a/src/tools/actor_input_schema.ts +++ b/src/tools/actor_input_schema.ts @@ -80,14 +80,15 @@ export function buildApifySpecificProperties( } /** - * Filters schema properties to include only the necessary fields. - * This is done to reduce the size of the input schema and to make it more readable. + * Filters schema properties to the fields MCP tools need: core shape, examples/defaults, + * and JSON Schema validation keywords. UI-only Actor hints (`editor`, `enumTitles`, …) are + * dropped to keep advertised schemas smaller. * - * TODO(#675): This object literal unconditionally assigns every whitelisted key, - * including `default: undefined`, on properties that didn't declare them upstream. - * This creates phantom keys that broke `fixZodSchemaRequired()` via key-presence - * checks (#637). The symptom is patched in `src/utils/ajv.ts` (value-check), but - * this function should only emit keys whose upstream value is not `undefined`. + * Only emits keys whose upstream value is not `undefined` — assigning `default: undefined` + * (and similar) created phantom keys that broke `'default' in field` checks (#637 / #675). + * + * Validation keywords (`minimum`, `maximum`, `minLength`, …) are preserved so AJV compile + * and tools/list honor Actor-declared bounds instead of silently accepting out-of-range input. * * @param properties */ @@ -96,21 +97,50 @@ export function filterSchemaProperties(properties: { [key: string]: SchemaProper } { const filteredProperties: { [key: string]: SchemaProperties } = {}; for (const [key, property] of Object.entries(properties)) { - filteredProperties[key] = { - title: property.title, - description: property.description, - enum: property.enum, - type: property.type, - default: property.default, - prefill: property.prefill, - properties: property.properties, - items: property.items, - required: property.required, - }; + filteredProperties[key] = pickDefinedSchemaFields(property); } return filteredProperties; } +/** Optional SchemaProperties keys that survive filtering when the upstream value is defined. */ +const OPTIONAL_SCHEMA_FIELD_KEYS = [ + 'enum', + 'default', + 'prefill', + 'properties', + 'items', + 'required', + 'examples', + 'minimum', + 'maximum', + 'exclusiveMinimum', + 'exclusiveMaximum', + 'minLength', + 'maxLength', + 'minItems', + 'maxItems', + 'uniqueItems', + 'pattern', +] as const satisfies readonly (keyof SchemaProperties)[]; + +function pickDefinedSchemaFields(property: SchemaProperties): SchemaProperties { + const filtered: SchemaProperties = { + type: property.type, + title: property.title, + description: property.description, + }; + + for (const field of OPTIONAL_SCHEMA_FIELD_KEYS) { + const value = property[field]; + if (value !== undefined) { + // Indexed assignment through a keyof union needs a narrow cast. + (filtered as Record)[field] = value; + } + } + + return filtered; +} + /** * For array properties missing items.type, infers and sets the type using inferArrayItemType. * @param properties diff --git a/src/types.ts b/src/types.ts index d0a6aac8..36d1f30c 100644 --- a/src/types.ts +++ b/src/types.ts @@ -36,6 +36,20 @@ export type SchemaProperties = { properties?: Record; required?: string[]; + + // JSON Schema validation keywords from Actor input schemas. Kept through + // filterSchemaProperties so AJV and tools/list honor Actor-declared bounds. + minimum?: number; + maximum?: number; + exclusiveMinimum?: number; + exclusiveMaximum?: number; + minLength?: number; + maxLength?: number; + minItems?: number; + maxItems?: number; + uniqueItems?: boolean; + // Advertised to clients; AJV drops `pattern` at compile time (ReDoS guard in ajv.ts). + pattern?: string; }; export type ActorInputSchema = { diff --git a/src/utils/ajv.ts b/src/utils/ajv.ts index 806915bf..95020362 100644 --- a/src/utils/ajv.ts +++ b/src/utils/ajv.ts @@ -21,7 +21,8 @@ ajv.removeKeyword('format'); * breaks AJV compilation. * * Uses a value-check (`field.default !== undefined`) instead of key-presence (`'default' in field`) - * because `filterSchemaProperties()` assigns phantom `default: undefined` on every property (#675). + * so a future regression that reintroduces phantom `default: undefined` keys cannot clear + * required fields again (#637). `filterSchemaProperties` no longer emits those phantoms. * * @see https://github.com/apify/apify-mcp-server/issues/637 */ diff --git a/tests/unit/tools.utils.test.ts b/tests/unit/tools.utils.test.ts index 511fb1d8..79795515 100644 --- a/tests/unit/tools.utils.test.ts +++ b/tests/unit/tools.utils.test.ts @@ -8,6 +8,7 @@ import { decodeDotPropertyNames, encodeDotPropertyNames, filterAndShortenEnum, + filterSchemaProperties, fixedAjvCompile, inferArrayItemsTypeIfMissing, inferArrayItemType, @@ -748,21 +749,14 @@ describe('transformActorInputSchemaProperties', () => { expect(result.proxy.properties?.useApifyProxy).toBeDefined(); expect(result.sources.items).toBeDefined(); expect(result.sources.items?.properties?.url).toBeDefined(); - // 3. filterSchemaProperties: only allowed fields present - // NOTE: includes phantom `default: undefined` etc. from filterSchemaProperties (#675). - expect(Object.keys(result['foo-dot-bar'])).toEqual( - expect.arrayContaining([ - 'title', - 'description', - 'type', - 'default', - 'prefill', - 'properties', - 'items', - 'required', - 'enum', - ]), - ); + // 3. filterSchemaProperties: only defined allowed fields (no phantom undefined keys) + expect(Object.keys(result['foo-dot-bar']).sort()).toEqual(['description', 'title', 'type']); + expect(result['foo-dot-bar']).not.toHaveProperty('default'); + expect(result['foo-dot-bar']).not.toHaveProperty('prefill'); + expect(result['foo-dot-bar']).not.toHaveProperty('enum'); + // UI-only `editor` is stripped from proxy / sources after filtering + expect(result.proxy).not.toHaveProperty('editor'); + expect(result.sources).not.toHaveProperty('editor'); // 4. shortenProperties: longDesc is truncated, enumProp.enum is shortened expect(result.longDesc.description.length).toBeLessThanOrEqual(ACTOR_MAX_DESCRIPTION_LENGTH + 3); if (result.enumProp.enum) { @@ -860,6 +854,125 @@ describe('transformActorInputSchemaProperties', () => { }); }); +describe('filterSchemaProperties()', () => { + it('preserves JSON Schema validation keywords and omits undefined optionals', () => { + const filtered = filterSchemaProperties({ + maxResults: { + type: 'integer', + title: 'Max results', + description: 'Cap', + minimum: 1, + maximum: 100, + default: 10, + }, + query: { + type: 'string', + title: 'Query', + description: 'Search', + minLength: 1, + maxLength: 200, + pattern: '^[a-z]+$', + }, + tags: { + type: 'array', + title: 'Tags', + description: 'Tags', + minItems: 1, + maxItems: 5, + uniqueItems: true, + items: { type: 'string', title: 'Tag', description: 'One tag' }, + }, + }); + + expect(filtered.maxResults).toEqual({ + type: 'integer', + title: 'Max results', + description: 'Cap', + minimum: 1, + maximum: 100, + default: 10, + }); + expect(filtered.maxResults).not.toHaveProperty('prefill'); + expect(filtered.query).toMatchObject({ + minLength: 1, + maxLength: 200, + pattern: '^[a-z]+$', + }); + expect(filtered.tags).toMatchObject({ + minItems: 1, + maxItems: 5, + uniqueItems: true, + }); + expect(filtered.tags).not.toHaveProperty('editor'); + }); + + it('keeps exclusive bounds when declared', () => { + const filtered = filterSchemaProperties({ + score: { + type: 'number', + title: 'Score', + description: '0–1 exclusive', + exclusiveMinimum: 0, + exclusiveMaximum: 1, + }, + }); + expect(filtered.score.exclusiveMinimum).toBe(0); + expect(filtered.score.exclusiveMaximum).toBe(1); + }); +}); + +describe('transformActorInputSchemaProperties — validation keywords', () => { + it('survives the full transform pipeline and is enforced by AJV', () => { + const properties = transformActorInputSchemaProperties({ + type: 'object', + properties: { + maxResults: { + type: 'integer', + title: 'Max results', + description: 'Cap', + minimum: 1, + maximum: 10, + default: 5, + }, + name: { + type: 'string', + title: 'Name', + description: 'Name', + minLength: 2, + maxLength: 8, + }, + urls: { + type: 'array', + title: 'URLs', + description: 'URL list', + minItems: 1, + maxItems: 3, + items: { type: 'string', title: 'URL', description: 'One URL' }, + }, + }, + }); + + expect(properties.maxResults.minimum).toBe(1); + expect(properties.maxResults.maximum).toBe(10); + expect(properties.name.minLength).toBe(2); + expect(properties.name.maxLength).toBe(8); + expect(properties.urls.minItems).toBe(1); + expect(properties.urls.maxItems).toBe(3); + + const validate = fixedAjvCompile(ajv, { + type: 'object', + properties, + required: [], + }); + + expect(validate({ maxResults: 5, name: 'ab', urls: ['https://a'] })).toBe(true); + expect(validate({ maxResults: 11, name: 'ab', urls: ['https://a'] })).toBe(false); + expect(validate({ maxResults: 5, name: 'a', urls: ['https://a'] })).toBe(false); + expect(validate({ maxResults: 5, name: 'ab', urls: [] })).toBe(false); + expect(validate({ maxResults: 5, name: 'ab', urls: ['a', 'b', 'c', 'd'] })).toBe(false); + }); +}); + describe('inferArrayItemType', () => { it('infers array item type from editor', () => { const property = {