Skip to content

Hide experimental properties from the JSON source generator to avoid MCPEXP001 diagnostics#1260

Open
MackinnonBuck wants to merge 4 commits intomainfrom
mbuck/experimental-json-fix
Open

Hide experimental properties from the JSON source generator to avoid MCPEXP001 diagnostics#1260
MackinnonBuck wants to merge 4 commits intomainfrom
mbuck/experimental-json-fix

Conversation

@MackinnonBuck
Copy link
Collaborator

Summary

Prevent experimental API types (Tasks, ToolExecution) from leaking into user code via the System.Text.Json source generator, eliminating spurious MCPEXP001 diagnostics for users who don't use experimental features.

Description

When users define their own JsonSerializerContext that includes MCP protocol types (e.g., Tool, ServerCapabilities), the System.Text.Json source generator follows property types transitively and generates code that references experimental types. This causes MCPEXP001 errors in user projects even when they aren't using experimental features like Tasks.

Approach: Object Backing Field Pattern

For each stable type that exposes an experimental property, the public typed property is now marked with [JsonIgnore] and backed by an internal object? field annotated with [JsonInclude], [JsonPropertyName], and a new [JsonConverter(typeof(ExperimentalJsonConverter<T>))]. Because the source generator sees object? rather than the experimental type, it no longer walks the experimental type graph, so no MCPEXP001 diagnostic is emitted in consuming projects.

The ExperimentalJsonConverter<T> delegates serialization to McpJsonUtilities.DefaultOptions, which already contains source-generated contracts for all experimental types. This preserves NativeAOT compatibility and ensures the experimental properties continue to round-trip correctly over the wire.

A DiagnosticSuppressor was considered but is not necessary since the backing field pattern fully hides experimental types from the source generator (and it wouldn't help anyway because DiagnosticSuppressors cannot suppress error-severity diagnostics).

Other changes

  • Added [Experimental(MCPEXP001)] to the 6 experimental properties that were missing it (only Tool.Execution had it previously). Happy to undo this change if the omission of the [Experimental] attribute was deliberate.
  • Added 11 tests covering round-trip serialization, null handling, JSON property names, and WriteIndented propagation.

Fixes #1255

@MackinnonBuck
Copy link
Collaborator Author

MackinnonBuck commented Feb 13, 2026

Pushed an update addressing review feedback:

  • Added MCPEXP001Suppressor to suppress MCPEXP001 in .g.cs files (this is actually possible, contrary to what I thought I had tested earlier). Public fields cause the source generator to emit expressions like new ExperimentalJsonConverter<McpTaskMetadata>(), resurfacing the diagnostic. The suppressor targets generated files only; hand-written references still produce diagnostics.
  • Added regression testing. MCPEXP001SuppressorTests covers generated vs. hand-written scenarios. A dedicated SuppressorRegressionTest project strips MCPEXP001 from NoWarn and fails to build if the suppressor breaks.
  • Added stabilization guardrails. ExperimentalBackingFieldTests uses a typed registry (ExperimentalProperty / StabilizedProperty / IgnoredExperimentalProperty) to enforce the backing field pattern via reflection. Tests check attribute correctness, registry membership, and getter/setter delegation. The test file documents the two-phase stabilization process (stabilize the attributes, then remove the backing field in a later release).

Comment on lines +46 to +51
/// <summary>Backing field for <see cref="Task"/>. This field is not intended to be used directly.</summary>
[JsonInclude]
[JsonPropertyName("task")]
public McpTaskMetadata? Task { get; set; }
[JsonConverter(typeof(ExperimentalJsonConverter<McpTaskMetadata>))]
[EditorBrowsable(EditorBrowsableState.Never)]
public object? _task;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend a much stronger statement about not being intended for consumption. We can take inspiration from efcore. Example: https://github.com/dotnet/efcore/blob/a471ebfd9564bc14fb188407a84a031b69d29f77/src/EFCore/Query/QueryContext.cs#L109-L116

We should also give the backing field a more egregious name to emphasize the internal aspect too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Experimental APIs force users to suppress diagnostics even when they are not used

3 participants