v2 codex-post-#51: partial-skipUser guard regression + enum serialization parity

Addresses Codex second-pass on commit 51d57153:

Fix #A (High, behavioral regression) — ShouldTransitionToAwaitingHello in
DecisionEngine.Shared.cs was gated on Profile.EspConfig ∈ {DeviceEspOnly,
NoEsp}, which requires BOTH skipUser + skipDevice observations before the
derived enum leaves Unknown. A realistic bootstrap where only the skipUser
half lands (skipDevice missing) therefore blocked the AwaitingHello
promotion, while the pre-refactor `state.SkipUserEsp?.Value == true` guard
would have unlocked it. The guard now reads the raw half-fact
`ScenarioObservations.SkipUserEsp?.Value` directly, restoring exact legacy
semantics. Profile.EspConfig keeps its "needs both halves" shape — it is a
derived classification for other consumers, not a substitute for the raw
observation here.

Two new regression tests pin the fix:
 - EspConfigDetectedSkipUserTrue_onlyHalfPayload_stillUnblocksAwaitingHello
 - EspExiting_partialSkipUserTrue_armsHelloSafety

Fix #B (Medium, serialization parity) — DecisionCoreJsonSettings did not
register UnknownFallbackEnumConverter<T> for the five new profile enums
(EnrollmentMode, EnrollmentJoinMode, EspConfig, PreProvisioningSide,
ProfileConfidence), so they round-tripped as numeric JSON with no
cross-version fallback. Now registered alongside the other DecisionCore
enums; fallbacks follow each enum's Unknown-equivalent value (None for
PreProvisioningSide, Low for ProfileConfidence, Unknown for the rest).

Six new SerializationTests verify string-form serialization, known-value
round-trip, and unknown-value fallback for each of the five enums plus an
end-to-end DecisionState round-trip through StateSerializer that asserts
the profile dimensions persist as string literals, not integers.

