Add DeferChangedEvents() to McpServerPrimitiveCollection for batched change notifications

[Copilot]

Fixes #1688

Registering a batch of N tools/prompts/resources emits N list_changed wire notifications -- one per TryAdd -- instead of one. This can trip clients that debounce or rate-limit notifications, and is wasteful for progressive-disclosure patterns.

Changes

• DeferChangedEvents() on McpServerPrimitiveCollection<T> -- returns an IDisposable scope that suppresses Changed events. A single notification fires when the outermost scope disposes, only if at least one mutation occurred. Nesting is supported via a depth counter.
• Thread-safe implementation -- _activeDeferralScopes and _hasDeferredChangeEvents are guarded by _deferralLock. RaiseChanged() defers when _activeDeferralScopes > 0, and ChangeDeferralScope.Dispose() is idempotent via Interlocked.Exchange. Changed is invoked outside the lock.
• Unit tests covering: no-mutation scope, single/multiple mutations, mixed add+remove, Clear, nested scopes, idempotent dispose, no-handler, baseline non-deferred behavior, concurrent mutations, mutation racing with dispose, derived-type coalescing, exception-safety, and prompt/resource collection coverage.
• End-to-end tests in McpServerBuilderExtensionsToolsTests, McpServerBuilderExtensionsPromptsTests, and McpServerBuilderExtensionsResourcesTests asserting exactly one wire notification for a batch add.

Usage

using (options.ToolCollection.DeferChangedEvents())
{
    foreach (var tool in group.Tools)
        options.ToolCollection.TryAdd(tool);
} // exactly one notifications/tools/list_changed emitted here

[tarekgh]

Faithful implementation of the agreed design, and it picks up the gate-at-RaiseChanged and net-zero-fires points. One correctness issue on the concurrency handling, plus a small doc gap, noted inline.

[tarekgh]

A couple of tests are missing to back the review points, plus a few worth adding:

Required:

1. Concurrency test: N threads each TryAdd a distinct tool inside one shared DeferChanges() scope, then assert exactly one Changed and Count == N after all scopes dispose. Add a variant where one thread mutates while another disposes the last scope, asserting the notification is not lost. This exercises the thread-safety claim and pairs with the locking fix above (it may be flaky against the current lock-free code and becomes deterministic once locked).
2. Derived-type coalescing: a subclass that calls RaiseChanged() directly inside a scope, asserting it coalesces to one notification. Nothing currently covers the gate-at-RaiseChanged path that McpServerResourceCollection relies on.

Recommended:

3. End-to-end: every test counts the in-process Changed event, not the wire message. Add one through a real client/server (see the list_changed pattern in McpServerBuilderExtensionsToolsTests) that batches inside DeferChanges() and asserts the client receives exactly one notifications/tools/list_changed.
4. Exception-safety: mutate, throw inside the using block, then assert one Changed fired and the next mutation after the scope fires immediately (deferral fully reset). This validates the guarantee stated in the XML docs.
5. Prompt/resource coverage: all tests use McpServerTool; add one on a prompt or resource collection to confirm the uniform behavior the PR claims.

[jeffhandley]

@copilot Please address the following excellent feedback from @tarekgh:

1. Add DeferChangedEvents() to McpServerPrimitiveCollection for batched change notifications #1689 (comment), where it's proven that the Interlocked.Exchange approach was insufficient and we need to use a lock instead. That means we can use a bool for flagging when there are pending changes vs. the esoteric int approach. This also addresses the idempotent Changed invocation outside a lock feedback noted in Add batched change notifications to McpServerPrimitiveCollection<T> #1688 (comment)
2. Add DeferChangedEvents() to McpServerPrimitiveCollection for batched change notifications #1689 (comment), which requests adding a <remarks> blurb about threading behavior.
3. Add DeferChangedEvents() to McpServerPrimitiveCollection for batched change notifications #1689 (comment), which requests:
   a. Adding concurrency tests
   b. Derived-type coalescing, which is also captured as a design shortfall in the Add batched change notifications to McpServerPrimitiveCollection<T> #1688 (comment) comment on the backing issue (after this PR was created)
   c. Add end-to-end functional tests in the appropriate test classes that covers list_changed event payloads for all collections that gain this behavior by using this collection type
   d. Exception-exercising tests that assert the disposal still functions as expected raising the events

