MCP Events: Zod schemas and type exports

[elijahr]

Summary

• All event protocol types as Zod schemas (EventParams, EventEmitNotification, Subscribe/Unsubscribe/List)
• Wired into ServerNotification, ClientRequest, ServerResult, ServerCapabilities unions
• Pagination support for events/list (PaginatedRequest/Result)
• EventsCapability uses looseObject for extensibility
• 52 tests with rejection coverage for all schemas

Context

TypeScript protocol types for MCP events SEP reference implementation.
Mirrors the python-sdk event types as Zod schemas.

Test plan

• pnpm -r test

[gemini-code-assist]

Code Review

This pull request introduces a comprehensive event system to the core package, adding Zod schemas for event effects, topics, capabilities, and various request/response/notification types such as subscribe, unsubscribe, list, and emit. It also includes corresponding TypeScript type exports and a new test suite. Feedback suggests enhancing validation for date-time strings using Zod's datetime validator and ensuring that subscription and unsubscription topic arrays are non-empty to prevent unintentional empty requests.

[gemini-code-assist]

For improved type safety and validation, consider using a more specific schema for timestamp strings. Zod's datetime validator can ensure the strings are valid ISO 8601 datetimes with timezone offsets.

I suggest changing z.string().optional() to z.string().datetime({ offset: true }).optional() for both timestamp and expires_at fields.

Suggested change

	timestamp: z.string().optional(),
	retained: z.boolean().optional(),
	source: z.string().optional(),
	correlation_id: z.string().optional(),
	requested_effects: z.array(EventEffectSchema).optional(),
	expires_at: z.string().optional(),
	timestamp: z.string().datetime({ offset: true }).optional(),
	retained: z.boolean().optional(),
	source: z.string().optional(),
	correlation_id: z.string().optional(),
	requested_effects: z.array(EventEffectSchema).optional(),
	expires_at: z.string().datetime({ offset: true }).optional(),

[gemini-code-assist]

A request to subscribe to an empty list of topics is a valid operation but might not be intentional. To enforce that at least one topic is provided for subscription, consider adding .nonempty() to the topics array schema. This would make the intent clearer and fail early for empty requests.

If you apply this change, please also update the tests in packages/core/test/event-schemas.test.ts to assert that an empty array is rejected.

Suggested change

	topics: z.array(z.string()),
	topics: z.array(z.string()).nonempty(),

[gemini-code-assist]

For improved type safety and to ensure a valid date-time format, I recommend using Zod's datetime validator for the timestamp field. This ensures the value is a valid ISO 8601 string with a timezone offset.

Suggested change

	timestamp: z.string().optional(),
	timestamp: z.string().datetime({ offset: true }).optional(),

[gemini-code-assist]

Similar to the subscription parameters, a request to unsubscribe from an empty list of topics is likely not a meaningful operation. To ensure the request is intentional and contains topics to unsubscribe from, I suggest adding .nonempty() to the topics array schema.

Suggested change

	topics: z.array(z.string()),
	topics: z.array(z.string()).nonempty(),

[elijahr]

@gemini-code-assist please re-review

[gemini-code-assist]

Code Review

This pull request introduces a comprehensive event system to the core package, adding Zod schemas and TypeScript types for event subscriptions, emissions, and topic management. It also includes a new test suite to validate these schemas. The review feedback identifies naming inconsistencies in the new schemas, specifically recommending the use of camelCase instead of snake_case for properties such as eventId, correlationId, and requestedEffects to align with the rest of the SDK.

[gemini-code-assist]

The property names in EventParamsSchema use snake_case (event_id, correlation_id, requested_effects, expires_at), which is inconsistent with the camelCase naming convention used throughout the rest of the MCP TypeScript SDK (e.g., requestId, taskId, lastModified, websiteUrl). To maintain consistency and follow TypeScript/JavaScript idioms, these should be renamed to camelCase. The documentation comment should also be updated to use relatedRequestId.

Suggested change

	/**
	* Parameters for events/emit notification.
	* Extends NotificationsParamsSchema to inherit _meta for related_request_id tracking.
	*/
	export const EventParamsSchema = NotificationsParamsSchema.extend({
	topic: z.string(),
	event_id: z.string(),
	payload: z.unknown(),
	timestamp: z.iso.datetime({ offset: true }).optional(),
	retained: z.boolean().optional(),
	source: z.string().optional(),
	correlation_id: z.string().optional(),
	requested_effects: z.array(EventEffectSchema).optional(),
	expires_at: z.iso.datetime({ offset: true }).optional(),
	});
	/**
	* Parameters for events/emit notification.
	* Extends NotificationsParamsSchema to inherit _meta for relatedRequestId tracking.
	*/
	export const EventParamsSchema = NotificationsParamsSchema.extend({
	topic: z.string(),
	eventId: z.string(),
	payload: z.unknown(),
	timestamp: z.iso.datetime({ offset: true }).optional(),
	retained: z.boolean().optional(),
	source: z.string().optional(),
	correlationId: z.string().optional(),
	requestedEffects: z.array(EventEffectSchema).optional(),
	expiresAt: z.iso.datetime({ offset: true }).optional(),
	});

