fix(service-datasource): read the introspected primary key at the isPrimary/primaryKey seam

.changeset/external-catalog-introspected-primary-key.md

@@ -0,0 +1,39 @@
+---
+"@objectstack/service-datasource": patch
+---
+
+Restore the introspected primary key in the persisted `external_catalog`
+(#10676). `ExternalDatasourceService` reads `column.primaryKey` — the
+`packages/spec` `IntrospectedColumn` spelling — but `plugin.ts` hands it the
+driver's `introspectSchema()` result unmodified, and `SqlDriver` (and
+`SqliteWasmDriver`, which extends it) speaks the other `IntrospectedColumn`
+contract, from `packages/objectql/src/util.ts`: it sets `column.isPrimary` and
+fills `table.primaryKeys`, never `column.primaryKey`.
+
+Measured against a live SQLite database: for a table declared
+`primary key (id)`, the driver's `id` column carries `isPrimary: true` and the
+table carries `primaryKeys: ['id']`, while `primaryKey` is `undefined`. Because
+`ExternalCatalogSchema` defaults `primaryKey` to `false`, `refreshCatalog`
+persisted a catalog in which **every** column of **every** remote table claimed
+not to be part of the remote key — so Studio's schema browser and the boot gate
+read a catalog that shows no primary keys at all.
+
+The seam now reads the union of all three signals (`primaryKey`, `isPrimary`,
+`table.primaryKeys`) rather than any one of them. No in-tree producer uses a
+`false` to negate a key another signal asserts, and taking the union means a
+producer that fills only the table-level list — or only the per-column flag —
+cannot lose half a composite key. No response or record shape changes: a field
+that should always have carried the introspected value starts carrying it.
+
+The regression pin drives the service off a **real** `SqlDriver.introspectSchema()`
+result rather than a hand-written fixture. The pre-existing suite could not see
+this defect precisely because it hand-wrote its fixture in the spec spelling, so
+no test ever fed the service what a driver actually emits.
+
+Not fixed here: `generateObjectDraft` still drops the key from the generated
+object definition. Its destination is an open contract question rather than a
+missing read — `fields.<name>.primaryKey` is **not** an authorable spec field
+key (an object literal carrying it fails `tsc` against `ServiceObject` with
+TS2353, and `ObjectSchema.safeParse` with `unrecognized_keys`), and there is no
+key on `ObjectExternalBindingSchema` to hold a remote primary key either. See
+#10676 for the routing decision.

packages/services/service-datasource/src/__tests__/external-introspection-seam.test.ts

