fix(core-internal): cache AJV validators for schemas without $id

.changeset/fix-ajv-validator-cache.md

@@ -0,0 +1,5 @@
+---
+'@modelcontextprotocol/core-internal': patch
+---
+
+Fix a memory leak in the validator providers: schemas without an `$id` were recompiled (or re-instantiated) on every `getValidator()` call and retained forever, so long-running clients that periodically refresh their tool catalogue (e.g. via `Client.listTools()`) grew the heap without bound. Identical schemas are now cached by their canonical serialization (key order independent), each distinct schema compiles at most once, compiled snapshots defeat Ajv's identity-based cache on in-place mutation, and the cache is FIFO-bounded so schemas that genuinely come and go cannot accumulate forever. Applies to both `AjvJsonSchemaValidator` and `CfWorkerJsonSchemaValidator` (the latter keys by schema + draft).

packages/core-internal/src/validators/ajvProvider.ts

@@ -8,6 +8,7 @@ import { Ajv2020 } from 'ajv/dist/2020.js';
 import _addFormats from 'ajv-formats';
 
 import { declaredDialect } from './dialects';
+import { canonicalJson, createBoundedCache, VALIDATOR_CACHE_LIMIT } from './schemaCache';
 import type { JsonSchemaType, JsonSchemaValidator, jsonSchemaValidator, JsonSchemaValidatorResult } from './types';
 
 /** Structural subset of the AJV interface used by {@link AjvJsonSchemaValidator}. */
@@ -81,6 +82,15 @@ export class AjvJsonSchemaValidator implements jsonSchemaValidator {
     private _ajvDraft7: AjvLike | undefined;
     /** Lazy 2019-09 engine, built on the first 2019-09-declared schema. */
     private _ajv2019: AjvLike | undefined;
+    /**
+     * Compiled validators for schemas without a usable `$id`, keyed by the
+     * schema's canonical serialization and bounded to {@link VALIDATOR_CACHE_LIMIT}
+     * distinct schemas. AJV only deduplicates compilations by `$id`; without
+     * this cache every `getValidator` call recompiles the same schema and the
+     * engine retains each compiled validator forever, so long-running clients
+     * that refresh their tool catalogue grow the heap without bound (#2605).
+     */
+    private readonly _compiledBySource = createBoundedCache<AjvValidateFunction>(VALIDATOR_CACHE_LIMIT);
     /** True iff the constructor received a caller-supplied engine; the `$schema` dispatch is skipped. */
     private readonly _userAjv: boolean;
 
@@ -133,7 +143,7 @@ export class AjvJsonSchemaValidator implements jsonSchemaValidator {
         const ajvValidator =
             '$id' in schema && typeof schema.$id === 'string'
                 ? (engine.getSchema(schema.$id) ?? engine.compile(schema))
-                : engine.compile(schema);
+                : this._compiledValidator(engine, schema);
 
         return (input: unknown): JsonSchemaValidatorResult<T> => {
             const valid = ajvValidator(input);
@@ -151,6 +161,30 @@ export class AjvJsonSchemaValidator implements jsonSchemaValidator {
                   };
         };
     }
+
+    /** Compile a schema, reusing the previous compilation for identical schemas. */
+    private _compiledValidator(engine: AjvLike, schema: JsonSchemaType): AjvValidateFunction {
+        const key = canonicalJson(schema);
+        if (key === undefined) {
+            // Non-serializable schema (e.g. cyclic): skip the content cache and
+            // let the engine compile it as before.
+            return engine.compile(schema);
+        }
+
+        const cached = this._compiledBySource.get(key);
+        if (cached !== undefined) {
+            return cached;
+        }
+
+        // Compile a snapshot derived from the key, never the caller's object.
+        // AJV caches compiled schemas by object identity, so if a caller
+        // mutates its schema in place, compiling the live object could return
+        // a stale validator that we would then store under the fresh key
+        // (#2605).
+        const compiled = engine.compile(JSON.parse(key) as JsonSchemaType);
+        this._compiledBySource.set(key, compiled);
+        return compiled;
+    }
 }
 
 /**

packages/core-internal/src/validators/cfWorkerProvider.ts

@@ -11,6 +11,7 @@
 import { Validator } from '@cfworker/json-schema';
 
 import { declaredDialect } from './dialects';
+import { canonicalJson, createBoundedCache, VALIDATOR_CACHE_LIMIT } from './schemaCache';
 import type { JsonSchemaType, JsonSchemaValidator, jsonSchemaValidator, JsonSchemaValidatorResult } from './types';
 
 /**
@@ -52,6 +53,14 @@ export class CfWorkerJsonSchemaValidator implements jsonSchemaValidator {
     private readonly shortcircuit: boolean;
     /** Caller-supplied draft; when set, the `$schema` check is skipped (caller owns dialect). */
     private readonly draft?: CfWorkerSchemaDraft;
