Skip to content

Commit d12b0c1

Browse files
fix(service-automation): a notify node naming email and another channel mails an unheld written-out address and writes no inbox row for it (#26039)
<!-- claude-projects-attribution: {"github_login":"os-project-manager"} --> _Requested by **Leehom** · [project thread](https://claude.ai/code/project/chan_01TenzsRopZj4jMbvjX8uz1X?thread=cmsg_01TenzsRopZj4jMbvjX8uz1X1vxQ5eGqwz99QtDkLa1w1V)_ Fixes #25990 Clause-②: no Metadata-protocol: none The sibling of #25758 (PR #25964), under the card's ruling: on a `notify` node naming both `inbox` and `email`, a written-out address no user holds goes to the email channel only, the inbox writes no row for it, and the node still succeeds. Identity: claim `6412253095` (branch `claude/project-thread-puin6e`). ## What changed | `notify` node with `channels` naming `email` and another channel | Before | Now | |---|---|---| | written-out address no user holds, bare | mailed, AND a `sys_inbox_message` row whose `user_id` is the address; success | mailed only, no inbox row; success | | `user:ADDRESS` (any address, held or not) | same junk row (the resolver hands the remainder on as the id with no lookup) | mailed only; success | | address whose holder cannot be looked up (no data engine, lookup throws) | junk row + mail; success | mailed only; success | | written-out address a user holds, bare | that user's inbox + mail | unchanged | | computed address on a mailing node | resolved to its user, or the node fails (ruling-B screen) | unchanged | | node without `email` | fails on an unheld address (#25964) | unchanged | - `service-automation` `notify-node.ts`: new `mailOnlyAddresses` collects the written-out recipients only the email channel can deliver, using the exact test `refuseUndeliverableAddresses` (#25964) makes, now shared as `holderOf` (the `sys_user` by `email` lookup, id projected, system read) and `namesAddressAsUserId` (the `user:` rule). `screenComputedAddresses` and `refuseUndeliverableAddresses` call the same `holderOf`, so there is one address test in the file, not a second resolver. - When the node mails and names another channel, `execute` emits those recipients apart with `channels: ['email']`, and every other recipient as before on the node's `channels`. `notificationId` names the every-channel notification (the one the inbox shows) when there is one; `delivered`, `enqueued` and `failed` add up both emits. A node with no such recipient makes exactly the one emit it made before. - The `recipients` help served to the Studio inspector says what such an address does. - `scripts/engine-double-contract.pinned.json`: the new test's `findOne` double, written by `check:engine-double-contract --write`. - Changeset: `@objectstack/service-automation` `patch`. ## Where the skip lives (PM hypothesis, measured) In the node, not the inbox channel. Measured on `main` `b0384f3f`: - `EmitInput` (`service-messaging/src/messaging-service.ts:75`) carries ONE `audience` and ONE `channels` list; there is no per-recipient channel restriction a producer can pass, so a node-side skip is two emits, and a messaging-side skip would be a new field on the service contract. - `RecipientResolver.resolveEmail` keeps an unheld address verbatim on purpose ("a downstream channel may still key a row by it"), and the inbox channel keys `user_id` by whatever recipient it receives. Making the inbox refuse a non-id would change what `user_id` means to that channel for every producer (`collab.mention`, audit writers, any `emit`), and on the outbox path it would still enqueue a `sys_notification_delivery` row that then fails. - The node is the one place that knows a recipient was WRITTEN OUT as an address and which `channels` its author named, so the skip is there, and every channel and the resolver keep their one meaning of a recipient. ## Is it a narrowing (measured against AGENTS.md) No. Every node that succeeded still succeeds and every node that failed still fails: the split runs only after the parse, the ruling-B screen and the #25758 refusal have answered, and it only chooses which channels an address is handed to. No key, value or stored shape moves and the parse is unchanged, so `Clause-②: no`, `Metadata-protocol: none`, `patch`. The claim carried `Clause-②: yes (expected: … unchanged; the dev measures)`; measured, the accept set is unchanged in both directions, so the line is `no` with no arm. ## Tests - New `notify-both-channels-unheld-address.integration.test.ts` (7 cases), real `MessagingService` (inline fan-out), real inbox and email channels, data engine recording every row: bare unheld address, `user:ADDRESS` unheld and held, a list beside a user (inbox row for the user only, both mailed, `selected: 2, acted: 3`, two `sys_notification` events), a directory lookup that throws; controls: a held bare address reaches its user's inbox and is mailed, a computed held address reaches its user's inbox. - **Red first** (commit `da686d76`, before the fix, on `main` `b0384f3f`): 5 failed (each on the inbox row whose `user_id` is the address), the 2 controls passed. - **Ablation** (`scripts/ablation-replace.mjs`, one behaviour: the `mailOnlyAddresses` call replaced by an empty set; the test imports the node from `src`, no `dist` in the path): 5 failed, 17 passed across this file and `notify-written-address-channels.integration.test.ts` (the 5 are exactly the fixed cases). Restore: blob `e7907826` equals HEAD, `git diff HEAD` empty. ## Verification Under the verify lock, at `1ce55062` (the ledger pin `745715539` after it changes no source): - `pnpm --filter @objectstack/service-automation test`: 205 files / 2689 passed; `typecheck` (with `check:test-typecheck`): exit 0. - `eslint --no-inline-config packages/services/service-automation`: exit 0. - Derived gates (`dispatch-gates --commands`, 76 families on the final diff): 75 exit 0; `check:dual-build-cjs-loads` exit 3 PREREQUISITE NOT MET (needs a full-repo build; CI runs it). Reconciled with `--ran`: 76 accounted, 0 unrun. - Open PRs on `notify-node.ts`: none found (open-PR search on `notify` / `service-automation`, 2026-10-26T10:02Z). ## Acceptance notes - A run whose recipients mix users and such addresses now writes two `sys_notification` events for one node (users on the node's `channels`, addresses on `email`). The node's output keeps its keys; `notificationId` names the first. If the second emit throws after the first was accepted, the node fails with the users' notification already written: a partial outcome one emit could not have. `emit()` throws only before its fan-out (a malformed keyed text, an event write the engine refuses), and both emits carry the same input, so the second refusing after the first was accepted needs the engine to change its answer between them. - The rule covers every channel beside `email`, not only `inbox`: on a node naming `email` and `sms`, such an address is no longer handed to sms, where it used to be a failed delivery (`no phone number for recipient`), so `failed` drops by one for it. - `@objectstack/spec` describe text for `recipients` (`io-node-config.zod.ts`) does not say what a both-channels node does with an unheld address; that is the `domain:spec` text, not in this PR. --- _Generated by [Claude Code](https://claude.ai/code/session_01617Hq2GbkLYhXsfVgtvNuR)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent a52cfaf commit d12b0c1

4 files changed

Lines changed: 305 additions & 17 deletions

File tree

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
'@objectstack/service-automation': patch
3+
---
4+
5+
fix(service-automation): a `notify` node naming `email` and another channel mails an address no user holds and no longer writes an inbox row addressed to it
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
An email address written out in `recipients` is the author's constant: on a node whose `channels` name `email` it is mailed as written. The inbox delivers to a user, keying its row by the recipient as a user id, and so does every channel but `email`. So on a node whose `channels` name `email` and another channel, such as `['inbox', 'email']`, an address no user holds now goes to the email channel alone.
11+
12+
- **Fixed.** Before, the address went to every channel as it stood: the email channel mailed it, and the inbox also wrote a `sys_inbox_message` row whose `user_id` was the address, which no inbox reads. Now the node emits that address apart, on the `email` channel only, and the inbox writes no row for it. `user:ADDRESS` is handled the same way, whoever holds the address, because `user:` hands the address on as the user id with no lookup. An address whose holder cannot be looked up (no data engine, a lookup that fails) is mailed alone too.
13+
- **Unchanged.** The node succeeds as before. An address a user holds, written bare, still reaches that user's inbox and is mailed. A value expression that evaluates to an address on a node naming `email` still resolves to the user who holds it, or fails the node pointing to `{ emailField }`. A node that does not name `email` still fails on an address no user holds.
14+
- **What a run records.** When a node's recipients mix users and such addresses, the node writes two `sys_notification` events: one for the users on the node's `channels`, which the node's `notificationId` output names, and one for the addresses on `email` alone. The `delivered`, `enqueued` and `failed` outputs add up both.
15+
- **Who moves.** Nobody needs to change a flow. A report or audit over `sys_inbox_message` no longer sees rows whose `user_id` is an address.
Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect } from 'vitest';
4+
import {
5+
MessagingService,
6+
NotificationTemplateStore,
7+
createEmailChannel,
8+
createInboxChannel,
9+
INBOX_OBJECT,
10+
NOTIFICATION_EVENT_OBJECT,
11+
} from '@objectstack/service-messaging';
12+
import { assertEngineFindOnePredicate, type EngineFindOneQueryInput } from '@objectstack/metadata-core';
13+
import { AutomationEngine } from '../engine.js';
14+
import { registerNotifyNode } from './notify-node.js';
15+
16+
/**
17+
* [#25990] On a node whose `channels` name `email` AND another channel, an
18+
* address written out in `recipients` that no user holds is mailed as written
19+
* (ruling 6386629237 letter B). The other channels deliver to a user: the inbox
20+
* keys its row by the recipient as a user id. So the address goes to the email
21+
* channel only, and the inbox writes no row for it; before, it wrote a
22+
* `sys_inbox_message` row whose `user_id` was the address, which no inbox reads.
23+
*
24+
* The node still succeeds when the email delivery succeeds, and an address a
25+
* user holds keeps reaching that user's inbox and is mailed as before.
26+
*
27+
* Wired through the REAL `MessagingService` (inline fan-out) and the REAL
28+
* inbox and email channels, over a data engine that stores the directory and
29+
* records every row the stack writes.
30+
*/
31+
32+
function silentLogger(): any {
33+
const l: any = { info: () => {}, warn: () => {}, error: () => {}, debug: () => {} };
34+
l.child = () => l;
35+
return l;
36+
}
37+
38+
interface Row { [key: string]: unknown; id: string }
39+
40+
/** A data engine holding the users, and recording every row the stack writes. */
41+
function dataEngine(users: Row[], opts: { failUserLookup?: boolean } = {}) {
42+
const written: Array<{ object: string; row: Record<string, unknown> }> = [];
43+
let seq = 0;
44+
const tables: Record<string, Row[]> = { sys_user: users };
45+
const engine = {
46+
async insert(object: string, row: Record<string, unknown>) {
47+
const stored = { ...row, id: row.id != null ? String(row.id) : `row_${++seq}` };
48+
written.push({ object, row: stored });
49+
return stored;
50+
},
51+
async find() { return []; },
52+
async findOne(object: string, query?: EngineFindOneQueryInput) {
53+
assertEngineFindOnePredicate(object, query);
54+
const where = (query as { where?: Record<string, unknown> } | undefined)?.where ?? {};
55+
if (opts.failUserLookup && object === 'sys_user' && 'email' in where) throw new Error('directory offline');
56+
// Equality on plain fields only: a combinator or operator is refused, never read as a field name.
57+
const hit = (tables[object] ?? []).find((r) => Object.entries(where).every(([k, v]) => {
58+
if (k.startsWith('$') || (typeof v === 'object' && v !== null)) {
59+
throw new Error(`this double matches equality on plain fields only: ${JSON.stringify(where)}`);
60+
}
61+
return r[k] === v;
62+
}));
63+
return hit ? { ...hit } : undefined;
64+
},
65+
};
66+
return { engine, written };
67+
}
68+
69+
const EXEC = { id: 'usr_exec', email: 'exec@company.example' };
70+
71+
/** The stack: messaging with the inbox and email channels, and the notify node over them. */
72+
function boot(opts: { failUserLookup?: boolean } = {}) {
73+
const { engine: data, written } = dataEngine([EXEC], opts);
74+
const getData = () => data as any;
75+
const mailed: unknown[] = [];
76+
const messaging = new MessagingService({ logger: silentLogger(), getData });
77+
messaging.registerChannel(createInboxChannel({ getData }));
78+
messaging.registerChannel(createEmailChannel({
79+
getEmail: () => ({
80+
async send(input) {
81+
mailed.push(input.to);
82+
return { id: `mail_${mailed.length}` };
83+
},
84+
}),
85+
getData,
86+
store: new NotificationTemplateStore({ getData: () => undefined }),
87+
}));
88+
const engine = new AutomationEngine(silentLogger());
89+
registerNotifyNode(engine, {
90+
logger: silentLogger(),
91+
getService: (name: string) => (name === 'messaging' ? messaging : name === 'data' ? getData() : undefined),
92+
} as any);
93+
return { engine, written, mailed };
94+
}
95+
96+
function notifyFlow(config: Record<string, unknown>): any {
97+
return {
98+
name: 'escalation',
99+
label: 'Escalation',
100+
type: 'autolaunched',
101+
nodes: [
102+
{ id: 'start', type: 'start', label: 'Start' },
103+
{ id: 'alert_exec', type: 'notify', label: 'Alert Exec', config: { title: 'Critical', ...config } },
104+
{ id: 'end', type: 'end', label: 'End' },
105+
],
106+
edges: [
107+
{ id: 'e1', source: 'start', target: 'alert_exec' },
108+
{ id: 'e2', source: 'alert_exec', target: 'end' },
109+
],
110+
};
111+
}
112+
113+
async function runOnce(stack: ReturnType<typeof boot>, config: Record<string, unknown>, record?: Record<string, unknown>) {
114+
stack.engine.registerFlow('escalation', notifyFlow(config));
115+
const context = record ? { tenantId: 'org_1', object: 'crm_lead', record } : { tenantId: 'org_1' };
116+
const run = await stack.engine.execute('escalation', context as any);
117+
const runs = await stack.engine.listRuns('escalation');
118+
const step = runs[0]?.steps.find((s) => s.nodeId === 'alert_exec');
119+
return { run, step };
120+
}
121+
122+
const rowsOf = (stack: ReturnType<typeof boot>, object: string) =>
123+
stack.written.filter((w) => w.object === object).map((w) => w.row);
124+
125+
const BOTH = ['inbox', 'email'];
126+
127+
describe('#25990 — a written-out address no user holds, on a node naming inbox and email', () => {
128+
it.each([
129+
['bare', 'exec@example.com'],
130+
['as user:ADDRESS', 'user:exec@example.com'],
131+
['as user:ADDRESS of an address a user holds', 'user:exec@company.example'],
132+
])('is mailed as written and writes no inbox row (%s)', async (_label, recipient) => {
133+
const stack = boot();
134+
const { run } = await runOnce(stack, { recipients: [recipient], channels: BOTH });
135+
expect(run.success).toBe(true);
136+
expect(stack.mailed).toEqual([recipient.replace(/^user:/, '')]);
137+
expect(rowsOf(stack, INBOX_OBJECT)).toEqual([]);
138+
// The one recipient, and the one delivery a channel accepted: its mail.
139+
expect(run.summary).toMatchObject({ selected: 1, acted: 1 });
140+
});
141+
142+
it('beside a user, the user still gets the inbox row and the mail; the address only the mail', async () => {
143+
const stack = boot();
144+
const { run } = await runOnce(stack, { recipients: ['usr_exec', 'stranger@example.com'], channels: BOTH });
145+
expect(run.success).toBe(true);
146+
expect(rowsOf(stack, INBOX_OBJECT).map((r) => r.user_id)).toEqual(['usr_exec']);
147+
expect([...stack.mailed].sort()).toEqual(['exec@company.example', 'stranger@example.com']);
148+
// Both recipients are counted, and every delivery a channel accepted:
149+
// the user's inbox row and mail, and the address's mail.
150+
expect(run.summary).toMatchObject({ selected: 2, acted: 3 });
151+
// The address's mail is its own notification, on the email channel alone.
152+
expect(rowsOf(stack, NOTIFICATION_EVENT_OBJECT)).toHaveLength(2);
153+
});
154+
155+
it('a written-out address a user holds keeps reaching that user\'s inbox and is mailed, as before', async () => {
156+
const stack = boot();
157+
const { run } = await runOnce(stack, { recipients: ['exec@company.example'], channels: BOTH });
158+
expect(run.success).toBe(true);
159+
expect(rowsOf(stack, INBOX_OBJECT).map((r) => r.user_id)).toEqual(['usr_exec']);
160+
expect(stack.mailed).toEqual(['exec@company.example']);
161+
});
162+
163+
it('when the directory cannot say who holds it, the address is still mailed and never keys an inbox row', async () => {
164+
const stack = boot({ failUserLookup: true });
165+
const { run } = await runOnce(stack, { recipients: ['exec@example.com'], channels: BOTH });
166+
expect(run.success).toBe(true);
167+
expect(stack.mailed).toEqual(['exec@example.com']);
168+
expect(rowsOf(stack, INBOX_OBJECT)).toEqual([]);
169+
});
170+
171+
it('a computed address a user holds reaches that user\'s inbox on a both-channels node', async () => {
172+
const stack = boot();
173+
const { run } = await runOnce(
174+
stack,
175+
{ recipients: { dialect: 'cel', source: 'record.owner_email' }, channels: BOTH },
176+
{ id: 'lead_1', owner_email: 'exec@company.example' },
177+
);
178+
expect(run.success).toBe(true);
179+
expect(rowsOf(stack, INBOX_OBJECT).map((r) => r.user_id)).toEqual(['usr_exec']);
180+
expect(stack.mailed).toEqual(['exec@company.example']);
181+
});
182+
});

0 commit comments

Comments
 (0)