diff --git a/.changeset/29366-field-patches-key-grammar.md b/.changeset/29366-field-patches-key-grammar.md index 101bafe4aed..6942c3fcff3 100644 --- a/.changeset/29366-field-patches-key-grammar.md +++ b/.changeset/29366-field-patches-key-grammar.md @@ -19,6 +19,6 @@ Skills-docs: updated here — `content/docs/data-modeling/object-extensions.mdx` - **Before.** `fieldPatches: { Industry: { label: 'Sector' } }` parsed in `defineStack`, was stored by the save door and booted: the registry fold named the patch once in the log and applied nothing, and `os validate` refused it only when the target object was in the same stack. - **Now.** One `invalid_key` issue at `fieldPatches.Industry`, naming the grammar and an example. The key schema is the one `ObjectSchema.fields` reads, hoisted to one constant, so the two cannot drift. - **Unchanged.** A snake_case key parses as before, and a `__c` key is still refused by the `__c` rule with its own prescription. -- **Who sees it.** No extension in this repository's examples, skills, docs or templates writes such a key. A stored `object_extension` row that carries one no longer parses, so the sync names the row (`INVALID_METADATA`) and registers none of it, its `fields` included; its next save is refused until the key is renamed. `os migrate meta --stored` does not list such a row beforehand: the stored pass has no stack collection for an `object_extension` row and hands it back untouched, so check each stored extension's `fieldPatches` keys against the grammar before upgrading. +- **Who sees it.** No extension in this repository's examples, skills, docs or templates writes such a key. A stored `object_extension` row that carries one no longer parses, so the sync names the row (`INVALID_METADATA`) and registers none of it, its `fields` included; its next save is refused until the key is renamed. `os migrate meta --stored` lists such a row beforehand, one TODO for each refused key at `fieldPatches.KEY`, and leaves it as stored for its owner to rename. **FROM → TO.** `fieldPatches: { Industry: { label: 'Sector' } }` → `fieldPatches: { industry: { label: 'Sector' } }`: key each patch by the name of the field the extended object declares (the key of its `fields` entry), never by its label. The upgrade guide entry is `object-extension-field-patch-key-grammar-refused`. diff --git a/.changeset/29554-stored-object-extension-rows.md b/.changeset/29554-stored-object-extension-rows.md new file mode 100644 index 00000000000..b47ed851be9 --- /dev/null +++ b/.changeset/29554-stored-object-extension-rows.md @@ -0,0 +1,15 @@ +--- +'@objectstack/spec': patch +--- + +fix(spec): the stored pass reads a stored `object_extension` row as an `objectExtensions` entry, so conversions replay over it and `os migrate meta --stored` lists a `fieldPatches` key no field name can be + +Clause-②: no (no accept set moves; the stored pass and the `--stored` listing widen) + +Metadata-protocol: none + +`applyConversionsToStoredItem` wraps a stored row in the stack collection its type maps to, and returned a row untouched when its type mapped to none. `object_extension` (ADR-0146 D10) mapped to none, so no conversion that walks `objectExtensions` ever ran over a stored extension row at a rehydration seam, and no refused-shape detector could list one: `os migrate meta --stored` read every stored extension as already on protocol. + +- **Now.** A stored `object_extension` row is read as one `objectExtensions` entry, beside `translation` in the stored-only map; the manifest-collection maps still carry no `objectExtensions`, because a stack's extensions merge into their targets rather than registering as named items. A stored extension with a `time` default written with a `Z` suffix, for example, is now served without it, with the conversion's notice, as an object row already was. +- **Listed.** `os migrate meta --stored` lists each `fieldPatches` key the extension's own parse refuses as no field name (`Industry`, `Annual Revenue`, `annual-revenue`): one TODO at `fieldPatches.KEY` under the row, naming the extended object, leaving the row as stored. Rename each listed key to the field it meant before upgrading. The upgrade guide entry `object-extension-field-patch-key-grammar-refused` says so instead of naming a manual check. +- **Unchanged.** A canonical extension row passes through by reference, every other row type converts exactly as before, and no schema accepts or refuses anything it did not. diff --git a/packages/spec/src/conversions/refused-shapes.test.ts b/packages/spec/src/conversions/refused-shapes.test.ts index e7aede152db..267f0791c27 100644 --- a/packages/spec/src/conversions/refused-shapes.test.ts +++ b/packages/spec/src/conversions/refused-shapes.test.ts @@ -4,7 +4,7 @@ import { describe, expect, it } from 'vitest'; import { FLOW_NODE_SIBLING_EXPRESSION_BLOCKS } from '../automation/flow-node-expression-paths.js'; import { flowNodeValueTemplateRefusals } from '../automation/flow-value-slot-template.js'; -import { ObjectSchema } from '../data/object.zod.js'; +import { ObjectExtensionSchema, ObjectSchema } from '../data/object.zod.js'; import { MIGRATIONS_BY_MAJOR } from '../migrations/registry.js'; import { ComponentPropsMap } from '../ui/component.zod.js'; import { DashboardSchema } from '../ui/dashboard.zod.js'; @@ -73,6 +73,7 @@ describe('the refused-shape list — registered against the migration chain', () 'visibility-predicate-has-argument-refused', 'object-duplicate-action-name-refused', 'ui-dashboard-global-filter-duplicate-name-refused', + 'object-extension-field-patch-key-grammar-refused', ]); }); @@ -853,3 +854,58 @@ describe('#29212 — a stored dashboard that declares one global filter key twic } }); }); + +// ── #29554 (the #29366 refusal) ─────────────────────────────────────────── + +const patchRow = (fieldPatches: Record) => ({ extend: 'crm_account', fieldPatches }); +/** The `invalid_key` issues the extension parse locates at `fieldPatches.KEY`, as `[path, message]`. */ +const patchKeyIssues = (row: unknown) => { + const parsed = ObjectExtensionSchema.safeParse(row); + return parsed.success + ? [] + : parsed.error.issues + .filter((i) => i.code === 'invalid_key' && i.path[0] === 'fieldPatches') + .map((i) => [i.path.join('.'), i.message]); +}; + +describe('#29554 — a stored object extension whose fieldPatches key cannot be a field name is listed by extension', () => { + it('one TODO per refused key, at fieldPatches.KEY, naming the target; the row is not rewritten', () => { + const row = patchRow({ + Industry: { label: 'Sector' }, + 'annual-revenue': { label: 'Revenue' }, + region: { label: 'Territory' }, + }); + const { item, todos } = storedTodos('object_extension', row); + expect(item).toEqual(row); + expect(todos.map((t) => [t.code, t.conversionId, t.path])).toEqual([ + [CONVERSION_TODO_CODE, 'object-extension-field-patch-key-grammar-refused', 'objectExtensions[0].fieldPatches.Industry'], + [CONVERSION_TODO_CODE, 'object-extension-field-patch-key-grammar-refused', 'objectExtensions[0].fieldPatches.annual-revenue'], + ]); + const todo = todos[0]!; + expect(todo.reason).toMatch(/^The extension of `crm_account` keys a field patch by `Industry`\. No lossless rewrite:/); + expect(todo.reason).not.toMatch(/#\d{3,}/); + expect(JSON.parse(todo.from)).toEqual({ Industry: { label: 'Sector' } }); + }); + + it('agrees with the refusing door: each listed key is one the extension parse refuses as a key, and no other', () => { + const row = patchRow({ Industry: { label: 'Sector' }, 'Annual Revenue': { label: 'Revenue' }, region: { label: 'R' } }); + const { todos } = storedTodos('object_extension', row); + const issues = patchKeyIssues(row); + expect(issues.map(([path]) => path)).toEqual(['fieldPatches.Industry', 'fieldPatches.Annual Revenue']); + expect(todos.map((t) => t.path)).toEqual(issues.map(([path]) => `objectExtensions[0].${path}`)); + // The door's own message closes each reason, so the listing says what the parse says. + todos.forEach((t, i) => expect(t.reason.endsWith(issues[i]![1]!)).toBe(true)); + }); + + it('snake_case keys are not listed, and the parse accepts them', () => { + const row = patchRow({ industry: { label: 'Sector' }, annual_revenue: { label: 'Revenue' } }); + expect(storedTodos('object_extension', row).todos).toEqual([]); + expect(ObjectExtensionSchema.safeParse(row).success).toBe(true); + }); + + it('a key on another arm — a customer extension field spelled `__c` — is not listed by this detector', () => { + const row = patchRow({ region__c: { label: 'Territory' } }); + expect(storedTodos('object_extension', row).todos).toEqual([]); + expect(patchKeyIssues(row)).toEqual([]); + }); +}); diff --git a/packages/spec/src/conversions/refused-shapes.ts b/packages/spec/src/conversions/refused-shapes.ts index 56057f0aa78..d205cc7d3b7 100644 --- a/packages/spec/src/conversions/refused-shapes.ts +++ b/packages/spec/src/conversions/refused-shapes.ts @@ -45,7 +45,9 @@ * ({@link externalBindingDatasourceRefusal}), and the one-name-per-action * judge of the object refinement ({@link duplicateActionNames}), and the * one-key-per-global-filter judge of the dashboard refinement - * ({@link duplicateGlobalFilterNames}). A site is listed here exactly + * ({@link duplicateGlobalFilterNames}), and the `fieldPatches` key + * grammar of `ObjectExtensionSchema`'s own parse + * ({@link refusedFieldPatchKeys}). A site is listed here exactly * when that door refuses it. One door lives outside this package: the * page/view `visibleWhen` rule in `@objectstack/lint` judges a `has()` * argument on the engine's AST, which this package cannot parse, so its @@ -69,6 +71,7 @@ import { } from '../data/controlled-by-parent-master.js'; import { duplicateActionNames } from '../data/duplicate-action-names.js'; import { externalBindingDatasourceRefusal } from '../data/external-binding-datasource.js'; +import { ObjectExtensionSchema } from '../data/object.zod.js'; import { ComponentPropsMap } from '../ui/component.zod.js'; import { duplicateGlobalFilterNames } from '../ui/duplicate-global-filter-names.js'; import { ViewDataSchema } from '../ui/view.zod.js'; @@ -584,6 +587,53 @@ const dashboardDuplicateGlobalFilterName: RefusedStoredShape = { }, }; +// ── #29554: an object extension whose `fieldPatches` key cannot be a field name (the #29366 refusal) ── + +/** + * The `fieldPatches` keys `ObjectExtensionSchema` refuses as no field name: + * each `invalid_key` issue the extension's own parse locates at + * `fieldPatches.KEY`, with that issue's message. The parse is the door's own, + * the one the save door and the sync that registers stored extensions run, so + * a key is listed here exactly when that parse refuses it as a key. + */ +function refusedFieldPatchKeys(extension: Dict): Array<{ key: string; message: string }> { + if (!isDict(extension.fieldPatches)) return []; + const parsed = ObjectExtensionSchema.safeParse(extension); + if (parsed.success) return []; + const refused: Array<{ key: string; message: string }> = []; + for (const issue of parsed.error.issues) { + if (issue.code !== 'invalid_key' || issue.path.length !== 2 || issue.path[0] !== 'fieldPatches') continue; + const key = issue.path[1]; + if (typeof key === 'string') refused.push({ key, message: issue.message }); + } + return refused; +} + +const objectExtensionFieldPatchKeyGrammar: RefusedStoredShape = { + id: 'object-extension-field-patch-key-grammar-refused', + toMajor: 18, + surface: 'objectExtensions[].fieldPatches — a key outside the snake_case grammar of an object\'s `fields` keys', + detect(stack, report) { + mapCollection(stack, 'objectExtensions', (extension, path) => { + const target = typeof extension.extend === 'string' ? `\`${extension.extend}\`` : 'its target'; + for (const { key, message } of refusedFieldPatchKeys(extension)) { + report({ + path: `${path}.fieldPatches.${key}`, + from: JSON.stringify({ [key]: (extension.fieldPatches as Dict)[key] }), + reason: `The extension of ${target} keys a field patch by \`${key}\`. ` + + 'No lossless rewrite: re-spelling the key (lower-casing it, turning `-` or a space into `_`) can land ' + + 'on a different field or on none, so which field the patch meant is the author\'s to name. Left as ' + + 'stored, the row no longer parses: the sync that registers stored extensions names it ' + + '(INVALID_METADATA) and registers none of it, its `fields` included, and its next save is refused ' + + 'until the key is renamed to the field it meant. ' + + message, + }); + } + return extension; + }); + }, +}; + /** * Every refused stored shape, in report order. A new entry is a reviewed edit: * `refused-shapes.test.ts` pins each one. @@ -597,6 +647,7 @@ export const REFUSED_STORED_SHAPES: readonly RefusedStoredShape[] = [ visibilityHasArgument, objectDuplicateActionName, dashboardDuplicateGlobalFilterName, + objectExtensionFieldPatchKeyGrammar, ]; /** diff --git a/packages/spec/src/conversions/stored.test.ts b/packages/spec/src/conversions/stored.test.ts index b0ad188b6b7..34bfa73e83c 100644 --- a/packages/spec/src/conversions/stored.test.ts +++ b/packages/spec/src/conversions/stored.test.ts @@ -181,6 +181,56 @@ describe('applyConversionsToStoredItem (stored sys_metadata rows)', () => { }); }); + // #29554 — a stored `object_extension` row is one `objectExtensions` entry, + // and until this the stored pass had no collection to wrap it in: every + // conversion that reaches an extension, and every refused-shape detector, + // skipped it, so the row read as already on protocol. + describe('stored object_extension rows (wrapped as objectExtensions)', () => { + const storedExtension = () => ({ + extend: 'crm_account', + fields: { opens_at: { type: 'time', defaultValue: '12:00Z' } }, + fieldPatches: { industry: { label: 'Sector' } }, + }); + + it('replays a conversion that walks objectExtensions over a stored extension row, loudly', () => { + const notices: ConversionNotice[] = []; + const row = storedExtension(); + const out = applyConversionsToStoredItem('object_extension', row, { onNotice: (n) => notices.push(n) }); + expect(out).toEqual({ + extend: 'crm_account', + fields: { opens_at: { type: 'time', defaultValue: '12:00' } }, + fieldPatches: { industry: { label: 'Sector' } }, + }); + expect(notices.map((n) => [n.conversionId, n.path])).toEqual([ + ['time-default-utc-suffix-dropped', 'objectExtensions[0].fields.opens_at.defaultValue'], + ]); + expect(row).toEqual(storedExtension()); + }); + + it('CONTROL — a canonical extension row passes through by reference', () => { + const row = { extend: 'crm_account', fields: { opens_at: { type: 'time', defaultValue: '12:00' } } }; + expect(applyConversionsToStoredItem('object_extension', row)).toBe(row); + }); + + it('CONTROL — the same body under an object row converts exactly as before', () => { + const notices: ConversionNotice[] = []; + const row = { name: 'shift', fields: { opens_at: { type: 'time', defaultValue: '12:00Z' } } }; + const out = applyConversionsToStoredItem('object', row, { onNotice: (n) => notices.push(n) }); + expect(out).toEqual({ name: 'shift', fields: { opens_at: { type: 'time', defaultValue: '12:00' } } }); + expect(notices.map((n) => [n.conversionId, n.path])).toEqual([ + ['time-default-utc-suffix-dropped', 'objects[0].fields.opens_at.defaultValue'], + ]); + }); + + // No plural row spelling is mapped: the read fallback finds none for this + // type, and the stored-metadata pass skips a row stored under one before + // reading its body. So neither spelling manufactures a collection. + it.each(['object_extensions', 'objectExtensions'])('CONTROL — a `%s` row type is not mapped and passes through by reference', (type) => { + const row = storedExtension(); + expect(applyConversionsToStoredItem(type, row)).toBe(row); + }); + }); + // [#23543 F5-http] A stored flow row written with a retired callout name // still loads, as `http`: the rename left the authoring funnel, not this // seam. Until 18 a caller's live-name set naming the old token made it a diff --git a/packages/spec/src/conversions/stored.ts b/packages/spec/src/conversions/stored.ts index 6da63ab77ce..e26e3a1e192 100644 --- a/packages/spec/src/conversions/stored.ts +++ b/packages/spec/src/conversions/stored.ts @@ -46,7 +46,7 @@ import { PLURAL_TO_SINGULAR, SINGULAR_TO_PLURAL } from '../shared/metadata-colle * Stored row types whose conversions walk a stack collection the * MANIFEST-COLLECTION maps deliberately do not carry (#19620). * - * `translation` is the one: `PLURAL_TO_SINGULAR` has no `translations` + * `translation` is the first: `PLURAL_TO_SINGULAR` has no `translations` * spelling because a stack's `translations` entries are locale-keyed BUNDLES, * not named metadata items (`manifest-collection-spelling.ts` says why, and * `check:stack-collection-maps` holds that map to the stack schema). But the @@ -59,12 +59,27 @@ import { PLURAL_TO_SINGULAR, SINGULAR_TO_PLURAL } from '../shared/metadata-colle * reaching the runtime from rows stored before. Kept HERE rather than added to * the shared maps, which would advertise `translations` as a named-item stack * collection to every other reader of them. + * + * `object_extension` is the second (ADR-0146 D10), for the same reason. A + * stored extension row is exactly one `objectExtensions` entry (a body that + * carries `extend` and no `name`), and every conversion that reaches an + * extension walks that collection. But a stack's `objectExtensions` are not + * named items either: the registry merges each one into its target, and + * `check:stack-collection-maps` records the manifest map's omission of the key + * as deliberate. Without this entry the stored pass returned every stored + * extension row untouched, so no conversion and no refused-shape detector ever + * ran over one, and `os migrate meta --stored` read each such row as already + * on protocol. There is no plural row spelling to add: the protocol's + * singular/plural read fallback finds none for `object_extension`, and the + * stored-metadata pass reports a row stored under any other spelling of it as + * `skipped` before its body is read. */ const STORED_ONLY_COLLECTIONS: Readonly> = { translation: 'translations', // The legacy plural row spelling, as the protocol's singular/plural read // fallback still finds it. translations: 'translations', + object_extension: 'objectExtensions', }; /** diff --git a/packages/spec/src/migrations/entries/semantic/18.object-extension-field-patch-key-grammar-refused.ts b/packages/spec/src/migrations/entries/semantic/18.object-extension-field-patch-key-grammar-refused.ts index 7199aa8b717..0af5ba36bdd 100644 --- a/packages/spec/src/migrations/entries/semantic/18.object-extension-field-patch-key-grammar-refused.ts +++ b/packages/spec/src/migrations/entries/semantic/18.object-extension-field-patch-key-grammar-refused.ts @@ -8,12 +8,11 @@ import type { SemanticMigration } from '../../types.js'; // the spelling (lower-casing, `-` to `_`) can land on a different field or on // none, so which field is the author's to name. // -// No `REFUSED_STORED_SHAPES` detector lists a stored row in this shape, because -// none could be reached: `applyConversionsToStoredItem` maps a row type onto a -// stack collection, and `object_extension` has none (it is in neither -// `SINGULAR_TO_PLURAL` nor the stored-only map), so the stored pass hands the -// row back untouched and runs no conversion or detector over it. The criteria -// below say so, and name the query that finds such a row instead. +// A stored row in this shape is listed, not rewritten: the stored pass wraps an +// `object_extension` row as an `objectExtensions` entry (the stored-only map in +// `conversions/stored.ts`), and the `REFUSED_STORED_SHAPES` detector of this id +// (`conversions/refused-shapes.ts`) lists each key the extension's own parse +// refuses as a key, under its row, through `os migrate meta --stored`. export const entry: SemanticMigration = { id: 'object-extension-field-patch-key-grammar-refused', surface: @@ -37,8 +36,7 @@ export const entry: SemanticMigration = { + 'at `fieldPatches.KEY` that names the grammar and an example. A stored `object_extension` row whose ' + 'body still carries such a key no longer parses, so the sync that registers stored extensions names ' + 'the row (`INVALID_METADATA`) and registers none of it, its `fields` included, and its next save is ' - + 'refused until the key is renamed to the field it meant. `os migrate meta --stored` does not list such ' - + 'a row: the stored pass maps no `object_extension` row onto a stack collection, so it reads the row as ' - + 'already on protocol. Find one before upgrading by reading each stored `object_extension` row\'s ' - + '`fieldPatches` keys against the snake_case grammar of `fields` keys.', + + 'refused until the key is renamed to the field it meant. `os migrate meta --stored` lists such a row ' + + 'before the upgrade, one TODO for each refused key at `fieldPatches.KEY` naming the extended object, ' + + 'and leaves it as stored: rename each listed key to the field it meant and save the extension.', };