fix: key A2AJsonConverter safe-options cache per JsonSerializerOptions instance

src/A2A.V0_3/A2AJsonConverter.cs

@@ -1,4 +1,4 @@
-using System.Diagnostics;
+using System.Runtime.CompilerServices;
 using System.Text.Json;
 using System.Text.Json.Serialization;
 using System.Text.Json.Serialization.Metadata;
@@ -14,9 +14,6 @@ internal interface IA2AJsonConverter;
     /// <typeparam name="T">The type to convert.</typeparam>
     internal class A2AJsonConverter<T> : JsonConverter<T>, IA2AJsonConverter where T : notnull
     {
-        private static JsonSerializerOptions? _serializerOptionsWithoutThisConverter;
-        private static JsonSerializerOptions? _outsideSerializerOptions;
-
         /// <summary>
         /// Reads and converts the JSON to type <typeparamref name="T"/>.
         /// </summary>
@@ -77,6 +74,8 @@ public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions
         /// <param name="options">The serializer options to use.</param>
         protected virtual void SerializeImpl(Utf8JsonWriter writer, T value, JsonSerializerOptions options) => JsonSerializer.Serialize(writer, value, (JsonTypeInfo<T>)options.GetTypeInfo(value.GetType()));
 
+        private static readonly ConditionalWeakTable<JsonSerializerOptions, JsonSerializerOptions> _safeOptionsByCaller = new();
+
         /// <summary>
         /// Returns a copy of the provided <see cref="JsonSerializerOptions"/> with this
         /// <see cref="A2AJsonConverter{T}"/> removed from its <see cref="JsonSerializerOptions.Converters"/> chain.
@@ -87,43 +86,34 @@ public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions
         /// would be selected again, causing infinite recursion and a stack overflow. Cloning the options and removing
         /// this converter ensures the underlying, "real" converter handles serialization/deserialization of <typeparamref name="T"/>.
         ///
-        /// The returned options are cached per closed generic type to avoid repeated allocations. The cache assumes a
-        /// stable options instance; if multiple distinct options are used, the first encountered configuration is captured.
+        /// The derived options are cached per caller <see cref="JsonSerializerOptions"/> instance in a
+        /// <see cref="ConditionalWeakTable{TKey, TValue}"/>, so distinct options instances each get their own
+        /// converter-stripped copy rather than sharing a single global one. The table is reference-keyed (correct,
+        /// since <see cref="JsonSerializerOptions"/> has no value equality) and its entries are collected together
+        /// with the caller options, so the cache neither collides across instances nor leaks.
         /// </remarks>
         /// <param name="options">Caller options; used as a template for the safe copy.</param>
         /// <returns>A copy of the options that can safely resolve the underlying converter for <typeparamref name="T"/>.</returns>
-        private static JsonSerializerOptions GetSafeOptions(JsonSerializerOptions options)
-        {
-            if (_serializerOptionsWithoutThisConverter is null)
+        private static JsonSerializerOptions GetSafeOptions(JsonSerializerOptions options) =>
+            _safeOptionsByCaller.GetValue(options, static o =>
             {
-                // keep reeference to original options for cache validation 
-                _outsideSerializerOptions = options;
-
                 // Clone options so we can modify the converters chain safely
-                var baseOptions = new JsonSerializerOptions(options);
+                var clone = new JsonSerializerOptions(o);
 
                 // Remove this converter so base/source-generated converter handles T, otherwise stack overflow
-                for (int i = baseOptions.Converters.Count - 1; i >= 0; i--)
+                for (int i = clone.Converters.Count - 1; i >= 0; i--)
                 {
-                    if (baseOptions.Converters[i] is A2AJsonConverter<T>)
+                    if (clone.Converters[i] is A2AJsonConverter<T>)
                     {
-                        baseOptions.Converters.RemoveAt(i);
+                        clone.Converters.RemoveAt(i);
                         break;
                     }
                 }
 
-                _serializerOptionsWithoutThisConverter = baseOptions;
-            }
-            else if (_outsideSerializerOptions != options && options != _serializerOptionsWithoutThisConverter)
-            {
-                // Unexpected!!! This caching is based on promise that A2A will use only ONE instance of SerializerOptions
-                // and we can therefore cache modified SerializerOptions without dealing with invalidation and pairing.
-                // Since this is possible only by internal code, some recent code changes must have had broke this promise
-                Debug.Fail("This should never happen");
-                throw new InvalidOperationException();
-            }
-
-            return _serializerOptionsWithoutThisConverter;
-        }
+                // Freeze the shared cached copy so it cannot be mutated after publication,
+                // matching A2AJsonUtilities.DefaultOptions.
+                clone.MakeReadOnly();
+                return clone;
+            });
     }
 }

tests/A2A.V0_3.UnitTests/GitHubIssues/Issue298.cs

@@ -0,0 +1,49 @@
+using System.Text.Json;
+using Microsoft.Extensions.AI;
+
+namespace A2A.V0_3.UnitTests.GitHubIssues
+{
+    // https://github.com/a2aproject/a2a-dotnet/issues/298
+    // FileContent (de)serialization poisoned a static JsonSerializerOptions cache in
+    // A2AJsonConverter<T>.GetSafeOptions: the first options instance was captured in a
+    // static field, and any second, distinct instance threw InvalidOperationException
+    // ("Operation is not valid due to the current state of the object"), surfaced as
+    // A2AException. A host layer such as Microsoft.Agents.AI.Hosting builds its own
+    // options from A2AJsonUtilities.DefaultOptions with a rebuilt resolver chain; that
+    // is exactly the second instance that tripped the guard.
+    public sealed class Issue298
+    {
+        [Fact]
+        public void Issue_298_SecondOptionsInstance_DoesNotPoisonConverterCache()
+        {
+            var file = new FileContent(new Uri("https://example.com/file.txt")) { MimeType = "text/plain" };
+
+            // Prime the converter cache with the shared default options instance.
+            var defaultOptions = A2AJsonUtilities.DefaultOptions;
+            _ = JsonSerializer.Serialize(file, defaultOptions);
+
+            // A second, distinct options instance built the way a host layer builds one:
+            // a clone of the defaults with the resolver chain rebuilt (MEAI resolver first).
+            var hostOptions = new JsonSerializerOptions(A2AJsonUtilities.DefaultOptions);
+            hostOptions.TypeInfoResolverChain.Clear();
+            hostOptions.TypeInfoResolverChain.Add(AIJsonUtilities.DefaultOptions.TypeInfoResolver!);
+            foreach (var resolver in A2AJsonUtilities.DefaultOptions.TypeInfoResolverChain)
+            {
+                hostOptions.TypeInfoResolverChain.Add(resolver);
+            }
+
+            // Before the fix this threw A2AException ("Operation is not valid due to the
+            // current state of the object"); it must now round-trip cleanly.
+            var json = JsonSerializer.Serialize(file, hostOptions);
+            var roundTripped = JsonSerializer.Deserialize<FileContent>(json, hostOptions);
+
+            Assert.NotNull(roundTripped);
+            Assert.Equal("https://example.com/file.txt", roundTripped!.Uri?.ToString());
+            Assert.Equal("text/plain", roundTripped.MimeType);
+
+            // The shared default instance must remain usable after the second instance was seen.
+            var again = JsonSerializer.Serialize(file, defaultOptions);
+            Assert.Contains("https://example.com/file.txt", again);
+        }
+    }
+}