Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions .changeset/29898-picklist-binding-unknown-refused.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
---
'@objectstack/metadata-protocol': minor
'@objectstack/spec': minor
'@objectstack/cli': minor
---

fix(metadata-protocol)!: the metadata save and publish doors refuse a select field bound to a picklist no loaded package declares, instead of storing it and then refusing every write to the field (#29898)

Clause-②: no (narrowing) — the metadata save door (draft and live), the per-item and package publish, and the restore doors refuse an `object` or `object_extension` whose field names a picklist no loaded package declares, a body they accepted before. Nothing is widened.
Metadata-protocol: narrowed
Skills-docs: no move — `skills/**` teaches no picklist reference verdict and `content/docs/**` carries no severity table for it; the prescription ships in the refusal text, the FROM → TO below and the upgrade-guide entry.

<!-- adr-0087: registered picklist-binding-unknown-refused -->

**BREAKING** accept-set narrowing at the metadata save and publish doors: `PUT /api/v1/meta/object/NAME` and `PUT /api/v1/meta/object_extension/NAME` (draft and live), `POST /api/v1/packages/PKG/publish-drafts`, the per-item publish, and the rollback / revert restore doors.

- **Before.** A select (or multiselect) field with `picklist: 'nope_list'`, a name no loaded package declares, saved `200` with its draft reading `_diagnostics.valid: true`, and `publish-drafts` answered `outcome: published`. The field was then served with no options, and every create or update that set it answered `400 VALIDATION_FAILED` ("takes its values from picklist \"nope_list\", which no loaded package declares"). The boot and a late install already refuse the same shape for a package's own objects, and `os validate` / `os build` refuse it for a stack's own fields.
- **Now.** The doors refuse it `422 INVALID_METADATA`, one finding per unknown binding at `fields.FIELD.picklist`, naming the field and the list. A list is declared when the engine's registry resolves it, the same read that serves the field's options: the package's own, a dependency's, the base package's of a `.custom` pair, or any other package installed here.
- **Unchanged.** A binding the object already holds is not judged again on a re-save, so an object published before this release keeps saving; the stored-metadata healing pass (`os migrate meta --stored`) is not judged. A draft stored before this release is judged when it is published.
- **`@objectstack/spec`.** The upgrade guide entry `picklist-binding-unknown-refused` (a semantic migration: no automatic rewrite, because whether the author meant another list, one still to be declared or inline options is a judgement about the field).
- **`@objectstack/cli`.** A source comment in the `os validate` picklist judge no longer says the runtime write path is unjudged.

**FROM → TO.** `fields.t: { type: 'select', picklist: 'nope_list' }` → `picklist: 'NAME'` of a list a package installed here declares, or declare `nope_list` in a code package (a `*.picklist.ts` file, or `defineStack({ picklists })`), or `options: [...]` in place of `picklist`.
108 changes: 104 additions & 4 deletions packages/cli/src/utils/picklist-references.boot-parity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ import { afterEach, describe, expect, it } from 'vitest';
import { ObjectKernel, type Plugin, type PluginContext } from '@objectstack/core';
import { ObjectQLPlugin } from '@objectstack/objectql';

import { judgePicklistReferences, PICKLIST_RESIDUAL_UNREFERENCED } from './picklist-references.js';
import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol';

import { judgePicklistReferences, PICKLIST_REFERENCE_UNKNOWN, PICKLIST_RESIDUAL_UNREFERENCED } from './picklist-references.js';

type AnyRec = Record<string, unknown>;

Expand Down Expand Up @@ -59,22 +61,37 @@ afterEach(async () => {
}
});