[gemini-code-assist]

Rename event_id to eventId to match the camelCase naming convention used in the rest of the project.

Suggested change

	export const RetainedEventSchema = z.object({
	topic: z.string(),
	event_id: z.string(),
	timestamp: z.iso.datetime({ offset: true }).optional(),
	payload: z.unknown(),
	});
	export const RetainedEventSchema = z.object({
	topic: z.string(),
	eventId: z.string(),
	timestamp: z.iso.datetime({ offset: true }).optional(),
	payload: z.unknown(),
	});

[elijahr]

@gemini-code-assist please re-review

[gemini-code-assist]

Code Review

This pull request introduces a new "Events" capability to the MCP SDK, including Zod schemas, TypeScript types, and comprehensive tests for event subscription, unsubscription, listing, and emission. Feedback focuses on aligning field naming with the camelCase convention used in the project, ensuring protocol compliance by using JSONValueSchema for payloads, and extending base request schemas to support standard metadata. Additionally, it was noted that manual changes to spec.types.ts should be avoided as it is an auto-generated file.

[gemini-code-assist]

The field names in EventParamsSchema should follow the camelCase convention used throughout the MCP specification and this repository (e.g., eventId instead of event_id, correlationId instead of correlation_id). Additionally, the payload field should use JSONValueSchema to ensure protocol compliance by only allowing JSON-serializable data, consistent with other generic payload fields in the SDK.

Suggested change

	export const EventParamsSchema = NotificationsParamsSchema.extend({
	topic: z.string(),
	event_id: z.string(),
	payload: z.unknown(),
	timestamp: z.iso.datetime({ offset: true }).optional(),
	retained: z.boolean().optional(),
	source: z.string().optional(),
	correlation_id: z.string().optional(),
	requested_effects: z.array(EventEffectSchema).optional(),
	expires_at: z.iso.datetime({ offset: true }).optional(),
	});
	export const EventParamsSchema = NotificationsParamsSchema.extend({
	topic: z.string(),
	eventId: z.string(),
	payload: JSONValueSchema,
	timestamp: z.iso.datetime({ offset: true }).optional(),
	retained: z.boolean().optional(),
	source: z.string().optional(),
	correlationId: z.string().optional(),
	requestedEffects: z.array(EventEffectSchema).optional(),
	expiresAt: z.iso.datetime({ offset: true }).optional(),
	});

[gemini-code-assist]

Request parameter schemas should extend BaseRequestParamsSchema to inherit the _meta field. This is important for supporting protocol-level features like progress notifications and task association, which are standard across MCP requests.

Suggested change

	export const EventSubscribeParamsSchema = z.object({
	topics: z.array(z.string()),
	});
	export const EventSubscribeParamsSchema = BaseRequestParamsSchema.extend({
	topics: z.array(z.string()),
	});

[gemini-code-assist]

Field names should be camelCase (eventId) and payload should use JSONValueSchema for consistency and strict protocol validation.

Suggested change

	export const RetainedEventSchema = z.object({
	topic: z.string(),
	event_id: z.string(),
	timestamp: z.iso.datetime({ offset: true }).optional(),
	payload: z.unknown(),
	});
	export const RetainedEventSchema = z.object({
	topic: z.string(),
	eventId: z.string(),
	timestamp: z.iso.datetime({ offset: true }).optional(),
	payload: JSONValueSchema,
	});

[gemini-code-assist]

Request parameter schemas should extend BaseRequestParamsSchema to inherit the _meta field.

Suggested change

	export const EventUnsubscribeParamsSchema = z.object({
	topics: z.array(z.string()),
	});
	export const EventUnsubscribeParamsSchema = BaseRequestParamsSchema.extend({
	topics: z.array(z.string()),
	});

[gemini-code-assist]

The file spec.types.ts is automatically generated and contains a strict warning against manual edits. Manual changes will be overwritten during the next update cycle. These types should instead be defined in schemas.ts and inferred in types.ts until they are officially incorporated into the upstream specification and the generator is updated.

[elijahr]

@gemini-code-assist please re-review

[gemini-code-assist]

