Skip to content

Commit 321c192

Browse files
authored
feat(review): same-rule repeat alarm — detection + alert, no autonomous action (#8092)
Closes #7983. The existing self-correction system only detects a systematically- wrong rule via precision-over-time (auto-tune.ts), which needs a real, DECIDED sample (>= AUTOTUNE_MIN_DECIDED) accumulated over however long that takes -- too slow for a bug that can mis-close 4 PRs within hours, as the 2026-07-21/22 metagraphed incident did. A much cheaper, ground-truth-free signal already exists: the SAME deterministic rule/blocker code rejecting several DIFFERENT PRs in a short window in the same repo is itself a strong "something's broken" signal, independent of whether any of those rejections is ever confirmed or reversed by a human. New packages/loopover-engine/src/calibration/signal-tracking.ts: evaluateRuleRepeatAlarm(ruleId, fired, threshold) — pure, no ground truth needed, mirrors src/orb/analytics.ts's gamingPatternFlags precedent ("Detection only — never an automatic action"). New src/review/rule-repeat-alarm-wire.ts wires this into ORB for real: every gate block now records a #7982 rule-fired signal per blocker code (nothing called the ORB adapter until now), scoped per-(repo, code) so an unrelated repo or code never contributes to another's count, and checks the repeat alarm inline, immediately after each block — not on a later cron tick, matching the "hours, not days" urgency the incident exposed. A triggered alarm logs a structured console.error (forwarded to Sentry, the same "detected an anomaly" channel src/review/ops-wire.ts's own runOpsAlerts already uses) and writes a cooldown marker so an ongoing incident doesn't re-alert on every subsequent PR. Note on the issue's own cited alert channel: notify-discord.ts/ notify-slack turned out to be the wrong fit on inspection — that's a per-REPO, community-facing channel for PR action notifications, not an operator-facing "an ORB rule may be systematically broken" signal that can span any repo the instance reviews. Sentry (via the existing structured-log forwarder) is the channel actually already used for this class of alert. Validated against a replay of the exact #7469/#7589/#7591/#7594 incident shape: triggers on the 3rd distinct PR, matching the issue's own "should have alerted after the 2nd or 3rd occurrence" bar.
1 parent f442184 commit 321c192

5 files changed

Lines changed: 403 additions & 6 deletions

File tree

packages/loopover-engine/src/calibration/signal-tracking.ts

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -102,12 +102,46 @@ export function computeRulePrecision(ruleId: string, fired: readonly RuleFiredEv
102102
}
103103

104104
/**
105-
* Count how many times `ruleId` fired against the exact same `targetKey` within `fired` -- the #7983
106-
* "same-rule repeat alarm" primitive (a rule re-firing against a target it already fired against once is a
107-
* stronger signal than a bare one-off fire, independent of whether either fire has been overridden yet).
108-
* Pure counting, no time-windowing here -- a caller windows `fired` itself before calling this (e.g. via
109-
* `queryRuleHistory`'s own `sinceMs`), matching how this whole module leaves all storage/scoping to the host.
105+
* Count how many times `ruleId` fired against the exact same `targetKey` within `fired` (a rule re-firing
106+
* against a target it already fired against once -- e.g. an unresolved contributor PR re-triggering the same
107+
* blocker on every push -- is a different signal than a fresh one-off fire, independent of whether either fire
108+
* has been overridden yet). Pure counting, no time-windowing here -- a caller windows `fired` itself before
109+
* calling this (e.g. via `queryRuleHistory`'s own `sinceMs`), matching how this whole module leaves all
110+
* storage/scoping to the host. See {@link evaluateRuleRepeatAlarm} for the DIFFERENT #7983 signal: the same
111+
* rule firing against several DIFFERENT targets, not the same one repeatedly.
110112
*/
111113
export function computeRuleRepeatCount(ruleId: string, targetKey: string, fired: readonly RuleFiredEvent[]): number {
112114
return fired.reduce((count, event) => (event.ruleId === ruleId && event.targetKey === targetKey ? count + 1 : count), 0);
113115
}
116+
117+
/** A same-rule repeat-alarm verdict (#7983): whether `ruleId` has fired against enough DISTINCT targets within
118+
* the caller's already-windowed `fired` list to be a "something is systematically broken" signal --
119+
* independent of whether any of those firings has been confirmed or reversed by a human yet (unlike
120+
* {@link computeRulePrecision}, this needs no ground truth at all, which is exactly why it can fire fast: the
121+
* 2026-07-21/22 metagraphed incident mis-closed 4 DISTINCT PRs within ~3 hours on the same rule, far faster
122+
* than a precision-over-time breaker's `AUTOTUNE_MIN_DECIDED` sample could ever accumulate real outcomes). */
123+
export type RuleRepeatAlarmVerdict = {
124+
ruleId: string;
125+
/** Every distinct targetKey `ruleId` fired against, in first-seen order. */
126+
affectedTargets: string[];
127+
threshold: number;
128+
triggered: boolean;
129+
};
130+
131+
/**
132+
* Evaluate the #7983 same-rule repeat alarm for `ruleId` over an already-windowed `fired` list: `triggered` is
133+
* true once the rule has fired against at least `threshold` DISTINCT targets. Deliberately returns a
134+
* detection-only verdict -- no action, no severity beyond the boolean -- mirroring `src/orb/analytics.ts`'s
135+
* `gamingPatternFlags` precedent ("Detection only — never an automatic action") and this module's own
136+
* "no autonomous behavior" boundary; the host decides how (or whether) to surface a triggered verdict.
137+
*/
138+
export function evaluateRuleRepeatAlarm(ruleId: string, fired: readonly RuleFiredEvent[], threshold: number): RuleRepeatAlarmVerdict {
139+
const affectedTargets: string[] = [];
140+
const seen = new Set<string>();
141+
for (const event of fired) {
142+
if (event.ruleId !== ruleId || seen.has(event.targetKey)) continue;
143+
seen.add(event.targetKey);
144+
affectedTargets.push(event.targetKey);
145+
}
146+
return { ruleId, affectedTargets, threshold, triggered: affectedTargets.length >= threshold };
147+
}

packages/loopover-engine/test/signal-tracking.test.ts

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
11
import assert from "node:assert/strict";
22
import { test } from "node:test";
33

4-
import { computeRulePrecision, computeRuleRepeatCount, type HumanOverrideEvent, type RuleFiredEvent } from "../dist/index.js";
4+
import {
5+
computeRulePrecision,
6+
computeRuleRepeatCount,
7+
evaluateRuleRepeatAlarm,
8+
type HumanOverrideEvent,
9+
type RuleFiredEvent,
10+
} from "../dist/index.js";
511

612
function fired(ruleId: string, targetKey: string, overrides: Partial<RuleFiredEvent> = {}): RuleFiredEvent {
713
return { ruleId, targetKey, outcome: "block", occurredAt: "2026-07-22T00:00:00.000Z", ...overrides };
@@ -90,3 +96,61 @@ test("computeRuleRepeatCount: counts only fires matching BOTH ruleId and targetK
9096
test("computeRuleRepeatCount: zero fired events yields 0, not an error", () => {
9197
assert.equal(computeRuleRepeatCount("rule_a", "a#1", []), 0);
9298
});
99+
100+
test("barrel: the public entrypoint re-exports evaluateRuleRepeatAlarm (#7983)", () => {
101+
assert.equal(typeof evaluateRuleRepeatAlarm, "function");
102+
});
103+
104+
test("evaluateRuleRepeatAlarm: not triggered below the threshold", () => {
105+
const verdict = evaluateRuleRepeatAlarm("rule_a", [fired("rule_a", "a#1"), fired("rule_a", "a#2")], 3);
106+
assert.equal(verdict.triggered, false);
107+
assert.deepEqual(verdict.affectedTargets, ["a#1", "a#2"]);
108+
assert.equal(verdict.threshold, 3);
109+
});
110+
111+
test("evaluateRuleRepeatAlarm: triggers once distinct targets reach the threshold", () => {
112+
const verdict = evaluateRuleRepeatAlarm("rule_a", [fired("rule_a", "a#1"), fired("rule_a", "a#2"), fired("rule_a", "a#3")], 3);
113+
assert.equal(verdict.triggered, true);
114+
assert.deepEqual(verdict.affectedTargets, ["a#1", "a#2", "a#3"]);
115+
});
116+
117+
test("evaluateRuleRepeatAlarm: replays the #7469/#7589/#7591/#7594 incident shape -- triggers on the 3rd distinct PR", () => {
118+
const incidentEvents = [
119+
fired("rule_a", "metagraphed/metagraphed#7469"),
120+
fired("rule_a", "metagraphed/metagraphed#7589"),
121+
];
122+
// Should NOT have triggered yet after only 2 distinct PRs (threshold 3).
123+
assert.equal(evaluateRuleRepeatAlarm("rule_a", incidentEvents, 3).triggered, false);
124+
incidentEvents.push(fired("rule_a", "metagraphed/metagraphed#7591"));
125+
// The 3rd distinct PR crosses the threshold -- exactly the "should have alerted after the 2nd or 3rd
126+
// occurrence" bar #7983 itself sets.
127+
const thirdVerdict = evaluateRuleRepeatAlarm("rule_a", incidentEvents, 3);
128+
assert.equal(thirdVerdict.triggered, true);
129+
assert.deepEqual(thirdVerdict.affectedTargets, [
130+
"metagraphed/metagraphed#7469",
131+
"metagraphed/metagraphed#7589",
132+
"metagraphed/metagraphed#7591",
133+
]);
134+
});
135+
136+
test("evaluateRuleRepeatAlarm: the SAME target firing repeatedly counts once, not once per fire -- only a DISTINCT target grows the count", () => {
137+
const verdict = evaluateRuleRepeatAlarm(
138+
"rule_a",
139+
[fired("rule_a", "a#1"), fired("rule_a", "a#1"), fired("rule_a", "a#1")],
140+
2,
141+
);
142+
assert.equal(verdict.affectedTargets.length, 1);
143+
assert.equal(verdict.triggered, false);
144+
});
145+
146+
test("evaluateRuleRepeatAlarm: ignores fired events for a DIFFERENT ruleId entirely", () => {
147+
const verdict = evaluateRuleRepeatAlarm("rule_a", [fired("rule_a", "a#1"), fired("rule_b", "a#2"), fired("rule_b", "a#3")], 2);
148+
assert.equal(verdict.affectedTargets.length, 1);
149+
assert.equal(verdict.triggered, false);
150+
});
151+
152+
test("evaluateRuleRepeatAlarm: zero fired events never triggers", () => {
153+
const verdict = evaluateRuleRepeatAlarm("rule_a", [], 1);
154+
assert.equal(verdict.triggered, false);
155+
assert.deepEqual(verdict.affectedTargets, []);
156+
});

src/queue/processors.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -636,6 +636,7 @@ import {
636636
} from "../review/outcomes-wire";
637637
import { neutralHoldReasonCode, nativeGateActionFromConclusion, recordNativeGateDecision } from "../review/parity-wire";
638638
import { recordContributorGateDecision } from "../review/contributor-calibration";
639+
import { recordGateBlockersAndCheckRepeatAlarm } from "../review/rule-repeat-alarm-wire";
639640
import { recordPredictedGateCalibration } from "../review/predicted-gate-calibration-ledger";
640641
import type { SubmissionOutcome } from "../review/submitter-reputation";
641642
import type {
@@ -10326,6 +10327,13 @@ async function maybePublishPrPublicSurface(
1032610327
outcome: "completed",
1032710328
metadata: { blockerCodes },
1032810329
});
10330+
// #7983: same-rule repeat alarm — detection + alert only, never adjusts the gate. See
10331+
// rule-repeat-alarm-wire.ts's own header comment.
10332+
await recordGateBlockersAndCheckRepeatAlarm(env, {
10333+
repoFullName,
10334+
pullNumber: pr.number,
10335+
blockerCodes,
10336+
}).catch(() => undefined);
1032910337
}
1033010338
// #preconv-parity (convergence prep): SHADOW-record the gittensory-native gate decision (source=
1033110339
// 'gittensory-native') into review_audit so the pre-cutover parity harness has data to read. RECORD-ONLY,
Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
// ORB wiring for #7983's same-rule repeat alarm. Records a #7982 rule-fired signal for each gate blocker code
2+
// on every gate block, then checks whether that SAME (repo, blocker code) pair has now fired against enough
3+
// DISTINCT PRs within a short window to be a "something is systematically broken" signal — independent of
4+
// whether any of those blocks has since been confirmed or reversed by a human (unlike the precision-over-time
5+
// circuit breaker in auto-tune.ts, which needs a real, DECIDED sample of >= AUTOTUNE_MIN_DECIDED and however
6+
// long that takes to accumulate). This is exactly the gap the 2026-07-21/22 metagraphed incident exposed: 4
7+
// distinct PRs mis-closed by the same rule within ~3 hours, far faster than any ground-truth-based breaker
8+
// could ever react.
9+
//
10+
// DETECTION + ALERT ONLY (#7983's own stated boundary, mirroring src/orb/analytics.ts's gamingPatternFlags
11+
// precedent: "Detection only — never an automatic action"). This module never holds, closes, or otherwise
12+
// changes any gate/disposition decision — it only records a signal and, once, surfaces a structured alert.
13+
//
14+
// Alert channel: NOT notify-discord.ts/notify-slack (that's a per-REPO, community-facing channel for PR
15+
// action notifications — the wrong audience for "an ORB rule may be systematically broken," which is an
16+
// OPERATOR concern that can span any repo the instance reviews). Uses the same console.error(JSON.stringify(
17+
// {level:"error",...})) idiom src/review/ops-wire.ts's runOpsAlerts already uses for its own operator-anomaly
18+
// detection — forwarded to Sentry by selfhost/sentry.ts's forwardStructuredLogToSentry, the actually-live
19+
// operator-facing channel for exactly this class of "detected an anomaly, not a caught exception" alert.
20+
21+
import { evaluateRuleRepeatAlarm, type SignalStore } from "@loopover/engine";
22+
23+
import { hasRecentAuditEvent, recordAuditEvent } from "../db/repositories";
24+
import { nowIso } from "../utils/json";
25+
import { createSignalStore } from "./signal-tracking-wire";
26+
27+
/** How far back to look for repeat fires. Matches #7983's own "e.g. 1-24h, tunable" proposal — chosen at the
28+
* wide end so a slow-burn (not just a fast-burst) repeat still gets caught. */
29+
export const RULE_REPEAT_ALARM_WINDOW_MS = 24 * 60 * 60 * 1000;
30+
/** Distinct-target count that trips the alarm. #7983's own proposal ("e.g. >= 3 within 24h") and its own
31+
* validation bar ("should have alerted after the 2nd or 3rd occurrence" against the real incident replay). */
32+
export const RULE_REPEAT_ALARM_THRESHOLD = 3;
33+
/** Once triggered, don't re-alert for the SAME (repo, code) pair more often than this — an already-known,
34+
* ongoing incident re-alerting on every subsequent PR would be noise, not new information. Shorter than
35+
* {@link RULE_REPEAT_ALARM_WINDOW_MS} so a genuinely NEW burst (a different day, a fix that regressed again)
36+
* still re-alerts well before the detection window itself would naturally reset. */
37+
const RULE_REPEAT_ALARM_ALERT_COOLDOWN_MS = 6 * 60 * 60 * 1000;
38+
39+
/** Repo-scoped rule id (#7983 wants the alarm keyed by "(deployment/repo-or-cohort, rule code)", not a bare
40+
* code across the whole fleet — a code that's simply common everywhere must not look like one repo's rule
41+
* going haywire). Reuses the same signal-tracking `ruleId` seam #7982 already defined; the repo scope is
42+
* folded directly into the id rather than needing a second dimension on {@link SignalStore}. */
43+
function repeatAlarmRuleId(repoFullName: string, blockerCode: string): string {
44+
return `${repoFullName}:${blockerCode}`;
45+
}
46+
47+
function alertAuditEventType(ruleId: string): string {
48+
return `rule_repeat_alarm:${ruleId}`;
49+
}
50+
51+
async function checkAndAlertRuleRepeat(
52+
env: Env,
53+
store: SignalStore,
54+
ruleId: string,
55+
blockerCode: string,
56+
repoFullName: string,
57+
): Promise<void> {
58+
const history = await store.queryRuleHistory(ruleId, Date.now() - RULE_REPEAT_ALARM_WINDOW_MS);
59+
const verdict = evaluateRuleRepeatAlarm(ruleId, history.fired, RULE_REPEAT_ALARM_THRESHOLD);
60+
if (!verdict.triggered) return;
61+
const alertEventType = alertAuditEventType(ruleId);
62+
const alreadyAlerted = await hasRecentAuditEvent(
63+
env,
64+
"loopover",
65+
alertEventType,
66+
new Date(Date.now() - RULE_REPEAT_ALARM_ALERT_COOLDOWN_MS).toISOString(),
67+
);
68+
if (alreadyAlerted) return;
69+
console.error(
70+
JSON.stringify({
71+
level: "error",
72+
event: "same_rule_repeat_alarm",
73+
ev: ruleId,
74+
repo: repoFullName,
75+
blockerCode,
76+
distinctTargetCount: verdict.affectedTargets.length,
77+
threshold: verdict.threshold,
78+
affectedTargets: verdict.affectedTargets,
79+
at: nowIso(),
80+
}),
81+
);
82+
await recordAuditEvent(env, {
83+
eventType: alertEventType,
84+
actor: "loopover",
85+
targetKey: ruleId,
86+
outcome: "completed",
87+
detail: `same-rule repeat alarm: ${blockerCode} fired against ${verdict.affectedTargets.length} distinct PR(s) in ${repoFullName} within ${RULE_REPEAT_ALARM_WINDOW_MS / (60 * 60 * 1000)}h`,
88+
metadata: { repoFullName, blockerCode, affectedTargets: verdict.affectedTargets },
89+
}).catch(() => undefined);
90+
}
91+
92+
/**
93+
* Records a #7982 rule-fired signal for every blocker code on a gate block, then runs the #7983 repeat-alarm
94+
* check for each. Best-effort throughout (a failure anywhere in this path is swallowed) — this is a pure
95+
* measurement/alerting side channel and must never affect, delay, or fail the gate decision that produced the
96+
* blocker codes it's recording.
97+
*/
98+
export async function recordGateBlockersAndCheckRepeatAlarm(
99+
env: Env,
100+
args: { repoFullName: string; pullNumber: number; blockerCodes: readonly string[]; occurredAt?: string },
101+
): Promise<void> {
102+
if (args.blockerCodes.length === 0) return;
103+
// createSignalStore is pure object construction (no I/O), so it never throws — no try/catch needed here.
104+
// recordRuleFired below already swallows its own write failures internally (signal-tracking-wire.ts), so it
105+
// never rejects either; only queryRuleHistory (inside checkAndAlertRuleRepeat) can genuinely reject, which
106+
// this loop's own .catch below covers.
107+
const store = createSignalStore(env);
108+
const occurredAt = args.occurredAt ?? nowIso();
109+
const targetKey = `${args.repoFullName}#${args.pullNumber}`;
110+
for (const blockerCode of new Set(args.blockerCodes)) {
111+
const ruleId = repeatAlarmRuleId(args.repoFullName, blockerCode);
112+
await store.recordRuleFired({ ruleId, targetKey, outcome: "block", occurredAt });
113+
await checkAndAlertRuleRepeat(env, store, ruleId, blockerCode, args.repoFullName).catch(() => undefined);
114+
}
115+
}

0 commit comments

Comments
 (0)