From 482d8470481ae8aa47d910db5655109f073583ce Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 20:17:21 +0000 Subject: [PATCH 1/2] fix(objectql,driver-mongodb): declare the tenant index in `indexes[]` (#6810) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `applySystemFields` provisioned the injected `organization_id` column with `indexed: opts.multiTenant`. `indexed` is not a `FieldSchema` key — #2377 / ADR-0049 removed it because a field-level index flag built no index — and `FieldSchema` is a `strictObject`, so a field carrying it is rejected by name. `registerObject` runs `applySystemFields` before storing and `getItem('object', …)` serves that post-injection document, so the key reached `/meta`, where `decorateMetadataItem` re-parsed the served body and stamped `_diagnostics: { valid: false, errors: [{ path: 'fields.organization_id', code: 'unrecognized_keys' }] }` on every registry-backed object — both tenancy modes, both read exits. That is the channel Studio renders invalid-metadata banners from and an AI author reads to judge its own document, so the platform was reporting a defect on its own column and drowning real authoring errors. The tenant index is now declared in the object's `indexes[]`, where every other index in this system is declared: `{ fields: ['organization_id'] }` on a multi-tenant stack, nothing at all on a single-tenant one (absence is what `indexed: false` meant). `driver-mongodb` — the sole reader of the retired flag — reads declared indexes instead, generating the same index name it used to, so a re-synced collection finds its existing `idx_organization_id`. `driver-sql` already materialized `indexes[]`, so this is the first time the intent is enforced there at all. No `FieldSchema` change: re-declaring `indexed` would restore exactly the declared-but-unenforced key #2377 removed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JE8Bbwb8qhau3yNtP3ftLJ --- .../tenant-index-declared-in-indexes.md | 62 ++++++ packages/drivers/driver-mongodb/README.md | 8 +- .../driver-mongodb/src/mongodb-driver.test.ts | 8 +- .../mongodb-schema-declared-indexes.test.ts | 159 +++++++++++++++ .../driver-mongodb/src/mongodb-schema.ts | 60 +++++- .../injected-system-columns-parity.test.ts | 8 +- .../src/registry-tenancy-posture.test.ts | 51 +++-- .../registry-tenant-index-declaration.test.ts | 192 ++++++++++++++++++ packages/objectql/src/registry.test.ts | 13 +- packages/objectql/src/registry.ts | 77 ++++++- 10 files changed, 607 insertions(+), 31 deletions(-) create mode 100644 .changeset/tenant-index-declared-in-indexes.md create mode 100644 packages/drivers/driver-mongodb/src/mongodb-schema-declared-indexes.test.ts create mode 100644 packages/objectql/src/registry-tenant-index-declaration.test.ts diff --git a/.changeset/tenant-index-declared-in-indexes.md b/.changeset/tenant-index-declared-in-indexes.md new file mode 100644 index 0000000000..50f0dd515a --- /dev/null +++ b/.changeset/tenant-index-declared-in-indexes.md @@ -0,0 +1,62 @@ +--- +"@objectstack/objectql": patch +"@objectstack/driver-mongodb": patch +--- + +fix(objectql,driver-mongodb): declare the tenant index in `indexes[]`, so a registry-backed object stops reporting itself invalid (#6810) + +`applySystemFields` provisioned the injected `organization_id` column with +`indexed: opts.multiTenant`. `indexed` is **not a `FieldSchema` key** — #2377 / +ADR-0049 removed it because a field-level index flag built no index — and +`FieldSchema` is a `strictObject`, so a field carrying it is rejected **by +name**, with a purpose-written message. + +`registerObject` runs `applySystemFields` *before* storing and +`getItem('object', …)` serves that post-injection document, so the key travelled +all the way out to `/meta`, where `decorateMetadataItem` re-parsed the served +body and stamped the verdict on it. Measured on every registry-backed object, in +**both** tenancy modes, at **both** read exits: + +``` +_diagnostics: { valid: false, + errors: [{ path: 'fields.organization_id', code: 'unrecognized_keys' }] } +``` + +`_diagnostics` is what Studio renders invalid-metadata banners from and what an +AI author reads to judge a document it produced. So the platform was reporting a +defect on its own column — one the author never wrote and could not fix — and +making the verdict useless as a signal on those objects, because a real +authoring error was indistinguishable from this one. + +**Two directions, both of them user-visible:** + +- **The false `valid: false` verdict is gone.** A tenancy-enabled object + registered through the real `SchemaRegistry` now reads back + `_diagnostics: { valid: true }` at both `/meta` exits, in both tenancy modes. + Nothing else about the served field changed — `type`, `reference`, and the + governance keys that decide who may write it are byte-identical. +- **The tenant index moved from a field-level flag to `indexes[]`**, the one + surface an index is declared on in this system. On a multi-tenant stack the + object now declares `{ fields: ['organization_id'] }`; on a single-tenant + stack it declares **nothing** — the absence *is* what `indexed: false` used to + say, since nothing filters by organization on an unwalled stack. + +This is also the first time the intent is actually **enforced**. The sole reader +of the old flag was one line in `driver-mongodb`; `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. Expect the tenant +index to now appear as ordinary index drift on existing SQL tables +(`idx__organization_id`), created by `os migrate apply` or by the +`autoMigrate: 'safe'` path in dev, like any other declared index. + +`driver-mongodb` reads declared `indexes[]` in place of the retired flag. The +generated index name matches the field-level convention already in that file +(`idx_` / `idx__unique`), so a re-synced collection finds its +existing `idx_organization_id` rather than building a second index under a new +name. Declarations are materialized over their columns **verbatim** at every +`unique` scope, `'organization'` included — the same call the driver's +field-level `unique` documents, because it implements no row-level tenancy and +refuses to boot into a multi-tenant deployment (#3724). + +No `FieldSchema` change: re-declaring `indexed` would restore exactly the +declared-but-unenforced key #2377 removed. diff --git a/packages/drivers/driver-mongodb/README.md b/packages/drivers/driver-mongodb/README.md index 102197a8b0..0ace28d9be 100644 --- a/packages/drivers/driver-mongodb/README.md +++ b/packages/drivers/driver-mongodb/README.md @@ -158,14 +158,20 @@ try { Schema sync creates collections and indexes: +Field-level `unique` and lookup fields index themselves; everything else is +declared in the object's `indexes[]` — the one surface an index is declared on +(a field-level `indexed` flag is not a `FieldSchema` key and never built an +index, #2377 / #6810). + ```typescript await driver.syncSchema('account', { name: 'account', fields: { name: { type: 'string', unique: true }, - email: { type: 'email', indexed: true }, + email: { type: 'email' }, company_id: { type: 'lookup', reference_to: 'company' }, }, + indexes: [{ fields: ['email'] }], }); // Creates: idx_id_unique, idx_name_unique, idx_email, idx_company_id_lookup ``` diff --git a/packages/drivers/driver-mongodb/src/mongodb-driver.test.ts b/packages/drivers/driver-mongodb/src/mongodb-driver.test.ts index efd0f76b86..7e26f41115 100644 --- a/packages/drivers/driver-mongodb/src/mongodb-driver.test.ts +++ b/packages/drivers/driver-mongodb/src/mongodb-driver.test.ts @@ -343,9 +343,15 @@ describe.skipIf(!sharedMongod)('MongoDBDriver', () => { name: 'account', fields: { name: { type: 'string', unique: true }, - email: { type: 'email', indexed: true }, + email: { type: 'email' }, company_id: { type: 'lookup', reference_to: 'company' }, }, + // [#6810] `email` used to carry a field-level `indexed: true` here. That + // was never a `FieldSchema` key (#2377 / ADR-0049); the index is + // declared in `indexes[]`, where every other index in this system is + // declared. Same resulting index name — the assertions are unchanged, + // which is the point. + indexes: [{ fields: ['email'] }], }); const db = driver.getDb(); diff --git a/packages/drivers/driver-mongodb/src/mongodb-schema-declared-indexes.test.ts b/packages/drivers/driver-mongodb/src/mongodb-schema-declared-indexes.test.ts new file mode 100644 index 0000000000..f9fb1a3ca8 --- /dev/null +++ b/packages/drivers/driver-mongodb/src/mongodb-schema-declared-indexes.test.ts @@ -0,0 +1,159 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #6810 — `syncCollectionSchema` materializes the object's DECLARED `indexes[]`. +// +// This driver used to read a field-level `indexed` flag instead. That flag was +// never a `FieldSchema` key (#2377 / ADR-0049 removed it, and `FieldSchema` is a +// `strictObject` that rejects it by name), so its one remaining producer — the +// kernel's `organization_id` injection — was stamping every registry-backed +// object with a document the platform's own schema refused. The declaration +// moved to `indexes[]`; this pins that the DDL outcome came with it, rather than +// the fix quietly deleting an index the flag used to build. +// +// Driven against a fake `Db` on purpose: the `mongodb-memory-server` suite in +// this package is OPT-IN (it downloads a real server binary, #5517), so a DDL +// assertion parked there would not run on any ordinary CI lane — which is +// exactly the lane that has to notice if this regresses. + +import { describe, it, expect } from 'vitest'; +import type { Db } from 'mongodb'; +import { syncCollectionSchema } from './mongodb-schema.js'; + +interface CreatedIndex { + spec: Record; + options: Record; +} + +/** + * The narrow slice of `Db` `syncCollectionSchema` touches, recording every + * `createIndex` call in order. Nothing is stubbed beyond that slice — the + * function under test runs verbatim. + */ +function fakeDb(existingCollections: string[] = []) { + const created: CreatedIndex[] = []; + const collectionsCreated: string[] = []; + const db = { + listCollections: ({ name }: { name: string }) => ({ + toArray: async () => (existingCollections.includes(name) ? [{ name }] : []), + }), + createCollection: async (name: string) => { + collectionsCreated.push(name); + }, + collection: () => ({ + createIndex: async (spec: Record, options: Record) => { + created.push({ spec, options }); + }, + }), + } as unknown as Db; + return { db, created, collectionsCreated }; +} + +/** Every index name the sync asked MongoDB to create, core indexes included. */ +const names = (created: CreatedIndex[]) => created.map((c) => c.options.name); + +/** The one recorded creation for `name`, or `undefined`. */ +const byName = (created: CreatedIndex[], name: string) => + created.find((c) => c.options.name === name); + +describe('#6810 — syncCollectionSchema materializes declared indexes[]', () => { + it('creates the kernel-declared tenant index, byte-identical to what the retired flag built', async () => { + // THE regression pin. Before #6810 this DDL came from + // `organization_id: { …, indexed: true }`; it now comes from the object's + // `indexes[]`. Same collection, same key spec, same index NAME — a + // re-synced deployment finds its index already present rather than + // building a second one under a new name. + const { db, created } = fakeDb(); + await syncCollectionSchema(db, 'lead', { + name: 'lead', + fields: { + first_name: { type: 'text' }, + organization_id: { type: 'lookup' }, + }, + indexes: [{ fields: ['organization_id'] }], + }); + + const tenant = byName(created, 'idx_organization_id'); + expect(tenant).toBeDefined(); + expect(tenant!.spec).toEqual({ organization_id: 1 }); + // A plain lookup index, never a constraint. + expect(tenant!.options.unique).toBeUndefined(); + }); + + it('declares no tenant index when the object declares none', async () => { + // The single-tenant half: `multiTenant: false` now declares NOTHING rather + // than a `false` flag, so absence has to stay absence here. + const { db, created } = fakeDb(); + await syncCollectionSchema(db, 'lead', { + name: 'lead', + fields: { first_name: { type: 'text' }, organization_id: { type: 'lookup' } }, + }); + + expect(names(created)).not.toContain('idx_organization_id'); + // The core set is untouched by any of this. + expect(names(created)).toEqual(['idx_id_unique', 'idx_created_at', 'idx_updated_at']); + }); + + it('honours an explicit index name and a multi-column declaration', async () => { + const { db, created } = fakeDb(); + await syncCollectionSchema(db, 'lead', { + name: 'lead', + fields: { organization_id: { type: 'lookup' }, code: { type: 'text' } }, + indexes: [{ name: 'lead_scope_idx', fields: ['organization_id', 'code'] }], + }); + + const idx = byName(created, 'lead_scope_idx'); + expect(idx).toBeDefined(); + // Key order follows the declaration — a compound index is order-sensitive. + expect(Object.keys(idx!.spec)).toEqual(['organization_id', 'code']); + }); + + it('materializes a declared unique index, at every scope, over the columns VERBATIM', async () => { + // `'organization'` is NOT scoped up with a tenant key part here, and that is + // deliberate — the same call `FieldDef.unique` documents in the source. + // This driver implements no row-level tenancy and refuses to boot into a + // multi-tenant deployment (#3724), so a `(tenant, field)` index would + // advertise an isolation it does not deliver. + for (const scope of [true, 'global', 'organization'] as const) { + const { db, created } = fakeDb(); + await syncCollectionSchema(db, 'lead', { + name: 'lead', + fields: { organization_id: { type: 'lookup' }, code: { type: 'text' } }, + indexes: [{ fields: ['code'], unique: scope }], + }); + + const idx = byName(created, 'idx_code_unique'); + expect(idx, String(scope)).toBeDefined(); + expect(idx!.spec).toEqual({ code: 1 }); + expect(idx!.options.unique).toBe(true); + expect(names(created)).not.toContain('idx_organization_id_code_unique'); + } + }); + + it('a declared index and a field-level `unique` on the same column converge on ONE index', async () => { + // Why the generated name mirrors the field-level convention: two routes + // asking for the same constraint must land on the same name, or MongoDB + // sees two indexes over one key spec. + const { db, created } = fakeDb(); + await syncCollectionSchema(db, 'lead', { + name: 'lead', + fields: { email: { type: 'email', unique: true } }, + indexes: [{ fields: ['email'], unique: true }], + }); + + expect(names(created).filter((n) => n === 'idx_email_unique')).toHaveLength(2); + const [a, b] = created.filter((c) => c.options.name === 'idx_email_unique'); + expect(a.spec).toEqual(b.spec); + expect(a.options).toEqual(b.options); + }); + + it('skips a declaration with no usable fields rather than emitting empty DDL', async () => { + const { db, created } = fakeDb(); + await syncCollectionSchema(db, 'lead', { + name: 'lead', + fields: { code: { type: 'text' } }, + indexes: [{ fields: [] }, { name: 'ghost' } as { name: string }], + }); + + expect(names(created)).toEqual(['idx_id_unique', 'idx_created_at', 'idx_updated_at']); + }); +}); diff --git a/packages/drivers/driver-mongodb/src/mongodb-schema.ts b/packages/drivers/driver-mongodb/src/mongodb-schema.ts index 0f1a7a2e97..7373499ec6 100644 --- a/packages/drivers/driver-mongodb/src/mongodb-schema.ts +++ b/packages/drivers/driver-mongodb/src/mongodb-schema.ts @@ -33,18 +33,37 @@ interface FieldDef { * `SqlDriver.uniqueIndexesFromFields`. */ unique?: boolean | 'global'; - indexed?: boolean; required?: boolean; reference_to?: string; multiple?: boolean; } +/** + * A declared object-level index — `IndexSchema` in @objectstack/spec, narrowed + * to what this driver materializes. + * + * [#6810] This is the ONE surface an index is declared on. The field-level + * `indexed` flag this driver used to read alongside it was never a + * `FieldSchema` key: #2377 / ADR-0049 removed it because a field-level index + * flag built no index, and `FieldSchema` — a `strictObject` — rejects it by + * name. The single producer still emitting it was the kernel's + * `organization_id` injection, which therefore stamped every registry-backed + * object with a document its own schema refused. That declaration moved to + * `indexes[]`; nothing else in the repo ever read the flag, so it is gone. + */ +interface IndexDef { + name?: string; + fields?: string[]; + unique?: boolean | 'global' | 'organization'; +} + /** * ObjectStack object definition (subset needed for schema sync). */ interface ObjectDef { name: string; fields?: Record; + indexes?: IndexDef[]; } /** @@ -53,8 +72,9 @@ interface ObjectDef { * - Creates the collection if it doesn't exist * - Creates a unique index on `id` * - Creates indexes on `created_at` and `updated_at` - * - Creates indexes for fields marked `unique` or `indexed` + * - Creates indexes for fields marked `unique` * - Creates indexes on lookup (reference) fields + * - Creates the object's DECLARED `indexes[]` (#6810) */ export async function syncCollectionSchema( db: Db, @@ -84,11 +104,6 @@ export async function syncCollectionSchema( spec: { [fieldName]: 1 }, options: { unique: true, sparse: true, name: `idx_${fieldName}_unique` }, }); - } else if (field.indexed) { - indexOps.push({ - spec: { [fieldName]: 1 }, - options: { name: `idx_${fieldName}` }, - }); } // Lookup + user (a lookup specialized to sys_user) fields get an index for @@ -106,6 +121,37 @@ export async function syncCollectionSchema( } } + // Declared object-level indexes (#6810) — the surface `indexes[]`, which is + // where every other index in this system is declared and where the kernel now + // declares the tenant index on `organization_id`. + // + // The generated name is `idx_` / `idx__unique`, matching the + // field-level convention above so the two routes converge on ONE index rather + // than racing to create two with different options on the same column set + // (Mongo index names are per-collection, so no table qualifier is needed — + // unlike `SqlDriver`'s `buildIndexName`, which is why neither driver takes a + // name from the declaration when the author left it out). + // + // Every `unique` scope materializes the columns VERBATIM, `'organization'` + // included. That is the same call `FieldDef.unique` documents above and for + // the same reason: this driver implements no row-level tenancy at all and + // refuses to boot into a multi-tenant deployment (#3724), so prepending a + // tenant key part would advertise an isolation it does not deliver. + for (const idx of schema.indexes ?? []) { + const fields = (idx.fields ?? []).filter((f) => typeof f === 'string' && f.length > 0); + if (fields.length === 0) continue; + const unique = Boolean(idx.unique); + const spec: Record = {}; + for (const f of fields) spec[f] = 1; + indexOps.push({ + spec: spec as IndexSpecification, + options: { + ...(unique ? { unique: true, sparse: true } : {}), + name: idx.name ?? `idx_${fields.join('_')}${unique ? '_unique' : ''}`, + }, + }); + } + // Create indexes (idempotent — MongoDB ignores duplicates) for (const { spec, options } of indexOps) { try { diff --git a/packages/objectql/src/injected-system-columns-parity.test.ts b/packages/objectql/src/injected-system-columns-parity.test.ts index b6959e10a6..35c90276ac 100644 --- a/packages/objectql/src/injected-system-columns-parity.test.ts +++ b/packages/objectql/src/injected-system-columns-parity.test.ts @@ -96,7 +96,11 @@ describe('[#5378] resolveInjectedSystemColumns ↔ applySystemFields parity', () } const mt = applySystemFields({ name: 'crm_contact', fields: fields() } as any, { multiTenant: true }); const st = applySystemFields({ name: 'crm_contact', fields: fields() } as any, { multiTenant: false }); - expect((mt.fields as any).organization_id.indexed).toBe(true); - expect((st.fields as any).organization_id.indexed).toBe(false); + // [#6810] The index moved from a field-level `indexed` boolean — never a + // `FieldSchema` key — to the object's `indexes[]`. The property this test + // guards is unchanged: the flag moves the INDEX, never the column set. + expect((mt as any).indexes).toEqual([{ fields: ['organization_id'] }]); + expect((st as any).indexes).toBeUndefined(); + expect((mt.fields as any).organization_id).toEqual((st.fields as any).organization_id); }); }); diff --git a/packages/objectql/src/registry-tenancy-posture.test.ts b/packages/objectql/src/registry-tenancy-posture.test.ts index 517b78fea2..436d67f8e5 100644 --- a/packages/objectql/src/registry-tenancy-posture.test.ts +++ b/packages/objectql/src/registry-tenancy-posture.test.ts @@ -18,6 +18,16 @@ // deployment. That is the fact these tests pin, deliberately and narrowly, // rather than the broader "columns go missing" reading. // +// [#6810] WHERE that index is declared moved, and these assertions moved with +// it: from a field-level `organization_id.indexed` boolean to an entry in the +// object's `indexes[]`. The boolean was never a `FieldSchema` key (#2377 / +// ADR-0049) and only `driver-mongodb` ever read it, so what #5262 pinned as +// "the index" was in truth a flag that built nothing on the SQL drivers every +// walled deployment runs. Reading `indexes[]` is the first spelling of this +// fact a driver actually materializes — and `multiTenant: false` now says +// "no index declared" rather than "an index declared false". The posture logic +// under test is untouched; only the surface the answer is read off. +// // Driven through the REAL `SchemaRegistry` constructor + `registerObject` // pipeline, reading the stored definition back out — the same path the kernel // takes at boot. Nothing about the resolver is stubbed: the tests set the real @@ -49,10 +59,20 @@ const registerUnder = (env: { posture?: string; legacy?: string }) => { 'crm', 'own', ); - const stored = (registry as any).objectContributors.get('lead')[0].definition; - return stored.fields.organization_id; + return (registry as any).objectContributors.get('lead')[0].definition; }; +/** + * [#6810] Is the tenant index declared on the object this posture produced? + * + * The `indexes[]` entry the drivers materialize — the successor to the + * field-level `indexed` boolean these tests used to read. + */ +const indexesTenantColumn = (stored: any): boolean => + (stored.indexes ?? []).some( + (i: any) => Array.isArray(i?.fields) && i.fields.length === 1 && i.fields[0] === 'organization_id', + ); + beforeEach(() => { delete process.env.OS_TENANCY_POSTURE; delete process.env.OS_MULTI_ORG_ENABLED; @@ -69,11 +89,11 @@ describe('#5262 — SchemaRegistry keys its multi-tenant default off OS_TENANCY_ // THE regression. Configured exactly as the v17 docs say: the authoritative // knob and nothing else. Before the fix `resolveMultiOrgEnabled()` returned // false here and the column landed UNINDEXED on a fully walled deployment. - const field = registerUnder({ posture: 'isolated' }); + const stored = registerUnder({ posture: 'isolated' }); - expect(field).toBeDefined(); - expect(field.reference).toBe('sys_organization'); - expect(field.indexed).toBe(true); + expect(stored.fields.organization_id).toBeDefined(); + expect(stored.fields.organization_id.reference).toBe('sys_organization'); + expect(indexesTenantColumn(stored)).toBe(true); }); it('`group` is a walled posture too — not just `isolated`', () => { @@ -82,29 +102,29 @@ describe('#5262 — SchemaRegistry keys its multi-tenant default off OS_TENANCY_ // just as much (ADR-0105 D1); it only widens READ scope to the membership // set, and `organization_id IN (...)` needs the index every bit as much as // `organization_id = ?` does. - expect(registerUnder({ posture: 'group' }).indexed).toBe(true); + expect(indexesTenantColumn(registerUnder({ posture: 'group' }))).toBe(true); }); it('legacy-boolean-only deployment keeps working — back-compat via the posture resolver', () => { // Nothing already deployed changes behaviour: `resolveTenancyPosture()` // falls back to `OS_MULTI_ORG_ENABLED` when the posture knob is unset, so // the pre-ADR-0105 configuration still resolves to `isolated`. - expect(registerUnder({ legacy: 'true' }).indexed).toBe(true); + expect(indexesTenantColumn(registerUnder({ legacy: 'true' }))).toBe(true); }); it('single-org deployments still leave the column unindexed', () => { // Intent unchanged — only the knob is corrected. Nothing filters by // organization on an unwalled stack, so the index would be dead weight. - expect(registerUnder({ posture: 'single' }).indexed).toBe(false); - expect(registerUnder({ legacy: 'false' }).indexed).toBe(false); - expect(registerUnder({}).indexed).toBe(false); + expect(indexesTenantColumn(registerUnder({ posture: 'single' }))).toBe(false); + expect(indexesTenantColumn(registerUnder({ legacy: 'false' }))).toBe(false); + expect(indexesTenantColumn(registerUnder({}))).toBe(false); }); it('an explicit legacy `false` does not veto the authoritative posture', () => { // The precise inversion the demotion created: the canonical knob asks for a // wall, the superseded one says "no multi-org". The canonical knob wins — // otherwise the legacy flag would still be authoritative in disguise. - expect(registerUnder({ posture: 'isolated', legacy: 'false' }).indexed).toBe(true); + expect(indexesTenantColumn(registerUnder({ posture: 'isolated', legacy: 'false' }))).toBe(true); }); it('an explicit `multiTenant` option still overrides the env entirely', () => { @@ -114,14 +134,15 @@ describe('#5262 — SchemaRegistry keys its multi-tenant default off OS_TENANCY_ const registry = new SchemaRegistry({ multiTenant: false }); registry.registerObject({ name: 'lead', fields: {} }, 'crm', 'crm', 'own'); const stored = (registry as any).objectContributors.get('lead')[0].definition; - expect(stored.fields.organization_id.indexed).toBe(false); + expect(indexesTenantColumn(stored)).toBe(false); + expect(stored.fields.organization_id).toBeDefined(); }); it('the column itself is provisioned either way — only the index moves', () => { // Guards the claim this file is scoped on. If a future change makes the // COLUMN conditional again, the blast radius of a posture misread grows // from "slow" to "the wall has nothing to filter on", and this fails. - expect(registerUnder({ posture: 'isolated' })).toBeDefined(); - expect(registerUnder({ posture: 'single' })).toBeDefined(); + expect(registerUnder({ posture: 'isolated' }).fields.organization_id).toBeDefined(); + expect(registerUnder({ posture: 'single' }).fields.organization_id).toBeDefined(); }); }); diff --git a/packages/objectql/src/registry-tenant-index-declaration.test.ts b/packages/objectql/src/registry-tenant-index-declaration.test.ts new file mode 100644 index 0000000000..0f4bd219f4 --- /dev/null +++ b/packages/objectql/src/registry-tenant-index-declaration.test.ts @@ -0,0 +1,192 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #6810 — the injected `organization_id` no longer carries a key `FieldSchema` +// rejects, so a registry-backed object reads back `_diagnostics: { valid: true }`. +// +// The defect: `applySystemFields` provisioned the tenant column with +// `indexed: opts.multiTenant`. `indexed` is not a `FieldSchema` key — #2377 / +// ADR-0049 removed it because a field-level index flag built no index — and +// `FieldSchema` is a `strictObject`, so it was rejected BY NAME with a +// purpose-written message. `registerObject` runs `applySystemFields` BEFORE +// storing and `getItem('object', …)` serves that post-injection document, so the +// key travelled out to `/meta`, where `decorateMetadataItem` re-parsed the served +// body and stamped `valid: false` on it. Measured on `origin/main` @ `4fedb11`: +// BOTH tenancy modes, BOTH read exits, on every registry-backed object — a +// defect report the platform wrote about its OWN column, on a document the +// author never wrote and could not fix, in the exact channel Studio renders +// invalid-metadata banners from and an AI author reads to judge its own work. +// +// The fix declares the tenant index in the object's `indexes[]` instead, which +// is where every other index in this system is declared. So these tests pin two +// halves that must travel together: the false verdict is gone, AND the index is +// still declared — a fix that merely deleted the key would pass the first half +// while silently dropping the intent. +// +// Driven through the REAL `SchemaRegistry` + the REAL +// `ObjectStackProtocolImplementation`, i.e. the same path a `/meta` request +// takes at runtime. Nothing about the parse or the decoration is stubbed. + +import { describe, it, expect } from 'vitest'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { SchemaRegistry } from './registry.js'; +import { applySystemFields } from './registry.js'; + +/** A plain business object — one authored field, nothing else. */ +const LEAD = { name: 'lead', label: 'Lead', fields: { first_name: { type: 'text' } } } as any; + +/** + * The registry-backed `/meta` surface for a tenancy-enabled object, with no DB + * behind it: every overlay lookup answers empty, so both exits are served from + * the SchemaRegistry — the path the defect lived on. + */ +function metaSurface(multiTenant: boolean) { + const registry = new SchemaRegistry({ multiTenant }); + registry.registerObject(LEAD, 'crm', 'crm', 'own'); + const engine = { + registry, + find: async () => [], + findOne: async () => null, + insert: async () => ({ id: 'x' }), + update: async () => ({ id: 'x' }), + delete: async () => ({ deleted: 0 }), + count: async () => 0, + aggregate: async () => [], + } as any; + return { registry, protocol: new ObjectStackProtocolImplementation(engine) }; +} + +/** The stored (post-injection) definition, as `registerObject` left it. */ +const storedDefinition = (registry: SchemaRegistry) => + (registry as any).objectContributors.get('lead')[0].definition; + +/** Declared indexes whose column list is exactly `['organization_id']`. */ +const tenantIndexes = (def: any) => + (def.indexes ?? []).filter( + (i: any) => Array.isArray(i?.fields) && i.fields.length === 1 && i.fields[0] === 'organization_id', + ); + +describe('#6810 — a registry-backed object reads back valid at both /meta exits', () => { + for (const multiTenant of [true, false]) { + describe(`multiTenant=${multiTenant}`, () => { + it('getMetaItem answers _diagnostics: { valid: true }', async () => { + const { protocol } = metaSurface(multiTenant); + const res: any = await protocol.getMetaItem({ type: 'object', name: 'lead' }); + + // Asserted as the whole object, not `.valid` alone: the pre-fix body + // carried an `errors` array alongside the verdict, and a fix that left + // errors behind a `true` verdict would be a different defect. + expect(res.item._diagnostics).toEqual({ valid: true }); + }); + + it('getMetaItems (the list exit) answers _diagnostics: { valid: true }', async () => { + const { protocol } = metaSurface(multiTenant); + const res: any = await protocol.getMetaItems({ type: 'object' }); + const lead = res.items.find((i: any) => i.name === 'lead'); + + expect(lead).toBeDefined(); + expect(lead._diagnostics).toEqual({ valid: true }); + }); + + it('no served field carries the retired `indexed` key', async () => { + // The verdict is the symptom; this is the cause. Asserted across EVERY + // injected column so a future injection cannot reintroduce the shape + // one field over (`owner_id`, `owning_business_unit_id`, the audit + // family) and be caught only by the aggregate verdict above. + const { protocol } = metaSurface(multiTenant); + const res: any = await protocol.getMetaItem({ type: 'object', name: 'lead' }); + + const carrying = Object.entries(res.item.fields as Record) + .filter(([, f]) => f && Object.hasOwn(f, 'indexed')) + .map(([name]) => name); + expect(carrying).toEqual([]); + }); + }); + } + + it('multiTenant=true DECLARES the tenant index in indexes[]', async () => { + // The other half of the fix. Deleting the key would satisfy every + // assertion above while silently dropping the index the flag was asking + // for — so the declaration is pinned in its own right, at the surface the + // drivers actually materialize from. + const { registry } = metaSurface(true); + const def = storedDefinition(registry); + + expect(tenantIndexes(def)).toEqual([{ fields: ['organization_id'] }]); + // No `name`: each driver derives its own. SQL's is table-qualified + // (`idx_lead_organization_id`) because index names are schema-global there; + // Mongo's is not, because they are per-collection. A name pinned here could + // not be right for both. + expect(tenantIndexes(def)[0].name).toBeUndefined(); + // A lookup index, never a constraint. + expect(tenantIndexes(def)[0].unique).toBeUndefined(); + }); + + it('multiTenant=false declares NO tenant index — absence, not a false flag', async () => { + // What `indexed: false` used to say, said the way `indexes[]` says it. + // Nothing filters by organization on an unwalled stack, so the index is + // dead weight. + const { registry } = metaSurface(false); + const def = storedDefinition(registry); + + expect(tenantIndexes(def)).toEqual([]); + // And the COLUMN is still provisioned either way — that decoupling is + // older than this fix and must survive it (sudo writers stamp the column + // on single-tenant stacks; it just stays NULL). + expect(def.fields.organization_id).toBeDefined(); + }); + + it('the served document is otherwise byte-identical to the pre-fix one, minus the key', async () => { + // Scopes the blast radius: this changed exactly two things about the served + // field — nothing about type, reference, or the governance keys that decide + // who may write it. + const out: any = applySystemFields(LEAD, { multiTenant: true }); + expect(out.fields.organization_id).toEqual({ + type: 'lookup', + reference: 'sys_organization', + label: 'Organization', + required: false, + hidden: true, + readonly: true, + system: true, + description: + 'Tenant scope (auto-populated by org-scoping on insert; NULL on single-tenant stacks).', + }); + }); + + it("appends to an author's indexes[] rather than replacing them", async () => { + const authored: any = { + name: 'lead', + fields: { code: { type: 'text' } }, + indexes: [{ name: 'lead_code_idx', fields: ['code'], unique: true }], + }; + const out: any = applySystemFields(authored, { multiTenant: true }); + + expect(out.indexes).toEqual([ + { name: 'lead_code_idx', fields: ['code'], unique: true }, + { fields: ['organization_id'] }, + ]); + }); + + it("does not duplicate an author's own single-column organization_id index", async () => { + // An array append is the one part of this injection that is not naturally + // idempotent, unlike the field merge beside it. + const authored: any = { + name: 'lead', + fields: { code: { type: 'text' } }, + indexes: [{ name: 'my_tenant_idx', fields: ['organization_id'] }], + }; + const out: any = applySystemFields(authored, { multiTenant: true }); + + expect(out.indexes).toEqual([{ name: 'my_tenant_idx', fields: ['organization_id'] }]); + }); + + it('leaves objects that opt out of the tenant column entirely alone', async () => { + // `systemFields: false` is the hard opt-out — no column, and therefore no + // index declaration either. + const optedOut: any = { name: 'seed_table', fields: { code: { type: 'text' } }, systemFields: false }; + const out: any = applySystemFields(optedOut, { multiTenant: true }); + + expect(out.fields.organization_id).toBeUndefined(); + expect(out.indexes).toBeUndefined(); + }); +}); diff --git a/packages/objectql/src/registry.test.ts b/packages/objectql/src/registry.test.ts index b6d5a95a63..7a76cfd005 100644 --- a/packages/objectql/src/registry.test.ts +++ b/packages/objectql/src/registry.test.ts @@ -572,8 +572,12 @@ describe('applySystemFields', () => { expect(out.fields.organization_id).toBeDefined(); expect(out.fields.organization_id.type).toBe('lookup'); expect(out.fields.organization_id.reference).toBe('sys_organization'); - // Multi-tenant stacks index the column (per-tenant filtering). - expect(out.fields.organization_id.indexed).toBe(true); + // [#6810] Multi-tenant stacks index the column (per-tenant filtering) — + // declared in `indexes[]`, which is what a driver materializes from. The + // field-level `indexed: true` this used to read was never a + // `FieldSchema` key (#2377 / ADR-0049) and only ever reached one driver. + expect((out as any).indexes).toEqual([{ fields: ['organization_id'] }]); + expect(out.fields.organization_id.indexed).toBeUndefined(); // author-declared field still present expect(out.fields.first_name).toBeDefined(); }); @@ -585,7 +589,10 @@ describe('applySystemFields', () => { const out = applySystemFields(baseLead, { multiTenant: false }); expect(out.fields.organization_id).toBeDefined(); expect(out.fields.organization_id.type).toBe('lookup'); - expect(out.fields.organization_id.indexed).toBe(false); + // [#6810] "Unindexed" is now the ABSENCE of a declaration, not a + // declaration whose value is false. + expect((out as any).indexes).toBeUndefined(); + expect(out.fields.organization_id.indexed).toBeUndefined(); // audit fields are tenant-independent — still injected expect(out.fields.created_at).toBeDefined(); expect(out.fields.updated_at).toBeDefined(); diff --git a/packages/objectql/src/registry.ts b/packages/objectql/src/registry.ts index a098346621..eaa194624a 100644 --- a/packages/objectql/src/registry.ts +++ b/packages/objectql/src/registry.ts @@ -427,6 +427,9 @@ 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) { additions.organization_id = { @@ -434,13 +437,48 @@ export function applySystemFields( reference: 'sys_organization', label: 'Organization', required: false, - indexed: opts.multiTenant, hidden: true, readonly: true, system: true, description: 'Tenant scope (auto-populated by org-scoping on insert; NULL on single-tenant stacks).', }; + + // [#6810] The tenant index is declared in `indexes[]`, NOT as a field-level + // `indexed` flag. + // + // Until #6810 this line read `indexed: opts.multiTenant` on the field def + // above. `indexed` is not a `FieldSchema` key — #2377 / ADR-0049 removed it + // precisely because a field-level index flag built no index — and + // `FieldSchema` is a `strictObject` that rejects it BY NAME. `registerObject` + // runs this function BEFORE storing, and `getItem('object', …)` serves the + // post-injection document, so the key travelled out to `/meta` where + // `decorateMetadataItem` re-parsed it and stamped + // `_diagnostics: { valid: false, errors: [{ path: 'fields.organization_id', + // code: 'unrecognized_keys' }] }` on EVERY registry-backed object, in both + // tenancy modes and at both read exits. That is a defect report the platform + // wrote about its own column, on a document the author never wrote and could + // not fix — and it drowned real authoring errors in the same channel. + // + // Declaring it here instead 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` — this is 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'] }); + } } if (wantAudit) { @@ -543,16 +581,51 @@ export function applySystemFields( }; } - if (Object.keys(additions).length === 0 && Object.keys(overrides).length === 0) return schema; + if ( + Object.keys(additions).length === 0 && + Object.keys(overrides).length === 0 && + indexAdditions.length === 0 + ) { + 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] } + : {}), }; } +/** + * [#6810] Is the tenant index already declared on this object? + * + * The append is the one part of this injection that is not naturally + * idempotent — the field injection re-runs harmlessly because + * `!schema.fields?.organization_id` stops it, an array push does not — so an + * author who hand-wrote `indexes: [{ fields: ['organization_id'] }]` must not + * end up with the platform's duplicate beside it. + * + * Matched on the single-column shape this function emits, deliberately: an + * author's composite (`['organization_id', 'code']`) is a leading-column match + * on some dialects and not on others, so it is not treated as a substitute. + */ +function declaresTenantIndex(schema: ServiceObject): boolean { + const declared = (schema as any).indexes; + if (!Array.isArray(declared)) return false; + return declared.some( + (idx: any) => + Array.isArray(idx?.fields) && + idx.fields.length === 1 && + idx.fields[0] === 'organization_id', + ); +} + /** * Generic-write `apiMethods` verbs mapped to the {@link resolveCrudAffordances} * flag each one needs. Read verbs (`get`/`list`/`search`/`history`/…) are From 915359e5ed7c7faf46fbf3896ebac45c965a15e3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 20:51:26 +0000 Subject: [PATCH 2/2] test(objectql): open the new fake engine's write verbs with the dispatch guards (#6810) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `check:engine-double-contract` pins every engine double to ObjectQL's own `delete`/`update` dispatch predicates — a fake looser than the producer is how #4434 shipped a dead REST route with its suite green. The fake in `registry-tenant-index-declaration.test.ts` now routes both verbs through `assertEngineDeleteDispatch` / `assertEngineUpdateDispatch` from `@objectstack/metadata-core`, matching the pinned fake in `protocol-meta-effective-schema.test.ts` next to it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01JE8Bbwb8qhau3yNtP3ftLJ --- .../registry-tenant-index-declaration.test.ts | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/objectql/src/registry-tenant-index-declaration.test.ts b/packages/objectql/src/registry-tenant-index-declaration.test.ts index 0f4bd219f4..dcc0de9481 100644 --- a/packages/objectql/src/registry-tenant-index-declaration.test.ts +++ b/packages/objectql/src/registry-tenant-index-declaration.test.ts @@ -28,8 +28,10 @@ import { describe, it, expect } from 'vitest'; import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; -import { SchemaRegistry } from './registry.js'; -import { applySystemFields } from './registry.js'; +// [#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, applySystemFields } from './registry.js'; /** A plain business object — one authored field, nothing else. */ const LEAD = { name: 'lead', label: 'Lead', fields: { first_name: { type: 'text' } } } as any; @@ -47,8 +49,14 @@ function metaSurface(multiTenant: boolean) { find: async () => [], findOne: async () => null, insert: async () => ({ id: 'x' }), - update: async () => ({ id: 'x' }), - delete: async () => ({ deleted: 0 }), + update: async (_t: string, data: Record, opts?: Record) => { + assertEngineUpdateDispatch(data, opts); + return { id: 'x' }; + }, + delete: async (_t: string, opts?: Record) => { + assertEngineDeleteDispatch(opts); + return { deleted: 0 }; + }, count: async () => 0, aggregate: async () => [], } as any;