fix(services): notify's run summary stops claiming a delivery that dead-lettered (#7747)

.changeset/notify-acted-dead-letter.md

@@ -0,0 +1,42 @@
+---
+"@objectstack/service-messaging": patch
+"@objectstack/service-automation": patch
+---
+
+fix(services): a notify flow-run summary no longer reports a delivery the delivery record dead-lettered (#7747)
+
+Boot a stack without the `push` channel registered, fire a flow whose `notify`
+node targets `['push']`, and the two records an operator can read **contradicted
+each other**: `sys_notification_delivery` held `status: 'dead'`,
+`error: "channel 'push' not registered"`, while the flow-run summary said
+`status: 'success', acted: 1`. Nothing was delivered, and the surface built to
+answer "did this sweep actually do anything" (#4354) said it had.
+
+The seam is `EmitResult.delivered`. With the durable outbox in play (ADR-0030
+P1), `emit()` returns as soon as the `(recipient × channel)` rows are enqueued —
+the dispatcher sends and decides the outcome afterwards — but `delivered`
+counted those *enqueued* rows anyway, under a name that says they arrived. The
+`notify` node then fed that number straight into `acted`, so a count minted
+before any send attempt survived unrevised through the dead-letter. It was never
+a "stale by a moment" number either: nothing ever revisits it.
+
+- `EmitResult` now separates the two. `delivered` means a channel **accepted**
+ the delivery — a terminal, observed outcome, which only the inline (P0)
+ fan-out can report. New `enqueued` carries the outbox path's accepted rows:
+ durable, unsent, outcome pending on `sys_notification_delivery`.
+- The `notify` node counts only what was delivered toward `acted`. When
+ deliveries are merely enqueued it reports `unmeasuredEffect` instead — the
+ qualifier a `connector_action` already uses for an effect the platform cannot
+ count, and deliberately **not** a bare `acted: 0`, which would claim the run
+ did nothing. The broken-sweep alert is
+ `selected > 0 AND acted = 0 AND unmeasured = 0`, so a pending delivery
+ suppresses the alert without asserting success. The node's output gains
+ `enqueued` alongside `delivered` and `notificationId`.
+
+The run still reports `success`: the flow did everything it can do
+synchronously, and failing it would let a channel registered a moment later
+retroactively break the flow. Notify does not block a flow on a downstream
+channel, so "delivered" is not a claim it is ever in a position to make — what
+changes is that it no longer makes it. Inline (P0) fan-out is untouched: it has
+the channel's answer by the time `emit()` returns, so `acted` stays a real
+measurement there, including the measured zero for an unregistered channel.

packages/services/service-automation/package.json

@@ -28,6 +28,7 @@
  "@objectstack/objectql": "workspace:*",
  "@objectstack/plugin-security": "workspace:*",
  "@objectstack/service-job": "workspace:*",
+ "@objectstack/service-messaging": "workspace:*",
  "@types/node": "^26.1.2",
  "typescript": "^6.0.3",
  "vitest": "^4.1.10"

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

@@ -0,0 +1,194 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+import { describe, it, expect } from 'vitest';
+import {
+ MessagingService,
+ MemoryNotificationOutbox,
+ NotificationDispatcher,
+} from '@objectstack/service-messaging';
+import type { MessagingChannel } from '@objectstack/service-messaging';
+import { AutomationEngine } from '../engine.js';
+import { registerNotifyNode } from './notify-node.js';
+
+/**
+ * #7747 — the run summary an operator reads must not claim a delivery that
+ * `sys_notification_delivery` records as dead.
+ *
+ * The QA repro verbatim: boot WITHOUT the `push` channel registered, fire a
+ * flow whose notify node targets `['push']`, then read the run summary and the
+ * delivery record. This wires the REAL `MessagingService` (outbox-backed, P1)
+ * and the REAL `NotificationDispatcher` behind the notify node rather than a
+ * fake, because the defect lives in the seam BETWEEN them: `emit()` returns
+ * once the row is enqueued and the dispatcher decides the outcome afterwards,
+ * so a fake that answers `emit()` in one shot cannot express the disagreement
+ * at all.
+ *
+ * The assertions are deliberately on the two DURABLE operator-facing records —
+ * the folded run summary and the outbox row — not on how many times anything
+ * 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.
+ */
+
+function silentLogger(): any {
+ const l: any = { info: () => {}, warn: () => {}, error: () => {}, debug: () => {} };
+ l.child = () => l;
+ return l;
+}
+
+/** A channel that records what it was handed, so a real send is distinguishable. */
+function recordingChannel(id: string): { channel: MessagingChannel; sent: unknown[] } {
+ const sent: unknown[] = [];
+ return {
+ sent,
+ channel: {
+ id,
+ async send(_ctx, delivery) {
+ sent.push(delivery);
+ return { ok: true };
+ },
+ },
+ };
+}
+
+/** Wire the notify node against a given messaging service. */
+function engineWith(messaging: MessagingService): AutomationEngine {
+ const engine = new AutomationEngine(silentLogger());
+ registerNotifyNode(engine, {
+ logger: silentLogger(),
+ getService: (name: string) => (name === 'messaging' ? messaging : undefined),
+ } as any);
+ return engine;
+}
+
+/**
+ * A stack booted the way the repro describes: messaging present and
+ * outbox-backed (P1), with only the channels named here registered.
+ */
+function bootOutboxStack(registered: MessagingChannel[]) {
+ const outbox = new MemoryNotificationOutbox(1);
+ const messaging = new MessagingService({ logger: silentLogger(), outbox });
+ for (const c of registered) messaging.registerChannel(c);
+
+ const dispatcher = new NotificationDispatcher({
+ nodeId: 'node-test',
+ outbox,
+ channels: messaging,
+ channelContext: { logger: silentLogger() },
+ partitionCount: 1,
+ intervalMs: 10_000, // ticks are driven manually
+ });
+
+ return { outbox, messaging, dispatcher, engine: engineWith(messaging) };
+}
+
+function notifyFlow(channels: string[]) {
+ return {
+ name: 'nudge',
+ label: 'Nudge',
+ type: 'autolaunched' as const,
+ nodes: [
+ { id: 'start', type: 'start' as const, label: 'Start' },
+ {
+ id: 'notify',
+ type: 'notify' as const,
+ label: 'Notify',
+ config: { recipients: ['user_1'], title: 'Renewal due', message: 'Ping', channels },
+ },
+ { id: 'end', type: 'end' as const, label: 'End' },
+ ],
+ edges: [
+ { id: 'e1', source: 'start', target: 'notify' },
+ { id: 'e2', source: 'notify', target: 'end' },
+ ],
+ };
+}
+
+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 () => {
+ // 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.
+ 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");
+
+ // 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).
+ expect(run.success).toBe(true);
+ expect(run.summary).toMatchObject({ acted: 0, unmeasured: 1 });
+
+ // The finding itself, as one assertion: the summary must not out-count
+ // what the durable record shows was actually delivered (here: nothing).
+ const notDead = rows.filter((r) => r.status !== 'dead').length;
+ expect(run.summary!.acted).toBeLessThanOrEqual(notDead);
+ });
+
+ it('reports the same uncountable effect for a channel that IS registered — the outcome is simply not known yet', async () => {
+ // The counterpart that stops the fix from degenerating into "unregistered
+ // channels are special": at the moment the run settles, a healthy
+ // outbox-backed delivery is equally unsent. What separates the two cases
+ // is the outbox row — which is exactly where `unmeasured` points.
+ const inbox = recordingChannel('inbox');
+ const { outbox, dispatcher, engine } = bootOutboxStack([inbox.channel]);
+
+ engine.registerFlow('nudge', notifyFlow(['inbox']));
+ const run = await engine.execute('nudge');
+
+ expect(run.success).toBe(true);
+ expect(run.summary).toMatchObject({ acted: 0, unmeasured: 1 });
+ // Nothing had been sent when the run settled…
+ expect(inbox.sent).toHaveLength(0);
+ // …and the delivery lands afterwards, on the record that owns the truth.
+ await dispatcher.tick();
+ expect(inbox.sent).toHaveLength(1);
+ expect((await outbox.list())[0].status).toBe('success');
+ });
+
+ it('still reports a countable act when the messaging stack delivers inline (no outbox)', async () => {
+ // The inline (P0) path really does know the outcome by the time `emit()`
+ // returns, so `acted` stays a measurement there — the fix narrows what
+ // `acted` may claim, it does not blanket every notify as unmeasurable.
+ const inbox = recordingChannel('inbox');
+ const messaging = new MessagingService({ logger: silentLogger() });
+ messaging.registerChannel(inbox.channel);
+ const engine = engineWith(messaging);
+
+ engine.registerFlow('nudge', notifyFlow(['inbox']));
+ const run = await engine.execute('nudge');
+
+ expect(inbox.sent).toHaveLength(1);
+ expect(run.summary).toMatchObject({ acted: 1, unmeasured: 0 });
+ });
+
+ it('an inline send to an unregistered channel is a measured zero, not an unmeasured shrug', async () => {
+ // Inline fan-out DOES observe "channel not registered" synchronously, so
+ // that run is correctly eligible for the broken-sweep alert.
+ const messaging = new MessagingService({ logger: silentLogger() });
+ messaging.registerChannel(recordingChannel('inbox').channel);
+ const engine = engineWith(messaging);
+
+ engine.registerFlow('nudge', notifyFlow(['push']));
+ const run = await engine.execute('nudge');
+
+ expect(run.summary).toMatchObject({ acted: 0, unmeasured: 0 });
+ });
+});

