Skip to content

Comments

Make UrlElicitationRequiredErrorData.Elicitations consistent with other Protocol DTOs#1335

Merged
stephentoub merged 3 commits intomainfrom
copilot/fix-inconsistent-collection-types
Feb 21, 2026
Merged

Make UrlElicitationRequiredErrorData.Elicitations consistent with other Protocol DTOs#1335
stephentoub merged 3 commits intomainfrom
copilot/fix-inconsistent-collection-types

Conversation

Copy link
Contributor

Copilot AI commented Feb 20, 2026

UrlElicitationRequiredErrorData.Elicitations was the sole outlier among Protocol DTOs, using IReadOnlyList<T> with init instead of IList<T> with set.

Changes

  • UrlElicitationRequiredErrorData.cs: IReadOnlyList<ElicitRequestParams> + initIList<ElicitRequestParams> + set
  • UrlElicitationRequiredException.cs:
    • Internal _elicitations field narrowed from IReadOnlyList<T> to List<T> (matching what Validate() already returns), eliminating the need for a cast when assigning to Elicitations
    • Validate() return type updated to List<T> accordingly
    • TryParseElicitations(): out parameter changed from IReadOnlyList<T> to IList<T>, allowing the deserialized value to be assigned directly without any conversion

The public Elicitations property on the exception remains IReadOnlyList<T>—that's the exception type's API, not a Protocol DTO, so read-only exposure is appropriate there.

Original prompt

Problem

The Protocol DTOs in src/ModelContextProtocol.Core/Protocol/ consistently use IList<T> for collection properties on their public API surface. However, UrlElicitationRequiredErrorData.Elicitations is the sole outlier — it uses IReadOnlyList<ElicitRequestParams> instead of IList<ElicitRequestParams>.

Inconsistency

Every other Protocol DTO uses IList<T>:

File Property Type
Prompt.cs Arguments IList<PromptArgument>?
Prompt.cs Icons IList<Icon>?
Tool.cs Icons IList<Icon>?
ResourceTemplate.cs Icons IList<Icon>?
ListResourcesResult.cs Resources IList<Resource>
ListPromptsResult.cs Prompts IList<Prompt>
ListRootsResult.cs Roots IList<Root>
Completion.cs Values IList<string>
Annotations.cs Audience IList<Role>?
Icon.cs Sizes IList<string>?
ElicitRequestParams.RequestSchema Required IList<string>?
UrlElicitationRequiredErrorData.cs Elicitations IReadOnlyList<ElicitRequestParams> ⬅️ outlier

I also checked for IDictionary vs IReadOnlyDictionary inconsistencies in the Protocol folder. All Protocol DTOs consistently use IDictionary<TKey, TValue> (e.g., CompleteContext.Arguments, CallToolRequestParams.Arguments, GetPromptRequestParams.Arguments, ElicitResult.Content, ElicitRequestParams.RequestSchema.Properties, ClientCapabilities.Experimental, ServerCapabilities.Experimental), so there is no inconsistency there.

Fix

Change UrlElicitationRequiredErrorData.Elicitations from IReadOnlyList<ElicitRequestParams> to IList<ElicitRequestParams> to be consistent with all other Protocol DTOs. Also change the init accessor to set to match the pattern used by the other DTOs.

The file is at: src/ModelContextProtocol.Core/Protocol/UrlElicitationRequiredErrorData.cs

Current code:

public required IReadOnlyList<ElicitRequestParams> Elicitations { get; init; }

Should be changed to:

public required IList<ElicitRequestParams> Elicitations { get; set; }

Note: UrlElicitationRequiredException in src/ModelContextProtocol.Core/UrlElicitationRequiredException.cs stores this data internally as IReadOnlyList<ElicitRequestParams> and exposes it via its Elicitations property as IReadOnlyList<ElicitRequestParams>. That's fine — that class is an exception type, not a Protocol DTO, and using IReadOnlyList on the exception's public API is appropriate. However, any code that creates a UrlElicitationRequiredErrorData and assigns to its Elicitations property will need to be compatible with the change from IReadOnlyList to IList. Since List<T> implements both IList<T> and IReadOnlyList<T>, typical usages should continue to work. Verify that the code in UrlElicitationRequiredException.CreateErrorDataNode() and TryParseElicitations still compiles and works correctly after this change, making any necessary adjustments.

This pull request was created from Copilot chat.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

…IReadOnlyList -> IList, init -> set)

Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix inconsistent collection types in Protocol DTOs Make UrlElicitationRequiredErrorData.Elicitations consistent with other Protocol DTOs Feb 20, 2026
…y ToList()

Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
@stephentoub stephentoub marked this pull request as ready for review February 20, 2026 22:28
@jeffhandley jeffhandley added the breaking-change This issue or PR introduces a breaking change label Feb 21, 2026
@jeffhandley jeffhandley added this to the Stable public API milestone Feb 21, 2026
@stephentoub stephentoub merged commit 4648d3e into main Feb 21, 2026
16 of 17 checks passed
@stephentoub stephentoub deleted the copilot/fix-inconsistent-collection-types branch February 21, 2026 04:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change This issue or PR introduces a breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants