diff --git a/.changeset/25758-notify-written-address-no-email-channel.md b/.changeset/25758-notify-written-address-no-email-channel.md new file mode 100644 index 00000000000..ad13039ef67 --- /dev/null +++ b/.changeset/25758-notify-written-address-no-email-channel.md @@ -0,0 +1,21 @@ +--- +'@objectstack/service-automation': patch +--- + +fix(service-automation)!: a `notify` node that does not name the `email` channel fails on an address no user holds, written out or computed, instead of reporting success + +Clause-②: no (narrowing: a `notify` node whose `channels` do not include `email`, and whose `recipients` hold an email address no user holds, written out or evaluated by a value expression, bare or as `user:ADDRESS`, now fails the node at run time instead of reporting success) +Metadata-protocol: none + +An email address written out in `recipients` is the author's constant and is sent as written, and only the email channel sends to an address itself. The inbox delivers to a user, and so does the sms channel (or to a literal phone number). So on a node whose `channels` do not include `email` (the inbox, the default, or `inbox` or `sms` named), an address reaches someone only when a user holds it, wherever the address came from. + +- **BREAKING (run time): such a node now fails when no user holds the address, written out or computed.** Before, the address went on as it stood: the inbox wrote a `sys_inbox_message` row whose `user_id` was the address, which no inbox reads, and the node reported success. Now the node fails before anything is written, and the message names the node, each such recipient and the fix. A written-out address is named; one a value expression (a CEL envelope) evaluated to is named by the expression's source, never by its value. `user:ADDRESS` fails the same way, because `user:` takes a user id and the address is handed on as one with no lookup. With no data engine to look the address up, or a lookup that fails, the node fails too. +- **Unchanged.** An address a user holds, written bare, still reaches that user. A node whose `channels` include `email` sends a written-out address as written, as before. On a node whose `channels` include `email`, a computed address no user holds still fails through the ruling-B screen, pointing to `{ emailField }`. The node never adds the `email` channel its author did not declare. +- **Who moves.** A flow whose `notify` node does not name `email` and writes out an address no user holds, or whose recipient expression evaluates to one (for example a record field holding an address that is not a user's). Fix: add `email` to the node's `channels` to mail a written-out address, or name a user: a user id, an audience selector such as `role:NAME`, or the address that user holds, written bare. A computed address of a person who is not a user goes to its own node with `recipients: { emailField: 'FIELD' }` and `channels: 'email'`. +- **Who moves in this repository (app-showcase).** + - `alert_exec` (flow `showcase_project_escalation`) wrote out `exec@example.com`, which no user holds, on the inbox. It now notifies `role:owner`. + - `notify_cleared` (flow `showcase_invoice_signoff`, the seeded Invoice Dual Sign-off demo) notified `record.owner` on the inbox. The seeded invoices' owners (`ada@`, `linus@` and `grace@example.com`) are deliberately not users: they are the fixture of the ADR-0055 isolation demo, where an operator signs up as one of them, so provisioning them would delete that demo. Approving a seeded invoice would now have failed the run at this node. It now notifies `role:owner`, the Default Organization's admin on a dev boot, who also holds the finance and legal positions that clear the request. `approval-resume-relation-expand.test.ts` approves the seeded INV-1010 through it. + - Its other inbox nodes whose expressions read `record.owner`, `record.assignee` or `ownerId` (projects, tasks) evaluate to addresses its seeds provision as users (pinned by `inert-wirings.test.ts`), so they keep delivering. + - Four nodes write out `admin@objectos.ai` on the inbox: `digest` (`showcase_scheduled_digest`), `log_fail` (`showcase_project_escalation`), `notify_done` (`showcase_release_signoff`) and `report` (`showcase_inquiry_purge`). On a dev boot, where `plugin-auth` seeds that admin, they deliver as before. On a boot without that user they now fail loud, where before they wrote an inbox row nobody reads. + + diff --git a/examples/app-showcase/src/automation/flows/index.ts b/examples/app-showcase/src/automation/flows/index.ts index eac7accd0f0..86e04584a3d 100644 --- a/examples/app-showcase/src/automation/flows/index.ts +++ b/examples/app-showcase/src/automation/flows/index.ts @@ -1344,15 +1344,19 @@ export const InvoiceDualSignoffFlow = defineFlow({ label: 'Notify: Cleared', config: { topic: 'invoice.signoff', - // The INVOICE's own owner, not `record.account.owner` (#7381). - // `showcase_account` has no `owner` field at all — its people-ish keys - // are `billing_email` and the injected `owner_id` — so that hop resolved - // to nothing however the relation was hydrated, and the notify node - // refuses a run with no recipients: approving the demo stranded its run - // instead of delivering this message. `showcase_invoice.owner` is the - // seeded rep (the same RLS anchor the contributor permission set uses), - // which is who "your invoice cleared sign-off" is addressed to anyway. - recipients: [{ dialect: 'cel', source: 'has(record.owner) ? record.owner : null' }], + // [#25758] The organization's owners (`sys_member.role` `owner`), the + // Default Organization's admin on a dev boot, who also holds the + // finance and legal positions that clear the seeded request. + // + // Not `record.owner`: the seeded invoices' owners (ada@, linus@, + // grace@example.com) are deliberately NOT users — they are the + // fixture of the ADR-0055 isolation demo, where an operator signs UP + // as one of them (`demo-personas.ts`, `inert-wirings.test.ts` §5) — + // and an inbox-only node fails on an address no user holds, so + // approving the seeded invoice stranded the run here. Not + // `record.account.owner` either (#7381): `showcase_account` has no + // `owner` field, so that hop resolved to nothing. + recipients: 'role:owner', channels: ['inbox'], severity: 'info', // The expanded relation is read HERE — `{{ record.account.name }}` is what @@ -1432,7 +1436,12 @@ export const ProjectEscalationFlow = defineFlow({ }, { name: 'Exec', - nodes: [{ id: 'alert_exec', type: 'notify', label: 'Alert Exec', config: { recipients: 'exec@example.com', title: '🔴 Critical project: {{ record.name }}', severity: 'critical', sourceObject: 'showcase_project', sourceId: { dialect: 'cel', source: 'record.id' } } }], + // [#25758] The organization's owners (`sys_member.role` `owner`): + // the Default Organization's is its admin. This inbox-only node + // wrote out `exec@example.com`, which no user holds, so the branch + // reached nobody; such a node now fails, and only `channels: 'email'` + // sends to an address itself. + nodes: [{ id: 'alert_exec', type: 'notify', label: 'Alert Exec', config: { recipients: 'role:owner', title: '🔴 Critical project: {{ record.name }}', severity: 'critical', sourceObject: 'showcase_project', sourceId: { dialect: 'cel', source: 'record.id' } } }], edges: [], }, ], diff --git a/examples/app-showcase/test/approval-resume-relation-expand.test.ts b/examples/app-showcase/test/approval-resume-relation-expand.test.ts index 36dcd7cb85d..9761752bc1d 100644 --- a/examples/app-showcase/test/approval-resume-relation-expand.test.ts +++ b/examples/app-showcase/test/approval-resume-relation-expand.test.ts @@ -58,7 +58,8 @@ import { TaskDoneNotifyOwnerFlow, NotifyOwnerSubflow, } from '../src/automation/flows/index.js'; -import { installDefaultOrganization } from './default-organization.testkit.js'; +import { ShowcaseSeedData } from '../src/data/seed/index.js'; +import { DEFAULT_ORGANIZATION_ID, installDefaultOrganization } from './default-organization.testkit.js'; /** Everything the approvals service treats as an unconditional actor. */ const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] } as any; @@ -85,6 +86,62 @@ afterEach(async () => { } }); +/** The dev admin: the Default Organization's `owner`, who clears the seeded invoice. */ +const ADMIN = { id: 'usr_admin', email: 'admin@objectos.ai', name: 'Admin' }; +/** A provisioned demo persona, owning the project the sweep test hops through. */ +const MEI = { id: 'usr_showcase_phone_demo', email: 'phone.demo@example.com', name: 'Mei Phone (demo)' }; + +/** + * [#25758] The directory a dev boot has: the admin, an `owner` member of the + * Default Organization, and a provisioned persona. An inbox-only `notify` + * delivers an address only to the user who holds it, and `role:owner` expands + * through `sys_member`. The seeded invoice owners (grace@example.com here) are + * deliberately not users, as on a dev boot. Seeded through the driver, like + * the Default Organization: data at rest, not a write under test. + */ +async function installDirectory(engine: any): Promise { + if (!engine.getSchema('sys_user')) { + engine.registry.registerObject( + { + name: 'sys_user', + label: 'User', + managedBy: 'better-auth', + fields: { email: { type: 'email', label: 'Email' }, name: { type: 'text', label: 'Name' } }, + }, + '@objectstack/example-showcase', + ); + } + if (!engine.getSchema('sys_member')) { + engine.registry.registerObject( + { + name: 'sys_member', + label: 'Member', + managedBy: 'better-auth', + fields: { + user_id: { type: 'text', label: 'User' }, + role: { type: 'text', label: 'Role' }, + organization_id: { type: 'text', label: 'Organization' }, + }, + }, + '@objectstack/example-showcase', + ); + } + await engine.syncSchemas(); + const users = engine.getDriverForObject('sys_user')!; + for (const user of [ADMIN, MEI]) { + if (!(await users.findOne('sys_user', { where: { id: user.id } }))) await users.create('sys_user', user); + } + const members = engine.getDriverForObject('sys_member')!; + if (!(await members.findOne('sys_member', { where: { id: 'mem_admin_owner' } }))) { + await members.create('sys_member', { + id: 'mem_admin_owner', + user_id: ADMIN.id, + role: 'owner', + organization_id: DEFAULT_ORGANIZATION_ID, + }); + } +} + interface Booted { automation: AutomationEngine & Record; approvals: any; @@ -132,6 +189,7 @@ async function bootShowcaseApprovals( // A real `single` install's Default Organization: the system writes below // derive it (ADR-0131 D9), as a fresh NOT NULL tenant column requires (D1). await installDefaultOrganization(objectql); + await installDirectory(objectql); for (const flow of flows) automation.registerFlow(flow.name, flow); @@ -342,7 +400,15 @@ function inboxSettled(booted: Booted, flowName: string, status: string): () => P }; } -/** Insert the demo's account + a `sent` invoice owned by a real rep. */ +/** + * The app's own seed row for INV-1010, a `sent` invoice: what a dev boot + * stores and the approval demo opens a sign-off on. [#25758] Its owner is one + * of the ADR-0055 isolation fixture's addresses, which no user holds. + */ +const SEEDED_INV_1010 = (ShowcaseSeedData.find((d: any) => d.object === 'showcase_invoice') as any) + .records.find((r: any) => r.name === 'INV-1010'); + +/** Insert the demo's account + the seeded `sent` invoice INV-1010. */ async function seedInvoice(data: any): Promise<{ invoice: any; account: any }> { const account = await one(await data.insert('showcase_account', { name: 'Fabrikam', @@ -352,10 +418,10 @@ async function seedInvoice(data: any): Promise<{ invoice: any; account: any }> { }, SYS)); const invoice = await one(await data.insert('showcase_invoice', { - name: 'INV-1010', + name: SEEDED_INV_1010.name, account: account.id, - owner: 'grace@example.com', - status: 'sent', + owner: SEEDED_INV_1010.owner, + status: SEEDED_INV_1010.status, issued_on: '2026-05-27', tax_rate: 20, region: 'emea', @@ -393,7 +459,7 @@ async function pendingRequest(data: any): Promise { } describe('#7381 — approving Invoice Dual Sign-off delivers the cleared notice', () => { - it('satisfies both unanimous slots and runs through to end_ok with the inbox message delivered', async () => { + it('approving the seeded INV-1010, whose owner is not a user, runs through to end_ok with the inbox message delivered', async () => { const booted = await bootShowcaseApprovals(); const { invoice, account } = await seedInvoice(booted.data); @@ -455,10 +521,10 @@ describe('#7381 — approving Invoice Dual Sign-off delivers the cleared notice' const notice = delivered[0]; expect(notice.title).toBe('Invoice cleared: INV-1010'); - // The recipient is the INVOICE's owner — the field that actually holds a - // person. This is the assertion the shipped `{record.account.owner}` could - // never satisfy, because `showcase_account` has no such field. - expect(notice.user_id).toBe('grace@example.com'); + // [#25758] The seeded invoice's owner holds no account, as on a dev boot; + // the cleared notice reaches the organization's owner, a real user. + expect(await booted.data.findOne('sys_user', { where: { email: invoice.owner }, ...SYS })).toBeFalsy(); + expect(notice.user_id).toBe(ADMIN.id); // The start node's `expand: ['account']` is what makes this readable — an // un-hydrated relation renders the raw FK, or nothing at all. expect(notice.body_md).toContain(account.name); @@ -613,7 +679,7 @@ describe('#7381 — approving Invoice Dual Sign-off delivers the cleared notice' name: 'Compliance Audit', account: account.id, status: 'planned', - owner: 'grace@example.com', + owner: MEI.email, }, SYS)); const task = one(await booted.data.insert('showcase_task', { @@ -638,7 +704,7 @@ describe('#7381 — approving Invoice Dual Sign-off delivers the cleared notice' // annotation is free to name the only other way it can fail (#10106). const delivered = await deliveredInbox(booted, 'Project update', 1); expect(delivered, 'the subflow notified more than one recipient').toHaveLength(1); - expect(delivered[0].user_id, 'the recipient is not the project owner the hop names').toBe('grace@example.com'); + expect(delivered[0].user_id, 'the recipient is not the project owner the hop names').toBe(MEI.id); }, 30_000); }); diff --git a/packages/services/service-automation/src/builtin/notify-email-field-recipient.integration.test.ts b/packages/services/service-automation/src/builtin/notify-email-field-recipient.integration.test.ts index f19c56d8076..cd0116dbff3 100644 --- a/packages/services/service-automation/src/builtin/notify-email-field-recipient.integration.test.ts +++ b/packages/services/service-automation/src/builtin/notify-email-field-recipient.integration.test.ts @@ -330,13 +330,17 @@ describe('#25647 — a computed recipient address must be a user', () => { it.each([ ['names no channel (inbox, the default)', undefined], ['names inbox only', ['inbox']], - ])('E5: a node that %s mails nobody, so its computed address goes to messaging as before', async (_label, channels) => { + ])('E5: a node that %s mails nobody, so the screen does not run; its computed address must still reach a user (#25758)', async (_label, channels) => { const { engine, emitted } = bootUnchecked(); engine.registerFlow('applicant_notice', notifyFlow({ recipients: [CEL('record.contact_text')], channels })); const run = await engine.execute('applicant_notice', onCreate('lead_1', LEAD)); - expect(run.success).toBe(true); - expect(emitted).toHaveLength(1); - expect(emitted[0].audience).toEqual(['stranger@outside.example']); + // No directory to check it: refused as undeliverable, not by the mail screen. + expect(run.success).toBe(false); + expect(emitted).toHaveLength(0); + const runs = await engine.listRuns('applicant_notice'); + const message = runs[0]?.steps.find((s) => s.nodeId === 'notify')?.error?.message ?? ''; + expect(message).toContain('Only the email channel sends to an address'); + expect(message).not.toContain('A computed recipient must be a user'); }); it('E6: a node whose channel list includes email screens its computed address', async () => { diff --git a/packages/services/service-automation/src/builtin/notify-node.ts b/packages/services/service-automation/src/builtin/notify-node.ts index 23d59800446..52a0de75d43 100644 --- a/packages/services/service-automation/src/builtin/notify-node.ts +++ b/packages/services/service-automation/src/builtin/notify-node.ts @@ -209,17 +209,20 @@ type Resolved = { readonly ok: true; readonly value: T } | { readonly ok: fal * refused it. `empty` names each envelope that evaluated to no recipient, for * the guard that fails a node left with none. `computed` names each recipient * an envelope evaluated to that reads as an email address, for the check that - * a user holds it ({@link screenComputedAddresses}). + * a user holds it ({@link screenComputedAddresses}). `addressed` names each + * recipient, written out or computed, that reads as one, for the check that a + * node which does not mail can deliver it ({@link refuseUndeliverableAddresses}). */ function resolveRecipients( engine: AutomationEngine, value: unknown, variables: VariableMap, context: AutomationContext, -): Resolved<{ recipients: string[]; empty: string[]; computed: ComputedAddress[] }> { +): Resolved<{ recipients: string[]; empty: string[]; computed: ComputedAddress[]; addressed: AddressedRecipient[] }> { const recipients: string[] = []; const empty: string[] = []; const computed: ComputedAddress[] = []; + const addressed: AddressedRecipient[] = []; const entries: Array<[string, unknown]> = Array.isArray(value) ? value.map((entry, index) => [`recipients[${index}]`, entry]) : value === undefined ? [] : [['recipients', value]]; @@ -242,15 +245,20 @@ function resolveRecipients( } const recipient = toStr(item); if (recipient === undefined) continue; - const address = isExpressionEnvelopeShaped(raw) ? mailedAddressOf(recipient) : undefined; + const address = mailedAddressOf(recipient); if (address !== undefined) { - computed.push({ index: recipients.length, what, source: sourceOf(raw), address }); + if (isExpressionEnvelopeShaped(raw)) { + computed.push({ index: recipients.length, what, source: sourceOf(raw), address }); + addressed.push({ recipient, address, computed: { what, source: sourceOf(raw) } }); + } else { + addressed.push({ recipient, address }); + } } recipients.push(recipient); } if (isExpressionEnvelopeShaped(raw) && recipients.length === before) empty.push(sourceOf(raw)); } - return { ok: true, value: { recipients, empty, computed } }; + return { ok: true, value: { recipients, empty, computed, addressed } }; } /** The user identity object an address is checked against — the messaging resolver's `USER_OBJECT`. */ @@ -276,6 +284,16 @@ interface ComputedAddress { readonly address: string; } +/** A recipient, written out or computed, that reads as an email address. */ +interface AddressedRecipient { + /** The recipient (`a@b.c`, or `user:a@b.c`). */ + readonly recipient: string; + /** The address in it. */ + readonly address: string; + /** Where a value envelope computed it; absent for one written out as a string. */ + readonly computed?: { readonly what: string; readonly source: string }; +} + /** * The address the email channel would send `recipient` to, or `undefined` * when it sends to none — mirrored from `@objectstack/service-messaging`, @@ -328,7 +346,9 @@ function readsAsAddress(recipient: string): boolean { * So the screen runs only for a node whose `channels` name `email`, the one * channel that sends to the address itself. Any other node (`inbox` alone, * the messaging default) mails nobody, and its recipients go to the messaging - * resolver as before, which maps an address a user holds to that user. + * resolver as before, which maps an address a user holds to that user; an + * address no user holds fails such a node for another reason, that nobody can + * receive it ({@link refuseUndeliverableAddresses}, #25758). * * The lookup is the one the messaging resolver makes (`sys_user` by `email`, * the id projected), so "a user holds it" means here what it means there. No @@ -378,6 +398,84 @@ async function screenComputedAddresses( return { ok: true, value: screened }; } +/** + * [#25758] An address in `recipients`, written out or computed, on a node + * whose `channels` do not include `email`, must reach a user, or the node + * fails: the message names the node, each such recipient and the fix. + * `undefined` when every such address reaches one. + * + * Only the email channel sends to an address itself. The other channels + * deliver to a user: the inbox keys its row by the recipient as a user id, and + * the sms channel texts a user's number (or a literal phone number). So on any + * other node an address reaches someone only when the messaging resolver maps + * it to the user who holds it, and it maps only a bare address (the remainder + * of `user:a@b.c` is handed on as the id with no lookup). Before, an address no + * user held went on as it stood: the inbox wrote a row whose `user_id` was the + * address, which no inbox reads, and the node reported success. + * + * Both provenances are judged alike, because where the address came from does + * not change who can receive it. A written-out address is the author's + * constant (ruling B on #25647), so it is named; a computed one may be record + * text a visitor typed, so it is named by the envelope's `source` and never by + * its value, as {@link screenComputedAddresses} names it. + * + * ⛔ Never re-routed: the node does not add the `email` channel its author did + * not declare, and never writes the row. The lookup is the resolver's own + * (`sys_user` by `email`, the id projected), as in + * {@link screenComputedAddresses}; no data engine, or a lookup that fails, + * fails the node, because an address that could not be checked would reach + * nobody either. + */ +async function refuseUndeliverableAddresses( + data: IDataEngine | undefined, + nodeId: string, + channels: readonly string[], + addressed: readonly AddressedRecipient[], +): Promise { + if (addressed.length === 0) return undefined; + const where = channels.length === 0 + ? 'names no `channels`, so it delivers to the inbox, the default' + : `names \`channels\` ${channels.map((c) => `\`${c}\``).join(', ')} and not \`email\``; + const fixFor = (computed: boolean): string => 'To mail the address as written, add `email` to the node\'s ' + + '`channels`. To reach a user, name the user: a user id, an audience selector such as `role:NAME`, or the ' + + 'address that user holds, written bare.' + + (computed + ? ' A value expression names a user the same way; to mail a person who is not a user, give that address ' + + 'its own `notify` node with `recipients: { emailField: \'FIELD\' }` and `channels: \'email\'`.' + : ''); + const nameOf = (entry: AddressedRecipient): string => (entry.computed + ? `${entry.computed.what} an email address — source: \`${entry.computed.source}\`` + : `\`${entry.recipient}\``); + const undeliverable: string[] = []; + for (const entry of addressed) { + if (entry.recipient !== entry.address) { + undeliverable.push(`${nameOf(entry)} (\`user:\` takes a user id, not an email address)`); + continue; + } + const which = entry.computed ? nameOf(entry) : `its recipient ${nameOf(entry)} is an email address`; + const lead = `notify: node \`${nodeId}\` ${where}, and ${which}. ` + + 'Only the email channel sends to an address; the others deliver to a user'; + if (!data) { + return `${lead}, and there is no data engine to look up whether a user holds it, so nothing was sent. ` + + fixFor(entry.computed !== undefined); + } + let user: Record | null | undefined; + try { + user = await data.findOne(USER_OBJECT, { where: { email: entry.address }, fields: ['id'] }, RECIPIENT_READ); + } catch (err) { + return `${lead}, and looking up the user who holds it failed (${(err as Error).message}), so nothing was ` + + `sent. ${fixFor(entry.computed !== undefined)}`; + } + if (toStr(user?.id) === undefined) undeliverable.push(`${nameOf(entry)} (no user holds that address)`); + } + if (undeliverable.length === 0) return undefined; + const one = undeliverable.length === 1; + return `notify: node \`${nodeId}\` ${where}, and has ${one ? 'a recipient' : 'recipients'} no user receives: ` + + `${undeliverable.join(', ')}. Only the email channel sends to an address; the others deliver to a user, ` + + `so nobody would receive ${one ? 'it' : 'them'}, and nothing was sent. ` + + fixFor(addressed.some((entry) => entry.computed !== undefined)); +} + /** * True when `value` is ONE email address and nothing else — the check the * address an email-field recipient reads must pass before it is mailed. @@ -635,7 +733,7 @@ export function registerNotifyNode(engine: AutomationEngine, ctx: PluginContext) // Studio form keeps its recipients picker, and the marker // rides `NotifyConfigSchema` (`LEDGER_DECLARED_NODE_CONFIG_SCHEMAS`). recipients: { - description: 'Who the notification goes to: a user id or an audience selector (such as team:ID) as a string, or a list of them — each entry a string or a CEL value envelope { dialect: \'cel\', source } evaluated per run; an entry that evaluates to a list adds each of its recipients, and with channels: email an address it evaluates to must be a user\'s. A single-brace {token} is refused. To mail a person who is not a user, use { emailField: \'FIELD\' } as the whole value, naming an email field of the record that triggered the flow, with channels: email; at most 5 such emails go to one address per hour.', + description: 'Who the notification goes to: a user id or an audience selector (such as team:ID) as a string, or a list of them — each entry a string or a CEL value envelope { dialect: \'cel\', source } evaluated per run; an entry that evaluates to a list adds each of its recipients, and with channels: email an address it evaluates to must be a user\'s. An email address written out is sent as written: with channels: email to the address itself, and otherwise to the user who holds it, the node failing when no user does — as it does for an address an entry evaluates to. A single-brace {token} is refused. To mail a person who is not a user, use { emailField: \'FIELD\' } as the whole value, naming an email field of the record that triggered the flow, with channels: email; at most 5 such emails go to one address per hour.', }, // The form edits the bare-string spelling of these two // template slots; the contract (`NotifyConfigSchema`) also @@ -741,6 +839,8 @@ export function registerNotifyNode(engine: AutomationEngine, ctx: PluginContext) // value: the address in that field of the trigger record. [#25647] // Any other recipient an envelope computed that reads as an address // must be a user's when the node mails (`screenComputedAddresses`). + // [#25758] Any address, written out or computed, must reach a + // user when it does not (`refuseUndeliverableAddresses`). const emailField = isEmailFieldRecipient(cfg.recipients) ? cfg.recipients.emailField : undefined; let recipients: string[]; let emptyRecipients: string[] = []; @@ -756,6 +856,14 @@ export function registerNotifyNode(engine: AutomationEngine, ctx: PluginContext) ? await screenComputedAddresses(getData(), audience.value.recipients, audience.value.computed) : { ok: true as const, value: audience.value.recipients }; if (!screened.ok) return { success: false, error: screened.error }; + // [#25758] A node that does not mail delivers only to users, + // so an address among its recipients must be one a user holds. + if (!mails) { + const refused = await refuseUndeliverableAddresses( + getData(), node.id, toStringList(cfg.channels), audience.value.addressed, + ); + if (refused !== undefined) return { success: false, error: refused }; + } recipients = screened.value; emptyRecipients = audience.value.empty; } diff --git a/packages/services/service-automation/src/builtin/notify-written-address-channels.integration.test.ts b/packages/services/service-automation/src/builtin/notify-written-address-channels.integration.test.ts new file mode 100644 index 00000000000..be6dd573377 --- /dev/null +++ b/packages/services/service-automation/src/builtin/notify-written-address-channels.integration.test.ts @@ -0,0 +1,233 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { + MessagingService, + NotificationTemplateStore, + createEmailChannel, + createInboxChannel, + INBOX_OBJECT, + NOTIFICATION_EVENT_OBJECT, +} from '@objectstack/service-messaging'; +import { assertEngineFindOnePredicate, type EngineFindOneQueryInput } from '@objectstack/metadata-core'; +import { AutomationEngine } from '../engine.js'; +import { registerNotifyNode } from './notify-node.js'; + +/** + * [#25758] An address WRITTEN OUT in `recipients` is the author's constant + * (ruling 6386629237 letter B), and only the email channel sends to an address + * itself. The inbox keys a message by a user id, and the sms channel texts a + * user's number or a literal phone number. So on a node whose `channels` do + * not include `email`, an address no user holds reaches nobody: the inbox used + * to write a `sys_inbox_message` row whose `user_id` was the address, which no + * inbox reads, and the node reported success. + * + * Such a node now fails, naming the node, the recipient and the fix; nothing + * is written, and the node never adds the email channel it did not declare. + * + * Wired through the REAL `MessagingService` (inline fan-out) and the REAL + * inbox and email channels, over a data engine that stores the directory and + * records every row the stack writes. + * + * A. A written-out address no user holds, on a node that does not name + * `email`, fails the node and writes nothing. + * B. A written-out address a user holds reaches that user's inbox, as before. + * C. A node whose `channels` include `email` sends the address as written. + * D. With no directory to check it, the address is not sent. + * E. The same holds for an address a value envelope COMPUTES: on a node that + * does not name `email`, one no user holds fails the node, naming the + * envelope's source and never the value; one a user holds reaches that + * user. On a node that names `email`, ruling B's screen answers as before. + */ + +function silentLogger(): any { + const l: any = { info: () => {}, warn: () => {}, error: () => {}, debug: () => {} }; + l.child = () => l; + return l; +} + +interface Row { [key: string]: unknown; id: string } + +/** A data engine holding the users, and recording every row the stack writes. */ +function dataEngine(users: Row[]) { + const written: Array<{ object: string; row: Record }> = []; + let seq = 0; + const tables: Record = { sys_user: users }; + const engine = { + async insert(object: string, row: Record) { + const stored = { ...row, id: row.id != null ? String(row.id) : `row_${++seq}` }; + written.push({ object, row: stored }); + return stored; + }, + async find() { return []; }, + async findOne(object: string, query?: EngineFindOneQueryInput) { + assertEngineFindOnePredicate(object, query); + const where = (query as { where?: Record } | undefined)?.where ?? {}; + // Equality on plain fields only: a combinator or operator is refused, never read as a field name. + const hit = (tables[object] ?? []).find((r) => Object.entries(where).every(([k, v]) => { + if (k.startsWith('$') || (typeof v === 'object' && v !== null)) { + throw new Error(`this double matches equality on plain fields only: ${JSON.stringify(where)}`); + } + return r[k] === v; + })); + return hit ? { ...hit } : undefined; + }, + }; + return { engine, written }; +} + +const EXEC = { id: 'usr_exec', email: 'exec@company.example' }; + +/** The stack: messaging with the inbox and email channels, and the notify node over them. */ +function boot(opts: { withData?: boolean } = {}) { + const { engine: data, written } = dataEngine([EXEC]); + const getData = () => (opts.withData === false ? undefined : (data as any)); + const mailed: unknown[] = []; + const messaging = new MessagingService({ logger: silentLogger(), getData }); + messaging.registerChannel(createInboxChannel({ getData })); + messaging.registerChannel(createEmailChannel({ + getEmail: () => ({ + async send(input) { + mailed.push(input.to); + return { id: `mail_${mailed.length}` }; + }, + }), + getData, + store: new NotificationTemplateStore({ getData: () => undefined }), + })); + const engine = new AutomationEngine(silentLogger()); + registerNotifyNode(engine, { + logger: silentLogger(), + getService: (name: string) => (name === 'messaging' ? messaging : name === 'data' ? getData() : undefined), + } as any); + return { engine, written, mailed }; +} + +function notifyFlow(config: Record): any { + return { + name: 'escalation', + label: 'Escalation', + type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'alert_exec', type: 'notify', label: 'Alert Exec', config: { title: 'Critical', ...config } }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'alert_exec' }, + { id: 'e2', source: 'alert_exec', target: 'end' }, + ], + }; +} + +async function runOnce(stack: ReturnType, config: Record, record?: Record) { + stack.engine.registerFlow('escalation', notifyFlow(config)); + const context = record ? { tenantId: 'org_1', object: 'crm_lead', record } : { tenantId: 'org_1' }; + const run = await stack.engine.execute('escalation', context as any); + const runs = await stack.engine.listRuns('escalation'); + const step = runs[0]?.steps.find((s) => s.nodeId === 'alert_exec'); + return { run, step }; +} + +const rowsOf = (stack: ReturnType, object: string) => + stack.written.filter((w) => w.object === object).map((w) => w.row); + +describe('#25758 — a written-out address on a node that does not name email', () => { + it.each([ + ['names no channel (inbox, the default)', { recipients: 'exec@example.com' }, 'exec@example.com'], + ['names inbox only', { recipients: ['exec@example.com'], channels: ['inbox'] }, 'exec@example.com'], + ['names sms only', { recipients: ['exec@example.com'], channels: 'sms' }, 'exec@example.com'], + ['writes it as user:ADDRESS', { recipients: ['user:exec@example.com'] }, 'user:exec@example.com'], + ['writes a user\'s address as user:ADDRESS', { recipients: ['user:exec@company.example'] }, 'user:exec@company.example'], + ])('A: a node that %s fails, naming the node, the recipient and the fix, and writes nothing', async (_label, config, recipient) => { + const stack = boot(); + const { run, step } = await runOnce(stack, config); + expect(run.success).toBe(false); + const message = step?.error?.message ?? ''; + expect(message).toContain('`alert_exec`'); + expect(message).toContain(`\`${recipient}\``); + expect(message).toContain('`channels`'); + expect(message).toContain('user id'); + expect(rowsOf(stack, NOTIFICATION_EVENT_OBJECT)).toHaveLength(0); + expect(rowsOf(stack, INBOX_OBJECT)).toHaveLength(0); + // Never re-routed to the channel the author did not declare. + expect(stack.mailed).toHaveLength(0); + }); + + it('A: names every such recipient of a list, not just the first', async () => { + const stack = boot(); + const { run, step } = await runOnce(stack, { recipients: ['usr_exec', 'exec@example.com', 'cfo@example.com'] }); + expect(run.success).toBe(false); + expect(step?.error?.message).toContain('`exec@example.com`'); + expect(step?.error?.message).toContain('`cfo@example.com`'); + expect(rowsOf(stack, INBOX_OBJECT)).toHaveLength(0); + }); + + it('B: a written-out address a user holds reaches that user\'s inbox, as before', async () => { + const stack = boot(); + const { run } = await runOnce(stack, { recipients: 'exec@company.example' }); + expect(run.success).toBe(true); + expect(rowsOf(stack, INBOX_OBJECT).map((r) => r.user_id)).toEqual(['usr_exec']); + }); + + it.each([ + ['email only', 'email'], + ['inbox and email', ['inbox', 'email']], + ])('C: a node whose channels are %s sends the address as written', async (_label, channels) => { + const stack = boot(); + const { run } = await runOnce(stack, { recipients: ['exec@example.com'], channels }); + expect(run.success).toBe(true); + expect(stack.mailed).toEqual(['exec@example.com']); + }); + + it('D: with no directory to check it, a written-out address on an inbox node is not sent', async () => { + const stack = boot({ withData: false }); + const { run, step } = await runOnce(stack, { recipients: 'exec@example.com' }); + expect(run.success).toBe(false); + expect(step?.error?.message).toContain('`exec@example.com`'); + expect(stack.mailed).toHaveLength(0); + }); +}); + +describe('#25758 — a computed address on a node that does not name email', () => { + const CEL = (source: string) => ({ dialect: 'cel', source }); + const LEAD = { + id: 'lead_1', + contact: 'stranger@outside.example', + prefixed: 'user:exec@company.example', + owner_email: 'exec@company.example', + }; + + it.each([ + ['an address no user holds', 'record.contact', undefined], + ['user: plus an address', 'record.prefixed', undefined], + ['an address no user holds, on sms', 'record.contact', 'sms'], + ])('E: an envelope that evaluates to %s fails the node, naming its source and not the value', async (_label, source, channels) => { + const stack = boot(); + const { run, step } = await runOnce(stack, { recipients: [CEL(source)], ...(channels ? { channels } : {}) }, LEAD); + expect(run.success).toBe(false); + const message = step?.error?.message ?? ''; + expect(message).toContain('`alert_exec`'); + expect(message).toContain(`source: \`${source}\``); + expect(message).toContain('`channels`'); + expect(message).not.toContain('stranger@outside.example'); + expect(rowsOf(stack, NOTIFICATION_EVENT_OBJECT)).toHaveLength(0); + expect(rowsOf(stack, INBOX_OBJECT)).toHaveLength(0); + expect(stack.mailed).toHaveLength(0); + }); + + it('E: an envelope that evaluates to an address a user holds reaches that user\'s inbox', async () => { + const stack = boot(); + const { run } = await runOnce(stack, { recipients: CEL('record.owner_email') }, LEAD); + expect(run.success).toBe(true); + expect(rowsOf(stack, INBOX_OBJECT).map((r) => r.user_id)).toEqual(['usr_exec']); + }); + + it('E: on a node that names email, ruling B\'s screen answers as before', async () => { + const stack = boot(); + const { run, step } = await runOnce(stack, { recipients: [CEL('record.contact')], channels: ['inbox', 'email'] }, LEAD); + expect(run.success).toBe(false); + expect(step?.error?.message).toContain('no user holds it'); + expect(step?.error?.message).toContain("`recipients: { emailField: 'FIELD' }`"); + }); +}); diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 1751d20eb92..662ae6961c5 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -4486,6 +4486,11 @@ "verb": "findOne", "pinned": 1 }, + { + "file": "packages/services/service-automation/src/builtin/notify-written-address-channels.integration.test.ts", + "verb": "findOne", + "pinned": 1 + }, { "file": "packages/services/service-automation/src/builtin/notify-zero-delivery-visibility.integration.test.ts", "verb": "findOne",