diff --git a/.changeset/memory-unique-sticky-tenancy-opt-out.md b/.changeset/memory-unique-sticky-tenancy-opt-out.md new file mode 100644 index 0000000000..d877f97e20 --- /dev/null +++ b/.changeset/memory-unique-sticky-tenancy-opt-out.md @@ -0,0 +1,68 @@ +--- +"@objectstack/driver-memory": minor +"@objectstack/driver-sql": patch +"@objectstack/objectql": patch +--- + +fix(driver-memory,driver-sql): an explicit `tenancy.enabled: false` opt-out is sticky, so a partial `syncSchema` re-registration no longer flips a platform-global object's UNIQUE partition (#16729) + +## What was wrong + +`InMemoryDriver.syncSchema` recomputed its uniqueness constraints from whatever +schema THAT call happened to carry. A second registration without a `tenancy` +block — the `{ name, fields }` shape — fell through to the implicit +`organization_id` heuristic, so a `unique` field moved from **one row per +install** (`scopeField: null`, which is what `tenancy.enabled: false` declares) +to **one row per organization**. A duplicate the declaration refuses then +landed. Measured at the driver door on `origin/main` `d61139f1ba`: + +| sequence | second `key: 'K'`, different organization | +|:--|:--| +| register with `tenancy.enabled: false` | `REFUSED` — `UNIQUE_VIOLATION` / 409 | +| …then re-register with `{ name, fields }` | **`LANDED`** | + +`SqlDriver` running the same sequence refuses in **both** cases: it has kept a +sticky `tenantOptOutByTable` since #3249. `driver-memory` had mirrored the inner +`computeTenantField` and not the wrapper that consults the record, so "mirrors +`computeTenantField` arm for arm" stayed literally true while the pair diverged. + +It is silent in both directions — nothing logs the flip, and the refusal names +the field, never the partition. That is the declared-vs-enforced shape Prime +Directive #10 forbids, reached by a state change rather than by a missing check. + +## What it does now + +- **`@objectstack/driver-memory`** gains `computeAndRecordTenantField`, the + sticky resolver, and the `TenantOptOutRecord` type for the per-instance record + a driver owns. `InMemoryDriver` holds one and resolves through it, handing + BOTH declaration surfaces — field-level `unique` and declared `indexes[]` — + the same resolved column. `uniqueConstraintsFromFields` and + `uniqueConstraintsFromDeclaredIndexes` accept that column as an optional + second argument; called with one argument they answer exactly as before. + `tenantFieldOf` is unchanged and still a pure function of its argument. +- **`@objectstack/driver-sql`**: the shard leaf resolved its tenant column with + the BARE `computeTenantField`, so a `rotateShards` sweep carrying no `tenancy` + block gave a shard an organization key part the base table's index does not + have — one object, two partitions, decided by which physical table a row + landed in. It now resolves through the record, keyed by the base table. +- **`@objectstack/objectql`**: `LifecycleObjectLike` declares `tenancy`. The + Archiver hands that object straight to `cold.syncSchema`, and the published + type refused the key while the driver below read it — so an author writing a + fresh literal was pushed into producing exactly the partial re-registration + above. Same correction #16711 made where the shard leaf narrowed the key off + the object it was handed. + +The record is deliberately narrow. Only the explicit OPT-OUT is sticky: a +declared `tenancy.tenantField` is not recorded, matching `SqlDriver`. An object +that never declared the opt-out never enters the record, so a genuinely +org-scoped object keeps its `organization_id` partition across a partial +re-registration — an implementation answering `null` more often would not be +stickier, it would be tenant isolation switched off. A carried `tenancy` block +stays authoritative in both directions and CLEARS a recorded opt-out. + +`@objectstack/driver-memory` is `minor` for the two new public-entry exports. +The behaviour repairs themselves are `patch`: each restores an implementation to +the `tenancy.enabled: false` contract (`isTenancyDisabled`, ADR-0066) it was +already declaring, rather than replacing one legal published answer with +another. The `objectql` entry is a published type WIDENING — a key the interface +refused is now accepted, and nothing that compiled before stops compiling. diff --git a/packages/drivers/driver-memory/src/index.ts b/packages/drivers/driver-memory/src/index.ts index 86cee78a7c..b6a6066139 100644 --- a/packages/drivers/driver-memory/src/index.ts +++ b/packages/drivers/driver-memory/src/index.ts @@ -31,10 +31,19 @@ export type { TenancyAwareSchema } from './memory-tenancy-guard.js'; // identity and the scoping helpers, exported so a consumer can assert the // envelope (`code` AND `status`, never merely "it threw") without // string-matching the message. +// +// [#16729] `computeAndRecordTenantField` is published BESIDE `tenantFieldOf`, +// not kept private, because publishing only the inner half is what let this +// package diverge in the first place: `tenantFieldOf` mirrors +// `SqlDriver.computeTenantField`, the stickiness lives in the wrapper AROUND +// that function on the SQL side, and a reader who found only the inner half +// exported mirrored only the inner half. The next driver reproducing this pair +// sees both halves or repeats the same omission. export { UNIQUE_VIOLATION_CODE, UNIQUE_VIOLATION_STATUS, assertNoUniqueViolation, + computeAndRecordTenantField, declaredIndexViolationError, isDeclaredIndexConstraint, tenantFieldOf, @@ -48,6 +57,7 @@ export type { MemoryDeclaredIndexConstraint, MemoryUniqueConstraint, MemoryUniqueEnforcement, + TenantOptOutRecord, UniqueAwareSchema, } from './memory-unique-constraint.js'; diff --git a/packages/drivers/driver-memory/src/memory-driver.ts b/packages/drivers/driver-memory/src/memory-driver.ts index a219221fb7..a9b90d7546 100644 --- a/packages/drivers/driver-memory/src/memory-driver.ts +++ b/packages/drivers/driver-memory/src/memory-driver.ts @@ -49,6 +49,7 @@ import { // module docblock. import { assertNoUniqueViolation, + computeAndRecordTenantField, uniqueConstraintsFromDeclaredIndexes, uniqueConstraintsFromFields, type MemoryUniqueEnforcement, @@ -404,6 +405,27 @@ export class InMemoryDriver implements IDataDriver { * the data it happens to hold. */ private uniqueConstraints: Map = new Map(); + + /** + * [#16729] Objects whose schema EXPLICITLY declared `tenancy.enabled: false`, + * this driver's counterpart of `SqlDriver.tenantOptOutByTable` and the record + * {@link computeAndRecordTenantField} maintains. + * + * Sticky across re-registrations on purpose: a later `syncSchema` that omits + * the `tenancy` block must NOT resurrect org-scoping of the uniqueness key + * via the implicit `organization_id` heuristic. Without it a platform-global + * object's UNIQUE partition silently moved from one row per install to one + * row per organization, and the duplicate its declaration refuses LANDED. + * + * Unlike {@link uniqueConstraints} it is deliberately NOT cleared by + * `dropTable`. That map is cleared because a constraint outliving its table + * would be ENFORCED over a table nobody declared; this record enforces + * nothing on its own — it only decides which partition the NEXT declaration + * resolves to, and the last authoritative word on this object was still + * "platform-global". Dropping a table is not a schema declaring itself + * tenant-scoped, and only such a declaration clears the record. + */ + private tenantOptOutByObject: Set = new Set(); private transactions: Map = new Map(); private persistenceAdapter: PersistenceAdapterInterface | null = null; @@ -1978,9 +2000,18 @@ export class InMemoryDriver implements IDataDriver { // an already-duplicated pair is reported by the first write that touches // it — the same posture `driver-sql` takes when a unique index cannot be // built over dirty data (it announces, it does not delete rows). + // [#16729] Resolve the tenant column through the STICKY record rather than + // from this call's schema alone. `syncSchema` is idempotent and is called + // again with whatever schema the caller happens to hold; a call carrying no + // `tenancy` block would otherwise fall through to the implicit + // `organization_id` heuristic and re-scope an object that declared itself + // platform-global. Both surfaces are handed the SAME resolved column, so + // the field-level and declared-index keys of one object cannot disagree + // about which partition it lives in. + const tenantField = computeAndRecordTenantField(this.tenantOptOutByObject, object, schema); this.uniqueConstraints.set(object, [ - ...uniqueConstraintsFromFields(schema), - ...uniqueConstraintsFromDeclaredIndexes(schema), + ...uniqueConstraintsFromFields(schema, tenantField), + ...uniqueConstraintsFromDeclaredIndexes(schema, tenantField), ]); if (kinds.size > 0) { const table = this.db[object]; diff --git a/packages/drivers/driver-memory/src/memory-unique-constraint.ts b/packages/drivers/driver-memory/src/memory-unique-constraint.ts index 2af468e651..97cc29ccf6 100644 --- a/packages/drivers/driver-memory/src/memory-unique-constraint.ts +++ b/packages/drivers/driver-memory/src/memory-unique-constraint.ts @@ -300,6 +300,82 @@ export function tenantFieldOf(schema: UniqueAwareSchema | null | undefined): str return null; } +/** + * [#16729] One driver's record of the objects whose schema EXPLICITLY declared + * `tenancy.enabled === false` — this package's counterpart of `SqlDriver`'s + * `tenantOptOutByTable`. + * + * Owned by the DRIVER INSTANCE, never by this module. Two `InMemoryDriver`s in + * one process are two independent stores (this driver's whole shape is + * per-instance state), so a module-level record would let one store's + * declaration decide another store's uniqueness partition — a fresh + * cross-instance channel introduced by the fix for a cross-registration one. + * `SqlDriver` holds its own record per instance for the same reason. + */ +export type TenantOptOutRecord = Set; + +/** + * [#16729] {@link tenantFieldOf} + maintenance of the sticky explicit-opt-out + * record. Mirrors `SqlDriver.computeAndRecordTenantField` arm for arm, and it + * is the arm this package was missing. + * + * ## Why the mirror needed a SECOND function, not a change to the first + * + * `driver-memory` already reproduced `SqlDriver.computeTenantField` faithfully + * as {@link tenantFieldOf}. The stickiness, though, does not live in that + * function on the SQL side either: it lives in the WRAPPER around it. So the + * inner half was mirrored and the outer half was not, and "this mirrors + * `computeTenantField` arm for arm" stayed literally true while the pair as a + * whole diverged. {@link tenantFieldOf} is therefore UNCHANGED and still + * answers from the passed schema alone — a pure function of its argument is + * what its own pins assert, and they remain correct. + * + * ## What the record buys + * + * A schema that carries a `tenancy` declaration is authoritative: it sets or + * clears the record and is computed normally. A schema WITHOUT one — a partial + * re-registration — preserves a previously declared opt-out instead of letting + * the implicit `organization_id` heuristic re-scope a platform-global object. + * Without it, a second `syncSchema` carrying only `{ name, fields }` silently + * moves a `unique` field from ONE row per install (`scopeField: null`, which is + * what `tenancy.enabled: false` declares) to one row per organization: a + * duplicate the declaration refuses then LANDS, and nothing announces the + * change — the declared-vs-enforced divergence Prime Directive #10 forbids, + * reached by a state change rather than by a missing check. + * + * ## Only the OPT-OUT is sticky, deliberately + * + * A declared `tenancy.tenantField` is NOT recorded, so a partial + * re-registration of a custom-tenant-column object still falls back to + * `organization_id`. That is not an oversight: it is what `SqlDriver` does, and + * this module's contract is to answer as `driver-sql` answers. Recording more + * here would be a second, easier answer to "what does `unique` mean" — the + * one-contract-two-numbers defect this module exists to close. + * + * A genuinely tenant-scoped object (no `tenancy` block, an `organization_id` + * column) never enters the record, so it keeps its `organization_id` partition + * across a partial re-registration exactly as before. An implementation that + * answered `null` more often than this one would not be stickier, it would be + * tenant isolation switched off. + */ +export function computeAndRecordTenantField( + record: TenantOptOutRecord, + key: string, + schema: UniqueAwareSchema | null | undefined, +): string | null { + // A carried `tenancy` block is AUTHORITATIVE in both directions: it records a + // fresh opt-out, and it CLEARS a stale one. `!= null` is the SQL side's test + // — the declaration's PRESENCE is what makes it authoritative, so a + // `tenancy: {}` that declares no opt-out clears the record too. + if (schema?.tenancy != null) { + if (isTenancyDisabled(schema)) record.add(key); + else record.delete(key); + return tenantFieldOf(schema); + } + if (record.has(key)) return null; + return tenantFieldOf(schema); +} + /** * The constraints an object's field-level `unique` declarations ask for. * @@ -307,13 +383,22 @@ export function tenantFieldOf(schema: UniqueAwareSchema | null | undefined): str * so the create, update and update-many paths cannot disagree about what one * means — the same reason `uniqueIndexesFromFields` is the single place on the * SQL side. + * + * [#16729] `tenantField` is the RESOLVED tenant column. It defaults to + * {@link tenantFieldOf} of this very schema, so the published one-argument call + * answers exactly as before; a caller holding a {@link TenantOptOutRecord} + * passes {@link computeAndRecordTenantField}'s answer instead, and a partial + * re-registration then cannot re-scope an object that declared itself + * platform-global. `driver-sql` threads the resolved column into + * `syncDeclaredIndexes` rather than letting that leaf recompute it, for this + * same reason. */ export function uniqueConstraintsFromFields( schema: UniqueAwareSchema | null | undefined, + tenantField: string | null = tenantFieldOf(schema), ): MemoryUniqueConstraint[] { const fields = schema?.fields; if (!fields) return []; - const tenantField = tenantFieldOf(schema); const out: MemoryUniqueConstraint[] = []; for (const [name, field] of Object.entries(fields)) { const unique = (field as { unique?: unknown } | null | undefined)?.unique; @@ -344,13 +429,22 @@ export function uniqueConstraintsFromFields( * `driver-memory` must not depend on `driver-sql`, so the arms are reproduced * and pinned here (`memory-declared-index-unique.test.ts`), the way * {@link tenantFieldOf} reproduces `SqlDriver.computeTenantField`. + * + * [#16729] `tenantField` is the RESOLVED tenant column. It defaults to + * {@link tenantFieldOf} of this very schema, so the published one-argument call + * answers exactly as before; a caller holding a {@link TenantOptOutRecord} + * passes {@link computeAndRecordTenantField}'s answer instead, and a partial + * re-registration then cannot re-scope an object that declared itself + * platform-global. `driver-sql` threads the resolved column into + * `syncDeclaredIndexes` rather than letting that leaf recompute it, for this + * same reason. */ export function uniqueConstraintsFromDeclaredIndexes( schema: UniqueAwareSchema | null | undefined, + tenantField: string | null = tenantFieldOf(schema), ): MemoryDeclaredIndexConstraint[] { const declared = schema?.indexes; if (!Array.isArray(declared)) return []; - const tenantField = tenantFieldOf(schema); const out: MemoryDeclaredIndexConstraint[] = []; for (const idx of declared) { // The same filter the SQL side applies, and nothing more: a non-string or diff --git a/packages/drivers/driver-memory/src/memory-unique-sticky-tenancy.test.ts b/packages/drivers/driver-memory/src/memory-unique-sticky-tenancy.test.ts new file mode 100644 index 0000000000..44e7473bcf --- /dev/null +++ b/packages/drivers/driver-memory/src/memory-unique-sticky-tenancy.test.ts @@ -0,0 +1,236 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#16729] The explicit `tenancy.enabled: false` opt-out is STICKY across a + * partial `syncSchema` re-registration — the uniqueness partition a + * platform-global object declares survives a later call that carries only + * `{ name, fields }`. + * + * ## What was broken + * + * `InMemoryDriver.syncSchema` recomputed the uniqueness constraints from + * whatever schema THAT call happened to carry. A second registration without a + * `tenancy` block fell through to the implicit `organization_id` heuristic, so + * `scopeField` moved from `null` (one row per install — what + * `tenancy.enabled: false` declares) to `'organization_id'` (one row per + * organization). A duplicate the declaration refuses then LANDED, silently: + * nothing logs the flip, and the refusal message names the field, never the + * partition. `SqlDriver` had held the same record since #3249; this package + * mirrored the inner `computeTenantField` and not the wrapper around it. + * + * ## Why the negative controls carry as much weight as the positive ones + * + * "Preserve the opt-out" has a trivially green wrong implementation: answer + * `null` always. That passes every assertion about the platform-global object + * AND switches uniqueness scoping off for every genuinely tenant-scoped one, so + * two organizations could no longer hold the same record number. Every + * behavioural case below is therefore paired with the object that must NOT + * move, and both directions of the record are driven: a carried `tenancy` + * declaration is authoritative and CLEARS a recorded opt-out. + * + * Assertions read `uniqueConstraintsFromFields` and the driver's own refusal — + * not `tenantFieldOf` alone. The harm surface is the uniqueness partition, and + * there is a layer between the two. + */ + +import { describe, it, expect } from 'vitest'; +import { InMemoryDriver } from './memory-driver.js'; +import { + UNIQUE_VIOLATION_CODE, + UNIQUE_VIOLATION_STATUS, + computeAndRecordTenantField, + tenantFieldOf, + uniqueConstraintsFromDeclaredIndexes, + uniqueConstraintsFromFields, + type TenantOptOutRecord, +} from './memory-unique-constraint.js'; + +/** A platform-global object: declares the opt-out, still carries an org FK. */ +const GLOBAL_FIELDS = { + id: { type: 'string' }, + key: { type: 'string', unique: true }, + organization_id: { type: 'string' }, +} as const; + +const globalFull = { name: 'sys_license', fields: GLOBAL_FIELDS, tenancy: { enabled: false } }; +/** The shape a partial re-registration carries — no `tenancy` block. */ +const globalPartial = { name: 'sys_license', fields: GLOBAL_FIELDS }; + +/** The control: genuinely org-scoped — no `tenancy` block, an org column. */ +const scopedPartial = { name: 'crm_account', fields: GLOBAL_FIELDS }; + +async function seedAndCollide( + driver: InMemoryDriver, + object: string, +): Promise<'refused' | 'landed'> { + await driver.create(object, { id: '1', key: 'K', organization_id: 'org_a' }); + try { + await driver.create(object, { id: '2', key: 'K', organization_id: 'org_b' }); + return 'landed'; + } catch { + return 'refused'; + } +} + +describe('[#16729] the explicit tenancy opt-out is sticky across a partial re-registration', () => { + describe('the resolved partition', () => { + it('keeps scopeField null when a later syncSchema carries no tenancy block', () => { + const record: TenantOptOutRecord = new Set(); + expect(computeAndRecordTenantField(record, 'sys_license', globalFull)).toBeNull(); + expect(computeAndRecordTenantField(record, 'sys_license', globalPartial)).toBeNull(); + + // ⚠️ The partition, not just the tenant column — the damage surface is a + // layer past `tenantFieldOf`. + const partition = uniqueConstraintsFromFields( + globalPartial, + computeAndRecordTenantField(record, 'sys_license', globalPartial), + ); + expect(partition).toEqual([{ field: 'key', scopeField: null }]); + }); + + it('leaves a genuinely org-scoped object scoped after the same sequence', () => { + const record: TenantOptOutRecord = new Set(); + expect(computeAndRecordTenantField(record, 'crm_account', scopedPartial)).toBe('organization_id'); + expect(computeAndRecordTenantField(record, 'crm_account', scopedPartial)).toBe('organization_id'); + expect( + uniqueConstraintsFromFields( + scopedPartial, + computeAndRecordTenantField(record, 'crm_account', scopedPartial), + ), + ).toEqual([{ field: 'key', scopeField: 'organization_id' }]); + }); + + it('CLEARS the record when a later schema carries an authoritative tenancy declaration', () => { + const record: TenantOptOutRecord = new Set(); + computeAndRecordTenantField(record, 'sys_license', globalFull); + expect(record.has('sys_license')).toBe(true); + + // A carried block is authoritative in BOTH directions. `enabled: true` + // and a block that declares no opt-out at all each clear the record. + expect( + computeAndRecordTenantField(record, 'sys_license', { + ...globalPartial, + tenancy: { enabled: true }, + }), + ).toBe('organization_id'); + expect(record.has('sys_license')).toBe(false); + // …and having been cleared, the object is scoped again from then on. + expect(computeAndRecordTenantField(record, 'sys_license', globalPartial)).toBe('organization_id'); + + computeAndRecordTenantField(record, 'sys_license', globalFull); + expect(record.has('sys_license')).toBe(true); + expect(computeAndRecordTenantField(record, 'sys_license', { ...globalPartial, tenancy: {} })).toBe( + 'organization_id', + ); + expect(record.has('sys_license')).toBe(false); + }); + + it('hands both declaration surfaces the SAME resolved column', () => { + const record: TenantOptOutRecord = new Set(); + const declared = { + ...globalPartial, + indexes: [{ fields: ['key'], unique: 'organization' }], + }; + computeAndRecordTenantField(record, 'sys_license', globalFull); + const tenantField = computeAndRecordTenantField(record, 'sys_license', declared); + + expect(uniqueConstraintsFromFields(declared, tenantField)).toEqual([ + { field: 'key', scopeField: null }, + ]); + expect(uniqueConstraintsFromDeclaredIndexes(declared, tenantField)).toEqual([ + { columns: ['key'], nullSafeColumns: [] }, + ]); + }); + + it('leaves tenantFieldOf a pure function of its argument', () => { + // The record is the WRAPPER's business. `tenantFieldOf` still answers + // from the passed schema alone, which is what its own pins assert. + const record: TenantOptOutRecord = new Set(); + computeAndRecordTenantField(record, 'sys_license', globalFull); + expect(tenantFieldOf(globalPartial)).toBe('organization_id'); + expect(tenantFieldOf(globalFull)).toBeNull(); + }); + }); + + describe('at the driver door', () => { + it('refuses a cross-organization duplicate after a partial re-registration', async () => { + const driver = new InMemoryDriver(); + await driver.syncSchema('sys_license', globalFull); + await driver.syncSchema('sys_license', globalPartial); + + await driver.create('sys_license', { id: '1', key: 'K', organization_id: 'org_a' }); + await expect( + driver.create('sys_license', { id: '2', key: 'K', organization_id: 'org_b' }), + ).rejects.toMatchObject({ + // ⛔ Never merely "it threw" — a bare Error from an unrelated fault + // passes that and says nothing about the contract (#6144). + code: UNIQUE_VIOLATION_CODE, + status: UNIQUE_VIOLATION_STATUS, + }); + }); + + it('refuses it on the DECLARED-INDEX surface too', async () => { + // Both surfaces are wired through the same resolved column; a fix that + // routed only the field-level one would leave a declared + // `unique: 'organization'` index re-scoped by the same partial call. + const declaredFull = { + name: 'sys_seat', + fields: GLOBAL_FIELDS, + indexes: [{ fields: ['key'], unique: 'organization' }], + tenancy: { enabled: false }, + }; + const declaredPartial = { ...declaredFull, tenancy: undefined }; + + const driver = new InMemoryDriver(); + await driver.syncSchema('sys_seat', declaredFull); + await driver.syncSchema('sys_seat', declaredPartial); + + await driver.create('sys_seat', { id: '1', key: 'K', organization_id: 'org_a' }); + await expect( + driver.create('sys_seat', { id: '2', key: 'K', organization_id: 'org_b' }), + ).rejects.toMatchObject({ + code: UNIQUE_VIOLATION_CODE, + status: UNIQUE_VIOLATION_STATUS, + }); + }); + + it('still lets two organizations hold the same key on a genuinely scoped object', async () => { + const driver = new InMemoryDriver(); + await driver.syncSchema('crm_account', scopedPartial); + await driver.syncSchema('crm_account', scopedPartial); + + expect(await seedAndCollide(driver, 'crm_account')).toBe('landed'); + // …and the partition is still ENFORCED inside one organization. + await expect( + driver.create('crm_account', { id: '3', key: 'K', organization_id: 'org_a' }), + ).rejects.toMatchObject({ + code: UNIQUE_VIOLATION_CODE, + status: UNIQUE_VIOLATION_STATUS, + }); + }); + + it('re-scopes the object when a later registration declares tenancy authoritatively', async () => { + const driver = new InMemoryDriver(); + await driver.syncSchema('sys_license', globalFull); + // A block that declares no opt-out is authoritative and clears the + // record; `enabled: true` cannot be driven through this door because the + // #6915 guard refuses a tenant-scoped object outright. + await driver.syncSchema('sys_license', { ...globalPartial, tenancy: {} }); + + expect(await seedAndCollide(driver, 'sys_license')).toBe('landed'); + }); + + it('keeps the record per driver instance', async () => { + const declaring = new InMemoryDriver(); + await declaring.syncSchema('sys_license', globalFull); + + // A second store in the same process is a different store: one store's + // declaration must not decide another's uniqueness partition. + const other = new InMemoryDriver(); + await other.syncSchema('sys_license', globalPartial); + expect(await seedAndCollide(other, 'sys_license')).toBe('landed'); + + expect(await seedAndCollide(declaring, 'sys_license')).toBe('refused'); + }); + }); +}); diff --git a/packages/drivers/driver-sql/src/sql-driver-shard-tenancy-opt-out.test.ts b/packages/drivers/driver-sql/src/sql-driver-shard-tenancy-opt-out.test.ts new file mode 100644 index 0000000000..bf86f77a3b --- /dev/null +++ b/packages/drivers/driver-sql/src/sql-driver-shard-tenancy-opt-out.test.ts @@ -0,0 +1,160 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#16729] The shard path resolves its tenant column through the STICKY + * opt-out record, so a rotation-declared platform-global object scopes its + * declared `unique: 'organization'` index identically on the base table and on + * every shard. + * + * ## The gap + * + * `tenantOptOutByTable` (#3249) keeps an explicit `tenancy.enabled: false` + * declaration from being lost to a later partial re-registration, and + * `computeAndRecordTenantField` is the resolver that consults it. The shard + * leaf called the BARE `computeTenantField` instead — the one that reads this + * call's schema and nothing else — so a `rotateShards` sweep carrying no + * `tenancy` block fell through to the implicit `organization_id` heuristic and + * gave the shard an organization key part the base table's own index does not + * have. One object, two partitions, decided by which physical table a row + * happened to land in. + * + * The sweep-time entry point is exactly where that shape appears: the + * LifecycleService calls `rotateShards` on every sweep with whatever object it + * holds, long after `initObjects` recorded the declaration. + * + * ## What is asserted + * + * The tenant column handed to `syncDeclaredIndexes` — the value that decides + * the partition — captured per table. Both directions: the opted-out object + * must resolve `null` on base AND shard, and a genuinely org-scoped object must + * still resolve `organization_id` on both. An implementation that answered + * `null` everywhere would satisfy the first pair and quietly unscope the + * second. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { SqlDriver } from './index.js'; + +const T0 = Date.parse('2036-08-01T12:00:00.000Z'); + +/** Captures the tenant column each `syncDeclaredIndexes` call was given. */ +class RecordingDriver extends SqlDriver { + readonly resolved: Array<{ table: string; tenantField: string | null | undefined }> = []; + + protected override async syncDeclaredIndexes( + tableName: string, + indexes: any, + physicalColumns: any, + tenantField?: string | null, + ): Promise { + this.resolved.push({ table: tableName, tenantField }); + return super.syncDeclaredIndexes(tableName, indexes, physicalColumns, tenantField); + } +} + +const ROTATION = { strategy: 'rotation' as const, shards: 3, unit: 'day' as const }; + +const FIELDS = { + key: { type: 'text' }, + organization_id: { type: 'text' }, + created_at: { type: 'datetime' }, +}; +const INDEXES = [{ name: 'by_key', fields: ['key'], unique: 'organization' }]; + +/** Platform-global AND rotation-declared. */ +const GLOBAL_FULL = { + name: 'rot_license', + fields: FIELDS, + indexes: INDEXES, + tenancy: { enabled: false }, + lifecycle: { class: 'telemetry', storage: ROTATION }, +}; +/** The same object as a sweep hands it back — no `tenancy` block. */ +const GLOBAL_PARTIAL = { + name: 'rot_license', + fields: FIELDS, + indexes: INDEXES, + lifecycle: { class: 'telemetry', storage: ROTATION }, +}; +/** The control: genuinely org-scoped, same shape otherwise. */ +const SCOPED_PARTIAL = { ...GLOBAL_PARTIAL, name: 'rot_event' }; + +/** The same declaration on the MANAGED-table path — no rotation policy. */ +const MAIN_FULL = { name: 'lic_main', fields: FIELDS, indexes: INDEXES, tenancy: { enabled: false } }; +const MAIN_PARTIAL = { name: 'lic_main', fields: FIELDS, indexes: INDEXES }; + +describe('[#16729] shard path honours the sticky tenancy opt-out', () => { + let driver: RecordingDriver; + + beforeEach(() => { + driver = new RecordingDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); + }); + + afterEach(async () => { + await driver.disconnect(); + }); + + it('resolves null on base AND shard for a declared platform-global object', async () => { + await driver.initObjects([GLOBAL_FULL as any]); + driver.resolved.length = 0; + + // The sweep-time call, carrying no `tenancy` block. + const rotated = await driver.rotateShards(GLOBAL_PARTIAL as any, T0); + expect(rotated.shards.length).toBeGreaterThan(0); + + const shardReadings = driver.resolved.filter((r) => r.table !== 'rot_license'); + expect(shardReadings.length).toBeGreaterThan(0); + for (const reading of shardReadings) { + expect(reading.tenantField ?? null).toBeNull(); + } + }); + + it('agrees with the MAIN-TABLE path for the same declaration', async () => { + // A rotation object's base NAME becomes the read view, so its managed-table + // reading has to come from a non-rotation twin of the same declaration — + // same fields, same declared index, same explicit opt-out. + const main = new RecordingDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); + try { + await main.initObjects([MAIN_FULL as any]); + main.resolved.length = 0; + await main.initObjects([MAIN_PARTIAL as any]); + const mainReadings = main.resolved.filter((r) => r.table === 'lic_main'); + expect(mainReadings.length).toBeGreaterThan(0); + + await driver.initObjects([GLOBAL_FULL as any]); + driver.resolved.length = 0; + await driver.rotateShards(GLOBAL_PARTIAL as any, T0); + const shardReadings = driver.resolved.filter((r) => r.table !== 'rot_license'); + expect(shardReadings.length).toBeGreaterThan(0); + + // ⚠️ The point of the card: one declaration, ONE partition, whichever + // physical table the row lands in. + const partitions = new Set( + [...mainReadings, ...shardReadings].map((r) => r.tenantField ?? null), + ); + expect([...partitions]).toEqual([null]); + } finally { + await main.disconnect(); + } + }); + + it('leaves a genuinely org-scoped object scoped on its shards', async () => { + await driver.initObjects([SCOPED_PARTIAL as any]); + driver.resolved.length = 0; + + await driver.rotateShards(SCOPED_PARTIAL as any, T0); + const shardReadings = driver.resolved.filter((r) => r.table !== 'rot_event'); + expect(shardReadings.length).toBeGreaterThan(0); + for (const reading of shardReadings) { + expect(reading.tenantField).toBe('organization_id'); + } + }); +}); diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index 40d5707258..9a8fe235be 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -9473,7 +9473,7 @@ export class SqlDriver implements IDataDriver { // Column-sync every retained shard (creates the current one; adds any // newly declared columns to older shards so the UNION stays uniform). for (const shard of retained) { - await this.ensureShardTable(shard, obj); + await this.ensureShardTable(shard, obj, tableName); this.aliasShardBookkeeping(tableName, shard); } @@ -9589,10 +9589,16 @@ export class SqlDriver implements IDataDriver { * two links above still narrowing the same value, so a caller spelling * `indexes` in a fresh literal to `rotateShards` would still be refused by a * type while the driver read the key regardless. + * + * `baseTable` is the table the shard belongs to — the key its tenancy record + * is held under until {@link aliasShardBookkeeping} copies it across. It + * defaults to `shardName` so an existing override or external caller keeps + * today's behaviour, and the one in-tree call site names the base. */ protected async ensureShardTable( shardName: string, obj: { fields?: Record; tenancy?: any; indexes?: any[] }, + baseTable: string = shardName, ): Promise { const builtinColumns = new Set(['id', 'created_at', 'updated_at']); // [#12015] Both branches below drop a declared field named after a builtin @@ -9641,11 +9647,26 @@ export class SqlDriver implements IDataDriver { ...idx, name: typeof idx?.name === 'string' && idx.name.trim() ? `${shardName}__${idx.name.trim()}` : undefined, })); - // Shard bookkeeping is aliased AFTER this method runs, so resolve the - // tenant column from the object schema itself — a declared - // `unique: 'organization'` index (ADR-0120 D1) must scope identically on - // every shard of the base table. - await this.syncDeclaredIndexes(shardName, perShard, new Set(Object.keys(colInfo)), this.computeTenantField(obj)); + // Shard bookkeeping is aliased AFTER this method runs, so `shardName` has + // no entry of its own yet — a declared `unique: 'organization'` index + // (ADR-0120 D1) must scope identically on every shard of the base table, + // so the answer has to come from the base. + // + // [#16729] Keyed by `baseTable`, and through the RECORDING resolver, not + // the bare `computeTenantField`. The bare one reads this call's schema + // alone, so a shard synced from a partial re-registration — one carrying + // no `tenancy` block — fell through to the implicit `organization_id` + // heuristic and gave the shard an organization key part the base table's + // own index does not have. `tenantOptOutByTable` already holds the base's + // explicit opt-out (`initObjects` recorded it under exactly this key, and + // `aliasShardBookkeeping` propagates it to the shard afterwards), so + // consulting it here is what makes the two paths answer the same. + await this.syncDeclaredIndexes( + shardName, + perShard, + new Set(Object.keys(colInfo)), + this.computeAndRecordTenantField(baseTable, obj), + ); } } diff --git a/packages/objectql/src/lifecycle/lifecycle-service.test.ts b/packages/objectql/src/lifecycle/lifecycle-service.test.ts index f633cdc358..4c9d58a323 100644 --- a/packages/objectql/src/lifecycle/lifecycle-service.test.ts +++ b/packages/objectql/src/lifecycle/lifecycle-service.test.ts @@ -936,6 +936,39 @@ describe('LifecycleService.sweep — Archiver (P3)', () => { expect(report.skipped).toEqual([]); }); + it('[#16729] carries the tenancy declaration through to the cold store', async () => { + // The Archiver hands the cold driver the object it was given, and every + // driver resolves a UNIQUENESS PARTITION from `tenancy`: + // `enabled: false` is one row per install, its absence one row per + // organization. A `LifecycleObjectLike` that could not SPELL the key would + // reach `syncSchema` as the `{ name, fields }` shape — the partial + // re-registration a driver's sticky opt-out record exists to survive — so + // this pins the key on the published type and on the call in one assertion. + const GLOBAL_OBJ: LifecycleObjectLike = { + name: 'sys_license', + fields: { key: { type: 'string', unique: true }, organization_id: { type: 'string' } }, + tenancy: { enabled: false }, + lifecycle: { + class: 'audit', + retention: { maxAge: '90d' }, + archive: { after: '90d', to: 'archive', keep: '7y' }, + } as any, + }; + const cold = coldStore(); + const hot = hotStore([{ id: 'a', created_at: '2020-01-01T00:00:00.000Z' }]); + const { engine } = captureEngine([GLOBAL_OBJ], { + driver: hot.driver, + datasources: { archive: cold.driver }, + }); + + await service(engine).sweep(); + + expect(cold.driver.syncSchema).toHaveBeenCalledWith( + 'sys_license', + expect.objectContaining({ tenancy: { enabled: false } }), + ); + }); + it('retains everything and reports archive-pending when the archive datasource is missing', async () => { const hot = hotStore([{ id: 'a', created_at: '2020-01-01T00:00:00.000Z' }]); const { engine, deletes } = captureEngine([AUDIT_OBJ], { driver: hot.driver }); diff --git a/packages/objectql/src/lifecycle/lifecycle-service.ts b/packages/objectql/src/lifecycle/lifecycle-service.ts index 272b74542c..789a83f175 100644 --- a/packages/objectql/src/lifecycle/lifecycle-service.ts +++ b/packages/objectql/src/lifecycle/lifecycle-service.ts @@ -108,6 +108,25 @@ export interface LifecycleObjectLike { name: string; lifecycle?: Lifecycle; fields?: Record; + /** + * [#16729] The object's tenancy posture, DECLARED here because the Archiver + * hands this very object to a driver that reads the key + * (`cold.syncSchema(object, obj)` below), and every driver resolves a + * uniqueness partition from it: `tenancy.enabled: false` means one row per + * INSTALL, its absence means one row per organization. + * + * The registry objects that reach the Archiver at runtime carry the block, so + * the pass-through already worked — but this type is PUBLISHED, and without + * the key an author writing a fresh `LifecycleObjectLike` literal (a test + * double, an embedder's registry) is refused by the type for spelling + * `tenancy` and therefore omits it. The object then reaches `syncSchema` as + * the `{ name, fields }` shape, which is exactly the partial re-registration + * `SqlDriver.computeAndRecordTenantField`'s sticky record exists to survive. + * Declaring the key is the same correction #16711 made where the shard leaf + * narrowed `indexes` and `tenancy` off the object it was handed: a type must + * not refuse a key the code below it reads. + */ + tenancy?: { enabled?: boolean; tenantField?: string } | null; } export interface LifecycleLoggerLike {