diff --git a/.changeset/meta-object-tenant-index-convergence.md b/.changeset/meta-object-tenant-index-convergence.md new file mode 100644 index 0000000000..b9b0c56085 --- /dev/null +++ b/.changeset/meta-object-tenant-index-convergence.md @@ -0,0 +1,31 @@ +--- +"@objectstack/objectql": patch +--- + +fix(objectql): `GET /meta/object/:name` serves the multi-tenant tenant-scope index (#8375) + +On a multi-tenant deployment the registry stamps `indexes: [{ fields: +['organization_id'] }]` onto every object it materializes, but the by-name +`/meta` read served the same object with **no `indexes` key at all** whenever the +answer came from the `metadata` service or a `sys_metadata` overlay row. Same +object, same moment, same host — the list read reported the index and the by-name +read denied it. + +`indexes` is not decoration: a consumer reading that answer concludes the object +has no tenant index, which is the input to migration planning, to index-advice +tooling and to any consumer reasoning about query cost. The platform does create +the index; only this read denied it. + +The cause was a second implementation rather than a missing line. The read exits +converge the injected system columns with `applyInjectedSystemColumns` +(`@objectstack/metadata-core`), which cannot import the producer +(`applySystemFields`, `@objectstack/objectql`) without running up the dependency +graph — so it re-implemented the half it could reach, the fields map, and +silently omitted the index. The fix deletes that split: the decision is now one +function called by the producer and by the registry's object-materialization +seam, which every read exit already replays, so the two answers are one answer. + +The write path takes it back off again, exactness-bounded, so the standard Studio +GET → edit → PUT still stores a byte-identical body: an author's own tenant index +— named, ordered before their others, or declared on a single-tenant deployment +where the platform would add none — survives the round trip untouched. diff --git a/packages/objectql/src/meta-object-tenant-index-roundtrip.test.ts b/packages/objectql/src/meta-object-tenant-index-roundtrip.test.ts new file mode 100644 index 0000000000..02de5125c3 --- /dev/null +++ b/packages/objectql/src/meta-object-tenant-index-roundtrip.test.ts @@ -0,0 +1,298 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#8375] The write path takes back exactly what the converged read added + * (#4326) — the multi-tenant `indexes` half. + * + * `packages/rest/src/meta-object-materialization-agreement.test.ts` pins the + * READ: `GET /meta/object/:name` now serves the tenant-scope index the registry + * stamps, so an object answered from the `metadata` service or a `sys_metadata` + * overlay row no longer reports that a walled deployment has no index on its + * hottest predicate. That convergence writes a real, authorable key onto bodies + * that did not have one, and the write path persists a request body VERBATIM by + * design (ADR-0005 §Validation) — so without a strip counterpart the ordinary + * Studio GET → edit → PUT bakes a platform-computed index into + * `sys_metadata.metadata`, into its checksum, and into every history diff. + * + * ## Why this file exists rather than one more case in the `nameField` pin + * + * `indexes` is the first CONCATENATING key to cross this seam, and that changes + * the shape of the risk rather than repeating it. `nameField` is a scalar and + * `fields` is keyed by name, so a re-added stamp overwrites its predecessor and + * the worst case is a wrong value. A list under `mergeObjectDefinitions` + * accumulates: a strip that is not exactness-bounded leaves the entry in the + * stored row, and every actor that concatenates over that row — the extender + * fold, an overlay merge — is then working from a base that already contains + * what it is about to contribute. + * + * So the measurement that matters here is not "does one round trip come back + * clean" but "does the list stay the same length across TWO of them, with the + * stored row unchanged". A strip that never fires and a strip that is bounded + * are indistinguishable on a single cycle read only at the served document — + * both serve one entry. They differ in the ROW, immediately, and in the list + * length as soon as anything concatenates. + * + * ## The boundary + * + * The strip removes the LAST entry identical to the platform's own and keeps the + * removal only when re-stamping the remainder reproduces the arriving list + * byte-for-byte (see `stripProvisionedTenantIndexFrom`). The cases below are the + * four that boundary has to separate, and each is a real authoring shape: a + * named entry, an author's own tenant index sitting before their others, the + * same entry on a SINGLE-TENANT deployment where the seam would add nothing at + * all, and an object that opts out of tenancy entirely. + * + * Lives in this package for the reason its two siblings do: the claim is about + * the REAL `SchemaRegistry` and the REAL protocol write agreeing, and only this + * package has both — `@objectstack/objectql` depends on + * `@objectstack/metadata-protocol`, never the reverse. + */ + +import { describe, it, expect } from 'vitest'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +// [#5619] The producer's OWN write-verb dispatch decisions, so the fake engine +// below cannot accept a call ObjectQL refuses. +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch } from '@objectstack/metadata-core'; +import { SchemaRegistry } from './registry.js'; + +interface Row { + id: string; + type: string; + name: string; + organization_id: string | null; + package_id: string | null; + state: string; + metadata: string; + checksum?: string; + version?: number; +} + +/** The platform's own entry — the exact value the seam appends. */ +const PLATFORM_TENANT_INDEX = { fields: ['organization_id'] }; + +/** A plain business object: one authored field, no indexes of its own. */ +const AUTHORED = { + name: 'crm_lead', + label: 'Lead', + fields: { + name: { name: 'name', label: 'Name', type: 'text' }, + code_label: { name: 'code_label', label: 'Code label', type: 'text' }, + }, +}; + +const clone = (v: T): T => JSON.parse(JSON.stringify(v)) as T; + +function matches(r: Row, where: Record): boolean { + for (const [k, v] of Object.entries(where)) { + if (v === undefined) continue; + if ((r as any)[k] !== v) return false; + } + return true; +} + +function keyOf(w: Record) { + return `${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}|${w.package_id ?? '__nopkg__'}`; +} + +/** A `sys_metadata` store over a REAL {@link SchemaRegistry}. */ +function makeHost(multiTenant: boolean) { + // Companions OFF: this file is about the tenant index, and leaving the other + // stamps' deployment gate out keeps a failure here unambiguous about which + // half moved. The title designation still travels — it is not gated. + const registry = new SchemaRegistry({ multiTenant, searchCompanion: false } as never); + const rows = new Map(); + let nextId = 0; + const findRow = (w: Record) => { + for (const [k, r] of rows) if (matches(r, w)) return { key: k, row: r }; + return null; + }; + const engine: any = { + registry, + async findOne(_t: string, o: { where: Record }) { + return findRow(o.where)?.row ?? null; + }, + async find(_t: string, o: { where: Record }) { + return Array.from(rows.values()).filter((r) => matches(r, o.where)); + }, + async insert(table: string, data: Record) { + if (table !== 'sys_metadata') return { id: 'side_table' }; + const row = { id: `r_${++nextId}`, ...(data as any) } as Row; + rows.set(keyOf(data), row); + return { id: row.id }; + }, + async update(table: string, data: Record, o: { where: Record }) { + assertEngineUpdateDispatch(data, o); + if (table !== 'sys_metadata') return { id: null }; + const found = findRow(o.where); + if (!found) return { id: null }; + const merged = { ...found.row, ...(data as any) }; + rows.delete(found.key); + rows.set(keyOf(merged), merged); + return { id: found.row.id }; + }, + async delete(_t: string, o?: Record) { + assertEngineDeleteDispatch(o); + return { deleted: 0 }; + }, + async transaction(cb: (c: any, i: { owned: boolean }) => Promise): Promise { + return cb(undefined, { owned: true }); + }, + async syncObjectSchema() { /* no DDL in this stub */ }, + async count() { return 0; }, + async aggregate() { return []; }, + }; + const protocol = new ObjectStackProtocolImplementation(engine as never, () => new Map() as never); + const row = () => Array.from(rows.values()).find((r) => r.name === AUTHORED.name); + const storedBody = () => { + const r = row(); + return r ? (JSON.parse(r.metadata) as Record) : undefined; + }; + return { protocol, rows, registry, storedBody, row }; +} + +async function seed(multiTenant: boolean, item: Record = clone(AUTHORED)) { + const host = makeHost(multiTenant); + await host.protocol.saveMetaItem({ type: 'object', name: AUTHORED.name, item } as never); + return host; +} + +/** The served document, as a client actually holds it. */ +async function served(host: { protocol: ObjectStackProtocolImplementation }) { + return (await host.protocol.getMetaItem({ + type: 'object', name: AUTHORED.name, + } as never)).item as any; +} + +describe('[#8375] the write path takes back the tenant index the read added (#4326)', () => { + it('GET → PUT → GET → PUT: the list does not grow, and the row never carries the stamp', async () => { + // The anti-vacuity arm of this file. One cycle cannot separate a strip + // that is exactness-bounded from one that never fires — both SERVE a + // single entry. Two cycles plus the stored row can. + const host = await seed(true); + const firstStored = host.storedBody()!; + // Precondition: the author's own row never carried an index at all. + expect(firstStored.indexes).toBeUndefined(); + + for (const cycle of [1, 2]) { + const item = await served(host); + // The read really does add it — non-vacuous on every cycle, not + // only the first. + expect(item.indexes, `cycle ${cycle} served`).toEqual([PLATFORM_TENANT_INDEX]); + // …and adds exactly ONE, however many times we have been round. + expect(item.indexes.length, `cycle ${cycle} length`).toBe(1); + + await host.protocol.saveMetaItem({ + type: 'object', name: AUTHORED.name, item, + } as never); + + // The row is where duplication would accumulate, and it is the + // assertion a served-document check cannot make for you. + expect(host.storedBody()!.indexes, `cycle ${cycle} stored`).toBeUndefined(); + expect(host.storedBody(), `cycle ${cycle} body`).toEqual(firstStored); + } + }); + + it('a round-trip with NO edit leaves the stored body and its checksum identical', async () => { + const host = await seed(true); + const firstStored = host.storedBody()!; + const firstChecksum = host.row()!.checksum; + + await host.protocol.saveMetaItem({ + type: 'object', name: AUTHORED.name, item: await served(host), + } as never); + + expect(host.storedBody()).toEqual(firstStored); + // The checksum is the half a byte-identity assertion can still miss — + // it is what history diffs and change detection read. + expect(host.row()!.checksum).toBe(firstChecksum); + }); + + it('an EDIT round-trip stores the edit and nothing else', async () => { + const host = await seed(true); + const firstStored = host.storedBody()!; + + const item = await served(host); + await host.protocol.saveMetaItem({ + type: 'object', name: AUTHORED.name, item: { ...item, label: 'Lead (edited)' }, + } as never); + + const stored = host.storedBody()!; + expect(stored.label).toBe('Lead (edited)'); + expect(stored.indexes).toBeUndefined(); + // Everything except the edited key is byte-identical to the first save. + expect({ ...stored, label: firstStored.label }).toEqual(firstStored); + }); + + // ── The boundary that makes the strip safe ────────────────────────────── + + it('KEEPS an author’s NAMED tenant index — never a candidate for the strip', async () => { + // A named entry is not the value the seam appends, so the seam leaves it + // alone on the way out (`declaresTenantIndex` already covers the single + // organization_id column, named or not) and the strip never considers + // it on the way in. + const authored = { ...clone(AUTHORED), indexes: [{ name: 'my_tenant_idx', fields: ['organization_id'] }] }; + const host = await seed(true, authored); + expect(host.storedBody()!.indexes).toEqual(authored.indexes); + + const item = await served(host); + // The read adds nothing: the object already declares a tenant index. + expect(item.indexes).toEqual(authored.indexes); + + await host.protocol.saveMetaItem({ type: 'object', name: AUTHORED.name, item } as never); + expect(host.storedBody()!.indexes).toEqual(authored.indexes); + }); + + it('KEEPS an author’s own tenant index declared BEFORE their other indexes', async () => { + // The ORDER case, and the reason the strip compares whole lists rather + // than asking "is there a matching entry". The author's tenant index is + // byte-identical to the platform's, but it is not where the seam APPENDS + // — so re-stamping the remainder produces a different list and the + // removal is refused. + const authored = { + ...clone(AUTHORED), + indexes: [{ fields: ['organization_id'] }, { fields: ['code_label'] }], + }; + const host = await seed(true, authored); + expect(host.storedBody()!.indexes).toEqual(authored.indexes); + + const item = await served(host); + expect(item.indexes).toEqual(authored.indexes); + + await host.protocol.saveMetaItem({ type: 'object', name: AUTHORED.name, item } as never); + expect(host.storedBody()!.indexes).toEqual(authored.indexes); + }); + + it('KEEPS the identical entry on a SINGLE-TENANT deployment — the seam adds nothing there', async () => { + // The control that separates "bounded" from "removes anything that + // looks like the platform's entry". These are the same BYTES as the + // stamp; what differs is that on this deployment the seam would never + // have produced them, so re-stamping cannot reproduce the list. + const authored = { ...clone(AUTHORED), indexes: [{ fields: ['organization_id'] }] }; + const host = await seed(false, authored); + expect(host.storedBody()!.indexes).toEqual(authored.indexes); + + const item = await served(host); + // …and the read adds no second copy either. + expect(item.indexes).toEqual(authored.indexes); + + await host.protocol.saveMetaItem({ type: 'object', name: AUTHORED.name, item } as never); + expect(host.storedBody()!.indexes).toEqual(authored.indexes); + }); + + it('adds and strips NOTHING on an object that opts out of the tenant column', async () => { + // The stamp is gated on the spec's own derivation, not on the + // deployment flag alone: `systemFields.tenant: false` withholds the + // index exactly as it withholds the column, on a multi-tenant host. + const opted = { ...clone(AUTHORED), systemFields: { tenant: false } }; + const host = await seed(true, opted); + const firstStored = host.storedBody()!; + expect(firstStored.indexes).toBeUndefined(); + + const item = await served(host); + expect(item.indexes).toBeUndefined(); + expect(Object.keys(item.fields)).not.toContain('organization_id'); + + await host.protocol.saveMetaItem({ type: 'object', name: AUTHORED.name, item } as never); + expect(host.storedBody()).toEqual(firstStored); + }); +}); diff --git a/packages/objectql/src/protocol-meta-effective-schema.test.ts b/packages/objectql/src/protocol-meta-effective-schema.test.ts index 6f6977cb68..42fe1643da 100644 --- a/packages/objectql/src/protocol-meta-effective-schema.test.ts +++ b/packages/objectql/src/protocol-meta-effective-schema.test.ts @@ -286,16 +286,26 @@ describe.each([true, false])('[#6562] /meta object read — effective schema (mu ); expect(tenantIndexes(registryBacked)).toEqual(multiTenant ? [{ fields: ['organization_id'] }] : []); - // The one residual this fix leaves, recorded rather than left to be - // rediscovered: the DECLARATION does not converge the way the field set - // does. `divergences()` above compares fields, and the overlay-backed - // answer is rebuilt from the stored body, which declares no indexes. It - // is inert on this surface — a driver materializes from the REGISTERED - // schema, never from a served document (the same reasoning #6562 used to - // leave the flag at the injection site), and both answers parse green - // either way. If a served-document consumer of `indexes[]` ever appears, - // this is the line that says so. - expect(tenantIndexes(overlayBacked)).toEqual([]); + // [#8375 — FLIPPED, deliberately] This read `.toEqual([])`, under a note + // recording the residual #6562/#6810 left: "the DECLARATION does not + // converge the way the field set does… If a served-document consumer of + // `indexes[]` ever appears, this is the line that says so." + // + // It appeared, and the line said so. `GET /meta/object/:name` IS a + // served-document consumer of `indexes[]`: a caller reading the + // overlay-backed answer concluded the object has no tenant index, which + // is the input to migration planning, to index-advice tooling and to any + // consumer reasoning about query cost — while the platform does create + // the index and only this read denied it. + // + // What converges it is the registry's own materialization seam replayed + // onto the served body (`materializeServedObjectOnto`, #8268), so the two + // answers are ONE answer rather than two derivations that happen to + // agree — which is why the assertion is written against the registry's + // answer first and the literal second. + expect(tenantIndexes(overlayBacked)).toEqual(tenantIndexes(registryBacked)); + expect(tenantIndexes(overlayBacked)) + .toEqual(multiTenant ? [{ fields: ['organization_id'] }] : []); }); it('the served correction never becomes a phantom customization', async () => { diff --git a/packages/objectql/src/registry.ts b/packages/objectql/src/registry.ts index 379df68a98..1954a95a1d 100644 --- a/packages/objectql/src/registry.ts +++ b/packages/objectql/src/registry.ts @@ -1,6 +1,6 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. -import { ServiceObject, ObjectSchema, ObjectOwnership, provisionPrimary, resolveInjectedSystemColumns, checkManagedApiMethodAffordances, LEGACY_API_METHODS, AUDIT_PROVENANCE_FIELDS } from '@objectstack/spec/data'; +import { ServiceObject, ObjectSchema, ObjectOwnership, provisionPrimary, resolveInjectedSystemColumns, isInjectedColumnDefinition, checkManagedApiMethodAffordances, LEGACY_API_METHODS, AUDIT_PROVENANCE_FIELDS } from '@objectstack/spec/data'; // [#4513] The audit-family governance table, and [#6562] the injected-column // DEFINITION tables it governs — see the re-exports below for why both live in a // package `objectql` and `metadata-protocol` both depend on. @@ -491,9 +491,6 @@ export function applySystemFields( // Platform-owned field settings that must WIN over a declared field, rather // than lose to it like `additions` does (#4447). const overrides: Record = {}; - // Platform-owned index declarations, appended to the object's `indexes[]` — - // the ONE surface an index is declared on in this system (#6810, below). - const indexAdditions: Array<{ fields: string[] }> = []; if (wantTenant && !schema.fields?.organization_id) { // [#6562] The authorable shape is the shared table's, spread verbatim. @@ -517,28 +514,10 @@ export function applySystemFields( // invalid-metadata banners from. additions.organization_id = { ...TENANT_SCOPE_FIELD_DEF }; - // [#6810] So the tenant index is declared where every other index in this - // system is declared: the object's `indexes[]`. - // - // This is also the first time the intent is actually ENFORCED. The sole - // reader of the old flag was one line in `driver-mongodb` - // (`mongodb-schema.ts`), while `driver-sql` — which every walled deployment - // runs — only ever materialized `indexes[]`, so the wall's hottest predicate - // ran unindexed no matter what the flag said. - // - // No `name`: each driver derives its own (SQL's `buildIndexName` is - // table-qualified, which a hardcoded name could not be without colliding - // across tables on Postgres; Mongo's index names are per-collection). - // `unique` is left at its default `false` — a plain lookup index, never a - // constraint. - // - // `multiTenant: false` declares NO index rather than a false one: on an - // unwalled stack nothing filters by organization, so the index is dead - // weight — the same intent the old flag's value carried, expressed as - // presence instead of a boolean. - if (opts.multiTenant && !declaresTenantIndex(schema)) { - indexAdditions.push({ fields: ['organization_id'] }); - } + // [#6810] The tenant INDEX that goes with this column is no longer decided + // here. It is {@link provisionTenantScopeIndex}, called on the merged result + // at the tail of this function — see #8375 for why a decision spelled out at + // this one site could not be reached by the `/meta` read exits. } if (wantAudit) { @@ -620,25 +599,131 @@ export function applySystemFields( additions[OWNING_BUSINESS_UNIT_FIELD] = { ...OWNING_BUSINESS_UNIT_FIELD_DEF }; } - if ( - Object.keys(additions).length === 0 && - Object.keys(overrides).length === 0 && - indexAdditions.length === 0 - ) { - return schema; + // [#8375] The tenant INDEX is decided over the MERGED result, never over the + // input — so this function and the read-exit seam ask the same question of the + // same document shape. Both cases below route through it, including the one + // that adds no field at all (an object whose every system column is already + // declared still owes the index decision). + if (Object.keys(additions).length === 0 && Object.keys(overrides).length === 0) { + return provisionTenantScopeIndex(schema, opts); } + return provisionTenantScopeIndex( + { + ...schema, + // `additions` LOSE to an author's field (a declared `owner_id` is theirs); + // `overrides` WIN over it (the audit family's governance is not authorable). + fields: { ...additions, ...(schema.fields ?? {}), ...overrides }, + }, + opts, + ); +} + +/** + * [#6810, made shareable by #8375] Declare the platform's tenant-scope index on + * an object that carries a platform-provisioned `organization_id`, on a + * multi-tenant deployment — the ONE implementation of that decision. + * + * ## Why it is a function and not four lines inside `applySystemFields` + * + * It was those four lines, and that is precisely how the stamp diverged. The + * `/meta` read exits converge the injected system COLUMNS through + * `applyInjectedSystemColumns` (`@objectstack/metadata-core`, #6562) — which + * cannot reach `applySystemFields`: `@objectstack/objectql` depends on + * `@objectstack/metadata-protocol` depends on `@objectstack/metadata-core`, so + * the import runs UP the dependency graph. So the converger re-implemented the + * half it could reach (the fields map) and silently did not implement the half + * it could not (this index), and `GET /meta/object/:name` served a multi-tenant + * object reporting no tenant index while the registry's own answer carried one + * (#8375). + * + * The fix is not an `indexes` line added to that copy — a second implementation + * is what drifted in the first place. It is this: ONE function, called by the + * producer below and by {@link SchemaRegistry.materializeBaseLayer}, which is + * the seam #8268 built so a stamp converges at every read exit the day it is + * added. `__search` converges by delegating to the registry and has never + * drifted; this stamp now converges the same way. + * + * ## The predicate, stated exactly + * + * "The object carries a tenant column the PLATFORM provisioned" — not "this call + * just injected one". The distinction is the whole reason the decision moved out + * of the injection branch: at a read exit the column is ALREADY present + * (`governServedItem` runs `applyInjectedSystemColumns` before the seam), so a + * condition reading `!schema.fields?.organization_id` is false exactly where the + * convergence is needed. `isInjectedColumnDefinition` answers the question the + * old nesting was really asking, at either site, in either order. + * + * An author-declared `organization_id` of the author's own shape therefore still + * gets NO platform index, exactly as before. The one behaviour this widens is an + * author who declares the column BYTE-IDENTICAL to `TENANT_SCOPE_FIELD_DEF` + * (all eight keys, same values) and then gets the index — accepted for the + * reason `stripProvisionedSearchCompanionFrom` accepts its twin: the two are + * indistinguishable by construction, and the platform's own answer for that + * column is the one being served either way. + * + * `multiTenant: false` declares NO index rather than a false one: on an unwalled + * stack nothing filters by organization, so the index is dead weight — the same + * intent the retired `indexed` flag's value carried (#6810), expressed as + * presence instead of a boolean. + * + * No `name` on the entry: each driver derives its own (SQL's `buildIndexName` is + * table-qualified, which a hardcoded name could not be without colliding across + * tables on Postgres; Mongo's index names are per-collection). `unique` is left + * at its default `false` — a plain lookup index, never a constraint. + * + * Idempotent, via {@link declaresTenantIndex}: it runs twice on every + * registration (once at the tail of `applySystemFields`, once at the seam) and + * the second call is a no-op. Returns `schema` BY REFERENCE when nothing is + * owed, so a registry-sourced body pays a comparison and no copy. + */ + +/** + * [#8375] The index entry the platform declares for the tenant scope column — + * the ONE spelling of it, so the stamp and its write-side strip cannot disagree + * about what "the platform's own entry" looks like. + */ +const TENANT_SCOPE_INDEX: { fields: string[] } = { fields: ['organization_id'] }; + +function provisionTenantScopeIndex( + schema: ServiceObject, + opts: { multiTenant: boolean }, +): ServiceObject { + if (!opts.multiTenant) return schema; + if (!platformOwnsTenantColumn(schema)) return schema; + if (declaresTenantIndex(schema)) return schema; + return { ...schema, - // `additions` LOSE to an author's field (a declared `owner_id` is theirs); - // `overrides` WIN over it (the audit family's governance is not authorable). - fields: { ...additions, ...(schema.fields ?? {}), ...overrides }, // [#6810] Author-declared indexes keep their position; the platform's // tenant index is APPENDED, never merged into or reordering theirs. - ...(indexAdditions.length > 0 - ? { indexes: [...((schema as any).indexes ?? []), ...indexAdditions] } - : {}), - }; + // The write-side inverse depends on that append being at the END — + // see {@link SchemaRegistry.stripProvisionedTenantIndexFrom}. + indexes: [...((schema as any).indexes ?? []), { ...TENANT_SCOPE_INDEX }], + } as ServiceObject; +} + +/** + * [#8375] Is this object's `organization_id` the PLATFORM's column rather than + * one the author declared? + * + * Two independent conditions, and both are load-bearing: + * + * - the object must carry a tenant column at all — the spec's own derivation + * (`resolveInjectedSystemColumns`, #5378), so `systemFields: false`, + * `systemFields.tenant: false`, `tenancy.enabled: false` and + * `managedBy: 'better-auth'` all withhold the index exactly as they withhold + * the column. Re-deriving any of those rows here is the drift that plan + * exists to prevent; + * - the column present must be the platform's own definition, or absent + * (the pre-injection shape). `isInjectedColumnDefinition` compares against + * the shipped table byte-for-byte, which is the same exactness + * `stripInjectedSystemColumns` uses to decide the same question in reverse. + */ +function platformOwnsTenantColumn(schema: ServiceObject): boolean { + if (!resolveInjectedSystemColumns(schema).tenant) return false; + const declared = (schema.fields as Record | undefined)?.organization_id; + return declared === undefined || isInjectedColumnDefinition(declared, TENANT_SCOPE_FIELD_DEF); } /** @@ -1613,6 +1698,15 @@ export class SchemaRegistry { out = provisionSearchCompanion(out); } + // [#8375] The tenant-scope index — the fourth stamp of this seam, and the + // first one added AFTER #8268 generalised it. It needed no new method here, + // no new method in the protocol and no new convergence at the read exits: + // being a step of this block IS the convergence, which is the property + // #8268 exists to provide. Runs LAST because it is the only stamp gated on + // a DEPLOYMENT flag rather than on the document, so nothing it appends can + // change the answer the two steps above reached over the fields map. + out = provisionTenantScopeIndex(out, { multiTenant: this.multiTenant }); + return out; } @@ -1794,18 +1888,98 @@ export class SchemaRegistry { * regression that a landed round-trip pin catches, and this card measured * exactly that before adding this method. * - * ⛔ Strips in the REVERSE of the seam's order — companion first, then the - * title designation — because the companion's canonical definition is - * recomputed over a body that still carries the pointer, which is the state - * it was stamped in. Stripping the pointer first would ask - * `provisionSearchCompanion` a different question than the one it was + * ⛔ Strips in the REVERSE of the seam's order — tenant index first, then the + * companion, then the title designation — because each strip recomputes its + * canonical answer over a body that still carries every stamp applied AFTER + * it, which is the state it was stamped in. Stripping the pointer first would + * ask `provisionSearchCompanion` a different question than the one it was * answered with. * * Returns `base` by reference when nothing was owed. */ stripMaterializedStampsFrom(base: T): T { if (base === null || typeof base !== 'object') return base; - return this.stripProvisionedPrimaryFrom(this.stripProvisionedSearchCompanionFrom(base)); + return this.stripProvisionedPrimaryFrom( + this.stripProvisionedSearchCompanionFrom(this.stripProvisionedTenantIndexFrom(base)), + ); + } + + /** + * [#8375] Remove the tenant-scope index entry {@link materializeBaseLayer} + * appended — and only that one. + * + * ## Why this strip is harder than the two beside it + * + * `indexes` is a CONCATENATING key under `mergeObjectDefinitions`, not a + * last-writer-wins scalar like `nameField` or a keyed record like `fields`. + * The two existing strips can ask "is this value the platform's?" of a single + * slot; here the platform's entry sits in a list beside the author's, entries + * may legitimately repeat, and a strip that removes "any entry that looks like + * the platform's" silently deletes an author's own declaration, while a strip + * that removes nothing bakes a phantom index into `sys_metadata.metadata`, its + * checksum and every history diff on the first Studio GET → edit → PUT + * (#4326). + * + * ## The bound, and why it is exact in BOTH directions + * + * Not "does an entry match" but "would the seam have produced EXACTLY this + * list": remove the LAST entry identical to the platform's own (the seam + * APPENDS, so its entry is the last matching one), re-stamp the remainder + * through `provisionTenantScopeIndex` — the stamping function itself, so the + * strip and the stamp cannot drift — and keep the removal only when the + * re-stamped list is byte-identical to the list that arrived. Anything else + * returns `base` untouched. + * + * That single comparison decides every case correctly, and each of these was + * measured: + * + * - the platform's appended entry on a body with no `indexes` of its own → + * removed, and the KEY is deleted rather than left as `indexes: []`, so the + * round trip is byte-identical and not merely equivalent; + * - an author's own `{ fields: ['organization_id'] }` on a SINGLE-TENANT + * deployment → the re-stamp adds nothing, the lists differ, kept; + * - an author's own tenant index declared BEFORE their other indexes → the + * re-stamp appends at the end, the ORDER differs, kept; + * - a named entry (`{ name: 'my_tenant_idx', fields: [...] }`) or a composite + * → not identical to the platform's entry, never a candidate, kept. + * + * ⚠️ The one case it cannot separate, stated plainly because it is the same + * trade {@link stripProvisionedPrimaryFrom} documents: an author who declares + * `{ fields: ['organization_id'] }` as their LAST index on a MULTI-TENANT + * deployment has written exactly what the seam appends, so it is dropped on + * the first save that carries it. What bounds the harm is identical: the entry + * is RE-DERIVED at every load — `registerObject` runs this seam over every + * base layer — so the resolved answer, and the index the driver materializes, + * is the same with or without the stored entry, now and at every future boot. + * + * Returns `base` by reference when nothing was owed. + */ + private stripProvisionedTenantIndexFrom(base: T): T { + if (base === null || typeof base !== 'object') return base; + const present = (base as { indexes?: unknown }).indexes; + if (!Array.isArray(present) || present.length === 0) return base; + + const platformEntry = stableStringify(TENANT_SCOPE_INDEX); + let at = -1; + for (let i = present.length - 1; i >= 0; i--) { + if (stableStringify(present[i]) === platformEntry) { at = i; break; } + } + if (at === -1) return base; + + const kept = present.filter((_entry, i) => i !== at); + const without = { ...(base as Record) }; + // An emptied list is the ABSENCE of the key, not an empty array: the seam + // added `indexes` to a body that had none, so the inverse must take the key + // away again or the round trip stores a shape the author never wrote. + if (kept.length === 0) delete without.indexes; + else without.indexes = kept; + + const restamped = provisionTenantScopeIndex(without as never, { + multiTenant: this.multiTenant, + }) as { indexes?: unknown }; + if (stableStringify(restamped.indexes) !== stableStringify(present)) return base; + + return without as unknown as T; } /** diff --git a/packages/rest/src/meta-object-materialization-agreement.test.ts b/packages/rest/src/meta-object-materialization-agreement.test.ts index ad199d4837..97363e9c0a 100644 --- a/packages/rest/src/meta-object-materialization-agreement.test.ts +++ b/packages/rest/src/meta-object-materialization-agreement.test.ts @@ -46,25 +46,34 @@ // (2026-08-08, Option B): the read serves the EFFECTIVE runtime schema and the // stored-layer minority converges on the registry-backed majority. // -// ## Two divergences this file MEASURES but does not fix +// ## The stamp this file MEASURED as diverging, and now pins as converged // -// Both were found by diffing whole keys rather than the fields map, both are -// out of this card's region, and both are filed. They are pinned here as -// EXPECTED so that the day either is fixed, this file fails and is updated -// deliberately rather than silently drifting: +// [#8375] `indexes` — on a MULTI-TENANT deployment the registry also stamps +// `indexes: [{ fields: ['organization_id'] }]`, and the read exit's +// `applyInjectedSystemColumns` converged the FIELDS MAP only. It was the fourth +// stamp of this seam and the one whose converger was a SECOND IMPLEMENTATION of +// its producer rather than a delegation to it — `applyInjectedSystemColumns` +// lives in `@objectstack/metadata-core`, which cannot import the producer +// (`applySystemFields`, `@objectstack/objectql`) without running UP the +// dependency graph, so it re-implemented the half it could reach and silently +// omitted the half it could not. // -// 1. [#8375] `indexes` — on a MULTI-TENANT deployment `applySystemFields` also -// stamps `indexes: [{ fields: ['organization_id'] }]`, and the read exit's -// `applyInjectedSystemColumns` converges the FIELDS MAP only. A fourth -// stamp of the same seam, in `metadata-core`, and the one whose converger -// is a SECOND IMPLEMENTATION of its producer rather than a delegation to it. -// 2. [#8376] `__search` on an extended title-less base — `registerObject` -// materializes the BASE and `resolveObject` folds `extend` contributors on -// afterwards WITHOUT re-materializing, while the read exits transform the -// ALREADY FOLDED document. So a base with no title-eligible field that an -// extension gives a text field to gets a companion from both `/meta` reads -// and none from the registry. A POSITION defect of the seam, not a stamp -// defect, live since #8038 and unchanged here. +// This file MEASURED that divergence as EXPECTED rather than fixing it, so the +// day it was fixed this file would fail and be updated deliberately instead of +// drifting. That is what happened: #8375 moved the decision into ONE function +// called by the producer and by `materializeBaseLayer` — the #8268 seam — and +// the case below now pins agreement plus the single-tenant control, without +// which "converged" would be indistinguishable from "always stamps an index". +// +// ## The divergence this file still MEASURES but does not fix +// +// [#8376] `__search` on an extended title-less base — `registerObject` +// materializes the BASE and `resolveObject` folds `extend` contributors on +// afterwards WITHOUT re-materializing, while the read exits transform the +// ALREADY FOLDED document. So a base with no title-eligible field that an +// extension gives a text field to gets a companion from both `/meta` reads and +// none from the registry. A POSITION defect of the seam, not a stamp defect, +// live since #8038 and unchanged here. import { describe, it, expect, vi } from 'vitest'; import { SchemaRegistry } from '@objectstack/objectql'; @@ -408,21 +417,53 @@ describe('[#8268] every /meta object read exit materializes the base the way the expect(host.byName?.[NAME_FIELD]).toBe('name'); }); - // ── The two divergences this card measured and did NOT fix ────────────── - // Pinned as EXPECTED so the day either is fixed this file fails and is - // updated deliberately. See the header for what each one is. + // ── The multi-tenant `indexes` stamp, converged by #8375 ──────────────── + // This case measured the divergence before #8375 and pins the agreement + // after it. See the header for what changed and why the fix was not an + // `indexes` line added to the converger. - it('MEASURES the un-converged multi-tenant `indexes` stamp — filed as #8375', async () => { + it('converges the multi-tenant `indexes` stamp — the seam’s fourth stamp (#8375)', async () => { const host = await measure({ serviceMode: 'artifact', multiTenant: true }); expectNonEmptyRead(host); - // The stamp the registry applies and the read exit does not. + // Non-vacuous in both directions: the service's copy genuinely lacks the + // stamp, so agreement cannot be reached by this host having had nothing + // to converge… + expect(host.serviceBody?.indexes).toBeUndefined(); + // …and the registry genuinely applies it, so it cannot be reached by the + // registry quietly dropping it either. expect(host.registryResolved?.indexes).toEqual([{ fields: ['organization_id'] }]); - expect(host.byName?.indexes).toBeUndefined(); - // `indexes` is the ONLY key still diverging on this host — `nameField` - // is converged, which is what this card changed. - expect(divergingKeys(host.byName, host.registryResolved)).toEqual(['indexes']); + // The convergence, named explicitly so a failure says WHICH stamp moved. + expect(host.byName?.indexes).toEqual([{ fields: ['organization_id'] }]); + + // …and at the level of the CLASS: no key diverges on this host at all. + // Before #8375 this read `toEqual(['indexes'])`. + expect(divergingKeys(host.byName, host.registryResolved)).toEqual([]); + expect(divergingKeys(host.byName, host.listed)).toEqual([]); + expect(divergingKeys(host.layerEffective, host.registryResolved)).toEqual([]); + }); + + it('stamps NO tenant index on a SINGLE-TENANT deployment — on either route', async () => { + // The control the convergence above is worthless without: a read exit + // that simply stamped an index on every object would pass every + // assertion in that case and fail every one of these. The index is + // deployment-gated (#6810: on an unwalled stack nothing filters by + // organization, so the index is dead weight), and the gate must survive + // being routed through the shared seam. + const host = await measure({ serviceMode: 'artifact', multiTenant: false }); + expectNonEmptyRead(host); + + // The tenant COLUMN still exists on both routes — only the index is + // gated, so this case cannot pass by the object having no tenancy at all. + expect(Object.keys((host.registryResolved?.fields ?? {}) as Record)) + .toContain('organization_id'); + expect(Object.keys((host.byName?.fields ?? {}) as Record)) + .toContain('organization_id'); + + expect(host.registryResolved?.indexes).toBeUndefined(); + expect(host.byName?.indexes).toBeUndefined(); + expect(divergingKeys(host.byName, host.registryResolved)).toEqual([]); }); it('MEASURES the companion over-provisioned on an extended title-less base — filed as #8376', async () => {