From a00ce2db0cbad1517615ae5b085b048bc0d33e87 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 02:59:13 +0000 Subject: [PATCH 01/11] wip(service-messaging,metadata-protocol): preference user_id as a sys_user reference, NULL-safe index, boot conversion Claude-Session: https://claude.ai/code/session_01UnHcTADjeg3VwxF21QHj7W Co-authored-by: Claude --- .../migrate/duplicates.contract.test.ts | 7 +- .../migrate/duplicates.integration.test.ts | 4 +- ...fication-preference-identity-index.test.ts | 274 ++++++++++++++++ .../notification-preference-identity-index.ts | 309 ++++++++++++++++++ .../runtime-index-preflight.test.ts | 33 ++ .../src/migrations/runtime-index-preflight.ts | 26 +- packages/metadata-protocol/src/plugin.ts | 22 ++ .../src/messaging-service-plugin.ts | 15 + .../objects/notification-preference.object.ts | 70 +++- .../src/preference-resolver.ts | 46 ++- .../src/preference-user-conversion.ts | 212 ++++++++++++ 11 files changed, 988 insertions(+), 30 deletions(-) create mode 100644 packages/metadata-protocol/src/migrations/notification-preference-identity-index.test.ts create mode 100644 packages/metadata-protocol/src/migrations/notification-preference-identity-index.ts create mode 100644 packages/services/service-messaging/src/preference-user-conversion.ts 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/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..e3f7005e152 --- /dev/null +++ b/packages/metadata-protocol/src/migrations/notification-preference-identity-index.test.ts @@ -0,0 +1,274 @@ +// 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.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/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-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.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..f75ad35fe21 --- /dev/null +++ b/packages/services/service-messaging/src/preference-user-conversion.ts @@ -0,0 +1,212 @@ +// 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, operatorFacingErrorText } 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'; + +/** 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 system context this conversion runs under (ADR-0118 D3: a migration is an entry point). */ +function systemContext(organizationId: string | null): Record { + // The row's own organization is threaded on the write, so it is tenant-scoped + // like any other write to this object — no audit bypass is needed for it. + return organizationId ? { isSystem: true, tenantId: organizationId } : { isSystem: true }; +} + +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, + opts: { objectName?: string } = {}, +): 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 = opts.objectName ?? PREFERENCE_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 } } as never, + )) ?? []; + } 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 = operatorFacingErrorText(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 { + await engine.update( + object, + { user_id: null }, + { where: { id }, context: systemContext(organizationId) } as never, + ); + 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: operatorFacingErrorText(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 }; +} From ed0933865e088daf9b64eb408bb5d3d80c17904c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 03:07:09 +0000 Subject: [PATCH 02/11] test(service-messaging): pin the user reference, title, cascade, boot conversion and NULL-safe index end to end Claude-Session: https://claude.ai/code/session_01UnHcTADjeg3VwxF21QHj7W Co-authored-by: Claude --- ...fication-preference-identity-index.test.ts | 37 ++ .../src/messaging-service-plugin.test.ts | 63 ++++ .../notification-display-title.test.ts | 26 +- .../notification-keyed-text-bounds.test.ts | 12 +- .../src/preference-resolver.test.ts | 41 ++- ...ference-user-reference.integration.test.ts | 316 ++++++++++++++++++ 6 files changed, 475 insertions(+), 20 deletions(-) create mode 100644 packages/services/service-messaging/src/preference-user-reference.integration.test.ts 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 index e3f7005e152..7e02b9ab534 100644 --- a/packages/metadata-protocol/src/migrations/notification-preference-identity-index.test.ts +++ b/packages/metadata-protocol/src/migrations/notification-preference-identity-index.test.ts @@ -272,3 +272,40 @@ describe('sys_notification_preference: one admin-global default per key', () => }); }); }); + +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; + const { assembleMetadataProtocol } = await import('../plugin.js'); + 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/services/service-messaging/src/messaging-service-plugin.test.ts b/packages/services/service-messaging/src/messaging-service-plugin.test.ts index 25c2e593d32..b166d8833e1 100644 --- a/packages/services/service-messaging/src/messaging-service-plugin.test.ts +++ b/packages/services/service-messaging/src/messaging-service-plugin.test.ts @@ -421,3 +421,66 @@ 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 { ObjectQL } = await import('@objectstack/objectql'); + const { SqlDriver } = await import('@objectstack/driver-sql'); + const { SysUser } = await import('@objectstack/platform-objects'); + const { installDefaultOrganization, DEFAULT_ORGANIZATION_ID } = await import('./default-organization.testkit.js'); + 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 } } as any))!.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/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/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-user-reference.integration.test.ts b/packages/services/service-messaging/src/preference-user-reference.integration.test.ts new file mode 100644 index 00000000000..f0b46f53346 --- /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 }, ...ADMIN } as any)); + 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 } } as any); + + 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'] }, + ]); + }); +}); From dc1c70274c045a0c616cf6c485e78a00eb1e152f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 03:18:31 +0000 Subject: [PATCH 03/11] test(driver-sql): the preference platform key is a plain user id, not the retired sentinel Claude-Session: https://claude.ai/code/session_01UnHcTADjeg3VwxF21QHj7W Co-authored-by: Claude --- .../src/sql-driver-tenant-scoped-declared-unique.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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 From c1fd8419cc953b4210af66f9e74ad4819f0d9cdc Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 03:28:43 +0000 Subject: [PATCH 04/11] chore(service-messaging): changeset; zh-CN help for the User reference Claude-Session: https://claude.ai/code/session_01UnHcTADjeg3VwxF21QHj7W Co-authored-by: Claude --- .changeset/24895-preference-user-reference.md | 23 +++++++++++++++++++ .../translations/zh-CN.objects.generated.ts | 2 +- 2 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 .changeset/24895-preference-user-reference.md 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/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..d4e9a384bd2 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 @@ -201,7 +201,7 @@ export const zhCNObjects: NonNullable = { }, user_id: { label: "用户", - help: "接收用户 ID,'*' 表示管理员全局默认值。" + help: "此偏好适用的用户。留空即为适用于所有用户的默认值。" }, topic: { label: "主题", From 3fcab473b88606f931c81bee65bcecd071cfa079 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 03:37:33 +0000 Subject: [PATCH 05/11] test(metadata-protocol): pin the arming in its own file, imported at module top Claude-Session: https://claude.ai/code/session_01UnHcTADjeg3VwxF21QHj7W Co-authored-by: Claude --- ...fication-preference-identity-index.test.ts | 36 -------------- ...otification-preference-index-armed.test.ts | 49 +++++++++++++++++++ 2 files changed, 49 insertions(+), 36 deletions(-) create mode 100644 packages/metadata-protocol/src/plugin.notification-preference-index-armed.test.ts 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 index 7e02b9ab534..ec17fd167d8 100644 --- a/packages/metadata-protocol/src/migrations/notification-preference-identity-index.test.ts +++ b/packages/metadata-protocol/src/migrations/notification-preference-identity-index.test.ts @@ -273,39 +273,3 @@ describe('sys_notification_preference: one admin-global default per key', () => }); }); -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; - const { assembleMetadataProtocol } = await import('../plugin.js'); - 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.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([]); + }); +}); From 80ad1c40d667e9eb062f5968f6fd454a929bb55f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 03:44:22 +0000 Subject: [PATCH 06/11] chore(service-messaging): regenerate the en bundle for the User reference help Claude-Session: https://claude.ai/code/session_01UnHcTADjeg3VwxF21QHj7W Co-authored-by: Claude --- .../src/translations/en.objects.generated.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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", From 15b5f4df25d73d71e5b0d5bd809130410941feb9 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 04:29:46 +0000 Subject: [PATCH 07/11] fix(service-messaging): census-readable conversion write; regenerate the tenant-audit census; retranslate the stale preference help The conversion's write names its object through a same-file const and its context inline, so the tenant-audit census places it (one new decidable, tenant-threaded, elevated write call site). Its generated region and audit ledger are regenerated and the page's hand-written counts follow (236 -> 237). The es-ES / ja-JP / zh-CN help for the User reference and the title are retranslated from the current source; the extract dropped their copy digests. Test options are typed instead of erased. Claude-Session: https://claude.ai/code/session_01UnHcTADjeg3VwxF21QHj7W Co-authored-by: Claude --- .../docs/permissions/tenant-audit-census.mdx | 36 +++++++++---------- ...08-tenant-audit-write-call-sites.counts.md | 19 +++++----- .../src/messaging-service-plugin.test.ts | 2 +- .../src/preference-user-conversion.ts | 29 ++++++++------- ...ference-user-reference.integration.test.ts | 4 +-- .../translations/es-ES.objects.generated.ts | 4 +-- .../es-ES.source-hashes.generated.ts | 2 -- .../translations/ja-JP.objects.generated.ts | 4 +-- .../ja-JP.source-hashes.generated.ts | 2 -- .../translations/zh-CN.objects.generated.ts | 2 +- .../zh-CN.source-hashes.generated.ts | 1 - 11 files changed, 52 insertions(+), 53 deletions(-) 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/services/service-messaging/src/messaging-service-plugin.test.ts b/packages/services/service-messaging/src/messaging-service-plugin.test.ts index b166d8833e1..ea4087c31d9 100644 --- a/packages/services/service-messaging/src/messaging-service-plugin.test.ts +++ b/packages/services/service-messaging/src/messaging-service-plugin.test.ts @@ -465,7 +465,7 @@ describe("MessagingServicePlugin — converts stored user_id '*' to the empty re 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 } } as any))!.user_id ?? null; + (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(); } }; } diff --git a/packages/services/service-messaging/src/preference-user-conversion.ts b/packages/services/service-messaging/src/preference-user-conversion.ts index f75ad35fe21..bb0e690d08d 100644 --- a/packages/services/service-messaging/src/preference-user-conversion.ts +++ b/packages/services/service-messaging/src/preference-user-conversion.ts @@ -52,6 +52,13 @@ 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; @@ -86,13 +93,6 @@ export interface PreferenceUserConversionResult { detail?: string; } -/** The system context this conversion runs under (ADR-0118 D3: a migration is an entry point). */ -function systemContext(organizationId: string | null): Record { - // The row's own organization is threaded on the write, so it is tenant-scoped - // like any other write to this object — no audit bypass is needed for it. - return organizationId ? { isSystem: true, tenantId: organizationId } : { isSystem: true }; -} - function textOrNull(value: unknown): string | null { return value === null || value === undefined ? null : String(value); } @@ -105,14 +105,13 @@ function textOrNull(value: unknown): string | null { export async function convertWildcardPreferenceUsers( engine: IDataEngine | undefined, logger?: PreferenceUserConversionLogger, - opts: { objectName?: string } = {}, ): 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 = opts.objectName ?? PREFERENCE_OBJECT; + const object = OBJECT; let converted = 0; const missed: PreferenceUserConversionMiss[] = []; @@ -124,14 +123,14 @@ export async function convertWildcardPreferenceUsers( let rows: Array>; try { rows = (await engine.find( - object, + 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 } } as never, + { context: { isSystem: true } }, )) ?? []; } catch (err) { // No table yet holds no '*' row — a fresh database, or a boot whose @@ -157,10 +156,14 @@ export async function convertWildcardPreferenceUsers( 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, + OBJECT, { user_id: null }, - { where: { id }, context: systemContext(organizationId) } as never, + { where: { id }, context: { isSystem: true, tenantId: organizationId ?? undefined } }, ); converted += 1; } catch (err) { 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 index f0b46f53346..d9d4a70042a 100644 --- a/packages/services/service-messaging/src/preference-user-reference.integration.test.ts +++ b/packages/services/service-messaging/src/preference-user-reference.integration.test.ts @@ -130,7 +130,7 @@ describe('the write door refuses an id no user holds', () => { 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 }, ...ADMIN } as any)); + 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); @@ -178,7 +178,7 @@ describe('deleting a user deletes their preferences (cascade)', () => { 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 } } as any); + 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']]); 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 d4e9a384bd2..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,7 +197,7 @@ 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: "用户", 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", }; From f08bc6268adf1786909fb3985f073a9dedb778df Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 04:44:51 +0000 Subject: [PATCH 08/11] test(service-messaging): load the plugin test's engine chain at module top, not in a clocked window Claude-Session: https://claude.ai/code/session_01UnHcTADjeg3VwxF21QHj7W Co-authored-by: Claude --- .../src/messaging-service-plugin.test.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) 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 ea4087c31d9..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'; @@ -432,10 +435,6 @@ describe('MessagingServicePlugin — a failed provision is an error, once per ob */ describe("MessagingServicePlugin — converts stored user_id '*' to the empty reference at kernel:ready", () => { async function bootWithStarRow() { - const { ObjectQL } = await import('@objectstack/objectql'); - const { SqlDriver } = await import('@objectstack/driver-sql'); - const { SysUser } = await import('@objectstack/platform-objects'); - const { installDefaultOrganization, DEFAULT_ORGANIZATION_ID } = await import('./default-organization.testkit.js'); 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 }); From d272b23af854655633413c536f60da1609d1949b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 05:57:12 +0000 Subject: [PATCH 09/11] test(dogfood): sys_notification_preference's organization unique is runtime-managed now, like sys_setting's The metadata protocol's kernel:ready migration rebuilds it over (organization_id, COALESCE(user_id, ''), topic, channel) on every boot, so the 18.0 COALESCE-organization spelling does not survive a HEAD boot; the byte-for-byte pins exclude it the way they exclude sys_setting's. Claude-Session: https://claude.ai/code/session_01UnHcTADjeg3VwxF21QHj7W Co-authored-by: Claude --- ...rade-18x-index-equivalence.dogfood.test.ts | 33 ++++++++++++------- 1 file changed, 22 insertions(+), 11 deletions(-) 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; From cd4c2cb59b0c9605869b4d114862314407e11d60 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 06:40:39 +0000 Subject: [PATCH 10/11] chore(spec): classify sys_notification_preference.user_id's empty state as scope in the empty-state registry Declared cross-lane on the spec seat post (#6017, 6334445279): one row, no schema, scanner or other entry moves. An empty User is the admin-wide default (ruling A), it selects the users a delivery toggle covers and grants no access. Claude-Session: https://claude.ai/code/session_01UnHcTADjeg3VwxF21QHj7W Co-authored-by: Claude --- packages/spec/scripts/liveness/empty-state-registry.mts | 8 ++++++++ 1 file changed, 8 insertions(+) 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 ------------------------------------- From 55663d72e68fe4fede0df09e69f6e09d9741a93a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 07:35:50 +0000 Subject: [PATCH 11/11] fix(service-messaging): the boot conversion reports the engine's own error text, not the raw-SQL seam helper's types' test:repo enumerates every operatorFacingErrorText caller with a sentinel pin against raw statements; the conversion became an unlisted caller. It never touches the raw seam: its reads and writes go through the data engine, whose errors carry no composed seam fault and whose SQL driver already redacts statements, so the package's plain message reading is the right one. Claude-Session: https://claude.ai/code/session_01UnHcTADjeg3VwxF21QHj7W Co-authored-by: Claude --- .../src/preference-user-conversion.ts | 22 ++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/packages/services/service-messaging/src/preference-user-conversion.ts b/packages/services/service-messaging/src/preference-user-conversion.ts index bb0e690d08d..4c9ee36d722 100644 --- a/packages/services/service-messaging/src/preference-user-conversion.ts +++ b/packages/services/service-messaging/src/preference-user-conversion.ts @@ -43,7 +43,7 @@ */ import type { IDataEngine } from '@objectstack/spec/contracts'; -import { isMissingTableError, isUniqueViolationError, operatorFacingErrorText } from '@objectstack/types'; +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. */ @@ -93,6 +93,22 @@ export interface PreferenceUserConversionResult { 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); } @@ -136,7 +152,7 @@ export async function convertWildcardPreferenceUsers( // 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 = operatorFacingErrorText(err); + 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: ' + @@ -174,7 +190,7 @@ export async function convertWildcardPreferenceUsers( topic: textOrNull(row.topic), channel: textOrNull(row.channel), reason: duplicate ? 'duplicate' : 'failed', - detail: operatorFacingErrorText(err), + detail: messageOf(err), }); } }