Skip to content

Commit 74131c1

Browse files
authored
fix(observability): forward ops-alerts anomalies to Sentry, add a review-burst rule (#3766)
Two changes to the hourly ops-alerts anomaly scan: 1. Fix a Sentry-visibility gap found while investigating the CI-stuck repeat-review incident: every existing anomaly (gate false-positive spike, slop score inverting, recommendations not panning out) was logged via console.warn with an `ev` field. forwardStructuredLogToSentry only wraps console.log/console.error and keys the issue off a field literally named `event` -- so none of these anomalies has EVER reached Sentry, regardless of whether Sentry is active. They only ever reached Workers Logs, which nobody was watching -- part of why the CI-stuck bleed went unnoticed for 20+ hours. Switched to console.error + an `event` field, matching the convention already used by selfhost_ai_provider_failed / regate_repair_exhausted / ci_stuck_review_repeat_suppressed. 2. Add a 4th anomaly rule: "review burst" -- flags a PR that published more than 6 review surfaces within a 2-hour rolling window, the exact signature of a stuck-CI finalize loop or sweep retry storm. Backed by a new findHottestReviewTargetForRepo query over the existing audit_events ledger (github_app.pr_public_surface_published is a genuine INSERT-only event per publish pass, unlike ai_review_cache or review_audit's gate_decision rows, which are both upserted at a fixed head SHA and so cannot see a same-head repeat). This runs on the existing hourly ops-alerts cron, so a future recurrence of this class of bleed surfaces within an hour instead of requiring a human to notice and query the database directly.
1 parent 5172dc4 commit 74131c1

5 files changed

Lines changed: 162 additions & 30 deletions

File tree

src/db/repositories.ts

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { and, asc, desc, eq, gte, inArray, not, or, sql, type SQL } from "drizzle-orm";
1+
import { and, asc, desc, eq, gte, inArray, like, not, or, sql, type SQL } from "drizzle-orm";
22
import { getDb } from "./client";
33
import {
44
activeReviewTracking,
@@ -2552,6 +2552,40 @@ export async function countRecentAuditEventsForActorAndTarget(env: Env, actor: s
25522552
return row.count;
25532553
}
25542554

2555+
/** #orb-ci-stuck-repeat / #orb-retry-storm ops-alerts signal: the single PR within `repoFullName` that published
2556+
* the most review surfaces in the last `sinceIso`-bounded window, and how many. `github_app.pr_public_surface_
2557+
* published` is a genuine INSERT-only event (never upserted) recorded once per successful publish pass
2558+
* (processors.ts's finishPublicSurfacePublication), so unlike ai_review_cache or review_audit's gate_decision
2559+
* rows (both keyed + upserted on `(repo, pr, headSha)`, so a repeat pass at an UNCHANGED head silently
2560+
* overwrites rather than accumulates), this correctly counts repeat publishes even when the head SHA never
2561+
* changes -- exactly the shape of a stuck-CI or sweep retry-storm bleed. Returns null when the repo published
2562+
* no surfaces in the window at all. */
2563+
export async function findHottestReviewTargetForRepo(
2564+
env: Env,
2565+
repoFullName: string,
2566+
sinceIso: string,
2567+
): Promise<{ targetKey: string; count: number } | null> {
2568+
const db = getDb(env.DB);
2569+
const [row] = await db
2570+
.select({ targetKey: auditEvents.targetKey, count: sql<number>`count(*)` })
2571+
.from(auditEvents)
2572+
.where(
2573+
and(
2574+
eq(auditEvents.eventType, "github_app.pr_public_surface_published"),
2575+
like(auditEvents.targetKey, `${repoFullName}#%`),
2576+
gte(auditEvents.createdAt, sinceIso),
2577+
),
2578+
)
2579+
.groupBy(auditEvents.targetKey)
2580+
.orderBy(desc(sql`count(*)`))
2581+
.limit(1);
2582+
/* v8 ignore next -- the WHERE clause's `like(auditEvents.targetKey, ...)` can never match a NULL target_key
2583+
* (SQL LIKE against NULL is NULL, never true), so a returned row always has a non-null targetKey; the
2584+
* column's nullable TS type is a schema-wide default this specific query structurally rules out. */
2585+
if (!row || row.targetKey === null) return null;
2586+
return { targetKey: row.targetKey, count: row.count };
2587+
}
2588+
25552589
/** Moderation-rules engine (#selfhost-mod-engine): the actor's TOTAL violation count across every rule type in
25562590
* `eventTypes` and EVERY repo this install tracks (no targetKey/route scoping -- `audit_events` carries no
25572591
* repo/installation column at all, so this is inherently install-wide, mirroring the install-wide contributor

src/review/ops-wire.ts

Lines changed: 46 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,23 @@
1414
// services/outcome-calibration.ts (buildRepoOutcomeCalibration).
1515
//
1616
// NOTIFY PATH: gittensory has NO Discord / operator webhook (notifications/service.ts is a per-recipient,
17-
// pull-based BADGE feed — the wrong channel for an operator anomaly). So, per the task ("Discord/webhook if
18-
// present, else a structured log"), an anomaly emits a structured `console.warn` log line (the house
19-
// `JSON.stringify({ ev: ... })` convention used across the worker) that Workers Logs/Observability surfaces.
17+
// pull-based BADGE feed — the wrong channel for an operator anomaly). So an anomaly emits a structured
18+
// `console.error` log line with an `event` field (#orb-ci-stuck-repeat: this was previously `console.warn` with
19+
// an `ev` field — forwardStructuredLogToSentry, src/selfhost/sentry.ts, only wraps console.log/console.error
20+
// -- never console.warn -- and keys the Sentry issue off a field literally named `event`, not `ev`. Under the
21+
// old shape, every anomaly this module ever found was invisible to Sentry regardless of whether Sentry was
22+
// active; it only ever reached Workers Logs, which is why a 20+-hour token-usage bleed went unnoticed until a
23+
// human queried the database directly. `console.error` + `event` is the same convention every other Sentry-
24+
// visible anomaly signal in this codebase already uses (selfhost_ai_provider_failed, regate_repair_exhausted,
25+
// ci_stuck_review_repeat_suppressed).
2026
//
2127
// DEFERRED (NOT implemented here): the auto-tune / auto-apply config-mutation self-improve loop. The ported
2228
// pure logic + D1 store already exist in src/review/auto-apply.ts, but actually CLOSING the loop (mutating a
2329
// live gate's tunables from the cron) is sensitive — it needs the `tunables_overrides` / `_shadow` /
2430
// `override_audit` D1 tables (none of which exist in gittensory's migrations yet) plus a careful soak/promote
2531
// design. This module is READ-ONLY observability: it reports drift; it never changes what blocks a live PR.
2632

27-
import { listRepositories } from "../db/repositories";
33+
import { findHottestReviewTargetForRepo, listRepositories } from "../db/repositories";
2834
import { isAgentConfigured } from "../settings/autonomy";
2935
import { resolveRepositorySettings } from "../settings/repository-settings";
3036
import { loadGatePrecisionReport, type GatePrecisionReport } from "../services/gate-precision";
@@ -47,12 +53,24 @@ const GATE_FALSE_POSITIVE_THRESHOLD = 0.3;
4753
const RECOMMENDATION_NEGATIVE_THRESHOLD = 0.5;
4854
/** Don't judge the recommendation negative-rate off a trickle of resolved outcomes. */
4955
const MIN_RECOMMENDATION_RESOLVED = 5;
50-
51-
/** One repo's outcome reports + the repo it covers — the input to the pure anomaly detector. */
56+
/** #orb-ci-stuck-repeat / #orb-retry-storm: more than this many published review surfaces for the SAME PR within
57+
* REVIEW_BURST_WINDOW_HOURS is not normal iteration (a human pushing a few follow-up commits tops out well
58+
* below this) -- it is the signature of a stuck-CI finalize loop or a sweep retry storm. Conservative on
59+
* purpose: an actively-iterated PR with several quick pushes should never trip this. */
60+
const REVIEW_BURST_THRESHOLD = 6;
61+
/** Rolling window the review-burst count is computed over. Short enough that the hourly ops-alerts cron catches
62+
* a live bleed within one or two ticks, not the 20+ hours it took a human to notice the incident this exists
63+
* to prevent from recurring. */
64+
const REVIEW_BURST_WINDOW_HOURS = 2;
65+
66+
/** One repo's outcome reports + the repo it covers — the input to the pure anomaly detector. `reviewBurst` is
67+
* optional so existing snapshot-fixture tests need not be touched; absent/null means "not computed", not
68+
* "healthy" -- the caller (runOpsAlerts/computeOpsStats) always populates it today. */
5269
export interface RepoOutcomeSnapshot {
5370
repoFullName: string;
5471
gatePrecision: GatePrecisionReport;
5572
calibration: OutcomeCalibration;
73+
reviewBurst?: { targetKey: string; count: number } | null | undefined;
5674
}
5775

5876
/**
@@ -93,6 +111,15 @@ export function detectOutcomeAnomalies(snapshot: RepoOutcomeSnapshot): string[]
93111
);
94112
}
95113

114+
// REVIEW BURST (#orb-ci-stuck-repeat / #orb-retry-storm): the same PR published far more review surfaces than
115+
// normal iteration ever produces within a short window -- catch a stuck-CI finalize loop or sweep retry storm
116+
// within this scan's own next tick instead of requiring a human to notice hours later.
117+
if (snapshot.reviewBurst && snapshot.reviewBurst.count >= REVIEW_BURST_THRESHOLD) {
118+
out.push(
119+
`review burst: ${snapshot.reviewBurst.targetKey} published ${snapshot.reviewBurst.count} review surfaces in the last ${REVIEW_BURST_WINDOW_HOURS}h — likely a stuck-CI finalize loop or retry storm, not normal iteration. Investigate why this PR keeps re-triggering a fresh review.`,
120+
);
121+
}
122+
96123
return out;
97124
}
98125

@@ -127,25 +154,28 @@ async function opsScanRepos(env: Env): Promise<string[]> {
127154
*/
128155
export async function runOpsAlerts(env: Env): Promise<Record<string, string[]>> {
129156
const found: Record<string, string[]> = {};
157+
const reviewBurstSinceIso = new Date(Date.now() - REVIEW_BURST_WINDOW_HOURS * 60 * 60 * 1000).toISOString();
130158
try {
131159
const repos = await opsScanRepos(env);
132160
for (const repoFullName of repos) {
133161
try {
134-
const [gatePrecision, calibration] = await Promise.all([
162+
const [gatePrecision, calibration, reviewBurst] = await Promise.all([
135163
loadGatePrecisionReport(env, repoFullName),
136164
buildRepoOutcomeCalibration(env, repoFullName),
165+
findHottestReviewTargetForRepo(env, repoFullName, reviewBurstSinceIso),
137166
]);
138-
const anomalies = detectOutcomeAnomalies({ repoFullName, gatePrecision, calibration });
167+
const anomalies = detectOutcomeAnomalies({ repoFullName, gatePrecision, calibration, reviewBurst });
139168
if (anomalies.length === 0) continue;
140169
found[repoFullName] = anomalies;
141-
// Structured log = gittensory's notify path (no Discord/operator webhook exists). One line per repo.
142-
console.warn(JSON.stringify({ ev: "ops_anomaly", repo: repoFullName, at: nowIso(), anomalies }));
170+
// Structured log = gittensory's notify path (no Discord/operator webhook exists) AND the Sentry path
171+
// (level:"error" + an `event` field reaches forwardStructuredLogToSentry). One line per repo.
172+
console.error(JSON.stringify({ level: "error", event: "ops_anomaly", repo: repoFullName, at: nowIso(), anomalies }));
143173
} catch (error) {
144-
console.warn(JSON.stringify({ ev: "ops_anomaly_repo_error", repo: repoFullName, message: errorMessage(error).slice(0, 200) }));
174+
console.error(JSON.stringify({ level: "error", event: "ops_anomaly_repo_error", repo: repoFullName, message: errorMessage(error).slice(0, 200) }));
145175
}
146176
}
147177
} catch (error) {
148-
console.warn(JSON.stringify({ ev: "ops_anomaly_error", message: errorMessage(error).slice(0, 200) }));
178+
console.error(JSON.stringify({ level: "error", event: "ops_anomaly_error", message: errorMessage(error).slice(0, 200) }));
149179
}
150180
return found;
151181
}
@@ -177,11 +207,13 @@ export interface OpsStatsPayload {
177207
export async function computeOpsStats(env: Env): Promise<OpsStatsPayload> {
178208
const repos = await opsScanRepos(env);
179209
const rows: OpsStatsRepoRow[] = [];
210+
const reviewBurstSinceIso = new Date(Date.now() - REVIEW_BURST_WINDOW_HOURS * 60 * 60 * 1000).toISOString();
180211
for (const repoFullName of repos) {
181212
try {
182-
const [gatePrecision, calibration] = await Promise.all([
213+
const [gatePrecision, calibration, reviewBurst] = await Promise.all([
183214
loadGatePrecisionReport(env, repoFullName),
184215
buildRepoOutcomeCalibration(env, repoFullName),
216+
findHottestReviewTargetForRepo(env, repoFullName, reviewBurstSinceIso),
185217
]);
186218
rows.push({
187219
repoFullName,
@@ -196,7 +228,7 @@ export async function computeOpsStats(env: Env): Promise<OpsStatsPayload> {
196228
discriminates: calibration.slop.discriminates,
197229
},
198230
recommendations: calibration.recommendations,
199-
anomalies: detectOutcomeAnomalies({ repoFullName, gatePrecision, calibration }),
231+
anomalies: detectOutcomeAnomalies({ repoFullName, gatePrecision, calibration, reviewBurst }),
200232
});
201233
} catch {
202234
/* a per-repo failure must not blank the whole feed */

test/unit/db-parsers.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
countRecentDeadLetters,
55
countRecentDeadLettersByType,
66
countRecentAuditEventsForActorAndTarget,
7+
findHottestReviewTargetForRepo,
78
hasAuditEventForDelivery,
89
getLatestScorePreview,
910
getRepoAuthorPullRequestHistory,
@@ -556,6 +557,33 @@ describe("database row parser hardening", () => {
556557
expect(await countRecentAuditEventsForActorAndTarget(env, "chatty", "github_app.review_nag_ping", "owner/repo#1", "2026-06-24T13:00:00.000Z")).toBe(0); // none after the cutoff → count(*) returns 0
557558
});
558559

560+
it("findHottestReviewTargetForRepo returns the PR with the most published surfaces in the window, scoped to ONE repo (#orb-ci-stuck-repeat)", async () => {
561+
const env = createTestEnv();
562+
const publish = (targetKey: string, createdAt: string) =>
563+
recordAuditEvent(env, { eventType: "github_app.pr_public_surface_published", actor: "contributor", targetKey, outcome: "completed", createdAt });
564+
// owner/repo#1: 3 publishes in-window -- the hottest target for this repo.
565+
await publish("owner/repo#1", "2026-06-24T10:00:00.000Z");
566+
await publish("owner/repo#1", "2026-06-24T10:05:00.000Z");
567+
await publish("owner/repo#1", "2026-06-24T10:10:00.000Z");
568+
// owner/repo#2: only 1 publish -- must not win over #1.
569+
await publish("owner/repo#2", "2026-06-24T10:00:00.000Z");
570+
// A DIFFERENT event type on the SAME PR must not count (the eventType filter).
571+
await recordAuditEvent(env, { eventType: "github_app.ai_review_cache_hit", actor: "contributor", targetKey: "owner/repo#1", outcome: "completed", createdAt: "2026-06-24T10:07:00.000Z" });
572+
// A DIFFERENT repo with an overlapping numeric suffix must not leak into this repo's count (the LIKE scope).
573+
await publish("owner/repo-fork#1", "2026-06-24T10:00:00.000Z");
574+
await publish("owner/repo-fork#1", "2026-06-24T10:05:00.000Z");
575+
await publish("owner/repo-fork#1", "2026-06-24T10:06:00.000Z");
576+
await publish("owner/repo-fork#1", "2026-06-24T10:07:00.000Z");
577+
578+
const hottest = await findHottestReviewTargetForRepo(env, "owner/repo", "2026-06-24T09:00:00.000Z");
579+
expect(hottest).toEqual({ targetKey: "owner/repo#1", count: 3 });
580+
581+
// A cutoff AFTER all the recorded publishes must find nothing.
582+
expect(await findHottestReviewTargetForRepo(env, "owner/repo", "2026-06-24T11:00:00.000Z")).toBeNull();
583+
// An unregistered/unpublished repo must find nothing.
584+
expect(await findHottestReviewTargetForRepo(env, "owner/nothing-here", "2026-06-24T09:00:00.000Z")).toBeNull();
585+
});
586+
559587
it("hasAuditEventForDelivery finds a matching deliveryId inside metadata_json, scoped to actor+eventType+targetKey (#2560)", async () => {
560588
const env = createTestEnv();
561589
await recordAuditEvent(env, {

0 commit comments

Comments
 (0)