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\<sid>\... 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) <noreply@anthropic.com>
2026-10-24 04:32:32 +00:00 · 2026-05-26 17:19:25 +02:00
parent 3a62d86b77
commit dcf89a83f7
2 changed files with 156 additions and 2 deletions
@@ -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\<sid>\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()
        {
@@ -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
            }
        }

+        /// <summary>
+        /// Pre-populate the started + completed dedup sets with every package sub-key already
+        /// present at arming time. Caller MUST hold <see cref="_stateLock"/>. 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.
+        /// </summary>
+        private void SeedExistingPackageIds(
+            RegistryHive hive,
+            string packagesPath,
+            HashSet<string> startedSet,
+            HashSet<string> 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; } }