Code Review

This pull request introduces a new event system to the core package, enabling servers to publish events to clients. It adds comprehensive Zod schemas and TypeScript interfaces for event topics, subscriptions, unsubscriptions, and notifications. These new structures are integrated into the existing server capabilities and message unions, and a new test suite is provided to ensure schema validation and integration work as expected. I have no feedback to provide.

[elijahr]

@gemini-code-assist please re-review

[gemini-code-assist]

Code Review

This pull request introduces an experimental Events API to the Model Context Protocol (MCP), enabling servers to push asynchronous notifications to clients via named topics. The changes encompass new documentation in docs/server.md, Zod schemas for protocol validation in packages/core, corresponding TypeScript interfaces, and a comprehensive test suite. The review feedback highlights opportunities to improve documentation regarding multi-segment wildcards and suggests refining several Zod schemas to ensure better type safety, correct field optionality, and alignment with the protocol's extensibility patterns.

[gemini-code-assist]

The documentation mentions the + wildcard for single-segment matching but omits the # wildcard for multi-segment matching, which is used extensively in the test suite (e.g., build/#). For clarity and completeness, both MQTT-style wildcards should be documented.

[gemini-code-assist]

There are three improvements for EventParamsSchema:

1. Optionality: The documentation states that the payload is optional, but the schema marks it as required. It should be made optional to match the spec.
2. Type Safety: Using JSONValueSchema instead of z.unknown() ensures that the payload is a valid JSON value, which is appropriate for a JSON-RPC protocol.
3. Extensibility: The base NotificationSchema makes its parameters loose by default. Overriding it with a strict schema here limits future protocol extensibility. Adding .loose() maintains consistency with the base schema.

Suggested change

	export const EventParamsSchema = NotificationsParamsSchema.extend({
	topic: z.string(),
	event_id: z.string(),
	payload: z.unknown(),
	timestamp: z.iso.datetime({ offset: true }).optional(),
	retained: z.boolean().optional(),
	source: z.string().optional(),
	correlation_id: z.string().optional(),
	requested_effects: z.array(EventEffectSchema).optional(),
	expires_at: z.iso.datetime({ offset: true }).optional(),
	});
	export const EventParamsSchema = NotificationsParamsSchema.extend({
	topic: z.string(),
	event_id: z.string(),
	payload: JSONValueSchema.optional(),
	timestamp: z.iso.datetime({ offset: true }).optional(),
	retained: z.boolean().optional(),
	source: z.string().optional(),
	correlation_id: z.string().optional(),
	requested_effects: z.array(EventEffectSchema).optional(),
	expires_at: z.iso.datetime({ offset: true }).optional(),
	}).loose();

[gemini-code-assist]

The base RequestSchema makes its parameters loose by default to allow for protocol extensions. Overriding it with a strict schema here prevents future additions to the events/subscribe parameters. It is recommended to make this schema loose for consistency.

Suggested change

	export const EventSubscribeParamsSchema = z.object({
	topics: z.array(z.string()),
	});
	export const EventSubscribeParamsSchema = z.object({
	topics: z.array(z.string()),
	}).loose();

[gemini-code-assist]

Similar to EventParamsSchema, the payload in RetainedEventSchema should be optional and use JSONValueSchema for better type safety and consistency with the protocol documentation.

Suggested change

	export const RetainedEventSchema = z.object({
	topic: z.string(),
	event_id: z.string(),
	timestamp: z.iso.datetime({ offset: true }).optional(),
	payload: z.unknown(),
	});
	export const RetainedEventSchema = z.object({
	topic: z.string(),
	event_id: z.string(),
	timestamp: z.iso.datetime({ offset: true }).optional(),
	payload: JSONValueSchema.optional(),
	});

[gemini-code-assist]

For consistency with other request parameter schemas and to allow for future protocol extensions, EventUnsubscribeParamsSchema should be made loose.

Suggested change

	export const EventUnsubscribeParamsSchema = z.object({
	topics: z.array(z.string()),
	});
	export const EventUnsubscribeParamsSchema = z.object({
	topics: z.array(z.string()),
	}).loose();

[elijahr]

@gemini-code-assist please re-review

[gemini-code-assist]

Code Review

This pull request implements an experimental Events API, allowing servers to push asynchronous notifications to clients through a topic-based subscription model. The changes encompass detailed documentation, Zod schemas for protocol messages, and updated TypeScript definitions. Reviewer feedback highlights opportunities to strengthen validation by enforcing non-empty topic lists in subscription requests, improving the clarity of test descriptions, and ensuring consistent test coverage across subscription and unsubscription schemas.

[gemini-code-assist]

