fix(metadata-protocol)!: refuse a keyed seed whose externalId names no field of its object

.changeset/28952-seed-external-id-refusal.md

@@ -0,0 +1,21 @@
+---
+'@objectstack/metadata-protocol': minor
+---
+
+fix(metadata-protocol)!: a keyed seed whose `externalId` names no field of its object is refused at save and at publish, and the boot loader warns once for a keyed dataset whose every record keys empty (#28952)
+
+Clause-②: no (narrowing) — the metadata save door (`saveMetaItem`, draft and publish mode), the publish promotion (`publishMetaItem`, `publishPackageDrafts`) and the restore doors refuse a `seed` in `upsert`, `update` or `ignore` mode whose `externalId` (each member of a composite; an undeclared one is the default `name`) is not a field of the seed's object, which they stored and published before. Nothing is widened.
+Metadata-protocol: narrowed
+
+<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped, and no export or type changes: the save and publish doors now judge an existing key, `externalId`, against the seed's object. A seed that names a field its object has saves as before; the fix for a refused one is to name a field the object declares, which no migration can choose for the author. -->
+
+**BREAKING** accept-set narrowing at the metadata save and publish doors (`@objectstack/metadata-protocol`).
+
+- **Before.** A seed with `mode: 'upsert'` and `externalId: 'no_such_field'` was saved (`_diagnostics.valid: true`) and published. The loader keys every existing row and every incoming record on that field, which none of them carries, so every key was empty and nothing matched: each publish answered `seedApplied {"success":true,"inserted":1,"updated":0}` and inserted every record again, with no probe issue and no log line.
+- **Now.** The save is refused with the save door's envelope for a body it judges (the one an object naming an unknown datasource gets), `422 INVALID_METADATA`, one finding per missing member at `externalId` (`externalId.N` for a composite member), naming the field and the object; nothing is stored. A seed draft stored before this change, or one whose object was still a draft at its save, is refused when it is published, before any seed row is written, and a package publish aborts on it. An object drafted in the same package batch is judged as it will be published.
+- **What counts as a field.** The object's fields as the engine registers them (extension fields included) and the columns the registry injects (`id`, the audit and tenant columns).
+- **Not judged.** `insert` and `replace` seeds, which never match by key; a draft seed whose object is itself still a draft (judged at its publish); a seed whose object is neither registered nor drafted (the load reports it); a `migrate-stored` re-save.
+- **The boot loader never refuses.** For a keyed dataset whose every record has no value for the key, it now logs one `warn` naming the object and the key, and loads exactly as before: a replay of such a stored seed still inserts every record again (`upsert`, `ignore`) or skips it (`update`).
+- **Who sees it.** A package author whose seed key names a field the object does not have (a typo, or a field since renamed). Measured: no seed shipped in this repository's examples or in hotcrm names such a key.
+
+**The remedy.** Set `externalId` to a field the object declares, or use `mode: 'insert'` for rows with no natural key.

packages/metadata-protocol/src/protocol-authoring-gate.ts

@@ -90,6 +90,7 @@ export interface AuthoringGateHost {
     runAuthoringGate: ObjectStackProtocolImplementation['runAuthoringGate'];
     tenancyPostureInForce: ObjectStackProtocolImplementation['tenancyPostureInForce'];
     unknownDatasourceSaveRefusal: ObjectStackProtocolImplementation['unknownDatasourceSaveRefusal'];
+    seedExternalIdSaveRefusal: ObjectStackProtocolImplementation['seedExternalIdSaveRefusal'];
 }
 
 /** The body of `ObjectStackProtocolImplementation#restoredBodyDerivation`; the method's docblock there is its contract. */
@@ -123,7 +124,7 @@ export function restoredBodyWriter(
 
 /** The body of `ObjectStackProtocolImplementation#refuseRestoredBodyTheSaveDoorRefuses`; the method's docblock there is its contract. */
 export async function refuseRestoredBodyTheSaveDoorRefuses(
-    this: Pick<AuthoringGateHost, 'anonymousElevatedFlowRefusal' | 'assertRuntimeAuthoringRules' | 'authoringChannel' | 'credentialChannelFor' | 'defaultRecordPageSaveRefusal' | 'externalBindingSaveRefusal' | 'getOverlayRepo' | 'permissionSetNameCollisionRefusal' | 'runAuthoringGate' | 'unknownDatasourceSaveRefusal'>,
+    this: Pick<AuthoringGateHost, 'anonymousElevatedFlowRefusal' | 'assertRuntimeAuthoringRules' | 'authoringChannel' | 'credentialChannelFor' | 'defaultRecordPageSaveRefusal' | 'externalBindingSaveRefusal' | 'getOverlayRepo' | 'permissionSetNameCollisionRefusal' | 'runAuthoringGate' | 'seedExternalIdSaveRefusal' | 'unknownDatasourceSaveRefusal'>,
     type: string,
     name: string,
     body: unknown,
@@ -140,6 +141,8 @@ export async function refuseRestoredBodyTheSaveDoorRefuses(
     if (bindingRefusal) throw bindingRefusal;
     const datasourceRefusal = await this.unknownDatasourceSaveRefusal(type, body, name);
     if (datasourceRefusal) throw datasourceRefusal;
+    const seedKeyRefusal = await this.seedExternalIdSaveRefusal(type, body, name);
+    if (seedKeyRefusal) throw seedKeyRefusal;
     const repo = this.getOverlayRepo();
     const credentialsRefRefusal = await datasourceCredentialsRefSaveRefusal({
         headAt: (...a) => repo.headAt(...a), type, name, item: body,

packages/metadata-protocol/src/protocol-save-door-refusals.ts

@@ -62,7 +62,7 @@ import {
 import { resolveItemLock, withItemLockFamily } from './item-lock.js';
 import type { ItemAddress } from './item-lock.js';
 import { itemDiscriminator } from '@objectstack/metadata-core';
-import { externalBindingDatasourceRefusal } from '@objectstack/spec/data';
+import { externalBindingDatasourceRefusal, resolveInjectedSystemColumns } from '@objectstack/spec/data';
 import { METADATA_ITEM_NAME_PATTERN } from '@objectstack/spec/shared';
 import { defaultRecordPageObject } from '@objectstack/spec/ui';
 import { MetadataProtectionFields, validateObjectNamespacePrefix } from '@objectstack/spec/kernel';
@@ -677,3 +677,83 @@ export function objectNamespacePrefixViolation(
     if (typeof packageId !== 'string' || packageId === '') return null;
     return validateObjectNamespacePrefix(name, this.engine?.registry?.getPackage?.(packageId)?.manifest?.namespace);
 }
+
+/** The seed modes whose load matches incoming records to existing rows by the dataset's `externalId`. */
+const KEYED_SEED_MODES: ReadonlySet<string> = new Set(['upsert', 'update', 'ignore']);
+
+/**
+ * The body of `ObjectStackProtocolImplementation#seedExternalIdSaveRefusal`;
+ * the method's docblock there is its contract. Written here, beside the other
+ * refusals the save door and the publish promotion ask in turn, rather than
+ * moved: its reads are the engine's registered object and, on a draft save,
+ * the object drafts.
+ */
+export async function seedExternalIdSaveRefusal(
+    this: Pick<SaveDoorRefusalHost, 'engine' | 'listDrafts'>,
+    singularType: string,
+    item: unknown,
+    name: string,
+    pending?: 'drafts' | ReadonlyArray<unknown>,
+    writeFace?: MetadataWriteFace,
+): Promise<(Error & { code: 'INVALID_METADATA'; status: 422; issues: MetadataIssueEntry[] }) | undefined> {
+    if (singularType !== 'seed') return undefined;
+    if (!item || typeof item !== 'object' || Array.isArray(item)) return undefined;
+    const seed = item as { object?: unknown; mode?: unknown; externalId?: unknown };
+    if (typeof seed.object !== 'string' || seed.object === '') return undefined;
+    const objectName = seed.object;
+    // `SeedSchema`'s defaults, as the loader applies them (`mode: 'upsert'`,
+    // `externalId: 'name'`): a body read before the parse filled them keys the
+    // same way a parsed one does.
+    const mode = typeof seed.mode === 'string' ? seed.mode : 'upsert';
+    if (!KEYED_SEED_MODES.has(mode)) return undefined;
+    const declared = seed.externalId !== undefined;
+    const externalId = declared ? seed.externalId : 'name';
+    const composite = Array.isArray(externalId);
+    const members = (composite ? externalId : [externalId]) as unknown[];
+    // The object as the publish will have it: a draft of it in the same
+    // package batch, else the engine's registered definition (its fields as
+    // the runtime serves them, extension fields and injected columns
+    // included). On a draft save, an object that is itself still a draft may
+    // gain the field before it is published, so the seed is judged at its
+    // publish instead. An object resolved by neither is not judged here:
+    // the load reports an object it cannot find.
+    const pendingObject = Array.isArray(pending)
+        ? pending.find((o) => (o as { name?: unknown } | null)?.name === objectName) as { fields?: unknown } | undefined
+        : undefined;
+    if (!pendingObject && pending === 'drafts'
+        && (await this.listDrafts({ type: 'object' })).drafts.some((d) => d.name === objectName)) {
+        return undefined;
+    }
+    const registered = (this.engine?.registry as { getObject?(name: string): unknown } | undefined)
+        ?.getObject?.(objectName) as { fields?: unknown } | undefined;
+    if (!pendingObject && !registered) return undefined;
+    const fieldNames = new Set<string>(resolveInjectedSystemColumns(pendingObject ?? registered).names);
+    for (const def of [registered, pendingObject]) {
+        const fields = def?.fields;
+        if (fields && typeof fields === 'object' && !Array.isArray(fields)) {
+            for (const key of Object.keys(fields)) fieldNames.add(key);
+        }
+    }
+    const missing = members
+        .map((member, index) => ({ member, index }))
+        .filter(({ member }) => typeof member === 'string' && !fieldNames.has(member));
+    if (missing.length === 0) return undefined;
+    const consequence = `so a load in \`${mode}\` mode could never match a row it wrote before: each publish and each `
+        + `replay would ${mode === 'update' ? 'skip every record' : 'insert every record again'}. Name a field the object `
+        + "declares, or use `mode: 'insert'` for rows with no natural key.";
+    const issues: MetadataIssueEntry[] = missing.map(({ member, index }) => ({
+        path: composite ? `externalId.${index}` : 'externalId',
+        message: (declared
+            ? `Seed '${name}' keys on externalId member '${String(member)}', which is not a field of object '${objectName}', `
+            : `Seed '${name}' declares no externalId, so it keys on the default 'name', which is not a field of object '${objectName}', `)
+            + consequence,
+        code: 'custom',
+    }));
+    const err = new Error(
+        `${singularType}/${name} failed spec validation: ${specValidationFindings(writeFace, issues)}`,
+    ) as Error & { code: 'INVALID_METADATA'; status: 422; issues: MetadataIssueEntry[] };
+    err.code = 'INVALID_METADATA';
+    err.status = 422;
+    err.issues = issues;
+    return err;
+}

packages/metadata-protocol/src/protocol.restore-doors-save-refusal-family.test.ts

@@ -8,7 +8,8 @@
  * family of shapes after its schema gate: a hook with no `body`, a second
  * default record page of an object, an `external` object binding on a managed
  * datasource (both halves of the pair), an object `datasource` that names no
- * datasource (#26724), a `runAs: 'system'` flow an anonymous
+ * datasource (#26724), a keyed seed whose `externalId` names no field of its
+ * object (#28952), a `runAs: 'system'` flow an anonymous
  * endpoint names, the shared runtime authoring rules, and the authoring gate a
  * domain plugin registers. Measured on `origin/main` `5801a4f2c` before this
  * file: each restore door wrote every one of those shapes back, so a version
@@ -95,6 +96,8 @@ function makeStubEngine() {
     const registered = new Map<string, Map<string, unknown>>();
     // [#26724] The drivers the engine has registered by name, with no datasource item.
     const drivers = new Set<string>();
+    /** The engine's registered objects, by name (empty unless a case stages one). */
+    const objects = new Map<string, unknown>();
     let nextId = 0;
     let commit: unknown = null;
 
@@ -188,7 +191,7 @@ function makeStubEngine() {
             listItems: (type: string) => Array.from(registered.get(type)?.values() ?? []),
             getItem: (type: string, name: string) => registered.get(type)?.get(name),
             getAllObjects: () => [],
-            getObject: () => undefined,
+            getObject: (name: string) => objects.get(name),
             getPackage: () => undefined,
             getArtifactItem: () => undefined,
             isPackageDisabled: () => false,
@@ -200,6 +203,7 @@ function makeStubEngine() {
         historyRows,
         registered,
         drivers,
+        objects,
         serveCommit: (c: unknown) => { commit = c; },
     };
 }
@@ -502,6 +506,31 @@ pinBothDoors({
     },
 });
 
+pinBothDoors({
+    title: 'a seed restored keyed on a field its object no longer has (seedExternalIdSaveRefusal)',
+    type: 'seed',
+    name: 'wh_order_seed',
+    code: 'INVALID_METADATA',
+    status: 422,
+    async stage(h) {
+        const seedOf = (externalId: string, label?: string) => ({
+            object: 'wh_order', mode: 'upsert', externalId, records: [{ name: 'O1', code: 'o1' }],
+            ...(label ? { label } : {}),
+        });
+        h.objects.set('wh_order', { name: 'wh_order', fields: { name: { type: 'text' }, code: { type: 'text' } } });
+        await h.protocol.saveMetaItem({ type: 'seed', name: 'wh_order_seed', item: seedOf('code'), mode: 'draft' });
+        await h.protocol.saveMetaItem({ type: 'seed', name: 'wh_order_seed', item: seedOf('name'), mode: 'draft' });
+        await h.protocol.publishMetaItem({ type: 'seed', name: 'wh_order_seed' });
+        // The object lost the field v1 is keyed on.
+        h.objects.set('wh_order', { name: 'wh_order', fields: { name: { type: 'text' } } });
+        return { refused: 1, accepted: 2 };
+    },
+    expectDetail: (error) => {
+        expect(error.issues).toEqual([expect.objectContaining({ path: 'externalId', code: 'custom' })]);
+        expect(error.issues[0].message.startsWith("Seed 'wh_order_seed' keys on externalId member 'code', which is not a field of object 'wh_order'")).toBe(true);
+    },
+});
+
 pinBothDoors({
     title: 'a body the runtime authoring rules refuse (assertRuntimeAuthoringRules)',
     type: 'object',