packages/services/service-automation/src/builtin/notify-node.ts

@@ -26,7 +26,23 @@ export interface MessagingServiceSurface {
  source?: { object: string; id: string };
  actorId?: string;
  channels?: string[];
- }): Promise<{ notificationId: string; delivered: number; failed: number }>;
+ }): Promise<{
+ notificationId: string;
+ /** Deliveries a channel ACCEPTED — a terminal, observed outcome. */
+ delivered: number;
+ failed: number;
+ /**
+ * Deliveries durably accepted into the messaging outbox but NOT yet
+ * attempted; their real outcome lands on `sys_notification_delivery`
+ * afterwards (#7747).
+ *
+ * Optional because this is a STRUCTURAL mirror of a service resolved at
+ * runtime: an older or third-party messaging implementation may not
+ * report it, and absent reads as "nothing is in flight" — which is the
+ * only answer such a stack could honestly give.
+ */
+ enqueued?: number;
+ }>;
 }
 
 /**
@@ -279,18 +295,46 @@ export function registerNotifyNode(engine: AutomationEngine, ctx: PluginContext)
  actorId,
  channels: channels.length ? channels : undefined,
  });
+ const delivered = Number(result.delivered) || 0;
+ const enqueued = Number(result.enqueued) || 0;
  return {
  success: true,
  output: {
  notificationId: result.notificationId,
- delivered: result.delivered,
+ delivered,
+ // Surfaced so a flow author templating the outcome can
+ // tell "sent" from "handed to the outbox", and so the
+ // notification id above has a stated reason to be
+ // followed into `sys_notification_delivery`.
+ enqueued,
  failed: result.failed,
  },
  // A notification IS the action for a nudge/alert sweep, so it
  // counts toward `acted` (#4354) — otherwise the flow whose
  // whole job is to notify would report acting on nothing, and
  // the broken-sweep detector would fire on every healthy run.
- metrics: { acted: Number(result.delivered) || 0 },
+ //
+ // But only a delivery a channel ACCEPTED is countable. With
+ // the outbox in play (ADR-0030 P1) `emit()` returns once the
+ // rows are durable and the dispatcher decides the outcome
+ // afterwards — including dead-lettering an unregistered
+ // channel — so counting the enqueue as `acted` made the run
+ // summary assert a delivery that `sys_notification_delivery`
+ // recorded as `dead` (#7747). The honest answer at the moment
+ // the run settles is "an effect I cannot count yet", which is
+ // exactly `unmeasuredEffect` — the same qualifier a
+ // `connector_action` uses, and pointedly NOT a bare
+ // `acted: 0`, which would claim the run did nothing and trip
+ // the broken-sweep alert on every healthy notify. The alert
+ // is `selected > 0 AND acted = 0 AND unmeasured = 0`, so a
+ // pending delivery correctly suppresses it while refusing to
+ // claim success.
+ //
+ // Waiting for the real outcome is not on the table: a notify
+ // node must not block a flow on a downstream channel.
+ metrics: enqueued > 0
+ ? { ...(delivered > 0 ? { acted: delivered } : {}), unmeasuredEffect: true }
+ : { acted: delivered },
  };
  } catch (err) {
  return { success: false, error: `notify failed: ${(err as Error).message}` };

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

@@ -101,6 +101,10 @@ describe('MessagingService', () => {
  expect(inbox.seen[0].notification.title).toBe('Deal closed');
  expect(result.delivered).toBe(2);
  expect(result.failed).toBe(0);
+ // Inline fan-out leaves nothing in flight — the counterpart to the
+ // outbox pin below, and what keeps `delivered` a terminal count on
+ // BOTH paths rather than a name two things share (#7747).
+ expect(result.enqueued).toBe(0);
  expect(result.notificationId).toMatch(/^evt_/); // synthesized w/o data layer
  expect(result.deliveries[0]).toMatchObject({ channel: 'inbox', recipient: 'user_1', ok: true, externalId: 'row_1' });
  });
@@ -308,7 +312,14 @@ describe('MessagingService', () => {
 
  // Nothing sent inline — the dispatcher owns the send.
  expect(inbox.seen).toHaveLength(0);
- expect(result.delivered).toBe(2); // 2 enqueued (accepted)
+ // …so nothing is DELIVERED yet, and the result says so (#7747). This
+ // pin used to read `delivered: 2` with the comment "2 enqueued
+ // (accepted)" — the conflation itself, written down: callers were
+ // handed an enqueue count under the name `delivered`, and it stayed
+ // put when the dispatcher later dead-lettered the row.
+ expect(result.enqueued).toBe(2);
+ expect(result.delivered).toBe(0);
+ expect(result.failed).toBe(0);
  const rows = await outbox.list();
  expect(rows).toHaveLength(2);
  expect(rows.every((r) => r.status === 'pending')).toBe(true);