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
2 changes: 1 addition & 1 deletion .changeset/29366-field-patches-key-grammar.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
---
'@objectstack/spec': minor
---
Expand All @@ -19,6 +19,6 @@
- **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`.
15 changes: 15 additions & 0 deletions .changeset/29554-stored-object-extension-rows.md
Original file line number Diff line number Diff line change
@@ -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.
58 changes: 57 additions & 1 deletion packages/spec/src/conversions/refused-shapes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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',
]);
});

Expand Down Expand Up @@ -853,3 +854,58 @@ describe('#29212 — a stored dashboard that declares one global filter key twic
}
});
});

// ── #29554 (the #29366 refusal) ───────────────────────────────────────────

const patchRow = (fieldPatches: Record<string, unknown>) => ({ 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([]);
});
});
53 changes: 52 additions & 1 deletion packages/spec/src/conversions/refused-shapes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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';
Expand Down Expand Up @@ -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.
Expand All @@ -597,6 +647,7 @@ export const REFUSED_STORED_SHAPES: readonly RefusedStoredShape[] = [
visibilityHasArgument,
objectDuplicateActionName,
dashboardDuplicateGlobalFilterName,
objectExtensionFieldPatchKeyGrammar,
];

/**
Expand Down
50 changes: 50 additions & 0 deletions packages/spec/src/conversions/stored.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 16 additions & 1 deletion packages/spec/src/conversions/stored.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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<Record<string, string>> = {
translation: 'translations',
// The legacy plural row spelling, as the protocol's singular/plural read
// fallback still finds it.
translations: 'translations',
object_extension: 'objectExtensions',
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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.',
};
Loading