fix(notification): stamp matched types and provider cache in getCommonNotificationTxes

[clayrisser]

Problem

getCommonNotificationTxes (server-plugins/notification-resources/src/index.ts:96-137) computes the matched notification types, then throws them away:

  await pushInboxNotifications(
    …
    modifiedOn,
    [],            // <- :131  the matched types, discarded
    true,
    tx
  )

pushInboxNotifications writes that argument straight onto the notification document (:389-398, types at :396), so every notification created through this function is born types: [] — regardless of what actually matched.

It also never populates AvailableProvidersCache. getNotificationTxes:638-645 publishes the allowed providers after creating its notification; getCommonNotificationTxes does not, and it is the only other place that creates one.

The sibling function does both correctly, four hundred lines away in the same file:

// getNotificationTxes:622-645
const ids = notifyResult.get(notification.providers.InboxNotificationProvider)?.map((it) => it._id)
…
if (notificationTx !== undefined) {
  const current: AvailableProvidersCache = control.contextCache.get(AvailableProvidersCacheKey) ?? new Map()
  const providers = Array.from(notifyResult.keys())
  if (providers.length > 0) {
    current.set(notificationTx.objectId, providers)

That asymmetry is the whole bug.

Three callers are affected

caller	notification class	what it hands in
server-plugins/time-resources/src/index.ts:303,314-328	CommonInboxNotification	isShouldNotifyTx(...)'s full notifyResult
server-plugins/activity-resources/src/index.ts:138-160	ReactionInboxNotification	new Map(allowedProviders.map((it) => [it, [type]]))
server-plugins/activity-resources/src/references.ts:136-172	MentionInboxNotification	same shape, MentionNotificationType

The reaction and mention callers are the tell: each resolves a NotificationType into a local variable, puts it in notifyResult, and the callee drops it on the floor.

This defeats the tree's own fallbacks

push.ts:248-260 carries a fallback whose comment describes exactly the situation it is then defeated by:

  // Fallback: if cache doesn't have the provider info (e.g. scheduled notifications created outside tx-trigger paths),
  // compute allowed providers from notification type + user settings.
      const type = (n.types ?? [])[0]
      if (type === undefined) continue

With types: [], type is undefined and the fallback gives up. gmail-resources/src/index.ts:221,324 has the same fallback and logs when it fires — NotificationsHandler: skipping notification without type. Both halves have to be fixed: a consumer that gates on the cache never gets as far as reading types, and a consumer that falls back to types finds it empty.

Fix

14 lines, both blocks lifted verbatim from getNotificationTxes so the two paths agree:

   const notifyContexts = await control.findAll(ctx, notification.class.DocNotifyContext, { objectId: attachedTo })
+  const types = (notifyResult.get(notification.providers.InboxNotificationProvider) ?? []).map((it) => it._id)
 
-  await pushInboxNotifications(
+  const notificationTx = await pushInboxNotifications(
     …
-    [],
+    types,
     true,
     tx
   )
+
+  if (notificationTx !== undefined) {
+    const current: AvailableProvidersCache = control.contextCache.get(AvailableProvidersCacheKey) ?? new Map()
+    const providers = Array.from(notifyResult.keys())
+    if (providers.length > 0) {
+      current.set(notificationTx.objectId, providers)
+      control.contextCache.set(AvailableProvidersCacheKey, current)
+    }
+  }

Scope and residual risk

Nothing is force-enabled. notifyResult is computed by the callers from isAllowed, which already honours ignoredTypes, the per-user Settings → Notifications toggles, and each type's defaultEnabled. What changes is that the answer stops being discarded.

That does mean this unblocks delivery for every notification born through this function, for every provider isAllowed already approved — so it is worth being explicit about what starts flowing:

family	today	after
ToDo (time:ids:ToDoCreated)	allowed by settings, dropped for want of a type	delivered
reactions (activity:ids:AddReactionNotification)	allowed, dropped	delivered
mentions (MentionInboxNotification) → email	approved by isAllowed, then dropped	email begins sending
ToDo → email	defaultEnabled: false, no enabledTypes entry	unchanged, still no email

⚠️ The mention-email row is the one to look at before merging. notification:ids:MentionNotificationType is defaultEnabled: true (models/notification/src/index.ts:599-611) and gmail's provider is defaultEnabled: true with an ignoredTypes that does not list it (models/gmail/src/notification.ts:53-81), so isAllowed approves email for mentions today and only the missing type suppresses the send. After this change a mention inside a followed conversation can produce two emails — one from the ActivityInboxNotification that already worked, and one from the MentionInboxNotification.

I have deliberately not changed that here. Suppressing it would mean adding MentionNotificationType to gmail's ignoredTypes, which is a product call rather than a bug fix, and I would rather surface the consequence than quietly bundle a behaviour decision into a correctness fix. Happy to add it in this PR if you'd prefer it landed together.

Deliberately not touched: getNotificationTxes:653 sets the cache with the string literal 'AvailableNotificationProviders' rather than the exported AvailableProvidersCacheKey. Equal today, a drift hazard tomorrow — but it is not this bug, and widening a fix to carry a nit is how patches stop applying cleanly. Flagging rather than fixing.

Verification

server-plugins/notification-resources had jest --passWithNoTests and no suites; commonNotificationTypes.test.ts is the first one in it. jest, matching the package's own config.

	before	after
server-plugins/notification-resources	3 failed / 6 passed	9 passed
server-plugins/activity-resources (a caller)	3 passed	3 passed
server-plugins/calendar-resources	7 passed	7 passed

The three reds were exactly the three new behaviours — the type on the document, the provider-cache entry, and the cache entry under the ignored-provider shape. The six that passed red are the control: three of them drive the real isShouldNotifyTx with the real ToDoCreated shape over a bare TxCreateDoc and assert that a type is matched, which pins that the matcher was never the problem and the loss happens after it.

Cases added:

• stamps the ToDo type onto the notification it creates
• the activity path is unchanged › keeps writing the types it is handed
• does not invent a type for a genuinely typeless notification — types stays [], so gmail/push still skip it
• still writes nothing at all when the inbox provider is absent
• drops the type when the provider ignores it
• drops the type when it is neither enabled nor defaultEnabled
• leaves a type out of the cache for a provider that did not allow it

tsc --noEmit: the src/index.ts error set is byte-identical before and after once line numbers are normalised — the fix adds none.

No UI change, so there is nothing to screenshot.

Provenance

Found while integrating a third-party notification consumer against a self-hosted deployment: ToDo notifications were created but never delivered, and the gmail handler's skipping notification without type line was the thread that led here. The mechanism above is read from the code and pinned by the tests; I have not measured the mention-email volume change on a live instance, which is why it is flagged as a question rather than asserted as safe.