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
16 changes: 16 additions & 0 deletions .changeset/26886-picklist-diagnostics-authored-shape.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
'@objectstack/metadata-core': patch
'@objectstack/metadata-protocol': patch
'@objectstack/objectql': patch
---

fix(metadata-protocol): load-time diagnostics no longer report "`picklist` and `options` cannot both be declared" on a field that declares only `picklist`

Clause-②: no
Metadata-protocol: none

A select field that names a shared list (`picklist: 'industry'`) is served with the list's options written beside `picklist`, and the diagnostics judged that served copy, so every shared-picklist field of every app was reported as declaring both keys: in an item's `_diagnostics`, in the list read's, and in `GET /meta/diagnostics` (Setup › All Metadata Types). They now judge the field as its author wrote it.

- **The rule stands.** A field that authors both `picklist` and `options` is still reported, with the same message and severity, at `fields.NAME.options`.
- **What `/meta` serves is unchanged.** A picklist-bound field is still served with `picklist` and the resolved `options`.
- **How the two are told apart.** The picklist resolution marks the options array it derives; `@objectstack/metadata-core` exports `markResolvedPicklistOptions` and `isResolvedPicklistOptions`, and the diagnostics leave a marked array out of the judgement. The mark is not part of any response or stored row.
6 changes: 6 additions & 0 deletions packages/metadata-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,3 +133,9 @@ export * from './object-field-type.js';
// `view` write in `@objectstack/metadata-protocol`; both read this one rule, so
// a form withdrawn by either declared switch is withdrawn everywhere.
export * from './anonymous-form-intake.js';

// [#26886] The mark on an `options` array resolved from a shared picklist,
// shared by the resolution (`@objectstack/objectql`'s lean core closure) and the
// load-time diagnostics (`@objectstack/metadata-protocol`), so the diagnostics
// judge a field as its author wrote it.
export * from './resolved-picklist-options.js';
39 changes: 39 additions & 0 deletions packages/metadata-core/src/resolved-picklist-options.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* #26886 — the mark on options resolved from a shared picklist rides the array
* itself and nothing else: never a JSON key, never on a copy. A copy reads as
* authored, so a reader of it reports a diagnostic rather than missing one.
*/
import { describe, expect, it } from 'vitest';
import { isResolvedPicklistOptions, markResolvedPicklistOptions } from './resolved-picklist-options.js';

describe('resolved picklist options mark', () => {
it('marks the array it is given, in place, and only that array', () => {
const options = [{ label: 'Tech', value: 'tech' }];
expect(markResolvedPicklistOptions(options)).toBe(options);
expect(isResolvedPicklistOptions(options)).toBe(true);
expect(isResolvedPicklistOptions(markResolvedPicklistOptions(options))).toBe(true);
expect(isResolvedPicklistOptions([{ label: 'Tech', value: 'tech' }])).toBe(false);
expect(isResolvedPicklistOptions(undefined)).toBe(false);
});

it('is invisible to JSON and to key enumeration', () => {
const options = markResolvedPicklistOptions([{ label: 'Tech', value: 'tech' }]);
expect(JSON.stringify({ options })).toBe('{"options":[{"label":"Tech","value":"tech"}]}');
expect(Object.keys(options)).toEqual(['0']);
expect(Object.getOwnPropertySymbols(options).map((s) => Object.getOwnPropertyDescriptor(options, s)?.enumerable)).toEqual([false]);
});

it('does not survive a copy of the array', () => {
const options = markResolvedPicklistOptions([{ label: 'Tech', value: 'tech' }]);
expect(isResolvedPicklistOptions(options.map((o) => ({ ...o })))).toBe(false);
expect(isResolvedPicklistOptions([...options])).toBe(false);
expect(isResolvedPicklistOptions(JSON.parse(JSON.stringify(options)))).toBe(false);
});

it('survives a copy of the field that holds it', () => {
const field = { picklist: 'industry', options: markResolvedPicklistOptions([{ label: 'Tech', value: 'tech' }]) };
expect(isResolvedPicklistOptions({ ...field }.options)).toBe(true);
});
});
45 changes: 45 additions & 0 deletions packages/metadata-core/src/resolved-picklist-options.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* [#26886] The mark on an `options` array the runtime RESOLVED from a shared
* picklist, as opposed to one an author wrote on the field.
*
* A field authors `picklist: '<name>'` and no `options`; every object read
* serves it with the list's options written beside `picklist` (the registry's
* picklist resolution in `@objectstack/objectql`). The load-time diagnostics in
* `@objectstack/metadata-protocol` judge what the author wrote, so the field
* schema's rule refusing `picklist` and `options` together must not read the
* copy the runtime wrote. The resolution marks the array it derived and the
* diagnostics leave only a marked array out.
*
* Here because both sides need the one spelling: the diagnostics live in
* `@objectstack/metadata-protocol`, which `@objectstack/objectql` depends on,
* and the resolution sits in the lean `@objectstack/objectql/core` closure,
* which must not import `@objectstack/metadata-protocol` (ADR-0076 D2). Both
* already declare this package.
*
* The mark is a non-enumerable property, so no response, stored row or JSON
* key ever carries it, and it is keyed with `Symbol.for` so every loaded copy
* of a package reads the one mark. A copy of the array (`.map`, a spread, a
* JSON round trip) does not carry it: whoever reads the copy sees authored
* options, so the failure direction is a reported diagnostic, never a missed one.
*/

