Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions migrations/0181_alert_dedup_claims.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
-- Dedicated dedup-claim store for the anomaly-alerter (#8901). src/review/alerts.ts's
-- `runAnomalyAlerts` throttles Discord alerts by INSERT ... ON CONFLICT(project, target_id,
-- notification_key) DO NOTHING against a claim table whose columns are (project, target_id,
-- notification_key) — a completely different shape than the migrated `notification_deliveries`
-- badge read-model (dedup_key/channel/recipient_login/...). The port was written against
-- `notification_deliveries` by name, so the moment it's wired to a cron path it would throw on its
-- first INSERT (no such columns / no such unique constraint). Give it its own table with the exact
-- (project, target_id, notification_key) unique index its ON CONFLICT target needs.
CREATE TABLE IF NOT EXISTS alert_dedup_claims (
id TEXT PRIMARY KEY,
project TEXT NOT NULL,
target_id TEXT NOT NULL,
notification_key TEXT NOT NULL,
status TEXT NOT NULL DEFAULT 'sent',
created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP
);

CREATE UNIQUE INDEX alert_dedup_claims_project_target_key_unique
ON alert_dedup_claims(project, target_id, notification_key);
1 change: 1 addition & 0 deletions scripts/check-schema-drift.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const MIGRATIONS_DIR = process.env.CHECK_SCHEMA_DRIFT_DIR || "migrations";
// table here without also confirming it is genuinely raw-SQL-only is a reviewer-visible diff, not a silent
// gap this check would otherwise catch.
export const RAW_SQL_ONLY_TABLES: Set<string> = new Set([
"alert_dedup_claims",
"ams_instances",
"ams_signals",
"contributor_gate_history",
Expand Down
4 changes: 2 additions & 2 deletions src/review/alerts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ export async function runAnomalyAlerts(env: Env, config: AlertAgentConfig, deps:
// computes + maybe alerts; the other 59 short-circuit here before touching D1.
const hourBucket = new Date().toISOString().slice(0, 13); // YYYY-MM-DDTHH
const checkClaim = await storage(env).prepare(
`INSERT INTO notification_deliveries (id, project, target_id, notification_key, status)
`INSERT INTO alert_dedup_claims (id, project, target_id, notification_key, status)
VALUES (?, ?, '__healthcheck__', ?, 'sent')
ON CONFLICT(project, target_id, notification_key) DO NOTHING`,
)
Expand All @@ -246,7 +246,7 @@ export async function runAnomalyAlerts(env: Env, config: AlertAgentConfig, deps:
// Throttle: claim a per-(condition-set, hour) key so a repeated condition alerts at most hourly.
const key = await sha256Hex(`anomaly:${anomalies.join("|")}:${hourBucket}`);
const claim = await storage(env).prepare(
`INSERT INTO notification_deliveries (id, project, target_id, notification_key, status)
`INSERT INTO alert_dedup_claims (id, project, target_id, notification_key, status)
VALUES (?, ?, '__anomaly__', ?, 'sent')
ON CONFLICT(project, target_id, notification_key) DO NOTHING`,
)
Expand Down
42 changes: 38 additions & 4 deletions test/unit/alerts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
detectAnomalies,
runAnomalyAlerts,
} from "../../src/review/alerts";
import { createTestEnv } from "../helpers/d1";

const healthy: AgentHealth = {
byStatus: {},
Expand Down Expand Up @@ -174,10 +175,11 @@ describe("runAnomalyAlerts guards", () => {
});

// ── runAnomalyAlerts send path ───────────────────────────────────────────────────────────────────────
// loopover's migrated `notification_deliveries` table is the badge read-model — a DIFFERENT schema than
// the native port's claim SQL (project/target_id/notification_key). So we emulate the claim store: the
// INSERT ... ON CONFLICT(project, target_id, notification_key) DO NOTHING returns changes=1 the first time a
// (project, target_id, notification_key) tuple is seen and changes=0 on a repeat (the per-hour throttle).
// The port claims dedup slots in `alert_dedup_claims` (its own (project, target_id, notification_key)
// table — #8901), so we emulate the claim store: the INSERT ... ON CONFLICT(project, target_id,
// notification_key) DO NOTHING returns changes=1 the first time a (project, target_id, notification_key)
// tuple is seen and changes=0 on a repeat (the per-hour throttle). The real-schema test at the bottom of
// this file exercises the same INSERTs against the actual migrated table.
function claimEnv(extra: Record<string, unknown> = {}): Env {
const seen = new Set<string>();
return {
Expand Down Expand Up @@ -361,3 +363,35 @@ describe("runAnomalyAlerts — send path", () => {
expect(fetchSpy).not.toHaveBeenCalled(); // but the anomaly claim conflicted → no POST
});
});

// ── real alert_dedup_claims schema (#8901) ─────────────────────────────────────────────────────────────
// Regression guard for the latent schema collision: alerts.ts used to INSERT into `notification_deliveries`
// (project/target_id/notification_key columns + ON CONFLICT on that tuple), but the migrated
// `notification_deliveries` is the badge read-model with a totally different shape, so the first real INSERT
// would throw. These run the ACTUAL INSERT ... ON CONFLICT against a fully-migrated DB (createTestEnv applies
// migrations/**, including 0181_alert_dedup_claims.sql) to prove the write path now succeeds end-to-end.
describe("runAnomalyAlerts — real alert_dedup_claims schema (#8901)", () => {
afterEach(() => vi.unstubAllGlobals());

it("lands both dedup claims in the real migrated table and POSTs once, then throttles the repeat", async () => {
const fetchSpy = vi.fn(async () => new Response(null, { status: 204 }));
vi.stubGlobal("fetch", fetchSpy);
const env = createTestEnv();
const config = { slug: "ac", features: { discordNotify: true }, secrets: {}, discordWebhookUrl: WEBHOOK } as AlertAgentConfig;
const deps: AnomalyAlertDeps = { computeAgentHealth: async () => anomalousHealth, computeCalibration: async () => driftCal };

await runAnomalyAlerts(env, config, deps);
expect(fetchSpy).toHaveBeenCalledTimes(1); // the INSERTs succeeded against the real (project, target_id, notification_key) schema

const rows = await env.DB.prepare("SELECT project, target_id, status FROM alert_dedup_claims ORDER BY target_id")
.all<{ project: string; target_id: string; status: string }>();
expect(rows.results.map((r) => r.target_id)).toEqual(["__anomaly__", "__healthcheck__"]);
expect(rows.results.every((r) => r.project === "ac" && r.status === "sent")).toBe(true);

// A second run the same hour re-hits the per-hour healthcheck claim → ON CONFLICT DO NOTHING → no new POST.
await runAnomalyAlerts(env, config, deps);
expect(fetchSpy).toHaveBeenCalledTimes(1);
const after = await env.DB.prepare("SELECT count(*) AS n FROM alert_dedup_claims").first<{ n: number }>();
expect(after?.n).toBe(2); // still exactly the two claims — the conflicting re-insert added nothing
});
});