fix(agent): correct V2 HAADJ detection + drop duplicate snapshot ticks

Two V2 bugs surfaced by Bayer session c4c8d206-… (HAADJ on Lenovo T14 Gen 5,
Win11 23H2 22631.4317):

* `IsHybridJoin=false` in the Sessions row though the Autopilot profile is
  `Win_Hybrid physical`. Root cause: `EnrollmentRegistryDetector.DetectHybridJoin`
  reads only the top-level `CloudAssignedDomainJoinMethod` registry value. On
  this device that value is persisted only inside the `PolicyJsonCache` JSON
  blob — `DeviceInfoCollector` already had the JSON fallback, the registration-
  time detector did not. Refactored into a pure `ResolveHybridJoinFromValues`
  helper with the same fallback ladder; full xUnit coverage incl. the real
  payload from the failing session.

* Duplicate `performance_snapshot` events with timestamps 0–1 ms apart, the
  second one carrying a fresh-baseline net delta of 0. Root cause: System
  .Threading.Timer dispatches callbacks on different ThreadPool threads when a
  callback exceeds the period, and after the long stall the OS network re-init
  slowed `NetworkInterface.GetAllNetworkInterfaces()` enough to overlap the
  next 30 s tick → two parallel `Collect()` calls racing on shared baseline
  fields. Added an `Interlocked.CompareExchange` reentrancy gate in
  `CollectorBase.CollectSafe`; protects all three subclasses (Performance,
  AgentSelfMetrics, DeliveryOptimization) at once. xUnit coverage for the
  single-tick, parallel-tick-dropped, post-exception, and 16-thread stress
  cases.

V2-only per phase-out policy. Full V2 suite green (812 + 14 new); pre-existing
RecoveryPathTests parallel-flake confirmed unrelated.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Security/EnrollmentRegistryDetectorTests.cs

