diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Ime/ImeLogTrackerPhaseSnapshotTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Ime/ImeLogTrackerPhaseSnapshotTests.cs index d4afd1ad..a0a57035 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Ime/ImeLogTrackerPhaseSnapshotTests.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Ime/ImeLogTrackerPhaseSnapshotTests.cs @@ -108,6 +108,84 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Monitoring.Ime Assert.Equal(3, all.Count(p => p.Targeted == AppTargeted.User)); } + [Fact] + public void PhasePackageSnapshots_survive_save_and_load_roundtrip() + { + // Codex follow-up (882fef64 PR3-PR5 review): on hybrid-join + multi-reboot + // enrollments the agent commonly restarts mid-AccountSetup. Before this fix the + // snapshot dict was lost on restart and DeviceSetup apps disappeared from + // FinalStatus + app_tracking_summary. Round-trip the persistence path on the + // same state directory to prove the dict survives. + using var tmp = new TempDirectory(); + var stateDir = System.IO.Path.Combine(tmp.Path, "State"); + var logger = new AgentLogger(tmp.Path, AgentLogLevel.Info); + + using (var trackerA = new ImeLogTracker( + logFolder: tmp.Path, + patterns: new List(), + logger: logger, + stateDirectory: stateDir)) + { + trackerA.SeedPhaseSnapshotForTesting("DeviceSetup", new[] + { + NewPkg("device-1", AppTargeted.Device, AppInstallationState.Installed), + NewPkg("device-2", AppTargeted.Device, AppInstallationState.Installed), + }); + trackerA.PackageStates.Add(NewPkg("user-1", AppTargeted.User, AppInstallationState.Installed)); + + trackerA.SaveStateForTest(); + } + + // Fresh tracker on the same state dir — simulates an agent restart picking up the + // persisted state. LoadStateForTest replaces the synchronous Start() path so file + // watchers stay out of the test. + using (var trackerB = new ImeLogTracker( + logFolder: tmp.Path, + patterns: new List(), + logger: logger, + stateDirectory: stateDir)) + { + trackerB.LoadStateForTest(); + + var all = trackerB.GetAllKnownPackageStates(); + Assert.Equal(3, all.Count); + Assert.Equal(2, all.Count(p => p.Targeted == AppTargeted.Device)); + Assert.Equal(1, all.Count(p => p.Targeted == AppTargeted.User)); + Assert.Contains(all, p => p.Id == "device-1"); + Assert.Contains(all, p => p.Id == "device-2"); + Assert.Contains(all, p => p.Id == "user-1"); + } + } + + [Fact] + public void LoadState_tolerates_old_snapshot_files_without_PhasePackageSnapshots_field() + { + // Backwards-compat: agent rolled out before this field exists has snapshot files + // with no PhasePackageSnapshots entry. Loading must NOT throw and must leave the + // in-memory dict empty (degrades to live-only view). + using var tmp = new TempDirectory(); + var stateDir = System.IO.Path.Combine(tmp.Path, "State"); + System.IO.Directory.CreateDirectory(stateDir); + + // Hand-craft an old-shape state file: contains only fields that pre-dated the + // PhasePackageSnapshots addition. JSON-deserialiser must default the new field + // to null without complaining. + var oldJson = "{\"CurrentPhaseOrder\":1,\"LastEspPhaseDetected\":\"DeviceSetup\",\"AllAppsCompletedFired\":false,\"LogPhaseIsCurrentPhase\":true,\"SeenAppIds\":[],\"IgnoreList\":[],\"CurrentPackageId\":null,\"Packages\":[],\"FilePositions\":{}}"; + System.IO.File.WriteAllText(System.IO.Path.Combine(stateDir, "ime-tracker-state.json"), oldJson); + + var logger = new AgentLogger(tmp.Path, AgentLogLevel.Info); + using var tracker = new ImeLogTracker( + logFolder: tmp.Path, + patterns: new List(), + logger: logger, + stateDirectory: stateDir); + + // Must not throw. + tracker.LoadStateForTest(); + // No snapshots restored, no live packages → empty consolidated view. + Assert.Empty(tracker.GetAllKnownPackageStates()); + } + [Fact] public void GetAllKnownPackageStates_dedupes_by_id_with_live_winning() { diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/Ime/ImeLogTracker.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/Ime/ImeLogTracker.cs index 89d8c25b..5e12c631 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/Ime/ImeLogTracker.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/Ime/ImeLogTracker.cs @@ -382,23 +382,30 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.Ime _packageStates.Clear(); foreach (var p in state.Packages) { - var pkg = AppPackageState.Restore( - p.Id, p.ListPos, p.Name, - (AppRunAs)p.RunAs, (AppIntent)p.Intent, (AppTargeted)p.Targeted, - p.DependsOn != null ? new HashSet(p.DependsOn) : new HashSet(), - (AppInstallationState)p.InstallationState, p.DownloadingOrInstallingSeen, - p.ProgressPercent, p.BytesDownloaded, p.BytesTotal, - errorPatternId: p.ErrorPatternId, errorDetail: p.ErrorDetail, errorCode: p.ErrorCode, - exitCode: p.ExitCode, hresultFromWin32: p.HResultFromWin32, - doFileSize: p.DoFileSize, doTotalBytesDownloaded: p.DoTotalBytesDownloaded, - doBytesFromPeers: p.DoBytesFromPeers, doPercentPeerCaching: p.DoPercentPeerCaching, - doBytesFromLanPeers: p.DoBytesFromLanPeers, doBytesFromGroupPeers: p.DoBytesFromGroupPeers, - doBytesFromInternetPeers: p.DoBytesFromInternetPeers, doDownloadMode: p.DoDownloadMode, - doDownloadDuration: p.DoDownloadDuration, doBytesFromHttp: p.DoBytesFromHttp, - hasDoTelemetry: p.HasDoTelemetry, - appVersion: p.AppVersion, appType: p.AppType, - attemptNumber: p.AttemptNumber, detectionResult: p.DetectionResult); - _packageStates.Add(pkg); + _packageStates.Add(FromPackageStateData(p)); + } + } + + // Codex follow-up (882fef64 PR3-PR5 review): restore per-phase snapshots so + // GetAllKnownPackageStates() still sees DeviceSetup apps after a mid-AccountSetup + // restart. Old state files that pre-date the field have null here — leave the + // dict empty in that case (degrades to live-only view, no crash). + _phasePackageSnapshots.Clear(); + if (state.PhasePackageSnapshots != null) + { + foreach (var kv in state.PhasePackageSnapshots) + { + if (string.IsNullOrEmpty(kv.Key) || kv.Value == null) continue; + var restored = new List(kv.Value.Count); + foreach (var p in kv.Value) + { + try { restored.Add(FromPackageStateData(p)); } + catch (Exception ex) + { + _logger.Warning($"ImeLogTracker: failed to restore phase-snapshot package '{p?.Id ?? "(null)"}' for phase '{kv.Key}': {ex.Message}"); + } + } + _phasePackageSnapshots[kv.Key] = restored; } } @@ -438,42 +445,18 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.Ime SeenAppIds = _seenAppIds.ToList(), IgnoreList = _packageStates.IgnoreList.ToList(), CurrentPackageId = _packageStates.CurrentPackageId, - Packages = _packageStates.Select(p => new PackageStateData - { - Id = p.Id, - ListPos = p.ListPos, - Name = p.Name, - RunAs = (int)p.RunAs, - Intent = (int)p.Intent, - Targeted = (int)p.Targeted, - DependsOn = p.DependsOn?.ToList() ?? new List(), - InstallationState = (int)p.InstallationState, - DownloadingOrInstallingSeen = p.DownloadingOrInstallingSeen, - ProgressPercent = p.ProgressPercent, - BytesDownloaded = p.BytesDownloaded, - BytesTotal = p.BytesTotal, - ErrorPatternId = p.ErrorPatternId, - ErrorDetail = p.ErrorDetail, - ErrorCode = p.ErrorCode, - ExitCode = p.ExitCode, - HResultFromWin32 = p.HResultFromWin32, - DoFileSize = p.DoFileSize, - DoTotalBytesDownloaded = p.DoTotalBytesDownloaded, - DoBytesFromPeers = p.DoBytesFromPeers, - DoPercentPeerCaching = p.DoPercentPeerCaching, - DoBytesFromLanPeers = p.DoBytesFromLanPeers, - DoBytesFromGroupPeers = p.DoBytesFromGroupPeers, - DoBytesFromInternetPeers = p.DoBytesFromInternetPeers, - DoDownloadMode = p.DoDownloadMode, - DoDownloadDuration = p.DoDownloadDuration, - DoBytesFromHttp = p.DoBytesFromHttp, - HasDoTelemetry = p.HasDoTelemetry, - AppVersion = p.AppVersion, - AppType = p.AppType, - AttemptNumber = p.AttemptNumber, - DetectionResult = p.DetectionResult - }).ToList(), - FilePositions = new Dictionary() + Packages = _packageStates.Select(ToPackageStateData).ToList(), + FilePositions = new Dictionary(), + + // Codex follow-up (882fef64 PR3-PR5 review): persist per-phase snapshots so + // hybrid-join / multi-reboot enrollments don't lose DeviceSetup apps from + // FinalStatus + app_tracking_summary across an agent restart. The dict key is + // the ESP phase name (e.g. "DeviceSetup"); the value is the dehydrated snapshot + // captured on _packageStates.Clear() during the next-phase transition. + PhasePackageSnapshots = _phasePackageSnapshots.ToDictionary( + kv => kv.Key, + kv => kv.Value.Select(ToPackageStateData).ToList(), + StringComparer.Ordinal), }; // Store file positions by filename only (log folder is known) @@ -490,6 +473,64 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.Ime _statePersistence.Save(state); } + // Codex follow-up (882fef64 PR3-PR5 review): single source of truth for AppPackageState + // <-> PackageStateData mapping. Used both for the live _packageStates list and for the + // _phasePackageSnapshots restore path so the field set never drifts between the two. + private static PackageStateData ToPackageStateData(AppPackageState p) => + new PackageStateData + { + Id = p.Id, + ListPos = p.ListPos, + Name = p.Name, + RunAs = (int)p.RunAs, + Intent = (int)p.Intent, + Targeted = (int)p.Targeted, + DependsOn = p.DependsOn?.ToList() ?? new List(), + InstallationState = (int)p.InstallationState, + DownloadingOrInstallingSeen = p.DownloadingOrInstallingSeen, + ProgressPercent = p.ProgressPercent, + BytesDownloaded = p.BytesDownloaded, + BytesTotal = p.BytesTotal, + ErrorPatternId = p.ErrorPatternId, + ErrorDetail = p.ErrorDetail, + ErrorCode = p.ErrorCode, + ExitCode = p.ExitCode, + HResultFromWin32 = p.HResultFromWin32, + DoFileSize = p.DoFileSize, + DoTotalBytesDownloaded = p.DoTotalBytesDownloaded, + DoBytesFromPeers = p.DoBytesFromPeers, + DoPercentPeerCaching = p.DoPercentPeerCaching, + DoBytesFromLanPeers = p.DoBytesFromLanPeers, + DoBytesFromGroupPeers = p.DoBytesFromGroupPeers, + DoBytesFromInternetPeers = p.DoBytesFromInternetPeers, + DoDownloadMode = p.DoDownloadMode, + DoDownloadDuration = p.DoDownloadDuration, + DoBytesFromHttp = p.DoBytesFromHttp, + HasDoTelemetry = p.HasDoTelemetry, + AppVersion = p.AppVersion, + AppType = p.AppType, + AttemptNumber = p.AttemptNumber, + DetectionResult = p.DetectionResult, + }; + + private static AppPackageState FromPackageStateData(PackageStateData p) => + AppPackageState.Restore( + p.Id, p.ListPos, p.Name, + (AppRunAs)p.RunAs, (AppIntent)p.Intent, (AppTargeted)p.Targeted, + p.DependsOn != null ? new HashSet(p.DependsOn) : new HashSet(), + (AppInstallationState)p.InstallationState, p.DownloadingOrInstallingSeen, + p.ProgressPercent, p.BytesDownloaded, p.BytesTotal, + errorPatternId: p.ErrorPatternId, errorDetail: p.ErrorDetail, errorCode: p.ErrorCode, + exitCode: p.ExitCode, hresultFromWin32: p.HResultFromWin32, + doFileSize: p.DoFileSize, doTotalBytesDownloaded: p.DoTotalBytesDownloaded, + doBytesFromPeers: p.DoBytesFromPeers, doPercentPeerCaching: p.DoPercentPeerCaching, + doBytesFromLanPeers: p.DoBytesFromLanPeers, doBytesFromGroupPeers: p.DoBytesFromGroupPeers, + doBytesFromInternetPeers: p.DoBytesFromInternetPeers, doDownloadMode: p.DoDownloadMode, + doDownloadDuration: p.DoDownloadDuration, doBytesFromHttp: p.DoBytesFromHttp, + hasDoTelemetry: p.HasDoTelemetry, + appVersion: p.AppVersion, appType: p.AppType, + attemptNumber: p.AttemptNumber, detectionResult: p.DetectionResult); + /// /// Deletes persisted state file. Called on enrollment complete to ensure /// a fresh state on the next enrollment cycle. @@ -499,6 +540,12 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.Ime _statePersistence?.Delete(); } + // Test-only seams: exercise the persistence round-trip without driving Start() (which + // also spins up file watchers). Codex follow-up (882fef64 PR3-PR5 review) needs these + // to verify PhasePackageSnapshots survive a simulated agent restart. + internal void SaveStateForTest() => SaveState(); + internal void LoadStateForTest() => LoadState(); + #endregion } diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/Ime/ImeTrackerStatePersistence.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/Ime/ImeTrackerStatePersistence.cs index 795d69cb..bf68df56 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/Ime/ImeTrackerStatePersistence.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/Ime/ImeTrackerStatePersistence.cs @@ -106,6 +106,16 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.Ime public string CurrentPackageId { get; set; } public List Packages { get; set; } public Dictionary FilePositions { get; set; } + + // Codex follow-up (882fef64 PR3-PR5 review): per-phase package snapshots that the + // tracker captures just before _packageStates.Clear() on the next-phase transition. + // Without persistence, an agent restart between phases (common on hybrid-join + + // multi-reboot enrollments) loses the earlier-phase apps from the union returned by + // GetAllKnownPackageStates, which in turn drops them from FinalStatus + the + // app_tracking_summary terminal event. Null on agents from before this field was + // added — LoadState handles that as "no snapshots" and the in-memory dict stays empty + // (degrades gracefully back to the live _packageStates view). + public Dictionary> PhasePackageSnapshots { get; set; } } public class PackageStateData diff --git a/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt b/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt index 1ce6b02d..947e93bc 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt +++ b/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt @@ -1 +1 @@ -339 +341 diff --git a/src/Agent/AutopilotMonitor.Agent/buildcounter.txt b/src/Agent/AutopilotMonitor.Agent/buildcounter.txt index 169ec560..0a7f95a2 100644 --- a/src/Agent/AutopilotMonitor.Agent/buildcounter.txt +++ b/src/Agent/AutopilotMonitor.Agent/buildcounter.txt @@ -1 +1 @@ -1165 +1166 diff --git a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Sessions.cs b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Sessions.cs index 2c82fa0f..b62e97d5 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Sessions.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.Sessions.cs @@ -1222,6 +1222,18 @@ namespace AutopilotMonitor.Functions.Services if (currentPhase.HasValue) forceUpdate["CurrentPhase"] = (int)currentPhase.Value; + // Codex follow-up (882fef64 PR3-PR5 review): mirror the + // terminal-phase override from the normal path (line ~1019). V2 + // emits enrollment_complete with Phase=Unknown (per + // feedback_phase_strategy), so currentPhase.HasValue is false + // here. Without this override the force-update path leaves + // CurrentPhase at its prior (in-flight or -1) value forever + // when ETag retries are exhausted exactly at completion. + if (status == SessionStatus.Succeeded) + { + forceUpdate["CurrentPhase"] = (int)EnrollmentPhase.Complete; + } + var freshStartedAt = freshSession.GetDateTimeOffset("StartedAt")?.UtcDateTime ?? DateTime.MaxValue; if (earliestEventTimestamp.HasValue && earliestEventTimestamp.Value < freshStartedAt) forceUpdate["StartedAt"] = EnsureUtc(earliestEventTimestamp.Value);