diff --git a/.changeset/30471-field-name-indexes-extension-fields.md b/.changeset/30471-field-name-indexes-extension-fields.md new file mode 100644 index 00000000000..bc9c9c5a7cf --- /dev/null +++ b/.changeset/30471-field-name-indexes-extension-fields.md @@ -0,0 +1,29 @@ +--- +'@objectstack/lint': minor +--- + +fix(lint)!: the field-name indexes judge an object as the registry serves it, so a read, write or sort naming a field an `objectExtensions` entry adds is no longer refused as unknown, and the readonly, sortability and translation rules judge that field by its served definition (#30471) + +Clause-②: yes (narrowing) — `os validate` / `os build` / `os lint` refuse three shapes they accepted before: a hook body's literal `ctx.api.object(NAME).update(…)` / `.updateById(…)` / `.insert(…)` writing a field an `objectExtensions` entry adds and declares `readonly: true` moves from a `hook-body-write-unknown-field` warning to a `hook-api-update-readonly-field` error; and a translation key naming a field or validation rule that only a replaced earlier entry of the same package adds, or only a top-level `objectExtensions` beside `packages[]` adds, is now `translation-target-unknown` at `error`, where it resolved, and with that top-level-beside shape no longer read, a translation subtree `objects.X` keyed to an object the stack does not define and only such an entry extends loses the extended-object skip (rung 2b) and is `translation-target-unknown` at `error` too; and a field an `objectExtensions` entry RE-DECLARES on its owner's object is judged on the extension's definition, which replaces the owner's whole in the fold as in the registry's `mergeObjectDefinitions` (`{ ...base.fields, ...extension.fields }`), so an owner `amount: number` re-declared as `formula`, `readonly: true` or `text` now gets that definition's `sort-field-unsortable`, readonly-write or expression type-mismatch verdict, each at `error` (ADR-0146 D10 already refuses that shape on the extension; no CLI door does). The widening half: a formula, validation, flow condition, hook condition, sharing-rule condition, flow-node, action-body or hook-body write, react page field prop, translation key, page sort or list-view sort naming a field an `objectExtensions` entry of the same artifact adds is no longer refused as an unknown field. +Metadata-protocol: narrowed +Skills-docs: updated here — content/docs/automation/hook-bodies.mdx + + + +**BREAKING** accept-set narrowing in `os validate` / `os build` / `os lint` (`@objectstack/lint`), for multi-package artifacts whose bodies add fields to another package's object through `objectExtensions`. + +- **Before.** The field-name indexes of the expression passes (validation rules, formulas, flow and hook conditions, sharing-rule conditions), the three write rules, a react page's field props, the three readonly write rules and the sort rule read an object's declared `fields` only, so a name an `objectExtensions` entry adds was refused as unknown although the runtime serves the field: `flow-node-write-unknown-field` and `sort-field-unknown` / `page-field-unknown` (page sorts) at `error`, `action-body-write-unknown-field`, `hook-body-write-unknown-field` and `page-field-unknown` (react props) at `warning`, the expression passes' unknown-field error. The translation rule folded extensions through a reader of its own that kept every entry and also read a top-level `objectExtensions` beside `packages[]`. +- **Now.** Every one of them reads the one shared fold in `object-graph.ts` (since #30383 the field-path rules' fold): the declared fields plus each field an `objectExtensions` entry adds, one entry per package per object, `packages[]` alone when the artifact carries it. A name no body declares or adds keeps the same refusal, rule id, severity and message; a misspelling within edit distance 3 of an extension-added name now gets a did-you-mean naming it. A name the fold adds is judged by its served definition, so these findings move (union artifact and per-package leg alike): + - a hook body's `ctx.api` update / updateById / insert of a `readonly: true` field: `hook-body-write-unknown-field` `warning` → `hook-api-update-readonly-field` `error`, the severity a declared readonly field already gets (a non-system hook's `ctx.api` write is stripped at run time and the call returns success); + - a translation key naming a field or rule only a replaced earlier entry of the same package, or only a top-level `objectExtensions` beside `packages[]`, adds: resolved → `translation-target-unknown` `error`, as the registry serves the object; + - a translation subtree `objects.X` keyed to an object the stack does not define and only a top-level `objectExtensions` beside `packages[]` extends: skipped as an extended object (rung 2b) → `translation-target-unknown` `error`; + - an owner's field an `objectExtensions` entry re-declares is judged on the extension's definition, which replaces the owner's whole as the registry's `mergeObjectDefinitions` does (`{ ...base.fields, ...extension.fields }`): an owner `amount: number` re-declared as `formula` and sorted (page or list view) → `sort-field-unsortable` `error`; re-declared `readonly: true` and written by a flow `update_record` or a hook's `ctx.api` update → `flow-update-readonly-field` / `hook-api-update-readonly-field` `error`; re-declared `text` and multiplied in a validation rule → the expression type mismatch at `error`. Each passed before, the owner's definition being the one judged. ADR-0146 D10 refuses this shape on the extension at registration; no CLI door does; + - a flow `update_record` / `create_record` of a `readonly: true` field: `flow-node-write-unknown-field` `error` → `flow-update-readonly-field` `error`; + - a page sort (every slot) or list-view sort by a `formula` field or one declaring `sortable: false`: `page-field-unknown` / `sort-field-unknown` `error` → `sort-field-unsortable` `error`; + - a `readonlyWhen` field written by a flow `update_record` (`error` → `flow-update-readonly-when-field` `warning`), a hook's `ctx.api` update (`warning` → `hook-api-update-readonly-when-field` `warning`) or an action's `ctx.api` update (`warning` → `action-api-update-readonly-when-field` `warning`); + - an expression reading the field: the unknown-field error becomes the served type's verdict (a type mismatch or a missing null guard, at `error` in validation rules and hook conditions, at `warning` in formulas and sharing-rule conditions) or nothing. + Every other read, write or sort naming such a field passes: a hook's `ctx.input` stamp, an action's `ctx.api` write of a `readonly: true` field (an action runs elevated) and a `create_record` of a `readonlyWhen` field land at run time. +- **Not judged.** A field two packages write at the same deciding priority is a served name, but no rule judges its type, nullability, readonly or sortability: which definition wins is registration order. An object that declares no field of its own stays unjudged by the write, readonly and sort rules, extended or not (an introspected schema). A react page's `searchableFields` still reads declared fields only. +- **Not changed.** Which extension entries the fold keeps, the runtime, and `defineStack` (its related-list check stays additive). A stack that names no extension field reads as before. + +**What an author does.** For a hook writing a readonly extension field, declare the hook `runAs: 'system'`, stamp the value in the record's own `beforeInsert` / `beforeUpdate` hook, or drop the field from the payload. For a translation key the fold drops, key it to a field or rule of the entry the registry keeps (the package's last entry for that object, in a `packages[]` body), or delete it. diff --git a/content/docs/automation/hook-bodies.mdx b/content/docs/automation/hook-bodies.mdx index ad3af42ba62..0e58a87d3f9 100644 --- a/content/docs/automation/hook-bodies.mdx +++ b/content/docs/automation/hook-bodies.mdx @@ -217,7 +217,7 @@ Static validation around a hook is asymmetric, and it is worth knowing exactly w - **Checked — read side.** `hook.condition` is validated at build time against the target object's fields by the expression validator (`@objectstack/lint`), including array-valued `hook.object` targets. A condition referencing a nonexistent field fails the lint. - **Checked — capability side.** `body.capabilities` gates which `ctx` APIs the body may call at all; the sandbox throws on an undeclared call. -- **Checked — write side, literal-only.** Since [#4271](https://github.com/objectstack-ai/objectstack/issues/4271), `body.source` is **parsed** (never executed, never type-checked) and the field names it writes are resolved against the target object's declarations. An unknown field raises `hook-body-write-unknown-field` — a **warning** carrying a did-you-mean suggestion, which never blocks a build. Action bodies get the same check on their `ctx.api` writes (`action-body-write-unknown-field`). Both run under `os validate`, `os lint` and `os compile`. The *existence* question is advisory like this because the answer can depend on a package the build cannot see; the separate *writability* question — [writing a `readonly` field through `ctx.api`](#writing-a-readonly-field) — **does** gate, because both halves of that judgement are declared in the stack being checked. +- **Checked — write side, literal-only.** Since [#4271](https://github.com/objectstack-ai/objectstack/issues/4271), `body.source` is **parsed** (never executed, never type-checked) and the field names it writes are resolved against the target object's declarations, which include every field an `objectExtensions` entry of the same artifact adds to it. An unknown field raises `hook-body-write-unknown-field` — a **warning** carrying a did-you-mean suggestion, which never blocks a build. Action bodies get the same check on their `ctx.api` writes (`action-body-write-unknown-field`). Both run under `os validate`, `os lint` and `os compile`. The *existence* question is advisory like this because the answer can depend on a package the build cannot see; the separate *writability* question — [writing a `readonly` field through `ctx.api`](#writing-a-readonly-field) — **does** gate, because both halves of that judgement are declared in the stack being checked. - **Checked — writes to a system column the object has no storage for.** Since [#8663](https://github.com/objectstack-ai/objectstack/issues/8663), a write to an injected system column is no longer exempted on the strength of its NAME alone. The registry injects `owner_id` / `organization_id` / the audit family onto an ADR-0015 [`external` object](/docs/data-modeling/external-datasources) exactly as onto a local one, but the remote database owns that schema and no column exists behind them. Writing one raises `hook-body-write-unprovisioned-anchor` (or `action-body-write-unprovisioned-anchor` / `flow-node-write-unprovisioned-anchor` on the other two surfaces) — a **warning** on all three, including the flow-node rule that otherwise gates, because the claim is about a remote schema the build cannot see. A column you **declare** yourself is untouched: on a federated object a declared `owner_id` maps a remote column you vouch for. Why it matters more than an ordinary typo: an undeclared name is refused upstream by the engine's own write-path validator (`INVALID_FIELD`), whereas the injected anchor is in the registered schema and passes it — so it is the one payload key that reaches the remote database raw, where a SQL remote aborts the **whole statement** with an untyped `no such column` and takes the correctly named fields of the same payload with it. - **Checked — writes that reach nothing at all.** Since [#4345](https://github.com/objectstack-ai/objectstack/issues/4345), an action body assigning to `ctx.record` raises `action-record-write-discarded`, also a warning. This one is **not** a field-resolution question: an action's `ctx.record` is a snapshot the runtime never writes back, so the assignment is discarded whether or not the field is declared — see [Signature conventions](#signature-conventions) below. @@ -309,7 +309,7 @@ There is an asymmetry here that costs data if you learn it the hard way, so lear The dropped case is the dangerous one: nothing fails, the step reports success, and the column is simply always null. Because both halves of that judgement are declared in your own stack, it is checked at author time and **gates the build**: -- `hook-api-update-readonly-field` — **error**. A body's literal `ctx.api.object('…').update()` / `.updateById()` / `.insert()` writes a field the named object declares `readonly: true`. Since [#15394](https://github.com/objectstack-ai/objectstack/issues/15394) the `insert` row of the table above is reported at build time exactly like the `update` row — same id, same severity, a message naming the verb — unless the hook declares `runAs: 'system'`. Only the static shape is judged on an insert: a `readonlyWhen` field has no prior record to lock on and the engine runs no conditional strip on INSERT, so no warning is produced there. +- `hook-api-update-readonly-field` — **error**. A body's literal `ctx.api.object('…').update()` / `.updateById()` / `.insert()` writes a field the named object declares `readonly: true`, a field an `objectExtensions` entry of the same artifact adds to it included. Since [#15394](https://github.com/objectstack-ai/objectstack/issues/15394) the `insert` row of the table above is reported at build time exactly like the `update` row — same id, same severity, a message naming the verb — unless the hook declares `runAs: 'system'`. Only the static shape is judged on an insert: a `readonlyWhen` field has no prior record to lock on and the engine runs no conditional strip on INSERT, so no warning is produced there. - `hook-api-update-readonly-when-field` — **warning**. The same write against a `readonlyWhen` field, which strips per record *state*. The own-hook stamp **is** the workaround here, exactly as it is for static `readonly`: since [#9107](https://github.com/objectstack-ai/objectstack/issues/9107) the conditional strip judges the *caller's* entry payload, so a value a `beforeUpdate` hook **derives** is not caller-supplied and lands even on a locked record. (Deriving is the operative word — a hook that merely echoes the caller's own value back has written nothing the strip can tell from the caller's, and it still goes.) What does **not** help is elevation: unlike the static strip, the conditional lock is **not** waived by a system context, so neither `runAs: 'system'` nor the `sudo()` a body cannot reach makes a caller-supplied value survive. On this shape, confirm the write only targets records whose predicate is `false`, or derive the field in a `beforeUpdate` hook on the target object. Which hooks these rules can *see* depends on the command, because every rule in this family opens on `body.language === 'js'`. A hook authored as an inline `handler` function carries no `body`, so it is judged only where the command has first lowered the handler to a metadata body: `objectstack build` always has (it lowers before it parses — see [How the build lowers a handler](#build-pipeline)), and since [#16095](https://github.com/objectstack-ai/objectstack/issues/16095) `objectstack lint` judges that same lowered view, so an author who runs only the pre-flight is told the same thing the build would refuse. Since [#16544](https://github.com/objectstack-ai/objectstack/issues/16544) `objectstack validate` lowers before it parses as well, so all three commands judge the same view of a handler-authored hook — a stack `objectstack validate` passes is one `objectstack build` does not refuse on this family. A handler the build cannot lower (a forbidden token, a module-scope identifier) has no body on any command and is reported by the lowering rules instead, never guessed at here. diff --git a/packages/lint/src/object-graph.test.ts b/packages/lint/src/object-graph.test.ts index aaafbf175cb..7ef614d204b 100644 --- a/packages/lint/src/object-graph.test.ts +++ b/packages/lint/src/object-graph.test.ts @@ -34,6 +34,17 @@ import { RLS_PREDICATE_UNKNOWN_FIELD, validateRlsPredicateEnforceability } from import { SHARING_RULE_UNLOWERABLE_CONDITION, validateSharingRuleEnforceability } from './validate-sharing-rule-enforceability.js'; import { THANK_YOU_RECORD_FIELD, validateThankYouRecordFields } from './validate-thank-you-record-fields.js'; import { MEASURE_AGGREGATE_INCOHERENT, WIDGET_FILTER_FIELD_UNKNOWN, validateWidgetBindings } from './validate-widget-bindings.js'; +import { validateStackExpressions } from './validate-expressions.js'; +import { FLOW_NODE_WRITE_UNKNOWN_FIELD, validateFlowNodeWrites } from './validate-flow-node-writes.js'; +import { ACTION_BODY_WRITE_UNKNOWN_FIELD, validateActionBodyWrites } from './validate-action-body-writes.js'; +import { HOOK_BODY_WRITE_UNKNOWN_FIELD, validateHookBodyWrites } from './validate-hook-body-writes.js'; +import { validateReactPageProps } from './validate-react-page-props.js'; +import { TRANSLATION_TARGET_UNKNOWN, validateTranslationReferences } from './validate-translation-references.js'; +import { FLOW_UPDATE_READONLY_FIELD, FLOW_UPDATE_READONLY_WHEN_FIELD, validateReadonlyFlowWrites } from './validate-readonly-flow-writes.js'; +import { HOOK_API_UPDATE_READONLY_FIELD, HOOK_API_UPDATE_READONLY_WHEN_FIELD, validateReadonlyHookWrites } from './validate-readonly-hook-writes.js'; +import { SORT_FIELD_UNKNOWN, SORT_FIELD_UNSORTABLE, validateSortableFields } from './validate-sortable-fields.js'; +import { ACTION_API_UPDATE_READONLY_WHEN_FIELD, validateReadonlyActionWrites } from './validate-readonly-action-writes.js'; +import { SEARCHABLE_FIELD_UNKNOWN } from './validate-searchable-fields.js'; // [#16340] Read back the registry's OWN definition table to assert the graph // derives its injected types rather than carrying a second copy of them. import { injectedSystemColumnDefs } from '@objectstack/spec/data'; @@ -805,3 +816,350 @@ describe('[#30383] the field-path rules read the objects as served', () => { }); }); }); + +// [#30471] The field-NAME indexes outside the graph read the same fold: the +// `record.FIELD` index of the expression passes (validations, formulas, flows, +// hooks, sharing-rule conditions), the write rules' index (flow nodes, action +// and hook bodies), a react page's field props and the translation universe. +// The #30383 artifact, plus one extension rule for the translation rung. +describe('[#30471] the field-name indexes read the objects as served', () => { + type Rec = Record; + type Finding = { rule: string; severity?: unknown; where?: unknown; path?: unknown; message?: unknown }; + type Run = (stack: Rec) => ReadonlyArray; + const deal = { name: 'xs_deal', label: 'Deal', fields: { name: { type: 'text' }, amount: { type: 'number' } } }; + const account = { name: 'xs_account', label: 'Account', fields: { name: { type: 'text' } } }; + const added = { region: { type: 'text' }, score: { type: 'number' } }; + const extensions = [{ + extend: 'xs_deal', + fields: added, + validations: [{ name: 'region_set', type: 'script', condition: 'record.region == null', message: 'Set a region' }], + }]; + const artifact = (extra: Rec, objectExtensions: unknown[] = extensions, objects: unknown[] = [deal, account]): Rec => ({ + objects, + packages: [ + { manifest: { id: 'app', objects } }, + { manifest: { id: 'mod', objectExtensions } }, + ], + ...extra, + }); + // The per-package leg of `os build` (`packageBodyAsStack` in the cli). + const perPackage = (extra: Rec, objectExtensions: unknown[] = extensions): Rec => { + const body = { id: 'app', objects: [deal, account], ...extra }; + return { ...body, manifest: body, packages: [{ manifest: body }, { manifest: { id: 'mod', objectExtensions } }] }; + }; + const ids = (run: Run, stack: Rec) => run(stack).map((f) => f.rule); + // The expression passes carry no rule id: every issue they raise on these + // stacks is about the one expression each names, read as `expression`. + const expressions: Run = (stack) => validateStackExpressions(stack).map((issue) => ({ ...issue, rule: 'expression' })); + const validationOn = (condition: string): Rec => ({ + objects: [{ ...deal, validations: [{ name: 'v', type: 'script', condition, message: 'M' }] }, account], + }); + const pageSort = (field: string): Rec => ({ + pages: [{ name: 'p', type: 'list', object: 'xs_deal', interfaceConfig: { source: 'xs_deal', sort: [{ field, order: 'asc' }] } }], + }); + const viewSort = (field: string): Rec => ({ + views: [{ name: 'v', objectName: 'xs_deal', list: { type: 'grid', columns: ['name'], sort: [{ field, order: 'asc' }] } }], + }); + + // rule · its unknown-field id · the stack naming FIELD + const existence: Array<[string, string, Run, (field: string) => Rec]> = [ + ['validate-expressions (a validation condition)', 'expression', expressions, (f) => validationOn(`record.${f} == 'x'`)], + ['validate-expressions (a sharing-rule condition)', 'expression', expressions, (f) => ({ + sharingRules: [{ name: 'sr', object: 'xs_deal', condition: `record.${f} == 'x'`, sharedWith: { type: 'role', value: 'r' }, accessLevel: 'read' }], + })], + ['validate-flow-node-writes', FLOW_NODE_WRITE_UNKNOWN_FIELD, validateFlowNodeWrites, (f) => ({ + flows: [{ name: 'fl', nodes: [{ id: 'n', type: 'update_record', config: { objectName: 'xs_deal', fields: { [f]: 'x' } } }] }], + })], + ['validate-action-body-writes', ACTION_BODY_WRITE_UNKNOWN_FIELD, validateActionBodyWrites, (f) => ({ + actions: [{ name: 'act', label: 'Act', body: { language: 'js', source: `await ctx.api.object('xs_deal').updateById(id, { ${f}: 'x' });` } }], + })], + ['validate-hook-body-writes', HOOK_BODY_WRITE_UNKNOWN_FIELD, (stack) => validateHookBodyWrites(stack), (f) => ({ + hooks: [{ name: 'h', object: 'xs_deal', events: ['beforeInsert'], body: { language: 'js', source: `ctx.input.${f} = 'x';` } }], + })], + ['validate-react-page-props', PAGE_FIELD_UNKNOWN, validateReactPageProps, (f) => ({ + pages: [{ + name: 'p', kind: 'react', + source: `function Page(){ return ; }`, + }], + })], + ['validate-translation-references', TRANSLATION_TARGET_UNKNOWN, validateTranslationReferences, (f) => ({ + translations: [{ 'zh-CN': { objects: { xs_deal: { fields: { [f]: { label: 'L' } } } } } }], + })], + ['validate-sortable-fields (a page sort)', PAGE_FIELD_UNKNOWN, validateSortableFields, (f) => pageSort(f)], + ['validate-sortable-fields (a list-view sort)', SORT_FIELD_UNKNOWN, validateSortableFields, (f) => viewSort(f)], + ]; + + it.each(existence)('%s: an extension-added field passes; a field nothing adds is still refused', (_rule, id, run, naming) => { + expect(ids(run, artifact(naming('region')))).not.toContain(id); + expect(ids(run, artifact(naming('region'), []))).toContain(id); + expect(ids(run, artifact(naming('nowhere')))).toContain(id); + }); + + it.each(existence)('%s, on the per-package leg: the same verdicts', (_rule, id, run, naming) => { + expect(ids(run, perPackage(naming('region')))).not.toContain(id); + expect(ids(run, perPackage(naming('region'), []))).toContain(id); + expect(ids(run, perPackage(naming('nowhere')))).toContain(id); + }); + + it.each(existence)('%s: a field nothing adds is refused byte-for-byte as on the declared objects alone', (_rule, id, run, naming) => { + const refusal = (stack: Rec) => run(stack) + .filter((f) => f.rule === id) + .map(({ severity, where, path, message }) => ({ severity, where, path, message })); + const served = refusal(artifact(naming('zq_absent'))); + expect(served).toHaveLength(1); + expect(served).toEqual(refusal(artifact(naming('zq_absent'), []))); + }); + + it('validate-translation-references: a `_validations` key for a rule an extension adds resolves', () => { + const bundle = (rule: string) => ({ translations: [{ 'zh-CN': { objects: { xs_deal: { _validations: { [rule]: { message: 'M' } } } } } }] }); + expect(ids(validateTranslationReferences, artifact(bundle('region_set')))).toEqual([]); + expect(ids(validateTranslationReferences, artifact(bundle('region_set'), []))).toEqual([TRANSLATION_TARGET_UNKNOWN]); + expect(ids(validateTranslationReferences, artifact(bundle('nowhere')))).toEqual([TRANSLATION_TARGET_UNKNOWN]); + }); + + // The retired private reader kept every entry; the fold keeps one per + // package per target, so a later entry of the same package replaces an + // earlier one whole, its rules with its fields. + it('validate-translation-references: a key only a replaced entry adds is an orphan, as the registry serves it', () => { + const replaced = [...extensions, { extend: 'xs_deal', fields: { mood: { type: 'text' } } }]; + const keys = (fields: Rec, rules: Rec = {}) => ({ translations: [{ 'zh-CN': { objects: { xs_deal: { fields, _validations: rules } } } }] }); + expect(ids(validateTranslationReferences, artifact(keys({ mood: { label: 'M' } }), replaced))).toEqual([]); + expect(ids(validateTranslationReferences, artifact(keys({ region: { label: 'R' } }), replaced))).toEqual([TRANSLATION_TARGET_UNKNOWN]); + expect(ids(validateTranslationReferences, artifact(keys({}, { region_set: { message: 'M' } }), replaced))).toEqual([TRANSLATION_TARGET_UNKNOWN]); + }); + + // The two refusals the retired private reader did not raise, both at + // `error`: it kept every entry, and read a top-level `objectExtensions` + // beside `packages[]` too. The fold keeps one entry per package per target + // and reads `packages[]` alone when there is one, as the registry does. + it('validate-translation-references: the keys only the fold drops are refused at error', () => { + const bundle = { translations: [{ 'zh-CN': { objects: { xs_deal: { fields: { region: { label: 'R' } }, _validations: { region_set: { message: 'M' } } } } } }] }; + const refused = [[TRANSLATION_TARGET_UNKNOWN, 'error'], [TRANSLATION_TARGET_UNKNOWN, 'error']]; + const verdicts = (stack: Rec) => validateTranslationReferences(stack).map((f) => [f.rule, f.severity]); + const replaced = [...extensions, { extend: 'xs_deal', fields: { mood: { type: 'text' } } }]; + expect(verdicts(artifact(bundle, replaced))).toEqual(refused); + expect(verdicts(perPackage(bundle, replaced))).toEqual(refused); + const beside = { objects: [deal, account], objectExtensions: extensions, packages: [{ manifest: { id: 'app', objects: [deal, account] } }], ...bundle }; + expect(verdicts(beside)).toEqual(refused); + // With no `packages[]`, the top level is the artifact's own and is read. + const { packages: _packages, ...alone } = beside; + expect(verdicts(alone)).toEqual([]); + }); + + // The write rules leave an object that declares no field unjudged (an + // external, introspected schema): an extension adding to it does not make + // its other fields knowable. + it('the write rules keep an object that declares no field unjudged, extended or not', () => { + const bare = { name: 'xs_deal', label: 'Deal', fields: {} }; + const write = { flows: [{ name: 'fl', nodes: [{ id: 'n', type: 'update_record', config: { objectName: 'xs_deal', fields: { remote_col: 'x' } } }] }] }; + expect(ids(validateFlowNodeWrites, artifact(write, extensions, [bare, account]))).toEqual([]); + expect(ids(validateFlowNodeWrites, artifact(write, [], [bare, account]))).toEqual([]); + }); + + // A field two packages write at the deciding priority: its name is served, + // its definition is registration order. The two definition questions of the + // expression passes (its type, whether it is nullable) leave it unjudged; + // the single-writer controls show both verdicts the tie would otherwise get. + it('validate-expressions: a field two packages tie on is a served name, judged on no definition', () => { + const tie = (a: Rec, b: Rec): Rec => ({ + ...validationOn('record.score + 1 > 2'), + packages: [ + { manifest: { id: 'app', objects: [deal, account] } }, + { manifest: { id: 'mod', objectExtensions: [{ extend: 'xs_deal', fields: a }] } }, + { manifest: { id: 'mod2', objectExtensions: [{ extend: 'xs_deal', fields: b }] } }, + ], + }); + const messages = (a: Rec, b: Rec) => validateStackExpressions(tie(a, b)).map((issue) => issue.message); + const num = { score: { type: 'number' } }; + const text = { score: { type: 'text' } }; + expect(messages(num, text)).toEqual([]); + expect(messages(text, num)).toEqual([]); + expect(messages(text, {})).toEqual([expect.stringContaining('type mismatch'), expect.stringContaining('nullable')]); + expect(messages(num, {})).toEqual([expect.stringContaining('nullable')]); + }); + + it('validate-expressions, on the per-package leg: a tied field is judged on no definition either', () => { + const body = { id: 'app', objects: [{ ...deal, validations: [{ name: 'v', type: 'script', condition: 'record.score + 1 > 2', message: 'M' }] }, account] }; + const tie = (a: Rec, b: Rec): Rec => ({ + ...body, + manifest: body, + packages: [ + { manifest: body }, + { manifest: { id: 'mod', objectExtensions: [{ extend: 'xs_deal', fields: a }] } }, + { manifest: { id: 'mod2', objectExtensions: [{ extend: 'xs_deal', fields: b }] } }, + ], + }); + const messages = (a: Rec, b: Rec) => validateStackExpressions(tie(a, b)).map((issue) => issue.message); + const num = { score: { type: 'number' } }; + const text = { score: { type: 'text' } }; + expect(messages(num, text)).toEqual([]); + expect(messages(text, num)).toEqual([]); + expect(messages(text, {})).toEqual([expect.stringContaining('type mismatch'), expect.stringContaining('nullable')]); + }); + + // The readonly write rules judge an extension field's `readonly` / + // `readonlyWhen` as a declared field's: without that, moving the existence + // index above would have left such a write silent where `main` at least + // refused the name. Readonly is the field's definition, so a tie is unjudged. + describe('the readonly write rules read the served definition', () => { + const lock = { locked: { type: 'text', readonly: true } }; + const gate = { gated: { type: 'number', readonlyWhen: "record.name == 'x'" } }; + // rule · its readonly id · the extension fields it needs · the stack writing them + const readonly: Array<[string, string, Run, Rec, Rec]> = [ + ['validate-readonly-flow-writes', FLOW_UPDATE_READONLY_FIELD, validateReadonlyFlowWrites, lock, { + flows: [{ name: 'fl', nodes: [{ id: 'n', type: 'update_record', config: { objectName: 'xs_deal', fields: { locked: 'x' } } }] }], + }], + ['validate-readonly-hook-writes', HOOK_API_UPDATE_READONLY_FIELD, (stack) => validateReadonlyHookWrites(stack), lock, { + hooks: [{ name: 'h', object: 'xs_account', events: ['afterInsert'], body: { language: 'js', source: "await ctx.api.object('xs_deal').update({ id: dealId, locked: 'x' });" } }], + }], + ['validate-readonly-action-writes', ACTION_API_UPDATE_READONLY_WHEN_FIELD, validateReadonlyActionWrites, gate, { + actions: [{ name: 'act', label: 'Act', objectName: 'xs_deal', body: { language: 'js', source: "await ctx.api.object('xs_deal').update({ id: ctx.recordId, gated: 1 });" } }], + }], + ]; + const plain = { locked: { type: 'text' }, gated: { type: 'number' } }; + const ext = (fields: Rec) => [{ extend: 'xs_deal', fields }]; + const tie = (a: Rec, b: Rec, extra: Rec, perPackageLeg = false): Rec => { + const mods = [ + { manifest: { id: 'mod', objectExtensions: ext(a) } }, + { manifest: { id: 'mod2', objectExtensions: ext(b) } }, + ]; + if (!perPackageLeg) return { objects: [deal, account], packages: [{ manifest: { id: 'app', objects: [deal, account] } }, ...mods], ...extra }; + const body = { id: 'app', objects: [deal, account], ...extra }; + return { ...body, manifest: body, packages: [{ manifest: body }, ...mods] }; + }; + + it.each(readonly)('%s: an extension field declared readonly is refused with the readonly id', (_rule, id, run, fields, writing) => { + expect(ids(run, artifact(writing, ext(fields)))).toEqual([id]); + expect(ids(run, perPackage(writing, ext(fields)))).toEqual([id]); + // The same name added writable, or not added at all, is not this rule's. + expect(ids(run, artifact(writing, ext(plain)))).toEqual([]); + expect(ids(run, artifact(writing, []))).toEqual([]); + }); + + it.each(readonly)('%s: a field two packages tie on, one of them readonly, is unjudged', (_rule, id, run, fields, writing) => { + for (const perPackageLeg of [false, true]) { + expect(ids(run, tie(plain, fields, writing, perPackageLeg))).toEqual([]); + expect(ids(run, tie(fields, plain, writing, perPackageLeg))).toEqual([]); + expect(ids(run, tie(fields, {}, writing, perPackageLeg))).toEqual([id]); + } + }); + }); + + // `indexObjectSearchTargets` stays on the declared objects (its type + // question is the searchable-fields rule's, not moved here), so a react + // page's `searchableFields` naming an extension field is still refused. + it('validate-react-page-props: searchableFields still reads the declared objects', () => { + const search = (field: string) => ({ + pages: [{ + name: 'p', kind: 'react', + source: `function Page(){ return ; }`, + }], + }); + expect(ids(validateReactPageProps, artifact(search('region')))).toEqual([SEARCHABLE_FIELD_UNKNOWN]); + expect(ids(validateReactPageProps, artifact(search('name')))).toEqual([]); + }); + + // Every refusal this change adds, one row each: a write or a sort naming an + // extension field whose served definition the runtime refuses or drops. On + // the declared objects alone (what `main` read) the same stack was the + // existence rule's unknown name; now the existence rule passes it and the + // definition rule owns it, at that rule's own severity. The hook rows are + // the one severity move: `hook-body-write-unknown-field` was a warning. + describe('each refusal this change adds is the definition rule\'s, at its own severity', () => { + const lock = { locked: { type: 'text', readonly: true } }; + const gate = { gated: { type: 'number', readonlyWhen: "record.name == 'x'" } }; + const calc = { calc: { type: 'formula', expression: 'record.amount * 2' } }; + const flowNode = (type: string, field: string): Rec => ({ + flows: [{ name: 'fl', nodes: [{ id: 'n', type, config: { objectName: 'xs_deal', fields: { [field]: 'x' } } }] }], + }); + const hookApi = (call: string): Rec => ({ + hooks: [{ name: 'h', object: 'xs_account', events: ['afterInsert'], body: { language: 'js', source: `await ctx.api.object('xs_deal').${call};` } }], + }); + const actionApi = (field: string): Rec => ({ + actions: [{ name: 'act', label: 'Act', objectName: 'xs_deal', body: { language: 'js', source: `await ctx.api.object('xs_deal').update({ id: ctx.recordId, ${field}: 1 });` } }], + }); + const hooks: Run = (stack) => validateReadonlyHookWrites(stack); + const hookExistence: Run = (stack) => validateHookBodyWrites(stack); + // the write · the rule judging it now · the extension fields · the stack · + // its id and severity now · the existence rule and the id it raised before + const refusals: Array<[string, Run, Rec, Rec, string, string, Run, string]> = [ + ['a flow update_record of a readonly field', validateReadonlyFlowWrites, lock, flowNode('update_record', 'locked'), + FLOW_UPDATE_READONLY_FIELD, 'error', validateFlowNodeWrites, FLOW_NODE_WRITE_UNKNOWN_FIELD], + ['a flow create_record of a readonly field', validateReadonlyFlowWrites, lock, flowNode('create_record', 'locked'), + FLOW_UPDATE_READONLY_FIELD, 'error', validateFlowNodeWrites, FLOW_NODE_WRITE_UNKNOWN_FIELD], + ['a flow update_record of a readonlyWhen field', validateReadonlyFlowWrites, gate, flowNode('update_record', 'gated'), + FLOW_UPDATE_READONLY_WHEN_FIELD, 'warning', validateFlowNodeWrites, FLOW_NODE_WRITE_UNKNOWN_FIELD], + ['a hook ctx.api update of a readonly field', hooks, lock, hookApi("update({ id: dealId, locked: 'x' })"), + HOOK_API_UPDATE_READONLY_FIELD, 'error', hookExistence, HOOK_BODY_WRITE_UNKNOWN_FIELD], + ['a hook ctx.api insert of a readonly field', hooks, lock, hookApi("insert({ locked: 'x' })"), + HOOK_API_UPDATE_READONLY_FIELD, 'error', hookExistence, HOOK_BODY_WRITE_UNKNOWN_FIELD], + ['a hook ctx.api update of a readonlyWhen field', hooks, gate, hookApi('update({ id: dealId, gated: 1 })'), + HOOK_API_UPDATE_READONLY_WHEN_FIELD, 'warning', hookExistence, HOOK_BODY_WRITE_UNKNOWN_FIELD], + ['an action ctx.api update of a readonlyWhen field', validateReadonlyActionWrites, gate, actionApi('gated'), + ACTION_API_UPDATE_READONLY_WHEN_FIELD, 'warning', validateActionBodyWrites, ACTION_BODY_WRITE_UNKNOWN_FIELD], + ['a page sort by a formula field', validateSortableFields, calc, pageSort('calc'), + SORT_FIELD_UNSORTABLE, 'error', validateSortableFields, PAGE_FIELD_UNKNOWN], + ['a list-view sort by a formula field', validateSortableFields, calc, viewSort('calc'), + SORT_FIELD_UNSORTABLE, 'error', validateSortableFields, SORT_FIELD_UNKNOWN], + ]; + // The stack on both CLI legs (the union artifact, then the per-package + // body), each extension list its own package. + const onLegs = (extra: Rec, packages: unknown[][], objects: unknown[] = [deal, account]): Rec[] => { + const mods = packages.map((objectExtensions, i) => ({ manifest: { id: `mod${i}`, objectExtensions } })); + const body = { id: 'app', objects, ...extra }; + return [ + { objects, packages: [{ manifest: { id: 'app', objects } }, ...mods], ...extra }, + { ...body, manifest: body, packages: [{ manifest: body }, ...mods] }, + ]; + }; + const ext = (fields: Rec) => [{ extend: 'xs_deal', fields }]; + + it.each(refusals)('%s', (_write, run, fields, stack, id, severity, existence, unknownId) => { + for (const served of onLegs(stack, [ext(fields)])) { + expect(run(served).map((f) => [f.rule, f.severity])).toEqual([[id, severity]]); + expect(ids(existence, served)).not.toContain(unknownId); + } + for (const declared of onLegs(stack, [[]])) { + expect(ids(existence, declared)).toContain(unknownId); + expect(ids(run, declared)).not.toContain(id); + } + }); + + it.each(refusals)('%s, on a field two packages tie on: unjudged', (_write, run, fields, stack, id) => { + const plain = Object.fromEntries(Object.keys(fields).map((name) => [name, { type: 'text' }])); + for (const [a, b] of [[plain, fields], [fields, plain]]) { + for (const tied of onLegs(stack, [ext(a), ext(b)])) expect(ids(run, tied)).not.toContain(id); + } + for (const single of onLegs(stack, [ext(fields), ext({})])) expect(ids(run, single)).toContain(id); + }); + + // An object that declares no field (an introspected schema) is left + // unjudged by the write rules and skip 2 of the sort rule; an extension + // adding to it does not make it judgeable by the definition rules either. + it.each(refusals)('%s, on an object that declares no field: unjudged, extended or not', (_write, run, fields, stack) => { + const bare = { name: 'xs_deal', label: 'Deal', fields: {} }; + for (const packages of [[ext(fields)], [[]]]) { + for (const s of onLegs(stack, packages, [bare, account])) expect(ids(run, s)).toEqual([]); + } + }); + + // An extension that RE-DECLARES an owner's field replaces it whole, in the + // fold as in the registry's `mergeObjectDefinitions` + // (`{ ...base.fields, ...extension.fields }`), so the owner's field is + // judged on the extension's definition. On the declared objects alone (what + // `main` read) the owner's `amount: number` is known, sortable and writable, + // and nothing is raised. ADR-0146 D10 refuses this shape on the extension; + // no CLI door does. + it.each<[string, Run, Rec, Rec, string]>([ + ['a list-view sort by an owner field re-declared as formula', validateSortableFields, + { amount: { type: 'formula', expression: 'record.name' } }, viewSort('amount'), SORT_FIELD_UNSORTABLE], + ['a flow update_record of an owner field re-declared readonly', validateReadonlyFlowWrites, + { amount: { type: 'number', readonly: true } }, flowNode('update_record', 'amount'), FLOW_UPDATE_READONLY_FIELD], + ])('%s: judged on the extension\'s definition', (_write, run, fields, stack, id) => { + for (const served of onLegs(stack, [ext(fields)])) { + expect(run(served).map((f) => [f.rule, f.severity])).toEqual([[id, 'error']]); + } + for (const declared of onLegs(stack, [[]])) expect(run(declared)).toEqual([]); + }); + }); +}); diff --git a/packages/lint/src/object-graph.ts b/packages/lint/src/object-graph.ts index a3ff9236a8c..6d6f1b2146e 100644 --- a/packages/lint/src/object-graph.ts +++ b/packages/lint/src/object-graph.ts @@ -378,6 +378,14 @@ export interface VisibleObjectExtension { priority: number; /** Its field records, name folded in ({@link recordsOf}). */ fields: AnyRec[]; + /** + * [#30471] Its validation rule records ({@link recordsOf}), carried for the + * one reader that addresses an object's rules by name + * (`validate-translation-references.ts`, `_validations.RULE`). Carrying them + * changes nothing about which entries survive: a replaced entry's rules go + * with it, as its fields do. + */ + validations: AnyRec[]; } /** @@ -430,6 +438,7 @@ export function objectExtensionsByTarget(stack: unknown): Map> } { const objects = isRec(stack) ? recordsOf(stack.objects) : []; diff --git a/packages/lint/src/rule-explanations.ts b/packages/lint/src/rule-explanations.ts index 36fe92a6071..3596b0a140f 100644 --- a/packages/lint/src/rule-explanations.ts +++ b/packages/lint/src/rule-explanations.ts @@ -1096,7 +1096,8 @@ const DECLARED_FIELD_DOOR = 'every driver, before any statement is built. The whole payload is refused, so the correctly named ' + 'fields beside the bad key never land either. The refusal arrives at run time, on whichever record ' + 'first exercises the write and far from the line that made the mistake, which is why the rule ' + - 'reports it at author time, naming the field, the object and the write.'; + 'reports it at author time, naming the field, the object and the write. Declared includes every field ' + + 'an `objectExtensions` entry of the same artifact adds to the object, which the registry serves.'; /** The cause and the consequence of writing an unprovisioned anchor — the three anchor ids. */ const UNPROVISIONED_ANCHOR_WRITE: readonly string[] = [ @@ -3903,8 +3904,8 @@ const TRANSLATION_TARGET_UNKNOWN_EXPLANATION: RuleExplanation = { 'The verdict names the nearest declared name when one is close ("Did you mean"), and the fix ' + 'line names the remedy only, without listing what is declared; what each key may name follows.', 'Under `objects.OBJECT` (an object this stack, or a sibling package of the same artifact, ' + - 'defines): `fields.FIELD` a declared field (a system column, `_id`, `name` and `space` are ' + - 'never reported); `_views.VIEW` a view\'s `name`, not its label — the runtime keys a nameless ' + + 'defines): `fields.FIELD` a declared field, fields an `objectExtensions` entry of the same ' + + 'artifact adds included (a system column, `_id`, `name` and `space` are never reported); `_views.VIEW` a view\'s `name`, not its label — the runtime keys a nameless ' + 'default `list` `default`, never `list`; `_sections.KEY` a ' + '`fieldGroups[].key`, a named form-view section or a named `record:details` section — a ' + 'section is translatable only through a stable name; `_tabs.TAB` a filter-preset tab that a ' + @@ -4332,7 +4333,8 @@ const PAGE_FIELD_UNKNOWN_EXPLANATION: RuleExplanation = { covers: 'how a page binding names a field', paragraphs: [ 'A page component binds fields by name, and each name must match one of the bound object\'s ' + - 'declared fields, or a system field, exactly: a misspelt or renamed field resolves to nothing, ' + + 'declared fields (declared includes fields an `objectExtensions` entry of the same artifact ' + + 'adds), or a system field, exactly: a misspelt or renamed field resolves to nothing, ' + 'at the cost the message names.', 'To fix it, correct the name (the `fix:` line names the nearest declared field when one is ' + 'close) or add the field to the object. Not judged here: a dotted relationship path ' + diff --git a/packages/lint/src/validate-expressions.ts b/packages/lint/src/validate-expressions.ts index 66c75435a1e..d6910463119 100644 --- a/packages/lint/src/validate-expressions.ts +++ b/packages/lint/src/validate-expressions.ts @@ -153,7 +153,7 @@ import type { FlowCelEntrance } from './flow-cel-root-scope.js'; import { injectedColumnDefsFor, injectedColumnsFor, unprovisionedInjectedColumnsFor } from './system-fields.js'; import { findUnguardedNullableOperands, nullGuardMessage, provenNonNullKeys } from './validate-null-guards.js'; import type { NullGuardOptions, NullGuardOutcome } from './validate-null-guards.js'; -import { recordsOf } from './object-graph.js'; +import { objectsAsServed, objectsWithoutTiedFields, recordsOf } from './object-graph.js'; export interface ExprIssue { where: string; message: string; @@ -198,6 +198,9 @@ type AnyRec = Record; * `owner_id`, so `record.owner_id` there stays the error it should be. A blanket * union would have traded a false positive for a false negative and called it * fixed. + * + * [#30471] Built over the objects as served ({@link objectsAsServed}), so the + * authored half includes every field an `objectExtensions` entry adds. */ function buildFieldIndex(objects: AnyRec[]): Map { const idx = new Map(); @@ -2235,10 +2238,23 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression return actionCopyTemplateIssues(stack, { actions: false, objects: false, pages: true }); } const objects = recordsOf(stack.objects); - const fieldIndex = buildFieldIndex(objects); + // [#30471] The field indexes read the objects as the registry SERVES them, + // every `objectExtensions` field of any body folded in (`objectsAsServed`, + // the package's one fold): a `record.FIELD` read of a field a sibling + // package adds resolves as it does at runtime, and a field no body declares + // or extends is still unknown. The walk below stays on the declared + // `objects`, so every finding keeps its declared path. The two indexes that + // judge a field's DEFINITION (its type, whether it is nullable) leave out a + // field two packages tie on (`objectsWithoutTiedFields`): which definition + // the registry serves is registration order, which the stack does not state, + // so such a field is a known name with no type and no null-guard verdict. + // The read-attachment index is object-level and an extension carries none. + const { objects: servedObjects, tied } = objectsAsServed(stack); + const typedObjects = objectsWithoutTiedFields({ objects: servedObjects, tied }); + const fieldIndex = buildFieldIndex(servedObjects); const attachedOnReadIndex = buildAttachedOnReadIndex(objects); - const fieldTypeIndex = buildFieldTypeIndex(objects); - const nullableIndex = buildNullableFieldIndex(objects); + const fieldTypeIndex = buildFieldTypeIndex(typedObjects); + const nullableIndex = buildNullableFieldIndex(typedObjects); // [#24316] object name → its references into a stored-metadata-body table, // read the first time a site of that object serves a hop, or (#24677) when // the walk reaches the object's fields, whichever comes first — never before @@ -3461,7 +3477,10 @@ export function runStackExpressionPasses(stack: AnyRec, options: StackExpression // The type is read from the field-type index, never off the field again: // the #5017 receiver meta-test pins the keys this walk reads off `f`. The // collector admits only a field whose `type` is a string, and the index - // records every such field, so each collected field has its type here. + // records every such field, so each collected field has its type here, + // save one two packages tie on (#30471): the index leaves its definition + // out, so it is skipped rather than judged on a definition the registry + // may not serve. if (objectName) { const familyRefs = storedMetadataBodyReferencesFor(objectName); const ownFieldTypes = fieldTypeIndex.get(objectName); diff --git a/packages/lint/src/validate-hook-body-writes.ts b/packages/lint/src/validate-hook-body-writes.ts index 70eca679e0e..d6a8b8eb607 100644 --- a/packages/lint/src/validate-hook-body-writes.ts +++ b/packages/lint/src/validate-hook-body-writes.ts @@ -103,7 +103,7 @@ import { unprovisionedAnchorFix, unprovisionedAnchorVerdict, } from './system-fields.js'; -import { recordsOf } from './object-graph.js'; +import { objectsAsServed, recordsOf } from './object-graph.js'; // The TypeScript compiler must NOT be imported at module top level: it is // ~9 MB of CJS, and @objectstack/lint sits on the kernel boot path — while @@ -497,22 +497,46 @@ export function hookBodyFindingLocation( } /** - * object name → its declared field names (both `fields` authoring shapes). + * object name → its field names as the registry SERVES the object (both + * `fields` authoring shapes). * - * Exported for `validate-action-body-writes.ts` only (see - * {@link IMPLICIT_FIELDS} for why the two rules share rather than copy). + * Exported for `validate-action-body-writes.ts` and + * `validate-flow-node-writes.ts` (see {@link IMPLICIT_FIELDS} for why the + * rules share rather than copy). + * + * [#30471] Read through the package's one extension fold + * ({@link objectsAsServed}): a write naming a field a sibling package adds + * through `objectExtensions` lands, as it does at runtime, and a field no body + * declares or extends is still unknown. A tied field is a served name, so it + * is kept: these rules ask whether the name exists, never its definition. + * Whether the column is WRITABLE (`readonly`) is the readonly rules' question, + * not this index's. + * + * Judgeability stays the declaration's ({@link judgeableFieldsOf}) for the + * stack this is handed: an object that declares no field at all (external, + * datasource-introspected) keeps an empty set even when an extension adds to + * it, because the fields it does serve are still unknown here. The guard reads + * the declaration from `stack.objects`, so a caller that hands it objects + * already served (`{ objects: objectsAsServed(stack).objects }`, as the react + * page rule does with the page rule's own index) bypasses it by construction. */ export function indexObjectFields(stack: AnyRec): Map> { const out = new Map>(); - for (const obj of recordsOf(stack.objects)) { - const name = typeof obj.name === 'string' ? obj.name : undefined; - if (!name) continue; + const namesOf = (obj: AnyRec | undefined): Set => { const names = new Set(); - for (const f of recordsOf(obj.fields)) { + for (const f of recordsOf(obj?.fields)) { if (typeof f.name === 'string' && f.name) names.add(f.name); } - out.set(name, names); - } + return names; + }; + const declared = recordsOf(stack.objects); + // `objectsAsServed` returns the declared objects in order, one per entry. + objectsAsServed(stack).objects.forEach((obj, i) => { + const name = typeof obj.name === 'string' ? obj.name : undefined; + if (!name) return; + const own = namesOf(declared[i]); + out.set(name, own.size > 0 ? namesOf(obj) : own); + }); return out; } diff --git a/packages/lint/src/validate-react-page-props.ts b/packages/lint/src/validate-react-page-props.ts index 8269227e721..3b1e462c2aa 100644 --- a/packages/lint/src/validate-react-page-props.ts +++ b/packages/lint/src/validate-react-page-props.ts @@ -69,6 +69,7 @@ import { // #5020's zod-rejection renderer, shared with the SDUI component-props gate // since #5068 — see `zod-issue-format.ts` for why one copy matters here. import { describeIssue, type LintZodIssue } from './zod-issue-format.js'; +import { objectsAsServed } from './object-graph.js'; import { createSourceFileChecked, describeParseFailure, PARSE_FAILURE_FIX } from './checked-parse.js'; // [#22161] The verdict half of a forwarded `@objectstack/spec` refusal — one // cut for every rule that forwards the same producer's words. @@ -1136,7 +1137,16 @@ function localComponentNames(tsc: typeof ts, sf: ts.SourceFile): Set { export function validateReactPageProps(stack: AnyRec): ReactPropFinding[] { const findings: ReactPropFinding[] = []; - const objectFields = indexObjectFields(stack); + // [#30471] Over the objects as served, every `objectExtensions` field + // folded in, as `validatePageFieldBindings` reads them: a prop naming a + // field a sibling package adds is served, and one no body declares or + // extends is still unknown. Handing the index pre-served objects bypasses + // the declares-nothing guard by construction: an object that declares no + // field but is extended gets the extension's names, not an empty set. No + // verdict moves toward a refusal: this rule judged that empty set on main + // too (`checkFieldRefs` and `checkObjectChart` skip only a missing object), + // so every ref was unknown there and the extension's names now resolve. + const objectFields = indexObjectFields({ objects: objectsAsServed(stack).objects }); // [#8340] The provenance index alongside the existence one — same keying, // asked on the path where the existence check stays silent. const unprovisionedAnchors = indexUnprovisionedAnchors(stack); diff --git a/packages/lint/src/validate-readonly-action-writes.ts b/packages/lint/src/validate-readonly-action-writes.ts index 21f81248392..2bd40851f8f 100644 --- a/packages/lint/src/validate-readonly-action-writes.ts +++ b/packages/lint/src/validate-readonly-action-writes.ts @@ -108,8 +108,7 @@ import { extractHookBodyWriteSet, type BodyWritePatternExclusion, } from './validate-hook-body-writes.js'; -import { buildReadonlyIndex, READONLY_WHEN_STRIP_SCOPE } from './validate-readonly-flow-writes.js'; -import { recordsOf } from './object-graph.js'; +import { buildReadonlyIndex, buildServedReadonlyIndex, READONLY_WHEN_STRIP_SCOPE } from './validate-readonly-flow-writes.js'; export type ReadonlyActionWriteSeverity = 'warning'; @@ -264,7 +263,9 @@ export function validateReadonlyActionWrites(stack: AnyRec): ReadonlyActionWrite ); if (writes.length === 0) continue; - roIndex ??= buildReadonlyIndex(recordsOf(stack.objects)); + // [#30471] The readonly index reads the objects as served + // (`buildServedReadonlyIndex`). + roIndex ??= buildServedReadonlyIndex(stack); const where = `action "${site.name}" > body`; const reported = new Set(); diff --git a/packages/lint/src/validate-readonly-flow-writes.ts b/packages/lint/src/validate-readonly-flow-writes.ts index 8f06a3fbbae..f78d6845afd 100644 --- a/packages/lint/src/validate-readonly-flow-writes.ts +++ b/packages/lint/src/validate-readonly-flow-writes.ts @@ -64,7 +64,7 @@ // flows are held to the same bar. import { walkFlowNodes, flowNodeLabel } from './flow-walk.js'; -import { recordsOf } from './object-graph.js'; +import { objectsAsServed, objectsWithoutTiedFields, recordsOf } from './object-graph.js'; export type ReadonlyFlowWriteSeverity = 'error' | 'warning'; @@ -162,6 +162,25 @@ export function buildReadonlyIndex(objects: AnyRec[]): Map> { + const idx = buildReadonlyIndex(objectsWithoutTiedFields(objectsAsServed(stack))); + for (const [name, own] of buildReadonlyIndex(recordsOf(stack.objects))) { + if (own.size === 0) idx.set(name, own); + } + return idx; +} + /** * The `managedBy` buckets the engine's create-side strip steps around, mirroring * `PLATFORM_INTERNAL_MANAGED_BUCKETS` in @@ -245,7 +264,10 @@ export function validateReadonlyFlowWrites(stack: AnyRec): ReadonlyFlowWriteFind if (flows.length === 0) return findings; const objects = recordsOf(stack.objects); - const roIndex = buildReadonlyIndex(objects); + // [#30471] The readonly index reads the objects as served + // (`buildServedReadonlyIndex`). The insert-strip exemption is object-level + // and stays on the declared objects. + const roIndex = buildServedReadonlyIndex(stack); const insertStripExempt = buildInsertStripExemptObjects(objects); flows.forEach((flow, flowIndex) => { diff --git a/packages/lint/src/validate-readonly-hook-writes.ts b/packages/lint/src/validate-readonly-hook-writes.ts index 81a7d615526..e75c087fc66 100644 --- a/packages/lint/src/validate-readonly-hook-writes.ts +++ b/packages/lint/src/validate-readonly-hook-writes.ts @@ -174,6 +174,7 @@ import { } from './validate-hook-body-writes.js'; import { buildReadonlyIndex, + buildServedReadonlyIndex, buildInsertStripExemptObjects, READONLY_INSERT_STRIP_OUTCOME, READONLY_WHEN_STRIP_SCOPE, @@ -404,7 +405,10 @@ export function validateReadonlyHookWrites( ); if (writes.length === 0) return; - roIndex ??= buildReadonlyIndex(recordsOf(stack.objects)); + // [#30471] The readonly index reads the objects as served + // (`buildServedReadonlyIndex`). The insert-strip exemption is object-level + // and stays on the declared objects. + roIndex ??= buildServedReadonlyIndex(stack); insertStripExempt ??= buildInsertStripExemptObjects(recordsOf(stack.objects)); const hookName = typeof hook.name === 'string' && hook.name ? hook.name : `#${hookIndex}`; diff --git a/packages/lint/src/validate-security-posture.test.ts b/packages/lint/src/validate-security-posture.test.ts index ea802e06090..7137f3def8c 100644 --- a/packages/lint/src/validate-security-posture.test.ts +++ b/packages/lint/src/validate-security-posture.test.ts @@ -1910,7 +1910,7 @@ const READ_SURFACES: Array<{ receiver: string; expected: string[]; declaredBy: s }, { receiver: 'ext', - expected: ['extend', 'fields', 'priority'], + expected: ['extend', 'fields', 'priority', 'validations'], declaredBy: 'ObjectStackSchema.objectExtensions[]', keys: () => shapeKeysOf(ObjectStackSchema.shape.objectExtensions), }, diff --git a/packages/lint/src/validate-sortable-fields.ts b/packages/lint/src/validate-sortable-fields.ts index c4224e93bee..daf10175aac 100644 --- a/packages/lint/src/validate-sortable-fields.ts +++ b/packages/lint/src/validate-sortable-fields.ts @@ -256,7 +256,7 @@ import { type FieldSortability, } from '@objectstack/spec/api'; -import { recordsOf, suggestName } from './object-graph.js'; +import { objectsAsServed, objectsWithoutTiedFields, recordsOf, suggestName } from './object-graph.js'; import { walkPageComponents } from './page-walk.js'; import { PAGE_FIELD_UNKNOWN, PAGE_FIELD_UNPROVISIONED } from './validate-page-field-bindings.js'; import { @@ -512,6 +512,38 @@ export function indexObjectSortability(stack: AnyRec): Map; + sortability: Map; +} { + const served = objectsAsServed(stack); + const fieldsByObject = indexObjectSearchTargets({ objects: served.objects }); + const sortability = indexObjectSortability({ objects: objectsWithoutTiedFields(served) }); + const declaredSortability = indexObjectSortability(stack); + for (const [name, target] of indexObjectSearchTargets(stack)) { + if (target !== null) continue; + fieldsByObject.set(name, null); + const own = declaredSortability.get(name); + if (own) sortability.set(name, own); + } + return { fieldsByObject, sortability }; +} + /** * The finding for one sort key whose projection entry says `sortable: false`, * or `undefined` when it does not (no entry — unknown, dotted, a registry @@ -787,12 +819,14 @@ export function validateSortableFields(stack: AnyRec): SortableFieldFinding[] { : isRec(stack.objects) ? Object.entries(stack.objects).map(([name, def]) => ({ name, ...(def as AnyRec) })) : []; - const fieldsByObject = indexObjectSearchTargets(stack); + // [#30471] The existence index and the sortability projection read the + // objects as served ({@link indexServedSortFields}); the walk above stays on + // the declared objects, so every finding keeps its declared path. + const { fieldsByObject, sortability } = indexServedSortFields(stack); // [#10474] The provenance index alongside the existence one — same keying, - // asked on the path where the existence check stays silent. + // asked on the path where the existence check stays silent. Object-level + // (`external`, the injected anchors), so it stays on the declared objects. const unprovisionedAnchors = indexUnprovisionedAnchors(stack); - // [#30023] The sortability projection the list door refuses by, per object. - const sortability = indexObjectSortability(stack); const check = ( declared: unknown, diff --git a/packages/lint/src/validate-translation-references.ts b/packages/lint/src/validate-translation-references.ts index 41fd360f303..b083c1ca2ce 100644 --- a/packages/lint/src/validate-translation-references.ts +++ b/packages/lint/src/validate-translation-references.ts @@ -242,8 +242,8 @@ * ⛔ Each fold widens what a key may RESOLVE against and nothing else: a name * no declaration and no contribution carries is still an orphan and still * errors, on the per-package leg exactly as on the union one. See - * {@link contributedNavItemsByApp} and {@link objectExtensionsByTarget} for the - * fold facts each rests on, and this rule's test file for the lit controls. + * {@link contributedNavItemsByApp} and the object-extension note above + * {@link artifactProvidedRecords} for the fold facts each rests on, and this rule's test file for the lit controls. */ import { AUDIENCE_ANCHOR_POSITIONS, expandViewContainer } from '@objectstack/spec'; @@ -259,7 +259,7 @@ import { walkAddressedPageComponents, } from '@objectstack/spec/system'; import { walkFlowNodes } from './flow-walk.js'; -import { packagesOf, recordsOf, suggestName } from './object-graph.js'; +import { objectExtensionsByTarget, packagesOf, recordsOf, suggestName } from './object-graph.js'; import { walkPageComponents } from './page-walk.js'; import { SYSTEM_FIELDS } from './system-fields.js'; import { viewObjectName } from './view-walk.js'; @@ -1059,8 +1059,8 @@ function contributedNavItemsByApp(stack: AnyRec): Map { return byApp; } -/** - * Target object name → every `objectExtensions[]` entry aimed at it (#18441). +/* + * ## Object extensions in the universe (#18441) * * ## Why the universe has to include these at all * @@ -1100,59 +1100,26 @@ function contributedNavItemsByApp(stack: AnyRec): Map { * symmetry": an extension cannot contribute any of them, so a key there is an * orphan exactly as it was, and widening those rungs would silence real ones. * - * ## Why a union is FAITHFUL here rather than a second implementation - * - * Same property as {@link contributedNavItemsByApp}, one collection over: the - * registry merges EVERY extension layer onto the target and `priority` decides - * only which layer WINS A CONFLICT — never whether a name arrives. So the set - * of addressable names is invariant under the merge, and set membership is the - * only question asked here. ⛔ The registry is not imported to answer it - * (`@objectstack/lint` depends on `@objectstack/spec` and never on a runtime). - * - * ## Which two carriers are read, and why both + * ## Read through the package's one fold (#30471) * - * `objectExtensions` is a STACK collection (`StackSchema`, disposition - * `concat`), not a manifest key — so unlike the nav surface there is no - * `stack.manifest` form to read: - * - * 1. `stack.objectExtensions` — the stack in hand. On the union leg that is - * every package's extensions already concatenated; on the per-package leg - * (`packageBodyAsStack`) it is this package's own, at the top level of its - * assembled body. - * 2. `packages[].manifest.objectExtensions` — the ADR-0130 D4 artifact - * entries, whose bodies carry the collection (`concat` is one of the - * `ASSEMBLED_PACKAGE_BODY_DISPOSITIONS`). Read for the reason #16611 gave - * for object NAMES: the artifact is the co-ownership boundary, so a - * per-package leg that cannot see a sibling package's extension reports - * THIS RUN's blind spot as the author's mistake. Without it the union leg - * and the per-package leg would disagree about one key. + * The entries come from `objectExtensionsByTarget` in `object-graph.ts`, the + * fold every field-path rule of this package reads, which carries each + * entry's `fields` and `validations`. It reads extensions as the registry + * registers them: with `packages[]`, every body's `objectExtensions` (the + * per-package leg's own entries arrive in its `packages[]` entry too, so the + * top level is not read beside them); without it, the stack's own. ONE entry + * per package per target: a later entry of a package replaces its earlier + * one whole, so a key addressing a field or rule only the replaced entry adds + * is an orphan, as the registry serves it. This file kept its own fold until + * then (top level plus every body, every entry kept), because the shared fold + * did not carry validation rules. * * ⛔ What this cannot see, stated rather than implied: fields injected * IMPERATIVELY by plugin code are not metadata and no static rule can read * them — the same bound {@link contributedNavItemsByApp} records for * contributions registered from a plugin's `init`. */ -function objectExtensionsByTarget(stack: AnyRec): Map { - const byTarget = new Map(); - const add = (extensions: unknown) => { - for (const extension of recordsOf(extensions)) { - // An extension names ONE target object. An entry with no `extend` merges - // into nothing at runtime, so it must not make anything addressable. - const target = strName(extension.extend); - if (!target) continue; - const entries = byTarget.get(target) ?? []; - entries.push(extension); - byTarget.set(target, entries); - } - }; - add(stack.objectExtensions); - for (const entry of packagesOf(stack)) { - const body = entry.manifest; - if (!isRec(body)) continue; - add(body.objectExtensions); - } - return byTarget; -} + /** * Every RECORD of one metadata collection that an entry of `packages[]` @@ -1164,7 +1131,7 @@ function objectExtensionsByTarget(stack: AnyRec): Map { * Exactly the keys whose `COMPOSE_KEY_DISPOSITIONS` disposition is one of * `ASSEMBLED_PACKAGE_BODY_DISPOSITIONS` (`concat`, `objects`, `functions`) — * that membership IS what puts a key inside an ADR-0130 D4 entry's assembled - * body, so it is the same proof {@link objectExtensionsByTarget} rests on one + * body, so it is the same proof the object-extension fold rests on one * collection over rather than a fresh assumption per rung. `objects` carries * disposition `objects`; `views`, `pages`, `actions`, `apps`, `dashboards` and * `flows` each carry `concat`. @@ -1199,7 +1166,7 @@ function objectExtensionsByTarget(stack: AnyRec): Map { * collector. ⛔ A name-only fold would resolve the object key and then judge * the owner's own field keys against an EMPTY fact set, reporting every one of * them as an orphan: the same false positive one level up, which is the mistake - * {@link objectExtensionsByTarget} records for a target it cannot see. + * the object-extension note above records for a target it cannot see. * * ⇒ And because the sibling's declaration IS readable here, this is NOT rung * 2b. The subtree stays judged, against the sibling's own fields and rules, so @@ -1214,7 +1181,7 @@ function objectExtensionsByTarget(stack: AnyRec): Map { * * ⛔ Nothing else is read off the entry, and there is no `stack.manifest` form * beside it: `objects` is a STACK collection, not a manifest key — the same - * asymmetry {@link objectExtensionsByTarget} records one collection over, and + * asymmetry the object-extension fold has one collection over, and * the reason the single-`defineStack` shape is untouched by this fold. */ function artifactProvidedRecords(stack: AnyRec, collection: string): AnyRec[] { @@ -1326,9 +1293,9 @@ function buildUniverse(stack: AnyRec): Universe { // // Folded AFTER the declaration loop above, into the facts it built, so the // two layers answer one question: is this name addressable at all. Only the - // two rungs an extension can reach are folded — see - // `objectExtensionsByTarget` for the measured surface, and ⛔ do not add the - // others. + // two rungs an extension can reach are folded — see the object-extension + // note above `artifactProvidedRecords` for the measured surface, and ⛔ do + // not add the others. // // A target this stack does NOT define gets no entry here: inventing one // would resolve the object key and then judge its fields against the @@ -1346,7 +1313,7 @@ function buildUniverse(stack: AnyRec): Universe { const facts = objects.get(target); if (!facts) continue; for (const extension of extensions) { - for (const field of recordsOf(extension.fields)) { + for (const field of extension.fields) { const fieldName = strName(field.name); if (!fieldName) continue; // The object's OWN declaration keeps the slot. Which layer's @@ -1360,7 +1327,7 @@ function buildUniverse(stack: AnyRec): Universe { // `_validations.` is keyed by the rule's own `name` (#14253), and // an extension's `validations[]` merge into the target's rule set — so // the same recursion the declaration loop uses, branches included. - for (const rule of recordsOf(extension.validations)) { + for (const rule of extension.validations) { collectValidationRuleNames(rule, facts.validations); } }