/** Boot the stack through the `manifest` service; the boot's refusal, or `undefined`. */
async function bootRefusal(stack: AnyRec): Promise<(Error & { code?: string }) | undefined> {
/**
* Boot the stack through the `manifest` service. Answers the boot's refusal (or
* `undefined`) and the engine it registered into, so a pin can ask the
* REGISTRY what the boot made of the stack.
*/
async function bootStack(stack: AnyRec): Promise<{
refusal?: Error & { code?: string };
engine?: { registry: unknown };
}> {
const kernel = new ObjectKernel({ logger: { level: 'silent' }, gracefulShutdown: false });
kernels.push(kernel);
await kernel.use(new ObjectQLPlugin());
let engine: { registry: unknown } | undefined;
await kernel.use({
name: 'test.parity.app',
dependencies: ['com.objectstack.engine.objectql'],
init: async (ctx: PluginContext) => {
engine = ctx.getService<{ registry: unknown }>('objectql');
await ctx.getService<{ register(m: unknown): unknown }>('manifest')
.register({ ...(stack.manifest as AnyRec), ...stack });
},
} as Plugin);
return kernel.bootstrap().then(() => undefined, (e: Error & { code?: string }) => e);
return kernel.bootstrap().then(
() => ({ engine }),
(e: Error & { code?: string }) => ({ refusal: e, engine }),
);
}

/** Boot the stack through the `manifest` service; the boot's refusal, or `undefined`. */
const bootRefusal = async (stack: AnyRec) => (await bootStack(stack)).refusal;

describe('picklist-residual-unreferenced — the author-time door and the boot agree', () => {
it.each(Object.keys(STACKS))('%s', async (label) => {
const stack = STACKS[label];
Expand All @@ -90,3 +107,86 @@ describe('picklist-residual-unreferenced — the author-time door and the boot a
expect(boot?.message).toBe(`${door[0].message} ${door[0].hint}`);
});
});

/**
* [#29898] The second parity, on the same stacks: `picklist-reference-unknown`
* (this door, over the PARSED artifact) and the metadata save door's
* `unknownPicklistBindingSaveRefusal` (`@objectstack/metadata-protocol`, over
* the BOOTED registry) must call the same name declared.
*
* They cannot share one judgement the way the residual pair above does: this
* door resolves a reference against the artifact's own `picklists` and weighs
* the declaring package's outside dependencies, while the save door asks the
* live registry (`resolvePicklistOptions`, the read that serves the field's
* options) because a runtime write is judged against the packages INSTALLED
* here. So the agreement is pinned rather than inherited — and pinned where
* the two can disagree: a list a SIBLING package of the same artifact owns,
* which this door resolves under ADR-0130 D4 and the save door must resolve
* because the boot registered it.
*
* Asked of the save door's judge directly, over the real registry the boot
* filled. `readMetaItem` is stubbed to "no stored row", so the case is a FIRST
* write of the binding; the held-binding exemption, the other doors and the
* envelope are pinned in
* `packages/metadata-protocol/src/protocol.unknown-picklist-binding-refusal.test.ts`,
* and the real-store controls (a dependency's list, the base's list of a
* `.custom` pair, another installed package's) in
* `packages/runtime/src/domains/meta-picklist-binding-door.test.ts`.
*/
describe('[#29898] picklist-reference-unknown — the author-time door and the metadata save door agree', () => {
/** The object body a case writes: one select field bound to `picklist`. */
const boundCase = (picklist: string) =>
caseObj({ subject: { type: 'text', label: 'Subject' }, status: { type: 'select', label: 'Status', picklist } });

/** One artifact: the core body carries the object, the app body declares `parity_status`. */
const artifactWith = (picklist: string): AnyRec => ({
manifest: manifest(STACK_ID),
packages: [
{ manifest: { ...manifest('com.example.parity.core'), objects: [boundCase(picklist)] } },
{ manifest: { ...manifest('com.example.parity.app'), picklists: [status] } },
],
});

/**
* The save door's judge, over the registry a boot of the DECLARING artifact
* filled — the live deployment an author then writes into. ⛔ Not a boot of
* the typo artifact: that is the shape the boot already refuses (the pin
* above), which is why the save door is the door this card is about.
*/
const saveDoorRefusal = async (body: AnyRec) => {
const { refusal, engine } = await bootStack(artifactWith('parity_status'));
expect(refusal, String(refusal?.message)).toBeUndefined();
const protocol = new ObjectStackProtocolImplementation(engine as never, () => new Map()) as unknown as {
readMetaItem: unknown;
unknownPicklistBindingSaveRefusal(type: string, item: unknown, name: string): Promise<{ issues?: Array<{ path: string }> } | undefined>;
};
protocol.readMetaItem = async () => undefined;
return protocol.unknownPicklistBindingSaveRefusal('object', body, String(body.name));
};

it('a list a SIBLING package of the same artifact declares: both doors accept it', async () => {
const stack = artifactWith('parity_status');

const door = judgePicklistReferences(stack);
const saveDoor = await saveDoorRefusal(boundCase('parity_status'));

expect(door.refusals.filter((r) => r.rule === PICKLIST_REFERENCE_UNKNOWN)).toEqual([]);
expect(door.notices).toEqual([]);
expect(saveDoor, JSON.stringify(saveDoor?.issues)).toBeUndefined();
});

// The two paths are rooted in their own SUBJECTS — the artifact for this
// door, the one body for the save door — so the field is the tail of the
// artifact-rooted path, never the whole of it.
it('a name no package of the artifact declares: both doors refuse it, at the same field', async () => {
const stack = artifactWith('parity_typo');

const door = judgePicklistReferences(stack).refusals.filter((r) => r.rule === PICKLIST_REFERENCE_UNKNOWN);
const saveDoor = await saveDoorRefusal(boundCase('parity_typo'));

expect(door).toHaveLength(1);
expect(saveDoor?.issues?.map((i) => i.path)).toEqual(['fields.status.picklist']);
expect(door[0].path, door[0].path).toBe('packages[0].manifest.objects[0].fields.status.picklist');
expect(door[0].path?.endsWith(saveDoor!.issues![0].path)).toBe(true);
});
});
6 changes: 4 additions & 2 deletions packages/cli/src/utils/picklist-references.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,10 @@
* package reading, the same class as `findViewContainerNameRefusals`, and both
* doors call it (`validate.ts` step 2d, `compile.ts` step 3a-bis).
*
* ⚠️ Bound, stated rather than hidden: the runtime metadata write path (a field
* saved through Studio or REST `/meta`) is not judged here.
* The runtime metadata write path (a field saved through Studio or REST
* `/meta`) is not judged here: the metadata save and publish doors judge it
* against the engine's registry (`unknownPicklistBindingSaveRefusal`,
* `@objectstack/metadata-protocol`).
*
* Reads the PARSED stack — what `defineStack()` hands the boot wrap, and what
* `os build` serializes.
Expand Down
5 changes: 4 additions & 1 deletion packages/metadata-protocol/src/protocol-authoring-gate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ export interface AuthoringGateHost {
tenancyPostureInForce: ObjectStackProtocolImplementation['tenancyPostureInForce'];
unknownDatasourceSaveRefusal: ObjectStackProtocolImplementation['unknownDatasourceSaveRefusal'];
unknownGrantObjectSaveRefusal: ObjectStackProtocolImplementation['unknownGrantObjectSaveRefusal'];
unknownPicklistBindingSaveRefusal: ObjectStackProtocolImplementation['unknownPicklistBindingSaveRefusal'];
seedExternalIdSaveRefusal: ObjectStackProtocolImplementation['seedExternalIdSaveRefusal'];
}

Expand Down Expand Up @@ -126,7 +127,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' | 'seedExternalIdSaveRefusal' | 'unknownDatasourceSaveRefusal' | 'unknownGrantObjectSaveRefusal'>,
this: Pick<AuthoringGateHost, 'anonymousElevatedFlowRefusal' | 'assertRuntimeAuthoringRules' | 'authoringChannel' | 'credentialChannelFor' | 'defaultRecordPageSaveRefusal' | 'externalBindingSaveRefusal' | 'getOverlayRepo' | 'permissionSetNameCollisionRefusal' | 'runAuthoringGate' | 'seedExternalIdSaveRefusal' | 'unknownDatasourceSaveRefusal' | 'unknownGrantObjectSaveRefusal' | 'unknownPicklistBindingSaveRefusal'>,
type: string,
name: string,
body: unknown,
Expand All @@ -145,6 +146,8 @@ export async function refuseRestoredBodyTheSaveDoorRefuses(
if (datasourceRefusal) throw datasourceRefusal;
const grantRefusal = await this.unknownGrantObjectSaveRefusal(type, body, name);
if (grantRefusal) throw grantRefusal;
const picklistRefusal = await this.unknownPicklistBindingSaveRefusal(type, body, name);
if (picklistRefusal) throw picklistRefusal;
const seedKeyRefusal = await this.seedExternalIdSaveRefusal(type, body, name);
if (seedKeyRefusal) throw seedKeyRefusal;
const repo = this.getOverlayRepo();
Expand Down
65 changes: 65 additions & 0 deletions packages/metadata-protocol/src/protocol-save-door-refusals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ import { externalBindingDatasourceRefusal, resolveInjectedSystemColumns } from '
import { METADATA_ITEM_NAME_PATTERN } from '@objectstack/spec/shared';
import { defaultRecordPageObject } from '@objectstack/spec/ui';
import { MetadataProtectionFields, validateObjectNamespacePrefix } from '@objectstack/spec/kernel';
// [#29898] The one walk of which picklists an object body's fields name — the
// boot audit's. (The `os validate` door reads the parsed artifact with its own
// walk and agrees with this door by pin, not by sharing it.) ⛔ Never a second
// walk here.
import { collectPicklistReferences } from '@objectstack/metadata/picklist-references';
import { OBJECT_EXTENSION_TYPE } from './protocol-object-extension-item.js';

/** The lock family `body` carries: each member with a value, and nothing else. */
function lockFamilyOf(body: unknown): Record<string, unknown> {
Expand Down Expand Up @@ -640,6 +646,65 @@ export async function unknownGrantObjectSaveRefusal(
return err;
}

/**
* The body of `ObjectStackProtocolImplementation#unknownPicklistBindingSaveRefusal`;
* the method's docblock there is its contract. Written here beside
* {@link unknownGrantObjectSaveRefusal}, whose shape it follows: the engine's
* registry, then the definition the object already holds.
*/
export async function unknownPicklistBindingSaveRefusal(
this: Pick<SaveDoorRefusalHost, 'engine' | 'readMetaItem'>,
singularType: string,
item: unknown,
name: string,
writeFace?: MetadataWriteFace,
): Promise<(Error & { code: 'INVALID_METADATA'; status: 422; issues: MetadataIssueEntry[] }) | undefined> {
if (singularType !== 'object' && singularType !== OBJECT_EXTENSION_TYPE) return undefined;
if (!item || typeof item !== 'object' || Array.isArray(item)) return undefined;
// An extension's fields join the object it extends; its row is stored
// under that object's name.
const extend = (item as { extend?: unknown }).extend;
const object = singularType === OBJECT_EXTENSION_TYPE && typeof extend === 'string' && extend !== '' ? extend : name;
let unknown = collectPicklistReferences(object, item, undefined);
if (unknown.length === 0) return undefined;
// The registry is the authority that SERVES a bound field's options
// (`resolvePicklistOptions`): a list it resolves is one a loaded package
// declares — the package's own, a dependency's, the base package's of a
// `.custom` pair, or any other package installed here. A registry that
// cannot resolve a list cannot say a name is unknown, so nothing is
// refused on it.
const registry = this.engine?.registry as { resolvePicklistOptions?: unknown } | undefined;
const resolvePicklistOptions = registry?.resolvePicklistOptions;
if (typeof resolvePicklistOptions !== 'function') return undefined;
unknown = unknown.filter((ref) => resolvePicklistOptions.call(registry, ref.picklist) === undefined);
if (unknown.length === 0) return undefined;
// A binding the object already holds is not this save's: a re-save, a
// stored-row migration or a restore that keeps it is judged at the save
// that introduced it, and a list that has gone since is not the
// re-saving author's to answer for.
const current = (await this.readMetaItem({ type: 'object', name: object }))?.item;
const bindingOf = (ref: { field: string; picklist: string }) => JSON.stringify([ref.field, ref.picklist]);
const held = new Set(collectPicklistReferences(object, current, undefined).map(bindingOf));
unknown = unknown.filter((ref) => !held.has(bindingOf(ref)));
if (unknown.length === 0) return undefined;
const issues: MetadataIssueEntry[] = unknown.map((ref) => ({
path: `fields.${ref.field}.picklist`,
message: `Field '${ref.object}.${ref.field}' is bound to picklist '${ref.picklist}', which no loaded package `
+ 'declares, so the field would be served with no options and every write that sets it would be refused. '
+ 'A field bound to a picklist takes its options from that list and has none of its own. Correct the name '
+ 'to a picklist a package installed here declares, declare the picklist (a `*.picklist.ts` file, or '
+ '`defineStack({ picklists })`) in a package installed here, or give the field its own `options` instead.',
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;
}

/** The body of `ObjectStackProtocolImplementation#objectsBoundExternallyTo`; the method's docblock there is its contract. */
export async function objectsBoundExternallyTo(
this: Pick<SaveDoorRefusalHost, 'engine' | 'readActiveOverlayRows' | 'rethrowUnlessMetadataStoreUnprovisioned' | 'storedOverlayEntries'>,
Expand Down
Loading
Loading