fix(service-messaging): the durable fan-out refuses an unregistered channel instead of writing a delivery row that can only dead-letter

.changeset/great-jars-sleep.md

@@ -0,0 +1,23 @@
+---
+'@objectstack/service-messaging': patch
+---
+
+fix(service-messaging): the durable fan-out refuses a channel nobody registered instead of writing a delivery row for it
+
+`MessagingService.emit()` on the reliable-delivery (outbox) path wrote one
+`sys_notification_delivery` row per recipient for a channel the composition had
+never registered, and the dispatcher dead-lettered every one of them on attempt
+one. The inline path had always refused this case; only the durable path wrote
+the rows, so a deployment whose flows notify on `['inbox','email']` without an
+email plugin accumulated guaranteed-dead rows in the hot delivery table.
+
+The durable path now reports the same failed delivery outcome the inline path
+reports — `ok: false`, `error: "channel '<id>' not registered"`, counted in
+`EmitResult.failed` — and writes no row. The refusal is logged once per channel
+per emit with the number of rows it refused, not once per recipient.
+
+The refusal is deliberately **not** recorded in
+`sys_notification.suppressed_channels`: that key answers "why can this tenant not
+send on this channel", and an unregistered channel is a composition fact,
+identical for every tenant in the process. The event row's column set is
+unchanged.

packages/services/service-automation/src/builtin/notify-delivery-outcome.integration.test.ts

@@ -28,8 +28,24 @@ import { registerNotifyNode } from './notify-node.js';
  * was called: the finding is precisely that those two records contradict each
  * other, so an internal call-count assertion would pass while the defect stands.
  *