@@ -0,0 +1,206 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * The pin the original suite could not be: this service driven off a REAL
+ * `SqlDriver.introspectSchema()` result, not a hand-written fake.
+ *
+ * `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
+ * introspects a real in-memory SQLite database rather than describing one.
+ *
+ * Both directions are pinned deliberately: an implementation that stamped the
+ * key onto the first column would satisfy a positive-only suite.
+ */
+
+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<void> } }).knex?.destroy();
+ } catch {
+ /* the pool may never have opened */
+ }
+ }
+});
+
+/**
+ * A live in-memory SQLite database, introspected by the real driver. Returns
+ * the driver's own result, deliberately NOT reshaped — anything this service
+ * needs, it must read from the bytes the driver actually produces.
+ */
+async function introspectReal(ddl: (knex: never) => Promise<void>): Promise<unknown> {
+ 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 a fixed introspection result, exactly as `plugin.ts` does. */
+function serviceOver(schema: unknown): ExternalDatasourceService {
+ return new ExternalDatasourceService({
+ introspect: async () => schema as IntrospectedSchema,
+ getDatasource: async (name): Promise<DatasourceLike> => ({ name, schemaMode: 'external' }),
+ getObject: async () => undefined,
+ listObjects: async () => [],
+ });
+}
+
+/** The catalog columns for one remote table, keyed by column name. */
+async function catalogColumns(
+ schema: unknown,
+ remoteName: string,
+): Promise<Record<string, { primaryKey: boolean; nullable: boolean; sqlType: string }>> {
+ const catalog = await serviceOver(schema).refreshCatalog('showcase_external');
+ const table = catalog.tables.find((t) => t.remoteName === remoteName);
+ expect(table, `no '${remoteName}' in the refreshed catalog`).toBeDefined();
+ return Object.fromEntries(table!.columns.map((c) => [c.name, c])) as never;
+}
+
+describe('the introspection seam, as a real SqlDriver actually spells it', () => {
+ it('the driver speaks isPrimary/primaryKeys and never the spec spelling', async () => {
+ const schema = (await introspectReal(async (knex: never) => {
+ await (knex as { schema: { createTable(n: string, cb: (t: never) => void): Promise<void> } }).schema.createTable(
+ 'customers',
+ (t: never) => {
+ const b = t as unknown as { string(n: string): { primary(): void }; integer(n: string): void };
+ b.string('id').primary();
+ b.string('name');
+ b.integer('age');
+ },
+ );
+ })) as { tables: Record<string, { primaryKeys: string[]; columns: Record<string, unknown>[] }> };
+
+ 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.
+ expect(table.primaryKeys).toEqual(['id']);
+ expect(id.isPrimary).toBe(true);
+ expect(id.primaryKey).toBeUndefined();
+ });
+
+ it('refreshCatalog carries the introspected key onto the right column only', async () => {
+ const schema = await introspectReal(async (knex: never) => {
+ await (knex as { schema: { createTable(n: string, cb: (t: never) => void): Promise<void> } }).schema.createTable(
+ 'customers',
+ (t: never) => {
+ const b = t as unknown as { string(n: string): { primary(): void }; integer(n: string): void };
+ b.string('id').primary();
+ b.string('name');
+ b.integer('age');
+ },
+ );
+ });
+
+ const cols = await catalogColumns(schema, 'customers');
+ expect(cols.id.primaryKey).toBe(true);
+ // …and nothing else was promoted.
+ expect(cols.name.primaryKey).toBe(false);
+ expect(cols.age.primaryKey).toBe(false);
+ });
+
+ it('refreshCatalog invents no key for a table that declares none', async () => {
+ const schema = await introspectReal(async (knex: never) => {
+ await (knex as { schema: { createTable(n: string, cb: (t: never) => void): Promise<void> } }).schema.createTable(
+ 'events',
+ (t: never) => {
+ const b = t as unknown as { string(n: string): unknown };
+ b.string('label');
+ b.string('payload');
+ },
+ );
+ });
+
+ const cols = await catalogColumns(schema, 'events');
+ // Still a usable catalog entry…
+ expect(Object.keys(cols)).toEqual(['label', 'payload']);
+ // …and no column was promoted, least of all the first one.
+ expect(cols.label.primaryKey).toBe(false);
+ expect(cols.payload.primaryKey).toBe(false);
+ });
+});
+
+describe('the seam read, where the two spellings disagree', () => {
+ /**
+ * No in-tree driver produces a disagreement — `SqlDriver` derives
+ * `isPrimary` FROM `primaryKeys`, so the two always agree. This case is
+ * therefore hand-built ON PURPOSE, and it is the one place in this file
+ * where that is the right instrument: it fixes the behaviour under a
+ * disagreement no live database can currently stage, so a future producer
+ * that fills only one of the two signals cannot silently lose half a
+ * composite key. The spelling seam itself is pinned above, against a real
+ * driver, where a fake would have been blind.
+ */
+ function serviceOverRaw(table: unknown): ExternalDatasourceService {
+ return serviceOver({ tables: { order_lines: table } });
+ }
+
+ const cols = [
+ { name: 'order_id', type: 'varchar', nullable: false },
+ { name: 'line_no', type: 'varchar', nullable: false },
+ { name: 'sku', type: 'varchar', nullable: true },
+ ];
+
+ it('takes the union when the table-level list is wider than the per-column flag', async () => {
+ const catalog = await serviceOverRaw({
+ name: 'order_lines',
+ primaryKeys: ['order_id', 'line_no'],
+ columns: cols.map((c) => ({ ...c, isPrimary: c.name === 'order_id' })),
+ }).refreshCatalog('showcase_external');
+
+ const byName = Object.fromEntries(catalog.tables[0].columns.map((c) => [c.name, c]));
+ expect(byName.order_id.primaryKey).toBe(true);
+ expect(byName.line_no.primaryKey).toBe(true);
+ expect(byName.sku.primaryKey).toBe(false);
+ });
+
+ it('takes the union when the per-column flag is wider than the table-level list', async () => {
+ const catalog = await serviceOverRaw({
+ name: 'order_lines',
+ primaryKeys: ['order_id'],
+ columns: cols.map((c) => ({ ...c, isPrimary: c.name !== 'sku' })),
+ }).refreshCatalog('showcase_external');
+
+ const byName = Object.fromEntries(catalog.tables[0].columns.map((c) => [c.name, c]));
+ expect(byName.order_id.primaryKey).toBe(true);
+ expect(byName.line_no.primaryKey).toBe(true);
+ expect(byName.sku.primaryKey).toBe(false);
+ });
+
+ it('still honours the spec spelling on its own — the pre-existing fixtures keep working', async () => {
+ const catalog = await serviceOverRaw({
+ name: 'order_lines',
+ // No `primaryKeys`, no `isPrimary` — the hand-written shape the rest of
+ // this package's suite feeds in.
+ columns: cols.map((c) => ({ ...c, primaryKey: c.name === 'order_id' })),
+ }).refreshCatalog('showcase_external');
+
+ const byName = Object.fromEntries(catalog.tables[0].columns.map((c) => [c.name, c]));
+ expect(byName.order_id.primaryKey).toBe(true);
+ expect(byName.line_no.primaryKey).toBe(false);
+ });
+});

packages/services/service-datasource/src/external-datasource-service.ts

@@ -22,6 +22,7 @@ import type {
  SchemaValidationReport,
  IntrospectedSchema,
  IntrospectedTable,
+ IntrospectedColumn,
 } from '@objectstack/spec/contracts';
 import type { SchemaDiffEntry } from '@objectstack/spec/shared';
 import {
@@ -93,6 +94,63 @@ export interface ExternalDatasourceServiceConfig {
 /** Columns ObjectStack manages itself — never validated against the remote. */
 const BUILTIN_COLUMNS = new Set(['id', 'created_at', 'updated_at']);
 
+/**
+ * Read "is this column part of the remote primary key" across the TWO
+ * introspection contracts that meet at this service.
+ *
+ * `plugin.ts` hands the driver's `introspectSchema()` result to this service
+ * unmodified, and the driver does not speak the contract this file is typed
+ * against:
+ *
+ * | producer | per-column | table-level |
+ * | ---------------------------------------------- | -------------- | -------------- |
+ * | `SqlDriver` (+ `SqliteWasmDriver`, which extends it) | `isPrimary` | `primaryKeys` |
+ * | `packages/spec` `IntrospectedColumn` (what this file's types say) | `primaryKey` | — |
+ *
+ * Measured against a live SQLite database at `368e7a06f`: the driver's column
+ * for a `primary key (id)` table carries `isPrimary: true` and the table
+ * carries `primaryKeys: ['id']`, while `primaryKey` is `undefined`. Reading
+ * only `col.primaryKey` therefore reads a key no in-tree driver ever sets, and
+ * the remote key is silently lost.
+ *
+ * This reads the UNION of all three signals rather than picking one:
+ *
+ * - No in-tree producer uses `primaryKey: false` / `isPrimary: false` to
+ * NEGATE a key another signal asserts — the falses are just "not a key",
+ * written by producers that fill exactly one of the three. A precedence
+ * chain would therefore drop a real key whenever the winning signal is the
+ * one its producer left blank, which is the defect being repaired here.
+ * - A producer that fills only `table.primaryKeys` (the shape a table-level
+ * reader would naturally emit) is covered without needing a per-column flag,
+ * and vice versa.
+ *
+ * When the per-column flag and the table-level list DISAGREE, the union takes
+ * both. That is deliberate: for a federated table, under-reporting the key
+ * costs the caller its addressing key, and no in-tree consumer treats a
+ * column's PK-ness as an exclusive claim. Note that no in-tree driver produces
+ * such a disagreement today — `SqlDriver` derives `isPrimary` FROM
+ * `primaryKeys`, so the two always agree, including where both are wrong (a
+ * SQLite composite key reports only its first column, because
+ * `introspectPrimaryKeys` filters `PRAGMA table_info` on `pk === 1` while
+ * SQLite numbers composite members `1, 2, ...`). That truncation is upstream
+ * of this seam and is not repaired here.
+ *
+ * Deliberately structural: the extra spellings are read off the value without
+ * widening any declared contract, because reconciling
+ * `packages/objectql/src/util.ts` with
+ * `packages/spec/src/contracts/schema-diff-service.ts` is a spec-owned change.
+ */
+function primaryKeyReader(table: IntrospectedTable): (col: IntrospectedColumn) => boolean {
+ const declared = (table as unknown as { primaryKeys?: unknown }).primaryKeys;
+ const listed = new Set(
+ Array.isArray(declared) ? declared.filter((n): n is string => typeof n === 'string') : [],
+ );
+ return (col) =>
+ col.primaryKey === true ||
+ (col as { isPrimary?: unknown }).isPrimary === true ||
+ listed.has(col.name);
+}
+
 /** Split a possibly schema-qualified name (`mart.fact_orders`). */
 function parseQualified(raw: string): { schema?: string; name: string } {
  const idx = raw.indexOf('.');
@@ -302,14 +360,20 @@ export class ExternalDatasourceService implements IExternalDatasourceService {
  dialect: schema.dialect,
  tables: Object.values(schema.tables).map((t) => {
  const { schema: s, name } = parseQualified(t.name);
+ // The introspection seam: a real driver spells this `isPrimary` /
+ // `primaryKeys`, never `primaryKey`. `ExternalCatalogSchema` defaults
+ // the key to `false`, so reading only `c.primaryKey` persisted a
+ // catalog in which EVERY column claimed not to be part of the remote
+ // key — including the ones that are.
+ const isPk = primaryKeyReader(t);
  return {
  remoteSchema: s,
  remoteName: name,
  columns: t.columns.map((c) => ({
  name: c.name,
  sqlType: c.type,
  nullable: c.nullable,
- primaryKey: c.primaryKey,
+ primaryKey: isPk(c),
  suggestedFieldType: suggestFieldTypeForSqlType(c.type, schema.dialect as SqlDialect),
  })),
  };