diff --git a/.changeset/20216-view-container-object-refused.md b/.changeset/20216-view-container-object-refused.md new file mode 100644 index 00000000000..353a41c6191 --- /dev/null +++ b/.changeset/20216-view-container-object-refused.md @@ -0,0 +1,57 @@ +--- +'@objectstack/lint': minor +--- + +fix(lint)!: a view container whose `object` names no object is refused by `os validate`, `os build` and `os lint` (`object-reference-unknown`), and the refusal names the namespace-prefixed object when that is the one the stack declares (#20216) + +Clause-②: no (narrowing) + +**BREAKING** — an accept-set narrowing on one authored key, shipped as `minor` under the +launch-window convention (`check-changeset-no-major` refuses `major` until GA; breaking-ness +is carried by this banner and the ADR-0087 disposition below, not by the level). + +**What changed.** `ViewSchema.object` is how a stack-level `views: [...]` container says +which object its views belong to, and it is the key the runtime indexes views by +(`getViewsByObject()` / `GET /meta/view?object=`). The schema declares it `z.string()`, and +nothing resolved it: `defineStack`'s cross-reference check reads a container's +`list.data` / `form.data` bindings, never the container's own key. So a container bound to a +name no object carries passed: `os validate` printed "Validation passed" and exited 0, +saying nothing about the view, and `os build` / `os lint` run the same rule table. At +runtime none of its views was found for any object. The common case is not a typo but a +missing namespace prefix — `object: 'order_line'` in a project whose object is +`my_app_order_line` — which is exactly what `os generate view` wrote in every namespaced +project until its template learned the prefix. + +The key now joins `validateObjectReferences` and rides the ladder every other object-name +site on that rule uses, resolved against the same set as a field's relationship target: + +1. the stack's own objects, or an object an entry of the artifact's `packages[]` provides → ok; +2. a known platform object (`PLATFORM_PROVIDED_OBJECT_NAMES`) → ok; +3. unresolved and not platform-prefixed → **`error`** `object-reference-unknown` at + `views[N].object`, so `os validate` / `os build` / `os lint` exit 1; +4. unresolved, platform-prefixed, registered by nothing → the existing + `object-reference-unregistered-platform` advisory. + +The refusal lists the objects the stack does declare, and when the bound name is exactly a +declared object minus the stack's `manifest.namespace` prefix, the hint names that prefixed +object outright. Not judged, on purpose: a container that carries no `object` (its binding +then falls back to `list.data.object` / `form.data.object` / its `name`, a different +reference), and a container authored at runtime (this rule does not run on a `view` write at +the runtime publish gate; that door is unchanged). + +## The accept set, before and after + +This is a behaviour table, not a rewrite: the FROM column is what the door did, the TO +column is what it does now. + +| where | FROM | TO | +|:--|:--|:--| +| `os validate`, `os build`, `os lint` on a view container bound to a name no object carries | exit 0, no finding | exit 1, `object-reference-unknown` at `views[N].object` | +| the same, on a platform-prefixed name nothing registers | exit 0, no finding | the `object-reference-unregistered-platform` advisory, exit unchanged | +| a runtime `view` write | unchanged | unchanged | + +Nothing an author writes changes spelling, and no key or value is retired. A container that +is refused was already dead at runtime; the finding's own hint says which object to bind it +to. + + diff --git a/packages/cli/test/generate-scaffold-validates.test.ts b/packages/cli/test/generate-scaffold-validates.test.ts index 2ba5543dfb5..0feac039304 100644 --- a/packages/cli/test/generate-scaffold-validates.test.ts +++ b/packages/cli/test/generate-scaffold-validates.test.ts @@ -132,8 +132,11 @@ afterAll(() => { fs.rmSync(TMP_ROOT, { recursive: true, force: true }); }); -/** A legal, minimal host stack. Only the collection under test is populated. */ -const hostStack = (collection: string, artifact: unknown) => ({ +/** + * A legal, minimal host stack: the collection under test, plus the objects a + * binding scaffold needs present (see {@link boundObjects}). + */ +const hostStack = (collection: string, artifact: unknown, objects: readonly unknown[] = []) => ({ manifest: { id: 'com.example.scaffold', name: 'scaffold', @@ -141,22 +144,52 @@ const hostStack = (collection: string, artifact: unknown) => ({ type: 'app' as const, namespace: 'scaffold', }, + ...(objects.length > 0 ? { objects: [...objects] } : {}), [collection]: [artifact], }); +/** Materialize one scaffold through the loader `os validate` uses (see the header). */ +async function loadScaffold(fileStem: string, source: string): Promise { + const file = path.join(TMP_ROOT, `${fileStem}.scaffold.ts`); + fs.writeFileSync(file, source, 'utf8'); + const { mod } = await bundleRequire({ filepath: file, external: BUNDLE_REQUIRE_EXTERNALS }); + return (mod as { default?: unknown }).default ?? mod; +} + +/** + * The object a BINDING scaffold names, as `os g object` writes it for the same + * name — the precondition the author's own project supplies. + * + * A generator flagged `namesObject` (other than `object` itself) writes a + * binding to `objectNameFor(STEM)`: a view container's `object`, an action's or + * a flow start node's `objectName`, an app nav entry's `objectName`. The + * templates are written to COMPOSE — `os g object NAME` then `os g view NAME` + * — so each binding names exactly the object the object scaffold declares. + * Validating a binding scaffold in a stack WITHOUT that object judged it against + * an empty object set, and once the author-time rules resolved a view + * container's `object` (`object-reference-unknown` at `views[0].object`), the + * harness's own omission read as the scaffold's defect. So the object is + * scaffolded here, through the same loader, and carried beside the artifact — + * ⛔ never special-cased in a rule, and ⛔ never the scaffold under test edited + * to fit the harness. + */ +async function boundObjects(type: string): Promise { + const target = GENERATOR_SCAFFOLD_TARGETS.find((t) => t.type === type); + if (!target?.namesObject || type === 'object') return []; + const objectTarget = GENERATOR_SCAFFOLD_TARGETS.find((t) => t.type === 'object'); + if (!objectTarget) throw new Error('the `object` generator must exist to seed a binding scaffold'); + return [await loadScaffold('bound-object', objectTarget.generate(STEM))]; +} + /** * Load a scaffold the way `os validate` loads authored TypeScript, then run * the two steps `Validate.run()` runs on it. */ async function validateScaffold(type: string, source: string) { - const file = path.join(TMP_ROOT, `${type}.scaffold.ts`); - fs.writeFileSync(file, source, 'utf8'); - - const { mod } = await bundleRequire({ filepath: file, external: BUNDLE_REQUIRE_EXTERNALS }); - const artifact = (mod as { default?: unknown }).default ?? mod; + const artifact = await loadScaffold(type, source); const normalized = normalizeStackInput( - hostStack(singularToPlural(type), artifact) as Record, + hostStack(singularToPlural(type), artifact, await boundObjects(type)) as Record, ) as Record; const unknownKeys = [ @@ -203,6 +236,22 @@ describe('[#14087] every `os generate` scaffold passes `os validate`', () => { expect(Object.keys(KNOWN_UNVALIDATED_SCAFFOLDS)).not.toContain('flow'); }); + it('a binding scaffold is judged beside the object `os g object` writes for the same name', async () => { + // The precondition `boundObjects` supplies is only honest while the view's + // binding and the object's name are the SAME spelling. Pinned directly, so + // a template drifting one side of the pair turns this red rather than + // quietly handing the view scaffold an object it does not bind. + const [object] = (await boundObjects('view')) as { name?: unknown }[]; + const view = GENERATOR_SCAFFOLD_TARGETS.find((t) => t.type === 'view'); + expect(view, 'the view generator must exist').toBeDefined(); + const container = (await loadScaffold('view-binding', view!.generate(STEM))) as { object?: unknown }; + expect(object?.name).toBe(STEM); + expect(container.object).toBe(object?.name); + // …and a non-binding generator is judged with no object carried at all. + expect(await boundObjects('dashboard')).toEqual([]); + expect(await boundObjects('object')).toEqual([]); + }); + const clean = GENERATOR_SCAFFOLD_TARGETS.filter((t) => !(t.type in KNOWN_UNVALIDATED_SCAFFOLDS)); const known = GENERATOR_SCAFFOLD_TARGETS.filter((t) => t.type in KNOWN_UNVALIDATED_SCAFFOLDS); diff --git a/packages/lint/src/validate-object-references.test.ts b/packages/lint/src/validate-object-references.test.ts index 831c430ea0d..10f4ab7ba0c 100644 --- a/packages/lint/src/validate-object-references.test.ts +++ b/packages/lint/src/validate-object-references.test.ts @@ -6,6 +6,7 @@ import { OBJECT_REFERENCE_UNKNOWN, OBJECT_REFERENCE_UNREGISTERED_PLATFORM, } from './validate-object-references.js'; +import { runAuthoringRules, splitBySeverity } from './authoring-rules.js'; /** Minimal stack with one own object, mirroring the HotCRM shape. */ const baseStack = () => ({ @@ -670,3 +671,121 @@ describe('[#19289] validateObjectReferences — a `user` target comes from the T expect(findings[0].rule).toBe(OBJECT_REFERENCE_UNKNOWN); }); }); + +/** + * [#20216] A view CONTAINER's own `object` — the key `getViewsByObject()` + * indexes by. Modelled on an `os init -t app` project (namespace `my_app`), + * where the object is `my_app_order_line` and the pre-fix `os generate view` + * template wrote the bare short name. + */ +describe('[#20216] validateObjectReferences — view container `object`', () => { + const appStack = (views: unknown, extra: Record = {}) => ({ + manifest: { id: 'com.example.my_app', namespace: 'my_app' }, + objects: [ + { name: 'my_app_order', fields: { name: { type: 'text' } } }, + { name: 'my_app_order_line', fields: { name: { type: 'text' } } }, + ], + views, + ...extra, + }); + const container = (object: string, name = 'order_line') => ({ + name, + label: 'Order Line', + object, + list: { type: 'grid', columns: [{ field: 'name' }] }, + }); + + it('refuses a container bound to the un-prefixed short name, and names the prefixed object', () => { + const findings = validateObjectReferences(appStack([container('order_line')])); + expect(findings).toHaveLength(1); + const [f] = findings; + expect(f.severity).toBe('error'); + expect(f.rule).toBe(OBJECT_REFERENCE_UNKNOWN); + expect(f.where).toBe('view "order_line"'); + expect(f.path).toBe('views[0].object'); + expect(f.message).toContain('view container object "order_line"'); + // The namespace prescription names the ONE spelling that resolves… + expect(f.hint).toContain('manifest.namespace "my_app"'); + expect(f.hint).toContain('write "my_app_order_line", not "order_line"'); + // …and the hint still lists every object the stack does declare. + expect(f.hint).toContain('Defined objects: my_app_order, my_app_order_line.'); + // What the miss costs, not only that it is a miss. + expect(f.hint).toContain('getViewsByObject()'); + }); + + it('CONTROL — the correctly prefixed container is clean', () => { + expect(validateObjectReferences(appStack([container('my_app_order_line')]))).toEqual([]); + }); + + it('reads the map form of `views` too, locating the finding by position', () => { + const findings = validateObjectReferences( + appStack({ order_line: { object: 'order_line', list: { type: 'grid', columns: ['name'] } } }), + ); + expect(findings.map((f) => [f.path, f.where, f.rule])).toEqual([ + ['views[0].object', 'view "order_line"', OBJECT_REFERENCE_UNKNOWN], + ]); + }); + + it('a miss that is NOT a missing prefix is refused without the namespace prescription', () => { + const findings = validateObjectReferences(appStack([container('invoice')])); + expect(findings).toHaveLength(1); + expect(findings[0].severity).toBe('error'); + expect(findings[0].path).toBe('views[0].object'); + // `my_app_invoice` is not declared, so prefixing is not the fix and is not offered. + expect(findings[0].hint).not.toContain('namespace prefix'); + expect(findings[0].hint).toContain('Defined objects: my_app_order, my_app_order_line.'); + }); + + it('passes a container over an object a `packages[]` sibling provides (rung ①, artifact scope)', () => { + const CORE_BODY = { id: 'com.example.core', objects: [{ name: 'crm_account', fields: {} }] }; + const UI_BODY = { id: 'com.example.ui', namespace: 'crm', views: [container('crm_account', 'crm_account')] }; + const perPackage = { ...UI_BODY, manifest: UI_BODY, packages: [{ manifest: UI_BODY }, { manifest: CORE_BODY }] }; + expect(validateObjectReferences(perPackage)).toEqual([]); + // Control: the same package judged ALONE refuses it — the context is what resolves it. + const alone = validateObjectReferences({ ...UI_BODY, manifest: UI_BODY }); + expect(alone.map((f) => [f.path, f.severity])).toEqual([['views[0].object', 'error']]); + }); + + it('keeps the platform ladder: a known platform object passes, an unregistered platform name advises', () => { + expect(validateObjectReferences(appStack([container('sys_user', 'sys_user')]))).toEqual([]); + const findings = validateObjectReferences(appStack([container('sys_approval_process', 'approvals')])); + expect(findings).toHaveLength(1); + expect(findings[0].severity).toBe('warning'); + expect(findings[0].rule).toBe(OBJECT_REFERENCE_UNREGISTERED_PLATFORM); + expect(findings[0].path).toBe('views[0].object'); + }); + + it('stays silent on a stack with no views, and on a container that carries no `object`', () => { + expect(validateObjectReferences(appStack(undefined))).toEqual([]); + expect(validateObjectReferences(appStack([]))).toEqual([]); + // No top-level `object`: the binding falls back to `list.data.object` / + // `name`, a different reference this leg does not own. + const unbound = { list: { type: 'grid', data: { provider: 'object', object: 'order_line' }, columns: ['name'] } }; + expect(validateObjectReferences(appStack([unbound]))).toEqual([]); + }); +}); + +/** + * [#20216] The same finding through the ONE rule table all three commands run + * (`runAuthoringRules`), at the gating tier — so `os validate`, `os build` and + * `os lint` exit 1 on it exactly as they do on the sibling relationship-target + * leg, rather than the rule merely existing. + */ +describe('[#20216] a dangling view container `object` gates every CLI command', () => { + const stack = (object: string) => ({ + manifest: { id: 'com.example.my_app', namespace: 'my_app' }, + objects: [{ name: 'my_app_order_line', fields: { name: { type: 'text' } } }], + views: [{ name: 'order_line', object, list: { type: 'grid', columns: [{ field: 'name' }] } }], + }); + const hits = (command: 'validate' | 'build' | 'lint', object: string) => + splitBySeverity(runAuthoringRules(command, { normalized: stack(object) as never })) + .errors.filter((f) => f.rule === OBJECT_REFERENCE_UNKNOWN) + .map((f) => f.path); + + for (const command of ['validate', 'build', 'lint'] as const) { + it(`\`${command}\` refuses the un-prefixed binding and passes the prefixed one`, () => { + expect(hits(command, 'order_line')).toEqual(['views[0].object']); + expect(hits(command, 'my_app_order_line')).toEqual([]); + }); + } +}); diff --git a/packages/lint/src/validate-object-references.ts b/packages/lint/src/validate-object-references.ts index befd1c7517b..ff59ed0e010 100644 --- a/packages/lint/src/validate-object-references.ts +++ b/packages/lint/src/validate-object-references.ts @@ -46,6 +46,11 @@ * typed. Dead → the record picker asks the REST layer for an object that * is not registered (404 `OBJECT_NOT_FOUND`), `$expand` on the field * fails, and the form renders a control that can never resolve a value. + * - a view container's own `object` (#20216) — `ViewSchema.object`, the + * binding a stack-level `views: [...]` entry uses to say which object its + * views belong to. `getViewsByObject()` indexes views by exactly this key, + * so a container bound to a name nothing registers is dead at runtime — + * none of its views is ever found — while authoring stayed green. * * ── Severity ladder (the point of the rule) ────────────────────────────── * @@ -328,6 +333,61 @@ export function validateObjectReferences(stack: AnyRec): ObjectRefFinding[] { } } + // ── View containers → the container's own `object` (#20216) ── + // `ViewSchema.object` is `z.string()`, and nothing resolved it: `defineStack`'s + // `validateCrossReferences` reads a container's `list.data` / `form.data` + // bindings, never the container's own key. Yet that key is the one the + // runtime indexes by — `getViewsByObject()` / `GET /meta/view?object=` match a + // view's `object` against the object asked for — so a container bound to a + // name no object carries is never found for ANY object. Measured at the + // public door with this leg disabled: `os validate` printed "Validation + // passed" and exited 0, saying nothing about the view, on + // `object: 'order_line'` in a project whose object is + // `my_app_order_line`. That is exactly the shape `os generate view` wrote in + // every namespaced project until its template learned the prefix, and any + // hand or AI author can still write it. + // + // The same ladder and the same `resolvable` set as the relationship leg above: + // this stack's objects plus what its `packages[]` provide (rung ①), a known + // platform object (rung ③), a platform-shaped miss advises (rung ④), and an + // unprefixed miss is the typo class and gates (rung ②). + // + // The one thing this leg adds is the namespace prescription. The dominant + // miss is not a typo but a MISSING PREFIX — ADR-0028 names every object + // `${manifest.namespace}_${shortName}`, and the short name is what an author + // naturally types — so when exactly that prefixed spelling is declared, the + // hint says so by name rather than leaving the author to infer it from the + // edit-distance suggestion. + // + // ⛔ A container with no `object` is not judged here: its binding then falls + // back to `list.data.object` / `form.data.object` / its `name` + // (`deriveViewContainerObject`), which is a different reference with its own + // owner. ⛔ Nor is a runtime-authored container — that is the runtime's door, + // and this member does not run on a `view` write (`REFERENCE_INTEGRITY_RULES`). + const namespace = strName((stack.manifest as AnyRec | undefined)?.namespace); + const views = recordsOf(stack.views); + for (let vi = 0; vi < views.length; vi++) { + const view = views[vi]; + const bound = strName(view.object); + if (!bound) continue; + const prefixed = namespace ? `${namespace}_${bound}` : undefined; + const namespaceHint = + prefixed && !bound.startsWith(`${namespace}_`) && resolvable.has(prefixed) + ? ` Object names carry the package namespace prefix (manifest.namespace "${namespace}"): ` + + `write "${prefixed}", not "${bound}".` + : ''; + check( + bound, + `view ${strName(view.name) ? `"${strName(view.name)}"` : `#${vi}`}`, + `views[${vi}].object`, + 'view container object', + 'The runtime indexes a container\'s views by this key (`getViewsByObject()` / ' + + '`GET /meta/view?object=`), so a container bound to an object nothing registers is ' + + 'never found: none of its views appears for any object.' + + namespaceHint, + ); + } + // ── Actions (global + object-embedded) → param object targets ── const checkActionParams = (action: AnyRec, actionPath: string, actionLabel: string) => { const params = recordsOf(action.params);