diff --git a/.changeset/24895-preference-user-reference.md b/.changeset/24895-preference-user-reference.md new file mode 100644 index 00000000000..de9ff1be3dd --- /dev/null +++ b/.changeset/24895-preference-user-reference.md @@ -0,0 +1,23 @@ +--- +'@objectstack/service-messaging': patch +'@objectstack/metadata-protocol': patch +--- + +fix(service-messaging,metadata-protocol)!: a notification preference's User is a `sys_user` reference, so the record title names the person and the write door refuses an id no user holds; the default for every user is an empty User instead of `'*'` + +Clause-②: yes (narrowing) +Metadata-protocol: none + + + +**BREAKING** (a platform object's write accept set narrows, and one stored value is rewritten). It changes what a data writer can store in `sys_notification_preference`, not what a metadata author can write, so it ships as `patch` with this banner. + +- **FROM:** `sys_notification_preference.user_id` was a required text column holding a user id or the literal `'*'`, which meant "the default for every user". Any string was stored: a mistyped id matched nobody, silently. The record title (Setup → Notification Preferences) read the raw id, for example `ISu3jHPIrdgD45L1rrC2FuE7DIcsk2ap · urgent_task · inbox`. Two defaults for every user could be stored for one topic and channel in one organization, and which one applied was unspecified. +- **TO:** `user_id` is an optional `sys_user` lookup (ADR-0118 D1: no sentinel in a user column). An empty User is the default for every user, and a user's own row still overrides it. + - A write that names an id no user holds, `'*'` included, is refused: the REST data door answers `400` with `code: 'VALIDATION_FAILED'` and a finding on `user_id` with `code: 'reference_not_found'`. + - The title names the person: their name, or their email where the name is blank, then the topic and the channel. A row with an empty User reads `All users · TOPIC · CHANNEL`. A row whose User this reader cannot read, or that no longer exists, has no title rather than a misleading one. + - Deleting a user deletes that user's preference rows (`deleteBehavior: 'cascade'`). Under the lookup default (`set_null`) the person's own mute would have become the default for every user. + - At every boot (`kernel:ready`), the messaging service rewrites each stored `user_id = '*'` to an empty User, in every organization. A second run changes nothing. A `'*'` row whose topic and channel already have a default with an empty User is left as stored and reported once at `error`, naming the row; it is never deleted for you. + - On SQLite and PostgreSQL, `@objectstack/metadata-protocol` rebuilds the index `uniq_sys_notification_preference_a22d7d27` at `kernel:ready` as `(organization_id, COALESCE(user_id, ''), topic, channel)`, so a second default for one topic and channel in one organization is refused (`409`). If existing rows already break that rule, it refuses to migrate: the previous index stays, no row is touched, and the boot log and `os migrate duplicates` list the rows. MySQL/MariaDB and driver-memory keep the declared index, which does not constrain empty Users, and the boot log says so on MySQL. +- **Unchanged:** `topic = '*'` and `channel = '*'` are still the topic and channel wildcards. Most-specific-wins resolution, mandatory topics, quiet hours and digests behave as before. A row for a real user is read, matched and titled as before, except for the title text. The physical column is still a `varchar(255)`, so no DDL change is needed for it. +- **The fix for an operator:** nothing, normally; the boot rewrites the old `'*'` rows. If the boot log reports a `'*'` row that shares its topic and channel with a default whose User is already empty, delete the one that should not apply and restart. If it reports that the one-default-per-key index could not be built because two such defaults exist, list them with `os migrate duplicates`, delete one, and restart. A script or import that wrote `user_id: '*'` writes an empty `user_id` (omit it, or send `null`) for the default for every user. diff --git a/content/docs/permissions/tenant-audit-census.mdx b/content/docs/permissions/tenant-audit-census.mdx index 96e2a95752a..305b9cc5718 100644 --- a/content/docs/permissions/tenant-audit-census.mdx +++ b/content/docs/permissions/tenant-audit-census.mdx @@ -122,7 +122,7 @@ are reported as `undecidable` rather than assumed either way. The same holds twice over for the context. An options argument spelled as a literal can be read; one spelled `options`, `{ ...opts }`, or handed through a -forwarding shim cannot, and **51 of the 236 sites are spelled that way**. A +forwarding shim cannot, and **51 of the 237 sites are spelled that way**. A context resolved from an inline literal or a local `const` can be tested for `isSystem`; one arriving from a helper call cannot. @@ -187,10 +187,10 @@ reproduce them. Where it disagrees, it disagrees on the page: | carried figure | where it survives | this census | | :--- | :--- | ---: | -| 175 write call sites | quoted in the merged changeset | **236** | +| 175 write call sites | quoted in the merged changeset | **237** | | 24 carrying no tenant context | quoted in the merged changeset | **2** provable and tenancy-enabled; **29** more whose options argument is unreadable | -| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **159 of 236** decidable, **77** undecidable | -| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 134 decidably elevated, 0 decidably not, 94 undecidable | +| 127 of 175 statically decidable, 48 runtime-parameter-name sites | restated on the `isSystem`-scoping card | **160 of 237** decidable, **77** undecidable | +| 135 (77%) silenced by the `isSystem` guard before the posture gate | the lost issue body — **no surviving corroboration** | **not reproduced**: 135 decidably elevated, 0 decidably not, 94 undecidable | | 141 and 132, two independent re-derivations | the card that filed this work | — | **The differences are not reconciled, and deliberately so.** The old census's @@ -201,17 +201,17 @@ at any commit. Two structural facts do plausibly widen this reading against any hand or regex one, and both are counted in the generated tables below: the 41 sites reached -through an erased (`any`) receiver, and the 50 that name their object through a +through an erased (`any`) receiver, and the 51 that name their object through a `const` rather than inline. An instrument that read either the way a person does would report a smaller number and would not say so. The fourth row is the one worth flagging to anyone citing it. **The 135 / 77% figure has no surviving corroboration anywhere in the tree.** This census reads -134 of 236 (57%) as decidably elevated, with 94 more whose elevation is a +135 of 237 (57%) as decidably elevated, with 94 more whose elevation is a run-time fact — so the claim is neither confirmed nor refuted, and the honest answer is that a static reading cannot settle it. -⇒ **Cite `2 / 236`, and say what it is**: the sites whose options argument was +⇒ **Cite `2 / 237`, and say what it is**: the sites whose options argument was READ and holds no tenant context, against a decidably tenancy-enabled object. That is the control's provable yield surface. ⛔ Do not cite it as "the sites without tenant context" — **29 further sites** have an options argument this @@ -223,29 +223,29 @@ cannot read, and they are neither in nor out. | what | count | | :--- | ---: | -| write call sites on the application surface | **236** | -| …whose object name is statically decidable | 159 | +| write call sites on the application surface | **237** | +| …whose object name is statically decidable | 160 | | …whose object name is chosen at run time | 77 | -| …against an object with tenancy ENABLED | 158 | +| …against an object with tenancy ENABLED | 159 | | …against an object that declares tenancy off | 1 | -| threading a tenant context | 177 | +| threading a tenant context | 178 | | PROVABLY carrying none (options read, no context key) | **8** | | …of those, against a decidably tenancy-enabled object | **2** | | options argument UNREADABLE — may or may not carry one | 51 | | …of those, against a decidably tenancy-enabled object | 29 | -| threading a decidably ELEVATED (`isSystem`) context | 134 | +| threading a decidably ELEVATED (`isSystem`) context | 135 | | threading a context that is decidably NOT elevated | 0 | | threading a context whose elevation is a run-time fact | 94 | | how the instrument reached the site | count | | :--- | ---: | -| receiver carried a readable engine type | 195 | +| receiver carried a readable engine type | 196 | | receiver erased, placed by the object NAME | 31 | | receiver erased, placed by an `object: string` PARAMETER | 5 | | receiver erased, placed by an `UNTYPED_RECEIVERS` row | 5 | | object name spelled inline | 109 | -| object name spelled through a `const` | 50 | +| object name spelled through a `const` | 51 | | object name is an `object: string` parameter | 7 | | object name is some other run-time expression | 70 | @@ -297,13 +297,13 @@ holds still. They are required to be HERE and to say WHEN they were true; their values are not compared. The reasoning, and the measurement behind it, are in `scripts/check-tenant-audit-census.mjs`. -Measured on 2026-10-20 at `a454cbcc4`. +Measured on 2026-10-22 at `452e9963c`. | corpus scale (not enforced) | count | | :--- | ---: | -| tracked non-test sources scanned | 684 | -| engine-shaped types recognised | 82 | -| declared objects in the registry | 118 | +| tracked non-test sources scanned | 700 | +| engine-shaped types recognised | 86 | +| declared objects in the registry | 117 | | same-named calls subtracted as non-engine | 169 | {/* END GENERATED: tenant-audit-census */} diff --git a/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md b/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md index b5ee0ccd14c..b92a558cff9 100644 --- a/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md +++ b/docs/audits/2026-08-tenant-audit-write-call-sites.counts.md @@ -33,17 +33,17 @@ silent, and `node scripts/tenant-audit-census.mjs --write` is the resolution. | Measure | Value | |---|---:| -| Write call sites | 236 | -| Object name statically decidable | 159 | +| Write call sites | 237 | +| Object name statically decidable | 160 | | Object name chosen at run time | 77 | -| Against a tenancy-enabled object | 158 | +| Against a tenancy-enabled object | 159 | | Against an object declaring tenancy off | 1 | -| Threading a tenant context | 177 | +| Threading a tenant context | 178 | | Provably carrying none | 8 | | …and decidably tenancy-enabled | 2 | | Options argument unreadable | 51 | | …and decidably tenancy-enabled | 29 | -| Threading a decidably elevated context | 134 | +| Threading a decidably elevated context | 135 | | Threading a decidably non-elevated context | 0 | | Threading a context of undecidable elevation | 94 | @@ -90,13 +90,13 @@ holds still. They are required to be HERE and to say WHEN they were true; their values are not compared. The reasoning, and the measurement behind it, are in `scripts/check-tenant-audit-census.mjs`. -Measured on 2026-10-20 at `a454cbcc4`. +Measured on 2026-10-22 at `452e9963c`. | corpus scale (not enforced) | count | | :--- | ---: | -| tracked non-test sources scanned | 684 | -| engine-shaped types recognised | 82 | -| declared objects in the registry | 118 | +| tracked non-test sources scanned | 700 | +| engine-shaped types recognised | 86 | +| declared objects in the registry | 117 | | same-named calls subtracted as non-engine | 169 | ## Every site @@ -221,6 +221,7 @@ Measured on 2026-10-20 at `a454cbcc4`. | `packages/services/service-messaging/src/messaging-service.ts` | `insert` | `RECEIPT_OBJECT` | undecidable | context, elevation undecidable | 1 | | `packages/services/service-messaging/src/messaging-service.ts` | `update` | `RECEIPT_OBJECT` | undecidable | options unreadable | 1 | | `packages/services/service-messaging/src/messaging-service.ts` | `insert` | `sys_notification` | enabled | context, elevation undecidable | 1 | +| `packages/services/service-messaging/src/preference-user-conversion.ts` | `update` | `sys_notification_preference` | enabled | elevated | 1 | | `packages/services/service-messaging/src/sql-http-outbox.ts` | `insert` | `this.objectName` | undecidable | context, elevation undecidable | 1 | | `packages/services/service-messaging/src/sql-http-outbox.ts` | `update` | `this.objectName` | undecidable | context, elevation undecidable | 4 | | `packages/services/service-messaging/src/sql-http-outbox.ts` | `update` | `this.objectName` | undecidable | options unreadable | 1 | diff --git a/packages/cli/src/commands/migrate/duplicates.contract.test.ts b/packages/cli/src/commands/migrate/duplicates.contract.test.ts index 7c427584524..23ee0fa5434 100644 --- a/packages/cli/src/commands/migrate/duplicates.contract.test.ts +++ b/packages/cli/src/commands/migrate/duplicates.contract.test.ts @@ -231,7 +231,7 @@ describe('#8928 os migrate duplicates — the report document', () => { organizationCounters: [{ organization: 'org_x', lastValue: 4 }], }, ], - // One entry per index the two `kernel:ready` migrations tighten — THREE, + // One entry per index the three `kernel:ready` migrations tighten — FOUR, // because the overlay migration builds one per state and either can be // blocked on its own. Present whatever the outcome: an index left out // would make "nothing blocks it" and "it was never probed" the same @@ -253,7 +253,7 @@ describe('#8928 os migrate duplicates — the report document', () => { }); }); - it('names the three kernel:ready indexes, and the migration behind each — none for the retired sys_view_definition (ADR-0131 D13)', async () => { + it('names the four kernel:ready indexes, and the migration behind each — none for the retired sys_view_definition (ADR-0131 D13)', async () => { const report = await collect(); expect( report.runtimeIndexPreflight.map((p) => `${p.migration}:${p.table}:${p.index}`), @@ -261,6 +261,9 @@ describe('#8928 os migrate duplicates — the report document', () => { 'ensureMetadataOverlayIndexes:sys_metadata:idx_sys_metadata_overlay_active', 'ensureMetadataOverlayIndexes:sys_metadata:idx_sys_metadata_overlay_draft', 'ensureSysSettingIdentityIndex:sys_setting:uniq_sys_setting_organization_id_namespace_key_scope_user_id', + // One admin-global default (an empty User) per (organization, topic, + // channel): service-messaging's preference table, ADR-0118 D1. + 'ensureNotificationPreferenceIdentityIndex:sys_notification_preference:uniq_sys_notification_preference_a22d7d27', ]); }); diff --git a/packages/cli/src/commands/migrate/duplicates.integration.test.ts b/packages/cli/src/commands/migrate/duplicates.integration.test.ts index f06ae6e7f82..0e128ba634c 100644 --- a/packages/cli/src/commands/migrate/duplicates.integration.test.ts +++ b/packages/cli/src/commands/migrate/duplicates.integration.test.ts @@ -311,9 +311,9 @@ describe('#8928 os migrate duplicates — against a really booted stack', () => // sees that something is blocked without reading every entry. expect(produced.summary.runtimeIndexesBlocked).toBe(1); expect(produced.summary.runtimeIndexBlockingRows).toBe(2); - // `sys_setting` is absent on this fixture; whatever each status, the three + // `sys_setting` is absent on this fixture; whatever each status, the four // indexes are all accounted for. - expect(produced.runtimeIndexPreflight).toHaveLength(3); + expect(produced.runtimeIndexPreflight).toHaveLength(4); expect(produced.reportVersion).toBe(2); // The whole run — boot included — wrote nothing. If a future change arms a diff --git a/packages/drivers/driver-sql/src/sql-driver-tenant-scoped-declared-unique.test.ts b/packages/drivers/driver-sql/src/sql-driver-tenant-scoped-declared-unique.test.ts index d77e131a8e0..625fc5661d5 100644 --- a/packages/drivers/driver-sql/src/sql-driver-tenant-scoped-declared-unique.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-tenant-scoped-declared-unique.test.ts @@ -252,7 +252,11 @@ const SUBJECTS: Subject[] = [ control: { user_id: 'user_only_yi', topic: 'billing.invoice', channel: 'email' }, trailingControl: { user_id: 'user_u1', topic: 'billing.invoice', channel: 'push' }, filler: {}, - platformKey: { user_id: '*', topic: 'billing.invoice', channel: 'email' }, + // A plain user id like every other subject's platform key. It spelled + // `'*'` once, the text column's admin-global sentinel; `user_id` is now a + // `sys_user` reference whose EMPTY value is that default (ADR-0118 D1), so + // the sentinel is no stored value any more. At this layer it is just a key. + platformKey: { user_id: 'user_platform', topic: 'billing.invoice', channel: 'email' }, legacyName: 'uniq_sys_notification_preference_user_id_topic_channel', // ⚠️ HASH-SUFFIXED, and this is the case the PM's A1 flagged as the one // where a single-column assumption would hide. The base name diff --git a/packages/metadata-protocol/src/migrations/notification-preference-identity-index.test.ts b/packages/metadata-protocol/src/migrations/notification-preference-identity-index.test.ts new file mode 100644 index 00000000000..ec17fd167d8 --- /dev/null +++ b/packages/metadata-protocol/src/migrations/notification-preference-identity-index.test.ts @@ -0,0 +1,275 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { DatabaseSync } from 'node:sqlite'; + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; + +import { + buildNotificationPreferenceDuplicateProbeSql, + buildNotificationPreferenceIdentityIndexSql, + buildNotificationPreferencePresenceSql, + ensureNotificationPreferenceIdentityIndex, + notificationPreferenceIdentityKeyParts, + resolveNotificationPreferenceIndexSeam, + NOTIFICATION_PREFERENCE_IDENTITY_INDEX_NAME, + NOTIFICATION_PREFERENCE_IDENTITY_PROBE_INDEX_NAME, + NOTIFICATION_PREFERENCE_TABLE, +} from './notification-preference-identity-index.js'; +import type { IndexExec } from './partial-index-probe.js'; + +/** + * `sys_notification_preference` — one admin-global default per (organization, + * topic, channel), the NULL-safe index ADR-0118 D1's empty reference needs. + * + * Real SQLite (`node:sqlite`, as the sibling migration suites use), because the + * defect is a claim about what a UNIQUE index does with NULLs and the one + * interesting failure mode — existing duplicates — exists only over real rows. + * The end-to-end half (the real engine, the real driver, the real arming in + * `assembleMetadataProtocol`) lives in `service-messaging`'s + * `notification-preference-user-reference.integration.test.ts`. + */ +describe('sys_notification_preference: one admin-global default per key', () => { + let db: DatabaseSync; + let exec: IndexExec; + + /** + * ⚠️ MEASURED, not assembled: byte for byte what `SqlDriver.syncDeclaredIndexes` + * emits on SQLite for the shipped declaration `{ fields: ['user_id','topic', + * 'channel'], unique: 'organization' }` — read back from `sqlite_master` after + * a real `ObjectQL` + `SqlDriver` sync of the object. NULL-distinct on user_id. + */ + const DECLARED_INDEX_DDL = + 'CREATE UNIQUE INDEX `uniq_sys_notification_preference_a22d7d27` on ' + + '`sys_notification_preference` (`organization_id`, `user_id`, `topic`, `channel`)'; + /** What an 18.0 release built for the same declaration: ADR-0120 D3's NULL-safe organization part. */ + const DECLARED_INDEX_DDL_18_0 = + 'CREATE UNIQUE INDEX `uniq_sys_notification_preference_a22d7d27` ON `sys_notification_preference` ' + + "(COALESCE(`organization_id`, '__global__'), `user_id`, `topic`, `channel`)"; + + const indexDdl = (name: string): string | undefined => + (db.prepare("SELECT sql FROM sqlite_master WHERE type='index' AND name=?").get(name) as { sql?: string } | undefined) + ?.sql ?? undefined; + + const rows = (): Array> => + db.prepare(`SELECT id, organization_id, user_id, topic, channel FROM ${NOTIFICATION_PREFERENCE_TABLE} ORDER BY id`).all() as Array< + Record + >; + + /** One preference row. `user_id` NULL is the admin-global default. */ + const insert = ( + id: string, + over: { organization_id?: string | null; user_id?: string | null; topic?: string; channel?: string } = {}, + ): boolean => { + const r = { organization_id: 'org_jia', user_id: null as string | null, topic: 'deal.won', channel: 'email', ...over }; + try { + db.prepare( + `INSERT INTO ${NOTIFICATION_PREFERENCE_TABLE} (id, organization_id, user_id, topic, channel, enabled) VALUES (?,?,?,?,?,0)`, + ).run(id, r.organization_id, r.user_id, r.topic, r.channel); + return true; + } catch { + return false; + } + }; + + beforeEach(() => { + db = new DatabaseSync(':memory:'); + db.exec(`CREATE TABLE sys_notification_preference ( + id TEXT PRIMARY KEY, organization_id TEXT, user_id TEXT, topic TEXT, channel TEXT, enabled INTEGER + );`); + db.exec(DECLARED_INDEX_DDL); + exec = async (sql: string) => db.exec(sql); + }); + + afterEach(() => { + db.close(); + }); + + it('BEFORE: the declared index admits two global defaults for one key in one organization', () => { + expect(insert('a')).toBe(true); + expect(insert('b')).toBe(true); + // Control: the same real user twice IS refused, so it is the NULL. + expect(insert('u1', { user_id: 'usr_1' })).toBe(true); + expect(insert('u2', { user_id: 'usr_1' })).toBe(false); + }); + + it('AFTER: a second global default for the key is refused; NULL and the empty string are one bucket', async () => { + expect(await ensureNotificationPreferenceIdentityIndex(exec)).toEqual({ status: 'created' }); + expect(insert('a')).toBe(true); + expect(insert('b')).toBe(false); + // '' is the engine's other "no link" spelling; the resolver reads it as + // the global default too, so the index must not admit it beside a NULL. + expect(insert('c', { user_id: '' })).toBe(false); + }); + + it('AFTER (anti-vacuity): the key stays per organization, per topic and per channel', async () => { + await ensureNotificationPreferenceIdentityIndex(exec); + expect(insert('a')).toBe(true); + expect(insert('other-org', { organization_id: 'org_yi' })).toBe(true); + expect(insert('other-topic', { topic: 'deal.lost' })).toBe(true); + expect(insert('other-channel', { channel: 'inbox' })).toBe(true); + expect(insert('wildcard-topic', { topic: '*' })).toBe(true); + }); + + it('AFTER: a real user still gets one row per key, and a user row never collides with the global default', async () => { + await ensureNotificationPreferenceIdentityIndex(exec); + expect(insert('g')).toBe(true); + expect(insert('u1', { user_id: 'usr_1' })).toBe(true); + expect(insert('u1-again', { user_id: 'usr_1' })).toBe(false); + expect(insert('u2', { user_id: 'usr_2' })).toBe(true); + }); + + it('claims the DECLARED name with the NULL-safe key, leaves no probe index, and keeps the rows as stored', async () => { + insert('g'); + insert('u', { user_id: 'usr_1' }); + await ensureNotificationPreferenceIdentityIndex(exec); + const ddl = indexDdl(NOTIFICATION_PREFERENCE_IDENTITY_INDEX_NAME)!; + expect(ddl).toContain("COALESCE(user_id, '')"); + expect(ddl).not.toContain('__global__'); + expect(indexDdl(NOTIFICATION_PREFERENCE_IDENTITY_PROBE_INDEX_NAME)).toBeUndefined(); + // Storage untouched: the global default keeps its NULL. + expect(rows().map((r) => r.user_id)).toEqual([null, 'usr_1']); + }); + + it('is idempotent — a second run leaves a byte-identical definition', async () => { + await ensureNotificationPreferenceIdentityIndex(exec); + const first = indexDdl(NOTIFICATION_PREFERENCE_IDENTITY_INDEX_NAME); + expect(await ensureNotificationPreferenceIdentityIndex(exec)).toEqual({ status: 'created' }); + expect(indexDdl(NOTIFICATION_PREFERENCE_IDENTITY_INDEX_NAME)).toBe(first); + }); + + it('an 18.0 index under the declared name (COALESCE organization) is rebuilt to the plain organization key', async () => { + db.exec(`DROP INDEX ${NOTIFICATION_PREFERENCE_IDENTITY_INDEX_NAME}`); + db.exec(DECLARED_INDEX_DDL_18_0); + await ensureNotificationPreferenceIdentityIndex(exec); + expect(indexDdl(NOTIFICATION_PREFERENCE_IDENTITY_INDEX_NAME)).toBe( + buildNotificationPreferenceIdentityIndexSql(NOTIFICATION_PREFERENCE_IDENTITY_INDEX_NAME).replace(' IF NOT EXISTS', ''), + ); + }); + + describe('a database that already holds two global defaults for one key', () => { + beforeEach(() => { + insert('d1'); + insert('d2'); + }); + + it('refuses, deletes nothing, and leaves the previous index in place and enforcing', async () => { + expect((await ensureNotificationPreferenceIdentityIndex(exec)).status).toBe('conflict'); + expect(rows().map((r) => r.id)).toEqual(['d1', 'd2']); + expect(indexDdl(NOTIFICATION_PREFERENCE_IDENTITY_INDEX_NAME)).toBe(DECLARED_INDEX_DDL); + expect(indexDdl(NOTIFICATION_PREFERENCE_IDENTITY_PROBE_INDEX_NAME)).toBeUndefined(); + expect(insert('u1', { user_id: 'usr_1' })).toBe(true); + expect(insert('u2', { user_id: 'usr_1' })).toBe(false); + }); + + it('reports once at ERROR, naming the index, the consequence, and the query that lists the rows', async () => { + const logger = { error: vi.fn(), warn: vi.fn(), info: vi.fn() }; + await ensureNotificationPreferenceIdentityIndex(exec, logger); + expect(logger.error).toHaveBeenCalledTimes(1); + expect(logger.warn).not.toHaveBeenCalled(); + const [message] = logger.error.mock.calls[0]!; + expect(message).toContain(NOTIFICATION_PREFERENCE_IDENTITY_INDEX_NAME); + expect(message).toContain('The previous index is left in place'); + expect(message).toContain('no row is discarded automatically'); + expect(message).toContain(buildNotificationPreferenceDuplicateProbeSql()); + expect(message).toContain('os migrate duplicates'); + }); + + it('the shipped query lists exactly the offending key, on the folded user part', () => { + insert('other-org', { organization_id: 'org_yi' }); + insert('u', { user_id: 'usr_1' }); + expect(db.prepare(buildNotificationPreferenceDuplicateProbeSql()).all()).toEqual([ + { organization_id: 'org_jia', user_id_key: '', topic: 'deal.won', channel: 'email', duplicate_rows: 2 }, + ]); + }); + + it('keeps refusing until an operator deletes one — then converges', async () => { + expect((await ensureNotificationPreferenceIdentityIndex(exec)).status).toBe('conflict'); + db.prepare(`DELETE FROM ${NOTIFICATION_PREFERENCE_TABLE} WHERE id=?`).run('d2'); + expect(await ensureNotificationPreferenceIdentityIndex(exec)).toEqual({ status: 'created' }); + }); + }); + + describe('composition and dialect', () => { + it('no such table: a silent no-op that issues only the presence probe', async () => { + const bare = new DatabaseSync(':memory:'); + const logger = { error: vi.fn(), warn: vi.fn(), info: vi.fn() }; + const statements: string[] = []; + const bareExec: IndexExec = async (sql: string) => { + statements.push(sql); + return bare.exec(sql); + }; + expect(await ensureNotificationPreferenceIdentityIndex(bareExec, logger)).toEqual({ status: 'absent' }); + expect(logger.error).not.toHaveBeenCalled(); + expect(logger.warn).not.toHaveBeenCalled(); + expect(statements).toEqual([buildNotificationPreferencePresenceSql()]); + bare.close(); + }); + + it('no raw-SQL driver: a no-op', async () => { + const logger = { error: vi.fn(), warn: vi.fn() }; + expect(await ensureNotificationPreferenceIdentityIndex(undefined, logger)).toEqual({ status: 'no-driver' }); + expect(logger.error).not.toHaveBeenCalled(); + }); + + it('a dialect that rejects functional key parts (MySQL): degraded at error, previous index intact', async () => { + const logger = { error: vi.fn(), warn: vi.fn() }; + const refusing: IndexExec = async (sql: string) => { + if (/^CREATE UNIQUE INDEX/i.test(sql)) throw new Error("You have an error in your SQL syntax near '(COALESCE'"); + return db.exec(sql); + }; + expect((await ensureNotificationPreferenceIdentityIndex(refusing, logger)).status).toBe('unsupported'); + expect(indexDdl(NOTIFICATION_PREFERENCE_IDENTITY_INDEX_NAME)).toBe(DECLARED_INDEX_DDL); + const [message] = logger.error.mock.calls[0]!; + expect(message).toContain('stays NULL-distinct on user_id'); + // No identifier in the listing is reserved on MySQL, so the one + // statement is the remedy there too. + expect(message).toContain(buildNotificationPreferenceDuplicateProbeSql()); + }); + + it('an unclassifiable failure is still reported at error, with the previous index kept', async () => { + const logger = { error: vi.fn(), warn: vi.fn() }; + const broken: IndexExec = async (sql: string) => { + if (/^CREATE UNIQUE INDEX/i.test(sql)) throw new Error('disk I/O error'); + return db.exec(sql); + }; + expect((await ensureNotificationPreferenceIdentityIndex(broken, logger)).status).toBe('failed'); + expect(indexDdl(NOTIFICATION_PREFERENCE_IDENTITY_INDEX_NAME)).toBe(DECLARED_INDEX_DDL); + expect(logger.error).toHaveBeenCalledTimes(1); + }); + }); + + describe('the statements', () => { + it('key parts: the plain organization column, the folded user, then topic and channel', () => { + expect(notificationPreferenceIdentityKeyParts()).toEqual([ + 'organization_id', + "COALESCE(user_id, '')", + 'topic', + 'channel', + ]); + }); + + it('the CREATE spans every row (no WHERE) under the given name', () => { + expect(buildNotificationPreferenceIdentityIndexSql('x')).toBe( + "CREATE UNIQUE INDEX IF NOT EXISTS x ON sys_notification_preference (organization_id, COALESCE(user_id, ''), topic, channel)", + ); + }); + }); + + describe('resolveNotificationPreferenceIndexSeam', () => { + it('asks which driver owns the table, and returns nothing for a host with no raw-SQL driver', () => { + const asked: string[] = []; + const driver = { execute: async () => [], config: { client: 'better-sqlite3' } }; + const engine = { + getDriverForObject: (name: string) => { + asked.push(name); + return driver; + }, + }; + const seam = resolveNotificationPreferenceIndexSeam(engine); + expect(asked).toEqual([NOTIFICATION_PREFERENCE_TABLE]); + expect(typeof seam?.exec).toBe('function'); + expect(resolveNotificationPreferenceIndexSeam({})).toBeUndefined(); + }); + }); +}); + diff --git a/packages/metadata-protocol/src/migrations/notification-preference-identity-index.ts b/packages/metadata-protocol/src/migrations/notification-preference-identity-index.ts new file mode 100644 index 00000000000..b8eab057056 --- /dev/null +++ b/packages/metadata-protocol/src/migrations/notification-preference-identity-index.ts @@ -0,0 +1,309 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `sys_notification_preference` — one admin-global default per (organization, + * topic, channel), delivered at runtime. + * + * ## What is broken without it + * + * `service-messaging`'s `notification-preference.object.ts` declares + * + * ```ts + * { fields: ['user_id', 'topic', 'channel'], unique: 'organization' } + * ``` + * + * and `SqlDriver.syncDeclaredIndexes` materializes it, measured on real SQLite + * from the shipped declaration, as + * + * ```text + * CREATE UNIQUE INDEX `uniq_sys_notification_preference_a22d7d27` + * on `sys_notification_preference` (`organization_id`, `user_id`, `topic`, `channel`) + * ``` + * + * `user_id` is a `sys_user` reference whose EMPTY value is the admin-global + * default (ADR-0118 D1: no sentinel in a user column, so "every user" is the + * empty reference). SQL UNIQUE treats NULLs as mutually DISTINCT, so the + * declared key never constrains the global-default rows. Measured on a real + * engine (ObjectQL over driver-sql): two empty-reference rows for one + * (organization, topic, channel) were both accepted, and the control — one + * real user twice — was refused `409 DUPLICATE_RECORD`. `PreferenceResolver` + * then indexes the rows by key and keeps whichever arrived last, so which of two + * contradictory admin defaults applies is unspecified. + * + * ## Why THIS route + * + * The declared vocabulary (`IndexSchema`: `name` / `fields` / `unique`) has no + * way to mark a non-tenant column NULL-safe, and the driver derives its + * NULL-safe key parts from the tenant column alone. The route ADR-0120 D4 runs + * today for exactly this class is a runtime-managed `kernel:ready` index + * migration, and this module is its fourth instance: it copies + * `sys-setting-identity-index.ts` (`sys_setting`'s `user_id`, the same NULL + * reason) rather than inventing a variant — the same probe-first order + * (`partial-index-probe.ts`), the same seam (`driver-exec.ts`), the same reuse + * of the declared name, and the same refuse-to-migrate disposition. ADR-0120's + * parked declared-vocabulary route, when it lands, retires all four together. + * + * ## Why the index REUSES the declared name + * + * `syncDeclaredIndexes` skips by name, so claiming + * {@link NOTIFICATION_PREFERENCE_IDENTITY_INDEX_NAME} — the name the driver + * derives for the declaration (hash-suffixed, because the long form passes the + * driver's 60-character cap) — keeps every later boot from re-imposing the + * NULL-distinct form. The drift differ does not fight it back for the reason it + * leaves `sys_setting`'s alone: `isSyncReproducibleIndex` admits only a + * `COALESCE` over the tenant column, so this index's `COALESCE(user_id, '')` + * makes it runtime-managed and the differ skips it. + * + * ## The KEY + * + * - `organization_id` — plain, as the driver keys it since ADR-0131 D1 (the + * tenant column is NOT NULL). An index an 18.0 release built with + * `COALESCE(organization_id, '__global__')` is rebuilt to this key, because + * the migration rebuilds the definition on every boot. + * - `user_id` → `COALESCE(user_id, '')`. Copied from `sys_setting`'s + * `user_id` (and `ensureOverlayIndex`'s `package_id` before it). `''` is also + * the engine's own "no link" spelling, so a NULL and an empty-string + * reference are one global bucket, the way `PreferenceResolver` reads them. + * No user id is the empty string, so the bucket cannot collide with real + * data. + * - `topic`, `channel` — required, plain. + * + * ⚠️ Storage is NOT touched: the rows keep their NULL, only the index folds it. + * + * ## Refuse-to-migrate, never keep-one (ADR-0120 D4) + * + * A tightening can fail on a database that already holds two global defaults + * for one key. They are two admin decisions, so nothing here deletes, rewrites + * or reorders a row: the probe-first order leaves the PREVIOUS index in place, + * and the operator gets the key that is not enforced, the consequence, and the + * query that lists the rows ({@link buildNotificationPreferenceDuplicateProbeSql}, + * also what `os migrate duplicates` runs for this index through + * `runtime-index-preflight.ts`). + * + * ## Where it does not reach + * + * SQLite and PostgreSQL build it. MySQL/MariaDB refuse the unparenthesized + * functional key parts (`unsupported`, the previous index stays), and + * `driver-memory` is no SQL host — both keep the declared, NULL-distinct form, + * exactly as they do for `sys_setting`. + */ + +import { logProblem, probeThenReplaceIndex, resolveIndexSeamForTable, type IndexExec, type IndexMigrationLogger, type PartialIndexStatus } from './partial-index-probe.js'; +import { readTablePresence } from './read-probe.js'; + +/** The one table this migration touches. */ +export const NOTIFICATION_PREFERENCE_TABLE = 'sys_notification_preference'; + +/** + * The index name — the SAME one `SqlDriver.syncDeclaredIndexes` derives for the + * declaration, measured on the real driver (and pinned driver-side as the + * hash-suffixed replacement name of this object's organization-scoped key). + */ +export const NOTIFICATION_PREFERENCE_IDENTITY_INDEX_NAME = 'uniq_sys_notification_preference_a22d7d27'; + +/** Throwaway name the probe-first order builds under before touching anything. */ +export const NOTIFICATION_PREFERENCE_IDENTITY_PROBE_INDEX_NAME = 'idx_sys_notification_preference_identity_probe'; + +/** + * The index's key COLUMNS, in the driver's normalized order: the tenant column + * prepended to the three the declaration lists (ADR-0120 D1). + */ +export const NOTIFICATION_PREFERENCE_IDENTITY_INDEX_COLUMNS = [ + 'organization_id', + 'user_id', + 'topic', + 'channel', +] as const; + +/** The nullable key columns and the sentinel each one's NULL folds to. */ +export const NOTIFICATION_PREFERENCE_NULL_SENTINELS: Readonly> = { + user_id: '', +}; + +/** + * The index's key parts, in key order: a bare column, or its NULL-safe + * `COALESCE` form. One builder, so the CREATE, the duplicate listing and the + * degradation messages cannot describe different keys. + */ +export function notificationPreferenceIdentityKeyParts(): string[] { + return NOTIFICATION_PREFERENCE_IDENTITY_INDEX_COLUMNS.map((column) => { + const sentinel = NOTIFICATION_PREFERENCE_NULL_SENTINELS[column]; + return sentinel === undefined ? column : `COALESCE(${column}, '${sentinel}')`; + }); +} + +/** `CREATE UNIQUE INDEX …` under the given name, over the NULL-safe key parts. Every row: no `WHERE`. */ +export function buildNotificationPreferenceIdentityIndexSql(indexName: string): string { + return ( + `CREATE UNIQUE INDEX IF NOT EXISTS ${indexName} ` + + `ON ${NOTIFICATION_PREFERENCE_TABLE} (${notificationPreferenceIdentityKeyParts().join(', ')})` + ); +} + +/** The fallback "is this table here?" statement, for a dialect `read-probe.ts` has no catalog arm for. */ +export function buildNotificationPreferencePresenceSql(): string { + return `SELECT 1 FROM ${NOTIFICATION_PREFERENCE_TABLE} WHERE 1 = 0`; +} + +/** + * The rows blocking the tightening, grouped by exactly the index's key parts. + * + * The folded column is projected through its own `COALESCE` (as + * `user_id_key`), never bare, because PostgreSQL refuses a bare projection of a + * column grouped only inside an expression (the #6772 lesson `sys_setting`'s + * builder records). `user_id_key = ''` reads as "the admin-global default". + * A row whose plain key part is NULL conflicts with nothing under a UNIQUE + * index, so it is no duplicate to list. + * + * No MySQL-spelled twin: unlike `sys_setting`'s `key`, no identifier here is + * reserved on MySQL, so this one statement runs on all three dialects. + */ +export function buildNotificationPreferenceDuplicateProbeSql(): string { + const keyParts = notificationPreferenceIdentityKeyParts(); + const projected = NOTIFICATION_PREFERENCE_IDENTITY_INDEX_COLUMNS.map((column, i) => { + const keyPart = keyParts[i]!; + return keyPart === column ? keyPart : `${keyPart} AS ${column}_key`; + }); + const plain = NOTIFICATION_PREFERENCE_IDENTITY_INDEX_COLUMNS.filter( + (column) => NOTIFICATION_PREFERENCE_NULL_SENTINELS[column] === undefined, + ); + return ( + `SELECT ${projected.join(', ')}, COUNT(*) AS duplicate_rows ` + + `FROM ${NOTIFICATION_PREFERENCE_TABLE} ` + + `WHERE ${plain.map((column) => `${column} IS NOT NULL`).join(' AND ')} ` + + `GROUP BY ${keyParts.join(', ')} HAVING COUNT(*) > 1` + ); +} + +/** + * This migration's statuses: the shared vocabulary plus `'absent'` — no such + * table on this kernel (the messaging service is not composed, or the table is + * not provisioned yet). A silent no-op, never a degradation. + */ +export type EnsureNotificationPreferenceIndexStatus = PartialIndexStatus | 'absent'; + +export interface EnsureNotificationPreferenceIndexResult { + status: EnsureNotificationPreferenceIndexStatus; + /** Driver error text, when there was one. */ + detail?: string; +} + +/** Resolve the raw-SQL seam (and dialect) for `sys_notification_preference`. @see resolveIndexSeamForTable */ +export function resolveNotificationPreferenceIndexSeam( + engine: unknown, +): { exec: IndexExec; client?: string } | undefined { + return resolveIndexSeamForTable(engine, NOTIFICATION_PREFERENCE_TABLE); +} + +/** + * Replace the NULL-distinct declared UNIQUE index with the NULL-safe one. + * + * Idempotent: a re-run rebuilds the same definition. Best-effort by design — a + * boot never fails because an index could not be tightened, so every branch + * returns a status instead of throwing, and every branch that leaves the key + * unenforced says so at `error`. + */ +export async function ensureNotificationPreferenceIdentityIndex( + exec: IndexExec | undefined, + logger?: IndexMigrationLogger, + opts: { client?: string } = {}, +): Promise { + if (!exec) return { status: 'no-driver' }; + + const presence = await readTablePresence(exec, { + table: NOTIFICATION_PREFERENCE_TABLE, + client: opts.client, + fallbackSql: buildNotificationPreferencePresenceSql(), + }); + if (presence.verdict === 'absent') return { status: 'absent' }; + if (presence.verdict === 'unreadable') { + // Not absence: nothing looked, so an operator is told. + logProblem( + logger, + `[metadata-protocol] could not read whether "${NOTIFICATION_PREFERENCE_TABLE}" exists, so the ` + + 'one-global-default-per-key index tightening did NOT run and the table keeps whatever unique index ' + + 'it had. This is not the table being absent, which is a normal, silent no-op. Verify by hand with: ' + + buildNotificationPreferencePresenceSql(), + presence.detail ?? '', + ); + return { status: 'failed', detail: presence.detail }; + } + + const outcome = await probeThenReplaceIndex(exec, { + indexName: NOTIFICATION_PREFERENCE_IDENTITY_INDEX_NAME, + probeIndexName: NOTIFICATION_PREFERENCE_IDENTITY_PROBE_INDEX_NAME, + buildSql: buildNotificationPreferenceIdentityIndexSql, + }); + if (outcome.status === 'created') return { status: 'created' }; + + const detail = outcome.detail ?? ''; + if (outcome.failedAt === 'replace') { + // Only reachable on a race between the drop and the create: the probe + // already cleared dialect and data. + logProblem( + logger, + `[metadata-protocol] could not create '${NOTIFICATION_PREFERENCE_IDENTITY_INDEX_NAME}' on ` + + `"${NOTIFICATION_PREFERENCE_TABLE}" after the probe succeeded — the table may currently have NO ` + + `unique index on (${notificationPreferenceIdentityKeyParts().join(', ')}). Restart to retry.`, + detail, + ); + return { status: 'failed', detail }; + } + + reportDegradation(outcome.status, detail, logger); + return { status: outcome.status, detail }; +} + +/** + * Say what is NOT enforced and what fixes it (ADR-0120 D4's wording contract). + * + * Every arm is `error`, on the AGENTS.md question rather than by default: + * afterwards the platform looks healthy while two contradictory admin-global + * notification defaults for one (topic, channel) can coexist, and which one + * applies to every user is unspecified. + */ +function reportDegradation(status: PartialIndexStatus, detail: string, logger?: IndexMigrationLogger): void { + const columns = NOTIFICATION_PREFERENCE_IDENTITY_INDEX_COLUMNS.join(', '); + const keyParts = notificationPreferenceIdentityKeyParts().join(', '); + const duplicateQuery = buildNotificationPreferenceDuplicateProbeSql(); + const consequence = + 'two admin-global defaults (an empty User) for one topic and channel in ONE organization can coexist, ' + + 'and which of them applies to every user is unspecified'; + + if (status === 'unsupported') { + // MySQL/MariaDB: no (unparenthesized) functional key parts. The previous + // index stays — ADR-0120 D3's degradation, kept rather than rebuilt. + logProblem( + logger, + `[metadata-protocol] this database cannot build the NULL-safe notification preference index — ` + + `'${NOTIFICATION_PREFERENCE_IDENTITY_INDEX_NAME}' on "${NOTIFICATION_PREFERENCE_TABLE}" stays ` + + `NULL-distinct on user_id over (${columns}). The system keeps looking healthy while ${consequence}. ` + + 'There is no in-dialect fix: run this platform on SQLite/PostgreSQL for the guarantee, and meanwhile ' + + `watch for duplicates with: ${duplicateQuery}`, + detail, + ); + return; + } + + if (status === 'conflict') { + logProblem( + logger, + `[metadata-protocol] cannot tighten '${NOTIFICATION_PREFERENCE_IDENTITY_INDEX_NAME}' on ` + + `"${NOTIFICATION_PREFERENCE_TABLE}" — existing rows violate (${keyParts}). The previous index is ` + + `left in place and the NULL-safe key is NOT enforced, so ${consequence}. Preferences are ` + + 'admin-authored, so no row is discarded automatically and this migration keeps refusing until an ' + + `operator deletes the one that should not apply. List them with: ${duplicateQuery} — or run ` + + '"os migrate duplicates" — then restart (ADR-0120 D4).', + detail, + ); + return; + } + + logProblem( + logger, + `[metadata-protocol] could not rebuild '${NOTIFICATION_PREFERENCE_IDENTITY_INDEX_NAME}' on ` + + `"${NOTIFICATION_PREFERENCE_TABLE}" as the NULL-safe index; the existing index is unchanged, so ` + + `${consequence}, while everything else looks healthy. Fix the cause below and restart.`, + detail, + ); +} diff --git a/packages/metadata-protocol/src/migrations/runtime-index-preflight.test.ts b/packages/metadata-protocol/src/migrations/runtime-index-preflight.test.ts index 28e282aca5c..a3653ad9d14 100644 --- a/packages/metadata-protocol/src/migrations/runtime-index-preflight.test.ts +++ b/packages/metadata-protocol/src/migrations/runtime-index-preflight.test.ts @@ -15,6 +15,10 @@ import { buildSysSettingDuplicateProbeSqlMysql, buildSysSettingPresenceSql, } from './sys-setting-identity-index.js'; +import { + buildNotificationPreferenceDuplicateProbeSql, + buildNotificationPreferencePresenceSql, +} from './notification-preference-identity-index.js'; import type { IndexExec } from './partial-index-probe.js'; /** @@ -98,6 +102,22 @@ describe('kernel:ready index pre-flight (#8725)', () => { meta.run('m4', 'view', 'crm_case.retired', null, null, 'archived'); meta.run('m5', 'view', 'crm_case.retired', null, null, 'archived'); + // ── sys_notification_preference: two admin-global defaults for one key ── + // An EMPTY `user_id` is the admin-global default (ADR-0118 D1). Two of + // them for one (organization, topic, channel) is what the NULL-safe + // index refuses. Controls: the same key in ANOTHER organization, and a + // real user's row for the same topic and channel, are both legal. + db.exec(`CREATE TABLE sys_notification_preference ( + id TEXT PRIMARY KEY, organization_id TEXT, user_id TEXT, topic TEXT, channel TEXT + );`); + const pref = db.prepare( + 'INSERT INTO sys_notification_preference (id, organization_id, user_id, topic, channel) VALUES (?,?,?,?,?)', + ); + pref.run('p1', 'org_a', null, 'deal.won', 'email'); + pref.run('p2', 'org_a', null, 'deal.won', 'email'); + pref.run('p3', 'org_b', null, 'deal.won', 'email'); + pref.run('p4', 'org_a', 'usr_1', 'deal.won', 'email'); + // ⛔ `sys_setting` is deliberately NOT created: it is registered by the // OPTIONAL `service-settings`, so an ordinary kernel reaches // `kernel:ready` without it and the migration treats that as a silent @@ -121,6 +141,7 @@ describe('kernel:ready index pre-flight (#8725)', () => { 'idx_sys_metadata_overlay_active:blocked', 'idx_sys_metadata_overlay_draft:clear', 'uniq_sys_setting_organization_id_namespace_key_scope_user_id:table-absent', + 'uniq_sys_notification_preference_a22d7d27:blocked', ]); // The overlay conflict. [ADR-0131 C7b] The organization is no part of @@ -155,6 +176,13 @@ describe('kernel:ready index pre-flight (#8725)', () => { expect(by('idx_sys_metadata_overlay_draft', results).groups).toEqual([]); expect(by('uniq_sys_setting_organization_id_namespace_key_scope_user_id', results).groups).toEqual([]); + + // The preference conflict: org_a's two global defaults, and nothing + // else — not org_b's lone default, not usr_1's own row. + expect(by('uniq_sys_notification_preference_a22d7d27', results).groups).toEqual([ + { key: { organization_id: 'org_a', user_id_key: '', topic: 'deal.won', channel: 'email' }, rowCount: 2 }, + ]); + expect(by('uniq_sys_notification_preference_a22d7d27', results).rowScope).toBeNull(); }); it('[ADR-0131 C7b] names the legacy organization rows the re-keyed index now refuses: two organizations, or an organization and the environment, on one key', async () => { @@ -206,6 +234,7 @@ describe('kernel:ready index pre-flight (#8725)', () => { 'ensureMetadataOverlayIndexes:sys_metadata:idx_sys_metadata_overlay_active', 'ensureMetadataOverlayIndexes:sys_metadata:idx_sys_metadata_overlay_draft', 'ensureSysSettingIdentityIndex:sys_setting:uniq_sys_setting_organization_id_namespace_key_scope_user_id', + 'ensureNotificationPreferenceIdentityIndex:sys_notification_preference:uniq_sys_notification_preference_a22d7d27', ]); } }); @@ -219,11 +248,15 @@ describe('kernel:ready index pre-flight (#8725)', () => { expect(sql['uniq_sys_setting_organization_id_namespace_key_scope_user_id']).toBe( buildSysSettingDuplicateProbeSql(), ); + expect(sql['uniq_sys_notification_preference_a22d7d27']).toBe(buildNotificationPreferenceDuplicateProbeSql()); // The presence question is the migration's own too, so this cannot come // to a different verdict about the table than the migration does. expect( probes.find((p) => p.table === 'sys_setting')!.presenceSql, ).toBe(buildSysSettingPresenceSql()); + expect( + probes.find((p) => p.table === 'sys_notification_preference')!.presenceSql, + ).toBe(buildNotificationPreferencePresenceSql()); }); it('spells the sys_setting probe for MySQL, where the bare form is a parse error', () => { diff --git a/packages/metadata-protocol/src/migrations/runtime-index-preflight.ts b/packages/metadata-protocol/src/migrations/runtime-index-preflight.ts index 43be5f56236..4ad9fdbdad3 100644 --- a/packages/metadata-protocol/src/migrations/runtime-index-preflight.ts +++ b/packages/metadata-protocol/src/migrations/runtime-index-preflight.ts @@ -5,13 +5,14 @@ * * ## The gap this closes * - * Two migrations in this directory tighten an existing UNIQUE index into a + * Three migrations in this directory tighten an existing UNIQUE index into a * NULL-safe (and sometimes row-scoped) form at `kernel:ready`: * * | migration | table | index(es) | * |---|---|---| * | `ensureMetadataOverlayIndexes` | `sys_metadata` | active + draft | * | `ensureSysSettingIdentityIndex` | `sys_setting` | row identity | + * | `ensureNotificationPreferenceIdentityIndex` | `sys_notification_preference` | one global default per key | * * A third, `ensureViewDefinitionActiveIndex` (`sys_view_definition`), retired * with its table under ADR-0131 D13: no writer or reader of that table's rows @@ -92,6 +93,13 @@ import { buildSysSettingPresenceSql, sysSettingIdentityKeyParts, } from './sys-setting-identity-index.js'; +import { + NOTIFICATION_PREFERENCE_IDENTITY_INDEX_NAME, + NOTIFICATION_PREFERENCE_TABLE, + buildNotificationPreferenceDuplicateProbeSql, + buildNotificationPreferencePresenceSql, + notificationPreferenceIdentityKeyParts, +} from './notification-preference-identity-index.js'; /** The column every one of the duplicate-listing queries counts into. */ const DUPLICATE_ROWS_COLUMN = 'duplicate_rows'; @@ -168,7 +176,7 @@ function isMysqlClient(client?: string): boolean { } /** - * Every index the `kernel:ready` migrations tighten — THREE, from two + * Every index the `kernel:ready` migrations tighten — FOUR, from three * migrations, because `ensureMetadataOverlayIndexes` builds one index per * overlay state and either can be blocked independently. * @@ -177,7 +185,7 @@ function isMysqlClient(client?: string): boolean { * `sys_setting`'s listing query is the only one whose bare spelling is not * merely unidiomatic on MySQL but a parse error: `key` is a RESERVED word * there, measured as `ERROR 1064` on MySQL 8.0.46 (#9434), which is why the - * migration already ships a MySQL-spelled variant. The other two queries name + * migration already ships a MySQL-spelled variant. The other queries name * no MySQL-reserved identifier, so the platform's own spelling — the one the * migration prints in its boot report — runs on all three dialects, and * compiling a second variant of them would buy nothing and add a second @@ -213,6 +221,18 @@ export function runtimeIndexProbes(opts: { client?: string } = {}): RuntimeIndex ? buildSysSettingDuplicateProbeSqlMysql() : buildSysSettingDuplicateProbeSql(), }, + { + migration: 'ensureNotificationPreferenceIdentityIndex', + table: NOTIFICATION_PREFERENCE_TABLE, + index: NOTIFICATION_PREFERENCE_IDENTITY_INDEX_NAME, + keyParts: notificationPreferenceIdentityKeyParts(), + // No `WHERE`: every row, global defaults included — they are the + // rows the NULL-safe key exists to constrain. + rowScope: null, + // The migration's own fallback presence statement, as for `sys_setting`. + presenceSql: buildNotificationPreferencePresenceSql(), + duplicateSql: buildNotificationPreferenceDuplicateProbeSql(), + }, ]; } diff --git a/packages/metadata-protocol/src/plugin.notification-preference-index-armed.test.ts b/packages/metadata-protocol/src/plugin.notification-preference-index-armed.test.ts new file mode 100644 index 00000000000..b3cd5c1ec12 --- /dev/null +++ b/packages/metadata-protocol/src/plugin.notification-preference-index-armed.test.ts @@ -0,0 +1,49 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect, vi } from 'vitest'; + +import { assembleMetadataProtocol } from './plugin.js'; +import { NOTIFICATION_PREFERENCE_TABLE } from './migrations/notification-preference-identity-index.js'; + +/** + * `ensureNotificationPreferenceIdentityIndex` is ARMED from the one assembly + * both protocol mounts share, inside the `runPlatformMigrations`-gated + * `kernel:ready` handler beside the `sys_setting` tightening — and only there. + * The migration itself is pinned in its own file; this pins the wiring, over a + * recording seam (the claim is about which statements the hook issues). + */ +describe('armed from assembleMetadataProtocol, under runPlatformMigrations only', () => { + /** The assembly over a recording seam; returns the statements each kernel:ready handler issued. */ + async function readyStatements(runPlatformMigrations: boolean): Promise { + const statements: string[] = []; + const execute = async (sql: string) => { + statements.push(String(sql)); + return []; + }; + const driver = { execute, raw: execute, config: { client: 'better-sqlite3' } }; + const ql = { registerApp: () => {}, driver, getDriver: () => driver, getDriverForObject: () => driver }; + const hooks: Array<() => unknown> = []; + const ctx = { + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + registerService: vi.fn(), + getService: vi.fn(() => undefined), + getServices: () => new Map(), + hook: (name: string, handler: () => unknown) => { + if (name === 'kernel:ready') hooks.push(handler); + }, + } as any; + assembleMetadataProtocol(ctx, ql, undefined, { runPlatformMigrations }); + for (const handler of hooks) await handler(); + return statements; + } + + it('asks about sys_notification_preference at kernel:ready when the platform migrations are armed', async () => { + const statements = await readyStatements(true); + expect(statements.some((sql) => sql.includes(NOTIFICATION_PREFERENCE_TABLE))).toBe(true); + }); + + it('issues nothing against it when they are not (a per-project kernel)', async () => { + const statements = await readyStatements(false); + expect(statements.filter((sql) => sql.includes(NOTIFICATION_PREFERENCE_TABLE))).toEqual([]); + }); +}); diff --git a/packages/metadata-protocol/src/plugin.ts b/packages/metadata-protocol/src/plugin.ts index 1d27a1a0669..0cb4ea2c5d7 100644 --- a/packages/metadata-protocol/src/plugin.ts +++ b/packages/metadata-protocol/src/plugin.ts @@ -38,6 +38,10 @@ import { backfillSeedTenancy, resolveSeedTenancySeam, } from './migrations/seed-tenancy-backfill.js'; +import { + ensureNotificationPreferenceIdentityIndex, + resolveNotificationPreferenceIndexSeam, +} from './migrations/notification-preference-identity-index.js'; import { createRecordedBySentinelPlan } from './migrations/recorded-by-sentinel.js'; import { ObjectStackProtocolImplementation } from './protocol.js'; import type { MetadataAuthoringChannel } from './protocol.js'; @@ -338,6 +342,24 @@ export function assembleMetadataProtocol( { error: e instanceof Error ? e.message : String(e) }, ); } + // `sys_notification_preference`'s one-global-default-per-key + // index: the fourth runtime-managed tightening (ADR-0120 D4), + // the `sys_setting` paradigm above applied to a + // `user_id` whose EMPTY reference is the admin-global default + // (ADR-0118 D1). Same seam, same gate, own try/catch: the table + // belongs to `service-messaging`, so it may be absent here, + // which the migration answers as a silent no-op. + try { + const seam = resolveNotificationPreferenceIndexSeam(ql); + await ensureNotificationPreferenceIdentityIndex(seam?.exec, ctx.logger, { + client: seam?.client, + }); + } catch (e: unknown) { + ctx.logger.warn( + '[metadata-protocol] notification preference index migration skipped — the NULL-safe index that allows one admin-global default (an empty User) per topic and channel in an organization was not ensured this boot', + { error: e instanceof Error ? e.message : String(e) }, + ); + } }); } diff --git a/packages/qa/dogfood/test/upgrade-18x-index-equivalence.dogfood.test.ts b/packages/qa/dogfood/test/upgrade-18x-index-equivalence.dogfood.test.ts index eb05567cf32..8331eb8db71 100644 --- a/packages/qa/dogfood/test/upgrade-18x-index-equivalence.dogfood.test.ts +++ b/packages/qa/dogfood/test/upgrade-18x-index-equivalence.dogfood.test.ts @@ -34,6 +34,15 @@ import { relaxTenantColumns } from './pre-18-tenant-column.testkit.js'; const fixtureSql = readFileSync(new URL('./upgrade-harness/fixture-18.0/database.sql', import.meta.url), 'utf8'); +/** + * The organization uniques a `kernel:ready` migration in `metadata-protocol` + * rebuilds on every boot (runtime-managed, ADR-0120 D4), so no boot leaves + * their 18.0 spelling in place: `sys_setting`'s identity index, and + * `sys_notification_preference`'s, NULL-safe on `user_id`. + */ +const RUNTIME_MANAGED_TABLES = new Set(['sys_setting', 'sys_notification_preference']); +const RUNTIME_MANAGED_INDEX = /^uniq_(sys_setting_|sys_notification_preference_)/; + /** One unique index of the file, as SQLite recorded its DDL. */ interface UniqueIndex { name: string; @@ -166,12 +175,14 @@ describe('[18.x step 7b] the organization key part across the 18.0 → HEAD upgr expectNoDestructiveIndexDrift(await managedDrift(stack!)); }); - it('the 12 declared organization composites survive the HEAD boot byte for byte', () => { - // `sys_setting`'s identity index is the thirteenth: the metadata - // protocol's own runtime migration rebuilds it on every boot, now over - // the plain organization key. - const declared = COALESCE_INDEXES_18_0.filter((i) => !i.name.startsWith('uniq_sys_setting_')); - expect(declared).toHaveLength(12); + it('the 11 declared organization composites survive the HEAD boot byte for byte', () => { + // The other two are runtime-managed: the metadata protocol's own + // migrations rebuild them on every boot over the plain organization key — + // `sys_setting`'s identity index, and `sys_notification_preference`'s + // one-global-default-per-key index, NULL-safe on its `user_id` (an empty + // User is the admin-global default, ADR-0118 D1). + const declared = COALESCE_INDEXES_18_0.filter((i) => !RUNTIME_MANAGED_INDEX.test(i.name)); + expect(declared).toHaveLength(11); const after = new Map(organizationUniqueIndexes(restored).map((i) => [i.name, i.sql])); for (const index of declared) expect(after.get(index.name), index.name).toBe(index.sql); }); @@ -197,10 +208,10 @@ describe('[18.x step 7b] the organization key part across the 18.0 → HEAD upgr try { const ql = (await first.kernel.getServiceAsync('objectql')) as { getDriverForObject(object: string): unknown }; // The tables whose organization unique 18.0.0 keyed through COALESCE — - // the ones whose equivalence is under test. (`sys_setting`'s identity - // index is its runtime migration's, rebuilt every boot.) + // the ones whose equivalence is under test. (The runtime-managed + // indexes are their migrations', rebuilt every boot.) const tenantTables = [ - ...new Set(organizationUniqueIndexes(born).map((i) => i.table).filter((t) => t !== 'sys_setting')), + ...new Set(organizationUniqueIndexes(born).map((i) => i.table).filter((t) => !RUNTIME_MANAGED_TABLES.has(t))), ]; await relaxTenantColumns(ql, tenantTables); } finally { @@ -209,8 +220,8 @@ describe('[18.x step 7b] the organization key part across the 18.0 → HEAD upgr const db = new DatabaseSync(born); try { for (const index of organizationUniqueIndexes(born)) { - // `sys_setting`'s identity index is its runtime migration's, rebuilt every boot. - if (index.table === 'sys_setting') continue; + // A runtime-managed index is its migration's, rebuilt every boot. + if (RUNTIME_MANAGED_TABLES.has(index.table)) continue; const sql = index.sql.replace(/\(\s*([`"]?)organization_id\1\s*([,)])/, "(COALESCE($1organization_id$1, '__global__')$2") .replace(/,\s*([`"]?)organization_id\1\s*([,)])/, ", COALESCE($1organization_id$1, '__global__')$2"); if (sql === index.sql) continue; diff --git a/packages/services/service-messaging/src/messaging-service-plugin.test.ts b/packages/services/service-messaging/src/messaging-service-plugin.test.ts index 25c2e593d32..46904433d33 100644 --- a/packages/services/service-messaging/src/messaging-service-plugin.test.ts +++ b/packages/services/service-messaging/src/messaging-service-plugin.test.ts @@ -1,7 +1,10 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect, vi } from 'vitest'; -import { ObjectQLPlugin } from '@objectstack/objectql'; +import { ObjectQL, ObjectQLPlugin } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { SysUser } from '@objectstack/platform-objects'; +import { installDefaultOrganization, DEFAULT_ORGANIZATION_ID } from './default-organization.testkit.js'; import { MessagingServicePlugin } from './messaging-service-plugin.js'; import type { MessagingService } from './messaging-service.js'; import { assertEngineFindOnePredicate, type EngineFindOneQueryInput } from '@objectstack/metadata-core'; @@ -421,3 +424,62 @@ describe('MessagingServicePlugin — a failed provision is an error, once per ob expect(synced).toEqual(OBJECTS); }); }); + +/** + * [ADR-0118 D1] `sys_notification_preference.user_id` is a `sys_user` + * reference whose EMPTY value is the admin-global default. The plugin rewrites + * rows the text column stored as `'*'` at `kernel:ready`, on every boot, so no + * admin default silently stops applying across the upgrade. The conversion + * itself is pinned in `preference-user-reference.integration.test.ts`; this + * pins that the plugin ARMS it. + */ +describe("MessagingServicePlugin — converts stored user_id '*' to the empty reference at kernel:ready", () => { + async function bootWithStarRow() { + const silent: any = { debug() {}, info() {}, warn() {}, error() {}, child() { return silent; } }; + const driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }); + const ql = new ObjectQL({ logger: silent }); + ql.registerDriver(driver, true); + await ql.init(); + ql.registry.registerObject(SysUser as any, 'test'); + const services = new Map([ + ['data', ql], + // The plugin's own objects, registered the way the manifest service would. + ['manifest', { register(m: { objects?: unknown[] }) { for (const o of m.objects ?? []) ql.registry.registerObject(o as any, 'test'); } }], + ]); + const readyHooks: Array<() => Promise | void> = []; + const logs: string[] = []; + const logger = { info(msg: string) { logs.push(String(msg)); }, warn() {}, error() {}, debug() {}, child() { return logger; } }; + const ctx = { + logger, + getService(name: string) { return services.get(name); }, + registerService(name: string, svc: unknown) { services.set(name, svc); }, + hook(event: string, fn: () => Promise | void) { + if (event === 'kernel:ready') readyHooks.push(fn); + }, + } as any; + await new MessagingServicePlugin({ reliableDelivery: false }).init(ctx); + await ql.syncSchemas(); + await installDefaultOrganization(ql as any); + await driver.create('sys_notification_preference', { + id: 'p_star', organization_id: DEFAULT_ORGANIZATION_ID, user_id: '*', topic: 'deal.won', channel: 'email', enabled: false, + }); + const userIdOf = async () => + (await ql.findOne('sys_notification_preference', { where: { id: 'p_star' } }, { context: { isSystem: true } }))!.user_id ?? null; + return { ql, logs, userIdOf, fireReady: async () => { for (const fn of readyHooks) await fn(); } }; + } + + it('rewrites the row on kernel:ready, and a second ready changes nothing', async () => { + const { ql, logs, userIdOf, fireReady } = await bootWithStarRow(); + try { + expect(await userIdOf()).toBe('*'); + await fireReady(); + expect(await userIdOf()).toBeNull(); + expect(logs.filter((l) => l.includes("user_id '*'"))).toHaveLength(1); + await fireReady(); + expect(await userIdOf()).toBeNull(); + expect(logs.filter((l) => l.includes("user_id '*'"))).toHaveLength(1); + } finally { + await ql.destroy(); + } + }); +}); diff --git a/packages/services/service-messaging/src/messaging-service-plugin.ts b/packages/services/service-messaging/src/messaging-service-plugin.ts index d61c8501f86..9780050d20d 100644 --- a/packages/services/service-messaging/src/messaging-service-plugin.ts +++ b/packages/services/service-messaging/src/messaging-service-plugin.ts @@ -14,6 +14,7 @@ import { createEmailChannel } from './email-channel.js'; import { createSmsChannel } from './sms-channel.js'; import { NotificationTemplateStore } from './template-renderer.js'; import { NotificationTextRenderer, type NotificationTextI18n } from './notification-text.js'; +import { convertWildcardPreferenceUsers } from './preference-user-conversion.js'; import { InboxMessage, NotificationReceipt, @@ -365,6 +366,20 @@ export class MessagingServicePlugin implements Plugin { }); } + // [ADR-0118 D1] `sys_notification_preference.user_id` is a `sys_user` + // reference whose EMPTY value is the admin-global default; a row stored + // as the text column's `'*'` sentinel matches no recipient any more. + // Rewrite those rows to the empty reference on every boot, after the + // table provisioning above (hooks run in registration order), so no + // admin default silently stops applying across the upgrade and the + // resolver carries no `'*'` tolerance. Idempotent and never throws; see + // {@link convertWildcardPreferenceUsers}. + if (typeof ctx.hook === 'function') { + ctx.hook('kernel:ready', async () => { + await convertWildcardPreferenceUsers(getData(), ctx.logger); + }); + } + // Email channel (ADR-0030 P3) + SMS channel (#2780): mounted through a // LAZY PROVIDER, so the mount TRACKS the transport service instead of // recording a verdict about it (#18050). diff --git a/packages/services/service-messaging/src/objects/notification-display-title.test.ts b/packages/services/service-messaging/src/objects/notification-display-title.test.ts index 516d8e13e73..fa71c886dc7 100644 --- a/packages/services/service-messaging/src/objects/notification-display-title.test.ts +++ b/packages/services/service-messaging/src/objects/notification-display-title.test.ts @@ -2,9 +2,15 @@ /** * The record title of the notification objects that declared a - * `titleFormat` and no title pointer (#20044): `sys_notification_delivery`, - * `sys_notification_preference` and `sys_notification_receipt` (the fourth, - * `sys_notification_subscription`, is retired). + * `titleFormat` and no title pointer (#20044): `sys_notification_delivery` + * and `sys_notification_receipt` (`sys_notification_subscription` is retired). + * + * `sys_notification_preference` left this table when its `user_id` became a + * `sys_user` reference (ADR-0118 D1): its title now names the PERSON one hop + * through that reference ('All users' for the empty reference, the admin-global + * default), so it no longer equals the `titleFormat` text this file holds the + * others to. Its case — the raw-id title 'usr_alice · billing.invoice · email' + * — is reversed in `../preference-user-reference.integration.test.ts`. * * ADR-0079 resolves a record's title as `nameField ?? displayNameField ?? * derivation`, and an explicit `nameField` takes precedence over the @@ -15,8 +21,8 @@ * A renderer honouring the order therefore drew the raw id as the record * page's H1. * - * The two composite titles now point at `display_title`, a text formula over - * the columns `titleFormat` names. `sys_notification_receipt`'s title is one + * The composite title points at `display_title`, a text formula over the + * columns `titleFormat` names. `sys_notification_receipt`'s title is one * column (`{state}`), so its `nameField` names that column directly — the * describe's own migration for a single-field title. An explicit pointer is * honoured whatever the field's type (ADR-0079 D4); `select` is only kept out @@ -47,7 +53,6 @@ import { import { SqlDriver } from '@objectstack/driver-sql'; import { resolveDisplayField } from '@objectstack/spec/data'; import { NotificationDelivery } from './notification-delivery.object.js'; -import { NotificationPreference } from './notification-preference.object.js'; import { NotificationReceipt } from './notification-receipt.object.js'; import { installDefaultOrganization } from '../default-organization.testkit.js'; @@ -84,15 +89,6 @@ const CASES: Case[] = [ refused: ['channel', 'recipient_id'], defaulted: {}, }, - { - schema: NotificationPreference, - pointer: 'display_title', - row: () => ({ user_id: 'usr_alice', topic: 'billing.invoice', channel: 'email', created_at: NOW }), - title: 'usr_alice · billing.invoice · email', - refused: ['user_id'], - // `topic` / `channel` default to the '*' wildcard (ADR-0030 Layer 3). - defaulted: { topic: 'usr_alice · * · email', channel: 'usr_alice · billing.invoice · *' }, - }, { schema: NotificationReceipt, pointer: 'state', diff --git a/packages/services/service-messaging/src/objects/notification-keyed-text-bounds.test.ts b/packages/services/service-messaging/src/objects/notification-keyed-text-bounds.test.ts index b93e75582ba..c53de91db63 100644 --- a/packages/services/service-messaging/src/objects/notification-keyed-text-bounds.test.ts +++ b/packages/services/service-messaging/src/objects/notification-keyed-text-bounds.test.ts @@ -40,7 +40,17 @@ describe('sys_notification_* keyed-text bounds carry their producers’ widths ( expect(bound(NotificationDelivery, 'recipient_id')).toBe(PHYSICAL_ID_WIDTH); expect(bound(NotificationReceipt, 'notification_id')).toBe(PHYSICAL_ID_WIDTH); expect(bound(NotificationReceipt, 'user_id')).toBe(PHYSICAL_ID_WIDTH); - expect(bound(NotificationPreference, 'user_id')).toBe(PHYSICAL_ID_WIDTH); + }); + + it('a REFERENCE column carries no width of its own: sys_notification_preference.user_id (ADR-0118 D1)', () => { + // It was a text column bounded to the id width; it is now a `sys_user` + // lookup, which holds the referenced row's id — a width the parent's id + // column decides, not this field (driver-sql sizes every reference the + // same and ignores a declared `maxLength` on it). + const field = (NotificationPreference as any).fields.user_id; + expect(field.type).toBe('lookup'); + expect(field.reference).toBe('sys_user'); + expect(bound(NotificationPreference, 'user_id')).toBeUndefined(); }); it('topic columns equal sys_notification.topic’s own declared bound -- the event topic they are matched against', () => { diff --git a/packages/services/service-messaging/src/objects/notification-preference.object.ts b/packages/services/service-messaging/src/objects/notification-preference.object.ts index 5049b1bb589..f73a77ca0ee 100644 --- a/packages/services/service-messaging/src/objects/notification-preference.object.ts +++ b/packages/services/service-messaging/src/objects/notification-preference.object.ts @@ -14,10 +14,18 @@ import { F } from '@objectstack/spec'; * (user, topic, channel) → (user, topic, *) → (user, *, channel) → * (user, *, *) → ('*', topic, channel) → … → ('*', '*', '*') → default ON * - * `user_id = '*'` rows are the **admin global default**; a real-user row - * **overrides** it. `topic = '*'` / `channel = '*'` are wildcards. Mandatory + * A row whose `user_id` is EMPTY is the **admin global default**; a real-user + * row **overrides** it. `topic = '*'` / `channel = '*'` are wildcards. Mandatory * topics (configured on the service) bypass this object entirely. * + * [ADR-0118 D1] `user_id` is a `sys_user` reference, so "everyone" is the + * empty reference, never a sentinel in a user column. It used to be a text + * column holding a user id or the literal `'*'`: the write door stored any + * string (a mistyped id matched nobody, silently), and the record title could + * only show the raw id, because the formula read path reads one hop only + * through a reference. Stored `'*'` rows are rewritten to the empty reference + * at boot by `convertWildcardPreferenceUsers` (`../preference-user-conversion.ts`). + * * Belongs to `service-messaging` (owner of the delivery pipeline). */ export const NotificationPreference = ObjectSchema.create({ @@ -27,7 +35,7 @@ export const NotificationPreference = ObjectSchema.create({ icon: 'bell-ring', isSystem: true, // [ADR-0103, #3355] Admin/user-writable DATA on a platform-defined schema: a - // user authors their own mute/allow rows (and admins the `user_id = '*'` + // user authors their own mute/allow rows (and admins the empty-`user_id` // global defaults) from the Setup "Notification Preferences" grid. The bucket // default is full CRUD, so no `userActions` block is needed — RLS is the authz. managedBy: 'system-data', @@ -71,25 +79,46 @@ export const NotificationPreference = ObjectSchema.create({ id: Field.text({ label: 'Preference ID', required: true, readonly: true }), // [ADR-0079] The record title (`nameField` above). A formula is computed - // on read and has no stored column. `user_id`, `topic` and `channel` - // are all required, so the expression needs no null guard. + // on read and has no stored column. It names the PERSON, not the raw + // user id: the user's `name`, or their `email` where the name is blank + // (the platform's `name || email` handle rule), read ONE hop through + // `user_id` (ADR-0079 addendum), under the reader's own authority. + // + // The empty reference is the admin-global default (ADR-0118 D1), and + // the optional read (`.?field.orValue(…)`) is what answers it: an + // EMPTY reference reads 'All users'. A reference the read cannot + // resolve (a user that is gone, or one this reader may not read) is a + // different case and leaves the whole title null; it is never + // rendered as 'All users', which would name the wrong row. + // `user_id` is read only through the hop: a bare read beside a + // traversal is refused (`traversal-bare-and-traversed`). `topic` and + // `channel` are required and default to '*', so they need no guard. display_title: Field.formula({ label: 'Title', returnType: 'text', - expression: F`record.user_id + ' · ' + record.topic + ' · ' + record.channel`, - description: 'Record title: the user, the topic and the channel this toggle covers (computed on read)', + expression: F`(isBlank(trim(record.user_id.?name.orValue(''))) ? record.user_id.?email.orValue('All users') : record.user_id.?name.orValue('')) + ' · ' + record.topic + ' · ' + record.channel`, + description: 'Record title: the person (or All users), the topic and the channel this toggle covers (computed on read)', }), - user_id: Field.text({ + // [ADR-0118 D1] A `sys_user` reference. Empty = the admin-global + // default for every user; never a sentinel. The write door refuses an + // id no `sys_user` row holds (`reference_not_found`), so `'*'` and a + // mistyped id are refused instead of being stored to match nobody. + // + // `deleteBehavior: 'cascade'` is load-bearing. The lookup default is + // `set_null`, which empties the reference when the user is deleted, + // and here the empty reference MEANS everyone: that person's own mute + // would become the admin-global mute for every user. Their rows go + // with them instead. + // + // No `maxLength`: a reference holds the referenced row's id, whose + // width is the parent's id column, not this field's to declare. + user_id: Field.lookup('sys_user', { label: 'User', - required: true, + required: false, searchable: true, - // [commit e4902d2b9] Referenced-column bound (route A, ruling 2026-08-24): a - // `sys_user.id` — physical varchar(255), the id column driver-sql - // creates (`table.string('id').primary()`) — or the 1-char - // literal '*'. - maxLength: 255, - description: "Recipient user id, or '*' for the admin-global default.", + deleteBehavior: 'cascade', + description: 'The user this preference applies to. Leave empty for the default that applies to all users.', }), topic: Field.text({ @@ -162,6 +191,17 @@ export const NotificationPreference = ObjectSchema.create({ // The already-ruled `sys_user_preference` is `system-data` too; the // ruling's phrase is ADMIN-AUTHORED CONTENT — the provenance of the // rows, not the management mode of the object. + // + // [ADR-0118 D1, ADR-0120 D4] `user_id` is EMPTY on every admin-global + // default row, and SQL UNIQUE treats NULLs as distinct, so as declared + // this key does not stop two global defaults for one (topic, channel) + // in one organization. The NULL-safe form, + // `(organization_id, COALESCE(user_id, ''), topic, channel)` under this + // same index name, is built at `kernel:ready` by `metadata-protocol`'s + // `ensureNotificationPreferenceIdentityIndex` on SQLite and PostgreSQL. + // This declaration stays the fallback shape for a driver that never + // runs that migration (MySQL refuses the key parts; driver-memory keys + // the listed columns). { fields: ['user_id', 'topic', 'channel'], unique: 'organization' }, { fields: ['topic'] }, ], diff --git a/packages/services/service-messaging/src/preference-resolver.test.ts b/packages/services/service-messaging/src/preference-resolver.test.ts index 451830ee34d..522ec525215 100644 --- a/packages/services/service-messaging/src/preference-resolver.test.ts +++ b/packages/services/service-messaging/src/preference-resolver.test.ts @@ -38,8 +38,12 @@ function fakeData(rows: any[] = [], opts: { throwOnFind?: boolean } = {}) { }; } -function pref(over: Partial<{ user_id: string; topic: string; channel: string; enabled: boolean; organization_id: string }>) { - return { user_id: '*', topic: '*', channel: '*', enabled: true, ...over }; +/** + * One preference row. `user_id` defaults to the EMPTY reference — the + * admin-global default (ADR-0118 D1), as the engine stores it (`null`). + */ +function pref(over: Partial<{ user_id: string | null; topic: string; channel: string; enabled: boolean; organization_id: string }>) { + return { user_id: null, topic: '*', channel: '*', enabled: true, ...over }; } function resolver(getData: () => any, mandatoryTopics: string[] = []) { @@ -82,9 +86,9 @@ describe('PreferenceResolver', () => { expect(out).toEqual([{ recipient: 'u2', channels: ['inbox', 'email'] }]); }); - it('lets a per-user row override the admin-global default', async () => { + it('lets a per-user row override the admin-global default (the empty user reference)', async () => { const rows = [ - pref({ user_id: '*', topic: 'task.assigned', channel: 'email', enabled: false }), // global: email off + pref({ user_id: null, topic: 'task.assigned', channel: 'email', enabled: false }), // global: email off pref({ user_id: 'u1', topic: 'task.assigned', channel: 'email', enabled: true }), // u1 opts back in ]; const r = resolver(() => fakeData(rows).engine); @@ -96,6 +100,35 @@ describe('PreferenceResolver', () => { ]); }); + it("reads every engine spelling of the empty reference as the admin-global default — null, absent and ''", async () => { + for (const global of [{ user_id: null }, { user_id: '' }, {}]) { + const row = { ...pref({ topic: 'task.assigned', channel: 'email', enabled: false }), ...global }; + if (!('user_id' in global)) delete (row as Record).user_id; + const r = resolver(() => fakeData([row]).engine); + const out = await r.filter(['u1'], ['inbox', 'email'], { topic: 'task.assigned' }); + expect(out, JSON.stringify(global)).toEqual([{ recipient: 'u1', channels: ['inbox'] }]); + } + }); + + it("a stored user_id '*' is NOT the admin-global default: the retired sentinel matches no recipient (ADR-0118 D1)", async () => { + // The text column's sentinel. The boot conversion rewrites such rows to + // the empty reference; a row still holding it is a row for a user id + // nobody has, so the resolver carries no tolerance for it. + const rows = [pref({ user_id: '*', topic: 'task.assigned', channel: 'email', enabled: false })]; + const r = resolver(() => fakeData(rows).engine); + const out = await r.filter(['u1', 'u2'], ['inbox', 'email'], { topic: 'task.assigned' }); + expect(out).toEqual([ + { recipient: 'u1', channels: ['inbox', 'email'] }, + { recipient: 'u2', channels: ['inbox', 'email'] }, + ]); + // Control: the SAME row with the empty reference mutes both. + const control = resolver(() => fakeData([pref({ topic: 'task.assigned', channel: 'email', enabled: false })]).engine); + expect(await control.filter(['u1', 'u2'], ['inbox', 'email'], { topic: 'task.assigned' })).toEqual([ + { recipient: 'u1', channels: ['inbox'] }, + { recipient: 'u2', channels: ['inbox'] }, + ]); + }); + it('prefers the most specific row (topic+channel beats topic-wildcard)', async () => { const rows = [ pref({ user_id: 'u1', topic: 'task.assigned', channel: '*', enabled: false }), // mute all channels… diff --git a/packages/services/service-messaging/src/preference-resolver.ts b/packages/services/service-messaging/src/preference-resolver.ts index fd64c86688e..32b9143fc21 100644 --- a/packages/services/service-messaging/src/preference-resolver.ts +++ b/packages/services/service-messaging/src/preference-resolver.ts @@ -66,16 +66,46 @@ export interface QuietHours { /** Digest cadence declared on a preference row (P3b-2). `none` ⇒ no batching. */ export type DigestCadence = 'daily' | 'weekly'; +/** The topic / channel wildcard. ⛔ Not a user: see {@link GLOBAL_USER}. */ const WILDCARD = '*'; +/** + * The index key of the admin-global default's USER part. + * + * [ADR-0118 D1] `user_id` is a `sys_user` reference, and the row that applies + * to every user is the one whose reference is EMPTY. This key is internal to + * the in-memory index below and is never compared with a stored value: a row + * reaches it only through {@link isGlobalDefaultUser}. It is the empty string + * because a recipient id never is one. + * + * ⛔ A stored `user_id = '*'` is NOT read as the global default here. That was + * the text column's sentinel; it is rewritten to the empty reference at boot + * (`convertWildcardPreferenceUsers`), and a row still holding it is a row for + * a user id nobody has, so it matches no recipient. + */ +const GLOBAL_USER = ''; + +/** + * Is this stored `user_id` the empty reference — the admin-global default? + * + * `null`, `undefined` and `''` are the three spellings the engine itself reads + * as "no link" (its reference check skips all three), so they are one value + * here too. The NULL-safe unique index folds them the same way + * (`COALESCE(user_id, '')`), so at most one such row exists per (topic, + * channel) in an organization. + */ +function isGlobalDefaultUser(userId: unknown): boolean { + return userId === null || userId === undefined || userId === ''; +} + /** * PreferenceResolver — the ADR-0030 Layer-3 preference filter (P2). * * Given the resolved recipients and the requested channels, returns, per * recipient, the channels they actually accept for `topic`. Resolution is * most-specific-wins over `sys_notification_preference` rows with `*` wildcards - * for user / topic / channel; a real-user row overrides the `user_id='*'` - * admin-global default; the built-in default is **on**. + * for topic / channel; a real-user row overrides the admin-global default (the + * row whose `user_id` is empty, ADR-0118 D1); the built-in default is **on**. * * Two safety rules: * - **Mandatory topics bypass** the matrix (all channels kept). @@ -128,8 +158,8 @@ export class PreferenceResolver { const recipientSet = new Set(recipients); const index = new Map(); for (const r of rows) { - const user = String(r.user_id ?? ''); - if (user !== WILDCARD && !recipientSet.has(user)) continue; // ignore unrelated users + const user = isGlobalDefaultUser(r.user_id) ? GLOBAL_USER : String(r.user_id); + if (user !== GLOBAL_USER && !recipientSet.has(user)) continue; // ignore unrelated users const topic = String(r.topic ?? WILDCARD); const channel = String(r.channel ?? WILDCARD); index.set(`${user}|${topic}|${channel}`, { @@ -190,10 +220,10 @@ export class PreferenceResolver { /** * Most-specific-wins lookup for (user, topic, channel). User-specific beats - * the `*` user; topic/channel specific beats their wildcards. + * the admin-global default; topic/channel specific beats their wildcards. */ private resolveRow(index: Map, user: string, topic: string, channel: string): PrefRowLite | undefined { - for (const u of [user, WILDCARD]) { + for (const u of [user, GLOBAL_USER]) { for (const t of [topic, WILDCARD]) { for (const c of [channel, WILDCARD]) { const hit = index.get(`${u}|${t}|${c}`); @@ -210,7 +240,7 @@ export class PreferenceResolver { * channel-agnostic setting. Most-specific user/topic wins. */ private resolveQuietHours(index: Map, user: string, topic: string): QuietHours | undefined { - for (const u of [user, WILDCARD]) { + for (const u of [user, GLOBAL_USER]) { for (const t of [topic, WILDCARD]) { const hit = index.get(`${u}|${t}|${WILDCARD}`); if (hit?.quietHours) return hit.quietHours; @@ -225,7 +255,7 @@ export class PreferenceResolver { * per-person, channel-agnostic setting. Most-specific user/topic wins. */ private resolveDigest(index: Map, user: string, topic: string): DigestCadence | undefined { - for (const u of [user, WILDCARD]) { + for (const u of [user, GLOBAL_USER]) { for (const t of [topic, WILDCARD]) { const hit = index.get(`${u}|${t}|${WILDCARD}`); if (hit?.digest) return hit.digest; diff --git a/packages/services/service-messaging/src/preference-user-conversion.ts b/packages/services/service-messaging/src/preference-user-conversion.ts new file mode 100644 index 00000000000..4c9ee36d722 --- /dev/null +++ b/packages/services/service-messaging/src/preference-user-conversion.ts @@ -0,0 +1,231 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `sys_notification_preference.user_id`: the stored sentinel `'*'` → the empty + * reference (ADR-0118 D1; maintainer ruling A on the card that retyped the + * column, Q2 B of the seat call that set its execution). + * + * ## What changed underneath these rows + * + * `user_id` was a text column holding a `sys_user` id, or the literal `'*'` + * for the admin-global default. It is now a `sys_user` reference, and the + * admin-global default is the row whose reference is EMPTY — ADR-0118 D1's + * representation of "no particular user" in a user column, with no sentinel. + * `PreferenceResolver` reads only that: a row still holding `'*'` is a row for a + * user id nobody has, so it matches no recipient. + * + * ## Why this runs at boot, unprompted + * + * Unlike the `'system'` sentinel `sys_metadata_history.recorded_by` once held + * (display only), a `'*'` row here changes BEHAVIOUR: it is an admin's mute or + * allow for every user. Left unconverted it would stop applying the moment the + * new resolver loads, while everything keeps looking healthy, so the rewrite + * cannot wait for an operator to remember a command. It is tiny (admin-authored + * rows; no code path ever wrote `'*'`), so it runs on every boot at + * `kernel:ready` and costs one indexed read when there is nothing to do. + * + * ## Idempotency + * + * The read selects only rows still holding `'*'`, and each write sets the + * constant empty reference on one row addressed by id. A second run finds + * nothing and writes nothing. + * + * ## Refuse, never keep-one (ADR-0120 D4) + * + * The NULL-safe unique index (`metadata-protocol`'s + * `ensureNotificationPreferenceIdentityIndex`) admits ONE global default per + * (organization, topic, channel). A `'*'` row cannot collide with another + * `'*'` row (the previous index already keyed them), but it can collide with an + * empty-reference row for the same (topic, channel) written since. That pair is + * two admin decisions for one key, so nothing is discarded: the `'*'` row is + * left exactly as stored, and the operator is told which row and how to settle + * it. + */ + +import type { IDataEngine } from '@objectstack/spec/contracts'; +import { isMissingTableError, isUniqueViolationError } from '@objectstack/types'; +import { PREFERENCE_OBJECT } from './preference-resolver.js'; + +/** The sentinel the text column held for the admin-global default. Read here and nowhere else. */ +export const LEGACY_WILDCARD_USER = '*'; + +/** The engine's own code for a refused duplicate (ADR-0112) — branched on before any driver shape. */ +const DUPLICATE_RECORD_CODE = 'DUPLICATE_RECORD'; + +/** + * The table this conversion rewrites, spelled at its call sites so the tenant + * audit census can place them (an imported name reads as a runtime value). The + * type ties it to `PREFERENCE_OBJECT`: a different literal does not compile. + */ +const OBJECT: typeof PREFERENCE_OBJECT = 'sys_notification_preference'; + +/** How many rows one pass reads — the same ceiling `PreferenceResolver.loadRows` reads with. */ +const PASS_LIMIT = 10000; + +export interface PreferenceUserConversionLogger { + info(message: string, meta?: Record): void; + error(message: string, error?: Error, meta?: Record): void; +} + +/** One `'*'` row this run left as stored, and why. */ +export interface PreferenceUserConversionMiss { + id: string; + organizationId: string | null; + topic: string | null; + channel: string | null; + /** `duplicate`: an empty-reference row already holds the key. `failed`: the write was refused otherwise. */ + reason: 'duplicate' | 'failed'; + detail: string; +} + +export interface PreferenceUserConversionResult { + /** + * - `converted` — at least one row was rewritten (misses may sit beside it); + * - `nothing-to-convert` — the read ran and found no `'*'` row; + * - `absent` — the table does not exist on this database yet, so no row can hold `'*'`; + * - `no-engine` — no data engine that reads and writes rows; + * - `failed` — the read failed, or every row found was left as stored. + */ + status: 'converted' | 'nothing-to-convert' | 'absent' | 'no-engine' | 'failed'; + converted: number; + missed: PreferenceUserConversionMiss[]; + /** The read's own refusal, when the read failed. */ + detail?: string; +} + +/** + * The text an engine refusal reports, for the boot log and the result. + * + * Every read and write here goes through the data ENGINE, never the raw-SQL + * seam, so the error in hand is the engine's own (a validation finding, a + * `DUPLICATE_RECORD`, a driver error the SQL driver already redacts of its + * statement and bound values). That is why this is the package's plain + * `message` reading and not `operatorFacingErrorText`, the helper that digs a + * dialect's words out of the raw seam's composed fault: there is no such fault + * on this path, and each of that helper's callers is enumerated and pinned + * against raw statements this module never issues. + */ +function messageOf(err: unknown): string { + return err instanceof Error ? err.message : String(err); +} + +function textOrNull(value: unknown): string | null { + return value === null || value === undefined ? null : String(value); +} + +/** + * Rewrite every stored `user_id = '*'` to the empty reference, across every + * organization. Never throws: a boot must not fail on this, and each outcome is + * returned and, where it leaves a default not applying, logged at `error`. + */ +export async function convertWildcardPreferenceUsers( + engine: IDataEngine | undefined, + logger?: PreferenceUserConversionLogger, +): Promise { + // An engine that cannot read and write rows has no rows to convert either: + // the same probe `provisionSystemTables` makes of `syncObjectSchema`. + if (!engine || typeof engine.find !== 'function' || typeof engine.update !== 'function') { + return { status: 'no-engine', converted: 0, missed: [] }; + } + const object = OBJECT; + + let converted = 0; + const missed: PreferenceUserConversionMiss[] = []; + const seen = new Set(); + + // Pass until a read returns no row this run has not already handled. A row + // left as stored still matches the filter, so `seen` is what ends the loop. + for (;;) { + let rows: Array>; + try { + rows = (await engine.find( + OBJECT, + { + where: { user_id: LEGACY_WILDCARD_USER }, + fields: ['id', 'organization_id', 'topic', 'channel'], + limit: PASS_LIMIT, + }, + // Every organization's rows: the read is the platform's own, not a tenant's. + { context: { isSystem: true } }, + )) ?? []; + } catch (err) { + // No table yet holds no '*' row — a fresh database, or a boot whose + // schema sync is skipped. Anything else is "could not look". + if (isMissingTableError(err, object)) return { status: 'absent', converted, missed }; + const detail = messageOf(err); + logger?.error( + `[messaging] could not read ${object} to rewrite user_id '${LEGACY_WILDCARD_USER}' to the empty ` + + 'reference, so any admin-global notification default still stored that way is NOT applying: ' + + 'notifications an admin muted for every user are being delivered while everything looks ' + + 'healthy. Fix the cause below and restart; the rewrite runs on every boot.', + undefined, + { object, error: detail }, + ); + return { status: 'failed', converted, missed, detail }; + } + + const fresh = rows.filter((row) => !seen.has(String(row.id))); + if (fresh.length === 0) break; + + for (const row of fresh) { + const id = String(row.id); + seen.add(id); + const organizationId = textOrNull(row.organization_id); + try { + // ADR-0118 D3: a migration is an entry point, so it states the + // system context itself. The row's own organization is threaded + // on the write, so it is tenant-scoped like any other write to + // this object and needs no audit bypass. + await engine.update( + OBJECT, + { user_id: null }, + { where: { id }, context: { isSystem: true, tenantId: organizationId ?? undefined } }, + ); + converted += 1; + } catch (err) { + const duplicate = (err as { code?: unknown } | null)?.code === DUPLICATE_RECORD_CODE || isUniqueViolationError(err); + missed.push({ + id, + organizationId, + topic: textOrNull(row.topic), + channel: textOrNull(row.channel), + reason: duplicate ? 'duplicate' : 'failed', + detail: messageOf(err), + }); + } + } + } + + if (converted > 0) { + logger?.info( + `[messaging] rewrote ${converted} ${object} row(s) from user_id '${LEGACY_WILDCARD_USER}' to the empty ` + + 'reference, the admin-global default for every user (an empty User).', + { object, converted }, + ); + } + + if (missed.length > 0) { + // Once per boot, naming every row: from the outside nothing looks wrong + // while each of these admin defaults has stopped applying. + const duplicates = missed.filter((m) => m.reason === 'duplicate'); + logger?.error( + `[messaging] ${missed.length} ${object} row(s) still hold user_id '${LEGACY_WILDCARD_USER}' and are NOT ` + + 'applying as the admin-global default: each row is kept as stored, and its mute or allow for every ' + + 'user is ignored while everything looks healthy. ' + + (duplicates.length > 0 + ? `${duplicates.length} of them share an organization, topic and channel with a default that ` + + 'already has an empty User, and only one default per key is allowed: delete the one you do not ' + + 'want, then restart. ' + : '') + + 'Rows: ' + + missed.map((m) => `${m.id} (${m.topic ?? ''} · ${m.channel ?? ''}: ${m.reason})`).join(', ') + + '.', + undefined, + { object, missed }, + ); + } + + if (converted > 0) return { status: 'converted', converted, missed }; + if (missed.length > 0) return { status: 'failed', converted, missed }; + return { status: 'nothing-to-convert', converted, missed }; +} diff --git a/packages/services/service-messaging/src/preference-user-reference.integration.test.ts b/packages/services/service-messaging/src/preference-user-reference.integration.test.ts new file mode 100644 index 00000000000..d9d4a70042a --- /dev/null +++ b/packages/services/service-messaging/src/preference-user-reference.integration.test.ts @@ -0,0 +1,316 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `sys_notification_preference.user_id` is a `sys_user` reference, and the + * admin-global default is the EMPTY reference (ADR-0118 D1: no sentinel in a + * user column). Maintainer ruling A, with the seat call's execution parameters + * (the boot conversion, the runtime NULL-safe index, `cascade`). + * + * Pinned on a real `ObjectQL` over `SqlDriver` (better-sqlite3) with the real + * `sys_user`, and, for the index, the real `kernel:ready` arming in + * `metadata-protocol`'s `assembleMetadataProtocol`: + * + * 1. the write door refuses `'*'` and any id no user holds — `400 + * VALIDATION_FAILED`, `reference_not_found` on `user_id` — where the text + * column stored both to match nobody; a real user and the empty reference + * are accepted; + * 2. the record title names the person (email where the name is blank), the + * empty reference reads 'All users', and an id that resolves to nobody + * leaves the title null rather than reading as 'All users'; + * 3. deleting a user deletes their rows (`cascade`): the lookup default + * `set_null` would turn a person's own mute into the mute for everyone; + * 4. the boot conversion rewrites every stored `'*'` to the empty reference + * across organizations, twice over is a no-op, and a `'*'` row whose key + * already has a global default is kept and reported, never discarded; + * 5. on the SQL driver, once the platform migrations run, a second global + * default for one (organization, topic, channel) is refused `409`; before + * them the declared index admits it (the control). driver-memory keys the + * listed columns and stays NULL-distinct, as for `sys_setting`. + * 6. the resolver applies a converted default, and ignores an unconverted one. + */ + +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { SysUser } from '@objectstack/platform-objects'; +import { assembleMetadataProtocol } from '@objectstack/metadata-protocol'; +import { mapDataError } from '@objectstack/types'; +import type { ExecutionContext } from '@objectstack/spec/kernel'; +import { NotificationPreference } from './objects/notification-preference.object.js'; +import { PreferenceResolver, PREFERENCE_OBJECT } from './preference-resolver.js'; +import { convertWildcardPreferenceUsers, LEGACY_WILDCARD_USER } from './preference-user-conversion.js'; +import { DEFAULT_ORGANIZATION_ID, installDefaultOrganization } from './default-organization.testkit.js'; + +const OTHER_ORG = 'org_yi'; +/** + * A system write names its organization: with two organizations on the + * install, an organization-less system write is refused (ADR-0131 D9). + */ +const SYS = { context: { isSystem: true, tenantId: DEFAULT_ORGANIZATION_ID } } as any; +/** A system read across every organization. */ +const SYS_ALL = { context: { isSystem: true } } as any; +/** An admin at the Setup grid: a principal, not the system. */ +const ADMIN = { context: { userId: 'usr_admin', tenantId: DEFAULT_ORGANIZATION_ID } as ExecutionContext }; + +const silent: any = { debug() {}, info() {}, warn() {}, error() {}, fatal() {}, child() { return silent; } }; + +let engine: ObjectQL | undefined; + +afterEach(async () => { + try { await engine?.destroy(); } catch { /* noop */ } + engine = undefined; +}); + +async function boot(): Promise<{ ql: ObjectQL; driver: SqlDriver }> { + const driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }); + const ql = new ObjectQL({ logger: silent }); + ql.registerDriver(driver, true); + await ql.init(); + ql.registry.registerObject(SysUser as any, '@objectstack/service-messaging'); + ql.registry.registerObject(NotificationPreference as any, '@objectstack/service-messaging'); + await ql.syncSchemas(); + await installDefaultOrganization(ql as any); + await driver.create('sys_organization', { id: OTHER_ORG, name: 'Yi' }); + engine = ql; + return { ql, driver }; +} + +async function user(ql: ObjectQL, name: string, email: string): Promise { + return String((await ql.insert('sys_user', { name, email }, SYS)).id); +} + +/** The thrown write, and the status the REST data door answers it with. */ +async function refusal(write: Promise): Promise<{ error: any; status: number }> { + try { + await write; + } catch (error) { + return { error, status: mapDataError(error).status }; + } + throw new Error('the write was accepted; a refusal was expected'); +} + +async function storedRows(ql: ObjectQL): Promise>> { + const rows = await ql.find(PREFERENCE_OBJECT, { fields: ['id', 'organization_id', 'user_id', 'topic', 'channel', 'enabled'] }, SYS_ALL); + return (rows ?? []) + .map((r: any) => ({ id: r.id, organization_id: r.organization_id, user_id: r.user_id ?? null, topic: r.topic, channel: r.channel, enabled: r.enabled })) + .sort((a: any, b: any) => String(a.id).localeCompare(String(b.id))); +} + +/** A row as the text column left it, written straight through the driver. */ +async function storedRow(driver: SqlDriver, id: string, over: Record): Promise { + await driver.create(PREFERENCE_OBJECT, { + id, organization_id: DEFAULT_ORGANIZATION_ID, topic: 'deal.won', channel: 'email', enabled: false, ...over, + }); +} + +describe('the declaration', () => { + it('user_id is an optional sys_user lookup that cascades, with no width of its own', () => { + const field = (NotificationPreference as any).fields.user_id; + expect(field.type).toBe('lookup'); + expect(field.reference).toBe('sys_user'); + expect(field.required).toBe(false); + expect(field.deleteBehavior).toBe('cascade'); + expect(field.maxLength).toBeUndefined(); + }); +}); + +describe('the write door refuses an id no user holds', () => { + it("'*' and a mistyped id are refused 400 VALIDATION_FAILED, reference_not_found on user_id", async () => { + const { ql } = await boot(); + for (const value of [LEGACY_WILDCARD_USER, 'usr_nobody']) { + const { error, status } = await refusal(ql.insert(PREFERENCE_OBJECT, { user_id: value, topic: 'deal.won', channel: 'email' }, ADMIN)); + expect(error.code, value).toBe('VALIDATION_FAILED'); + expect(status, value).toBe(400); + expect(error.fields, value).toEqual(expect.arrayContaining([expect.objectContaining({ field: 'user_id', code: 'reference_not_found' })])); + } + expect(await storedRows(ql)).toEqual([]); + }); + + it("an update moving a row to '*' is refused the same way, and the row is left as stored", async () => { + const { ql } = await boot(); + const ann = await user(ql, 'Ann', 'ann@example.com'); + const row = await ql.insert(PREFERENCE_OBJECT, { user_id: ann, topic: 'deal.won', channel: 'email' }, ADMIN); + const { error, status } = await refusal(ql.update(PREFERENCE_OBJECT, { user_id: LEGACY_WILDCARD_USER }, { where: { id: row.id }, context: ADMIN.context })); + expect(error.code).toBe('VALIDATION_FAILED'); + expect(status).toBe(400); + expect((await storedRows(ql))[0]!.user_id).toBe(ann); + }); + + it('a real user and the empty reference are accepted', async () => { + const { ql } = await boot(); + const ann = await user(ql, 'Ann', 'ann@example.com'); + await ql.insert(PREFERENCE_OBJECT, { user_id: ann, topic: 'deal.won', channel: 'email' }, ADMIN); + await ql.insert(PREFERENCE_OBJECT, { topic: 'deal.won', channel: 'email' }, ADMIN); + expect((await storedRows(ql)).map((r) => r.user_id).sort()).toEqual([ann, null].sort()); + }); +}); + +describe('the record title names the person', () => { + it('name; email where the name is blank; All users for the empty reference; null for an id nobody holds', async () => { + const { ql, driver } = await boot(); + const bea = await user(ql, 'Bea Person', 'bea@example.com'); + const blank = await user(ql, 'Placeholder', 'no-name@example.com'); + await driver.getKnex()('sys_user').where({ id: blank }).update({ name: ' ' }); + + const title = async (id: string) => (await ql.findOne(PREFERENCE_OBJECT, { where: { id } }, SYS))!.display_title; + const person = await ql.insert(PREFERENCE_OBJECT, { user_id: bea, topic: 'deal.won', channel: 'inbox' }, SYS); + const nameless = await ql.insert(PREFERENCE_OBJECT, { user_id: blank, topic: 'deal.won', channel: 'inbox' }, SYS); + const everyone = await ql.insert(PREFERENCE_OBJECT, { topic: 'deal.won', channel: 'inbox' }, SYS); + const defaulted = await ql.insert(PREFERENCE_OBJECT, { channel: 'email' }, SYS); + // The system may write past the reference check; an id that resolves to + // nobody must not be titled as the global default. + const gone = await ql.insert(PREFERENCE_OBJECT, { user_id: 'usr_gone', topic: 'deal.lost', channel: 'inbox' }, SYS); + + expect(person.display_title).toBe('Bea Person · deal.won · inbox'); + expect(await title(person.id)).toBe('Bea Person · deal.won · inbox'); + expect(await title(person.id)).not.toContain(bea); + expect(await title(nameless.id)).toBe('no-name@example.com · deal.won · inbox'); + expect(await title(everyone.id)).toBe('All users · deal.won · inbox'); + expect(await title(defaulted.id)).toBe('All users · * · email'); + expect(await title(gone.id)).toBeNull(); + }); +}); + +describe('deleting a user deletes their preferences (cascade)', () => { + it("the person's own mute goes with them and never becomes the default for everyone", async () => { + const { ql } = await boot(); + const eve = await user(ql, 'Eve', 'eve@example.com'); + await ql.insert(PREFERENCE_OBJECT, { user_id: eve, topic: 'deal.won', channel: 'sms', enabled: false }, SYS); + await ql.insert(PREFERENCE_OBJECT, { topic: 'deal.lost', channel: 'sms', enabled: true }, SYS); + + await ql.delete('sys_user', { where: { id: eve }, context: { isSystem: true } }); + + const rows = await storedRows(ql); + expect(rows.map((r) => [r.user_id, r.topic])).toEqual([[null, 'deal.lost']]); + // No global default for the deleted person's topic was created. + expect(rows.filter((r) => r.topic === 'deal.won')).toEqual([]); + }); +}); + +describe("the boot conversion: stored '*' → the empty reference", () => { + it('rewrites every organization\'s rows, leaves real users alone, and a second run changes nothing', async () => { + const { ql, driver } = await boot(); + const ann = await user(ql, 'Ann', 'ann@example.com'); + await storedRow(driver, 'p_jia', { user_id: LEGACY_WILDCARD_USER }); + await storedRow(driver, 'p_yi', { user_id: LEGACY_WILDCARD_USER, organization_id: OTHER_ORG }); + await storedRow(driver, 'p_ann', { user_id: ann }); + const logger = { info: vi.fn(), error: vi.fn() }; + + const first = await convertWildcardPreferenceUsers(ql as any, logger); + expect(first).toEqual({ status: 'converted', converted: 2, missed: [] }); + const after = await storedRows(ql); + expect(after.map((r) => [r.id, r.organization_id, r.user_id])).toEqual([ + ['p_ann', DEFAULT_ORGANIZATION_ID, ann], + ['p_jia', DEFAULT_ORGANIZATION_ID, null], + ['p_yi', OTHER_ORG, null], + ]); + expect(logger.info).toHaveBeenCalledTimes(1); + expect(logger.error).not.toHaveBeenCalled(); + + const second = await convertWildcardPreferenceUsers(ql as any, logger); + expect(second).toEqual({ status: 'nothing-to-convert', converted: 0, missed: [] }); + expect(await storedRows(ql)).toEqual(after); + expect(logger.info).toHaveBeenCalledTimes(1); + }); + + it('a database with no such table yet is "absent", silently', async () => { + const driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true }); + const ql = new ObjectQL({ logger: silent }); + ql.registerDriver(driver, true); + await ql.init(); + ql.registry.registerObject(NotificationPreference as any, '@objectstack/service-messaging'); + engine = ql; + const logger = { info: vi.fn(), error: vi.fn() }; + expect(await convertWildcardPreferenceUsers(ql as any, logger)).toEqual({ status: 'absent', converted: 0, missed: [] }); + expect(logger.error).not.toHaveBeenCalled(); + }); + + it("no engine, or one that cannot read and write rows: 'no-engine'", async () => { + expect((await convertWildcardPreferenceUsers(undefined)).status).toBe('no-engine'); + expect((await convertWildcardPreferenceUsers({ syncObjectSchema: async () => {} } as any)).status).toBe('no-engine'); + }); +}); + +/** Run the real `kernel:ready` platform migrations `metadata-protocol` arms. */ +async function runPlatformMigrations(ql: ObjectQL): Promise { + const hooks: Array<() => unknown> = []; + const ctx = { + logger: silent, + registerService: () => {}, + getService: () => { throw new Error('not registered'); }, + getServices: () => new Map(), + hook: (name: string, handler: () => unknown) => { + if (name === 'kernel:ready') hooks.push(handler); + }, + } as any; + assembleMetadataProtocol(ctx, ql as any, undefined, { runPlatformMigrations: true }); + for (const handler of hooks) await handler(); +} + +describe('one admin-global default per (organization, topic, channel) on the SQL driver', () => { + it('CONTROL — before the platform migrations, the declared index admits a second global default', async () => { + const { ql } = await boot(); + await ql.insert(PREFERENCE_OBJECT, { topic: 'deal.won', channel: 'email', enabled: false }, SYS); + await ql.insert(PREFERENCE_OBJECT, { topic: 'deal.won', channel: 'email', enabled: true }, SYS); + expect((await storedRows(ql)).filter((r) => r.user_id === null)).toHaveLength(2); + }); + + it('after them, a second global default is refused 409, in the same organization only', async () => { + const { ql, driver } = await boot(); + await runPlatformMigrations(ql); + const ddl = await driver.getKnex().raw( + "SELECT sql FROM sqlite_master WHERE type='index' AND name='uniq_sys_notification_preference_a22d7d27'", + ); + expect(String(ddl[0]?.sql)).toContain("COALESCE(user_id, '')"); + + await ql.insert(PREFERENCE_OBJECT, { topic: 'deal.won', channel: 'email', enabled: false }, SYS); + const { error, status } = await refusal(ql.insert(PREFERENCE_OBJECT, { topic: 'deal.won', channel: 'email', enabled: true }, SYS)); + expect(error.code).toBe('DUPLICATE_RECORD'); + expect(status).toBe(409); + // Anti-vacuity: the key is per organization, topic and channel, and a + // person's own row is a different key. + const ann = await user(ql, 'Ann', 'ann@example.com'); + await ql.insert(PREFERENCE_OBJECT, { topic: 'deal.won', channel: 'inbox' }, SYS); + await ql.insert(PREFERENCE_OBJECT, { user_id: ann, topic: 'deal.won', channel: 'email' }, SYS); + await driver.create(PREFERENCE_OBJECT, { id: 'p_yi', organization_id: OTHER_ORG, topic: 'deal.won', channel: 'email', enabled: false }); + expect(await storedRows(ql)).toHaveLength(4); + }); + + it("a '*' row whose key already has a global default is kept as stored and reported once at error — never discarded", async () => { + const { ql, driver } = await boot(); + await runPlatformMigrations(ql); + await ql.insert(PREFERENCE_OBJECT, { topic: 'deal.won', channel: 'email', enabled: true }, SYS); + await storedRow(driver, 'p_star', { user_id: LEGACY_WILDCARD_USER }); + const logger = { info: vi.fn(), error: vi.fn() }; + + const result = await convertWildcardPreferenceUsers(ql as any, logger); + + expect(result.status).toBe('failed'); + expect(result.converted).toBe(0); + expect(result.missed).toEqual([expect.objectContaining({ id: 'p_star', reason: 'duplicate', topic: 'deal.won', channel: 'email' })]); + expect((await storedRows(ql)).find((r) => r.id === 'p_star')!.user_id).toBe(LEGACY_WILDCARD_USER); + expect(logger.error).toHaveBeenCalledTimes(1); + expect(String(logger.error.mock.calls[0]![0])).toContain('p_star'); + }); +}); + +describe('the resolver applies a converted default, and ignores an unconverted one', () => { + it("a stored '*' mute stops nobody until the conversion runs; afterwards it mutes every user", async () => { + const { ql, driver } = await boot(); + const resolver = new PreferenceResolver({ getData: () => ql as any, logger: silent }); + await storedRow(driver, 'p_star', { user_id: LEGACY_WILDCARD_USER, topic: 'deal.won', channel: 'email', enabled: false }); + const ctx = { topic: 'deal.won', organizationId: DEFAULT_ORGANIZATION_ID }; + + expect(await resolver.filter(['u1', 'u2'], ['inbox', 'email'], ctx)).toEqual([ + { recipient: 'u1', channels: ['inbox', 'email'] }, + { recipient: 'u2', channels: ['inbox', 'email'] }, + ]); + + await convertWildcardPreferenceUsers(ql as any); + + expect(await resolver.filter(['u1', 'u2'], ['inbox', 'email'], ctx)).toEqual([ + { recipient: 'u1', channels: ['inbox'] }, + { recipient: 'u2', channels: ['inbox'] }, + ]); + }); +}); diff --git a/packages/services/service-messaging/src/translations/en.objects.generated.ts b/packages/services/service-messaging/src/translations/en.objects.generated.ts index 3d6271d0804..d91c4f0eb92 100644 --- a/packages/services/service-messaging/src/translations/en.objects.generated.ts +++ b/packages/services/service-messaging/src/translations/en.objects.generated.ts @@ -197,11 +197,11 @@ export const enObjects: NonNullable = { }, display_title: { label: "Title", - help: "Record title: the user, the topic and the channel this toggle covers (computed on read)" + help: "Record title: the person (or All users), the topic and the channel this toggle covers (computed on read)" }, user_id: { label: "User", - help: "Recipient user id, or '*' for the admin-global default." + help: "The user this preference applies to. Leave empty for the default that applies to all users." }, topic: { label: "Topic", diff --git a/packages/services/service-messaging/src/translations/es-ES.objects.generated.ts b/packages/services/service-messaging/src/translations/es-ES.objects.generated.ts index af986047583..b865504343f 100644 --- a/packages/services/service-messaging/src/translations/es-ES.objects.generated.ts +++ b/packages/services/service-messaging/src/translations/es-ES.objects.generated.ts @@ -197,11 +197,11 @@ export const esESObjects: NonNullable = { }, display_title: { label: "Title", - help: "Record title: the user, the topic and the channel this toggle covers (computed on read)" + help: "Título del registro: la persona (o All users), el tema y el canal que cubre este ajuste (calculado al leer)" }, user_id: { label: "User", - help: "Recipient user id, or '*' for the admin-global default." + help: "El usuario al que se aplica esta preferencia. Déjelo vacío para el valor predeterminado que se aplica a todos los usuarios." }, topic: { label: "Topic", diff --git a/packages/services/service-messaging/src/translations/es-ES.source-hashes.generated.ts b/packages/services/service-messaging/src/translations/es-ES.source-hashes.generated.ts index 629e98df8d6..1704012a5ad 100644 --- a/packages/services/service-messaging/src/translations/es-ES.source-hashes.generated.ts +++ b/packages/services/service-messaging/src/translations/es-ES.source-hashes.generated.ts @@ -100,7 +100,6 @@ export const esESGeneratedSourceHashes: Readonly> = { "objects.sys_notification_preference.fields.digest.options.daily": "1dd810d2b5a02102", "objects.sys_notification_preference.fields.digest.options.none": "f0ac498bc08c6383", "objects.sys_notification_preference.fields.digest.options.weekly": "18ffbbec919e0d35", - "objects.sys_notification_preference.fields.display_title.help": "13aff17d09067e30", "objects.sys_notification_preference.fields.display_title.label": "70f7aadecce647a5", "objects.sys_notification_preference.fields.enabled.help": "94e2727fd8c18296", "objects.sys_notification_preference.fields.enabled.label": "41fb04f20e64dc4f", @@ -110,7 +109,6 @@ export const esESGeneratedSourceHashes: Readonly> = { "objects.sys_notification_preference.fields.topic.help": "cf04f6b5ad2a5a33", "objects.sys_notification_preference.fields.topic.label": "819afdb3853e9d80", "objects.sys_notification_preference.fields.updated_at.label": "aba63dc2a9c79b8d", - "objects.sys_notification_preference.fields.user_id.help": "0ecc5b30fb3ec13c", "objects.sys_notification_preference.fields.user_id.label": "6c0d22a931e235bd", "objects.sys_notification_preference.label": "614ba2a098ea782c", "objects.sys_notification_preference.pluralLabel": "8869f324c1bd545a", diff --git a/packages/services/service-messaging/src/translations/ja-JP.objects.generated.ts b/packages/services/service-messaging/src/translations/ja-JP.objects.generated.ts index 8562e844dee..d9a3aad68d6 100644 --- a/packages/services/service-messaging/src/translations/ja-JP.objects.generated.ts +++ b/packages/services/service-messaging/src/translations/ja-JP.objects.generated.ts @@ -197,11 +197,11 @@ export const jaJPObjects: NonNullable = { }, display_title: { label: "Title", - help: "Record title: the user, the topic and the channel this toggle covers (computed on read)" + help: "レコードのタイトル:この設定の対象ユーザー(または All users)、トピック、チャネル(読み取り時に計算)" }, user_id: { label: "User", - help: "Recipient user id, or '*' for the admin-global default." + help: "この設定を適用するユーザー。空のままにすると、すべてのユーザーに適用される既定値になります。" }, topic: { label: "Topic", diff --git a/packages/services/service-messaging/src/translations/ja-JP.source-hashes.generated.ts b/packages/services/service-messaging/src/translations/ja-JP.source-hashes.generated.ts index 0b142a8718f..9a7fe7b6a56 100644 --- a/packages/services/service-messaging/src/translations/ja-JP.source-hashes.generated.ts +++ b/packages/services/service-messaging/src/translations/ja-JP.source-hashes.generated.ts @@ -99,7 +99,6 @@ export const jaJPGeneratedSourceHashes: Readonly> = { "objects.sys_notification_preference.fields.digest.options.daily": "1dd810d2b5a02102", "objects.sys_notification_preference.fields.digest.options.none": "f0ac498bc08c6383", "objects.sys_notification_preference.fields.digest.options.weekly": "18ffbbec919e0d35", - "objects.sys_notification_preference.fields.display_title.help": "13aff17d09067e30", "objects.sys_notification_preference.fields.display_title.label": "70f7aadecce647a5", "objects.sys_notification_preference.fields.enabled.help": "94e2727fd8c18296", "objects.sys_notification_preference.fields.enabled.label": "41fb04f20e64dc4f", @@ -109,7 +108,6 @@ export const jaJPGeneratedSourceHashes: Readonly> = { "objects.sys_notification_preference.fields.topic.help": "cf04f6b5ad2a5a33", "objects.sys_notification_preference.fields.topic.label": "819afdb3853e9d80", "objects.sys_notification_preference.fields.updated_at.label": "aba63dc2a9c79b8d", - "objects.sys_notification_preference.fields.user_id.help": "0ecc5b30fb3ec13c", "objects.sys_notification_preference.fields.user_id.label": "6c0d22a931e235bd", "objects.sys_notification_preference.label": "614ba2a098ea782c", "objects.sys_notification_preference.pluralLabel": "8869f324c1bd545a", diff --git a/packages/services/service-messaging/src/translations/zh-CN.objects.generated.ts b/packages/services/service-messaging/src/translations/zh-CN.objects.generated.ts index eba14770a33..05f01b96d1b 100644 --- a/packages/services/service-messaging/src/translations/zh-CN.objects.generated.ts +++ b/packages/services/service-messaging/src/translations/zh-CN.objects.generated.ts @@ -197,11 +197,11 @@ export const zhCNObjects: NonNullable = { }, display_title: { label: "Title", - help: "Record title: the user, the topic and the channel this toggle covers (computed on read)" + help: "记录标题:此开关适用的人员(或 All users)、主题和通道(读取时计算)" }, user_id: { label: "用户", - help: "接收用户 ID,'*' 表示管理员全局默认值。" + help: "此偏好适用的用户。留空即为适用于所有用户的默认值。" }, topic: { label: "主题", diff --git a/packages/services/service-messaging/src/translations/zh-CN.source-hashes.generated.ts b/packages/services/service-messaging/src/translations/zh-CN.source-hashes.generated.ts index cf80d98e3d5..74a44ab7254 100644 --- a/packages/services/service-messaging/src/translations/zh-CN.source-hashes.generated.ts +++ b/packages/services/service-messaging/src/translations/zh-CN.source-hashes.generated.ts @@ -23,6 +23,5 @@ export const zhCNGeneratedSourceHashes: Readonly> = { "objects.sys_http_delivery.fields.status.help": "e437ecf81dfb4715", "objects.sys_notification_delivery.fields.display_title.help": "0803d3f90a14907c", "objects.sys_notification_delivery.fields.display_title.label": "70f7aadecce647a5", - "objects.sys_notification_preference.fields.display_title.help": "13aff17d09067e30", "objects.sys_notification_preference.fields.display_title.label": "70f7aadecce647a5", }; diff --git a/packages/spec/scripts/liveness/empty-state-registry.mts b/packages/spec/scripts/liveness/empty-state-registry.mts index 2ee12df958e..07cc5b5b9b7 100644 --- a/packages/spec/scripts/liveness/empty-state-registry.mts +++ b/packages/spec/scripts/liveness/empty-state-registry.mts @@ -209,6 +209,14 @@ export const EMPTY_STATE_REGISTRY: EmptyStateEntry[] = [ rationale: 'Which files under a storage prefix get indexed into a knowledge source. Ingestion scope, not read authorization — retrieval is gated separately. NOTE the shape though: it is a vacuous-allow-list ("allow-list" whose empty value admits everything), the same shape as #3896. `storage.zod.ts` models the identical concept the better way — an explicit `mode: "whitelist"` discriminator plus `.min(1)`, so an empty whitelist cannot be expressed at all. Prefer that form for new allow-lists; changing this one is a breaking change to an authorable field and is not in scope here.', }, + { + file: 'packages/services/service-messaging/src/objects/notification-preference.object.ts', + property: 'user_id', + semantics: 'scope', + rationale: + "Which users a notification delivery toggle covers. An empty User makes the row the admin-wide default for every user of the organization — ADR-0118 D1's empty reference in a user column rather than a sentinel, as maintainer ruling A decided (ruling record 6304380498). It selects the range a mute or allow applies to and grants no data access: a user's own row overrides it, and mandatory topics bypass the matrix entirely. The one way the empty state could widen by accident — the lookup default `set_null` turning a deleted person's own row into the default for everyone — is closed by `deleteBehavior: 'cascade'` on the field.", + evidence: 'packages/services/service-messaging/src/preference-resolver.ts (resolveRow — the user-specific row is consulted before the empty-User default)', + }, // ---- Engine output, not authorable -------------------------------------