@@ -0,0 +1,131 @@
+using System;
+using System.Collections.Generic;
+using AutopilotMonitor.Agent.V2.Core.Security;
+using Xunit;
+
+namespace AutopilotMonitor.Agent.V2.Core.Tests.Security
+{
+    /// <summary>
+    /// Regression coverage for the IsHybridJoin detection that runs at session-registration
+    /// time. Field evidence (session c4c8d206-…, Lenovo ThinkPad T14 Gen 5, Win11 23H2
+    /// 22631.4317): the AutopilotPolicyCache key on this device exposes
+    /// <c>CloudAssignedDomainJoinMethod</c> only inside the embedded <c>PolicyJsonCache</c>
+    /// blob, not as a top-level registry value. Without the JSON fallback the agent registered
+    /// the session with <c>IsHybridJoin=false</c> even though the device was a HAADJ target,
+    /// while the later-running <c>DeviceInfoCollector</c> (which already had the fallback)
+    /// emitted <c>autopilot_profile</c> with <c>isHybridJoin=true</c>, producing a confusing
+    /// inconsistency in the Sessions table vs. the Events stream.
+    /// </summary>
+    public sealed class EnrollmentRegistryDetectorTests
+    {
+        private static Func<string, object> Lookup(Dictionary<string, object> values)
+            => name => values.TryGetValue(name, out var v) ? v : null!;
+
+        [Fact]
+        public void TopLevel_DomainJoinMethod_one_returns_true()
+        {
+            var values = new Dictionary<string, object>
+            {
+                ["CloudAssignedDomainJoinMethod"] = 1,
+            };
+            Assert.True(EnrollmentRegistryDetector.ResolveHybridJoinFromValues(Lookup(values)));
+        }
+
+        [Fact]
+        public void TopLevel_DomainJoinMethod_zero_returns_false_even_with_hybrid_in_json()
+        {
+            // Top-level value is authoritative: explicit "0" wins over a contradicting JSON
+            // blob — the JSON fallback only kicks in when the top-level is absent.
+            var values = new Dictionary<string, object>
+            {
+                ["CloudAssignedDomainJoinMethod"] = 0,
+                ["PolicyJsonCache"] = "{\"CloudAssignedDomainJoinMethod\":1}",
+            };
+            Assert.False(EnrollmentRegistryDetector.ResolveHybridJoinFromValues(Lookup(values)));
+        }
+
+        [Fact]
+        public void Missing_TopLevel_falls_back_to_PolicyJsonCache_one_returns_true()
+        {
+            // Real-world payload from the Bayer Lenovo T14 Gen 5 / Win11 23H2 22631.4317
+            // device that triggered this fix. Truncated for readability — the only relevant
+            // key is CloudAssignedDomainJoinMethod=1.
+            const string policyJson =
+                "{\r\n  \"CloudAssignedTenantDomain\": \"bayergroup.onmicrosoft.com\",\r\n" +
+                "  \"CloudAssignedDomainJoinMethod\": 1,\r\n" +
+                "  \"AutopilotMode\": 2\r\n}";
+
+            var values = new Dictionary<string, object>
+            {
+                ["PolicyJsonCache"] = policyJson,
+            };
+            Assert.True(EnrollmentRegistryDetector.ResolveHybridJoinFromValues(Lookup(values)));
+        }
+
+        [Fact]
+        public void Missing_TopLevel_falls_back_to_PolicyJsonCache_zero_returns_false()
+        {
+            var values = new Dictionary<string, object>
+            {
+                ["PolicyJsonCache"] = "{\"CloudAssignedDomainJoinMethod\":0}",
+            };
+            Assert.False(EnrollmentRegistryDetector.ResolveHybridJoinFromValues(Lookup(values)));
+        }
+
+        [Fact]
+        public void Missing_TopLevel_and_PolicyJsonCache_without_key_returns_false()
+        {
+            var values = new Dictionary<string, object>
+            {
+                ["PolicyJsonCache"] = "{\"CloudAssignedTenantDomain\":\"contoso.onmicrosoft.com\"}",
+            };
+            Assert.False(EnrollmentRegistryDetector.ResolveHybridJoinFromValues(Lookup(values)));
+        }
+
+        [Fact]
+        public void Missing_TopLevel_and_malformed_PolicyJsonCache_returns_false_without_throw()
+        {
+            var values = new Dictionary<string, object>
+            {
+                ["PolicyJsonCache"] = "{not valid json",
+            };
+            // Must not throw — the detector runs on the registration hot-path and a failure
+            // here would block session registration.
+            Assert.False(EnrollmentRegistryDetector.ResolveHybridJoinFromValues(Lookup(values)));
+        }
+
+        [Fact]
+        public void Missing_TopLevel_and_empty_PolicyJsonCache_returns_false()
+        {
+            var values = new Dictionary<string, object>
+            {
+                ["PolicyJsonCache"] = string.Empty,
+            };
+            Assert.False(EnrollmentRegistryDetector.ResolveHybridJoinFromValues(Lookup(values)));
+        }
+
+        [Fact]
+        public void All_values_missing_returns_false()
+        {
+            var values = new Dictionary<string, object>();
+            Assert.False(EnrollmentRegistryDetector.ResolveHybridJoinFromValues(Lookup(values)));
+        }
+
+        [Fact]
+        public void Null_lookup_returns_false()
+        {
+            Assert.False(EnrollmentRegistryDetector.ResolveHybridJoinFromValues(null));
+        }
+
+        [Fact]
+        public void DetectHybridJoin_returns_false_when_registry_key_is_unreadable()
+        {
+            // Smoke test against the real registry. On a developer machine the AutopilotPolicyCache
+            // key is typically absent so DetectHybridJoin must degrade to false without throwing.
+            // (CI / test boxes that DO have the key — vanishingly unlikely — would still observe a
+            // boolean, never an exception.)
+            var actual = EnrollmentRegistryDetector.DetectHybridJoin();
+            Assert.IsType<bool>(actual);
+        }
+    }
+}

src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Telemetry/CollectorBaseReentrancyTests.cs

