From 3da26ee8d446a025d3591ac8983fb0f828e8648b Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Fri, 3 Jul 2026 10:56:45 +0200 Subject: [PATCH] fix(agent-v2): two-phase StartupEventGate, restart-safe disk_space_low, prefetch fixes (M3-M6, L17) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Delta-review 2026-07-02 findings: - M4: StartupEventGate persisted the new fingerprint INSIDE ShouldEmit, before the caller emitted — a process death between the save and the spool append suppressed that event for the rest of the enrollment (permanent for static payloads like tpm_info / console_prefetch_detected). ShouldEmit now claims in-memory only; the new MarkEmitted(key) commits to disk AFTER the emission, and SaveSafe writes the committed view only, so other keys` uncommitted claims never ride along. All five consumers updated. - M3: disk_space_low hysteresis latch was an in-memory bool → the Warning (ImmediateUpload) re-fired after every stall restart on a disk-starved device. Latch is now seeded from + persisted via the StartupEventGate (new read-only HasFingerprint peek); the re-arm transition persists too. - M5: ConsolePrefetchScanner compared timestamps from two clock epochs (.pf last-write vs now-minus-uptime); an OOBE NTP correction could swallow a real Shift+F10 hit. 10-min skew tolerance on the after-boot test. - M6: prefetch Warning is once-per-enrollment now — the old fingerprint contained prefetchLastRunUtc, which every ESP `cmd /c` installer wrapper refreshes, so multi-reboot sessions re-warned per restart. (Also moots L11.) - L17: a geo re-lookup that only feeds the pending timezone retry no longer emits a device_location failure after an earlier success; outbound_ip got its own latch instead of riding the device_location one. Co-Authored-By: Claude Fable 5 --- .../Analyzers/ConsolePrefetchScannerTests.cs | 36 ++++++++++ .../Persistence/StartupEventGateTests.cs | 65 +++++++++++++++++- .../PerformanceCollectorDiskLowTests.cs | 37 +++++++--- .../Analyzers/ConsolePrefetchScanner.cs | 37 +++++++--- .../Analyzers/IntegrityBypassAnalyzer.cs | 4 ++ .../Telemetry/Analyzers/LocalAdminAnalyzer.cs | 5 ++ .../Analyzers/SoftwareInventoryAnalyzer.cs | 3 + .../DeviceInfoCollector.NetworkAndSecurity.cs | 4 ++ .../Periodic/PerformanceCollector.cs | 40 +++++++++-- .../Orchestration/DefaultComponentFactory.cs | 3 +- .../Hosts/PeriodicCollectorLifecycleHost.cs | 8 ++- .../Persistence/StartupEventGate.cs | 68 ++++++++++++++++--- .../Runtime/StartupEnvironmentProbes.cs | 30 +++++--- 13 files changed, 297 insertions(+), 43 deletions(-) diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Analyzers/ConsolePrefetchScannerTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Analyzers/ConsolePrefetchScannerTests.cs index 9b62dde9..c4946bc8 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Analyzers/ConsolePrefetchScannerTests.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Analyzers/ConsolePrefetchScannerTests.cs @@ -113,6 +113,42 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Monitoring.Analyzers Assert.Null(rig.PrefetchEvent()); } + [Fact] + public void Artifact_within_clock_skew_tolerance_before_boot_is_still_flagged() + { + // M5: an NTP correction between the .pf write and the scan can push the derived boot + // time past a REAL console's artifact timestamp. Within the tolerance the artifact + // must still count as after-boot. + using var rig = new Rig(); + rig.WritePf("CMD.EXE-0BD30981.pf", BootUtc.AddMinutes(-5)); // inside the 10-min tolerance + + rig.NewScanner().AnalyzeAtStartup(); + + Assert.NotNull(rig.PrefetchEvent()); + } + + [Fact] + public void Gate_suppresses_even_when_installer_cmd_refreshed_the_artifact() + { + // M6: ESP `cmd /c` installer wrappers refresh CMD.EXE-*.pf between reboots. The + // scanner cannot attribute the newer timestamp to a human console, so the Warning is + // once-per-enrollment — a refreshed artifact must NOT re-emit on the next restart. + using var rig = new Rig(); + rig.WritePf("CMD.EXE-0BD30981.pf", BootUtc.AddMinutes(4)); + var gate = rig.NewGate(); + + rig.NewScanner(gate: gate).AnalyzeAtStartup(); + + rig.WritePf("CMD.EXE-0BD30981.pf", BootUtc.AddMinutes(45)); // installer churn + rig.NewScanner(gate: gate).AnalyzeAtStartup(); + + var emitted = rig.Sink.Posted.Count(p => + p.Payload != null + && p.Payload.TryGetValue(SignalPayloadKeys.EventType, out var et) + && et == "console_prefetch_detected"); + Assert.Equal(1, emitted); + } + [Fact] public void No_cmd_artifact_stays_silent() { diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Persistence/StartupEventGateTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Persistence/StartupEventGateTests.cs index cd6ae860..81fbd62c 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Persistence/StartupEventGateTests.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Persistence/StartupEventGateTests.cs @@ -33,13 +33,14 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Persistence } [Fact] - public void ShouldEmit_state_survives_an_agent_restart() + public void ShouldEmit_state_survives_an_agent_restart_once_committed() { using var tmp = new TempDirectory(); var logger = NewLogger(tmp.Path); var firstRun = new StartupEventGate(tmp.Path, logger); Assert.True(firstRun.ShouldEmit("aad_join_status", "not-joined")); + firstRun.MarkEmitted("aad_join_status"); // event went out → commit // New instance over the same state directory = agent restarted after a reboot. var secondRun = new StartupEventGate(tmp.Path, logger); @@ -47,6 +48,68 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Persistence Assert.True(secondRun.ShouldEmit("aad_join_status", "joined")); // late join → emits } + [Fact] + public void Uncommitted_claim_does_not_survive_a_restart() + { + // M4 (delta review 2026-07-02): a crash between ShouldEmit and the actual emission + // must NOT suppress the event for the rest of the enrollment — only MarkEmitted + // (called after the emission) persists the claim. + using var tmp = new TempDirectory(); + var logger = NewLogger(tmp.Path); + + var firstRun = new StartupEventGate(tmp.Path, logger); + Assert.True(firstRun.ShouldEmit("tpm_info", "fp-static")); + // process dies here — no MarkEmitted + + var secondRun = new StartupEventGate(tmp.Path, logger); + Assert.True(secondRun.ShouldEmit("tpm_info", "fp-static")); // re-emits after the crash + } + + [Fact] + public void MarkEmitted_commits_only_its_own_key() + { + // Two events claim; only A's emission goes out before the crash. B must re-emit. + using var tmp = new TempDirectory(); + var logger = NewLogger(tmp.Path); + + var firstRun = new StartupEventGate(tmp.Path, logger); + Assert.True(firstRun.ShouldEmit("event_a", "fp-a")); + Assert.True(firstRun.ShouldEmit("event_b", "fp-b")); + firstRun.MarkEmitted("event_a"); // only A emitted before the crash + + var secondRun = new StartupEventGate(tmp.Path, logger); + Assert.False(secondRun.ShouldEmit("event_a", "fp-a")); + Assert.True(secondRun.ShouldEmit("event_b", "fp-b")); + } + + [Fact] + public void MarkEmitted_without_prior_claim_is_a_noop() + { + using var tmp = new TempDirectory(); + var gate = new StartupEventGate(tmp.Path, NewLogger(tmp.Path)); + + gate.MarkEmitted("gate_exempt_type"); // never claimed → must not throw or persist + Assert.True(gate.ShouldEmit("gate_exempt_type", "fp")); + } + + [Fact] + public void HasFingerprint_peeks_without_claiming() + { + using var tmp = new TempDirectory(); + var logger = NewLogger(tmp.Path); + + var firstRun = new StartupEventGate(tmp.Path, logger); + Assert.False(firstRun.HasFingerprint("disk_space_low", "low")); + Assert.True(firstRun.ShouldEmit("disk_space_low", "low")); + firstRun.MarkEmitted("disk_space_low"); + + var secondRun = new StartupEventGate(tmp.Path, logger); + Assert.True(secondRun.HasFingerprint("disk_space_low", "low")); + Assert.False(secondRun.HasFingerprint("disk_space_low", "rearmed")); + // The peek made no claim: an actual state change still goes through normally. + Assert.True(secondRun.ShouldEmit("disk_space_low", "rearmed")); + } + [Fact] public void Keys_are_independent() { diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Telemetry/PerformanceCollectorDiskLowTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Telemetry/PerformanceCollectorDiskLowTests.cs index 66387a06..acb00190 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Telemetry/PerformanceCollectorDiskLowTests.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Telemetry/PerformanceCollectorDiskLowTests.cs @@ -7,7 +7,8 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Telemetry /// MON-B10 — covers the state-change-only transition logic behind the one-shot /// disk_space_low warning. The warning must fire exactly once on crossing below 2 GB, /// stay silent while the drive remains low (no heartbeat), and only re-arm after free space - /// recovers past the 3 GB hysteresis mark. + /// recovers past the 3 GB hysteresis mark. The rearmed out-flag (M3) tells the caller + /// when to persist the re-arm so the latch survives agent restarts. /// public class PerformanceCollectorDiskLowTests { @@ -15,24 +16,27 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Telemetry public void AboveThreshold_DoesNotWarn_AndStaysDisarmed() { bool warned = false; - Assert.False(PerformanceCollector.EvaluateDiskLowTransition(50.0, ref warned)); + Assert.False(PerformanceCollector.EvaluateDiskLowTransition(50.0, ref warned, out var rearmed)); Assert.False(warned); + Assert.False(rearmed); // was never latched → nothing to re-arm/persist } [Fact] public void CrossingBelowThreshold_WarnsOnce_AndLatches() { bool warned = false; - Assert.True(PerformanceCollector.EvaluateDiskLowTransition(1.9, ref warned)); + Assert.True(PerformanceCollector.EvaluateDiskLowTransition(1.9, ref warned, out var rearmed)); Assert.True(warned); + Assert.False(rearmed); } [Fact] public void StillLowAfterWarning_DoesNotReWarn() { bool warned = true; // already warned on a previous low read - Assert.False(PerformanceCollector.EvaluateDiskLowTransition(1.5, ref warned)); + Assert.False(PerformanceCollector.EvaluateDiskLowTransition(1.5, ref warned, out var rearmed)); Assert.True(warned); + Assert.False(rearmed); } [Fact] @@ -41,16 +45,18 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Telemetry // Between 2 GB (threshold) and 3 GB (recovery): not low enough to re-warn, // not recovered enough to re-arm. Must remain latched and silent. bool warned = true; - Assert.False(PerformanceCollector.EvaluateDiskLowTransition(2.5, ref warned)); + Assert.False(PerformanceCollector.EvaluateDiskLowTransition(2.5, ref warned, out var rearmed)); Assert.True(warned); + Assert.False(rearmed); } [Fact] public void RecoveryPastHysteresisMark_ReArms() { bool warned = true; - Assert.False(PerformanceCollector.EvaluateDiskLowTransition(3.0, ref warned)); + Assert.False(PerformanceCollector.EvaluateDiskLowTransition(3.0, ref warned, out var rearmed)); Assert.False(warned); + Assert.True(rearmed); // caller persists this transition (M3) } [Fact] @@ -59,15 +65,16 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Telemetry bool warned = false; // First drop → warns and latches. - Assert.True(PerformanceCollector.EvaluateDiskLowTransition(1.0, ref warned)); + Assert.True(PerformanceCollector.EvaluateDiskLowTransition(1.0, ref warned, out _)); Assert.True(warned); // Recovers past the hysteresis mark → re-arms. - Assert.False(PerformanceCollector.EvaluateDiskLowTransition(4.0, ref warned)); + Assert.False(PerformanceCollector.EvaluateDiskLowTransition(4.0, ref warned, out var rearmed)); Assert.False(warned); + Assert.True(rearmed); // Drops again → warns a second time. - Assert.True(PerformanceCollector.EvaluateDiskLowTransition(1.8, ref warned)); + Assert.True(PerformanceCollector.EvaluateDiskLowTransition(1.8, ref warned, out _)); Assert.True(warned); } @@ -76,8 +83,18 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Telemetry { // 2.0 GB is the boundary; the predicate is strict "below", so it must not warn. bool warned = false; - Assert.False(PerformanceCollector.EvaluateDiskLowTransition(2.0, ref warned)); + Assert.False(PerformanceCollector.EvaluateDiskLowTransition(2.0, ref warned, out var rearmed)); Assert.False(warned); + Assert.False(rearmed); + } + + [Fact] + public void AboveRecovery_WithoutPriorWarning_DoesNotReportRearm() + { + // A healthy disk must not spam the persisted gate state on every sample. + bool warned = false; + Assert.False(PerformanceCollector.EvaluateDiskLowTransition(10.0, ref warned, out var rearmed)); + Assert.False(rearmed); } } } diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Analyzers/ConsolePrefetchScanner.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Analyzers/ConsolePrefetchScanner.cs index 097139fc..b8393ace 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Analyzers/ConsolePrefetchScanner.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Analyzers/ConsolePrefetchScanner.cs @@ -37,6 +37,14 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Analyzers internal const string CmdPrefetchPattern = "CMD.EXE-*.pf"; internal const string ConhostPrefetchPattern = "CONHOST.EXE-*.pf"; + /// + /// M5: absorbs the clock-epoch mismatch between the .pf NTFS last-write (stamped under + /// the clock at write time) and the boot time derived at scan time as now-minus-uptime + /// (shifts with every NTP correction). OOBE clock corrections are typically seconds to a + /// few minutes; image/sysprep artifacts predate boot by far more than this. + /// + internal static readonly TimeSpan BootClockSkewTolerance = TimeSpan.FromMinutes(10); + private const string Attribution = "coarse: a console executed on this device; cannot distinguish a human Shift+F10 from a " + "legitimate install-launched cmd once ESP is running (cmd.exe shares one prefetch artifact)"; @@ -107,12 +115,19 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Analyzers return; } - bool ranAfterBoot = cmdArtifact.Value.lastRunUtc > bootTimeUtc.Value; + // M5 (delta review 2026-07-02): the two timestamps come from different clock epochs. + // The .pf last-write was stamped under the clock at write time; boot time is derived + // at SCAN time as now-minus-uptime, so an NTP correction in between (routine during + // OOBE) shifts it by the correction amount. A forward correction made a real + // Shift+F10 console look pre-boot ("stale image artifact"). The tolerance absorbs + // typical OOBE clock corrections; image/sysprep artifacts are hours-to-months older + // than boot and stay firmly below it. + bool ranAfterBoot = cmdArtifact.Value.lastRunUtc > bootTimeUtc.Value - BootClockSkewTolerance; if (!ranAfterBoot) { // Stale artifact from the image build / sysprep — not this boot's console. _logger.Debug($"{Name}: cmd prefetch last-run {cmdArtifact.Value.lastRunUtc:o} predates boot " + - $"{bootTimeUtc.Value:o} — stale image artifact, not flagged"); + $"{bootTimeUtc.Value:o} (incl. {BootClockSkewTolerance.TotalMinutes:F0} min skew tolerance) — stale image artifact, not flagged"); return; } @@ -132,14 +147,16 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Analyzers { "coverageComplete", false }, }; - // Restart dedup: the same artifact + last-run was already reported by a previous agent run - // in this enrollment. A NEW cmd run (newer last-run) changes the fingerprint and re-emits. + // Restart dedup — once per enrollment (M6, delta review 2026-07-02). The fingerprint + // used to include prefetchLastRunUtc + corroboratingArtifacts, but every ESP `cmd /c` + // installer wrapper refreshes CMD.EXE-*.pf between reboots, so multi-reboot sessions + // re-warned per restart. The scanner cannot distinguish a new human console from + // routine installer cmd churn via the artifact timestamp — one Warning per enrollment + // is the honest signal; the live ConsoleBypassWatcher covers consoles opened later. if (_startupGate != null - && !_startupGate.ShouldEmit( - Constants.EventTypes.ConsolePrefetchDetected, - Core.Persistence.StartupEventGate.ComputeFingerprint(data, excludedKeys: new[] { "bootTimeUtc" }))) + && !_startupGate.ShouldEmit(Constants.EventTypes.ConsolePrefetchDetected, "detected")) { - _logger.Debug($"{Name}: same cmd prefetch artifact already reported — suppressed (restart dedup)"); + _logger.Debug($"{Name}: console prefetch already reported this enrollment — suppressed (restart dedup)"); return; } @@ -158,6 +175,10 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Analyzers ImmediateUpload = true, Data = data, }); + + // M4: commit the gate claim only after the Warning went out — committing first let a + // crash in between suppress this security event for the rest of the enrollment. + _startupGate?.MarkEmitted(Constants.EventTypes.ConsolePrefetchDetected); } /// Newest (max last-write) artifact matching , or null. diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Analyzers/IntegrityBypassAnalyzer.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Analyzers/IntegrityBypassAnalyzer.cs index 70694954..b00d51d1 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Analyzers/IntegrityBypassAnalyzer.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Analyzers/IntegrityBypassAnalyzer.cs @@ -249,6 +249,10 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Analyzers Message = message, Data = data }); + + // M4: commit the gate claim only after the emission went out — committing first + // let a crash in between suppress the findings for the rest of the enrollment. + _startupGate?.MarkEmitted(Constants.EventTypes.IntegrityBypassAnalysis); } catch (Exception ex) { diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Analyzers/LocalAdminAnalyzer.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Analyzers/LocalAdminAnalyzer.cs index f8f60761..4274bfc9 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Analyzers/LocalAdminAnalyzer.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Analyzers/LocalAdminAnalyzer.cs @@ -234,6 +234,11 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Analyzers Message = message, Data = data }); + + // M4: commit the gate claim only after the emission went out — committing first + // let a crash in between suppress the findings for the rest of the enrollment. + if (trigger == "startup") + _startupGate?.MarkEmitted(Constants.EventTypes.LocalAdminAnalysis); } catch (Exception ex) { diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Analyzers/SoftwareInventoryAnalyzer.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Analyzers/SoftwareInventoryAnalyzer.cs index d7ee264b..9fdbadfe 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Analyzers/SoftwareInventoryAnalyzer.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Analyzers/SoftwareInventoryAnalyzer.cs @@ -111,6 +111,9 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Analyzers } EmitInventoryEvents("startup", EnrollmentPhase.Unknown, _startupInventory, null); + + // M4: commit the gate claim only after the (chunked) emission went out. + _startupGate?.MarkEmitted(Constants.EventTypes.SoftwareInventoryAnalysis + ":startup"); } catch (Exception ex) { diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/DeviceInfo/DeviceInfoCollector.NetworkAndSecurity.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/DeviceInfo/DeviceInfoCollector.NetworkAndSecurity.cs index 01a1b490..4563cb48 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/DeviceInfo/DeviceInfoCollector.NetworkAndSecurity.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/DeviceInfo/DeviceInfoCollector.NetworkAndSecurity.cs @@ -757,6 +757,10 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.DeviceInfo Message = message, Data = data }); + + // M4: commit the gate claim only after the emission went out (no-op for gate-exempt + // event types — they never claimed). + _startupGate?.MarkEmitted(eventType); } /// diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Periodic/PerformanceCollector.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Periodic/PerformanceCollector.cs index c465956c..50e379ac 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Periodic/PerformanceCollector.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Periodic/PerformanceCollector.cs @@ -26,9 +26,16 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Periodic // had no actionable signal. Emit a single disk_space_low Warning on the transition below the // threshold; re-arm only after free space recovers past a higher mark (hysteresis) so the // event stays state-change-only and never turns into a heartbeat. + // + // M3 (delta review 2026-07-02): the latch is seeded from + persisted via the + // StartupEventGate — a memory-only flag re-warned (ImmediateUpload) after every agent + // restart on a persistently disk-starved device (stall restarts: up to ~88×/session). private const double DiskLowThresholdGb = 2.0; private const double DiskRecoveryThresholdGb = 3.0; + private const string DiskLowFingerprint = "low"; + private const string DiskRearmedFingerprint = "rearmed"; private bool _diskLowWarned; + private readonly Core.Persistence.StartupEventGate _startupGate; // Network throughput tracking private string _activeNicId; @@ -39,9 +46,14 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Periodic private bool _networkInitialized; public PerformanceCollector(string sessionId, string tenantId, InformationalEventPost post, - AgentLogger logger, int intervalSeconds = 60) + AgentLogger logger, int intervalSeconds = 60, + Core.Persistence.StartupEventGate startupGate = null) : base(sessionId, tenantId, post, logger, intervalSeconds) { + _startupGate = startupGate; + // Seed the hysteresis latch from persisted state so a restart on a still-starved + // disk does not re-warn (read-only peek — no gate claim). + _diskLowWarned = _startupGate?.HasFingerprint(Constants.EventTypes.DiskSpaceLow, DiskLowFingerprint) ?? false; } /// Use a 5-second warm-up delay so counters are primed before first read. @@ -157,9 +169,14 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Periodic // MON-B10 — surface a dedicated Warning the moment the drive crosses below the // threshold, instead of leaving the only signal buried in this Debug snapshot. - if (EvaluateDiskLowTransition(freeGb, ref _diskLowWarned)) + if (EvaluateDiskLowTransition(freeGb, ref _diskLowWarned, out var rearmed)) { EmitDiskLowWarning(freeGb, totalGb, systemDrive); + PersistDiskLowState(DiskLowFingerprint); + } + else if (rearmed) + { + PersistDiskLowState(DiskRearmedFingerprint); } } catch (Exception ex) @@ -289,8 +306,9 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Periodic /// Current free space on the system drive, in GB. /// Per-collector latch: true while a low-disk warning is in /// effect. Updated in place to reflect the new armed state. - internal static bool EvaluateDiskLowTransition(double freeGb, ref bool alreadyWarned) + internal static bool EvaluateDiskLowTransition(double freeGb, ref bool alreadyWarned, out bool rearmed) { + rearmed = false; if (freeGb < DiskLowThresholdGb) { if (alreadyWarned) return false; // still low, already warned → stay quiet @@ -298,14 +316,28 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Periodic return true; } - if (freeGb >= DiskRecoveryThresholdGb) + if (freeGb >= DiskRecoveryThresholdGb && alreadyWarned) { alreadyWarned = false; // recovered with margin → re-arm for a future drop + rearmed = true; // caller persists the re-arm so it survives restarts (M3) } return false; } + /// + /// M3: records the disk-low latch state in the StartupEventGate so it survives agent + /// restarts. Uses the gate's claim+commit pair as a persisted state cell — there is no + /// separate event emission here (the Warning already went out for the "low" transition; + /// the re-arm transition is state-only by design). + /// + private void PersistDiskLowState(string fingerprint) + { + if (_startupGate == null) return; + if (_startupGate.ShouldEmit(Constants.EventTypes.DiskSpaceLow, fingerprint)) + _startupGate.MarkEmitted(Constants.EventTypes.DiskSpaceLow); + } + private void EmitDiskLowWarning(double freeGb, double totalGb, string drive) { var data = new Dictionary(capacity: 4, StringComparer.Ordinal) diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/DefaultComponentFactory.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/DefaultComponentFactory.cs index 2d307ac9..e6f680c6 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/DefaultComponentFactory.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/DefaultComponentFactory.cs @@ -313,7 +313,8 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration idleTimeoutMinutes: collectors.CollectorIdleTimeoutMinutes, networkMetrics: _networkMetrics, agentVersion: _agentVersion, - telemetrySpool: telemetrySpool)); + telemetrySpool: telemetrySpool, + startupGate: _startupEventGate)); // M3 — disk_space_low latch survives restarts } // V1 parity (CollectorCoordinator.StartOptionalCollectors:375-382) — wire the diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/Hosts/PeriodicCollectorLifecycleHost.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/Hosts/PeriodicCollectorLifecycleHost.cs index a1b1b674..71c1656d 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/Hosts/PeriodicCollectorLifecycleHost.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/Hosts/PeriodicCollectorLifecycleHost.cs @@ -54,6 +54,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration private readonly NetworkMetrics? _networkMetrics; private readonly string _agentVersion; private readonly ITelemetrySpool? _telemetrySpool; + private readonly Persistence.StartupEventGate? _startupGate; // Codex Finding 4 — reference to the concrete SignalIngress (when available) so we // can subscribe to its SignalPosted event. This lets us observe activity from @@ -86,7 +87,8 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration int idleTimeoutMinutes, NetworkMetrics? networkMetrics, string agentVersion, - ITelemetrySpool? telemetrySpool = null) + ITelemetrySpool? telemetrySpool = null, + Persistence.StartupEventGate? startupGate = null) { if (ingress == null) throw new ArgumentNullException(nameof(ingress)); if (clock == null) throw new ArgumentNullException(nameof(clock)); @@ -101,6 +103,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration _networkMetrics = networkMetrics; _agentVersion = agentVersion; _telemetrySpool = telemetrySpool; + _startupGate = startupGate; _lastRealEventTimeUtc = DateTime.UtcNow; // Post goes to the raw ingress — no per-host wrapping. Activity observation @@ -184,7 +187,8 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration if (_perfEnabled && _performanceCollector == null) { _performanceCollector = new PerformanceCollector( - _sessionId, _tenantId, _post, _logger, _perfIntervalSeconds); + _sessionId, _tenantId, _post, _logger, _perfIntervalSeconds, + startupGate: _startupGate); // M3 — disk_space_low latch survives restarts _performanceCollector.Start(); } if (_selfMetricsEnabled && _selfMetricsCollector == null && _networkMetrics != null) diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Persistence/StartupEventGate.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Persistence/StartupEventGate.cs index 4edebc90..9f74b493 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Persistence/StartupEventGate.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Persistence/StartupEventGate.cs @@ -40,23 +40,36 @@ namespace AutopilotMonitor.Agent.V2.Core.Persistence private readonly string _stateFilePath; private readonly AgentLogger _logger; private readonly object _lock = new object(); + + // M4 (delta review 2026-07-02): two views. `_entries` is the in-process truth that + // ShouldEmit claims against (immediate, so concurrent/repeated in-process checks stay + // deduped). `_persisted` is what actually reaches disk — a key is copied over only by + // MarkEmitted/MarkSucceeded, i.e. AFTER the caller's emission went through. Persisting + // inside ShouldEmit meant a process death between the fingerprint save and the spool + // append suppressed that event for the rest of the enrollment (fail-closed on telemetry, + // permanent for static payloads like tpm_info or console_prefetch_detected). private readonly Dictionary _entries; + private readonly Dictionary _persisted; public StartupEventGate(string stateDirectory, AgentLogger logger) { _stateDirectory = Environment.ExpandEnvironmentVariables(stateDirectory); _stateFilePath = Path.Combine(_stateDirectory, "startup-event-state.json"); _logger = logger; - _entries = LoadSafe(); + _persisted = LoadSafe(); + _entries = new Dictionary(_persisted, StringComparer.Ordinal); } public string StateFilePath => _stateFilePath; /// /// Emit-on-change: true when no fingerprint is recorded for or the - /// recorded one differs — the new fingerprint is persisted in the same call, so the caller - /// emits exactly when this returns true. False means "identical payload already emitted in - /// this session" (possibly by a previous agent run). + /// recorded one differs. The new fingerprint is claimed IN MEMORY only — the caller must + /// invoke after the emission actually went out, which is what + /// persists the claim across restarts. A crash (or an emit failure) between the two calls + /// therefore re-emits on the next run instead of silently suppressing forever. + /// False means "identical payload already emitted in this session" (possibly by a + /// previous agent run). /// public bool ShouldEmit(string key, string fingerprint) { @@ -75,11 +88,42 @@ namespace AutopilotMonitor.Agent.V2.Core.Persistence Succeeded = existing?.Succeeded ?? false, UpdatedUtc = DateTime.UtcNow, }; - SaveSafe(); return true; } } + /// + /// Commits the in-memory claim made by the preceding to disk. + /// Call after the event emission succeeded. No-op when the key was never claimed + /// (e.g. gate-exempt event types). + /// + public void MarkEmitted(string key) + { + if (string.IsNullOrEmpty(key)) return; + lock (_lock) + { + if (!_entries.TryGetValue(key, out var entry)) return; + _persisted[key] = entry; + SaveSafe(); + } + } + + /// + /// Read-only peek: true when the key's current fingerprint (possibly restored from a + /// previous agent run) equals . Unlike + /// this makes NO claim — use it to seed in-memory latches + /// (e.g. the disk_space_low hysteresis) from persisted state at construction time. + /// + public bool HasFingerprint(string key, string fingerprint) + { + if (string.IsNullOrEmpty(key)) return false; + lock (_lock) + { + return _entries.TryGetValue(key, out var entry) + && string.Equals(entry.Fingerprint, fingerprint, StringComparison.Ordinal); + } + } + /// Retry-until-success: true when latched this key in any run. public bool AlreadySucceeded(string key) { @@ -90,19 +134,25 @@ namespace AutopilotMonitor.Agent.V2.Core.Persistence } } - /// Latches a retry-until-success key; subsequent runs skip the probe entirely. + /// + /// Latches a retry-until-success key; subsequent runs skip the probe entirely. + /// Persists immediately — success latches happen AFTER the probe's emission by contract + /// (see StartupEnvironmentProbes), so there is no suppression window here. + /// public void MarkSucceeded(string key) { if (string.IsNullOrEmpty(key)) return; lock (_lock) { _entries.TryGetValue(key, out var existing); - _entries[key] = new GateEntry + var entry = new GateEntry { Fingerprint = existing?.Fingerprint, Succeeded = true, UpdatedUtc = DateTime.UtcNow, }; + _entries[key] = entry; + _persisted[key] = entry; SaveSafe(); } } @@ -170,7 +220,9 @@ namespace AutopilotMonitor.Agent.V2.Core.Persistence { Directory.CreateDirectory(_stateDirectory); var tempPath = _stateFilePath + ".tmp"; - File.WriteAllText(tempPath, JsonConvert.SerializeObject(_entries)); + // Persist the committed view only — uncommitted ShouldEmit claims of OTHER keys + // must not ride along, or their emissions could be suppressed by a crash too. + File.WriteAllText(tempPath, JsonConvert.SerializeObject(_persisted)); if (File.Exists(_stateFilePath)) { File.Replace(tempPath, _stateFilePath, destinationBackupFileName: null, ignoreMetadataErrors: true); diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Runtime/StartupEnvironmentProbes.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Runtime/StartupEnvironmentProbes.cs index f55e48ea..b251b654 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Runtime/StartupEnvironmentProbes.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Runtime/StartupEnvironmentProbes.cs @@ -116,24 +116,36 @@ namespace AutopilotMonitor.Agent.V2.Core.Runtime if (gate == null || !gate.AlreadySucceeded(Constants.EventTypes.DeviceLocation)) { SafeEmit(post, logger, BuildGeoEvent(configuration, attempt.Location)); - - // Outbound (public egress) IP — Trace severity so it stays out of the - // timeline by default; persisted/queryable for network correlation. - if (!string.IsNullOrEmpty(attempt.Location.Ip)) - SafeEmit(post, logger, BuildOutboundIpEvent(configuration, attempt.Location)); - gate?.MarkSucceeded(Constants.EventTypes.DeviceLocation); } else { logger.Debug("StartupEnvironmentProbes: location re-resolved for the pending timezone retry — device_location already emitted in a previous run."); } + + // Outbound (public egress) IP — Trace severity so it stays out of the timeline by + // default; persisted/queryable for network correlation. Own latch (L17): the first + // successful provider response can lack the ip field, so it must not ride the + // device_location latch — any later lookup (e.g. a timezone retry) that does carry + // it still emits the trace once. + if (!string.IsNullOrEmpty(attempt.Location.Ip) + && (gate == null || !gate.AlreadySucceeded(Constants.EventTypes.OutboundIp))) + { + SafeEmit(post, logger, BuildOutboundIpEvent(configuration, attempt.Location)); + gate?.MarkSucceeded(Constants.EventTypes.OutboundIp); + } } else { - // Failure event each failing start is intentional — it documents the retry trail, - // and the unset gate key makes the next agent run try again. - SafeEmit(post, logger, BuildGeoFailureEvent(configuration, attempt)); + // Failure event each failing start is intentional (retry trail) — but only while + // the location itself is still outstanding. When a prior run already emitted + // device_location and this lookup ran solely to feed the pending timezone retry, + // a failure event here would put a device_location failure AFTER its success on + // the session timeline (L17) — misleading noise, suppressed. + if (gate == null || !gate.AlreadySucceeded(Constants.EventTypes.DeviceLocation)) + SafeEmit(post, logger, BuildGeoFailureEvent(configuration, attempt)); + else + logger.Debug("StartupEnvironmentProbes: geo re-lookup for the pending timezone retry failed — failure event suppressed (location already reported)."); } if (configuration.EnableTimezoneAutoSet && !string.IsNullOrEmpty(attempt?.Location?.Timezone))