Repository navigation
Commit d68aab6
fix(service-messaging)!: sys_notification_template refuses a channel other than email or sms, stored rows included (#25028)
Fixes #24983
Clause-②: no (narrowing)
## What changes
The maintainer's ruling D on #24770 (record `6293376907`) reads: "②
refuses stored or new non-email/sms rows." The amended standing rule 「甲」
(#6015, `6303865025`) ships a narrowing that no conversion carries now,
in 18.x, as a BREAKING minor. The 18.x warning half is already in `main`
(#24924, PR #24976, `0603bfa1cf`). This PR turns that warning into the
refusal.
-
**`packages/services/service-messaging/src/objects/notification-template.object.ts`.**
`channel_renders_template` was a `format` rule at `severity: 'warning'`.
It is now a `cross_field` rule at `severity: 'error'`, with `fields:
['channel']` and the condition `!(record.channel in ["email","sms"])`.
The condition is built from `TEMPLATE_CHANNELS`, the only copy of the
list. The constant is now exported from the module for this package's
tests; it is not on the package index. The rule's message is now the
refusal with the ruling's prescription. The `channel` help now says that
any other value is refused. The field stays `text`, and its column
(`varchar(64)`) is unchanged.
- **Translations.** `en.objects.generated.ts` was regenerated with `node
scripts/check-i18n-bundles.mjs --write --filter=service-messaging`. The
zh-CN, ja-JP and es-ES values of the two changed keys (the rule message
and the `channel` help) were re-translated by hand, as AGENTS.md allows
for translated-locale values. The tool's check then reads
`services/service-messaging in sync (7 bundle(s))`. No source-hash entry
moved.
- **Tests.** `notification-template-channel.integration.test.ts` is
deleted and replaced by
`notification-template-channel-refusal.integration.test.ts`. The old
file had 15 cases pinning the advisory this PR removes, and each warning
case asserted that the write was accepted. The new file has 18 cases
(below).
- **`package.json`, `pnpm-lock.yaml`, `vitest.config.ts`.**
`@objectstack/metadata-protocol` becomes a devDependency (a 3-line
importer entry in the lockfile), so the pins drive the real data
protocol. `vitest.config.ts` aliases it to source, because
`check:test-source-alias` requires an alias and its registry is
shrink-only.
- **`.changeset/24983-template-channel-refusal.md`.**
`@objectstack/service-messaging` `minor`, in the #24813 BREAKING shape:
a headline with `!`, the `Clause-②: no (narrowing)` line, and the
ADR-0087 disposition marker `not-required (no-migration-prescription)`
with its category reasoning. It also carries the BREAKING banner and the
FROM / TO / Unchanged / fix bullets.
A contract-tier review of the narrowing is owed before the queue; the
seat runs it.
## Contract: the accept set, sentence by sentence
**Who is refused.** Every caller that writes `sys_notification_template`
is refused: Setup admins, API clients, imports, bulk edits and
in-process engine writes. A system-context write is refused too, because
an error-severity rule is never skipped for system writes
(`rule-validator.ts`, the "ERROR-severity rules are deliberately NOT
gated" note).
**What is refused.** A write is refused when the row it leaves behind
names a `channel` other than exactly `email` or `sms`. The check is
exact and case-sensitive, so `Email` and ` sms` are refused. A null
channel is out of the set too: a stored row whose channel is null is
refused on edit, and a write that nulls `channel` is still refused first
by `required`.
**New rows and moves, at each door** (Before = `main` at `73432299c0`):
| Write | Before | After |
|---|---|---|
| `POST /data/sys_notification_template` naming `inbox` (or `push`,
`webhook`, `Email`, ...) | `201`, stored, one advisory in `warnings` |
`400`, `code: VALIDATION_FAILED`, one finding `{ field: 'channel', code:
'rule_violation', message }` carrying the prescription; nothing stored |
| `PATCH /data/sys_notification_template/ID` moving an `email` / `sms`
row to another channel | `200` plus an advisory | `400` as above; the
row keeps its channel |
| Import commit with an `inbox` row (`runImport`, both dry run and
commit) | the row reports `ok: true`; the advisory appears only in the
server log | the row reports `ok: false, action: 'failed', field:
'channel', code: 'rule_violation', error: MESSAGE`; the other rows land
|
| Bulk update (`updateManyData`), a row that would leave its channel out
of the set | the row succeeds | the row reports `success: false, errors:
[{ code: 'VALIDATION_FAILED', httpStatus: 400, message }]`; with
`continueOnError`, the other rows land |
| Engine predicate update (`multi: true`) over a set holding such a row
| every matched row is written | the whole update is refused and no row
is written |
| An insert carrying an import's kept option values for `channel` |
stored, plus an advisory | refused |
| A write that leaves `email` or `sms` | accepted | accepted (unchanged)
|
**Rows already stored with another channel.** Nothing in this PR reads
them to rewrite them, and no data moves.
| Shape | Before | After |
|---|---|---|
| Read, list or `GET` | served | served (unchanged) |
| Delete | deleted | deleted (unchanged; a delete runs no validation
rule) |
| An edit of another column (`body`, `is_active`). Setup's edit form
sends only the changed fields, so this is its usual save. | accepted
silently | refused `400` with the prescription; the row is left as
stored |
| An edit that re-sends its channel | accepted, plus an advisory |
refused |
| An edit that moves it to `email` or `sms` | accepted | accepted, and
the row edits normally from then on |
| Delivery | rendered on no channel | rendered on no channel (unchanged;
no reader changes) |
This is the spec's documented "frozen, not bricked" invariant
(`packages/spec/src/data/validation.zod.ts`, "Transition gate vs
invariant"). Such a row can be read, deleted or repaired, and nothing
else.
## Why this spelling, from measurement
The table below was measured on a real `ObjectQL` over `SqlDriver`
(better-sqlite3) at `62bc129308`, through scratch tests that are not
committed. Every variant used the shipped object with only the rule or
field swapped.
| Shape | `format` at `error` | `cross_field` at `error` (chosen) |
`select` with options `email` / `sms` |
|---|---|---|---|
| New `inbox` insert | refused, `invalid_format`, the authored message |
refused, `rule_violation`, the authored message | refused,
`invalid_option`, the generic "Channel must be one of: email, sms" |
| Insert with an import's kept option values | refused | refused |
**admitted** |
| Import row (`insertMany`) | refused row | refused row | refused row,
generic sentence |
| Stored `inbox` row: edit `body` or `is_active` | **accepted** |
refused | **accepted** |
| Stored row: re-send `channel: 'inbox'` | refused | refused | refused,
generic sentence |
| Predicate update over the stored row and an email row | **both
written** | refused whole, nothing written | **both written** |
| `channel` column | `varchar(64)` | `varchar(64)` | **`varchar(255)`**
|
| Read and delete of a stored row | work | work | work |
How the card's four readings fare at this base:
- **The `select` generic sentence:** re-measured; it holds.
- **An import's `keptOptionValues`:** re-measured; it holds.
- **The column width:** re-measured; it holds.
- **objectui `SelectField` clearing an out-of-set value:** not
re-measured. It does not apply, because the field stays `text`.
One more source reading decides against the `format` spelling. At the
current console pin `cf153eb2e5`, `dirtyEditPayload`
(`packages/plugin-form/src/sanitize.ts`) sends only the changed fields
on an edit save. So under `format`, an admin's Setup edit of a stored
`inbox` row would be accepted silently, and the ruling's "refuses stored
rows" would hold at no door the console uses. That is a source reading,
not a browser run. ADR-0113 D1's non-regression invariant is scoped to
the field-level `required` / `requiredWhen`, not to object validation
rules.
## Decision analysis (four axes)
The three options are: A, `format` at `error` (it judges the write); B,
a `cross_field` invariant (chosen); and C, a `select`.
- **实际业务需求.** I measured who writes and who reads this object. Writers
are Setup admins (the Notification Templates grid, whose edit form sends
only changed fields), API clients and imports. No shipped seed writes a
row: the auth SMS seed is retired. No example writes one, and both
dogfood upgrade fixtures (`fixture-18.0`, `fixture-18.0.0`) hold 0 rows.
Readers are the email channel's inline path, the SMS channel and
plugin-auth's phone texts, all keyed on `email` / `sms`. So a row for
another channel has no reader, and nothing needs it to stay editable. B
gives the ruling's "refuses stored rows" with no data move. A leaves
stored rows editable and silent at Setup's usual save. C is no better
than A for stored rows, and it drops the prescription.
- **项目长远合理性.** B uses an existing declared mechanism whose stored-row
semantics the spec documents, with one rule and one list. A would leave
the silent class this card exists to close. C moves the column type on
deployed tables.
- **防 AI 写错.** B answers with a loud, authored prescription at every
door, stored rows included. C's catalog sentence cannot carry the
prescription, and kept option values bypass it.
- **创业阶段不扩散.** None of the options adds a gate or a surface; the new
devDependency is test-only.
- **What B costs.** A stored row for another channel can no longer be
deactivated with `is_active: false`; it must be deleted or moved to
`email` / `sms`. A predicate update over a set that holds such a row is
refused whole until the row is fixed. Both refusals carry the
prescription.
## Tests (head `677e07e66e`)
New file: `notification-template-channel-refusal.integration.test.ts`,
18 cases on a real `ObjectQL` over `SqlDriver`. It drives the real
`ObjectStackProtocolImplementation` and `mapDataError` (the REST data
door's classifier, which `POST` / `PATCH /data/:object` answer a thrown
write with), and runs `runImport` for the import commit. Every refusal
asserts the `code` (`VALIDATION_FAILED`), the status (`400`, or the
row's `httpStatus: 400`), the finding (`field: 'channel'`, `code:
'rule_violation'`), the message's first sentence and `email_template`,
and that the row is unchanged.
- **New rows:** five creates are refused (`inbox`, `push`, `webhook`,
`Email`, ` sms`), and the `email` and `sms` creates are accepted. Also
covered: a kept-option-values insert is refused, a by-id move from email
to `inbox` is refused, a by-id move between email and sms is accepted,
and an import commit lands the sms row and fails the inbox row.
- **Stored `inbox` row (written through the driver):**
- an edit of `body` or `is_active` is refused;
- a re-send of the channel is refused;
- in a bulk update, that row fails while the email row lands;
- a predicate update over a set holding it is refused and writes
nothing;
- a move to `email` is accepted, and so is a later edit;
- the row is still read and deleted;
- it still renders on no channel (`NotificationTemplateStore.load`
returns `null` for both channels).
Package runs (under the shared lock):
- `pnpm --filter @objectstack/service-messaging run test`: `Test Files
55 passed (55)` · `Tests 621 passed (621)`, at `677e07e66e` (after
merging `main`). Before the merge: 54 files and 611 tests at
`5be5094652`.
- `pnpm --filter @objectstack/service-messaging run typecheck`: exit 0.
`tsc --listFiles` includes the new test file.
## Ablation (three legs, one per pinned behaviour)
Each leg mutated the committed condition line through
`scripts/ablation-replace.mjs`. Each mutation was proved on disk: the
anchor count went from 1 to 0, the replacement count from 0 to 1, and
the blob changed. The leg then ran the new file under the lock.
Afterwards the file was restored with `git checkout HEAD -- PATH`, and
the restore was proved by blob equality with `HEAD` (`9675ee5837b5`) and
an empty `git diff HEAD`. The object is imported by a relative path from
`src`, so no `dist/` is involved.
| Leg | Mutation | Red | Green |
|---|---|---|---|
| A, the rule never refuses | condition becomes `false` | 12: every
refusal (five creates, the kept-option-values insert, the by-id move,
the import row, the stored-row edit, the re-send, the bulk row, the
predicate update) | 6: the positive twins, the email/sms move, the
repair, read and delete, renders nowhere |
| B, the rule refuses everything | condition becomes `true` | 8: every
acceptance (the email/sms creates, the email/sms move, the repair, the
import's sms row, the bulk's email row, and the two cases that set up an
email row) | 10 |
| C, a transition gate instead of the invariant | condition becomes "out
of set AND (insert OR the prior row was in set)" | 4: exactly the
stored-row refusals (the edit of another column, the re-send, the bulk
row, the predicate update) | 14: new-row refusals stay refused |
## Gates
Locally, at `677e07e66e`, the derived set ran under the shared lock as
one runner. CI runs everything else: the lint workflow's whole farm and
the PR check-runs are its readings, not re-run here.
- **Derivation.** `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derived 81 commands from 11 changed paths
against merge base `73432299c`. Each exit code was captured before any
pipe.
- **First pass.** 80 commands exited 0. `pnpm
check:dual-build-cjs-loads` exited 3, `PREREQUISITE NOT MET`, because 8
workspace packages outside this diff's closure had no `dist/`. I built
those 8 (`pnpm turbo run build` with eight filters, all 59 tasks cache
hits) and re-ran the gate: exit 0, "112 published require entry point(s)
across 69 package(s) load".
- **Reconciliation.** `node scripts/pm/dispatch-gates.mjs --ran
ran.list` reads "81 derived famil(ies) accounted for — 81 run, 0
NOT-MEASURED (a DERIVED zero — all 81 recorded an exit code and none of
them is 3)".
- **Named readings:**
- `check-adr-0087-registration --base origin/main`: "1 declared-breaking
changeset(s), each carrying an ADR-0087 disposition", read as
`[BREAKING+bang+clause-②-narrowing] not-required
(no-migration-prescription)`;
- `pnpm check:i18n` exit 0;
- `pnpm check:test-source-alias` exit 0. It was red once before the
alias was added, naming `@objectstack/metadata-protocol`;
- `pnpm check:workspace-manifest-cycles` exit 0;
- `pnpm check:nul-bytes` exit 0.
- **Not run locally.** `test:repo` (the vitest `repo` project) is CI's:
this diff edits none of its tests and the derivation does not name it.
`pnpm lint` is CI's too.
## Dogfood battery
The full battery ran once, on the final head `677e07e66e`, as CI runs
it: `bash scripts/pm/os-verify-lock.sh -c 'pnpm --filter
@objectstack/dogfood test'`, with no `OS_TEST_SHARD` and no path. Its
closure was built first with `pnpm turbo run build
--filter=@objectstack/dogfood --concurrency=2` (66 tasks).
- `os-verify-lock: VERDICT command-exit 0 · held the lock 741s
(12m21s)`, a reading on a shared box.
- `Test Files 284 passed | 3 skipped (287)` · `Tests 2361 passed | 18
skipped (2379)`. No failure.
- The upgrade-harness fixtures (`fixture-18.0`, `fixture-18.0.0`) hold 0
`sys_notification_template` rows and keep `channel varchar(64)`, so no
fixture is stale under this change.
## Acceptance notes
- **A pending changeset this change makes false if both ship together.**
`.changeset/24924-template-channel-email-sms.md` (`patch`, still pending
in `.changeset/`) says that a write naming another channel "is still
accepted and stored". If it ships in the same release as this PR, the
compiled CHANGELOG would carry that sentence beside this refusal.
Release notes are compiled centrally at release time, so this PR does
not edit another card's changeset. It is reported to the seat in the dev
report.
- **ADR-0012's template line.** ADR-0012 (`sys_notification_template`
row, "MJML for email, JSON for others") describes templates for other
channels that never got a reader. Ruling D narrows the object, and the
ruling asked for no ADR edit on this card (it asked for one only on card
①). Noted, not filed. Carrier: none.
- **The console's rendering of the refusal.** How the console displays
the `400` finding on `channel` when an admin edits `body` was not
measured in a browser. The finding names `channel`, so the field the
admin must fix is the one marked.
---
**Seat's append** (`domain:services` seat 1, from the dev's patch-round
report `6306289251` on #24983; the dev writes its PR body once):
## Corrected pending note
This PR also corrects `.changeset/24924-template-channel-email-sms.md`,
the pending note from PR #24976 (#24924). Once this PR lands, both
entries ship in one release, and parts of that note would no longer be
true. Only the false sentences are rewritten (commit `6974906eba`).
These are kept byte for byte:
- the frontmatter (`'@objectstack/service-messaging': patch`);
- the background paragraph;
- the copy sentence ("The object's description, its empty state and the
`channel` / `subject` help now say which channels use a row and that the
inbox never does.");
- the second sentence of **Unchanged**;
- the whole **What to do** bullet.
`Check Changeset` (`check-empty-changeset`) is red on purpose, as
DELIBERATE CORRECTION; the file has not been restored. The seat named
three sentences: the second sentence of the warning paragraph, the
`Clause-②` line and the first sentence of **Unchanged**. The headline
and the paragraph's other four sentences were false the same way, so
they are rewritten too and quoted here.
**1. The headline.**
- BEFORE: "fix(service-messaging): a `sys_notification_template` saved
for a channel other than `email` or `sms` is now warned about, because
no channel ever sends it"
- AFTER: "fix(service-messaging): the `sys_notification_template` copy
now says that only the email and SMS channels use a template row, and
that the in-app inbox never does"
**2. The warning paragraph.** Five sentences become one; the paragraph's
closing copy sentence is kept.
- BEFORE: "The object now carries a `channel_renders_template` rule at
`severity: 'warning'`. A write naming another channel is still accepted
and stored, and it now reports the warning. A single-record create,
clone or update answers it in the write answer's `warnings` (`rule`,
`severity`, `field: 'channel'`, `code`, `message`), and the server logs
it once per write. Bulk updates and imports log it. The message points
in-app content to an email template (`email_template`) named in a notify
step's `template`."
- AFTER: "This entry first added a `channel_renders_template` warning
for such a write; in the same release #24983's refusal superseded it, so
a write naming another channel is refused rather than warned about, as
that entry describes."
**3. The `Clause-②` line.**
- BEFORE: "Clause-②: no — nothing a write could send before is refused
now; a save naming another channel gains one advisory in the write
answer's existing `warnings`."
- AFTER: "Clause-②: no — this entry now describes copy only (the
object's description, its empty state and the field help); the narrowing
of what a write may send is #24983's, declared in its own entry."
**4. The first sentence of Unchanged.**
- BEFORE: "**Unchanged.** Every write that was accepted is still
accepted, and stored rows are not read, rewritten or flagged by anything
but a write that names their channel."
- AFTER: "**Unchanged.** Nothing this entry describes changes which
writes are accepted."
Code at the head is byte-identical to `677e07e66e`, where the tests, the
gates and the dogfood battery ran: `git diff --stat 677e07e
6974906` prints `.changeset/24924-template-channel-email-sms.md | 8
++++----` and `1 file changed, 4 insertions(+), 4 deletions(-)`. At the
new head, `node scripts/check-empty-changeset.mjs --base origin/main`
exits 1 with the DELIBERATE CORRECTION class line, and
`check-changeset-no-major`, `check-adr-0087-registration`,
`check-changeset-fixed` and `check:nul-bytes` exit 0.
---
_Generated by [Claude
Code](https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 7f99616 commit d68aab6
12 files changed
Lines changed: 357 additions & 251 deletions
File tree
- .changeset
- packages/services/service-messaging
- src
- objects
- translations
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
5 | | - | |
| 5 | + | |
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
9 | | - | |
| 9 | + | |
10 | 10 | | |
11 | | - | |
| 11 | + | |
12 | 12 | | |
13 | | - | |
| 13 | + | |
14 | 14 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
32 | 32 | | |
33 | 33 | | |
34 | 34 | | |
| 35 | + | |
35 | 36 | | |
36 | 37 | | |
37 | 38 | | |
| |||
0 commit comments