@@ -0,0 +1,173 @@
+using System;
+using System.IO;
+using System.Threading;
+using System.Threading.Tasks;
+using AutopilotMonitor.Agent.V2.Core.Logging;
+using AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry;
+using AutopilotMonitor.Agent.V2.Core.Orchestration;
+using AutopilotMonitor.Agent.V2.Core.Tests.Harness;
+using AutopilotMonitor.Agent.V2.Core.Tests.Orchestration;
+using Xunit;
+
+namespace AutopilotMonitor.Agent.V2.Core.Tests.Telemetry
+{
+    /// <summary>
+    /// Regression coverage for the duplicate <c>performance_snapshot</c> events observed on
+    /// session c4c8d206-… (Bayer / Lenovo T14 Gen 5): two snapshots fired with timestamps
+    /// 0–1ms apart, the second one carrying a fresh-baseline net delta of 0. Root cause:
+    /// <see cref="System.Threading.Timer"/> is documented to dispatch callbacks on different
+    /// ThreadPool threads when a callback exceeds the period, and the
+    /// <c>PerformanceCollector</c>'s <c>Collect()</c> reads shared baseline fields without
+    /// any coordination. After the long stall the OS network re-init slowed
+    /// <c>NetworkInterface.GetAllNetworkInterfaces()</c> enough to overlap the next 30s tick
+    /// → two parallel <c>Collect()</c> calls → race.
+    /// <para>
+    /// The fix lives in <c>CollectorBase.CollectSafe</c>: any tick that arrives while a
+    /// previous <c>Collect()</c> is still running is dropped via an
+    /// <see cref="Interlocked.CompareExchange(ref int, int, int)"/> gate. These tests pin the
+    /// gate behaviour so a future "let's just remove the guard, it never fires" refactor
+    /// has a hard signal.
+    /// </para>
+    /// </summary>
+    public sealed class CollectorBaseReentrancyTests
+    {
+        private static AgentLogger NewLogger(TempDirectory tmp)
+            => new AgentLogger(Path.Combine(tmp.Path, "logs"), AgentLogLevel.Debug);
+
+        private static InformationalEventPost NewPost()
+            => new InformationalEventPost(
+                new FakeSignalIngressSink(),
+                new VirtualClock(new DateTime(2026, 4, 29, 17, 16, 0, DateTimeKind.Utc)));
+
+        [Fact]
+        public void CollectSafe_runs_collect_when_no_previous_call_is_in_flight()
+        {
+            using var tmp = new TempDirectory();
+            var collector = new BlockingCollector(NewPost(), NewLogger(tmp));
+
+            collector.ReleaseImmediately = true;
+            collector.CollectSafe();
+
+            Assert.Equal(1, collector.CollectInvocationCount);
+        }
+
+        [Fact]
+        public async Task CollectSafe_skips_when_previous_collect_is_still_running()
+        {
+            using var tmp = new TempDirectory();
+            var collector = new BlockingCollector(NewPost(), NewLogger(tmp));
+
+            // First call enters Collect() and parks on the gate, simulating a slow tick.
+            var firstTick = Task.Run(() => collector.CollectSafe());
+            Assert.True(collector.WaitUntilInsideCollect(TimeSpan.FromSeconds(2)),
+                "First Collect did not enter within the timeout.");
+
+            // Second tick fires while the first is still parked — must be dropped.
+            collector.CollectSafe();
+
+            // Release the first one and wait for it to finish.
+            collector.Release();
+            await firstTick;
+
+            Assert.Equal(1, collector.CollectInvocationCount);
+        }
+
+        [Fact]
+        public void CollectSafe_resets_gate_after_collect_throws()
+        {
+            using var tmp = new TempDirectory();
+            var collector = new BlockingCollector(NewPost(), NewLogger(tmp));
+
+            // First tick throws — base must catch and reset the gate so the next tick runs.
+            collector.ThrowOnNextCollect = true;
+            collector.ReleaseImmediately = true;
+            collector.CollectSafe();
+            Assert.Equal(1, collector.CollectInvocationCount);
+
+            // Second tick must execute; the gate is no longer locked.
+            collector.CollectSafe();
+            Assert.Equal(2, collector.CollectInvocationCount);
+        }
+
+        [Fact]
+        public async Task Concurrent_CollectSafe_calls_serialize_through_the_gate()
+        {
+            // Stress the Interlocked gate: many parallel ticks against a slow Collect must
+            // produce exactly one Collect invocation while the slow one is in flight.
+            using var tmp = new TempDirectory();
+            var collector = new BlockingCollector(NewPost(), NewLogger(tmp));
+
+            var firstTick = Task.Run(() => collector.CollectSafe());
+            Assert.True(collector.WaitUntilInsideCollect(TimeSpan.FromSeconds(2)),
+                "First Collect did not enter within the timeout.");
+
+            // 16 concurrent racing ticks — none of them should manage to enter Collect.
+            var racers = new Task[16];
+            for (int i = 0; i < racers.Length; i++)
+                racers[i] = Task.Run(() => collector.CollectSafe());
+            await Task.WhenAll(racers);
+
+            collector.Release();
+            await firstTick;
+
+            Assert.Equal(1, collector.CollectInvocationCount);
+
+            // After the slow one finished the gate is open again — next tick runs normally.
+            collector.ReleaseImmediately = true;
+            collector.CollectSafe();
+            Assert.Equal(2, collector.CollectInvocationCount);
+        }
+
+        /// <summary>
+        /// Test-only collector subclass: signals when <see cref="Collect"/> is entered and
+        /// blocks until the test releases it, so we can exercise the reentrancy gate
+        /// deterministically without relying on real timer scheduling.
+        /// </summary>
+        private sealed class BlockingCollector : CollectorBase
+        {
+            private readonly ManualResetEventSlim _insideCollect = new ManualResetEventSlim(false);
+            private readonly ManualResetEventSlim _release = new ManualResetEventSlim(false);
+            private int _collectInvocationCount;
+
+            public BlockingCollector(InformationalEventPost post, AgentLogger logger)
+                : base(
+                      sessionId: Guid.NewGuid().ToString(),
+                      tenantId: Guid.NewGuid().ToString(),
+                      post: post,
+                      logger: logger,
+                      intervalSeconds: 30)
+            {
+            }
+
+            public int CollectInvocationCount => Volatile.Read(ref _collectInvocationCount);
+
+            /// <summary>When <c>true</c>, <see cref="Collect"/> returns immediately.</summary>
+            public bool ReleaseImmediately { get; set; }
+
+            /// <summary>One-shot: next <see cref="Collect"/> call throws.</summary>
+            public bool ThrowOnNextCollect { get; set; }
+
+            public bool WaitUntilInsideCollect(TimeSpan timeout) => _insideCollect.Wait(timeout);
+
+            public void Release() => _release.Set();
+
+            protected override void Collect()
+            {
+                Interlocked.Increment(ref _collectInvocationCount);
+                _insideCollect.Set();
+
+                if (ThrowOnNextCollect)
+                {
+                    ThrowOnNextCollect = false;
+                    throw new InvalidOperationException("simulated collect failure");
+                }
+
+                if (ReleaseImmediately) return;
+
+                _release.Wait(TimeSpan.FromSeconds(5));
+                _release.Reset();
+                _insideCollect.Reset();
+            }
+        }
+    }
+}