- * On `origin/main` the first test fails with `acted: 1` — the notify node
- * counts `EmitResult.delivered`, which in outbox mode is an ENQUEUED count.
+ * At the time #7747 landed, the first test failed with `acted: 1` — the notify
+ * node counted `EmitResult.delivered`, which in outbox mode is an ENQUEUED count.
+ *
+ * ## Amended by #18050 — the first case's durable record is now EMPTY
+ *
+ * #7747's repro boots without `push` registered, and back then the durable
+ * fan-out enqueued a row for it anyway that the dispatcher could only
+ * dead-letter. #18050 fixed that at the producer: `enqueueDeliveries` refuses an
+ * unregistered channel before it writes, reporting the same failed
+ * `DeliveryOutcome` the inline path always did. So the first test's scenario
+ * moved buckets — from "an effect I cannot count YET" (`unmeasured: 1`, the
+ * dispatcher decides later) to "an effect I have counted and it is zero"
+ * (`acted: 0, unmeasured: 0`, refused synchronously).
+ *
+ * ⛔ That is not this file's invariant weakening. #7747's invariant is "the
+ * summary must not out-count what the durable record shows was delivered", and
+ * it is asserted below against a bound that went from 0-non-dead-rows to
+ * 0-rows-at-all. What changed is the producer, not what is demanded of it.
  */
 
 function silentLogger(): any {
@@ -107,37 +123,52 @@ function notifyFlow(channels: string[]) {
 }
 
 describe('notify run summary vs. the durable delivery record (#7747)', () => {
-    it('does not report a countable act for a delivery that dead-letters on an unregistered channel', async () => {
+    it('reports a MEASURED zero — not a countable act — for an unregistered channel on the durable path', async () => {
         // 1) Boot without the `push` channel registered.
         const { outbox, dispatcher, engine } = bootOutboxStack([recordingChannel('inbox').channel]);
 
         // 2) Fire a flow whose notify node targets ['push'].
         engine.registerFlow('nudge', notifyFlow(['push']));
         const run = await engine.execute('nudge');
 
-        // 3a) The durable record: the dispatcher dead-letters the row, because
-        //     no transport for `push` exists.
+        // 3a) The durable record: NOTHING — and that is the #18050 change.
+        //     This assertion used to read `toHaveLength(1)` + `status: 'dead'`:
+        //     the durable fan-out enqueued a row for a channel with no transport
+        //     and the dispatcher dead-lettered it on attempt ONE. That row was
+        //     itself the defect #18050 fixed, so `enqueueDeliveries` now refuses
+        //     the channel up front and writes no row at all. The tick is kept
+        //     deliberately: it proves nothing APPEARS later either, which is a
+        //     strictly stronger statement than the old "a row exists and is dead".
         await dispatcher.tick();
         const rows = await outbox.list();
-        expect(rows).toHaveLength(1);
-        expect(rows[0].channel).toBe('push');
-        expect(rows[0].status).toBe('dead');
-        expect(rows[0].error).toContain("channel 'push' not registered");
+        expect(rows).toHaveLength(0);
 
         // 3b) The record an operator reads. The run still SUCCEEDS — the flow
-        //     did everything it can do synchronously, and failing it would make
-        //     a channel that registers a moment later retroactively break the
-        //     flow. What must not survive is the claim that it DELIVERED:
-        //     `acted` is the count the broken-sweep alert trusts, and the honest
-        //     answer at the moment the run settles is "an effect I cannot count
-        //     yet" — which the platform already spells `unmeasured`, and which
-        //     is not the same as `acted: 0` alone (that would claim the run did
-        //     nothing, and trip the alert on every healthy notify).
+        //     did everything it can do synchronously. What must not survive is
+        //     the claim that it DELIVERED.
+        //
+        //     ⚠️ `unmeasured` moved 1 -> 0 here, and that is the POINT, not a
+        //     relaxation. `unmeasuredEffect` means "the count is unknown because
+        //     the dispatcher decides later". Since #18050 there is no later: the
+        //     refusal is synchronous, so the count is KNOWN and it is zero —
+        //     exactly the reading `notify-node.ts` demands ("this count is known
+        //     and it is zero; claiming otherwise would take the run OUT of the
+        //     broken-sweep filter ... on precisely the run that should be inside
+        //     it"). `selected: 1, acted: 0, unmeasured: 0` puts this run INSIDE
+        //     the `selected > 0 AND acted = 0 AND unmeasured = 0` alert, which is
+        //     where a notify that reached nobody and never will belongs.
+        //
+        //     It is also what makes the two fan-out paths agree: the inline case
+        //     four tests down asserts this same triple and calls it "correctly
+        //     eligible for the broken-sweep alert". The durable path is not a
+        //     duplicate of it — it is the other side of the seam this file
+        //     exists for, and it is the side that used to disagree.
         expect(run.success).toBe(true);
-        expect(run.summary).toMatchObject({ acted: 0, unmeasured: 1 });
+        expect(run.summary).toMatchObject({ selected: 1, acted: 0, unmeasured: 0 });
 
-        // The finding itself, as one assertion: the summary must not out-count
-        // what the durable record shows was actually delivered (here: nothing).
+        // The #7747 finding itself, unchanged in force: the summary must not
+        // out-count what the durable record shows was actually delivered. With
+        // no row at all the bound is 0, so this is tighter than it was before.
         const notDead = rows.filter((r) => r.status !== 'dead').length;
         expect(run.summary!.acted).toBeLessThanOrEqual(notDead);
     });

packages/services/service-messaging/src/channel-availability.test.ts

@@ -339,8 +339,14 @@ describe('channel availability at fan-out (#17732)', () => {
 
     it('leaves an UNREGISTERED channel on its existing path — ⛔ not folded into suppression', async () => {
         // Out of the ruling's scope on purpose: an unregistered channel has no
-        // implementation to ask, so it keeps today's behaviour exactly. Pinned
-        // so the boundary is deliberate rather than accidental.
+        // implementation to ask, so it is not a SUPPRESSION. Pinned so the
+        // boundary is deliberate rather than accidental.
+        //
+        // ⚠️ This is the INLINE path, and it is unchanged. #18050 later gave the
+        // DURABLE path the same answer this one already gave — a failed
+        // `DeliveryOutcome` and no row — so "keeps today's behaviour exactly" is
+        // no longer true of the outbox half; `unregistered-channel.test.ts`
+        // pins that half, including that it still records no suppression.
         const data = capturingEngine();
         const service = new MessagingService({ logger: silentLogger(), getData: () => data.engine });
         service.registerChannel(channelDouble('inbox').channel);

packages/services/service-messaging/src/messaging-service.ts

@@ -879,8 +879,9 @@ export class MessagingService {
      * The single notification ingress. Writes the L2 event, resolves the
      * audience, and fans the result out to its channels. An unregistered
      * channel, or a channel that throws, is reported as a failed delivery — it
-     * never aborts the rest of the fan-out. A `dedupKey` that matches an
-     * existing event short-circuits: the event id is returned and no new
+     * never aborts the rest of the fan-out, and on the durable path it costs no
+     * `sys_notification_delivery` row either (#18050). A `dedupKey` that matches
+     * an existing event short-circuits: the event id is returned and no new
      * deliveries are produced.
      *
      * A channel that answers `isAvailable: { available: false }` for the tenant
@@ -1036,11 +1037,14 @@ export class MessagingService {
      *    default would mute every channel that has not been updated, which is a
      *    far worse failure than the workless rows this exists to stop.
      *    `channel-availability.test.ts` pins it from both sides.
-     * 2. **An UNREGISTERED channel is left alone.** It has no implementation to
-     *    ask, so it keeps today's path exactly: the inline fan-out reports it as
-     *    a failed delivery, the outbox enqueues a row the dispatcher
-     *    dead-letters. That is a real, separate defect — it is filed, ⛔ not
-     *    widened into this ruling.
+     * 2. **An UNREGISTERED channel is not answered here.** It has no
+     *    implementation to ask, so this consult cannot reach it and its absence
+     *    is ⛔ NOT a suppression: `suppressed_channels` answers "why can this
+     *    TENANT not send", and an unregistered channel is a composition fact.
+     *    Both fan-out paths refuse it instead, reporting one failed
+     *    {@link DeliveryOutcome} per `(recipient × channel)` and writing no
+     *    delivery row at all — inline in {@link MessagingService.fanOut}, on the
+     *    durable path in {@link MessagingService.enqueueDeliveries} (#18050).
      * 3. **A throw is AVAILABLE.** Fail-open, matching the preference filter one
      *    step down: a broken probe must degrade into today's behaviour, never
      *    into a silent notification outage. Logged at `warn` — the degradation
@@ -1085,6 +1089,28 @@ export class MessagingService {
      * dispatcher does the actual send + retry; here `ok` means "accepted for
      * delivery" (enqueued), not yet delivered — progress is observable on the
      * `sys_notification_delivery` row.
+     *
+     * ## An UNREGISTERED channel is refused here, not enqueued (#18050)
+     *
+     * A channel nobody registered has no transport to reach, so a row written
+     * for it is a row the dispatcher can only dead-letter on attempt ONE —
+     * `processRow` / `processDigestGroup` both ack `dead: true` the moment
+     * `getChannel()` answers nothing. Writing it costs an insert, a claim, an
+     * update and a retained terminal row per recipient, to record a fact known
+     * before the first write.
+     *
+     * The refusal is reported as the SAME failed {@link DeliveryOutcome} the
+     * inline path already produces for this case, so "nothing was sent and here
+     * is why" has one shape on both paths and the caller's `failed` count keeps
+     * its meaning.
+     *
+     * ⛔ NOT folded into `sys_notification.suppressed_channels`. That vocabulary
+     * answers "why can this TENANT not send on this channel" — a per-tenant
+     * configuration fact an operator filters and reports on. An unregistered
+     * channel is a COMPOSITION fact: identical for every tenant in the process,
+     * and fixed by mounting the channel, not by configuring the tenant.
+     * Recording it there would make a per-tenant report assert a
+     * deployment-wide misconfiguration.
      */
     private async enqueueDeliveries(
         outbox: INotificationOutbox,
@@ -1107,8 +1133,23 @@ export class MessagingService {
             actorId: input.actorId,
         };
         const deliveries: DeliveryOutcome[] = [];
+        // [#18050] Counted, not logged in place: an emit to a 500-recipient
+        // audience would otherwise print 500 identical lines for one missing
+        // channel. Said ONCE per channel below, with the volume it refused —
+        // which is the number an operator needs to size the misconfiguration.
+        const refused = new Map<string, number>();
         for (const { recipient, channels, notBefore, digest } of targets) {
             for (const channel of channels) {
+                if (!this.channels.has(channel)) {
+                    refused.set(channel, (refused.get(channel) ?? 0) + 1);
+                    deliveries.push({
+                        channel,
+                        recipient,
+                        ok: false,
+                        error: `channel '${channel}' not registered`,
+                    });
+                    continue;
+                }
                 try {
                     const id = await outbox.enqueue({
                         notificationId,
@@ -1130,6 +1171,13 @@ export class MessagingService {
                 }
             }
         }
+        for (const [channel, count] of refused) {
+            this.ctx.logger.warn(
+                `[messaging] emit: channel '${channel}' is not registered; refused ${count} ` +
+                `delivery row(s) the dispatcher could only dead-letter. Register the channel ` +
+                `(or drop it from this notify's channel list) — nothing was sent on it.`,
+            );
+        }
         return deliveries;
     }