feat(webhook): dedupe deliveries with logicalEventId

[telivity-otaip]

Summary

Deduplicates webhook HTTP deliveries by persisted logical event id so crash-safe re-enqueue and replay do not create duplicate delivery rows or duplicate subscriber POSTs.

Changes

• Migration 0022_webhook_logical_event_id.sql — logical_event_id column + unique index on (property_id, subscription_id, logical_event_id)
• Migration runner — pnpm db:migrate now runs run-migrations.ts: push-schema baseline (0001–0021) + tracked SQL from 0022 onward via schema_migrations ledger (transactional, once-only)
• logical_event_id DDL removed from push-schema.ts; upgrade path is migration 0022 only
• dist/migrations/ shipped on build for Docker/Render deploys
• WebhookDeliveryService.enqueue() accepts optional logicalEventId; replays reuse the same row and stable X-HAIP-Event-Id header
• WebhookService.dispatchPersisted() forwards logical event id to async listeners
• Connect wiring: ConnectEventsService forwards logicalEventId into enqueue() so agent webhook subscriptions dedupe end-to-end
• Concurrency/replay tests for insert races and partial unique-index conflicts
• PostgreSQL migration integration tests (fresh DB, pre-0022 upgrade, ledger idempotency, unique-index enforcement)
• docs/webhooks.md — X-HAIP-Event-Id is the logical event uuid when present, otherwise the delivery row uuid (legacy events)

How to test

pnpm build
pnpm db:migrate
pnpm --filter @telivityhaip/database test
pnpm --filter @telivityhaip/api test -- src/modules/webhook/
pnpm --filter @telivityhaip/api test -- src/modules/connect/connect-events.service.spec.ts

Contributor credit

@agustinjch

[agustinjch]

Thanks — the database constraint and conflict-safe recovery are a good foundation. I checked the PR at 5cec622; typecheck and the focused webhook suite pass (12 tests).

I found three things to address before merge:

1. packages/database/src/push-schema.ts adds the new column/index through sql.raw(...). The repository rules explicitly allow raw SQL only in migration files. 0022_webhook_logical_event_id.sql is the correct place for this DDL; please avoid adding the duplicate raw-SQL path to push-schema.ts (or formally establish a documented exception before extending it).
2. The PR currently does not provide end-to-end deduplication on its own. dispatchPersisted() adds logicalEventId, but the real ConnectEventsService listener still calls WebhookDeliveryService.enqueue() without forwarding it. PostgreSQL therefore stores NULL and the unique index does not deduplicate those deliveries. If feat(booking-requests): add optional booking-requests module #348 intentionally supplies that wiring, please describe feat(webhook): dedupe deliveries with logicalEventId #350 as prerequisite infrastructure and add an integration test in feat(booking-requests): add optional booking-requests module #348 covering dispatchPersisted -> listener -> one delivery row; otherwise wire it here.
3. The core webhook documentation still describes X-HAIP-Event-Id as a delivery UUID, while the implementation now emits the logical event ID when present. Please update the documented contract.

The concurrency/replay tests around the new row identity look good. My blocker here is mainly the raw-SQL violation and making the actual scope of the deduplication explicit.

[telivity-otaip]

Thanks for the review — all three blockers addressed on 73b42b1 (current head).

1. push-schema duplicate DDL
Removed the ALTER TABLE / CREATE UNIQUE INDEX duplicates from the push-schema alter block. Upgrade path is migration 0022_webhook_logical_event_id.sql only. Fresh docker/demo installs still get logical_event_id from the CREATE TABLE webhook_deliveries definition (same pattern as other push-schema tables).

2. End-to-end deduplication
Wired here: ConnectEventsService.handleEvent() forwards payload.logicalEventId to WebhookDeliveryService.enqueue() when present. Added coverage in connect-events.service.spec.ts (forwards logicalEventId to WebhookDeliveryService for persisted dedup).

3. Documentation
Updated docs/webhooks.md: X-HAIP-Event-Id is the logical event uuid when present, otherwise the delivery row uuid for legacy events.

PR description updated with scope and push-schema note. Ready for your re-review.

[agustinjch]

@telivity-otaip Thanks for addressing the earlier points. The remaining blocker is that pnpm db:migrate still runs push-schema.ts, but that path does not execute 0022_webhook_logical_event_id.sql.

My preferred fix is:

1. Make the core migrate command actually execute numbered SQL migrations.
2. Since the legacy schema pusher already incorporates migrations 0001-0021, establish those as the baseline and begin tracked execution with 0022.
3. Record applied migrations in a migration ledger and execute each new migration once, transactionally.
4. Remove the new logical_event_id and unique-index DDL from push-schema.ts; keep the new DDL exclusively in 0022_webhook_logical_event_id.sql.
5. Ensure migration SQL files are present in the deployment/package path used by the migrate command.

Please add real PostgreSQL coverage for:

• a fresh database
• an existing pre-0022 database
• running the migration twice
• rejection of duplicate (property_id, subscription_id, logical_event_id) values

I would not try to rewrite all historical raw SQL in this PR. A sensible boundary is to baseline the legacy pusher and require all new schema evolution from 0022 onward to go through the real migration runner. That resolves this regression and stops adding new raw DDL outside migrations.

[telivity-otaip]

Thanks @agustinjch — all five migration-runner points addressed on 89cf718.

1. Core migrate executes numbered SQL migrations
pnpm db:migrate now runs run-migrations.ts (also wired in Docker, Render preDeploy, release smoke).

2. Baseline 0001–0021 / tracked from 0022
Push-schema remains the legacy idempotent baseline; the runner applies .sql files with version ≥ 0022.

3. Migration ledger + transactional once-only execution
New schema_migrations table records applied versions; each pending file runs inside sql.begin() and is skipped on re-run.

4. logical_event_id removed from push-schema
Column/index DDL lives exclusively in 0022_webhook_logical_event_id.sql. Also guarded push-schema CLI so importing it from the runner does not double-execute baseline DDL.

5. SQL files ship in deployment path
tsup onSuccess copies src/migrations → dist/migrations; production entrypoint is node packages/database/dist/run-migrations.js.

PostgreSQL coverage added (migration-runner.spec.ts):

• Fresh database → 0022 applied, column + unique index present
• Pre-0022 database (push-schema only) → upgrade via 0022
• Second migrate pass → ledger skip (no re-apply)
• Duplicate (property_id, subscription_id, logical_event_id) → 23505 unique violation

PR #350 scope verified intact: ConnectEventsService forwards logicalEventId, webhook docs updated, webhook + connect specs pass (27 + 5 migration tests).

Ready for re-review.