Webhooks: notification keys name rows, not announcements, so a re-announcement is dropped and a long list overflows the column #413

New issue

Closed

opened 2026-10-01 10:46:31 +00:00 by tiagoagueda · 0 comments

tiagoagueda commented

2026-10-01 10:46:31 +00:00

Owner

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading the notification keys the announcers build against the webhook queue that deduplicates on them and the column that stores them, and confirmed by running announce_due_reminders twice around a postpone_reminder. Related to #240, which added the webhook notifier and its deduplication by "the notification's stable key"; this issue shows that three announcers build a key that is stable across announcements that are meant to be distinct, and that two of them build one with no upper bound.

Checked again on 2026-10-01 against main at 0a5cad587: still present. The code is now at notifications/management/commands/send_due_reminders.py:90 (the reminder:<pk> key) and applications/views.py:771 (the reminder edit form's call to postpone_reminder); the other locations have not moved.

Three announcers build their key from row ids and nothing else: reminder:<pk>, went_quiet:<ids> and posting_closing:<ids>. The webhook queue refuses a second delivery row for a key a connection has already seen and never prunes its rows, so when Postulo deliberately announces the same reminder, quiet application or closing listing a second time, email, Apprise and the browser get it and the webhook silently does not. The two batched keys also list every row of the pass, so they can exceed the 200 characters WebhookDelivery.key holds, which fails the insert on PostgreSQL.

Where:
- notifications/webhooks.py:60-74, enqueue: returns None when WebhookDelivery.objects.filter(connection=connection, key=notification.key).exists(), and otherwise stores the key unchanged. Nothing in src/ ever deletes a WebhookDelivery row, so a key is refused for the life of the connection.
- notifications/models.py:68: key = models.CharField(_("key"), max_length=200, blank=True, db_index=True).
- notifications/management/commands/send_due_reminders.py:88: key=f"reminder:{reminder.pk}".
- applications/quiet.py:144, _announcement: key="went_quiet:" + ",".join(str(row.pk) for row in applications).
- jobs/closing.py:149, _announcement: key="posting_closing:" + ",".join(str(row.pk) for row in postings).
- The flows that announce again on purpose: applications/services.py:582 (postpone_reminder clears notified_at; used by Later, by the reminder edit form at applications/views.py:725 and by the API at api/routers/reminders.py:87), applications/services.py:468 (reschedule_interview, the same for the interview's reminder), applications/quiet.py:65-66 ("one that woke up and went quiet again is announced again") and jobs/closing.py:89-92 ("Announced again when the date moves").
- The announcers that already get it right put the moving part in the key: notifications/slow.py:169 and :212 (the event id) and :190 (interview:{id}:{starts_at}).
- The contract as written: notifications/base.py:73-75 ("reminder:41 is the same message however many times it is announced") and, in the wiki, Webhooks.md:55-57 ("the same thing announced twice carries the same key").

1. A deliberate re-announcement reuses the old key and is dropped

What happens:
1. A reminder falls due and is announced; a row keyed reminder:41 is queued and delivered. The person presses Later, edits the due time, or reschedules the interview it belongs to; notified_at is cleared and the reminder is announced again at its new time. enqueue finds the old reminder:41 row and writes nothing. In a probe on the snapshot, announce_due_reminders() returned (1, 1) both times and WebhookDelivery.objects.count() stayed at 1.
2. A person is told a listing closes in three days. The employer extends the deadline and the person edits closes_at. The next pass claims the listing again; if the id list is the same as last time (the usual case is one listing) the key is again posting_closing:<pk> and the message is not queued.
3. An application announced as quiet has some activity and then goes quiet again. applications/quiet.py:85 treats it as fresh; alone in its pass, its key is again went_quiet:<pk> and the message is not queued.
In all three WebhookNotifier.send returns normally, so notifications/service.py:91-92 records the connection as working and counts it as delivered.

2. The joined ids can exceed 200 characters

What happens: a went_quiet key holds 38 four-digit ids in 200 characters and a posting_closing key 37; one row more is 205. A pass that large is plausible for quiet applications: the first pass after the scheduler is started on an instance already in use, or after a spreadsheet of past applications is imported, announces every quiet application of a person in one message. On PostgreSQL WebhookDelivery.objects.create raises DataError; notifications/service.py:85-89 logs it and writes it on the connection. The rows were stamped before sending (applications/quiet.py:93-99, jobs/closing.py:93-99), so the announcement is never tried again. SQLite does not enforce the length, so the test suite cannot see it.
Why it matters: the webhook is the notifier an automation relies on, and it drops exactly the announcements that carry news: the ones the person caused by moving something, and the largest ones. The first failure leaves no trace (the connection shows last worked: now); the second leaves an error on the connection but loses the message for good, on PostgreSQL, which the project supports with its own Compose file (docker/compose.postgres.yml). The key is also sent in the payload, and the wiki advises receivers to deduplicate on it, so a receiver following that advice would discard a second announcement even if Postulo sent it.
Fix: make each key name one announcement and keep it bounded.
- reminder:{pk}:{due_at.isoformat()}.
- For the batched announcers, the prefix plus a digest of what the message says, for example "posting_closing:" + sha256(",".join(f"{row.pk}@{row.closes_at.isoformat()}" for row in postings).encode()).hexdigest()[:40], and for went_quiet the ids with the quiet_announced_at stamp just claimed (the now already passed to _announcement). The ids stay available in data. A retry of the same announcement keeps its key; a real re-announcement gets a new one.
- In enqueue, refuse or hash any key longer than the column, so a later announcer cannot reintroduce the overflow.
- Reword the Notification.key comment in notifications/base.py:73-79 and the wiki's Webhooks.md:55-57 to say that a key names one announcement, not one object, and update the assertions that pin the old shape (tests/test_deadlines.py:365, tests/test_notifications.py:149).

Done when

A test in tests/test_webhooks.py announces a due reminder to a webhook connection, postpones it with postpone_reminder to a time already past, announces again and expects two WebhookDelivery rows; it fails today.
A test in tests/test_deadlines.py moves a listing's closes_at after its first announcement and expects a second WebhookDelivery row, and one in tests/test_quiet.py does the same for an application that goes quiet, wakes and goes quiet again; both fail today.
Tests in tests/test_quiet.py and tests/test_deadlines.py build the announcement for a hundred rows and expect len(message.key) to be at most WebhookDelivery._meta.get_field("key").max_length; both fail today.
test_a_key_means_one_delivery_however_often_it_is_announced in tests/test_webhooks.py still passes: one announcement notified twice is one row.

Found on 2026-09-29 in a read-only review of postulo at 95348908d, reading the notification keys the announcers build against the webhook queue that deduplicates on them and the column that stores them, and confirmed by running `announce_due_reminders` twice around a `postpone_reminder`. Related to #240, which added the webhook notifier and its deduplication by "the notification's stable key"; this issue shows that three announcers build a key that is stable across announcements that are meant to be distinct, and that two of them build one with no upper bound. Checked again on 2026-10-01 against `main` at `0a5cad587`: still present. The code is now at `notifications/management/commands/send_due_reminders.py:90` (the `reminder:<pk>` key) and `applications/views.py:771` (the reminder edit form's call to `postpone_reminder`); the other locations have not moved. Three announcers build their key from row ids and nothing else: `reminder:<pk>`, `went_quiet:<ids>` and `posting_closing:<ids>`. The webhook queue refuses a second delivery row for a key a connection has already seen and never prunes its rows, so when Postulo deliberately announces the same reminder, quiet application or closing listing a second time, email, Apprise and the browser get it and the webhook silently does not. The two batched keys also list every row of the pass, so they can exceed the 200 characters `WebhookDelivery.key` holds, which fails the insert on PostgreSQL. - **Where:** - `notifications/webhooks.py:60-74`, `enqueue`: returns `None` when `WebhookDelivery.objects.filter(connection=connection, key=notification.key).exists()`, and otherwise stores the key unchanged. Nothing in `src/` ever deletes a `WebhookDelivery` row, so a key is refused for the life of the connection. - `notifications/models.py:68`: `key = models.CharField(_("key"), max_length=200, blank=True, db_index=True)`. - `notifications/management/commands/send_due_reminders.py:88`: `key=f"reminder:{reminder.pk}"`. - `applications/quiet.py:144`, `_announcement`: `key="went_quiet:" + ",".join(str(row.pk) for row in applications)`. - `jobs/closing.py:149`, `_announcement`: `key="posting_closing:" + ",".join(str(row.pk) for row in postings)`. - The flows that announce again on purpose: `applications/services.py:582` (`postpone_reminder` clears `notified_at`; used by *Later*, by the reminder edit form at `applications/views.py:725` and by the API at `api/routers/reminders.py:87`), `applications/services.py:468` (`reschedule_interview`, the same for the interview's reminder), `applications/quiet.py:65-66` ("one that woke up and went quiet again is announced again") and `jobs/closing.py:89-92` ("Announced again when the date moves"). - The announcers that already get it right put the moving part in the key: `notifications/slow.py:169` and `:212` (the event id) and `:190` (`interview:{id}:{starts_at}`). - The contract as written: `notifications/base.py:73-75` ("`reminder:41` is the same message however many times it is announced") and, in the wiki, `Webhooks.md:55-57` ("the same thing announced twice carries the same key"). ## 1. A deliberate re-announcement reuses the old key and is dropped - **What happens:** 1. A reminder falls due and is announced; a row keyed `reminder:41` is queued and delivered. The person presses *Later*, edits the due time, or reschedules the interview it belongs to; `notified_at` is cleared and the reminder is announced again at its new time. `enqueue` finds the old `reminder:41` row and writes nothing. In a probe on the snapshot, `announce_due_reminders()` returned `(1, 1)` both times and `WebhookDelivery.objects.count()` stayed at 1. 2. A person is told a listing closes in three days. The employer extends the deadline and the person edits `closes_at`. The next pass claims the listing again; if the id list is the same as last time (the usual case is one listing) the key is again `posting_closing:<pk>` and the message is not queued. 3. An application announced as quiet has some activity and then goes quiet again. `applications/quiet.py:85` treats it as fresh; alone in its pass, its key is again `went_quiet:<pk>` and the message is not queued. In all three `WebhookNotifier.send` returns normally, so `notifications/service.py:91-92` records the connection as working and counts it as delivered. ## 2. The joined ids can exceed 200 characters - **What happens:** a `went_quiet` key holds 38 four-digit ids in 200 characters and a `posting_closing` key 37; one row more is 205. A pass that large is plausible for quiet applications: the first pass after the scheduler is started on an instance already in use, or after a spreadsheet of past applications is imported, announces every quiet application of a person in one message. On PostgreSQL `WebhookDelivery.objects.create` raises `DataError`; `notifications/service.py:85-89` logs it and writes it on the connection. The rows were stamped before sending (`applications/quiet.py:93-99`, `jobs/closing.py:93-99`), so the announcement is never tried again. SQLite does not enforce the length, so the test suite cannot see it. - **Why it matters:** the webhook is the notifier an automation relies on, and it drops exactly the announcements that carry news: the ones the person caused by moving something, and the largest ones. The first failure leaves no trace (the connection shows *last worked: now*); the second leaves an error on the connection but loses the message for good, on PostgreSQL, which the project supports with its own Compose file (`docker/compose.postgres.yml`). The key is also sent in the payload, and the wiki advises receivers to deduplicate on it, so a receiver following that advice would discard a second announcement even if Postulo sent it. - **Fix:** make each key name one announcement and keep it bounded. - `reminder:{pk}:{due_at.isoformat()}`. - For the batched announcers, the prefix plus a digest of what the message says, for example `"posting_closing:" + sha256(",".join(f"{row.pk}@{row.closes_at.isoformat()}" for row in postings).encode()).hexdigest()[:40]`, and for `went_quiet` the ids with the `quiet_announced_at` stamp just claimed (the `now` already passed to `_announcement`). The ids stay available in `data`. A retry of the same announcement keeps its key; a real re-announcement gets a new one. - In `enqueue`, refuse or hash any key longer than the column, so a later announcer cannot reintroduce the overflow. - Reword the `Notification.key` comment in `notifications/base.py:73-79` and the wiki's `Webhooks.md:55-57` to say that a key names one announcement, not one object, and update the assertions that pin the old shape (`tests/test_deadlines.py:365`, `tests/test_notifications.py:149`). ## Done when - A test in `tests/test_webhooks.py` announces a due reminder to a webhook connection, postpones it with `postpone_reminder` to a time already past, announces again and expects two `WebhookDelivery` rows; it fails today. - A test in `tests/test_deadlines.py` moves a listing's `closes_at` after its first announcement and expects a second `WebhookDelivery` row, and one in `tests/test_quiet.py` does the same for an application that goes quiet, wakes and goes quiet again; both fail today. - Tests in `tests/test_quiet.py` and `tests/test_deadlines.py` build the announcement for a hundred rows and expect `len(message.key)` to be at most `WebhookDelivery._meta.get_field("key").max_length`; both fail today. - `test_a_key_means_one_delivery_however_often_it_is_announced` in `tests/test_webhooks.py` still passes: one announcement notified twice is one row.

tiagoagueda added this to the 0.5.0 milestone

2026-10-01 10:46:31 +00:00

tiagoagueda added the

bug

tier

labels

2026-10-01 10:46:31 +00:00

tiagoagueda referenced this issue from a commit

2026-10-04 10:04:59 +00:00

Name each announcement by its own key, and keep the key short

tiagoagueda closed this issue

2026-10-04 10:04:59 +00:00