From 97e895433953cd33a5069122cc4d86cb96687cdb Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Thu, 21 May 2026 19:18:26 +0200 Subject: [PATCH] fix(decisioncore): tighten Classic Hello-disabled FastPath to strong AccountSetup gate The HelloDisabledFastPath in HandleDesktopArrivedV1 used a weak guard (AccountSetupEnteredUtc != null) while HandleEspExitingV1 already used the strong gate (ShouldTransitionToAwaitingHello: AccountSetupProvisioningSucceededUtc OR ScenarioObservations.SkipUserEsp). Session 08c99638 exposed the asymmetry: Shell-Core event 62407 fired esp_exiting with errorCode=0 while AccountSetup categorySucceeded was still in_progress (0/N apps complete). A late desktop_arrived then drove enrollment_complete via the weak guard. The FastPath now reuses ShouldTransitionToAwaitingHello, restoring parity with the EspExiting path. Sessions where ESP "exits" before AccountSetup genuinely finishes stay parked in AwaitingDesktop and eventually fall to the max-lifetime failure path. Tests: - HelloDisabled_FastPath_skipped_when_AccountSetup_entered_but_provisioning_never_succeeded (08c99638 repro) - HelloDisabled_FastPath_fires_on_SkipUserEsp_flow_even_without_AccountSetupProvisioningComplete (SkipUser arm of the strong gate) DecisionCore 324/324 + V2 Core 1135/1135 green. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../buildcounter.txt | 2 +- .../AutopilotMonitor.Agent/buildcounter.txt | 2 +- .../ClassicHelloDisabledCompletionTests.cs | 101 +++++++++++++++++- .../Engine/DecisionEngine.Classic.cs | 42 +++++--- 4 files changed, 131 insertions(+), 16 deletions(-) diff --git a/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt b/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt index 4e18fc9c..8a8c012a 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt +++ b/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt @@ -1 +1 @@ -848 +853 diff --git a/src/Agent/AutopilotMonitor.Agent/buildcounter.txt b/src/Agent/AutopilotMonitor.Agent/buildcounter.txt index f2523706..80b6d67e 100644 --- a/src/Agent/AutopilotMonitor.Agent/buildcounter.txt +++ b/src/Agent/AutopilotMonitor.Agent/buildcounter.txt @@ -1 +1 @@ -1393 +1394 diff --git a/src/Agent/AutopilotMonitor.DecisionCore.Tests/ClassicHelloDisabledCompletionTests.cs b/src/Agent/AutopilotMonitor.DecisionCore.Tests/ClassicHelloDisabledCompletionTests.cs index 161b831c..38ba291b 100644 --- a/src/Agent/AutopilotMonitor.DecisionCore.Tests/ClassicHelloDisabledCompletionTests.cs +++ b/src/Agent/AutopilotMonitor.DecisionCore.Tests/ClassicHelloDisabledCompletionTests.cs @@ -15,13 +15,23 @@ namespace AutopilotMonitor.DecisionCore.Tests /// dual-path mechanisms: /// /// Fast-path in HandleDesktopArrivedV1: when HelloPolicyEnabled==false AND - /// AccountSetup has been reached, synthesize HelloOutcome="Skipped" on - /// DesktopArrived and route through Finalizing immediately. + /// the strong post-AccountSetup gate (ShouldTransitionToAwaitingHello) holds — + /// i.e. either AccountSetupProvisioningSucceededUtc is set OR + /// ScenarioObservations.SkipUserEsp is observed as true — synthesise + /// HelloOutcome="Skipped" on DesktopArrived and route through Finalizing + /// immediately. /// Reducer-path via EspExiting → HelloSafety 300s deadline → synthetic /// HelloOutcome="Timeout" fact (independent of the Hello policy). /// /// The dual-path design ensures completion even if one half regresses (missing /// HelloPolicyDetected adapter or missing EspExiting adapter). + /// + /// Session 08c99638 fix (2026-05-21): the fast-path guard was tightened from + /// AccountSetupEnteredUtc != null to the strong gate above, restoring parity with + /// HandleEspExitingV1. Without the tightening, a late desktop_arrived would + /// drive enrollment_complete on a session whose AccountSetup category never reached + /// categorySucceeded=true (apps stuck in_progress, 0/N subcategories complete). + /// /// public sealed class ClassicHelloDisabledCompletionTests { @@ -233,6 +243,93 @@ namespace AutopilotMonitor.DecisionCore.Tests Assert.DoesNotContain(step.Effects, e => e.Kind == DecisionEffectKind.CancelDeadline); } + // -------------------------------------------- Fix 3 — session 08c99638 strong-gate parity + + [Fact] + public void HelloDisabled_FastPath_skipped_when_AccountSetup_entered_but_provisioning_never_succeeded() + { + // Session 08c99638 (2026-05-21) repro: Classic-v1 + Hello-disabled. + // ESP "exits" via Shell-Core event 62407 (errorCode=0) while AccountSetupCategory.Status + // is still categorySucceeded=in_progress (apps never finished, 0/N subcategories). + // AccountSetupProvisioningComplete is therefore NEVER raised → AccountSetupProvisioningSucceededUtc + // stays null. The pre-fix weak gate (AccountSetupEnteredUtc != null) let a late + // DesktopArrived drive enrollment_complete; the strong gate (parity with + // ShouldTransitionToAwaitingHello) MUST keep the session parked. + var engine = new DecisionEngine(); + var state = DecisionState.CreateInitial("sess-08c99638", "tenant-08c99638", T0); + state = engine.Reduce(state, MakeSignal(0, DecisionSignalKind.SessionStarted, T0, null)).NewState; + state = engine.Reduce(state, MakeSignal(1, DecisionSignalKind.EspPhaseChanged, T0.AddMinutes(1), + new Dictionary { [SignalPayloadKeys.EspPhase] = "DeviceSetup" })).NewState; + state = engine.Reduce(state, MakeSignal(2, DecisionSignalKind.HelloPolicyDetected, T0.AddMinutes(1).AddSeconds(5), + new Dictionary { [SignalPayloadKeys.HelloEnabled] = "false" })).NewState; + state = engine.Reduce(state, MakeSignal(3, DecisionSignalKind.EspPhaseChanged, T0.AddMinutes(10), + new Dictionary { [SignalPayloadKeys.EspPhase] = "AccountSetup" })).NewState; + // NO AccountSetupProvisioningComplete — AccountSetup never reached categorySucceeded=true. + // EspExiting is observed (Shell-Core 62407, errorCode=0), recording EspFinalExitUtc as + // an observability fact but not promoting to AwaitingHello (strong gate rejects). + state = engine.Reduce(state, MakeSignal(4, DecisionSignalKind.EspExiting, T0.AddMinutes(25), null)).NewState; + + // Preconditions for the fast-path: Hello disabled + AccountSetup entered + Desktop not yet seen. + Assert.False(state.HelloPolicyEnabled!.Value); + Assert.NotNull(state.AccountSetupEnteredUtc); + Assert.NotNull(state.EspFinalExitUtc); + Assert.Null(state.AccountSetupProvisioningSucceededUtc); + Assert.Null(state.HelloResolvedUtc); + Assert.NotEqual(SessionStage.AwaitingHello, state.Stage); + + // Desktop arrives 35 min after EspExiting (mirroring 08c99638's 35-min gap). + var step = engine.Reduce(state, MakeSignal(10, DecisionSignalKind.DesktopArrived, T0.AddMinutes(60), null)); + + // Strong gate rejects: no Finalizing, no synthetic HelloOutcome, no FinalizingGrace deadline. + Assert.NotEqual(SessionStage.Finalizing, step.NewState.Stage); + Assert.Null(step.NewState.HelloResolvedUtc); + Assert.Null(step.NewState.HelloOutcome); + Assert.NotNull(step.NewState.DesktopArrivedUtc); // observability fact IS recorded + Assert.DoesNotContain(step.NewState.Deadlines, d => d.Name == DeadlineNames.FinalizingGrace); + Assert.DoesNotContain(step.Effects, + e => e.Kind == DecisionEffectKind.EmitEventTimelineEntry + && e.Parameters != null + && e.Parameters.TryGetValue("eventType", out var et) && et == "phase_transition"); + Assert.DoesNotContain(step.Effects, + e => e.Kind == DecisionEffectKind.ScheduleDeadline + && e.Deadline?.Name == DeadlineNames.FinalizingGrace); + } + + [Fact] + public void HelloDisabled_FastPath_fires_on_SkipUserEsp_flow_even_without_AccountSetupProvisioningComplete() + { + // Mirror of the strong-gate's second arm: SkipUserEsp=true (no User-ESP page on this + // flow) is sufficient to satisfy the gate. AccountSetupProvisioningSucceededUtc need + // not be set — there's no AccountSetup phase to wait for. + var engine = new DecisionEngine(); + var state = DecisionState.CreateInitial("sess-skipuser", "tenant-skipuser", T0); + state = engine.Reduce(state, MakeSignal(0, DecisionSignalKind.SessionStarted, T0, null)).NewState; + state = engine.Reduce(state, MakeSignal(1, DecisionSignalKind.EspConfigDetected, T0.AddSeconds(5), + new Dictionary + { + [SignalPayloadKeys.SkipUserEsp] = "true", + [SignalPayloadKeys.SkipDeviceEsp] = "false", + })).NewState; + state = engine.Reduce(state, MakeSignal(2, DecisionSignalKind.EspPhaseChanged, T0.AddMinutes(1), + new Dictionary { [SignalPayloadKeys.EspPhase] = "DeviceSetup" })).NewState; + state = engine.Reduce(state, MakeSignal(3, DecisionSignalKind.HelloPolicyDetected, T0.AddMinutes(1).AddSeconds(5), + new Dictionary { [SignalPayloadKeys.HelloEnabled] = "false" })).NewState; + state = engine.Reduce(state, MakeSignal(4, DecisionSignalKind.EspPhaseChanged, T0.AddMinutes(3), + new Dictionary { [SignalPayloadKeys.EspPhase] = "AccountSetup" })).NewState; + // NO AccountSetupProvisioningComplete — but SkipUserEsp=true unlocks the gate. + + Assert.True(state.ScenarioObservations.SkipUserEsp?.Value); + Assert.Null(state.AccountSetupProvisioningSucceededUtc); + + var step = engine.Reduce(state, MakeSignal(10, DecisionSignalKind.DesktopArrived, T0.AddMinutes(10), null)); + + // Fast-path fires via the SkipUserEsp arm of the strong gate. + Assert.Equal(SessionStage.Finalizing, step.NewState.Stage); + Assert.Equal("Skipped", step.NewState.HelloOutcome!.Value); + Assert.NotNull(step.NewState.HelloResolvedUtc); + Assert.Contains(step.NewState.Deadlines, d => d.Name == DeadlineNames.FinalizingGrace); + } + // ====================================================================== test helpers /// diff --git a/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.Classic.cs b/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.Classic.cs index ce6b1a72..5433a338 100644 --- a/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.Classic.cs +++ b/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.Classic.cs @@ -37,11 +37,12 @@ namespace AutopilotMonitor.DecisionCore.Engine EnrollmentPhase.AccountSetup => SessionStage.EspAccountSetup, // 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. + // handoff, not the true final. Only promote to AwaitingHello when the strong + // post-AccountSetup gate (ShouldTransitionToAwaitingHello) holds: either + // AccountSetupProvisioningSucceededUtc is set OR SkipUserEsp is observed as true. + // 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, @@ -307,13 +308,27 @@ namespace AutopilotMonitor.DecisionCore.Engine /// resolved the session completes here. /// /// Hello-disabled fast-path: when is - /// explicitly false AND we have reached AccountSetup (so the Hello-skip is the - /// genuine post-ESP state rather than a premature mid-DeviceSetup classification), - /// synthesise HelloOutcome="Skipped" here and route directly through Finalizing. + /// explicitly false AND the post-AccountSetup completion gate + /// () holds — i.e. either + /// is set (ESP genuinely + /// finished AccountSetup) OR + /// is true (no User-ESP page on this flow) — synthesise + /// HelloOutcome="Skipped" here and route directly through Finalizing. /// This avoids waiting out the full 5-min HelloSafety window when the policy reader /// already told us no Hello wizard is expected. Belt-and-suspenders alongside the /// EspExiting → HelloSafety reducer path: the path that fires first wins. /// + /// + /// Session 08c99638 fix (2026-05-21): the gate was previously + /// AccountSetupEnteredUtc != null, which is too weak. Shell-Core event 62407 + /// (CommercialOOBE_ESPProgress_Page_Exiting) can fire with errorCode=0 + /// while AccountSetup is still categorySucceeded=in_progress (apps never finished, + /// 0/N subcategories complete). The old gate let a late desktop_arrived + Hello + /// disabled drive enrollment_complete for a session that should have stalled. + /// The strong gate keeps parity with 's + /// check — both paths now require the same + /// post-AccountSetup evidence before any forward transition. + /// /// private DecisionStep HandleDesktopArrivedV1(DecisionState state, DecisionSignal signal) { @@ -327,14 +342,17 @@ namespace AutopilotMonitor.DecisionCore.Engine // Hello-disabled fast-path. Both guards required: // 1. HelloPolicyEnabled?.Value == false — policy reader confirmed no Hello wizard - // 2. AccountSetupEnteredUtc != null — same gate as ShouldTransitionToAwaitingHello, - // prevents a premature pre-AccountSetup completion if HelloPolicyDetected fires - // before AccountSetup is observed. + // 2. ShouldTransitionToAwaitingHello(state) — strong post-AccountSetup gate, identical + // to the one used by HandleEspExitingV1. Requires AccountSetupProvisioningSucceededUtc + // to be set OR SkipUserEsp observed as true. The prior weak guard + // (AccountSetupEnteredUtc != null) allowed completion when ESP "exited" via + // Shell-Core 62407 while AccountSetup categorySucceeded was still in_progress + // (session 08c99638). // Skipping unknown policy (HelloPolicyEnabled == null) preserves the prior pessimistic // behaviour: keep waiting for Hello / HelloSafety. if (!helloAlreadyResolved && state.HelloPolicyEnabled?.Value == false - && state.AccountSetupEnteredUtc != null) + && ShouldTransitionToAwaitingHello(state)) { builder.HelloResolvedUtc = new SignalFact(signal.OccurredAtUtc, signal.SessionSignalOrdinal); builder.HelloOutcome = new SignalFact("Skipped", signal.SessionSignalOrdinal);