From dcf89a83f793b045a3d914eb9d41ce50d29a1fd9 Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Tue, 26 May 2026 17:19:25 +0200 Subject: [PATCH] fix(agent/v2): RealmJoinWatcher seeds pre-existing package IDs at arming so only post-arming sub-keys surface Pre-existing package sub-keys observed at arming time (typically RJ packages installed during ESP before DeploymentPhase >= 100) were silently fired as realmjoin_package_started by the initial CheckMachinePackages sweep. The phase gate only prevented IME/RJ subtree-write misattribution; it did not distinguish historical RJ rows from rows that genuinely appear after arming. Attach now seeds both _*PackagesStarted and _*PackagesCompleted with every sub-key already present under HKLM\SOFTWARE\RealmJoin\Packages (and the HKU\\... mirror), without firing events. The seed runs inside the state-lock BEFORE RegistryWatcher.Start() so no subtree wake-up can race past it. Only sub-keys that appear AFTER arming surface as events. Added 3 unit tests covering the ESP-leftover suppression, the per-PackageId scope of the suppression (siblings still fire), and the HKU mirror. Outdated comment on the immediate-arm test corrected. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../RealmJoinWatcherAdapterTests.cs | 74 +++++++++++++++- .../SystemSignals/RealmJoinWatcher.cs | 84 +++++++++++++++++++ 2 files changed, 156 insertions(+), 2 deletions(-) diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/SignalAdapters/RealmJoinWatcherAdapterTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/SignalAdapters/RealmJoinWatcherAdapterTests.cs index 5d9ec9a8..d0d9f805 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/SignalAdapters/RealmJoinWatcherAdapterTests.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/SignalAdapters/RealmJoinWatcherAdapterTests.cs @@ -270,8 +270,10 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.SignalAdapters // Agent boots into a session where RJ is already at CompletedFirstDeployment // (110) — e.g. pre-installed on an image, or recovered after a long absence. // The first phase observation is already past threshold, so package watchers - // arm in that same notify pass; the historic package rows enumerated by - // CheckMachinePackages then surface as started+completed pairs. + // arm in that same notify pass. Historic package rows that exist at arming + // time are seeded into the dedup sets by AttachMachineRealmJoinSubtreeWatcher + // and are intentionally suppressed — only sub-keys that appear AFTER arming + // surface as started/completed events. using var f = new Fixture(); using var adapter = new RealmJoinWatcherAdapter(f.Watcher, f.Ingress, f.Clock); @@ -282,6 +284,74 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.SignalAdapters Assert.Contains(f.Ingress.Posted, p => p.Kind == DecisionSignalKind.RealmJoinResolved); } + [Fact] + public void Seeded_pre_existing_machine_package_does_not_fire_started_or_completed() + { + // Models the ESP-leftover case: when the package watcher arms, RJ has already + // installed packages during ESP and their sub-keys are present. The seed pass + // pre-fills both dedup sets so the historical rows are silently absorbed. + using var f = new Fixture(); + using var adapter = new RealmJoinWatcherAdapter(f.Watcher, f.Ingress, f.Clock); + + f.Watcher.SeedMachinePackageIdsForTest("generic-esp-leftover"); + + var snap = new RealmJoinPackageSnapshot( + packageId: "generic-esp-leftover", + displayName: "Pre-ESP package", + version: "1.0.0", + success: true, + lastExitCode: 0); + f.Watcher.TriggerMachinePackageObservationFromTest(snap); + + Assert.DoesNotContain(f.Ingress.Posted, p => p.Kind == DecisionSignalKind.RealmJoinPackageStarted); + Assert.DoesNotContain(f.Ingress.Posted, p => p.Kind == DecisionSignalKind.RealmJoinPackageCompleted); + } + + [Fact] + public void Seed_only_suppresses_seeded_ids_genuinely_new_machine_packages_still_fire() + { + // Seed suppression is per-PackageId — a sibling package that appears AFTER arming + // must still fire normally. Guards against an over-broad implementation that + // would mute the whole hive once any seed entry is present. + using var f = new Fixture(); + using var adapter = new RealmJoinWatcherAdapter(f.Watcher, f.Ingress, f.Clock); + + f.Watcher.SeedMachinePackageIdsForTest("generic-esp-leftover"); + + var snap = new RealmJoinPackageSnapshot( + packageId: "generic-new-after-arming", + displayName: "New package", + version: "2.0.0", + success: null, + lastExitCode: null); + f.Watcher.TriggerMachinePackageObservationFromTest(snap); + + var started = f.Ingress.Posted.Single(p => p.Kind == DecisionSignalKind.RealmJoinPackageStarted); + Assert.Equal("generic-new-after-arming", started.Payload![DecisionEngine.RealmJoinPayloadKeys.PackageId]); + } + + [Fact] + public void Seeded_pre_existing_user_package_does_not_fire_started_or_completed() + { + // Same semantics as the machine variant — user-hive packages enumerated under + // HKU\\SOFTWARE\RealmJoin\Packages at arming time must be silently absorbed. + using var f = new Fixture(); + using var adapter = new RealmJoinWatcherAdapter(f.Watcher, f.Ingress, f.Clock); + + f.Watcher.SeedUserPackageIdsForTest("generic-user-leftover"); + + var snap = new RealmJoinPackageSnapshot( + packageId: "generic-user-leftover", + displayName: "Pre-existing user package", + version: "1.0.0", + success: true, + lastExitCode: 0); + f.Watcher.TriggerUserPackageObservationFromTest(snap); + + Assert.DoesNotContain(f.Ingress.Posted, p => p.Kind == DecisionSignalKind.RealmJoinPackageStarted); + Assert.DoesNotContain(f.Ingress.Posted, p => p.Kind == DecisionSignalKind.RealmJoinPackageCompleted); + } + [Fact] public void Watcher_fires_PackageStarted_even_when_DisplayName_is_missing() { diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/RealmJoinWatcher.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/RealmJoinWatcher.cs index ea166329..d294c6f2 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/RealmJoinWatcher.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Enrollment/SystemSignals/RealmJoinWatcher.cs @@ -381,6 +381,18 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals if (_machineRealmJoinWatcher != null) return; try { + // Seed dedup sets with every package sub-key already present at arming time. + // Pre-existing rows are typically RJ packages installed during ESP — they are + // historical, not "new" from the watcher's perspective, and must not surface + // as started/completed events. Must run BEFORE Start() so no subtree wake-up + // can race past the seed and emit pre-existing IDs. + SeedExistingPackageIds( + RegistryHive.LocalMachine, + RealmJoinInfo.MachinePackagesRegistryPath, + _hklmPackagesStarted, + _hklmPackagesCompleted, + scopeLabel: "hklm"); + _machineRealmJoinDebounce = new Timer(_ => CheckMachinePackages(), null, Timeout.Infinite, Timeout.Infinite); _machineRealmJoinWatcher = new RegistryWatcher( RegistryHive.LocalMachine, @@ -412,6 +424,15 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals if (_userRealmJoinWatcher != null) return; try { + // Seed dedup sets — see AttachMachineRealmJoinSubtreeWatcher for rationale. + var userPackagesPath = fullPath + "\\Packages"; + SeedExistingPackageIds( + RegistryHive.Users, + userPackagesPath, + _hkuPackagesStarted, + _hkuPackagesCompleted, + scopeLabel: "hku"); + _userRealmJoinDebounce = new Timer(_ => CheckUserPackages(), null, Timeout.Infinite, Timeout.Infinite); _userRealmJoinWatcher = new RegistryWatcher( RegistryHive.Users, @@ -585,6 +606,43 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals } } + /// + /// Pre-populate the started + completed dedup sets with every package sub-key already + /// present at arming time. Caller MUST hold . Pre-existing keys + /// are historical (typically RJ packages installed during ESP before the phase gate + /// fired) and are intentionally suppressed — only sub-keys that appear AFTER arming + /// surface as started/completed events. Seeding both sets means a pre-existing key is + /// fully ignored, including any late completion-marker write. + /// + private void SeedExistingPackageIds( + RegistryHive hive, + string packagesPath, + HashSet startedSet, + HashSet completedSet, + string scopeLabel) + { + try + { + var ids = RealmJoinInfo.EnumeratePackageIds(hive, packagesPath); + if (ids.Count == 0) + { + _logger.Info($"RealmJoinWatcher: seed pass ({scopeLabel}) found no pre-existing packages under {packagesPath}"); + return; + } + int seeded = 0; + foreach (var id in ids) + { + if (startedSet.Add(id)) seeded++; + completedSet.Add(id); + } + _logger.Info($"RealmJoinWatcher: seeded {seeded} pre-existing package id(s) under {packagesPath} ({scopeLabel}) — events suppressed for these"); + } + catch (Exception ex) + { + _logger.Warning($"RealmJoinWatcher: seed pass ({scopeLabel}) under {packagesPath} threw: {ex.Message}"); + } + } + private void MaybeFirePackageEvents( string scope, RealmJoinPackageSnapshot snap, @@ -668,6 +726,32 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Enrollment.SystemSignals internal void TriggerNotifyRealmJoinPresenceFromTest(int? phase) => NotifyRealmJoinPresence(phase); + // Test seam — directly seed the dedup sets without touching the registry, so that a + // subsequent MaybeFirePackageEvents observation can verify suppression behavior. + internal void SeedMachinePackageIdsForTest(params string[] ids) + { + lock (_stateLock) + { + foreach (var id in ids) + { + _hklmPackagesStarted.Add(id); + _hklmPackagesCompleted.Add(id); + } + } + } + + internal void SeedUserPackageIdsForTest(params string[] ids) + { + lock (_stateLock) + { + foreach (var id in ids) + { + _hkuPackagesStarted.Add(id); + _hkuPackagesCompleted.Add(id); + } + } + } + internal bool PackageWatchersArmedForTest { get { lock (_stateLock) { return _packageWatchersArmed; } }