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
Closed
Conversation
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
… (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>
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
src/review/alerts.ts'srunAnomalyAlertswrites hourly dedup claims with the shape(id, project, target_id, notification_key, status)andON CONFLICT(project, target_id, notification_key) DO NOTHING, but pointed those rawINSERTs atnotification_deliveries— whosereal schema (migration 0031) has none of those columns and no such unique constraint.
runAnomalyAlertshas 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).0181_alert_dedup_claims.sqlcreating a distinctly-namedalert_dedup_claimstablewith the real
(project, target_id, notification_key)unique index the port needs.alerts.tsclaim inserts at the new table.Test plan
runAnomalyAlertsagainst the real migrated D1 (createTestEnv, not the mocked claim store the other send-path tests use) — both claim inserts succeed, both rows land inalert_dedup_claims, and a second same-hour run is throttled by the unique constraint (no second Discord POST)notification_deliveriesreproduces the exact production error (no column named project) and the test failsnpx vitest run test/unit/alerts.test.ts— 30/30 pass;tsc --noEmitcleanCloses #8901