diff --git a/docs/log.md b/docs/log.md index 73bdac17..6ac8bba9 100644 --- a/docs/log.md +++ b/docs/log.md @@ -1,5 +1,9 @@ # Log +## 2026-08-29 (11) + +* **Update**: `rules/gather-rule-phase-scoping.md` — `on_event` feedback-loop guard: `TimelineEventStream` publishes the event source and `GatherRuleExecutorHost` never dispatches `on_event` for gather-emitted events (a rule triggering on its own output type looped for the whole session); per-rule on_event execution cap (100/session); backend rejects `on_event` rules whose trigger equals their output type or `gather_result`. + ## 2026-08-29 (10) * **Update**: `backend/tenant-activation-welcome-mail.md` — welcome/farewell mails HTML-encode the domain at substitution; `DomainName` is frozen on the full-model config PUT (was persisted verbatim) and the first-login seed only accepts a strict DNS host name. diff --git a/docs/rules/gather-rule-phase-scoping.md b/docs/rules/gather-rule-phase-scoping.md index 953b2541..20d418de 100644 --- a/docs/rules/gather-rule-phase-scoping.md +++ b/docs/rules/gather-rule-phase-scoping.md @@ -8,7 +8,7 @@ tags: - backend - gather-rules - config -timestamp: 2026-08-04T00:00:00+02:00 +timestamp: 2026-08-29T00:00:00+02:00 --- # Problem @@ -89,6 +89,18 @@ Consequences of the emitted-event feed: the raw signal stream — the documented "On Event with `enrollment_complete`" end-of-enrollment pattern only works because of this. This restores V1 semantics (V1 fired triggers from every emitted `EnrollmentEvent`). +* **Feedback-loop guard** (2026-08-29): gather output travels the same post → ingress → + reducer → emitter → stream path, so the stream would hand the executor's own events straight + back to `OnEvent`. A rule with `triggerEventType == outputEventType` (or two rules chained + A→B→A) then executed and emitted without bound for the whole session — `emitMode: + on_change` does not help when the collected result varies. `TimelineEventStream` therefore + publishes the event **source**, and `GatherRuleExecutorHost` never dispatches `on_event` for + `Source == GatherRuleExecutor.SourceName` (phase declarations on such events are still + honoured). Second layer: `GatherRuleExecutor.MaxOnEventExecutionsPerRule` (100 per rule and + session) bounds any cycle that reaches the executor through a non-gather event; the boundary + is logged once (agent log Warning + debug-trace `trigger` line). Backend validation rejects + `on_event` rules whose trigger equals their own output type or `gather_result` — such a rule + could never fire. * Other hosts (`DeviceInfoHost`, `UserEspKeepAwakeHost`, `ProvisioningPackageHost`) still key off the raw signal stream deliberately — their collections are about OS state at the raw boundary (ESP exit, keep-awake), not about timeline agreement. @@ -103,7 +115,8 @@ Consequences of the emitted-event feed: `_startupRulesExecuted` dedup; it is not part of the `WaitForStartupRules` latch). * `phase_change` — evaluated against the **new** phase; the scope gate runs before the per-(rule, phase) dedup so an out-of-scope pass does not consume the execution slot. - * `on_event` — gated by the current phase. + * `on_event` — gated by the current phase; never fires on gather output; capped at + `MaxOnEventExecutionsPerRule` per rule and session. * **Before the first phase signal** of a session (`_currentPhase == Unknown`) scoped rules are inactive. Sessions that never produce phase signals never run them (documented behavior). * **From-phase latch**: activates when `phase != Unknown && phase != Failed && (int)phase >= @@ -164,4 +177,4 @@ Consequences of the emitted-event feed: * `src/Backend/AutopilotMonitor.Functions/Functions/Rules/GatherRulesFunction.cs` — `ValidateScopeAndEmitMode` * `src/Backend/AutopilotMonitor.Functions/Services/GatherRuleService.cs` — partial-PUT merge, `ContentEquivalent` * `src/Shared/AutopilotMonitor.Shared/Models/Rules/GatherRule.cs` — field contracts -* Tests: `GatherRuleExecutorPhaseScopeTests`, `GatherRuleExecutorEmitModeTests`, `GatherRuleExecutorHostTimelineFeedTests` (agent); `GatherRuleUpdatePartialMergeTests`, `GatherRuleScopeFieldsTests` (backend) +* Tests: `GatherRuleExecutorPhaseScopeTests`, `GatherRuleExecutorEmitModeTests`, `GatherRuleExecutorHostTimelineFeedTests`, `GatherRuleExecutorOnEventCapTests` (agent); `GatherRuleUpdatePartialMergeTests`, `GatherRuleScopeFieldsTests` (backend) diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Gather/GatherRuleExecutorOnEventCapTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Gather/GatherRuleExecutorOnEventCapTests.cs new file mode 100644 index 00000000..6818af2a --- /dev/null +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Monitoring/Gather/GatherRuleExecutorOnEventCapTests.cs @@ -0,0 +1,107 @@ +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Threading; +using AutopilotMonitor.Agent.V2.Core.Logging; +using AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather; +using AutopilotMonitor.Agent.V2.Core.Tests.Harness; +using AutopilotMonitor.Shared.Models; +using Xunit; + +namespace AutopilotMonitor.Agent.V2.Core.Tests.Monitoring.Gather +{ + /// + /// on_event has no once-per-(rule, phase) dedup like the phase triggers. The per-rule + /// execution cap () bounds any + /// trigger cycle that survives the host's source guard (e.g. two rules chained through an + /// intermediate non-gather event) so a mis-authored rule can never spin for a whole session. + /// Executions are observed through the emitted events; the rule uses a registry path that + /// exists on every Windows box so each execution emits exactly one event. + /// + [Collection("SerialThreading")] // rules execute on the shared ThreadPool + public sealed class GatherRuleExecutorOnEventCapTests : IDisposable + { + private readonly TempDirectory _tmp = new TempDirectory(); + private readonly List _events = new List(); + private readonly object _gate = new object(); + private readonly GatherRuleExecutor _executor; + + public GatherRuleExecutorOnEventCapTests() + { + var logger = new AgentLogger(_tmp.Path, AgentLogLevel.Info); + _executor = new GatherRuleExecutor("sess", "tenant", + evt => { lock (_gate) _events.Add(evt); }, logger) + { + UnrestrictedMode = true + }; + } + + public void Dispose() + { + _executor.Dispose(); + _tmp.Dispose(); + } + + private int EventCount { get { lock (_gate) return _events.Count; } } + + private bool WaitForEventCount(int count, int timeoutMs = 20000) + { + var sw = Stopwatch.StartNew(); + while (sw.ElapsedMilliseconds < timeoutMs) + { + if (EventCount >= count) return true; + Thread.Sleep(25); + } + return EventCount >= count; + } + + private static GatherRule OnEventRule(string id, string triggerEventType) => new GatherRule + { + RuleId = id, + Title = id, + CollectorType = "registry", + Target = "HKLM\\SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion", + Trigger = "on_event", + TriggerEventType = triggerEventType, + OutputEventType = "gather_cap_test", + Enabled = true, + }; + + [Fact] + public void OnEvent_ExecutesAtMostMaxOnEventExecutionsPerRule_PerSession() + { + _executor.UpdateRules(new List { OnEventRule("GATHER-CAP-001", "ext_event") }); + + var cap = GatherRuleExecutor.MaxOnEventExecutionsPerRule; + for (var i = 0; i < cap + 25; i++) + _executor.OnEvent("ext_event"); + + Assert.True(WaitForEventCount(cap), $"expected {cap} executions, got {EventCount}"); + Thread.Sleep(500); + Assert.Equal(cap, EventCount); + + // Further triggers stay ignored for the rest of the session. + _executor.OnEvent("ext_event"); + Thread.Sleep(300); + Assert.Equal(cap, EventCount); + } + + [Fact] + public void OnEventCap_IsPerRule_NotGlobal() + { + _executor.UpdateRules(new List + { + OnEventRule("GATHER-CAP-002", "ext_event"), + OnEventRule("GATHER-CAP-003", "ext_event"), + }); + + var cap = GatherRuleExecutor.MaxOnEventExecutionsPerRule; + for (var i = 0; i < cap + 5; i++) + _executor.OnEvent("ext_event"); + + Assert.True(WaitForEventCount(2 * cap), $"expected {2 * cap} executions, got {EventCount}"); + Thread.Sleep(500); + Assert.Equal(2 * cap, EventCount); + } + } +} diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Orchestration/GatherRuleExecutorHostTimelineFeedTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Orchestration/GatherRuleExecutorHostTimelineFeedTests.cs index ccf29d73..744bc7f4 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Orchestration/GatherRuleExecutorHostTimelineFeedTests.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Orchestration/GatherRuleExecutorHostTimelineFeedTests.cs @@ -122,13 +122,13 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Orchestration // The raw-world sequence of session 32312a32 as it reaches the emitted timeline: // esp_exiting and hello_wizard_started are emitted WITHOUT a phase declaration // (the engine holds FinalizingSetup back behind the RealmJoin gate) — no fire. - _stream.Publish("esp_exiting", EnrollmentPhase.Unknown); - _stream.Publish("hello_wizard_started", EnrollmentPhase.Unknown); + _stream.Publish("esp_exiting", EnrollmentPhase.Unknown, "Engine"); + _stream.Publish("hello_wizard_started", EnrollmentPhase.Unknown, "Engine"); Thread.Sleep(300); Assert.Equal(0, _sink.CountOf("gather_host_test")); // Only the engine's actual phase declaration fires the rule. - _stream.Publish("phase_transition", EnrollmentPhase.FinalizingSetup); + _stream.Publish("phase_transition", EnrollmentPhase.FinalizingSetup, "Engine"); Assert.True(WaitForCount("gather_host_test", 1)); } @@ -140,7 +140,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Orchestration // enrollment_complete" trigger was dead under the previous SignalPosted feed. BuildHost(Rule("GATHER-HOST-002", "on_event", triggerEventType: "enrollment_complete")); - _stream.Publish("enrollment_complete", EnrollmentPhase.Unknown); + _stream.Publish("enrollment_complete", EnrollmentPhase.Unknown, "Engine"); Assert.True(WaitForCount("gather_host_test", 1)); } @@ -151,14 +151,14 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Orchestration // identical phase (e.g. duplicate esp_phase_changed) must not count as one. BuildHost(Rule("GATHER-HOST-003", "phase_change", triggerPhase: "")); - _stream.Publish("esp_phase_changed", EnrollmentPhase.DeviceSetup); + _stream.Publish("esp_phase_changed", EnrollmentPhase.DeviceSetup, "Engine"); Assert.True(WaitForCount("gather_host_test", 1)); - _stream.Publish("esp_phase_changed", EnrollmentPhase.DeviceSetup); + _stream.Publish("esp_phase_changed", EnrollmentPhase.DeviceSetup, "Engine"); Thread.Sleep(300); Assert.Equal(1, _sink.CountOf("gather_host_test")); - _stream.Publish("esp_phase_changed", EnrollmentPhase.AccountSetup); + _stream.Publish("esp_phase_changed", EnrollmentPhase.AccountSetup, "Engine"); Assert.True(WaitForCount("gather_host_test", 2)); } @@ -167,11 +167,44 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Orchestration { BuildHost(Rule("GATHER-HOST-004", "phase_exit", triggerPhase: "AccountSetup")); - _stream.Publish("esp_phase_changed", EnrollmentPhase.AccountSetup); + _stream.Publish("esp_phase_changed", EnrollmentPhase.AccountSetup, "Engine"); Thread.Sleep(300); Assert.Equal(0, _sink.CountOf("gather_host_test")); - _stream.Publish("phase_transition", EnrollmentPhase.FinalizingSetup); + _stream.Publish("phase_transition", EnrollmentPhase.FinalizingSetup, "Engine"); + Assert.True(WaitForCount("gather_host_test", 1)); + } + + [Fact] + public void OnEventRule_NeverFiresOnGatherExecutorOutput_BreaksSelfTriggerLoop() + { + // A rule whose TriggerEventType equals its own OutputEventType: the event it emits + // travels post → ingress → reducer → emitter → stream and comes back here with + // Source == GatherRuleExecutor. Without the source guard that re-enters OnEvent and + // the rule loops for the whole session (execute → emit → trigger → execute …). + BuildHost(Rule("GATHER-HOST-006", "on_event", triggerEventType: "gather_host_test")); + + _stream.Publish("gather_host_test", EnrollmentPhase.Unknown, + Core.Monitoring.Telemetry.Gather.GatherRuleExecutor.SourceName); + Thread.Sleep(300); + Assert.Equal(0, _sink.CountOf("gather_host_test")); + + // The same event type from any other origin still triggers exactly once per event. + _stream.Publish("gather_host_test", EnrollmentPhase.Unknown, "Engine"); + Assert.True(WaitForCount("gather_host_test", 1)); + Thread.Sleep(300); + Assert.Equal(1, _sink.CountOf("gather_host_test")); + } + + [Fact] + public void PhaseDeclarationOnGatherSourcedEvent_StillDrivesPhaseTriggers() + { + // The source guard only suppresses on_event dispatch; a phase declaration is a + // timeline fact regardless of which component carried it. + BuildHost(Rule("GATHER-HOST-007", "phase_change", triggerPhase: "DeviceSetup")); + + _stream.Publish("gather_host_test", EnrollmentPhase.DeviceSetup, + Core.Monitoring.Telemetry.Gather.GatherRuleExecutor.SourceName); Assert.True(WaitForCount("gather_host_test", 1)); } @@ -181,7 +214,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Orchestration BuildHost(Rule("GATHER-HOST-005", "on_event", triggerEventType: "enrollment_complete")); _host!.Stop(); - _stream.Publish("enrollment_complete", EnrollmentPhase.Unknown); + _stream.Publish("enrollment_complete", EnrollmentPhase.Unknown, "Engine"); Thread.Sleep(300); Assert.Equal(0, _sink.CountOf("gather_host_test")); } diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Telemetry/Events/EventTimelineEmitterTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Telemetry/Events/EventTimelineEmitterTests.cs index 9823fbba..031724ea 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Telemetry/Events/EventTimelineEmitterTests.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Telemetry/Events/EventTimelineEmitterTests.cs @@ -161,7 +161,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Telemetry.Events using var r = new Rig(); var stream = new AutopilotMonitor.Agent.V2.Core.Orchestration.TimelineEventStream(); var published = new List<(string EventType, EnrollmentPhase Phase)>(); - stream.EventEmitted += (eventType, phase) => published.Add((eventType, phase)); + stream.EventEmitted += (eventType, phase, _) => published.Add((eventType, phase)); var sut = new EventTimelineEmitter(r.Inner, stream); sut.Emit( @@ -190,8 +190,8 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Telemetry.Events using var r = new Rig(); var stream = new AutopilotMonitor.Agent.V2.Core.Orchestration.TimelineEventStream(); var secondSubscriberCalls = 0; - stream.EventEmitted += (_, _) => throw new InvalidOperationException("subscriber bug"); - stream.EventEmitted += (_, _) => secondSubscriberCalls++; + stream.EventEmitted += (_, _, _) => throw new InvalidOperationException("subscriber bug"); + stream.EventEmitted += (_, _, _) => secondSubscriberCalls++; var sut = new EventTimelineEmitter(r.Inner, stream); sut.Emit( diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Gather/Collectors/CommandCollector.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Gather/Collectors/CommandCollector.cs index cb5c88e9..a26d243e 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Gather/Collectors/CommandCollector.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Gather/Collectors/CommandCollector.cs @@ -53,7 +53,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather.Collectors Timestamp = DateTime.UtcNow, EventType = Constants.EventTypes.SecurityWarning, Severity = EventSeverity.Warning, - Source = "GatherRuleExecutor", + Source = GatherRuleExecutor.SourceName, Message = $"Blocked command not on allowlist: {command} (Rule: {rule.RuleId})", Data = data }); diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Gather/Collectors/LogParserCollector.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Gather/Collectors/LogParserCollector.cs index 94cc8c97..a9d8051b 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Gather/Collectors/LogParserCollector.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Gather/Collectors/LogParserCollector.cs @@ -291,7 +291,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather.Collectors Timestamp = DateTime.UtcNow, EventType = eventType, Severity = severity, - Source = "GatherRuleExecutor", + Source = GatherRuleExecutor.SourceName, Message = $"Gather: {rule.Title}", Data = data }); @@ -360,7 +360,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather.Collectors Timestamp = entryUtc, EventType = eventType, Severity = severity, - Source = "GatherRuleExecutor", + Source = GatherRuleExecutor.SourceName, Message = $"Gather: {rule.Title}", Data = data }); diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Gather/GatherRuleContext.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Gather/GatherRuleContext.cs index 3b7ced93..23eca47f 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Gather/GatherRuleContext.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Gather/GatherRuleContext.cs @@ -76,7 +76,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather Timestamp = DateTime.UtcNow, EventType = Constants.EventTypes.SecurityWarning, Severity = EventSeverity.Warning, - Source = "GatherRuleExecutor", + Source = GatherRuleExecutor.SourceName, Message = $"Blocked {collectorType} target not on allowlist: {target} (Rule: {rule.RuleId})", Data = data }); diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Gather/GatherRuleExecutor.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Gather/GatherRuleExecutor.cs index b0dbb510..731fae2d 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Gather/GatherRuleExecutor.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Monitoring/Telemetry/Gather/GatherRuleExecutor.cs @@ -21,6 +21,21 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather /// public class GatherRuleExecutor : IDisposable { + /// + /// label stamped on every event the gather pipeline + /// emits (executor, context guard events, collectors). The timeline feed uses it to keep + /// gather output from re-triggering on_event rules. + /// + public const string SourceName = "GatherRuleExecutor"; + + /// + /// Upper bound on on_event executions per rule and session. on_event has no natural + /// once-per-(rule, phase) dedup like the phase triggers; this cap bounds any remaining + /// cycle (e.g. two rules chained through an intermediate non-gather event) and any + /// pathological trigger type that fires on every timeline entry. + /// + internal const int MaxOnEventExecutionsPerRule = 100; + private readonly AgentLogger _logger; private readonly GatherRuleDebugLog _debug; // null unless EnableGatherRuleDebugLog / --gather-debug-log private readonly GatherRuleContext _context; @@ -30,6 +45,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather private readonly Dictionary _intervalTimers = new Dictionary(); private readonly HashSet _startupRulesExecuted = new HashSet(); private readonly HashSet _phaseRulesExecuted = new HashSet(); + private readonly Dictionary _onEventExecutions = new Dictionary(); private CountdownEvent _startupRulesLatch; // non-null only while startup rules are pending // Phase-scope + emit-mode state (ActivePhases / ActiveFromPhase / EmitMode on GatherRule). @@ -394,6 +410,27 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather continue; } + // Per-rule execution cap (see MaxOnEventExecutionsPerRule). The counter is + // bumped once past the cap so the boundary is logged exactly once. + int executions; + lock (_scopeLock) + { + _onEventExecutions.TryGetValue(rule.RuleId, out executions); + if (executions <= MaxOnEventExecutionsPerRule) + _onEventExecutions[rule.RuleId] = executions + 1; + } + + if (executions >= MaxOnEventExecutionsPerRule) + { + if (executions == MaxOnEventExecutionsPerRule) + { + _logger.Warning($"Gather rule {rule.RuleId} reached the on_event execution cap ({MaxOnEventExecutionsPerRule}) for this session — further {eventType} triggers are ignored"); + DebugLog(rule.RuleId, GatherRuleDebugLog.StageTrigger, + $"on_event {eventType} ignored: per-rule execution cap ({MaxOnEventExecutionsPerRule}) reached for this session"); + } + continue; + } + _logger.Info($"Event triggered rule {rule.RuleId} (event: {eventType})"); DebugLog(rule.RuleId, GatherRuleDebugLog.StageTrigger, $"on_event {eventType} fired"); ThreadPool.QueueUserWorkItem(_ => ExecuteRule(rule)); @@ -482,7 +519,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Monitoring.Telemetry.Gather Timestamp = DateTime.UtcNow, EventType = eventType, Severity = severity, - Source = "GatherRuleExecutor", + Source = SourceName, Message = $"Gather: {rule.Title}", Data = result }); diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/Hosts/GatherRuleExecutorHost.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/Hosts/GatherRuleExecutorHost.cs index ead5da24..6c25049b 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/Hosts/GatherRuleExecutorHost.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/Hosts/GatherRuleExecutorHost.cs @@ -33,7 +33,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration // the enrollment reaches this phase"). Null on test fakes / standalone configurations — // then phase/event triggers degrade off (startup + interval rules still run). private readonly TimelineEventStream? _timelineEvents; - private Action? _timelineHandler; + private Action? _timelineHandler; private readonly object _sync = new object(); private EnrollmentPhase _lastPhase = EnrollmentPhase.Unknown; @@ -101,8 +101,17 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration /// realmjoin_resolved, …) that never existed on the raw signal stream. The executor /// dispatches rule execution on the ThreadPool, so this stays off the ingress worker's /// effect path; it also dedups phase rules per (rule, phase). + /// + /// Feedback-loop guard: events the gather executor emitted itself (Source == + /// ) travel the + /// same post → ingress → reducer → emitter → stream path and would otherwise re-enter + /// OnEvent. A rule whose TriggerEventType equals its own OutputEventType (or two + /// rules chained A→B→A) then executes and emits without bound for the whole session. + /// Gather output therefore never dispatches on_event rules; phase declarations are still + /// honoured (gather events carry Phase=Unknown, so this is a no-op in practice). + /// /// - private void OnTimelineEventEmitted(string eventType, EnrollmentPhase phase) + private void OnTimelineEventEmitted(string eventType, EnrollmentPhase phase, string source) { try { @@ -114,7 +123,8 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration _executor.OnPhaseChanged(phase); } - if (!string.IsNullOrEmpty(eventType)) + if (!string.IsNullOrEmpty(eventType) + && !string.Equals(source, Monitoring.Telemetry.Gather.GatherRuleExecutor.SourceName, StringComparison.Ordinal)) { _executor.OnEvent(eventType); } diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/TimelineEventStream.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/TimelineEventStream.cs index 7b6daa3b..db65a979 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/TimelineEventStream.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Orchestration/TimelineEventStream.cs @@ -33,18 +33,21 @@ namespace AutopilotMonitor.Agent.V2.Core.Orchestration /// Raised once per emitted timeline event with its /// and parsed ( /// for the vast majority of events — only phase-declaration events carry a real phase, - /// which is exactly the set the UI timeline groups on). + /// which is exactly the set the UI timeline groups on), plus the originating + /// component label. The source lets subscribers that + /// themselves emit timeline events (the gather-rule executor) recognise their own output + /// and not re-trigger on it — see . /// - public event Action? EventEmitted; + public event Action? EventEmitted; - internal void Publish(string eventType, EnrollmentPhase phase) + internal void Publish(string eventType, EnrollmentPhase phase, string source) { var handler = EventEmitted; if (handler == null) return; - foreach (Action subscriber in handler.GetInvocationList()) + foreach (Action subscriber in handler.GetInvocationList()) { - try { subscriber(eventType, phase); } + try { subscriber(eventType, phase, source); } catch { // Subscriber isolation — see class doc: the emit path must never throw. diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Telemetry/Events/EventTimelineEmitter.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Telemetry/Events/EventTimelineEmitter.cs index 74a397f6..635efec7 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Telemetry/Events/EventTimelineEmitter.cs +++ b/src/Agent/AutopilotMonitor.Agent.V2.Core/Telemetry/Events/EventTimelineEmitter.cs @@ -148,7 +148,7 @@ namespace AutopilotMonitor.Agent.V2.Core.Telemetry.Events // Post-reduce observer surface: published AFTER the transport enqueue so subscribers // (gather-rule phase/event triggers) see exactly the ordering that reaches the wire. // Publish never throws (subscriber isolation inside the stream). - _timelineEvents?.Publish(evt.EventType, evt.Phase); + _timelineEvents?.Publish(evt.EventType, evt.Phase, evt.Source); } /// diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/GatherRuleScopeFieldsTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/GatherRuleScopeFieldsTests.cs index 0ee44979..ff6b3a2d 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/GatherRuleScopeFieldsTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/GatherRuleScopeFieldsTests.cs @@ -240,6 +240,45 @@ public class GatherRuleScopeFieldsTests ValidationRule(activePhases: new List { "" }))); } + private static GatherRule OnEventRule(string triggerEventType, string? outputEventType) + => new GatherRule + { + RuleId = "GATHER-VAL-EVT", + CollectorType = "registry", + Target = "HKLM\\SOFTWARE\\Contoso", + Trigger = "on_event", + TriggerEventType = triggerEventType, + OutputEventType = outputEventType!, + }; + + [Theory] + [InlineData("gather_proxy", "gather_proxy")] // self-cycle + [InlineData("GATHER_PROXY", "gather_proxy")] // case-insensitive (agent matches OrdinalIgnoreCase) + [InlineData("gather_result", null)] // self-cycle through the default output type + [InlineData("gather_result", "")] + [InlineData("gather_result", "gather_other")] // gather output namespace never triggers + public void Validate_RejectsOnEventRuleTriggeringOnGatherOutput(string trigger, string? output) + { + Assert.NotNull(GatherRulesFunction.ValidateScopeAndEmitMode(OnEventRule(trigger, output))); + } + + [Theory] + [InlineData("enrollment_complete", "gather_proxy")] + [InlineData("app_install_failed", null)] + [InlineData("", "gather_proxy")] // partial/toggle payload passes through + public void Validate_AcceptsOnEventRuleOnNonGatherEvents(string trigger, string? output) + { + Assert.Null(GatherRulesFunction.ValidateScopeAndEmitMode(OnEventRule(trigger, output))); + } + + [Fact] + public void Validate_OutputEqualsTrigger_IsOnlyCheckedForOnEventTrigger() + { + var rule = OnEventRule("gather_same", "gather_same"); + rule.Trigger = "startup"; + Assert.Null(GatherRulesFunction.ValidateScopeAndEmitMode(rule)); + } + [Fact] public void Validate_RejectsBothScopeFieldsSet() { diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Rules/GatherRulesFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Rules/GatherRulesFunction.cs index e48619a1..a810c6a9 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Rules/GatherRulesFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Rules/GatherRulesFunction.cs @@ -1,6 +1,7 @@ using System.Net; using AutopilotMonitor.Functions.Helpers; using AutopilotMonitor.Functions.Services; +using AutopilotMonitor.Shared; using AutopilotMonitor.Shared.Models; using Microsoft.Azure.Functions.Worker; using Microsoft.Azure.Functions.Worker.Http; @@ -167,12 +168,33 @@ namespace AutopilotMonitor.Functions.Functions.Rules }; /// - /// Validates the phase-scope + emit-mode fields on create/update. - /// Returns an error message for a 400 response, or null when valid. + /// Validates the phase-scope + emit-mode fields and the on_event trigger wiring on + /// create/update. Returns an error message for a 400 response, or null when valid. /// Toggle-style partial payloads carry none of these fields and pass through. + /// + /// on_event: the agent never dispatches on_event rules for events the gather executor + /// itself emitted (feedback-loop guard, GatherRuleExecutorHost), so a rule that + /// triggers on its own output — or on the gather_result default output type — + /// could never fire and is rejected here with an explanatory message instead of being + /// stored as a dead rule. + /// /// internal static string? ValidateScopeAndEmitMode(GatherRule rule) { + if (string.Equals(rule.Trigger, "on_event", StringComparison.OrdinalIgnoreCase) + && !string.IsNullOrEmpty(rule.TriggerEventType)) + { + var outputEventType = string.IsNullOrEmpty(rule.OutputEventType) + ? Constants.EventTypes.GatherResult + : rule.OutputEventType; + + if (string.Equals(rule.TriggerEventType, outputEventType, StringComparison.OrdinalIgnoreCase)) + return "triggerEventType must differ from the rule's own outputEventType — gather rules never trigger on their own output."; + + if (string.Equals(rule.TriggerEventType, Constants.EventTypes.GatherResult, StringComparison.OrdinalIgnoreCase)) + return $"triggerEventType '{Constants.EventTypes.GatherResult}' is the gather-rule output type — gather rules never trigger on gather output."; + } + var hasActivePhases = rule.ActivePhases != null && rule.ActivePhases.Count > 0; var hasFromPhase = !string.IsNullOrEmpty(rule.ActiveFromPhase);