fix(service-datasource): stop rendering the unauthorable fields.*.primaryKey in object drafts

.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","<f>"]`.
+
+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.

.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.<f>.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.<f>.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.

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<string, { type: string; primaryKey?: boolean }>;
- 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.<f>.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<string, unknown>;
  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 () => {

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.<f>.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.<f>.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<string, { type: string }>;
+ 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<string, unknown> }> = [];