From b9f0a232e86bcf752e37d74e179b1d24b24d42b2 Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Wed, 10 Jun 2026 22:33:20 +0200 Subject: [PATCH] fix(agent-v2): address Wave-4 review findings P2 + P3 P2 (MON-C2): HelloTracker.ResetForEspResumption did not clear the new _helloUnknownGraceUsed one-shot flag. After a grace-consumed run, a fresh post-resume Hello-wait that hit an undetected policy would skip its grace re-arm and resolve straight to not_configured. Reset the flag + add a regression test (unknown timeout -> reset -> unknown timeout re-grants grace). P3 (ARCH-F1): the completion-gate docs overpromised "add a gate to s_completionGates and every site inherits it". Tightened to state precisely that the collection governs only the completion-ATTEMPT sites routed through CompleteThroughFinalizingOrDefer; the RealmJoin release path (pre-resolution state) and the SelfDeploying deadline path (gate-specific deferral) do NOT consult the collection and need explicit wiring for a second gate. Doc-only. V2.Core.Tests 1297/1297; DecisionCore.Tests 359/359. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../HelloTrackerPolicyDetectionTests.cs | 21 +++++++++++++++ .../Enrollment/SystemSignals/HelloTracker.cs | 4 +++ .../buildcounter.txt | 2 +- .../Engine/DecisionEngine.Completion.cs | 27 ++++++++++++++----- 4 files changed, 46 insertions(+), 8 deletions(-) diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/HelloTrackerPolicyDetectionTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/HelloTrackerPolicyDetectionTests.cs index 68abb9c3..abc3bf10 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/HelloTrackerPolicyDetectionTests.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/HelloTrackerPolicyDetectionTests.cs @@ -171,6 +171,27 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Monitoring Assert.True(f.Tracker.IsCompletionTimerActiveForTest); } + [Fact] + public void Esp_resumption_reset_re_grants_the_unknown_policy_grace() + { + // The one-shot grace flag must reset on ESP resumption (hybrid-join mid-enrollment + // reboot). Without the reset a fresh post-resume wait that hits an undetected policy + // would skip its grace and resolve straight to not_configured. + using var f = new Fixture(); + f.Tracker.StartHelloWaitTimer(); + f.Tracker.TriggerWaitTimeoutForTest(); // unknown → grace consumed + Assert.False(f.Tracker.IsHelloCompleted); + + f.Tracker.ResetForEspResumption(); + + f.Tracker.StartHelloWaitTimer(); + f.Tracker.TriggerWaitTimeoutForTest(); // unknown again → must re-grant grace, not complete + + Assert.False(f.Tracker.IsHelloCompleted); + Assert.Null(f.Tracker.HelloOutcome); + Assert.True(f.Tracker.IsWaitTimerActiveForTest); + } + [Fact] public void HelloTerminalEvent_after_policy_disabled_emits_mismatch_warning() { diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/HelloTracker.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/HelloTracker.cs index 23282059..60d12343 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/HelloTracker.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/HelloTracker.cs @@ -217,6 +217,10 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals _helloWizardStarted = false; _espExitSeen = false; HelloOutcome = null; + // MON-C2 — clear the one-shot unknown-policy grace flag too, otherwise a fresh + // post-resume wait that hits an undetected policy would skip its grace re-arm and + // resolve straight to not_configured. + _helloUnknownGraceUsed = false; } _logger.Info("HelloTracker: Reset for ESP resumption — Hello tracking restarted"); } diff --git a/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt b/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt index 34d9aaeb..0eb5aea3 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt +++ b/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt @@ -1 +1 @@ -1043 +1047 diff --git a/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.Completion.cs b/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.Completion.cs index 6948d6af..510c5f8c 100644 --- a/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.Completion.cs +++ b/src/Shared/AutopilotMonitor.DecisionCore/Engine/DecisionEngine.Completion.cs @@ -10,9 +10,20 @@ namespace AutopilotMonitor.DecisionCore.Engine // SelfDeploying / RealmJoin engine partials, each with its own near-identical // RealmJoinGateOpen check + ":RealmJoinGateClosed" defer block. That duplication was the // origin of the premature/duplicate enrollment_complete regressions (8b8d611d, 08c99638, - // 330f73f3). This partial centralizes the gate SET and the deferral shape so adding a new - // completion precondition (e.g. WDP v2 device-association settle) is one entry in - // s_completionGates rather than an edit at every completion site. + // 330f73f3). This partial centralizes the gate SET and the deferral shape for the + // completion-ATTEMPT sites — the four Classic/Shared handlers that route a "both + // prerequisites in" decision through CompleteThroughFinalizingOrDefer. Those sites inherit a + // new gate purely by it appearing in s_completionGates. + // + // Two paths intentionally do NOT consult the collection (single-gate-correct today; a second + // gate must wire them explicitly — they are the gate's release/origin, not attempt sites): + // * the RealmJoin release path (CompleteIfDeferredOrBookkeep, DecisionEngine.RealmJoin.cs) + // completes directly because it evaluates the PRE-resolution state where the gate it just + // opened still reads closed; a second gate that must re-block here needs a post-state + // re-check; + // * the SelfDeploying deadline path (HandleDeviceOnlyEspDetectionDeadlineFired) owns a + // gate-specific deferral (SelfDeployingDeferredCompletion on RealmJoinFacts, released by + // the RealmJoin handlers) and so checks RealmJoinGateOpen directly. public sealed partial class DecisionEngine { /// @@ -37,10 +48,12 @@ namespace AutopilotMonitor.DecisionCore.Engine /// /// The ordered set of completion preconditions. Append-only: a new enrollment - /// scenario registers one gate here and every completion site that routes through - /// inherits it — no per-site edits. - /// RealmJoin is the only gate today (the SelfDeploying deadline path additionally owns a - /// gate-specific deferral; see HandleDeviceOnlyEspDetectionDeadlineFired). + /// scenario registers one gate here and every completion-ATTEMPT site that routes through + /// inherits it with no per-site edits. + /// RealmJoin is the only gate today. Note this collection does NOT cover the RealmJoin + /// release path or the SelfDeploying deadline path — both are gate-specific by design + /// (see the class-level remarks); a second gate that must block there needs explicit + /// wiring, not just an entry here. /// private static readonly CompletionGate[] s_completionGates = {