Suites: DecisionCore 211/211 (+8 new tests), Agent.V2.Core 650/650,
Functions 735/735; 0 warnings.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Oliver Kieselbach
2026-04-24 15:37:26 +02:00
parent 51d57153ad
commit 511ed70361
6 changed files with 222 additions and 13 deletions
@@ -1 +1 @@
306
308
@@ -1 +1 @@
1143
1144
@@ -198,6 +198,83 @@ namespace AutopilotMonitor.DecisionCore.Tests
Assert.Equal(SessionStage.AwaitingHello, afterFinalizing.NewState.Stage);
}
[Fact]
public void EspConfigDetectedSkipUserTrue_onlyHalfPayload_stillUnblocksAwaitingHello()
{
// Codex second-pass Hoch (post-#51) — partial-payload regression guard.
// EspConfigDetected arrives with ONLY skipUser=true (skipDevice missing, e.g.
// registry key not yet present). Under the original legacy code this was sufficient
// to unlock AwaitingHello because the guard read SkipUserEsp?.Value directly. The
// derived Profile.EspConfig enum would stay Unknown (it needs BOTH halves), so the
// guard must NOT be gated on EspConfig. This test pins the behaviour: one half +
// FinalizingSetup → AwaitingHello.
var engine = new DecisionEngine();
var seed = DecisionState.CreateInitial("s", "t")
.ToBuilder()
.WithStage(SessionStage.EspDeviceSetup)
.WithStepIndex(1)
.WithLastAppliedSignalOrdinal(0)
.Build();
var partial = new DecisionSignal(
sessionSignalOrdinal: 1,
sessionTraceOrdinal: 1,
kind: DecisionSignalKind.EspConfigDetected,
kindSchemaVersion: 1,
occurredAtUtc: Fixed,
sourceOrigin: "EnrollmentOrchestrator",
evidence: new Evidence(EvidenceKind.Raw, "esp_config_detected_bootstrap_partial", "SkipUser=true only"),
payload: new Dictionary<string, string>
{
[SignalPayloadKeys.SkipUserEsp] = "true",
// NOTE: skipDevice key intentionally missing.
});
var afterConfig = engine.Reduce(seed, partial).NewState;
// Only the raw half-fact is set; derived EspConfig enum stays Unknown.
Assert.True(afterConfig.ScenarioObservations.SkipUserEsp!.Value);
Assert.Null(afterConfig.ScenarioObservations.SkipDeviceEsp);
Assert.Equal(EspConfig.Unknown, afterConfig.ScenarioProfile.EspConfig);
// The guard MUST still unlock AwaitingHello — the semantic "Account-ESP is skipped"
// is fully known from SkipUser alone, regardless of skipDevice.
var finalizing = MakePhaseSignal(EnrollmentPhase.FinalizingSetup, ordinal: 2);
var afterFinalizing = engine.Reduce(afterConfig, finalizing);
Assert.Equal(SessionStage.AwaitingHello, afterFinalizing.NewState.Stage);
}
[Fact]
public void EspExiting_partialSkipUserTrue_armsHelloSafety()
{
// Symmetric with EspExiting (the path that actually arms HelloSafety) — partial
// bootstrap payload must not block this path either.
var engine = new DecisionEngine();
var seed = DecisionState.CreateInitial("s", "t")
.ToBuilder()
.WithStage(SessionStage.EspDeviceSetup)
.WithStepIndex(1)
.WithLastAppliedSignalOrdinal(0)
.Build();
var partial = new DecisionSignal(
sessionSignalOrdinal: 1,
sessionTraceOrdinal: 1,
kind: DecisionSignalKind.EspConfigDetected,
kindSchemaVersion: 1,
occurredAtUtc: Fixed,
sourceOrigin: "EnrollmentOrchestrator",
evidence: new Evidence(EvidenceKind.Raw, "esp_config_detected_bootstrap_partial", "SkipUser=true only"),
payload: new Dictionary<string, string>
{
[SignalPayloadKeys.SkipUserEsp] = "true",
});
var afterConfig = engine.Reduce(seed, partial).NewState;
var afterExit = engine.Reduce(afterConfig, MakeExitingSignal(ordinal: 2));
Assert.Equal(SessionStage.AwaitingHello, afterExit.NewState.Stage);
Assert.Contains(afterExit.NewState.Deadlines, d => d.Name == DeadlineNames.HelloSafety);
}
[Fact]
public void Finalizing_beforeEspConfigDetected_staysInStage_evenIfRegistryWouldSaySkipUser()
{
@@ -245,5 +245,128 @@ namespace AutopilotMonitor.DecisionCore.Tests
Assert.Equal(HypothesisLevel.Unknown, result);
}
// ================================================================ Codex follow-up #5 (post-#51)
// EnrollmentScenarioProfile enums — the five dimensions of the new profile aggregate
// must be registered in DecisionCoreJsonSettings just like every other DecisionCore
// enum, otherwise snapshots round-trip as numeric JSON and unknown cross-version
// values crash instead of landing on the Unknown-fallback. These tests pin the
// registration contract.
[Fact]
public void UnknownFallbackEnumConverter_EnrollmentMode_roundtripsByName_andFallsBackOnUnknown()
{
var settings = DecisionCoreJsonSettings.Create();
var json = JsonConvert.SerializeObject(EnrollmentMode.WhiteGlove, settings);
Assert.Contains("WhiteGlove", json);
Assert.Equal(EnrollmentMode.WhiteGlove, JsonConvert.DeserializeObject<EnrollmentMode>(json, settings));
Assert.Equal(EnrollmentMode.Unknown,
JsonConvert.DeserializeObject<EnrollmentMode>("\"FutureMode\"", settings));
}
[Fact]
public void UnknownFallbackEnumConverter_EnrollmentJoinMode_roundtripsByName_andFallsBackOnUnknown()
{
var settings = DecisionCoreJsonSettings.Create();
var json = JsonConvert.SerializeObject(EnrollmentJoinMode.HybridAzureAdJoin, settings);
Assert.Contains("HybridAzureAdJoin", json);
Assert.Equal(EnrollmentJoinMode.HybridAzureAdJoin,
JsonConvert.DeserializeObject<EnrollmentJoinMode>(json, settings));
Assert.Equal(EnrollmentJoinMode.Unknown,
JsonConvert.DeserializeObject<EnrollmentJoinMode>("\"FutureJoinMode\"", settings));
}
[Fact]
public void UnknownFallbackEnumConverter_EspConfig_roundtripsByName_andFallsBackOnUnknown()
{
var settings = DecisionCoreJsonSettings.Create();
var json = JsonConvert.SerializeObject(EspConfig.DeviceEspOnly, settings);
Assert.Contains("DeviceEspOnly", json);
Assert.Equal(EspConfig.DeviceEspOnly, JsonConvert.DeserializeObject<EspConfig>(json, settings));
Assert.Equal(EspConfig.Unknown,
JsonConvert.DeserializeObject<EspConfig>("\"NewEspShape\"", settings));
}
[Fact]
public void UnknownFallbackEnumConverter_PreProvisioningSide_roundtripsByName_andFallsBackToNone()
{
var settings = DecisionCoreJsonSettings.Create();
var json = JsonConvert.SerializeObject(PreProvisioningSide.Technician, settings);
Assert.Contains("Technician", json);
Assert.Equal(PreProvisioningSide.Technician,
JsonConvert.DeserializeObject<PreProvisioningSide>(json, settings));
// The fallback for PreProvisioningSide is None (there is no Unknown member).
Assert.Equal(PreProvisioningSide.None,
JsonConvert.DeserializeObject<PreProvisioningSide>("\"FutureSide\"", settings));
}
[Fact]
public void UnknownFallbackEnumConverter_ProfileConfidence_roundtripsByName_andFallsBackToLow()
{
var settings = DecisionCoreJsonSettings.Create();
var json = JsonConvert.SerializeObject(ProfileConfidence.High, settings);
Assert.Contains("High", json);
Assert.Equal(ProfileConfidence.High,
JsonConvert.DeserializeObject<ProfileConfidence>(json, settings));
Assert.Equal(ProfileConfidence.Low,
JsonConvert.DeserializeObject<ProfileConfidence>("\"Unverified\"", settings));
}
[Fact]
public void ScenarioProfile_stateRoundtripsThroughStateSerializer_enumsAsStrings()
{
// End-to-end regression: the full DecisionState must serialize the new Profile
// dimensions as string literals (not integers) so cross-version reads stay robust.
var initial = DecisionState.CreateInitial("s", "t");
var state = initial
.ToBuilder()
.Apply(b => b.ScenarioProfile = b.ScenarioProfile.With(
mode: EnrollmentMode.WhiteGlove,
joinMode: EnrollmentJoinMode.HybridAzureAdJoin,
espConfig: EspConfig.DeviceEspOnly,
preProvisioningSide: PreProvisioningSide.Technician,
confidence: ProfileConfidence.High,
evidenceOrdinal: 7,
reason: "test_round_trip"))
.Build();
var json = StateSerializer.Serialize(state);
// Enums persist as their string names, not their underlying integer values.
Assert.Contains("\"Mode\":\"WhiteGlove\"", json);
Assert.Contains("\"JoinMode\":\"HybridAzureAdJoin\"", json);
Assert.Contains("\"EspConfig\":\"DeviceEspOnly\"", json);
Assert.Contains("\"PreProvisioningSide\":\"Technician\"", json);
Assert.Contains("\"Confidence\":\"High\"", json);
var roundtripped = StateSerializer.Deserialize(json);
var p = roundtripped.ScenarioProfile;
Assert.Equal(EnrollmentMode.WhiteGlove, p.Mode);
Assert.Equal(EnrollmentJoinMode.HybridAzureAdJoin, p.JoinMode);
Assert.Equal(EspConfig.DeviceEspOnly, p.EspConfig);
Assert.Equal(PreProvisioningSide.Technician, p.PreProvisioningSide);
Assert.Equal(ProfileConfidence.High, p.Confidence);
Assert.Equal(7, p.EvidenceOrdinal);
Assert.Equal("test_round_trip", p.Reason);
}
}
internal static class DecisionStateBuilderTestExtensions
{
public static DecisionStateBuilder Apply(this DecisionStateBuilder b, Action<DecisionStateBuilder> mutate)
{
mutate(b);
return b;
}
}
}
@@ -591,10 +591,9 @@ namespace AutopilotMonitor.DecisionCore.Engine
/// on Classic V1 paths. Returns <c>true</c> only when the promotion is legitimate:
/// <list type="bullet">
/// <item>AccountSetup has already been entered (the post-Account-ESP final exit case), or</item>
/// <item>The profile's <see cref="EnrollmentScenarioProfile.EspConfig"/> resolves to
/// a shape where Account-ESP never runs (<see cref="EspConfig.DeviceEspOnly"/>
/// = skipUser=true, or <see cref="EspConfig.NoEsp"/> = skipUser=true AND
/// skipDevice=true). In those shapes the first esp_exiting IS the final exit.</item>
/// <item><see cref="EnrollmentScenarioObservations.SkipUserEsp"/> is observed as
/// <c>true</c> (Account-ESP phase is skipped; first esp_exiting IS the final
/// exit on device-only / full-skip flows).</item>
/// </list>
/// Otherwise returns <c>false</c> — a FinalizingSetup / EspExiting signal arriving
/// before AccountSetup on a non-SkipUser enrollment is either a collector bug or the
@@ -602,18 +601,19 @@ namespace AutopilotMonitor.DecisionCore.Engine
/// Keeping this helper in the reducer means even a regression in Fix 7 cannot drive a
/// premature <c>AwaitingHello</c> + HelloSafety arm.
/// <para>
/// Codex follow-up #5: the legacy <c>SkipUserEsp?.Value == true</c> check now reads
/// <see cref="DecisionState.ScenarioProfile"/> (derived enum) instead of the
/// half-fact. Semantically equivalent: DeviceEspOnly ≡ skipUser=true && skipDevice=false,
/// NoEsp ≡ skipUser=true && skipDevice=true. An <see cref="EspConfig.Unknown"/> profile
/// (EspConfigDetected not yet seen, or only one half parsed) blocks the promotion.
/// Codex follow-up #5 + post-#51 fix: the legacy <c>state.SkipUserEsp?.Value == true</c>
/// check now reads <see cref="DecisionState.ScenarioObservations"/>.<see cref="EnrollmentScenarioObservations.SkipUserEsp"/>
/// directly — NOT the derived <see cref="EnrollmentScenarioProfile.EspConfig"/> enum.
/// The derived enum requires BOTH halves (skipUser + skipDevice) to be observed before
/// it leaves <see cref="EspConfig.Unknown"/>, so a partial bootstrap payload carrying
/// only <c>skipUser=true</c> would block the promotion indefinitely under the previous
/// pass. The raw half-fact mirrors the old <c>SkipUserEsp?.Value</c> behaviour exactly.
/// </para>
/// </summary>
private static bool ShouldTransitionToAwaitingHello(DecisionState state)
{
if (state.AccountSetupEnteredUtc != null) return true;
var esp = state.ScenarioProfile.EspConfig;
if (esp == EspConfig.DeviceEspOnly || esp == EspConfig.NoEsp) return true;
if (state.ScenarioObservations.SkipUserEsp?.Value == true) return true;
return false;
}
@@ -51,6 +51,15 @@ namespace AutopilotMonitor.DecisionCore.Serialization
settings.Converters.Add(new UnknownFallbackEnumConverter<DecisionEffectKind>(DecisionEffectKind.PersistSnapshot));
settings.Converters.Add(new UnknownFallbackEnumConverter<EnrollmentPhase>(EnrollmentPhase.Unknown));
// Codex follow-up #5 (post-#51) — EnrollmentScenarioProfile dimensions. Without
// these registrations the new enums would round-trip as numeric JSON with no
// unknown-value fallback, breaking drift-tolerated cross-version reads.
settings.Converters.Add(new UnknownFallbackEnumConverter<EnrollmentMode>(EnrollmentMode.Unknown));
settings.Converters.Add(new UnknownFallbackEnumConverter<EnrollmentJoinMode>(EnrollmentJoinMode.Unknown));
settings.Converters.Add(new UnknownFallbackEnumConverter<EspConfig>(EspConfig.Unknown));
settings.Converters.Add(new UnknownFallbackEnumConverter<PreProvisioningSide>(PreProvisioningSide.None));
settings.Converters.Add(new UnknownFallbackEnumConverter<ProfileConfidence>(ProfileConfidence.Low));
return settings;
}
}