From 39ffa93ee779306c543885dfc09852de5e44f484 Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Fri, 5 Jun 2026 20:54:54 +0200 Subject: [PATCH] fix(agent-v2): emit non-failure subcategory_state_change transitions DetectSubcategoryTransitions silently dropped every non-failure, non-completion, non-noise subcategory transition (e.g. succeeded -> in_progress retry, failed -> in_progress recovery, in_progress -> notStarted reset) due to a dead inner 'if (isFailure)' guard that is always false after the outer failure/completion branches. The esp_provisioning_status event downgraded to a plain progress update without a subcategory_state_change changeType. Record those as non-failure transitions so they surface; add regression coverage. --- .../SubcategoryTransitionDetectionTests.cs | 129 ++++++++++++++++++ .../ProvisioningStatusTracker.cs | 7 +- .../buildcounter.txt | 2 +- 3 files changed, 135 insertions(+), 3 deletions(-) create mode 100644 src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/SystemSignals/SubcategoryTransitionDetectionTests.cs diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/SystemSignals/SubcategoryTransitionDetectionTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/SystemSignals/SubcategoryTransitionDetectionTests.cs new file mode 100644 index 00000000..d24fa91b --- /dev/null +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/SystemSignals/SubcategoryTransitionDetectionTests.cs @@ -0,0 +1,129 @@ +using System; +using System.Collections.Generic; +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.Tests.Harness; +using AutopilotMonitor.Agent.V2.Core.Tests.Orchestration; +using AutopilotMonitor.DecisionCore.Engine; +using AutopilotMonitor.DecisionCore.Signals; +using Xunit; + +namespace AutopilotMonitor.Agent.V2.Core.Tests.Monitoring.SystemSignals +{ + /// + /// Regression coverage for the previously-unreachable non-failure branch in + /// DetectSubcategoryTransitions. The branch was guarded by an inner + /// if (isFailure) that is always false once the outer + /// isFailure || isCompletion has been excluded, so every non-failure, + /// non-completion, non-noise transition (e.g. succeeded -> in_progress retry) + /// was silently dropped — the esp_provisioning_status event downgraded to a + /// plain progress update without a subcategory_state_change changeType. + /// After the fix those transitions must surface in the event. + /// + public sealed class SubcategoryTransitionDetectionTests + { + private static readonly DateTime Fixed = new DateTime(2026, 6, 5, 9, 0, 0, DateTimeKind.Utc); + + private sealed class Fixture : IDisposable + { + public TempDirectory Tmp { get; } = new TempDirectory(); + public FakeSignalIngressSink Sink { get; } = new FakeSignalIngressSink(); + public ProvisioningStatusTracker Tracker { get; } + + public Fixture() + { + var clock = new VirtualClock(Fixed); + var post = new InformationalEventPost(Sink, clock); + var logger = new AgentLogger(Tmp.Path, AgentLogLevel.Info); + Tracker = new ProvisioningStatusTracker( + sessionId: "S1", + tenantId: "T1", + post: post, + logger: logger); + } + + public IReadOnlyList<(string EventType, IReadOnlyDictionary Data)> CapturedEvents() + { + return Sink.Posted + .Where(p => p.Kind == DecisionSignalKind.InformationalEvent && p.Payload != null) + .Select(p => + { + var eventType = p.Payload!.TryGetValue(SignalPayloadKeys.EventType, out var et) ? et : ""; + var data = p.TypedPayload as IReadOnlyDictionary + ?? (IReadOnlyDictionary)new Dictionary(); + return (eventType, data); + }) + .ToList(); + } + + public void Dispose() + { + Tracker.Dispose(); + Tmp.Dispose(); + } + } + + [Fact] + public void NonFailureSubcategoryTransition_IsRecorded_AndSurfacedAsStateChange() + { + using var f = new Fixture(); + + // First seen — Apps already succeeded. + f.Tracker.ProcessCategoryStatusForTest("DeviceSetupCategory.Status", @"{ + ""categorySucceeded"": null, + ""AppsSubcategory"": {""subcategoryState"":""succeeded"",""subcategoryStatusText"":""Apps (Identified)""} + }"); + + // Apps regresses succeeded -> in_progress (retry). categorySucceeded unchanged (null), + // so processing falls through to subcategory-transition detection. + f.Tracker.ProcessCategoryStatusForTest("DeviceSetupCategory.Status", @"{ + ""categorySucceeded"": null, + ""AppsSubcategory"": {""subcategoryState"":""in_progress"",""subcategoryStatusText"":""Apps (Downloading)""} + }"); + + var captured = f.CapturedEvents(); + var statusEvents = captured + .Where(e => e.EventType == AutopilotMonitor.Shared.Constants.EventTypes.EspProvisioningStatus) + .ToList(); + + // A subcategory_state_change event must be emitted carrying the transition. + var transitionEvent = statusEvents.Single(e => + e.Data.TryGetValue("changeType", out var ct) && (string)ct == "subcategory_state_change"); + + var transitions = Assert.IsType>>(transitionEvent.Data["transitions"]); + var appsTransition = Assert.Single(transitions, t => t["subcategory"] == "Apps"); + Assert.Equal("succeeded", appsTransition["previousState"]); + Assert.Equal("in_progress", appsTransition["newState"]); + + // Non-failure transition: no failed-subcategory enrichment. + Assert.False(transitionEvent.Data.ContainsKey("failedSubcategories")); + } + + [Fact] + public void NotStartedToInProgress_IsTreatedAsNoise_NotRecorded() + { + using var f = new Fixture(); + + f.Tracker.ProcessCategoryStatusForTest("DeviceSetupCategory.Status", @"{ + ""categorySucceeded"": null, + ""AppsSubcategory"": {""subcategoryState"":""notStarted"",""subcategoryStatusText"":""Apps (Waiting)""} + }"); + + f.Tracker.ProcessCategoryStatusForTest("DeviceSetupCategory.Status", @"{ + ""categorySucceeded"": null, + ""AppsSubcategory"": {""subcategoryState"":""in_progress"",""subcategoryStatusText"":""Apps (Downloading)""} + }"); + + var captured = f.CapturedEvents(); + var statusEvents = captured + .Where(e => e.EventType == AutopilotMonitor.Shared.Constants.EventTypes.EspProvisioningStatus) + .ToList(); + + // notStarted -> in_progress is benign progress noise: no state-change event. + Assert.DoesNotContain(statusEvents, e => + e.Data.TryGetValue("changeType", out var ct) && (string)ct == "subcategory_state_change"); + } + } +} diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/ProvisioningStatusTracker.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/ProvisioningStatusTracker.cs index 37e9c9e8..2b2570b0 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/ProvisioningStatusTracker.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/ProvisioningStatusTracker.cs @@ -1164,8 +1164,11 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals } else if (!isNoise) { - if (isFailure) - transitions.Add(new SubcategoryTransition(sub.Name, oldState, sub.State, true)); + // Any other real state change (e.g. succeeded -> in_progress retry, + // failed -> in_progress recovery, in_progress -> notStarted reset) is a + // non-failure transition. Record it so the subcategory_state_change event + // carries it; without this the change is silently dropped (observability gap). + transitions.Add(new SubcategoryTransition(sub.Name, oldState, sub.State, false)); } } diff --git a/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt b/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt index 72f91c4b..209fc0eb 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt +++ b/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt @@ -1 +1 @@ -920 +923