src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/CollectorBase.cs

@@ -26,6 +26,14 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry
         private readonly int _intervalSeconds;
         private Timer _timer;
         private bool _disposed;
+        // 0 = idle, 1 = a Collect call is in flight. System.Threading.Timer is documented to
+        // fire callbacks on different ThreadPool threads when the previous callback exceeds
+        // the period — observed in production on stall-recovery (network reinit makes the
+        // PerformanceCollector's NetworkInterface.GetAllNetworkInterfaces() call slow enough
+        // to overlap the next 30s tick), producing two parallel Collect calls that race on
+        // the collector's shared baseline state and emit duplicate snapshots. Using
+        // Interlocked here keeps the sample interval honest and prevents ghost samples.
+        private int _collecting;
 
         protected CollectorBase(
             string sessionId,
@@ -96,10 +104,26 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry
             _timer.Change(TimeSpan.Zero, interval);
         }
 
-        private void CollectSafe()
+        /// <summary>
+        /// Reentrancy-guarded entry point invoked by <see cref="_timer"/>. <c>internal</c> so the
+        /// V2 test assembly (via <c>InternalsVisibleTo</c>) can drive the guard directly without
+        /// having to spin up a real timer + wait on real wall-clock time.
+        /// </summary>
+        internal void CollectSafe()
         {
+            // Skip the tick if a previous Collect is still running. The Timer keeps firing in
+            // the background, so the next eligible tick will pick up where this one declined —
+            // the sample cadence stays aligned with the configured interval rather than the
+            // duration of any single slow Collect.
+            if (Interlocked.CompareExchange(ref _collecting, 1, 0) != 0)
+            {
+                Logger.Debug($"[{GetType().Name}] tick skipped — previous Collect still running");
+                return;
+            }
+
             try { Collect(); }
             catch (Exception ex) { Logger.Error($"[{GetType().Name}] Collection error: {ex.Message}", ex); }
+            finally { Interlocked.Exchange(ref _collecting, 0); }
         }
     }
 }

