Skip to content

fix(notify): make notification tests, delivery passes, and queued alerts reliable - #829

Merged
crypt0rr merged 1 commit into
mainfrom
fix/notification-delivery-correctness
Sep 25, 2026
Merged

crypt0rr merged 1 commit into
mainfrom
fix/notification-delivery-correctness

Conversation

@crypt0rr

Copy link
Copy Markdown
Owner

What changes

  • One test message per destination. The global notification test sends one message to each deployment destination and each enabled web-managed destination. This applies to both the console and edgewatch notify test. The legacy SHA-256 alias of a deployment URL still routes outbox rows created before opaque IDs, but it no longer adds a second test send.
  • Locked destinations fail the test. A web-managed destination is locked when the notification key is missing, replaced or unreadable. If an enabled destination is locked, the global test now fails with ErrManagedNotificationLocked. The error names the destination ID and lock reason, never its URL.
    • edgewatch notify test prints {"tested","failed","locked"}, exits non-zero and records operation=global status=failed.
    • POST /api/v1/notifications/test returns 503 notification_key_unavailable instead of 200 {"sent":0}. The success response shape is unchanged.
    • Paused destinations and an empty configuration still pass.
  • The delivery pass window no longer kills sends. The daemon's 90-second window (Notifier.DrainWithin) now only stops new claims and dispatch.
    • Sends already running finish under the daemon context and the 15-second provider timeout. A slow but healthy provider is no longer killed, charged a deferral or delayed by 30 minutes.
    • Claims that never started are still released without using a retry budget.
    • Shutdown still cancels sends in flight.
  • Schema 49: a rename no longer drops a queued alert. The new column managed_notifications.credential_revision records the revision that last changed a destination's credentials. When the alert's event transaction commits:
    • If the destination's credentials have not changed since the alert captured it, for example after a rename, the alert is queued under the current revision.
    • If the credentials changed or the destination was deleted, the alert is still discarded rather than sent to new credentials. The discard is audited as notifications.pending_discarded.
    • Existing rows are backfilled to their current revision.
  • notification.key follows the database file. The default key path now comes from the normalized database file (Store.FilePath). A database: file:… URI no longer creates the key under the working directory. DefaultKeyPath accepts every DSN form, and memory databases have no key path.
  • Docs. README.md and SECURITY.md describe:
    • the new notify test output and exit status;
    • rename versus credential-change behavior for queued alerts;
    • where the default key lives;
    • schema 49.

Why

Validation

  • Every issue has a regression test that fails on the previous code for the reported reason.
  • go test -race -count=1 passes for internal/notify, internal/store, internal/app, internal/web and cmd/edgewatch.
  • go vet ./..., gofmt and the pinned golangci-lint (--new-from-rev=origin/main) are clean.
  • ./scripts/check-schema-docs.sh reports schema 49.
  • Go changed-line coverage is 92.35%.
  • The migration 49 table guard matches the original managed_notifications definition.
  • Frontend and e2e suites were not run: this PR changes no frontend code or browser flows.

Compatibility

  • Schema 49 is forward-only. An older binary refuses the upgraded database.
  • The console notification test returns 503 when an enabled web-managed destination is locked.
  • edgewatch notify test now prints a JSON summary and exits non-zero in that case.

Closes #776
Closes #777
Closes #778
Closes #779
Closes #780

…rts reliable

The global notification test now sends one message per destination. The
legacy SHA-256 alias of each deployment URL still routes outbox rows created
before opaque IDs, but it no longer adds a second test send.

An enabled web-managed destination that is locked by a missing, replaced,
or unreadable key now fails the global test with
ErrManagedNotificationLocked. `edgewatch notify test` prints the tested,
failed, and locked counts, exits non-zero, and audits status=failed. The
console test returns 503 notification_key_unavailable instead of
{"sent":0}. Paused destinations and an empty configuration still pass.

The daemon's 90-second delivery pass window now bounds only claiming and
dispatch (Notifier.DrainWithin). Sends already in flight finish under the
daemon context and their own provider timeout, so a slow but healthy
provider is no longer killed, charged a deferral, and delayed 30 minutes.
Undispatched claims are still released without using a retry budget, and
shutdown still cancels in-flight sends.

Schema 49 adds managed_notifications.credential_revision. It records the
revision that last changed a destination's credentials. When an alert's
event transaction commits, a managed destination captured at an earlier
revision is queued under the current revision if its credentials have not
changed since, so a rename no longer drops the alert. A credential change or
deletion still discards the intent and now records it as
notifications.pending_discarded. Existing rows start with their current
revision.

The default notification.key path is now derived from the normalized
database file (Store.FilePath) instead of the DSN. A file: URI database no
longer creates the key under the working directory. DefaultKeyPath accepts
every DSN form, and memory databases have no key path.

Closes #776
Closes #777
Closes #778
Closes #779
Closes #780
@crypt0rr crypt0rr added the bug Something isn't working label Sep 25, 2026
@crypt0rr
crypt0rr merged commit 5121513 into main Sep 25, 2026
7 checks passed
@crypt0rr
crypt0rr deleted the fix/notification-delivery-correctness branch September 25, 2026 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment