Skip to content

Allow a new content_change carrier once the last carrier's digest is sent or skipped #451

Description

@HMarzban

Summary

notify_document_content_change skips a reader who holds an unread content_change carrier from the last 24 hours. Sending the digest does not mark the carrier read. So after a digest goes out, new edits make no new carrier until the old one turns 24 hours old. If editing stops inside that gap, the reader is never told about those edits.

This issue does not explain the missing highlights in the issue #448. It is a separate delivery flaw.

Where

  • packages/supabase/scripts/10-func-notifications.sql:673-681: the not exists clause skips any carrier with readed_at is null and created_at within 24 hours.
  • packages/supabase/scripts/07-5-email-notifications-pgmq.sql:
    • lines 333-336: a daily row is scheduled for 09:00 local on the next calendar day, even when today's 09:00 is still ahead.
    • lines 342-344: a weekly row is scheduled for 09:00 on the Monday of the next week.
    • lines 330 and 357-359: an immediate user's content_change is pinned to the digest path at now() + 15 minutes.
    • Neither status writer touches notifications. The worker writes skipped through update_email_status (751-770), which also sets sent_at when given sent. The worker's own sent and sent_at write is updateSupabaseEmailStatus (apps/hocuspocus.server/src/lib/email/sender.ts:200-224), called from queue.ts:73-83.
    • compile_digest_emails (from line 543) picks only pending rows, each at its own scheduled_for.
    • email_queue.notification_id (line 28) has no index, here (lines 38-43) or in 11-indexes.sql.
  • apps/hocuspocus.server/src/lib/email/pgmqConsumer.ts:221-224: the skip arm does not touch the carrier either.
  • The only writers of notifications.readed_at are the webapp's notification panel actions (markNotificationAsRead.ts, markAllNotificationsAsRead.ts). Opening the document does not mark the carrier read.
  • The code already guards this hazard for the defer arm (digestMessage.ts:72-75, pgmqConsumer.ts:212-215) and for quiet hours (07-5-email-notifications-pgmq.sql:353-359). The send and skip arms have no guard.
  • These texts describe the old dedupe and must be corrected in the same change:
    • apps/hocuspocus.server/src/lib/email/digestMessage.ts:72-74 ("Acking it marks the carriers read").
    • apps/hocuspocus.server/src/lib/email/__tests__/digestMessage.test.ts:131-133.
    • apps/hocuspocus.server/src/lib/email/pgmqConsumer.ts:212-215.
    • apps/hocuspocus.server/src/lib/metrics.ts:204-205.
    • The comment on function text at 10-func-notifications.sql:689.
    • apps/hocuspocus.server/CLAUDE.md: the decideDigestOutcome bullet in §Testability Seams, and the none-notified sentence in §Digest Email Links And Counts.

What happens

All timings are read from code, not measured.

  • Daily reader R. Member M edits at 23:00 on day 1, and carrier C is made. The digest for C goes out at 09:00 on day 2. M edits again at 14:00 on day 2, then stops. C is still unread and under 24 hours old, so the fan-out inserts nothing. R is never told about the 14:00 edit.
  • For daily readers, a carrier made after 09:00 leaves a gap of its local time minus 09:00, so 0 to 15 hours. A carrier made before 09:00 leaves no gap.
  • Immediate reader. The carrier is mailed after 15 minutes, so the gap is about 23 h 45 min.
  • Recovery. Any edit after the old carrier expires makes a new carrier. Its window starts at Last left, so it still covers the earlier edits. The loss needs editing to stop inside the gap.

Expected

After a carrier's digest is sent or skipped, the next real edit can make a new carrier, and the reader gets a later digest for it.

