From cff765f544aa59d56c5d8805336f8985eddb80ca Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Tue, 28 Apr 2026 22:16:11 +0200 Subject: [PATCH] fix(analyze): break self-referencing loop in correlation allMatches list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previous fix slimmed evidence + added a size guard, deployed (47ffb348), did NOT solve the real problem. Re-queueing the poison message after deploy hit the same StoreRuleResultAsync false return — but the App Insights exception trail this time exposed the actual underlying error: Newtonsoft.Json.JsonSerializationException: Self referencing loop detected with type 'System.Collections.Generic.Dictionary'. Path 'enforcement_error_resolved.allMatches'. Pre-existing latent bug in EvaluateEventCorrelationCondition since v1: matchedPairs[0] IS primary by reference. primary["allMatches"] = matchedPairs (or a Take(N).ToList() slice that still contains the same reference) creates primary -> allMatches[0] -> primary, which Newtonsoft refuses to serialize. JsonConvert.SerializeObject throws inside StoreRuleResultAsync, the catch returns false, and the queue handler propagates a retry — eventually hitting poison after 5 attempts. Why this never fired before: correlation rules typically produce 0 or 1 match, and the if (matchedPairs.Count > 1) branch was dead code in production. The 12-day stuck session that surfaced this was the first real session with multiple correlated app_install_failed/completed pairs. Fix: clone each pair into a new Dictionary when populating allMatches. The clones are independent instances with the same data, so Newtonsoft sees them as distinct objects and the cycle is broken. Pair flat-clone is sufficient — pairs only contain primitive values + small strings, no nested dicts. Also kept: the slim evidence (no message, no errorDetail) and the 60KB size guard from 47ffb348. Both still good defense-in-depth — the size guard never got a chance to trigger before because serialize threw first. Test: RuleEngineCorrelationEvidenceSlimTests.Correlation_with_multiple_pairs_serializes_without_self_referencing_loop runs JsonConvert.SerializeObject(MatchedConditions) end-to-end and fails fast if the loop sneaks back in. 774/774 backend pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../buildcounter.txt | 2 +- .../AutopilotMonitor.Agent/buildcounter.txt | 2 +- .../RuleEngineCorrelationEvidenceSlimTests.cs | 33 +++++++++++++++++++ .../RuleEngine.ConditionEvaluators.cs | 22 +++++++------ 4 files changed, 47 insertions(+), 12 deletions(-) diff --git a/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt b/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt index 1e6fd033..e5a135a5 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt +++ b/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt @@ -1 +1 @@ -444 +445 diff --git a/src/Agent/AutopilotMonitor.Agent/buildcounter.txt b/src/Agent/AutopilotMonitor.Agent/buildcounter.txt index b0536459..e944bea9 100644 --- a/src/Agent/AutopilotMonitor.Agent/buildcounter.txt +++ b/src/Agent/AutopilotMonitor.Agent/buildcounter.txt @@ -1 +1 @@ -1205 +1206 diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/RuleEngineCorrelationEvidenceSlimTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/RuleEngineCorrelationEvidenceSlimTests.cs index 545c03a9..caa895d4 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/RuleEngineCorrelationEvidenceSlimTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/RuleEngineCorrelationEvidenceSlimTests.cs @@ -3,6 +3,7 @@ using AutopilotMonitor.Shared.DataAccess; using AutopilotMonitor.Shared.Models; using Microsoft.Extensions.Logging.Abstractions; using Moq; +using Newtonsoft.Json; namespace AutopilotMonitor.Functions.Tests; @@ -76,6 +77,38 @@ public class RuleEngineCorrelationEvidenceSlimTests Assert.True((bool)primary["matchesTruncated"]); } + [Fact] + public async Task Correlation_with_multiple_pairs_serializes_without_self_referencing_loop() + { + // Regression pin: production session d434a3ba (2026-04-28) hit + // "Self referencing loop detected with type 'Dictionary'. + // Path 'enforcement_error_resolved.allMatches'." + // because matchedPairs[0] WAS primary by reference, and primary["allMatches"] = matchedPairs + // looped primary -> allMatches[0] -> primary. Cloning the dicts in allMatches breaks the + // cycle. This test fails fast if a future refactor re-introduces the shared reference. + var rule = BuildCorrelationRule(); + var baseTime = new DateTime(2026, 4, 28, 12, 0, 0, DateTimeKind.Utc); + var events = new List(); + for (var i = 0; i < 5; i++) + { + events.Add(BuildEvent($"evt-A-{i}", "app_install_failed", appId: $"app-{i}", sequence: i * 2, timestamp: baseTime.AddSeconds(i * 10))); + events.Add(BuildEvent($"evt-B-{i}", "app_install_completed", appId: $"app-{i}", sequence: i * 2 + 1, timestamp: baseTime.AddSeconds(i * 10 + 5))); + } + + var (engine, _) = SutWithRule(rule, events); + var outcome = await engine.AnalyzeSessionAsync(TenantId, SessionId); + + Assert.Single(outcome.Results); + + // The actual production failure was JsonConvert.SerializeObject throwing + // JsonSerializationException at the StoreRuleResultAsync persist step. Reproduce that + // exact serialize call here — if the loop sneaks back, this throws. + var json = JsonConvert.SerializeObject(outcome.Results[0].MatchedConditions); + Assert.False(string.IsNullOrEmpty(json)); + Assert.Contains("allMatches", json); + Assert.Contains("totalMatches", json); + } + [Fact] public async Task Correlation_with_few_pairs_does_not_truncate() { diff --git a/src/Backend/AutopilotMonitor.Functions/Services/RuleEngine.ConditionEvaluators.cs b/src/Backend/AutopilotMonitor.Functions/Services/RuleEngine.ConditionEvaluators.cs index 6e2cb213..71298255 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/RuleEngine.ConditionEvaluators.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/RuleEngine.ConditionEvaluators.cs @@ -668,21 +668,23 @@ namespace AutopilotMonitor.Functions.Services // Cap protects against runaway evidence size — sessions with hundreds of correlated // pairs would otherwise produce MatchedConditionsJson > 64KB and fail Table Storage // persistence (see also defense-in-depth check in TableStorageService.StoreRuleResultAsync). + // + // CRITICAL: allMatches MUST be a list of cloned dictionaries, not the matchedPairs + // list itself. matchedPairs[0] IS primary by reference — if we assigned the raw list, + // primary["allMatches"][0] would point back to primary and Newtonsoft.Json would + // throw "Self referencing loop detected" on the StoreRuleResultAsync UpsertEntity + // serialize step. The flat clone via `new Dictionary(p)` snapshots + // each pair's data BEFORE primary["allMatches"] is set, so the clones never contain + // the back-reference. Pre-existing latent bug from before slim-down — never fired + // in production until a 12-day stuck session matched multiple correlation pairs. const int MaxAllMatches = 10; var primary = matchedPairs[0]; primary["totalMatches"] = matchedPairs.Count; if (matchedPairs.Count > 1) { - if (matchedPairs.Count > MaxAllMatches) - { - primary["allMatches"] = matchedPairs.Take(MaxAllMatches).ToList(); - primary["matchesTruncated"] = true; - } - else - { - primary["allMatches"] = matchedPairs; - primary["matchesTruncated"] = false; - } + var capped = matchedPairs.Take(MaxAllMatches); + primary["allMatches"] = capped.Select(p => new Dictionary(p)).ToList(); + primary["matchesTruncated"] = matchedPairs.Count > MaxAllMatches; } return (true, primary); }