src/Agent/AutopilotMonitor.Agent.V2.Core/Security/EnrollmentRegistryDetector.cs

@@ -22,6 +22,15 @@ namespace AutopilotMonitor.Agent.V2.Core.Security
         /// Reads <c>CloudAssignedDomainJoinMethod</c> from the Autopilot policy cache. Returns
         /// <c>true</c> when the profile was deployed with Hybrid Azure AD Join
         /// (<c>CloudAssignedDomainJoinMethod == 1</c>), <c>false</c> otherwise or on any error.
+        /// <para>
+        /// On some Windows builds (observed on Win11 23H2 Lenovo / 22631.4317) the
+        /// <c>CloudAssignedDomainJoinMethod</c> value is not persisted as a top-level registry
+        /// value — it lives only inside the JSON blob stored under <c>PolicyJsonCache</c>. The
+        /// <c>DeviceInfoCollector</c> already handles this when emitting the
+        /// <c>autopilot_profile</c> event; this detector mirrors the same fallback so the
+        /// session is registered with the correct <c>IsHybridJoin</c> flag instead of a stale
+        /// <c>false</c>.
+        /// </para>
         /// </summary>
         public static bool DetectHybridJoin()
         {
@@ -30,8 +39,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Security
                 using (var key = Registry.LocalMachine.OpenSubKey(AutopilotPolicyCacheKey))
                 {
                     if (key == null) return false;
-                    var domainJoinMethod = key.GetValue("CloudAssignedDomainJoinMethod")?.ToString();
-                    return domainJoinMethod == "1";
+                    return ResolveHybridJoinFromValues(key.GetValue);
                 }
             }
             catch
@@ -40,6 +48,46 @@ namespace AutopilotMonitor.Agent.V2.Core.Security
             }
         }
 
+        /// <summary>
+        /// Pure decision logic for <see cref="DetectHybridJoin"/>. Exposed internally so the
+        /// fallback can be exercised without a real registry. <paramref name="getValue"/> is
+        /// expected to behave like <see cref="RegistryKey.GetValue(string)"/> — returns
+        /// <c>null</c> when the named value is absent.
+        /// </summary>
+        internal static bool ResolveHybridJoinFromValues(Func<string, object> getValue)
+        {
+            if (getValue == null) return false;
+
+            // Top-level value is authoritative when present (covers both "0" and "1").
+            var topLevel = getValue("CloudAssignedDomainJoinMethod")?.ToString();
+            if (topLevel != null)
+            {
+                return topLevel == "1";
+            }
+
+            // Fallback: on devices where the policy cache only carries the embedded JSON
+            // blob, look up the same key inside PolicyJsonCache.
+            var policyJson = getValue("PolicyJsonCache")?.ToString();
+            if (string.IsNullOrWhiteSpace(policyJson)) return false;
+
+            try
+            {
+                using (var doc = System.Text.Json.JsonDocument.Parse(policyJson))
+                {
+                    if (doc.RootElement.TryGetProperty("CloudAssignedDomainJoinMethod", out var prop))
+                    {
+                        return prop.ToString() == "1";
+                    }
+                }
+            }
+            catch
+            {
+                // Malformed PolicyJsonCache → conservative non-HAADJ default. Detection is
+                // best-effort and must never throw at session-registration time.
+            }
+            return false;
+        }
+
         /// <summary>
         /// Classifies the Autopilot flow based on the <c>AutopilotSettings</c> registry:
         /// <list type="bullet">

src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt

@@ -1 +1 @@
-457
+460

src/Agent/AutopilotMonitor.Agent/buildcounter.txt

@@ -1 +1 @@
-1211
+1212