diff --git a/.changeset/24563-list-view-out-of-closure.md b/.changeset/24563-list-view-out-of-closure.md new file mode 100644 index 00000000000..ae959890d06 --- /dev/null +++ b/.changeset/24563-list-view-out-of-closure.md @@ -0,0 +1,36 @@ +--- +'@objectstack/lint': minor +--- + +fix(lint)!: the save door refuses a list view written into a package and bound outside that package's closure (#24563) + +Clause-②: yes (narrowing) + +**Why.** The runtime save door judges a write into a package against that package's closure (ruling #9612). The closure is the package's own objects, the objects of the packages its manifest declares in `dependencies`, platform/system objects, and provenance-less overlay rows; every other object is dropped from the snapshot before any rule runs. The list-view field rule read a binding missing from that snapshot as "an object this stack does not define" and skipped the whole list view. So a list view written into a package and bound to an object of a package it never declared, or to an object that exists nowhere, was stored with every field reference unjudged: `200`, no advisory. The same body as an overlay write is refused `422 list-view-field-unknown` for an unknown `columns` field. No other rule judged the binding on a `view` write, and such a view then fails at render ("This view's query was rejected"). + +**What is refused.** A list view written into a package whose bound object is outside the package's closure is refused with one `list-view-object-outside-closure` issue. The issue is located at the key the binding is read from: `views[0].data.object` for a standalone list view, otherwise the container's `objectName` / `object`. The hint names the remedy. An object that exists nowhere is refused the same way. This is the verdict the dataset door already gives an out-of-closure base object under the same closure (`object-reference-unknown`). An object written into a package whose built-in `listViews` entry retargets `data.object` outside the closure is refused the same way. The list view's field references stay unjudged until the binding is inside the closure. + +**BREAKING — what moves for consumers.** This is an accept-set narrowing on the metadata save door, shipped as `minor` under the launch-window convention for accept-set narrowings (`.changeset/pre.json` is in `exit` mode, so the next version pass cuts 18.0.0 and this change ships in it). + +- FROM: a publish-mode write of a list view into a package, bound outside the package's closure, answered `200` and was stored, with no issue. This covers `PUT /api/v1/meta/view/:name?package=…` (and `saveMetaItem` in publish mode), the promotion of a draft (`POST /api/v1/meta/view/:name/publish`, `publishMetaItem`), a package draft publish (`publishPackageDrafts`), and a package copy (`duplicatePackage`), which writes its copies through the same door. +- TO: `saveMetaItem` in publish mode, the draft promotion and `publishPackageDrafts` answer `422 INVALID_METADATA`, with the `list-view-object-outside-closure` issue at the binding, and nothing is stored. A package draft publish fails the item, and the batch with it. In `duplicatePackage`, a copied list view whose bound object stays outside the copy package's closure is refused with the same rule and listed in its `failed[]`. One way that happens is when that object's own copy was refused. +- Fix: declare a dependency on the package that defines the bound object in the `dependencies` of the writing package's manifest, or bind the view to an object the package may reach (its own, a declared dependency's, or a platform object). Then save again. Once the object is inside the closure its fields are judged: an unknown `columns` field is refused `422 list-view-field-unknown`, as at the overlay door. Saving the view as a draft (`mode: 'draft'`) is still allowed, because drafts are never gated; publishing that draft is judged. + +**The next save of a stored list view bound outside its package's closure is refused with the fix; stored views keep serving until then.** The door judges writes only. A stored row is not re-judged at boot, and it loads and is served as before. + +**Unchanged.** + +- A list view bound to an object inside the closure (the package's own, a declared dependency's, a platform/system object, an overlay row): same field verdicts, same refusals. +- A write that names no package (an overlay write) is judged against the whole collection, exactly as before. An object that exists nowhere stays unjudged there. +- The flow door: `flow-trigger-unknown-object` stays an advisory. +- `os validate`, `os build` and `os lint`: no closure is stated, so the rule skips an unresolvable binding as before. +- `OS_ALLOW_UNLINTED_METADATA_WRITES=1` still turns a refusal into a logged write. +- Measured before crossing: the 11 views shipped in this repository carry 39 list-view bindings, in `examples/app-crm`, `app-todo`, `app-showcase` and `app-multi-package`, the `create-objectstack` blank template, and `qa/downstream-contract`. None is outside its package's closure, and the door's `view` rule set refuses none of them, with the package's closure or without it. None of the 97 platform built-in list views retargets `data.object`. Deployed rows were not measured. + +**API.** + +- `AuthoringRuleContext` gains an optional `packageScope` (`{ packageId, dependencies }`). Only the runtime publish gate sets it, and only when it narrowed the snapshot's objects to a package closure. `runAuthoringRules` never sets it. +- `validateListViewFieldRefs(stack, options?)` accepts an optional `{ packageScope }` (`ListViewFieldRefOptions`, exported). +- `LIST_VIEW_OBJECT_OUTSIDE_CLOSURE` (`'list-view-object-outside-closure'`) is exported. + + diff --git a/content/docs/deployment/validating-metadata.mdx b/content/docs/deployment/validating-metadata.mdx index e238ec8691e..615d027c852 100644 --- a/content/docs/deployment/validating-metadata.mdx +++ b/content/docs/deployment/validating-metadata.mdx @@ -709,8 +709,11 @@ findings the item **added** (#4463). Its package dimension narrows that context the written item's **package closure** — the written package, its declared dependencies, and unconditionally both platform/system objects and rows carrying no package provenance (#9612); a provenance it cannot read narrows nothing at all. -That changes what a rule can **resolve**, never what it judges, and nothing at -this door walks `packages[]`. So "runs per package" names one thing at the three +That changes what a rule can **resolve**, never which rules run, and nothing at +this door walks `packages[]`. One rule is told the closure is in force: a list +view bound to an object outside it is refused with +`list-view-object-outside-closure` instead of having its field references +skipped (#24563). So "runs per package" names one thing at the three CLI doors and another one here, and the `runtime publish` column says neither. A clean run walks the registry and reports timing: diff --git a/packages/lint/src/authoring-rules.ts b/packages/lint/src/authoring-rules.ts index 2572e080ca6..e20b3e3df1a 100644 --- a/packages/lint/src/authoring-rules.ts +++ b/packages/lint/src/authoring-rules.ts @@ -372,6 +372,23 @@ export interface AuthoringRuleContext { * Read by `validateFlowApiTriggerSecret` only. */ restoredCredentialPaths?: ReadonlySet; + /** + * [#24563] The package closure (ruling #9612, `RuntimePackageScope` in + * `runtime-gate.ts`) the runtime publish gate narrowed the snapshot's + * `objects` to — the written package and the packages its manifest + * declares in `dependencies`. + * + * Set only by the runtime publish gate, and only when a closure was in + * force; ABSENT on the three CLI commands and on a write that names no + * package, whose snapshots carry the whole collection. It is what lets a + * rule tell "outside what this package declared it may reference" from + * "not in this stack": an object missing from a narrowed snapshot is the + * first, one missing from a whole one may come from another package. Read + * by `validateListViewFieldRefs` only (through the reference-integrity + * suite), which refuses a list view bound outside the closure instead of + * skipping it. + */ + packageScope?: { readonly packageId: string; readonly dependencies: readonly string[] }; } export interface AuthoringRule { diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index a384b3eec31..e93174a95f4 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -490,13 +490,18 @@ export type { // position whose name reaches a query door (the `$select` projection, or the // compiled filter), where that door refuses it by name — the loud-failing half // #14107 recorded and left open. +// [#24563] Its THIRD: a list view judged under a package closure whose bound +// object is outside that closure — refused (`error`) where skip 1 judged +// nothing; `ListViewFieldRefOptions` is how the runtime gate states the closure. export { validateListViewFieldRefs, LIST_VIEW_FIELD_UNKNOWN, LIST_VIEW_FIELD_DOTTED, + LIST_VIEW_OBJECT_OUTSIDE_CLOSURE, } from './validate-list-view-field-refs.js'; export type { ListViewFieldRefFinding, + ListViewFieldRefOptions, ListViewFieldRefSeverity, } from './validate-list-view-field-refs.js'; diff --git a/packages/lint/src/reference-integrity-suite.ts b/packages/lint/src/reference-integrity-suite.ts index 7889d149c29..469f7ed6b4f 100644 --- a/packages/lint/src/reference-integrity-suite.ts +++ b/packages/lint/src/reference-integrity-suite.ts @@ -306,7 +306,24 @@ export const REFERENCE_INTEGRITY_RULES: readonly ReferenceIntegrityRule[] = [ // here: it measured equally clean (0 findings over the same corpus) but its // crossing is its own adjudication with its own non-vacuity control, and // this card was scoped to the two axes its title names. - { name: 'validateListViewFieldRefs', runtimeTypes: ['flow', 'view', 'object'], run: validateListViewFieldRefs }, + // + // [#24563] The one member that reads `packageScope`. Under a package + // closure the snapshot's `objects` are that closure (ruling #9612), so a + // binding missing from it is outside what the package declared it may + // reference, or nowhere at all — and the member's skip 1 used to read that as + // "another package's object" and judge nothing. It now refuses the binding + // instead (`list-view-object-outside-closure`, `error`: the verdict the + // dataset door gives an out-of-closure base object under the same closure; + // the flow door's trigger-object advisory is a separate question). The + // missing-collection argument above is unchanged: the binding is judged + // against `stack.objects`, the collection every per-write snapshot carries, + // and the gate states the scope only when it narrowed that collection. + { + name: 'validateListViewFieldRefs', + runtimeTypes: ['flow', 'view', 'object'], + run: (stack, ctx) => + validateListViewFieldRefs(stack, ctx?.packageScope ? { packageScope: ctx.packageScope } : undefined), + }, // [#15254] The same question, one surface IN: the field-name lists the // OBJECT itself carries (`highlightFields`, `publicSharing.redactFields`), // which the three list-view members above do not walk because they are not @@ -657,6 +674,14 @@ export interface ReferenceIntegrityRunOptions { * `path` at the key the author actually wrote. */ loweredHookRefs?: ReadonlySet; + /** + * [#24563] The package closure the runtime publish gate narrowed + * `stack.objects` to — spells the same key as + * `AuthoringRuleContext.packageScope`. Present only when a closure was in + * force; ABSENT on the CLI commands and on a write that names no package. + * Read by `validateListViewFieldRefs` only. + */ + packageScope?: { readonly packageId: string; readonly dependencies: readonly string[] }; } /** diff --git a/packages/lint/src/runtime-gate.package-closure.test.ts b/packages/lint/src/runtime-gate.package-closure.test.ts index d8021d363ec..f1d6796c363 100644 --- a/packages/lint/src/runtime-gate.package-closure.test.ts +++ b/packages/lint/src/runtime-gate.package-closure.test.ts @@ -32,6 +32,7 @@ import { runRuntimeAuthoringRules, type RuntimePackageScope, } from './runtime-gate.js'; +import { validateListViewFieldRefs } from './validate-list-view-field-refs.js'; type AnyRec = Record; @@ -281,3 +282,154 @@ describe('ablations — the equivalence assertions above can fail (#9612)', () = expect(mutilated[0]).toContain('flow-trigger-unknown-object'); }); }); + +// ───────────────────────────────────────────────────────────────────────────── +// #24563 — the VIEW door under a package closure: a list view bound outside the +// closure is reported, never skipped; one bound inside keeps its field verdict. +// ───────────────────────────────────────────────────────────────────────────── + +/** + * A flattened list overlay — the shape `PUT /api/v1/meta/view` carries and the + * gate snapshots as `views: [item]` — bound to `object` with one known + * (`name`, every fixture object declares it) and optionally one unknown column. + */ +function listViewOn(object: string, columns: readonly string[] = ['name', 'zzz_nope']): AnyRec { + return { + name: `${object}.probe_list`, + object, + viewKind: 'list', + label: 'Probe', + type: 'grid', + data: { provider: 'object', object }, + columns: columns.map((field) => ({ field })), + }; +} + +describe('the view door under a package closure (#24563)', () => { + const OUTSIDE = 'list-view-object-outside-closure'; + const FIELD_UNKNOWN = 'list-view-field-unknown'; + const objects = [...packagedTenant(3), obj('tenant_overlay', undefined)]; + const scoped = (item: AnyRec, scope: RuntimePackageScope = SCOPE) => + runRuntimeAuthoringRules({ type: 'view', item, context: { objects }, packageScope: scope }); + const whole = (item: AnyRec) => runRuntimeAuthoringRules({ type: 'view', item, context: { objects } }); + const rules = (fs: AnyRec[]) => fs.map((f) => f.rule); + + it('REFUSES a binding into a package the written one never declared — at the binding, with the remedy', () => { + const result = scoped(listViewOn('other_0_0')); + + // An `error`, so the door refuses the write: the dataset door's verdict + // for an out-of-closure base object under the same closure. The flow + // door's `flow-trigger-unknown-object` (pinned above) stays advisory. + expect(result.advisories).toEqual([]); + expect(result.errors).toHaveLength(1); + expect(result.errors[0]).toMatchObject({ severity: 'error', rule: OUTSIDE }); + expect(result.errors[0].path).toMatch(/^views(\[0\]|\.other_0_0\.probe_list)\.data\.object$/); + // The fix names the remedy and the package it applies to. + expect(result.errors[0].hint).toContain(OWN); + expect(result.errors[0].hint).toContain('dependencies'); + // ⛔ The field positions stay unjudged: judging them against the whole + // collection would judge the package against what ruling #9612 says it may + // not reference. + expect(rules(result.errors)).not.toContain(FIELD_UNKNOWN); + }); + + it('REFUSES a binding to an object that exists nowhere the same way', () => { + const result = scoped(listViewOn('nosuch_obj')); + + expect(result.advisories).toEqual([]); + expect(rules(result.errors)).toEqual([OUTSIDE]); + }); + + it('REFUSES it under a closure that carries no object at all — an empty graph is still a verdict', () => { + const result = runRuntimeAuthoringRules({ + type: 'view', + item: listViewOn('nosuch_obj'), + context: { objects: [] }, + packageScope: SCOPE, + }); + + expect(rules(result.errors)).toEqual([OUTSIDE]); + }); + + it('REFUSES an unknown column on an object of the written package — the field verdict is unchanged', () => { + const result = scoped(listViewOn('crm_account')); + + expect(rules(result.errors)).toEqual([FIELD_UNKNOWN]); + expect(result.errors[0].path).toMatch(/columns\[1\]\.field$/); + expect(result.advisories).toEqual([]); + }); + + it('admits a known column on an object of the written package', () => { + const result = scoped(listViewOn('crm_account', ['name'])); + + expect(result.errors).toEqual([]); + expect(result.advisories).toEqual([]); + }); + + it('keeps a DECLARED DEPENDENCY inside the closure — known column admitted, unknown refused', () => { + // The discriminating case: a refusal of the binding here would mean the + // dependency limb never reached the rule. + expect(scoped(listViewOn('core_user', ['name']))).toMatchObject({ errors: [], advisories: [] }); + expect(rules(scoped(listViewOn('core_user')).errors)).toEqual([FIELD_UNKNOWN]); + }); + + it('keeps system objects inside the closure — by name prefix and by isSystem flag', () => { + for (const name of ['sys_audit_log', 'platform_ledger']) { + expect(scoped(listViewOn(name, ['name'])), name).toMatchObject({ errors: [], advisories: [] }); + expect(rules(scoped(listViewOn(name)).errors), name).toEqual([FIELD_UNKNOWN]); + } + }); + + it('keeps a provenance-less overlay object inside the closure — its field verdict is unchanged', () => { + expect(rules(scoped(listViewOn('tenant_overlay')).errors)).toEqual([FIELD_UNKNOWN]); + expect(scoped(listViewOn('tenant_overlay')).advisories).toEqual([]); + }); + + it('a write that names no package is unchanged — the whole collection, field verdict and silence alike', () => { + // ⛔ The overlay door: the stranger's object is in the whole collection, so + // its unknown column is refused exactly as before, and an object that + // exists nowhere stays skip 1's silence (no scope, so no closure verdict). + expect(rules(whole(listViewOn('other_0_0')).errors)).toEqual([FIELD_UNKNOWN]); + expect(whole(listViewOn('other_0_0')).advisories).toEqual([]); + expect(whole(listViewOn('nosuch_obj'))).toMatchObject({ errors: [], advisories: [] }); + // An empty package id narrows nothing, so it states no closure either. + const empty = { packageId: '', dependencies: [] } as RuntimePackageScope; + expect(scoped(listViewOn('nosuch_obj'), empty)).toMatchObject({ errors: [], advisories: [] }); + }); + + it('the flow verdict is unchanged, and a stored object\'s out-of-closure list view is not charged to it', () => { + // A context object of the written package whose built-in list view + // retargets an object outside the closure: the rule now refuses that list + // view, in the baseline and the candidate alike, so it cancels — a flow + // publish is not refused for a stored sibling's binding (#4463 D4). + const retargeting = obj('crm_board', OWN, { + listViews: { foreign: { label: 'Foreign', data: { provider: 'object', object: 'other_0_0' }, columns: ['name'] } }, + }); + const withSibling = [...objects, retargeting]; + const flow = (item: AnyRec) => + verdict(runRuntimeAuthoringRules({ type: 'flow', item, context: { objects: withSibling }, packageScope: SCOPE })); + + // Non-vacuous: written itself, that object IS refused — the list view + // rides the object door too, at the same verdict. + const asWritten = runRuntimeAuthoringRules({ type: 'object', item: retargeting, context: { objects }, packageScope: SCOPE }); + expect(rules(asWritten.errors)).toContain(OUTSIDE); + expect(rules(asWritten.advisories)).not.toContain(OUTSIDE); + + expect(flow(FLOW_INTO_OWN_PACKAGE)).toEqual([]); + const undeclared = flow(FLOW_INTO_UNDECLARED); + expect(undeclared).toHaveLength(1); + expect(undeclared[0]).toContain('flow-trigger-unknown-object'); + }); + + it('the CLI reading is unchanged: no scope stated, skip 1 judges nothing', () => { + // `os validate` / `os build` / `os lint` hand the rule no closure. + expect(validateListViewFieldRefs({ objects, views: [listViewOn('nosuch_obj')] })).toEqual([]); + // And the same stack under a stated closure is the verdict above. + expect( + validateListViewFieldRefs( + { objects, views: [listViewOn('nosuch_obj')] }, + { packageScope: SCOPE }, + ).map((f) => f.rule), + ).toEqual([OUTSIDE]); + }); +}); diff --git a/packages/lint/src/runtime-gate.ts b/packages/lint/src/runtime-gate.ts index 625a165b38b..3226f321906 100644 --- a/packages/lint/src/runtime-gate.ts +++ b/packages/lint/src/runtime-gate.ts @@ -416,7 +416,7 @@ export function narrowObjectsToPackageClosure( objects: readonly unknown[], scope: RuntimePackageScope | undefined, ): readonly unknown[] { - if (!scope || typeof scope.packageId !== 'string' || scope.packageId === '') return objects; + if (!packageScopeInForce(scope)) return objects; const reachable = new Set([scope.packageId, ...scope.dependencies]); return objects.filter((entry) => { if (!entry || typeof entry !== 'object') return true; @@ -433,6 +433,19 @@ export function narrowObjectsToPackageClosure( }); } +/** + * Whether `scope` narrows anything (#9612): a stated, non-empty package id. + * + * ONE predicate for both readers — {@link narrowObjectsToPackageClosure}, and + * {@link runRuntimeAuthoringRules} when it tells the rules a closure is in + * force (`AuthoringRuleContext.packageScope`, #24563) — so a rule can never be + * told "judged under a closure" about a snapshot that was not narrowed, or the + * reverse. + */ +function packageScopeInForce(scope: RuntimePackageScope | undefined): scope is RuntimePackageScope { + return !!scope && typeof scope.packageId === 'string' && scope.packageId !== ''; +} + /** * The provenance key the registry stamps an item's owning package onto. * `registry.ts` writes it (`_packageId = this.getObjectOwner(fqn)?.packageId`) @@ -1101,6 +1114,11 @@ export function runRuntimeAuthoringRules(args: { sduiManifest: args.sduiManifest, runtimeWriteType: args.type, judgeFilter: args.judgeFilter, + // [#24563] The closure the snapshots' `objects` were narrowed to, stated + // exactly when the builder above narrowed them (the same predicate), so a + // rule can tell an object outside the package's declared reach from one + // a whole-collection snapshot merely does not carry. + ...(packageScopeInForce(args.packageScope) ? { packageScope: args.packageScope } : {}), // [#20611] Spelled against the CANDIDATE, the one snapshot that holds the // written item. The baseline pass shares the set and cannot match it: the // item is not in the baseline, and every other entry sits at an index the diff --git a/packages/lint/src/validate-list-view-field-refs.ts b/packages/lint/src/validate-list-view-field-refs.ts index 8cf64884683..f38af8f0898 100644 --- a/packages/lint/src/validate-list-view-field-refs.ts +++ b/packages/lint/src/validate-list-view-field-refs.ts @@ -210,6 +210,52 @@ * ladder a local "not in this stack ⇒ error" would not have. When the bound * object does not resolve, this rule skips the whole list view (skip 1 * below) so one typo yields one finding rather than one per position. + * ONE exception, and it is this rule's own: a binding judged under a + * package closure (the next section). + * + * ## [#24563] A binding outside the written package's closure is REFUSED + * + * At the runtime save door, a write into a package is judged against that + * package's closure (ruling #9612, `RuntimePackageScope` in + * `runtime-gate.ts`): the package's own objects, its declared dependencies', + * platform/system objects and provenance-less overlay rows. Every other object + * is dropped from the snapshot before any rule runs. Skip 1 then read such a + * binding as "an object this stack does not define" and skipped the whole + * list view — so a list view written into a package and bound to an object + * the package never declared a dependency on (or to an object that exists + * nowhere) was stored with every field reference unjudged: `200`, no + * advisory, and an unknown `columns` field the overlay door refuses `422`. + * And no other rule judged the binding there: `validateObjectReferences` + * does not run on a `view` write. + * + * So when the gate states the closure it narrowed to + * (`ListViewFieldRefOptions.packageScope`), an unresolvable binding is + * reported here, once per list view, as + * {@link LIST_VIEW_OBJECT_OUTSIDE_CLOSURE} — never skipped: + * + * - **`error`**, so the save door refuses the publish-mode write (`422`): + * the package's declared `dependencies` bound what it MAY reference, so the + * binding is not resolvable BY DECLARATION. It is the verdict the dataset + * door already gives the same reference under the same closure + * (`object-reference-unknown` on `datasets[].object`). The flow door's + * `flow-trigger-unknown-object` stays advisory; that is its own question + * and this rule does not settle it. An object that exists nowhere lands on + * the same verdict, because the closure is all this snapshot can see. The + * door judges writes only: a stored row is judged at its next save, and a + * draft is never gated. + * - **The field positions stay unjudged** for that list view. Judging them + * against the whole collection would judge the package against objects + * ruling #9612 says it may not reference; the remedy the hint names — + * declare the dependency — is what puts the object, and with it the field + * judgement, inside the closure. + * - **Only under a stated closure.** The CLI commands and a write that names + * no package (an overlay write, the whole collection) pass no scope and + * keep skip 1 exactly as before: there an unresolved binding may come from + * another package, and `defineStack`'s cross-reference check and + * `validateObjectReferences` own it. + * + * Skip 2 is unchanged under a closure: an object that IS in the closure but + * declares no readable field map is still unjudgeable, never a finding. * * ## Skips — the same three every field-existence rule in this package takes * @@ -239,12 +285,19 @@ import { indexObjectGraph, isUnjudgeable, resolveFieldPath, + suggestName, type ObjectGraph, } from './object-graph.js'; /** A list-view field reference that resolves to no field on the bound object. */ export const LIST_VIEW_FIELD_UNKNOWN = 'list-view-field-unknown'; +/** + * [#24563] A list view, judged under a package closure, whose bound object is + * not in that closure — see the section of that name on this module. + */ +export const LIST_VIEW_OBJECT_OUTSIDE_CLOSURE = 'list-view-object-outside-closure'; + /** * [#14282] A list-view field reference written as a DOTTED path at a position * whose name reaches a query door — where the door refuses it by name. See the @@ -277,6 +330,18 @@ export interface ListViewFieldRefFinding { hint: string; } +/** + * What the caller states about the snapshot it hands this rule. + * + * `packageScope` — [#24563] the package closure the runtime publish gate + * narrowed `stack.objects` to (`RuntimePackageScope`), present only when one + * was in force. Absent on the CLI commands and on a write that names no + * package, which keep skip 1 as before. + */ +export interface ListViewFieldRefOptions { + packageScope?: { readonly packageId: string; readonly dependencies: readonly string[] }; +} + type AnyRec = Record; type Sev = ListViewFieldRefSeverity; @@ -607,12 +672,48 @@ function describeDottedRefusal( * safe on both the schema-parsed stack and the raw config the `lint` path * carries. */ -export function validateListViewFieldRefs(stack: AnyRec): ListViewFieldRefFinding[] { +export function validateListViewFieldRefs( + stack: AnyRec, + options?: ListViewFieldRefOptions, +): ListViewFieldRefFinding[] { const findings: ListViewFieldRefFinding[] = []; if (!isRec(stack)) return findings; const graph: ObjectGraph = indexObjectGraph(stack); - if (graph.size === 0) return findings; + const scope = options?.packageScope; + // [#24563] Under a stated closure an empty graph is still a verdict: every + // object binding is then outside it, and is refused below. + if (graph.size === 0 && !scope) return findings; + + /** + * [#24563] One list view whose binding is not in the stated closure — see + * the section of that name on this module. One finding per list view, at + * the key the binding was read from. + */ + const reportOutsideClosure = ( + closure: NonNullable, + object: string, + where: string, + path: string, + ): void => { + const pkg = closure.packageId; + findings.push({ + // `error`: the save door refuses it — see the section of that name on + // this module for why, and for the flow door's advisory. + severity: 'error', + rule: LIST_VIEW_OBJECT_OUTSIDE_CLOSURE, + where, + path, + message: + `binds object "${object}", which is outside package "${pkg}"'s closure (its own objects, the objects of ` + + `the packages its manifest declares in \`dependencies\`, and platform objects), so none of the view's ` + + `field references are checked.` + + suggestName(object, graph.keys()), + hint: + `Declare a dependency on the package that defines "${object}" in the \`dependencies\` of "${pkg}"'s ` + + `manifest, or bind the view to an object inside that closure`, + }); + }; /** * Judge ONE written reference at one position. @@ -690,8 +791,17 @@ export function validateListViewFieldRefs(stack: AnyRec): ListViewFieldRefFindin }); }; - /** Every position on ONE list view record. */ - const checkListView = (listView: AnyRec, object: string | undefined, where: string, path: string): void => { + /** + * Every position on ONE list view record. `bindingPath` is the key the + * binding `object` was read from, where an unresolvable binding is reported. + */ + const checkListView = ( + listView: AnyRec, + object: string | undefined, + where: string, + path: string, + bindingPath: string, + ): void => { if (!isRec(listView)) return; // ── Skip 4 (this surface's own): a list view bound to a non-`object` @@ -707,7 +817,14 @@ export function validateListViewFieldRefs(stack: AnyRec): ListViewFieldRefFindin // An unresolvable base object is `validate-object-references.ts`' finding; // resolving anything against it here would turn one typo into a finding // per position. - if (!graph.has(object) || !graph.get(object)) return; + // [#24563] Except under a stated package closure, where the base object + // was dropped by the closure or exists nowhere: refused once, and the + // positions stay unjudged (see the section of that name on this module). + if (!graph.has(object)) { + if (scope) reportOutsideClosure(scope, object, where, bindingPath); + return; + } + if (!graph.get(object)) return; // Bound to a `const` so the narrowing survives into the nested closures // below (a parameter's narrowing does not). const bound: string = object; @@ -804,6 +921,14 @@ export function validateListViewFieldRefs(stack: AnyRec): ListViewFieldRefFindin return isRec(data) ? strName(data.object) : undefined; }; + /** + * [#24563] The key a list view's binding was read from: its own + * `data.object` when it declares one, otherwise the fallback the caller + * resolved the binding through. + */ + const bindingPathOf = (listView: AnyRec, path: string, fallback: string): string => + listViewObject(listView) !== undefined ? `${path}.data.object` : fallback; + // ── An object's built-in named list views ── // The same rungs `validate-searchable-fields` and `validate-sortable-fields` // walk, so the three field axes on this surface cannot cover different sets @@ -824,6 +949,7 @@ export function validateListViewFieldRefs(stack: AnyRec): ListViewFieldRefFindin listViewObject(lv) ?? objName, `${label} › listViews.${key}`, `objects[${oi}].listViews.${key}`, + bindingPathOf(lv, `objects[${oi}].listViews.${key}`, `objects[${oi}].name`), ); } } @@ -843,6 +969,7 @@ export function validateListViewFieldRefs(stack: AnyRec): ListViewFieldRefFindin // declares none — the same resolution order `validate-list-view-mode` // reads. const viewObject = strName(view.objectName) ?? strName(view.object); + const viewObjectPath = strName(view.objectName) ? `views[${vi}].objectName` : `views[${vi}].object`; if (view.viewKind === 'list' && !isRec(view.config)) { checkListView( @@ -850,6 +977,7 @@ export function validateListViewFieldRefs(stack: AnyRec): ListViewFieldRefFindin listViewObject(view) ?? viewObject, `view "${viewLabel}" (flattened list overlay)`, `views[${vi}]`, + bindingPathOf(view, `views[${vi}]`, viewObjectPath), ); } @@ -859,6 +987,7 @@ export function validateListViewFieldRefs(stack: AnyRec): ListViewFieldRefFindin listViewObject(view.config) ?? viewObject, `view "${viewLabel}" (ViewItem record)`, `views[${vi}].config`, + bindingPathOf(view.config, `views[${vi}].config`, viewObjectPath), ); } @@ -868,6 +997,7 @@ export function validateListViewFieldRefs(stack: AnyRec): ListViewFieldRefFindin listViewObject(view.list) ?? viewObject, `view "${viewLabel}" › list`, `views[${vi}].list`, + bindingPathOf(view.list, `views[${vi}].list`, viewObjectPath), ); } @@ -879,6 +1009,7 @@ export function validateListViewFieldRefs(stack: AnyRec): ListViewFieldRefFindin listViewObject(lv) ?? viewObject, `view "${viewLabel}" › listViews.${key}`, `views[${vi}].listViews.${key}`, + bindingPathOf(lv, `views[${vi}].listViews.${key}`, viewObjectPath), ); } } diff --git a/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts b/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts index 4991e862ea5..5b62ceefecd 100644 --- a/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts +++ b/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts @@ -3486,3 +3486,238 @@ describe('runtime authoring gate on PAGE writes — a component `data` naming th expect(JSON.parse(activePageRows(rows)[0]!.metadata)).toEqual(fixed); }); }); + +// A list view written into a PACKAGE is judged against that package's closure +// (ruling #9612): the package, its declared dependencies, platform objects and +// provenance-less rows. Through the real `saveMetaItem`, `publishMetaItem` and +// `publishPackageDrafts`: a binding outside the closure — an object of a +// package this one never declared, or an object that exists nowhere — is +// REFUSED `422 INVALID_METADATA` at the binding, with the remedy (declare the +// dependency), never skipped; it is the verdict the dataset door gives an +// out-of-closure base object under the same closure (the flow door's trigger +// advisory, `protocol.package-closure-gate.test.ts`, is a separate question). +// A binding inside the closure keeps its field verdict, so an unknown +// `columns` field is refused `422 list-view-field-unknown` as the overlay door +// refuses it. A write that names no package is judged against the whole +// collection, exactly as before. Drafts are not gated; a stored row is judged +// at its next save. +describe('runtime authoring gate on VIEW writes into a package — the closure verdict (#24563)', () => { + const OWN = 'com.acme.crm'; + const DEP = 'com.acme.core'; + const STRANGER = 'com.other.analytics'; + const PLATFORM = 'com.objectstack.platform'; + const OUTSIDE = 'list-view-object-outside-closure'; + const FIELD_UNKNOWN = 'list-view-field-unknown'; + + const object = (name: string, packageId: string) => ({ + name, + label: name, + _packageId: packageId, + fields: [{ name: 'title', type: 'text', label: 'Title' }], + }); + /** The card's body: a flattened list overlay bound to `bound`, one known and one unknown column by default. */ + const listView = (bound: string, columns: readonly string[] = ['title', 'zzz_nope']) => ({ + name: `${bound}.qavfy_r1`, + object: bound, + viewKind: 'list', + label: 'r1', + type: 'grid', + data: { provider: 'object', object: bound }, + columns: columns.map((field) => ({ field })), + }); + + /** A registry holding three packages — the written one, the one it declares, a stranger — and a platform object. */ + function makePackagedProtocol() { + const { engine, rows } = makeStubEngine(); + const packages: Record }> = { + [OWN]: { manifest: { name: OWN, version: '1.0.0', dependencies: { [DEP]: '^1.0.0' } } }, + [DEP]: { manifest: { name: DEP, version: '1.0.0' } }, + [STRANGER]: { manifest: { name: STRANGER, version: '1.0.0' } }, + }; + engine.registry.listItems = (type: string) => + type === 'object' + ? [ + object('crm_account', OWN), + object('core_user', DEP), + object('showcase_task', STRANGER), + object('sys_widget_log', PLATFORM), + ] + : []; + engine.registry.getPackage = (id: string) => packages[id]; + const protocol = new ObjectStackProtocolImplementation(engine, () => new Map(), 'env_test') as any; + return { protocol, rows }; + } + + const saveView = (protocol: any, item: { name: string }, extra: Record = {}) => + protocol.saveMetaItem({ type: 'view', name: item.name, item, ...extra }); + const activeViewRows = (rows: Map) => + Array.from(rows.values()).filter((r) => r.type === 'view' && r.state === 'active'); + const rulesOf = (findings: any[] | undefined) => (findings ?? []).map((f: any) => f.rule); + /** The one outside-closure refusal issue, with the shape the door returns. */ + const expectOutsideClosure = (issues: any[] | undefined) => { + expect(rulesOf(issues), JSON.stringify(issues)).toEqual([OUTSIDE]); + expect(issues![0]).toMatchObject({ severity: 'error', path: 'views[0].data.object' }); + // The fix names the remedy and the package it applies to. + expect(issues![0].hint).toContain(OWN); + expect(issues![0].hint).toContain('dependencies'); + }; + /** A refused publish-mode write: the 422 envelope, carrying the closure issue. */ + const expectRefusedOutsideClosure = (err: any, what: string) => { + expect(err, `${what}: the write resolved — the door still accepts the out-of-closure binding`).toBeInstanceOf(Error); + expect({ code: err.code, status: err.status }).toEqual({ code: 'INVALID_METADATA', status: 422 }); + expectOutsideClosure(err.issues); + }; + + let warn: ReturnType; + beforeEach(() => { + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + delete process.env.OS_ALLOW_UNLINTED_METADATA_WRITES; + }); + afterEach(() => { + warn.mockRestore(); + delete process.env.OS_ALLOW_UNLINTED_METADATA_WRITES; + }); + + it('(a) REFUSES an active save bound to a package the written one never declared — 422 at the binding, nothing stored', async () => { + const { protocol, rows } = makePackagedProtocol(); + + const err = await saveView(protocol, listView('showcase_task'), { packageId: OWN }).catch((e: any) => e); + + expectRefusedOutsideClosure(err, 'stranger object'); + // A gate that refuses after persisting is a log line. + expect(activeViewRows(rows)).toEqual([]); + }); + + it('(a) REFUSES an active save bound to an object that exists nowhere the same way', async () => { + const { protocol, rows } = makePackagedProtocol(); + + const err = await saveView(protocol, listView('nosuch_obj'), { packageId: OWN }).catch((e: any) => e); + + expectRefusedOutsideClosure(err, 'nosuch_obj'); + expect(activeViewRows(rows)).toEqual([]); + }); + + it('(a) a DRAFT save of the same body is not gated — the draft door stays open, its promotion is judged', async () => { + const { protocol } = makePackagedProtocol(); + + await expect(saveView(protocol, listView('showcase_task'), { packageId: OWN, mode: 'draft' })) + .resolves.toMatchObject({ success: true }); + }); + + it('(b) REFUSES an unknown `columns` field on an object inside the closure — own, declared dependency, platform', async () => { + for (const bound of ['crm_account', 'core_user', 'sys_widget_log']) { + const { protocol, rows } = makePackagedProtocol(); + + const err = await saveView(protocol, listView(bound), { packageId: OWN }).catch((e: any) => e); + + expect(err, `${bound}: the save resolved — the field verdict was skipped`).toBeInstanceOf(Error); + expect({ code: err.code, status: err.status }).toEqual({ code: 'INVALID_METADATA', status: 422 }); + expect(err.issues.map((i: any) => [i.rule, i.path])).toEqual([[FIELD_UNKNOWN, 'views[0].columns[1].field']]); + expect(activeViewRows(rows)).toEqual([]); + } + }); + + it('(b) admits a known `columns` field on an object inside the closure, with no advisory', async () => { + for (const bound of ['crm_account', 'core_user', 'sys_widget_log']) { + const { protocol, rows } = makePackagedProtocol(); + + const result = await saveView(protocol, listView(bound, ['title']), { packageId: OWN }); + + expect(result, bound).toMatchObject({ success: true }); + expect(rulesOf(result.advisories), bound).toEqual([]); + expect(activeViewRows(rows)).toHaveLength(1); + } + }); + + it('(c) a write that names no package is unchanged — the overlay door refuses the stranger column and stays silent on nowhere', async () => { + const overlay = makePackagedProtocol(); + const err = await saveView(overlay.protocol, listView('showcase_task')).catch((e: any) => e); + expect({ code: err?.code, status: err?.status }).toEqual({ code: 'INVALID_METADATA', status: 422 }); + expect(err.issues.map((i: any) => [i.rule, i.path])).toEqual([[FIELD_UNKNOWN, 'views[0].columns[1].field']]); + expect(activeViewRows(overlay.rows)).toEqual([]); + + const nowhere = makePackagedProtocol(); + const result = await saveView(nowhere.protocol, listView('nosuch_obj')); + expect(result).toMatchObject({ success: true }); + expect(rulesOf(result.advisories)).toEqual([]); + }); + + it('(d) the draft → active promotion refuses the out-of-closure binding, and the in-closure unknown field', async () => { + const outside = makePackagedProtocol(); + await expect(saveView(outside.protocol, listView('showcase_task'), { packageId: OWN, mode: 'draft' })) + .resolves.toMatchObject({ success: true }); + const promoted = await outside.protocol + .publishMetaItem({ type: 'view', name: 'showcase_task.qavfy_r1', packageId: OWN }) + .catch((e: any) => e); + expectRefusedOutsideClosure(promoted, 'promotion'); + expect(activeViewRows(outside.rows)).toEqual([]); + + const inside = makePackagedProtocol(); + await saveView(inside.protocol, listView('crm_account'), { packageId: OWN, mode: 'draft' }); + const err = await inside.protocol + .publishMetaItem({ type: 'view', name: 'crm_account.qavfy_r1', packageId: OWN }) + .catch((e: any) => e); + expect({ code: err?.code, status: err?.status }).toEqual({ code: 'INVALID_METADATA', status: 422 }); + expect(rulesOf(err.issues)).toEqual([FIELD_UNKNOWN]); + }); + + it('(e) a package draft publish refuses the out-of-closure binding, and the in-closure unknown field', async () => { + const outside = makePackagedProtocol(); + await saveView(outside.protocol, listView('showcase_task'), { packageId: OWN, mode: 'draft' }); + const published = await outside.protocol.publishPackageDrafts({ packageId: OWN }); + expect(published).toMatchObject({ success: false, publishedCount: 0 }); + expect(published.failed[0]).toMatchObject({ name: 'showcase_task.qavfy_r1', code: 'INVALID_METADATA' }); + expectOutsideClosure(published.failed[0].issues); + expect(activeViewRows(outside.rows)).toEqual([]); + + const inside = makePackagedProtocol(); + await saveView(inside.protocol, listView('crm_account'), { packageId: OWN, mode: 'draft' }); + const refused = await inside.protocol.publishPackageDrafts({ packageId: OWN }); + expect(refused).toMatchObject({ success: false, publishedCount: 0 }); + expect(refused.failed[0]).toMatchObject({ name: 'crm_account.qavfy_r1', code: 'INVALID_METADATA' }); + expect(rulesOf(refused.failed[0].issues)).toEqual([FIELD_UNKNOWN]); + }); + + it('(g) a stored view with the binding keeps loading; its next save into the package is refused, the row untouched', async () => { + const { protocol, rows } = makePackagedProtocol(); + // Stored before the crossing, seeded straight into the store because the + // save door now refuses this body. + const stored = listView('showcase_task', ['title']); + rows.set(keyOf({ type: 'view', name: stored.name, organization_id: null, state: 'active' }), { + id: 'r_seed', type: 'view', name: stored.name, organization_id: null, state: 'active', + metadata: JSON.stringify(stored), + }); + + // The boot path runs no authoring rule. + await expect(protocol.loadMetaFromDb()).resolves.toMatchObject({ loaded: 1, errors: 0, invalid: 0 }); + + // The next save is a write, and is judged. + const err = await saveView(protocol, stored, { packageId: OWN }).catch((e: any) => e); + expectRefusedOutsideClosure(err, 'next save of a stored row'); + expect(JSON.parse(activeViewRows(rows)[0]!.metadata)).toEqual(stored); + }); + + it('(f) a view bound to an object drafted in the SAME batch is inside the closure — no phantom refusal, its fields judged', async () => { + const batchObject = { + name: 'crm_ticket', + label: 'Ticket', + sharingModel: 'private', + fields: { title: { type: 'text', label: 'Title' } }, + }; + + const clean = makePackagedProtocol(); + await clean.protocol.saveMetaItem({ type: 'object', name: 'crm_ticket', item: batchObject, packageId: OWN, mode: 'draft' }); + await saveView(clean.protocol, listView('crm_ticket', ['title']), { packageId: OWN, mode: 'draft' }); + const res = await clean.protocol.publishPackageDrafts({ packageId: OWN }); + expect(res.failed).toEqual([]); + expect(rulesOf(res.published.find((p: any) => p.name === 'crm_ticket.qavfy_r1')?.advisories)).not.toContain(OUTSIDE); + + const broken = makePackagedProtocol(); + await broken.protocol.saveMetaItem({ type: 'object', name: 'crm_ticket', item: batchObject, packageId: OWN, mode: 'draft' }); + await saveView(broken.protocol, listView('crm_ticket'), { packageId: OWN, mode: 'draft' }); + const refused = await broken.protocol.publishPackageDrafts({ packageId: OWN }); + const causal = refused.failed.find((f: any) => f.name === 'crm_ticket.qavfy_r1'); + expect(causal?.code).toBe('INVALID_METADATA'); + expect(rulesOf(causal.issues)).toEqual([FIELD_UNKNOWN]); + }); +}); diff --git a/packages/objectql/src/protocol-duplicate-package-names-in-step.test.ts b/packages/objectql/src/protocol-duplicate-package-names-in-step.test.ts index 68b7be398fe..a631b60143e 100644 --- a/packages/objectql/src/protocol-duplicate-package-names-in-step.test.ts +++ b/packages/objectql/src/protocol-duplicate-package-names-in-step.test.ts @@ -237,7 +237,13 @@ describe('ADR-0070 D4 — duplicatePackage keeps a copied name in step with what await protocol.installPackage({ manifest: { id: R, name: 'Other', type: 'app' } }); await save(R, 'object', accountBody); await save(R, 'object', extBody); - await protocol.installPackage({ manifest: { id: P, name: 'Source', type: 'app' } }); + // [#24563] The source DECLARES the third package it references: the save + // door judges a package's list view against the package's closure and + // refuses a binding outside it (`list-view-object-outside-closure`), so + // `dupext_account.board` and `dupsrc_ext.board` are views of objects + // outside the package that the package may reach. The copy inherits the + // declaration with the rest of the manifest. + await protocol.installPackage({ manifest: { id: P, name: 'Source', type: 'app', dependencies: { [R]: '*' } } }); expect(engine.registry.getPackage(P)?.manifest?.namespace).toBe('dupsrc'); await save(P, 'object', customerBody); await save(P, 'object', ticketBody); diff --git a/packages/objectql/src/protocol-duplicate-package-unprefixed-object.test.ts b/packages/objectql/src/protocol-duplicate-package-unprefixed-object.test.ts index e033e2d3be8..91303a5fa7d 100644 --- a/packages/objectql/src/protocol-duplicate-package-unprefixed-object.test.ts +++ b/packages/objectql/src/protocol-duplicate-package-unprefixed-object.test.ts @@ -284,8 +284,8 @@ describe('ADR-0070 D4 — duplicatePackage of an object whose name lacks the sou const res = await protocol.duplicatePackage({ sourcePackageId: P, targetPackageId: Q }); expect(res.success).toBe(false); - expect(res.failed).toHaveLength(1); - const [refusal] = res.failed; + expect(res.failed).toHaveLength(2); + const [refusal, viewRefusal] = res.failed; expect(refusal).toMatchObject({ type: 'object', name: 'ticket' }); // Located: the object, the name it would take, who holds that name, // and the source rename that clears it. @@ -293,6 +293,11 @@ describe('ADR-0070 D4 — duplicatePackage of an object whose name lacks the sou expect(refusal.error).toContain("'dupdst_ticket'"); expect(refusal.error).toContain("'dupsrc_ticket'"); expect(refusal.error).toContain("Rename 'ticket' on the source"); + // [#24563] The `ticket` copy was refused, so the copy of `ticket_views` + // keeps the source's `ticket` — an object outside the copy package's + // closure — and the save door refuses it with the same rule. + expect(viewRefusal).toMatchObject({ type: 'view', name: 'ticket_views' }); + expect(viewRefusal.error).toContain('views[0].object [list-view-object-outside-closure]'); // NEGATIVE CONTROL: the prefixed sibling took the name, with its own // body, and nothing of `ticket` was written under it. diff --git a/packages/objectql/src/publish-package-drafts-response-conformance.test.ts b/packages/objectql/src/publish-package-drafts-response-conformance.test.ts index 649b12e4e12..690f0426fdd 100644 --- a/packages/objectql/src/publish-package-drafts-response-conformance.test.ts +++ b/packages/objectql/src/publish-package-drafts-response-conformance.test.ts @@ -149,8 +149,16 @@ async function makeProtocol() { const PKG = 'app.edu'; +/** + * A list view bound to an object the package may reach (#24563): the save door + * judges a write into a package against that package's closure, and refuses a + * binding outside it (`list-view-object-outside-closure`). `app.edu` declares + * no dependency and owns no object here, so the binding is a platform object + * the harness registers — kept in every package's closure — and `id` is a + * field it declares. The view is advisory-free, which is what these cases need. + */ const viewBody = (name: string, label: string) => ({ - name, type: 'grid', label, columns: ['id'], object: 'case', viewKind: 'list', + name, type: 'grid', label, columns: ['id'], object: 'sys_metadata', viewKind: 'list', }); /** diff --git a/packages/qa/dogfood/test/view-container-cross-package-default.dogfood.test.ts b/packages/qa/dogfood/test/view-container-cross-package-default.dogfood.test.ts index 1c99c16abd7..a0951f8f9a6 100644 --- a/packages/qa/dogfood/test/view-container-cross-package-default.dogfood.test.ts +++ b/packages/qa/dogfood/test/view-container-cross-package-default.dogfood.test.ts @@ -138,9 +138,16 @@ describe('dogfood: a bare-list container on another package\'s object leaves its // 1. Baseline. await expectDefaultUnchangedOnBothDoors(); - // 2. A Studio-created package to author into. + // 2. A Studio-created package to author into. It DECLARES the showcase as a + // dependency (#24563): the save door judges a package's view against the + // package's closure and refuses a binding outside it + // (`list-view-object-outside-closure`), so a container for an object + // another package ships is authored from a package that may reach it. const created = await stack.apiAs(token, 'POST', '/packages', { - manifest: { id: REPAIR, name: 'Repair assets', version: '0.1.0', type: 'app', namespace: 'repair' }, + manifest: { + id: REPAIR, name: 'Repair assets', version: '0.1.0', type: 'app', namespace: 'repair', + dependencies: { [SHOWCASE]: '*' }, + }, }); expect(created.status).toBe(201);