mirror of
https://github.com/okieselbach/AutopilotMonitor.git
synced 2026-10-24 04:32:32 +00:00
rules: hard-block high-frequency event types as evaluateOn on_event triggers
A warning does not stop an end user — telemetry-cadence types (performance_snapshot, agent_metrics_snapshot, download_progress, network_state_change/connectivity_check, log_entry, agent_trace, stall_probe_check) would turn every ingest batch into an analyze run. Now enforced in three layers: - AnalyzeRuleTriggers.BlockedOnEventTypes (code-only enforcement): CRUD write is rejected with an explicit message, and OnEventTypes() drops blocked types at runtime so even a persisted row (pre-validation data, direct table write) is inert for the trigger registry and the engine alike. - rules/guardrails.json gains blockedInterimTriggerEventTypes as the display/ pre-flight mirror (baked into the MCP RULE_GUARDRAILS via combine.js); a parity test pins mirror and code set against drift — same pattern as the gather hard blocks. - MCP validate_rule: the high-frequency finding is now an ERROR sourced from the baked mirror, not a hardcoded warning. Suites: Backend 4155, MCP 548. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -86,7 +86,7 @@ Interim evaluation changes what a rule's conditions mean mid-flight. Three new l
|
||||
|
||||
# Cost & Safety
|
||||
|
||||
- Interim runs are bounded by trigger frequency, and the chosen v1 triggers are rare by construction (`hybrid_login_pending` ≤1 per boot, `whiteglove_sealed` once per session). A tenant authoring `on_event:performance_snapshot` would be expensive — the trigger registry intersection happens per ingest batch, so worst case is one envelope per batch; if that ever hurts, a per-(session, reason) throttle marker is the v2 lever. The lint should warn on high-frequency trigger types (catalog already knows them).
|
||||
- Interim runs are bounded by trigger frequency, and the chosen v1 triggers are rare by construction (`hybrid_login_pending` ≤1 per boot, `whiteglove_sealed` once per session). High-frequency telemetry event types (performance_snapshot, agent_metrics_snapshot, download_progress, network churn, log_entry, agent_trace, stall_probe_check) are **hard-blocked** as `on_event` triggers, not merely advised against: the CRUD write is rejected (`AnalyzeRuleTriggers.BlockedOnEventTypes`, code-only enforcement), the runtime matcher drops them from any row that slipped in (defense in depth — the trigger registry and the engine both go through `OnEventTypes`), and the MCP lint reports them as an error. The JSON mirror lives in `rules/guardrails.json` (`blockedInterimTriggerEventTypes`) with a parity test pinning it to the C# set — the same pattern the gather hard blocks use. If a legitimate rare-but-bursty type ever hurts, a per-(session, reason) throttle marker is the v2 lever.
|
||||
- Each run loads the full event stream once (existing engine behavior, no change); rule filtering makes interim passes strictly cheaper than terminal ones.
|
||||
- Queue semantics (retry via visibility timeout, poison after MaxDequeueCount, idempotent partial-success re-runs) are inherited unchanged; upserts keep idempotency.
|
||||
|
||||
|
||||
+11
-1
@@ -272,5 +272,15 @@
|
||||
"Format-Volume",
|
||||
"Set-ExecutionPolicy"
|
||||
],
|
||||
"maxCommandLength": 2000
|
||||
"maxCommandLength": 2000,
|
||||
"blockedInterimTriggerEventTypes": [
|
||||
"performance_snapshot",
|
||||
"agent_metrics_snapshot",
|
||||
"download_progress",
|
||||
"network_state_change",
|
||||
"network_connectivity_check",
|
||||
"log_entry",
|
||||
"agent_trace",
|
||||
"stall_probe_check"
|
||||
]
|
||||
}
|
||||
|
||||
@@ -77,7 +77,7 @@
|
||||
},
|
||||
"evaluateOn": {
|
||||
"type": "array",
|
||||
"description": "When this rule is evaluated. Absent = [\"enrollment_end\"] (terminal-only, the historical behavior). Interim triggers let the rule fire before the session is terminal: \"whiteglove_sealed\" (first genuine WhiteGlove seal) and \"on_event:<eventType>\" (an ingest batch contained that event type). Interim runs suppress markSessionAsFailed and record no stats; findings notify once per (session, rule). Interim-enabled rules need monotonic conditions — a not_exists precondition on enrollment_complete/enrollment_failed/session_timeout passes trivially mid-run.",
|
||||
"description": "When this rule is evaluated. Absent = [\"enrollment_end\"] (terminal-only, the historical behavior). Interim triggers let the rule fire before the session is terminal: \"whiteglove_sealed\" (first genuine WhiteGlove seal) and \"on_event:<eventType>\" (an ingest batch contained that event type). High-frequency telemetry event types are HARD-BLOCKED as on_event triggers (guardrails blockedInterimTriggerEventTypes): the backend rejects them on save and the runtime ignores them. Interim runs suppress markSessionAsFailed and record no stats; findings notify once per (session, rule). Interim-enabled rules need monotonic conditions — a not_exists precondition on enrollment_complete/enrollment_failed/session_timeout passes trivially mid-run.",
|
||||
"items": {
|
||||
"type": "string",
|
||||
"pattern": "^(enrollment_end|whiteglove_sealed|on_event:[a-z0-9_]{1,128})$"
|
||||
|
||||
@@ -328,7 +328,7 @@ public class RuleEngineEvaluateOnTests
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void BuiltIn_catalog_evaluateOn_triggers_are_all_grammatical()
|
||||
public void BuiltIn_catalog_evaluateOn_triggers_are_all_grammatical_and_unblocked()
|
||||
{
|
||||
foreach (var rule in BuiltInAnalyzeRules.GetAll())
|
||||
{
|
||||
@@ -338,10 +338,74 @@ public class RuleEngineEvaluateOnTests
|
||||
{
|
||||
Assert.True(AnalyzeRuleTriggers.IsValidTrigger(trigger),
|
||||
$"Rule {rule.RuleId} ships invalid evaluateOn trigger '{trigger}'");
|
||||
|
||||
if (trigger.StartsWith("on_event:", StringComparison.OrdinalIgnoreCase))
|
||||
{
|
||||
var eventType = trigger.Substring("on_event:".Length);
|
||||
Assert.False(AnalyzeRuleTriggers.IsBlockedOnEventType(eventType),
|
||||
$"Rule {rule.RuleId} ships hard-blocked evaluateOn trigger '{trigger}' — the runtime would silently drop it");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// ── Hard-blocked high-frequency trigger types ───────────────────────────
|
||||
|
||||
[Fact]
|
||||
public void Blocked_on_event_types_are_dropped_by_the_runtime_matcher()
|
||||
{
|
||||
// Defense in depth: even a persisted row carrying a blocked trigger is inert.
|
||||
var rule = new AnalyzeRule
|
||||
{
|
||||
EvaluateOn = new List<string> { "on_event:performance_snapshot", $"on_event:{ProbeEventType}" },
|
||||
};
|
||||
Assert.Equal(new[] { ProbeEventType }, AnalyzeRuleTriggers.OnEventTypes(rule));
|
||||
Assert.False(AnalyzeRuleTriggers.MatchesOnEvent(rule, new[] { "performance_snapshot" }));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Create_rule_with_blocked_on_event_trigger_is_rejected()
|
||||
{
|
||||
var repo = new Mock<IRuleRepository>();
|
||||
repo.Setup(r => r.GetAnalyzeRulesAsync(It.IsAny<string>())).ReturnsAsync(new List<AnalyzeRule>());
|
||||
repo.Setup(r => r.GetRuleStatesAsync(It.IsAny<string>())).ReturnsAsync(new Dictionary<string, RuleState>());
|
||||
repo.Setup(r => r.AnalyzeRuleExistsAsync(It.IsAny<string>(), It.IsAny<string>())).ReturnsAsync(false);
|
||||
var service = new AnalyzeRuleService(repo.Object, NullLogger<AnalyzeRuleService>.Instance);
|
||||
|
||||
var rule = ProbeRule("ANALYZE-CUSTOM-901",
|
||||
evaluateOn: new List<string> { "enrollment_end", "on_event:performance_snapshot" });
|
||||
rule.IsBuiltIn = false;
|
||||
|
||||
var ex = await Assert.ThrowsAsync<InvalidOperationException>(() => service.CreateRuleAsync(TenantId, rule));
|
||||
Assert.Contains("blocked", ex.Message, StringComparison.OrdinalIgnoreCase);
|
||||
Assert.Contains("performance_snapshot", ex.Message);
|
||||
repo.Verify(r => r.StoreAnalyzeRuleAsync(It.IsAny<AnalyzeRule>(), It.IsAny<string>()), Times.Never);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Blocked_set_matches_the_guardrails_json_mirror()
|
||||
{
|
||||
// Enforcement is code-only (AnalyzeRuleTriggers.BlockedOnEventTypes); the JSON mirror
|
||||
// feeds the MCP validate_rule pre-flight. This parity test keeps the two from drifting —
|
||||
// an addition on either side fails here until the other side follows.
|
||||
var guardrailsPath = Path.Combine(FindRepoRoot(), "rules", "guardrails.json");
|
||||
var json = Newtonsoft.Json.Linq.JObject.Parse(File.ReadAllText(guardrailsPath));
|
||||
var mirror = json["blockedInterimTriggerEventTypes"]!.ToObject<string[]>()!;
|
||||
|
||||
Assert.Equal(
|
||||
AnalyzeRuleTriggers.BlockedOnEventTypes.OrderBy(t => t, StringComparer.Ordinal),
|
||||
mirror.OrderBy(t => t, StringComparer.Ordinal));
|
||||
}
|
||||
|
||||
private static string FindRepoRoot()
|
||||
{
|
||||
var dir = new DirectoryInfo(AppContext.BaseDirectory);
|
||||
while (dir != null && !File.Exists(Path.Combine(dir.FullName, "AutopilotMonitor.sln")))
|
||||
dir = dir.Parent;
|
||||
Assert.NotNull(dir);
|
||||
return dir!.FullName;
|
||||
}
|
||||
|
||||
// ── Fixture ─────────────────────────────────────────────────────────────
|
||||
|
||||
private sealed record Fixture(
|
||||
|
||||
@@ -638,6 +638,22 @@ namespace AutopilotMonitor.Functions.Services
|
||||
$"Invalid evaluateOn trigger(s): {string.Join(", ", invalid)}. " +
|
||||
"Allowed: 'enrollment_end', 'whiteglove_sealed', 'on_event:<event_type>' (lowercase snake_case).");
|
||||
}
|
||||
|
||||
// Hard block, not a hint: high-frequency telemetry event types would enqueue an
|
||||
// interim analyze run on effectively every ingest batch. The runtime additionally
|
||||
// ignores blocked types (AnalyzeRuleTriggers.OnEventTypes), but rejecting the write
|
||||
// keeps the author from persisting a trigger that silently never fires.
|
||||
var blocked = rule.EvaluateOn
|
||||
.Where(t => t != null && t.StartsWith(AnalyzeRuleTriggers.OnEventPrefix, StringComparison.OrdinalIgnoreCase))
|
||||
.Select(t => t!.Substring(AnalyzeRuleTriggers.OnEventPrefix.Length).Trim())
|
||||
.Where(AnalyzeRuleTriggers.IsBlockedOnEventType)
|
||||
.ToList();
|
||||
if (blocked.Count > 0)
|
||||
{
|
||||
throw new InvalidOperationException(
|
||||
$"evaluateOn on_event trigger(s) blocked for high-frequency telemetry event type(s): {string.Join(", ", blocked)}. " +
|
||||
"These occur on nearly every ingest batch and would run analysis continuously — pick a rare, problem-indicating event type.");
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
||||
@@ -421,14 +421,23 @@ describe('evaluateOn (interim evaluation) lint', () => {
|
||||
expect(warnings(validateRuleDraft(terminalOnly).findings).some((m) => m.includes('passes trivially mid-run'))).toBe(false);
|
||||
});
|
||||
|
||||
it('unknown on_event types warn, high-frequency types warn explicitly', () => {
|
||||
it('unknown on_event types warn, hard-blocked high-frequency types are ERRORS', () => {
|
||||
const unknown = validAnalyze();
|
||||
unknown.evaluateOn = ['enrollment_end', 'on_event:made_up_event'];
|
||||
expect(warnings(validateRuleDraft(unknown).findings).some((m) => m.includes('made_up_event'))).toBe(true);
|
||||
|
||||
const hot = validAnalyze();
|
||||
hot.evaluateOn = ['enrollment_end', 'on_event:performance_snapshot'];
|
||||
expect(warnings(validateRuleDraft(hot).findings).some((m) => m.includes('high-frequency'))).toBe(true);
|
||||
// Every blocked type from the guardrails mirror must produce an ERROR (valid=false) —
|
||||
// a warning would not stop an end user.
|
||||
for (const blocked of [
|
||||
'performance_snapshot', 'agent_metrics_snapshot', 'download_progress', 'network_state_change',
|
||||
'network_connectivity_check', 'log_entry', 'agent_trace', 'stall_probe_check',
|
||||
]) {
|
||||
const hot = validAnalyze();
|
||||
hot.evaluateOn = ['enrollment_end', `on_event:${blocked}`];
|
||||
const r = validateRuleDraft(hot);
|
||||
expect(r.valid, blocked).toBe(false);
|
||||
expect(errors(r.findings).some((m) => m.includes('HARD-BLOCKED')), blocked).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it('KO with interim-only triggers warns (the KO would never apply)', () => {
|
||||
|
||||
@@ -220,8 +220,10 @@ export const RULE_AUTHORING_GUIDE = {
|
||||
'WHEN the rule is evaluated. Absent = ["enrollment_end"] — the terminal-only default. ' +
|
||||
'Interim triggers evaluate the rule BEFORE the session is terminal: "whiteglove_sealed" ' +
|
||||
'(first genuine WhiteGlove seal, technician still at the bench) and "on_event:<eventType>" ' +
|
||||
'(an ingest batch contained that event type; pick a rare, problem-indicating type — never ' +
|
||||
'high-frequency telemetry). Interim semantics: markSessionAsFailed is SUPPRESSED, no stats ' +
|
||||
'(an ingest batch contained that event type; pick a rare, problem-indicating type — ' +
|
||||
'high-frequency telemetry types are HARD-BLOCKED: the backend rejects them on save and the ' +
|
||||
'runtime ignores them, see guardrails blockedInterimTriggerEventTypes). Interim semantics: ' +
|
||||
'markSessionAsFailed is SUPPRESSED, no stats ' +
|
||||
'are recorded, findings notify once per (session, rule) and render as "preliminary" until ' +
|
||||
'the enrollment-end pass finalizes or resolves them. CRITICAL: interim-enabled rules need ' +
|
||||
'MONOTONIC conditions — a not_exists precondition on enrollment_complete / enrollment_failed ' +
|
||||
|
||||
@@ -319,7 +319,7 @@ export const ANALYZE_RULE_SCHEMA: Record<string, unknown> = {
|
||||
},
|
||||
"evaluateOn": {
|
||||
"type": "array",
|
||||
"description": "When this rule is evaluated. Absent = [\"enrollment_end\"] (terminal-only, the historical behavior). Interim triggers let the rule fire before the session is terminal: \"whiteglove_sealed\" (first genuine WhiteGlove seal) and \"on_event:<eventType>\" (an ingest batch contained that event type). Interim runs suppress markSessionAsFailed and record no stats; findings notify once per (session, rule). Interim-enabled rules need monotonic conditions — a not_exists precondition on enrollment_complete/enrollment_failed/session_timeout passes trivially mid-run.",
|
||||
"description": "When this rule is evaluated. Absent = [\"enrollment_end\"] (terminal-only, the historical behavior). Interim triggers let the rule fire before the session is terminal: \"whiteglove_sealed\" (first genuine WhiteGlove seal) and \"on_event:<eventType>\" (an ingest batch contained that event type). High-frequency telemetry event types are HARD-BLOCKED as on_event triggers (guardrails blockedInterimTriggerEventTypes): the backend rejects them on save and the runtime ignores them. Interim runs suppress markSessionAsFailed and record no stats; findings notify once per (session, rule). Interim-enabled rules need monotonic conditions — a not_exists precondition on enrollment_complete/enrollment_failed/session_timeout passes trivially mid-run.",
|
||||
"items": {
|
||||
"type": "string",
|
||||
"pattern": "^(enrollment_end|whiteglove_sealed|on_event:[a-z0-9_]{1,128})$"
|
||||
@@ -1034,5 +1034,15 @@ export const RULE_GUARDRAILS = {
|
||||
"Format-Volume",
|
||||
"Set-ExecutionPolicy"
|
||||
],
|
||||
"maxCommandLength": 2000
|
||||
"maxCommandLength": 2000,
|
||||
"blockedInterimTriggerEventTypes": [
|
||||
"performance_snapshot",
|
||||
"agent_metrics_snapshot",
|
||||
"download_progress",
|
||||
"network_state_change",
|
||||
"network_connectivity_check",
|
||||
"log_entry",
|
||||
"agent_trace",
|
||||
"stall_probe_check"
|
||||
]
|
||||
} as const;
|
||||
|
||||
@@ -387,10 +387,10 @@ function lintAnalyzeRule(rule: Record<string, unknown>): ValidationFinding[] {
|
||||
if (trigger.startsWith('on_event:')) {
|
||||
const eventType = trigger.slice('on_event:'.length);
|
||||
findings.push(...checkEventTypeKnown(eventType, `evaluateOn "${trigger}"`));
|
||||
if (HIGH_FREQUENCY_EVENT_TYPES.has(eventType)) {
|
||||
if (BLOCKED_INTERIM_TRIGGER_EVENT_TYPES.has(eventType)) {
|
||||
findings.push({
|
||||
level: 'warning',
|
||||
message: `evaluateOn "${trigger}": ${eventType} is a high-frequency telemetry event — this would enqueue an interim analyze run on nearly every ingest batch. Pick a rare, problem-indicating event type instead.`,
|
||||
level: 'error',
|
||||
message: `evaluateOn "${trigger}": ${eventType} is a HARD-BLOCKED high-frequency telemetry event — it occurs on nearly every ingest batch and would run analysis continuously. The backend rejects this trigger on save and the runtime ignores it. Pick a rare, problem-indicating event type instead.`,
|
||||
});
|
||||
}
|
||||
}
|
||||
@@ -430,18 +430,14 @@ function lintAnalyzeRule(rule: Record<string, unknown>): ValidationFinding[] {
|
||||
|
||||
/**
|
||||
* Event types emitted on effectively every batch (telemetry cadence) — an on_event interim
|
||||
* trigger keyed to one of these would turn every ingest into an analyze run.
|
||||
* trigger keyed to one of these would turn every ingest into an analyze run. Sourced from
|
||||
* the baked guardrails mirror (rules/guardrails.json → RULE_GUARDRAILS); enforcement itself
|
||||
* is code-only in the backend (AnalyzeRuleTriggers.BlockedOnEventTypes) with a parity test
|
||||
* pinning the two sets.
|
||||
*/
|
||||
const HIGH_FREQUENCY_EVENT_TYPES = new Set([
|
||||
'performance_snapshot',
|
||||
'agent_metrics_snapshot',
|
||||
'download_progress',
|
||||
'network_state_change',
|
||||
'network_connectivity_check',
|
||||
'log_entry',
|
||||
'agent_trace',
|
||||
'stall_probe_check',
|
||||
]);
|
||||
const BLOCKED_INTERIM_TRIGGER_EVENT_TYPES = new Set<string>(
|
||||
((RULE_GUARDRAILS as Record<string, unknown>).blockedInterimTriggerEventTypes as readonly string[] | undefined) ?? [],
|
||||
);
|
||||
|
||||
// ── Gather-rule semantic lint ───────────────────────────────────────────────
|
||||
|
||||
|
||||
@@ -22,6 +22,31 @@ namespace AutopilotMonitor.Shared.Models
|
||||
/// <summary>Prefix for event-driven interim triggers: <c>on_event:<eventType></c>.</summary>
|
||||
public const string OnEventPrefix = "on_event:";
|
||||
|
||||
/// <summary>
|
||||
/// HARD-BLOCKED on_event trigger types: telemetry-cadence events that occur on
|
||||
/// effectively every ingest batch — an interim trigger keyed to one of these would turn
|
||||
/// every batch into an analyze run (full event-stream read per batch, per session).
|
||||
/// Enforcement is code-only by design (this set gates CRUD validation AND the runtime
|
||||
/// matching in <see cref="OnEventTypes"/>, so a row that ever slips past validation is
|
||||
/// still inert). rules/guardrails.json carries the display/pre-flight mirror consumed
|
||||
/// by the MCP validate_rule lint; a parity test pins the two against drift.
|
||||
/// </summary>
|
||||
public static readonly IReadOnlyCollection<string> BlockedOnEventTypes = new HashSet<string>(StringComparer.OrdinalIgnoreCase)
|
||||
{
|
||||
"performance_snapshot",
|
||||
"agent_metrics_snapshot",
|
||||
"download_progress",
|
||||
"network_state_change",
|
||||
"network_connectivity_check",
|
||||
"log_entry",
|
||||
"agent_trace",
|
||||
"stall_probe_check",
|
||||
};
|
||||
|
||||
/// <summary>True when the event type is hard-blocked as an on_event interim trigger.</summary>
|
||||
public static bool IsBlockedOnEventType(string? eventType)
|
||||
=> !string.IsNullOrWhiteSpace(eventType) && BlockedOnEventTypes.Contains(eventType!.Trim());
|
||||
|
||||
/// <summary>
|
||||
/// The rule's effective trigger list: EvaluateOn, or the enrollment_end default when
|
||||
/// the field is absent/empty (legacy rules and rows predating the feature).
|
||||
@@ -44,6 +69,9 @@ namespace AutopilotMonitor.Shared.Models
|
||||
/// <summary>
|
||||
/// The event types this rule wants an interim run for (empty when it has no
|
||||
/// on_event triggers). Types are returned lower-cased for set intersection.
|
||||
/// Hard-blocked types (<see cref="BlockedOnEventTypes"/>) are silently dropped —
|
||||
/// defense in depth: even a persisted row carrying one (pre-validation data, direct
|
||||
/// table write) can never cause an interim run.
|
||||
/// </summary>
|
||||
public static IReadOnlyList<string> OnEventTypes(AnalyzeRule rule)
|
||||
{
|
||||
@@ -51,7 +79,7 @@ namespace AutopilotMonitor.Shared.Models
|
||||
.Where(t => t != null && t.StartsWith(OnEventPrefix, StringComparison.OrdinalIgnoreCase)
|
||||
&& t.Length > OnEventPrefix.Length)
|
||||
.Select(t => t.Substring(OnEventPrefix.Length).Trim().ToLowerInvariant())
|
||||
.Where(t => t.Length > 0)
|
||||
.Where(t => t.Length > 0 && !IsBlockedOnEventType(t))
|
||||
.ToList();
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user