[jeffhandley]

@copilot add unit tests that verify out-of-order disposals result in the Changed event being raised once all deferrals have been disposed. This can be achieved by capturing the IDisposable instances and calling Dispose() rather than with using blocks.

These tests should ensure re-disposing the same deferral instance multiple times also does not result in the count decrementing more than once (potentially leading to the Changed event being raised while there are still undisposed deferrals.

With this, we should change the "outermost" phrasing to say once all deferrals have been disposed, as the nesting isn't a requirement.

If you find bugs with these new tests, fix them.

[Copilot]

@copilot add unit tests that verify out-of-order disposals result in the Changed event being raised once all deferrals have been disposed. This can be achieved by capturing the IDisposable instances...

Done in the latest commit. Added four new tests:

• DeferChangedEvents_OutOfOrderDisposal_FiresOnceWhenAllScopesDisposed -- two scopes created in order 1, 2, disposed in reverse order 2, 1; verifies Changed does not fire after scope 2 disposes (scope 1 still active) and fires exactly once after scope 1 disposes.
• DeferChangedEvents_DoubleDisposeSingleScope_DoesNotDecrementCountTwice -- scope 1 and scope 2 both active; scope 1 disposed twice; verifies Changed does not fire (scope 2 still active) and the second dispose of scope 1 is a no-op that does not decrement the counter.
• DeferChangedEvents_OutOfOrderDisposalNoMutation_DoesNotFireChanged -- same out-of-order pattern with no mutations; verifies no spurious Changed.
• DeferChangedEvents_Nested_FiresOnceWhenAllScopesDisposed -- renamed from ...OutermostDispose to match the updated contract.

Updated the DeferChangedEvents() XML docs to say "fires once all active scopes have been disposed" (removed "outermost" and the nesting framing), and updated the RaiseChanged() remarks similarly. All 30 tests pass.

[Copilot]

Pull request overview

Adds a batching mechanism to McpServerPrimitiveCollection<T> so multiple tool/prompt/resource registrations can be performed as one logical operation without emitting a Changed event (and corresponding list_changed wire notification) for every individual mutation.

Changes:

• Introduces DeferChangedEvents() with nested-scope support and a lock-guarded deferral state; emits at most one Changed when the outermost scope ends and at least one mutation occurred.
• Adds a comprehensive unit test suite for deferral semantics, nesting, idempotent disposal, concurrency, and derived-type RaiseChanged() behavior.
• Adds end-to-end tests for tools/prompts/resources ensuring a batch add emits exactly one wire notification.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/ModelContextProtocol.Core/Server/McpServerPrimitiveCollection.cs	Implements deferrable/coalesced Changed notifications via an IDisposable deferral scope.
tests/ModelContextProtocol.Tests/Server/McpServerPrimitiveCollectionTests.cs	Adds unit tests validating baseline and deferred Changed behavior, including concurrency and nesting.
tests/ModelContextProtocol.Tests/Configuration/McpServerBuilderExtensionsToolsTests.cs	Adds an end-to-end test asserting a batched tool add emits exactly one tools/list_changed notification.
tests/ModelContextProtocol.Tests/Configuration/McpServerBuilderExtensionsPromptsTests.cs	Adds an end-to-end test asserting a batched prompt add emits exactly one prompts/list_changed notification.
tests/ModelContextProtocol.Tests/Configuration/McpServerBuilderExtensionsResourcesTests.cs	Adds an end-to-end test asserting a batched resource add emits exactly one resources/list_changed notification.