Skip to content

Durable one-time terminal-final webhook delivery + atomic sequence reservation #1606

Description

@numarasSigmaSoftware

Follow-up to #1575 (delivery-webhook correctness). Two coupled correctness gaps in the delivery-webhook path that both require an outbox-style durable-delivery approach. They are out of scope for #1575 (focused test/correctness fixes), but the terminal-derivation and dedup/sequence machinery that PR touched surfaces them.

Gap A — terminal-final notification is not integrated into the lifecycle

derive_notification_type() (src/core/tools/_media_buy_status.py:140) has final branches for rejected/canceled/failed and a scheduled branch for paused, but these are unreachable via real entrypoints:

So the only statuses ever fed to the helper are active/completed.

Even completed → final is unreliable: media_buy_status_scheduler flips ended buys to persisted completed roughly every 60s (media_buy_status_scheduler.py:157), which removes them from the hourly webhook batch; and the widened 24h any-success dedup (delivery_webhook_scheduler.py:220) can suppress the final send when a scheduled report already succeeded.

Gap B — sequence allocation is neither atomic nor durably acknowledged

DeliveryRepository.get_max_sequence_number() (delivery.py:207) performs a plain MAX(success)+1 read, assigned at delivery_webhook_scheduler.py:294, and the external send happens before success is recorded.

  • Concurrent scheduler/manual requests can both read N, send, and emit N+1 (the manual admin trigger bypasses dedup: operations.py:626); WebhookDeliveryLog has no stream-sequence uniqueness constraint (models.py:2215).
  • After a successful HTTP send, _write_delivery_log() swallows DB errors (protocol_webhook_service.py:247) and still returns True, so a transient log-write failure lets the buyer receive N+1 while the next attempt re-emits N+1.

Fix direction

An authoritative per-media-buy sequence stream with transactional reservation and serialized / outbox delivery:

  • Reserve the sequence in a transaction; derive terminal intent before dedup; distinguish scheduled-period dedup from final delivery state; deliver once; record durably.
  • Do not swallow the success-log persistence failure.
  • A uniqueness constraint alone is insufficient because the external side effect currently happens first.
  • Requires a DB migration.

Tests to add (via real entrypoints, not helper-direct)

  • scheduled-success → flight-end → guaranteed one-time final (through the real scheduler ordering + dedup).
  • canceled / rejected / failed / paused terminal transitions → correct notification_type.
  • Concurrent scheduler + manual trigger → no duplicate sequence.
  • Post-HTTP success-log persistence failure → no reused sequence on recovery.

Also

Reconcile a pre-existing schema-key mismatch: production reads reporting_webhook.frequency while the pinned AdCP schema uses reporting_frequency. Fix production, fixtures, and schema together.

Metadata

Metadata

Type

No type

Projects

Status
Blocked

Relationships

None yet

Development

No branches or pull requests

Issue actions