Fix

  • In the not exists clause of notify_document_content_change, ignore a carrier whose email row has left pending:
    and not exists (select 1 from public.email_queue eq where eq.notification_id = n.id and eq.status <> 'pending')
  • Treat processing as not pending, because that row is already being mailed.
  • The clause also re-arms after a skipped or failed row, on purpose. Neither one delivered a mail.
  • A user with email off has no email_queue row, so the in-app panel dedupe does not change for them. A reader with email on can now hold more than one unread carrier for one document: some mailed, at most one pending. That is expected.
  • Cadence (maintainer ruling, 2026-10-07): enforce the 2-hour gap in queue_email_notification, inside the content_change pin at 07-5-email-notifications-pgmq.sql:357-359. Push the new row back:
    schedule_time := greatest(schedule_time, (
        select max(coalesce(eq.sent_at, now())) + interval '2 hours'
          from public.email_queue eq
          join public.notifications n on n.id = eq.notification_id
         where eq.user_id = new.receiver_user_id
           and eq.status in ('processing', 'sent')
           and n.type = 'content_change'
           and n.channel_id = new.channel_id
    ));
    • A processing row has no sent_at yet, so now() stands in for it.
    • greatest ignores the null for a reader with no earlier mail.
    • Daily and weekly rows are already due the next day or week, so the line changes nothing for them (read from code, not measured).
  • Do not put the cadence in the fan-out clause. A blocked save makes no carrier, so an edit inside those 2 hours is lost when editing stops. A delayed row stays pending, so the fan-out folds later edits into it.
  • Add create index if not exists idx_email_queue_notification_id on public.email_queue (notification_id); in 07-5-email-notifications-pgmq.sql, beside idx_email_queue_user. Keep it plain, not partial. The new subquery runs inside every save's fan-out. Its cost is read from code, not measured.
  • Ship it as script changes (10-func-notifications.sql, 07-5-email-notifications-pgmq.sql) plus one paired migration. The migration recreates notify_document_content_change and queue_email_notification whole, each copied from its edited script, and creates the index. queue_email_notification lives in scripts/ only, so remote gets the change only through this migration. Regenerate seed.sql through generate-seed.ts, never by hand. Then run bun run --filter @docs.plus/supabase_back types, and include the regenerated file.
  • Correct every out-of-date text named under Where. The defer arm stays, because a skip still drops that window's changes unless someone edits again. Only its stated reason changes.

Out of scope

Acceptance criteria

  • After a carrier's email row leaves pending (processing, sent, skipped or failed), the next save by another member makes a new carrier for that reader.
  • While the row is still pending, no second carrier is made, as today.
  • A reader with email off keeps today's panel dedupe.
  • Immediate readers get at most one content digest per document every 2 hours. A carrier made within 2 hours of that document's last sent digest row gets scheduled_for equal to that sent_at plus 2 hours. With no earlier sent or processing row, it stays now() + 15 minutes. A daily reader's row still reads 09:00 local the next day.
  • 07-5-email-notifications-pgmq.sql and the migration both create idx_email_queue_notification_id on public.email_queue (notification_id).
  • The scripts, the migration and seed.sql match, and the regenerated types are included.
  • No file in the Where list still says that an ack marks carriers read. None says that a sent or skipped digest blocks the fan-out for 24 hours.
  • The 2-hour cadence is written down. The places are apps/hocuspocus.server/API.md §Email (the pipeline paragraph), apps/hocuspocus.server/CLAUDE.md §Digest Email Links And Counts, and the root CHANGELOG.md [Unreleased] under ### Fixed.

Verification

No SQL test harness exists in packages/supabase/. Add apps/hocuspocus.server/scripts/e2e-content-change-rearm.ts. Run it from apps/hocuspocus.server with bun --env-file=../../.env.local scripts/e2e-content-change-rearm.ts. It needs local Supabase only: no servers and no Redis.

  • Reuse scripts/lib/e2eHarness.ts: mintTestUser, deleteTestUser and createTally.
  • As the reader, call join_workspace on a fresh document id. That writes the workspaces and channels rows; without them the fan-out returns 0 (10-func-notifications.sql:605-620).
  • With the service-role client, set the reader's users.notification_preferences to email_enabled: true and email_frequency: 'immediate'.
  • Call notify_document_content_change with the service-role client and p_only_user set to the reader. Settle rows with the update_email_status RPC. Do not write email_queue directly.

Steps:

  1. The first call inserts 1 carrier, and its queue row is pending.
  2. A second call inserts 0.
  3. Settle that row skipped. The next call inserts 1, due about now() + 15 minutes.
  4. Settle that row sent. The next call inserts 1, and its scheduled_for is that row's sent_at plus 2 hours.
  5. The next call inserts 0, because the delayed row is still pending.
  6. Set email_enabled: false and join a second fresh document. The first call inserts 1 carrier with no queue row, and a second call inserts 0.

Steps 3 and 4 must fail on today's main. Delete the rows and the user the script made.

No activity

Activity on this issue will appear here.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions