diff --git a/.changeset/diagnostics-clean-baseline.md b/.changeset/diagnostics-clean-baseline.md new file mode 100644 index 0000000000..fae18d808b --- /dev/null +++ b/.changeset/diagnostics-clean-baseline.md @@ -0,0 +1,58 @@ +--- +"@objectstack/objectql": patch +"@objectstack/runtime": patch +--- + +fix(objectql,runtime): stop the platform's own stamps from failing spec validation — `/meta/diagnostics` reads clean again (#7561) + +`GET /api/v1/meta/diagnostics` reported **94 of 94** registry entries INVALID — +every entry, `sys_*` and `showcase_*` alike. The endpoint reports entries that +fail their registered Zod schema, so at a 94/94 baseline it carried **no +signal**: a genuinely broken object was indistinguishable from a healthy one, +and any gate or dashboard built on it read permanently red. + +Both error shapes behind the 94 were self-inflicted — the platform reporting +defects about columns it wrote itself, on documents no author wrote or could +fix. + +**`fields.__search: Unrecognized key 'index'`.** `provisionSearchCompanion` +stamped `index: true` on the hidden `__search` companion column. Field-level +`index` was removed from `FieldSchema` in the 16.x line (#2377, ADR-0049) +because a field-level index flag built no index, and `FieldSchema` is a +`strictObject`, so the key was rejected by name. The companion is provisioned +before the document is stored and `/meta` re-parses the served body, so the +stamp badged `_diagnostics: { valid: false }` onto every object the platform +provisions a companion for. This is the #6810 mechanism one field over +(`applySystemFields` stamping `indexed` on `organization_id`), and the same +retired key. The stamp is gone, along with the docblock claim that the column +"IS `index`ed". + +Unlike #6810 the index is **not** re-declared in the object's `indexes[]`, and +that difference is measured rather than overlooked. #6810's predicate is +`organization_id = ?` — equality, which a B-tree serves. This column's only +reader is `buildSearchFilter`, which emits `{ __search: { $contains: term } }` +— a leading-wildcard `LIKE '%term%'` no B-tree can answer — and `IndexSchema` +spells nothing else (`name` / `fields` / `unique`; no trigram/GIN method). +Declaring one would buy write amplification on every row for a read path that +cannot use it. Search behaviour is unchanged either way: nothing read the flag. + +**`config: expected record, received undefined`.** The datasource-visibility +registration in `DefaultDatasourcePlugin` published the `default` row without +`config`, which `DatasourceSchema` requires. It is now stamped `{}` — +deliberately empty, not the host's real config, which carries connection +credentials that would otherwise land on `GET /api/v1/meta/datasources` for +every metadata reader. No information is lost versus the omitted key; only the +spelling changes to the one the contract accepts. Fixed at the producer rather +than by widening the spec: a real datasource document genuinely needs its +config, so relaxing the schema would trade one honest verdict for a permanently +weaker one. + +**So it stops recurring.** Two pins land with the fix, because patching one key +at a time is what turned #6810 into this card. A class pin walks every field +the platform stamps — `applySystemFields` and `provisionSearchCompanion`, +across every ownership / tenancy / `systemFields` branch — through +`FieldSchema`, so the next retired-key stamp turns a suite red instead of +poisoning diagnostics. A baseline pin asserts a realistically-built registry +sweeps clean, naming both of this card's error shapes explicitly; the 94/94 +state survived undetected until a human read the endpoint by hand, because +nothing asserted the baseline. diff --git a/packages/objectql/src/diagnostics-clean-baseline.test.ts b/packages/objectql/src/diagnostics-clean-baseline.test.ts new file mode 100644 index 0000000000..ebf2edeaa8 --- /dev/null +++ b/packages/objectql/src/diagnostics-clean-baseline.test.ts @@ -0,0 +1,151 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#7561] THE BASELINE PIN — a healthy registry validates clean. + * + * ## Why this file exists + * + * `GET /api/v1/meta/diagnostics` reports every metadata entry that fails its + * registered Zod schema. That verdict is only worth reading if the baseline is + * ZERO: at 94 of 94 entries INVALID — where the platform's own + * `applySystemFields`/`provisionSearchCompanion` stamps supplied both error + * shapes — a genuinely broken object is indistinguishable from the baseline, + * and any gate or dashboard built on the endpoint reads permanently red. + * + * The 94/94 state was not caught by any suite. It survived until a human read + * the endpoint by hand during a QA run (#7514), which is the actual failure + * this file addresses: nothing asserted the baseline. So this pin walks a + * registry built the way the platform builds one — every stamper live, the + * tenancy branch that #6810 broke armed — and asserts the sweep finds NOTHING. + * + * Companion to `stamped-system-fields-spec-conformance.test.ts`: that one pins + * the PRODUCERS (whatever a stamper writes, `FieldSchema` accepts); this one + * pins the OBSERVABLE the card was filed against (the served documents sweep + * clean). A regression that slipped past the first — a bad key on a document + * assembled somewhere other than a stamper — still fails here. + */ + +import { computeMetadataDiagnostics } from '@objectstack/metadata-protocol'; +import { describe, it, expect } from 'vitest'; + +import { SchemaRegistry } from './registry.js'; + +const PKG = 'showcase'; + +/** + * A baseline shaped like the registry the card was filed against: `sys_*` and + * `showcase_*` alike, spanning the branches that decide which system columns + * get stamped, and every one carrying a title-eligible field so the `__search` + * companion is actually provisioned (an object with no companion could not + * reproduce #7561 and would pass vacuously). + */ +const OBJECTS: Array> = [ + { name: 'showcase_account', label: 'Account', fields: { name: { type: 'text' }, revenue: { type: 'number' } } }, + { + name: 'showcase_contact', + label: 'Contact', + ownership: 'user', + fields: { name: { type: 'text' }, email: { type: 'email' } }, + }, + { + name: 'showcase_order', + label: 'Order', + ownership: 'org', + fields: { name: { type: 'text' }, total: { type: 'currency' } }, + }, + { name: 'sys_thing', label: 'Thing', fields: { name: { type: 'text' } } }, + { + name: 'showcase_note', + label: 'Note', + managedBy: 'platform', + fields: { name: { type: 'text' }, body: { type: 'textarea' } }, + }, +]; + +/** + * The `datasource` row `DefaultDatasourcePlugin.registerVisibility` publishes. + * Reproduced as a literal rather than imported because the pin is about the + * SHAPE that reaches the metadata list — importing `@objectstack/runtime` here + * would invert the package dependency (runtime depends on objectql). + * `default-datasource-plugin.ts` carries the matching `[#7561]` note. + */ +const DEFAULT_DATASOURCE_ROW = { + name: 'default', + label: 'Default', + driver: 'sqlite', + config: {}, + origin: 'code', +}; + +function buildBaseline(multiTenant: boolean): SchemaRegistry { + // `searchCompanion: true` explicitly: the flag defaults off the environment + // (`OS_SEARCH_PINYIN_ENABLED`), and a pin that silently skipped the companion + // would be green on exactly the deployments #7561 was reported from. + const registry = new SchemaRegistry({ multiTenant, searchCompanion: true }); + for (const def of OBJECTS) registry.registerObject(structuredClone(def) as any, PKG); + return registry; +} + +describe('[#7561] the baseline registry sweeps clean through /meta/diagnostics', () => { + describe.each([true, false])('multiTenant: %s', (multiTenant) => { + it('every served object document is spec-valid', () => { + const registry = buildBaseline(multiTenant); + const served = registry.getAllObjects(PKG); + expect(served.length, 'no objects registered — pin would pass vacuously').toBe(OBJECTS.length); + + // Report the SUBSTANCE — which entry, which path, which code — so a + // regression names itself instead of asserting a bare boolean. + const invalid: string[] = []; + for (const doc of served) { + const diag = computeMetadataDiagnostics('object', doc); + expect(diag, `object/${(doc as any).name}: no schema registered`).toBeDefined(); + for (const err of diag!.errors ?? []) { + invalid.push(`object/${(doc as any).name} → ${err.path}: ${err.code}`); + } + } + expect(invalid).toEqual([]); + }); + + it('the platform stamps the companion, and it is one of the entries swept', () => { + // Guards the guard: the sweep above is only meaningful while `__search` + // is actually present on the documents it validates. + const registry = buildBaseline(multiTenant); + const withCompanion = registry + .getAllObjects(PKG) + .filter((o: any) => o.fields?.__search !== undefined); + expect(withCompanion.length).toBe(OBJECTS.length); + }); + }); + + it('the default datasource row is spec-valid as registered', () => { + // The second of the card's two error shapes: `config: expected record, + // received undefined`, produced by the datasource-visibility registration + // omitting a key `DatasourceSchema` requires. + const diag = computeMetadataDiagnostics('datasource', DEFAULT_DATASOURCE_ROW); + expect(diag, 'datasource has no registered schema').toBeDefined(); + expect( + (diag!.errors ?? []).map((e) => `${e.path}: ${e.code}`), + ).toEqual([]); + }); + + it("neither of the card's two error shapes appears anywhere in the sweep", () => { + // Named explicitly so a REINTRODUCTION of either fails on the shape the + // card reported, not on a generic count. These are the two strings a human + // read off the endpoint during #7514. + const all: Array<{ entry: string; path: string; message: string }> = []; + for (const multiTenant of [true, false]) { + for (const doc of buildBaseline(multiTenant).getAllObjects(PKG)) { + for (const err of computeMetadataDiagnostics('object', doc)?.errors ?? []) { + all.push({ entry: `object/${(doc as any).name}`, path: err.path ?? '', message: err.message ?? '' }); + } + } + } + for (const err of computeMetadataDiagnostics('datasource', DEFAULT_DATASOURCE_ROW)?.errors ?? []) { + all.push({ entry: 'datasource/default', path: err.path ?? '', message: err.message ?? '' }); + } + + expect(all.filter((e) => e.path === 'fields.__search')).toEqual([]); + expect(all.filter((e) => /Unrecognized key/i.test(e.message) && /`index`/.test(e.message))).toEqual([]); + expect(all.filter((e) => e.path === 'config' && /expected record/i.test(e.message))).toEqual([]); + }); +}); diff --git a/packages/objectql/src/search-companion.test.ts b/packages/objectql/src/search-companion.test.ts index 6d5f23d3f5..5e2bd0cafa 100644 --- a/packages/objectql/src/search-companion.test.ts +++ b/packages/objectql/src/search-companion.test.ts @@ -85,7 +85,18 @@ describe('provisionSearchCompanion', () => { expect(col.readonly).toBe(true); expect(col.system).toBe(true); expect(col.searchable).toBe(false); - expect(col.index).toBe(true); + // [#7561] Was `expect(col.index).toBe(true)` — this line pinned the DEFECT + // rather than the contract. `index` is not a `FieldSchema` key (removed in + // the 16.x line, #2377 / ADR-0049, because a field-level index flag built no + // index), and `FieldSchema` is a `strictObject`, so stamping it badged every + // object carrying a companion `_diagnostics: { valid: false }` and drove + // `GET /api/v1/meta/diagnostics` to 94/94 INVALID. The key is now absent, + // and no index is declared in its place: this column's only reader is a + // `$contains`, which no B-tree serves. See the docblock in + // `search-companion.ts` and the pins in + // `stamped-system-fields-spec-conformance.test.ts`. + expect(col.index).toBeUndefined(); + expect(Object.keys(col)).not.toContain('index'); }); it('is idempotent and skips ineligible / opted-out objects unchanged', () => { diff --git a/packages/objectql/src/search-companion.ts b/packages/objectql/src/search-companion.ts index 8e159b5192..7da81b3961 100644 --- a/packages/objectql/src/search-companion.ts +++ b/packages/objectql/src/search-companion.ts @@ -128,8 +128,31 @@ export function resolveSearchCompanionSources(schema: CompanionObjectMeta | unde * it never appears in auto-generated views/forms, is excluded from the * `$search` auto-default (hidden fields are skipped) and from `$searchFields` * overrides (the override intersects with the allowed set), and non-system - * callers cannot forge it on update (#2948 readonly write guard). It IS - * `index`ed — every search touches it. + * callers cannot forge it on update (#2948 readonly write guard). + * + * [#7561] It carries NO index — and the stamp that claimed otherwise is gone. + * This block used to append `index: true` and the paragraph above used to read + * "It IS `index`ed — every search touches it". Both were false, in the #6810 + * shape one field over (`applySystemFields` stamping `indexed` on + * `organization_id`): `index` was removed from `FieldSchema` in the 16.x line + * (#2377, ADR-0049) because a field-level index flag built no index, and + * `FieldSchema` is a `strictObject`, so the key was rejected BY NAME. The + * companion is provisioned BEFORE the document is stored and `/meta` re-parses + * the served body, so the stamp put `_diagnostics: { valid: false, errors: + * [{ path: 'fields.__search', code: 'unrecognized_keys' }] }` on every object + * the platform provisions a companion for — a defect the platform reported + * about its own column, on a document no author wrote or could fix. + * + * Unlike #6810 the index is NOT re-declared in the object's `indexes[]`, and + * that is a measured difference rather than an omission. #6810's predicate is + * `organization_id = ?` — equality, which a B-tree serves. This column's ONLY + * reader is `buildSearchFilter`, which emits `{ __search: { $contains: term } }` + * (`search-filter.ts`) — a leading-wildcard `LIKE '%term%'` that no B-tree can + * answer, and `IndexSchema` spells nothing else (`name` / `fields` / `unique`; + * no trigram/GIN method). Declaring one would buy write amplification on every + * row for a read path that cannot use it. If the companion ever warrants a + * real substring index, it needs an `IndexSchema` that can express one — a + * separate change, not a dead index declared here. * * Objects that opt out of search entirely (`searchable: false`, ADR-0061 D2) * are skipped: a companion no query will ever read is dead weight. @@ -152,7 +175,6 @@ export function provisionSearchCompanion(schema: readonly: true, system: true, searchable: false, - index: true, description: `Search-normalized forms of the display/name field (normalizers: ${SEARCH_COMPANION_NORMALIZERS.join(', ')}) — ` + 'e.g. full pinyin + initials for CJK names. Maintained by plugin-pinyin-search; never hand-edited. See #2486.', diff --git a/packages/objectql/src/stamped-system-fields-spec-conformance.test.ts b/packages/objectql/src/stamped-system-fields-spec-conformance.test.ts new file mode 100644 index 0000000000..087282de1d --- /dev/null +++ b/packages/objectql/src/stamped-system-fields-spec-conformance.test.ts @@ -0,0 +1,139 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#7561] THE CLASS PIN — every platform-stamped field, through `FieldSchema`. + * + * ## Why this file exists rather than a third one-key patch + * + * The platform writes fields the author never wrote: `applySystemFields` adds + * the tenant/audit/owner columns, `provisionPrimary` designates and may add the + * title column, `provisionSearchCompanion` adds `__search`. All three run at + * `registerObject` time — BEFORE the document is stored — and `/meta` re-parses + * the served body through `FieldSchema`, which is a `strictObject`. So a + * stamper that writes ONE key `FieldSchema` does not admit does not produce one + * bad field: it stamps `_diagnostics: { valid: false }` on every object that + * stamper touches, on a document no author wrote and none can fix. + * + * That has now happened twice, one key and one field apart: + * + * - #6810 — `applySystemFields` spread `indexed: opts.multiTenant` onto + * `organization_id`. Every registry-backed object read answered invalid. + * - #7561 — `provisionSearchCompanion` stamped `index: true` on `__search`. + * `GET /api/v1/meta/diagnostics` answered 94 of 94 entries INVALID, and + * the baseline stayed that way until a human read it by hand. + * + * Both keys are the SAME retired key: field-level `index`/`indexed`, removed in + * the 16.x line (#2377, ADR-0049) because a field-level index flag built no + * index. Patching one stamper at a time is what produced the second card, so + * this test asserts the property for ALL of them at once: whatever the stampers + * write, `FieldSchema` accepts. A future stamper that reaches for a retired or + * misspelled key turns this suite red instead of poisoning the diagnostics + * surface for everyone. + * + * The matrix is deliberately the union of every branch each stamper has, so a + * key stamped only under one condition (`multiTenant`, an ownership mode, a + * CJK-eligible name field) cannot slip through on an unexercised path. + */ + +import { FieldSchema } from '@objectstack/spec/data'; +import { describe, it, expect } from 'vitest'; + +import { applySystemFields } from './registry.js'; +import { provisionSearchCompanion, SEARCH_COMPANION_FIELD } from './search-companion.js'; + +/** A title-eligible name field, so `provisionSearchCompanion` has a source. */ +const nameFields = () => ({ name: { type: 'text' }, amount: { type: 'number' } }); + +/** + * Every branch of the injection/provisioning passes. Same spirit as the #5378 + * parity matrix in `injected-system-columns-parity.test.ts` — that one pins + * WHICH columns get added, this one pins that WHAT gets added parses. + */ +const CASES: Array<[string, Record]> = [ + ['default business object', { name: 'crm_contact', fields: nameFields() }], + ["ownership: 'user'", { name: 'crm_contact', ownership: 'user', fields: nameFields() }], + ["ownership: 'org'", { name: 'crm_contact', ownership: 'org', fields: nameFields() }], + ["ownership: 'none'", { name: 'crm_contact', ownership: 'none', fields: nameFields() }], + ["ownership: 'business_unit'", { name: 'crm_contact', ownership: 'business_unit', fields: nameFields() }], + ['sys_* namespace', { name: 'sys_thing', fields: nameFields() }], + ["managedBy: 'platform'", { name: 'crm_contact', managedBy: 'platform', fields: nameFields() }], + ["managedBy: 'better-auth'", { name: 'crm_contact', managedBy: 'better-auth', fields: nameFields() }], + ['systemFields.tenant: false', { name: 'crm_contact', systemFields: { tenant: false }, fields: nameFields() }], + ['systemFields.audit: false', { name: 'crm_contact', systemFields: { audit: false }, fields: nameFields() }], + ['tenancy.enabled: false', { name: 'crm_contact', tenancy: { enabled: false }, fields: nameFields() }], +]; + +/** Run both stampers over `def` and return only what they ADDED. */ +function stampedFields( + def: Record, + multiTenant: boolean, +): Record> { + const before = new Set(Object.keys((def.fields ?? {}) as Record)); + const after = provisionSearchCompanion( + applySystemFields(structuredClone(def) as any, { multiTenant }) as any, + ); + const added: Record> = {}; + for (const [key, value] of Object.entries((after.fields ?? {}) as Record)) { + if (!before.has(key)) added[key] = value; + } + return added; +} + +describe('[#7561] platform-stamped system fields conform to FieldSchema', () => { + describe.each([true, false])('multiTenant: %s', (multiTenant) => { + it.each(CASES)('%s', (label, def) => { + const added = stampedFields(def, multiTenant); + // Guard the guard: a matrix row that stamps nothing would pass vacuously. + expect(Object.keys(added).length, `${label}: stamped no fields`).toBeGreaterThan(0); + + for (const [fieldName, fieldDef] of Object.entries(added)) { + const parsed = FieldSchema.safeParse(fieldDef); + // Assert the SUBSTANCE — which key, on which field — not just a boolean, + // so a regression names itself in the failure output. + expect( + parsed.success + ? [] + : parsed.error.issues.map((i) => `${[fieldName, ...i.path].join('.')}: ${i.message}`), + `${label}: stamped field \`${fieldName}\``, + ).toEqual([]); + } + }); + }); + + it('the retired field-level index key appears on NO stamped field (#2377, ADR-0049)', () => { + // The specific key both #6810 and #7561 reached for, named so a + // reintroduction under either spelling fails on the key rather than on a + // generic parse error — and so this stays true on paths a future + // `FieldSchema` edit might quietly re-admit. + for (const [label, def] of CASES) { + for (const multiTenant of [true, false]) { + for (const [fieldName, fieldDef] of Object.entries(stampedFields(def, multiTenant))) { + expect(Object.keys(fieldDef), `${label}/${fieldName} (multiTenant: ${multiTenant})`) + .not.toContain('index'); + expect(Object.keys(fieldDef), `${label}/${fieldName} (multiTenant: ${multiTenant})`) + .not.toContain('indexed'); + } + } + } + }); + + it('the `__search` companion is stamped, parses, and declares no index', () => { + // The #7561 instance specifically: the companion IS added (so the pin above + // is not passing because the column vanished), it parses clean, and no + // `indexes[]` entry was declared in place of the removed flag — the column's + // only reader is a `$contains`, which no B-tree can serve. + const provisioned = provisionSearchCompanion({ + name: 'showcase_account', + fields: nameFields(), + } as any); + const companion = (provisioned.fields as any)[SEARCH_COMPANION_FIELD]; + expect(companion, 'companion column not provisioned').toBeDefined(); + + const parsed = FieldSchema.safeParse(companion); + expect( + parsed.success ? [] : parsed.error.issues.map((i) => `${i.path.join('.')}: ${i.message}`), + ).toEqual([]); + + expect((provisioned as any).indexes).toBeUndefined(); + }); +}); diff --git a/packages/runtime/src/default-datasource-plugin.ts b/packages/runtime/src/default-datasource-plugin.ts index 39d6d2f517..21406fba69 100644 --- a/packages/runtime/src/default-datasource-plugin.ts +++ b/packages/runtime/src/default-datasource-plugin.ts @@ -242,6 +242,27 @@ export class DefaultDatasourcePlugin implements Plugin { * metadata service implements either method, in this repo or its history's * reach, so the probe never fired and the branch advertised parity it never * delivered. registerInMemory IS the datasource-visibility path. + * + * [#7561] `config` is stamped EMPTY, and deliberately so. `DatasourceSchema` + * requires it (`config: z.record(…)`, not optional), and omitting it made + * `/meta` re-parse this row into `_diagnostics: { valid: false, errors: + * [{ path: 'config', code: 'invalid_type' }] }` — the second of the two error + * shapes behind the 94/94-INVALID diagnostics baseline, alongside the + * `fields.__search` stamp. The fix belongs at this producer, not in the + * schema: a real datasource document genuinely needs its connection config, + * so widening the spec would trade one honest verdict for a permanently + * weaker one. + * + * Empty rather than `this.def.config` because this registration is a + * deliberately NON-SECRET projection. The host's real config carries + * connection credentials (`user` / `password` for postgres/mongo), and + * `DatasourceSchema` itself refuses an inlined `password` — publishing it + * here would put those credentials on `GET /api/v1/meta/datasources` for + * every metadata reader. `{}` states exactly what this row has always + * carried: the datasource EXISTS with this driver and label, and its + * connection details are not surfaced through metadata. No information is + * lost versus the omitted key — only the spelling changes, to the one the + * contract accepts. */ private async registerVisibility(ctx: PluginContext): Promise { try { @@ -251,6 +272,7 @@ export class DefaultDatasourcePlugin implements Plugin { name: 'default', label: this.def.label ?? 'Default', driver: this.def.driver, + config: {}, origin: 'code', }); }