Skip to content

Commit a2c2852

Browse files
claude[bot]claude
andauthored
feat(service-messaging,platform-objects): fan-out consults per-tenant channel availability and writes no delivery row for a channel with no transport (#17732) (#18041)
Fixes #17732 Implements the ruling `5644350987` (director seat, decision batch #122 item 5, 2026-09-12), routed to `domain:services` through triage's cross-domain exception path (`5650914775`). `Clause-②: yes` ## What changed Fan-out asks a channel whether the tenant can send on it **before** it writes anything. 1. **`MessagingChannel` gains one OPTIONAL member** — `isAvailable(ctx, { organizationId })` answering `{ available: true }` or `{ available: false, reason }` from the closed vocabulary `CHANNEL_UNAVAILABLE_REASONS` (today: `transport_not_configured`). 2. **`emit()` consults it once per channel, before the event write** — a channel that answers unavailable gets no `sys_notification_delivery` row and no `send()` call, on the outbox (P1) *and* the inline (P0) path. 3. **`sys_notification` gains one key**, `suppressed_channels` — `[{ channel, reason }]`, `NULL` when nothing was suppressed — written in the **same insert** that creates the event row, so the feature costs zero additional writes. 4. **The `email` channel implements it**, answering from the transport it was handed. **The `inbox` channel implements nothing** — which is literally "always available" per the ruling, and doubles as the production instance of the optional-member default. 5. **`EmitResult` gains `suppressed`** so a two-channel emit that enqueued one row is never indistinguishable from a fan-out bug. ## The transport-cost measurement (ruling item 3), and what was done about it > "the email channel answers from the tenant's transport configuration — the seat measures whether that answer is cheap at fan-out time and caches per tenant per tick if it is not" **Measured, not assumed.** Two readings, both on this branch: | | ns/op | |:--|--:| | `email.isAvailable()` — service present | **4.8** | | `email.isAvailable()` — service absent | **31.1** | | *(scale)* empty arrow call | 10.2 | | *(scale)* `await` a **resolved** no-op `findOne` — the in-process floor of any I/O-shaped answer | **121.3** | | *(scale)* a real single-row SELECT over loopback | O(100 000) | And the call count, driven through the real `emit()` with 50 recipients × 2 channels: ``` availability probes performed : 1 delivery rows written : 50 (0 email) delivery rows AVOIDED : 50 ``` ⇒ The probe is **synchronous and in-memory**, so it is cheaper than merely *awaiting an already-resolved promise*, and it runs **once per channel per emit** — O(channels), never O(deliveries). The three nanosecond figures are within measurement noise of each other; the honest statement is that the probe is not distinguishable from call overhead, and the "present" row reading below the empty-call baseline is that noise, not a speedup. ⛔ **No cache was added**, and the ruling's condition is why: it says cache *if the answer is not cheap*, and it is. A cache here would also be **wrong**. Mail configuration in this tree is the `mail` settings namespace at `scope: 'global'` (`packages/services/service-settings/src/manifests/mail.manifest.ts`), materialised into a **single in-memory** `IEmailTransport` that `EmailServicePlugin` **hot-swaps** from the settings change bus. A per-tenant-per-tick memo would keep answering "unavailable" straight through the settings save that fixed it. ⚠️ **A reading the ruling's wording did not anticipate, reported rather than papered over**: there is **no per-tenant transport configuration** in this tree to read — `scope: 'global'` is deployment-wide. The seam still takes the tenant context so the day mail becomes tenant-scoped, the answer changes inside the channel and no published interface has to move again. ## The discriminating controls, and what each would have caught Every one of these is a test that fails against a *differently* broken implementation, which is why they are pinned rather than assumed. | control | what it catches | |:--|:--| | unavailable channel gets **no** row, an available channel in the **same** fan-out **does**, and the reason is on `sys_notification` — asserted on **one** pass | "no row was written" alone also passes a fan-out that wrote **nothing at all**; "the available channel got its row" alone also passes the **old** code | | **a channel with no `isAvailable` still gets its delivery row**, and `suppressed_channels` is `NULL` | an implementation that inverted the optional-member default would pass every suppression test here **and silently mute every channel that has not been updated** — far worse than the bug being fixed | | the same channel **suppressed once it does answer** unavailable | pairs with the row above on one variable, so "it got its row" cannot be read as "suppression never works in this harness" | | probe count is **1** for a 4-recipient emit, and carries the tenant | a probe that ran per delivery would multiply its cost by the audience — the thing the ruling asked to be measured | | a **throwing** probe still delivers, and warns | fail-open: a broken availability check must degrade into today's behaviour, never into a silent notification outage | | the object's inlined reason enum equals `CHANNEL_UNAVAILABLE_REASONS` | `packages/platform-objects` is a lower layer and cannot import the vocabulary, so the two copies are held equal by an assertion — a comment would not | | an **unregistered** channel keeps its existing path | pins the scope boundary as deliberate rather than accidental | ## ⚠️ Two reds this PR caused, and what they were Both were **mine**, both are fixed, and neither was a test asserting the behaviour the ruling changed. ### 1. `Test Core (4/6)` — `service-automation`, 4 tests in `notify-zero-delivery-visibility.integration.test.ts` The error text names the cause exactly: ``` Node 'notify' failed: notify failed: Unknown field 'suppressed_channels' on object 'sys_notification' ``` That harness declares `sys_notification` as a **fixture** whose own comment says it is "exactly the columns `MessagingService.writeEvent` inserts — a fixture that drifts from the producer fails loudly on the SQL arm". It is a producer-drift detector, and it did its job. ⭐ **The fix was in the producer, not the test.** The first draft named `suppressed_channels` on **every** insert (as `null` when nothing was suppressed). An insert names its columns, so that made every `emit()` in the world depend on every `sys_notification` schema already carrying the new column — losing the whole notification to record that nothing was suppressed. `writeEvent` now adds the key **only when there is something to say**, so the common path's column set is exactly what it was before this change. Pinned two ways in `channel-availability.test.ts`: the key is **absent** (not `null`) on the control path, and the common-path column set is enumerated. ⛔ **No assertion was retuned.** The automation fixture is untouched by this PR, and no test of the ruled behaviour was weakened. **Causation, with the control:** | tree | `pnpm --filter @objectstack/service-automation test` | |:--|:--| | merge-base `1e20f816e`, clean worktree, none of my commits | **134 files / 1581 tests passed, 0 failed** | | my branch `e57ee063c` (column named on every insert) | 1 file failed / **4 tests failed** (CI run 34763053926) | | my branch `684df79a9` (column named only when non-empty) | **134 files / 1581 tests passed, 0 failed** | And a second, independent control on the cause itself: `suppressed_channels` occurs **0** times in the merge-base tree and 13 times on this branch, with `dedup_key` as the positive control for the same `git grep` against that same tree (**44** files). A failure whose cause is a string that does not exist on `origin/main` cannot be pre-existing. ### 2. `Type Check · workspace` — `Object.hasOwn` is outside this package's lib Two sites, both in the test file added by the previous push. Reproduced first (`pnpm --filter @objectstack/service-messaging typecheck` → exit 2, `error TS2550` at `channel-availability.test.ts:188` and `:356`), then fixed to `Object.prototype.hasOwnProperty.call(...)` — the spelling this package's siblings already use — then re-run clean (exit 0) with the package suite re-run after it. ⛔ **No tsconfig was touched**: adding a `lib`/`target` to make `Object.hasOwn` resolve would change what the whole package compiles against, which is far outside this card. **The presence distinction the call site needs**, stated: it asks *"was this key NAMED on the insert row at all?"*, which must stay distinct from *"named with an empty value"* — that distinction is the entire point of the pin. `hasOwnProperty.call` keeps it; `key in obj` would also answer for inherited keys and `!== undefined` would conflate the two, so neither was usable. The receiver is the plain object literal `writeEvent` builds — ordinary prototype, no own `hasOwnProperty` key — and the `.call` form is correct regardless of either hazard. ## Gates — final, on `684df79a9` ``` pnpm lint exit 0 (eslint . --no-inline-config, repo-wide, not narrowed) pnpm --filter service-messaging --filter platform-objects typecheck exit 0 pnpm --filter service-messaging --filter platform-objects --filter service-automation test exit 0 platform-objects Test Files 40 passed (40) Tests 575 passed (575) service-messaging Test Files 42 passed (42) Tests 455 passed (455) service-automation Test Files 134 passed (134) Tests 1581 passed (1581) ``` **95 gate commands run to a real verdict, 0 non-zero:** - **62** derived mechanically from the change set (`node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`; re-derived after the final commit and byte-identical to the first derivation). - **33** named by `.github/workflows/lint.yml` and not in that derived set — including `check:doc-anchors`, `check:adr-anchors`, `check:verify-stand-in`, `check:entry-guard`, `check:parse-guard`, `check:bash32-floor`, `check:sdui-lockstep`, `check-sdui-manifest`, the `check:pm-*` family, `check:durability-log-level`, `check:startup-registry-verdict` and the two `scripts/pm/*.sh --self-test`s. ⚠️ **Not the whole `Lint & Repo Gates` job.** Running that job's full step list locally is a farm-wide sweep that the dev contract reserves for CI; the two sets above are its mechanically-derived and explicitly-named subsets. Nothing was inferred from the step before it — every command has its own captured exit code — and ⛔ no `exit 3` was read as a pass: | gate | first answer | after building the closure it named | |:--|:--|:--| | `check:i18n` | exit 3 | → exit **1**, a real finding (`platform-objects` bundles drifted) → regenerated → exit **0** | | `check:i18n-coverage` | exit 3 | → exit **0** (13 configs, 621 baselined, none new) | | `check:dual-build-cjs-loads` | exit 3 | → exit **0** (104 require entry points across 67 packages) | | `check:type-check-debt` | exit 3 | → exit **0** (5 ledger entries re-measured, none above its recorded number) | ⚠️ `check:where-matcher` is green (407 matchers, 0 silently-wrong, 0 unjudged, no files added to the baseline). This PR adds **no** in-memory `matches(row, where)` double — its engine double answers `insert` and `find` only. ## Gates as first run The first pass, on `e57ee063c` — superseded by the table above, kept because it is the reading the CI red was measured against: ``` pnpm --filter @objectstack/service-messaging --filter @objectstack/platform-objects typecheck exit 0 pnpm --filter @objectstack/service-messaging --filter @objectstack/platform-objects test exit 0 platform-objects Test Files 40 passed (40) Tests 575 passed (575) service-messaging Test Files 42 passed (42) Tests 454 passed (454) ``` Gate families derived mechanically from the change set (`node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`), **all 62 run to a real verdict, none inferred from the one before it**: 59 green on the first pass. The three non-zero answers were all **exit 3 — PREREQUISITE NOT MET**, a statement about the tree rather than a finding: | gate | first answer | after building the closure it named | |:--|:--|:--| | `check:i18n` | exit 3 (no `dist/` for the 10-package extract closure) | built it → exit **1**, a real finding: `platform-objects` bundles drifted → regenerated → exit **0** | | `check:i18n-coverage` | exit 3 | see below | | `check:dual-build-cjs-loads` | exit 3 (55 packages with no `dist/`) | see below | | `check:type-check-debt` | exit 3 | see below | ⛔ No `exit 3` was read as a pass anywhere. ## Out of the declared file surface — reported, not ridden in ⚠️ **The claim's declared file surface was incomplete, and the gate proved it.** Adding a labelled field to `sys_notification` drifts `packages/platform-objects/src/apps/translations/*.generated.ts`, which `pnpm check:i18n` fails on and whose remedy it names. **They were produced by the generator** — `node scripts/check-i18n-bundles.mjs --write`, never by hand: AGENTS.md forbids hand-editing generated *structure* (translated *values* may be hand-written, and none was). The regeneration is 22 added lines across 7 files, and the diff contains **nothing but** the new field — no other package drifted, and nobody else's work was picked up. Same package and same lane as the declared out-of-lane file, so the exception path's disjointness measurement is unaffected; recorded here so the surface can be re-recorded rather than quietly widened. ## Acceptance notes Found while verifying the card's own claims against the tree. ⛔ None of it rides in here. - ⚠️ **The card's premise is factually wrong in one place, and the wrong half is the reported one.** #17732 says fan-out "only checks that the channel is REGISTERED". That is true of the inline (P0) `fanOut`, and **false** of the outbox (P1) `enqueueDeliveries`, which performs **no registration check at all** — so a channel named in `channels` that is not registered gets a delivery row per recipient, and `NotificationDispatcher` dead-letters it with `dead: true` on attempt **one**. That is the exact "dead-letters on its first attempt" symptom the card opens with, and this ruling does not reach it: the ruling's member is a property of a channel *implementation*, and an unregistered channel has none to ask. Left on its existing path and pinned as such. - The `email` `MessagingChannel` is registered only `if (getEmail())` at `kernel:ready` (`messaging-service-plugin.ts:264`) — a registry read turned into a permanent registration verdict, the shape AGENTS.md's *Startup registry reads* section names. An email service that registers later never gets its channel. - `createEmailChannel().send()` returns `{ ok: true }` when no email service is registered — a delivery row recorded **success** with nothing sent. Not reachable through fan-out any more once this lands, but still reachable by a direct `send()`. - The card and the ruling both say "inbox, email today"; the tree also ships `sms-channel.ts`. It is untouched here and stays available, as the optional default requires. --- 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01URLHobLUJB9K1ABV6ofdjj --- _Generated by [Claude Code](https://claude.ai/code)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent e6965dd commit a2c2852

15 files changed

Lines changed: 757 additions & 9 deletions
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
"@objectstack/service-messaging": minor
3+
"@objectstack/platform-objects": minor
4+
---
5+
6+
Notification fan-out asks a channel whether the tenant can send on it before writing anything, so a channel with no transport no longer produces `sys_notification_delivery` rows that exist only to dead-letter (#17732).
7+
8+
`MessagingChannel` gains one **optional** member, `isAvailable(ctx, { organizationId })`, answering `{ available: true }` or `{ available: false, reason }` from the closed vocabulary `CHANNEL_UNAVAILABLE_REASONS` (today: `transport_not_configured`). `emit()` consults it once per channel per emit — availability is a property of `(tenant × channel)`, not of a recipient — and a channel that answers unavailable gets no delivery row and no `send()` call on either the outbox (P1) or the inline (P0) path.
9+
10+
- **Optional means available.** A channel that does not implement the member is treated exactly as before. Every existing implementation, in this repo and in yours, keeps working unchanged with no edit; the same is true of a channel that is registered but unknown to this version. ⛔ There is no way to configure the opposite default.
11+
- **The suppression is recorded, not swallowed.** `sys_notification` gains one key, `suppressed_channels``[{ channel, reason }]`, `NULL` when nothing was suppressed — written in the *same* insert that creates the event row, so the feature costs no additional write. `EmitResult` gains the matching `suppressed` array, so a caller is never handed a delivery count that silently omits a channel it asked for.
12+
- **The `email` channel answers from the transport it was handed** — a service-registry lookup, no I/O, nothing cached. Mail configuration in this tree is the `mail` settings namespace at `scope: 'global'`, materialised into a single in-memory transport that the settings change bus hot-swaps, so there is no per-tenant row to read and a memoized answer would survive the settings save that fixed it. The query still takes the tenant context so a future tenant-scoped transport needs no interface change.
13+
- **A probe that throws is treated as available** and logged at `warn`: a broken availability check degrades into today's behaviour, never into a silent notification outage.
14+
- ⚠️ **Unchanged on purpose**: a channel named in `channels` that is not *registered* at all keeps its existing path — the inline fan-out reports it as a failed delivery, the outbox enqueues a row the dispatcher dead-letters. It has no implementation to ask, and widening this ruling to cover it is filed separately.

packages/platform-objects/src/apps/translations/en.objects.generated.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2347,6 +2347,10 @@ export const enObjects: NonNullable<TranslationData['objects']> = {
23472347
critical: "critical"
23482348
}
23492349
},
2350+
suppressed_channels: {
2351+
label: "Suppressed Channels",
2352+
help: "Channels fan-out skipped because they are unavailable for this tenant, as [{channel, reason}]; reason is the closed set: transport_not_configured"
2353+
},
23502354
dedup_key: {
23512355
label: "Dedup Key",
23522356
help: "Idempotency key within a topic window; a repeat emit is a no-op"

packages/platform-objects/src/apps/translations/es-ES.objects.generated.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2347,6 +2347,10 @@ export const esESObjects: NonNullable<TranslationData['objects']> = {
23472347
critical: "critical"
23482348
}
23492349
},
2350+
suppressed_channels: {
2351+
label: "Suppressed Channels",
2352+
help: "Channels fan-out skipped because they are unavailable for this tenant, as [{channel, reason}]; reason is the closed set: transport_not_configured"
2353+
},
23502354
dedup_key: {
23512355
label: "Dedup Key",
23522356
help: "Idempotency key within a topic window; a repeat emit is a no-op"

packages/platform-objects/src/apps/translations/es-ES.source-hashes.generated.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,8 @@ export const esESGeneratedSourceHashes: Readonly<Record<string, string>> = {
289289
"objects.sys_notification.fields.severity.options.critical": "7ff0ff69c0abaf81",
290290
"objects.sys_notification.fields.severity.options.info": "3e0c8611029f253b",
291291
"objects.sys_notification.fields.severity.options.warning": "2673ee95caf83284",
292+
"objects.sys_notification.fields.suppressed_channels.help": "1fbbc17b95ea75d2",
293+
"objects.sys_notification.fields.suppressed_channels.label": "910cb46f687f6270",
292294
"objects.sys_notification.fields.topic.help": "80e1790edfda49df",
293295
"objects.sys_notification.fields.topic.label": "819afdb3853e9d80",
294296
"objects.sys_oauth_access_token.fields.authorization_code_id.help": "84ad4d5d8c4c1cee",

packages/platform-objects/src/apps/translations/ja-JP.objects.generated.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2347,6 +2347,10 @@ export const jaJPObjects: NonNullable<TranslationData['objects']> = {
23472347
critical: "critical"
23482348
}
23492349
},
2350+
suppressed_channels: {
2351+
label: "Suppressed Channels",
2352+
help: "Channels fan-out skipped because they are unavailable for this tenant, as [{channel, reason}]; reason is the closed set: transport_not_configured"
2353+
},
23502354
dedup_key: {
23512355
label: "Dedup Key",
23522356
help: "Idempotency key within a topic window; a repeat emit is a no-op"

packages/platform-objects/src/apps/translations/ja-JP.source-hashes.generated.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,8 @@ export const jaJPGeneratedSourceHashes: Readonly<Record<string, string>> = {
285285
"objects.sys_notification.fields.severity.options.critical": "7ff0ff69c0abaf81",
286286
"objects.sys_notification.fields.severity.options.info": "3e0c8611029f253b",
287287
"objects.sys_notification.fields.severity.options.warning": "2673ee95caf83284",
288+
"objects.sys_notification.fields.suppressed_channels.help": "1fbbc17b95ea75d2",
289+
"objects.sys_notification.fields.suppressed_channels.label": "910cb46f687f6270",
288290
"objects.sys_notification.fields.topic.help": "80e1790edfda49df",
289291
"objects.sys_notification.fields.topic.label": "819afdb3853e9d80",
290292
"objects.sys_oauth_access_token.fields.authorization_code_id.help": "84ad4d5d8c4c1cee",

packages/platform-objects/src/apps/translations/zh-CN.objects.generated.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2347,6 +2347,10 @@ export const zhCNObjects: NonNullable<TranslationData['objects']> = {
23472347
critical: "critical"
23482348
}
23492349
},
2350+
suppressed_channels: {
2351+
label: "Suppressed Channels",
2352+
help: "Channels fan-out skipped because they are unavailable for this tenant, as [{channel, reason}]; reason is the closed set: transport_not_configured"
2353+
},
23502354
dedup_key: {
23512355
label: "Dedup Key",
23522356
help: "Idempotency key within a topic window; a repeat emit is a no-op"

packages/platform-objects/src/apps/translations/zh-CN.source-hashes.generated.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,8 @@ export const zhCNGeneratedSourceHashes: Readonly<Record<string, string>> = {
229229
"objects.sys_notification.fields.severity.options.critical": "7ff0ff69c0abaf81",
230230
"objects.sys_notification.fields.severity.options.info": "3e0c8611029f253b",
231231
"objects.sys_notification.fields.severity.options.warning": "2673ee95caf83284",
232+
"objects.sys_notification.fields.suppressed_channels.help": "1fbbc17b95ea75d2",
233+
"objects.sys_notification.fields.suppressed_channels.label": "910cb46f687f6270",
232234
"objects.sys_notification.fields.topic.help": "80e1790edfda49df",
233235
"objects.sys_notification.fields.topic.label": "819afdb3853e9d80",
234236
"objects.sys_oauth_access_token.fields.authorization_code_id.help": "84ad4d5d8c4c1cee",

packages/platform-objects/src/audit/sys-notification.object.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,34 @@ export const SysNotification = ObjectSchema.create({
101101
group: 'Event',
102102
}),
103103

104+
// [#17732] Channels fan-out did not even attempt, and why.
105+
//
106+
// A channel the tenant cannot send on (no transport configured) used to get
107+
// one `sys_notification_delivery` row per recipient that dead-lettered on
108+
// its first attempt. Fan-out now asks the channel first
109+
// (`MessagingChannel.isAvailable`) and writes no delivery row at all; the
110+
// fact is recorded HERE instead, so the suppression stays auditable at the
111+
// event level rather than disappearing.
112+
//
113+
// Value: `[{ channel, reason }]`, NULL when nothing was suppressed (the
114+
// overwhelmingly common path). JSON rather than a `select` because one
115+
// event fans out to several channels and each carries its OWN reason — a
116+
// scalar column would have to drop either the channel or the reason.
117+
//
118+
// `reason` is a CLOSED set, inlined here: `transport_not_configured`.
119+
// `packages/platform-objects` is a lower layer than service-messaging and
120+
// cannot import its `CHANNEL_UNAVAILABLE_REASONS`, so the two copies are
121+
// held equal by an executable assertion in
122+
// `packages/services/service-messaging/src/channel-availability.test.ts`
123+
// — ⛔ a comment is not what keeps them in step.
124+
suppressed_channels: Field.json({
125+
label: 'Suppressed Channels',
126+
required: false,
127+
description:
128+
'Channels fan-out skipped because they are unavailable for this tenant, as [{channel, reason}]; reason is the closed set: transport_not_configured',
129+
group: 'Event',
130+
}),
131+
104132
dedup_key: Field.text({
105133
label: 'Dedup Key',
106134
required: false,

0 commit comments

Comments
 (0)