diff --git a/.changeset/studio-object-field-ref-refusal.md b/.changeset/studio-object-field-ref-refusal.md new file mode 100644 index 0000000000..bbed1c08b0 --- /dev/null +++ b/.changeset/studio-object-field-ref-refusal.md @@ -0,0 +1,26 @@ +--- +"@objectstack/lint": minor +"@objectstack/metadata-protocol": minor +--- + +A publish now refuses an object whose `highlightFields` names a field that does not exist on it — the same gate that refuses a code-authored stack. + +`list-view-field-unknown` inspects `view.columns`, and Studio's app builder mints no `view` items at all, so the reference-integrity family had nothing to inspect on the only artifacts the click path authors. What it authors is the **object**, and an object-level field-name list was covered by nothing that could refuse: measured on `origin/main`, `runtimeAuthoringRulesFor('object')` dispatched seven rules with no reference-integrity rule among them, while the object-level existence check that did exist (`semantic-role-field-unknown`) is `warning`, advisory-tier and CLI-only. So `os validate` exited 0 on a dangling reference and the runtime publish door — the only door a Studio, REST `/meta` or MCP author has — said nothing at all. + +The reproduction is the natural click order, not a contrived one: click-create a field (Studio mints it as `field_10`), add it to `highlightFields`, then give it a label — the API name auto-derives to `health_score` and `highlightFields` keeps `field_10`. Anyone who names a field after placing it produces this. + +- **New rule `object-field-ref-unknown` (`error`)**, in `@objectstack/lint`, over the object-level field-name **lists** that no rule owned: `highlightFields` (ADR-0085) and `publicSharing.redactFields`. It resolves through the same `object-graph` seam as the rest of the family, so the three shared skips hold — an object outside the stack, an object with no readable field map (ADR-0015 `external`), and a registry-injected system column resolved **per object** (`highlightFields: ['owner_id']` is a live pointer on an owned object and a real miss under `ownership: 'none'`). +- **It runs on the runtime publish door.** The reference-integrity suite entry's `runtimeTypes` gains `object`, and the suite's per-member declaration keeps the crossing narrow: this is the only member that judges an object snapshot; every other member keeps `['flow', 'view']` or the frozen `['flow']` default. +- **`validateSemanticRoles` keeps the provenance question** at the same position (`semantic-role-field-unprovisioned`, still `warning`) and no longer restates existence — one finding per path, at one tier. +- **`probes.checked` gained an `objects` counter.** Its absence was the tell: a receipt reading `{seeds: 0, views: 0, widgets: 0}` was accurate while the objects the package published were probed by nothing. + +## Migration + +**A publish that used to succeed can now be refused (HTTP 422, `INVALID_METADATA`).** The receipt names the rule id `object-field-ref-unknown` and the offending path, name-keyed on the wire — for example `objects.proj_task.highlightFields[1]` — plus the string that was written and the fields the object actually has. + +To fix a dangling reference, do one of: + +- rewrite the entry to the field's current API name (after a Studio label edit the derived name is the one to use — `field_10` becomes `health_score`); or +- drop the entry from the list. + +`os validate` / `os build` / `os lint` report the same finding at `error`, so a stack can be repaired before it reaches a publish. If an object legitimately points at a platform-injected system column, no change is needed — the rule resolves those per object and stays silent where the platform really provisions them. diff --git a/packages/lint/src/authoring-rules.ts b/packages/lint/src/authoring-rules.ts index edbf225371..e2796765e3 100644 --- a/packages/lint/src/authoring-rules.ts +++ b/packages/lint/src/authoring-rules.ts @@ -682,8 +682,37 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ // crossing would be a silent no-op that reads as coverage — the very shape // #9313 was filed about. Flow snapshots are unchanged: every member keeps // the default `flow` declaration. + // + // [#15254] `object` joins, and the granularity mechanism above is what + // makes it a NARROW crossing rather than the whole suite arriving at the + // hottest write door: this entry says an `object` write dispatches the + // suite; the suite's per-member `runtimeTypes` says exactly ONE member + // judges that snapshot (`validateObjectFieldRefs`), because it is the + // only member that resolves an object's references against the object's + // OWN field map. Every other member keeps `['flow', 'view']` (or the + // frozen `['flow']` default) and does not run on an object write. + // + // The measured state that forced it: `runtimeAuthoringRulesFor('object')` + // dispatched seven rules and no reference-integrity rule among them, so + // the ONLY door a Studio tenant has ran no reference-integrity judgement + // at all on the artifact Studio actually authors — while the app builder + // mints no `view` items, leaving the members crossed by #9313 with + // nothing to inspect on the click path. That is not a rule that missed a + // case; it is a wall whose fourth door was pointed at a surface the click + // path never produces. + // + // ⚠️ This is a REFUSAL widening on the object door, and unlike #4716 it is + // not fenced by the advisory tier: an object republished with a + // pre-existing dangling `highlightFields` entry is now refused (422) + // rather than warned. That is the acceptance bar of the card, stated in + // its own words ("Warning-level is not enough for the claim; it has to + // refuse"), and it is called out in the changeset's Migration section + // because it turns a publish that used to succeed into one that does not. + // The gate's differential keeps it honest in the one direction that + // matters: a STORED sibling already in violation is never charged to this + // write (#4463 D4). surfaces: CLI_AND_RUNTIME, - runtimeTypes: ['flow', 'view'], + runtimeTypes: ['flow', 'view', 'object'], run: (stack, ctx) => validateReferenceIntegrity(stack, ctx), }, // ADR-0078 / #5068 — the SDUI component-props gate. `PageComponent.properties` diff --git a/packages/lint/src/index.ts b/packages/lint/src/index.ts index 273467ff0f..a865b97f38 100644 --- a/packages/lint/src/index.ts +++ b/packages/lint/src/index.ts @@ -454,6 +454,22 @@ export type { ListViewFieldRefSeverity, } from './validate-list-view-field-refs.js'; +// [#15254] The object-level half of the same sweep: the field-name LISTS an +// object carries about its own fields (`highlightFields`, +// `publicSharing.redactFields`). `error`, and on the runtime publish door as +// well as the three commands — Studio's app builder mints no `view` items, so +// the list-view members above have nothing to inspect on the only artifacts +// the click path authors, and a dangling `highlightFields` reference produced +// by clicking in the natural order published green. +export { + validateObjectFieldRefs, + OBJECT_FIELD_REF_UNKNOWN, +} from './validate-object-field-refs.js'; +export type { + ObjectFieldRefFinding, + ObjectFieldRefSeverity, +} from './validate-object-field-refs.js'; + export { validateActionNameRefs, ACTION_NAME_UNDEFINED } from './validate-action-name-refs.js'; export type { ActionNameRefFinding, ActionNameRefSeverity } from './validate-action-name-refs.js'; diff --git a/packages/lint/src/reference-integrity-suite.test.ts b/packages/lint/src/reference-integrity-suite.test.ts index 6e907edb5e..fba734b3bd 100644 --- a/packages/lint/src/reference-integrity-suite.test.ts +++ b/packages/lint/src/reference-integrity-suite.test.ts @@ -22,6 +22,12 @@ describe('reference-integrity suite — membership', () => { // field-naming position the two members above do not own. Placed beside // them because the three walk the identical rungs. 'validateListViewFieldRefs', + // [#15254] The object-level half of the same sweep: the field-name lists + // an object writes about its OWN fields. Placed beside the list-view + // members because it completes the object's field surface — and because + // Studio's app builder mints no `view` items, which is what left those + // members with nothing to inspect on the click path. + 'validateObjectFieldRefs', 'validateActionNameRefs', 'validatePageFieldBindings', // [#14073] The same page, one question out: the BINDING behind each diff --git a/packages/lint/src/reference-integrity-suite.ts b/packages/lint/src/reference-integrity-suite.ts index d90e64c914..205ab87ce7 100644 --- a/packages/lint/src/reference-integrity-suite.ts +++ b/packages/lint/src/reference-integrity-suite.ts @@ -103,6 +103,7 @@ import { validateObjectReferences } from './validate-object-references.js'; import { validateSearchableFields } from './validate-searchable-fields.js'; import { validateSortableFields } from './validate-sortable-fields.js'; import { validateListViewFieldRefs } from './validate-list-view-field-refs.js'; +import { validateObjectFieldRefs } from './validate-object-field-refs.js'; import { validateActionNameRefs } from './validate-action-name-refs.js'; import { validatePageFieldBindings } from './validate-page-field-bindings.js'; import { validatePageVisualizationBindings } from './validate-page-visualization-bindings.js'; @@ -232,6 +233,41 @@ export const REFERENCE_INTEGRITY_RULES: readonly ReferenceIntegrityRule[] = [ // MCP/AI author writes goes through that door and no CLI, so a // build-time-only rule would never reach the author who made the typo. { name: 'validateListViewFieldRefs', runtimeTypes: ['flow', 'view'], run: validateListViewFieldRefs }, + // [#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 + // on a list view. Placed directly after them because it completes the same + // sweep — every field name an object or its built-in views write down. + // + // `runtimeTypes` is `['flow', 'object']` — the ONLY member of this suite + // that names `object`, and `flow` for the floor the suite keeps. + // + // It names `object` because that is the point of the member: Studio's app + // builder mints no `view` items at all, so the list-view members have + // nothing to inspect on the only artifacts the click path authors. What it + // authors is the OBJECT. The crossing carries the #9313 property that makes + // it safe — this member resolves only against `stack.objects`, the + // collection the per-write snapshot does carry, so it has no + // missing-collection false-positive channel; and it resolves each name + // against the object's OWN field map, so a one-object snapshot is not + // merely sufficient, it is the whole universe the question has. + // + // It names `flow` because EVERY member of this suite does — the #4463 P1 + // surface is the floor the member axis was never meant to narrow, and + // `runtime-gate.view-writes.test.ts` pins it as an invariant over the whole + // roster rather than a preference per member. Worth being plain about what + // it buys here: on a flow snapshot the objects are CONTEXT, present in the + // baseline and the candidate alike, so anything this member could raise + // there cancels in the gate's differential (#4463 D4 — a stored object + // already in violation is never charged to someone else's write). So it + // adds a pass, not a verdict. That is the right trade against being the + // first member to leave the floor. + // + // It does NOT name `view`, and that IS an argued omission: the `crossed` + // list in that same test is written out precisely so each view crossing is + // argued, and this member judges no list view. A view write cannot change + // an object's own field-name lists. + { name: 'validateObjectFieldRefs', runtimeTypes: ['flow', 'object'], run: validateObjectFieldRefs }, { name: 'validateActionNameRefs', run: validateActionNameRefs }, { name: 'validatePageFieldBindings', run: validatePageFieldBindings }, // [#14073] The same page, one question out. `validatePageFieldBindings` diff --git a/packages/lint/src/runtime-gate.object-writes.test.ts b/packages/lint/src/runtime-gate.object-writes.test.ts index f76ac3bb5c..816efe5545 100644 --- a/packages/lint/src/runtime-gate.object-writes.test.ts +++ b/packages/lint/src/runtime-gate.object-writes.test.ts @@ -113,6 +113,13 @@ describe('the object write door dispatches at the adjudicated scope (#4716)', () 'validateFunctionalCompleteness', 'validateManagedApiMethods', 'validatePresetComparands', // #8793 — at this door before #4716 + // [#15254] The reference-integrity suite, dispatched here so its ONE + // object-judging member runs (`validateObjectFieldRefs`). The entry + // arrives; the suite's per-member `runtimeTypes` decides who judges the + // snapshot, and every other member keeps `['flow','view']` or the frozen + // `['flow']` default. Before this, the only door a Studio tenant has ran + // no reference-integrity rule at all on an object write. + 'validateReferenceIntegrity', 'lintAutonumberFormats', 'validateSecurityPosture', // #8310 — at this door before #4716 'validateRuleCompilability', @@ -240,6 +247,7 @@ describe('the object write door dispatches at the adjudicated scope (#4716)', () 'validateFunctionalCompleteness', 'validateManagedApiMethods', 'validatePresetComparands', + 'validateReferenceIntegrity', // [#15254] 'lintAutonumberFormats', 'validateSecurityPosture', 'validateRuleCompilability', diff --git a/packages/lint/src/validate-object-field-refs.test.ts b/packages/lint/src/validate-object-field-refs.test.ts new file mode 100644 index 0000000000..b3d084a9d4 --- /dev/null +++ b/packages/lint/src/validate-object-field-refs.test.ts @@ -0,0 +1,326 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, expect, it } from 'vitest'; + +import { AUTHORING_RULES } from './authoring-rules.js'; +import { + OBJECT_FIELD_REF_UNKNOWN, + validateObjectFieldRefs, +} from './validate-object-field-refs.js'; +import { runRuntimeAuthoringRules, runtimeAuthoringRulesFor } from './runtime-gate.js'; +import { SEMANTIC_ROLE_FIELD_UNKNOWN, validateSemanticRoles } from './validate-semantic-roles.js'; + +const obj = (over: Record = {}) => ({ + name: 'proj_task', + label: 'Task', + sharingModel: 'private', + fields: { + name: { type: 'text', label: 'Name' }, + health_score: { type: 'number', label: 'Health Score' }, + }, + nameField: 'name', + ...over, +}); + +const stackOf = (over: Record = {}) => ({ objects: [obj(over)] }); + +describe('validateObjectFieldRefs — highlightFields', () => { + it('REFUSES a dangling entry at `error`, naming the rule id and the offending path', () => { + const findings = validateObjectFieldRefs(stackOf({ highlightFields: ['name', 'field_10'] })); + + expect(findings).toHaveLength(1); + expect(findings[0]).toMatchObject({ + severity: 'error', + rule: OBJECT_FIELD_REF_UNKNOWN, + path: 'objects[0].highlightFields[1]', + }); + // The author must read back the string they typed and the object it was + // resolved against — a finding that names neither is unactionable. + expect(findings[0]!.message).toContain('field_10'); + expect(findings[0]!.message).toContain('proj_task'); + // …and the fields that DO exist, so the fix is one read away. + expect(findings[0]!.hint).toContain('health_score'); + }); + + it('passes a list whose every entry names a real field', () => { + expect(validateObjectFieldRefs(stackOf({ highlightFields: ['name', 'health_score'] }))) + .toEqual([]); + }); + + it('reports EVERY dangling entry, not only the first', () => { + const findings = validateObjectFieldRefs( + stackOf({ highlightFields: ['nope_one', 'name', 'nope_two'] }), + ); + expect(findings.map((f) => f.path)).toEqual([ + 'objects[0].highlightFields[0]', + 'objects[0].highlightFields[2]', + ]); + }); + + it('judges the retired `compactLayout` spelling at the same position (raw `lint` input)', () => { + // Not an accepted spelling — `ObjectSchema` refuses it and the ADR-0085 + // conversion normalizes it away before the parsed tier. Read here only so + // the raw `lint` path keeps the coverage this clause took over from + // `validateSemanticRoles`. See the rule's module note. + const findings = validateObjectFieldRefs(stackOf({ compactLayout: ['name', 'field_10'] })); + expect(findings).toHaveLength(1); + expect(findings[0]).toMatchObject({ + severity: 'error', + rule: OBJECT_FIELD_REF_UNKNOWN, + path: 'objects[0].compactLayout[1]', + }); + }); +}); + +describe('validateObjectFieldRefs — the Studio click path (#15254)', () => { + // The reproduction from the card, in the order an author actually clicks: + // 1. click-create a Number field → it is minted as `field_10` + // 2. add it to `highlightFields` → the list references `field_10` + // 3. set its label to "Health Score" → the API name auto-derives to + // `health_score`, and `highlightFields` still says `field_10` + // Naming a field after placing it is the natural order, so this is the + // shape ANY author produces — not a contrived mutation. + const afterDerivedRename = stackOf({ + // step 3 has happened: the field is `health_score` … + fields: { name: { type: 'text' }, health_score: { type: 'number', label: 'Health Score' } }, + // … and step 2's reference was never rewritten. + highlightFields: ['field_10'], + }); + + it('REFUSES the derived-rename scenario `field_10` → `health_score`', () => { + const findings = validateObjectFieldRefs(afterDerivedRename); + expect(findings).toHaveLength(1); + expect(findings[0]!.severity).toBe('error'); + expect(findings[0]!.rule).toBe(OBJECT_FIELD_REF_UNKNOWN); + expect(findings[0]!.path).toBe('objects[0].highlightFields[0]'); + }); + + it('the SAME body publishes clean once the reference is rewritten', () => { + expect(validateObjectFieldRefs(stackOf({ + fields: { name: { type: 'text' }, health_score: { type: 'number' } }, + highlightFields: ['health_score'], + }))).toEqual([]); + }); + + it('the RUNTIME publish door refuses it — the door a Studio tenant actually has', () => { + // The whole point of the card: this is the fourth wall (#4463), reached by + // Studio, REST `/meta` and MCP authors alike, and it is the ONLY one a + // tenant has. Before #15254 it dispatched no reference-integrity rule at + // all on an object write. + expect(runtimeAuthoringRulesFor('object').map((r) => r.name)) + .toContain('validateReferenceIntegrity'); + + const result = runRuntimeAuthoringRules({ + type: 'object', + item: afterDerivedRename.objects[0], + context: { objects: [] }, + }); + + const refusal = result.errors.find((f) => f.rule === OBJECT_FIELD_REF_UNKNOWN); + expect(refusal, JSON.stringify(result.errors)).toBeDefined(); + // Name-keyed on the wire (#10064), which is the path the card asks to see + // on screen: `objects..highlightFields[i]`, never a snapshot index. + expect(refusal!.path).toBe('objects.proj_task.highlightFields[0]'); + expect(refusal!.severity).toBe('error'); + // `rulesRun` is non-empty, so "clean" and "nothing ran" stay distinguishable. + expect(result.rulesRun).toContain('validateReferenceIntegrity'); + }); + + it('a clean object still publishes through that door', () => { + const result = runRuntimeAuthoringRules({ + type: 'object', + item: obj({ highlightFields: ['name', 'health_score'] }), + context: { objects: [] }, + }); + expect(result.errors, JSON.stringify(result.errors)).toEqual([]); + }); + + it('does not blame this write for a STORED sibling already dangling (#4463 D4)', () => { + const stored = obj({ name: 'legacy_thing', highlightFields: ['long_gone'] }); + const result = runRuntimeAuthoringRules({ + type: 'object', + item: obj({ highlightFields: ['name'] }), + context: { objects: [stored] }, + }); + expect(result.errors, JSON.stringify(result.errors)).toEqual([]); + }); +}); + +describe('validateObjectFieldRefs — publicSharing.redactFields', () => { + it('REFUSES a dangling redaction — the one that fails OPEN', () => { + const findings = validateObjectFieldRefs(stackOf({ + publicSharing: { enabled: true, redactFields: ['helth_score'] }, + })); + expect(findings).toHaveLength(1); + expect(findings[0]).toMatchObject({ + severity: 'error', + rule: OBJECT_FIELD_REF_UNKNOWN, + path: 'objects[0].publicSharing.redactFields[0]', + }); + // The consequence sentence is the point of the position: it is not that + // the field renders short, it is that it is SERVED. + expect(findings[0]!.message).toMatch(/fails OPEN/i); + // A near-miss carries the suggestion. + expect(findings[0]!.message).toContain('health_score'); + }); + + it('passes a redaction list whose entries all resolve', () => { + expect(validateObjectFieldRefs(stackOf({ + publicSharing: { enabled: true, redactFields: ['health_score'] }, + }))).toEqual([]); + }); +}); + +describe('validateObjectFieldRefs — the three shared skips', () => { + it('skip 3: a registry-injected system column is a LIVE pointer, not a miss (#5378)', () => { + expect(validateObjectFieldRefs({ + objects: [obj({ ownership: 'user', highlightFields: ['name', 'owner_id'] })], + })).toEqual([]); + }); + + it('skip 3, the other direction: `ownership: none` injects no owner_id, so it IS a miss', () => { + const findings = validateObjectFieldRefs({ + objects: [obj({ ownership: 'none', highlightFields: ['owner_id'] })], + }); + expect(findings.map((f) => f.rule)).toEqual([OBJECT_FIELD_REF_UNKNOWN]); + }); + + it('skip 2: an object with no readable field map is never judged (ADR-0015 external)', () => { + expect(validateObjectFieldRefs({ + objects: [{ + name: 'remote_thing', + external: { remoteName: 'things', writable: false }, + highlightFields: ['whatever_the_remote_calls_it'], + }], + })).toEqual([]); + }); + + it('is inert on junk: no objects, unnamed entries, non-array lists, non-string members', () => { + expect(validateObjectFieldRefs({})).toEqual([]); + expect(validateObjectFieldRefs({ objects: [] })).toEqual([]); + // An entry with no `name` indexes into no graph and is skipped. + expect(validateObjectFieldRefs({ objects: [{ highlightFields: ['x'] }] })).toEqual([]); + expect(validateObjectFieldRefs(stackOf({ highlightFields: 'not-an-array' }))).toEqual([]); + expect(validateObjectFieldRefs(stackOf({ highlightFields: [null, 3, ''] }))).toEqual([]); + // A name-keyed `objects` map, the other shape the raw `lint` path carries. + expect(validateObjectFieldRefs({ + objects: { proj_task: { fields: { name: {} }, highlightFields: ['nope'] } }, + })).toHaveLength(1); + // ⛔ NOT asserted here: `objects: [null, …]`. `indexObjectGraph` throws a + // TypeError on a null collection entry before any member of this suite is + // reached — a pre-existing fragility of the shared seam, filed separately + // rather than worked around in one member (a local guard here would leave + // the same crash in every sibling and hide it). + }); +}); + +// --------------------------------------------------------------------------- +// [#5378] The injected-column derivation, at the position that moved. +// +// These counter-examples used to stand in `validate-semantic-roles.test.ts`, +// against the warning-tier clause this rule took over. They are re-pinned here +// at `error` rather than dropped: the derivation is exactly as load-bearing +// under a gate as it was under an advisory, and a false finding now REFUSES a +// publish instead of adding a line to a warning list. +// --------------------------------------------------------------------------- +describe('validateObjectFieldRefs — the #5378 derivation under a gate', () => { + const tag = (over: Record) => ({ + objects: [{ name: 'crm_tag', fields: { name: {} }, ...over }], + }); + + it.each(['none', 'org'])( + "REFUSES highlightFields: [owner_id] on ownership: '%s' — the platform injects none", + (ownership) => { + const findings = validateObjectFieldRefs(tag({ + ownership, highlightFields: ['name', 'owner_id'], + })); + expect(findings).toHaveLength(1); + expect(findings[0]).toMatchObject({ severity: 'error', rule: OBJECT_FIELD_REF_UNKNOWN }); + expect(findings[0]!.message).toContain('owner_id'); + }, + ); + + it('REFUSES an organization_id highlight when the object opts out of tenancy', () => { + const findings = validateObjectFieldRefs(tag({ + tenancy: { enabled: false }, highlightFields: ['organization_id'], + })); + expect(findings).toHaveLength(1); + expect(findings[0]!.message).toContain('organization_id'); + }); + + it('REFUSES a typo that merely LOOKS like a system column', () => { + const findings = validateObjectFieldRefs({ + objects: [{ name: 'crm_contact', fields: { name: {} }, highlightFields: ['owner_ids', 'creatd_at'] }], + }); + expect(findings).toHaveLength(2); + expect(findings.map((f) => f.message).join(' ')).toMatch(/owner_ids/); + expect(findings.map((f) => f.message).join(' ')).toMatch(/creatd_at/); + }); + + it.each([ + 'created_at', 'created_by', 'updated_at', 'updated_by', + 'organization_id', 'owning_business_unit_id', 'id', + ])('stays silent on highlightFields: [%s] — a real column at render time', (column) => { + expect(validateObjectFieldRefs({ + objects: [{ name: 'crm_contact', fields: { name: {} }, highlightFields: ['name', column] }], + })).toEqual([]); + }); + + it('a withheld anchor on an EXTERNAL object is still the existence verdict (#8116)', () => { + const findings = validateObjectFieldRefs({ + objects: [{ + name: 'ext_customer', + external: { remoteName: 'customers' }, + fields: { email: { type: 'email' } }, + ownership: 'none', + highlightFields: ['owner_id'], + }], + }); + expect(findings).toHaveLength(1); + expect(findings[0]!.rule).toBe(OBJECT_FIELD_REF_UNKNOWN); + }); +}); + +describe('the clause that moved out of validateSemanticRoles', () => { + it('semantic roles no longer double-reports highlightFields EXISTENCE', () => { + const stack = stackOf({ highlightFields: ['field_10'] }); + const semantic = validateSemanticRoles(stack).filter( + (f) => f.rule === SEMANTIC_ROLE_FIELD_UNKNOWN && f.path.includes('highlightFields'), + ); + expect(semantic, JSON.stringify(semantic)).toEqual([]); + // One finding on the path, at the gating tier, not two at two tiers. + expect(validateObjectFieldRefs(stack)).toHaveLength(1); + }); + + it('semantic roles KEEPS stageField — a scalar role pointer, still advisory', () => { + const findings = validateSemanticRoles(stackOf({ stageField: 'pipeline' })); + expect(findings.map((f) => f.rule)).toContain(SEMANTIC_ROLE_FIELD_UNKNOWN); + expect(findings.find((f) => f.rule === SEMANTIC_ROLE_FIELD_UNKNOWN)!.severity).toBe('warning'); + // …and this rule does not take it: lists only, see the module note. + expect(validateObjectFieldRefs(stackOf({ stageField: 'pipeline' }))).toEqual([]); + }); + + it('semantic roles KEEPS the PROVENANCE question at the highlightFields position', () => { + // An ADR-0015 external object: the injected anchor RESOLVES (so this rule + // is silent, skip 3) but nothing provisions storage behind it (#8116). + const external = { + name: 'remote_thing', + external: { remoteName: 'things', writable: false }, + fields: { email: { type: 'text' } }, + ownership: 'user', + highlightFields: ['email', 'owner_id'], + }; + const findings = validateSemanticRoles({ objects: [external] }); + expect(findings.map((f) => f.rule)).toContain('semantic-role-field-unprovisioned'); + }); +}); + +describe('registry wiring', () => { + it('reaches all three commands through the reference-integrity suite entry', () => { + const entry = AUTHORING_RULES.find((r) => r.name === 'validateReferenceIntegrity')!; + expect(entry.tier).toBe('gating'); + expect(entry.commands).toEqual(expect.arrayContaining(['validate', 'build', 'lint'])); + expect(entry.surfaces).toContain('runtime-publish'); + expect(entry.runtimeTypes).toContain('object'); + }); +}); diff --git a/packages/lint/src/validate-object-field-refs.ts b/packages/lint/src/validate-object-field-refs.ts new file mode 100644 index 0000000000..140acef4dd --- /dev/null +++ b/packages/lint/src/validate-object-field-refs.ts @@ -0,0 +1,291 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#15254 — object-level field-name list reference integrity] Every field name + * an OBJECT names in one of its own field-name LISTS — `highlightFields` + * today — must name a field that object actually has. + * + * ## The state this rule ends, measured on `origin/main` (f01adfa5c) + * + * The sibling `validate-list-view-field-refs.ts` answers the same question for + * a LIST VIEW, and between them the two siblings that predate it + * (`validate-searchable-fields`, `validate-sortable-fields`) cover a list + * view's remaining field axes. Nothing answered it for the object's OWN + * pointers, at the tier that refuses: + * + * ``` + * highlightFields[i] warning/semantic-role-field-unknown (advisory, cli-only) + * publicSharing.redactFields[i] nothing + * ``` + * + * The gap is not that the miss went unreported — for `highlightFields` it was + * reported, as an ADVISORY on the CLI alone. It is that no surface REFUSED it, + * and one surface never saw it at all. Both halves were measured: + * + * - `os validate` / `os build` / `os lint` reported + * `semantic-role-field-unknown` at `warning`, so the command exits 0 and + * the stack is declared valid. An author who reads the verdict rather than + * the warning list ships the dangling pointer. + * - The RUNTIME publish door reported nothing whatsoever. + * `runtimeAuthoringRulesFor('object')` dispatched seven rules and the + * reference-integrity suite was not among them (its entry declares + * `runtimeTypes: ['flow', 'view']`), while `validateSemanticRoles` is held + * off that door entirely by the #4716 advisory-volume fence. So the only + * door a Studio tenant, a REST `/meta` author or an MCP/AI author has ran + * NO reference-integrity rule at all on an object write. + * + * ## Why the click path produces this, in the natural order + * + * Studio's app builder mints no `view` items, so every rule in the list-view + * half of this family has nothing to inspect on the artifacts Studio actually + * authors. What it DOES author is the object, and the object's + * `highlightFields`. The reproduction is not contrived: place a field (it is + * minted as `field_10`), add it to `highlightFields`, then give it a label — + * the API name auto-derives to `health_score` and `highlightFields` keeps + * `field_10`. Naming a field after placing it is the natural order, so any + * author who does it produces a dangling reference, and the publish accepted + * it (`outcome: 'published'`, `failedCount: 0`). + * + * ## Severity: `error`, and why this family's two-tier rule lands here + * + * The list-view sibling grades per position, on whether the miss is REFUSED + * downstream or merely renders wrong. Neither test is the one that decides + * this rule, because an object-level list is consumed by renderers that all + * degrade quietly — nothing 400s, and that is precisely the complaint. The + * deciding property is the one ADR-0078 names: the reference is PARSED, + * UNMARKED and SILENTLY INERT, on the authoring surface AI authors and humans + * share, and the platform's own asymmetry (a code author is warned, a click + * author is not told at all) is what makes it a defect rather than a hint. + * Warning is the tier that produced the measured state above; it is not + * enough for this judgement, so both surfaces gate. + * + * ## What this rule owns, and what it deliberately does NOT + * + * It owns the object-level keys whose value is a LIST of names of fields on + * THAT object, and that no other rule already resolves: + * + * - **`highlightFields[]`** (ADR-0085, `packages/spec/src/data/object.zod.ts` + * line 2092) — drives default list columns, cards, previews and the detail + * highlight strip. A dangling entry is dropped by every consumer. + * - **`publicSharing.redactFields[]`** (`object.zod.ts` line 2258) — the + * field names stripped from records served through a share link. This one + * fails OPEN, which the schema's own `history` note says in as many words: + * a redaction the author wrote and mis-spelled does not redact, and the + * field is served to whoever holds the link. Same shape, worse consequence. + * + * NOT owned, each with the reason its schema gives: + * + * - **`searchableFields[]`** — `validate-searchable-fields.ts` owns it, at + * `error`, with a runtime-admissibility verdict on top of existence. + * Re-measured on this base: a dangling entry already reports + * `searchable-field-unknown`. + * - **`listViews.*`** — the three list-view members of this suite own every + * field-naming position inside a built-in list view. Re-measured: a + * dangling `listViews.all.columns` entry already reports + * `list-view-field-unknown`. + * - **`stageField` / `nameField` / `displayNameField`** — SCALAR pointers, + * not lists, and the first is `validateSemanticRoles`' at `warning` while + * the title pair is `validateRecordTitle`'s axis. Promoting a scalar role + * pointer to `error` is the same judgement one key over, but it is a + * separate decision with its own blast radius and it is left to one. + * - **`indexes[].fields[]`** — a list of names, but the question there is a + * STORAGE one (does the driver create the index?), owned by the index + * registration path, and it is answered against the physical column set + * rather than the authored field map. + * - **`tenancy.tenantField` / `tenancy.organizationField` / + * `lifecycle.ttl.field` / `activityMilestones[].field`** — scalars, and + * the first three habitually name REGISTRY-INJECTED columns + * (`organization_id`, `created_at`), which is the #5378 false-finding trap; + * they are judgeable, but each needs its own injected-column measurement. + * - **`external.columnMap` / `external.ignoreColumns` / `systemFields`** — + * by their schema these are REMOTE column names and system-column registry + * names, not names in this object's own field map. Out by definition, not + * by deferral. + * - **`titleFormat`** — a template expression, owned by the expression rules. + * + * ## The retired `compactLayout` alias + * + * `compactLayout` was renamed to `highlightFields` in `@objectstack/spec` + * 11.7.0 (ADR-0085) and the `object-compactLayout-to-highlightFields` + * conversion normalizes it before a parsed stack reaches any rule, so on the + * parsed tier this alias cannot appear. It is read here anyway, at the same + * position, for one reason and not as a tolerance: the clause this rule takes + * over from `validateSemanticRoles` read it, and the `lint` path carries raw + * config that has not been through the conversion. Dropping the read would be + * a silent coverage regression on that path, which is the failure mode this + * whole family exists to end. It is NOT an accepted spelling — the parse + * refuses it — and nothing else in this file widens to an alias. + * + * ## Skips — the same three every field-existence rule in this package takes + * + * Resolution is {@link resolveFieldPath}'s and its `unknowable` verdicts are + * never reported (ADR-0072 D1): an object this stack does not define, an + * object that declares no readable field map (ADR-0015 `external`, + * datasource-introspected schemas), and a registry-injected system column — + * the last resolved per object, so `highlightFields: ['owner_id']` is a live + * pointer on an owned object and a real miss under `ownership: 'none'` + * (#5378). + */ + +import { + describeFieldPathVerdict, + indexObjectGraph, + isUnjudgeable, + resolveFieldPath, + type ObjectGraph, +} from './object-graph.js'; + +/** An object-level field-name list entry that resolves to no field on the object. */ +export const OBJECT_FIELD_REF_UNKNOWN = 'object-field-ref-unknown'; + +export type ObjectFieldRefSeverity = 'error' | 'warning'; + +export interface ObjectFieldRefFinding { + /** Always `error` — see the severity note on this module. */ + severity: ObjectFieldRefSeverity; + /** Diagnostic rule id. */ + rule: string; + /** Human-readable location, e.g. `object "proj_task" › highlightFields`. */ + where: string; + /** Config path, e.g. `objects[0].highlightFields[1]`. */ + path: string; + /** What is wrong. */ + message: string; + /** How to fix it. */ + hint: string; +} + +type AnyRec = Record; + +function isRec(v: unknown): v is AnyRec { + return !!v && typeof v === 'object' && !Array.isArray(v); +} + +/** Coerce a collection (array or name-keyed map) to an array of records. */ +function asArray(v: unknown): AnyRec[] { + if (Array.isArray(v)) return v as AnyRec[]; + if (v && typeof v === 'object') { + return Object.entries(v as AnyRec).map(([name, def]) => ({ name, ...(def as AnyRec) })); + } + return []; +} + +/** + * The object-level field-name LIST positions, as + * `[block, key]` where `''` is the object's own top level. + * + * Declarative for the same reason the list-view sibling's table is: the + * failure this family exists to end is a position nobody remembered to walk, + * and a table can be read against `ObjectSchema` key by key. `consequence` is + * the sentence the author reads after the miss — what actually happens when + * the reference resolves to nothing — and it differs per position, which is + * why it is data here rather than one shared string. + */ +interface ListPosition { + /** `''` = the object's own top level, else the nested block that holds `key`. */ + block: string; + /** The key whose value is the array of field names. */ + key: string; + /** + * Also read this retired spelling at the same position. See the + * `compactLayout` note on this module — coverage preservation on the raw + * `lint` path, never an accepted spelling. + */ + retiredAlias?: string; + /** What the platform does with an entry that resolves to nothing. */ + consequence: string; + /** The prescription half of the hint. */ + prescription: string; +} + +const LIST_POSITIONS: readonly ListPosition[] = [ + { + block: '', + key: 'highlightFields', + retiredAlias: 'compactLayout', + consequence: + 'Every consumer silently skips the entry: it drives the object\'s default list columns, ' + + 'record cards, previews and the detail highlight strip, and each of them renders one ' + + 'field short with no error anywhere.', + prescription: + 'Fix the field name, or drop the entry. `highlightFields` is ordered — the first entry ' + + 'wins where only one field fits (ADR-0085).', + }, + { + block: 'publicSharing', + key: 'redactFields', + consequence: + 'The redaction never applies, and it fails OPEN: records served through a share link ' + + 'still carry the field to whoever holds the link. A mis-spelled entry is silently ' + + 'indistinguishable from an entry that was never written.', + prescription: + 'Fix the field name so the redaction binds, or drop the entry if the field is meant to ' + + 'be served through share links.', + }, +]; + +/** + * Validate every object's own field-name lists against the object graph. + * Returns findings (empty = clean). Pure `(stack) => Finding[]`; no I/O, and + * safe on both the schema-parsed stack and the raw config the `lint` path + * carries. + */ +export function validateObjectFieldRefs(stack: AnyRec): ObjectFieldRefFinding[] { + const findings: ObjectFieldRefFinding[] = []; + if (!isRec(stack)) return findings; + + const graph: ObjectGraph = indexObjectGraph(stack); + if (graph.size === 0) return findings; + + const objects = asArray(stack.objects); + for (let oi = 0; oi < objects.length; oi++) { + const obj = objects[oi]; + if (!isRec(obj)) continue; + const objName = typeof obj.name === 'string' && obj.name.length > 0 ? obj.name : undefined; + if (!objName) continue; + + // ── Skips 1 & 2, once for the whole object ── + // An object with no entry, or a null entry (no readable field map), is + // `resolveFieldPath`'s `unknowable` — asking per entry would report the + // same non-answer once per list member. + if (!graph.has(objName) || !graph.get(objName)) continue; + + const label = `object "${objName}"`; + const objPath = `objects[${oi}]`; + + for (const position of LIST_POSITIONS) { + const host = position.block === '' ? obj : obj[position.block]; + if (!isRec(host)) continue; + const hostPath = position.block === '' ? objPath : `${objPath}.${position.block}`; + + // The canonical key, else the retired alias at the same position. + const written = Array.isArray(host[position.key]) + ? position.key + : (position.retiredAlias && Array.isArray(host[position.retiredAlias])) + ? position.retiredAlias + : undefined; + if (!written) continue; + const list = host[written] as unknown[]; + + list.forEach((entry, i) => { + if (typeof entry !== 'string' || entry.length === 0) return; + const verdict = resolveFieldPath(graph, objName, entry); + if (isUnjudgeable(verdict) || !verdict) return; + const subject = `${written}[${i}]`; + const account = describeFieldPathVerdict(verdict, entry, subject); + if (!account) return; // the name resolves — nothing to say + + findings.push({ + severity: 'error', + rule: OBJECT_FIELD_REF_UNKNOWN, + where: `${label} › ${written}`, + path: `${hostPath}.${written}[${i}]`, + message: `${account.message} ${position.consequence}`, + hint: `${position.prescription} ${account.detail}`, + }); + }); + } + } + + return findings; +} diff --git a/packages/lint/src/validate-semantic-roles.test.ts b/packages/lint/src/validate-semantic-roles.test.ts index 90782065ad..6bad75af87 100644 --- a/packages/lint/src/validate-semantic-roles.test.ts +++ b/packages/lint/src/validate-semantic-roles.test.ts @@ -202,22 +202,18 @@ describe('validateSemanticRoles (ADR-0085)', () => { expect(optedOut).toEqual([]); }); - it('flags unknown highlightFields entries, including via the compactLayout alias', () => { + // [#15254] `highlightFields` EXISTENCE — and the `compactLayout` alias read + // that went with it — moved to `validate-object-field-refs.ts`, where it + // gates instead of advising. The assertions that used to live here moved + // with it (`validate-object-field-refs.test.ts`); the boundary they now pin + // from THIS side is below, under "the existence clause that moved out". + it('stays silent on a dangling highlightFields entry — no longer this rule\'s (#15254)', () => { const findings = validateSemanticRoles(stack([{ name: 'account', highlightFields: ['name', 'industy'], // typo fields: { name: {}, industry: {} }, }])); - expect(findings).toHaveLength(1); - expect(findings[0].message).toContain('industy'); - - const aliased = validateSemanticRoles(stack([{ - name: 'account', - compactLayout: ['ghost'], - fields: { name: {} }, - }])); - expect(aliased).toHaveLength(1); - expect(aliased[0]).toMatchObject({ rule: SEMANTIC_ROLE_FIELD_UNKNOWN }); + expect(findings, JSON.stringify(findings)).toEqual([]); }); it('accepts objects as a name-keyed map and tolerates junk shapes', () => { @@ -275,41 +271,25 @@ describe('validateSemanticRoles — injected system columns (#5378)', () => { // ── Counter-examples ── - it.each(['none', 'org'])( - "still warns for highlightFields: [owner_id] on ownership: '%s'", - (ownership) => { - const findings = validateSemanticRoles(stack([{ - name: 'crm_tag', - ownership, - highlightFields: ['name', 'owner_id'], - fields: { name: {} }, - }])); - expect(findings).toHaveLength(1); - expect(findings[0]).toMatchObject({ rule: SEMANTIC_ROLE_FIELD_UNKNOWN }); - expect(findings[0].message).toContain('owner_id'); - }, - ); - - it('still warns for an organization_id highlight when the object opts out of tenancy', () => { + // [#15254] The three counter-examples that stood here — `ownership: 'none'` + // / `'org'` withholding `owner_id`, tenancy opt-out withholding + // `organization_id`, and a typo that merely LOOKS like a system column — + // pinned the DERIVATION at the `highlightFields` position, and that position + // is `object-field-ref-unknown`'s now. They moved verbatim in substance to + // `validate-object-field-refs.test.ts` (at `error`), rather than being + // dropped: the #5378 derivation is exactly as load-bearing there, and a + // counter-example deleted in a move is a counter-example nobody re-runs. + // `stageField` keeps its own copies below — this rule still owns that one. + it("still warns for a stageField the derivation withholds — ownership: 'none'", () => { const findings = validateSemanticRoles(stack([{ name: 'crm_tag', - tenancy: { enabled: false }, - highlightFields: ['organization_id'], + ownership: 'none', + stageField: 'owner_id', fields: { name: {} }, }])); expect(findings).toHaveLength(1); - expect(findings[0].message).toContain('organization_id'); - }); - - it('still warns for a typo that merely LOOKS like a system column', () => { - const findings = validateSemanticRoles(stack([{ - name: 'crm_contact', - highlightFields: ['owner_ids', 'creatd_at'], - fields: { name: {} }, - }])); - expect(findings).toHaveLength(2); - expect(findings.map((f) => f.message).join(' ')).toMatch(/owner_ids/); - expect(findings.map((f) => f.message).join(' ')).toMatch(/creatd_at/); + expect(findings[0]).toMatchObject({ rule: SEMANTIC_ROLE_FIELD_UNKNOWN }); + expect(findings[0].message).toContain('owner_id'); }); // Widening the EXISTENCE question must not make an injected column a group @@ -395,12 +375,26 @@ describe('validateSemanticRoles — unprovisioned anchors on external objects (# expect(findings).toEqual([]); }); - it('a withheld anchor still gets the UNKNOWN finding, never the provenance one', () => { - // `ownership: 'none'` ⇒ no owner_id anywhere ⇒ rule (c)'s existence warning - // owns the defect. One pointer, one finding, the right one. + it('a withheld anchor gets the EXISTENCE finding, never the provenance one', () => { + // `ownership: 'none'` ⇒ no owner_id anywhere ⇒ the existence verdict owns + // the defect, not this rule's provenance one. One pointer, one finding, + // the right one — and since #15254 the existence half is + // `object-field-ref-unknown`, so what this rule must do here is stay + // SILENT. The positive half is pinned in + // `validate-object-field-refs.test.ts`. const findings = validateSemanticRoles(stack([ externalObject({ ownership: 'none', highlightFields: ['owner_id'] }), ])); + expect(findings, JSON.stringify(findings)).toEqual([]); + }); + + it('the same withheld anchor on a stageField DOES still warn here', () => { + // The control that keeps the test above from being vacuous: this rule's + // silence is about the position that moved, not about the rule going + // quiet everywhere. + const findings = validateSemanticRoles(stack([ + externalObject({ ownership: 'none', stageField: 'owner_id' }), + ])); expect(findings).toHaveLength(1); expect(findings[0].rule).toBe(SEMANTIC_ROLE_FIELD_UNKNOWN); }); diff --git a/packages/lint/src/validate-semantic-roles.ts b/packages/lint/src/validate-semantic-roles.ts index e3fed006b6..bb1385942b 100644 --- a/packages/lint/src/validate-semantic-roles.ts +++ b/packages/lint/src/validate-semantic-roles.ts @@ -10,12 +10,22 @@ * inert" shape ADR-0078 prohibits — so the completeness lint flags it here, * uniformly for `os build`/`os validate`, MCP authoring and hand authors. * - * All three rules are warnings, not errors: every consumer degrades + * Every rule here is a warning, not an error: every consumer degrades * gracefully (an unknown `Field.group` renders in the ungrouped bucket, an - * unknown highlight name is skipped, an unknown `stageField` falls back to - * heuristics), so nothing is fully broken — but the author almost certainly - * typo'd a name and should be told at author time, not discover it by - * staring at an unchanged page. + * unknown `stageField` falls back to heuristics), so nothing is fully broken + * — but the author almost certainly typo'd a name and should be told at + * author time, not discover it by staring at an unchanged page. + * + * [#15254] ⛔ ONE position left, and the boundary is worth stating because + * the key is still read below. `highlightFields` EXISTENCE — "the entry names + * no field on this object" — is `validate-object-field-refs.ts`' judgement + * (`object-field-ref-unknown`, `error`, on the runtime publish door as well + * as the three commands), because the measured state was that a warning on + * the CLI alone let the Studio click path publish a dangling reference green. + * This module keeps the PROVENANCE question at that position + * (`semantic-role-field-unprovisioned`): the entry resolves, and the column + * behind it is an injected anchor with no storage. Existence and provenance + * are different questions, and only one of them moved. */ import { injectedColumnsFor, unprovisionedInjectedColumnsFor } from './system-fields.js'; @@ -184,24 +194,17 @@ export function validateSemanticRoles(stack: AnyRec): SemanticRoleFinding[] { : Array.isArray(obj.compactLayout) // deprecated alias (pre-normalization input) ? obj.compactLayout : []; + // [#15254] The EXISTENCE half of this position moved to + // `validate-object-field-refs.ts` (`object-field-ref-unknown`, `error`, + // and on the runtime publish door as well as the CLI). It is not + // restated here: two findings on one path is noise, and the two tiers + // would disagree about the same fact. What stays is the PROVENANCE + // question, which is a different one — the entry resolves, and the + // column behind it is an unprovisioned injected anchor (#8116). for (const entry of highlights) { if (typeof entry !== 'string' || entry.length === 0) continue; - if (fieldNames.has(entry)) { - if (unprovisioned.has(entry)) findings.push(unprovisionedPointer('highlightFields', entry)); - continue; - } - findings.push({ - severity: 'warning', - rule: SEMANTIC_ROLE_FIELD_UNKNOWN, - where, - path: `${path}.highlightFields`, - message: - `${objName}: highlightFields entry "${entry}" is not a field on this ` + - `object — it is silently skipped by every consumer`, - hint: - `Fix the field name (highlightFields drives default columns, cards, ` + - `previews and the detail highlight strip, in order).`, - }); + if (!fieldNames.has(entry)) continue; + if (unprovisioned.has(entry)) findings.push(unprovisionedPointer('highlightFields', entry)); } // ── (d) declared group fully shadowed by the detail highlight strip ── diff --git a/packages/metadata-protocol/src/build-probes.ts b/packages/metadata-protocol/src/build-probes.ts index 8fddaedc55..015f56c22a 100644 --- a/packages/metadata-protocol/src/build-probes.ts +++ b/packages/metadata-protocol/src/build-probes.ts @@ -34,7 +34,16 @@ export interface BuildProbeReport { /** Findings, empty when every probe passed. */ issues: RuntimeBuildIssue[]; /** How many probes actually ran, per plane (0s mean nothing to probe). */ - checked: { seeds: number; views: number; widgets: number }; + /** + * [#15254] `objects` joined the planes because its ABSENCE was the tell. + * A publish of a package Studio's app builder authored reported + * `{seeds: 0, views: 0, widgets: 0}` — three zeroes, no `objects` key — + * and that receipt was accurate: the builder mints no `view` items, so + * every plane the report carried had genuinely nothing to inspect while + * the objects it DID publish were probed by nothing. A count that cannot + * go up is indistinguishable from a plane that found nothing wrong. + */ + checked: { seeds: number; views: number; widgets: number; objects: number }; } /** The single read the probes need from the data engine. */ @@ -108,7 +117,10 @@ async function hasRows( * renderer uses must not throw (`view_read_failed`); * • per published `dashboard` widget — its real dataset selection must * execute (`widget_query_failed`) and must not return empty on an object - * that HAS rows (`empty_query` — the four-layer staging incident class). + * that HAS rows (`empty_query` — the four-layer staging incident class); + * • per published `object` — the field-name lists it writes about its own + * fields must resolve (`object_field_ref_unknown`, #15254). The one plane + * that judges rather than reads; see its own note below. * * All probes are reads (limit-1 / single aggregate); a probe crash is * reported, never thrown — verification must not break the publish it @@ -116,7 +128,7 @@ async function hasRows( */ export async function runBuildProbes(opts: RunBuildProbesOptions): Promise { const issues: RuntimeBuildIssue[] = []; - const checked = { seeds: 0, views: 0, widgets: 0 }; + const checked = { seeds: 0, views: 0, widgets: 0, objects: 0 }; const { engine, getItem, published, analytics, organizationId } = opts; // Memoized active-item reads (a dashboard and its widgets share datasets). @@ -195,6 +207,66 @@ export async function runBuildProbes(opts: RunBuildProbesOptions): Promise x.type === 'object'); + if (publishedObjects.length > 0) { + let validateObjectFieldRefs: + | ((stack: Record) => Array<{ path: string; message: string; hint: string }>) + | undefined; + try { + ({ validateObjectFieldRefs } = await import('@objectstack/lint')); + } catch { + validateObjectFieldRefs = undefined; + } + for (const p of publishedObjects) { + if (!validateObjectFieldRefs) break; + const body = asRec(await readItem('object', p.name)); + if (!body) continue; + checked.objects += 1; + // One object is the whole universe the question has: every name in + // an object-level field-name list resolves against that object's + // OWN field map, never a sibling's. See the rule's module note. + let findings: Array<{ path: string; message: string; hint: string }> = []; + try { + findings = validateObjectFieldRefs({ objects: [{ ...body, name: p.name }] }) ?? []; + } catch { + findings = []; + } + for (const f of findings) { + issues.push({ + layer: 'runtime', + severity: 'error', + artifact: { type: 'object', name: p.name }, + code: 'object_field_ref_unknown', + message: `Object "${p.name}" names a field that does not exist (${f.path}): ${f.message}`, + fix: f.hint, + }); + } + } + } + // ── Dashboard widgets: the real dataset selection must return data ────── const dashboards = published.filter((x) => x.type === 'dashboard'); let widgetsToProbe = 0; diff --git a/packages/metadata-protocol/src/protocol-publish-drafts-object-field-refs.test.ts b/packages/metadata-protocol/src/protocol-publish-drafts-object-field-refs.test.ts new file mode 100644 index 0000000000..e0a197b941 --- /dev/null +++ b/packages/metadata-protocol/src/protocol-publish-drafts-object-field-refs.test.ts @@ -0,0 +1,296 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #15254 — the batch publish door REFUSES an object whose `highlightFields` + * names a field that does not exist on it. + * + * ## The claim this pins, and the state it replaces + * + * The card is a finding from the promo-video lane, and the film's one claim is + * that "a change made by clicking in Studio is refused by the same gate that + * refuses code". On the pinned stack nothing Studio could produce was refused, + * and the reproduction is the natural click order rather than a contrived one: + * click-create a Number field (Studio mints it as `field_10`), add it to the + * object's `highlightFields`, then give it a label — the API name auto-derives + * to `health_score` and `highlightFields` still says `field_10`. Publishing + * that draft answered HTTP 200, `outcome: "published"`, `failedCount: 0`, and + * the app then silently ignored the dangling reference at runtime. + * + * Two independent reasons the gate stayed quiet, both measured on `origin/main` + * before this change: + * + * 1. `list-view-field-unknown` inspects `view.columns`, and Studio's app + * builder mints no `view` items at all — the rule had nothing to inspect + * on the only artifacts the click path authors. + * 2. `runtimeAuthoringRulesFor('object')` dispatched seven rules and NO + * reference-integrity rule among them (the suite entry declared + * `runtimeTypes: ['flow', 'view']`), while the object-level existence + * check that did exist — `semantic-role-field-unknown` — is `warning`, + * advisory-tier and CLI-only. So the one door a tenant has ran no + * reference-integrity judgement on an object write, and the one command + * that spoke exited 0. + * + * ⭐ Warning-level is not enough for the claim; it has to REFUSE. That is what + * this file measures, end to end, through the real `publishPackageDrafts`. + * + * Harness: the same faithful stub engine as + * `protocol-publish-drafts-advisories.test.ts` / `-org-scope.test.ts`, kept + * local — self-contained harnesses are the established shape here, so two + * tripwires can fail independently. Objects are saved as package-bound drafts + * (draft saves are never gated, #4463 D1) and published through the REAL + * `publishPackageDrafts`; nothing on the gate path is stubbed. + */ + +import { describe, expect, it } from 'vitest'; +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, assertEngineFindOnePredicate } from '@objectstack/metadata-core'; +import { ObjectStackProtocolImplementation } from './protocol.js'; + +interface Row { + id: string; + type: string; + name: string; + organization_id: string | null; + package_id: string | null; + state: string; + metadata: string; + checksum?: string; + version?: number; + updated_at?: string; + created_at?: string; +} + +interface HistoryRow { + id: string; + event_seq: number; + name: string; + type: string; + version: number; + operation_type: string; + metadata: string | null; + checksum: string | null; + previous_checksum: string | null; + change_note?: string | null; + source?: string | null; + organization_id: string | null; + recorded_by?: string | null; + recorded_at: string; +} + +// Overlay rows are keyed by (type, name, org, state, package_id) — the ADR-0048 key. +function keyOf(w: Record) { + return `${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}|${w.package_id ?? '__nopkg__'}`; +} + +function matchesMetadataWhere(r: Row, where: Record): boolean { + for (const [k, v] of Object.entries(where)) { + if (k === '$or') { + const clauses = v as Array>; + if (!clauses.some((c) => matchesMetadataWhere(r, c))) return false; + continue; + } + if (v === undefined) continue; + if ((r as any)[k] !== v) return false; + } + return true; +} + +function makeStubEngine() { + const rows = new Map(); + const historyRows: HistoryRow[] = []; + let nextId = 0; + + const findRow = (w: Record): { key: string; row: Row } | null => { + if (w.id !== undefined) { + for (const [k, r] of rows) if (r.id === w.id) return { key: k, row: r }; + return null; + } + if (w.package_id !== undefined) { + const k = keyOf(w); + const r = rows.get(k); + return r ? { key: k, row: r } : null; + } + for (const [k, r] of rows) if (matchesMetadataWhere(r, w)) return { key: k, row: r }; + return null; + }; + + const matchesHistory = (h: HistoryRow, w: Record): boolean => { + if (w.organization_id !== undefined && h.organization_id !== w.organization_id) return false; + if (w.type !== undefined && h.type !== w.type) return false; + if (w.name !== undefined && h.name !== w.name) return false; + if (w.version !== undefined && h.version !== w.version) return false; + if (w.operation_type !== undefined && h.operation_type !== w.operation_type) return false; + return true; + }; + + const engine: any = { + async findOne(table: string, opts: { where: Record }) { + assertEngineFindOnePredicate(table, opts); + if (table === 'sys_metadata_history') { + return historyRows.find((h) => matchesHistory(h, opts.where)) ?? null; + } + return findRow(opts.where)?.row ?? null; + }, + async find(table: string, opts: { where: Record; limit?: number }) { + const matched = table === 'sys_metadata_history' + ? historyRows.filter((h) => matchesHistory(h, opts.where)) + : Array.from(rows.values()).filter((r) => matchesMetadataWhere(r, opts.where)); + // The caller's bound, applied AFTER the filter and by PRESENCE — a + // double that silently ignores `limit` answers more rows than the + // real engine would, and every assertion downstream of it is then + // measuring a shape production never produces. + return typeof opts?.limit === 'number' ? matched.slice(0, opts.limit) : matched; + }, + async insert(table: string, data: Record) { + if (table === 'sys_metadata_audit') return { id: 'audit_skip' }; + if (table === 'sys_metadata_history') { + nextId += 1; + const h: HistoryRow = { id: `h_${nextId}`, ...(data as any) }; + historyRows.push(h); + return { id: h.id }; + } + nextId += 1; + const row = { id: `r_${nextId}`, ...(data as any) } as Row; + rows.set(keyOf(data), row); + return { id: row.id }; + }, + async update(_t: string, data: Record, opts: { where: Record }) { + assertEngineUpdateDispatch(data, opts); + const found = findRow(opts.where); + if (!found) return { id: null }; + const merged = { ...found.row, ...(data as any) }; + rows.delete(found.key); + rows.set(keyOf(merged), merged); + return { id: found.row.id }; + }, + async delete(_t: string, opts: { where: Record }) { + assertEngineDeleteDispatch(opts); + const found = findRow(opts.where); + if (!found) return { deleted: 0 }; + rows.delete(found.key); + return { deleted: 1 }; + }, + async transaction(cb: (ctx: any, info: { owned: boolean }) => Promise): Promise { + return cb(undefined, { owned: true }); + }, + registry: { + registerItem: () => {}, + registerObject: () => {}, + // No declared package namespace → publishPackageDrafts skips the + // ADR-0028 prefix check (legacy-grandfathered path). + getPackage: () => undefined, + }, + }; + return { engine, rows, historyRows }; +} + +const PKG = 'app.studio'; + +/** + * The card's click path, at the moment the author hits Publish. + * + * 1. click-create a Number field → Studio mints it as `field_10` + * 2. add it to `highlightFields` → the list references `field_10` + * 3. set its label to "Health Score" → the API name auto-derives to + * `health_score`; `highlightFields` is NOT rewritten + * + * `sharingModel` is load-bearing for the same reason `runAs: 'system'` is in + * the sibling harness: without it `security-owd-unset` fires at `error` and + * the refusal under test would be someone else's. + */ +const danglingHighlightObject = (name: string) => ({ + name, + label: 'Task', + sharingModel: 'private', + fields: { + name: { type: 'text', label: 'Name' }, + health_score: { type: 'number', label: 'Health Score' }, + }, + nameField: 'name', + highlightFields: ['name', 'field_10'], +}); + +/** The same object with step 2's reference rewritten to the derived name. */ +const repairedObject = (name: string) => ({ + ...danglingHighlightObject(name), + highlightFields: ['name', 'health_score'], +}); + +/** Stage one package-bound object draft (Studio's "Save Draft" shape). */ +async function stageObjectDraft( + protocol: ObjectStackProtocolImplementation, + name: string, + item: unknown, +): Promise { + await (protocol as any).saveMetaItem({ + type: 'object', name, item, packageId: PKG, mode: 'draft', + }); +} + +describe('publishPackageDrafts refuses a dangling object field-name list (#15254)', () => { + it('REFUSES the click path: outcome is not `published`, and the receipt names the rule and the path', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + await stageObjectDraft(protocol, 'proj_task', danglingHighlightObject('proj_task')); + + const res = await protocol.publishPackageDrafts({ packageId: PKG }); + + // The bar, in the card's own terms: the publish FAILS. + expect(res.outcome).not.toBe('published'); + expect(res).toMatchObject({ success: false, publishedCount: 0, failedCount: 1 }); + expect(res.published).toEqual([]); + + const causal = res.failed.find((f) => f.name === 'proj_task')!; + expect(causal, JSON.stringify(res.failed)).toBeDefined(); + expect(causal.code).toBe('INVALID_METADATA'); + + // …with a rule id and the offending path, which is what has to reach + // the author's screen. The path is name-keyed on the wire (#10064) — + // `objects..[i]`, never the gate's private snapshot index. + const wire = JSON.stringify(causal); + expect(wire).toContain('object-field-ref-unknown'); + expect(wire).toContain('objects.proj_task.highlightFields[1]'); + // The author reads back the string they typed. + expect(wire).toContain('field_10'); + }); + + it('the SAME draft publishes once the reference is repaired — the refusal is about the reference, not the object', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + await stageObjectDraft(protocol, 'proj_task', repairedObject('proj_task')); + + const res = await protocol.publishPackageDrafts({ packageId: PKG }); + + expect(res.outcome).toBe('published'); + expect(res).toMatchObject({ success: true, publishedCount: 1, failedCount: 0 }); + expect(res.published.map((p) => p.name)).toEqual(['proj_task']); + }); + + it('reports the objects it inspected — `probes.checked.objects` is no longer a count that cannot go up', async () => { + // The absence of this key is what let the card's filer diagnose the + // gap: a receipt reading `{seeds: 0, views: 0, widgets: 0}` was + // ACCURATE (the builder mints none of those) while the objects the + // package did publish were probed by nothing at all. + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + await stageObjectDraft(protocol, 'proj_task', repairedObject('proj_task')); + + const res = await protocol.publishPackageDrafts({ packageId: PKG }); + const probes = res.probes as { checked: Record; issues: unknown[] } | undefined; + + expect(probes, 'probes rode the response').toBeDefined(); + expect(probes!.checked).toHaveProperty('objects'); + expect(probes!.checked.objects).toBe(1); + // A clean object raises nothing on the plane. + expect(probes!.issues).toEqual([]); + }); + + it('a draft save is NEVER gated (#4463 D1) — the refusal belongs to the publish', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + // No throw: the author keeps working on a half-finished object, and is + // stopped at the moment they claim it is ready. + await expect( + stageObjectDraft(protocol, 'proj_task', danglingHighlightObject('proj_task')), + ).resolves.toBeUndefined(); + }); +}); diff --git a/packages/objectql/src/build-probes.test.ts b/packages/objectql/src/build-probes.test.ts index 62f4c9fc58..059290cd96 100644 --- a/packages/objectql/src/build-probes.test.ts +++ b/packages/objectql/src/build-probes.test.ts @@ -167,7 +167,11 @@ describe('runBuildProbes — dashboard widgets', () => { ], }); // getItem failures mean no object bindings resolve — nothing probed, nothing thrown. - expect(report.checked).toEqual({ seeds: 0, views: 0, widgets: 0 }); + // [#15254] `objects: 0` for the same reason as the other three here: + // `getItem` threw, so no published object body was readable and none was + // judged. The plane is present and counted nothing, which is exactly the + // distinction the counter exists to make. + expect(report.checked).toEqual({ seeds: 0, views: 0, widgets: 0, objects: 0 }); }); }); diff --git a/packages/runtime/src/domains/packages-publish-drafts-response-conformance.test.ts b/packages/runtime/src/domains/packages-publish-drafts-response-conformance.test.ts index 284aff7e16..39aca7c79c 100644 --- a/packages/runtime/src/domains/packages-publish-drafts-response-conformance.test.ts +++ b/packages/runtime/src/domains/packages-publish-drafts-response-conformance.test.ts @@ -77,7 +77,7 @@ function makeDoor(opts: { // without it is a drifted seam, not a minimal one. success: true, outcome: 'published', publishedCount: 1, failedCount: 0, published: [{ type: 'flow', name: 'nightly_rollup', version: 'sha256:aa11' }], failed: [], - probes: { issues: [], checked: { seeds: 0, views: 0, widgets: 0 } }, + probes: { issues: [], checked: { seeds: 0, views: 0, widgets: 0, objects: 0 } }, commitId: 'cmt_01', }; const publishPackageDrafts = vi.fn().mockImplementation(async () => @@ -157,7 +157,7 @@ describe('publish-drafts wire payload conforms to PublishPackageDraftsResponseSc // The route-attached ADR-0045 receipt is ON the wire and declared. expect(parsed.unhiddenApps).toEqual(['crm', 'ops']); // Opaque probes crossed the route untouched. - expect(parsed.probes).toEqual({ issues: [], checked: { seeds: 0, views: 0, widgets: 0 } }); + expect(parsed.probes).toEqual({ issues: [], checked: { seeds: 0, views: 0, widgets: 0, objects: 0 } }); expect(parsed.commitId).toBe('cmt_01'); }); diff --git a/packages/spec/src/api/protocol.test.ts b/packages/spec/src/api/protocol.test.ts index 0e9fccede8..179b7c2e64 100644 --- a/packages/spec/src/api/protocol.test.ts +++ b/packages/spec/src/api/protocol.test.ts @@ -1310,7 +1310,7 @@ describe('PublishPackageDraftsResponseSchema (#9406 — declares the batch publi it('probes is opaque BY DECLARATION (#9406 ruling): any shape passes through unmodified, unstripped', () => { // The real `BuildProbeReport` shape of today… - const report = { issues: [], checked: { seeds: 1, views: 2, widgets: 0 } }; + const report = { issues: [], checked: { seeds: 1, views: 2, widgets: 0, objects: 3 } }; const parsed = PublishPackageDraftsResponseSchema.parse({ ...realResponse, probes: report }); expect(parsed.probes).toEqual(report); // …and a future shape this contract deliberately does NOT constrain. If a diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index ba0ef11772..f3b8727f12 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -161,6 +161,21 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/metadata-protocol/src/protocol-publish-drafts-object-field-refs.test.ts", + "verb": "delete", + "pinned": 1 + }, + { + "file": "packages/metadata-protocol/src/protocol-publish-drafts-object-field-refs.test.ts", + "verb": "findOne", + "pinned": 1 + }, + { + "file": "packages/metadata-protocol/src/protocol-publish-drafts-object-field-refs.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/metadata-protocol/src/protocol-publish-drafts-org-scope.test.ts", "verb": "delete",