diff --git a/.changeset/external-object-draft-drops-unauthorable-primary-key.md b/.changeset/external-object-draft-drops-unauthorable-primary-key.md new file mode 100644 index 0000000000..ef86a43df6 --- /dev/null +++ b/.changeset/external-object-draft-drops-unauthorable-primary-key.md @@ -0,0 +1,38 @@ +--- +"@objectstack/service-datasource": patch +--- + +`os datasource introspect --primary-key` (and `POST /object-draft` with +`primaryKey`) now generates an object draft that compiles and parses (#11000). + +The generator emitted a field-level `primaryKey: true` — into the definition +and onto the rendered field line. `primaryKey` is **not a key of the spec field +schema**, so the `*.object.ts` the review-before-commit flow handed the user was +refused by both instruments the file is annotated for: + +- `tsc --noEmit` against `ServiceObject` — `TS2353: Object literal may only + specify known properties, and 'primaryKey' does not exist in type …`; +- `ObjectSchema.safeParse` — `unrecognized_keys` at `["fields",""]`. + +This was the last reason the `opts.primaryKey` path did not build. With #10712's +namespace/`sharingModel` repairs already landed, **both** paths — `primaryKey` +set and unset — now clear `defineStack()`'s namespace check, the +`authoringRulesFor('build')` rule set, and `tsc --noEmit` over the rendered +source. + +The introspected key is not discarded: it is preserved as a comment above the +`fields` block, naming the column(s) the draft was given as the key — + +```ts + // Remote primary key: order_id, line_no +``` + +— with the reason it is a comment rather than a field key, and an explicit +caveat that for a composite key some drivers report only the first column +(#10997), so the list is a lower bound rather than a verified complete key. A +table with no reported key gets no comment at all. + +Per the maintainer ruling of 2026-08-22, an authorable spelling for a federated +object's remote key (`external.primaryKey: string[]` on the binding schema) is +**deferred, not rejected** — it returns as its own `packages/spec` change when +federated upsert has a live runtime consumer to justify the surface. diff --git a/.changeset/external-object-draft-passes-os-build.md b/.changeset/external-object-draft-passes-os-build.md index 84dbebd253..f7430a2165 100644 --- a/.changeset/external-object-draft-passes-os-build.md +++ b/.changeset/external-object-draft-passes-os-build.md @@ -27,6 +27,7 @@ prefix — mirroring `defineStack`, which skips the check entirely rather than inventing one, and avoiding an `_customers` that would trade one invalid draft for another. -Note the `opts.primaryKey` path still does not build: it emits -`fields..primaryKey`, which is not an authorable spec field key. That is -#11000, a separate open contract question, untouched here. +At the time this landed, the `opts.primaryKey` path still did not build: it +emitted `fields..primaryKey`, which is not an authorable spec field key. +That was #11000, and it is fixed separately in this same release — both paths +build now. See that changeset for what replaced the key. diff --git a/packages/services/service-datasource/src/__tests__/external-datasource-service.test.ts b/packages/services/service-datasource/src/__tests__/external-datasource-service.test.ts index 65f31c2108..6d45521db1 100644 --- a/packages/services/service-datasource/src/__tests__/external-datasource-service.test.ts +++ b/packages/services/service-datasource/src/__tests__/external-datasource-service.test.ts @@ -89,7 +89,23 @@ describe('generateObjectDraft', () => { expect(draft.name).toBe('fact_orders'); expect(draft.datasource).toBe('warehouse'); const fields = draft.definition.fields as Record; - expect(fields.order_id).toEqual({ type: 'text', primaryKey: true }); + /** + * `order_id` is the table's primary key, and the field carries NOTHING + * about that. + * + * This assertion used to read `{ type: 'text', primaryKey: true }`. It was + * flipped by the maintainer ruling of 2026-08-22 (「同意所有」, item 8 = D, + * recorded on #11000): `fields..primaryKey` is not a key of the spec + * field schema, so the draft it pinned was one `tsc --noEmit` refused + * (`TS2353`) and `ObjectSchema.safeParse` refused (`unrecognized_keys`). + * The pin is kept, not deleted — inverted, it is now the guard that the + * unauthorable key does not come back. `toEqual` (not `toMatchObject`) is + * load-bearing here: it is what makes the assertion fail on an EXTRA key. + * + * Where the key went instead is pinned in + * `external-object-draft-primary-key.test.ts`. + */ + expect(fields.order_id).toEqual({ type: 'text' }); expect(fields.amount.type).toBe('number'); expect(fields.ordered_at.type).toBe('datetime'); expect(fields.metadata.type).toBe('json'); @@ -101,7 +117,11 @@ describe('generateObjectDraft', () => { expect(draft.source).toContain("remoteName: 'fact_orders'"); expect(draft.source).toContain("remoteSchema: 'mart'"); expect(draft.source).toContain('REVIEW:'); - expect(draft.source).toContain("order_id: { type: 'text', primaryKey: true }"); + // Same flip, source side: the rendered field line no longer carries the + // key, and the introspected key survives as the comment ruling D requires. + expect(draft.source).toContain("order_id: { type: 'text' },"); + expect(draft.source).not.toContain('primaryKey: true'); + expect(draft.source).toContain('// Remote primary key: order_id'); }); it('honours include/exclude/rename/primaryKey options', async () => { @@ -113,6 +133,10 @@ describe('generateObjectDraft', () => { }); const fields = draft.definition.fields as Record; expect(Object.keys(fields)).toEqual(['order_id', 'total']); + // `opts.primaryKey` still HAS an effect after ruling D — it moved from the + // definition to the comment. An implementation that dropped the option + // entirely would pass every other assertion in this file. + expect(draft.source).toContain('// Remote primary key: order_id'); }); it('throws when the remote table is missing', async () => { diff --git a/packages/services/service-datasource/src/__tests__/external-object-draft-os-build.test.ts b/packages/services/service-datasource/src/__tests__/external-object-draft-os-build.test.ts index 0cf031820e..474335418c 100644 --- a/packages/services/service-datasource/src/__tests__/external-object-draft-os-build.test.ts +++ b/packages/services/service-datasource/src/__tests__/external-object-draft-os-build.test.ts @@ -58,12 +58,22 @@ import { * the object NAME comes from the table name and the OWD is a constant, so a * real introspection would add cost and no coverage. * - * Every column is spelled `primaryKey: false` on purpose. That keeps the whole - * file on the `opts.primaryKey`-unset path, where the generator emits no - * `fields..primaryKey` — the key that is NOT authorable (#11000, an open - * contract question in `packages/spec`, deliberately untouched here). Pinning - * these two repairs on a draft that also carries #11000's key would produce - * cases that cannot go green until a card this lane does not own is decided. + * Every column is spelled `primaryKey: false`, so the cases above run on the + * `opts.primaryKey`-UNSET path. That was originally a workaround: #11000's + * unauthorable `fields..primaryKey` made the key-set path un-buildable, and + * pinning these two repairs on top of it would have produced cases that could + * not go green until a card this lane did not own was decided. + * + * #11000 is now decided (maintainer, 2026-08-22, 「同意所有」 item 8 = D) and + * fixed: the generator no longer emits that key. The fixture keeps the unset + * spelling because these two defects genuinely do not read a column's PK-ness + * — but the path split is no longer a limitation, and the block at the bottom + * of this file re-runs the same three checks with `opts.primaryKey` SET. + * ⭐ Keep them separate anyway: before the namespace/OWD repairs landed, the + * key-set path failed on `unrecognized_keys` BEFORE the namespace check ran, + * so on that path the namespace defect was masked rather than absent. Error + * ordering hides defects in this pipeline; one path's verdict never covers the + * other's. */ function remoteSchema(): IntrospectedSchema { return { @@ -234,6 +244,48 @@ describe('an absent or blank namespace must not trade one invalid draft for anot }); }); +/** + * The `opts.primaryKey`-SET path, which #11000 removed the last blocker from. + * + * Until ruling D, this path produced a draft that failed + * `ObjectSchema.safeParse` on `unrecognized_keys` — and failed it EARLY ENOUGH + * that the namespace and OWD repairs pinned above were never reached on it. + * Re-running all three checks here is what makes "both paths build" a measured + * claim rather than an inference from the unset path. + */ +describe('both paths build — the key-set path is no longer the exception', () => { + /** The same service, driven with an explicit remote key. */ + const withKey = (ns: string | undefined = 'wh') => + serviceWith(ns).generateObjectDraft('warehouse', 'customers', { primaryKey: ['id'] }); + + it('stage 1 — the namespace prefix rule accepts the name on the key-set path too', async () => { + const draft = await withKey(); + expect(draft.name).toBe('wh_customers'); + expect(validateObjectNamespacePrefix(draft.name, 'wh')).toBeNull(); + }); + + it('stage 2 — the definition PARSES, and carries the declared OWD', async () => { + const draft = await withKey(); + const parsed = ObjectSchema.safeParse(draft.definition); + expect(parsed.success, JSON.stringify((parsed as { error?: unknown }).error)).toBe(true); + expect(draft.definition.sharingModel).toBe('private'); + expect(CANONICAL_OWD).toContain(draft.definition.sharingModel); + }); + + it('still-generates — every field, the binding and the remote name survive the key path', async () => { + const draft = await withKey(); + const fields = draft.definition.fields as Record; + const external = draft.definition.external as { remoteName?: string; remoteSchema?: string }; + + expect(Object.keys(fields)).toEqual(['id', 'name', 'signed_up_at']); + expect(fields.signed_up_at.type).toBe('datetime'); + expect(external.remoteName).toBe('customers'); + expect(external.remoteSchema).toBe('mart'); + expect(draft.source).toContain("remoteSchema: 'mart', remoteName: 'customers'"); + expect(draft.source).toContain("sharingModel: 'private'"); + }); +}); + describe('importObject inherits both repairs from the draft pipeline', () => { it('persists the prefixed name and the explicit OWD', async () => { const persisted: Array<{ name: string; def: Record }> = []; diff --git a/packages/services/service-datasource/src/__tests__/external-object-draft-primary-key.test.ts b/packages/services/service-datasource/src/__tests__/external-object-draft-primary-key.test.ts new file mode 100644 index 0000000000..c01cc629cb --- /dev/null +++ b/packages/services/service-datasource/src/__tests__/external-object-draft-primary-key.test.ts @@ -0,0 +1,285 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The introspected remote primary key: absent from the definition, present as + * a comment in the rendered source. + * + * ## The defect this pins closed (#11000) + * + * `generateObjectDraft` emitted `fields..primaryKey: true` and + * `renderObjectSource` rendered `, primaryKey: true` onto the field line. + * `primaryKey` is **not a key of the spec field schema**, so the generator had + * a pinned path producing a draft the platform's own toolchain refused on both + * instruments the rendered file is annotated for: + * + * - `tsc --noEmit` against `ServiceObject` — `TS2353: Object literal may only + * specify known properties, and 'primaryKey' does not exist in type …`; + * - `ObjectSchema.safeParse` — `unrecognized_keys` at `["fields",""]`. + * + * ## The ruling + * + * Maintainer, 2026-08-22 live session (「同意所有」, item 8) — **D**: + * stop emitting the key; the introspected key survives **as a comment** in the + * generated source (information preserved for the reader, zero contract face). + * The alternative of an authorable spelling on the binding + * (`external.primaryKey: string[]`) is **deferred, not rejected** — it returns + * as its own `packages/spec` card when federated upsert has a live runtime + * consumer. + * + * ## Why FOUR directions and not one + * + * "The draft parses now" is satisfiable by an implementation that simply drops + * `opts.primaryKey` on the floor — it would go green on a parse-only suite + * while discarding exactly what the ruling said to preserve. So the + * information-preservation direction is pinned as its own case, and is the one + * that reddens under ablation of the comment renderer. + * + * ## The instruments + * + * `ObjectSchema.safeParse` is asserted in full (`success === true`), not merely + * "no `unrecognized_keys`": what the field-drop has to buy is the whole + * definition's verdict, and #11059's `sharingModel` repair is part of that same + * verdict. The `tsc --noEmit` half of the acceptance runs against the built + * artifact in the PR's harness rather than in-process here — this file imports + * the service through a RELATIVE specifier (`../external-datasource-service.js`), + * which cannot route through the package's `exports`, so these cases measure + * `src/` and are correct without a build. + */ + +import { describe, it, expect } from 'vitest'; +import type { IntrospectedSchema } from '@objectstack/spec/contracts'; +import { ObjectSchema } from '@objectstack/spec/data'; +import { + ExternalDatasourceService, + type DatasourceLike, +} from '../external-datasource-service.js'; + +/** + * A remote schema with three shapes of key in one fixture: a single-column key + * (`orders.order_id`), a COMPOSITE key (`order_lines.order_id + line_no`) and a + * table with NO key at all (`events`). + * + * The composite table spells BOTH members `primaryKey: true`. That is what a + * complete introspection reports; #10997 (SQLite returns only the first column + * of a composite key) is a separate, unfixed defect in the engine lane, and + * pinning this generator against the truncated shape would bake that defect in + * as if it were the contract. What this file does owe #10997 is that the + * rendered comment must not CLAIM completeness — asserted below. + */ +function remoteSchema(): IntrospectedSchema { + return { + dialect: 'postgres', + introspectedAt: '2026-08-22T00:00:00.000Z', + tables: { + 'mart.orders': { + name: 'mart.orders', + indexes: [], + columns: [ + { name: 'order_id', type: 'text', nullable: false, primaryKey: true }, + { name: 'amount', type: 'numeric(10,2)', nullable: true, primaryKey: false }, + ], + }, + 'mart.order_lines': { + name: 'mart.order_lines', + indexes: [], + columns: [ + { name: 'order_id', type: 'text', nullable: false, primaryKey: true }, + { name: 'line_no', type: 'integer', nullable: false, primaryKey: true }, + { name: 'sku', type: 'text', nullable: true, primaryKey: false }, + ], + }, + 'mart.events': { + name: 'mart.events', + indexes: [], + columns: [ + { name: 'payload', type: 'jsonb', nullable: true, primaryKey: false }, + { name: 'at', type: 'timestamptz', nullable: true, primaryKey: false }, + ], + }, + }, + }; +} + +function svc(): ExternalDatasourceService { + return new ExternalDatasourceService({ + introspect: async () => remoteSchema(), + getDatasource: async (name): Promise => ({ name, schemaMode: 'external' }), + getObject: async () => undefined, + listObjects: async () => [], + getNamespace: () => 'wh', + }); +} + +/** Every field record in a draft definition, flattened for key inspection. */ +function fieldRecords(draft: { definition: Record }): Array<[string, object]> { + return Object.entries(draft.definition.fields as Record); +} + +describe('direction 1 — the unauthorable key is ABSENT from the definition', () => { + it('drops it on the option-supplied path', async () => { + const draft = await svc().generateObjectDraft('warehouse', 'orders', { + primaryKey: ['order_id'], + }); + + for (const [name, record] of fieldRecords(draft)) { + expect(Object.keys(record), `field '${name}'`).toEqual(['type']); + } + // Spelled twice on purpose: the loop above forbids ANY extra key, this + // line names the one the defect was about, so a regression reports it. + expect(JSON.stringify(draft.definition)).not.toContain('primaryKey'); + }); + + it('drops it on the introspection-supplied path', async () => { + const draft = await svc().generateObjectDraft('warehouse', 'orders'); + + const fields = draft.definition.fields as Record; + expect(Object.keys(fields.order_id)).toEqual(['type']); + expect(JSON.stringify(draft.definition)).not.toContain('primaryKey'); + }); + + it('renders no `primaryKey:` onto the field LINE either', async () => { + const draft = await svc().generateObjectDraft('warehouse', 'orders', { + primaryKey: ['order_id'], + }); + expect(draft.source).toContain("order_id: { type: 'text' },"); + expect(draft.source).not.toContain('primaryKey: true'); + }); +}); + +describe('direction 2 — the draft PARSES, which it could not before', () => { + it('accepts the whole definition on the key-set path', async () => { + const draft = await svc().generateObjectDraft('warehouse', 'orders', { + primaryKey: ['order_id'], + }); + + const parsed = ObjectSchema.safeParse(draft.definition); + expect(parsed.success, JSON.stringify((parsed as { error?: unknown }).error)).toBe(true); + }); + + it('accepts it on the composite-key path too', async () => { + const draft = await svc().generateObjectDraft('warehouse', 'order_lines', { + primaryKey: ['order_id', 'line_no'], + }); + const parsed = ObjectSchema.safeParse(draft.definition); + expect(parsed.success, JSON.stringify((parsed as { error?: unknown }).error)).toBe(true); + }); + + it('POSITIVE CONTROL — the same definition with the key put BACK is refused', async () => { + const draft = await svc().generateObjectDraft('warehouse', 'orders', { + primaryKey: ['order_id'], + }); + const fields = draft.definition.fields as Record>; + const poisoned = { + ...draft.definition, + fields: { ...fields, order_id: { ...fields.order_id, primaryKey: true } }, + }; + + const parsed = ObjectSchema.safeParse(poisoned); + expect(parsed.success).toBe(false); + // The instrument is live and refusing for THE reason #11000 measured, not + // for some incidental one. Without this, the green above could be a schema + // that stopped refusing anything at all. + expect(JSON.stringify((parsed as { error: unknown }).error)).toContain('unrecognized_keys'); + }); +}); + +describe('direction 3 — the information is PRESERVED as a comment (load-bearing)', () => { + it('names the single key column', async () => { + const draft = await svc().generateObjectDraft('warehouse', 'orders', { + primaryKey: ['order_id'], + }); + expect(draft.source).toContain('// Remote primary key: order_id'); + }); + + it('names EVERY member of a composite key, in order', async () => { + const draft = await svc().generateObjectDraft('warehouse', 'order_lines', { + primaryKey: ['order_id', 'line_no'], + }); + expect(draft.source).toContain('// Remote primary key: order_id, line_no'); + }); + + it('names the field name, not the remote column, when the field was renamed', async () => { + // The comment sits directly above the `fields:` block, so it has to speak + // that block's vocabulary — otherwise it points at a line that is not there. + const draft = await svc().generateObjectDraft('warehouse', 'orders', { + primaryKey: ['order_id'], + rename: { order_id: 'remote_order_id' }, + }); + expect(draft.source).toContain('// Remote primary key: remote_order_id'); + }); + + it('does NOT overclaim completeness — #10997 is unfixed and in another lane', async () => { + const draft = await svc().generateObjectDraft('warehouse', 'order_lines', { + primaryKey: ['order_id', 'line_no'], + }); + // The caveat, not merely "some comment exists": a reader who trusts this + // list as the complete key can be wrong today, through no fault of this + // generator, and the file has to say so. + expect(draft.source).toContain('#10997'); + expect(draft.source).toContain('lower bound'); + }); + + it('says WHY the key is a comment rather than a field key', async () => { + const draft = await svc().generateObjectDraft('warehouse', 'orders', { + primaryKey: ['order_id'], + }); + // A bare column list would read as an oversight to the next person to touch + // this generator — exactly the shape that put the key on the field in the + // first place. + expect(draft.source).toContain('#11000'); + expect(draft.source).toContain('no authorable key'); + }); + + it('emits NO comment at all when no key was reported', async () => { + const draft = await svc().generateObjectDraft('warehouse', 'events'); + expect(draft.source).not.toContain('Remote primary key'); + // …and the keyless draft is still a valid one. + expect(ObjectSchema.safeParse(draft.definition).success).toBe(true); + }); + + it('the comment survives INTO the source only — never into the definition', async () => { + const draft = await svc().generateObjectDraft('warehouse', 'orders', { + primaryKey: ['order_id'], + }); + expect(JSON.stringify(draft.definition)).not.toContain('Remote primary key'); + }); +}); + +describe('direction 4 — the `opts.primaryKey`-UNSET path is unchanged from #11059', () => { + it('still builds, still carries the namespace prefix and the declared OWD', async () => { + const draft = await svc().generateObjectDraft('warehouse', 'orders'); + + expect(draft.name).toBe('wh_orders'); + expect(draft.definition.sharingModel).toBe('private'); + expect(ObjectSchema.safeParse(draft.definition).success).toBe(true); + }); + + it('reports the introspected key in the comment without being asked', async () => { + // `opts.primaryKey` unset → the key comes from `col.primaryKey`. Pinned so + // that the fix cannot be read as "the option is ignored now". + const draft = await svc().generateObjectDraft('warehouse', 'orders'); + expect(draft.source).toContain('// Remote primary key: order_id'); + }); +}); + +describe('importObject persists the parseable definition', () => { + it('never writes the unauthorable key into the metadata store', async () => { + const persisted: Array<{ name: string; def: Record }> = []; + const service = new ExternalDatasourceService({ + introspect: async () => remoteSchema(), + getDatasource: async (name): Promise => ({ name, schemaMode: 'external' }), + getObject: async () => undefined, + listObjects: async () => [], + getNamespace: () => 'wh', + persistObject: async (name, def) => { + persisted.push({ name, def }); + }, + }); + + await service.importObject('warehouse', 'orders', { primaryKey: ['order_id'] }); + + expect(persisted).toHaveLength(1); + expect(JSON.stringify(persisted[0]?.def)).not.toContain('primaryKey'); + expect(ObjectSchema.safeParse(persisted[0]?.def).success).toBe(true); + }); +}); diff --git a/packages/services/service-datasource/src/external-datasource-service.ts b/packages/services/service-datasource/src/external-datasource-service.ts index 60cc5dfde7..942f2c6ea2 100644 --- a/packages/services/service-datasource/src/external-datasource-service.ts +++ b/packages/services/service-datasource/src/external-datasource-service.ts @@ -209,6 +209,40 @@ function toObjectName(remoteName: string): string { */ const GENERATED_SHARING_MODEL = 'private'; +/** + * The lead-in of the comment that carries the introspected remote primary key + * into the generated source — and the one place the reason is written down. + * + * `fields..primaryKey` is **not a key of the spec field schema**. Emitting + * it produced a `*.object.ts` the platform's own toolchain refused on both + * instruments it is annotated for: `tsc --noEmit` against `ServiceObject` + * (`TS2353 … 'primaryKey' does not exist in type`) and + * `ObjectSchema.safeParse` (`unrecognized_keys` at `["fields",""]`). So the + * generator had a pinned path that produced a draft neither the compiler nor + * the validator would take (#11000). + * + * Maintainer ruling, 2026-08-22 live session (「同意所有」, item 8) — **D**: + * + * > `generateObjectDraft`/`renderObjectSource` stop emitting + * > `fields..primaryKey`; the introspected key survives **as a comment** in + * > the generated source (information preserved for the reader, zero contract + * > face); the existing pin tests that assert the invalid emission are updated + * > as part of the fix. + * + * Both halves are load-bearing, and the second is the one an implementation + * can silently skip: simply dropping `opts.primaryKey` on the floor would make + * every parse-and-compile assertion green while discarding exactly what the + * ruling said to keep. + * + * The rejected alternatives, so they are not re-litigated from scratch: + * routing the key to `fields..externalId` was refused as semantically + * different and itself of unproven enforcement, and an authorable spelling on + * the binding (`external.primaryKey: string[]`) is **deferred, not rejected** — + * it returns as its own `packages/spec` card once federated upsert has a live + * runtime consumer to justify the surface. + */ +const REMOTE_PRIMARY_KEY_COMMENT = '// Remote primary key: '; + /** * Normalise an injected namespace. Blank / whitespace-only reads as ABSENT: * `validateObjectNamespacePrefix` skips a falsy namespace, but `' '` is @@ -331,7 +365,11 @@ export class ExternalDatasourceService implements IExternalDatasourceService { const exclude = opts.excludeColumns ? new Set(opts.excludeColumns) : new Set(); const pkOverride = opts.primaryKey ? new Set(opts.primaryKey) : undefined; - const fields: Record = {}; + const fields: Record = {}; + // The remote key is collected here rather than onto the field, because + // there is no authorable field key to put it on — see + // REMOTE_PRIMARY_KEY_COMMENT for the ruling and the measurements. + const primaryKeyFields: string[] = []; const review: ObjectDraft['review'] = []; for (const col of table.columns) { @@ -356,7 +394,8 @@ export class ExternalDatasourceService implements IExternalDatasourceService { } const isPk = pkOverride ? pkOverride.has(col.name) : col.primaryKey; - fields[fieldName] = isPk ? { type: fieldType, primaryKey: true } : { type: fieldType }; + fields[fieldName] = { type: fieldType }; + if (isPk) primaryKeyFields.push(fieldName); } // ADR-0028: every object a package defines must be named @@ -387,7 +426,7 @@ export class ExternalDatasourceService implements IExternalDatasourceService { name, datasource, definition, - source: renderObjectSource(definition, fields, review, namespace), + source: renderObjectSource(definition, fields, review, namespace, primaryKeyFields), review, }; } @@ -657,23 +696,46 @@ export class ExternalDatasourceService implements IExternalDatasourceService { * because the two absent cases are NOT the same file: a name that is already * prefixed and a name that could not be prefixed both read as "starts with * something" from here, and only the second one needs the TODO. + * + * `primaryKeyFields` is rendered as a COMMENT and nowhere else — see + * {@link REMOTE_PRIMARY_KEY_COMMENT}. It is passed separately from `fields` on purpose: + * a field record that could carry the key at all is a field record something + * could accidentally serialise into the definition again. */ function renderObjectSource( definition: Record, - fields: Record, + fields: Record, review: ObjectDraft['review'], namespace?: string, + primaryKeyFields: readonly string[] = [], ): string { const reviewByColumn = new Map(review.map((r) => [r.column, r.note])); const external = definition.external as { remoteSchema?: string; remoteName?: string }; const fieldLines = Object.entries(fields).map(([fieldName, f]) => { const note = reviewByColumn.get(fieldName); - const pk = f.primaryKey ? ', primaryKey: true' : ''; const comment = note ? ` // REVIEW: ${note}` : ''; - return ` ${fieldName}: { type: '${f.type}'${pk} },${comment}`; + return ` ${fieldName}: { type: '${f.type}' },${comment}`; }); + // The whole of ruling D's second half: information for the reader, zero + // contract face. Rendered only when a key was actually reported — with no + // key there is nothing to preserve, and an empty "none reported" banner would + // be noise in every draft of every keyless table. + const primaryKeyComment = + primaryKeyFields.length > 0 + ? [ + ` ${REMOTE_PRIMARY_KEY_COMMENT}${primaryKeyFields.join(', ')}`, + ` // Preserved as a COMMENT because 'ServiceObject' has no authorable key for a`, + ` // federated object's remote primary key (#11000): 'fields..primaryKey' is`, + ` // not part of the field schema, so emitting it produced a draft that neither`, + ` // 'tsc' nor 'ObjectSchema' accepted. Nothing below reads this line.`, + ` // It names the column(s) THIS DRAFT WAS GIVEN as the key. For a COMPOSITE key`, + ` // some drivers report only the first column (#10997), so treat the list as a`, + ` // lower bound and check it against the remote table before relying on it.`, + ] + : []; + const externalLine = external.remoteSchema ? ` external: { remoteSchema: '${external.remoteSchema}', remoteName: '${external.remoteName}' },` : ` external: { remoteName: '${external.remoteName}' },`; @@ -704,6 +766,7 @@ function renderObjectSource( ` label: '${definition.label as string}',`, ` datasource: '${definition.datasource as string}',`, externalLine, + ...primaryKeyComment, ` fields: {`, ...fieldLines, ` },`,