diff --git a/.changeset/index-type-partial-removed.md b/.changeset/index-type-partial-removed.md new file mode 100644 index 0000000000..54196cbdfa --- /dev/null +++ b/.changeset/index-type-partial-removed.md @@ -0,0 +1,70 @@ +--- +"@objectstack/spec": major +"@objectstack/metadata-core": patch +--- + +refactor(spec)!: retire `indexes[].type` and `indexes[].partial` — two authorable index keys no driver ever read (#5248, #4943) + +`IndexSchema` declared five keys; only three of them ever reached a `CREATE +INDEX`. `SqlDriver.syncDeclaredIndexes` builds every declared index through +knex's `table.index(fields, name)` / `table.unique(fields, { indexName })`, and +the drift differ's `DeclaredIndexInput` carries `name` / `fields` / `unique` / +`nullSafeColumns`. So: + +- **`partial`** — documented as *"Partial index condition (SQL WHERE clause)"* — + produced a **full** index with the predicate silently discarded. This was the + damaging half, because it reads as a correctness control: the platform's own + `sys_metadata` declared `partial: "state = 'active'"` for overlay uniqueness, + and what the declaration alone materialized was an *unrestricted* unique index. +- **`type`** additionally carried `.default('btree')`, so it appeared in **every** + parse output of **every** index — an access-method knob that had never + influenced a single statement, rendered as live configuration. (It was pinned + as such in a `sys_presence` test, on an object that never declared it.) + +Both are the ADR-0078 no-silently-inert / ADR-0049 enforce-or-remove shape. +Remove was chosen over enforce: enforcing needs per-dialect algorithm mapping +(`gin`/`gist` Postgres-only, `fulltext` MySQL-family), raw-SQL `CREATE INDEX … +WHERE` on the dialects that have partial indexes at all (MySQL does not), and a +redesign of how `isSyncReproducibleIndex` excludes partial indexes from +incremental sync — design cost for a capability with no demand. If a real need +appears it returns enforce-first. + +## Migration + +| FROM | TO | +| :--- | :--- | +| `indexes: [{ fields: […], type: 'gin' }]` | `indexes: [{ fields: […] }]` — create the specialised index from a database-layer migration | +| `indexes: [{ fields: […], partial: "state = 'active'" }]` | `indexes: [{ fields: […] }]` — issue `CREATE [UNIQUE] INDEX … WHERE …` from a runtime migration | + +**One-line fix: delete the key.** Neither removal changes any DDL, because no +DDL ever depended on them — verified byte-for-byte against the `CREATE INDEX` +statements SQLite actually stores +(`packages/drivers/driver-sql/src/declared-index-retired-keys.test.ts`). + +Both capabilities remain available where they are implementable. The index +method is the driver/dialect's choice. A partial index is issued as raw SQL from +a runtime migration — exactly what `metadata-protocol`'s `ensureOverlayIndex` +already does for `sys_metadata`, and what actually delivers that table's +active-row-scoped uniqueness today. + +⚠️ **Not affected:** driver-sql's own `partial` flag (`parseIndexDdl` / +`introspectIndexes` / `isSyncReproducibleIndex`). That is a boolean parsed back +out of the *database's own* DDL for drift detection — the opposite direction — +so migration-created partial indexes stay recognized and exempt from incremental +sync, unchanged. + +## The retirement kit + +- `retiredKey()` tombstones at `IndexSchema` (the shape is deliberately + `.strip()`, so a plain delete would swap one silent no-op for another): writing + either key is now a `tsc` error and a parse error carrying the prescription. + They sit at the bottom of the shape per the #5606 renderer note. +- **ADR-0087 D2 conversion + D3 chain step** (`object-index-type-partial-removed`, + `toMajor: 17`, wired into the existing step-17 chain): strips both keys from + `objects[]` and `objectExtensions[]`; `os migrate meta --from 16` rewrites sources + mechanically. A pure lossless delete — there was no effect to lose. +- **Producers flipped:** `sys_metadata` (`idx_sys_metadata_overlay_active`, the + case #4943 named) and `sys_view_definition` (`idx_sys_view_def_active`), both + with their comments corrected to say what is actually materialized. +- Published skill (`objectstack-data`), `content/docs/data-modeling/objects.mdx`, + liveness ledger note and generated baselines updated. diff --git a/content/docs/data-modeling/objects.mdx b/content/docs/data-modeling/objects.mdx index 75e71bfcc1..232b5eaf59 100644 --- a/content/docs/data-modeling/objects.mdx +++ b/content/docs/data-modeling/objects.mdx @@ -209,20 +209,34 @@ Optimize query performance: {/* os:check */} ```typescript indexes: [ - { fields: ['name'], type: 'btree', unique: false }, - { fields: ['email'], type: 'btree', unique: 'organization' }, - { fields: ['type', 'status'], type: 'btree', unique: false }, + { fields: ['name'] }, + { fields: ['email'], unique: 'organization' }, + { fields: ['type', 'status'] }, ] ``` | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | | `fields` | `string[]` | ✅ | Fields in the index | -| `type` | `enum` | optional | `'btree'`, `'hash'`, `'gin'`, `'gist'`, `'fulltext'` (default: `'btree'`) | -| `unique` | `boolean` | optional | Enforce uniqueness (default: `false`) | -| `partial` | `string` | optional | Conditional index (SQL WHERE clause) | +| `unique` | `boolean \| 'global' \| 'organization'` | optional | Enforce uniqueness, and at which scope (default: `false`) | | `name` | `string` | optional | Index name (auto-generated if omitted) | + + **`type` and `partial` were retired in protocol 17** (#5248, #4943). Neither + had a driver consumer: declared indexes are created through knex's + `table.index()` / `table.unique()`, so an authored `type` selected no access + method and an authored `partial` produced a **full** index with the predicate + silently discarded. Writing either now fails `tsc` and the parse with a + migration prescription — run `os migrate meta --from 16` to strip them. + + Both capabilities remain available where they are actually implementable: the + index method is the driver/dialect's choice, and a partial index is issued as + raw SQL from a runtime migration (`CREATE [UNIQUE] INDEX … WHERE …`, the way + `metadata-protocol` builds `sys_metadata`'s overlay index). Drift detection + reads partiality back from the database's own DDL, so migration-created + partial indexes are recognized and left alone. + + ### Additional Properties | Property | Type | Description | @@ -347,8 +361,8 @@ export const ProjectTask = ObjectSchema.create({ }, indexes: [ - { fields: ['status'], type: 'btree', unique: false }, - { fields: ['project', 'status'], type: 'btree', unique: false }, + { fields: ['status'] }, + { fields: ['project', 'status'] }, ], enable: { diff --git a/content/docs/references/data/object.mdx b/content/docs/references/data/object.mdx index 178cc6c957..cc913a50b0 100644 --- a/content/docs/references/data/object.mdx +++ b/content/docs/references/data/object.mdx @@ -67,9 +67,9 @@ const result = ApiMethod.parse(data); | :--- | :--- | :--- | :--- | | **name** | `string` | optional | Index name (auto-generated if not provided) | | **fields** | `string[]` | ✅ | Fields included in the index | -| **type** | `Enum<'btree' \| 'hash' \| 'gin' \| 'gist' \| 'fulltext'>` | ✅ | Index algorithm type | | **unique** | `boolean \| 'global' \| 'organization'` | ✅ | Whether the index enforces uniqueness, and at which scope (ADR-0120). 'global' = materialized over exactly `fields`, no organization column injected — one holder across the whole installation; 'organization' = the driver prepends the NULL-safe organization key part (COALESCE(organization_id, '__global__')) at registration — one holder per organization; bare true = deprecated positional spelling of 'global' (warned in 17.x by lint unique/unscoped-declared-index, rejected at protocol 18, #5082) — state the scope. 'tenant'/'org' are rejected — the word is 'organization' | -| **partial** | `string` | optional | Partial index condition (SQL WHERE clause for conditional indexes) | +| **type** | `any` | optional | [REMOVED] `indexes[].type` was removed in @objectstack/spec 17.0.0 (#5248, ADR-0049) — no driver ever read it. `SqlDriver.syncDeclaredIndexes` creates every declared index through knex's `table.index()` / `table.unique()`, which cannot express an access method, so the value changed no DDL; its `.default('btree')` merely made an inert knob show up in every parse output. Delete the key. The index method is the driver/dialect's decision (Postgres defaults to B-tree; `gin`/`gist`/`fulltext` are dialect-specific and are chosen by a database-layer migration when a workload actually needs one). Run `os migrate meta --from 16` to rewrite it automatically. | +| **partial** | `any` | optional | [REMOVED] `indexes[].partial` was removed in @objectstack/spec 17.0.0 (#5248, #4943, ADR-0049) — no driver ever emitted the `WHERE` clause, so a declared partial index was materialized as a FULL index and the predicate silently did nothing. Delete the key. Partial indexes are built at the database layer, not the declaration surface: issue `CREATE [UNIQUE] INDEX … WHERE ` from a runtime migration (this is what `metadata-protocol`'s `ensureOverlayIndex` already does for `sys_metadata`). Drift detection is unaffected — it reads partiality back from the database's own DDL, never from this key. Run `os migrate meta --from 16` to rewrite it automatically. | --- @@ -122,7 +122,7 @@ const result = ApiMethod.parse(data); | **datasource** | `string` | optional | Target Datasource ID. "default" is the primary DB. | | **external** | `{ remoteName?: string; remoteSchema?: string; writable?: boolean; columnMap?: Record; … }` | optional | Remote table binding for federated (external) objects. | | **fields** | `Record; description?: string; … }>` | ✅ | Field definitions map. Keys must be snake_case identifiers. | -| **indexes** | `{ name?: string; fields: string[]; type?: Enum<'btree' \| 'hash' \| 'gin' \| 'gist' \| 'fulltext'>; unique?: boolean \| 'global' \| 'organization'; … }[]` | optional | Database performance indexes | +| **indexes** | `{ name?: string; fields: string[]; unique?: boolean \| 'global' \| 'organization'; type?: any; … }[]` | optional | Database performance indexes | | **fieldGroups** | `{ key: string; label: string; icon?: string; description?: string; … }[]` | optional | Ordered list of field groups (array order = display order). See ObjectFieldGroupSchema. | | **tenancy** | `{ enabled: boolean; tenantField?: string }` | optional | Multi-tenancy configuration for SaaS applications | | **access** | `{ default?: Enum<'public' \| 'private'> }` | optional | [ADR-0066 D2] Object exposure posture (public-by-default vs private secure-by-default). | @@ -196,7 +196,7 @@ const result = ApiMethod.parse(data); | **pluralLabel** | `string` | optional | Override plural label for the extended object | | **description** | `string` | optional | Override description for the extended object | | **validations** | `any[]` | optional | Additional validation rules to merge into the target object | -| **indexes** | `{ name?: string; fields: string[]; type?: Enum<'btree' \| 'hash' \| 'gin' \| 'gist' \| 'fulltext'>; unique?: boolean \| 'global' \| 'organization'; … }[]` | optional | Additional indexes to merge into the target object | +| **indexes** | `{ name?: string; fields: string[]; unique?: boolean \| 'global' \| 'organization'; type?: any; … }[]` | optional | Additional indexes to merge into the target object | | **priority** | `integer` | optional | Merge priority (higher = applied later) | diff --git a/docs/protocol-upgrade-guide.md b/docs/protocol-upgrade-guide.md index 29bef973dd..4bf2939c0e 100644 --- a/docs/protocol-upgrade-guide.md +++ b/docs/protocol-upgrade-guide.md @@ -208,6 +208,8 @@ The same is true of the protocol-17 retirement that closes this list, and the pa The last enforce-or-remove entry of this step is on the RUNTIME context rather than on anything authorable: `HookContext.session.roles` (#5050). It was declared in `data/hook.zod.ts`, read by exactly two consumers — the approvals record lock and the delegation write guard, each opening with `session.roles?.includes('admin')` — and produced by nobody on the hook path: ObjectQL's `buildSession()` writes the session field by field (`userId`, `organizationId`, `accessToken`, `isSystem`, `actor`, the skip flags) and has no `roles` write, here or in `cloud`, whose hook consumers read `hookContext?.session?.userId` and nothing else (an ACTION body's `ctx.session` is a different untyped object that does carry one, tracked apart). So both branches were dead on every real engine path: an authorization decision in shape only, and — worse for a reader — a SECOND admin dialect competing with the one ADR-0095 D3 sanctions. #4839 (PR #5049) deleted the two readers on the maintainer's ruling; this step removes the declaration that outlived them, which is what ADR-0049 asks for once a key has neither end. Nothing observable changes: a key nobody wrote and nothing read cannot alter a single decision. It is tombstoned rather than deleted because `HookContextSchema` is deliberately NOT `.strict()` (strictness there would make an engine-internal enrichment a breaking change for anyone parsing a context they were handed, as `provenance` was in #3712), so a plain delete would strip the key in silence — the #3733 / ADR-0104 failure this whole pass exists to end. There is NO conversion and no source rewrite: a HookContext is built per operation by the engine and never stored, so no `sys_metadata` row, example or template can carry the key — the `openApi31` / `activationEvents` shape, one semantic TODO for hook authors. The live vocabulary is untouched and deliberately elsewhere: gate on `session.userId` / `session.isSystem` in the hook, and judge PRIVILEGE through the security service, which reads capability grants (`permissions`), placements (`positions`) and the derived posture off the execution context. +Finally it retires the two inert `IndexSchema` keys, `indexes[].type` and `indexes[].partial` (#5248, #4943). Neither ever had a DDL consumer: `SqlDriver.syncDeclaredIndexes` creates declared indexes through knex's `table.index()` / `table.unique()`, and the drift differ's `DeclaredIndexInput` carries only `name`/`fields`/`unique`/`nullSafeColumns` — so an authored `type` selected no access method and an authored `partial` produced a FULL index with its predicate discarded. `partial` was the more damaging of the two because it read as a correctness control: the platform's own `sys_metadata` declared it for overlay uniqueness, and what the declaration alone materialized was an unrestricted unique index (the active-row scoping is delivered by a runtime migration, `metadata-protocol`'s `ensureOverlayIndex`, not by the key). `type` was the louder: its `.default('btree')` put an inert knob into every parse output, so it read as live configuration — the ADR-0078 no-silently-inert shape. Remove was chosen over enforce (maintainer ruling, 2026-08-06): enforcing needs per-dialect algorithm mapping (`gin`/`gist` Postgres-only, `fulltext` MySQL-family), raw-SQL `CREATE INDEX … WHERE` on the dialects that have partial indexes at all (MySQL does not), and a redesign of how `isSyncReproducibleIndex` excludes partial indexes from incremental sync — design cost for a capability nothing has asked for. Both are lossless deletes: no DDL changes, because no DDL ever depended on them. Drift detection is untouched — the `partial` flag it consumes is parsed back out of the database's OWN `CREATE INDEX` DDL and never came from this key. + ### Mechanical (applied for you) | Conversion | Surface | Change | Load window | @@ -255,6 +257,7 @@ The last enforce-or-remove entry of this step is on the RUNTIME context rather t | `connector-rate-limit-config-removed` | `connector.rateLimitConfig` | connector key 'rateLimitConfig' removed (#4911 — no outbound rate-limiting engine exists; the runtime's only token bucket limits INBOUND requests, so every knob here was inert while reading like a configured cap. The whole ConnectorRateLimitConfig shape went with it) | retired — `migrate meta` only | | `theme-inert-token-scales-removed` | `theme.typography.fontSize / theme.typography.fontWeight / theme.typography.lineHeight / theme.typography.letterSpacing / theme.typography.fontFamily.heading / theme.typography.fontFamily.mono / theme.animation / theme.zIndex` | theme keys 'typography.fontSize'/'fontWeight'/'lineHeight'/'letterSpacing', 'typography.fontFamily.heading'/'mono', 'animation' and 'zIndex' removed (#5021, ADR-0049 — the engine emitted --font-size-*, --font-weight-*, --line-height-*, --letter-spacing-*, --duration-*, --timing-*, --z-*, --font-heading and --font-mono faithfully, and no first-party component or stylesheet has ever read one. Re-declare any variable you actually consume under customVars, which emits it verbatim) | retired — `migrate meta` only | | `page-header-subtitle-alias` | `page.component.page-header.description` | page-header component prop 'description' → 'subtitle' (objectui#3226 — the `subtitle ?? description` fallback retires) | live — protocol 17 loader accepts the old shape | +| `object-index-type-partial-removed` | `object.indexes[].type / object.indexes[].partial` | object index keys 'indexes[].type'/'indexes[].partial' removed (#5248, #4943 — no driver ever read either: the index method is the dialect's choice and a partial index is built by a database-layer migration, not declared) | retired — `migrate meta` only | ### Semantic (delegated to you, with acceptance criteria) diff --git a/packages/drivers/driver-sql/src/declared-index-retired-keys.test.ts b/packages/drivers/driver-sql/src/declared-index-retired-keys.test.ts new file mode 100644 index 0000000000..a704d17ff0 --- /dev/null +++ b/packages/drivers/driver-sql/src/declared-index-retired-keys.test.ts @@ -0,0 +1,162 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { describe, it, expect, afterEach, vi } from 'vitest'; + +import { SqlDriver, normalizeDeclaredIndex } from '../src/index.js'; + +/** + * `indexes[].type` / `indexes[].partial` were retired at protocol 17 (#5248, + * #4943, ADR-0049 enforce-or-remove). The retirement rests on ONE factual + * claim, and this file is that claim's proof rather than its restatement: + * + * **Dropping either key changes no DDL, because no DDL ever read them.** + * + * The claim is worth pinning because the two platform objects that authored + * `partial` (`sys_metadata`, `sys_view_definition`) both did so for a + * correctness reason — active-row-scoped uniqueness — and a reader who sees + * the key disappear from those declarations is entitled to ask whether a + * constraint was just relaxed. It was not: the constraint the DECLARATION + * produced was always the unrestricted one. What actually delivers the + * active-row scoping on `sys_metadata` is `metadata-protocol`'s runtime + * `ensureOverlayIndex` migration, which issues `CREATE UNIQUE INDEX … + * WHERE state = 'active'` in raw SQL and is untouched by this retirement. + * + * ⚠️ Do NOT confuse this with the driver's own `partial`. `parseIndexDdl` / + * `introspectIndexes` / `isSyncReproducibleIndex` carry a `partial: boolean` + * parsed back out of the DATABASE's own `CREATE INDEX` statement, consumed by + * drift detection so a DB-authored partial index is exempt from incremental + * sync. That is a different fact in a different direction (DB → us, not us → + * DB) and keeps working exactly as before — the tests in + * `sql-driver-overlay-index-drift.test.ts` own it. + */ +describe('retired declared-index keys are DDL-inert (#5248 / #4943)', () => { + let tmp: string | undefined; + let knexInstance: any; + + const makeDriver = () => { + tmp = mkdtempSync(join(tmpdir(), 'os-idx-retire-')); + const d = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: join(tmp, 'test.db') }, + useNullAsDefault: true, + }); + knexInstance = (d as any).knex; + (d as any).logger = { warn: vi.fn(), info: vi.fn(), error: vi.fn() }; + return d; + }; + + afterEach(async () => { + await knexInstance?.destroy(); + knexInstance = undefined; + if (tmp) rmSync(tmp, { recursive: true, force: true }); + tmp = undefined; + }); + + /** + * Create `table` with the given declared indexes and return the CREATE INDEX + * statements SQLite actually stored, normalized for comparison. + */ + const createdIndexDdl = async (driver: any, table: string, indexes: any[]): Promise => { + await driver.knex.schema.createTable(table, (t: any) => { + t.string('type'); + t.string('name'); + t.string('organization_id'); + t.string('package_id'); + t.string('owner'); + t.string('state'); + t.string('tags'); + }); + const physical = new Set(['type', 'name', 'organization_id', 'package_id', 'owner', 'state', 'tags']); + await driver.syncDeclaredIndexes(table, indexes, physical, null); + const rows = await driver + .knex('sqlite_master') + .where({ type: 'index', tbl_name: table }) + .whereNotNull('sql') + .orderBy('name'); + return rows.map((r: any) => String(r.sql).replace(/\s+/g, ' ').trim()); + }; + + // The two real producers this PR flipped, plus a `type`-carrying index. + const cases: Array<{ label: string; withKeys: any; without: any }> = [ + { + label: 'sys_metadata / idx_sys_metadata_overlay_active', + withKeys: { + name: 'idx_sys_metadata_overlay_active', + fields: ['type', 'name', 'organization_id', 'package_id'], + unique: true, + partial: "state = 'active'", + }, + without: { + name: 'idx_sys_metadata_overlay_active', + fields: ['type', 'name', 'organization_id', 'package_id'], + unique: true, + }, + }, + { + label: 'sys_view_definition / idx_sys_view_def_active', + withKeys: { + name: 'idx_sys_view_def_active', + fields: ['name', 'organization_id', 'owner'], + unique: true, + partial: "state = 'active'", + }, + without: { + name: 'idx_sys_view_def_active', + fields: ['name', 'organization_id', 'owner'], + unique: true, + }, + }, + { + label: 'a non-default index method (gin)', + withKeys: { name: 'idx_tags', fields: ['tags'], type: 'gin' }, + without: { name: 'idx_tags', fields: ['tags'] }, + }, + ]; + + for (const { label, withKeys, without } of cases) { + it(`${label}: identical CREATE INDEX with and without the retired key(s)`, async () => { + const a = makeDriver(); + const withDdl = await createdIndexDdl(a, 't_probe', [withKeys]); + await knexInstance.destroy(); + knexInstance = undefined; + rmSync(tmp!, { recursive: true, force: true }); + tmp = undefined; + + const b = makeDriver(); + const withoutDdl = await createdIndexDdl(b, 't_probe', [without]); + + // The real database's own stored DDL, byte-for-byte. + expect(withDdl).toEqual(withoutDdl); + expect(withDdl).toHaveLength(1); + // And it is a FULL index — no WHERE clause was ever emitted, which is + // exactly why the declared predicate was inert. + expect(withDdl[0]!.toLowerCase()).not.toContain(' where '); + }); + } + + it('normalizeDeclaredIndex ignores both keys (the single seam every index create goes through)', () => { + const normalized = normalizeDeclaredIndex( + 'sys_metadata', + { + name: 'idx_sys_metadata_overlay_active', + fields: ['type', 'name', 'organization_id', 'package_id'], + unique: true, + // Cast: `DeclaredIndexInput` never declared these — that is the point. + ...({ type: 'gin', partial: "state = 'active'" } as Record), + } as any, + null, + ); + expect(normalized).toEqual({ + name: 'idx_sys_metadata_overlay_active', + columns: ['type', 'name', 'organization_id', 'package_id'], + unique: true, + }); + // Neither key survives into the shape the create path consumes. + expect(normalized).not.toHaveProperty('partial'); + expect(normalized).not.toHaveProperty('type'); + }); +}); diff --git a/packages/drivers/driver-sql/src/sql-driver-overlay-index-drift.test.ts b/packages/drivers/driver-sql/src/sql-driver-overlay-index-drift.test.ts index 716bc5dab1..12a3580b4e 100644 --- a/packages/drivers/driver-sql/src/sql-driver-overlay-index-drift.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-overlay-index-drift.test.ts @@ -83,11 +83,17 @@ describe('overlay index drift on a fresh database (#4884)', () => { state: { type: 'string' }, }, indexes: [ + // Mirrors `metadata-core`'s sys-metadata.object.ts. It carried + // `partial: "state = 'active'"` until #5248 / #4943 retired the key; + // the declaration never produced a predicate (knex's `table.unique()` + // cannot express one), so dropping it here changes nothing this file + // measures — it keeps the fixture honest about what a real boot + // declares. The partial UNIQUE the drift cases below care about is the + // one `runEnsureOverlayIndex` issues in raw SQL. { name: 'idx_sys_metadata_overlay_active', fields: ['type', 'name', 'organization_id', 'package_id'], unique: true, - partial: "state = 'active'", }, { name: 'idx_sys_metadata_org_type', fields: ['organization_id', 'type'] }, { fields: ['state'] }, diff --git a/packages/metadata-core/src/objects/sys-metadata.object.ts b/packages/metadata-core/src/objects/sys-metadata.object.ts index 0281d79f7b..fda98fc776 100644 --- a/packages/metadata-core/src/objects/sys-metadata.object.ts +++ b/packages/metadata-core/src/objects/sys-metadata.object.ts @@ -200,21 +200,33 @@ export const SysMetadataObject = ObjectSchema.create({ indexes: [ // ADR-0005 (revised 2026-05) + ADR-0048: overlay uniqueness is scoped by - // (type, name, organization_id, package_id), restricted to active rows so - // resets / archived versions don't collide. `package_id` is part of the + // (type, name, organization_id, package_id). `package_id` is part of the // discriminator so two installed packages shipping the same `type`/`name` // each get their OWN customization row (a package-less / global overlay // uses NULL). environment_id is deprecated and not part of the - // discriminator. The runtime layer (protocol.ts ensureOverlayIndex) issues - // a DROP-then-CREATE migration that uses `COALESCE(package_id,'')` so the - // package-less rows stay unique among themselves (SQLite treats NULLs as - // distinct in a plain unique index); this declaration is the fallback shape - // for drivers without the runtime migration. + // discriminator. + // + // ⚠️ The active-row restriction is NOT declared here, and never was in any + // effective sense: this entry carried `partial: "state = 'active'"` until + // #5248 / #4943 retired the key, and no driver ever emitted the predicate + // — `syncDeclaredIndexes` builds indexes through knex's `table.unique()`, + // which cannot express a `WHERE`. So the shape this declaration has always + // materialized is the UNRESTRICTED unique index below; dropping the key is + // a zero-DDL change (verified byte-for-byte against the created SQL). + // + // What actually delivers active-row scoping is the runtime layer: + // `protocol.ts ensureOverlayIndex` issues a DROP-then-CREATE migration for + // `CREATE UNIQUE INDEX … ON sys_metadata (type, name, organization_id, + // COALESCE(package_id,'')) WHERE state = 'active'` — which additionally + // fixes NULL-distinctness for package-less rows, something this + // declaration could not express either. This entry is therefore the + // coarser fallback that a driver without that runtime migration gets: it + // still prevents duplicate ACTIVE overlays, at the cost of also colliding + // with archived/reset rows. { name: 'idx_sys_metadata_overlay_active', fields: ['type', 'name', 'organization_id', 'package_id'], unique: true, - partial: "state = 'active'", }, { name: 'idx_sys_metadata_org_type', fields: ['organization_id', 'type'] }, { fields: ['type', 'scope'] }, diff --git a/packages/metadata-core/src/objects/sys-view-definition.object.ts b/packages/metadata-core/src/objects/sys-view-definition.object.ts index 8b0e6abe55..0f0be07148 100644 --- a/packages/metadata-core/src/objects/sys-view-definition.object.ts +++ b/packages/metadata-core/src/objects/sys-view-definition.object.ts @@ -119,13 +119,24 @@ export const SysViewDefinitionObject = ObjectSchema.create({ }, indexes: [ - // A given view name is unique per (organization, owner) among active rows — - // a shared view (owner NULL) and each user's personal views don't collide. + // A given view name is unique per (organization, owner) — a shared view + // (owner NULL) and each user's personal views don't collide. + // + // ⚠️ This entry carried `partial: "state = 'active'"` until #5248 / #4943 + // retired the key, intending "among ACTIVE rows". No driver ever emitted + // the predicate (`syncDeclaredIndexes` builds indexes through knex's + // `table.unique()`, which cannot express a `WHERE`), so the index that has + // always been created is the unrestricted one below — dropping the key is + // a zero-DDL change. Unlike `sys_metadata`, there is NO runtime migration + // issuing the partial form for this table, so the active-row scoping is + // simply not delivered anywhere today: an archived/reset view still + // occupies its (name, organization_id, owner) slot. Tracked separately — + // deciding whether this table wants an `ensureOverlayIndex`-style + // migration is a behaviour change, out of scope for the key retirement. { name: 'idx_sys_view_def_active', fields: ['name', 'organization_id', 'owner'], unique: true, - partial: "state = 'active'", }, // The switcher query: views for one object within a tenant. { name: 'idx_sys_view_def_object', fields: ['organization_id', 'object'] }, diff --git a/packages/services/service-realtime/src/objects/sys-presence.object.test.ts b/packages/services/service-realtime/src/objects/sys-presence.object.test.ts index 9f3e84e1b0..8b18ac8b08 100644 --- a/packages/services/service-realtime/src/objects/sys-presence.object.test.ts +++ b/packages/services/service-realtime/src/objects/sys-presence.object.test.ts @@ -60,10 +60,16 @@ describe('SysPresence object definition', () => { }); it('should have indexes on user_id, session_id, and status', () => { + // No `type` here: `sys-presence.object.ts` never declared one. It used to + // show up anyway because `IndexSchema.type` carried `.default('btree')`, + // so the parse materialized an access-method knob that no driver has ever + // read into every index of every object — this assertion was pinning that + // phantom. The key was retired at protocol 18 (#5248), and the parsed + // shape is now exactly what the author wrote. expect(SysPresence.indexes).toEqual([ - { fields: ['user_id'], unique: false, type: 'btree' }, - { fields: ['session_id'], unique: true, type: 'btree' }, - { fields: ['status'], unique: false, type: 'btree' }, + { fields: ['user_id'], unique: false }, + { fields: ['session_id'], unique: true }, + { fields: ['status'], unique: false }, ]); }); diff --git a/packages/spec/authorable-surface.base.json b/packages/spec/authorable-surface.base.json index 4da66f15de..5b95013956 100644 --- a/packages/spec/authorable-surface.base.json +++ b/packages/spec/authorable-surface.base.json @@ -1,6 +1,6 @@ { "description": "In-tree anchor for the authorable-surface deletion gate (#4650, #5235): a verbatim copy of the keys in authorable-surface.json as they stood at `baseRev`, a commit on origin/main. A build that CAN reach origin/main anchors on the merge base instead, and re-verifies this file against `baseRev` — so a PR that edits it to hide a deletion goes red wherever the network exists. A build that CANNOT reach GitHub (image-build stages, air-gapped, fork, historical-tag reproduction) anchors here instead of failing. Written only by `gen:schema`, only from a git-resolved baseline — never from the build that is being checked. See #5235.", - "baseRev": "5acb93add66435880ffed0d3aff79db29ae1e932", + "baseRev": "fc5f536a13b3217c74e18788566200903644e0eb", "keys": [ "ai/AIModelConfig:maxTokens", "ai/AIModelConfig:model", @@ -529,6 +529,7 @@ "api/ApiRoutes:data", "api/ApiRoutes:discovery", "api/ApiRoutes:i18n", + "api/ApiRoutes:mcp", "api/ApiRoutes:metadata", "api/ApiRoutes:notifications", "api/ApiRoutes:packages", @@ -875,6 +876,7 @@ "api/Discovery:name", "api/Discovery:routes", "api/Discovery:schemaDiscovery", + "api/Discovery:scoping", "api/Discovery:services", "api/Discovery:version", "api/DispatcherConfig:fallback", @@ -1063,6 +1065,7 @@ "api/GetDiscoveryResponse:name", "api/GetDiscoveryResponse:routes", "api/GetDiscoveryResponse:schemaDiscovery", + "api/GetDiscoveryResponse:scoping", "api/GetDiscoveryResponse:services", "api/GetDiscoveryResponse:version", "api/GetEffectivePermissionsResponse:objects", @@ -3814,6 +3817,7 @@ "data/StateMachineValidation:type", "data/StringOperator:$contains", "data/StringOperator:$endsWith", + "data/StringOperator:$icontains", "data/StringOperator:$notContains", "data/StringOperator:$startsWith", "data/TenancyConfig:enabled", @@ -6923,6 +6927,9 @@ "ui/ActionParam:requiresFeature", "ui/ActionParam:type", "ui/ActionParam:visible", + "ui/ActionSession:organizationId", + "ui/ActionSession:roles", + "ui/ActionSession:userId", "ui/AddRecordConfig:enabled", "ui/AddRecordConfig:formView", "ui/AddRecordConfig:mode", @@ -7699,6 +7706,7 @@ "ui/RecordDetailsProps:aria", "ui/RecordDetailsProps:columns", "ui/RecordDetailsProps:fields", + "ui/RecordDetailsProps:hideFields", "ui/RecordDetailsProps:layout", "ui/RecordDetailsProps:sections", "ui/RecordHighlightsProps:aria", diff --git a/packages/spec/authorable-surface.json b/packages/spec/authorable-surface.json index 31cd8796ef..30541001af 100644 --- a/packages/spec/authorable-surface.json +++ b/packages/spec/authorable-surface.json @@ -3437,8 +3437,8 @@ "data/ImportFieldMapping:transform", "data/Index:fields", "data/Index:name", - "data/Index:partial", - "data/Index:type", + "data/Index:partial [RETIRED]", + "data/Index:type [RETIRED]", "data/Index:unique", "data/JSONValidation:_lock", "data/JSONValidation:_lockDocsUrl", diff --git a/packages/spec/liveness/object.json b/packages/spec/liveness/object.json index fd0aa0e577..0d7b6b7e8d 100644 --- a/packages/spec/liveness/object.json +++ b/packages/spec/liveness/object.json @@ -78,7 +78,8 @@ "indexes": { "status": "live", "evidence": "packages/drivers/driver-sql/src/sql-driver.ts:1181", - "note": "DDL." + "verifiedAt": "2026-08-06", + "note": "DDL — but only through `name`/`fields`/`unique`. `syncDeclaredIndexes` creates every declared index via knex's `table.index()` / `table.unique()`, and the differ's `DeclaredIndexInput` carries `name`/`fields`/`unique`/`nullSafeColumns`. The other two child keys were RETIRED 2026-08-06 (#5248, #4943, ADR-0049): `indexes[].type` selected no access method (and its `.default('btree')` materialized an inert knob into every parse output) and `indexes[].partial` produced a FULL index with the predicate discarded. Both are `retiredKey()` tombstones at `IndexSchema` and are stripped from sources by the protocol-17 conversion `object-index-type-partial-removed`. The container stays `live` because the surviving keys drive real DDL. ⚠ Not to be confused with driver-sql's own `partial: boolean`, which is parsed back out of the database's own CREATE INDEX DDL for drift detection and is unrelated to the declaration surface." }, "validations": { "status": "live", diff --git a/packages/spec/spec-changes.json b/packages/spec/spec-changes.json index d147a8658a..c15f6c5e6e 100644 --- a/packages/spec/spec-changes.json +++ b/packages/spec/spec-changes.json @@ -325,6 +325,12 @@ "to": "page-header component prop 'description' → 'subtitle' (objectui#3226 — the `subtitle ?? description` fallback retires)", "conversionId": "page-header-subtitle-alias", "toMajor": 17 + }, + { + "surface": "object.indexes[].type / object.indexes[].partial", + "to": "object index keys 'indexes[].type'/'indexes[].partial' removed (#5248, #4943 — no driver ever read either: the index method is the dialect's choice and a partial index is built by a database-layer migration, not declared)", + "conversionId": "object-index-type-partial-removed", + "toMajor": 17 } ], "migrated": [ @@ -1033,6 +1039,12 @@ "to": "page-header component prop 'description' → 'subtitle' (objectui#3226 — the `subtitle ?? description` fallback retires)", "conversionId": "page-header-subtitle-alias", "toMajor": 17 + }, + { + "surface": "object.indexes[].type / object.indexes[].partial", + "to": "object index keys 'indexes[].type'/'indexes[].partial' removed (#5248, #4943 — no driver ever read either: the index method is the dialect's choice and a partial index is built by a database-layer migration, not declared)", + "conversionId": "object-index-type-partial-removed", + "toMajor": 17 } ], "migrated": [ diff --git a/packages/spec/src/conversions/registry.ts b/packages/spec/src/conversions/registry.ts index b9f349916a..4ab6cc9303 100644 --- a/packages/spec/src/conversions/registry.ts +++ b/packages/spec/src/conversions/registry.ts @@ -3583,6 +3583,117 @@ const objectEnableTrashMruRemoved: MetadataConversion = { }, }; +/** + * `indexes[].type` / `indexes[].partial` leave the surface (protocol 17, + * #5248 + #4943, ADR-0049 enforce-or-remove). + * + * Neither key had a single DDL consumer. `SqlDriver.syncDeclaredIndexes` builds + * every declared index through knex's `table.index(fields, name)` / + * `table.unique(fields, { indexName })`, and the differ's `DeclaredIndexInput` + * declares `name` / `fields` / `unique` / `nullSafeColumns` — so an authored + * `type` selected no access method and an authored `partial` produced a FULL + * index with the predicate silently discarded. `type` additionally carried + * `.default('btree')`, so it was materialized into *every* parse output: the + * ADR-0078 shape where an inert knob reads as live configuration. + * + * The maintainer chose remove over enforce (#5248, 2026-08-06): enforcing means + * per-dialect algorithm mapping, raw-SQL `CREATE INDEX … WHERE` (MySQL has no + * partial index), and reworking how `isSyncReproducibleIndex` excludes partial + * indexes from incremental sync — design cost for a capability with no demand. + * + * ⚠️ NOT the same `partial` as the driver's: `schema-drift.ts` carries a + * `partial: boolean` parsed back out of the DATABASE's own `CREATE INDEX` DDL + * (`parseIndexDdl`), consumed by drift detection. That is untouched here, and + * the exemption it grants DB-authored partial indexes still stands. + * + * `retiredFromLoadPath`: both keys are tombstoned at `IndexSchema`, so a live + * author is taught by the parse error. This entry exists so stored ≤16 rows + * replay clean through `applyConversionsToStoredItem` (without it a + * pre-removal row would flag `metadata_spec_invalid` forever) and so + * `os migrate meta --from 16` rewrites sources mechanically. + */ +const objectIndexTypePartialRemoved: MetadataConversion = { + id: 'object-index-type-partial-removed', + toMajor: 17, + retiredFromLoadPath: true, + surface: 'object.indexes[].type / object.indexes[].partial', + summary: + "object index keys 'indexes[].type'/'indexes[].partial' removed (#5248, #4943 — no driver " + + 'ever read either: the index method is the dialect\'s choice and a partial index is built ' + + 'by a database-layer migration, not declared)', + apply(stack, emit) { + // `indexes` is an ARRAY one level down, so `stripKeys` (top-level only) + // cannot reach it — drill in and copy-on-write, so an object whose indexes + // carry neither key keeps its identity. Both `objects[]` and + // `objectExtensions[]` embed `IndexSchema`, so both are walked. + const stripIndexes = (owner: Dict, path: string): Dict => { + const indexes = owner.indexes; + if (!Array.isArray(indexes)) return owner; + let changed = false; + const next = indexes.map((idx, i) => { + if (!isDict(idx)) return idx; + const stripped = stripKeys(idx, ['type', 'partial'], emit, `${path}.indexes[${i}]`); + if (stripped !== idx) changed = true; + return stripped; + }); + return changed ? { ...owner, indexes: next } : owner; + }; + let out = mapCollection(stack, 'objects', stripIndexes); + out = mapCollection(out, 'objectExtensions', stripIndexes); + return out; + }, + fixture: { + before: { + objects: [ + { + name: 'crm_invoice', + label: 'Invoice', + indexes: [ + // both retired keys on one index, alongside surviving ones + { + name: 'idx_invoice_active_no', + fields: ['invoice_no'], + unique: 'global', + type: 'btree', + partial: "state = 'active'", + }, + // an index carrying neither key passes through untouched + { name: 'idx_invoice_customer', fields: ['customer'] }, + ], + }, + ], + objectExtensions: [ + { + extend: 'crm_invoice', + indexes: [{ fields: ['tags'], type: 'gin' }], + }, + ], + }, + // Three notices: two keys on the object's first index, one on the + // extension's. The surviving `name`/`fields`/`unique` prove the strip is + // surgical rather than an index-level delete. + after: { + objects: [ + { + name: 'crm_invoice', + label: 'Invoice', + indexes: [ + { name: 'idx_invoice_active_no', fields: ['invoice_no'], unique: 'global' }, + { name: 'idx_invoice_customer', fields: ['customer'] }, + ], + }, + ], + objectExtensions: [ + { + extend: 'crm_invoice', + indexes: [{ fields: ['tags'] }], + }, + ], + }, + expectedNotices: 3, + }, +}; + /** * The retry policy converges to one declaration (protocol 17, #4661 — the * #4535 C8 dual-source cluster). @@ -4321,6 +4432,7 @@ export const CONVERSIONS_BY_MAJOR: Readonly { expect(parsed.indexes[0].fields).toEqual(['name']); }); - it('the drift is real: the console offers `where`, this schema declares `partial`', () => { + it('the console drift outlived BOTH spellings: `where` strips, `partial` is now retired', () => { // objectui `metadata-admin/EmbeddedItemEditor.tsx` → FALLBACK_SCHEMAS.index - // publishes `where` for the partial-index predicate. The spec's key is - // `partial`. The editor splices its output into `object.indexes[]` and PUTs - // the whole object, and `saveMetaItem` keeps the body verbatim — so closing - // this shape 422s a control the console itself renders (the #5114 class). - accept(IndexSchema, { fields: ['name'], partial: "status = 'open'" }); + // publishes `where` for the partial-index predicate. The spec's key WAS + // `partial` — until #5248 / #4943 retired it (no driver ever emitted the + // predicate). So the editor now renders a control for a key that does not + // exist under either spelling, which is #5247's job to delete. + // + // The two halves fail DIFFERENTLY, and that difference is the point: + // - `where` was never declared here, and the shape still `.strip()`s, so + // it is discarded in silence — the held-open defect above; + // - `partial` IS declared, as a tombstone, so it is rejected loudly with + // the migration prescription rather than stripped (the whole reason a + // non-strict schema gets a tombstone instead of a plain delete). const parsedWhere = accept(IndexSchema, { fields: ['name'], where: "status = 'open'" }) as Record; expect(parsedWhere.where, 'the console spelling is silently discarded today').toBeUndefined(); + + const retired = IndexSchema.safeParse({ fields: ['name'], partial: "status = 'open'" }); + expect(retired.success, '`partial` is retired, not stripped').toBe(false); + expect(retired.error!.issues[0]!.message).toMatch(/`indexes\[\]\.partial` was removed.*Delete the key/s); }); it('its SIBLINGS in the same file are closed — this is a held site, not an unwalked file', () => { diff --git a/packages/spec/src/data/object.test.ts b/packages/spec/src/data/object.test.ts index 0dcddb80c0..e24bebcb62 100644 --- a/packages/spec/src/data/object.test.ts +++ b/packages/spec/src/data/object.test.ts @@ -229,6 +229,62 @@ describe('IndexSchema', () => { }); }); +/** + * `indexes[].type` / `indexes[].partial` retirement (#5248, #4943, ADR-0049). + * + * Both were authorable with zero DDL consumers. `IndexSchema` is not + * `.strict()`, so a plain delete would have Zod strip an authored value in + * silence — swapping one no-op for another (#3726 / #3733, the ADR-0104 + * class). The tombstone is what makes the removal audible, so these tests pin + * the PRESCRIPTION, not merely the rejection. + */ +describe('IndexSchema retired keys (#5248 / #4943)', () => { + it('REJECTS `type`, with the fix and the reason in the message', () => { + expect(() => IndexSchema.parse({ fields: ['tags'], type: 'gin' })) + .toThrow(/`indexes\[\]\.type` was removed.*no driver ever read it.*Delete the key/s); + }); + + it('REJECTS `partial`, naming the database-layer replacement', () => { + expect(() => IndexSchema.parse({ fields: ['name'], partial: "state = 'active'" })) + .toThrow(/`indexes\[\]\.partial` was removed.*Delete the key.*CREATE \[UNIQUE\] INDEX/s); + }); + + it('points at the CLI conversion rather than naming the conversion id', () => { + for (const bad of [{ fields: ['a'], type: 'btree' }, { fields: ['a'], partial: 'x' }]) { + expect(() => IndexSchema.parse(bad)).toThrow(/os migrate meta --from 16/); + } + }); + + it('the live keys are untouched — the retirement is surgical', () => { + const parsed = IndexSchema.parse({ + name: 'idx_invoice_no', + fields: ['invoice_no'], + unique: 'organization', + }); + expect(parsed).toEqual({ name: 'idx_invoice_no', fields: ['invoice_no'], unique: 'organization' }); + }); + + it('no longer materializes a phantom `btree` into every parsed index', () => { + // The louder half of the defect: `type` carried `.default('btree')`, so an + // access-method knob nothing has ever read appeared in the parse output of + // every index of every object (it was pinned in service-realtime's + // sys_presence test, on an object that never declared it). + const parsed = IndexSchema.parse({ fields: ['email'] }); + expect(parsed).not.toHaveProperty('type'); + expect(parsed).not.toHaveProperty('partial'); + expect(parsed).toEqual({ fields: ['email'], unique: false }); + }); + + it('rejects the retired keys through a whole object too, not just the sub-schema', () => { + expect(() => ObjectSchema.parse({ + name: 'crm_invoice', + label: 'Invoice', + fields: { name: { type: 'text', label: 'Name' } }, + indexes: [{ fields: ['name'], partial: "state = 'active'" }], + })).toThrow(/`indexes\[\]\.partial` was removed/s); + }); +}); + describe('ObjectSchema', () => { describe('Basic Object Properties', () => { it('should accept minimal valid object', () => { diff --git a/packages/spec/src/data/object.zod.ts b/packages/spec/src/data/object.zod.ts index cd905725f0..7ac54be5b6 100644 --- a/packages/spec/src/data/object.zod.ts +++ b/packages/spec/src/data/object.zod.ts @@ -15,6 +15,7 @@ import { MetadataProtectionFields } from '../kernel/metadata-protection.zod'; import { strictUnknownKeyError } from '../shared/suggestions.zod'; import { strictObject } from '../shared/strict-object'; import { ProtectionSchema } from '../shared/protection.zod'; +import { retiredKey } from '../shared/retired-key'; export const ApiMethod = z.enum([ 'get', 'list', // Read 'create', 'update', 'delete', // Write @@ -266,16 +267,51 @@ export const ObjectCapabilities = z.object({ /** * Schema for database indexes. - * Enhanced with additional index types and configuration options - * + * + * The declaration surface is exactly what the driver materializes: + * `name` / `fields` / `unique` (ADR-0120 scope). Nothing else — see the + * retirement note below. + * * @example * { * name: "idx_account_name", * fields: ["name"], - * type: "btree", * unique: true * } * + * ## `type` / `partial` were RETIRED at protocol 17 (#5248, #4943, ADR-0049) + * + * Both were authorable and had **zero** DDL consumers. + * `SqlDriver.syncDeclaredIndexes` builds every declared index through knex's + * `table.unique(fields, { indexName })` / `table.index(fields, name)`, and the + * differ's `DeclaredIndexInput` (`driver-sql/src/schema-drift.ts`) carries + * `name` / `fields` / `unique` / `nullSafeColumns` — neither key ever reached + * a `CREATE INDEX`. `type` was the louder of the two because it also carried + * `.default('btree')`, so it appeared in *every* parse output: a knob that had + * never influenced a single statement, rendered as live configuration. That is + * the exact shape ADR-0078 (no-silently-inert-metadata) and ADR-0049 + * (enforce-or-remove) exist to delete. + * + * The maintainer chose **remove** over **enforce** (2026-08-06, #5248): + * enforcing would mean per-dialect algorithm mapping (`gin`/`gist` Postgres-only, + * `fulltext` MySQL-only), raw-SQL `CREATE INDEX … WHERE` (MySQL has no partial + * index at all), and a redesign of how `isSyncReproducibleIndex` excludes + * partial indexes from incremental sync — real design cost for a capability + * nothing has asked for. If a genuine need appears, it comes back enforce-first. + * + * Replacements: an index **method** is the driver/dialect's choice, not a + * declaration-surface concern. A **partial** index is built at the database + * layer (a runtime migration issuing `CREATE UNIQUE INDEX … WHERE`, the way + * `metadata-protocol`'s `ensureOverlayIndex` does for `sys_metadata`); drift + * detection's exemption for DB-authored partial indexes is unaffected — + * `isSyncReproducibleIndex` reads a boolean parsed out of the database's OWN + * DDL (`parseIndexDdl`), which never had anything to do with this string. + * + * ⚠️ The tombstones sit at the BOTTOM of the shape deliberately (#5606): the + * docs renderer prints only the first `INLINE_KEY_LIMIT` keys of an inline + * shape and has no `z.never()` branch, so a tombstone high in the shape prints + * as `any` and reads as a free-form slot. + * * ## ⛔ Deliberately still `.strip()` — #4001 批 20 held this one site * * Every other inner block of this file was closed in that batch. This one was @@ -286,30 +322,18 @@ export const ObjectCapabilities = z.object({ * (`objectui` → `metadata-admin/EmbeddedItemEditor.tsx`, `FALLBACK_SCHEMAS.index`) * ships its own hand-copied JSON-Schema for this shape — the framework does not * publish one, because `index` is an embedded-only sub-type with no metadata - * type of its own. That copy has drifted: - * - * - it offers **`where`** for the partial-index predicate; this schema - * declares **`partial`**; - * - it offers **`brin`** in the algorithm enum, which this schema does not. - * - * The editor splices its form output into `object.indexes[]` and PUTs the whole - * object, and `saveMetaItem` keeps the body verbatim while validating it. So - * today an admin who fills in "Partial-index predicate" gets a clean save and a - * key that this schema silently drops on every later parse — and closing this - * shape would turn that same click into a 422 on a control the console itself - * renders (the #5114 class). - * - * Closing it is still the right end state, but it is gated on the producer - * being fixed first (contract-first: the drift is in the copy, not here), and - * on an ADR-0049 answer for `type`/`partial` (#5247 / #5248) — neither is read by any driver - * (`syncDeclaredIndexes` in `driver-sql` consumes `name`/`fields`/`unique` - * only), so pointing an author at `partial` today would be a guidance entry - * that claims more than the platform delivers (ledger finding 18). + * type of its own. That copy has drifted: it offers **`where`** for the partial + * predicate and **`brin`** in the algorithm enum. Both of its drifted controls + * now edit keys that no longer exist at all, which is #5247's job to delete + * (it is `Blocked-by` this retirement). Until that lands, an admin filling in + * "Partial-index predicate" still gets a clean save of a key this schema drops + * — closing the shape would turn that same click into a 422 on a control the + * console itself renders (the #5114 class), so the strip stays until the + * producer is fixed (contract-first: the drift is in the copy, not here). */ export const IndexSchema = lazySchema(() => z.object({ name: z.string().optional().describe('Index name (auto-generated if not provided)'), fields: z.array(z.string()).describe('Fields included in the index'), - type: z.enum(['btree', 'hash', 'gin', 'gist', 'fulltext']).optional().default('btree').describe('Index algorithm type'), // Unique scope on a DECLARED index (ADR-0120 D1, amending #3696): // // - `'global'` — the VERBATIM contract: materialized over exactly the @@ -337,7 +361,36 @@ export const IndexSchema = lazySchema(() => z.object({ // but new code says `unique: 'organization'` — the hand-written composite // is NOT NULL-safe (#5030). unique: UniqueScopeSchema.optional().default(false).describe("Whether the index enforces uniqueness, and at which scope (ADR-0120). 'global' = materialized over exactly `fields`, no organization column injected — one holder across the whole installation; 'organization' = the driver prepends the NULL-safe organization key part (COALESCE(organization_id, '__global__')) at registration — one holder per organization; bare true = deprecated positional spelling of 'global' (warned in 17.x by lint unique/unscoped-declared-index, rejected at protocol 18, #5082) — state the scope. 'tenant'/'org' are rejected — the word is 'organization'"), - partial: z.string().optional().describe('Partial index condition (SQL WHERE clause for conditional indexes)'), + + // ── Tombstones (ADR-0049 / ADR-0087) ───────────────────────────────── + // Kept LAST in the shape on purpose — see the #5606 note in the block + // comment above. `IndexSchema` is not `.strict()`, so a plain delete would + // make Zod strip an authored value silently, which is the same no-op these + // keys already were (#3726 / #3733, the ADR-0104 class). The tombstone + // makes the removal audible in the two channels an upgrading author + // actually reads: `tsc` (input type `never`) and the parse itself. + // `object-index-type-partial-removed` strips both from stored/authored + // sources on the protocol-17 migration. + type: retiredKey( + '`indexes[].type` was removed in @objectstack/spec 17.0.0 (#5248, ADR-0049) — no driver ever ' + + 'read it. `SqlDriver.syncDeclaredIndexes` creates every declared index through knex\'s ' + + '`table.index()` / `table.unique()`, which cannot express an access method, so the value ' + + 'changed no DDL; its `.default(\'btree\')` merely made an inert knob show up in every parse ' + + 'output. Delete the key. The index method is the driver/dialect\'s decision (Postgres ' + + 'defaults to B-tree; `gin`/`gist`/`fulltext` are dialect-specific and are chosen by a ' + + 'database-layer migration when a workload actually needs one). ' + + 'Run `os migrate meta --from 16` to rewrite it automatically.', + ), + partial: retiredKey( + '`indexes[].partial` was removed in @objectstack/spec 17.0.0 (#5248, #4943, ADR-0049) — no ' + + 'driver ever emitted the `WHERE` clause, so a declared partial index was materialized as a ' + + 'FULL index and the predicate silently did nothing. Delete the key. Partial indexes are ' + + 'built at the database layer, not the declaration surface: issue `CREATE [UNIQUE] INDEX … ' + + 'WHERE ` from a runtime migration (this is what `metadata-protocol`\'s ' + + '`ensureOverlayIndex` already does for `sys_metadata`). Drift detection is unaffected — it ' + + 'reads partiality back from the database\'s own DDL, never from this key. ' + + 'Run `os migrate meta --from 16` to rewrite it automatically.', + ), })); /** diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index 6b0c49d558..fd44d56ce6 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -991,7 +991,27 @@ const step17: MigrationStep = { + 'live vocabulary is untouched and deliberately elsewhere: gate on `session.userId` / ' + '`session.isSystem` in the hook, and judge PRIVILEGE through the security service, which ' + 'reads capability grants (`permissions`), placements (`positions`) and the derived posture ' - + 'off the execution context.', + + 'off the execution context.\n\n' + + 'Finally it retires the two inert `IndexSchema` keys, `indexes[].type` and ' + + '`indexes[].partial` (#5248, #4943). Neither ever had a DDL consumer: ' + + '`SqlDriver.syncDeclaredIndexes` creates declared indexes through knex\'s `table.index()` / ' + + '`table.unique()`, and the drift differ\'s `DeclaredIndexInput` carries only ' + + '`name`/`fields`/`unique`/`nullSafeColumns` — so an authored `type` selected no access ' + + 'method and an authored `partial` produced a FULL index with its predicate discarded. ' + + '`partial` was the more damaging of the two because it read as a correctness control: the ' + + 'platform\'s own `sys_metadata` declared it for overlay uniqueness, and what the ' + + 'declaration alone materialized was an unrestricted unique index (the active-row scoping ' + + 'is delivered by a runtime migration, `metadata-protocol`\'s `ensureOverlayIndex`, not by ' + + 'the key). `type` was the louder: its `.default(\'btree\')` put an inert knob into every ' + + 'parse output, so it read as live configuration — the ADR-0078 no-silently-inert shape. ' + + 'Remove was chosen over enforce (maintainer ruling, 2026-08-06): enforcing needs ' + + 'per-dialect algorithm mapping (`gin`/`gist` Postgres-only, `fulltext` MySQL-family), ' + + 'raw-SQL `CREATE INDEX … WHERE` on the dialects that have partial indexes at all (MySQL ' + + 'does not), and a redesign of how `isSyncReproducibleIndex` excludes partial indexes from ' + + 'incremental sync — design cost for a capability nothing has asked for. Both are lossless ' + + 'deletes: no DDL changes, because no DDL ever depended on them. Drift detection is ' + + 'untouched — the `partial` flag it consumes is parsed back out of the database\'s OWN ' + + '`CREATE INDEX` DDL and never came from this key.', conversionIds: [ 'action-execute-to-target', 'field-conditionalRequired-to-requiredWhen', @@ -1036,6 +1056,7 @@ const step17: MigrationStep = { 'dashboard-widget-compareto-converged', 'theme-inert-token-scales-removed', 'page-header-subtitle-alias', + 'object-index-type-partial-removed', ], semantic: [ { diff --git a/skills/objectstack-data/SKILL.md b/skills/objectstack-data/SKILL.md index f9d3d708f8..c8ef3bf05b 100644 --- a/skills/objectstack-data/SKILL.md +++ b/skills/objectstack-data/SKILL.md @@ -379,17 +379,24 @@ See [rules/validation.md](./rules/validation.md) for all types and examples. ### Index Patterns -**Omit default values:** `type` defaults to `'btree'`, `unique` defaults to `false`. +**The whole declaration surface is `fields` / `unique` / `name`.** `unique` +defaults to `false`; omit it when that is what you mean. ```typescript indexes: [ - { fields: ['status', 'created_at'] }, // btree (default) + { fields: ['status', 'created_at'] }, // composite { fields: ['email'], unique: 'organization' }, // unique per organization { fields: ['hostname'], unique: 'global' }, // unique platform-wide - { fields: ['description'], type: 'fulltext' }, // non-default type + { name: 'idx_acct_status', fields: ['status'] }, // custom name ] ``` +> **`type` and `partial` were retired at protocol 17** (#5248, #4943): no driver +> ever read either, so an authored `type` chose no access method and an authored +> `partial` produced a full index with the predicate discarded. Both are now a +> `tsc` error and a parse error; `os migrate meta --from 16` strips them. Access +> methods and partial predicates are database-layer migrations. + > **A unique index must state its scope** — `'organization'` (one holder per > organization, NULL-safe) or `'global'` (one holder across the installation). > On a declared index bare `unique: true` is the deprecated spelling of @@ -397,7 +404,8 @@ indexes: [ > warns and protocol 18 rejects it. On a FIELD, `unique: true` means > `'organization'` and stays valid. -See [rules/indexing.md](./rules/indexing.md) for composite/partial/gin/gist indexes. +See [rules/indexing.md](./rules/indexing.md) for composite indexes, unique scope, +and how to build partial / gin / gist indexes at the database layer. ### Lifecycle Hooks diff --git a/skills/objectstack-data/rules/indexing.md b/skills/objectstack-data/rules/indexing.md index 6e77dd9aed..2937d8afee 100644 --- a/skills/objectstack-data/rules/indexing.md +++ b/skills/objectstack-data/rules/indexing.md @@ -9,28 +9,37 @@ ObjectStack automatically creates indexes for: - Foreign keys (lookup/master_detail fields) - Unique constraints -**Only declare non-default values.** `type` defaults to `'btree'` and `unique` defaults to `false` — omit them when using defaults. +**Only declare non-default values.** `unique` defaults to `false` — omit it when using the default. -## Index Types +## The declaration surface is exactly three keys -| Type | Default? | When to Use | Performance | -|:-----|:---------|:------------|:------------| -| `btree` | ✅ Yes | Equality and range queries (`=`, `<`, `>`, `BETWEEN`) | Excellent | -| `hash` | No | Exact equality only (`=`) — rare use case | Fast for `=`, poor for ranges | -| `fulltext` | No | Text search columns (descriptions, notes) | Text search only | -| `gin` | No | Array / JSONB containment, full-text search | JSONB, arrays, tags | -| `gist` | No | Geospatial / range types | Location, geometry | +A declared index has `name`, `fields` and `unique`. That is the whole surface, +and it is the whole surface *because* it is all the driver materializes: +`syncDeclaredIndexes` creates every declared index through knex's +`table.index(fields, name)` / `table.unique(fields, { indexName })`. + +| Key | Required | Meaning | +|:----|:---------|:--------| +| `fields` | ✅ | The indexed columns, in order (left-to-right rule below) | +| `unique` | optional | Uniqueness **and its scope** — see ADR-0120 section below | +| `name` | optional | Custom index name; auto-generated when omitted | + +> **Retired at protocol 17 (#5248, #4943): `type` and `partial`.** Both were +> authorable and neither was ever read by any driver — an authored `type` +> selected no access method, and an authored `partial` produced a **full** +> index with the predicate silently discarded. Writing either is now a `tsc` +> error and a parse error carrying the migration prescription; run +> `os migrate meta --from 16` to strip them automatically. What to do instead +> is the subject of "Access methods and partial indexes" below. ## Syntax ```typescript indexes: [ - { fields: ['status', 'created_at'] }, // btree (default) - { fields: ['email'], unique: 'organization' }, // btree + unique per org - { fields: ['hostname'], unique: 'global' }, // btree + unique platform-wide - { fields: ['description'], type: 'fulltext' }, // non-default type - { fields: ['tags'], type: 'gin' }, // non-default type - { fields: ['location'], type: 'gist' }, // non-default type + { fields: ['status', 'created_at'] }, // plain composite + { fields: ['email'], unique: 'organization' }, // unique per org + { fields: ['hostname'], unique: 'global' }, // unique platform-wide + { name: 'idx_acct_status', fields: ['status'] }, // custom name ] ``` @@ -85,7 +94,8 @@ Notes an author has to know: 1. **Join columns** — Non-foreign-key join fields 2. **Frequent aggregations** — GROUP BY columns 3. **Range queries** — Date ranges, numeric ranges -4. **Partial data** — Use partial indexes for subset queries +4. **Subset queries** — a partial index can help, but it is a database-layer + migration, not a declaration (see below) ### ❌ Avoid Indexing @@ -127,82 +137,25 @@ indexes: [ ] ``` -### Partial Index - -```typescript -indexes: [ - // Only index active records - { - fields: ['created_at'], - partial: "status = 'active'", - }, - - // Only index non-deleted records - { - fields: ['email'], - unique: 'organization', - partial: "deleted_at IS NULL", - }, -] -``` - -### Full-Text Index - -```typescript -indexes: [ - { - fields: ['description', 'notes'], - type: 'fulltext', - }, -] -``` - -### GIN Index (JSONB/Array) - -```typescript -indexes: [ - // JSONB field - { - fields: ['metadata'], - type: 'gin', - }, - - // Array field - { - fields: ['tags'], - type: 'gin', - }, -] -``` - -### Geospatial Index (GIST) - -```typescript -indexes: [ - { - fields: ['location'], - type: 'gist', - }, -] -``` - ## Incorrect vs Correct -### ❌ Incorrect — Redundant Default Values +### ❌ Incorrect — Retired and Redundant Keys ```typescript indexes: [ - { fields: ['status'], type: 'btree', unique: false }, // ❌ Redundant defaults - { fields: ['email'], type: 'btree', unique: 'organization' }, // ❌ Redundant type + { fields: ['status'], type: 'btree', unique: false }, // ❌ `type` retired; `unique: false` redundant + { fields: ['description'], type: 'fulltext' }, // ❌ `type` retired (#5248) + { fields: ['created_at'], partial: "status = 'active'" }, // ❌ `partial` retired (#5248) ] ``` -### ✅ Correct — Omit Defaults +### ✅ Correct — Declare Only What the Driver Materializes ```typescript indexes: [ - { fields: ['status'] }, // ✅ btree and unique: false are defaults - { fields: ['email'], unique: 'organization' }, // ✅ btree is default; the scope is required + { fields: ['status'] }, // ✅ unique: false is the default + { fields: ['email'], unique: 'organization' }, // ✅ the scope is required + { fields: ['description', 'notes'] }, // ✅ plain index; see below for full-text ] ``` @@ -281,36 +234,47 @@ Place most **selective** (unique) fields first, then range/sort fields last. { fields: ['created_at', 'status', 'tenant_id'] } ``` -## Partial Indexes +## Access methods and partial indexes -Use partial indexes to index only a subset of rows: +Both are real database capabilities. Neither is part of the **declaration** +surface, and the keys that used to pretend otherwise (`type`, `partial`) were +retired at protocol 17 (#5248, #4943) precisely because nothing consumed them. -```typescript -// Only index active records (common query) -{ - fields: ['created_at'], - partial: "status = 'active'", -} +**Access method (`btree` / `hash` / `gin` / `gist` / `fulltext`).** The driver +and dialect decide. Postgres defaults to B-tree, which is the right choice for +the equality, range and sort patterns this guide is about. The specialised +methods are dialect-specific — `gin`/`gist` are Postgres, `fulltext` is +MySQL-family — so a portable declaration could not name one anyway. When a +workload genuinely needs one, issue it from a database-layer migration against +the dialect you are actually running. -// Only index high-value accounts -{ - fields: ['annual_revenue'], - partial: "annual_revenue > 1000000", -} +**Partial index (`CREATE INDEX … WHERE `).** Supported on Postgres +and SQLite (≥ 3.8.9), absent on MySQL. Because it cannot be expressed +portably — and because knex's index builders have no way to emit a predicate — +it is issued as raw SQL from a runtime migration. The platform does exactly +this for its own overlay uniqueness: `metadata-protocol`'s `ensureOverlayIndex` +runs -// Only index non-deleted records -{ - fields: ['email'], - unique: 'organization', - partial: "deleted_at IS NULL", -} +```sql +CREATE UNIQUE INDEX IF NOT EXISTS idx_sys_metadata_overlay_active + ON sys_metadata (type, name, organization_id, COALESCE(package_id, '')) + WHERE state = 'active'; ``` -**Benefits:** +with a plain-index fallback for dialects that reject the predicate. Follow that +shape: declare the coarse index (or none) in metadata, and build the partial +form in a migration. + +**Benefits of a partial index, when you do build one:** - Smaller index size - Faster writes (fewer rows to maintain) - Faster queries (focused data subset) +> Drift detection understands database-authored partial indexes and leaves them +> alone — it reads partiality back out of the database's own DDL, so a partial +> index you create in a migration is not reported as drift and is never +> targeted by `os migrate apply --allow-destructive`. + ## Performance Trade-offs ### Index Benefits @@ -367,8 +331,8 @@ SHOW INDEX FROM your_table; 1. **Index foreign keys** — Always (automatic in ObjectStack) 2. **Composite for common queries** — Combine frequently filtered columns 3. **Order matters** — Most selective field first -4. **Partial for subsets** — Index only relevant rows -5. **Unique for constraints** — Enforce at DB level +4. **Partial for subsets** — but build it in a migration, not a declaration +5. **Unique for constraints** — Enforce at DB level, and always state the scope 6. **Monitor usage** — Remove unused indexes 7. **Limit total indexes** — Balance read/write performance 8. **Avoid over-indexing** — More indexes ≠ better performance @@ -395,29 +359,18 @@ indexes: [ ] ``` -### Text Search - -```typescript -// Query: WHERE description ILIKE '%keyword%' -indexes: [ - { fields: ['description'], type: 'fulltext' }, -] -``` - -### Array/JSONB Containment - -```typescript -// Query: WHERE tags @> ['urgent'] -indexes: [ - { fields: ['tags'], type: 'gin' }, -] -``` +### Text Search / Containment / Geospatial -### Location-Based Queries +These three want a specialised access method (`fulltext`, `gin`, `gist`), which +is **not** declarable — see "Access methods and partial indexes" above. Declare +the plain index if the column is also filtered or sorted normally, and create +the specialised one from a database-layer migration on the dialect you run. ```typescript -// Query: WHERE ST_DWithin(location, point, distance) +// Declaration: plain, portable, and all the driver can build indexes: [ - { fields: ['location'], type: 'gist' }, + { fields: ['description'] }, // text search: add a fulltext/GIN index in a migration + { fields: ['tags'] }, // containment (tags @> [...]): GIN, in a migration + { fields: ['location'] }, // ST_DWithin(...): GIST, in a migration ] ```