The topics array can currently be empty. While technically valid, allowing subscription to an empty list of topics can lead to ambiguous behavior and might be better treated as a client error. Consider making it a non-empty array to be more explicit about the expected input.

topics: z.array(z.string()).nonempty()

This change should also be applied to EventUnsubscribeParamsSchema for consistency.

If you apply this change, please also update the tests in packages/core/test/event-schemas.test.ts to assert that an empty array is rejected.

Suggested change

	topics: z.array(z.string())
	topics: z.array(z.string()).nonempty()

[gemini-code-assist]

The title of this test case is misleading. It says it should reject an empty topics array, but the test actually checks for the wrong type ('not-an-array'). An empty array [] would currently pass validation.

Please consider renaming this test to reflect what it's actually testing.

Suggested change

	it('should reject empty topics array', () => {
	it('should reject wrong type for topics array', () => {

[gemini-code-assist]

The tests for EventUnsubscribeParamsSchema are less comprehensive than for EventSubscribeParamsSchema. For consistency and better coverage, consider adding tests for invalid types, similar to what's done for EventSubscribeParamsSchema.

For example:

it('should reject wrong type for topics', () => {
    expect(() => EventUnsubscribeParamsSchema.parse({ topics: 'not-an-array' })).toThrow();
});

[elijahr]

@gemini-code-assist please re-review

[gemini-code-assist]

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

[elijahr]

@gemini-code-assist please re-review

[gemini-code-assist]

Code Review

This pull request introduces an experimental Events API to the Model Context Protocol, allowing servers to push asynchronous notifications to clients via named topics. The changes include new protocol schemas, client-side methods for subscription management, a priority-aware event queue, and provenance metadata for tracking event sources. Feedback focuses on improving the robustness of the McpEventQueue by handling edge cases where maxSize is zero and replacing flaky setTimeout calls in integration tests with promise-based synchronization.

[gemini-code-assist]

There's a potential crash in enqueue if maxSize is set to 0. In that scenario, _findMinPriorityIndex would be called on an empty queue, leading to an error when this.priorityFn(this.queue[0]) is executed because this.queue[0] would be undefined.

To prevent this, I suggest adding a guard to handle cases where maxSize is not a positive number.

On a related note, the current implementation of enqueue has a time complexity of O(N) when the queue is full, and drain is O(N log N). This is likely acceptable for the default queue size, but if this becomes a performance bottleneck, consider using a more efficient data structure like a binary heap.

Suggested change

	enqueue(envelope: ProvenanceEnvelope): boolean {
	if (this.queue.length >= this.maxSize) {
	const minIdx = this._findMinPriorityIndex();
	const minPriority = this.priorityFn(this.queue[minIdx]);
	const newPriority = this.priorityFn(envelope);
	if (newPriority > minPriority) {
	this.queue.splice(minIdx, 1);
	} else {
	return false;
	}
	}
	this.queue.push(envelope);
	return true;
	}
	enqueue(envelope: ProvenanceEnvelope): boolean {
	if (this.maxSize <= 0) {
	return false;
	}
	if (this.queue.length >= this.maxSize) {
	const minIdx = this._findMinPriorityIndex();
	const minPriority = this.priorityFn(this.queue[minIdx]);
	const newPriority = this.priorityFn(envelope);
	if (newPriority > minPriority) {
	this.queue.splice(minIdx, 1);
	} else {
	return false;
	}
	}
	this.queue.push(envelope);
	return true;
	}

[gemini-code-assist]

Using a fixed setTimeout to wait for asynchronous operations can lead to flaky tests. If the event propagation takes longer than the specified 50ms for any reason, the test will fail. A more robust approach is to use a Promise that is resolved by the event handler, which ensures the test waits exactly as long as needed.

    it('onEvent() should receive events/emit notifications', async () => {
        const receivedPromise = new Promise<EventParams>(resolve => {
            client.onEvent(event => {
                resolve(event);
            });
        });

        // Server emits an event notification
        await server.notification({
            method: 'events/emit',
            params: {
                topic: 'myapp/status',
                payload: { status: 'ok' },
                event_id: 'evt-1',
            },
        });

        const receivedEvent = await receivedPromise;

        expect(receivedEvent.topic).toBe('myapp/status');
        expect(receivedEvent.payload).toEqual({ status: 'ok' });
    });

[elijahr]

@gemini-code-assist please re-review

[gemini-code-assist]

Code Review

This pull request introduces an experimental "Events" API, allowing servers to push asynchronous notifications to clients on named topics. This includes new protocol schemas, client-side methods for subscribing, unsubscribing, listing events, and handling incoming event notifications. Additionally, a ProvenanceEnvelope for client-side event metadata and a McpEventQueue for priority-aware event handling have been added, along with comprehensive documentation and tests. Review feedback suggests refactoring the _findMinPriorityIndex method in McpEventQueue for improved readability and type safety, and enhancing the escapeXml function in ProvenanceEnvelope to also escape single quotes for better robustness.

[gemini-code-assist]

This method can be refactored for better readability and type safety by removing the need for non-null assertions (!) and the temporary minPri variable. This makes the code cleaner and less prone to errors if the calling logic changes.

Suggested change

	private _findMinPriorityIndex(): number {
	let minIdx = 0;
	// queue is non-empty when this is called (only called after length >= maxSize check)
	// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
	let minPri = this.priorityFn(this.queue[0]!);
	for (let i = 1; i < this.queue.length; i++) {
	// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
	const pri = this.priorityFn(this.queue[i]!);
	if (pri < minPri) {
	minPri = pri;
	minIdx = i;
	}
	}
	return minIdx;
	}
	private _findMinPriorityIndex(): number {
	if (this.isEmpty) {
	// This path should be unreachable given the current call site in `enqueue`.
	// Throwing an error makes it fail-fast if logic changes.
	throw new Error('Cannot find minimum priority in an empty queue.');
	}
	let minIndex = 0;
	for (let i = 1; i < this.queue.length; i++) {
	if (this.priorityFn(this.queue[i]) < this.priorityFn(this.queue[minIndex])) {
	minIndex = i;
	}
	}
	return minIndex;
	}

[gemini-code-assist]

The escapeXml function doesn't handle single quotes ('). While the current XML generation uses double quotes for attributes, it's more robust to escape single quotes as '. This prevents potential rendering issues or injection vulnerabilities if the XML is processed by other tools or if single quotes are used in attributes in the future.

Suggested change

	function escapeXml(s: string): string {
	return s
	.replace(/&/g, '&')
	.replace(/</g, '&lt;')
	.replace(/>/g, '&gt;')
	.replace(/"/g, '&quot;');
	}
	function escapeXml(s: string): string {
	return s
	.replace(/&/g, '&amp;')
	.replace(/</g, '&lt;')
	.replace(/>/g, '&gt;')
	.replace(/"/g, '&quot;')
	.replace(/'/g, '&apos;');
	}

[elijahr]

@gemini-code-assist please re-review

[gemini-code-assist]

Code Review

This pull request introduces an experimental Events API to the Model Context Protocol, enabling servers to push asynchronous notifications to clients via named topics. The changes include protocol-level schema definitions, client-side methods for event subscription and management, and a priority-aware event queue implementation. My review identified a potential runtime error in the event queue constructor due to lack of input validation for maxSize, and a recommendation to improve the robustness of the XML escaping utility by including single-quote handling.

[gemini-code-assist]

The maxSize option is not validated. If a user provides maxSize: 0 or a negative number, the enqueue method can cause a runtime error. Specifically, if maxSize is 0, this.queue.length >= this.maxSize will be true for an empty queue, causing _findMinPriorityIndex to be called on an empty queue, which will then attempt to access this.queue[0], resulting in an error.

It's recommended to validate that maxSize is a positive number in the constructor. Consider also adding a test case to packages/core/test/shared/eventQueue.test.ts to verify that an invalid maxSize throws an error.

        this.maxSize = options?.maxSize ?? 1000;
        if (this.maxSize <= 0) {
            throw new Error('McpEventQueue maxSize must be a positive number.');
        }
        this.priorityFn = options?.priorityFn ?? (() => 0);

[gemini-code-assist]

The escapeXml function is incomplete. It should also escape single quotes (') to be fully robust, especially if the generated XML is ever consumed by processors that might have issues with unescaped apostrophes in attribute values, or if this function is reused in a context where attributes might be single-quoted. The standard XML entity for a single quote is '.

Consider adding a test case to packages/core/test/shared/provenance.test.ts to cover escaping of single quotes.

    return s
        .replaceAll('&', '&')
        .replaceAll('<', '&lt;')
        .replaceAll('>', '&gt;')
        .replaceAll('"', '&quot;')
        .replaceAll('\'', '&apos;');

[elijahr]

@gemini-code-assist please re-review

[gemini-code-assist]

Code Review

This pull request introduces an experimental Events API to the Model Context Protocol (MCP), enabling servers to push asynchronous notifications to clients. The changes include comprehensive documentation, new Zod schemas, and TypeScript types for event-related messages. Core additions include a priority-aware McpEventQueue and a ProvenanceEnvelope for tracking event metadata and rendering events as XML. The Client class has been updated with methods to subscribe, unsubscribe, list events, and register event handlers. A potential runtime error was identified in the XML rendering logic when processing undefined event payloads.

[gemini-code-assist]

The JSON.stringify(this.event.payload) call can return undefined if this.event.payload is undefined. Since escapeXml expects a string, this will cause a runtime error (e.g., TypeError: Cannot read properties of undefined (reading 'replaceAll')). Given that payload is optional in the protocol schema, this is a likely scenario.

        const payloadXml =
            typeof this.event.payload === 'string' ? escapeXml(this.event.payload) : escapeXml(JSON.stringify(this.event.payload) ?? '');

[elijahr]

@gemini-code-assist please re-review

[gemini-code-assist]

Code Review

This pull request introduces an experimental Events API to the Model Context Protocol, enabling servers to push asynchronous notifications to clients via named topics. The changes include new protocol schemas for event subscription, unsubscription, and emission, as well as client-side utilities like a priority-aware event queue and provenance metadata for tracking event origins. Feedback focuses on improving the robustness of the XML rendering for events with missing payloads, preventing potential crashes in the event queue when initialized with a zero size, and addressing performance inefficiencies in the queue's draining logic.

[gemini-code-assist]

The toXml method will throw a TypeError if this.event.payload is undefined. This occurs because JSON.stringify(undefined) returns undefined, and escapeXml expects a string input. Since the EventParams schema defines payload as optional, events without a payload are valid and will cause this method to crash when rendered. You should handle the undefined case to ensure the method is robust.

Suggested change

	const payloadXml =
	typeof this.event.payload === 'string' ? escapeXml(this.event.payload) : escapeXml(JSON.stringify(this.event.payload));
	const payload = this.event.payload;
	const payloadString = typeof payload === 'string' ? payload : JSON.stringify(payload);
	const payloadXml = payloadString !== undefined ? escapeXml(payloadString) : '';

[gemini-code-assist]

The McpEventQueue constructor does not validate the maxSize option. If a user provides maxSize: 0, the enqueue method will crash on the first call because the condition this.queue.length >= this.maxSize (0 >= 0) will be true, leading to _findMinPriorityIndex being called on an empty array. This results in this.queue[0] being undefined, which causes a crash when passed to priorityFn. Consider enforcing a minimum maxSize of 1 or handling the zero-size case explicitly in enqueue.

Suggested change

	constructor(options?: McpEventQueueOptions) {
	this.maxSize = options?.maxSize ?? 1000;
	this.priorityFn = options?.priorityFn ?? (() => 0);
	}
	constructor(options?: McpEventQueueOptions) {
	this.maxSize = Math.max(1, options?.maxSize ?? 1000);
	this.priorityFn = options?.priorityFn ?? (() => 0);
	}

[gemini-code-assist]

The drain implementation sorts the entire queue on every call, which is $O(N \log N)$. If drain is called frequently with a small count (e.g., to process events one by one), this becomes inefficient ($O(N^2 \log N)$ total for draining the whole queue). Since enqueue is already $O(N)$ when the queue is full (due to finding the minimum priority index), you could maintain the array in a sorted state during enqueue to make drain an $O(1)$ or $O(count)$ operation, or use a binary heap for $O(\log N)$ performance on both operations.

[elijahr]

@gemini-code-assist please re-review

[gemini-code-assist]

Code Review

This pull request introduces an experimental Events API to the Model Context Protocol, enabling servers to push asynchronous notifications to clients via named topics. The changes include new protocol schemas for event subscription, unsubscription, and listing, as well as client-side support for handling these events. Additionally, a priority-aware McpEventQueue and a ProvenanceEnvelope for tracking event metadata and rendering XML for LLM context injection have been added. Feedback focuses on improving the robustness of the McpEventQueue against zero-sized configurations, enhancing XML escaping in the provenance utility, and replacing flaky timeouts in integration tests with promise-based synchronization.

[gemini-code-assist]

There's a potential crash in the enqueue method if maxSize is initialized to 0. When maxSize is 0, the condition this.queue.length >= this.maxSize becomes 0 >= 0, which is true. This leads to _findMinPriorityIndex being called on an empty queue, causing a crash when it tries to access this.queue[0].

You can fix this by adding a guard at the beginning of the enqueue method to handle the maxSize: 0 case.

    enqueue(envelope: ProvenanceEnvelope): boolean {
        if (this.maxSize === 0) {
            return false;
        }
        if (this.queue.length >= this.maxSize) {
            const minIdx = this._findMinPriorityIndex();
            // queue[minIdx] is always defined: queue is non-empty (length >= maxSize >= 1)
            // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
            const minPriority = this.priorityFn(this.queue[minIdx]!);
            const newPriority = this.priorityFn(envelope);
            if (newPriority > minPriority) {
                this.queue.splice(minIdx, 1);
            } else {
                return false;
            }
        }
        this.queue.push(envelope);
        return true;
    }

[gemini-code-assist]

The escapeXml function doesn't escape single quotes ('). While not strictly necessary when attribute values are double-quoted, it's a good practice to escape them to ' for more robust XML generation, especially if the generated XML might be consumed by systems that are stricter about this.

Suggested change

	function escapeXml(s: string): string {
	return s.replaceAll('&', '&amp;').replaceAll('<', '&lt;').replaceAll('>', '&gt;').replaceAll('"', '&quot;');
	}
	function escapeXml(s: string): string {
	return s.replaceAll('&', '&amp;').replaceAll('<', '&lt;').replaceAll('>', '&gt;').replaceAll('"', '&quot;').replaceAll("'", "&apos;");
	}

[gemini-code-assist]

This test uses setTimeout to wait for the event notification to be processed. This can lead to flaky tests if the propagation takes longer than the timeout. A more robust approach is to use a promise that resolves when the event handler is called.

    it('onEvent() should receive events/emit notifications', async () => {
        const receivedPromise = new Promise<EventParams>(resolve => {
            client.onEvent(event => {
                resolve(event);
            });
        });

        // Server emits an event notification
        await server.notification({
            method: 'events/emit',
            params: {
                topic: 'myapp/status',
                payload: { status: 'ok' },
                event_id: 'evt-1'
            }
        });

        const received = await receivedPromise;

        expect(received.topic).toBe('myapp/status');
        expect(received.payload).toEqual({ status: 'ok' });
    });

[elijahr]

@gemini-code-assist please re-review

[gemini-code-assist]

Code Review

This pull request introduces an experimental Events API to the Model Context Protocol, enabling servers to push asynchronous notifications to clients via named topics. The implementation includes new client methods for event subscription and management, Zod schemas for protocol validation, and utility classes for priority-aware event queuing and provenance tracking. Feedback was provided regarding the performance of the McpEventQueue.drain method, which currently sorts the entire queue on every call regardless of the requested count.

[gemini-code-assist]

The drain method sorts the entire queue on every call, which is inefficient if count is provided and much smaller than the queue size. For a queue of up to 1000 items, this results in $O(N \log N)$ complexity per drain. If drain(count) is called repeatedly in a loop to process events, it could lead to $O(N^2 \log N)$ total time. Consider using a more efficient data structure like a binary heap for the priority queue, or at least skip sorting if the priorityFn is the default no-op function.

[elijahr]

@gemini-code-assist please re-review

[gemini-code-assist]

Code Review

This pull request introduces an experimental Events API to the Model Context Protocol, enabling servers to push asynchronous notifications to clients via named topics. The changes include comprehensive documentation, new Zod schemas for protocol messages, and client-side methods for managing subscriptions and event handlers. Additionally, the PR adds utility classes for tracking event provenance and a priority-aware event queue. Review feedback suggests enhancing the robustness of the client-side event listener with runtime validation and optimizing the event queue's performance by using a more efficient data structure like a min-heap.

[gemini-code-assist]

The onEvent handler uses a type cast as EventParams for incoming notification parameters. This is unsafe as it doesn't perform runtime validation on data received from the server. A malformed events/emit notification could lead to runtime errors in the handler.

To improve robustness, I recommend validating the notification parameters against the EventParamsSchema before passing them to the user-provided handler. You can use EventParamsSchema.safeParse() for this.

Suggested change

	this.setNotificationHandler('events/emit', notification => {
	handler(notification.params as EventParams);
	});
	this.setNotificationHandler('events/emit', notification => {
	const parsed = EventParamsSchema.safeParse(notification.params);
	if (parsed.success) {
	handler(parsed.data);
	} else {
	this.log('error', 'Received invalid events/emit notification from server', { error: parsed.error });
	}
	});

[gemini-code-assist]

The current implementation of McpEventQueue uses a plain array, which leads to suboptimal performance for a priority queue. Specifically:

• enqueue() is O(N) when the queue is full due to _findMinPriorityIndex() iterating the entire array.
• drain() is O(N log N) due to this.queue.sort().

For use cases with a high volume of events or large queue sizes, this could become a performance bottleneck. Consider using a more efficient data structure like a min-heap, which would offer O(log N) for enqueue and O(k log N) for draining k items. There are several well-tested third-party libraries available for this (e.g., tinyqueue or js-priority-queue).

[elijahr]

@gemini-code-assist please re-review

[gemini-code-assist]

Code Review

This pull request introduces an experimental Events API to the Model Context Protocol, allowing servers to broadcast asynchronous notifications to clients. Key changes include new Zod schemas, documentation, and core utilities like a priority-aware event queue and provenance tracking. Feedback suggests adding schema validation to the client's event handler to prevent runtime errors and optimizing the event queue's sorting and eviction logic to improve performance under high throughput.

[gemini-code-assist]

The onEvent convenience method registers a handler for events/emit notifications but does not perform any schema validation on the incoming parameters. Since notification.params is typed as optional and loose in the base protocol, it could be undefined or malformed at runtime, potentially causing crashes in user-provided handlers that expect a valid EventParams object. It is recommended to validate the parameters against EventParamsSchema before invoking the handler.

    onEvent(handler: (event: EventParams) => void): void {
        this.setNotificationHandler('events/emit', notification => {
            const result = parseSchema(EventParamsSchema, notification.params);
            if (result.success) {
                handler(result.data);
            }
        });
    }

[gemini-code-assist]

The drain method sorts the entire queue every time it is called. With a default maxSize of 1000, this results in $O(N \log N)$ complexity per call. If drain is called frequently with a small count (e.g., to process events one by one), this becomes a significant performance bottleneck ($O(N^2 \log N)$ to empty the queue). Additionally, if the default priorityFn is used (which returns 0 for all items), the sort is entirely unnecessary as the queue already maintains FIFO order via push. Consider optimizing for the default case or using a more efficient data structure like a heap.

[gemini-code-assist]

The enqueue method performs an $O(N)$ search via _findMinPriorityIndex whenever the queue is full. While acceptable for small buffers, this makes enqueuing into a full queue inefficient. If this class is intended for high-throughput event processing, consider maintaining the queue in a sorted state or using a min-heap to allow $O(\log N)$ eviction and insertion.

[elijahr]

@gemini-code-assist please re-review

[gemini-code-assist]

Code Review

This pull request introduces an experimental Events API to the Model Context Protocol, enabling servers to push asynchronous notifications to clients. Key changes include the addition of protocol schemas for event subscription and emission, a priority-aware McpEventQueue for managing event flow, and a ProvenanceEnvelope class for tracking event metadata and rendering events as XML. The Client class has been updated with methods to subscribe to topics and register event handlers. Feedback focuses on enhancing runtime validation for incoming event notifications, optimizing the performance of the event queue, and ensuring that event identifiers are correctly captured in the XML representation.

[gemini-code-assist]

The EventParamsSchema is missing from the imports, but it is required if you intend to perform runtime validation of incoming event notifications in the onEvent method.

Suggested change

	EventListResultSchema,
	EventListResultSchema,
	EventParamsSchema,

[gemini-code-assist]

The onEvent method performs an unsafe type cast of notification.params to EventParams. Since notification.params is optional in the base JSON-RPC notification structure and could be malformed or missing required fields (like topic or event_id), this cast may lead to runtime errors in the user-provided handler. Consistent with the patterns used in setRequestHandler for elicitation and sampling, you should validate the parameters against EventParamsSchema before invoking the handler.

    onEvent(handler: (event: EventParams) => void): void {
        this.setNotificationHandler('events/emit', notification => {
            const result = parseSchema(EventParamsSchema, notification.params);
            if (result.success) {
                handler(result.data);
            }
        });
    }

[gemini-code-assist]

The drain method performs a full sort of the queue on every invocation, resulting in $O(N \log N)$ complexity. For a class explicitly named McpEventQueue and intended for priority management, this is inefficient, particularly if drain is called frequently with a small count. Additionally, the enqueue method performs a linear search ($O(N)$) to find the minimum priority item when the queue is full. Consider using a binary heap or maintaining a sorted array to achieve $O(\log N)$ complexity for insertions and extractions.

[gemini-code-assist]

The toXml method only includes an event_id attribute if provenance.original_event_id is explicitly set, ignoring the event_id property that is part of the MCP event itself. Since the protocol schemas (like EventParamsSchema) require an event_id, this implementation may result in the loss of the event's unique identifier in the XML representation used for LLM context. It is recommended to use this.event.event_id as the primary source or a fallback for the XML attribute.

Suggested change

	if (this.provenance.original_event_id) {
	attrs.push(`event_id="${escapeXml(this.provenance.original_event_id)}"`);
	}
	const eventId = this.provenance.original_event_id ?? this.event.event_id;
	if (eventId) {
	attrs.push(`event_id="${escapeXml(eventId)}"`);
	}