From 8a824d73fc49fbf9c9d5102f972d5a60f3af89b7 Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Tue, 14 Jul 2026 14:02:02 +0200 Subject: [PATCH] refactor(decision-core): EnrollmentScenarioObservations as record with with-expressions The 8-parameter constructor plus 7 hand-threaded With* methods meant every new observation field touched 8 copy sites - a threading-bug risk flagged in the daily commit review (2026-07-14). Now a sealed record with init-only properties: the With* methods keep the set-once guard but their bodies collapse to one-line `this with { ... }` expressions, so the next field addition is one property plus one With* method. - IsExternalInit polyfill (internal) added: DecisionCore targets netstandard2.0, which lacks the C# 9 marker type for init setters. Consumers never need it - init/with usage stays inside the assembly. - Snapshot compatibility unchanged: identical JSON shape (property names + SignalFact {Value, SourceSignalOrdinal}); the record deserializes via init setters instead of the removed constructor. Pinned by a new StateSerializer round-trip test covering facts, ordinals, and unset-stays-null. - Semantics unchanged: set-once still returns `this` (Assert.Same tests hold); `==` is not used on the type anywhere, so record value equality is inert. Verified: DecisionCore 468 tests, Agent V2 Core 1776 tests on net48 (the consumer that mandated the netstandard2.0 target), Backend 2928 tests - all passing. Co-Authored-By: Claude Fable 5 --- .../SerializationTests.cs | 37 +++++ .../IsExternalInit.cs | 11 ++ .../State/EnrollmentScenarioObservations.cs | 128 +++--------------- 3 files changed, 66 insertions(+), 110 deletions(-) create mode 100644 src/Shared/AutopilotMonitor.DecisionCore/IsExternalInit.cs diff --git a/src/Agent/AutopilotMonitor.DecisionCore.Tests/SerializationTests.cs b/src/Agent/AutopilotMonitor.DecisionCore.Tests/SerializationTests.cs index 55430867..6fd601eb 100644 --- a/src/Agent/AutopilotMonitor.DecisionCore.Tests/SerializationTests.cs +++ b/src/Agent/AutopilotMonitor.DecisionCore.Tests/SerializationTests.cs @@ -474,6 +474,43 @@ namespace AutopilotMonitor.DecisionCore.Tests Assert.Equal(7, p.EvidenceOrdinal); Assert.Equal("test_round_trip", p.Reason); } + + [Fact] + public void ScenarioObservations_stateRoundtripsThroughStateSerializer() + { + // Snapshot-compat pin for the record conversion (2026-07-14): observations were a + // ctor-based class before; the record deserializes via init setters instead. The + // persisted JSON shape (property names + SignalFact {Value, SourceSignalOrdinal}) + // is identical, so on-disk snapshots written by either shape must rehydrate — this + // round-trip guards facts, ordinals, and unset-stays-null across that seam. + var initial = DecisionState.CreateInitial("s", "t"); + var state = initial + .ToBuilder() + .Apply(b => b.ScenarioObservations = b.ScenarioObservations + .WithShellCoreWhiteGloveSuccessSeen(sourceSignalOrdinal: 3) + .WithSkipUserEsp(value: true, sourceSignalOrdinal: 4) + .WithEspSyncFailureTimeoutMinutes(value: 90, sourceSignalOrdinal: 5) + .WithRegistrySelfDeployingProfile(value: false, sourceSignalOrdinal: 6)) + .Build(); + + var json = StateSerializer.Serialize(state); + var o = StateSerializer.Deserialize(json).ScenarioObservations; + + Assert.True(o.ShellCoreWhiteGloveSuccessSeen!.Value); + Assert.Equal(3, o.ShellCoreWhiteGloveSuccessSeen!.SourceSignalOrdinal); + Assert.True(o.SkipUserEsp!.Value); + Assert.Equal(4, o.SkipUserEsp!.SourceSignalOrdinal); + Assert.Equal(90, o.EspSyncFailureTimeoutMinutes!.Value); + Assert.Equal(5, o.EspSyncFailureTimeoutMinutes!.SourceSignalOrdinal); + Assert.False(o.RegistrySelfDeployingProfile!.Value); + Assert.Equal(6, o.RegistrySelfDeployingProfile!.SourceSignalOrdinal); + + // Never-observed facts must rehydrate as null (null ≠ veto — session 62e603c9). + Assert.Null(o.WhiteGloveSealingPatternSeen); + Assert.Null(o.AadUserJoinWithUserObserved); + Assert.Null(o.SkipDeviceEsp); + Assert.Null(o.EspAllowContinueAnyway); + } } internal static class DecisionStateBuilderTestExtensions diff --git a/src/Shared/AutopilotMonitor.DecisionCore/IsExternalInit.cs b/src/Shared/AutopilotMonitor.DecisionCore/IsExternalInit.cs new file mode 100644 index 00000000..659c7bc0 --- /dev/null +++ b/src/Shared/AutopilotMonitor.DecisionCore/IsExternalInit.cs @@ -0,0 +1,11 @@ +// netstandard2.0 polyfill: the C# 9 compiler requires this marker type to emit +// init-only property setters (records / `with`-expressions). It ships in the BCL +// from .NET 5 onward but not in netstandard2.0. Internal on purpose — consumers +// (Agent net48, Backend net8.0) never need it: init/`with` usage stays inside +// this assembly, and net8.0 consumers resolve the BCL type anyway. +namespace System.Runtime.CompilerServices +{ + internal static class IsExternalInit + { + } +} diff --git a/src/Shared/AutopilotMonitor.DecisionCore/State/EnrollmentScenarioObservations.cs b/src/Shared/AutopilotMonitor.DecisionCore/State/EnrollmentScenarioObservations.cs index 54fe3dcc..c9703004 100644 --- a/src/Shared/AutopilotMonitor.DecisionCore/State/EnrollmentScenarioObservations.cs +++ b/src/Shared/AutopilotMonitor.DecisionCore/State/EnrollmentScenarioObservations.cs @@ -23,43 +23,15 @@ namespace AutopilotMonitor.DecisionCore.State /// /// /// - public sealed class EnrollmentScenarioObservations + public sealed record EnrollmentScenarioObservations { - public static readonly EnrollmentScenarioObservations Empty = new EnrollmentScenarioObservations( - shellCoreWhiteGloveSuccessSeen: null, - whiteGloveSealingPatternSeen: null, - aadUserJoinWithUserObserved: null, - skipUserEsp: null, - skipDeviceEsp: null, - espSyncFailureTimeoutMinutes: null, - espAllowContinueAnyway: null, - registrySelfDeployingProfile: null); - - public EnrollmentScenarioObservations( - SignalFact? shellCoreWhiteGloveSuccessSeen, - SignalFact? whiteGloveSealingPatternSeen, - SignalFact? aadUserJoinWithUserObserved, - SignalFact? skipUserEsp, - SignalFact? skipDeviceEsp, - SignalFact? espSyncFailureTimeoutMinutes, - SignalFact? espAllowContinueAnyway, - SignalFact? registrySelfDeployingProfile) - { - ShellCoreWhiteGloveSuccessSeen = shellCoreWhiteGloveSuccessSeen; - WhiteGloveSealingPatternSeen = whiteGloveSealingPatternSeen; - AadUserJoinWithUserObserved = aadUserJoinWithUserObserved; - SkipUserEsp = skipUserEsp; - SkipDeviceEsp = skipDeviceEsp; - EspSyncFailureTimeoutMinutes = espSyncFailureTimeoutMinutes; - EspAllowContinueAnyway = espAllowContinueAnyway; - RegistrySelfDeployingProfile = registrySelfDeployingProfile; - } + public static readonly EnrollmentScenarioObservations Empty = new EnrollmentScenarioObservations(); /// True once has fired. - public SignalFact? ShellCoreWhiteGloveSuccessSeen { get; } + public SignalFact? ShellCoreWhiteGloveSuccessSeen { get; init; } /// True once has fired. - public SignalFact? WhiteGloveSealingPatternSeen { get; } + public SignalFact? WhiteGloveSealingPatternSeen { get; init; } /// /// Payload-carrying observation from . @@ -68,13 +40,13 @@ namespace AutopilotMonitor.DecisionCore.State /// Independent of , which reflects the /// SessionStarted registry hint. /// - public SignalFact? AadUserJoinWithUserObserved { get; } + public SignalFact? AadUserJoinWithUserObserved { get; init; } /// Raw payload half-fact from . - public SignalFact? SkipUserEsp { get; } + public SignalFact? SkipUserEsp { get; init; } /// Raw payload half-fact from . - public SignalFact? SkipDeviceEsp { get; } + public SignalFact? SkipDeviceEsp { get; init; } /// /// FirstSync SyncFailureTimeout in minutes — Intune ESP setting @@ -83,7 +55,7 @@ namespace AutopilotMonitor.DecisionCore.State /// with the actual timeout instead of a generic "ESP timed out" string. /// Set-once from . /// - public SignalFact? EspSyncFailureTimeoutMinutes { get; } + public SignalFact? EspSyncFailureTimeoutMinutes { get; init; } /// /// Decoded bit 4 of the FirstSync BlockInStatusPage bitmask — Intune ESP @@ -94,7 +66,7 @@ namespace AutopilotMonitor.DecisionCore.State /// reaching the desktop. Set-once from /// . /// - public SignalFact? EspAllowContinueAnyway { get; } + public SignalFact? EspAllowContinueAnyway { get; init; } /// /// Raw registry fact from @@ -106,110 +78,46 @@ namespace AutopilotMonitor.DecisionCore.State /// behavioural device_only_esp_detection deadline (session 62e603c9). null /// means the fact was never observed and must NOT be treated as a veto. /// - public SignalFact? RegistrySelfDeployingProfile { get; } + public SignalFact? RegistrySelfDeployingProfile { get; init; } public EnrollmentScenarioObservations WithShellCoreWhiteGloveSuccessSeen(long sourceSignalOrdinal) => ShellCoreWhiteGloveSuccessSeen != null ? this - : new EnrollmentScenarioObservations( - new SignalFact(true, sourceSignalOrdinal), - WhiteGloveSealingPatternSeen, - AadUserJoinWithUserObserved, - SkipUserEsp, - SkipDeviceEsp, - EspSyncFailureTimeoutMinutes, - EspAllowContinueAnyway, - RegistrySelfDeployingProfile); + : this with { ShellCoreWhiteGloveSuccessSeen = new SignalFact(true, sourceSignalOrdinal) }; public EnrollmentScenarioObservations WithWhiteGloveSealingPatternSeen(long sourceSignalOrdinal) => WhiteGloveSealingPatternSeen != null ? this - : new EnrollmentScenarioObservations( - ShellCoreWhiteGloveSuccessSeen, - new SignalFact(true, sourceSignalOrdinal), - AadUserJoinWithUserObserved, - SkipUserEsp, - SkipDeviceEsp, - EspSyncFailureTimeoutMinutes, - EspAllowContinueAnyway, - RegistrySelfDeployingProfile); + : this with { WhiteGloveSealingPatternSeen = new SignalFact(true, sourceSignalOrdinal) }; public EnrollmentScenarioObservations WithAadUserJoinWithUserObserved(bool value, long sourceSignalOrdinal) => AadUserJoinWithUserObserved != null ? this - : new EnrollmentScenarioObservations( - ShellCoreWhiteGloveSuccessSeen, - WhiteGloveSealingPatternSeen, - new SignalFact(value, sourceSignalOrdinal), - SkipUserEsp, - SkipDeviceEsp, - EspSyncFailureTimeoutMinutes, - EspAllowContinueAnyway, - RegistrySelfDeployingProfile); + : this with { AadUserJoinWithUserObserved = new SignalFact(value, sourceSignalOrdinal) }; public EnrollmentScenarioObservations WithSkipUserEsp(bool value, long sourceSignalOrdinal) => SkipUserEsp != null ? this - : new EnrollmentScenarioObservations( - ShellCoreWhiteGloveSuccessSeen, - WhiteGloveSealingPatternSeen, - AadUserJoinWithUserObserved, - new SignalFact(value, sourceSignalOrdinal), - SkipDeviceEsp, - EspSyncFailureTimeoutMinutes, - EspAllowContinueAnyway, - RegistrySelfDeployingProfile); + : this with { SkipUserEsp = new SignalFact(value, sourceSignalOrdinal) }; public EnrollmentScenarioObservations WithSkipDeviceEsp(bool value, long sourceSignalOrdinal) => SkipDeviceEsp != null ? this - : new EnrollmentScenarioObservations( - ShellCoreWhiteGloveSuccessSeen, - WhiteGloveSealingPatternSeen, - AadUserJoinWithUserObserved, - SkipUserEsp, - new SignalFact(value, sourceSignalOrdinal), - EspSyncFailureTimeoutMinutes, - EspAllowContinueAnyway, - RegistrySelfDeployingProfile); + : this with { SkipDeviceEsp = new SignalFact(value, sourceSignalOrdinal) }; public EnrollmentScenarioObservations WithEspSyncFailureTimeoutMinutes(int value, long sourceSignalOrdinal) => EspSyncFailureTimeoutMinutes != null ? this - : new EnrollmentScenarioObservations( - ShellCoreWhiteGloveSuccessSeen, - WhiteGloveSealingPatternSeen, - AadUserJoinWithUserObserved, - SkipUserEsp, - SkipDeviceEsp, - new SignalFact(value, sourceSignalOrdinal), - EspAllowContinueAnyway, - RegistrySelfDeployingProfile); + : this with { EspSyncFailureTimeoutMinutes = new SignalFact(value, sourceSignalOrdinal) }; public EnrollmentScenarioObservations WithEspAllowContinueAnyway(bool value, long sourceSignalOrdinal) => EspAllowContinueAnyway != null ? this - : new EnrollmentScenarioObservations( - ShellCoreWhiteGloveSuccessSeen, - WhiteGloveSealingPatternSeen, - AadUserJoinWithUserObserved, - SkipUserEsp, - SkipDeviceEsp, - EspSyncFailureTimeoutMinutes, - new SignalFact(value, sourceSignalOrdinal), - RegistrySelfDeployingProfile); + : this with { EspAllowContinueAnyway = new SignalFact(value, sourceSignalOrdinal) }; public EnrollmentScenarioObservations WithRegistrySelfDeployingProfile(bool value, long sourceSignalOrdinal) => RegistrySelfDeployingProfile != null ? this - : new EnrollmentScenarioObservations( - ShellCoreWhiteGloveSuccessSeen, - WhiteGloveSealingPatternSeen, - AadUserJoinWithUserObserved, - SkipUserEsp, - SkipDeviceEsp, - EspSyncFailureTimeoutMinutes, - EspAllowContinueAnyway, - new SignalFact(value, sourceSignalOrdinal)); + : this with { RegistrySelfDeployingProfile = new SignalFact(value, sourceSignalOrdinal) }; } }