diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/EspAndHelloTrackerFinalizingGuardTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/EspAndHelloTrackerFinalizingGuardTests.cs new file mode 100644 index 00000000..b2863099 --- /dev/null +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/EspAndHelloTrackerFinalizingGuardTests.cs @@ -0,0 +1,173 @@ +using System; +using System.Linq; +using AutopilotMonitor.Agent.V2.Core.Logging; +using AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals; +using AutopilotMonitor.Agent.V2.Core.Orchestration; +using AutopilotMonitor.Agent.V2.Core.SignalAdapters; +using AutopilotMonitor.Agent.V2.Core.Tests.Harness; +using AutopilotMonitor.Agent.V2.Core.Tests.Orchestration; +using AutopilotMonitor.DecisionCore.Engine; +using AutopilotMonitor.DecisionCore.Signals; +using AutopilotMonitor.Shared.Models; +using Xunit; + +namespace AutopilotMonitor.Agent.V2.Core.Tests.Monitoring +{ + /// + /// Plan §6 Fix 7 — the coordinator must NOT forward a + /// Shell-Core esp_exiting as a synthetic EspPhaseChanged(FinalizingSetup) + /// decision signal when the enrollment is a Classic V1 flow (SkipUser=false) and we have + /// not yet seen any AccountSetup activity — because that first exit is the Device-ESP to + /// Account-ESP handoff, not the true final ESP exit. Forwarding prematurely drives the + /// reducer into AwaitingHello and arms HelloSafety from the wrong baseline (see + /// session 30410cd7). + /// + /// Exit forwards that must keep working: + /// + /// + /// SkipUser=true — device-only / full-skip flows; single esp_exiting IS final. + /// SkipUser unknown — defensive; never block when we don't know. + /// Non-esp_exiting reasons (e.g. hello_wizard_started) — the guard + /// only targets the ambiguous exit event. + /// SkipUser=false BUT AccountSetup activity observed — this is the post-AccountSetup + /// final exit. + /// + /// + public sealed class EspAndHelloTrackerFinalizingGuardTests + { + private static readonly DateTime Fixed = new DateTime(2026, 4, 23, 18, 57, 45, DateTimeKind.Utc); + + private sealed class Fixture : IDisposable + { + public TempDirectory Tmp { get; } = new TempDirectory(); + public AgentLogger Logger { get; } + public FakeSignalIngressSink Ingress { get; } = new FakeSignalIngressSink(); + public VirtualClock Clock { get; } = new VirtualClock(Fixed); + + public EspAndHelloTracker BuildCoordinator( + bool? skipUser, + bool? skipDevice = false, + bool accountSetupSeen = false) + { + var trackerPost = new InformationalEventPost(new FakeSignalIngressSink(), Clock); + return new EspAndHelloTracker( + sessionId: "S1", + tenantId: "T1", + post: trackerPost, + logger: Logger, + skipConfigProbe: () => (skipUser, skipDevice), + accountSetupActivityProbe: () => accountSetupSeen); + } + + public Fixture() { Logger = new AgentLogger(Tmp.Path, AgentLogLevel.Debug); } + + public void Dispose() { Tmp.Dispose(); } + } + + [Fact] + public void ForwardsFinalizing_whenSkipUserTrue_pluginsEspExit() + { + // SkipUser=true → single Shell-Core esp_exiting IS the final exit. Forward. + using var f = new Fixture(); + using var coordinator = f.BuildCoordinator(skipUser: true); + using var adapter = new EspAndHelloTrackerAdapter(coordinator, f.Ingress, f.Clock); + + coordinator.TriggerFinalizingSetupPhaseForTest("esp_exiting"); + + var posted = Assert.Single(f.Ingress.Posted); + Assert.Equal(DecisionSignalKind.EspPhaseChanged, posted.Kind); + Assert.Equal(EnrollmentPhase.FinalizingSetup.ToString(), posted.Payload![SignalPayloadKeys.EspPhase]); + Assert.Equal("esp_exiting", posted.Payload["reason"]); + } + + [Fact] + public void DoesNotForwardFinalizing_whenSkipUserFalseAndNoAccountSetup() + { + // Classic V1 intermediate Device-ESP exit: SkipUser=false, AccountSetup not yet seen. + // Swallow — the real final exit arrives later. + using var f = new Fixture(); + using var coordinator = f.BuildCoordinator(skipUser: false, accountSetupSeen: false); + using var adapter = new EspAndHelloTrackerAdapter(coordinator, f.Ingress, f.Clock); + + coordinator.TriggerFinalizingSetupPhaseForTest("esp_exiting"); + + Assert.Empty(f.Ingress.Posted); + } + + [Fact] + public void ForwardsFinalizing_whenSkipUserFalseAndAccountSetupAlreadySeen() + { + // Post-AccountSetup second esp_exiting — genuine final exit. Forward. + using var f = new Fixture(); + using var coordinator = f.BuildCoordinator(skipUser: false, accountSetupSeen: true); + using var adapter = new EspAndHelloTrackerAdapter(coordinator, f.Ingress, f.Clock); + + coordinator.TriggerFinalizingSetupPhaseForTest("esp_exiting"); + + var posted = Assert.Single(f.Ingress.Posted); + Assert.Equal(DecisionSignalKind.EspPhaseChanged, posted.Kind); + Assert.Equal(EnrollmentPhase.FinalizingSetup.ToString(), posted.Payload![SignalPayloadKeys.EspPhase]); + } + + [Fact] + public void ForwardsFinalizing_whenSkipUserUnknown_regardlessOfAccountSetup() + { + // Defensive: unknown skip-flag → forward (preserves pre-fix behavior, no new breakage). + using var f = new Fixture(); + using var coordinator = f.BuildCoordinator(skipUser: null, accountSetupSeen: false); + using var adapter = new EspAndHelloTrackerAdapter(coordinator, f.Ingress, f.Clock); + + coordinator.TriggerFinalizingSetupPhaseForTest("esp_exiting"); + + Assert.Single(f.Ingress.Posted); + } + + [Fact] + public void ForwardsFinalizing_whenHelloWizardStarted_regardlessOfSkipUser() + { + // Only esp_exiting is ambiguous. hello_wizard_started and other reasons must keep + // forwarding unconditionally — SkipUser does not affect Hello-wizard detection. + using var f = new Fixture(); + using var coordinator = f.BuildCoordinator(skipUser: false, accountSetupSeen: false); + using var adapter = new EspAndHelloTrackerAdapter(coordinator, f.Ingress, f.Clock); + + coordinator.TriggerFinalizingSetupPhaseForTest("hello_wizard_started"); + + var posted = Assert.Single(f.Ingress.Posted); + Assert.Equal(DecisionSignalKind.EspPhaseChanged, posted.Kind); + Assert.Equal("hello_wizard_started", posted.Payload!["reason"]); + } + + [Fact] + public void SwallowedFirstExit_doesNotBlockSecondExitAfterAccountSetup() + { + // Full Classic V1 sequence: Device-ESP esp_exiting (swallowed) → AccountSetup + // activity appears → second esp_exiting (forwarded). Mirrors the Fix-7 production + // sequence for session 30410cd7. + using var f = new Fixture(); + bool accountSetupSeen = false; + var trackerPost = new InformationalEventPost(new FakeSignalIngressSink(), f.Clock); + using var coordinator = new EspAndHelloTracker( + sessionId: "S1", + tenantId: "T1", + post: trackerPost, + logger: f.Logger, + skipConfigProbe: () => ((bool?)false, (bool?)false), + accountSetupActivityProbe: () => accountSetupSeen); + using var adapter = new EspAndHelloTrackerAdapter(coordinator, f.Ingress, f.Clock); + + // First exit — swallowed. + coordinator.TriggerFinalizingSetupPhaseForTest("esp_exiting"); + Assert.Empty(f.Ingress.Posted); + + // Now AccountSetup activity arrives (registry picks up the subcategory JSON). + accountSetupSeen = true; + + // Second exit — forwarded. + coordinator.TriggerFinalizingSetupPhaseForTest("esp_exiting"); + var posted = Assert.Single(f.Ingress.Posted); + Assert.Equal(DecisionSignalKind.EspPhaseChanged, posted.Kind); + Assert.Equal(EnrollmentPhase.FinalizingSetup.ToString(), posted.Payload![SignalPayloadKeys.EspPhase]); + } + } +} diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/EspAndHelloTracker.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/EspAndHelloTracker.cs index 97a376e8..0a626348 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/EspAndHelloTracker.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/EspAndHelloTracker.cs @@ -35,12 +35,20 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals private readonly int _modernDeploymentBackfillLookbackMinutes; private readonly string _stateDirectory; private readonly int[] _modernDeploymentHarmlessEventIds; + private readonly Func<(bool? skipUser, bool? skipDevice)> _skipConfigProbe; + private readonly Func _accountSetupActivityProbe; private HelloTracker _helloTracker; private ShellCoreTracker _shellCoreTracker; private ProvisioningStatusTracker _provisioningTracker; private ModernDeploymentTracker _modernDeploymentTracker; + // Plan §6 Fix 7 — lazy-cached skip-user flag from HKLM\...\FirstSync\SkipUserStatusPage. + // Read once per tracker lifetime on first esp_exiting forward; null until read. + private bool? _skipUserEspCached; + private bool _skipConfigProbed; + private readonly object _skipConfigLock = new object(); + /// /// Callback invoked when Hello provisioning completes (successfully, failed, skipped, or timeout). /// Based on events 300/301/362 only — NOT on event 360 (which is just a snapshot). @@ -121,7 +129,9 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals bool modernDeploymentBackfillEnabled = true, int modernDeploymentBackfillLookbackMinutes = 30, string stateDirectory = null, - int[] modernDeploymentHarmlessEventIds = null) + int[] modernDeploymentHarmlessEventIds = null, + Func<(bool? skipUser, bool? skipDevice)> skipConfigProbe = null, + Func accountSetupActivityProbe = null) { _sessionId = sessionId ?? throw new ArgumentNullException(nameof(sessionId)); _tenantId = tenantId ?? throw new ArgumentNullException(nameof(tenantId)); @@ -134,6 +144,12 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals _modernDeploymentBackfillLookbackMinutes = modernDeploymentBackfillLookbackMinutes; _stateDirectory = stateDirectory != null ? Environment.ExpandEnvironmentVariables(stateDirectory) : null; _modernDeploymentHarmlessEventIds = modernDeploymentHarmlessEventIds; + // Test seam: allow injection of a fake skip-config reader; defaults to the real + // registry probe so production never has to think about this parameter. + _skipConfigProbe = skipConfigProbe ?? (() => EspSkipConfigurationProbe.Read(_logger)); + // Test seam: allow injection of a fake AccountSetup-activity probe. Production uses + // the provisioning tracker's live registry-derived flag. + _accountSetupActivityProbe = accountSetupActivityProbe ?? (() => _provisioningTracker?.HasAccountSetupActivity == true); } // ===================================================================== @@ -296,10 +312,86 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals private void OnFinalizingSetupPhaseTriggered(object sender, string reason) { + // Plan §6 Fix 7 — on Classic V1 enrollments (SkipUser=false) Shell-Core emits TWO + // esp_exiting (62407) events: one when Device-ESP hands off to Account-ESP, and one + // at the true final exit. Forwarding the first one as EspPhaseChanged(Finalizing) + // drives the reducer into AwaitingHello prematurely and arms HelloSafety from the + // wrong baseline (see session 30410cd7 where the deadline would have fired first). + // Swallow the intermediate forward when SkipUser is explicitly false AND the + // provisioning tracker has not yet seen any AccountSetup activity. + if (string.Equals(reason, "esp_exiting", StringComparison.OrdinalIgnoreCase) + && IsIntermediateDeviceEspExit()) + { + _logger.Info( + "EspAndHelloTracker: swallowing intermediate Device-ESP esp_exiting (SkipUser=false, no AccountSetup activity yet) — waiting for real final exit"); + return; + } + try { FinalizingSetupPhaseTriggered?.Invoke(this, reason); } catch (Exception ex) { _logger.Error("Error forwarding FinalizingSetupPhaseTriggered", ex); } } + /// + /// Plan §6 Fix 7 — returns true when the current esp_exiting forward + /// should be suppressed because it is the Device-ESP intermediate exit on a Classic + /// (SkipUser=false) enrollment that has not yet reached the AccountSetup phase. + /// + /// Returns false (i.e. "forward the signal") when any of the following hold: + /// + /// + /// SkipUser is unknown or explicitly true (device-only / SkipUser flow — + /// AwaitingHello is legitimately reachable directly after Device-ESP). + /// The provisioning tracker has observed AccountSetup activity (registry JSON + /// under AccountSetupCategory.Status has surfaced at least one subcategory), + /// i.e. the current exit is the final post-Account-ESP one. + /// + /// + private bool IsIntermediateDeviceEspExit() + { + // If AccountSetup activity has appeared, we're at the true final exit. + bool accountSetupSeen; + try { accountSetupSeen = _accountSetupActivityProbe(); } + catch (Exception ex) + { + _logger.Debug($"EspAndHelloTracker: account-setup-activity probe threw: {ex.Message}"); + accountSetupSeen = false; + } + if (accountSetupSeen) return false; + + var skipUser = GetSkipUserEspCached(); + + // Unknown or explicitly-skipping => forward as before. Only block when we are + // certain the enrollment does NOT skip the user ESP (i.e. a second esp_exiting is + // expected). + return skipUser == false; + } + + private bool? GetSkipUserEspCached() + { + lock (_skipConfigLock) + { + if (_skipConfigProbed) return _skipUserEspCached; + _skipConfigProbed = true; + try + { + var (skipUser, _) = _skipConfigProbe(); + _skipUserEspCached = skipUser; + } + catch (Exception ex) + { + _logger.Debug($"EspAndHelloTracker: skip-config probe threw: {ex.Message}"); + _skipUserEspCached = null; + } + return _skipUserEspCached; + } + } + + // Test seam — invokes the guarded forward path without driving the underlying ShellCore + // event-log watcher. Mirrors the arg signature of the live event handler. Internal so + // only the in-repo test project (InternalsVisibleTo) can use it. + internal void TriggerFinalizingSetupPhaseForTest(string reason) => + OnFinalizingSetupPhaseTriggered(this, reason); + private void OnWhiteGloveCompleted(object sender, EventArgs e) { try { WhiteGloveCompleted?.Invoke(this, EventArgs.Empty); } diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/EspSkipConfigurationProbe.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/EspSkipConfigurationProbe.cs new file mode 100644 index 00000000..e953439d --- /dev/null +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/EspSkipConfigurationProbe.cs @@ -0,0 +1,98 @@ +using System; +using AutopilotMonitor.Agent.V2.Core.Logging; +using Microsoft.Win32; + +namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals +{ + /// + /// Plan §6 Fix 7/9 — single source of truth for the Autopilot-enrollment ESP skip flags + /// (SkipUserStatusPage / SkipDeviceStatusPage) that the MDM CSP writes under + /// HKLM\SOFTWARE\Microsoft\Enrollments\{guid}\FirstSync during FirstSync. + /// + /// Two consumers: + /// + /// — emits the + /// esp_config_detected event + the EspConfigDetected decision signal + /// at agent start and at first DeviceSetup-phase detection. + /// — guards the coordinator's + /// synthetic EspPhaseChanged(FinalizingSetup) forward: a Classic V1 enrollment + /// with SkipUser=false sees TWO esp_exiting events (Device-ESP exit and + /// Account-ESP exit) and only the second is a true final exit. + /// + /// Reading from the same authoritative registry location in both places keeps the reducer's + /// fact and the + /// tracker's guard in lockstep — the alternative (passing the value around) has too many + /// subtle lifecycle order issues to be safe. + /// + /// + /// CSP values: 0xFFFFFFFF = skip (ESP page not shown), 0 = show, key missing + /// = unknown. Defensive interpretation: non-null AND non-zero = skip. + /// + /// + internal static class EspSkipConfigurationProbe + { + internal const string EnrollmentsKeyPath = @"SOFTWARE\Microsoft\Enrollments"; + internal const int MdmEnrollmentType = 6; + + /// + /// Reads the current device's SkipUserStatusPage / SkipDeviceStatusPage + /// flags. Returns (null, null) when the enrollment key is missing or unreadable — + /// callers must treat null as "unknown", not "false". + /// + /// Optional logger — Debug-level trace only; no warn/error. + public static (bool? skipUser, bool? skipDevice) Read(AgentLogger logger = null) + { + bool? skipUser = null; + bool? skipDevice = null; + + try + { + using (var enrollmentsKey = Registry.LocalMachine.OpenSubKey(EnrollmentsKeyPath)) + { + if (enrollmentsKey == null) + { + logger?.Debug("EspSkipConfigurationProbe: Enrollments registry key not found"); + return (null, null); + } + + foreach (var guid in enrollmentsKey.GetSubKeyNames()) + { + using (var enrollmentKey = enrollmentsKey.OpenSubKey(guid)) + { + if (enrollmentKey == null) continue; + + var enrollmentType = enrollmentKey.GetValue("EnrollmentType"); + if (enrollmentType == null || Convert.ToInt32(enrollmentType) != MdmEnrollmentType) + continue; + + using (var firstSyncKey = enrollmentKey.OpenSubKey("FirstSync")) + { + if (firstSyncKey == null) + { + logger?.Debug($"EspSkipConfigurationProbe: FirstSync subkey not found for enrollment {guid}"); + return (null, null); + } + + var rawSkipUser = firstSyncKey.GetValue("SkipUserStatusPage"); + if (rawSkipUser != null) + skipUser = Convert.ToInt32(rawSkipUser) != 0; + + var rawSkipDevice = firstSyncKey.GetValue("SkipDeviceStatusPage"); + if (rawSkipDevice != null) + skipDevice = Convert.ToInt32(rawSkipDevice) != 0; + } + + break; + } + } + } + } + catch (Exception ex) + { + logger?.Debug($"EspSkipConfigurationProbe: read threw: {ex.Message}"); + } + + return (skipUser, skipDevice); + } + } +} diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/DeviceInfo/DeviceInfoCollector.NetworkAndSecurity.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/DeviceInfo/DeviceInfoCollector.NetworkAndSecurity.cs index 5f7b9664..18456b1d 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/DeviceInfo/DeviceInfoCollector.NetworkAndSecurity.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/DeviceInfo/DeviceInfoCollector.NetworkAndSecurity.cs @@ -8,7 +8,10 @@ using System.Text.Json; using System.Threading.Tasks; using AutopilotMonitor.Agent.V2.Core.Logging; using AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals; +using AutopilotMonitor.Agent.V2.Core.Orchestration; using AutopilotMonitor.Agent.V2.Core.Security; +using AutopilotMonitor.DecisionCore.Engine; +using AutopilotMonitor.DecisionCore.Signals; using AutopilotMonitor.Shared.Models; using Microsoft.Win32; @@ -503,76 +506,31 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.DeviceInfo } /// - /// Reads ESP skip configuration from the enrollment FirstSync registry key. - /// CSP SkipUserStatusPage/SkipDeviceStatusPage are written as DWORD: - /// 0xFFFFFFFF = skip (ESP page not shown), 0 = show, key missing = unknown. - /// We use defensive interpretation: non-null AND non-zero = skip. + /// Reads ESP skip configuration from the enrollment FirstSync registry key via the + /// shared (also used by + /// so the tracker's + /// SkipUser guard and the reducer's SkipUserEsp fact stay in lockstep, plan §6 Fix 7/9). /// private (bool? skipUserStatusPage, bool? skipDeviceStatusPage) CollectEspConfiguration() { bool? skipUser = null; bool? skipDevice = null; - string enrollmentGuid = null; try { - const string enrollmentsKeyPath = @"SOFTWARE\Microsoft\Enrollments"; - - using (var enrollmentsKey = Registry.LocalMachine.OpenSubKey(enrollmentsKeyPath)) - { - if (enrollmentsKey == null) - { - _logger.Debug("EnrollmentTracker: Enrollments registry key not found — ESP config unknown"); - return (null, null); - } - - // Find the MDM enrollment entry (EnrollmentType == 6) - foreach (var guid in enrollmentsKey.GetSubKeyNames()) - { - using (var enrollmentKey = enrollmentsKey.OpenSubKey(guid)) - { - if (enrollmentKey == null) continue; - - var enrollmentType = enrollmentKey.GetValue("EnrollmentType"); - if (enrollmentType == null || Convert.ToInt32(enrollmentType) != 6) - continue; - - enrollmentGuid = guid; - - // Read FirstSync subkey - using (var firstSyncKey = enrollmentKey.OpenSubKey("FirstSync")) - { - if (firstSyncKey == null) - { - _logger.Debug($"EnrollmentTracker: FirstSync subkey not found for enrollment {guid}"); - break; - } - - var rawSkipUser = firstSyncKey.GetValue("SkipUserStatusPage"); - if (rawSkipUser != null) - skipUser = Convert.ToInt32(rawSkipUser) != 0; - - var rawSkipDevice = firstSyncKey.GetValue("SkipDeviceStatusPage"); - if (rawSkipDevice != null) - skipDevice = Convert.ToInt32(rawSkipDevice) != 0; - } - - break; // Found the MDM enrollment entry - } - } - } + (skipUser, skipDevice) = EspSkipConfigurationProbe.Read(_logger); var data = new Dictionary { { "source", "registry_firstsync" } }; - if (enrollmentGuid != null) data["enrollmentGuid"] = enrollmentGuid; if (skipUser.HasValue) data["skipUserStatusPage"] = skipUser.Value; if (skipDevice.HasValue) data["skipDeviceStatusPage"] = skipDevice.Value; var summary = $"SkipUser={skipUser?.ToString() ?? "unknown"}, SkipDevice={skipDevice?.ToString() ?? "unknown"}"; _logger.Info($"EnrollmentTracker: ESP configuration detected — {summary}"); EmitDeviceInfoEvent("esp_config_detected", $"ESP configuration: {summary}", data); + PostEspConfigDetectedSignal(skipUser, skipDevice); } catch (Exception ex) { @@ -764,6 +722,49 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.DeviceInfo }); } + /// + /// Plan §6 Fix 9 — post an decision + /// signal alongside the user-facing esp_config_detected event so the reducer's + /// / + /// facts get + /// populated. Fix 8's reducer guards then block premature AwaitingHello transitions + /// on Classic (SkipUser=false) enrollments. + /// + /// Fire-once per collector instance: re-collection paths + /// () would otherwise post the signal multiple times + /// and clutter the reducer journal with no-op steps. + /// + /// + private void PostEspConfigDetectedSignal(bool? skipUser, bool? skipDevice) + { + if (_signalIngress == null || _clock == null) return; + if (_espConfigSignalPosted) return; + if (skipUser == null && skipDevice == null) return; + _espConfigSignalPosted = true; + + var payload = new Dictionary(StringComparer.Ordinal); + if (skipUser.HasValue) + payload[SignalPayloadKeys.SkipUserEsp] = skipUser.Value ? "true" : "false"; + if (skipDevice.HasValue) + payload[SignalPayloadKeys.SkipDeviceEsp] = skipDevice.Value ? "true" : "false"; + + var derivationInputs = new Dictionary(payload, StringComparer.Ordinal) + { + ["source"] = "registry_firstsync", + }; + + _signalIngress.Post( + kind: DecisionSignalKind.EspConfigDetected, + occurredAtUtc: _clock.UtcNow, + sourceOrigin: "DeviceInfoCollector", + evidence: new Evidence( + kind: EvidenceKind.Raw, + identifier: "esp_config_detected", + summary: $"SkipUser={skipUser?.ToString() ?? "unknown"}, SkipDevice={skipDevice?.ToString() ?? "unknown"}", + derivationInputs: derivationInputs), + payload: payload); + } + private static string TryFormatJson(string input) { if (string.IsNullOrWhiteSpace(input)) diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/DeviceInfo/DeviceInfoCollector.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/DeviceInfo/DeviceInfoCollector.cs index 406b06cf..e267710e 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/DeviceInfo/DeviceInfoCollector.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/DeviceInfo/DeviceInfoCollector.cs @@ -8,6 +8,8 @@ using System.Text.Json; using System.Threading.Tasks; using AutopilotMonitor.Agent.V2.Core.Logging; using AutopilotMonitor.Agent.V2.Core.Orchestration; +using AutopilotMonitor.DecisionCore.Engine; +using AutopilotMonitor.DecisionCore.Signals; using AutopilotMonitor.Shared.Models; using Microsoft.Win32; using AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment; @@ -28,6 +30,9 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.DeviceInfo private readonly string _tenantId; private readonly InformationalEventPost _post; private readonly AgentLogger _logger; + private readonly ISignalIngressSink _signalIngress; + private readonly IClock _clock; + private bool _espConfigSignalPosted; private const string RegKeyWindowsCurrentVersion = @"HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows NT\CurrentVersion"; @@ -35,12 +40,16 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.DeviceInfo string sessionId, string tenantId, InformationalEventPost post, - AgentLogger logger) + AgentLogger logger, + ISignalIngressSink signalIngress = null, + IClock clock = null) { _sessionId = sessionId ?? throw new ArgumentNullException(nameof(sessionId)); _tenantId = tenantId ?? throw new ArgumentNullException(nameof(tenantId)); _post = post ?? throw new ArgumentNullException(nameof(post)); _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + _signalIngress = signalIngress; + _clock = clock; } /// diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/DefaultComponentFactory.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/DefaultComponentFactory.cs index c4996885..0df7c726 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/DefaultComponentFactory.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/DefaultComponentFactory.cs @@ -426,8 +426,11 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration if (clock == null) throw new ArgumentNullException(nameof(clock)); _logger = logger; var post = new InformationalEventPost(ingress, clock); + // Plan §6 Fix 9 — the collector also posts an EspConfigDetected decision signal + // when it reads the FirstSync SkipUser/SkipDevice registry values, so that Fix 8's + // reducer guards have the SkipUserEsp/SkipDeviceEsp state facts to read. _collector = new Monitoring.Telemetry.DeviceInfo.DeviceInfoCollector( - sessionId, tenantId, post, logger); + sessionId, tenantId, post, logger, ingress, clock); } public void Start() diff --git a/src/Agent/AutopilotMonitor.DecisionCore.Tests/ClassicAwaitingHelloGuardTests.cs b/src/Agent/AutopilotMonitor.DecisionCore.Tests/ClassicAwaitingHelloGuardTests.cs new file mode 100644 index 00000000..090ad325 --- /dev/null +++ b/src/Agent/AutopilotMonitor.DecisionCore.Tests/ClassicAwaitingHelloGuardTests.cs @@ -0,0 +1,204 @@ +using System; +using System.Collections.Generic; +using AutopilotMonitor.DecisionCore.Engine; +using AutopilotMonitor.DecisionCore.Signals; +using AutopilotMonitor.DecisionCore.State; +using AutopilotMonitor.Shared.Models; +using Xunit; + +namespace AutopilotMonitor.DecisionCore.Tests +{ + /// + /// Plan §6 Fix 8 — the Classic reducer must only promote to + /// when the promotion is legitimate: either + /// is set (i.e. we're past the post-Account-ESP final exit), or + /// is explicitly true (device-only / SkipUser + /// flow where a single esp_exiting IS the final one). + /// + /// Applies symmetrically to (FinalizingSetup) + /// AND . On an early / intermediate signal the + /// stage stays unchanged, no HelloSafety is armed, and observability facts + /// ( / ) + /// are still recorded so classifiers see the full picture. + /// + /// + public sealed class ClassicAwaitingHelloGuardTests + { + private static readonly DateTime Fixed = new DateTime(2026, 4, 23, 18, 57, 45, DateTimeKind.Utc); + + // ========================================================= EspPhaseChanged (Finalizing) + + [Fact] + public void EspPhaseChanged_FinalizingSetup_withSkipUserTrue_transitionsToAwaitingHello() + { + var engine = new DecisionEngine(); + var state = BuildState(stage: SessionStage.EspDeviceSetup, skipUser: true); + + var step = engine.Reduce(state, MakePhaseSignal(EnrollmentPhase.FinalizingSetup, ordinal: 5)); + + Assert.Equal(SessionStage.AwaitingHello, step.NewState.Stage); + Assert.True(step.Transition.Taken); + Assert.Equal(SessionStage.EspDeviceSetup, step.Transition.FromStage); + Assert.Equal(SessionStage.AwaitingHello, step.Transition.ToStage); + Assert.NotNull(step.NewState.FinalizingEnteredUtc); + Assert.Equal(EnrollmentPhase.FinalizingSetup, step.NewState.CurrentEnrollmentPhase!.Value); + } + + [Fact] + public void EspPhaseChanged_FinalizingSetup_withSkipUserFalse_beforeAccountSetup_keepsStage() + { + // Classic intermediate Device-ESP exit synthesized as FinalizingSetup. Guard blocks + // the AwaitingHello promotion; the stage stays on EspDeviceSetup. + var engine = new DecisionEngine(); + var state = BuildState(stage: SessionStage.EspDeviceSetup, skipUser: false); + + var step = engine.Reduce(state, MakePhaseSignal(EnrollmentPhase.FinalizingSetup, ordinal: 5)); + + Assert.Equal(SessionStage.EspDeviceSetup, step.NewState.Stage); + Assert.True(step.Transition.Taken); + Assert.Equal(step.Transition.FromStage, step.Transition.ToStage); + // Observability is still recorded. + Assert.NotNull(step.NewState.FinalizingEnteredUtc); + Assert.Equal(EnrollmentPhase.FinalizingSetup, step.NewState.CurrentEnrollmentPhase!.Value); + // No HelloSafety on this path (HandleEspPhaseChangedV1 never arms it — but verify + // anyway for belt-and-suspenders). + Assert.DoesNotContain(step.NewState.Deadlines, d => d.Name == DeadlineNames.HelloSafety); + } + + [Fact] + public void EspPhaseChanged_FinalizingSetup_withSkipUserFalse_afterAccountSetup_transitionsToAwaitingHello() + { + // Post-Account-ESP final exit: guard allows promotion. + var engine = new DecisionEngine(); + var state = BuildState(stage: SessionStage.EspAccountSetup, skipUser: false, accountSetupEntered: true); + + var step = engine.Reduce(state, MakePhaseSignal(EnrollmentPhase.FinalizingSetup, ordinal: 9)); + + Assert.Equal(SessionStage.AwaitingHello, step.NewState.Stage); + Assert.NotNull(step.NewState.FinalizingEnteredUtc); + } + + [Fact] + public void EspPhaseChanged_FinalizingSetup_withSkipUserUnknown_beforeAccountSetup_keepsStage() + { + // Defensive: unknown SkipUser does NOT unlock AwaitingHello. Otherwise a missing + // EspConfigDetected signal (e.g. registry race at enrollment start) would reopen the + // original bug. + var engine = new DecisionEngine(); + var state = BuildState(stage: SessionStage.EspDeviceSetup, skipUser: null); + + var step = engine.Reduce(state, MakePhaseSignal(EnrollmentPhase.FinalizingSetup, ordinal: 3)); + + Assert.Equal(SessionStage.EspDeviceSetup, step.NewState.Stage); + } + + // =============================================================== EspExiting (real arm) + + [Fact] + public void EspExiting_withSkipUserTrue_transitionsToAwaitingHello_armsHelloSafety() + { + var engine = new DecisionEngine(); + var state = BuildState(stage: SessionStage.EspDeviceSetup, skipUser: true); + + var step = engine.Reduce(state, MakeExitingSignal(ordinal: 5)); + + Assert.Equal(SessionStage.AwaitingHello, step.NewState.Stage); + Assert.NotNull(step.NewState.EspFinalExitUtc); + Assert.Contains(step.NewState.Deadlines, d => d.Name == DeadlineNames.HelloSafety); + Assert.Contains(step.Effects, + e => e.Kind == DecisionEffectKind.ScheduleDeadline && e.Deadline?.Name == DeadlineNames.HelloSafety); + } + + [Fact] + public void EspExiting_withSkipUserFalse_beforeAccountSetup_staysInStage_noHelloSafety_butRecordsFact() + { + // Device-ESP intermediate exit. Classic's HandleEspExitingV1 must preserve the + // EspFinalExitUtc fact for observability but block the stage change + deadline arm. + var engine = new DecisionEngine(); + var state = BuildState(stage: SessionStage.EspDeviceSetup, skipUser: false); + + var step = engine.Reduce(state, MakeExitingSignal(ordinal: 5)); + + Assert.Equal(SessionStage.EspDeviceSetup, step.NewState.Stage); + Assert.NotNull(step.NewState.EspFinalExitUtc); + Assert.Empty(step.NewState.Deadlines); + Assert.Empty(step.Effects); + Assert.True(step.Transition.Taken); + } + + [Fact] + public void EspExiting_withSkipUserFalse_afterAccountSetup_transitionsToAwaitingHello_armsHelloSafety() + { + var engine = new DecisionEngine(); + var state = BuildState( + stage: SessionStage.EspAccountSetup, + skipUser: false, + accountSetupEntered: true); + + var step = engine.Reduce(state, MakeExitingSignal(ordinal: 9)); + + Assert.Equal(SessionStage.AwaitingHello, step.NewState.Stage); + Assert.Contains(step.NewState.Deadlines, d => d.Name == DeadlineNames.HelloSafety); + } + + [Fact] + public void EspExiting_withSkipUserUnknown_beforeAccountSetup_staysInStage() + { + var engine = new DecisionEngine(); + var state = BuildState(stage: SessionStage.EspDeviceSetup, skipUser: null); + + var step = engine.Reduce(state, MakeExitingSignal(ordinal: 5)); + + Assert.Equal(SessionStage.EspDeviceSetup, step.NewState.Stage); + Assert.Empty(step.NewState.Deadlines); + } + + // ====================================================================== test helpers + + private static DecisionState BuildState( + SessionStage stage, + bool? skipUser, + bool accountSetupEntered = false) + { + var builder = DecisionState.CreateInitial("s", "t") + .ToBuilder() + .WithStage(stage) + .WithStepIndex(2) + .WithLastAppliedSignalOrdinal(1); + if (skipUser.HasValue) builder.SkipUserEsp = new SignalFact(skipUser.Value, 0); + if (accountSetupEntered) builder.AccountSetupEnteredUtc = new SignalFact(Fixed.AddMinutes(-1), 1); + return builder.Build(); + } + + private static DecisionSignal MakePhaseSignal(EnrollmentPhase phase, long ordinal) => + new DecisionSignal( + sessionSignalOrdinal: ordinal, + sessionTraceOrdinal: ordinal, + kind: DecisionSignalKind.EspPhaseChanged, + kindSchemaVersion: 1, + occurredAtUtc: Fixed, + sourceOrigin: "EspAndHelloTracker", + evidence: new Evidence( + kind: EvidenceKind.Derived, + identifier: "esp-phase-detector-v1", + summary: phase.ToString(), + derivationInputs: new Dictionary + { + [SignalPayloadKeys.EspPhase] = phase.ToString(), + }), + payload: new Dictionary + { + [SignalPayloadKeys.EspPhase] = phase.ToString(), + }); + + private static DecisionSignal MakeExitingSignal(long ordinal) => + new DecisionSignal( + sessionSignalOrdinal: ordinal, + sessionTraceOrdinal: ordinal, + kind: DecisionSignalKind.EspExiting, + kindSchemaVersion: 1, + occurredAtUtc: Fixed, + sourceOrigin: "ShellCoreTracker", + evidence: new Evidence(EvidenceKind.Raw, "esp_exiting", "ESP phase exiting")); + } +} diff --git a/src/Agent/AutopilotMonitor.DecisionCore.Tests/EspConfigDetectedHandlerTests.cs b/src/Agent/AutopilotMonitor.DecisionCore.Tests/EspConfigDetectedHandlerTests.cs new file mode 100644 index 00000000..a067ea27 --- /dev/null +++ b/src/Agent/AutopilotMonitor.DecisionCore.Tests/EspConfigDetectedHandlerTests.cs @@ -0,0 +1,225 @@ +using System; +using System.Collections.Generic; +using AutopilotMonitor.DecisionCore.Engine; +using AutopilotMonitor.DecisionCore.Serialization; +using AutopilotMonitor.DecisionCore.Signals; +using AutopilotMonitor.DecisionCore.State; +using Xunit; + +namespace AutopilotMonitor.DecisionCore.Tests +{ + /// + /// Plan §6 Fix 9 — populates the + /// / + /// facts. Set-once semantics: later signals with the same or different payload are no-ops + /// once a fact is present (monotonic, analogous to DeviceSetupEnteredUtc). + /// + public sealed class EspConfigDetectedHandlerTests + { + [Fact] + public void EspConfigDetected_populates_SkipUserEsp_and_SkipDeviceEsp_from_payload() + { + var engine = new DecisionEngine(); + var state = DecisionState.CreateInitial("s", "t"); + + var step = engine.Reduce(state, MakeSignal( + ordinal: 3, + skipUser: "true", + skipDevice: "false")); + + Assert.NotNull(step.NewState.SkipUserEsp); + Assert.True(step.NewState.SkipUserEsp!.Value); + Assert.Equal(3, step.NewState.SkipUserEsp!.SourceSignalOrdinal); + + Assert.NotNull(step.NewState.SkipDeviceEsp); + Assert.False(step.NewState.SkipDeviceEsp!.Value); + Assert.Equal(3, step.NewState.SkipDeviceEsp!.SourceSignalOrdinal); + + Assert.True(step.Transition.Taken); + Assert.Null(step.Transition.DeadEndReason); + Assert.Equal(nameof(DecisionSignalKind.EspConfigDetected), step.Transition.Trigger); + Assert.Empty(step.Effects); + } + + [Fact] + public void EspConfigDetected_leaves_stage_unchanged() + { + var engine = new DecisionEngine(); + var midFlight = DecisionState.CreateInitial("s", "t") + .ToBuilder() + .WithStage(SessionStage.EspDeviceSetup) + .WithStepIndex(5) + .WithLastAppliedSignalOrdinal(4) + .Build(); + + var step = engine.Reduce(midFlight, MakeSignal( + ordinal: 5, + skipUser: "false", + skipDevice: "false")); + + Assert.Equal(SessionStage.EspDeviceSetup, step.NewState.Stage); + Assert.Equal(6, step.NewState.StepIndex); + Assert.Equal(5, step.NewState.LastAppliedSignalOrdinal); + } + + [Fact] + public void EspConfigDetected_is_setonce_laterSignalDoesNotOverwrite() + { + var engine = new DecisionEngine(); + var state = DecisionState.CreateInitial("s", "t"); + + var step1 = engine.Reduce(state, MakeSignal( + ordinal: 1, + skipUser: "false", + skipDevice: "false")); + + // Second signal flips both values — must be ignored. + var step2 = engine.Reduce(step1.NewState, MakeSignal( + ordinal: 2, + skipUser: "true", + skipDevice: "true")); + + Assert.False(step2.NewState.SkipUserEsp!.Value); + Assert.Equal(1, step2.NewState.SkipUserEsp!.SourceSignalOrdinal); + Assert.False(step2.NewState.SkipDeviceEsp!.Value); + Assert.Equal(1, step2.NewState.SkipDeviceEsp!.SourceSignalOrdinal); + + // The second signal still bumps bookkeeping (taken transition, no effects). + Assert.True(step2.Transition.Taken); + Assert.Equal(2, step2.NewState.StepIndex); + Assert.Equal(2, step2.NewState.LastAppliedSignalOrdinal); + } + + [Fact] + public void EspConfigDetected_missingKeys_leavesFactsNull() + { + var engine = new DecisionEngine(); + var state = DecisionState.CreateInitial("s", "t"); + + var step = engine.Reduce(state, MakeSignal( + ordinal: 1, + skipUser: null, + skipDevice: null)); + + Assert.Null(step.NewState.SkipUserEsp); + Assert.Null(step.NewState.SkipDeviceEsp); + Assert.True(step.Transition.Taken); + Assert.Equal(1, step.NewState.StepIndex); + } + + [Fact] + public void EspConfigDetected_partialPayload_setsOnlyKnownFact_leavesOtherNull() + { + // Realistic: registry has SkipUserStatusPage but SkipDeviceStatusPage key missing. + var engine = new DecisionEngine(); + var state = DecisionState.CreateInitial("s", "t"); + + var step = engine.Reduce(state, MakeSignal( + ordinal: 1, + skipUser: "true", + skipDevice: null)); + + Assert.NotNull(step.NewState.SkipUserEsp); + Assert.True(step.NewState.SkipUserEsp!.Value); + Assert.Null(step.NewState.SkipDeviceEsp); + } + + [Fact] + public void EspConfigDetected_partialFirstSignal_secondSignalCanFillMissingFact() + { + // First signal sets only skipUser; second signal (different ordinal) can still fill + // in skipDevice because set-once is per-fact, not per-signal. + var engine = new DecisionEngine(); + var state = DecisionState.CreateInitial("s", "t"); + + var step1 = engine.Reduce(state, MakeSignal( + ordinal: 1, + skipUser: "true", + skipDevice: null)); + + var step2 = engine.Reduce(step1.NewState, MakeSignal( + ordinal: 2, + skipUser: null, + skipDevice: "false")); + + Assert.NotNull(step2.NewState.SkipUserEsp); + Assert.True(step2.NewState.SkipUserEsp!.Value); + Assert.Equal(1, step2.NewState.SkipUserEsp!.SourceSignalOrdinal); + + Assert.NotNull(step2.NewState.SkipDeviceEsp); + Assert.False(step2.NewState.SkipDeviceEsp!.Value); + Assert.Equal(2, step2.NewState.SkipDeviceEsp!.SourceSignalOrdinal); + } + + [Fact] + public void EspConfigDetected_schemaVersion2_fallsThroughAsUnhandled() + { + var engine = new DecisionEngine(); + var state = DecisionState.CreateInitial("s", "t"); + + var step = engine.Reduce(state, MakeSignal( + ordinal: 1, + skipUser: "true", + skipDevice: "false", + schemaVersion: 2)); + + Assert.False(step.Transition.Taken); + Assert.Equal("unhandled_signal_kind:EspConfigDetected:v2", step.Transition.DeadEndReason); + } + + [Fact] + public void EspConfigDetected_stateRoundtripsThroughSerializer() + { + var engine = new DecisionEngine(); + var state = DecisionState.CreateInitial("s", "t"); + + var populated = engine.Reduce(state, MakeSignal( + ordinal: 7, + skipUser: "true", + skipDevice: "false")).NewState; + + var json = StateSerializer.Serialize(populated); + var roundtripped = StateSerializer.Deserialize(json); + + Assert.NotNull(roundtripped.SkipUserEsp); + Assert.True(roundtripped.SkipUserEsp!.Value); + Assert.Equal(7, roundtripped.SkipUserEsp!.SourceSignalOrdinal); + + Assert.NotNull(roundtripped.SkipDeviceEsp); + Assert.False(roundtripped.SkipDeviceEsp!.Value); + Assert.Equal(7, roundtripped.SkipDeviceEsp!.SourceSignalOrdinal); + } + + [Fact] + public void EspConfigDetected_initialState_facts_areNull() + { + var state = DecisionState.CreateInitial("s", "t"); + Assert.Null(state.SkipUserEsp); + Assert.Null(state.SkipDeviceEsp); + } + + private static DecisionSignal MakeSignal( + long ordinal, + string? skipUser, + string? skipDevice, + int schemaVersion = 1) + { + var payload = new Dictionary(StringComparer.Ordinal); + if (skipUser != null) payload[SignalPayloadKeys.SkipUserEsp] = skipUser; + if (skipDevice != null) payload[SignalPayloadKeys.SkipDeviceEsp] = skipDevice; + + return new DecisionSignal( + sessionSignalOrdinal: ordinal, + sessionTraceOrdinal: ordinal, + kind: DecisionSignalKind.EspConfigDetected, + kindSchemaVersion: schemaVersion, + occurredAtUtc: new DateTime(2026, 4, 23, 18, 53, 21, DateTimeKind.Utc), + sourceOrigin: "DeviceInfoCollector", + evidence: new Evidence( + kind: EvidenceKind.Raw, + identifier: "esp_config_detected", + summary: $"SkipUser={skipUser ?? "unknown"}, SkipDevice={skipDevice ?? "unknown"}"), + payload: payload); + } + } +} diff --git a/src/Agent/AutopilotMonitor.DecisionCore.Tests/HelloSafetyBounceCancelTests.cs b/src/Agent/AutopilotMonitor.DecisionCore.Tests/HelloSafetyBounceCancelTests.cs new file mode 100644 index 00000000..a02a01a6 --- /dev/null +++ b/src/Agent/AutopilotMonitor.DecisionCore.Tests/HelloSafetyBounceCancelTests.cs @@ -0,0 +1,129 @@ +using System; +using System.Collections.Generic; +using AutopilotMonitor.DecisionCore.Engine; +using AutopilotMonitor.DecisionCore.Signals; +using AutopilotMonitor.DecisionCore.State; +using AutopilotMonitor.Shared.Models; +using Xunit; + +namespace AutopilotMonitor.DecisionCore.Tests +{ + /// + /// Plan §6 Fix 10 — defensive: if a premature EspPhaseChanged(FinalizingSetup) + /// moved the stage to before AccountSetup had + /// been observed, and HelloSafety was armed from that wrong baseline, a subsequent + /// EspPhaseChanged(AccountSetup) must cancel the stale deadline so it cannot fire + /// from the wrong window. + /// + /// Mirrors the pathology observed in session 30410cd7: HelloSafety armed at + /// 18:57:45 (Device-ESP exit synthesized as Finalizing) and was never cancelled when the + /// stage bounced back to EspAccountSetup at 18:57:55 — recovery only came from Hello + /// resolving at 19:03:38, 7m13s after arm vs. a 5-min window. Under slower conditions the + /// deadline would have fired first and mis-terminated the session. + /// + /// + public sealed class HelloSafetyBounceCancelTests + { + private static readonly DateTime Fixed = new DateTime(2026, 4, 23, 18, 57, 55, DateTimeKind.Utc); + + [Fact] + public void EspPhaseChanged_AccountSetup_whileInAwaitingHello_cancelsHelloSafetyDeadline() + { + var engine = new DecisionEngine(); + + // Starting state: stage AwaitingHello with HelloSafety armed (simulates the pre-fix + // premature transition from Device-ESP Finalizing synthesis at 18:57:45). + var helloSafetyDeadline = new ActiveDeadline( + name: DeadlineNames.HelloSafety, + dueAtUtc: Fixed.AddSeconds(-10).AddMinutes(5), // 5 min after the early arm + firesSignalKind: DecisionSignalKind.DeadlineFired, + firesPayload: new Dictionary + { + [SignalPayloadKeys.Deadline] = DeadlineNames.HelloSafety, + }); + var seed = DecisionState.CreateInitial("s", "t") + .ToBuilder() + .WithStage(SessionStage.AwaitingHello) + .WithStepIndex(3) + .WithLastAppliedSignalOrdinal(2) + .AddDeadline(helloSafetyDeadline) + .Build(); + + // Legitimate AccountSetup signal arrives (the bounce-back). + var signal = new DecisionSignal( + sessionSignalOrdinal: 3, + sessionTraceOrdinal: 3, + kind: DecisionSignalKind.EspPhaseChanged, + kindSchemaVersion: 1, + occurredAtUtc: Fixed, + sourceOrigin: "ProvisioningStatusTracker", + evidence: new Evidence( + kind: EvidenceKind.Raw, + identifier: "esp_phase_changed", + summary: "AccountSetup phase reached"), + payload: new Dictionary + { + [SignalPayloadKeys.EspPhase] = EnrollmentPhase.AccountSetup.ToString(), + }); + + var step = engine.Reduce(seed, signal); + + // Stage bounces back to EspAccountSetup. + Assert.Equal(SessionStage.EspAccountSetup, step.NewState.Stage); + + // The stale HelloSafety deadline must be gone. + Assert.DoesNotContain(step.NewState.Deadlines, d => d.Name == DeadlineNames.HelloSafety); + + // And a CancelDeadline effect for HelloSafety must have been emitted so the + // orchestrator's DeadlineScheduler unschedules it. + Assert.Contains(step.Effects, e => + e.Kind == DecisionEffectKind.CancelDeadline && + e.CancelDeadlineName == DeadlineNames.HelloSafety); + + // Existing DeviceOnlyEspDetection-cancel behavior must continue to fire too. + Assert.Contains(step.Effects, e => + e.Kind == DecisionEffectKind.CancelDeadline && + e.CancelDeadlineName == DeadlineNames.DeviceOnlyEspDetection); + } + + [Fact] + public void EspPhaseChanged_AccountSetup_fromEspDeviceSetup_doesNotEmitHelloSafetyCancelWhenNoneArmed() + { + // Sanity: Fix 10 must NOT synthesize a spurious HelloSafety cancel when the state + // was never in AwaitingHello and no HelloSafety deadline is armed. Backward compat + // with all existing Classic fixtures. + var engine = new DecisionEngine(); + var seed = DecisionState.CreateInitial("s", "t") + .ToBuilder() + .WithStage(SessionStage.EspDeviceSetup) + .WithStepIndex(1) + .WithLastAppliedSignalOrdinal(0) + .Build(); + + var signal = new DecisionSignal( + sessionSignalOrdinal: 1, + sessionTraceOrdinal: 1, + kind: DecisionSignalKind.EspPhaseChanged, + kindSchemaVersion: 1, + occurredAtUtc: Fixed, + sourceOrigin: "ProvisioningStatusTracker", + evidence: new Evidence(EvidenceKind.Raw, "esp_phase_changed", "AccountSetup"), + payload: new Dictionary + { + [SignalPayloadKeys.EspPhase] = EnrollmentPhase.AccountSetup.ToString(), + }); + + var step = engine.Reduce(seed, signal); + + Assert.Equal(SessionStage.EspAccountSetup, step.NewState.Stage); + // Only DeviceOnlyEspDetection cancel effect — no HelloSafety effect (precondition + // not met). + Assert.DoesNotContain(step.Effects, e => + e.Kind == DecisionEffectKind.CancelDeadline && + e.CancelDeadlineName == DeadlineNames.HelloSafety); + Assert.Contains(step.Effects, e => + e.Kind == DecisionEffectKind.CancelDeadline && + e.CancelDeadlineName == DeadlineNames.DeviceOnlyEspDetection); + } + } +} diff --git a/src/Shared/AutopilotMonitor.DecisionCore/Engine/DeadlineNames.cs b/src/Shared/AutopilotMonitor.DecisionCore/Engine/DeadlineNames.cs index c7579dc2..dc405d62 100644 --- a/src/Shared/AutopilotMonitor.DecisionCore/Engine/DeadlineNames.cs +++ b/src/Shared/AutopilotMonitor.DecisionCore/Engine/DeadlineNames.cs @@ -54,6 +54,12 @@ namespace AutopilotMonitor.DecisionCore.Engine /// On ImeUserSessionCompleted: matched pattern id. public const string ImePatternId = "patternId"; + /// On EspConfigDetected: "true" / "false"; missing → fact is not set. + public const string SkipUserEsp = "skipUserEsp"; + + /// On EspConfigDetected: "true" / "false"; missing → fact is not set. + public const string SkipDeviceEsp = "skipDeviceEsp"; + // --- InformationalEvent payload (plan §1.3, single-rail refactor) ------------ // Mirrors the EnrollmentEvent fields the reducer must reconstruct for the // EmitEventTimelineEntry effect. EventType / Source are mandatory; the rest are diff --git a/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.Classic.cs b/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.Classic.cs index c9f01fb8..40b8e70d 100644 --- a/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.Classic.cs +++ b/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.Classic.cs @@ -29,7 +29,16 @@ namespace AutopilotMonitor.DecisionCore.Engine { EnrollmentPhase.DeviceSetup => SessionStage.EspDeviceSetup, EnrollmentPhase.AccountSetup => SessionStage.EspAccountSetup, - EnrollmentPhase.FinalizingSetup => SessionStage.AwaitingHello, + // Plan §6 Fix 8 — Finalizing is a synthetic phase derived from the Shell-Core + // esp_exiting event; on Classic V1 enrollments the first exit is the Device-ESP + // handoff, not the true final. Only promote to AwaitingHello when reaching it is + // legitimate (AccountSetup already observed, OR SkipUser flow). Otherwise keep + // the current stage — the signal is still recorded as a taken transition with + // the FinalizingEnteredUtc fact + CurrentEnrollmentPhase updated below, but the + // HelloSafety deadline is NOT armed. + EnrollmentPhase.FinalizingSetup => ShouldTransitionToAwaitingHello(state) + ? SessionStage.AwaitingHello + : state.Stage, _ => state.Stage, }; @@ -84,6 +93,19 @@ namespace AutopilotMonitor.DecisionCore.Engine effectsList.Add(new DecisionEffect( DecisionEffectKind.CancelDeadline, cancelDeadlineName: DeadlineNames.DeviceOnlyEspDetection)); + + // Plan §6 Fix 10 — defensive belt-and-suspenders for the premature-AwaitingHello + // bounce-back case. If Fix 7's tracker guard or Fix 8's reducer guards ever fail + // and the stage reached AwaitingHello before AccountSetup (i.e. an early Finalizing + // synthesis armed HelloSafety from the wrong baseline), cancel the deadline here + // so it cannot fire from the wrong window after we resume EspAccountSetup. + if (state.Stage == SessionStage.AwaitingHello) + { + builder.CancelDeadline(DeadlineNames.HelloSafety); + effectsList.Add(new DecisionEffect( + DecisionEffectKind.CancelDeadline, + cancelDeadlineName: DeadlineNames.HelloSafety)); + } } var newState = builder.Build(); @@ -100,15 +122,36 @@ namespace AutopilotMonitor.DecisionCore.Engine /// /// Handle . Records - /// , transitions to - /// , and schedules the Hello-safety deadline - /// so a hang on Hello doesn't strand the session. + /// for observability; transitions to + /// and arms the Hello-safety deadline only + /// when holds (plan §6 Fix 8). On an + /// early / intermediate exit (e.g. the Device-ESP handoff on a Classic V1 enrollment) + /// the fact is still recorded but the stage is unchanged and no deadline is armed. /// private DecisionStep HandleEspExitingV1(DecisionState state, DecisionSignal signal) { var nextStep = state.StepIndex + 1; - var dueAtUtc = signal.OccurredAtUtc.Add(s_helloSafetyWindow); + var shouldTransition = ShouldTransitionToAwaitingHello(state); + var builder = state.ToBuilder() + .WithStepIndex(nextStep) + .WithLastAppliedSignalOrdinal(signal.SessionSignalOrdinal); + builder.EspFinalExitUtc = new SignalFact(signal.OccurredAtUtc, signal.SessionSignalOrdinal); + + if (!shouldTransition) + { + // Early / intermediate esp_exiting: keep stage, no HelloSafety arm. Observability + // still records EspFinalExitUtc so downstream classifiers see the full picture. + var noopTransition = BuildTakenTransition( + before: state, + signal: signal, + toStage: state.Stage, + nextStepIndex: nextStep, + trigger: nameof(DecisionSignalKind.EspExiting)); + return new DecisionStep(builder.Build(), noopTransition, Array.Empty()); + } + + var dueAtUtc = signal.OccurredAtUtc.Add(s_helloSafetyWindow); var helloSafety = new ActiveDeadline( name: DeadlineNames.HelloSafety, dueAtUtc: dueAtUtc, @@ -118,14 +161,9 @@ namespace AutopilotMonitor.DecisionCore.Engine [SignalPayloadKeys.Deadline] = DeadlineNames.HelloSafety, }); - var builder = state.ToBuilder() + builder = builder .WithStage(SessionStage.AwaitingHello) - .WithStepIndex(nextStep) - .WithLastAppliedSignalOrdinal(signal.SessionSignalOrdinal) .AddDeadline(helloSafety); - builder.EspFinalExitUtc = new SignalFact(signal.OccurredAtUtc, signal.SessionSignalOrdinal); - - var newState = builder.Build(); var transition = BuildTakenTransition( before: state, @@ -139,7 +177,7 @@ namespace AutopilotMonitor.DecisionCore.Engine new DecisionEffect(DecisionEffectKind.ScheduleDeadline, deadline: helloSafety), }; - return new DecisionStep(newState, transition, effects); + return new DecisionStep(builder.Build(), transition, effects); } /// diff --git a/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.Shared.cs b/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.Shared.cs index 52fcd112..ea5bff4b 100644 --- a/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.Shared.cs +++ b/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.Shared.cs @@ -333,6 +333,53 @@ namespace AutopilotMonitor.DecisionCore.Engine private DecisionStep HandleAutopilotProfileReadV1(DecisionState state, DecisionSignal signal) => RecordDiagnosticObservation(state, signal, nameof(DecisionSignalKind.AutopilotProfileRead)); + /// + /// Handle . Plan §6 Fix 9. + /// + /// Populates / + /// from the payload ( / + /// ). Set-once semantics — later signals with + /// identical payload are no-ops, and keys missing from the payload never clear a fact that + /// was already set (a re-read that failed to pick up one half must not invalidate the + /// other). Stage is unchanged; this is a fact-only signal that Fix 8's reducer guards + /// read. + /// + /// + private DecisionStep HandleEspConfigDetectedV1(DecisionState state, DecisionSignal signal) + { + var nextStep = state.StepIndex + 1; + var builder = state.ToBuilder() + .WithStepIndex(nextStep) + .WithLastAppliedSignalOrdinal(signal.SessionSignalOrdinal); + + if (signal.Payload != null) + { + if (state.SkipUserEsp == null + && signal.Payload.TryGetValue(SignalPayloadKeys.SkipUserEsp, out var rawSkipUser) + && bool.TryParse(rawSkipUser, out var skipUser)) + { + builder.SkipUserEsp = new SignalFact(skipUser, signal.SessionSignalOrdinal); + } + + if (state.SkipDeviceEsp == null + && signal.Payload.TryGetValue(SignalPayloadKeys.SkipDeviceEsp, out var rawSkipDevice) + && bool.TryParse(rawSkipDevice, out var skipDevice)) + { + builder.SkipDeviceEsp = new SignalFact(skipDevice, signal.SessionSignalOrdinal); + } + } + + var newState = builder.Build(); + var transition = BuildTakenTransition( + before: state, + signal: signal, + toStage: state.Stage, + nextStepIndex: nextStep, + trigger: nameof(DecisionSignalKind.EspConfigDetected)); + + return new DecisionStep(newState, transition, Array.Empty()); + } + private DecisionStep RecordDiagnosticObservation(DecisionState state, DecisionSignal signal, string trigger) { var newState = BumpStepBookkeeping(state, signal); @@ -419,6 +466,27 @@ namespace AutopilotMonitor.DecisionCore.Engine return new DecisionStep(bookkept, transition, Array.Empty()); } + /// + /// Plan §6 Fix 8 — gate for promoting to + /// on Classic V1 paths. Returns true only when the promotion is legitimate: + /// + /// AccountSetup has already been entered (the post-Account-ESP final exit case), or + /// SkipUser is explicitly true (the Account-ESP phase is skipped; first + /// esp_exiting IS the final exit on device-only / full-skip flows). + /// + /// Otherwise returns false — a FinalizingSetup / EspExiting signal arriving + /// before AccountSetup on a non-SkipUser enrollment is either a collector bug or the + /// Device-ESP intermediate exit that Fix 7 otherwise swallows at the tracker layer. + /// Keeping this helper in the reducer means even a regression in Fix 7 cannot drive a + /// premature AwaitingHello + HelloSafety arm. + /// + private static bool ShouldTransitionToAwaitingHello(DecisionState state) + { + if (state.AccountSetupEnteredUtc != null) return true; + if (state.SkipUserEsp?.Value == true) return true; + return false; + } + /// /// Determine the user-visible enrollment phase implied by an ESP phase-change signal. /// Plan §2.3 phase-fact mapping. Populated in M3.1 as Classic handlers come online. diff --git a/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.cs b/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.cs index 9f8da0a3..d79745fc 100644 --- a/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.cs +++ b/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.cs @@ -92,6 +92,7 @@ namespace AutopilotMonitor.DecisionCore.Engine // ----- Diagnostic observations (DecisionEngine.Shared.cs) — Plan §4.x M4.4.3 ----- (DecisionSignalKind.DeviceInfoCollected, 1) => HandleDeviceInfoCollectedV1(state, signal), (DecisionSignalKind.AutopilotProfileRead, 1) => HandleAutopilotProfileReadV1(state, signal), + (DecisionSignalKind.EspConfigDetected, 1) => HandleEspConfigDetectedV1(state, signal), // ----- Informational pass-through (DecisionEngine.Shared.cs) — single-rail §1.3 ----- (DecisionSignalKind.InformationalEvent, 1) => HandleInformationalEventV1(state, signal), diff --git a/src/Shared/AutopilotMonitor.DecisionCore/Signals/DecisionSignalKind.cs b/src/Shared/AutopilotMonitor.DecisionCore/Signals/DecisionSignalKind.cs index d7a8d208..989f3c34 100644 --- a/src/Shared/AutopilotMonitor.DecisionCore/Signals/DecisionSignalKind.cs +++ b/src/Shared/AutopilotMonitor.DecisionCore/Signals/DecisionSignalKind.cs @@ -26,6 +26,7 @@ namespace AutopilotMonitor.DecisionCore.Signals SystemRebootObserved, DeviceInfoCollected, AutopilotProfileRead, + EspConfigDetected, // --- Raw — WhiteGlove Part 2 (Post-Reboot User-Sign-In) --- UserAadSignInComplete, diff --git a/src/Shared/AutopilotMonitor.DecisionCore/State/DecisionState.cs b/src/Shared/AutopilotMonitor.DecisionCore/State/DecisionState.cs index bc6cc2b0..865d0052 100644 --- a/src/Shared/AutopilotMonitor.DecisionCore/State/DecisionState.cs +++ b/src/Shared/AutopilotMonitor.DecisionCore/State/DecisionState.cs @@ -46,6 +46,8 @@ namespace AutopilotMonitor.DecisionCore.State SignalFact? helloResolvedPart2Utc, SignalFact? desktopArrivedPart2Utc, SignalFact? accountSetupCompletedPart2Utc, + SignalFact? skipUserEsp, + SignalFact? skipDeviceEsp, IReadOnlyList deadlines, long lastAppliedSignalOrdinal, int stepIndex, @@ -86,6 +88,8 @@ namespace AutopilotMonitor.DecisionCore.State HelloResolvedPart2Utc = helloResolvedPart2Utc; DesktopArrivedPart2Utc = desktopArrivedPart2Utc; AccountSetupCompletedPart2Utc = accountSetupCompletedPart2Utc; + SkipUserEsp = skipUserEsp; + SkipDeviceEsp = skipDeviceEsp; Deadlines = deadlines ?? throw new ArgumentNullException(nameof(deadlines)); LastAppliedSignalOrdinal = lastAppliedSignalOrdinal; StepIndex = stepIndex; @@ -147,6 +151,13 @@ namespace AutopilotMonitor.DecisionCore.State public SignalFact? DesktopArrivedPart2Utc { get; } public SignalFact? AccountSetupCompletedPart2Utc { get; } + // --- ESP skip-flag facts from EspConfigDetected (plan §6 Fix 9). SkipUser=true means the + // Account-ESP phase never runs on this enrollment, so AwaitingHello is reachable directly + // after Device-ESP. When null the configuration has not been observed yet — guards that + // rely on an explicit bool must treat null as "unknown" rather than "false". + public SignalFact? SkipUserEsp { get; } + public SignalFact? SkipDeviceEsp { get; } + public IReadOnlyList Deadlines { get; } public long LastAppliedSignalOrdinal { get; } @@ -193,6 +204,8 @@ namespace AutopilotMonitor.DecisionCore.State helloResolvedPart2Utc: null, desktopArrivedPart2Utc: null, accountSetupCompletedPart2Utc: null, + skipUserEsp: null, + skipDeviceEsp: null, deadlines: Array.Empty(), lastAppliedSignalOrdinal: -1, stepIndex: 0); diff --git a/src/Shared/AutopilotMonitor.DecisionCore/State/DecisionStateBuilder.cs b/src/Shared/AutopilotMonitor.DecisionCore/State/DecisionStateBuilder.cs index b21eff61..dd09d821 100644 --- a/src/Shared/AutopilotMonitor.DecisionCore/State/DecisionStateBuilder.cs +++ b/src/Shared/AutopilotMonitor.DecisionCore/State/DecisionStateBuilder.cs @@ -44,6 +44,8 @@ namespace AutopilotMonitor.DecisionCore.State HelloResolvedPart2Utc = source.HelloResolvedPart2Utc; DesktopArrivedPart2Utc = source.DesktopArrivedPart2Utc; AccountSetupCompletedPart2Utc = source.AccountSetupCompletedPart2Utc; + SkipUserEsp = source.SkipUserEsp; + SkipDeviceEsp = source.SkipDeviceEsp; Deadlines = new List(source.Deadlines); LastAppliedSignalOrdinal = source.LastAppliedSignalOrdinal; StepIndex = source.StepIndex; @@ -75,6 +77,8 @@ namespace AutopilotMonitor.DecisionCore.State public SignalFact? HelloResolvedPart2Utc { get; set; } public SignalFact? DesktopArrivedPart2Utc { get; set; } public SignalFact? AccountSetupCompletedPart2Utc { get; set; } + public SignalFact? SkipUserEsp { get; set; } + public SignalFact? SkipDeviceEsp { get; set; } public List Deadlines { get; set; } public long LastAppliedSignalOrdinal { get; set; } public int StepIndex { get; set; } @@ -148,6 +152,8 @@ namespace AutopilotMonitor.DecisionCore.State helloResolvedPart2Utc: HelloResolvedPart2Utc, desktopArrivedPart2Utc: DesktopArrivedPart2Utc, accountSetupCompletedPart2Utc: AccountSetupCompletedPart2Utc, + skipUserEsp: SkipUserEsp, + skipDeviceEsp: SkipDeviceEsp, deadlines: Deadlines.ToArray(), lastAppliedSignalOrdinal: LastAppliedSignalOrdinal, stepIndex: StepIndex,