const RESOLVED_PICKLIST_OPTIONS = Symbol.for('@objectstack/metadata-core:resolved-picklist-options');

/**
* Mark an `options` array as resolved from a shared picklist rather than
* authored on the field. Returns the same array.
*/
export function markResolvedPicklistOptions<T extends object>(options: T): T {
if (!Object.prototype.hasOwnProperty.call(options, RESOLVED_PICKLIST_OPTIONS)) {
Object.defineProperty(options, RESOLVED_PICKLIST_OPTIONS, { value: true });
}
return options;
}

/** `true` when `options` is an array {@link markResolvedPicklistOptions} marked. */
export function isResolvedPicklistOptions(options: unknown): boolean {
return Array.isArray(options)
&& (options as unknown as Record<symbol, unknown>)[RESOLVED_PICKLIST_OPTIONS] === true;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* #26886 — load-time diagnostics judge a picklist-bound field as its author
* wrote it, never with the `options` the runtime resolved onto it.
*
* Every object read serves a field that authors `picklist: '<name>'` with the
* list's options written beside it, and the field schema refuses `picklist`
* and `options` together. Judging the served copy flagged every shared-picklist
* field of every app. The resolution marks the array it derived
* (`markResolvedPicklistOptions`), and only a marked array is left out of the
* judgement:
*
* - a field naming only a picklist, served with resolved options: clean;
* - a field that AUTHORS both keys (an unmarked array): still flagged, with
* the rule's own message, at `fields.NAME.options`;
* - a field with inline `options` only: clean, before and after.
*
* The mark never reaches a response or a memo key: a marked document and an
* unmarked one with the same JSON are two verdicts.
*/
import { describe, expect, it } from 'vitest';
import { markResolvedPicklistOptions } from '@objectstack/metadata-core';
import { computeMetadataDiagnostics, decorateMetadataItem } from './metadata-diagnostics.js';

const BOTH_KEYS = '`picklist` and `options` cannot both be declared';

const resolved = () => markResolvedPicklistOptions([{ label: 'Tech', value: 'tech' }]);
const authored = () => [{ label: 'Tech', value: 'tech' }];

function objectWith(fields: Record<string, unknown>) {
return { name: 'pk_probe', label: 'Probe', fields: { name: { name: 'name', type: 'text', label: 'Name' }, ...fields } };
}

const industry = (options: unknown[]) =>
({ name: 'industry', type: 'select', label: 'Industry', picklist: 'industry', options });

function bothKeysPaths(diagnostics: ReturnType<typeof computeMetadataDiagnostics>): string[] {
return (diagnostics?.errors ?? []).filter((e) => e.message.startsWith(BOTH_KEYS)).map((e) => e.path);
}

describe('diagnostics — options resolved from a shared picklist', () => {
it('a field naming only a picklist, served with its resolved options, is clean', () => {
expect(computeMetadataDiagnostics('object', objectWith({ industry: industry(resolved()) }))).toEqual({ valid: true });
});

it('a field that authors `options` beside `picklist` is still flagged, at its options path', () => {
const diagnostics = computeMetadataDiagnostics('object', objectWith({ industry: industry(authored()) }));
expect(diagnostics?.valid).toBe(false);
expect(bothKeysPaths(diagnostics)).toEqual(['fields.industry.options']);
});

it('a field with inline `options` only is clean', () => {
const inline = { name: 'stage', type: 'select', label: 'Stage', options: [{ label: 'Open', value: 'open' }] };
expect(computeMetadataDiagnostics('object', objectWith({ stage: inline }))).toEqual({ valid: true });
});

it('a marked array on a field with no `picklist` is judged as written', () => {
// A one-character option value is refused; left out, nothing would be.
const stray = { name: 'stage', type: 'select', label: 'Stage', options: markResolvedPicklistOptions([{ label: 'X', value: 'x' }]) };
const diagnostics = computeMetadataDiagnostics('object', objectWith({ stage: stray }));
expect(diagnostics?.errors?.map((e) => e.path)).toEqual(['fields.stage.options.0.value']);
});

it('a marked and an unmarked document with the same JSON are two verdicts, in either order', () => {
const marked = objectWith({ industry: industry(resolved()) });
const unmarked = objectWith({ industry: industry(authored()) });
expect(JSON.stringify(marked)).toBe(JSON.stringify(unmarked));
expect(bothKeysPaths(computeMetadataDiagnostics('object', unmarked))).toEqual(['fields.industry.options']);
expect(computeMetadataDiagnostics('object', marked)).toEqual({ valid: true });
expect(bothKeysPaths(computeMetadataDiagnostics('object', unmarked))).toEqual(['fields.industry.options']);
});

it('the served item keeps both keys and carries no trace of the mark', () => {
const served = decorateMetadataItem('object', objectWith({ industry: industry(resolved()) })) as any;
expect(served._diagnostics).toEqual({ valid: true });
expect(served.fields.industry).toMatchObject({ picklist: 'industry', options: [{ label: 'Tech', value: 'tech' }] });
expect(JSON.parse(JSON.stringify(served.fields.industry))).toEqual(industry(authored()));
expect(Object.keys(served.fields.industry.options)).toEqual(['0']);
});
});
53 changes: 52 additions & 1 deletion packages/metadata-protocol/src/metadata-diagnostics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ import { redactMetadataItem } from './metadata-redaction.js';
// runtime gate's own answer, so the publish verdict below is re-spelled
// against the same prefix the gate wrote it with.
import { stackKeyForType } from '@objectstack/lint/runtime';
// [#26886] The resolution's mark on options it derived from a shared picklist.
import { isResolvedPicklistOptions } from '@objectstack/metadata-core';
import type { RuntimeAuthoringIssue } from '@objectstack/spec/api';

/**
Expand Down Expand Up @@ -175,6 +177,52 @@ function copyDiagnostics(verdict: MetadataDiagnostics): MetadataDiagnostics {
return copy;
}

// ---------------------------------------------------------------------------
// [#26886] Options the runtime resolved from a shared picklist
// ---------------------------------------------------------------------------
//
// A field authors `picklist: '<name>'` and no `options`; every object read
// serves it with the list's options written beside `picklist` (the registry's
// picklist resolution, `PicklistServedFieldSchema`). Diagnostics judge what the
// author wrote, so the field schema's rule that refuses `picklist` and
// `options` together must not read the copy the runtime wrote: it flagged
// every picklist-bound field of every app.
//
// The resolution therefore MARKS the options array it derived
// (`markResolvedPicklistOptions`, `@objectstack/metadata-core`), and only a
// marked array is left out here. A field that authored its own `options`
// beside `picklist` arrives unmarked and is still flagged, with the same
// message and severity. The mark is a non-enumerable property, so it is no
// part of the memo key either: the key is the content of the candidate the
// mark has already shaped.

function withoutResolvedOptions(field: unknown): unknown {
if (!field || typeof field !== 'object') return field;
const { picklist, options } = field as { picklist?: unknown; options?: unknown };
if (typeof picklist !== 'string' || !isResolvedPicklistOptions(options)) return field;
const { options: _resolved, ...authored } = field as Record<string, unknown>;
return authored;
}

/**
* A document with the resolved `options` left out of every picklist-bound
* field of its name-keyed `fields` (the only spelling the object schema
* accepts). By reference when no field carries resolved options.
*/
function withoutResolvedPicklistOptions(document: unknown): unknown {
if (!document || typeof document !== 'object') return document;
const fields = (document as { fields?: unknown }).fields;
if (!fields || typeof fields !== 'object' || Array.isArray(fields)) return document;
let next: Record<string, unknown> | undefined;
for (const [name, field] of Object.entries(fields as Record<string, unknown>)) {
const judged = withoutResolvedOptions(field);
if (judged === field) continue;
next ??= { ...(fields as Record<string, unknown>) };
next[name] = judged;
}
return next ? { ...(document as object), fields: next } : document;
}

/**
* Compute spec diagnostics for a single metadata document.
*
Expand Down Expand Up @@ -224,7 +272,10 @@ export function computeMetadataDiagnostics(
// says. Same class as #6810 (`indexed`), different remedy — that key did
// not belong on the served body at all and left at its injection site,
// whereas this one is read by the UI and belongs on the response.
const candidate = stripReadDecorations(item);
// [#26886] …and judge each picklist-bound field as its author wrote it:
// the `options` the runtime resolved onto it are not part of that shape
// ({@link withoutResolvedPicklistOptions}).
const candidate = withoutResolvedPicklistOptions(stripReadDecorations(item));

// [#25783] A document parsed before against this same schema is answered
// from the memo — see "The verdict memo" above.
Expand Down
15 changes: 14 additions & 1 deletion packages/objectql/src/picklist-resolution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@
* validator refuses every value of the field instead of accepting any.
*/

// The mark that tells the load-time diagnostics these options were resolved
// here, not authored: they judge what the author wrote, and the field schema
// refuses `picklist` and `options` together.
import { isResolvedPicklistOptions, markResolvedPicklistOptions } from '@objectstack/metadata-core';

/** One option, as `SelectOptionSchema` declares it (only `value` is read here). */
export interface PicklistOptionLike {
value: string | number | boolean;
Expand Down Expand Up @@ -133,7 +138,15 @@ function resolveField(field: unknown, resolve: (picklist: string) => readonly Pi
const picklist = (field as { picklist?: unknown }).picklist;
if (typeof picklist !== 'string' || picklist.length === 0) return field;
const options = resolve(picklist);
if (options) return { ...(field as object), options: options.map((o) => ({ ...o })) };
if (options) {
const resolved = options.map((o) => ({ ...o }));
// Marked as derived unless the body brought `options` of its own: a field
// that AUTHORS both keys is still told so. A body that is itself a served
// copy (marked) stays marked.
const carried = (field as { options?: unknown }).options;
if (carried === undefined || isResolvedPicklistOptions(carried)) markResolvedPicklistOptions(resolved);
return { ...(field as object), options: resolved };
}
// Unresolved: drop any options the body carried, so a stale served copy
// cannot stand in for the list. The validator refuses every value of a
// picklist-bound field that has none.
Expand Down
49 changes: 49 additions & 0 deletions packages/objectql/src/protocol-picklist-served-roundtrip.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,3 +184,52 @@ describe('picklist — the protocol object read and the authoring door', () => {
expect(err.message).toContain('`picklist` and `options` cannot both be declared');
});
});

// [#26886] Every door that shows a load-time diagnostic judges a
// picklist-bound field as its author wrote it: the item's `_diagnostics`, the
// list's, and `GET /meta/diagnostics`. The read itself keeps serving both keys.
describe('picklist — the load-time diagnostics judge the authored field, not the served copy', () => {
const BOTH_KEYS = '`picklist` and `options` cannot both be declared';
let protocol: ObjectStackProtocolImplementation;

beforeEach(async () => {
const engine = new ObjectQL();
engine.registerDriver(makeStubDriver().driver, true);
await engine.init();
engine.registry.registerObject(sysMetadataObject);
for (const o of [SysMetadataHistoryObject, SysMetadataAuditObject, SysMetadataCommitObject]) engine.registry.registerObject(o as any);
engine.registerApp({
id: 'com.test.lists', name: 'lists',
picklists: [{ name: 'industry', label: 'Industry', options: [{ label: 'Tech', value: 'tech' }] }],
objects: [{
name: 'pd_obj', label: 'Probe', sharingModel: 'public_read_write',
fields: {
name: { name: 'name', type: 'text', label: 'Name' },
industry: { name: 'industry', type: 'select', label: 'Industry', picklist: 'industry' },
stage: { name: 'stage', type: 'select', label: 'Stage', options: [{ label: 'Open', value: 'open' }] },
both: { name: 'both', type: 'select', label: 'Both', picklist: 'industry', options: [{ label: 'Own', value: 'own' }] },
},
}],
} as any);
protocol = new ObjectStackProtocolImplementation(engine);
});

const bothKeysPaths = (diagnostics: any): string[] =>
(diagnostics?.errors ?? []).filter((e: any) => String(e.message).startsWith(BOTH_KEYS)).map((e: any) => e.path);

it('the item read: only the field that authored both keys is flagged, and both keys are still served', async () => {
const served: any = await protocol.getMetaItem({ type: 'object', name: 'pd_obj' } as any);
expect(served.item.fields.industry).toMatchObject({ picklist: 'industry', options: [{ label: 'Tech', value: 'tech' }] });
expect(bothKeysPaths(served.item._diagnostics)).toEqual(['fields.both.options']);
expect(served.item._diagnostics.errors.map((e: any) => e.path)).toEqual(['fields.both.options']);
});

it('the list read and `GET /meta/diagnostics` agree with the item read', async () => {
const listed: any = await protocol.getMetaItems({ type: 'object' } as any);
const item = (listed.items ?? listed).find((i: any) => i.name === 'pd_obj');
expect(bothKeysPaths(item._diagnostics)).toEqual(['fields.both.options']);
const sweep: any = await (protocol as any).getMetaDiagnostics({ type: 'object' });
const entry = sweep.entries.find((e: any) => e.name === 'pd_obj');
expect(entry.diagnostics.errors.map((e: any) => e.path)).toEqual(['fields.both.options']);
});
});
Loading