Skip to content

fix(db): give the anomaly-alert port its own alert_dedup_claims table (was colliding with notification_deliveries) - #8976

Closed
kai392 wants to merge 2 commits into
JSONbored:mainfrom
kai392:fix/critical-issue-alert-dedup-table
Closed

fix(db): give the anomaly-alert port its own alert_dedup_claims table (was colliding with notification_deliveries)#8976
kai392 wants to merge 2 commits into
JSONbored:mainfrom
kai392:fix/critical-issue-alert-dedup-table

Conversation

@kai392

@kai392 kai392 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Summary

src/review/alerts.ts's runAnomalyAlerts writes hourly dedup claims with the shape
(id, project, target_id, notification_key, status) and ON CONFLICT(project, target_id, notification_key) DO NOTHING, but pointed those raw INSERTs at notification_deliveries — whose
real schema (migration 0031) has none of those columns and no such unique constraint. runAnomalyAlerts
has zero callers today, so it hasn't fired, but the moment it's wired to a cron path, every
Discord-notify invocation throws at the first INSERT (table notification_deliveries has no column named project).

  • Adds migration 0181_alert_dedup_claims.sql creating a distinctly-named alert_dedup_claims table
    with the real (project, target_id, notification_key) unique index the port needs.
  • Points both alerts.ts claim inserts at the new table.

Test plan

  • New regression test runs runAnomalyAlerts against the real migrated D1 (createTestEnv, not the mocked claim store the other send-path tests use) — both claim inserts succeed, both rows land in alert_dedup_claims, and a second same-hour run is throttled by the unique constraint (no second Discord POST)
  • Non-vacuity verified: reverting the table name back to notification_deliveries reproduces the exact production error (no column named project) and the test fails
  • npx vitest run test/unit/alerts.test.ts — 30/30 pass; tsc --noEmit clean

Closes #8901

@kai392
kai392 requested a review from JSONbored as a code owner July 26, 2026 14:46
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

RealDiligent and others added 2 commits July 26, 2026 22:50
… (was colliding with notification_deliveries)

src/review/alerts.ts's runAnomalyAlerts writes hourly dedup claims with the shape
(id, project, target_id, notification_key, status) and ON CONFLICT(project, target_id, notification_key),
but pointed those raw INSERTs at notification_deliveries — whose real schema (migration 0031) has none
of those columns and no such unique constraint. runAnomalyAlerts has zero callers today, so it hasn't
fired, but the moment it's wired to a cron path every Discord-notify invocation throws at the first
INSERT. Add migration 0181 creating a distinctly-named alert_dedup_claims table with the real
(project, target_id, notification_key) unique index, and point both inserts at it.

Closes JSONbored#8901

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@kai392

kai392 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

Closing to re-file: validate-code failed db:schema-drift:check because the new alert_dedup_claims table needs an entry in scripts/check-schema-drift.ts's RAW_SQL_ONLY_TABLES (it's raw-SQL-only). Cannot push a fix to a one-shot PR; re-filing corrected.

@kai392 kai392 closed this Jul 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(db): notification_deliveries table name collides with an unrelated schema used by the currently-unwired src/review/alerts.ts anomaly-alert port

2 participants