ADR-018: Notify external-ID column projection
Formerly
docs/architecture/ADR-NOTIFY-EXTERNAL-ID-COLUMNS.md.
| Field | Value |
|---|---|
| Status | Accepted |
| Date | 2026-04-29 |
| Sprint | SPRINT_20260429_002_Notify_ack_bridge_durability (archived) |
| Drift ticket | DRIFT-NOTIFIER-ACKBRIDGE-001 |
| Authors | Notify guild |
For: engineers working on the Stella Ops Notify ack-bridge and anyone diagnosing why an inbound PagerDuty/OpsGenie acknowledgement fails to resolve its incident after a database restart. This ADR records why external provider identifiers are persisted as first-class columns on notify.deliveries rather than read out of JSON metadata at lookup time.
Context
Inbound acknowledgement webhooks from PagerDuty and OpsGenie carry an external identifier (PagerDuty dedup_key, OpsGenie alias) and need to resolve to the matching tenant + incident inside Stella Ops so the ack-bridge can transition the escalation state. The resolver (PostgresAckExternalIdResolver) issues a SQL join against notify.deliveries and notify.channels:
SELECT delivery.tenant_id, delivery.event_payload, delivery.id, delivery.created_at
FROM notify.deliveries delivery
JOIN notify.channels channel
ON channel.id = delivery.channel_id
AND channel.tenant_id = delivery.tenant_id
WHERE delivery.external_id = @externalId
AND channel.channel_type = @expectedType
ORDER BY delivery.created_at DESC
LIMIT 8;
The bug captured in DRIFT-NOTIFIER-ACKBRIDGE-001 was that the durable persistence mapper (NotifyPersistenceModelMapper.ToDeliveryEntity) never copied Metadata["externalId"] into DeliveryEntity.ExternalId. The external_id column stayed NULL forever; the value lived only inside event_payload.metadata.externalId (JSONB). The resolver’s join therefore returned no rows after a Postgres restart or a connection-pool drop — the moment the in-process cache that previously held a column-shaped mapping went away. The originally-failing test AckBridge_ResolvesPagerDutyExternalIdFromPostgresAfterRestart was tagged Skip until this fix landed.
Decision
Promote externalId and channelId to first-class persisted columns on notify.deliveries rather than continuing to address them via JSON metadata keys.
Specifically:
NotifyDelivery(the canonical model) gains nullableExternalIdandChannelIdproperties. The constructor backfills these fromMetadata["externalId"]/Metadata["channel"]so existing callers that pass the values via metadata keep working — but the column gets populated either way.NotifyPersistenceModelMapper.ToDeliveryEntitywritesentity.ExternalId = delivery.ExternalId. The previous code path left that column NULL; this is the root cause fix.- The persistence layer (
DeliveryRepository.UpsertAsync) already projectsexternal_idto the column via@p_external_idwithCOALESCE(EXCLUDED.external_id, notify.deliveries.external_id)semantics on conflict, so the column write is preserved across upserts. - Migration
009_deliveries_external_id_columns.sql(idempotent, embedded resource):- Re-asserts the columns exist (
ADD COLUMN IF NOT EXISTS). - Backfills
external_idfromevent_payload -> 'metadata' ->> 'externalId'for in-flight rows. - Adds the partial composite index
idx_notify_deliveries_external_id (channel_id, external_id) WHERE external_id IS NOT NULLfor the resolver join.
- Re-asserts the columns exist (
- The resolver retains a metadata-fallback path for one release cycle to handle rows written by an older mapper that the migration UPDATE missed (zero-downtime rollouts where rows are written by an older node after the migration ran). Hits are tracked via the
notifier_ack_bridge_metadata_fallback_totalcounter so operators can see when the fallback can be deleted.
Alternatives considered
A. Keep the JSON metadata path; fix the bug at read time
Teach the resolver to read delivery.event_payload -> 'metadata' ->> 'externalId' directly instead of the column.
Rejected:
- JSONB-key access cannot be efficiently indexed for the resolver’s join pattern. A functional / expression index on a deeply nested JSONB path is brittle, doesn’t compose well with the partition-aware planner, and doesn’t survive serializer changes (
JsonNamingPolicy.CamelCase, property ordering, etc.). - Validates the wrong precedent. External provider identifiers are contract-shaped lookup keys, not loose runtime metadata; treating them as the latter caused the bug.
- Deserialization cost on every resolver lookup is non-trivial and unnecessary.
B. Reverse the projection: keep externalId only as a column, drop it from metadata
Strip Metadata["externalId"] and Metadata["channel"] from NotifyDelivery on every write.
Deferred (not rejected): This is the correct end state, but doing it in the same change as the column-write fix risks breaking external consumers (UI, CLI, exports) that may still read those metadata keys. The follow-up sprint SPRINT_20260429_021_Notifier_ack_bridge_remove_metadata_fallback.md is gated on the notifier_ack_bridge_metadata_fallback_total counter staying at zero for at least 7 consecutive days post-deployment of one full release cycle that includes migration 009.
C. Store the mapping in a sidecar table
Introduce a dedicated notify.delivery_external_ids table.
Rejected:
- Splits the lookup across two writes; harder to keep consistent under partial failures.
- Adds a foreign-key constraint that the partitioned
notify.deliveriesparent table cannot enforce on a child schema (Postgres FK limits on partition tables). - The data is naturally a single-attribute extension of the delivery row, not a separate concept. Sidecar storage solves a problem we do not have.
Consequences
Positive
- Durability: the resolver result survives Postgres restart, pgbouncer reset, app process recycle. The original failing test (
AckBridge_ResolvesPagerDutyExternalIdFromPostgresAfterRestart) is un-skipped and green. - Performance: lookup uses an indexed column on the partitioned table. EXPLAIN ANALYZE against a 1000-row dataset shows a partition-local index scan with sub-millisecond execution times for both the single-key (
external_idpartial index) and join-side (channel_id, external_idcomposite) variants. - Testable: the column projection is round-trip checked in the forward-compat test and the backfill UPDATE is exercised in the backfill-compat test.
Negative
- Migration cost: the backfill UPDATE on a very large
notify.deliveriestable could lock the partition for minutes. The migration’sWHERE external_id IS NULL AND event_payload -> 'metadata' ? 'externalId'predicate touches only un-migrated rows; for installations with multi-million-row delivery tables, the runbook recommends running the backfill in chunks via a maintenance-window job rather than at startup. - Compatibility window: during the deprecation window, both write paths coexist (column AND metadata key), so the
event_payloadJSONB carries redundant data for one release cycle. The cleanup is tracked separately.
Neutral
- The
NotifyDeliverymodel gains two nullable fields. Existing code that doesn’t construct deliveries via the new parameters keeps working unchanged because of the metadata-backfill in the constructor.
Verification
- Migration
009_deliveries_external_id_columns.sqlis embedded inStellaOps.Notify.Persistence.csprojvia the existing<EmbeddedResource Include="Migrations\**\*.sql" />glob and is picked up by bothAddStartupMigrationsand the test fixtures’ApplyMigrationsFromAssemblyAsync. Idempotency is asserted byNotifyMigrationTests.ApplyMigrations_Twice_IsIdempotent. - Three integration tests cover the durability contract:
AckBridge_ResolvesPagerDutyExternalIdFromPostgresAfterRestart(originally-failing, restored).AckBridge_ResolvesPagerDutyExternalId_ViaColumnPath_AfterScopeDrop(forward-compat).AckBridge_ResolvesPagerDutyExternalId_ViaMetadataFallback_WhenColumnIsNull(backfill-compat).
- Test runs (2026-04-29):
StellaOps.Notifier.Tests573 pass / 0 skip;StellaOps.Notify.Persistence.Tests112 pass.
Cross-references
- ADR index
- DRIFT-NOTIFIER-ACKBRIDGE-001
- SPRINT_20260429_002_Notify_ack_bridge_durability (archived)
- SPRINT_20260429_021_Notifier_ack_bridge_remove_metadata_fallback (archived)
- Notify module architecture (deliveries schema section)
- Notifier README (“Operations: ack-bridge external-ID durability” runbook)
- Migration
src/Notify/__Libraries/StellaOps.Notify.Persistence/Migrations/009_deliveries_external_id_columns.sql
