From dd5edc46249dab4ec60a23dbaa3a19b5ce56434a Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 18:09:09 +0000 Subject: [PATCH 1/3] fix(driver-sql): emit the spec `primaryKey` column spelling, not `isPrimary` (#10676) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `SqlDriver.introspectSchema` declared its own `IntrospectedColumn` and spelled primary-key membership `isPrimary`, while everything downstream of `plugin.ts` is typed against `packages/spec/src/contracts/schema-diff-service.ts`, which spells it `primaryKey`. The value crossed between the two contracts untyped, so `ExternalDatasourceService.generateObjectDraft` — which reads `col.primaryKey` — saw `undefined` on every real driver result and every federated object drafted from a remote table silently lost its remote primary key. Maintainer ruling, 2026-08-22 (live session, 「同意所有」 item 9 = 驱动侧对齐 spec 契约): `packages/spec` is the one contract and the driver aligns to it. - `driver-sql` and `objectql/src/util.ts` now DERIVE `IntrospectedColumn` from the spec import instead of re-declaring it, so a key added to the contract fails their `tsc` until the producer emits it. Two divergences are kept explicitly (`defaultValue` stays `unknown` — Knex reports `null`; `isUnique` / `maxLength` are SQL extras the spec does not declare). - The driver emits `primaryKey` and no longer emits `isPrimary`: one spelling, so no consumer can key off the wrong one again. - New pin `external-object-draft-real-introspection.test.ts` drives `generateObjectDraft` off a REAL `introspectSchema()` result, which is what the ruling requires and what the hand-written fixtures could never do. It fails on the pre-fix driver (measured: the `// Remote primary key:` line is absent) and asserts the key does not return as the unauthorable `fields..primaryKey`. - `external-introspection-seam.test.ts` asserted the producer's OLD spelling as a deliberate landmine; its producer-spelling case is flipped to the new direction. The service's three-signal union read is untouched. Fixes #10676 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y --- ...omposite-primary-key-introspection.test.ts | 12 +- ...driver-introspection-spec-contract.test.ts | 76 +++++++++ packages/drivers/driver-sql/src/sql-driver.ts | 51 +++++- packages/objectql/src/util.test.ts | 42 ++--- packages/objectql/src/util.ts | 32 ++-- .../external-introspection-seam.test.ts | 35 ++-- ...al-object-draft-real-introspection.test.ts | 151 ++++++++++++++++++ 7 files changed, 340 insertions(+), 59 deletions(-) create mode 100644 packages/drivers/driver-sql/src/sql-driver-introspection-spec-contract.test.ts create mode 100644 packages/services/service-datasource/src/__tests__/external-object-draft-real-introspection.test.ts diff --git a/packages/drivers/driver-sql/src/sql-driver-composite-primary-key-introspection.test.ts b/packages/drivers/driver-sql/src/sql-driver-composite-primary-key-introspection.test.ts index 49a51882f8..495b542e50 100644 --- a/packages/drivers/driver-sql/src/sql-driver-composite-primary-key-introspection.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-composite-primary-key-introspection.test.ts @@ -11,8 +11,8 @@ * the first member of a composite key and silently dropped the rest. * * Both output signals were wrong together and for the same reason: - * `introspectSchema` derives `col.isPrimary` FROM `primaryKeys` - * (`if (primaryKeys.includes(col.name)) col.isPrimary = true`), so a consumer + * `introspectSchema` derives `col.primaryKey` FROM `primaryKeys` + * (`if (primaryKeys.includes(col.name)) col.primaryKey = true`), so a consumer * could not recover the missing member by cross-checking the two. Both are * asserted here. * @@ -80,7 +80,7 @@ describe('SqlDriver composite primary-key introspection (SQLite)', () => { expect(pkByName).toEqual({ order_id: 1, line_no: 2, sku: 0 }); }); - it('reports every member of a composite key, and derives isPrimary for all of them', async () => { + it('reports every member of a composite key, and derives primaryKey for all of them', async () => { await knexInstance.schema.createTable('order_lines', (t: any) => { t.string('order_id').notNullable(); t.integer('line_no').notNullable(); @@ -95,8 +95,8 @@ describe('SqlDriver composite primary-key introspection (SQLite)', () => { expect(table.primaryKeys).toEqual(['order_id', 'line_no']); // Signal 2: the per-column flag, derived FROM signal 1 — repaired with it. - const isPrimaryByName = Object.fromEntries(table.columns.map((c) => [c.name, c.isPrimary === true])); - expect(isPrimaryByName).toEqual({ order_id: true, line_no: true, sku: false }); + const primaryKeyByName = Object.fromEntries(table.columns.map((c) => [c.name, c.primaryKey === true])); + expect(primaryKeyByName).toEqual({ order_id: true, line_no: true, sku: false }); }); it('orders primaryKeys by pk ordinal, not by column position', async () => { @@ -164,7 +164,7 @@ describe('SqlDriver composite primary-key introspection (SQLite)', () => { expect(schema.tables['widgets'].primaryKeys).toEqual(['id']); expect(schema.tables['audit_lines'].primaryKeys).toEqual([]); - const auditPrimary = schema.tables['audit_lines'].columns.map((c) => c.isPrimary === true); + const auditPrimary = schema.tables['audit_lines'].columns.map((c) => c.primaryKey === true); expect(auditPrimary).toEqual([false, false]); }); }); diff --git a/packages/drivers/driver-sql/src/sql-driver-introspection-spec-contract.test.ts b/packages/drivers/driver-sql/src/sql-driver-introspection-spec-contract.test.ts new file mode 100644 index 0000000000..06d096aa4d --- /dev/null +++ b/packages/drivers/driver-sql/src/sql-driver-introspection-spec-contract.test.ts @@ -0,0 +1,76 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Pin: `SqlDriver.introspectSchema()` emits the `packages/spec` introspection + * contract — the shape every consumer downstream of `plugin.ts` is typed + * against — and not a second vocabulary of its own. + * + * The defect this closes was invisible to types on both sides. The driver + * declared its own `IntrospectedColumn` spelling key membership `isPrimary?`; + * `packages/spec/src/contracts/schema-diff-service.ts` declares `primaryKey`, + * `dialect` and a REQUIRED `introspectedAt`. Each side compiled against its + * own declaration, the value crossed between them untyped, and the consumer + * read keys no driver ever set. Maintainer ruling, 2026-08-22 (live session, + * 「同意所有」 item 9 = 驱动侧对齐 spec 契约): the driver aligns to the spec. + * + * Asserted on the BYTES of a live introspection rather than on a type, because + * a type is exactly what failed to catch this: a hand-written fixture in + * either spelling is blind to the seam. Only better-sqlite3 is executed here — + * every assertion below is on a value built at a single dialect-independent + * site in `introspectSchema`, downstream of the per-dialect helpers, so the + * SHAPE cannot vary by dialect even though the per-dialect CONTENT is not + * measured here. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { SqlDriver } from '../src/index.js'; + +describe('SqlDriver.introspectSchema emits the spec introspection contract', () => { + let driver: SqlDriver; + let knexInstance: any; + + beforeEach(async () => { + driver = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); + knexInstance = (driver as unknown as { knex: any }).knex; + await knexInstance.schema.createTable('customers', (t: any) => { + t.string('id').primary(); + t.string('name'); + t.integer('age'); + }); + }); + + afterEach(async () => { + await knexInstance?.destroy(); + }); + + it('spells column key membership `primaryKey`, the spec spelling', async () => { + const schema = await driver.introspectSchema(); + const byName = Object.fromEntries( + schema.tables['customers'].columns.map((c) => [c.name, c]), + ); + + expect(byName.id.primaryKey).toBe(true); + // Negative half: an implementation that stamped the key onto the first + // column, or onto every column, would satisfy the line above alone. + expect(byName.name.primaryKey).toBe(false); + expect(byName.age.primaryKey).toBe(false); + }); + + it('no longer emits the retired `isPrimary` spelling', async () => { + const schema = await driver.introspectSchema(); + const id = schema.tables['customers'].columns.find((c) => c.name === 'id')!; + + // `in`, not a truthiness check: the failure this closes was a consumer + // reading a key that was ABSENT, so absence is what has to be pinned. Two + // spellings emitted side by side would keep the second contract alive in + // the bytes even with both values agreeing today. + expect('isPrimary' in id).toBe(false); + expect(Object.keys(id)).toEqual( + expect.arrayContaining(['name', 'type', 'nullable', 'primaryKey']), + ); + }); +}); diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index 074283e965..3d3af01d3f 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -8,6 +8,10 @@ */ import type { DriverOptions, FilterCondition, SchemaMode } from '@objectstack/spec/data'; +// The ONE introspection contract (ADR-0015 / `ISchemaDiffService`). This +// driver's introspection types are DERIVED from these rather than +// re-declared next to them — see the `Introspection Types` region below. +import type { IntrospectedColumn as SpecIntrospectedColumn } from '@objectstack/spec/contracts'; import { parseAutonumberFormat, renderAutonumber, resolveAutonumberFormat, readAutonumberCounter, missingFieldValues, isTenancyDisabled, type AutonumberToken } from '@objectstack/spec/data'; // The DECLARED aggregate vocabulary (#5907). Read from the spec so this driver's // "the protocol has no such function" refusal cannot drift from what @@ -3645,16 +3649,45 @@ function nullSafeNegationOperand(node: Record): Record { + /** Raw driver-reported default. See the note above on why this is not `string`. */ defaultValue?: unknown; - isPrimary?: boolean; + /** SQL-introspection extra: the column carries a UNIQUE constraint. */ isUnique?: boolean; + /** SQL-introspection extra: declared maximum length for string types. */ maxLength?: number; } +/** No spec counterpart — foreign keys are a SQL-introspection extra. */ export interface IntrospectedForeignKey { columnName: string; referencedTable: string; @@ -9837,7 +9870,7 @@ export class SqlDriver implements IDataDriver { const uniqueConstraints = await this.introspectUniqueConstraints(tableName); for (const col of columns) { - if (primaryKeys.includes(col.name)) col.isPrimary = true; + if (primaryKeys.includes(col.name)) col.primaryKey = true; if (uniqueConstraints.includes(col.name)) col.isUnique = true; } @@ -12548,7 +12581,9 @@ export class SqlDriver implements IDataDriver { type, nullable: info.nullable !== false, defaultValue: info.defaultValue, - isPrimary: false, + // The spec contract's spelling, and the only one this driver emits. + // `introspectSchema` flips it from the table's key list below. + primaryKey: false, isUnique: false, maxLength, }); @@ -12695,7 +12730,7 @@ export class SqlDriver implements IDataDriver { // of the key", `1` for the first key column, `2` for the second, and so // on. Filtering on `pk === 1` therefore kept only the first member of a // composite key and silently dropped the rest, and because - // `introspectSchema` derives `col.isPrimary` FROM this list, both output + // `introspectSchema` derives `col.primaryKey` FROM this list, both output // signals were wrong together. // // Ordering by the ordinal (rather than taking `table_info`'s row order, diff --git a/packages/objectql/src/util.test.ts b/packages/objectql/src/util.test.ts index e79bb4d241..73048c0eb6 100644 --- a/packages/objectql/src/util.test.ts +++ b/packages/objectql/src/util.test.ts @@ -31,14 +31,14 @@ describe('convertIntrospectedSchemaToObjects', () => { users: { name: 'users', columns: [ - { name: 'id', type: 'integer', nullable: false, isPrimary: true }, - { name: 'name', type: 'varchar', nullable: false, maxLength: 255 }, - { name: 'email', type: 'varchar', nullable: false, isUnique: true, maxLength: 320 }, - { name: 'bio', type: 'text', nullable: true }, - { name: 'age', type: 'integer', nullable: true }, - { name: 'is_active', type: 'boolean', nullable: false, defaultValue: true }, - { name: 'created_at', type: 'timestamp', nullable: false }, - { name: 'updated_at', type: 'timestamp', nullable: true }, + { name: 'id', type: 'integer', nullable: false, primaryKey: true }, + { name: 'name', type: 'varchar', nullable: false, primaryKey: false, maxLength: 255 }, + { name: 'email', type: 'varchar', nullable: false, primaryKey: false, isUnique: true, maxLength: 320 }, + { name: 'bio', type: 'text', nullable: true, primaryKey: false }, + { name: 'age', type: 'integer', nullable: true, primaryKey: false }, + { name: 'is_active', type: 'boolean', nullable: false, primaryKey: false, defaultValue: true }, + { name: 'created_at', type: 'timestamp', nullable: false, primaryKey: false }, + { name: 'updated_at', type: 'timestamp', nullable: true, primaryKey: false }, ], foreignKeys: [], primaryKeys: ['id'], @@ -46,14 +46,14 @@ describe('convertIntrospectedSchemaToObjects', () => { posts: { name: 'posts', columns: [ - { name: 'id', type: 'integer', nullable: false, isPrimary: true }, - { name: 'title', type: 'varchar', nullable: false, maxLength: 500 }, - { name: 'body', type: 'text', nullable: true }, - { name: 'author_id', type: 'integer', nullable: false }, - { name: 'metadata', type: 'jsonb', nullable: true }, - { name: 'published_at', type: 'date', nullable: true }, - { name: 'created_at', type: 'timestamp', nullable: false }, - { name: 'updated_at', type: 'timestamp', nullable: true }, + { name: 'id', type: 'integer', nullable: false, primaryKey: true }, + { name: 'title', type: 'varchar', nullable: false, primaryKey: false, maxLength: 500 }, + { name: 'body', type: 'text', nullable: true, primaryKey: false }, + { name: 'author_id', type: 'integer', nullable: false, primaryKey: false }, + { name: 'metadata', type: 'jsonb', nullable: true, primaryKey: false }, + { name: 'published_at', type: 'date', nullable: true, primaryKey: false }, + { name: 'created_at', type: 'timestamp', nullable: false, primaryKey: false }, + { name: 'updated_at', type: 'timestamp', nullable: true, primaryKey: false }, ], foreignKeys: [ { @@ -189,10 +189,10 @@ describe('convertIntrospectedSchemaToObjects', () => { metrics: { name: 'metrics', columns: [ - { name: 'price', type: 'decimal', nullable: false }, - { name: 'weight', type: 'float', nullable: true }, - { name: 'score', type: 'real', nullable: true }, - { name: 'quantity', type: 'bigint', nullable: false }, + { name: 'price', type: 'decimal', nullable: false, primaryKey: false }, + { name: 'weight', type: 'float', nullable: true, primaryKey: false }, + { name: 'score', type: 'real', nullable: true, primaryKey: false }, + { name: 'quantity', type: 'bigint', nullable: false, primaryKey: false }, ], foreignKeys: [], primaryKeys: [], @@ -213,7 +213,7 @@ describe('convertIntrospectedSchemaToObjects', () => { schedule: { name: 'schedule', columns: [ - { name: 'start_time', type: 'time', nullable: false }, + { name: 'start_time', type: 'time', nullable: false, primaryKey: false }, ], foreignKeys: [], primaryKeys: [], diff --git a/packages/objectql/src/util.ts b/packages/objectql/src/util.ts index d5218cf35f..807930fe93 100644 --- a/packages/objectql/src/util.ts +++ b/packages/objectql/src/util.ts @@ -1,23 +1,35 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import type { ServiceObject } from '@objectstack/spec/data'; +import type { IntrospectedColumn as SpecIntrospectedColumn } from '@objectstack/spec/contracts'; // ── Introspection Types ────────────────────────────────────────────────────── /** * Column metadata from database introspection. + * + * DERIVED from `packages/spec/src/contracts/schema-diff-service.ts` — the one + * introspection contract — rather than declared a second time next to it. + * + * This file used to carry an independent declaration spelling primary-key + * membership `isPrimary?`, while `packages/spec` spells it `primaryKey`. A + * driver result flowed from one contract into a consumer typed against the + * other with no compiler between them, and the remote primary key was dropped + * on the way. Maintainer ruling, 2026-08-22 (live session, 「同意所有」 item 9 + * = 驱动侧对齐 spec 契约): `packages/spec` is the one contract, producers align + * to it, and this local declaration converges on the spec import rather than + * keeping a second contract. + * + * `defaultValue` stays `unknown` rather than the spec's `string`: SQL + * introspection reports whatever the driver read (`null`, a number, a boolean + * literal), and narrowing the declaration without normalising the value would + * move the lie rather than remove it. `isUnique` / `maxLength` are extra facts + * SQL introspection carries and the spec's diff-facing contract does not + * declare. */ -export interface IntrospectedColumn { - /** Column name */ - name: string; - /** Native database type (e.g., 'varchar', 'integer', 'timestamp') */ - type: string; - /** Whether the column is nullable */ - nullable: boolean; - /** Default value if any */ +export interface IntrospectedColumn extends Omit { + /** Default value if any — raw, as the driver reported it. */ defaultValue?: unknown; - /** Whether this is a primary key */ - isPrimary?: boolean; /** Whether this column has a unique constraint */ isUnique?: boolean; /** Maximum length for string types */ diff --git a/packages/services/service-datasource/src/__tests__/external-introspection-seam.test.ts b/packages/services/service-datasource/src/__tests__/external-introspection-seam.test.ts index 28cb5363f8..0ac0a62803 100644 --- a/packages/services/service-datasource/src/__tests__/external-introspection-seam.test.ts +++ b/packages/services/service-datasource/src/__tests__/external-introspection-seam.test.ts @@ -6,12 +6,15 @@ * * `external-datasource-service.test.ts` hand-writes its fixture with * `primaryKey: true` — the `packages/spec` contract spelling - * (`contracts/schema-diff-service.ts`). The driver emits the OTHER spelling: - * `SqlDriver.introspectSchema` sets `col.isPrimary` and fills - * `table.primaryKeys` (the `packages/objectql/src/util.ts` shape). `plugin.ts` - * hands the driver's result to this service unmodified, so the two contracts - * meet — and disagree — exactly here. A fixture written in EITHER spelling is - * blind to that; only a live introspection can see it, so every case below + * (`contracts/schema-diff-service.ts`). The driver USED TO emit the other + * spelling: `SqlDriver.introspectSchema` set `col.isPrimary` and filled + * `table.primaryKeys` (the `packages/objectql/src/util.ts` shape), `plugin.ts` + * handed that result to this service unmodified, and the two contracts met — + * and disagreed — exactly here. Since the driver was aligned to the spec + * contract (maintainer ruling 2026-08-22, 「同意所有」 item 9; #10676/#10998) + * the producer spells it `primaryKey`, and the first case below pins that + * rather than the collision. A fixture written in EITHER spelling is blind to + * this seam; only a live introspection can see it, so every case below * introspects a real in-memory SQLite database rather than describing one. * * Both directions are pinned deliberately: an implementation that stamped the @@ -77,7 +80,7 @@ async function catalogColumns( } describe('the introspection seam, as a real SqlDriver actually spells it', () => { - it('the driver speaks isPrimary/primaryKeys and never the spec spelling', async () => { + it('the driver speaks the spec spelling: primaryKey/primaryKeys', async () => { const schema = (await introspectReal(async (knex: never) => { await (knex as { schema: { createTable(n: string, cb: (t: never) => void): Promise } }).schema.createTable( 'customers', @@ -93,14 +96,18 @@ describe('the introspection seam, as a real SqlDriver actually spells it', () => const table = schema.tables.customers; const id = table.columns.find((c) => c.name === 'id')!; - // This is the whole defect, stated as an assertion on the producer's own - // output. If it ever flips — the driver starting to emit the spec - // spelling, or the two contracts being reconciled upstream — the union - // read in `primaryKeyReader` stops being load-bearing and should be - // re-derived rather than quietly relaxed. + // The producer's own output, stated as an assertion. This case was written + // asserting the OPPOSITE — `isPrimary: true`, `primaryKey: undefined` — + // so that it would redden the moment the driver was aligned. That flip has + // now happened (#10676/#10998), and it is pinned here in its new + // direction: the driver emits the spec spelling and no longer emits the + // retired one. The union read in `primaryKeyReader` below therefore no + // longer has an in-tree producer needing its `isPrimary` arm; collapsing + // it is this lane's call to make, deliberately not made from the driver + // change, and until then this file keeps both arms pinned. expect(table.primaryKeys).toEqual(['id']); - expect(id.isPrimary).toBe(true); - expect(id.primaryKey).toBeUndefined(); + expect(id.primaryKey).toBe(true); + expect(id.isPrimary).toBeUndefined(); }); it('refreshCatalog carries the introspected key onto the right column only', async () => { diff --git a/packages/services/service-datasource/src/__tests__/external-object-draft-real-introspection.test.ts b/packages/services/service-datasource/src/__tests__/external-object-draft-real-introspection.test.ts new file mode 100644 index 0000000000..7112f041b6 --- /dev/null +++ b/packages/services/service-datasource/src/__tests__/external-object-draft-real-introspection.test.ts @@ -0,0 +1,151 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The pin the maintainer ruling names: `generateObjectDraft` driven off a REAL + * `SqlDriver.introspectSchema()` result, not a hand-written fake. + * + * This is the case the original suite could not be. Every other draft fixture + * in this package writes its remote schema by hand in the `packages/spec` + * spelling (`primaryKey: true`), so the seam where the driver's output meets + * this service was never exercised: the driver spelled key membership + * `isPrimary`, this service reads `col.primaryKey`, and every federated object + * drafted from a real remote table silently lost its primary key. A fixture in + * EITHER spelling is blind to that — only a live introspection can see it. + * + * Where the key surfaces: `fields..primaryKey` is not an authorable field + * key, so the ruling (2026-08-22, item 8, option D) put the introspected key + * in a COMMENT in the generated source and nowhere else. "The draft carries + * the remote key" is therefore asserted on `draft.source`, and the definition + * is asserted to stay free of the unauthorable key — both halves, because an + * implementation that re-emitted the key onto the field would satisfy the + * first alone while re-opening a defect the platform's own validator refuses. + * + * `@objectstack/driver-sql` is imported as a PACKAGE (its `exports` resolve to + * `dist/`), so this file measures the driver's BUILT output. The service is + * imported relatively, so it measures `src/`. That asymmetry is deliberate and + * is the seam itself: rebuild the driver before reading a verdict here. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { SqlDriver } from '@objectstack/driver-sql'; +import type { IntrospectedSchema } from '@objectstack/spec/contracts'; +import { + ExternalDatasourceService, + type DatasourceLike, +} from '../external-datasource-service.js'; + +const opened: SqlDriver[] = []; + +afterEach(async () => { + while (opened.length) { + const d = opened.pop()!; + try { + await (d as unknown as { knex?: { destroy(): Promise } }).knex?.destroy(); + } catch { + /* the pool may never have opened */ + } + } +}); + +/** A live in-memory SQLite database, introspected by the real driver. */ +async function introspectReal(ddl: (knex: never) => Promise): Promise { + const driver = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + } as never); + opened.push(driver); + await ddl((driver as unknown as { knex: never }).knex); + return driver.introspectSchema(); +} + +/** Wire the service to that result, exactly as `plugin.ts` does — unmodified. */ +function serviceOver(schema: unknown): ExternalDatasourceService { + return new ExternalDatasourceService({ + introspect: async () => schema as IntrospectedSchema, + getDatasource: async (name): Promise => ({ name, schemaMode: 'external' }), + getObject: async () => undefined, + listObjects: async () => [], + }); +} + +type CreateTable = { + schema: { createTable(n: string, cb: (t: never) => void): Promise }; +}; +type TableBuilder = { + string(n: string): { primary(): void; notNullable(): { primary(): void } }; + integer(n: string): { notNullable(): unknown }; + primary(cols: string[]): void; +}; + +/** The `// Remote primary key: …` line of a generated source, or `undefined`. */ +function remoteKeyComment(source: string): string | undefined { + return source.split('\n').find((l) => l.includes('Remote primary key:'))?.trim(); +} + +describe('generateObjectDraft, driven off a real SqlDriver.introspectSchema()', () => { + it('carries the introspected primary key into the draft', async () => { + const schema = await introspectReal(async (knex: never) => { + await (knex as unknown as CreateTable).schema.createTable('customers', (t: never) => { + const b = t as unknown as TableBuilder; + b.string('id').primary(); + b.string('name'); + b.integer('age'); + }); + }); + + const draft = await serviceOver(schema).generateObjectDraft('showcase_external', 'customers'); + + // The whole card, as one assertion on a real producer's output: before the + // driver spoke the spec spelling this was `undefined` — the draft named no + // remote key at all, so a user committing it federated a table with no + // addressing key. + expect(remoteKeyComment(draft.source)).toBe('// Remote primary key: id'); + + // …and it is the introspected key, not "the first column": a table whose + // key is not first is covered below. + expect(draft.definition.fields).toEqual({ + id: { type: 'text' }, + name: { type: 'text' }, + age: { type: 'number' }, + }); + + // The key survives as a comment and NOWHERE else. `fields..primaryKey` + // is unauthorable (TS2353 against `ServiceObject`, `unrecognized_keys` on + // `ObjectSchema.safeParse`); this change must not reintroduce it. + expect(JSON.stringify(draft.definition)).not.toContain('primaryKey'); + }); + + it('names every member of a composite key, in declared key order', async () => { + const schema = await introspectReal(async (knex: never) => { + await (knex as unknown as CreateTable).schema.createTable('order_lines', (t: never) => { + const b = t as unknown as TableBuilder; + b.string('sku'); + b.string('order_id').notNullable(); + b.integer('line_no').notNullable(); + b.primary(['order_id', 'line_no']); + }); + }); + + const draft = await serviceOver(schema).generateObjectDraft('showcase_external', 'order_lines'); + + // Column order is (sku, order_id, line_no); key order is (order_id, + // line_no). Both members, and neither `sku` nor a first-column guess. + expect(remoteKeyComment(draft.source)).toBe('// Remote primary key: order_id, line_no'); + }); + + it('invents no key for a remote table that declares none', async () => { + const schema = await introspectReal(async (knex: never) => { + await (knex as unknown as CreateTable).schema.createTable('events', (t: never) => { + const b = t as unknown as TableBuilder; + b.string('label'); + b.string('payload'); + }); + }); + + const draft = await serviceOver(schema).generateObjectDraft('showcase_external', 'events'); + + expect(remoteKeyComment(draft.source)).toBeUndefined(); + expect(Object.keys(draft.definition.fields as object)).toEqual(['label', 'payload']); + }); +}); From a7f8f2c694473954a3edb7d17dfa5727d3f33def Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 18:25:34 +0000 Subject: [PATCH 2/3] fix(driver-sql): emit the spec `dialect` and required `introspectedAt` (#10998) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `introspectSchema()` returned `{ tables }` and nothing else, while `packages/spec/src/contracts/schema-diff-service.ts` declares `dialect` and a REQUIRED `introspectedAt`. Measured live on in-memory SQLite before this change: `Object.keys()` was `["tables"]`. Two silent consequences — type mapping ran with `dialect: undefined` on the whole federation path, so every per-dialect alias in `suggestFieldTypeForSqlType` was unreachable there, and `refreshCatalog` persisted `dialect: undefined` into the `external_catalog` record Studio's schema browser and the boot gate read back. Same ruling as #10676 (2026-08-22, 「同意所有」 item 9): the driver aligns to the one contract. - `IntrospectedTable` / `IntrospectedSchema` now derive from the spec import in both `driver-sql` and `objectql/src/util.ts`, so `introspectedAt` being required is enforced by `tsc` rather than by remembering. `indexes` is `Omit`ted rather than emitted empty: this driver does not introspect indexes, and `[]` would claim a table has none — filed separately, not guessed. - `dialect` is `this.dialectName` (`sqlite` / `postgres` / `mysql` / `unknown`), NOT the raw Knex client and NOT the spec's `SQLDialectSchema` enum: the only in-tree consumer keys `DIALECT_ALIASES` on the `SqlDialect` vocabulary of `type-compat.ts`, which spells PostgreSQL `postgres`. Emitting `postgresql` would satisfy `Object.keys()` while leaving the aliases just as unreachable. - `introspectedAt` is stamped before the reads begin, so it never claims to cover a moment later than the first table actually read. Measured after: `Object.keys()` is `["tables","dialect","introspectedAt"]`; the persisted catalog records `dialect: 'sqlite'`. On the SQLite arm the suggested field types are byte-identical before and after (its alias map overlaps the base map); the per-dialect payoff is on Postgres/MySQL, which are unreachable from this container and are therefore not claimed. Fixes #10998 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y --- ...driver-introspection-spec-contract.test.ts | 29 ++++++++++ packages/drivers/driver-sql/src/sql-driver.ts | 54 +++++++++++++++---- packages/objectql/src/util.test.ts | 12 ++++- packages/objectql/src/util.ts | 23 ++++++-- ...al-object-draft-real-introspection.test.ts | 22 ++++++++ 5 files changed, 123 insertions(+), 17 deletions(-) diff --git a/packages/drivers/driver-sql/src/sql-driver-introspection-spec-contract.test.ts b/packages/drivers/driver-sql/src/sql-driver-introspection-spec-contract.test.ts index 06d096aa4d..7529052b04 100644 --- a/packages/drivers/driver-sql/src/sql-driver-introspection-spec-contract.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-introspection-spec-contract.test.ts @@ -60,6 +60,35 @@ describe('SqlDriver.introspectSchema emits the spec introspection contract', () expect(byName.age.primaryKey).toBe(false); }); + it('emits `dialect` and the required `introspectedAt`', async () => { + const before = Date.now(); + const schema = await driver.introspectSchema(); + + // #10998's acceptance criterion, spelled exactly as it was written: the + // producer returned `{ tables }` alone while the contract declares three + // keys, so consumers read two nobody set — type mapping ran with no + // dialect on the whole federation path, and `refreshCatalog` persisted + // `dialect: undefined` into the record Studio and the boot gate read back. + expect(Object.keys(schema)).toEqual( + expect.arrayContaining(['tables', 'dialect', 'introspectedAt']), + ); + + // The dialect TOKEN, not merely the key's presence. The consumer is + // `suggestFieldTypeForSqlType(col.type, schema.dialect as SqlDialect)`, + // whose vocabulary spells these `sqlite` / `postgres` / `mysql`; a token + // outside it (`better-sqlite3`, or the spec enum's `postgresql`) would + // leave every per-dialect alias unreachable with the key still present. + expect(schema.dialect).toBe('sqlite'); + + // Required in the contract, so it is emitted unconditionally — and it is a + // real ISO 8601 instant, not a placeholder a consumer would have to guard. + expect(typeof schema.introspectedAt).toBe('string'); + expect(new Date(schema.introspectedAt).toISOString()).toBe(schema.introspectedAt); + const at = Date.parse(schema.introspectedAt); + expect(at).toBeGreaterThanOrEqual(before - 1000); + expect(at).toBeLessThanOrEqual(Date.now() + 1000); + }); + it('no longer emits the retired `isPrimary` spelling', async () => { const schema = await driver.introspectSchema(); const id = schema.tables['customers'].columns.find((c) => c.name === 'id')!; diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index 3d3af01d3f..d621c8f2b9 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -11,7 +11,11 @@ import type { DriverOptions, FilterCondition, SchemaMode } from '@objectstack/sp // The ONE introspection contract (ADR-0015 / `ISchemaDiffService`). This // driver's introspection types are DERIVED from these rather than // re-declared next to them — see the `Introspection Types` region below. -import type { IntrospectedColumn as SpecIntrospectedColumn } from '@objectstack/spec/contracts'; +import type { + IntrospectedColumn as SpecIntrospectedColumn, + IntrospectedSchema as SpecIntrospectedSchema, + IntrospectedTable as SpecIntrospectedTable, +} from '@objectstack/spec/contracts'; import { parseAutonumberFormat, renderAutonumber, resolveAutonumberFormat, readAutonumberCounter, missingFieldValues, isTenancyDisabled, type AutonumberToken } from '@objectstack/spec/data'; // The DECLARED aggregate vocabulary (#5907). Read from the spec so this driver's // "the protocol has no such function" refusal cannot drift from what @@ -3650,13 +3654,14 @@ function nullSafeNegationOperand(node: Record): Record): Record { columns: IntrospectedColumn[]; + /** SQL-introspection extra: outbound foreign keys. */ foreignKeys: IntrospectedForeignKey[]; + /** SQL-introspection extra: the table's primary-key columns, in key order. */ primaryKeys: string[]; } -export interface IntrospectedSchema { +/** + * `dialect` and `introspectedAt` are inherited from the spec contract, where + * `introspectedAt` is REQUIRED — so `tsc` now refuses an `introspectSchema` + * that omits them, which is the check that was missing while this type + * declared `{ tables }` alone. + */ +export interface IntrospectedSchema extends Omit { tables: Record; } @@ -9831,6 +9851,10 @@ export class SqlDriver implements IDataDriver { async introspectSchema(): Promise { const tables: Record = {}; + // Stamped BEFORE the reads, not after: a consumer asking "has the remote + // changed since this snapshot?" must not be told the snapshot covers a + // moment later than the first table it actually read. + const introspectedAt = new Date().toISOString(); let tableNames: string[] = []; if (this.isPostgres) { @@ -9877,7 +9901,15 @@ export class SqlDriver implements IDataDriver { tables[tableName] = { name: tableName, columns, foreignKeys, primaryKeys }; } - return { tables }; + // `dialectName` — not the raw Knex client, and not the spec's + // `SQLDialectSchema` enum. The only in-tree consumer of this key is + // `suggestFieldTypeForSqlType(col.type, schema.dialect as SqlDialect)`, + // whose `SqlDialect` vocabulary (`packages/spec/src/data/type-compat.ts`) + // spells PostgreSQL `postgres`, exactly as `dialectName` does. Emitting + // the enum's `postgresql` instead would put the key in `Object.keys()` + // while leaving every per-dialect type alias unreachable — the omission + // this repairs, wearing a fix's clothes. + return { tables, dialect: this.dialectName, introspectedAt }; } // =================================== diff --git a/packages/objectql/src/util.test.ts b/packages/objectql/src/util.test.ts index 73048c0eb6..d37b7bb997 100644 --- a/packages/objectql/src/util.test.ts +++ b/packages/objectql/src/util.test.ts @@ -27,6 +27,8 @@ describe('toTitleCase', () => { describe('convertIntrospectedSchemaToObjects', () => { const sampleSchema: IntrospectedSchema = { + dialect: 'sqlite', + introspectedAt: '2026-08-22T00:00:00.000Z', tables: { users: { name: 'users', @@ -179,12 +181,18 @@ describe('convertIntrospectedSchemaToObjects', () => { }); it('should handle empty schema', () => { - const objects = convertIntrospectedSchemaToObjects({ tables: {} }); + const objects = convertIntrospectedSchemaToObjects({ + dialect: 'sqlite', + introspectedAt: '2026-08-22T00:00:00.000Z', + tables: {}, + }); expect(objects).toHaveLength(0); }); it('should handle numeric types (float, decimal, real)', () => { const schema: IntrospectedSchema = { + dialect: 'sqlite', + introspectedAt: '2026-08-22T00:00:00.000Z', tables: { metrics: { name: 'metrics', @@ -209,6 +217,8 @@ describe('convertIntrospectedSchemaToObjects', () => { it('should handle time type', () => { const schema: IntrospectedSchema = { + dialect: 'sqlite', + introspectedAt: '2026-08-22T00:00:00.000Z', tables: { schedule: { name: 'schedule', diff --git a/packages/objectql/src/util.ts b/packages/objectql/src/util.ts index 807930fe93..264da21e4e 100644 --- a/packages/objectql/src/util.ts +++ b/packages/objectql/src/util.ts @@ -1,7 +1,11 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import type { ServiceObject } from '@objectstack/spec/data'; -import type { IntrospectedColumn as SpecIntrospectedColumn } from '@objectstack/spec/contracts'; +import type { + IntrospectedColumn as SpecIntrospectedColumn, + IntrospectedSchema as SpecIntrospectedSchema, + IntrospectedTable as SpecIntrospectedTable, +} from '@objectstack/spec/contracts'; // ── Introspection Types ────────────────────────────────────────────────────── @@ -52,10 +56,14 @@ export interface IntrospectedForeignKey { /** * Table metadata from database introspection. + * + * DERIVED from the spec contract, like {@link IntrospectedColumn}. `indexes` + * is `Omit`ted rather than required: SQL introspection here does not read + * indexes, and an empty array would claim a table HAS none when it merely was + * not asked. `foreignKeys` / `primaryKeys` are extras the spec's diff-facing + * contract does not declare. */ -export interface IntrospectedTable { - /** Table name */ - name: string; +export interface IntrospectedTable extends Omit { /** List of columns */ columns: IntrospectedColumn[]; /** List of foreign key relationships */ @@ -66,8 +74,13 @@ export interface IntrospectedTable { /** * Complete database schema introspection result. + * + * DERIVED from the spec contract, so `dialect` and the REQUIRED + * `introspectedAt` come from the one declaration rather than being omitted + * here and read downstream — which is how a producer shipped `{ tables }` + * alone while consumers read two keys nobody set. */ -export interface IntrospectedSchema { +export interface IntrospectedSchema extends Omit { /** Map of table name to table metadata */ tables: Record; } diff --git a/packages/services/service-datasource/src/__tests__/external-object-draft-real-introspection.test.ts b/packages/services/service-datasource/src/__tests__/external-object-draft-real-introspection.test.ts index 7112f041b6..b3c78c1f9c 100644 --- a/packages/services/service-datasource/src/__tests__/external-object-draft-real-introspection.test.ts +++ b/packages/services/service-datasource/src/__tests__/external-object-draft-real-introspection.test.ts @@ -134,6 +134,28 @@ describe('generateObjectDraft, driven off a real SqlDriver.introspectSchema()', expect(remoteKeyComment(draft.source)).toBe('// Remote primary key: order_id, line_no'); }); + it('carries the driver-reported dialect into the persisted catalog', async () => { + const schema = await introspectReal(async (knex: never) => { + await (knex as unknown as CreateTable).schema.createTable('customers', (t: never) => { + const b = t as unknown as TableBuilder; + b.string('id').primary(); + }); + }); + + // The other half of the same omission (#10998): `refreshCatalog` writes + // `dialect: schema.dialect` into the `external_catalog` record Studio's + // schema browser and the boot gate read back, and the producer never set + // it — so every persisted catalog recorded `undefined`. Asserted through + // the service rather than on the driver's return value, because the + // persisted record is what a consumer actually sees. + const catalog = await serviceOver(schema).refreshCatalog('showcase_external'); + expect(catalog.dialect).toBe('sqlite'); + + // …and it is a token `suggestFieldTypeForSqlType` understands, so the + // per-dialect aliases the federation path needs are reachable at all. + expect(['postgres', 'mysql', 'sqlite']).toContain(catalog.dialect); + }); + it('invents no key for a remote table that declares none', async () => { const schema = await introspectReal(async (knex: never) => { await (knex as unknown as CreateTable).schema.createTable('events', (t: never) => { From 86ccc458cc50b6b5705cfe8a68b884eed803a6d2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 18:27:51 +0000 Subject: [PATCH 3/3] chore(changeset): driver-sql emits the spec introspection contract (#10676, #10998) Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y --- .../driver-emits-spec-introspection-shape.md | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 .changeset/driver-emits-spec-introspection-shape.md diff --git a/.changeset/driver-emits-spec-introspection-shape.md b/.changeset/driver-emits-spec-introspection-shape.md new file mode 100644 index 0000000000..814a566683 --- /dev/null +++ b/.changeset/driver-emits-spec-introspection-shape.md @@ -0,0 +1,60 @@ +--- +"@objectstack/driver-sql": minor +"@objectstack/objectql": minor +--- + +fix(driver-sql): `introspectSchema()` emits the spec introspection contract — `primaryKey`, `dialect`, `introspectedAt` (#10676, #10998) + +**BREAKING** change to the value `SqlDriver.introspectSchema()` returns, shipped +as `minor` under the repo's launch-window convention for breaking changes. + +`packages/spec/src/contracts/schema-diff-service.ts` declares one introspection +contract. The driver declared a second one beside it and, separately, so did +`packages/objectql/src/util.ts`. The three agreed on the idea and disagreed on +the vocabulary: the driver spelled a column's primary-key membership +`isPrimary`, the spec spells it `primaryKey`; the spec declares `dialect` and a +REQUIRED `introspectedAt` that the driver's schema type never mentioned and +`introspectSchema()` therefore never emitted. Nothing was type-unsound — each +side compiled against its own declaration and the value crossed between them +with no compiler in the middle. + +Measured on a live in-memory SQLite database before this change: the id column +of a `primary key (id)` table came back carrying `isPrimary: true` with no +`primaryKey` key at all, and `Object.keys()` of the schema was `["tables"]`. +Two consequences, both silent: + +- `ExternalDatasourceService.generateObjectDraft` reads `col.primaryKey`, so + every federated object drafted from a real remote table lost the remote + primary key — the addressing key for the federated table, dropped by the + codegen meant to produce it (#10676). +- type mapping ran with `dialect: undefined` across the whole federation path, + making every per-dialect alias in `suggestFieldTypeForSqlType` unreachable + there, and `refreshCatalog` persisted `dialect: undefined` into the + `external_catalog` record Studio's schema browser and the boot gate read + back (#10998). + +Maintainer ruling, 2026-08-22 (live session, 「同意所有」 item 9 = +驱动侧对齐 spec 契约): `packages/spec` is the one contract and the driver +aligns to it. + +What the driver now returns: every column carries the boolean `primaryKey`, the +schema carries `dialect` and `introspectedAt`, and the retired `isPrimary` +member is gone rather than emitted alongside — one spelling, so no consumer can +key off the wrong one again. `dialect` is the driver's canonical dialect name +(`sqlite`, `postgres`, `mysql`, `unknown`), which is the vocabulary the only +in-tree consumer keys its alias tables on; `introspectedAt` is an ISO 8601 +instant stamped before the reads begin. + +`IntrospectedColumn`, `IntrospectedTable` and `IntrospectedSchema` in both +`@objectstack/driver-sql` and `@objectstack/objectql` are now derived from the +spec contract instead of re-declared, so a key added there fails their `tsc` +until the producer emits it. Two divergences are kept explicitly: `defaultValue` +stays `unknown` at the SQL layer because Knex reports `null`, and `indexes` is +omitted rather than emitted empty because this driver does not introspect +indexes and an empty array would tell a schema differ that a table has none. + +TypeScript consumers of the removed member are told by the compiler, precisely +and at every site: `Property 'isPrimary' does not exist on type +'IntrospectedColumn'`. + +