Skip to content

feat(listener): enforce database integrity constraints on core tables - #892

Merged
Abd-Standard merged 5 commits into
Core-Foundry:mainfrom
najeebullahii:feat/844-db-integrity-constraints
Oct 3, 2026
Merged

Abd-Standard merged 5 commits into
Core-Foundry:mainfrom
najeebullahii:feat/844-db-integrity-constraints

Conversation

@najeebullahii

Copy link
Copy Markdown
Contributor

Closes #844.

Changes:

  • CHECK constraints restricting every closed status/state domain to its exact code-defined enums: scheduled notifications (PENDING/PROCESSING/COMPLETED/FAILED/CANCELLED), execution attempts (SUCCESS/FAILED/RETRY), processed events (PROCESSED/SKIPPED/ERROR), idempotency keys (PROCESSED/EXPIRED), backpressure (ACTIVATED/DEACTIVATED), rate-limit client types (IP/API_KEY), and notification archive terminal states.
  • Required relationships enforced: existing parent-child FKs retained with deliberate actions (CASCADE for execution-log/dead-letter/idempotency children the archive worker deletes; RESTRICT for template audit history); PRAGMA foreign_keys=ON verified at Database.connect() AND independently in the migration runner, with tests proving enforcement.
  • Invalid states rejected at the storage layer; existing semantic unique keys (processed fingerprint, cursor contract, idempotency key, dead-letter notification id) preserved and pre-audited.
  • Migration 004 uses the SQLite table-rebuild pattern in a single idempotent transaction: preflight audits count would-be violations and ABORT with an actionable table+count error (fail closed) before any DDL; rows copy unchanged; indexes/triggers recreated. Mirrored identically in schema.sql and archive-schema.sql for fresh-install parity.
  • Fixes a latent migration-runner defect: callback-based sqlite3 run/all were awaited as if promise-returning, which let connections close before rollback completed and undermined transaction guarantees. Narrow promisify wrapper added.
  • Deliberately NOT added: UNIQUE(notification_id, attempt) — the dead-letter retry path resets retry_count, so (notification, attempt) pairs legitimately repeat. Documented in the migration comments.

Testing: legacy-shape migration test seeds pre-constraint rows across every rebuilt table (including archive), runs 004 via MigrationRunner, and asserts byte-identical survival, zero PRAGMA foreign_key_check violations, rejection of orphan/out-of-enum/duplicate-key writes, fail-closed abort on invalid legacy status, repeatability, and rollback-audit behaviour. Focused DB/dead-letter/dedup suites unaffected. New files Prettier-clean.

Notes: migration numbered 004 and independent of delivery_receipts to avoid collision with in-flight PR #889 (which adds 003). Pre-existing baseline untouched: full npm test red on main (~55 failing suites: request-id.ts syntax error, Stellar XDR test setup, config secret validation); npm run lint fails on index.ts/request-id.ts/security-headers.ts/discord-notification.ts; lockfile out of sync with package.json (this PR modifies neither).

…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.
@drips-wave

drips-wave Bot commented Sep 29, 2026

Copy link
Copy Markdown

@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! 🚀

Learn more about application limits

…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
…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.
@najeebullahii

Copy link
Copy Markdown
Contributor Author

CI baseline (verified at upstream/main HEAD): migrations and verify-frozen-lockfiles both fail on main itself — the former due to the duplicate-003 numbering collision (four files claiming migration 003, including merged #889), the latter due to the pre-existing lockfile/package.json desync. This PR's focused suites are green (004: 3/3; stacked 004+005: 6/6) and it modifies no package manifests.

…PC/circuit-breaker config, retry rework; DEAD_LETTERED added to 004 CHECKs for bootstrap/rebuild parity)
@najeebullahii

Copy link
Copy Markdown
Contributor Author

Union scope note (push 8368708): main has since merged a DEAD_LETTERED terminal status, RPC-fallback/circuit-breaker config, and a retry rework. The merge union restores main's config loaders and de-duplicates the retry path; migration CHECK enums now include DEAD_LETTERED wherever the repository writes it (bootstrap/rebuild parity, with explicit test coverage). Archive CHECK correctly remains limited to COMPLETED, FAILED, CANCELLED since the archive service only archives those terminal states. Focused suites green (004: 4/4, config: 58/58, dead-letter+retry: 79/79).

@Abd-Standard
Abd-Standard merged commit 51d6d7f into Core-Foundry:main Oct 3, 2026
1 of 5 checks passed
Abd-Standard pushed a commit that referenced this pull request Oct 3, 2026
…line. Closes #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 #892 so merge order is irrelevant.
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.

Add Database Data Integrity Constraints

2 participants