fix(manifest): a document parsed on two threads at once could be rejected as unparseable

docs/knowledge-graph.json

@@ -15,8 +15,8 @@
     "operator_facing_config_variables": 153,
     "operator_vars_missing_from_configuration_reference": 18,
     "operator_vars_absent_from_all_documentation": 0,
-    "test_files_declaring_facts": 731,
-    "declared_facts": 4399,
+    "test_files_declaring_facts": 733,
+    "declared_facts": 4403,
     "workflows": 62
   },
   "operator_vars_missing_from_configuration_reference": [
@@ -3769,6 +3769,13 @@
       "declared_facts": 8,
       "in_cert_gate_filter": false
     },
+    {
+      "file": "src/Ashlar.Tests.BackgroundAgents/SelfExtend/PolicyLoaderColdStartConcurrencyTests.cs",
+      "project": "Ashlar.Tests.BackgroundAgents",
+      "namespace": "Ashlar.Tests.BackgroundAgents.SelfExtend",
+      "declared_facts": 2,
+      "in_cert_gate_filter": false
+    },
     {
       "file": "src/Ashlar.Tests.BackgroundAgents/SelfExtend/SelfExtendAdmissionBridgeTests.cs",
       "project": "Ashlar.Tests.BackgroundAgents",
@@ -7255,6 +7262,13 @@
       "declared_facts": 3,
       "in_cert_gate_filter": false
     },
+    {
+      "file": "src/Ashlar.Tests.Kernel/YamlDeserializerNotSharedConventionTests.cs",
+      "project": "Ashlar.Tests.Kernel",
+      "namespace": "Ashlar.Tests.Kernel",
+      "declared_facts": 2,
+      "in_cert_gate_filter": false
+    },
     {
       "file": "src/Ashlar.Tests.Orchestration/Agents/AgentFactoryTests.cs",
       "project": "Ashlar.Tests.Orchestration",

docs/knowledge-graph.md

@@ -19,8 +19,8 @@ remembered to update — staleness here is a red build.
 | of those, operator-facing (not test-only) | 153 |
 | missing from `docs/Configuration.md` | 18 |
 | **mentioned in no document at all** | **0** |
-| test files declaring xUnit facts | 731 |
-| declared facts | 4399 |
+| test files declaring xUnit facts | 733 |
+| declared facts | 4403 |
 | workflows | 62 |
 
 ## Namespaces cert-gate selects

src/Ashlar.Manifest/ManifestLoader.cs

@@ -34,12 +34,20 @@ public static class ManifestLoader
         "policy",
     ];
 
-    private static readonly IDeserializer Deserializer = new DeserializerBuilder()
+    /// <summary>
+    /// NEW deserializers for every parse, for the reason given on
+    /// <c>PolicyLoader.NewDeserializer</c>: a YamlDotNet 13.7.1 deserializer is not safe to share
+    /// across threads during its first parses, and a shared one made this loader reject valid
+    /// manifests as "could not be parsed" (seen in <c>ProjectVerifierTests</c>, run 34062198636).
+    /// <c>YamlDeserializerNotSharedConventionTests</c> fails if any static field in this assembly
+    /// holds a YamlDotNet type again.
+    /// </summary>
+    private static IDeserializer NewDeserializer() => new DeserializerBuilder()
         .WithNamingConvention(CamelCaseNamingConvention.Instance)
         .IgnoreUnmatchedProperties()
         .Build();
 
-    private static readonly IDeserializer RawDeserializer = new DeserializerBuilder()
+    private static IDeserializer NewRawDeserializer() => new DeserializerBuilder()
         .IgnoreUnmatchedProperties()
         .Build();
 
