feat(listener): expire scheduled notifications past a configured deadline - #893
Merged
Abd-Standard merged 12 commits intoOct 3, 2026
Merged
Abd-Standard merged 12 commits into
Abd-Standard merged 12 commits into
Conversation
…Closes Core-Foundry#844. CHECK constraints on all closed status/state enums (scheduled notifications, execution attempts, processed events, idempotency, backpressure, rate-limit client types, notification archive); FK actions preserved deliberately (CASCADE for cleanup children, RESTRICT for template audit); PRAGMA foreign_keys verified at connect and in the migration runner. Migration 004 rebuilds tables in a single idempotent transaction with preflight audits that abort fail-closed on invalid legacy rows; valid legacy data passes through byte-identical (legacy-shape test asserts survival, clean foreign_key_check, and orphan/status/duplicate rejection). Constraints mirrored in schema.sql and archive-schema.sql for fresh-install parity. Also fixes a latent migration-runner defect: callback-based sqlite3 run/all were awaited as promises, undermining transaction/rollback guarantees. Deliberately NOT added: UNIQUE(notification_id, attempt) — the dead-letter retry path resets retry_count, so attempts legitimately repeat.
…line. Closes Core-Foundry#840. Adds expires_at (explicit override > NOTIFICATION_DEFAULT_TTL_SECONDS > never), EXPIRED terminal status persisted and enforced via extended CHECKs in migration 005 (scheduled + archive tables, preserving 004 constraints), expiry gates before provider dispatch in both scheduled and retry loops, terminal/archive/cleanup handling, API-boundary normalization of ISO/epoch expiry, focused tests, operator docs. Stacked on Core-Foundry#892 so merge order is irrelevant.
|
@najeebullahii Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
…schema refactor, dead-letter isolation + expiration wiring)
…cations rebuild. The bootstrap schema gained deduplication_key via merged 003-notification-deduplication-key after 005 was written; the rebuild omitted it, which would have dropped the column on existing DBs. Column now copied verbatim into the _v005 CREATE and the INSERT/SELECT lists, with a legacy-survival assertion and a bootstrap-vs-rebuild column-parity test to catch future drift. Also fixed a pre-existing missing parenthesis in getRows callback that blocked Prettier formatting.
…ough 004 rebuild (same drift class as 005 fix 93e25eb) with legacy-survival + bootstrap parity assertions; fix latent Database.connect bootstrap hang (handle assigned after first dereference) exposed by merge with main
…cation-expiration
…ource, not final bootstrap. Migration 005 owns expires_at, so the intermediate post-004 state legitimately lacks it; asserting against bootstrap would go red on main once Core-Foundry#893 merges. 004's invariant is exact preservation of its source column set.
…cation-expiration
… expiry gate preserved)
najeebullahii
force-pushed
the
feat/840-notification-expiration
branch
from
October 3, 2026 10:39
18767a0 to
8cca369
Compare
…PC/circuit-breaker config, retry rework; DEAD_LETTERED added to 004 CHECKs for bootstrap/rebuild parity)
…RED parity; stack consistency)
najeebullahii
force-pushed
the
feat/840-notification-expiration
branch
from
October 3, 2026 11:02
9ad19f8 to
51d6d7f
Compare
Contributor
Author
|
Union scope note (push 51d6d7f): main has since merged a DEAD_LETTERED terminal status, RPC-fallback/circuit-breaker config, and a retry rework. The merge unions restore main's config loaders and de-duplicate the retry path; migration CHECK enums include DEAD_LETTERED wherever the repository writes it (bootstrap/rebuild parity, with explicit test coverage), while EXPIRED remains owned by 005. Focused suites green (004: 4/4, 005: 3/3, config: 58/58, scheduler+api+retry+dead-letter: 136/136). |
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
2 participants
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.
Closes #840.
Changes:
expires_at(nullable DATETIME) onscheduled_notifications: explicit per-notification expiration normalized at the API boundary (ISO-8601 or epoch), elseNOTIFICATION_DEFAULT_TTL_SECONDS(absent/0 = never expire), else NULL; carried into archived rows.005rebuildsscheduled_notificationsand the archive table in one idempotent, fail-closed transaction: adds the column, extends both status CHECKs with EXPIRED, preserves every 004 constraint/index/trigger; mirrored inschema.sqlandarchive-schema.sqlfor fresh installs; legacy rows survive with NULL expiration.Testing: API validation tests for explicit/derived/absent expiry; scheduler and retry tests asserting zero provider calls for expired rows and normal delivery for non-expired/NULL rows (distinct recipients per batch); EXPIRED terminal + archived; migration 005 test asserting legacy survival, extended CHECK accept/reject, 004 constraints intact, idempotent repeat, fail-closed abort on bad legacy data.
Stacking note: based on #892 (migration 004) because 005 extends 004's CHECKs and must preserve its rebuilds (including the archive table); stacking makes merge order irrelevant — when #892 merges, this diff collapses to the 005 delta.
Pre-existing baseline (untouched): full
npm testred on main (~55 failing suites: request-id.ts syntax error, Stellar XDR test setup, config secret validation);npm run lintfails on index.ts/request-id.ts/security-headers.ts/discord-notification.ts; one scheduler assertion (retryCountexpected 3, actual 2) is pre-existing on main and left as-is; lockfile out of sync with package.json (this PR modifies neither).