From de411ba4ac7a9c0863f1cef338b3f0fa03221f3d Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Mon, 30 Mar 2026 11:11:57 +0000
Subject: [PATCH] fix: move logger calls outside locks, protect SignalsSeen
 reads

Self-review fixes for the EnrollmentTracker locking commit:
- NotifyDesktopArrived: move _logger.Debug outside lock (no I/O under lock)
- CollectDeviceInfoAtFinalizingSetup: same pattern
- EmitEnrollmentFailed: wrap _stateData.SignalsSeen copy in lock via
  SnapshotSignalsSeen() to prevent InvalidOperationException if
  RecordSignal modifies the list concurrently
- WriteFinalStatus: same fix for SignalsSeen access

https://claude.ai/code/session_01EWzmYXFmXUaPpeksi6WGKZ
---
 .../EnrollmentTracker.CompletionLogic.cs      | 33 ++++++++++++++-----
 .../Tracking/EnrollmentTracker.Diagnostics.cs | 18 ++++++----
 2 files changed, 36 insertions(+), 15 deletions(-)

diff --git a/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Tracking/EnrollmentTracker.CompletionLogic.cs b/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Tracking/EnrollmentTracker.CompletionLogic.cs
index 65974b3f..424f0166 100644
--- a/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Tracking/EnrollmentTracker.CompletionLogic.cs
+++ b/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Tracking/EnrollmentTracker.CompletionLogic.cs
@@ -345,7 +345,7 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Tracking
                 {
                     { "failureType", failureType },
                     { "failureSource", failureSource },
-                    { "signalsSeen", new List<string>(_stateData.SignalsSeen) },
+                    { "signalsSeen", SnapshotSignalsSeen() },
                     { "agentUptimeSeconds", (DateTime.UtcNow - _agentStartTimeUtc).TotalSeconds }
                 },
                 ImmediateUpload = true
@@ -601,6 +601,17 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Tracking
                 _logger.Debug($"EnrollmentTracker: signal '{signal}' already recorded — deduplicated");
         }
 
+        /// <summary>
+        /// Returns a thread-safe snapshot of _stateData.SignalsSeen for use in event data.
+        /// </summary>
+        private List<string> SnapshotSignalsSeen()
+        {
+            lock (_stateLock)
+            {
+                return new List<string>(_stateData.SignalsSeen);
+            }
+        }
+
         // ===== Unified Completion Logic =====
 
         /// <summary>
@@ -820,18 +831,24 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Tracking
         /// </summary>
         public void NotifyDesktopArrived()
         {
+            bool alreadyArrived;
             lock (_stateLock)
             {
-                if (_desktopArrived)
+                alreadyArrived = _desktopArrived;
+                if (!alreadyArrived)
                 {
-                    _logger.Debug("EnrollmentTracker: NotifyDesktopArrived called but already arrived — ignoring");
-                    return;
+                    _desktopArrived = true;
+                    _stateData.DesktopArrived = true;
+                    _stateData.DesktopArrivedUtc = DateTime.UtcNow;
                 }
-
-                _desktopArrived = true;
-                _stateData.DesktopArrived = true;
-                _stateData.DesktopArrivedUtc = DateTime.UtcNow;
             }
+
+            if (alreadyArrived)
+            {
+                _logger.Debug("EnrollmentTracker: NotifyDesktopArrived called but already arrived — ignoring");
+                return;
+            }
+
             RecordSignal("desktop_arrived");
             _logger.Info("EnrollmentTracker: Desktop arrival notified");
 
diff --git a/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Tracking/EnrollmentTracker.Diagnostics.cs b/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Tracking/EnrollmentTracker.Diagnostics.cs
index 0246ce7d..4e4f6ad6 100644
--- a/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Tracking/EnrollmentTracker.Diagnostics.cs
+++ b/src/Agent/AutopilotMonitor.Agent.Core/Monitoring/Tracking/EnrollmentTracker.Diagnostics.cs
@@ -47,14 +47,18 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Tracking
 
         private void CollectDeviceInfoAtFinalizingSetup(string triggerReason)
         {
+            bool alreadyCollected;
             lock (_stateLock)
             {
-                if (_finalDeviceInfoCollected)
-                {
-                    _logger.Debug($"EnrollmentTracker: final device info already collected, skipping (trigger: {triggerReason})");
-                    return;
-                }
-                _finalDeviceInfoCollected = true;
+                alreadyCollected = _finalDeviceInfoCollected;
+                if (!alreadyCollected)
+                    _finalDeviceInfoCollected = true;
+            }
+
+            if (alreadyCollected)
+            {
+                _logger.Debug($"EnrollmentTracker: final device info already collected, skipping (trigger: {triggerReason})");
+                return;
             }
             _logger.Info($"EnrollmentTracker: triggering final device info collection at FinalizingSetup (trigger: {triggerReason})");
             CollectDeviceInfoAtEnd();
@@ -338,7 +342,7 @@ namespace AutopilotMonitor.Agent.Core.Monitoring.Tracking
                     { "helloOutcome", _espAndHelloTracker?.HelloOutcome ?? "unknown" },
                     { "enrollmentType", _enrollmentType ?? "unknown" },
                     { "agentUptimeSeconds", (DateTime.UtcNow - _agentStartTimeUtc).TotalSeconds },
-                    { "signalsSeen", _stateData.SignalsSeen ?? new List<string>() },
+                    { "signalsSeen", SnapshotSignalsSeen() },
                     { "signalTimestamps", signalTimestamps },
                     { "appSummary", new Dictionary<string, object>
                         {