@@ -71,7 +79,7 @@ public static bool TryLoad(string? yaml, out AshlarManifest? manifest, out strin
         Dictionary<string, object>? raw;
         try
         {
-            raw = RawDeserializer.Deserialize<Dictionary<string, object>>(yaml!);
+            raw = NewRawDeserializer().Deserialize<Dictionary<string, object>>(yaml!);
         }
         catch (YamlException ex)
         {
@@ -97,7 +105,7 @@ public static bool TryLoad(string? yaml, out AshlarManifest? manifest, out strin
         AshlarManifest? parsed;
         try
         {
-            parsed = Deserializer.Deserialize<AshlarManifest>(yaml!);
+            parsed = NewDeserializer().Deserialize<AshlarManifest>(yaml!);
         }
         catch (YamlException ex)
         {

src/Ashlar.Manifest/PolicyLoader.cs

@@ -41,7 +41,30 @@ public static class PolicyLoader
     /// </summary>
     public static readonly IReadOnlyList<string> AddableKinds = ["brick"];
 
-    private static readonly IDeserializer Deserializer = new DeserializerBuilder()
+    /// <summary>
+    /// A NEW deserializer for every parse. Never make this a shared static again.
+    ///
+    /// <para>YamlDotNet 13.7.1 (Directory.Packages.props) is not safe to share across threads.
+    /// Each deserializer owns one <c>DefaultObjectFactory</c>, which caches every type's
+    /// <c>[OnDeserializing]</c>/<c>[OnDeserialized]</c> lookup in a plain <c>Dictionary</c> and
+    /// writes it, unlocked, the first time it meets that type. When this was one static, two
+    /// threads doing their first parse of a process at the same moment wrote it together; the
+    /// corrupted insert threw, YamlDotNet wrapped that as "Exception during deserialization", and
+    /// this loader rejected a VALID policy as one that "could not be parsed". The self-extend gate
+    /// then failed closed on it. That was the intermittent red of
+    /// <c>SelfExtendAdmissionBridgeTests</c> and <c>ManifestContractTests</c> in the readiness
+    /// gate, and any process parsing two policies at once for the first time could hit it.</para>
+    ///
+    /// <para>Caching the <see cref="DeserializerBuilder"/> instead would NOT help: a builder hands
+    /// the same lazily created object factory to every <c>Build()</c>. Upstream made the cache
+    /// concurrent in YamlDotNet 16.1.0. A policy is parsed once per command or self-extend cycle, so a
+    /// fresh deserializer costs nothing that matters, and with nothing shared there is nothing to
+    /// race on. <c>YamlDeserializerNotSharedConventionTests</c> fails if any static field in this
+    /// assembly holds a YamlDotNet type, which covers every caching shape. The race itself is
+    /// reproduced by <c>PolicyLoaderColdStartConcurrencyTests</c>, which catches an eagerly shared
+    /// deserializer or builder but not a lazily assigned one.</para>
+    /// </summary>
+    private static IDeserializer NewDeserializer() => new DeserializerBuilder()
         .WithNamingConvention(CamelCaseNamingConvention.Instance)
         .Build();
 
@@ -71,7 +94,7 @@ public static bool TryLoad(string? yaml, out AshlarPolicy? policy, out string re
         AshlarPolicy? parsed;
         try
         {
-            parsed = Deserializer.Deserialize<AshlarPolicy>(yaml!);
+            parsed = NewDeserializer().Deserialize<AshlarPolicy>(yaml!);
         }
         catch (YamlException ex)
         {

src/Ashlar.Tests.BackgroundAgents/SelfExtend/PolicyLoaderColdStartConcurrencyTests.cs

@@ -0,0 +1,216 @@
+using System.Reflection;
+using System.Runtime.Loader;
+using Ashlar.Manifest;
+using FluentAssertions;
+using Xunit;
+
+namespace Ashlar.Tests.BackgroundAgents.SelfExtend;
+
+/// <summary>
+/// Two threads parsing a policy for the first time in a process must both get the policy.
+///
+/// <para><b>Why this exists.</b> The self-extend gate fails closed on a policy it cannot parse,
+/// so a spurious parse failure is a spurious rejection. That is what reddened
+/// <see cref="SelfExtendAdmissionBridgeTests.A_share_failure_annotates_the_outcome_but_never_fails_the_cycle"/>
+/// on the macOS lane of readiness run 36740078979 with "GATE ERROR: REJECTED: policy could not be
+/// parsed: Exception during deserialization", and <c>ManifestContractTests</c> on Linux and
+/// Windows before it. <see cref="PolicyLoader"/> held ONE static YamlDotNet deserializer for the
+/// whole process. In YamlDotNet 13.7.1 that deserializer's object factory caches each type's
+/// <c>[OnDeserializing]</c>/<c>[OnDeserialized]</c> lookup in a plain <c>Dictionary</c> that it
+/// writes, unlocked, the first time it meets the type. Two test classes (or two self-extend
+/// cycles) doing their first parse at the same instant wrote it together; the corrupted insert
+/// threw, and YamlDotNet wrapped that as "Exception during deserialization".</para>
+///
+/// <para><b>Why a process start has to be rebuilt here.</b> The cache is written only on a miss,
+/// so the race exists only while it is cold: the first parses of a process. Re-running any test
+/// in a process that has already parsed a policy can never see it, which is why the original
+/// failure was ~0.1% per lane. Each trial below therefore loads FRESH copies of
+/// <c>Ashlar.Manifest</c> and <c>YamlDotNet</c> into their own collectible
+/// <see cref="AssemblyLoadContext"/>, where every static they own starts empty exactly as it does
+/// at process start, and releases all the threads into <c>PolicyLoader.TryLoad</c> through one
+/// barrier. With a shared deserializer restored it failed nearly every run when measured, pinned
+/// to two cores included; with no shared state there is nothing to race on and it cannot fail.</para>
+///
+/// <para><b>What this cannot see.</b> Only <see cref="PolicyLoader"/>. <c>ManifestLoader</c> keeps
+/// two static deserializers of the same kind and is not exercised here.</para>
+/// </summary>
+public sealed class PolicyLoaderColdStartConcurrencyTests
+{
+    /// <summary>More threads than any CI runner has cores: the barrier, not the core count, lines them up.</summary>
+    private const int Threads = 8;
+
+    /// <summary>
+    /// Cold starts per run. Pinned to two cores, 20 trials still missed a restored shared
+    /// deserializer in 2 runs of 20, so a small runner gets twice that.
+    /// </summary>
+    private const int Trials = 40;
+
+    /// <summary>Byte-for-byte what <c>SelfExtendAdmissionBridgeTests.WritePolicy("self-extending")</c> writes.</summary>
+    private const string Yaml = """
+        apiVersion: ashlar/v1
+        kind: Policy
+        sandbox:
+          root: .
+          writable: []
+        selfExtend:
+          mode: self-extending
+          budget:
+            extensions: 3
+            window: 24h
+          mayAdd: [brick]
+          gatesRequired: [sandbox]
+        never:
+          - modify_gate
+          - widen_sandbox
+          - access_signing_keys
+          - truncate_ledger
+          - grant_capability
+        """;
+
+    [Fact]
+    public void Concurrent_first_parses_of_a_process_all_get_the_policy()
+    {
+        var failures = new List<string>();
+        var calls = 0;
+
+        for (var trial = 0; trial < Trials; trial++)
+        {
+            var context = new ColdStartContext();
+            try
+            {
+                var tryLoad = context.PolicyLoaderTryLoad();
+                var results = RaceFirstParses(tryLoad);
+                calls += results.Length;
+                failures.AddRange(results.Where(r => r is not null).Select(r => $"trial {trial}: {r}"));
+
+                // The trial proves nothing unless YamlDotNet itself was a fresh copy too, not the
+                // default context's warm one.
+                context.FreshAssemblyNames().Should().Contain(
+                    ColdStartContext.Fresh,
+                    "every statically held YamlDotNet cache must start cold, as at process start");
+            }
+            finally
+            {
+                context.Unload();
+            }
+        }
+
+        calls.Should().Be(Threads * Trials, "every thread must have reached TryLoad and reported");
+        failures.Should().BeEmpty(
+            "a valid policy must parse however many threads meet it first, yet {0} of {1} first parses "
+            + "were rejected; a shared, unsynchronized deserializer rejects it at random, and the gate "
+            + "then fails closed on a policy that is fine",
+            failures.Count, calls);
+    }
+
+    [Fact]
+    public void The_cold_start_context_really_is_a_second_copy_of_the_loader()
+    {
+        // Non-vacuity for the fact above: if the load context quietly resolved to the default
+        // context's assemblies, every trial would share one warm deserializer and pass for the
+        // wrong reason.
+        var context = new ColdStartContext();
+        try
+        {
+            var tryLoad = context.PolicyLoaderTryLoad();
+
+            var freshLoader = tryLoad.DeclaringType!.Assembly;
+            freshLoader.Should().NotBeSameAs(typeof(PolicyLoader).Assembly);
+            AssemblyLoadContext.GetLoadContext(freshLoader).Should().BeSameAs(context);
+
+            var args = new object?[] { Yaml, null, null };
+            ((bool)tryLoad.Invoke(null, args)!).Should().BeTrue((string?)args[2]);
+            args[1].Should().NotBeNull().And.NotBeOfType<AshlarPolicy>(
+                "the policy type must come from the fresh copy of Ashlar.Manifest");
+
+            context.FreshAssemblyNames().Should().BeEquivalentTo(ColdStartContext.Fresh);
+        }
+        finally
+        {
+            context.Unload();
+        }
+    }
+
+    /// <summary>Releases <see cref="Threads"/> threads into TryLoad at once; null means that call got the policy.</summary>
+    private static string?[] RaceFirstParses(MethodInfo tryLoad)
+    {
+        var results = new string?[Threads];
+        var reported = new bool[Threads];
+        using var barrier = new Barrier(Threads);
+        var workers = new Thread[Threads];
+        for (var i = 0; i < Threads; i++)
+        {
+            var slot = i;
+            workers[i] = new Thread(() =>
+            {
+                barrier.SignalAndWait();
+                try
+                {
+                    var args = new object?[] { Yaml, null, null };
+                    var loaded = (bool)tryLoad.Invoke(null, args)!;
+                    results[slot] = loaded && args[1] is not null
+                        ? null
+                        : (string?)args[2] ?? "TryLoad returned no policy and no reason";
+                }
+                catch (Exception ex)
+                {
+                    results[slot] = "TryLoad threw " + (ex is TargetInvocationException { InnerException: { } inner } ? inner : ex);
+                }
+                reported[slot] = true;
+            })
+            { IsBackground = true };
+        }
+
+        foreach (var worker in workers)
+        {
+            worker.Start();
+        }
+        foreach (var worker in workers)
+        {
+            worker.Join();
+        }
+
+        reported.Should().OnlyContain(r => r, "a thread that never reported proves nothing");
+        return results;
+    }
+
+    /// <summary>
+    /// A collectible load context holding its own copies of the assemblies whose statics make up
+    /// "the first parse of a process". Everything else resolves to the default context.
+    /// </summary>
+    private sealed class ColdStartContext : AssemblyLoadContext
+    {
+        public static readonly string[] Fresh = ["Ashlar.Manifest", "YamlDotNet"];
+
+        private readonly string _directory;
+
+        public ColdStartContext()
+            : base("policy-cold-start", isCollectible: true)
+        {
+            var location = typeof(PolicyLoader).Assembly.Location;
+            location.Should().NotBeNullOrEmpty("the loader must be loadable from a file for a second copy to exist");
+            _directory = Path.GetDirectoryName(location)!;
+        }
+
+        public MethodInfo PolicyLoaderTryLoad()
+        {
+            var assembly = LoadFromAssemblyName(typeof(PolicyLoader).Assembly.GetName());
+            var loader = assembly.GetType(typeof(PolicyLoader).FullName!, throwOnError: true)!;
+            return loader.GetMethod(nameof(PolicyLoader.TryLoad), BindingFlags.Public | BindingFlags.Static)
+                ?? throw new InvalidOperationException("PolicyLoader.TryLoad not found on the fresh copy");
+        }
+
+        public IEnumerable<string> FreshAssemblyNames() =>
+            Assemblies.Select(a => a.GetName().Name!).Where(n => Fresh.Contains(n, StringComparer.Ordinal));
+
+        protected override Assembly? Load(AssemblyName assemblyName)
+        {
+            if (assemblyName.Name is { } name && Fresh.Contains(name, StringComparer.Ordinal))
+            {
+                return LoadFromAssemblyPath(Path.Combine(_directory, name + ".dll"));
+            }
+
+            return null;
+        }
+    }
+}