+    /**
+     * Instantiated validators by canonical schema + draft, bounded to
+     * {@link VALIDATOR_CACHE_LIMIT} distinct entries. `@cfworker/json-schema`
+     * compiles a fresh `Validator` per construction, so without caching
+     * repeated `getValidator` calls with identical schemas recompile each
+     * time and retain every instance (#2605).
+     */
+    private readonly _validators = createBoundedCache<Validator>(VALIDATOR_CACHE_LIMIT);
 
     /**
      * Create a validator
@@ -87,8 +96,22 @@ export class CfWorkerJsonSchemaValidator implements jsonSchemaValidator {
      */
     getValidator<T>(schema: JsonSchemaType): JsonSchemaValidator<T> {
         const draft = this.draft ?? this._draftFor(schema);
-        // Cast to the cfworker Schema type - our JsonSchemaType is structurally compatible
-        const validator = new Validator(schema as ConstructorParameters<typeof Validator>[0], draft, this.shortcircuit);
+        const key = canonicalJson(schema);
+        let validator: Validator;
+        if (key === undefined) {
+            // Non-serializable schema (e.g. cyclic): skip the cache entirely.
+            validator = new Validator(schema as ConstructorParameters<typeof Validator>[0], draft, this.shortcircuit);
+        } else {
+            const cacheKey = `${draft}:${key}`;
+            const cached = this._validators.get(cacheKey);
+            if (cached === undefined) {
+                // Cast to the cfworker Schema type - our JsonSchemaType is structurally compatible
+                validator = new Validator(schema as ConstructorParameters<typeof Validator>[0], draft, this.shortcircuit);
+                this._validators.set(cacheKey, validator);
+            } else {
+                validator = cached;
+            }
+        }
 
         return (input: unknown): JsonSchemaValidatorResult<T> => {
             const result = validator.validate(input);

packages/core-internal/src/validators/schemaCache.ts

@@ -0,0 +1,68 @@
+/**
+ * Shared bounded-cache helpers for validator providers.
+ *
+ * Both `AjvJsonSchemaValidator` and `CfWorkerJsonSchemaValidator` compile
+ * (or instantiate) a validator per distinct schema. Without caching, repeated
+ * calls with identical schemas recompile every time; without a bound, a
+ * caller whose schemas genuinely evolve keeps every distinct schema ever
+ * seen. See #2605.
+ */
+
+/** Number of distinct schemas a provider keeps compiled before evicting the oldest. */
+export const VALIDATOR_CACHE_LIMIT = 1000;
+
+/** Recursively sort object keys so structurally equal JSON has one representation. */
+export function sortJsonKeys(value: unknown): unknown {
+    if (Array.isArray(value)) {
+        return value.map(item => sortJsonKeys(item));
+    }
+    if (value !== null && typeof value === 'object') {
+        return Object.fromEntries(
+            Object.keys(value as Record<string, unknown>)
+                .toSorted()
+                .map(key => [key, sortJsonKeys((value as Record<string, unknown>)[key])])
+        );
+    }
+    return value;
+}
+
+/**
+ * Canonical cache key for a schema: JSON-serialized with object keys
+ * recursively sorted, so structurally identical schemas (regardless of key
+ * order or object identity) share one entry. Returns `undefined` for schemas
+ * that cannot be serialized (e.g. cyclic objects).
+ */
+export function canonicalJson(value: unknown): string | undefined {
+    try {
+        return JSON.stringify(sortJsonKeys(value));
+    } catch {
+        return undefined;
+    }
+}
+
+/**
+ * FIFO-bounded string-keyed cache. Evicts the oldest entry once `limit` is
+ * exceeded, so the cache cannot grow without bound. FIFO (rather than LRU) is
+ * deliberate: schema catalogs are refreshed wholesale, so recency is not a
+ * reliable signal of reuse.
+ */
+export function createBoundedCache<T>(limit: number): {
+    get(key: string): T | undefined;
+    set(key: string, value: T): void;
+} {
+    const entries = new Map<string, T>();
+    return {
+        get(key: string): T | undefined {
+            return entries.get(key);
+        },
+        set(key: string, value: T): void {
+            entries.set(key, value);
+            if (entries.size > limit) {
+                const oldest = entries.keys().next();
+                if (!oldest.done) {
+                    entries.delete(oldest.value);
+                }
+            }
+        }
+    };
+}

packages/core-internal/test/validators/ajvProviderCache.test.ts

@@ -0,0 +1,128 @@
+import { describe, expect, it } from 'vitest';
+
+import { AjvJsonSchemaValidator } from '../../src/validators/ajvProvider';
+
+/**
+ * Regression tests for #2605: `getValidator()` must not recompile a schema
+ * that has no `$id` on every call. Repeated `Client.listTools()` refreshes
+ * call `getValidator` with structurally identical schemas; without caching,
+ * each call compiles a fresh validator that the AJV engine retains forever,
+ * so the heap grows without bound in long-running clients.
+ */
+
+function makeFakeEngine() {
+    let compiles = 0;
+    let getSchemaCalls = 0;
+    const engine = {
+        compile: () => {
+            compiles += 1;
+            return Object.assign(() => true, { errors: undefined });
+        },
+        getSchema: () => {
+            getSchemaCalls += 1;
+            // eslint-disable-next-line unicorn/no-useless-undefined -- AjvLike.getSchema must return undefined
+            return undefined;
+        },
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        errorsText: (_errors?: any) => ''
+    };
+    return { engine, compileCount: () => compiles, getSchemaCount: () => getSchemaCalls };
+}
+
+describe('AjvJsonSchemaValidator validator caching', () => {
+    it('compiles a schema without $id only once across repeated getValidator calls', () => {
+        const { engine, compileCount } = makeFakeEngine();
+        const provider = new AjvJsonSchemaValidator(engine);
+        const schema = { type: 'object', properties: { a: { type: 'string' } } };
+
+        provider.getValidator(schema);
+        provider.getValidator(schema);
+
+        expect(compileCount()).toBe(1);
+    });
+
+    it('hits the cache for structurally identical schemas with different object identity', () => {
+        const { engine, compileCount } = makeFakeEngine();
+        const provider = new AjvJsonSchemaValidator(engine);
+
+        provider.getValidator({ type: 'object', properties: { a: { type: 'string' } } });
+        provider.getValidator({ type: 'object', properties: { a: { type: 'string' } } });
+
+        expect(compileCount()).toBe(1);
+    });
+
+    it('compiles distinct schemas independently', () => {
+        const { engine, compileCount } = makeFakeEngine();
+        const provider = new AjvJsonSchemaValidator(engine);
+
+        provider.getValidator({ type: 'string' });
+        provider.getValidator({ type: 'number' });
+        provider.getValidator({ type: 'object' });
+
+        expect(compileCount()).toBe(3);
+    });
+
+    it('shares one compilation for schemas that differ only in key order', () => {
+        const { engine, compileCount } = makeFakeEngine();
+        const provider = new AjvJsonSchemaValidator(engine);
+
+        provider.getValidator({ type: 'object', properties: { a: { type: 'string' } } });
+        provider.getValidator({ properties: { a: { type: 'string' } }, type: 'object' });
+
+        expect(compileCount()).toBe(1);
+    });
+
+    it('is not poisoned when a caller mutates its schema object in place', () => {
+        const { engine, compileCount } = makeFakeEngine();
+        const provider = new AjvJsonSchemaValidator(engine);
+
+        const schema: Record<string, unknown> = {
+            type: 'object',
+            properties: { a: { type: 'string' } }
+        };
+        provider.getValidator(schema);
+        expect(compileCount()).toBe(1);
+
+        // Mutate the same object in place — a fresh content key must trigger a
+        // fresh compilation instead of reusing the stale validator for the
+        // original content.
+        schema.properties = { b: { type: 'number' } };
+        provider.getValidator(schema);
+
+        expect(compileCount()).toBe(2);
+    });
+
+    it('falls back to compiling without caching when a schema is not serializable', () => {
+        const { engine, compileCount } = makeFakeEngine();
+        const provider = new AjvJsonSchemaValidator(engine);
+
+        const cyclic: Record<string, unknown> = { type: 'object' };
+        cyclic.self = cyclic;
+
+        expect(() => provider.getValidator(cyclic)).not.toThrow();
+        expect(compileCount()).toBe(1);
+    });
+
+    it('keeps using the $id-based lookup for schemas with an $id', () => {
+        const { engine, compileCount, getSchemaCount } = makeFakeEngine();
+        const provider = new AjvJsonSchemaValidator(engine);
+        const schema = { $id: 'https://example.com/schema', type: 'string' };
+
+        provider.getValidator(schema);
+        provider.getValidator(schema);
+
+        expect(getSchemaCount()).toBe(2);
+        // getSchema always misses in the fake engine, so each call compiles once —
+        // the $id path is unchanged by this fix.
+        expect(compileCount()).toBe(2);
+    });
+
+    it('returns a working validator after caching', () => {
+        const provider = new AjvJsonSchemaValidator();
+        const schema = { type: 'object', properties: { a: { type: 'string' } } };
+
+        const validate = provider.getValidator<{ a?: string }>(schema);
+        expect(validate({ a: 'x' }).valid).toBe(true);
+        expect(validate({ a: 1 }).valid).toBe(false);
+    });
+});

packages/core-internal/test/validators/schemaCache.test.ts

@@ -0,0 +1,73 @@
+import { describe, expect, it } from 'vitest';
+
+import { canonicalJson, createBoundedCache } from '../../src/validators/schemaCache';
+
+describe('createBoundedCache', () => {
+    it('returns undefined for missing keys', () => {
+        const cache = createBoundedCache<string>(10);
+        expect(cache.get('nope')).toBeUndefined();
+    });
+
+    it('stores and returns values', () => {
+        const cache = createBoundedCache<string>(10);
+        cache.set('a', '1');
+        expect(cache.get('a')).toBe('1');
+    });
+
+    it('evicts the oldest entry beyond the limit (FIFO)', () => {
+        const cache = createBoundedCache<string>(3);
+        cache.set('a', '1');
+        cache.set('b', '2');
+        cache.set('c', '3');
+        cache.set('d', '4'); // evicts 'a'
+
+        expect(cache.get('a')).toBeUndefined();
+        expect(cache.get('b')).toBe('2');
+        expect(cache.get('c')).toBe('3');
+        expect(cache.get('d')).toBe('4');
+    });
+
+    it('keeps only the most recent entries after repeated overflow', () => {
+        const cache = createBoundedCache<string>(2);
+        for (let i = 0; i < 5; i++) {
+            cache.set(`k${i}`, String(i));
+        }
+        expect(cache.get('k0')).toBeUndefined();
+        expect(cache.get('k3')).toBe('3');
+        expect(cache.get('k4')).toBe('4');
+    });
+
+    it('overwriting an existing key does not count as a new entry', () => {
+        const cache = createBoundedCache<string>(2);
+        cache.set('a', '1');
+        cache.set('b', '2');
+        cache.set('a', '1-updated'); // update, size still 2
+        cache.set('c', '3'); // evicts 'a' (oldest insertion)
+
+        expect(cache.get('a')).toBeUndefined();
+        expect(cache.get('b')).toBe('2');
+        expect(cache.get('c')).toBe('3');
+    });
+});
+
+describe('canonicalJson', () => {
+    it('serializes with recursively sorted keys', () => {
+        expect(canonicalJson({ b: 1, a: { d: 4, c: 3 } })).toBe('{"a":{"c":3,"d":4},"b":1}');
+    });
+
+    it('produces the same key for structurally identical schemas regardless of key order', () => {
+        const first = canonicalJson({ type: 'object', properties: { a: { type: 'string' } } });
+        const second = canonicalJson({ properties: { a: { type: 'string' } }, type: 'object' });
+        expect(first).toBe(second);
+    });
+
+    it('returns undefined for cyclic objects', () => {
+        const cyclic: Record<string, unknown> = { type: 'object' };
+        cyclic.self = cyclic;
+        expect(canonicalJson(cyclic)).toBeUndefined();
+    });
+
+    it('handles arrays', () => {
+        expect(canonicalJson({ a: [1, { b: 2, a: 1 }] })).toBe('{"a":[1,{"a":1,"b":2}]}');
+    });
+});